keyboardio / KeyboardioHID

A HID library for Arduino
MIT License
34 stars 17 forks source link

Add affordances for negotiating bootkeyboard/nkro based on host request #10

Closed obra closed 6 years ago

algernon commented 6 years ago

A note mostly to myself, so I'll find this tomorrow too: there is already an (untested) branch from Jesse at https://github.com/keyboardio/KeyboardioHID/commit/ad3819614590d5a6710dc19a4c9bbbf7f5676444. That seems reasonable, too.

The hardest part of this will likely be the edge cases, where either BIOSes or OSes fail to send a SET_PROTOCOL.

algernon commented 6 years ago

Another thing I'm wondering about if it is worth pulling in all of BootKeyboard. That seems a little bit excessive.

What I was thinking, inspired by this thread on GeekHack, is to play a bit with the USB descriptors, so that the boot stuff comes first, then the NKRO. If in boot mode, we'd only fill the boot report, otherwise only NKRO. We may need to mark the boot report constant (and assume BIOSes will ignore the constant parts). But this may not be the safest route...

Even if not going that route, BootKeyboard still seems like an overkill, and rolling the boot functionality into Keyboard would reduce code considerably, and allow us to reuse more of it, too. We'd register two nodes, but as we are multi-report, we'd just use a different report id (and when handling host->keyboard stuff, we can distinguish based on report id too). This, however, assumes that BIOSes can handle multi-report devices.

algernon commented 6 years ago

Oh, and: I'd love if we could make the protocol negotiation optional. I'd want to disable it for my sketch, because my BIOS works fine with NKRO, and the boot protocol code would be sitting there idle, useless. (And I'm already short on space...)

obra commented 6 years ago

I suspect that making it optional would be a lot easier with it as part of its own report. Also, I suspect that the keyboards that need a bootkeyboard implementation are the ones most likely to need a simple report.....

I'd be thrilled for you to experiment with this, of course :)

On Thu, Nov 30, 2017 at 2:54 PM, Gergely Nagy notifications@github.com wrote:

Oh, and: I'd love if we could make the protocol negotiation optional. I'd want to disable it for my sketch, because my BIOS works fine with NKRO, and the boot protocol code would be sitting there idle, useless. (And I'm already short on space...)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/keyboardio/KeyboardioHID/issues/10#issuecomment-348347559, or mute the thread https://github.com/notifications/unsubscribe-auth/AACxaDOhvRaqovqrp3Cil-7LBo1B6BbOks5s7zItgaJpZM4OnLCm .

algernon commented 6 years ago

The naive implementation does not appear to work, at least not when trying with FreeBSD (which does not support NKRO) in a VM with USB passthrough. Still need to verify if FreeBSD is sending us a SET_REPORT.

algernon commented 6 years ago

I spoke too soon. The naive implementation DOES work, if I force the keyboard into boot report mode. FreeBSD does not appear to send SET_REPORT though, so we have our first corner case already.

algernon commented 6 years ago

Went through the FreeBSD USB keyboard driver source, and it appears to not send a SET_REPORT. The mouse driver does.

So we already have a case where an OS does not adhere to the USB spec. There are likely many more. This suggests that any automation we come up with, will break in one way or the other. Thus, manual override seems to be the safest.

As BootKeyboard does work, it is tempting to stick to that instead of experimenting. Feels safer too. Just need to figure out a way to either make it considerably smaller, or to make it optional. And implement ways to force NKRO/Boot or boot mode. Perhaps make NKRO optional too! If someone wants to use the keyboard under FreeBSD, NKRO is just as much a dead weight as Boot is for me.

algernon commented 6 years ago

Checked NetBSD and OpenBSD too, they bot send SET_REPORT, and signal that they want report mode. Will check if they really support that, but this looks promising... we just need to fix FreeBSD to signal that it wants boot mode. It already does that for mice.

algernon commented 6 years ago

I spoke too soon. FreeBSD doesn't work. It worked, because I re-plugged the device, and that broke the pass-through, so it got emulated.

FreeBSD also does not support keyboard + mouse on the same node, and prefers the mouse. Even without MouseKeys, we still compile that in, due to kaleidoscope::hid stuff. Commenting those out, the keyboard shows signs of working, but... the report is not ok.

algernon commented 6 years ago

Oh, okay. It works. Just had to force boot mode harder. Sweet.