fix(behaviors): Fixing erroneous combo triggering, hold-taps sticking

* 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.
* 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 https://github.com/zmkfirmware/zmk/pull/1401
* If our handler dedides our undedided hold-tap,
  return early before continuing.
* Fix incorrect pointer logic, resulting in combo
  candidate filtering leaving incorrect timeout details.

Co-authored-by: Andrew Rae <ajrae.nv@gmail.com>
Co-authored-by: okke <okke@formsma.nl>
This commit is contained in:
Pete Johanson
2022-08-03 20:09:50 -04:00
committed by GitHub
parent eee7e1cd41
commit fc511e40cc
9 changed files with 103 additions and 2 deletions

View File

@@ -612,6 +612,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

View File

@@ -211,7 +211,7 @@ static int filter_timed_out_candidates(int64_t timestamp) {
if (candidate->timeout_at > timestamp) {
// reorder candidates so they're contiguous
candidates[num_candidates].combo = candidate->combo;
candidates[num_candidates].timeout_at = candidates->timeout_at;
candidates[num_candidates].timeout_at = candidate->timeout_at;
num_candidates++;
} else {
candidate->combo = NULL;

View File

@@ -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:
@@ -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: