From 0df71100581d040178bd0fe8ec0382d84dc59a40 Mon Sep 17 00:00:00 2001 From: Okke Formsma Date: Sat, 27 Feb 2021 22:34:15 +0100 Subject: [PATCH] fix(combos): Fix stuck keys when pressing long combos. To properly retrigger hold-taps when a combo is not activated, some position down events are reraised instead of released. The corresponding position up events were never reraised, causing a potential stuck key. --- app/src/combo.c | 56 +++++++++++-------- .../combos-and-holdtaps-0/events.patterns | 1 - .../combos-and-holdtaps-1/events.patterns | 3 +- .../combos-and-holdtaps-2/events.patterns | 3 +- .../combo/layer-filter-0/events.patterns | 3 +- .../combo/layer-filter-1/events.patterns | 3 +- .../overlapping-combos-0/events.patterns | 3 +- .../overlapping-combos-1/events.patterns | 3 +- .../overlapping-combos-2/events.patterns | 3 +- .../overlapping-combos-3/events.patterns | 3 +- .../press-release-long-combo/events.patterns | 1 + .../keycode_events.snapshot | 4 ++ .../native_posix.keymap | 35 ++++++++++++ .../events.patterns | 3 +- .../events.patterns | 1 - .../events.patterns | 3 +- 16 files changed, 82 insertions(+), 46 deletions(-) create mode 100644 app/tests/combo/press-release-long-combo/events.patterns create mode 100644 app/tests/combo/press-release-long-combo/keycode_events.snapshot create mode 100644 app/tests/combo/press-release-long-combo/native_posix.keymap diff --git a/app/src/combo.c b/app/src/combo.c index b50a0f61..72535b29 100644 --- a/app/src/combo.c +++ b/app/src/combo.c @@ -192,7 +192,7 @@ static inline bool candidate_is_completely_pressed(struct combo_cfg *candidate) return pressed_keys[candidate->key_position_len - 1] != NULL; } -static void cleanup(); +static int cleanup(); static int filter_timed_out_candidates(int64_t timestamp) { int num_candidates = 0; @@ -224,7 +224,7 @@ static int clear_candidates() { } static int capture_pressed_key(const zmk_event_t *ev) { - for (int i = 0; i < CONFIG_ZMK_COMBO_MAX_COMBOS_PER_KEY; i++) { + for (int i = 0; i < CONFIG_ZMK_COMBO_MAX_KEYS_PER_COMBO; i++) { if (pressed_keys[i] != NULL) { continue; } @@ -236,23 +236,25 @@ static int capture_pressed_key(const zmk_event_t *ev) { const struct zmk_listener zmk_listener_combo; -static void release_pressed_keys() { - // release the first key that was pressed - if (pressed_keys[0] == NULL) { - return; - } - ZMK_EVENT_RELEASE(pressed_keys[0]) - pressed_keys[0] = NULL; - - // reprocess events (see tests/combo/fully-overlapping-combos-3 for why this is needed) - for (int i = 1; i < CONFIG_ZMK_COMBO_MAX_COMBOS_PER_KEY; i++) { - if (pressed_keys[i] == NULL) { - return; - } +static int release_pressed_keys() { + for (int i = 0; i < CONFIG_ZMK_COMBO_MAX_KEYS_PER_COMBO; i++) { const zmk_event_t *captured_event = pressed_keys[i]; + if (pressed_keys[i] == NULL) { + return i; + } pressed_keys[i] = NULL; - ZMK_EVENT_RAISE(captured_event); + if (i == 0) { + LOG_DBG("combo: releasing position event %d", + as_zmk_position_state_changed(captured_event)->position); + ZMK_EVENT_RELEASE(captured_event) + } else { + // reprocess events (see tests/combo/fully-overlapping-combos-3 for why this is needed) + LOG_DBG("combo: reraising position event %d", + as_zmk_position_state_changed(captured_event)->position); + ZMK_EVENT_RAISE(captured_event); + } } + return CONFIG_ZMK_COMBO_MAX_KEYS_PER_COMBO; } static inline int press_combo_behavior(struct combo_cfg *combo, int32_t timestamp) { @@ -360,14 +362,14 @@ static bool release_combo_key(int32_t position, int64_t timestamp) { return false; } -static void cleanup() { +static int cleanup() { k_delayed_work_cancel(&timeout_task); clear_candidates(); if (fully_pressed_combo != NULL) { activate_combo(fully_pressed_combo); fully_pressed_combo = NULL; } - release_pressed_keys(); + return release_pressed_keys(); } static void update_timeout_task() { @@ -399,6 +401,7 @@ static int position_state_down(const zmk_event_t *ev, struct zmk_position_state_ update_timeout_task(); struct combo_cfg *candidate_combo = candidates[0].combo; + LOG_DBG("combo: capturing position event %d", data->position); int ret = capture_pressed_key(ev); switch (num_candidates) { case 0: @@ -418,13 +421,18 @@ static int position_state_down(const zmk_event_t *ev, struct zmk_position_state_ } } -static int position_state_up(struct zmk_position_state_changed *ev) { - cleanup(); - if (release_combo_key(ev->position, ev->timestamp)) { +static int position_state_up(const zmk_event_t *ev, struct zmk_position_state_changed *data) { + int released_keys = cleanup(); + if (release_combo_key(data->position, data->timestamp)) { return ZMK_EV_EVENT_HANDLED; - } else { - return 0; } + if (released_keys > 1) { + // The second and further key down events are re-raised. To preserve + // correct order for e.g. hold-taps, reraise the key up event too. + ZMK_EVENT_RAISE(ev); + return ZMK_EV_EVENT_CAPTURED; + } + return 0; } static void combo_timeout_handler(struct k_work *item) { @@ -447,7 +455,7 @@ static int position_state_changed_listener(const zmk_event_t *ev) { if (data->state) { // keydown return position_state_down(ev, data); } else { // keyup - return position_state_up(data); + return position_state_up(ev, data); } } diff --git a/app/tests/combo/combos-and-holdtaps-0/events.patterns b/app/tests/combo/combos-and-holdtaps-0/events.patterns index b90d7863..b1342af4 100644 --- a/app/tests/combo/combos-and-holdtaps-0/events.patterns +++ b/app/tests/combo/combos-and-holdtaps-0/events.patterns @@ -1,2 +1 @@ s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file diff --git a/app/tests/combo/combos-and-holdtaps-1/events.patterns b/app/tests/combo/combos-and-holdtaps-1/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/combos-and-holdtaps-1/events.patterns +++ b/app/tests/combo/combos-and-holdtaps-1/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/combos-and-holdtaps-2/events.patterns b/app/tests/combo/combos-and-holdtaps-2/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/combos-and-holdtaps-2/events.patterns +++ b/app/tests/combo/combos-and-holdtaps-2/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/layer-filter-0/events.patterns b/app/tests/combo/layer-filter-0/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/layer-filter-0/events.patterns +++ b/app/tests/combo/layer-filter-0/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/layer-filter-1/events.patterns b/app/tests/combo/layer-filter-1/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/layer-filter-1/events.patterns +++ b/app/tests/combo/layer-filter-1/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/overlapping-combos-0/events.patterns b/app/tests/combo/overlapping-combos-0/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/overlapping-combos-0/events.patterns +++ b/app/tests/combo/overlapping-combos-0/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/overlapping-combos-1/events.patterns b/app/tests/combo/overlapping-combos-1/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/overlapping-combos-1/events.patterns +++ b/app/tests/combo/overlapping-combos-1/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/overlapping-combos-2/events.patterns b/app/tests/combo/overlapping-combos-2/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/overlapping-combos-2/events.patterns +++ b/app/tests/combo/overlapping-combos-2/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/overlapping-combos-3/events.patterns b/app/tests/combo/overlapping-combos-3/events.patterns index b90d7863..833100f6 100644 --- a/app/tests/combo/overlapping-combos-3/events.patterns +++ b/app/tests/combo/overlapping-combos-3/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo//p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/press-release-long-combo/events.patterns b/app/tests/combo/press-release-long-combo/events.patterns new file mode 100644 index 00000000..b1342af4 --- /dev/null +++ b/app/tests/combo/press-release-long-combo/events.patterns @@ -0,0 +1 @@ +s/.*hid_listener_keycode_//p diff --git a/app/tests/combo/press-release-long-combo/keycode_events.snapshot b/app/tests/combo/press-release-long-combo/keycode_events.snapshot new file mode 100644 index 00000000..e7c0cb12 --- /dev/null +++ b/app/tests/combo/press-release-long-combo/keycode_events.snapshot @@ -0,0 +1,4 @@ +pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00 diff --git a/app/tests/combo/press-release-long-combo/native_posix.keymap b/app/tests/combo/press-release-long-combo/native_posix.keymap new file mode 100644 index 00000000..68736d8f --- /dev/null +++ b/app/tests/combo/press-release-long-combo/native_posix.keymap @@ -0,0 +1,35 @@ +#include +#include +#include + +/ { + combos { + compatible = "zmk,combos"; + combo_one { + timeout-ms = <80>; + key-positions = <0 1 2 3>; + bindings = <&kp Z>; + }; + }; + + keymap { + compatible = "zmk,keymap"; + label ="Default keymap"; + + default_layer { + bindings = < + &kp A &kp B + &kp C &kp D + >; + }; + }; +}; + +&kscan { + events = < + ZMK_MOCK_PRESS(1,1,10) + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_RELEASE(0,1,100) + ZMK_MOCK_RELEASE(1,1,100) + >; +}; \ No newline at end of file diff --git a/app/tests/combo/press1-press2-release1-release2/events.patterns b/app/tests/combo/press1-press2-release1-release2/events.patterns index 5f3e4cf7..833100f6 100644 --- a/app/tests/combo/press1-press2-release1-release2/events.patterns +++ b/app/tests/combo/press1-press2-release1-release2/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo/combo/p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/press1-press2-release2-release1/events.patterns b/app/tests/combo/press1-press2-release2-release1/events.patterns index b54b66b6..b1342af4 100644 --- a/app/tests/combo/press1-press2-release2-release1/events.patterns +++ b/app/tests/combo/press1-press2-release2-release1/events.patterns @@ -1,2 +1 @@ s/.*hid_listener_keycode_//p -s/.*combo/combo/p diff --git a/app/tests/combo/press1-release1-press2-release2/events.patterns b/app/tests/combo/press1-release1-press2-release2/events.patterns index 5f3e4cf7..833100f6 100644 --- a/app/tests/combo/press1-release1-press2-release2/events.patterns +++ b/app/tests/combo/press1-release1-press2-release2/events.patterns @@ -1,2 +1 @@ -s/.*hid_listener_keycode_//p -s/.*combo/combo/p \ No newline at end of file +s/.*hid_listener_keycode_//p \ No newline at end of file