Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(combos): clear combo timer only when affected keys are released #2494

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 71 additions & 15 deletions app/src/combo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -459,19 +474,60 @@ 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_candidate(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;
}
} else if (is_key_part_of_active_combo(data->position)) {
if (release_combo_key(data->position, data->timestamp)) {
return ZMK_EV_EVENT_HANDLED;
}
}

release_pressed_key(data->position);
return ZMK_EV_EVENT_BUBBLE;
}

Expand Down
1 change: 1 addition & 0 deletions app/tests/combo/other-key-release-2/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/other-key-release-2/keycode_events.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pressed: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00
released: usage_page 0x07 keycode 0x05 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
57 changes: 57 additions & 0 deletions app/tests/combo/other-key-release-2/native_posix_64.keymap
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#include <dt-bindings/zmk/keys.h>
#include <behaviors.dtsi>
#include <dt-bindings/zmk/kscan_mock.h>

/*
Combo on positions 01 and 23.

- press 2 and 3
- press 0
- release 2
- press 1
- release the rest

Both combos should actuate.
*/

/ {
combos {
compatible = "zmk,combos";

combo_a {
timeout-ms = <50>;
key-positions = <0 1>;
bindings = <&kp A>;
};

combo_b {
timeout-ms = <50>;
key-positions = <2 3>;
bindings = <&kp B>;
};
};

keymap {
compatible = "zmk,keymap";

default_layer {
bindings = <
&kp N0 &kp N1
&kp N2 &kp N3
>;
};
};
};

&kscan {
events = <
ZMK_MOCK_PRESS(1,0,10)
ZMK_MOCK_PRESS(1,1,10)
ZMK_MOCK_PRESS(0,0,10)
ZMK_MOCK_RELEASE(1,0,10)
ZMK_MOCK_PRESS(0,1,10)
ZMK_MOCK_RELEASE(1,1,0)
ZMK_MOCK_RELEASE(0,0,0)
ZMK_MOCK_RELEASE(0,1,0)
>;
};
1 change: 1 addition & 0 deletions app/tests/combo/other-key-release/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/other-key-release/keycode_events.snapshot
Original file line number Diff line number Diff line change
@@ -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
41 changes: 41 additions & 0 deletions app/tests/combo/other-key-release/native_posix_64.keymap
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#include <dt-bindings/zmk/keys.h>
#include <behaviors.dtsi>
#include <dt-bindings/zmk/kscan_mock.h>

/ {
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
>;
};
1 change: 1 addition & 0 deletions app/tests/combo/overlapping-combos-5/events.patterns
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
s/.*hid_listener_keycode_//p
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
released: 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 0x05 implicit_mods 0x00 explicit_mods 0x00
67 changes: 67 additions & 0 deletions app/tests/combo/overlapping-combos-5/native_posix_64.keymap
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#include <dt-bindings/zmk/keys.h>
#include <behaviors.dtsi>
#include <dt-bindings/zmk/kscan_mock.h>

/*
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)
>;
};
4 changes: 2 additions & 2 deletions app/tests/tap-dance/3b-tap-int-seq/keycode_events.snapshot
Original file line number Diff line number Diff line change
@@ -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