[Bug] Layer tap on home row doesn't work reliably

[TristanCacqueray]

Dear Pascal,

Following up on your advise to reduce thumbs use, I setup LT keys on the home-row similar to your keymap:

                Left            Right
Base layer | LT(SYM, KC_V) | LT(SYM, KC_M)
SYM layer  | KC_RPRN       | KC_1

These sequences all produce the desired 1 no matter the typing speed:

+----------+
| V        |
+----------+
     +----------+
     | M        |
     +----------+

+----------+
| V        |
+----------+
     +---+
     | M |
     +---+

However the opposite is not true, within the achordion timeout, this one emits a 0:

+----------+
| M        |
+----------+
     +---+
     | V |
     +---+

I can only get this sequence to reliably produce the desired ):

+----------+
| M        |
+----------+
     +----------+
     | V        |
     +----------+

It seems like you are using a custom shift key instead of using the right->left hand roll to produce a ! on your keymap, so I guess this is a known issue.
Do you think that can be fixed somehow?

Thanks in advance,
-Tristan

[TristanCacqueray]

Perhaps this is related to qmk/qmk_firmware#19405 . Using this PR fixed that issue!

[getreuer]

@TristanCacqueray great question. Yes, I have noticed this. It would be great to get to the bottom of it! Just to be sure, you are using PERMISSIVE_HOLD, is that right? If so, then indeed "M down, V down, V up, M up" should produce ).

The problem I've sometimes seen is with LT, where on the switched-to layer, a shifted keycode is pressed within the tapping term. This sometimes incorrectly produces the key without shift. My first thought was that Achordion was to blame. Yet in my observation, this happens regardless of whether Achordion is enabled, so the culprit is something else.

What's confusing is I cannot reproduce the bug reliably. If I try ten times, I might get the bugged behavior once or twice. This randomness suggests the problem is related to timing. QMK is sending key events in this scenario very quickly (shift mod down, base keycode down, base keycode up, shift mod up). I've definitely seen problems with my computer mishandling fast key events. And yes, like the PR you linked, it is well known that fast key events are problematic especially with remote desktop software.

A partial fix: I don't know that it helps at all for this LT problem specifically, but there is a tweak that I've found to help with other fast key event problems. In config.h, add:

// Delay between registering and unregistering a tapped key in ms.
#define TAP_CODE_DELAY 12

Then in your keymap, replace uses of SEND_STRING with SEND_STRING_DELAY("something", TAP_CODE_DELAY).

[TristanCacqueray]

@getreuer yes, I am using PERMISSIVE_HOLD and TAP_CODE_DELAY similar to your config, here is mine for the record: https://github.com/TristanCacqueray/qmk-config/blob/main/config.h .

It doesn't seem like this issue is related to Achordion, but since the behavior is not easy to reproduce, it's hard to tell. Though, the upstream issue matched my observations: the shift modifier somehow get lost, resulting in 0/3 to be sent instead of )/#. Adding the extra sleep in send_keyboard_report appears to have fix my issue.

Thanks again for sharing your features and your blog, they have been very useful!

[TristanCacqueray]

I have also found that the layer toggle (LT) is causing frequent miss fire during same hand roll. Since I rarely use same hand chording, I made the following patch to disable the holding state of the previous key when it is on the same hand:

diff --git a/features/achordion.c b/features/achordion.c
index 8d9dfe0..a3c1bfe 100644
--- a/features/achordion.c
+++ b/features/achordion.c
@@ -164,13 +164,23 @@ bool process_achordion(uint16_t keycode, keyrecord_t* record) {
     // events back into the handling pipeline so that QMK features and other
     // user code can see them. This is done by calling `process_record()`, which
     // in turn calls most handlers including `process_record_user()`.
-    if (!is_streak && (!is_key_event || keycode == KC_LEFT_SHIFT || (is_tap_hold && record->tap.count == 0) ||
+    if (!is_streak && (!is_key_event || keycode == KC_LEFT_SHIFT ||
+#ifndef ACHORDION_SAME_HAND_CHORDING_DISABLED
+                       // Keep hold when tap-hold keys are chorded
+                       (is_tap_hold && record->tap.count == 0) ||
+#endif
         achordion_chord(tap_hold_keycode, &tap_hold_record, keycode, record))) {
       dprintln("Achordion: Plumbing hold press.");
       settle_as_hold();
     } else {
       clear_eager_mods();  // Clear in case eager mods were set.

+#ifdef ACHORDION_SAME_HAND_CHORDING_DISABLED
+      // Ensure the current key modifiers are disabled.
+      clear_mods();
+      record->tap.count = 1;
+#endif
+
       dprintln("Achordion: Plumbing tap press.");
       tap_hold_record.tap.count = 1;  // Revise event as a tap.
       tap_hold_record.tap.interrupted = true;

That seems to be good at preventing accidental mod activations when rolling on the home rows. I find that dedicating a single key for shift simplifies home-row-mods usage quite a bit, and with the above patches, Achordion now works much better in that setup. It is still possible to chord modifiers by altering the holding hand, for example to do ctrl--:
right(ctrl) + left(sym layer) + left(-).

I have also tried using this PR but it doesn't handle layer toggle (LT) on the home-row, which is a necessary replacements for shifts. That's unfortunate because this implementation does record individual held keys so that they can be replayed as tap.

Perhaps that can be useful to others, and I'd be happy to propose this patch if that makes sense for the Achordion library.