Skip to content

Commit

Permalink
Combo + hold tap fixes
Browse files Browse the repository at this point in the history
Squashes the following commits

commit d1006ca
Author: Peter Johanson <peter@peterjohanson.com>
Date:   Sun Jul 31 03:44:57 2022 +0000

    fix(behaviors): Properly clean up timed out hold.

    * If our handler dedides our undedided hold-tap,
      return early before continuing.

commit 0b4198d
Author: okke <okke@formsma.nl>
Date:   Tue Jul 26 21:51:02 2022 +0200

    Behaviors: Update last_listener_index before running event handler

    An event can be captured and released in the same event handler, before
    the last_listener_index would have been updated. This causes some handlers
    to be triggered multiple times.

    The solution is to update the last_listener_index before calling the next
    event handler, so capturing and releasing within an event handler is harmless.

    Also see discussion at
    zmkfirmware#1401

commit 8e9db11
Author: Andrew Rae <ajrae.nv@gmail.com>
Date:   Mon Jul 25 11:02:57 2022 -0700

    fix(behaviors): Fixing erroneous combo triggering (zmkfirmware#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.
  • Loading branch information
urob committed Jul 31, 2022
1 parent 33d6131 commit 8004f3b
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 1 deletion.
4 changes: 4 additions & 0 deletions app/src/behaviors/behavior_hold_tap.c
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,10 @@ static int position_state_changed_listener(const zmk_event_t *eh) {
decide_hold_tap(undecided_hold_tap, HT_TIMER_EVENT);
}

if (undecided_hold_tap == NULL) {
return ZMK_EV_EVENT_BUBBLE;
}

if (!ev->state && find_captured_keydown_event(ev->position) == NULL) {
// no keydown event has been captured, let it bubble.
// we'll catch modifiers later in modifier_state_changed_listener
Expand Down
2 changes: 1 addition & 1 deletion app/src/event_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ int zmk_event_manager_handle_from(zmk_event_t *event, uint8_t start_index) {
if (ev_sub->event_type != event->event) {
continue;
}
event->last_listener_index = i;
ret = ev_sub->listener->callback(event);
switch (ret) {
case ZMK_EV_EVENT_BUBBLE:
Expand All @@ -35,7 +36,6 @@ int zmk_event_manager_handle_from(zmk_event_t *event, uint8_t start_index) {
goto release;
case ZMK_EV_EVENT_CAPTURED:
LOG_DBG("Listener captured the event");
event->last_listener_index = i;
// Listeners are expected to free events they capture
return 0;
default:
Expand Down
1 change: 1 addition & 0 deletions app/tests/combo/combos-and-holdtaps-3/events.patterns
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
s/.*hid_listener_keycode_//p
5 changes: 5 additions & 0 deletions app/tests/combo/combos-and-holdtaps-3/keycode_events.snapshot
Original file line number Diff line number Diff line change
@@ -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
40 changes: 40 additions & 0 deletions app/tests/combo/combos-and-holdtaps-3/native_posix_64.keymap
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#include <dt-bindings/zmk/keys.h>
#include <behaviors.dtsi>
#include <dt-bindings/zmk/kscan-mock.h>

&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)
>;
};
1 change: 1 addition & 0 deletions app/tests/combo/combos-and-holdtaps-4/events.patterns
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
s/.*hid_listener_keycode_//p
4 changes: 4 additions & 0 deletions app/tests/combo/combos-and-holdtaps-4/keycode_events.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pressed: usage_page 0x07 keycode 0xE1 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
released: usage_page 0x07 keycode 0xE1 implicit_mods 0x00 explicit_mods 0x00
46 changes: 46 additions & 0 deletions app/tests/combo/combos-and-holdtaps-4/native_posix_64.keymap
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#include <dt-bindings/zmk/keys.h>
#include <behaviors.dtsi>
#include <dt-bindings/zmk/kscan-mock.h>


#define ZMK_COMBO(name, combo_bindings, keypos, combo_term) \
/ { \
combos { \
compatible = "zmk,combos"; \
combo_ ## name { \
key-positions = <keypos>; \
bindings = <combo_bindings>; \
timeout-ms = <combo_term>; \
}; \
}; \
};

ZMK_COMBO(qmark, &kp QMARK, 0 3, 30)
ZMK_COMBO(dllr, &kp DLLR, 1 3, 50)
ZMK_COMBO(tilde, &kp TILDE, 3 4, 50)

/ {
keymap {
compatible = "zmk,keymap";
label = "Default keymap";

default_layer {
bindings = <
&none &none
&kp A &mt LSHFT T
&none
>;
};
};
};

&kscan {
rows = <3>;
columns = <2>;
events = <
ZMK_MOCK_PRESS(1,1,500)
ZMK_MOCK_PRESS(1,0,100)
ZMK_MOCK_RELEASE(1,0,500)
ZMK_MOCK_RELEASE(1,1,0)
>;
};

0 comments on commit 8004f3b

Please sign in to comment.