search

qmk / qmk_firmware

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

NKRO and Non-NKRO Modes do not send all key commands. #725

Closed IBNobody closed 8 years ago

IBNobody commented 8 years ago

I noticed that in non-NKRO mode, only a handful of keys between codes 0x74 (KC_EXECUTE) and 0xA4 (KC_EXSEL) are sent via register/unregister or even add_key/del_key. Only some of the international keys work. In NKRO mode, none of these keys are sent.

It is frustrating because I want to use some of the keys that my OS does not recognize as macro triggers for AutoHotkey or LuaMacros.

IBNobody commented 8 years ago

This is a separate issue from #726. I have verified that the fix for that other issue does not solve this one.

Here are the keys in this range that work for LUFA's 6KRO but not NKRO.

IBNobody commented 8 years ago

I restored the NKRO functionality to match the 6KRO by changing the descriptor, specifically LUFA's NKRO_EPSIZE from 16 to 32.

#define NKRO_EPSIZE 32

Help me understand the ramifications of a larger NKRO packet.

IBNobody commented 8 years ago

Someone on Geekhack pointed out this file to me...

https://github.com/jackhumbert/qmk_firmware/blob/master/doc/USB_NKRO.txt

... which shows that the 32 byte packet size is legitimate even though 16 is the default.

I have not had any issues with a 32 byte report size, but I have not tried to do anything in the BIOS.

One thing I did notice was that a 32 byte report only allowed me to send codes up to 0xF7. This is because byte 0 of the report contains modifier key information, and bytes 1-to-end contain the other keys in a bitmap fashion (1 key per bit). (I suspect mod keys are duplicated in this range, too.) With 32 bytes in the report, 31 are dedicated to the bitmap, giving us 31*8=248 bits.

IBNobody commented 8 years ago

I have verified that my work PC (a DELL) can enter and control the BIOS with 32 byte NKRO enabled.

Also, I suspect that the OS is just ignoring some of the keycodes and is not even registering them. Can anyone confirm?

jackhumbert commented 8 years ago

Oo nice! We should be able to enable NKRO across the board if others don't have any issues, right?

IBNobody commented 8 years ago

Yes, I believe so. I have been running 16 byte NKRO for awhile now. The only thing to be aware of is that 32 byte NKRO takes up an extra 16 bytes of RAM, even when it is enabled but turned off. I do not know how well we are doing on RAM.

fredizzimo commented 8 years ago

There's probably still risk of incompability with older bioses.

Anyway according to this thread it seems like better compability can be achieved by sending a standard 8 byte report followed by the bitmap report. I havent checked the source code, so I'm not sure it's implemented or not. Hasu was in the discussion though, so I would be surprised if it isn't.

algernon commented 8 years ago

If it does not work with older BIOSes, one can still disable NKRO with some bootmagic, no? So one can still use the keyboard with older stuff, without reflashing, even if NKRO is on by default. As far as I see, anyway.

IBNobody commented 8 years ago

I would counter the older BIOS issue with the fact that we had these concerns 4 years ago. These older BIOSes are now older by 4 more years. They are approaching "grandparent's PC" levels of obsolescence. They are only getting older, and people are replacing their PCs.

With magic keys, you can temporarily disable NKRO with the left-shift+right-shift+n.

NKRO is implemented as a mod byte + bitmap. Non NKRO is implemented as a mod byte + 8 byte filler + 8 byte rollover buffer.

EDIT: My oldest work Dell is 8 years old.

fredizzimo commented 8 years ago

I see one problem though.

The compatibility Soarer achieved was through a hack of sending oversized reports and I have checked our code quickly now and I'm pretty sure we don't do that. So it means that any keyboard that doesn't follow the SetProtocol or supports bitmap reports would fail two work.

On Soarer's list 2/8 or 25% of the computers didn't use SetProtocol and we have no idea if that has changed since then, some manufactures might simply stick with what they have, even if they update the other parts of the BIOS. I'm fairly sure that @IBNobody's computer uses the SetProtocol, rather than supporting the full HID stack and bitmap reports, so it doesn't really give us an additional data point.

Regarding @algernon's suggestion about the bootmagic. Well some keyboards have bootmagic disabled. Still it could be an option for those that have it enabled.

But due to the potential compatibility issues, even if NKRO was enabled by default, I would like to see it disabled by bootmagic by default, most users simple won't need NKRO, The standard report is already 6KRO, which means that you can send 6 keys and 8 modifiers simultaneously. I'm hard pressed to find use cases for more than that, except maybe for some very special games, especially when two or more players play on the same keyboards, stenographing usage might also run into problems.

Edit regarding the change of the size from 16 to 32 bytes, I don't see any real problems with that, except that some keyboards that would accept 16 bytes in the bios might run into buffer overflows and crash, or in the very worst case corrupt the bios. It also slightly reduces the bandwidth available for logging, virtual serial ports and so on.

IBNobody commented 8 years ago

Can you point me to Soarer's list? I do know that my work computers are finicky about what keyboards they work with in the bios, as I have had a few name brand HID compatible keyboards fail to work.

I don't care if we enable the NKRO device by default or not. I'm fine with it being set to NO in the default template.

However, I want to make sure that if someone changes their makefile to enable NKRO, it gets enabled without any fancy button presses.

On Wed, Sep 7, 2016, 12:34 AM fredizzimo notifications@github.com wrote:

I see one problem though.

The compatibility Soarer achieved was through a hack of sending oversized reports and I have checked our code quickly now and I'm pretty sure we don't do that. So it means that any keyboard that doesn't follow the SetProtocol or supports bitmap reports would fail two work.

On Soarer's list 2/8 or 25% of the computers didn't use SetProtocol and we have no idea if that has changed since then, some manufactures might simply stick with what they have, even if they update the other parts of the BIOS. I'm fairly sure that @IBNobody https://github.com/IBNobody's computer uses the SetProtocol, rather than supporting the full HID stack and bitmap reports, so it doesn't really give us an additional data point.

Regarding @algernon https://github.com/algernon's suggestion about the bootmagic. Well some keyboards have bootmagic disabled. Still it could be an option for those that have it enabled.

But due to the potential compatibility issues, even if NKRO was enabled by default, I would like to see it disabled by bootmagic by default, most users simple won't need NKRO, The standard report is already 6KRO, which means that you can send 6 keys and 8 modifiers simultaneously. I'm hard pressed to find use cases for more than that, except maybe for some very special games, especially when two or more players play on the same keyboards, stenographing usage might also run into problems.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/jackhumbert/qmk_firmware/issues/725#issuecomment-245179836, or mute the thread https://github.com/notifications/unsubscribe-auth/AFl50InU5prHD_NqGAdfd7OxDwEjVeyMks5qnkzvgaJpZM4J1PD4 .

fredizzimo commented 8 years ago

I don't have the original, I guess more complete list, but the 8 keyboard list is in the Geekhack thread, that I already previously linked, this post.

I think we already supports both ways enabling it, from the docs:

NKRO_ENABLE

This allows for n-key rollover (default is 6) to be enabled. It is off by default, but can be forced by adding #define FORCE_NKRO to your config.h.

I would prefer if both ways were possible from the Makefile though.

IBNobody commented 8 years ago

Ahh... Missed the link, thanks. Still, this is from 2010, before EFI, and some of the examples used were a Pentium M and an Asus Eeebox (back when netbooks were popular). Again, forward progress...

Regarding the FORCE_NKRO flag, I added that because I got tired of fighting with the EEPROM settings which default to off. There were always people posting asking how to actually enable NKRO, since it wasn't as straightforward as setting the makefile option. They had to use boot magic or a magic key (not Jack's boot magic replacement, but the command).

On Wed, Sep 7, 2016, 1:06 AM fredizzimo notifications@github.com wrote:

I don't have the original, I guess more complete list, but the 8 keyboard list is in the Geekhack thread https://geekhack.org/index.php?topic=13162.0, that I already previously linked, this post https://geekhack.org/index.php?topic=13162.msg257012#msg257012.

I think we already supports both ways enabling it, from the docs:

NKRO_ENABLE

This allows for n-key rollover (default is 6) to be enabled. It is off by default, but can be forced by adding #define FORCE_NKRO to your config.h.

I would prefer if both ways were possible from the Makefile though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jackhumbert/qmk_firmware/issues/725#issuecomment-245184099, or mute the thread https://github.com/notifications/unsubscribe-auth/AFl50P7pSdhZ0jLDNBG0CTHYoHBUP2JRks5qnlRZgaJpZM4J1PD4 .

IBNobody commented 8 years ago

I am closing this issue as I have resolved what I needed to resolve. I am still not happy with how NKRO comes up when the makefile switch is thrown, but I do not have any better ideas.