Macros #685

okke-formsma · 2021-02-18T13:06:58Z

This builds on #170.

I think "macro" is a better name than "simple macro", refactored the code and updated to the latest ZMK API.

petejohanson

A few questions. Thanks!

app/src/behaviors/behavior_macro.c

petejohanson · 2021-02-18T13:13:50Z

app/src/behaviors/behavior_macro.c

+            break;
+        }
+        behavior_keymap_binding_pressed(&cfg->behaviors[index], event);
+        k_msleep(cfg->sleep);


This theoretically ties up the system work queue/thread, especially for longer macros.. how well did this work in real life for longer ones? Dropped messages for the HOG queue?

I haven't tested it on long macros nor in real life.

These macros will not permit any other keyboard interaction until they have finished executing. If that means some queues get full, I suggest people increase their queues as a workaround?

I think a more robust solution for longer macros would be for to use delayed work items. https://docs.zephyrproject.org/1.9.0/kernel/threads/workqueues.html#submitting-a-work-item

I think the main changes here would be:

k_delayed_work_init a new item per macro. As shown in the link above, you can attach some state to the work item and give it the behavior_macro_config* and current index.

Pull the code inside the loop out into the work item handler function.

To start the macro, reset its index to 0 and k_delayed_work_submit the work item (probably with 0 delay).

Replace k_msleep with k_delayed_work_submit and continue to submit the same item until everything in the macro has been run.

This would allow for other keyboard interaction while a macro is running though, which might or might not be what you want (possibly on a per-macro basis?).

If we wanted to support both short macros that tie up the whole keyboard until they're done (to prevent input from conflicting with the macro) and longer macros that do not, we could add something like an exclusive property to the macro which selects between loop and sleep (exclusive) and work queue (non exclusive) execution.

joelspadin · 2021-02-19T15:13:01Z

app/src/behaviors/behavior_macro.c

+            break;
+        }
+        behavior_keymap_binding_pressed(&cfg->behaviors[index], event);
+        k_msleep(cfg->sleep);


I think a more robust solution for longer macros would be for to use delayed work items. https://docs.zephyrproject.org/1.9.0/kernel/threads/workqueues.html#submitting-a-work-item

I think the main changes here would be:

k_delayed_work_init a new item per macro. As shown in the link above, you can attach some state to the work item and give it the behavior_macro_config* and current index.

Pull the code inside the loop out into the work item handler function.

To start the macro, reset its index to 0 and k_delayed_work_submit the work item (probably with 0 delay).

Replace k_msleep with k_delayed_work_submit and continue to submit the same item until everything in the macro has been run.

This would allow for other keyboard interaction while a macro is running though, which might or might not be what you want (possibly on a per-macro basis?).

If we wanted to support both short macros that tie up the whole keyboard until they're done (to prevent input from conflicting with the macro) and longer macros that do not, we could add something like an exclusive property to the macro which selects between loop and sleep (exclusive) and work queue (non exclusive) execution.

joelspadin · 2021-02-19T15:49:11Z

app/dts/bindings/behaviors/zmk,behavior-macro.yaml

+
+include: zero_param.yaml
+
+properties:


It looks like this system can only press and release keys in order on key up/down or press all the keys in order and hold them until you release the key?

Just going by the macro systems I've seen in other PC software, I'm not sure this will do everything people would want to do with it. (Either that or I don't understand how it works and this needs better documentation.)

The main actions I usually see in macro systems are:

Press key

Release key

Press and release key

Delay for arbitrary time

It would be nice to be able to write things like

// This macro probably isn't useful, but something similar to it might be. // Alt+tab forward two windows, then dock the window to the left. bindings = <&down LALT>, <&kp TAB>, <&kp TAB>, <&up LALT>, <&kp LGUI(LEFT)>;

for scenarios where you want to keep one key held while pressing others.

Manually controlling the delays between items could be especially useful in macros for games. For example:

// Automate something I'm too bad at this game to do. sleep = 0, bindings = <&down DOWN>, <&wait 17>, <&down RIGHT>, <&wait 17>, <&up DOWN>, <&wait 17>, <&up RIGHT>, <&down Z>, <&wait 33>, <&up Z>, <&wait 134>, <&down X>, <&down C>, <&wait 33>, <&up X>, <&up C>;

That's a great idea. We can actually implement &down and &up and &wait independently from this PR.

I agree we should remove the delay that now exists between separate keypresses so people can set up their own &waits.

More macro ideas:

repeat boolean. If it's on, the macro repeats forever while it's held down.

If you do remove the delay between key presses, note that &kp will probably still need a delay between press and release. Maybe there should still be a configurable duration for &kp and then anything else you do with wait commands?

After thinking about this some more, having a special case to put a delay between press and release for just &kp seems weird. What if instead we added a duration value to each item, and then made macro execution work like this?

Press item

Wait item's duration

Release item

Move to next item and immediately return to step 1

Then for a delay between items you could just use &none with the duration you want to delay by.

I need to read up more on how Zephyr handles lists of DT things, so this might not be possible or there might be a better way to do it, but maybe we could add a special binding which the macro code looks for, and instead of pressing/releasing it, it uses its parameter to determine how long the following item should be held? If an item is not preceded by a duration, use something like the sleep property as a default (and default that property to 0).

default-duration = <10>; bindings = <&for 0 &down LALT>, <&kp TAB>, <&for 30 &none>. <&kp TAB>, <&for 0 &up LALT>, <&kp LGUI(LEFT)>;

Alternatively (and much more awkwardly), we could have parallel arrays of bindings and durations:

bindings = <&down LALT>, <kp TAB>, <&none>, <&kp TAB>, <&up LALT>, <&kp LGUI(LEFT)>; duration = <0>, <10>, <30>, <10>, <0>, <10>;

I don't see an issue with making a special case for &press and &release. &wait could be interpreted by the macro code to introduce a delayed work item, it doesn't have to sleep and block the main thread.

Devicetree doesn't allow nesting like &one &two; you can't give references as arguments. A &for as you suggested could be created (the syntax would even work), but it would have to be interpreted by the macro to override the next keypress-duration.

(this is a response to joelspadin)

Devicetree doesn't allow nesting like &one &two; you can't give references as arguments.

Macros are parameterless behaviors. It was tested here: https://github.com/BrainWart/zmk-kyria-keymap/blob/simple-macro/config/kyria.keymap

@BrainWart I like the idea. I think we can achieve the same without having to resort to a special keymp syntax just for macros:

gaming-macro? { macro = <&press DOWN>, <&wait 17>, <&press RIGHT>, <&wait 17>, <&release DOWN>, <&wait 17>, <&release RIGHT>, <&press Z>, <&wait 33>, <&release Z>, <&wait 134>, <&press X>, <&press C>, <&wait 33>, <&release X>, <&release C>; }

This could work just fine.

If I correctly understand that you mean &press, &release, and &wait would be special things interpreted only by macros and not regular behaviors that you could also stick into a keymap, I think that would work well. You could also C macro your ZMK macros to get something like &kp:

#define MACRO_KP(key, duration) <&press (key)>, <&wait (duration)>, <&release (key)>

That does mean that you wouldn't be able to do things like put an RGB underglow command in the middle of a macro though, but I'm not sure how useful that would be.

The thing I mainly didn't like about the previous proposal was that it was taking regular keymap behaviors but then needing to layer special macro things on top of every behavior. e.g. &kp should be pressed, wait for X milliseconds, then released, &press and &release should just be pressed (and maybe immediately released?) and continue with no delay, &wait isn't pressed at all, and I have no idea what should happen if you use a &mt...

I vote for either:

Macros use their own set of commands that are independent from keymap behaviors.

Macros use keymap behaviors, but we always execute each behavior the same way (press, wait, release) and have some means to control how long each one is held.

One of the strenghts of ZMK is the flexible keymap system using behaviors and devicetree bindings. Whatever we do with macros, we should support any and all behaviors from macros. This means option 1 (macros use their own set of commands) is a no-go in my opinion.

In option 2 you state that 'we always execute each behavior the same way (press, wait, release)'. I think we can make that work with an additional rule: wait is only executed when a behavior defines both the 'press' and 'release' bindings. Consider &press which is a behavior that only has a "press" binding, and &release, a behavior that only defines a "release" binding. This allows the user to fully in control timing when using &press and &release, and also any new (user-defined) behaviors in the future.

The &wait must be a little special, when the macro encounters a &wait it should not call this behavior but only execute a "wait" with the specified delay. I don't see any way to solve this wait within a behavior without blocking the main thread.

to summarize, I think we should:

leverage the power of the devicetree in macros, which lets us use all existing behaviors

add behaviors like &press and &release which could work inside and outside of macros

add a behavior like &wait which is a no-op in a normal keymap but introduces a wait inside a macro

(please don't reply to this message in this PR but move the conversation to #689)

okke-formsma · 2021-02-19T19:32:38Z

I'm a big fan of all the ideas for the macros. To keep a bit of velocity I'd like to keep the scope of this PR limited to the current functionality. I propose to track the suggestions in a new ticket and want to invite others to build on this base. :)

okke-formsma · 2021-03-07T14:33:16Z

If noone has major issues with this (v1!) PR I'd like to get it merged.

Otherwise I'll close this PR and make room for someone else to implement macros.

joelspadin · 2021-03-07T18:32:11Z

app/src/behaviors/behavior_macro.c

+                                     struct zmk_behavior_binding_event event) {
+    const struct device *dev = device_get_binding(binding->behavior_dev);
+    const struct behavior_macro_config *cfg = dev->config;
+    if (cfg->mode == MODE_KEY_DOWN) {


When I was first reviewing this, it was difficult for me to understand what it was doing and how it worked. I think that is mainly because of the way it does a completely different thing between hold and up/down modes.

It feels like these are conceptually two different things, and the only code they really share is the DT binding handling. I wonder if it would make more sense to split them into separate behaviors?

macro: Press and release a bunch of things in sequence when I press a key (with option to trigger on release instead)

multi(?): Press a bunch of things (optionally staggered with a delay) and hold them until I release the key, then release them in the same order

#689 seems like it would extend/replace the first but not the second.

joelspadin · 2021-03-07T18:36:06Z

app/src/behaviors/behavior_macro.c

+            break;
+        }
+        behavior_keymap_binding_released(&cfg->behaviors[index], event);
+        k_msleep(cfg->sleep);


Is there any reason we need to put delays between releasing the keys here? I can understand why you'd want to put a delay to ensure things are pressed in the same order, but I can't think of any reason the releases would need to be staggered.

joelspadin · 2021-03-07T18:41:15Z

app/src/behaviors/behavior_macro.c

+            break;
+        }
+        behavior_keymap_binding_pressed(&cfg->behaviors[index], event);
+        k_msleep(cfg->sleep);


In the event that you set sleep = 0 to press multiple things at the same time, I don't think we should call k_msleep. A sleep of 0 may be treated as a yield (see zephyr/kernel/sched.c z_tick_sleep())

joelspadin · 2021-03-07T18:51:39Z

If noone has major issues with this (v1!) PR I'd like to get it merged.

Otherwise I'll close this PR and make room for someone else to implement macros.

The only issue I see here is it feels like you're implementing two different concepts with a single behavior. See #685 (comment)

(Would also be nice to avoid calling k_msleep() if the sleep time is 0, since that is not a no-op.)

okke-formsma · 2021-03-10T19:10:51Z

I've simplified the macros a whole lot, it's easier to add something in the future than to remove something.

Now we won't be able to keep two keys pressed with one key, so if anyone wants that they'll have to think about how to extend macros properly.

joelspadin · 2021-03-11T00:45:51Z

It's easier to add something in the future than to remove something.

Agreed.

Now we won't be able to keep two keys pressed with one key, so if anyone wants that they'll have to think about how to extend macros properly.

That seems reasonable to me. From what I know of "macros" implemented in other programs and the few requests I've seen on Discord, I feel like the main use case for macros is pressing a sequence of things in order.

Will try to review this soon.

joelspadin

I'll let @petejohanson give the final approval as to whether using sleeps and tying up the main thread is okay, but this looks good to me.

joelspadin · 2021-03-11T00:56:21Z

app/dts/bindings/behaviors/zmk,behavior-macro.yaml

+  bindings:
+    type: phandle-array
+    required: true
+  sleep:


delay or delay-ms (to match the other places where things have a -ms suffix) might be easier to understand. The code is sleeping the thread for that long, but that's an implementation detail.

This makes use of the functionality in the following PR: zmkfirmware/zmk#685

yannickjmt · 2021-06-13T15:36:50Z

Successfully tested on a Kyria, using macros to output unicode.
Work great!
Only thing to say is that for some strange reason, I had to have at least one "&kp something" key on the layer containing only macros or I could not build due to an error devicetree error: expected property 'bindings' in /keymap/unicode_layer_windows in nice_nano.dts.pre.tmp to be assigned with 'bindings = < &foo 1 2 ... &bar 3 4 ... >' (a mix of phandles and numbers), not 'bindings = < &none &w_a_crc &w_e_aig &w_e_grv &w_c_eur &none &none &w_u_grv &none &w_o_crc &w_deg &none &none &w_a_grv &w_e_crc &w_e_trm &none &none &none &none &none &none &none &none &none &none &none &w_c_cdl &none &w_c_bht &none &none &none &none &none &w_mu &none &none &none &none &none &none &none &none &none &none &none &none &none &none >;'

dxmh · 2021-06-13T15:54:27Z

Only thing to say is that for some strange reason, I had to have at least one "&kp something" key on the layer containing only macros or I could not build due to an error

@yannickjmt, I believe this would be due to #241.

bitstarr · 2021-09-06T17:55:01Z

Will this be merged in near future?

brettmiller · 2021-10-22T04:39:16Z

Testing this on a Sweep using combos to call the macros. I found one issue. On keys with mod-taps (home row mods) the combos don't work. I've got lots of other combos on mod-taps that work fine, but the ones with combos output the tapped keys and not the macros. The combos with macros on none mod-taps work as expected. I verified that moving a macro combo that doesn't work on mod-tap keys to a none mod-tap keys works as expected.

I had to test with this branch merged w/ main locally because I'm using nice!nano v2 and needed main for that support since it was merged recently.

jvwdev · 2021-10-25T22:54:28Z

I tested a macro for my mail address. This works perfectly over USB, but when I try it over BT it stops at the second character and repeats it a couple times. The longer the macro gets, the longer it repeats the second character.

For example:
USB: abcde@mail.com
BT: abbbe@mail.com

USB: randomlongmail@mail.com
BT: raaaaaaaaaaaaaaaaaaaaaaaaaaal@mail.com

The following warning appears, only when I use BT:
<wrn> zmk: Keyboard message queue full, popping first message and queueing again

joelspadin · 2021-10-25T23:20:17Z

I wonder if the repeated keys might be a result of macros tying up the main thread instead of allowing it to process things during delays. Are there any important tasks for bluetooth that run on the main thread?

okke-formsma · 2021-12-16T20:14:10Z

Some thoughts about the future of this PR are in the macros ticket: #689 (comment)

petejohanson · 2022-03-28T17:21:26Z

Closing this, since we merged an updated version in #1166

okke-formsma force-pushed the macros branch from d588074 to 814ec57 Compare February 18, 2021 13:08

petejohanson reviewed Feb 18, 2021

View reviewed changes

okke-formsma force-pushed the macros branch from 814ec57 to fdf6831 Compare February 18, 2021 13:29

okke-formsma mentioned this pull request Feb 18, 2021

Create a simple macro behavior #170

Closed

joelspadin reviewed Feb 19, 2021

View reviewed changes

okke-formsma requested review from petejohanson and joelspadin February 19, 2021 19:32

okke-formsma mentioned this pull request Feb 20, 2021

Macros v2 #689

Closed

okke-formsma added behaviors enhancement New feature or request labels Feb 28, 2021

joelspadin reviewed Mar 7, 2021

View reviewed changes

okke-formsma force-pushed the macros branch from fdf6831 to 6d2babd Compare March 10, 2021 19:10

joelspadin reviewed Mar 11, 2021

View reviewed changes

dxmh added a commit to dxmh/zmk-config that referenced this pull request Apr 16, 2021

Configure macros for multi-key combos

b5413fb

This makes use of the functionality in the following PR: zmkfirmware/zmk#685

htang014 mentioned this pull request Oct 9, 2021

Feature backlight #904

Merged

feat(behavior): Macros

6852d72

okke-formsma force-pushed the macros branch from 6d2babd to 6852d72 Compare November 29, 2021 17:28

Nicell self-requested a review January 30, 2022 21:52

MvEerd referenced this pull request in MvEerd/zmk-config Mar 3, 2022

IBus emoji input (#7)

d58bb04

petejohanson mentioned this pull request Mar 12, 2022

feat(behaviors): Add macro support. #1166

Merged

4 tasks

petejohanson closed this Mar 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Macros #685

Macros #685

okke-formsma commented Feb 18, 2021

petejohanson left a comment

petejohanson Feb 18, 2021

okke-formsma Feb 18, 2021 •

edited

joelspadin Feb 19, 2021

joelspadin Feb 19, 2021

joelspadin Feb 19, 2021

okke-formsma Feb 19, 2021

okke-formsma Feb 19, 2021

joelspadin Feb 19, 2021

joelspadin Feb 20, 2021

okke-formsma Feb 20, 2021 •

edited

BrainWart Feb 20, 2021

okke-formsma Feb 20, 2021 •

edited

joelspadin Feb 20, 2021

okke-formsma Feb 20, 2021 •

edited

okke-formsma commented Feb 19, 2021

okke-formsma commented Mar 7, 2021

joelspadin Mar 7, 2021

joelspadin Mar 7, 2021

joelspadin Mar 7, 2021

joelspadin commented Mar 7, 2021

okke-formsma commented Mar 10, 2021 •

edited

joelspadin commented Mar 11, 2021

joelspadin left a comment

joelspadin Mar 11, 2021

yannickjmt commented Jun 13, 2021 •

edited

dxmh commented Jun 13, 2021 •

edited

bitstarr commented Sep 6, 2021

brettmiller commented Oct 22, 2021 •

edited

jvwdev commented Oct 25, 2021

joelspadin commented Oct 25, 2021

okke-formsma commented Dec 16, 2021

petejohanson commented Mar 28, 2022


		include: zero_param.yaml

		properties:

Macros #685

Macros #685

Conversation

okke-formsma commented Feb 18, 2021

petejohanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okke-formsma Feb 18, 2021 • edited

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

okke-formsma Feb 20, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okke-formsma Feb 20, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okke-formsma Feb 20, 2021 • edited

Choose a reason for hiding this comment

okke-formsma commented Feb 19, 2021

okke-formsma commented Mar 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelspadin commented Mar 7, 2021

okke-formsma commented Mar 10, 2021 • edited

joelspadin commented Mar 11, 2021

joelspadin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yannickjmt commented Jun 13, 2021 • edited

dxmh commented Jun 13, 2021 • edited

bitstarr commented Sep 6, 2021

brettmiller commented Oct 22, 2021 • edited

jvwdev commented Oct 25, 2021

joelspadin commented Oct 25, 2021

okke-formsma commented Dec 16, 2021

petejohanson commented Mar 28, 2022

okke-formsma Feb 18, 2021 •

edited

okke-formsma Feb 20, 2021 •

edited

okke-formsma Feb 20, 2021 •

edited

okke-formsma Feb 20, 2021 •

edited

okke-formsma commented Mar 10, 2021 •

edited

yannickjmt commented Jun 13, 2021 •

edited

dxmh commented Jun 13, 2021 •

edited

brettmiller commented Oct 22, 2021 •

edited