Rapidly typing alternating shifted/unshifted keys causes lost characters

[DBendit]

Issue

When typing alternating shifted and unshifted keys rapidly (e.g. + and =), characters end up not being emitted.

Replication instructions

Using the following config: https://gist.github.com/DBendit/2b3f2bebdf6943d80b9301b3a6ff2287

Hold down one of the SYMBOL keys and type KC.PLUS and KC.EQL repeatedly as fast as you can.

Expected results

Alternating + and = characters will be emitted, matching the keypresses.

Actual results

A sequence like ================ will be emitted, with the following logs: https://gist.github.com/DBendit/3d640614dd5af088ad591dc51f908406

Misc

My theory is that, since = and + are shifted and unshifted values of the same ANSI key, it's causing some issue as they key events get resolved. Maybe KMK is deduplicating events incorrectly as a result? I'm not really familiar with KMK's internals.

[klardotsh]

Shifts and unshifts are fun in KMK: the USB HID spec represents modifier keys (Ctrl, Alt, Super, Shift) by bitshifting the second byte in the 8-byte HID report that is sent to the host device (a HID report, assuming no NKRO support, consists of a single "header byte" indicating which type of report is coming, a second byte indicating modifiers, and six bytes indicating keycodes of any pressed keys).

I tried getting clever in the early days of KMK and building HID reports per report cycle somewhat idempotently, as found in https://github.com/KMKfw/kmk_firmware/blob/master/kmk/hid.py#L79-L119 and https://github.com/KMKfw/kmk_firmware/blob/master/kmk/hid.py#L148-L170, but not functionally purely since the underlying report memory is shared (for allocation/performance reasons). Since the bitshifted byte representing held modifiers is constructed every HID cycle, and is not necessarily the most performant code in the world, there are likely bugs that would manifest as this type of pseudo-race-condition, and the linked functions here are almost certainly where I would advise starting to put debug statements (print or LEDs or otherwise).

[xs5871]

I've been looking into into the HID part of KMK a lot for #625, and what @klardotsh is true, that may be part of the issue.
Additionally I can reproduce the + and = confusion when I roll-over between them, meaning pressing the second before releasing the first. That is something we can not "fix", because again, the HID spec never intended that use case. You can either shift everything, or nothing, there's no one-shiftend-and-one-not-shifted.

[tflori]

I'm wondering. Does every report have to contain every key that is hold plus modifiers? If not wouldn't be the solution to send only one key with each report? Or would it mean that the shift key got not released and the modifier is still active and the second key is also in shifted state?

[Hsad]

I have also just discovered this bug, but in my case rolling from ) to ; instead outputs ):
It does seem like it's caused by rolling fast enough that the previous key has not been released, and shift still counts as held.
My question is could the pressing of a key that is explicitly mapped to a non-shifted key force the shift flag to be unset? I suppose that would also necessitate the fake "unpressing" of the previous ) key. Which in my case would be fine, but I can see how in other cases that may be bad behavior.
I'll try to dig into this later, but if anyone knows which file I should start in, I wouldn't mind the guidance

[xs5871]

It's all over the place. Keys are defined in kmk.keys, the handler to apply those keys are in kmk.handlers, but tha actual logic for shifted keys appears to be in kmk.hid for some reason...
Good luck, btw.

[klardotsh]

Holdovers from the earliest spaghetti I wrote before I ever intended this to reach a bigger audience than like 5 people running on PyBoard v1.1s, if I recall. I don't think shifted keys have been touched in a major way since maybe 2018, from rough memory, because I do remember doing some hackery in the HID module to that effect to enable this. Don't recall the context, and I don't blame anyone for shredding it and doing it over - I certainly would.