feat(behaviors): Support parameterized macros. #1232

petejohanson · 2022-04-08T15:44:18Z

* Add two new compatibles for macros that
  take one or two parameters when bound in
  a keymap.
* Use `&macro_param_1to1`, `&macro_param_1to2`, `&macro_param_2to1`,
  and `&macro_param_2to2` control entries in the bindings for the macro
  to have the next binding entry have it's values substituted.

Ok, draft for now until I can create updated docs, but hoping for some feedback on this idea. We use more marker bindings to trigger different behavior for the next binding entry, e.g. &macro_param_1to1.

So a new macro that surround a given letter with quotes:

		quote_letter_macro: quote_letter_macro {
			#binding-cells = <1>;
			label = "ZMK_QLET";
			compatible = "zmk,behavior-macro-one-param";
			wait-ms = <40>;
			tap-ms = <40>;
			bindings = <
				&kp QUOT
				&macro_param_1to1 &kp MACRO_PLACEHOLDER
				&kp QUOT>;
		};

Could then be used in your keymap like &quote_letter A.

Thoughts?

joelspadin · 2022-04-12T16:58:24Z

The most likely scenario I can think where this could cause an issue is if a behavior packs multiple values into one parameter such that 0xFFFF is a valid and common value, for example RGBW(255, 255, 255, 255) for a 4-channel LED color.

joelspadin · 2023-03-10T18:03:46Z

Copying over my suggestion from Discord so it doesn't get lost:

What if instead of having a special parameter value that gets replaced, we add a macro-specific behavior that modifies the following binding? For example, we could have something like

&macro_param1_1st replace 1st param with macro param 1
&macro_param1_2nd replace 2nd param with macro param 1
&macro_param2_1st replace 1st param with macro param 2
&macro_param2_2nd replace 2nd param with macro param 2
&macro_param1 alias for &macro_param1_1st (for bindings that only take one param)
&macro_param2 alias for &macro_param2_1st

petejohanson · 2023-03-22T06:18:27Z

I dig this idea. Implementation seemed simple enough. I opted for &macro_param_1to1, &macro_param_2to1 but I'm really open to some feedback/ideas on better naming on how to handle this.

I also left a define MACRO_PLACEHOLDER in, that could be used just for clarity in the bindings for a macro, but you could just as easily just hardcode 0 there if you wanted.

From the test:

		quote_letter_macro: quote_letter_macro {
			#binding-cells = <1>;
			label = "ZMK_QLET";
			compatible = "zmk,behavior-macro-one-param";
			wait-ms = <40>;
			tap-ms = <40>;
			bindings = <
				&kp QUOT
				&macro_param_1to1 &kp MACRO_PLACEHOLDER
				&kp QUOT>;
		};

Thoughts?

petejohanson · 2023-04-17T08:55:20Z

@joelspadin Rebased onto the new Zephyr 3.2 main, still ready for review. TIA.

kvietcong · 2023-04-18T06:25:44Z

Used to make a dynamic CTRL+KEY macro for easy use with a tap_hold key.

No weird behaviors as everything seems to be working just fine, allowing me to hold keys like V (&tct V V) to get a CTRL+V tap (not hold like with a regular hold_tap). With some documentation, I think the feature would be really easy to use for anyone. I also needed to make sure to #include <dt-bindings/zmk/macro.h>, which wasn't mentioned in the PR yet.

Actual Configuration


behaviors: {
    tct: tap_ctrl_tap {
        compatible = "zmk,behavior-hold-tap";
        label = "TAP_CTRL_TAP";
        #binding-cells = <2>;
        tapping_term_ms = <300>;
        flavor = "tap-preferred";
        bindings = <&ctrl_kp>, <&kp>;
    };
};

macros {
    ctrl_kp: macro_ctrl_kp {
        #binding-cells = <1>;
        label = "MACRO_CTRL_KP";
        compatible = "zmk,behavior-macro-one-param";
        bindings
            = <&macro_press &kp LCTL>
            , <&macro_param_1to1 &macro_tap &kp MACRO_PLACEHOLDER>
            , <&macro_release &kp LCTL>;
    };
};

joelspadin

Needs docs updates, but the implementation looks good to me.

joelspadin · 2023-04-23T16:41:15Z

app/src/behaviors/behavior_macro.c


 LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);

-#if DT_HAS_COMPAT_STATUS_OKAY(DT_DRV_COMPAT)
+#if DT_HAS_COMPAT_STATUS_OKAY(zmk_behavior_macro) ||                                               \


Could this be simplified using Kconfig + CMake, or would you rather do that in a separate commit?

Yes, it can be! Rebased onto main where we have the Kconfig.behaviors and tweaked this to use the DT_HAS defines and simplify the code by only compiling if we have any of the macro DT compatibles enabled.

joelspadin · 2023-04-23T16:46:22Z

app/src/behaviors/behavior_macro.c

+    switch (state->param1_source) {
+    case PARAM_SOURCE_MACRO_1ST:
+        binding->param1 = macro_binding->param1;
+        break;
+    case PARAM_SOURCE_MACRO_2ND:
+        binding->param1 = macro_binding->param2;
+        break;
+    default:
+        break;
+    }
+
+    switch (state->param2_source) {
+    case PARAM_SOURCE_MACRO_1ST:
+        binding->param2 = macro_binding->param1;
+        break;
+    case PARAM_SOURCE_MACRO_2ND:
+        binding->param2 = macro_binding->param2;
+        break;
+    default:
+        break;
+    }


This could be deduplicated with something like

update_param(&binding->param1, state->param1_source, &macro_binding); update_param(&binding->param2, state->param2_source, &macro_binding);

Or more functional programming style:

binding->param1 = update_param(state->param1_source, binding->param1, &macro_binding); binding->param2 = update_param(state->param2_source, binding->param2, &macro_binding);

Good catch. Used the more functional style.

joelspadin · 2023-05-15T02:59:31Z

app/src/behaviors/behavior_macro.c

 #define TRANSFORMED_BEHAVIORS(n)                                                                   \
-    {LISTIFY(DT_PROP_LEN(DT_DRV_INST(n), bindings), BINDING_WITH_COMMA, (, ), n)},
+    {LISTIFY(DT_PROP_LEN(n, bindings), ZMK_KEYMAP_EXTRACT_BINDING, (, ), n)},

 #define MACRO_INST(n)                                                                              \
    static struct behavior_macro_state behavior_macro_state_##n = {};                              \
    static struct behavior_macro_config behavior_macro_config_##n = {                              \
-        .default_wait_ms = DT_INST_PROP_OR(n, wait_ms, CONFIG_ZMK_MACRO_DEFAULT_WAIT_MS),          \
-        .default_tap_ms = DT_INST_PROP_OR(n, tap_ms, CONFIG_ZMK_MACRO_DEFAULT_TAP_MS),             \
-        .count = DT_INST_PROP_LEN(n, bindings),                                                    \
+        .default_wait_ms = DT_PROP_OR(n, wait_ms, CONFIG_ZMK_MACRO_DEFAULT_WAIT_MS),               \
+        .default_tap_ms = DT_PROP_OR(n, tap_ms, CONFIG_ZMK_MACRO_DEFAULT_TAP_MS),                  \
+        .count = DT_PROP_LEN(n, bindings),                                                         \
        .bindings = TRANSFORMED_BEHAVIORS(n)};                                                     \
-    DEVICE_DT_INST_DEFINE(n, behavior_macro_init, NULL, &behavior_macro_state_##n,                 \
-                          &behavior_macro_config_##n, APPLICATION,                                 \
-                          CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, &behavior_macro_driver_api);
-
-DT_INST_FOREACH_STATUS_OKAY(MACRO_INST)
+    DEVICE_DT_DEFINE(n, behavior_macro_init, NULL, &behavior_macro_state_##n,                      \
+                     &behavior_macro_config_##n, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, \
+                     &behavior_macro_driver_api);


n is now a node ID instead of an integer. Should it be renamed?

joelspadin · 2023-05-15T03:03:48Z

docs/docs/behaviors/macros.md

+```
+
+:::note
+The `MACRO_PLACEHOLDER` is defined as a convenient way to avoid just putting `0` in that location. It does _not_ control what parameters are substituted; only the macro controls do that.


Might be useful to first explain that you still need to put something in for every binding, even if it will just get overwritten due to &macro_param_XtoY. Otherwise, it might be confusing why you would need to put 0 there, and therefore why you might use this instead.

Yeah, good call. I expanded on this and pulled it out of the note aside it had been hiding in.

joelspadin · 2023-05-15T03:04:54Z

docs/docs/behaviors/macros.md

+### Parameters
+
+When creating a macro that takes parameter(s), there are control when the parameters passed to the macro are used
+within the macro itself. All of the controls are "one shot" and will change how the passed in parameters are used for the very next behavior in the `bindings` list of the macro.


Might be useful to clarify that it applies to the next non-macro control behavior, so &macro_param1to1 &macro_param2to2 @macro_press &foo 0 0 is a valid thing that works.

Tweaked the wording here.

caksoylar · 2023-05-15T05:50:52Z

docs/docs/behaviors/macros.md

@@ -62,6 +78,30 @@ bindings
 There are a set of special macro controls that can be included in the `bindings` list to modify the
 way the macro is processed.

+### Parameters
+
+When creating a macro that takes parameter(s), there are control when the parameters passed to the macro are used


This sentence (specifically "there are control when the parameters [...]") seems to be broken, I couldn't tell the intention.

This was... incomprehensible.... Remind me not to write docs late at night when I'm tired. Reworded, to hopefully actually make sense now. Thanks!

docs/docs/behaviors/macros.md

caksoylar

Docs LGTM, noted a couple things in the logging and about the tests.

app/src/behaviors/behavior_macro.c

app/tests/macros/place-holder-parameters/native_posix_64.keymap

* Add two new compatibles for macros that take one or two parameters when bound in a keymap. * Use `&macro_param_1to1`, `&macro_param_1to2`, `&macro_param_2to1`, and `&macro_param_2to2` control entries in the bindings for the macro to have the next binding entry have it's values substituted. Co-authored-by: Cem Aksoylar <caksoylar@users.noreply.github.com>

petejohanson added enhancement New feature or request behaviors labels Apr 8, 2022

petejohanson self-assigned this Apr 8, 2022

joelspadin mentioned this pull request Mar 18, 2023

feat: macro parameter #1717

Closed

petejohanson force-pushed the behaviors/macro-parameters branch from 7b159f0 to 8ec44e1 Compare March 22, 2023 06:16

petejohanson requested a review from joelspadin March 22, 2023 06:20

petejohanson marked this pull request as ready for review March 23, 2023 04:16

petejohanson force-pushed the behaviors/macro-parameters branch from 8ec44e1 to 6df23ef Compare April 17, 2023 08:54

joelspadin reviewed Apr 23, 2023

View reviewed changes

petejohanson force-pushed the behaviors/macro-parameters branch 4 times, most recently from a48ffa9 to 8a0c9b5 Compare May 14, 2023 21:29

petejohanson requested a review from joelspadin May 14, 2023 21:31

joelspadin reviewed May 15, 2023

View reviewed changes

petejohanson force-pushed the behaviors/macro-parameters branch from 8a0c9b5 to d759650 Compare May 15, 2023 05:38

petejohanson requested a review from joelspadin May 15, 2023 05:45

caksoylar reviewed May 15, 2023

View reviewed changes

petejohanson force-pushed the behaviors/macro-parameters branch from d759650 to 91f5721 Compare June 5, 2023 06:14

caksoylar reviewed Jun 5, 2023

View reviewed changes

docs/docs/behaviors/macros.md Outdated Show resolved Hide resolved

docs/docs/behaviors/macros.md Outdated Show resolved Hide resolved

petejohanson force-pushed the behaviors/macro-parameters branch from a300849 to 2cfbe1b Compare June 5, 2023 06:35

petejohanson requested a review from caksoylar June 20, 2023 02:52

caksoylar approved these changes Jun 20, 2023

View reviewed changes

petejohanson force-pushed the behaviors/macro-parameters branch from 2cfbe1b to 80beaec Compare June 20, 2023 16:35

petejohanson requested a review from a team as a code owner June 20, 2023 16:35

petejohanson force-pushed the behaviors/macro-parameters branch from a0df8bf to de25544 Compare June 20, 2023 16:41

petejohanson force-pushed the behaviors/macro-parameters branch from de25544 to 3130e41 Compare June 20, 2023 16:50

petejohanson merged commit 805dd4a into zmkfirmware:main Jun 20, 2023
44 checks passed

aaronkollasch mentioned this pull request Jun 21, 2023

feat(behaviors): Add macro-pause-for-layer behavior #1804

Open

JeffDess mentioned this pull request Jun 27, 2023

Parameterized macros support urob/zmk-helpers#20

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(behaviors): Support parameterized macros. #1232

feat(behaviors): Support parameterized macros. #1232

petejohanson commented Apr 8, 2022 •

edited

joelspadin commented Apr 12, 2022

joelspadin commented Mar 10, 2023

petejohanson commented Mar 22, 2023

petejohanson commented Apr 17, 2023

kvietcong commented Apr 18, 2023

joelspadin left a comment

joelspadin Apr 23, 2023

petejohanson May 14, 2023

joelspadin Apr 23, 2023

petejohanson May 14, 2023

joelspadin May 15, 2023

petejohanson May 15, 2023

joelspadin May 15, 2023

petejohanson May 15, 2023

joelspadin May 15, 2023

petejohanson May 15, 2023

caksoylar May 15, 2023

petejohanson Jun 5, 2023

caksoylar left a comment

feat(behaviors): Support parameterized macros. #1232

feat(behaviors): Support parameterized macros. #1232

Conversation

petejohanson commented Apr 8, 2022 • edited

joelspadin commented Apr 12, 2022

joelspadin commented Mar 10, 2023

petejohanson commented Mar 22, 2023

petejohanson commented Apr 17, 2023

kvietcong commented Apr 18, 2023

joelspadin 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

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

caksoylar left a comment

Choose a reason for hiding this comment

petejohanson commented Apr 8, 2022 •

edited