From ad5a12a7bcdc7c0962fdef7c23eaa0cf14d0b7ed Mon Sep 17 00:00:00 2001 From: okke Date: Sun, 27 Feb 2022 14:11:36 +0100 Subject: [PATCH] fix(behaviors): Fix bug in nested sticky keys If multiple sticky keys with quick release were nested, only the first one was properly released. This fix makes sure all of them are released properly. Fixes https://github.com/zmkfirmware/zmk/issues/1149 --- app/src/behaviors/behavior_sticky_key.c | 14 +++- .../sticky-keys/10-sl-sl-kp/events.patterns | 1 + .../10-sl-sl-kp/keycode_events.snapshot | 8 +++ .../10-sl-sl-kp/native_posix.keymap | 65 +++++++++++++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 app/tests/sticky-keys/10-sl-sl-kp/events.patterns create mode 100644 app/tests/sticky-keys/10-sl-sl-kp/keycode_events.snapshot create mode 100644 app/tests/sticky-keys/10-sl-sl-kp/native_posix.keymap diff --git a/app/src/behaviors/behavior_sticky_key.c b/app/src/behaviors/behavior_sticky_key.c index 3c75a7a3..7909e1af 100644 --- a/app/src/behaviors/behavior_sticky_key.c +++ b/app/src/behaviors/behavior_sticky_key.c @@ -188,6 +188,9 @@ static int sticky_key_keycode_state_changed_listener(const zmk_event_t *eh) { if (ev == NULL) { return ZMK_EV_EVENT_BUBBLE; } + + // keep track whether the event has been reraised, so we only reraise it once + bool event_reraised = false; for (int i = 0; i < ZMK_BHV_STICKY_KEY_MAX_HELD; i++) { struct active_sticky_key *sticky_key = &active_sticky_keys[i]; if (sticky_key->position == ZMK_BHV_STICKY_KEY_POSITION_FREE) { @@ -223,10 +226,12 @@ static int sticky_key_keycode_state_changed_listener(const zmk_event_t *eh) { if (sticky_key->timer_started) { stop_timer(sticky_key); if (sticky_key->config->quick_release) { - // continue processing the event. Release the sticky key afterwards. - ZMK_EVENT_RAISE_AFTER(eh, behavior_sticky_key); + // immediately release the sticky key after the key press is handled. + if (!event_reraised) { + ZMK_EVENT_RAISE_AFTER(eh, behavior_sticky_key); + event_reraised = true; + } release_sticky_key_behavior(sticky_key, ev->timestamp); - return ZMK_EV_EVENT_CAPTURED; } } sticky_key->modified_key_usage_page = ev->usage_page; @@ -240,6 +245,9 @@ static int sticky_key_keycode_state_changed_listener(const zmk_event_t *eh) { } } } + if (event_reraised) { + return ZMK_EV_EVENT_CAPTURED; + } return ZMK_EV_EVENT_BUBBLE; } diff --git a/app/tests/sticky-keys/10-sl-sl-kp/events.patterns b/app/tests/sticky-keys/10-sl-sl-kp/events.patterns new file mode 100644 index 00000000..833100f6 --- /dev/null +++ b/app/tests/sticky-keys/10-sl-sl-kp/events.patterns @@ -0,0 +1 @@ +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/sticky-keys/10-sl-sl-kp/keycode_events.snapshot b/app/tests/sticky-keys/10-sl-sl-kp/keycode_events.snapshot new file mode 100644 index 00000000..fc0f29b9 --- /dev/null +++ b/app/tests/sticky-keys/10-sl-sl-kp/keycode_events.snapshot @@ -0,0 +1,8 @@ +pressed: usage_page 0x07 keycode 0x1e implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x1e implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x1e implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x1e implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 diff --git a/app/tests/sticky-keys/10-sl-sl-kp/native_posix.keymap b/app/tests/sticky-keys/10-sl-sl-kp/native_posix.keymap new file mode 100644 index 00000000..e9b87f42 --- /dev/null +++ b/app/tests/sticky-keys/10-sl-sl-kp/native_posix.keymap @@ -0,0 +1,65 @@ +#include +#include +#include + +/* + sticky layers should quick-release. + Thus, the second keypress should be on the default layer, not on the lower_layer. +*/ + +/ { + keymap { + compatible = "zmk,keymap"; + label ="Default keymap"; + + default_layer { + bindings = < + &sl 1 &kp A + &none &none>; + }; + + layer_1 { + bindings = < + &sl 2 &none + &none &none>; + }; + + layer_2 { + bindings = < + &none &kp NUM_1 + &none &none>; + }; + }; +}; + +&kscan { + events = < + /* press sl 1 */ + ZMK_MOCK_PRESS(0,0,10) + ZMK_MOCK_RELEASE(0,0,10) + /* press sl 2 */ + ZMK_MOCK_PRESS(0,0,10) + ZMK_MOCK_RELEASE(0,0,10) + /* press 1 */ + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_RELEASE(0,1,10) + /* press A */ + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_RELEASE(0,1,10) + + /* repeat test to check if cleanup is done correctly */ + /* press sl 1 */ + ZMK_MOCK_PRESS(0,0,10) + ZMK_MOCK_RELEASE(0,0,10) + /* press sl 2 */ + ZMK_MOCK_PRESS(0,0,10) + ZMK_MOCK_RELEASE(0,0,10) + /* press 1 */ + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_RELEASE(0,1,10) + /* press A */ + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_RELEASE(0,1,10) + + >; +}; \ No newline at end of file