fix(underglow): avoid saving automatic state changes - #3463
Conversation
Use non-persistent internal on/off helpers for idle and USB auto-off transitions. User-triggered public APIs continue to save the underglow state.
There was a problem hiding this comment.
🟡 Not ready to approve
The auto-off state machine still appears to mis-handle the case where both idle and USB auto-off are enabled, allowing one “wake” condition to restore underglow while the other condition still requires it to stay off.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR prevents automatic underglow state transitions (idle/USB auto-off) from being persisted to settings storage, while keeping user-invoked on/off actions persistent.
Changes:
- Split underglow on/off into internal non-persistent “immediate” helpers and public persistent APIs.
- Updated the idle/USB auto-state handler to use the non-persistent helpers to avoid saving automatic transitions.
File summaries
| File | Description |
|---|---|
| app/src/rgb_underglow.c | Adds non-persistent on/off helpers and routes auto idle/USB transitions through them to avoid saving automatic state changes. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| if (sleep_state.is_awake) { | ||
| if (sleep_state.rgb_state_before_sleeping) { | ||
| return zmk_rgb_underglow_on(); | ||
| return zmk_rgb_underglow_on_immediate(); | ||
| } else { | ||
| return zmk_rgb_underglow_off(); | ||
| return zmk_rgb_underglow_off_immediate(); | ||
| } | ||
| } else { | ||
| sleep_state.rgb_state_before_sleeping = state.on; | ||
| return zmk_rgb_underglow_off(); | ||
| return zmk_rgb_underglow_off_immediate(); | ||
| } |
There was a problem hiding this comment.
Thanks for pointing this out. The interaction between the two auto-off conditions is valid, but it is a pre-existing issue and is outside the scope of this PR. The shared sleep_state logic and the mapping of idle/USB events to rgb_underglow_auto_state() are unchanged here; this PR only replaces the persistent public on/off calls with non-persistent internal helpers.
Correctly combining both options would require tracking the idle and USB inhibit conditions independently, which would be better handled in a separate issue/PR. With either option enabled on its own, this change preserves the existing auto-state behavior while preventing automatic transitions from being written to settings.
Summary
Root cause
The automatic state handler called the public
zmk_rgb_underglow_on()andzmk_rgb_underglow_off()APIs. Both APIs schedule the complete underglowstate for storage, so an automatic idle transition persisted
on=falseasif the user had explicitly disabled underglow.
This supersedes #1667 and follows the auto-state work in #2244 and #2284.
Validation
hw75_dynamic@Aandhw75_keyboard_f303cc@1.1with the change.helper with underglow on and activity idle, without subsequently entering
the underglow settings save work.
rgb/underglow/staterecord remainedon=true.PR check-list