fix(keymaps): Handle matching `then-layer`s.

* Proporly handle multiple conditonal layers w/ the same target `then-layer` values. * Move handling to work callback, to avoid re-entrance for cascading layers enabling other layers.
3 years ago · 4af3d272fc
10 changed files with 298 additions and 13 deletions
--- a/app/src/conditional_layer.c
+++ b/app/src/conditional_layer.c
@ -7,6 +7,7 @@
				@@ -7,6 +7,7 @@
 #define DT_DRV_COMPAT zmk_conditional_layers

 #include <stdint.h>
+#include <kernel.h>

 #include <devicetree.h>
 #include <logging/log.h>
@ -19,6 +20,8 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
				@@ -19,6 +20,8 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);

 #if DT_HAS_COMPAT_STATUS_OKAY(DT_DRV_COMPAT)

+static K_SEM_DEFINE(conditional_layer_sem, 1, 1);
+
 // Conditional layer configuration that activates the specified then-layer when all if-layers are
 // active. With two if-layers, this is referred to as "tri-layer", and is commonly used to activate
 // a third "adjust" layer if and only if the "lower" and "raise" layers are both active.
@ -66,22 +69,53 @@ static void conditional_layer_deactivate(int8_t layer) {
				@@ -66,22 +69,53 @@ static void conditional_layer_deactivate(int8_t layer) {
    }
 }

-// On layer state changes, examines each conditional layer config to determine if then-layer in the
-// config should activate based on the currently active set of if-layers.
 static int layer_state_changed_listener(const zmk_event_t *ev) {
-    for (int i = 0; i < NUM_CONDITIONAL_LAYER_CFGS; i++) {
-        const struct conditional_layer_cfg *cfg = CONDITIONAL_LAYER_CFGS + i;
-        zmk_keymap_layers_state_t mask = cfg->if_layers_state_mask;
-
-        // Activate then-layer if and only if all if-layers are already active. Note that we
-        // reevaluate the current layer state for each config since activation of one layer can also
-        // trigger activation of another.
-        if ((zmk_keymap_layer_state() & mask) == mask) {
-            conditional_layer_activate(cfg->then_layer);
-        } else {
-            conditional_layer_deactivate(cfg->then_layer);
+    static bool conditional_layer_updates_needed;
+
+    conditional_layer_updates_needed = true;
+
+    // Semaphore ensures we don't re-enter the loop in the middle of doing update, and
+    // ensures that "waterfalling layer updates" are all processed to trigger subsequent
+    // nested conditional layers properly.
+    if (k_sem_take(&conditional_layer_sem, K_NO_WAIT) < 0) {
+        return 0;
+    }
+
+    while (conditional_layer_updates_needed) {
+        int8_t max_then_layer = -1;
+        uint32_t then_layers = 0;
+        uint32_t then_layer_state = 0;
+
+        conditional_layer_updates_needed = false;
+
+        // On layer state changes, examines each conditional layer config to determine if then-layer
+        // in the config should activate based on the currently active set of if-layers.
+        for (int i = 0; i < NUM_CONDITIONAL_LAYER_CFGS; i++) {
+            const struct conditional_layer_cfg *cfg = CONDITIONAL_LAYER_CFGS + i;
+            zmk_keymap_layers_state_t mask = cfg->if_layers_state_mask;
+            then_layers |= BIT(cfg->then_layer);
+            max_then_layer = MAX(max_then_layer, cfg->then_layer);
+
+            // Activate then-layer if and only if all if-layers are already active. Note that we
+            // reevaluate the current layer state for each config since activation of one layer can
+            // also trigger activation of another.
+            if ((zmk_keymap_layer_state() & mask) == mask) {
+                then_layer_state |= BIT(cfg->then_layer);
+            }
+        }
+
+        for (uint8_t layer = 0; layer <= max_then_layer; layer++) {
+            if ((BIT(layer) & then_layers) != 0U) {
+                if ((BIT(layer) & then_layer_state) != 0U) {
+                    conditional_layer_activate(layer);
+                } else {
+                    conditional_layer_deactivate(layer);
+                }
+            }
        }
    }
+
+    k_sem_give(&conditional_layer_sem);
    return 0;
 }

--- a/app/tests/conditional-layer/same-layer-reached-both-ways/events.patterns
+++ b/app/tests/conditional-layer/same-layer-reached-both-ways/events.patterns
@ -0,0 +1,3 @@
				@@ -0,0 +1,3 @@
+s/.*hid_listener_keycode/kp/p
+s/.*mo_keymap_binding/mo/p
+s/.*conditional_layer/cl/p
--- a/app/tests/conditional-layer/same-layer-reached-both-ways/keycode_events.snapshot
+++ b/app/tests/conditional-layer/same-layer-reached-both-ways/keycode_events.snapshot
@ -0,0 +1,20 @@
				@@ -0,0 +1,20 @@
+mo_pressed: position 2 layer 1
+mo_pressed: position 3 layer 2
+cl_activate: layer 4
+mo_pressed: position 1 layer 3
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+mo_released: position 1 layer 3
+mo_released: position 3 layer 2
+cl_deactivate: layer 4
+mo_released: position 2 layer 1
+mo_pressed: position 1 layer 3
+mo_pressed: position 2 layer 1
+cl_activate: layer 4
+mo_pressed: position 3 layer 2
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+mo_released: position 3 layer 2
+mo_released: position 2 layer 1
+cl_deactivate: layer 4
+mo_released: position 1 layer 3
--- a/app/tests/conditional-layer/same-layer-reached-both-ways/native_posix_64.keymap
+++ b/app/tests/conditional-layer/same-layer-reached-both-ways/native_posix_64.keymap
@ -0,0 +1,73 @@
				@@ -0,0 +1,73 @@
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/keys.h>
+#include <dt-bindings/zmk/kscan_mock.h>
+
+/ {
+    conditional_layers {
+        compatible = "zmk,conditional-layers";
+        conditional_layer_1 {
+            if-layers = <1 2>;
+            then-layer = <4>;
+        };
+        conditional_layer_2 {
+            if-layers = <1 3>;
+            then-layer = <4>;
+        };
+    };
+
+    keymap {
+        compatible = "zmk,keymap";
+        default_layer {
+            bindings = <
+                &kp A &mo 3
+                &mo 1 &mo 2
+            >;
+        };
+        layer_1 {
+            bindings = <
+                &kp B &trans
+                &trans &trans
+            >;
+        };
+        layer_2 {
+            bindings = <
+                &kp C &trans
+                &trans &trans
+            >;
+        };
+        layer_3 {
+            bindings = <
+                &kp D &trans
+                &trans &trans
+            >;
+        };
+        layer_4 {
+            bindings = <
+                &kp E &trans
+                &trans &trans
+            >;
+        };
+    };
+};
+
+&kscan {
+    events = <
+        ZMK_MOCK_PRESS(1,0,10)
+        ZMK_MOCK_PRESS(1,1,10)
+        ZMK_MOCK_PRESS(0,1,10)
+        ZMK_MOCK_PRESS(0,0,10)
+        ZMK_MOCK_RELEASE(0,0,10)
+        ZMK_MOCK_RELEASE(0,1,10)
+        ZMK_MOCK_RELEASE(1,1,10)
+        ZMK_MOCK_RELEASE(1,0,10)
+
+        ZMK_MOCK_PRESS(0,1,10)
+        ZMK_MOCK_PRESS(1,0,10)
+        ZMK_MOCK_PRESS(1,1,10)
+        ZMK_MOCK_PRESS(0,0,10)
+        ZMK_MOCK_RELEASE(0,0,10)
+        ZMK_MOCK_RELEASE(1,1,10)
+        ZMK_MOCK_RELEASE(1,0,10)
+        ZMK_MOCK_RELEASE(0,1,10)
+    >;
+};
--- a/app/tests/conditional-layer/same-layer-reached-differently/events.patterns
+++ b/app/tests/conditional-layer/same-layer-reached-differently/events.patterns
@ -0,0 +1,3 @@
				@@ -0,0 +1,3 @@
+s/.*hid_listener_keycode/kp/p
+s/.*mo_keymap_binding/mo/p
+s/.*conditional_layer/cl/p
--- a/app/tests/conditional-layer/same-layer-reached-differently/keycode_events.snapshot
+++ b/app/tests/conditional-layer/same-layer-reached-differently/keycode_events.snapshot
@ -0,0 +1,16 @@
				@@ -0,0 +1,16 @@
+mo_pressed: position 2 layer 1
+mo_pressed: position 3 layer 2
+cl_activate: layer 4
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+mo_released: position 3 layer 2
+cl_deactivate: layer 4
+mo_released: position 2 layer 1
+mo_pressed: position 1 layer 3
+mo_pressed: position 2 layer 1
+cl_activate: layer 4
+kp_pressed: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x08 implicit_mods 0x00 explicit_mods 0x00
+mo_released: position 2 layer 1
+cl_deactivate: layer 4
+mo_released: position 1 layer 3
--- a/app/tests/conditional-layer/same-layer-reached-differently/native_posix_64.keymap
+++ b/app/tests/conditional-layer/same-layer-reached-differently/native_posix_64.keymap
@ -0,0 +1,69 @@
				@@ -0,0 +1,69 @@
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/keys.h>
+#include <dt-bindings/zmk/kscan_mock.h>
+
+/ {
+    conditional_layers {
+        compatible = "zmk,conditional-layers";
+        conditional_layer_1 {
+            if-layers = <1 2>;
+            then-layer = <4>;
+        };
+        conditional_layer_2 {
+            if-layers = <1 3>;
+            then-layer = <4>;
+        };
+    };
+
+    keymap {
+        compatible = "zmk,keymap";
+        default_layer {
+            bindings = <
+                &kp A &mo 3
+                &mo 1 &mo 2
+            >;
+        };
+        layer_1 {
+            bindings = <
+                &kp B &trans
+                &trans &trans
+            >;
+        };
+        layer_2 {
+            bindings = <
+                &kp C &trans
+                &trans &trans
+            >;
+        };
+        layer_3 {
+            bindings = <
+                &kp D &trans
+                &trans &trans
+            >;
+        };
+        layer_4 {
+            bindings = <
+                &kp E &trans
+                &trans &trans
+            >;
+        };
+    };
+};
+
+&kscan {
+    events = <
+        ZMK_MOCK_PRESS(1,0,10)
+        ZMK_MOCK_PRESS(1,1,10)
+        ZMK_MOCK_PRESS(0,0,10)
+        ZMK_MOCK_RELEASE(0,0,10)
+        ZMK_MOCK_RELEASE(1,1,10)
+        ZMK_MOCK_RELEASE(1,0,10)
+
+        ZMK_MOCK_PRESS(0,1,10)
+        ZMK_MOCK_PRESS(1,0,10)
+        ZMK_MOCK_PRESS(0,0,10)
+        ZMK_MOCK_RELEASE(0,0,10)
+        ZMK_MOCK_RELEASE(1,0,10)
+        ZMK_MOCK_RELEASE(0,1,10)
+    >;
+};
--- a/app/tests/conditional-layer/tri-layer-lt/events.patterns
+++ b/app/tests/conditional-layer/tri-layer-lt/events.patterns
@ -0,0 +1,3 @@
				@@ -0,0 +1,3 @@
+s/.*hid_listener_keycode/kp/p
+s/.*mo_keymap_binding/mo/p
+s/.*conditional_layer/cl/p
--- a/app/tests/conditional-layer/tri-layer-lt/keycode_events.snapshot
+++ b/app/tests/conditional-layer/tri-layer-lt/keycode_events.snapshot
@ -0,0 +1,8 @@
				@@ -0,0 +1,8 @@
+mo_pressed: position 2 layer 1
+mo_pressed: position 3 layer 2
+cl_activate: layer 3
+kp_pressed: usage_page 0x07 keycode 0x0A implicit_mods 0x00 explicit_mods 0x00
+kp_released: usage_page 0x07 keycode 0x0A implicit_mods 0x00 explicit_mods 0x00
+mo_released: position 3 layer 2
+cl_deactivate: layer 3
+mo_released: position 2 layer 1
--- a/app/tests/conditional-layer/tri-layer-lt/native_posix_64.keymap
+++ b/app/tests/conditional-layer/tri-layer-lt/native_posix_64.keymap
@ -0,0 +1,56 @@
				@@ -0,0 +1,56 @@
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/keys.h>
+#include <dt-bindings/zmk/kscan_mock.h>
+
+&lt {
+    flavor = "balanced";
+};
+
+/ {
+    conditional_layers {
+        compatible = "zmk,conditional-layers";
+        tri_layer {
+            if-layers = <1 2>;
+            then-layer = <3>;
+        };
+    };
+
+    keymap {
+        compatible = "zmk,keymap";
+        default_layer {
+            bindings = <
+                &kp A &kp B
+                &lt 1 I &lt 2 J
+            >;
+        };
+        layer_1 {
+            bindings = <
+                &kp C &kp D
+                &trans &trans
+            >;
+        };
+        layer_2 {
+            bindings = <
+                &kp E &kp F
+                &trans &trans
+            >;
+        };
+        layer_3 {
+            bindings = <
+                &kp G &kp H
+                &trans &trans
+            >;
+        };
+    };
+};
+
+&kscan {
+    events = <
+        ZMK_MOCK_PRESS(1,0,10)
+        ZMK_MOCK_PRESS(1,1,10)
+        ZMK_MOCK_PRESS(0,0,10)
+        ZMK_MOCK_RELEASE(0,0,10)
+        ZMK_MOCK_RELEASE(1,1,10)
+        ZMK_MOCK_RELEASE(1,0,10)
+    >;
+};