Wrong keycode with Custom Shift Keys Feature

[wheredoesyourmindgo]

I'm getting the incorrect keycode when I use the custom shift keys feature with keycodes that don't require a shift press, ex. comma and opening arrow bracket --> comma and semi-colon. The shifted code is always being returned, in my example, the colon is firing instead of the semi-colon. My workaround is to use unregister_mods() over del_mods(). Does that seem like the proper fix?

https://github.com/getreuer/qmk-keymap/blob/90267e06df2bb2ba35afbad5501fa7ba72a88472/features/custom_shift_keys.c#L43

[wheredoesyourmindgo]

oh nevermind. it looks like b102238e1c21116572bec2c9ec79ec57b5e3a996 fixes this with del_weak_mods()

[wheredoesyourmindgo]

double nevermind, it looks like I'm still having the same issue unless i use use unregister_mods() so i re-opened the issue. sorry about the confusion.

[getreuer]

Thanks for the report and suggested fix!

What kind of shift keys are observing this with? is it the plain KC_LSFT shift keys or something else?

[wheredoesyourmindgo]

of course.

i'm not sure if i understand the question, but whether i hold the left shift or right shift and press comma I get colon instead of semi-colon. my repo is qmk fork if it helps. if i recall i remember i encountered this same issue in qmk's custom keys feature so maybe I'm missing some detail.

[getreuer]

Thanks for sharing your keymap. It's awesome to see all the custom code you have in there, I'll be studying this.

About my earlier question, I was thinking your shift keys could be one-shot mods or something like that with special handling, vs. the plain KC_LSFT and KC_RSFT keys. I wouldn't be surprised if the source of this bug is an edge case in how custom_shift_keys interacts with another special feature.

I see your keymap can apply shifts in several ways, including through a userspace implementation of one-shot mods (XOSM_LSFT) and a magic_shift feature that applies shift under combinations of (other mods) + layer key (if I understood correctly). Do you observe the : vs. ; bug with any of these methods of shifting, or one in particular?

My workaround is to use unregister_mods() over del_mods(). Does that seem like the proper fix?

Thanks again for the suggestion. I looked further into this. When a custom shift key is pressed with a shift mod active, the current code does

del_mods(MOD_MASK_SHIFT);
del_weak_mods(MOD_MASK_SHIFT);
register_code16(registered_keycode);

Under the hood, unregister_mods() is (code link):

void unregister_mods(uint8_t mods) {
    if (mods) {
        del_mods(mods);
        send_keyboard_report();
    }
}

This function is simply del_mods() + send_keyboard_report(). I wouldn't have thought that making this additional send_keyboard_report() has an effect since register_code16() itself sends a keyboard report. But I haven't been able to repro the bug to test such things. Maybe the difference is this extra report sequences the release of the shift key before the ; key is pressed. Maybe whether this matters depends on the OS.

I suggest we test that. I just pushed a commit https://github.com/getreuer/qmk-keymap/commit/df111136ed673f633af06856c1dc98eae8d84977 to add a send_keyboard_report() call between removing the shift mods and registering the keycode. Please pull or copy this change to your keymap and test whether this fixes the bug.

If it's possible, it would also help if you could test your keymap with other shift-handling features disabled, like the userspace oneshot and magic_shift. That might narrow down this bug to some edge-case interaction between our custom codes.

[getreuer]

An update: I just got a similar bug report from reddit user u/uolot about weak mods in Caps Word. Interestingly, the bug there was also happening on Mac OS and fixed when adding a send_keyboard_report() call after setting mods and before registering the next key. This fits with what I was thinking: adding this send_keyboard_report() call sequences the shift release vs. next key, so that the OS knows not to shift that key. Otherwise, without this send_keyboard_report(), maybe it is OS dependent how exactly it treats the next key event. Maybe that explains how I haven't run into this problem on Linux.

Thank you both for reporting and help fixing it! It sounds like we have solved both bugs, but please reopen if there are still problems.