[Bug] Reset function for Tap Dance skipped when multiple Tap Dance keys are used

[bmijanovich]

Describe the Bug

This is hard to describe, apologies up front. Say we have the following scenario:

• Tap Dance key 1, that sends a mouse button on a single tap, and enters layer 1 on a single hold.
• Tap Dance key 2, that sends a mouse button on a single tap, and enters layer 2 on a single hold.
• Tap Dance state determination that favors instant hold on interrupt (pressed state triggers hold regardless of TAPPING_TERM expiry
• A keymap like this:

Layer 0: TD_KEY_1, TD_KEY_2
Layer 1: _______, FOO
Layer 2: BAR, _______

Then, quickly perform the following, maybe using a rolling motion:

1. Press TD_KEY_1
2. Press TD_KEY_2
3. Release TD_KEY_1
4. Release TD_KEY_2

What I would expect is for FOO to execute, and to end up in Layer 0. Instead, sometimes FOO executes and sometimes it doesn't, and I often end up stuck in Layer 2 until I tap TD_KEY_2 again.

This behavior works fine if you use a non-Tap Dance keycode on key 2 on Layer 0.

System Information

• Keyboard: Ploopy Classic
  • Revision: 1.005
• Operating system: Mac OS Catalina 10.15.7
• AVR GCC version:
  N/A, using QMK Docker container
• ARM GCC version:
  N/A, using QMK Docker container
• QMK Firmware version:
  0.13.26
• Any keyboard related software installed?
  No

[bmijanovich]

A previous version of my Ploopy keymap will demonstrate this bug: https://github.com/bmijanovich/qmk_firmware/blob/ee6e4bceb67b799bf7d6c5cfa769d684990fbc84/keyboards/ploopyco/trackball/keymaps/bmijanovich/keymap.c

[drashna]

Tap dances doesn't handle interrupts well, so it's not processing the "hold" part most likely, but instead, the tap part.

[bmijanovich]

Any suggestions for working around this with the advanced tap dance callback functions?

…

On Mon, Aug 9, 2021, 2:36 AM Drashna Jaelre ***@***.***> wrote: Tap dances doesn't handle interrupts well, so it's not processing the "hold" part most likely, but instead, the tap part. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#13764 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMRIPY4HWJQO2FQDATDWGTT35ZO7ANCNFSM5BFDUZIA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

[Anomalocaridid]

I had the same issue trying to create a tap dance key on the non-base layer of my Ploopy Classic Trackball. The issue seems to be caused by overwriting your keymap's static variable for the tap dance state (td_state in the tap dance examples in the documentation).

Since I was using ACTION_TAP_DANCE_FN(), I did not need a reset function that would make recording the state necessary and I was able to fix it by simply not overwriting the state variable and instead directly branching on the output of the function to determine the tap dance mode (cur_state in the examples).

I doubt this would work with ACTION_TAP_DANCE_FN_ADVANCED(). But it may be worth trying to store the previous tap dance state in the "finished" function in another variable before overwriting it and then restoring it at the end of the "reset" function.

[joukewitteveen]

You could test against #16394. The reset logic was cleaned up a bit and indeed there were some subtle issues with keeping track of the 'active' tap dance that got fixed.

[joukewitteveen]

I'm fairly sure this can be closed now that #16394 is merged.