feat!(combos): Overhaul the combo system #2765
Draft
+1,186
−456
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR overhauls ZMK's combo system, moving from one based on key positions triggering combos to one where behaviors trigger combos. This approach allows for more complex interactions between combos and other behaviors, such as placing combo triggers inside of a hold-tap for tap-only combos.
This is a breaking change, as it requires users to adjust their keymaps significantly in order to use this system. Preserving position-based combos for backwards compatibility would be very complex and messy, and the two forms would not interact with each other well. I do believe that this approach has advantages that make it worth the breaking change, though.
Overview of changes
Action Behavior Event
A new event was added,
action_behavior_triggered
(different name suggestions welcome). This event is triggered automatically when a so-called "action" behavior is triggered. Action behaviors are those which cause some effect to happen, such as a key press. They are contrasted with "control flow" behaviors, which select between branches based on a condition (e.g. mod-morph). The type of a behavior is determined in itsbehavior_driver_api
. By default behaviors are marked as control flow - this is because most of the external behaviors found in modules I know of are control flow, so less stuff would break there. I do not consider a behavior that is in-between the two types to be good design so I do not think such a behavior should be considered a possibility.This event is used to let combos interrupt correctly, as they can no longer be interrupted by position events if their triggers are in behaviors, but keycode state events happen too late in the execution and are too inflexible to let them insert their outputs correctly.
Combos raising position events
To let combo events interrupt hold-tap etc. properly, combos raise a position event when invoking behaviors. They catch and handle their own behavior when it comes back to them. A better solution would be nice, and may be able to be found in a future PR if hold-tap and tap-dance could have their interrupt situation improved by action behavior event.
Array of behaviors
A new control flow behavior which is just an array of behaviors was added. This is just to make using combo-trigger easier, but also has benefits for e.g. hold-tap. I do plan on putting a similar layerX behavior in a module, as that would allow for some passable combo configuration within Studio now, at the cost of some layers.
Tap dance adjustment
Tap dance needed a minor adjustment to its tracking of the position of active tap-dances. It now holds an array of potential positions, using position events and action-behavior events to allocate potential positions to it correctly.
Memory improvements
Some refactoring I did during this PR should result in less memory needed. Switching from key positions to triggers should also reduce the memory needed without restricting combos as much, as now far fewer empty slots in the
combo_lookup
array will exist. Other bits may have counteracted this, though.Other notes
I have not tested this live, only with posix-tests - my initial goal was to achieve backwards compatibility with existing functionality, before testing the behavior when things are nested. Plenty of additional tests to verify that everything works correctly will still need to be written. There might be some weirdness going on when there is a hold-tap inside a macro inside a combo that I need to investigate. This PR does not include the improvements from #2494, though I think they are good and could be added to this quite simply.
PR check-list
Closes #544. I think it closes #1701, but that will need testing.
EDIT:
After discussion and some pondering on my part, I feel as though this is the right overall path, but removing key position combos is likely non-negotiable. To allow for this approach to happen neatly alongside key position combos, the following will need to happen:
ZMK_BEHAVIOR_TRANSPARENT
Then adding combo-trigger behaviors to allow for advanced combos would work. My current thinking is to add two kconfig flags, one which enables/disables key position combos (on by default) and one for the number of additional triggers that are passed to combo-triggers.
This current PR seems to be fully functional as a proof of concept and can be used until then.