rust-embedded-community / usb-device

Experimental device-side USB framework for microcontrollers in Rust.
MIT License
414 stars 77 forks source link

Add control-buffer-512 and -1024 feature flags #62

Closed Windfisch closed 3 months ago

Windfisch commented 3 years ago

Hi,

thank you for your great work, it's great to see Rust becoming more and more capable also in the embedded domain!

Unfortunately, the usb-device crate provides a rather limited control buffer which is used for descriptors. This limits the descriptor size to a maximum of 256 when using the control-buffer-256 feature flag. For USB-MIDI, for example, this is only enough for 5 in + 5 out ports, while up to 1024 bytes can be needed for larger descriptors providing up to 16 ports.

(See https://github.com/btrepp/usbd-midi/pull/6 for a scenario where this is relevant).

I added two additional feature flags to provide buffer sizes of 512 and 1024 bytes, respectively. I admit this seems a bit hacky, maybe resorting to the typenum crate or waiting for const generics is the better way in the long run.

However, for now, I think that these additional feature flags bring the chance to greatly improve the usefulness of usb-device.

Feel free to contact me in case of any questions or change requests. :)

ryan-summers commented 1 year ago

Given that const generics were long-ago implemented, I'd be happy to see a PR doing this with const-generics on the usb class :) I dislike the many feature flags this approach implements (although I recognize this PR was made before const-generics stabilized).

If you'd be willing to update this code for const-generics, I would be happy to merge this @Windfisch - sorry for taking so long in getting to things :)

mvirkkunen commented 1 year ago

@ryan-summers fwiw I tried that once and the const generic argument ends up threading through everything making for annoying types. Passing in a buffer from the outside as a slice would be nicer.

ryan-summers commented 1 year ago

Doesn't that approach just replace the const-generic with a generic lifetime instead? (unless we just require it to be 'static

mvirkkunen commented 1 year ago

Yes, it would have to be 'static.