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

[BUG] Combo timeout is not enforced for small values #748

Open
Roming22 opened this issue Mar 17, 2023 · 5 comments
Open

[BUG] Combo timeout is not enforced for small values #748

Roming22 opened this issue Mar 17, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@Roming22
Copy link

Describe the bug
I've set my combos to trigger if key presses occur within 45ms. The combo gets triggered on longer delays.

To Reproduce
Steps to reproduce the behavior:

  • Add a combo on the default layer, with a timeout of 45ms.
  • Roll the keys of the combo
  • The combo is activated even though the delay between the 2 key presses is greater that the timeout.

Expected behavior
The combo should not activate, and individual key presses should be output.

Debug output

136262 kmk.kmk_keyboard: MatrixChange(ic=5, pressed=True)
136267 kmk.kmk_keyboard: KeyResolution(key=Key(code=1032, has_modifiers=None))
136288 kmk.kmk_keyboard: coordkeys_pressed={5: None}
136291 kmk.kmk_keyboard: keys_pressed=set()
136321 kmk.kmk_keyboard: processing timeouts
136347 kmk.kmk_keyboard: MatrixChange(ic=7, pressed=True)
136352 kmk.kmk_keyboard: KeyResolution(key=Key(code=1036, has_modifiers=None))
136358 kmk.kmk_keyboard: processing timeouts
136368 kmk.modules.combos: activate Chord([1032, 1036])
136381 kmk.kmk_keyboard: coordkeys_pressed={5: None, 7: None}
136384 kmk.kmk_keyboard: keys_pressed={Key(code=19, has_modifiers=None)}
136392 kmk.kmk_keyboard: MatrixChange(ic=5, pressed=False)
136397 kmk.kmk_keyboard: KeyResolution(key=Key(code=1032, has_modifiers=None))
136399 kmk.modules.combos: deactivate Chord([1032, 1036])

Additional context
The keys of the combos are MT keys. I would expect the combo and MT delays to be in parallel.

In my example the keys pressed are 't' and 'e', and output p instead of te. The delay between the key presses is 85ms, almost twice as long as the 45ms timeout configured, and yet the chord gets activated.

My layout: xs18.txt

@Roming22 Roming22 added the bug Something isn't working label Mar 17, 2023
@xs5871
Copy link
Collaborator

xs5871 commented Mar 17, 2023

That's because python on an MCU is slow (and KMK isn't super optimized). Can't be easily "fixed" as such.
Out of curiosity: what MCU is this on?

@Roming22
Copy link
Author

A sea-picro, which is an RP2040 based MCU using the pi micro footprint.

@regicidalplutophage
Copy link
Member

regicidalplutophage commented Mar 17, 2023

I have a suspicion that the slowness can be sidestepped somehow. Likely a tough issue, but keypad library is interrupt driven, so it shouldn't be impossible.
There's already ticks_ms attached to matrix changes, so if we could say "don't activate chord if matrix changes are this far apart" it would be significantly more precise.

@xs5871
Copy link
Collaborator

xs5871 commented Mar 17, 2023

That's not wrong, and in the long run I intended to look into basing timeouts on the KeyEvent timestamps, rather than now(). That could result in some funky behaviour though, especially for short timeouts and slow modules.
Another thing I've been looking into is a better data structure for timeouts, like an actual priority queue.

@Roming22
Copy link
Author

Roming22 commented Apr 1, 2023

I do not know about the internals of KMK. But if a timer was to be started on the first key press of a potential combo, and was to register an event on timeout, then that could be used to discriminate between a roll and a combo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants