From 7b0af05c013a523bbf83db90d4034cac767ec1a9 Mon Sep 17 00:00:00 2001
From: Okke Formsma <okke@formsma.nl>
Date: Wed, 11 Nov 2020 16:40:13 +0100
Subject: [PATCH] fix momentary layer bug when top layer is not &trans

Key release events released keys on the wrong layer if the 'top layer'
was not &trans above the &mo key.

base    <&mo 1>
layer 1 <&kp B>

This was caused by overwriting
`zmk_keymap_active_behavior_layer[position]` after the &mo key was
handled.
---
 app/src/keymap.c                              | 10 ++---
 .../events.patterns                           |  0
 .../1-normal/keycode_events.snapshot          |  4 ++
 .../1-normal/native_posix.keymap              | 32 +++++++++++++++
 .../events.patterns                           |  0
 .../keycode_events.snapshot                   |  0
 .../2-early-key-release/native_posix.keymap   | 32 +++++++++++++++
 .../momentary-layer/3-covered/events.patterns |  3 ++
 .../3-covered/keycode_events.snapshot         |  2 +
 .../3-covered/native_posix.keymap             | 33 ++++++++++++++++
 .../momentary-layer/4-nested/events.patterns  |  3 ++
 .../4-nested/keycode_events.snapshot          |  6 +++
 .../4-nested/native_posix.keymap              | 39 +++++++++++++++++++
 .../events.patterns                           |  3 ++
 .../keycode_events.snapshot                   |  6 +++
 .../native_posix.keymap                       | 39 +++++++++++++++++++
 .../momentary-layer/behavior_keymap.dtsi      | 12 ++----
 .../early-key-release/native_posix.keymap     |  8 ----
 .../normal/keycode_events.snapshot            |  4 --
 .../normal/native_posix.keymap                |  8 ----
 20 files changed, 209 insertions(+), 35 deletions(-)
 rename app/tests/momentary-layer/{early-key-release => 1-normal}/events.patterns (100%)
 create mode 100644 app/tests/momentary-layer/1-normal/keycode_events.snapshot
 create mode 100644 app/tests/momentary-layer/1-normal/native_posix.keymap
 rename app/tests/momentary-layer/{normal => 2-early-key-release}/events.patterns (100%)
 rename app/tests/momentary-layer/{early-key-release => 2-early-key-release}/keycode_events.snapshot (100%)
 create mode 100644 app/tests/momentary-layer/2-early-key-release/native_posix.keymap
 create mode 100644 app/tests/momentary-layer/3-covered/events.patterns
 create mode 100644 app/tests/momentary-layer/3-covered/keycode_events.snapshot
 create mode 100644 app/tests/momentary-layer/3-covered/native_posix.keymap
 create mode 100644 app/tests/momentary-layer/4-nested/events.patterns
 create mode 100644 app/tests/momentary-layer/4-nested/keycode_events.snapshot
 create mode 100644 app/tests/momentary-layer/4-nested/native_posix.keymap
 create mode 100644 app/tests/momentary-layer/5-nested-early-key-release/events.patterns
 create mode 100644 app/tests/momentary-layer/5-nested-early-key-release/keycode_events.snapshot
 create mode 100644 app/tests/momentary-layer/5-nested-early-key-release/native_posix.keymap
 delete mode 100644 app/tests/momentary-layer/early-key-release/native_posix.keymap
 delete mode 100644 app/tests/momentary-layer/normal/keycode_events.snapshot
 delete mode 100644 app/tests/momentary-layer/normal/native_posix.keymap

diff --git a/app/src/keymap.c b/app/src/keymap.c
index 1d289e51..70db2fa5 100644
--- a/app/src/keymap.c
+++ b/app/src/keymap.c
@@ -130,14 +130,12 @@ int zmk_keymap_apply_position_state(int layer, u32_t position, bool pressed, s64
 }
 
 int zmk_keymap_position_state_changed(u32_t position, bool pressed, s64_t timestamp) {
+    if (pressed) {
+        zmk_keymap_active_behavior_layer[position] = zmk_keymap_layer_state;
+    }
     for (int layer = ZMK_KEYMAP_LAYERS_LEN - 1; layer >= zmk_keymap_layer_default; layer--) {
-        u32_t layer_state =
-            pressed ? zmk_keymap_layer_state : zmk_keymap_active_behavior_layer[position];
-        if (is_active_layer(layer, layer_state)) {
+        if (is_active_layer(layer, zmk_keymap_active_behavior_layer[position])) {
             int ret = zmk_keymap_apply_position_state(layer, position, pressed, timestamp);
-
-            zmk_keymap_active_behavior_layer[position] = zmk_keymap_layer_state;
-
             if (ret > 0) {
                 LOG_DBG("behavior processing to continue to next layer");
                 continue;
diff --git a/app/tests/momentary-layer/early-key-release/events.patterns b/app/tests/momentary-layer/1-normal/events.patterns
similarity index 100%
rename from app/tests/momentary-layer/early-key-release/events.patterns
rename to app/tests/momentary-layer/1-normal/events.patterns
diff --git a/app/tests/momentary-layer/1-normal/keycode_events.snapshot b/app/tests/momentary-layer/1-normal/keycode_events.snapshot
new file mode 100644
index 00000000..608ce093
--- /dev/null
+++ b/app/tests/momentary-layer/1-normal/keycode_events.snapshot
@@ -0,0 +1,4 @@
+mo_pressed: position 1 layer 1
+kp_pressed: usage_page 0x07 keycode 0x06 mods 0x00
+kp_released: usage_page 0x07 keycode 0x06 mods 0x00
+mo_released: position 1 layer 1
diff --git a/app/tests/momentary-layer/1-normal/native_posix.keymap b/app/tests/momentary-layer/1-normal/native_posix.keymap
new file mode 100644
index 00000000..2fc24d2b
--- /dev/null
+++ b/app/tests/momentary-layer/1-normal/native_posix.keymap
@@ -0,0 +1,32 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+#include "../behavior_keymap.dtsi"
+
+/ {
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&kp B &mo 1
+				&none &none>;
+		};
+
+		layer_1 {
+			bindings = <
+				&kp C &trans
+				&none &none>;
+		};
+	};
+};
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(0,1,10) 
+		ZMK_MOCK_PRESS(0,0,10) 
+		ZMK_MOCK_RELEASE(0,0,10) 
+		ZMK_MOCK_RELEASE(0,1,10)
+	>;
+};
\ No newline at end of file
diff --git a/app/tests/momentary-layer/normal/events.patterns b/app/tests/momentary-layer/2-early-key-release/events.patterns
similarity index 100%
rename from app/tests/momentary-layer/normal/events.patterns
rename to app/tests/momentary-layer/2-early-key-release/events.patterns
diff --git a/app/tests/momentary-layer/early-key-release/keycode_events.snapshot b/app/tests/momentary-layer/2-early-key-release/keycode_events.snapshot
similarity index 100%
rename from app/tests/momentary-layer/early-key-release/keycode_events.snapshot
rename to app/tests/momentary-layer/2-early-key-release/keycode_events.snapshot
diff --git a/app/tests/momentary-layer/2-early-key-release/native_posix.keymap b/app/tests/momentary-layer/2-early-key-release/native_posix.keymap
new file mode 100644
index 00000000..9ffa5f6e
--- /dev/null
+++ b/app/tests/momentary-layer/2-early-key-release/native_posix.keymap
@@ -0,0 +1,32 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+#include "../behavior_keymap.dtsi"
+
+/ {
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&kp B &mo 1
+				&none &none>;
+		};
+
+		layer_1 {
+			bindings = <
+				&kp C &none
+				&none &none>;
+		};
+	};
+};
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(0,0,10) 
+		ZMK_MOCK_PRESS(0,1,10) 
+		ZMK_MOCK_RELEASE(0,0,10) 
+		ZMK_MOCK_RELEASE(0,1,10)
+	>;
+};
\ No newline at end of file
diff --git a/app/tests/momentary-layer/3-covered/events.patterns b/app/tests/momentary-layer/3-covered/events.patterns
new file mode 100644
index 00000000..08b1e987
--- /dev/null
+++ b/app/tests/momentary-layer/3-covered/events.patterns
@@ -0,0 +1,3 @@
+s/.*hid_listener_keycode/kp/p
+s/.*mo_keymap_binding/mo/p
+s/.*keymap_position_state_changed/kp_st/p
\ No newline at end of file
diff --git a/app/tests/momentary-layer/3-covered/keycode_events.snapshot b/app/tests/momentary-layer/3-covered/keycode_events.snapshot
new file mode 100644
index 00000000..87d12817
--- /dev/null
+++ b/app/tests/momentary-layer/3-covered/keycode_events.snapshot
@@ -0,0 +1,2 @@
+mo_pressed: position 1 layer 1
+mo_released: position 1 layer 1
diff --git a/app/tests/momentary-layer/3-covered/native_posix.keymap b/app/tests/momentary-layer/3-covered/native_posix.keymap
new file mode 100644
index 00000000..2484d8b8
--- /dev/null
+++ b/app/tests/momentary-layer/3-covered/native_posix.keymap
@@ -0,0 +1,33 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+
+/*
+this test verifies that the correct key is released when a layer is enabled "on top"
+and the original key is "covered".
+*/
+/ {
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&trans &mo 1
+				&trans &trans>;
+		};
+
+		layer_1 {
+			bindings = <
+				&trans &kp A
+				&trans &trans>;
+		};
+	};
+};
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(0,1,10) 
+		ZMK_MOCK_RELEASE(0,1,10)
+	>;
+};
diff --git a/app/tests/momentary-layer/4-nested/events.patterns b/app/tests/momentary-layer/4-nested/events.patterns
new file mode 100644
index 00000000..08b1e987
--- /dev/null
+++ b/app/tests/momentary-layer/4-nested/events.patterns
@@ -0,0 +1,3 @@
+s/.*hid_listener_keycode/kp/p
+s/.*mo_keymap_binding/mo/p
+s/.*keymap_position_state_changed/kp_st/p
\ No newline at end of file
diff --git a/app/tests/momentary-layer/4-nested/keycode_events.snapshot b/app/tests/momentary-layer/4-nested/keycode_events.snapshot
new file mode 100644
index 00000000..f03e4d5b
--- /dev/null
+++ b/app/tests/momentary-layer/4-nested/keycode_events.snapshot
@@ -0,0 +1,6 @@
+mo_pressed: position 1 layer 1
+mo_pressed: position 0 layer 2
+kp_pressed: usage_page 0x07 keycode 0x05 mods 0x00
+kp_released: usage_page 0x07 keycode 0x05 mods 0x00
+mo_released: position 0 layer 2
+mo_released: position 1 layer 1
diff --git a/app/tests/momentary-layer/4-nested/native_posix.keymap b/app/tests/momentary-layer/4-nested/native_posix.keymap
new file mode 100644
index 00000000..0eb77d20
--- /dev/null
+++ b/app/tests/momentary-layer/4-nested/native_posix.keymap
@@ -0,0 +1,39 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+
+/ {
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&none &mo 1
+				&none &none>;
+		};
+
+		layer_1 {
+			bindings = <
+				&mo 2 &none
+				&none &none>;
+		};
+
+		layer_2 {
+			bindings = <
+				&none &none
+				&kp B &none>;
+		};
+	};
+};
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(0,1,10)
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(1,0,10)
+		ZMK_MOCK_RELEASE(1,0,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+		ZMK_MOCK_RELEASE(0,1,10)
+	>;
+};
\ No newline at end of file
diff --git a/app/tests/momentary-layer/5-nested-early-key-release/events.patterns b/app/tests/momentary-layer/5-nested-early-key-release/events.patterns
new file mode 100644
index 00000000..08b1e987
--- /dev/null
+++ b/app/tests/momentary-layer/5-nested-early-key-release/events.patterns
@@ -0,0 +1,3 @@
+s/.*hid_listener_keycode/kp/p
+s/.*mo_keymap_binding/mo/p
+s/.*keymap_position_state_changed/kp_st/p
\ No newline at end of file
diff --git a/app/tests/momentary-layer/5-nested-early-key-release/keycode_events.snapshot b/app/tests/momentary-layer/5-nested-early-key-release/keycode_events.snapshot
new file mode 100644
index 00000000..15be601e
--- /dev/null
+++ b/app/tests/momentary-layer/5-nested-early-key-release/keycode_events.snapshot
@@ -0,0 +1,6 @@
+mo_pressed: position 1 layer 1
+mo_pressed: position 0 layer 2
+kp_pressed: usage_page 0x07 keycode 0x05 mods 0x00
+mo_released: position 1 layer 1
+mo_released: position 0 layer 2
+kp_released: usage_page 0x07 keycode 0x05 mods 0x00
diff --git a/app/tests/momentary-layer/5-nested-early-key-release/native_posix.keymap b/app/tests/momentary-layer/5-nested-early-key-release/native_posix.keymap
new file mode 100644
index 00000000..a67035bf
--- /dev/null
+++ b/app/tests/momentary-layer/5-nested-early-key-release/native_posix.keymap
@@ -0,0 +1,39 @@
+#include <dt-bindings/zmk/keys.h>
+#include <behaviors.dtsi>
+#include <dt-bindings/zmk/kscan-mock.h>
+
+/ {
+	keymap {
+		compatible = "zmk,keymap";
+		label ="Default keymap";
+
+		default_layer {
+			bindings = <
+				&none &mo 1
+				&none &none>;
+		};
+
+		layer_1 {
+			bindings = <
+				&mo 2 &none
+				&none &none>;
+		};
+
+		layer_2 {
+			bindings = <
+				&none &none
+				&kp B &none>;
+		};
+	};
+};
+
+&kscan {
+	events = <
+		ZMK_MOCK_PRESS(0,1,10)
+		ZMK_MOCK_PRESS(0,0,10)
+		ZMK_MOCK_PRESS(1,0,10)
+		ZMK_MOCK_RELEASE(0,1,10)
+		ZMK_MOCK_RELEASE(0,0,10)
+		ZMK_MOCK_RELEASE(1,0,10)
+	>;
+};
\ No newline at end of file
diff --git a/app/tests/momentary-layer/behavior_keymap.dtsi b/app/tests/momentary-layer/behavior_keymap.dtsi
index 6cc3140f..a5b6c12b 100644
--- a/app/tests/momentary-layer/behavior_keymap.dtsi
+++ b/app/tests/momentary-layer/behavior_keymap.dtsi
@@ -10,19 +10,13 @@
 		default_layer {
 			bindings = <
 				&kp B &mo 1
-				&kp D &kp G>;
+				&trans &trans>;
 		};
 
-		lower_layer {
+		layer_1 {
 			bindings = <
 				&kp C_NEXT &trans
-				&kp L  &kp J>;
-		};
-
-		raise_layer {
-			bindings = <
-				&kp W &kp U
-				&kp X  &kp M>;
+				&trans &trans>;
 		};
 	};
 };
diff --git a/app/tests/momentary-layer/early-key-release/native_posix.keymap b/app/tests/momentary-layer/early-key-release/native_posix.keymap
deleted file mode 100644
index e7628c0e..00000000
--- a/app/tests/momentary-layer/early-key-release/native_posix.keymap
+++ /dev/null
@@ -1,8 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan-mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <ZMK_MOCK_PRESS(0,0,10) ZMK_MOCK_PRESS(0,1,10) ZMK_MOCK_RELEASE(0,0,10) ZMK_MOCK_RELEASE(0,1,10)>;
-};
\ No newline at end of file
diff --git a/app/tests/momentary-layer/normal/keycode_events.snapshot b/app/tests/momentary-layer/normal/keycode_events.snapshot
deleted file mode 100644
index 247128fe..00000000
--- a/app/tests/momentary-layer/normal/keycode_events.snapshot
+++ /dev/null
@@ -1,4 +0,0 @@
-mo_pressed: position 1 layer 1
-kp_pressed: usage_page 0x0c keycode 0xb5 mods 0x00
-kp_released: usage_page 0x0c keycode 0xb5 mods 0x00
-mo_released: position 1 layer 1
diff --git a/app/tests/momentary-layer/normal/native_posix.keymap b/app/tests/momentary-layer/normal/native_posix.keymap
deleted file mode 100644
index 7f736904..00000000
--- a/app/tests/momentary-layer/normal/native_posix.keymap
+++ /dev/null
@@ -1,8 +0,0 @@
-#include <dt-bindings/zmk/keys.h>
-#include <behaviors.dtsi>
-#include <dt-bindings/zmk/kscan-mock.h>
-#include "../behavior_keymap.dtsi"
-
-&kscan {
-	events = <ZMK_MOCK_PRESS(0,1,10) ZMK_MOCK_PRESS(0,0,10) ZMK_MOCK_RELEASE(0,0,10) ZMK_MOCK_RELEASE(0,1,10)>;
-};
\ No newline at end of file