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

Improve mod-tap behaviour #69

Merged

Conversation

petejohanson
Copy link
Contributor

@petejohanson petejohanson commented Jul 31, 2020

The following is now working with this branch:

Mod-Tap Interrupts

  • The following behavior binding activation sequence:
    1. ⬇️ &mt MOD_LSFT L,
    2. ⬇️ &kp A
    3. ⬆️ &mt MOD_LSFT L
    4. ⬆️ &kp A
  • Generates the following keycode state events:
    1. ⬇️ L
    2. ⬇️ A
    3. ⬆️ L
    4. ⬆️ A

The same sequence also works when the second behavior used is a different &mt binding, when neither should be mods, and should have their tap behaviour.

Mod Behaviour When Tapping other keys or mod-taps

  • The following behavior binding activation sequence:
    1. ⬇️ &mt MOD_LSFT L,
    2. ⬇️ &kp A
    3. ⬆️ &kp A
    4. ⬆️ &mt MOD_LSFT L
  • Generates the following keycode state events:
    1. ⬇️ LEFT SHIFT + A
    2. ⬆️ A
    3. ⬆️ LEFT SHIFT

Which triggers the "mod" part of the mod-tap properly.

NOT IMPLEMENTED:

  1. Mod activation after a key is held, regardless of other input.
  • Requires we have a generic "key position hold" event system, which we don't have yet.
  • Once we have that, it shouldn't be hard to integrate with this code, marking those active_mod_tap entries as pending = false and enabling the mod via the endpoint reports.
  • Use case if for things like "hold a modifier so I can then use my mouse to ctrl + click"
  1. Queue for sending HID events over USB.
  • I'm still encountering occasional funkiness w/ sends over USB failing, causing unexpected behaviour, stuck keys, etc. This is especially noticeable when doing things like releasing multiple held mod-taps simultaneously.

* Not working: Roll over + mod-tap with multiple
  mod-tap bindings!
* Track active mods when mods or keycode
  events occur.
* Use the tracked mods when releasing or
  generating keycode events.
* Track pending/used status in one array, for
  improved storage efficency.
@petejohanson petejohanson added bug Something isn't working core Core functionality/behavior of ZMK behaviors labels Jul 31, 2020
@petejohanson petejohanson requested a review from Nicell Jul 31, 2020
@petejohanson petejohanson self-assigned this Jul 31, 2020
Nicell
Nicell approved these changes Jul 31, 2020
Copy link
Member

@Nicell Nicell left a comment

This seems like a nice solution. Definitely agreed on the fact that a timer for "on hold" is needed for cases where a modifier is held, but no key presses happen (shift+clicking is a common thing I do). It is clever to handle mod-tap by following sequences rather than only using timing. I hadn't thought of that. Should be more robust.

data->captured_keycode_events[i].active_mods = 0;
LOG_DBG("Re-sending latched key press for usage page 0x%02X keycode 0x%02X state %s", ev->usage_page, ev->keycode, (ev->state ? "pressed" : "released"));
ZMK_EVENT_RELEASE(ev);
k_msleep(10);
Copy link
Member

@Nicell Nicell Jul 31, 2020

Choose a reason for hiding this comment

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

I'm assuming these sleeps are only necessary until a queue + worker exists?

Copy link
Contributor Author

@petejohanson petejohanson Jul 31, 2020

Choose a reason for hiding this comment

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

Yeah, that's a band-aid to avoid the USB send failures more often. It's not perfect, since we can have key press/release which are super close together that still cause issues.

Copy link
Contributor Author

@petejohanson petejohanson left a comment

Merging!

@petejohanson petejohanson merged commit 24ec83c into zmkfirmware:main Aug 1, 2020
1 check passed
MangoIV referenced this pull request in MangoIV/zmk Dec 18, 2020
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 core Core functionality/behavior of ZMK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants