Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug] Key override sends wrong key on MacOS #24688

Open
1 of 2 tasks
Diaoul opened this issue Dec 7, 2024 · 5 comments
Open
1 of 2 tasks

[Bug] Key override sends wrong key on MacOS #24688

Diaoul opened this issue Dec 7, 2024 · 5 comments
Labels
bug help wanted

Comments

@Diaoul
Copy link
Contributor

Diaoul commented Dec 7, 2024 •
edited
Loading

Describe the Bug

This is a really weird behavior...

Defining this key override works on Linux but not MacOS

Configuration

Full config here, can be reproduced following the documentation with this very simple config:

// this one is working fine
const key_override_t dot_key_override  = ko_make_basic(MOD_MASK_SHIFT, KC_DOT, KC_COLN);
// this one is buggy
const key_override_t comm_key_override = ko_make_basic(MOD_MASK_SHIFT, KC_COMM, KC_SCLN);

Behavior

When pressing SHIFT , (S(KC_COMM)), I expect to see ; (KC_SCLN)

On MacOS

  • It outputs : (KC_COLN) instead of ; (KC_SCLN) when holding a shift modifier
  • However, it outputs correctly ; when used right after a one-shot shift

On Linux

Behaves as expected: : (KC_SCLN) is sent all the time

Alternative

I cannot reproduce the bug if using getreuer's custom shift key so I guess there is something wrong with the way it's implemented in QMK (replacement commit)

Keyboard Used

Kyria rev1

Link to product page (if applicable)

No response

Operating System

ArchLinux / MacOs

qmk doctor Output

❯ qmk doctor
Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.5
Ψ QMK home: /home/antoine/projects/keyboards/qmk/qmk_firmware
Ψ Detected Linux (Arch Linux).
⚠ Missing or outdated udev rules for 'at32-dfu' boards. Run 'sudo cp /home/antoine/projects/keyboards/qmk/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
Ψ Testing userspace candidate: /home/antoine/projects/keyboards/qmk -- Valid `qmk.json`
Ψ QMK userspace: /home/antoine/projects/keyboards/qmk
Ψ Userspace enabled: True
⚠ QMK home does not appear to be a Git repository! (no .git folder)
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 14.2.0
Ψ Successfully compiled using arm-none-eabi-gcc
Ψ Successfully tested arm-none-eabi-binutils using arm-none-eabi-size
Ψ Found avr-gcc version 14.1.0
Ψ Successfully compiled using avr-gcc
Ψ Successfully tested avr-binutils using avr-size
Ψ Found avrdude version 8.0
Ψ Found dfu-programmer version 1.1.0
Ψ Found dfu-util version 0.11
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2024-02-17 19:20:06  0000 --  (be44b3305f)
Ψ - lib/chibios-contrib: 2024-04-03 20:39:24  0800 --  (77cb0a4f)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-08-26 12:09:55  1000 --  (549b97320)
Ψ - lib/vusb: 2022-06-13 09:18:17  1000 --  (819dbc1)
Ψ - lib/printf: 2022-06-29 23:59:58  0300 --  (c2e3b4e)
Ψ - lib/pico-sdk: 2023-02-12 20:19:37  0100 --  (a3398d8)
Ψ - lib/lvgl: 2022-04-11 04:44:53 -0600 --  (e19410f8)
Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

  • AutoHotKey (Windows)
  • Karabiner (macOS), disabled

Other keyboard-related software installed

No response

Additional Context

No response
@sigprof
Copy link
Contributor

sigprof commented Dec 8, 2024

  • It outputs KC_SCLN (shifted KC_COLN) instead of KC_COLN when holding a shift modifier

Actually KC_COLN is S(KC_SEMICOLON), so if you are actually getting ; instead of : (assuming the US QWERTY layout), it would mean that the override somehow loses the Shift modifier included in KC_COLN. I wonder how that would happen, because the key override code should not send any reports between the suppression of triggering modifiers and setting of the override modifiers (which would be taken from the replacement keycode).

Does the problem happen with a plain KC_LSFT, or only when you hold OSM(MOD_LSFT)?

Also Karabiner is known to do strange things with modifiers in some cases, so make sure that it's not actually installed.

@Diaoul
Copy link
Contributor Author

Diaoul commented Dec 8, 2024 •
edited
Loading

Sorry, I realize that I completely messed up my explanation 🤦
I updated the OP with accurate information now 🙇

I thought that Karabiner was disabled but it was not, I cannot reproduce the issue when Karabiner is disabled. I guess problem solved?

Still very annoying to have to disable Karabiner. Curious that there is no issue with Karabiner with one-shot shifts and getreuer's different implementation.

Do you think this is fixable in QMK?

@sigprof
Copy link
Contributor

sigprof commented Dec 8, 2024

You may look at the evtest output on Linux (the data reported by that tool should correspond to the actual HID reports, so you may detect whether multiple changes had been sent in a single report). Looks like the key override code would send the release of the Shift modifier and the press of KC_SCLN in the same report; maybe Karabiner handles that case incorrectly and processes the KC_SCLN press before the Shift release.

The key override code already contains a Mac-specific workaround for media keys:

qmk_firmware/quantum/process_keycode/process_key_override.c

Lines 368 to 376 in 782f91a

if (IS_BASIC_KEYCODE(mod_free_replacement)) {
add_key(mod_free_replacement);
} else {
key_override_printf("NOT KEY 2\n");
send_keyboard_report();
// On macOS there seems to be a race condition when it comes to the keyboard report and consumer keycodes. It seems the OS may recognize a consumer keycode before an updated keyboard report, even if the keyboard report is actually sent before the consumer key. I assume it is some sort of race condition because it happens infrequently and very irregularly. Waiting for about at least 10ms between sending the keyboard report and sending the consumer code has shown to fix this.
wait_ms(10);
register_code(mod_free_replacement);
}

Maybe some similar workaround could be needed for the basic keycodes too (although there are also somewhat similar issues about modifiers in RDP, so maybe the proper way would be to introduce a delay between reporting the modifier changes and other key state changes at some even lower level).

@Diaoul
Copy link
Contributor Author

Diaoul commented Dec 8, 2024 •
edited
Loading

Results of evtest with pressing left shift followed by comma followed by releasing left shift.

Custom shift key (getreuer's implementation) reports this:

Event: time 1733682990.910067, -------------- SYN_REPORT ------------
Event: time 1733682998.135185, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733682998.135185, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 1
Event: time 1733682998.135185, -------------- SYN_REPORT ------------
Event: time 1733682998.136158, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733682998.136158, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 0
Event: time 1733682998.136158, -------------- SYN_REPORT ------------
Event: time 1733682998.137157, type 4 (EV_MSC), code 4 (MSC_SCAN), value 70033
Event: time 1733682998.137157, type 1 (EV_KEY), code 39 (KEY_SEMICOLON), value 1
Event: time 1733682998.137157, -------------- SYN_REPORT ------------
;Event: time 1733682998.143095, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733682998.143095, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 1
Event: time 1733682998.143095, type 4 (EV_MSC), code 4 (MSC_SCAN), value 70033
Event: time 1733682998.143095, type 1 (EV_KEY), code 39 (KEY_SEMICOLON), value 0
Event: time 1733682998.143095, -------------- SYN_REPORT ------------
Event: time 1733682998.286240, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733682998.286240, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 0
Event: time 1733682998.286240, -------------- SYN_REPORT ------------

Key override reports this:

Event: time 1733683096.770759, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733683096.770759, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 1
Event: time 1733683096.770759, -------------- SYN_REPORT ------------
Event: time 1733683096.771650, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733683096.771650, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 0
Event: time 1733683096.771650, type 4 (EV_MSC), code 4 (MSC_SCAN), value 70033
Event: time 1733683096.771650, type 1 (EV_KEY), code 39 (KEY_SEMICOLON), value 1
Event: time 1733683096.771650, -------------- SYN_REPORT ------------
Event: time 1733683096.772644, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733683096.772644, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 1
Event: time 1733683096.772644, type 4 (EV_MSC), code 4 (MSC_SCAN), value 70033
Event: time 1733683096.772644, type 1 (EV_KEY), code 39 (KEY_SEMICOLON), value 0
Event: time 1733683096.772644, -------------- SYN_REPORT ------------
;Event: time 1733683096.971676, type 4 (EV_MSC), code 4 (MSC_SCAN), value 700e1
Event: time 1733683096.971676, type 1 (EV_KEY), code 42 (KEY_LEFTSHIFT), value 0
Event: time 1733683096.971676, -------------- SYN_REPORT ------------

Is one way better than the other?

@Diaoul
Copy link
Contributor Author

Diaoul commented Dec 8, 2024

Thanks for taking the time to look into this, sounds like your intuition was correct 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Diaoul @sigprof