Trouble sending large report descriptors
Closed this issue · 9 comments
I am trying to implement a multitouch touchpad device, according to Windows Precision Touchpad specification.
So I started editing this crate's code as to have it use the sample report descriptor given here: https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-sample-report-descriptors.
Unfortunately, this does not work, my system does not even create the device (in kernel log, usbhid module yields an error -32... which should be a broken pipe error).
By fiddling and editing the descriptor in one of the provided examples, I found out that any (syntactically correct) descriptor shorter or equal to 128 bytes is accepted, and the hid-multitouch device will be created (provided a digitizer collection is defined). Any longer descriptor triggers the error.
I thought there was some hardware limitation (by the way, I am running these tests on a RP2040-zero), but it definitely is not. Ideed, if I generate large descriptors using the crate usbd-hid instead, there is no such issue. Same when I try with a CircuitPython firmware. Hence, with my limited knowledge, I am inclined to believe there is a bug in `usbd-human-interface-device' crate.
To sum up, the reproduction is just that :
- take one of the examples (keyboard-custom.rs for instance), adapt it to waveshare rp2040-zero
- run it: it works
- replace the descriptor by a longer one (more than 128 bytes)
- run the example : no device is created, host linux kernel log has lines of the form:
[400471.041635] usbhid 1-2:1.0: can't add hid device: -32
[400471.041644] usbhid: probe of 1-2:1.0 failed with error -32
Btw, I repeated the test with a Pi Pico Clone, with the same results.
Update: by comparing with usbd-hid, I found out that in the end, the difference was that in that crate the descriptor was sent thanks to function usb_device::control_pipe::ControlIn::accept_with_static, while in usbd-human-interface-device, the function usb_device::control_pipe::ControlIn::accept_with is used instead.
The static version comes with the following comment:
This method is useful when you have a large static descriptor to send as one packet.
Since trying to send a large static descriptor was exactly what I was trying to do, I changed the call in usb_class::USBHidClass::get_descriptor for the other function (and changed a few lifetimes here and there to 'static)... and that fixed my issue!
Is there any drawback in sending the report descriptor using accept_with_static?
Hi @ald0o. Thanks so much for investigating this and raising an issue. The main difference between accept_with and accept_with_static, other than the bug you've found in my crate, is that accept_with_static requires the buffer to have a static lifetime, i.e. defined at compile time or from a global compile time allocated buffer.
When I have some time I'll dig into the implementation differences and either fix the issue with accept_with or move to accept_with_static.
Cheers, Daniel
I'd already raised #117 to move away from accept_with to either accept or accept_with_static as it currently requires the data to be copied between buffers.
Ok so, if I understand well, this "fix" actually comes with a loss of functionality in the case somebody wants to create descriptors at run time. I suppose it is probably not a very common use case for embedded devices, but who knows?
Correct. I've never seen it done, but it would technically be a regression. If I'm happy to make that trade-off if it can't be made to work with accepts or accepts_with. I think large descriptors are more useful than runtime configurable descriptors.
https://github.com/rust-embedded-community/usb-device/blob/3a2f78ef7b9d57db37a823d8dd0d1b6f54b050f9/src/control_pipe.rs#L203 has some logic for writing static data that is longer than the packet size. I'd need to dig into this and see if you can do the same with non static data.
Related tickets in rust-embedded-community/usb-device:
@ald0o I've added a new constructor to InterfaceBuilder that allows you to use a static descriptor of any length: InterfaceBuilder::with_static_descriptor. The existing constructor should now fail if you use a descriptor of >128 bytes.
https://github.com/dlkj/usbd-human-interface-device/tree/static_descriptor
Does this work for you?