qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.31k stars 39.43k forks source link

[Bug] Shifted media keys appear to send the media key first and shift second, rather than the correct order #24612

Open hariganti opened 1 day ago

hariganti commented 1 day ago

Describe the Bug

When using S(KC_MUTE), the expected result would be:

However, it appears that the order is reversed for the media keys, where the media key is pressed first and the shift is pressed second, according to the output from wev

2024-11-17-123547_sway-screenshot

The red box shows regular media key usage and the green box shows where S(KC_<MEDIA_KEY>) was used instead

Keyboard Used

No response

Link to product page (if applicable)

No response

Operating System

Ubuntu 24.04 with Linux 6.8.0-48-generic

qmk doctor Output

❯ qmk doctor
Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.6
Ψ QMK home: /home/hari/Git/qmk_firmware
Ψ Detected Linux (Ubuntu 24.04.1 LTS).
⚠ Missing or outdated udev rules for 'atmel-dfu' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'kiibohd' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'stm32-dfu' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'apm32-dfu' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'gd32v-dfu' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'wb32-dfu' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'bootloadhid' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'usbasploader' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'usbtinyisp' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'md-boot' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Detected ModemManager without the necessary udev rules. Please either disable it or set the appropriate udev rules if you are using a Pro Micro.
⚠ Missing or outdated udev rules for 'caterina' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'hid-bootloader' boards. Run 'sudo cp /home/hari/Git/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
Ψ Userspace enabled: False
Ψ Git branch: master
⚠ Git has unstashed/uncommitted changes.
⚠ The official repository does not seem to be configured as git remote "upstream".
Ψ CLI installed in virtualenv.
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 13.2.1
Ψ Found avr-gcc version 7.3.0
Ψ Found avrdude version 7.1
Ψ Found dfu-programmer version 0.6.1
Ψ Found dfu-util version 0.11
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2024-02-17 19:20:06 +0000 --  (be44b3305)
Ψ - 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

Other keyboard-related software installed

No response

Additional Context

On a macropad (DOIO 16-key with triple encoder) with configuration using Via, this doesn't seem to be an issue. Configuring an encoder to send S(KC_MUTE) works as expected.

My current workaround is a custom keycode handler that sends shift at keydown and taps the media keycode before releasing shift at keyup. The issue is that this means I can't hold the key to repeat the action

sigprof commented 1 day ago

The problem is probably caused by the modifiers and media key events being sent over different USB endpoints, therefore the relative ordering of those events is not well defined (QMK just queues both reports, and the host may poll those USB endpoints in any order).

You can try enabling NKRO (note that it needs to be turned on with NK_ON or NK_TOGG), so that both kinds of events would be transmitted over the same shared endpoint. Another possible workaround is using KEYBOARD_SHARED_EP = yes in rules.mk (there might be some compatibility concerns though).

Also there were some PRs trying to work around the lost modifier issue commonly happening with RDP, which may help here too (by adding a delay between reporting the modifiers and the modified key); however, those PRs are not merged yet:

Finally, you can use a workaround with a custom keycode with some extra delay added:

My current workaround is a custom keycode handler that sends shift at keydown and taps the media keycode before releasing shift at keyup. The issue is that this means I can't hold the key to repeat the action

You should be able to do something like

    if (record->event.pressed) {
        register_code(KC_LSFT);
        wait_ms(2);
        register_code(KC_MUTE);
    } else {
        unregister_code(KC_MUTE);
        wait_ms(2);
        unregister_code(KC_LSFT);
    }

(although the extra delay at release time might be unneeded).

hariganti commented 15 hours ago

Thanks for the added context. Do you know if the shared endpoint option is also part of the JSON data-driven config yet? I might try that first and revert to using a delay (didn't know that function existed) if there are compatibility issues. I can set up a rules.mk file, but I'm trying to lean into using the data-driven config if that's going to be the future paradigm

sigprof commented 1 hour ago

KEYBOARD_SHARED_EP is mapped to usb.shared_endpoint.keyboard, but currently there is no good way to override config options using JSON at the keymap level (unless you are using keymap.json for everything).