From 5a26255735da5980851c3257d3fbbb8c8263ad11 Mon Sep 17 00:00:00 2001 From: Sviatoslav Bulbakha Date: Wed, 17 Jul 2024 03:55:22 +0400 Subject: [PATCH] feat(combos): do not interrupt combos on other key release --- app/src/combo.c | 81 +++++++++++++++---- .../combo/other-key-release/events.patterns | 1 + .../other-key-release/keycode_events.snapshot | 4 + .../other-key-release/native_posix_64.keymap | 41 ++++++++++ .../overlapping-combos-5/events.patterns | 1 + .../keycode_events.snapshot | 4 + .../native_posix_64.keymap | 67 +++++++++++++++ .../3b-tap-int-seq/keycode_events.snapshot | 4 +- 8 files changed, 186 insertions(+), 17 deletions(-) create mode 100644 app/tests/combo/other-key-release/events.patterns create mode 100644 app/tests/combo/other-key-release/keycode_events.snapshot create mode 100644 app/tests/combo/other-key-release/native_posix_64.keymap create mode 100644 app/tests/combo/overlapping-combos-5/events.patterns create mode 100644 app/tests/combo/overlapping-combos-5/keycode_events.snapshot create mode 100644 app/tests/combo/overlapping-combos-5/native_posix_64.keymap diff --git a/app/src/combo.c b/app/src/combo.c index c3334bdb754..f3fd641fe8c 100644 --- a/app/src/combo.c +++ b/app/src/combo.c @@ -270,12 +270,25 @@ static int capture_pressed_key(const struct zmk_position_state_changed *ev) { const struct zmk_listener zmk_listener_combo; -static int release_pressed_keys() { +static int release_pressed_key(int32_t position) { + for (int i = 0; i < pressed_keys_count; i++) { + struct zmk_position_state_changed_event *ev = &pressed_keys[i]; + + if (ev->data.position == position) { + LOG_DBG("combo: releasing position event %d", ev->data.position); + return pressed_keys_count--; + } + } + + return pressed_keys_count; +} + +static int release_pressed_keys(bool release_first) { uint32_t count = pressed_keys_count; pressed_keys_count = 0; for (int i = 0; i < count; i++) { struct zmk_position_state_changed_event *ev = &pressed_keys[i]; - if (i == 0) { + if (release_first && i == 0) { LOG_DBG("combo: releasing position event %d", ev->data.position); ZMK_EVENT_RELEASE(*ev); } else { @@ -348,7 +361,7 @@ static void activate_combo(struct combo_cfg *combo) { struct active_combo *active_combo = store_active_combo(combo); if (active_combo == NULL) { // unable to store combo - release_pressed_keys(); + release_pressed_keys(true); return; } move_pressed_keys_to_active_combo(active_combo); @@ -406,8 +419,10 @@ static int cleanup() { if (fully_pressed_combo != NULL) { activate_combo(fully_pressed_combo); fully_pressed_combo = NULL; + return release_pressed_keys(false); } - return release_pressed_keys(); + + return release_pressed_keys(true); } static void update_timeout_task() { @@ -459,19 +474,55 @@ static int position_state_down(const zmk_event_t *ev, struct zmk_position_state_ } } +static bool is_key_part_of_candidate(int32_t position) { + for (int i = 0; i < CONFIG_ZMK_COMBO_MAX_COMBOS_PER_KEY; i++) { + struct combo_candidate *candidate = &candidates[i]; + if (candidate->combo == NULL) { + continue; + } + + for (int j = 0; j < candidate->combo->key_position_len; j++) { + if (candidate->combo->key_positions[j] == position) { + return true; + } + } + } + return false; +} + +static bool is_key_part_of_active_combo(int32_t position) { + for (int i = 0; i < active_combo_count; i++) { + struct active_combo *active_combo = &active_combos[i]; + for (int j = 0; j < active_combo->key_positions_pressed_count; j++) { + if (active_combo->key_positions_pressed[j].data.position == position) { + return true; + } + } + } + return false; +} + +static bool is_key_part_of_any_combo(int32_t position) { + return is_key_part_of_candidate(position) || is_key_part_of_active_combo(position); +} + 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; - } - 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. - struct zmk_position_state_changed_event dupe_ev = - copy_raised_zmk_position_state_changed(data); - ZMK_EVENT_RAISE(dupe_ev); - return ZMK_EV_EVENT_CAPTURED; + // Check if the released key is part of any combo candidate or active combo + if (is_key_part_of_any_combo(data->position)) { + int released_keys = cleanup(); + if (release_combo_key(data->position, data->timestamp)) { + return ZMK_EV_EVENT_HANDLED; + } + 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. + struct zmk_position_state_changed_event dupe_ev = + copy_raised_zmk_position_state_changed(data); + ZMK_EVENT_RAISE(dupe_ev); + return ZMK_EV_EVENT_CAPTURED; + } } + release_pressed_key(data->position); return ZMK_EV_EVENT_BUBBLE; } diff --git a/app/tests/combo/other-key-release/events.patterns b/app/tests/combo/other-key-release/events.patterns new file mode 100644 index 00000000000..b1342af4d97 --- /dev/null +++ b/app/tests/combo/other-key-release/events.patterns @@ -0,0 +1 @@ +s/.*hid_listener_keycode_//p diff --git a/app/tests/combo/other-key-release/keycode_events.snapshot b/app/tests/combo/other-key-release/keycode_events.snapshot new file mode 100644 index 00000000000..3c8dc1381b1 --- /dev/null +++ b/app/tests/combo/other-key-release/keycode_events.snapshot @@ -0,0 +1,4 @@ +pressed: usage_page 0x07 keycode 0x06 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x06 implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x07 implicit_mods 0x00 explicit_mods 0x00 diff --git a/app/tests/combo/other-key-release/native_posix_64.keymap b/app/tests/combo/other-key-release/native_posix_64.keymap new file mode 100644 index 00000000000..f0210f17733 --- /dev/null +++ b/app/tests/combo/other-key-release/native_posix_64.keymap @@ -0,0 +1,41 @@ +#include +#include +#include + +/ { + combos { + compatible = "zmk,combos"; + + combo_d { + timeout-ms = <50>; + key-positions = <0 1>; + bindings = <&kp D>; + }; + + // not actuated combo to test behavior + combo_g { + timeout-ms = <50>; + key-positions = <2 3>; + bindings = <&kp G>; + }; + }; + + keymap { + compatible = "zmk,keymap"; + + default_layer { + bindings = <&kp A &kp B &kp C &kp D>; + }; + }; +}; + +&kscan { + events = < + ZMK_MOCK_PRESS(0,2,10) // C + ZMK_MOCK_PRESS(0,0,10) // A + ZMK_MOCK_RELEASE(0,2,10) // C + ZMK_MOCK_PRESS(0,1,10) // B + ZMK_MOCK_RELEASE(0,0,0) // A + ZMK_MOCK_RELEASE(0,1,0) // B + >; +}; diff --git a/app/tests/combo/overlapping-combos-5/events.patterns b/app/tests/combo/overlapping-combos-5/events.patterns new file mode 100644 index 00000000000..b1342af4d97 --- /dev/null +++ b/app/tests/combo/overlapping-combos-5/events.patterns @@ -0,0 +1 @@ +s/.*hid_listener_keycode_//p diff --git a/app/tests/combo/overlapping-combos-5/keycode_events.snapshot b/app/tests/combo/overlapping-combos-5/keycode_events.snapshot new file mode 100644 index 00000000000..bb47d85203c --- /dev/null +++ b/app/tests/combo/overlapping-combos-5/keycode_events.snapshot @@ -0,0 +1,4 @@ +pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00 diff --git a/app/tests/combo/overlapping-combos-5/native_posix_64.keymap b/app/tests/combo/overlapping-combos-5/native_posix_64.keymap new file mode 100644 index 00000000000..875b2df656e --- /dev/null +++ b/app/tests/combo/overlapping-combos-5/native_posix_64.keymap @@ -0,0 +1,67 @@ +#include +#include +#include + +/* +combo on 01 and 34 +overlapping combo on 012 and 345 + +press 01 and then 34 both in their combo terms +should actuate both combos +*/ + +/ { + combos { + compatible = "zmk,combos"; + + combo_a { + timeout-ms = <50>; + key-positions = <0 1>; + bindings = <&kp A>; + }; + + combo_b { + timeout-ms = <50>; + key-positions = <3 4>; + bindings = <&kp B>; + }; + + combo_c { + timeout-ms = <100>; + key-positions = <0 1 2>; + bindings = <&kp C>; + }; + + combo_d { + timeout-ms = <100>; + key-positions = <3 4 5>; + bindings = <&kp D>; + }; + }; + + keymap { + compatible = "zmk,keymap"; + + default_layer { + bindings = < + &kp N0 &kp N1 &kp N2 + &kp N3 &kp N4 &kp N5 + >; + }; + }; +}; + +&kscan { + rows = <2>; + columns = <3>; + events = < + ZMK_MOCK_PRESS(0,0,10) + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_PRESS(1,0,10) + ZMK_MOCK_PRESS(1,1,10) + ZMK_MOCK_RELEASE(0,0,10) + ZMK_MOCK_RELEASE(0,1,10) + ZMK_MOCK_RELEASE(1,0,10) + ZMK_MOCK_RELEASE(1,1,10) + >; +}; diff --git a/app/tests/tap-dance/3b-tap-int-seq/keycode_events.snapshot b/app/tests/tap-dance/3b-tap-int-seq/keycode_events.snapshot index 31113ffc4aa..38b560dbfde 100644 --- a/app/tests/tap-dance/3b-tap-int-seq/keycode_events.snapshot +++ b/app/tests/tap-dance/3b-tap-int-seq/keycode_events.snapshot @@ -1,10 +1,10 @@ td_binding_pressed: 2 created new tap dance td_binding_pressed: 2 tap dance pressed +td_binding_released: 2 tap dance keybind released kp_pressed: usage_page 0x07 keycode 0x1E implicit_mods 0x00 explicit_mods 0x00 +kp_released: usage_page 0x07 keycode 0x1E implicit_mods 0x00 explicit_mods 0x00 td_binding_pressed: 1 created new tap dance td_binding_pressed: 1 tap dance pressed kp_pressed: usage_page 0x07 keycode 0x16 implicit_mods 0x00 explicit_mods 0x00 -td_binding_released: 2 tap dance keybind released -kp_released: usage_page 0x07 keycode 0x1E implicit_mods 0x00 explicit_mods 0x00 td_binding_released: 1 tap dance keybind released kp_released: usage_page 0x07 keycode 0x16 implicit_mods 0x00 explicit_mods 0x00