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

tap-dance: holding a key triggers on_finished every TAPPING_TERM #574

Closed
algernon opened this Issue Jul 27, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@algernon
Contributor

algernon commented Jul 27, 2016

When a tap-dance key is held, the on_finished callback is triggered from the matrix scan every TAPPING_TERM. Ideally, it should only trigger once.

The easiest would be to have a bool that stores whether it was triggered before, but perhaps there's a better way. And in any case, this should only be worked on, once the other tap-dance issues are ironed out, as to not make things worse. Recording this here as an issue, so I don't forget to deal with this too at some point.

@cdlm

This comment has been minimized.

Show comment
Hide comment
@cdlm

cdlm Jul 27, 2016

Contributor

BTW, process_tap_dance.h needs to #include "action_tapping.h" else TAPPING_TERM isn't defined

Contributor

cdlm commented Jul 27, 2016

BTW, process_tap_dance.h needs to #include "action_tapping.h" else TAPPING_TERM isn't defined

@algernon

This comment has been minimized.

Show comment
Hide comment
@algernon

algernon Jul 27, 2016

Contributor

Thanks, will add that too!

Contributor

algernon commented Jul 27, 2016

Thanks, will add that too!

algernon added a commit to algernon/qmk_firmware that referenced this issue Aug 17, 2016

tap-dance: Major rework, to make it more reliable
This reworks how the tap-dance feature works: instead of one global
state, we have a state for each tap-dance key, so we can cancel them
when another tap-dance key is in flight. This fixes qmk#527.

Since we have a state for each key, we can avoid situation where a keyup
would mess with our global state. This fixes qmk#563.

And while here, we also make sure to fire events only once, and this
fixes qmk#574.

There is one breaking change, though: tap-dance debugging support was
removed, because dumping the whole state would increase the firmware
size too much. Any keymap that made use of this, will have to be
updated (but there's no such keymap in the repo).

Also, there's a nice trick used in this rework: we need to iterate
through tap_dance_actions in a few places, to check for timeouts, and so
on. For this, we'd need to know the size of the array. We can't discover
that at compile-time, because tap-dance gets compiled separately. We'd
like to avoid having to terminate the list with a sentinel value,
because that would require updates to all keymaps that use the feature.
So, we keep track of the highest tap-dance code seen so far, and iterate
until that index.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>

algernon added a commit to algernon/qmk_firmware that referenced this issue Aug 17, 2016

tap-dance: Major rework, to make it more reliable
This reworks how the tap-dance feature works: instead of one global
state, we have a state for each tap-dance key, so we can cancel them
when another tap-dance key is in flight. This fixes qmk#527.

Since we have a state for each key, we can avoid situation where a keyup
would mess with our global state. This fixes qmk#563.

And while here, we also make sure to fire events only once, and this
fixes qmk#574.

There is one breaking change, though: tap-dance debugging support was
removed, because dumping the whole state would increase the firmware
size too much. Any keymap that made use of this, will have to be
updated (but there's no such keymap in the repo).

Also, there's a nice trick used in this rework: we need to iterate
through tap_dance_actions in a few places, to check for timeouts, and so
on. For this, we'd need to know the size of the array. We can't discover
that at compile-time, because tap-dance gets compiled separately. We'd
like to avoid having to terminate the list with a sentinel value,
because that would require updates to all keymaps that use the feature.
So, we keep track of the highest tap-dance code seen so far, and iterate
until that index.

Signed-off-by: Gergely Nagy <algernon@madhouse-project.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment