Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!(combos): Overhaul the combo system #2765

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Nick-Munnich
Copy link
Contributor

@Nick-Munnich Nick-Munnich commented Jan 13, 2025

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 its behavior_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

  • 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.

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:

  • Refactor behaviors and keymaps to use a generalisation of action_behavior_event, where whether the behavior is action or not is a property of the event
  • To achieve the above, likely get rid of ZMK_BEHAVIOR_TRANSPARENT
  • To achieve the above, possibly refactor sensors will need to be refactored and placed under the input subsystem
  • Refactor combos, likely hold-tap and tap-dance as well, to make use of the new behavior event

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.

@Nick-Munnich Nick-Munnich added core Core functionality/behavior of ZMK combos labels Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
combos core Core functionality/behavior of ZMK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: combo doesn't interrupt tap-dance tap-only combos
1 participant