feat(power): configurable power button behavior + rename Power Mode → Power Options (OS-463)#2671
feat(power): configurable power button behavior + rename Power Mode → Power Options (OS-463)#2671elibosley wants to merge 4 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 44 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughAdds configurable physical power button behavior: a new ACPI handler script reads a setting from ChangesPhysical Power Button Behavior
Power Options Menu Reorganization
Sequence DiagramsequenceDiagram
participant Browser
participant update.php
participant dynamix.cfg
participant acpid
participant unraid_power_handler.sh
participant init
Browser->>update.php: POST powerbutton=ignore|shutdown
update.php->>dynamix.cfg: write powerbutton setting
Note over acpid: button/power ACPI event fires
acpid->>unraid_power_handler.sh: dispatch event
unraid_power_handler.sh->>dynamix.cfg: read powerbutton
alt powerbutton == ignore
unraid_power_handler.sh->>unraid_power_handler.sh: log press ignored
else shutdown
unraid_power_handler.sh->>init: /sbin/init 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@emhttp/plugins/dynamix/PowerModeCpu.page`:
- Around line 38-40: The jQuery selector `$(this).next('span')` in the disabled
radio button check loop is not finding the correct elements because radio inputs
are not directly followed by span siblings, but rather by text nodes or
differently structured elements. Inspect the actual HTML structure around the
radio inputs to determine what element actually follows them, then update the
traversal method in the loop (replacing `.next('span')`) to correctly locate the
element that should receive the unavailable label text. This may require using a
different jQuery method like .closest(), .parent(), or .find() depending on the
actual DOM hierarchy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 14608339-fb32-4abb-a61e-ba54b9d0fe05
📒 Files selected for processing (6)
emhttp/languages/en_US/helptext.txtemhttp/plugins/dynamix/PowerButton.pageemhttp/plugins/dynamix/PowerMode.pageemhttp/plugins/dynamix/PowerModeCpu.pageetc/acpi/unraid_power_handler.shetc/rc.d/rc.acpid
| $('input[type=radio]').each(function(){ | ||
| if ($(this).prop('disabled')) $(this).next('span').html(" <i>(_(unavailable)_)</i>"); | ||
| }); |
There was a problem hiding this comment.
Unavailable-label injection currently never matches the DOM.
At Line 39, $(this).next('span') does not find anything because the radio inputs are followed by text nodes, not <span> siblings. As written, disabled options won’t get the “unavailable” note.
Suggested fix
$(function(){
$('input[type=radio]').each(function(){
- if ($(this).prop('disabled')) $(this).next('span').html(" <i>(_(unavailable)_)</i>");
+ if ($(this).prop('disabled')) {
+ $(this).after(" <i>(_(unavailable)_)</i>");
+ }
});
<?if (exec("dmesg | grep -Pom1 'Hypervisor detected'")):?>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $('input[type=radio]').each(function(){ | |
| if ($(this).prop('disabled')) $(this).next('span').html(" <i>(_(unavailable)_)</i>"); | |
| }); | |
| $(function(){ | |
| $('input[type=radio]').each(function(){ | |
| if ($(this).prop('disabled')) { | |
| $(this).after(" <i>(_(unavailable)_)</i>"); | |
| } | |
| }); | |
| <?if (exec("dmesg | grep -Pom1 'Hypervisor detected'")):?> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@emhttp/plugins/dynamix/PowerModeCpu.page` around lines 38 - 40, The jQuery
selector `$(this).next('span')` in the disabled radio button check loop is not
finding the correct elements because radio inputs are not directly followed by
span siblings, but rather by text nodes or differently structured elements.
Inspect the actual HTML structure around the radio inputs to determine what
element actually follows them, then update the traversal method in the loop
(replacing `.next('span')`) to correctly locate the element that should receive
the unavailable label text. This may require using a different jQuery method
like .closest(), .parent(), or .find() depending on the actual DOM hierarchy.
|
Update — hardware testing on devgen.local revealed a second handler. The power button generates two events: an ACPI event (acpid → Added Note for reviewers: the new |
|
Update — added more actions + a plugin extension point (commit Power Button options are now: Shut down (default), Reboot, Sleep (suspend) (shown only when the kernel reports suspend-to-RAM support), and Do nothing. Rather than baking plugin-specific actions into core, the power button is now extensible so plugins add their own actions in their own repos:
This is the mechanism the User Scripts plugin would use to add a "Run script" action — intentionally left to that plugin rather than coupled into core. Verified on devgen.local: all four handler branches (shutdown/reboot/sleep/ignore) dispatch correctly, the |
|
Update — Sleep is now plugin-provided, not a core option (commit Dropped the built-in Sleep option and its generic Core now offers only the trivially-safe primitives: Shut down, Reboot, Do nothing. Sleep is added by the S3 Sleep plugin via the same extension hook as any other plugin action ( |
Rename the "Power Mode" settings page to "Power Options" and turn it into a tabbed page with two tabs: - Power Mode - the existing CPU governor controls (moved verbatim) - Power Button - new setting to choose what the physical power button does The power button behavior (Shut down / Do nothing) is stored in the [powermode] section of dynamix.cfg and read live by the ACPI handler on each press, so changes take effect without a reload or reboot. A setting-aware ACPI handler (etc/acpi/unraid_power_handler.sh) is installed over the stock Slackware acpi_handler.sh at boot by rc.acpid, so it reliably overrides the default poweroff regardless of package install order. This mirrors how rc.cpufreq applies the [powermode] governor setting at boot. Closes OS-463 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A physical power press fires both an ACPI event (handled by acpid) and an input key event handled by elogind, whose default HandlePowerKey=poweroff shuts the system down regardless of the acpid handler. Ship a logind.conf.d drop-in setting HandlePowerKey/HandlePowerKeyLongPress to ignore so acpid is the sole authority over the power button, making the "Do nothing" Power Options setting actually take effect. Verified on hardware: elogind D-Bus HandlePowerKey now reports "ignore". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Power Button now offers Shut down, Reboot, Sleep (suspend, shown only when the kernel supports suspend-to-RAM), and Do nothing. Plugins can add or override actions without touching core: - WebUI: PowerButton.page includes plugins/*/include/powerbutton-option.php (and powerbutton-extra.php for action-specific fields). - Action: unraid_power_handler.sh dispatches to /etc/acpi/powerbutton.d/<action> for non-builtin actions, and the builtin "sleep" defers to powerbutton.d/sleep when present (so the S3 Sleep plugin can do array-aware prep). Contract documented in /etc/acpi/powerbutton.d/README. This lets the User Scripts plugin add a "Run script" action in its own repo rather than coupling it into core. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the built-in Sleep option and its generic echo-mem suspend. A core suspend bypasses the array/Docker/VM/wake prep the S3 Sleep plugin performs and is hardware-fragile, and Unraid intentionally ships S3 sleep as a plugin. Core now offers only the trivially-safe primitives (Shut down, Reboot, Do nothing). Sleep is added by the S3 Sleep plugin via the same extension hook used for other plugin actions: it ships /etc/acpi/powerbutton.d/sleep and a powerbutton-option.php fragment. The handler already routes powerbutton="sleep" through the plugin dir. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
elibosley
force-pushed
the
feature/os-463-power-options
branch
from
b232893 to
1e7413b
Compare
…rop shared-file edits - Save mode is now much faster: pipe the single-pass streaming zip (scripts/flash_backup) into the destination file instead of appending each top-level item to a growing archive (one zip invocation vs many). - On completion the save job raises an Unraid notification (the tray bell) with a click-to-download link, exposes the saved file for download via a docroot symlink, and the GUI shows a Download button next to the saved path. - Revert the helptext.txt and .gitignore edits to base so this PR touches only its own files. helptext.txt was the single file shared with other in-flight PR test plugins (#2671, #2673), which made the stacked PR-plugin patch fail to apply; with it gone, #2677 stacks cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
Implements OS-463: a native WebUI setting to control what the physical power button does, so an accidental bump on small-form-factor hardware (NUCs, etc.) no longer forces a shutdown.
The existing Power Mode settings page is renamed to Power Options and split into two tabs:
Power Button actions
Core ships only the trivially-safe primitives:
/sbin/init 0) — preserves today's behavior/sbin/init 6)The choice is saved to
[powermode] powerbutton=indynamix.cfg(on the flash, so it persists) and read live on each press, so changes take effect with no reload or reboot. Unset/unexpected values fall back to Shut down.Two handlers, both neutralized
Hardware testing surfaced that a single power press fires two independent events:
/etc/acpi/acpi_handler.sh. We install our setting-aware handler over the stock Slackware one at boot viarc.acpid(acpid_configure), so it wins regardless of package install order — mirroring howrc.cpufreqapplies the[powermode]governor at boot.Power Buttoninput device (PNP0C0C); its defaultHandlePowerKey=poweroffshut the box down regardless of acpid. A shippedlogind.conf.ddrop-in setsHandlePowerKey=ignore, making acpid the sole authority.Both were verified on real hardware (elogind D-Bus
HandlePowerKeynow reportsignore).Plugin extension hook
Actions that need their own setup or are hardware-fragile (suspend, running a script) are not baked into core. Instead the power button is extensible so plugins add actions in their own repos:
PowerButton.pageincludesplugins/*/include/powerbutton-option.php(andpowerbutton-extra.phpfor action-specific fields).unraid_power_handler.shdispatches any non-builtin action to/etc/acpi/powerbutton.d/<action>.etc/acpi/powerbutton.d/README.Follow-up work to wire the first two consumers (tracked as sub-tasks of OS-463):
Files
emhttp/plugins/dynamix/PowerMode.page/Settings/PowerModeURLs still work)emhttp/plugins/dynamix/PowerModeCpu.pageemhttp/plugins/dynamix/PowerButton.pageetc/acpi/unraid_power_handler.shetc/acpi/powerbutton.d/READMEetc/rc.d/rc.acpidpowerbutton.d/at bootetc/elogind/logind.conf.d/20-unraid-powerbutton.confemhttp/languages/en_US/helptext.txtTesting
bash -nclean on the handler andrc.acpid;php -lclean on both page bodies.Notes for reviewers
etc/*files (acpi, elogind, rc.acpid) reach a system via the package build, not thedeploy_to_unraid.shdev script (which only syncsemhttp/).