twitchyliquid64 / usbd-hid

MIT License
87 stars 37 forks source link

Sticky volume keys when using MediaKeyboardReport #39

Closed 0xa10 closed 2 years ago

0xa10 commented 2 years ago

Im writing a USB volume control using a Pi Pico - and Im having issues with the volume control sticking or repeating on either direction (up or down) intermittently.

At first I thought that it was some sort of priority issue with the USB IRQ, but after going through the USB HID documentation I noticed that the media volume/up keys (0xe9 and 0xea usage ids) are marked as RTC or re-triggerable - which says they are re-triggered for as long as the key is "asserted."

Im not sure what asserted means in that regard - how does the USB host know when the key is released? I tried sending an additional Zero report right after the media key report - but that had no effect.

When debugging it I also noticed that whenever the key does "stick" (aka repeat), for as long as its repeated and sometimes for a few moments after my actual keyboard clicks on my actual keyboard don't register - which leads me back to believe that this has more to do with the USB device hogging the bus somehow.

A few more anecdotes -

  1. I looked at captures of the USB bus using Wireshark and USBcap - the messages do not repeat.
  2. the program doesn't work at all connected to my Mac - the USB IRQ doesnt fire and there's no USB communications, apart from a few messages at the beginning

Any ideas?

haata commented 2 years ago

There are two different Volume Up/Downs for USB. And different OSs support different ones.

I've also noticed that I often need to specify both to maximize Volume control compatibility as an OS tends to support one or the other.

https://github.com/kiibohd/controller/issues/57 This was the last time I dealt with this particular issue (was for C firmware, but generally these kinds of issues are USB descriptor + OS related problems unless you're sending too many reports/handling repeat rate yourself).

dlkj commented 2 years ago

Most off-the-shelf keyboards use the consumer control page for OS volume control. There are a few ways to do this. For a button you would send a report indicating the button being depressed, and a 2nd report when the button is released.

For a volume knob you would be better off sending a report with the change in the volume level - similar to a mouse wheel for a mouse report.

Take a look at the HID docs for the Input item: https://www.usb.org/sites/default/files/hid1_11.pdf page 29

I have some examples based on a rp2040 you can take a look at: https://github.com/dlkj/usbd-human-interface-device/blob/main/examples/src/bin/consumer_multiple.rs and https://github.com/dlkj/usbd-human-interface-device/blob/main/src/device/consumer.rs

0xa10 commented 2 years ago

Thanks for your answers and those examples, I haven't seen those before. With regards to my issue - I took a look at the captures after adding code that "releases" the key by sending a zero-filled packet and noticed that the second packet didn't always make it to the host endpoint. Im not quite sure if it's a scheduling issue where my USB_IRQ isn't called in time, or perhaps it's related to the polling interval - but adding a 10ms delay between sending the first and second packet seems to have resolved it.

I'm not sure if this is the intended behavior, but even if it I assume an error should somehow propagate up. I see "WouldBlock" errors every 5-10 packets but they aren't at all correlated with the behavior Im describing so Im not sure how thats related.

0xa10 commented 2 years ago

Most off-the-shelf keyboards use the consumer control page for OS volume control. There are a few ways to do this. For a button you would send a report indicating the button being depressed, and a 2nd report when the button is released.

For a volume knob you would be better off sending a report with the change in the volume level - similar to a mouse wheel for a mouse report.

Take a look at the HID docs for the Input item: https://www.usb.org/sites/default/files/hid1_11.pdf page 29

I have some examples based on a rp2040 you can take a look at: https://github.com/dlkj/usbd-human-interface-device/blob/main/examples/src/bin/consumer_multiple.rs and https://github.com/dlkj/usbd-human-interface-device/blob/main/src/device/consumer.rs

There are two different Volume Up/Downs for USB. And different OSs support different ones.

I've also noticed that I often need to specify both to maximize Volume control compatibility as an OS tends to support one or the other.

kiibohd/controller#57 This was the last time I dealt with this particular issue (was for C firmware, but generally these kinds of issues are USB descriptor + OS related problems unless you're sending too many reports/handling repeat rate yourself).

Thanks, Im using the media key reports and not standard keyboard reports which lines up with what's being said in the issue you linked. With regards to implementing this using usbd-hid - would it be possible to instantiate multiple consumers on the same USBDevice instance? Or is there some other way of feeding in both report types?

dlkj commented 2 years ago

You can mix items from different usb hid pages in the same report. Keyboards tend to avoid this to be comparable with the simplified usb hid boot descriptors - so they can work with a bios or other reduced usb functionality environment. They implement multiple interfaces to support multiple reports. You can have multiple reports on a single interfaces if you use report IDs (also not supported by boot descriptors) or multiple usb devices implemented by a single hardware device.

https://www.usbmadesimple.co.uk/index.html and https://www.beyondlogic.org/usbnutshell/usb1.shtml cover all this in some detail.

Switch between pages using a usage page item in the descriptor. See a typical mouse descriptor to see how you can combine data from multiple pages in a single descriptor/report:

https://www.usbmadesimple.co.uk/ums_5.htm

You can define a report that contains an array of arbitrary consumer control page values or a report that has a bitmap of a fixed set of consumer control page items that can be activated. In your case you may want to define the absolute or relative change in volume as a report value - this would be similar to the way you implement a mouse wheel.

On Tue, 17 May 2022, at 8:15 PM, Alon Livne wrote:

Most off-the-shelf keyboards use the consumer control page for OS volume control. There are a few ways to do this. For a button you would send a report indicating the button being depressed, and a 2nd report when the button is released.

For a volume knob you would be better off sending a report with the change in the volume level - similar to a mouse wheel for a mouse report.

Take a look at the HID docs for the Input item: https://www.usb.org/sites/default/files/hid1_11.pdf page 29

I have some examples based on a rp2040 you can take a look at: https://github.com/dlkj/usbd-human-interface-device/blob/main/examples/src/bin/consumer_multiple.rs and https://github.com/dlkj/usbd-human-interface-device/blob/main/src/device/consumer.rs

There are two different Volume Up/Downs for USB. And different OSs support different ones.

kiibohd/controller#57 https://github.com/kiibohd/controller/issues/57 This was the last time I dealt with this particular issue (was for C firmware, but generally these kinds of issues are USB descriptor + OS related problems unless you're sending too many reports/handling repeat rate yourself).

Thanks, Im using the media key reports and not standard keyboard reports which lines up with what's being said in the issue you linked. With regards to implementing this using usbd-hid - would it be possible to instantiate multiple consumers on the same USBDevice instance? Or is there some other way of feeding in both report types?

— Reply to this email directly, view it on GitHub https://github.com/twitchyliquid64/usbd-hid/issues/39#issuecomment-1129226816, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB3K45SSTHMJ4UTNUUAEYDVKPV45ANCNFSM5WFF6XUQ. You are receiving this because you commented.Message ID: @.***>

0xa10 commented 2 years ago

Not sure if I understand correctly - is sending multiple items in a report, or switching usage pages supported given the current API in usbd-hid? Im also curious about implementing a value-driven volume control - I havent seen any examples (in Rust or otherwise) where controlling the volume is done in a non-incremental manner. Could you direct me towards some sources?

0xa10 commented 2 years ago

Hi, I'd like to raise this question again here before opening a new issue - is there any way to implement composite HID devices using this library? I'd like to send both Consumer and Keyboard reports.

twitchyliquid64 commented 2 years ago

Yes- have your HID descriptor describe two reports: one for the keyboard and one for the consumer stuff. Then, when you send a report prefix it with the corresponding report ID you need for either consumer or keyboard.

I assume that’s what you mean by composite device?

On Tue, Jun 14, 2022 at 9:07 AM Alon Livne @.***> wrote:

Hi, I'd like to raise this question again here before opening a new issue

  • is there any way to implement composite HID devices using this library? I'd like to send both Consumer and Keyboard reports.

— Reply to this email directly, view it on GitHub https://github.com/twitchyliquid64/usbd-hid/issues/39#issuecomment-1155162096, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQJCDK4NADYUGG52JF3YGDVPB7Z7ANCNFSM5WFF6XUQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

0xa10 commented 2 years ago

That is indeed what I mean - but Im not sure how to accomplish this without making modifications to the library. When constructing the HidClass object - how do I feed it more than one descriptor? The closest thing I could think of would be creating my own composite report descriptor comprising both descriptors, but I'm not at all sure that would be best practice

0xa10 commented 2 years ago

Okay, reading further into custom keyboard layout it seems like the way to do this would be to assemble a custom HID descriptor with just the buttons I need (volume/up down + mute in the keyboard usage, volume increment/decrement + mute in the consumer control page). Will give it a go.

twitchyliquid64 commented 2 years ago

Sounds good. If you did want to use multiple reports instead, you do that by using the report_id bit when computing the HID descriptor. E.G:

https://github.com/twitchyliquid64/usbd-hid/blob/d0a80008476d74ef2017908ce256dc56bfc396b1/macros/src/lib.rs#L85-L88

When you then push some data, you need to use push_raw_input and prefix your report with the report ID your data corresponds to.

0xa10 commented 2 years ago

Thanks - will give it a shot. Much appreciated