feat(behaviors): Leader key #1380

nickconway · 2022-07-09T00:16:08Z

Adds leader key functionality

AlexCzar

Thank you so much, leader key is a very nice feature to have. Just a couple of questions/suggestions regarding the documentation.
Can't wait for this to be merged 👍🏽

AlexCzar · 2022-09-19T20:20:14Z

docs/docs/behaviors/leader-key.md

+
+#### `timerless`
+
+By default, the leader key will have a timeout, and will not wait for a sequence to be completed or another key to be pressed. Specify `timerless` if you want a timeout.


I struggle to understand this part

Where and how timerless should be specified? Is it a possible value for timeout-ms or is it a separate option alongside it?

Why is the opition called "timerless"? The statement says that we should specify it if we want a timeout, it might be a typo and you meant to write "don't want a timeout"? In the latter case, the naming is still a bit non-intuitive, maybe it should be replaced by no-timeout = <true>, or even removed in favour of timeout-ms = <0>; or <timeout-ms = <-1>, or, if timerless is a possible value for timeout-ms, maybe a shorter = <false> or even = <no> could be a better option? (shooting up the sky here).

Timerless would be a separate option, defined in the dts for the leader key along with timeout-ms and the other options.

Yeah, that was a typo, should be "don't want a timeout". Timeout-ms would need to be a positive number for the case of overlaps, unless we had something like overlap-timeout-ms and timeout-ms separate? Timerless is for when no sequence has been completed yet, and once an overlapped sequence is completed, timeout-ms is used.

AlexCzar · 2022-09-19T20:27:06Z

docs/docs/features/leader.md

+- `key-positions` is an array of key positions. See the info section below about how to figure out the positions on your board.
+- `layers = <0 1...>` will allow limiting a sequence to specific layers. This is an _optional_ parameter, when omitted it defaults to global scope.
+- `bindings` is the behavior that is activated when all the key positions are pressed.
+- (advanced) you can specify `immediate-trigger` if you want the sequence to be triggered as soon as all key positions are pressed. The default is to wait for the leader key's timeout to trigger the sequence if it overlaps another.


Where and how?

petejohanson

One important high level question on the design.

Thanks!

petejohanson · 2022-10-03T02:23:08Z

app/dts/behaviors/leader_key.dtsi

+
+ / {
+	behaviors {
+		/omit-if-no-ref/ leader: leader_key {


It seems to me we shouldn't limit this to just one global leader key, as this limits folks to just one leader key, and it's associated sequences.

Is there really no use case for different leader keys, with different possible sequences?

Seems like you could have child props in the yaml binding, and something like:

leader_foo: leader_foo { compatible = "zmk,behavior-leader"; ... seq1 { }; seq2 { }; };

That would give us a bit more flexibility.

If we really want to make it easy, we could still define one global leader you could enhance with children, e.g.:

&leader { seq1 { }; };

Right now I use multiple leader keys with layers (see here). I use macros to switch layers before pressing the leader key which has the same effect although is a bit more work for the user.

petejohanson · 2022-10-03T02:23:34Z

app/dts/bindings/behaviors/zmk,behavior-leader-key.yaml

+properties:
+  timerless:
+    type: boolean
+  timeout-ms:


This needs documenting as well. Also see the other reviewers question.

grezp · 2023-02-22T00:46:13Z

Any update on this branch merging to main?

vladsolokha · 2023-03-14T18:17:39Z

I don't understand the difference of this "leader key" and a sticky layer. Can someone differentiate?

cougarten · 2023-03-14T19:58:47Z

I don't understand the difference of this "leader key" and a sticky layer. Can someone differentiate?

you can do more than 2 key presses and it can chain arbitrary long sequences as the triggers.

e.g. "super > s > e" for "leader key" > "shortcuts" > "email" to trigger spelling your email address (or anything you want).

ujl123 · 2023-04-12T17:04:41Z

I don't understand the difference of this "leader key" and a sticky layer. Can someone differentiate?

Other uses I use leader keys in QMK is (all are for windows)

Leader Key + B O O T = qmk reset
Leader Key + R G B = toggle rgb
Leader Key + Q = Alt F4
Leader key + Z Z Z = system shutdown
Leader key + T O P = CTRL + SHIFT + ESC (task manager in windows)
Leader Key + MKDIR = CTRL + SHIFT + N (new folder)
Leader Key + RM = shift delete (hard delete in windows)
Leader KEy + PIN = windows pin (not recommended storing passwords on your keyboard but I'm lazy).

I'm hoping that this gets reviewed and can be merged into the main branch though as this is a major component that makes my 32 key keyboard work in QMK (along with tap mods, tap dances autoshifts, custom autoshift keys)

petejohanson

A few more items from my first pass at looking at this again. Thanks again for working on this!

petejohanson · 2023-07-07T04:40:29Z

app/src/leader.c

+#define LEADER_INST(n)                                                                             \
+    static struct leader_seq_cfg sequence_config_##n = {                                           \
+        .virtual_key_position = ZMK_KEYMAP_LEN + __COUNTER__,                                      \
+        .immediate_trigger = DT_PROP(n, immediate_trigger),                                        \
+        .is_pressed = false,                                                                       \
+        .key_positions = DT_PROP(n, key_positions),                                                \
+        .key_position_len = DT_PROP_LEN(n, key_positions),                                         \
+        .behavior = ZMK_KEYMAP_EXTRACT_BINDING(0, n),                                              \
+        .layers = DT_PROP(n, layers),                                                              \
+        .layers_len = DT_PROP_LEN(n, layers),                                                      \
+    };
+
+DT_INST_FOREACH_CHILD(0, LEADER_INST)


If we really are only supporting one instance of this node, then the FOREACH mechanics are superfluous and misleading. Just create one static struct instance directly.

petejohanson · 2023-07-07T04:41:11Z

app/src/leader.c

+
+static int leader_init() {
+    k_work_init_delayable(&release_timer, behavior_leader_key_timer_handler);
+    DT_INST_FOREACH_CHILD(0, INTITIALIAZE_LEADER_SEQUENCES);


Ditto, this adds unnecessary complexity since only a single instance of the node is supported.

petejohanson · 2023-07-07T04:41:36Z

app/src/leader.c

+bool leader_status;
+int32_t press_count;
+int32_t release_count;
+int32_t timeout_ms;
+int32_t active_leader_position;
+int8_t layer;
+bool first_release;
+struct k_work_delayable release_timer;
+int64_t release_at;
+bool timer_started;
+bool timer_cancelled;
+bool timerless;


These all need to be static.

petejohanson · 2023-07-07T04:43:52Z

app/include/zmk/leader.h

+
+void zmk_leader_activate(int32_t timeout, bool timeout_on_activation, uint32_t position);
+void zmk_leader_deactivate();
+bool zmk_leader_get_status();


status I don't love... is active? is enabled? Something a bit more aligned with the bool return value here.

petejohanson · 2023-07-07T04:44:39Z

app/src/behaviors/behavior_leader_key.c

+    const struct device *dev = device_get_binding(binding->behavior_dev);
+    const struct behavior_leader_key_config *cfg = dev->config;
+
+    zmk_leader_activate(cfg->timeout_ms, cfg->timerless, event.position);


If you press this again while it's active, should the behavior be to deactivate to interrupt it?

petejohanson · 2023-07-07T04:46:33Z

app/src/leader.c

+
+#define LEADER_INST(n)                                                                             \
+    static struct leader_seq_cfg sequence_config_##n = {                                           \
+        .virtual_key_position = ZMK_KEYMAP_LEN + __COUNTER__,                                      \


This should be refactored to use the virtual key position work @joelspadin added for combos, etc.

petejohanson · 2023-07-07T04:48:17Z

app/src/leader.c

+const struct zmk_position_state_changed
+    *leader_pressed_keys[CONFIG_ZMK_LEADER_MAX_KEYS_PER_SEQUENCE] = {NULL};
+
+uint32_t current_sequence[CONFIG_ZMK_LEADER_MAX_KEYS_PER_SEQUENCE] = {-1};
+// the set of candidate leader based on the currently leader_pressed_keys
+int num_candidates;
+struct leader_seq_cfg *sequence_candidates[CONFIG_ZMK_LEADER_MAX_SEQUENCES_PER_KEY];
+int num_comp_candidates;
+struct leader_seq_cfg *completed_sequence_candidates[CONFIG_ZMK_LEADER_MAX_SEQUENCES_PER_KEY];
+// a lookup dict that maps a key position to all sequences on that position
+struct leader_seq_cfg *sequence_lookup[ZMK_KEYMAP_LEN][CONFIG_ZMK_LEADER_MAX_SEQUENCES_PER_KEY] = {
+    NULL};


All should also be static

arabshapt · 2024-01-05T08:28:59Z

Please try to finish this feature... I will soon try to merge it on my branch, but it would be great to have it on master. I do not understand how people are not showing more interest for it. If it is easy to add key sequences(similar to vimrc) then it would be even better, but I think that can be done with a custom function... The main thing is having it on master and bugfree:) Thank you all but especially @nickconway and @petejohanson for working on it🙂

arabshapt · 2024-01-06T07:23:43Z

Is there any limitation regarding number of sequences that can be created? For combos there are (I think) 5 combos that can use the same key position. Could one create e.g. 900 sequences: leader + letter_1 + letter_2 = shortcut/macro?
So three key taps, around 900 unique sequences. Would it be possible with this implementation?

tvdab · 2024-02-25T21:37:12Z

I was a happy user of the leader key feature in my previous zmk build. Recently I tried updating my firmware to the latest release of zmk (c9c620d) and also update from zephyr sdk 0.15 to 0.16. Everything seems to work, except for the leader key. The leader key acts like a transparent key. Also running the test tests/leader/basic fails:

❯ west test tests/leader/basic
Running tests/leader/basic:
--- tests/leader/basic/keycode_events.snapshot	2024-02-25 22:11:10.516535082 +0100
+++ build/tests/leader/basic/keycode_events.log	2024-02-25 22:35:07.778445825 +0100
@@ -1,4 +1,2 @@
-leader: leader key activated
-pressed: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
-released: usage_page 0x07 keycode 0x04 implicit_mods 0x00 explicit_mods 0x00
-leader: leader key deactivated
+pressed: usage_page 0x07 keycode 0x1E implicit_mods 0x00 explicit_mods 0x00
+released: usage_page 0x07 keycode 0x1E implicit_mods 0x00 explicit_mods 0x00
FAILED: tests/leader/basic

Could someone verify if the leader key works with zephyr sdk 0.16 and a recent version of zmk? Maybe I'm doing something wrong...

caksoylar · 2024-02-25T22:25:19Z

I am guessing this needs to be rebased for #2028.

tvdab · 2024-03-30T14:59:18Z

I found why it wasn't working in Zephyr 3.5. This is what needs to be changed in order to make it work again:

--- a/app/src/behaviors/behavior_leader_key.c
+++ b/app/src/behaviors/behavior_leader_key.c
@@ -47,8 +47,8 @@ static const struct behavior_driver_api behavior_leader_key_driver_api = {
 #define LEAD_INST(n)                                                                               \
     static struct behavior_leader_key_config behavior_leader_key_config_##n = {                    \
         .timerless = DT_INST_PROP(n, timerless), .timeout_ms = DT_INST_PROP(n, timeout_ms)};       \
-    DEVICE_DT_INST_DEFINE(n, behavior_leader_key_init, NULL, NULL,                                 \
-                          &behavior_leader_key_config_##n, APPLICATION,                            \
+    BEHAVIOR_DT_INST_DEFINE(n, behavior_leader_key_init, NULL, NULL,                                 \
+                          &behavior_leader_key_config_##n, POST_KERNEL,                            \
                           CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, &behavior_leader_key_driver_api);

 DT_INST_FOREACH_STATUS_OKAY(LEAD_INST)

There are probably more changes to be made (#2028) to update everything to zephyr 3.5.

magoz · 2024-05-11T05:08:22Z

Looking forward to this, great work!

urob · 2024-09-11T14:56:49Z

I have been using this for a while now and really loving it! Just a few small comments to nitpick here:

Keycode vs position-based?

I see advantages for both sides. But overall I tend to think that this one might be more useful being keycode based.

The main reason is that in my experience leader sequences are almost always mnemonic (or are otherwise remembered in terms of their alphanumeric sequence). With this presumption, making them keycode based simplifies configuration and increases portability across layouts without much of a downside.

Purpose of `layer` option

I don't quite see the use case for this one. With the current position-based model, the sequence tracks every key press. So the active layer is a deterministic function of the layer on which &leader is placed and the sequence of keys, which can be matched against &leader_sequence. While it doesn't hurt to have the option, it might be unnecessary clutter?

(Similar, with a keycode-based model I can't really think of a use case either.)

Small bug?

Suppose I have two overlapping sequences:

<0> mapping to &kp A
<0 1> mapping to &kp B

Then the typing the sequence <0 2> within the leader term will eat up the sequence and not produce anything. I would expect it to match <0> and generate &kp A, and then terminate the processing of the sequence so that <2> can be processed by whatever behavior it is bound to.

nickconway force-pushed the leader-key branch 5 times, most recently from c986393 to ddbc05d Compare July 9, 2022 17:30

nickconway marked this pull request as ready for review July 9, 2022 19:01

nickconway changed the title ~~[WIP] feat(behaviors): Leader key~~ feat(behaviors): Leader key Jul 9, 2022

dxmh added enhancement New feature or request behaviors labels Jul 11, 2022

nickconway force-pushed the leader-key branch from b1cd1e8 to 6066fd2 Compare September 16, 2022 23:51

AlexCzar reviewed Sep 19, 2022

View reviewed changes

petejohanson reviewed Oct 3, 2022

View reviewed changes

petejohanson self-assigned this Oct 8, 2022

rmwphd mentioned this pull request Dec 15, 2022

Feature: Leader Key #82

Open

nickconway force-pushed the leader-key branch from e60c2e7 to f3edd06 Compare June 3, 2023 00:52

petejohanson requested changes Jul 7, 2023

View reviewed changes

nickconway force-pushed the leader-key branch from 2d761ea to 32ecdb9 Compare April 14, 2024 15:32

nickconway requested a review from a team as a code owner April 14, 2024 15:32

j-w-e added a commit to j-w-e/zmk that referenced this pull request Jun 5, 2024

Leader key, zmkfirmware#1380

16b6c4c

hendstephen mentioned this pull request Jul 18, 2024

Leader key (PR 1380) hendstephen/zmk#1

Merged

nickconway added 3 commits August 17, 2024 21:20

Leader key

bbd888e

Fix leader overlapping with timerless

aab348a

Update for 3.5 and refactor

d3cdc59

nickconway force-pushed the leader-key branch from 85a8c8f to d3cdc59 Compare August 18, 2024 01:23

nickconway requested a review from a team as a code owner August 18, 2024 01:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(behaviors): Leader key #1380

feat(behaviors): Leader key #1380

nickconway commented Jul 9, 2022

AlexCzar left a comment

AlexCzar Sep 19, 2022

nickconway Oct 18, 2022

AlexCzar Sep 19, 2022

petejohanson left a comment

petejohanson Oct 3, 2022

nickconway Oct 18, 2022

petejohanson Oct 3, 2022

grezp commented Feb 22, 2023

vladsolokha commented Mar 14, 2023

cougarten commented Mar 14, 2023

ujl123 commented Apr 12, 2023 •

edited

Loading

petejohanson left a comment

petejohanson Jul 7, 2023

petejohanson Jul 7, 2023

petejohanson Jul 7, 2023

petejohanson Jul 7, 2023

petejohanson Jul 7, 2023

petejohanson Jul 7, 2023

petejohanson Jul 7, 2023

arabshapt commented Jan 5, 2024

arabshapt commented Jan 6, 2024

tvdab commented Feb 25, 2024 •

edited

Loading

caksoylar commented Feb 25, 2024

tvdab commented Mar 30, 2024

magoz commented May 11, 2024

urob commented Sep 11, 2024 •

edited

Loading


		#### `timerless`

		By default, the leader key will have a timeout, and will not wait for a sequence to be completed or another key to be pressed. Specify `timerless` if you want a timeout.

feat(behaviors): Leader key #1380

Are you sure you want to change the base?

feat(behaviors): Leader key #1380

Conversation

nickconway commented Jul 9, 2022

AlexCzar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petejohanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grezp commented Feb 22, 2023

vladsolokha commented Mar 14, 2023

cougarten commented Mar 14, 2023

ujl123 commented Apr 12, 2023 • edited Loading

petejohanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arabshapt commented Jan 5, 2024

arabshapt commented Jan 6, 2024

tvdab commented Feb 25, 2024 • edited Loading

caksoylar commented Feb 25, 2024

tvdab commented Mar 30, 2024

magoz commented May 11, 2024

urob commented Sep 11, 2024 • edited Loading

Keycode vs position-based?

Purpose of layer option

Small bug?

ujl123 commented Apr 12, 2023 •

edited

Loading

tvdab commented Feb 25, 2024 •

edited

Loading

urob commented Sep 11, 2024 •

edited

Loading

Purpose of `layer` option