[Bug] Achordion affecting LM usage in combination with MT keys

sporkus commented 1 year ago

Hi Pascal, I'm having some issues with achordion preventing LM from processing when combined with other mod-tap keys in a quick, rolling scenario.

Affected scenario

----LM----      |
   ----MT----   | tapping term

With qmk's hold on other key press, the layer shift happens immediately, and the corresponding MT is sent. But with process_achordion in the loop, I'm seeing this in the console

sporkus:reviung41:1: Achordion: Key 0x422C pressed.
sporkus:reviung41:1: Achordion: Key released.
sporkus:reviung41:1: *** keycode 422c is configured to be hold on other key press: 1
sporkus:reviung41:1: Achordion: Key 0x2834 pressed.
sporkus:reviung41:1: Achordion: Key released.

Unaffected scenario 1

----LM--------    |
   ----MT----     | tapping term

If the MT key is fully released before the LM key is released, I was able to handle it with achordion_chord like this:

console:

sporkus:reviung41:1: Achordion: Key 0x422C pressed.
sporkus:reviung41:1: Achordion chord: hold is preferred for bottom row tap-hold keys
sporkus:reviung41:1: hold key: [3, 7], tap key: [1, 11]
sporkus:reviung41:1: Achordion: Plumbing hold press.
sporkus:reviung41:1: KL: kc: 0x422C, pressed: 1, time:  3091, int: 1, count: 0, mod state: 000000
sporkus:reviung41:1:
sporkus:reviung41:1: KL: kc: 0x0233, pressed: 1, time:  3239, int: 0, count: 1, mod state: 000000

Unaffected scenario 2

----LM----        |
   ----KC----     | tapping term

When the tapped key is a regular keycode, achordion_chord handles it properly, no matter how quick I roll.

Comment

Looks like Achordion is first completely releasing/processing the LM event, and then processes the MT event.

I tried injecting a conditional settle_as_hold() or a return true; on line 125 using get_hold_on_other_key_press(), and got the layer changed keycode of the mod-tap keys. But the layer is stuck on hold, because it's probably the wrong way to deal with this situation. For now, I'm conditionally skipping achordion for the thumb keys while still enjoying the functional benefits it brings to the home row.

Thanks again for the wonderful library! I hope I had described the issue clear enough.

Information

[x] Achordion (from this repo)

getreuer commented 1 year ago

Thanks for the detailed bug report!

I'm not certain I follow the affected scenario. LM keys, layer mod keys, are not tap-hold keys and always take effect immediately. Is it rather a layer-tap aka "LT" key? (QMK has too many confusable abbreviations...)

From that console log, I see Achordion reacting to what must be two tap-hold keycodes 0x422C and 0x2834. QMK sometimes changes keycode encodings between versions. I think, but please correct me, that the keys are

0x422C is LT(2, KC_SPC)
0x2834 is LGUI_T(KC_QUOT)

and the rolled press is across these two keys as:

---LT(2, KC_SPC)---       |
   ---LGUI_T(KC_QUOT)---  | tapping term

Did I get that right?

If an MT key is pressed and released within the tapping term, and no other key goes down after the MT is pressed, then I am fairly sure this guarantees that QMK core reports the MT as tapped, not held, for any tap-hold configuration. But this disagrees with the console log: a line like "Achordion: Key 0x422C pressed" gets logged when Achordion gets an event from QMK core saying that the key has been "held." Achordion only intervenes on hold events, tap events continue without logging or modification. So something else is going on.

Are you sure the MT is released before the tapping term? Thanks again for reporting this.

sporkus commented 1 year ago

You're absolutely right, I was having issues with Layer-Tap/LT instead of LM. I am sure that made it no sense to read. Sorry about that!

Upon further testing, I am experiencing the same issue without achordion. It was a core QMK issue after all. I had just gotten mislead by the extra achordion messages. I apologize for the wrongly filed issue and really appreciated your thoughtful response.

The keys in question:

Layer-Tap key: LT(2, KC_SPC), specified to be held when interrupted.
tapkey 1: a modtap key, with a different keycode on layer 2
tapkey 2: a "regular" key, again with different keycode on layer 2

The scenarios

---LT---         |
   ---tapkey---  | tapping term 

Without or without achordion, I get the `MT` key from layer1 for tapkey1 and layer2 key for tapkey2. 
I wrongly thought that this only happens with achordion previously.

---LT----------   |
   --tapkey--     | tapping term 

Layer 2 is active for both tapkey1 and tapkey2 as expected.

getreuer commented 1 year ago

No worries! I appreciate the follow up. It definitely gets more complicated when different firmware features interact, and Achordion is especially bad in this way. Wish you luck with your keymap!

getreuer / qmk-keymap