Conversation
Size Report
|
|
There is One Shot Sticky Modifier in main branch, what's the difference? |
OSM(LAlt) releases the modifier after one keypress — press OSM, release, press Tab, Alt+Tab fires once and Alt is gone. SM(Tab, LAlt) bundles the modifier and key together and keeps the modifier held across repeated presses of the same SM key: first press sends Alt+Tab, second press sends Alt+Tab again (Alt still held), and Alt only releases when you press something else entirely. OSM is for one-shot use; SM is specifically for cycling (Alt+Tab, Ctrl+Tab) where you need the modifier to persist across multiple presses of the same key. |
Great, maybe the two types be merged into a single type of behavior, i.e. a general "Sticky Key"? I'm imaging some like
With this, one-shot mod can be represented as What do you think? |
I like that idea, since the two are conceptually similar. Let me think through what implementing this in the one-shot would need (and what I'd want from it). Max repeat, on the other hand, isn't something I'd personally use, though I see the utility. Right now I'm thinking of the browser tabs I have open. As long as we can set "no max repeat" or "infinite" as an option, that works for me. I'd also like a timeout that's independent of the one-shot timeout. This probably isn't strictly necessary, but I suspect most people who use this would want a different timeout than the one for one-shot keys. I usually exit a sticky mod with my layer button, but when cycling through browser tabs I'll sometimes go through several, pause to look at the screen, then continue. Again, a personal preference I could make work with a shared timeout, but worth mentioning. I also wonder how this interacts with layer changes. I use this key on another layer (via MO) so that returning to the base layer automatically exits the sticky mod and I can resume typing immediately. So I'd want an optional per-key feature to exit on layer change. I wouldn't want this on my other one-shot keys, though, since I use those across layers constantly; I added it specifically to this sticky mod implementation. Forgive the verbosity; talking through it helped. I'm happy to fold this into the current one-shot keys implementation, but if you want to go that route, I would prefer a per-key timeout option and a per-key "exit on layer change" option. Adding max retries is a great idea. I don't know what you'd want as the default, but I'd like to have the max retries configurable to have infinite, or to assume infinite until the timeout from the last sticky mod key press. What do you think? |
Yes, I agree. Omitting it means "infinite", i.e. About the per-key timeout, I have no strong opinion on it. Using the shared timeout as the default and overriding it when a per-key timeout exists is fine with me. But it does increase the complexity and RAM/Flash usage.
Should it be included in the keep list? Also, the length of keep list is a bit tricky. I think it should be calculated at compile-time and applied to the type. TBH I don't know if the idea works, but I think it's worth a try at least. |
|
Just a quick update, I think I'll be done with this by the end of the week or earlier for you to look at. I may also wait until #854 is done as well and make sure that I have it working with that merge. |
|
Great! Only a minor comment left in #854, I think we can get it merged first. |
|
#854 is merged |
…ion, and release triggers
- Restore pre-existing comments in LayerOff, LayerToggle, DefaultLayer arms
- Fix typo in LayerToggle comment ("release" → "released")
- Remove unused HidKeyCode import from sticky_mod.rs
…ase guard Action::Key(KeyCode::Hid(LShift)) etc. are modifier keys expressed via the Key action rather than the Modifier action. The SM release guard now checks hid_key.is_modifier() so that holding Shift for reverse-Tab cycling doesn't break StickyMod state.
Five rusty_fork_test cases covering: basic two-press flow, layer-change cleanup, Shift-does-not-release-SM, rapid triple presses, and combined LCtrl|LShift modifier.
rusty-fork was used by keyboard_sticky_mod_test but missing from Cargo.toml dev-dependencies. Also add missing .await on process_action_layer_switch call in test_key_action_transparent (function became async in upstream refactor).
…d StickyMod docs - Make DurationMillis pub (was pub(crate), caused visibility warning via StickyModConfig pub field) - Add test_sm_action_parsing and test_sm_action_grammar to rmk-config/src/layout.rs - Add Sticky Modifiers section to behavior.md - Add SM(key, modifier) entry to layout.md advanced layer operations list
Move timeout tracking out of the release handler's blocking select and into the main run() loop, following the same pattern as mouse repeat deadlines. - StickyModState::Active now stores an optional Instant deadline - Deadline is set (and reset) on each SM key PRESS, so repeated presses extend the hold window rather than starting from the release - run() combines SM and mouse deadlines and uses with_deadline(); on expiry it calls release_sticky_mod_if_active() before continuing - Remove embassy_futures select from release handler (was fragile: any event arriving cancelled the timer, preventing timeout on 2nd+ press) - Add sticky_mod_timeout() accessor to KeyMap - Add 2 integration tests: test_sm_timeout and test_sm_timeout_resets_on_press
…shots - Replace map_or(false, ...) with is_some_and(...) in keyboard.rs run() loop - Regenerate endpoint key snapshots in rmk-types: Action::StickyMod added a variant to the Action enum, changing the postcard schema hash for keymap, combo, and morse endpoints
|
Thanks for the feedback on consolidating SM and OSM! I've rebased this branch onto the latest What changed:
Example usage: |
…es not yet implemented) Tests cover: basic flow, layer-change cleanup, shift coexistence, rapid presses, combined modifiers, global timeout, timeout reset, max_repeat, per-key timeout, exit_on_layer_change=true, and exit_on_layer_change=false (survives layer change). Compile fails on StickyKeyConfig, StickyKeyAction, sk! macro, and BehaviorConfig::sticky_key — all to be added in Tasks 3–8.
| /// Modifier and layer effects can coexist and therefore own distinct policies, | ||
| /// phases, sources, and deadlines. A tap key is exclusive with both. | ||
| #[derive(Clone, Copy, Debug, Default)] | ||
| pub(crate) struct StickyKeyState { |
There was a problem hiding this comment.
I think sticky key should be general enough to process all held Actions. Distinguishing between modifier/layer/keys makes the current implementation not general at all. That means, this state should not be the current shape.
There was a problem hiding this comment.
Thanks — I reworked the state around that model in 03eb1a91. StickyKeyState now owns a uniform bounded collection of StickyEntry values, and every entry stores its canonical held Action together with the shared phase, policy, deadline, repetition, and buffered-claim lifecycle. OSM, OSL, and modified tap keys no longer have separate top-level state slots or lifecycle types.
Action-specific handling is now limited to applying and releasing the concrete effect. The small amount of modifier-only metadata remains because modifiers can have multiple physical producers and must remember whether their HID effect has already been reported; layer and tap-key effects are represented completely by their stored Action. The OSM and OSL aliases remain unchanged. 46673893 adds parity and edge-case coverage to ensure the consolidation does not remove existing behavior.
There was a problem hiding this comment.
sorry maybe I didn't make it clear, the current code still has modifier/layer/tap-key processing logic in sticky key, which leads to the binary size bloats -- these actions' processing logic is duplicate in normal path and sticky key path.
Actually sticky key itself should not execute/classify any Action variant, all "effect" should be removed. Only a thin state like sticky: (Action, u8) is needed.
HaoboGu
left a comment
HaoboGu
left a comment
There was a problem hiding this comment.
For the release_on_keyup_after, I feel it's more like a Morse/Tap-Hold behavior, not Sticky Key's. But since we've promoted the StickyKey as a KeyAction, I don't know if there's other solutions, so your code might be right and the only way to achieve this.
I prefer to have a name like release_after_hold and set a longer threshold?
Works for me.
|
|
Sorry, I've been busy with a conference, getting ready to move soon, starting a new job and more. It has been a crazy month. Thus I've been unable to keep up on this PR. I resolved all the conflicts and now I'm working through anything that code reviews bring up (better models mean more identified potential issues) and I'm working through them right now. I'll also see if I cannot reduce the flash/ram usage as well one more time. Once they're all done (hopefully today) I'll tag you and HaoboGu to let you know that I think I got everything. Also, as I've been using this branch daily since July I did come across another minor issue that does come up on occasion. I fixed the issue and now (confirmed on my keyboard). For the record, the issue I mentioned above is explained with an example below: If I hold OSM(ctrl)+OSM(shift)+leftarrow (again just an example) and then I lift off of OSM(ctrl) and continue to hold OSM(shift)+leftarrow the OSM(ctrl) is still being sent. To get the desired behavior, I must release all keys on the keyboard before I can use OSM(shift)+leftarrow correctly. Also, note that I can lift up on the leftarrow while holding down the OSM(shift), but the keyboard is still sending the control and the shift simultaneously. If I left up on the OSM(ctrl) while still holding any other key. I want it to just send only the keys (be they OSM or not) that I'm holding down. I also want when I release that, OSM(ctrl) for the keyboard to immediately stop sending that OSM(ctrl) to the OS but continue sending the OSM(shift) which I am holding. |
|
Okay, I think I've gone about as far as I can but in one of the final reviews of the code codex found an issue with Auto Mouse Layer below is the small summary of the issue along with the recommended fix after exploring a few different possibilities.
I've tried as much as possible (though not always successfully) not to touch any other features, so I thought I would ask on this one. I've committed the fix that it came up with, but I'll undo it and tackle it some other way if you think there is a better approach or if you think we can just leave it as is. Other recent changes (mostly just to deal with merges, or to address failed tests) as summarized by codex.
Let me know if you need anything else. |
|
Thanks @ldsands ！ I have an idea about sharing state between processors. Would you mind me editing code in this PR? |
@HaoboGu Absolutely feel free. edit: I think I had said something about this before, but I am mostly self-taught in Python for data science. I know enough to be able to muddle my way through other programming languages and now with AI where it is, I'm able to make contributions that work, but as you have clearly observed, my ability to do programming of this kind "well" is limited. Thanks again for your patience with me on this pr. |
This branch has not been deployed
3 participants
Summary
Adds a new
SM(key, modifier)action — StickyMod — that holds a modifieracross repeated presses of the same key, then releases it automatically when
any non-SM, non-modifier key is pressed, the active layer changes, or an
optional timeout expires.
Primary use case: Alt+Tab window/tab cycling. Bind
SM(Tab, LAlt)to akey; the first press sends Alt+Tab, subsequent presses send Tab (Alt stays
held), and Alt releases as soon as you press any other key or switch layers.
Motivation
This is a direct port of the
KC.SK()(Sticky Key) behavior from KMK firmware.For those migrating from KMK,
SM(Tab, LAlt)replicatesKC.SK(KC.LALT)usedin conjunction with Tab for Alt+Tab cycling. This was the last regularly-used
KMK feature I needed in order to fully replicate my KMK keymap in RMK
(though others may find additional gaps).
This feature covers similar ground to #724 (Tabber), which I was aware of
before writing this implementation but found didn't quite fit my needs — I
preferred this approach because it generalizes to any key+modifier combination
rather than being Tab-specific, and includes timeout support. That said, I
have no attachment to the name
SMorStickyMod— happy to rename this towhatever fits best if this is merged.
How it differs from OSM
OSM(mod)SM(key, mod)Behavior details
modifier + keykeyagain; timeout resetsModifieractions and HID modifier keycodes (Shift, Ctrl, etc.)do not release SM — this lets Shift+Tab work for reverse cycling
Optional timeout
Timeout is measured from the last SM press — repeated presses extend the
hold window. Implemented via the main
run()loop deadline (same pattern asmouse repeat), so it fires reliably regardless of how many press/release
cycles have occurred.
Default: no timeout (modifier held until released by keypress or layer change).
TOML syntax
Changes
rmk-types: newAction::StickyMod(KeyCode, ModifierCombination)variantrmk: newkeyboard/sticky_mod.rsmodule —StickyModStatewith optional deadline, state machine, and processing logicrmk: integrated intokeyboard.rs— dispatch, modifier resolution, release triggers on layer deactivation, deadline-based timeout inrun()looprmk-config: TOML grammar (keymap.pest) and parser forSM(key, mod)syntaxrmk-macro: codegen support forSMinaction_parser.rsandbehavior.rsrmk:sm!()macro inlayout_macro.rsdocs:behavior.md(Sticky Modifiers section),layout.md(SM syntax entry)Tests
7 integration tests in
rmk/tests/keyboard_sticky_mod_test.rs:LCtrl|LShiftcombination