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

Fix Caps Lock LEDs once and for all #4824

Merged
merged 2 commits into from
Jan 12, 2019

Conversation

fauxpark
Copy link
Member

@fauxpark fauxpark commented Jan 11, 2019

Description

The status LEDs don't light up/light up incorrectly on Linux in certain circumstances. This is because, if NKRO is enabled it will send two LED states, one for the 6KRO interface, and one for the NKRO interface. QMK interprets the NKRO report ID as the LED state and turn on the num lock and scroll lock LEDs.

If we have two bytes, that probably means the first is a report ID. The 6KRO interface may or may not have one, but the NKRO interface always does, so we need to check this regardless of whether KEYBOARD_SHARED_EP is defined.

The status LEDs now function correctly on Windows 10, macOS 10.14 and Ubuntu 18.10 under the following conditions:

  • KEYBOARD_SHARED_EP=no, NKRO_ENABLE=no
  • KEYBOARD_SHARED_EP=no, NKRO_ENABLE=yes, NKRO off
  • KEYBOARD_SHARED_EP=no, NKRO_ENABLE=yes, NKRO on
  • KEYBOARD_SHARED_EP=yes, NKRO_ENABLE=no
  • KEYBOARD_SHARED_EP=yes, NKRO_ENABLE=yes, NKRO off
  • KEYBOARD_SHARED_EP=yes, NKRO_ENABLE=yes, NKRO on

Types of changes

  • Core
  • Bugfix
  • New Feature
  • Enhancement/Optimization
  • Keyboard (addition or update)
  • Keymap/Layout/Userspace (addition or update)
  • Documentation

Issues Fixed or Closed by this PR

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document. (https://docs.qmk.fm/#/contributing)
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

If we have two bytes, that probably means the first is a report ID. The 6KRO interface may or may not have one, but the NKRO interface always does, so we need to check this regardless of whether KEYBOARD_SHARED_EP is defined.
@fauxpark fauxpark changed the title Check the size of the SET_REPORT packet Fix Caps Lock LEDs once and for all Jan 11, 2019
@DidierLoiseau
Copy link
Contributor

Works perfectly fine for me on Ubuntu 18.10, with all combinations of KEYBOARD_SHARED_EP and NKRO_ENABLE.

@drashna
Copy link
Member

drashna commented Jan 12, 2019

Thanks!

@drashna drashna merged commit 2c41093 into qmk:master Jan 12, 2019
@fauxpark fauxpark deleted the fix-capslock-led-once-and-for-all branch January 12, 2019 01:27
@adzenith
Copy link
Contributor

I just came here to say I've been trying way too long to figure out why my caps lock light didn't work (macOS Mojave) and this fixed it like magic. Thanks @fauxpark!

@fauxpark fauxpark mentioned this pull request Jan 22, 2019
13 tasks
rseymour pushed a commit to rseymour/qmk_firmware that referenced this pull request Mar 13, 2019
* Check the size of the SET_REPORT packet

If we have two bytes, that probably means the first is a report ID. The 6KRO interface may or may not have one, but the NKRO interface always does, so we need to check this regardless of whether KEYBOARD_SHARED_EP is defined.

* Fix indentation
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
* Check the size of the SET_REPORT packet

If we have two bytes, that probably means the first is a report ID. The 6KRO interface may or may not have one, but the NKRO interface always does, so we need to check this regardless of whether KEYBOARD_SHARED_EP is defined.

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

Successfully merging this pull request may close these issues.

USB_LED_CAPS_LOCK not working with LED anymore [Ergodox EZ]
5 participants