getreuer / qmk-keymap

My keymap & reusable QMK gems
Apache License 2.0
301 stars 45 forks source link

achordion behaviour with layer taps #13

Closed bschwehn closed 2 years ago

bschwehn commented 2 years ago

Hi, first thanks for the extensions you developed and shared. Other than achordion I use layer lock and caps word, it's really nice!

With achordion I have a behaviour which I don't like and am not sure what is causing it. It works great for my home row mods (MT(MOD_LSFT, KC_S) and so on), but less so for the layer taps (LT(1,KC_COMMA) and so on).

For example, I have comma ',' set up to switch to a special character layer. When I press and hold ',' it switches to the layer and if I then press 'a', I get a colon ':'. Even if I override achordion_chord to always return true, I get the following behaviour when I press ',' and 'a' quickly (and the behaviour is different compared to when I press the home row mod shift and another key quickly):

  1. I get the hold behaviour less often as compared to when not enabling achordion. Shouldn't achordion not change the behaviour when achordion_chord returns true?
  2. In the cases I don't get the hold behaviour, the layer tap key is also not output, so I'm when pressing ',' and 'a' quickly, I either get ':' or 'a' but not ',a'. This seems different to when I press say 's' (home row mod for shift) and 'e' quickly, there I either get the hold action ('E') or the tap for both keys ('se').

Do you have an idea why that may be and if there is a way to make the layer taps behave the same way as the mod taps? I couldn't figure it out from reading the code. No worries if you don't have an immediate idea, I can try and do some debugging when I have some time, maybe it's an interaction with some of my other config.

Thanks, Ben

bschwehn commented 2 years ago

I did some more experimentation and one difference I see between achordion on and off is, I believe, when I press the mod key, then the second key, then release the mod key, then release the second key (maybe all within tapping term, not sure). In the logs, without achordion this looks like:

r/c 01234567
 00: 00000000
 01: 00000000
 02: 00000000
 03: 00010000
 04: 00000000
 05: 00000000
 06: 00000000
 07: 00000000
 08: 00000000
 09: 00000000
 0A: 00000000
 0B: 00000000

 r/c 01234567
 00: 00000000
 01: 00000000
 02: 00000000
 03: 00010000
 04: 00000000
 05: 00000000
 06: 00000000
 07: 00000000
 08: 00000000
 09: 00010000
 0A: 00000000
 0B: 00000000
 keyboard_report: 02 00 00 00 00 00 00 00
 keyboard_report: 02 00 2D 00 00 00 00 00

 r/c 01234567
 00: 00000000
 01: 00000000
 02: 00000000
 03: 00000000
 04: 00000000
 05: 00000000
 06: 00000000
 07: 00000000
 08: 00000000
 09: 00010000
 0A: 00000000
 0B: 00000000
 keyboard_report: 00 00 2D 00 00 00 00 00

with the mod key in row 03 and second key in row 09.

With achordion:

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00010000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00000000
0A: 00000000
0B: 00000000

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00010000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00010000
0A: 00000000
0B: 00000000
Achordion: Key 0x4106 pressed.

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00010000
0A: 00000000
0B: 00000000
Achordion: Key released.

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00000000
0A: 00000000
0B: 00000000
keyboard_report: 00 00 36 00 00 00 00 00
keyboard_report: 00 00 00 00 00 00 00 00

So with achordion it results in a press of the second key only, without achordion it results in the layer switched version of the second key.

As for the difference between MT and LT, I'm not sure, it's subtle. However, I noticed that when I enable HOLD_ON_OTHER_KEY_PRESS, the difference is very obvious. With achordion, pressing two MT keys on the same hand rapidly results in achordion kicking in with both normal characterr as output. But when the first key is a LT key and the second a MT key, it results in only the normal mode output of the second key with the first key neither being output nor getting their hold layer switch triggered. Maybe a similar difference is what is tripping me up without HOLD_ON_OTHER_KEY_PRESS enabled.

Settings:

#define LEADER_PER_KEY_TIMING
#define LEADER_TIMEOUT 400
#define IGNORE_MOD_TAP_INTERRUPT
#define HOLD_ON_OTHER_KEY_PRESS // I have this enabled for the test above only
#define TAPPING_FORCE_HOLD_PER_KEY
#define ONESHOT_TIMEOUT 1000
#define PERMISSIVE_HOLD

Layout: https://github.com/bschwehn/qmk_firmware/tree/master/keyboards/moonlander/keymaps/custom and https://github.com/bschwehn/qmk_firmware/tree/master/keyboards/cantor/keymaps/custom

bschwehn commented 2 years ago

Without HOLD_ON_OTHER_KEY_PRESS set and PERMISSIVE_HOLD and IGNORE_MOD_TAP_INTERRUPT set, I type a sequence: key1 press, key2 press, key1 release, key2 release.

I see that when the sequence is started with an MT key, process_record_user is triggered in between key2 press and key1 release:

r/c 01234567
00: 00000000
01: 00000000
02: 00010000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00000000
0A: 00000000
0B: 00000000

r/c 01234567
00: 00000000
01: 00000000
02: 00010000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00010000
0A: 00000000
0B: 00000000
Achordion: keycode: 0x6216 hold key code: 0x421D pressed: 1 state: 1
Achordion: is_mt: true is_taphold: true
Achordion: keycode: 0x6216 layertapmin: 0x4000 layertapmax: 0x4FFF
Achordion: is_physical: true
Achordion: state released
Achordion: press and considered hold
Achordion: set unsettled
keyboard_report: 02 00 00 00 00 00 00 00
Achordion: Key 0x6216 pressed. Set eager mods.

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00010000
0A: 00000000
0B: 00000000

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00000000
0A: 00000000
0B: 00000000
Achordion: keycode: 0x4136 hold key code: 0x6216 pressed: 1 state: 0
Achordion: is_mt: false is_taphold: true
Achordion: keycode: 0x4136 layertapmin: 0x4000 layertapmax: 0x4FFF

But when the sequence is started with an LT key, process_record_user is triggered later, after key1 release:

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00010000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00000000
0A: 00000000
0B: 00000000

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00010000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00010000
0A: 00000000
0B: 00000000

r/c 01234567
00: 00000000
01: 00000000
02: 00000000
03: 00000000
04: 00000000
05: 00000000
06: 00000000
07: 00000000
08: 00000000
09: 00010000
0A: 00000000
0B: 00000000
Achordion: keycode: 0x4106 hold key code: 0x421D pressed: 1 state: 1
Achordion: is_mt: false is_taphold: true
Achordion: keycode: 0x4106 layertapmin: 0x4000 layertapmax: 0x4FFF
Achordion: is_physical: true
Achordion: state released
keyboard_report: 00 00 06 00 00 00 00 00
Achordion: keycode: 0x4106 hold key code: 0x421D pressed: 0 state: 1
Achordion: is_mt: false is_taphold: true
Achordion: keycode: 0x4106 layertapmin: 0x4000 layertapmax: 0x4FFF
Achordion: is_physical: true
Achordion: state released
keyboard_report: 00 00 00 00 00 00 00 00

Not sure yet why that is, but for now it does not look this is caused by achordion.

bschwehn commented 2 years ago

https://github.com/qmk/qmk_firmware/issues/17281 seems to fix my issue :)

getreuer commented 2 years ago

Thanks for the report, detailed logging, and the pointer about https://github.com/qmk/qmk_firmware/issues/17281!

It sounds like you have already have this resolved, but if not, a couple other things worth checking:

  1. Check that you have the latest version of Achordion. There have been a few fixes along the way. In the latest version, "bypassing" with timeout of 0 should pipe the events immediately through without change.
  2. I have a couple LT home row keys (described here) that have been working well for me. So I wonder if a different tap hold configuration could explain the effect you observe. You could try copying my configuration.
bschwehn commented 2 years ago

Thanks for the additional hints!

I tried your layout (with the minimal changes required to make it run on my keyboard), and I /think/ I don't see the difference in MT and LT. I'm not 100% sure, as the layout is quite different, so maybe I just cannot reproduce the way I type on my normal layout. And it is possible that I just type in a particularly sloppy way on my normal layout...

I then tried moving my layout closer to yours (changing my config.h, rules.mk and process_record_user to closer match yours), but so far no luck, I still see the difference in MT vs LT without applying the patch. I'm curious why there seems to be a difference. If I can figure it out, I will let you know.

Until then, I have it working now in a way I like, so I'm quite happy, thanks again for sharing those libraries!