summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
authorJonathan Rascher <jon@bcat.name>2021-06-08 10:56:02 -0500
committerPete Johanson <peter@peterjohanson.com>2021-06-08 20:35:58 -0400
commit4e69a32103a2905b9a32c80c8d3d798fbb0d9a0f (patch)
tree28b2ddfdbf23187ba25fe345c8053929d2fef9f4 /app
parenteecc12c98022c1dce4a228914887ed328b1774c2 (diff)
fix(combos): Check each combo key, not just last
The current combo completion check only makes sure the last key in the combo is set. This works when the combo is typed correctly initially, or when reraising events in a combo of length two. However, it fails for longer combos since the last event in pressed_keys might be set, but the first (or subsequent) event in pressed_keys can be NULL thanks to release_pressed_keys. Also added a regression test.
Diffstat (limited to 'app')
-rw-r--r--app/src/combo.c13
-rw-r--r--app/tests/combo/press-release-long-combo-wrong-last-key/events.patterns1
-rw-r--r--app/tests/combo/press-release-long-combo-wrong-last-key/keycode_events.snapshot6
-rw-r--r--app/tests/combo/press-release-long-combo-wrong-last-key/native_posix.keymap37
4 files changed, 54 insertions, 3 deletions
diff --git a/app/src/combo.c b/app/src/combo.c
index 72535b2..16091cd 100644
--- a/app/src/combo.c
+++ b/app/src/combo.c
@@ -188,8 +188,15 @@ static int64_t first_candidate_timeout() {
static inline bool candidate_is_completely_pressed(struct combo_cfg *candidate) {
// this code assumes set(pressed_keys) <= set(candidate->key_positions)
// this invariant is enforced by filter_candidates
- // the only thing we need to do is check if len(pressed_keys) == len(combo->key_positions)
- return pressed_keys[candidate->key_position_len - 1] != NULL;
+ // since events may have been reraised after clearing one or more slots at
+ // the start of pressed_keys (see: release_pressed_keys), we have to check
+ // that each key needed to trigger the combo was pressed, not just the last.
+ for (int i = 0; i < candidate->key_position_len; i++) {
+ if (pressed_keys[i] == NULL) {
+ return false;
+ }
+ }
+ return true;
}
static int cleanup();
@@ -496,4 +503,4 @@ static int combo_init() {
SYS_INIT(combo_init, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);
-#endif \ No newline at end of file
+#endif
diff --git a/app/tests/combo/press-release-long-combo-wrong-last-key/events.patterns b/app/tests/combo/press-release-long-combo-wrong-last-key/events.patterns
new file mode 100644
index 0000000..b1342af
--- /dev/null
+++ b/app/tests/combo/press-release-long-combo-wrong-last-key/events.patterns
@@ -0,0 +1 @@
+s/.*hid_listener_keycode_//p
diff --git a/app/tests/combo/press-release-long-combo-wrong-last-key/keycode_events.snapshot b/app/tests/combo/press-release-long-combo-wrong-last-key/keycode_events.snapshot
new file mode 100644
index 0000000..d1b9db9
--- /dev/null
+++ b/app/tests/combo/press-release-long-combo-wrong-last-key/keycode_events.snapshot
@@ -0,0 +1,6 @@
+pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
+pressed: usage_page 0x07 keycode 0x05 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
+released: usage_page 0x07 keycode 0x05 implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
diff --git a/app/tests/combo/press-release-long-combo-wrong-last-key/native_posix.keymap b/app/tests/combo/press-release-long-combo-wrong-last-key/native_posix.keymap
new file mode 100644
index 0000000..b811718
--- /dev/null
+++ b/app/tests/combo/press-release-long-combo-wrong-last-key/native_posix.keymap
@@ -0,0 +1,37 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+
+/ {
+ combos {
+ compatible = "zmk,combos";
+ combo_one {
+ timeout-ms = <80>;
+ key-positions = <0 1 2>;
+ bindings = <&kp Z>;
+ };
+ };
+
+ keymap {
+ compatible = "zmk,keymap";
+ label ="Default keymap";
+
+ default_layer {
+ bindings = <
+ &kp A &kp B
+ &kp C &kp D
+ >;
+ };
+ };
+};
+
+&kscan {
+ events = <
+ ZMK_MOCK_PRESS(0,0,10)
+ ZMK_MOCK_PRESS(0,1,10)
+ ZMK_MOCK_PRESS(1,1,10)
+ ZMK_MOCK_RELEASE(1,1,100)
+ ZMK_MOCK_RELEASE(0,1,100)
+ ZMK_MOCK_RELEASE(0,0,100)
+ >;
+};