Skip to content

Comments

fix(behaviors): Correct macro release state for parametrized macros#2942

Merged
petejohanson merged 2 commits intozmkfirmware:mainfrom
caksoylar:macro-param-bug
Jul 29, 2025
Merged

fix(behaviors): Correct macro release state for parametrized macros#2942
petejohanson merged 2 commits intozmkfirmware:mainfrom
caksoylar:macro-param-bug

Conversation

@caksoylar
Copy link
Contributor

@caksoylar caksoylar commented May 15, 2025

This PR adds a test for a macro bug where parameter passing isn't working correctly after &macro_pause_for_release, and proposes a fix.

The root cause seems to be that when pre-computing release state in behavior_macro_init, the parameter passing logic of the on-press portion is only partially taken into account. That is, control bindings like &macro_param_1to1 are properly consumed (through the handle_control_binding calls) but whether the parameters are then used by a non-control behavior isn't considered: the release state precomputation does not replicate the resetting of param*_source variables that happens within replace_params during the on-press portion of the bindings. So the fix is to do this reset whenever a non-control behavior is encountered, emulating the replace_params call inside queue_macro.

As an example, in the added test, &kp MACRO_PLACEHOLDER consumes the previous &macro_param_1to1 directive, both before &macro_pause_for_release. But &mo 1 that comes after the release still uses the first parameter because the release state precomputation didn't apply the param*_source resetting logic.

Fixes #2921.

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • Pre-commit used to check formatting of files, commit messages, etc.
  • Includes any necessary documentation changes.

@caksoylar caksoylar requested a review from a team as a code owner May 15, 2025 06:32
@caksoylar caksoylar added bug Something isn't working behaviors labels May 15, 2025
@caksoylar caksoylar requested a review from petejohanson May 15, 2025 06:40
@nmunnich
Copy link
Contributor

Nice spot. It feels a bit weird to me to apply a fix to every behaviour, though it could be argued that a refactor should happen to treat each behaviour individually. Shouldn't this sort of reset happen after the wait for release?

Copy link
Contributor

@nmunnich nmunnich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a brief discussion on Discord I am now convinced.

@caksoylar
Copy link
Contributor Author

I expect this might also fix #2031, but would need testing to confirm.

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, and thanks for the additional test!

@petejohanson petejohanson merged commit 1bac680 into zmkfirmware:main Jul 29, 2025
48 checks passed
@caksoylar caksoylar deleted the macro-param-bug branch July 29, 2025 22:45
jrsharp pushed a commit to jrsharp/zmk that referenced this pull request Nov 4, 2025
…mkfirmware#2942)

test(behaviors): Add parametrized macro test that fails

fix(behaviors): Correct macro release state for parametrized
drewlwhitney pushed a commit to drewlwhitney/zmk that referenced this pull request Dec 22, 2025
…mkfirmware#2942)

test(behaviors): Add parametrized macro test that fails

fix(behaviors): Correct macro release state for parametrized
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

behaviors bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] parameterized macros do not function properly with layers

3 participants