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.

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

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

Open
2 tasks
hariganti opened this issue Nov 17, 2024 · 5 comments

Comments

@hariganti
Copy link

Describe the Bug

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

  • LSFT down
  • MUTE down
  • LSFT and MUTE up (in any order)

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

  • AutoHotKey (Windows)
  • Karabiner (macOS)

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
Copy link
Contributor

sigprof commented Nov 18, 2024

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
Copy link
Author

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
Copy link
Contributor

sigprof commented Nov 19, 2024

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).

@hariganti
Copy link
Author

I currently use my keymap.json for everything (no config.h or rules.mk), which has been a little frustrating since not everything appears to be there yet, like setting combos to use the default layer for matching. I should have time to attempt this workaround today

@hariganti
Copy link
Author

Using a shared endpoint with the configuration option (didn't test with NKRO) didn't seem to work, but adding a delay of 5 ms--arbitrarily chosen--at keydown works well. I'm not sure if this issue should then be closed since a workaround is still required, and something like an optional mod delay, as a #define or in the keymap.json would be preferable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants