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

hidboot.h: Lock Led Default Implementation not Working #577

Closed
Jonas-commits opened this issue Dec 18, 2020 · 11 comments
Closed

hidboot.h: Lock Led Default Implementation not Working #577

Jonas-commits opened this issue Dec 18, 2020 · 11 comments

Comments

@Jonas-commits
Copy link

I've been working on a Keyboard-Layout converter utilizing an Arduino Mirco and a Micro USB Host shield, which works out well so far, besides using the SetReport function for setting the lock LEDs. There is a default implementation for HandleLockingKeys in hidboot.h, which is also used by the example USBHIDBootKbd.ino, as is not overriding the default method. In the example already, setting the Lock LEDs just does not work on both my Keyboards, one being a standard Cherry G80 and the being one being a perixx ergonomic keyboard.
I have no clue what's the issue here, but the LEDs just do not light up at all at any time.

@Jonas-commits Jonas-commits changed the title hidboot.h: lock Led example not working hidboot.h: Lock Led default implementation not working Dec 18, 2020
@Jonas-commits Jonas-commits changed the title hidboot.h: Lock Led default implementation not working hidboot.h: Lock Led Default Implementation not Working Dec 18, 2020
@DanielGibson
Copy link
Contributor

DanielGibson commented Dec 31, 2020

I'm trying to build something similar (with a Pro Micro and the USB Host Mini).
I'm having the same issue with two of my keyboards, both are Gaming/Multimedia keyboards (Ducky DK2108; KM-Gaming K-GK2, which identifies as "Holtek Semiconductor, Inc. Gaming keyboard") that also have additional keys to control volume, start a calculator etc. Those keyboards otherwise work as expected in the USBHIDBootKbd.ino example (keypresses of "normal" keys are printed to the serial console, those of multimedia keys aren't).

However, I also have a cheap old Cherry PS2 keyboard (Cherry RS 6000 M; this one has no extra keys) with a cheap USB adapter (identifies as "PCPlay Barcode PCP-BCG4209" - yes, that's weird but it works) and there the LEDs do work in USBHIDBootKbd.ino.

I have no idea what the issue is here, except I'm pretty sure it's not a power issue - I'm making sure the keyboard gets 5V and the KM-Gaming keyboard has illuminated keys which do work, furthermore its firmware flashes the 3 lock LEDs under some circumstances and that still works.

@tmk
Copy link
Contributor

tmk commented Dec 31, 2020

With USBHIDBootKbd.ino keyboard indicators works for me as expected.

This problem is keyboard specific probably. You will have to know what happens around HandleLockingKeys and narrow cause of the problem. Enable debugging with #define ENABLE_UHS_DEBUGGING 1 in setting.h and use USBTRACE to add debug prints there.

@DanielGibson
Copy link
Contributor

I forgot to mention, I already did that and return (hid->SetReport(0, 0/*hid->GetIface()*/, 2, 0, 1, &lockLeds)); indeed gets called with values in lockLeds that look sensible, i.e. between 0 and 7, depending on how often I pressed which caps/scroll/num-lock

@Jonas-commits
Copy link
Author

@DanielGibson yeah also pretty sure this is no power issue. Just measured again, 4.65Vbus under load, which should be enough. Also, the media keys cannot work in hidboot, as checking the RAW HID, they are not setting any keycode on the Boot protocol bytes. And yes, the value of lockLeds is correct.

@tmk those are the default debug outputs provided by the library, not sure what else to output:
16:52:10.069 -> Start
16:52:10.529 -> BM Init
16:52:10.856 -> Addr:01
16:52:10.856 -> NC:01
16:52:10.856 -> HID_PROTOCOL_KEYBOARD
16:52:10.856 -> bNumEP:02
16:52:10.856 -> Cnf:01
16:52:12.851 -> bIfaceNum:00
16:52:12.851 -> bNumIface:00
16:52:12.851 ->
16:52:12.851 -> Interface:00
16:52:12.897 -> PROTOCOL SET HID_BOOT rcode:00
16:52:12.897 -> SET_IDLE rcode:00
16:52:12.897 -> RPIPE rcode:00
16:52:12.897 -> BM configured
16:52:12.944 -> 00 00 00 00 00 00 00 00
(Scroll-Lock)
16:52:32.652 -> DN >47<
16:52:32.652 -> 00 00 47 00 00 00 00 00
16:52:32.792 -> UP >47<
16:52:32.792 -> 00 00 00 00 00 00 00 00

@tmk
Copy link
Contributor

tmk commented Jan 1, 2021

Then, you will have to check return value of SetReport and whether something wrong happens in USB::ctrlReq, dispatchPkt, OutTransfer, and so on. You can narrow part of source code that doesn't work as expected.

Just for reference this is output with Cherry G80-3600 when typing ScrollLock and CapsLock.
Both indicators on the keyboard lit on as expected.

Start
BM Init
Addr:01
NC:01
HID_PROTOCOL_KEYBOARD
bNumEP:02
Cnf:01
bIfaceNum:00
bNumIface:00

Interface:00
PROTOCOL SET HID_BOOT rcode:00
SET_IDLE rcode:00
RPIPE rcode:00
BM configured
00 00 00 00 00 00 00 00 
DN      >47<     
00 00 47 00 00 00 00 00 
UP      >47<     
00 00 00 00 00 00 00 00 
DN      >39<     
00 00 39 00 00 00 00 00 
UP      >39<     
00 00 00 00 00 00 00 00 

@DanielGibson
Copy link
Contributor

DanielGibson commented Jan 1, 2021

SetReport() returns 0 in all cases - should I still dig in the other functions or can I expect that they succeeded if 0 is returned?

Could it be due to differences in the HID descriptors of the keyboards?
I attached the output of sudo lsusb -v (on Linux[1]) for those devices:
usbinfo-cherry.txt
usbinfo-ducky.txt

One difference is that the Cherry keyboard (with PCPlay PS/2 to USB adapter) seems to use only 3 bits for the LEDs (NumLock to Scroll Lock), while the Ducky keyboard uses 5 Bits (up to "Kana").
There's more differences, but TBH I don't understand those descriptors very well (yet) so I'm not sure about their meaning and significance.

[1] By default, lsusb -v will not show the descriptors but will output ** UNAVAILABLE ** instead; for them to work the keyboards USB device(s) need to be "unbound" first. When plugging the keyboard in, there will be a message in dmesg like
[313633.875043] input: DK2108 as /devices/pci0000:00/0000:00:14.0/usb1/1-2/1-2:1.0/0003:0F39:1083.0025/input/input55.
The important part of this is 1-2:1.0 - as root do echo "1-2:1.0" > /sys/bus/usb/drivers/usbhid/unbind.
There might be multiple devices(?), so you might also have to the same for 1-2:1.1 and 1-2:1.2 etc (of course 1-2:1.0 will likely be another value for you), dmesg should mention them, or you can look them up at /sys/devices/pci... (the path from dmesg with /sys/ prepended).

EDIT: In case that's more helpful, the raw HID descriptors as printed by the USBHID_desc.ino example:
Ducky (LEDs don't work):

0000: 05 01 09 06 A1 01 05 07 19 E0 29 E7 15 00 25 01 
0010: 75 01 95 08 81 02 75 08 95 01 81 03 05 08 19 01 
0020: 29 05 75 01 95 05 91 02 75 03 95 01 91 03 05 07 
0030: 19 00 29 A4 15 00 25 A4 75 08 95 06 81 00 C0 

Cherry/PCPlay (LEDs work):

0000: 05 01 09 06 A1 01 05 07 19 E0 29 E7 15 00 25 01 
0010: 75 01 95 08 81 02 95 01 75 08 81 01 95 03 75 01 
0020: 05 08 19 01 29 03 91 02 95 05 75 01 91 01 95 06 
0030: 75 08 26 FF 00 05 07 19 00 29 91 81 00 C0 

@tmk
Copy link
Contributor

tmk commented Jan 1, 2021

I just noticed that Arduino 'Library Manager' support only release versions and you use version 1.3.2(2018 Mar) probably? Try the latest code in this repo. I used the latest code for test before.

EDIT: confirmed that indicator doesn't work with my Cherry G80-3600 with version 1.3.2, but it still works with PD-KB401. With the latest code(9d41161) indicator works for both keyboards.

EDIT2: This USB descriptor of them if you are interested.
Cherry G80-3600: https://gist.github.com/tmk/61232069866950aee280623f4901ff84
PD-KB401: https://gist.github.com/tmk/5f22878a7ddca01e9174e5d6224395d2

@Jonas-commits
Copy link
Author

@tmk upgraded to the master branch, it actually works now 👍. Actually, 1.3.2 was the version I am using, as it is the latest release and therefore the latest release for the Arduino library manager.

@Lauszus maybe it makes sense to issue a new release?

@DanielGibson
Copy link
Contributor

The current git code works for me as well - thanks for the suggestion (should've thought of that myself..)! :)

I agree that a new release would make sense, we're not the only ones running into this problem (see also #160 (comment) and possibly #459)

@tmk
Copy link
Contributor

tmk commented Jan 2, 2021

Just for refrerence,

This commit fixes this problem.
#473

The problem happens only when keyboard replys with NAK on first OUT packet and OUTTransfer retries to send data. Without the commit the data are broken after bytesWr call. tmk#1

@Jonas-commits
Copy link
Author

I will close this issue and create a new one for a new release.

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

No branches or pull requests

3 participants