rust-embedded-community / usb-device

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

Device builder should consume the bus allocator #105

Open parasyte opened 2 years ago

parasyte commented 2 years ago

First a little context. I wasted a lot of time diagnosing why I couldn't create a usbd_serial::SerialPort. The reason is because I was creating the SerialPort after the UsbDevice. The device builder will freeze the UsbBusAllocator, disallowing any future uses. This causes SerialPort::new() to panic. I don't have a debugger setup for my target at the moment, so runtime panics are hard to debug!

What do I expect to happen? The following code should not compile:

let mut usb_dev = UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x16c0, 0x27dd))
    .product("Serial port")
    .device_class(USB_CLASS_CDC)
    .build();

// ERROR: `usb_bus` is frozen by the call to `build()`.
let mut serial = SerialPort::new(&usb_bus);

An easy way to prevent this UX issue is for UsbDeviceBuilder::new() to take ownership of the UsbBusAllocator:

// Notice `usb_bus` is moved here.
let mut usb_dev = UsbDeviceBuilder::new(usb_bus, UsbVidPid(0x16c0, 0x27dd))
    .product("Serial port")
    .device_class(USB_CLASS_CDC)
    .build();

// Compile error: attempt to access moved value `usb_bus`.
let mut serial = SerialPort::new(&usb_bus);

Perhaps there are valid use cases for the device builder to borrow the bus allocator. "Flexibility" is often cited in similar situations, and it always leads to extra complexity such as interior mutability and the kind of invalid states and runtime panics shown here.

I'm also aware this is a breaking change and could potentially end up being a great deal of work. I believe that it will provide a less error-prone interface and may even afford you a way to clean up some of the library internals. For these reasons, I think it is worth considering.

parasyte commented 2 years ago

I am thinking this suggestion won't work because the allocator needs to outlive the SerialPort.

The suggestions in #9 are tangentially related. The borrowing means that these types cannot easily be moved to a struct because it would create self-references.