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

Enable NKRO for Aurora Corne #19372

Closed
wants to merge 1 commit into from
Closed

Enable NKRO for Aurora Corne #19372

wants to merge 1 commit into from

Conversation

stephane
Copy link

I've just assembled my first keyboard (Aurora Corne from Splitkb with Elite-Pi) so I'm new to QMK.

My keyboard was sluggish (missing keys, huge latency) in the following conditions:

  • use of the RGB Matrix
  • TRRS Cable (with 2 USB cables and no TRRS it was OK but of course I couldn't change of layer ;)

My first idea was to reduce the power consumption but finally after many tests I found NKRO was the missing setting.

Description

Add nrko to the list of features.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • 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).

I've just assembled my first keyboard (Aurora Corne from Splitkb with Elite-Pi) so I'm new to QMK.

My keyboard was sluggish (missing keys, huge latency) in the following conditions:
- use of the RGB Matrix
- TRRS Cable (with 2 USB cables and no TRRS it was OK but of course I couldn't change of layer ;)

My first idea was to reduce the power consumption but finally after many tests I found NKRO was the missing setting.
@stephane
Copy link
Author

@leah-splitkb I don't really understand why I need this change, I just see it's a default option for new keyboard and why it was not in the original version.

@leah-splitkb
Copy link
Contributor

The reason to keep NKRO disabled by default is because some computers do not have NKRO support in their BIOS keyboard drivers. Having it enabled means your keyboard does not function at all in the BIOS. This can result in a very nasty chicken-and-egg problem, which we would like to avoid if at all possible.
As NKRO support is not strictly needed in most use cases (it is primarily a thing for a specific subset of gamers and does not affect regular typing), I would prefer it if this does not get merged to the Aurora Corne root configuration.

Regarding the issues you are facing (missing keys, latency): I have not heard of anyone having similar issues, and it is not immediately obvious to me why enabling NKRO would fix them. In my understanding, enabling NKRO in QMK solely changes the HID report used to communicate the pressed keys to the computer. Unless your computer has a completely broken "Boot Protocol" driver but somehow a working NKRO driver, the setting should not be able to have an impact on keyboard behaviour.
I'd be very interested to hear if anyone is aware of another potential link between the two I am missing here, though!

As an aside, any configuration change like this should not be done for a single keyboard, but for the entire Aurora range. We have a very strong desire to keep the entire range unified, with as identical of a setup as possible. Reducing the differences between keyboards greatly reduces the customer support burden by allowing a single set of high-quality documentation to cover multiple keyboards, and is in fact one of the primary reasons the Aurora range exists in the first place.

@stephane
Copy link
Author

stephane commented Dec 18, 2022

My computer doesn't really have a BIOS (more an EFI), it's a Macbook Pro M1 so it could explain the issue.

When I upload a build w/o nkro, it works fine with my Linux computer but not on the Mac (tested with exact same cables to be sure).

Could it be tied to the ID of the USB device?

@sigprof
Copy link
Contributor

sigprof commented Dec 18, 2022

Also just enabling the NKRO feature does not actually turn it on — you need to use, e.g., the NK_ON or NK_TOGG keycodes to actually make the keyboard send the keypresses over the NKRO interface. Is it actually turned on, or just the presence of the NKRO interface changes the behavior? You can check whether pressing more than 6 regular keys at once registers all of them in a key tester (the modifier keys are not counted in those 6 keys).

@sigprof
Copy link
Contributor

sigprof commented Dec 18, 2022

Although one more issue here is that NKRO for ChibiOS boards was effectively forced on when the feature was enabled until #17588, which was just merged into develop. The fix in #14814 also touches some NKRO-related code. But those problems look like they would break the “NKRO on” case, not “NKRO off”.

Still, it could make sense to test the code from the develop branch.

@stephane
Copy link
Author

Right, I didn't enable the NRKO feature by using an appropriate key code, I'm just enabling the feature to the build.
My testing method isn't not very scientific, it's based on my perception of the latencies but the delays are so long I think I can continue this way for the time being.

I've added NK_ON and NK_OFF to my keymap and rebased my branch on develop.
As you expected, NKRO was not forced on and the keyboard behaves strangely on boot but everything works again when I push the NK_ON key 😃

The problem didn't come back when I push the NK_OFF BTW.

@stephane
Copy link
Author

Like I said in my first comment of this PR, I have the issue only when I compile with RGB_MATRIX_ENABLE (for now OLED is disabled until I resolved this issue).
I try again to disable it and the keyboard works fine with NKRO disabled (not even built).

I tested different combinations of settings when RGB Matrix in on:

  • RGB_MATRIX_DRIVER = WS2812 and WS2812_DRIVER = vendor
  • use RGBLIGHT_ENABLE = on instead of RGB Matrix. In the hope to find an I/O problem or excessive CPU usage on Elite PI.

On another subject, I could create an issue if you find it relevant.
When we are used to a tech stack we forgot about the small details that doesn't make sense at first.
I had to read a reddit post to understand the differences between:

  • Backlight -> PWM of LED not RGB
  • LED Matrix -> LED not RGB LED
  • RGB Lighting -> Underglow only, but I'm able to controls front LEDs too because there are on same bus :/
  • RGB Matrix -> I've underglow 12 RGB LEDs (state 0x02 instead of 0x04) but many Matrix animations still use them as front LEDs so at some point I'll study the RGB Matrix code to understand the logic here...

I think I would be great to have a small introduction in Lighting section to explain the differences.

@stephane
Copy link
Author

stephane commented Dec 20, 2022

I had to revert a33bf8544a261 after rebasing my code in upstream master :/

@stephane
Copy link
Author

stephane commented Jan 4, 2023

I've enabled matrix scan and console so I'm able to see Failed to execute slave_matrix with or without NRO.
So this PR doesn't make sense, I didn't understand yet why I've been able to workaround the problem with NRO but it doesn't seem to be the cause of my issues.
Thank you for your feedback.

@stephane stephane closed this Jan 4, 2023
JohnAZoidberg added a commit to FrameworkComputer/qmk_firmware that referenced this pull request Mar 23, 2023
The keyboard can still detect NKRO but the HID report limits that to
6KRO.

Causes slow start up on Linux.
Probably be related to:

- qmk/qmk_firmware#8733
- qmk/qmk_firmware#14814
- qmk/qmk_firmware#19372

Signed-off-by: Daniel Schaefer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants