rust-embedded-community / usb-device

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

Add a dummy usb bus implementation #152

Closed sourcebox closed 3 months ago

sourcebox commented 4 months ago

This PR adds DummyUsbBuswhich doesn't implement any functionality but can be used when writing examples (e.g. on class implementations) to ensure they compile. Note that any code using it cannot be run though.

sourcebox commented 3 months ago

Without a dummy implementation, a real HAL is needed to compile the code. So you can't just setup an example in your class crate that is checked for syntax correctness. A good example is the current usbd-serial crate that has an example in the readme that is outdated and there's no way to have it checked. So a user that copies it from there will end up with errors thrown.

A working version of that example using the dummy implementation could be like this:

use usb_device::{
    bus::UsbBusAllocator,
    device::{UsbDeviceBuilder, UsbVidPid},
    dummy_bus::DummyUsbBus,
    prelude::*,
};
use usbd_serial::{SerialPort, USB_CLASS_CDC};

fn main() {
    let usb_bus = UsbBusAllocator::new(DummyUsbBus::new());

    let mut serial = SerialPort::new(&usb_bus);

    let mut control_buffer = [0; 256];

    let string_descriptors = [StringDescriptors::new(LangID::EN_US).product("Serial port")];

    let mut usb_dev =
        UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x16c0, 0x27dd), &mut control_buffer)
            .strings(&string_descriptors)
            .unwrap()
            .device_class(USB_CLASS_CDC)
            .build()
            .unwrap();

    loop {
        if !usb_dev.poll(&mut [&mut serial]) {
            continue;
        }

        let mut buf = [0u8; 64];

        match serial.read(&mut buf[..]) {
            Ok(count) => {
                // count bytes were read to &buf[..count]
            }
            Err(UsbError::WouldBlock) => todo!(), // No data received
            Err(err) => todo!(),                  // An error occurred
        };

        match serial.write(&[0x3a, 0x29]) {
            Ok(count) => {
                // count bytes were written
            }
            Err(UsbError::WouldBlock) => todo!(), // No data could be written (buffers full)
            Err(err) => todo!(),                  // An error occurred
        };
    }
}
eldruin commented 3 months ago

Yes, I understood the motivation. My question is if it is worth providing dummy implementations inside this crate, instead of adding it in-place if we have a very limited number of examples (for the documentation, this dummy implementation could be added hidden so that it is actually compiled but not shown, like we do over at embedded-hal) Also, I wonder if we will want to add dummy implementations for the rest of traits provided here.

use usb_device::{
    bus::UsbBusAllocator,
    device::{UsbDeviceBuilder, UsbVidPid},
    dummy_bus::DummyUsbBus,
    prelude::*,
};
use usbd_serial::{SerialPort, USB_CLASS_CDC};

fn main() {
    let usb_bus = UsbBusAllocator::new(DummyUsbBus::new());

    let mut serial = SerialPort::new(&usb_bus);

    let mut control_buffer = [0; 256];

    let string_descriptors = [StringDescriptors::new(LangID::EN_US).product("Serial port")];

    let mut usb_dev =
        UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x16c0, 0x27dd), &mut control_buffer)
            .strings(&string_descriptors)
            .unwrap()
            .device_class(USB_CLASS_CDC)
            .build()
            .unwrap();

    loop {
        if !usb_dev.poll(&mut [&mut serial]) {
            continue;
        }

        let mut buf = [0u8; 64];

        match serial.read(&mut buf[..]) {
            Ok(count) => {
                // count bytes were read to &buf[..count]
            }
            Err(UsbError::WouldBlock) => todo!(), // No data received
            Err(err) => todo!(),                  // An error occurred
        };

        match serial.write(&[0x3a, 0x29]) {
            Ok(count) => {
                // count bytes were written
            }
            Err(UsbError::WouldBlock) => todo!(), // No data could be written (buffers full)
            Err(err) => todo!(),                  // An error occurred
        };
    }
}

// In the docs we can prepend all this with # to hide it
struct DummyUsbBus;

impl DummyUsbBus {
    fn new() -> Self {
        Self
    }
}

impl UsbBus for DummyUsbBus {
    fn alloc_ep(
        &mut self,
        ep_dir: crate::UsbDirection,
        ep_addr: Option<crate::class_prelude::EndpointAddress>,
        ep_type: crate::class_prelude::EndpointType,
        max_packet_size: u16,
        interval: u8,
    ) -> crate::Result<crate::class_prelude::EndpointAddress> {  }
    fn enable(&mut self) {   }
    fn force_reset(&self) -> crate::Result<()> {  }
    fn is_stalled(&self, ep_addr: crate::class_prelude::EndpointAddress) -> bool {  }
    fn poll(&self) -> crate::bus::PollResult {   }
    fn read(
        &self,
        ep_addr: crate::class_prelude::EndpointAddress,
        buf: &mut [u8],
    ) -> crate::Result<usize> {  }
    fn reset(&self) {   }
    fn resume(&self) {   }
    fn set_device_address(&self, addr: u8) {  }
}
sourcebox commented 3 months ago

I understand that this dummy implementation feels a bit strange, but I have no better idea how to make testing easy for class crate authors. Rust in general motivates people to write examples that actually build, so this is just a convinient way to support this. I see it a bit similar to the test class that already exists in the crate.

The alternatives are:

eldruin commented 3 months ago

Hmm, I understand the situation better now, thanks. This is similar for driver crates, where I add a dev dependency on linux-embedded-hal so that I can get an I2C/SPI bus in the examples and docs. For that price, I get examples that compile on the popular Raspberry Pis. If there is not such a little-hassle HAL impl and platform for usb-device, I understand how the situation is more complicated.

Doing away with this by providing a dummy implementation first-party is certainly alluring from an engineering perspective. Conversely, the examples will compile but they need to be translated to a real HAL impl, which is a non-trivial burden.

If my recollection of the situation is correct, I am inclined to accept this (although maybe rename the module just dummy for possible future extension). What do you think @ryan-summers ?

ryan-summers commented 3 months ago

My only request is that we gate the dummy implementation behind a feature to prevent people from using it in actual projects (or at least hide it).

In the past, I've maintained these as separate crates (i.e. std-embedded-nal), but it has been a painful maintenance process. I like keeping it in a single location.

sourcebox commented 3 months ago

My only request is that we gate the dummy implementation behind a feature to prevent people from using it in actual projects (or at least hide it).

A good idea in general, but I'm not sure how the class crate authors should use it i.e. whether

sourcebox commented 3 months ago

One problem with features in general is though that patch.crates-io doesn't support them irc. Unfortunately, this crate is somewhat prone to be force-patched to ensure that all involved crates use the same version.

vitalyvb commented 3 months ago

Having a dummy bus would have been useful for one documentation example I had to write, but it's not a huge trait, and basically IDE does it automatically anyway (link). If would be significantly more useful if there were a lot of examples needing the bus, though.