From 8e9db111653e3e162757527728c1c332f9088942 Mon Sep 17 00:00:00 2001 From: Andrew Rae Date: Mon, 25 Jul 2022 11:02:57 -0700 Subject: [PATCH] fix(behaviors): Fixing erroneous combo triggering (#1395) This is a very simple fix to a rather complicated issue. Essentially, hold-taps will "release" (raise) their captured keys before actually telling the event manager they have captured a key. This means the event manager ends up assigning the `last_listener_index` to the hold-tap subscription rather than the combo. So when the combo calls `ZMK_EVENT_RELEASE` it raises after the hold-tap instead of after the combo as the combo code expects. The corresponding test (which fails without this change) has also been added. --- app/src/combo.c | 2 +- .../combos-and-holdtaps-3/events.patterns | 1 + .../keycode_events.snapshot | 5 +++ .../native_posix_64.keymap | 40 +++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 app/tests/combo/combos-and-holdtaps-3/events.patterns create mode 100644 app/tests/combo/combos-and-holdtaps-3/keycode_events.snapshot create mode 100644 app/tests/combo/combos-and-holdtaps-3/native_posix_64.keymap diff --git a/app/src/combo.c b/app/src/combo.c index 13ed1709..e24b510a 100644 --- a/app/src/combo.c +++ b/app/src/combo.c @@ -253,7 +253,7 @@ static int release_pressed_keys() { if (i == 0) { LOG_DBG("combo: releasing position event %d", as_zmk_position_state_changed(captured_event)->position); - ZMK_EVENT_RELEASE(captured_event) + ZMK_EVENT_RAISE_AFTER(captured_event, combo); } else { // reprocess events (see tests/combo/fully-overlapping-combos-3 for why this is needed) LOG_DBG("combo: reraising position event %d", diff --git a/app/tests/combo/combos-and-holdtaps-3/events.patterns b/app/tests/combo/combos-and-holdtaps-3/events.patterns new file mode 100644 index 00000000..833100f6 --- /dev/null +++ b/app/tests/combo/combos-and-holdtaps-3/events.patterns @@ -0,0 +1 @@ +s/.*hid_listener_keycode_//p \ No newline at end of file diff --git a/app/tests/combo/combos-and-holdtaps-3/keycode_events.snapshot b/app/tests/combo/combos-and-holdtaps-3/keycode_events.snapshot new file mode 100644 index 00000000..843832dd --- /dev/null +++ b/app/tests/combo/combos-and-holdtaps-3/keycode_events.snapshot @@ -0,0 +1,5 @@ +pressed: usage_page 0x07 keycode 0xE5 implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00 +pressed: usage_page 0x07 keycode 0x06 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00 +released: usage_page 0x07 keycode 0x06 implicit_mods 0x00 explicit_mods 0x00 diff --git a/app/tests/combo/combos-and-holdtaps-3/native_posix_64.keymap b/app/tests/combo/combos-and-holdtaps-3/native_posix_64.keymap new file mode 100644 index 00000000..d4053793 --- /dev/null +++ b/app/tests/combo/combos-and-holdtaps-3/native_posix_64.keymap @@ -0,0 +1,40 @@ +#include +#include +#include + +&mt { + flavor = "hold-preferred"; +}; + +/ { + combos { + compatible = "zmk,combos"; + combo_one { + timeout-ms = <40>; + key-positions = <0 1>; + bindings = <&kp X>; + }; + }; + + keymap { + compatible = "zmk,keymap"; + label = "Default keymap"; + + default_layer { + bindings = < + &kp A &kp B + &mt RSHFT RET &kp C + >; + }; + }; +}; + +&kscan { + events = < + ZMK_MOCK_PRESS(1,0,10) + ZMK_MOCK_PRESS(0,1,10) + ZMK_MOCK_PRESS(1,1,10) + ZMK_MOCK_RELEASE(0,1,50) + ZMK_MOCK_RELEASE(1,1,50) + >; +};