rust-embedded-community / usbd-serial

Work-in progress minimal CDC-ACM (USB serial port) class for usb-device
MIT License
116 stars 35 forks source link

support naming the CDC subinterfaces #31

Closed iostat closed 1 year ago

iostat commented 1 year ago

Hello!

If you're making a composite device with multiple CDC/SerialPort interfaces it's often useful to label them to be able to determine which serial port on the host machine corresponds to which interface.

This PR lets you specify name strings for the control and data interfaces that the host can query.

iostat commented 1 year ago

Changelog amended!

mangelajo commented 1 year ago

@eldruin can we merge this?, I'd find it very useful.

Best, Miguel

mangelajo commented 1 year ago

@iostat do you have an example for multiple interfaces?, I am trying that with no luck. I need to check with gdb but I get nothing enumerated as soon as I add two.

iostat commented 1 year ago

@iostat do you have an example for multiple interfaces?, I am trying that with no luck. I need to check with gdb but I get nothing enumerated as soon as I add two.

I can make a minimal one -- but first I'd ask what hardware platform you're using. For example, on the RP2040, you may be running into this: https://docs.rs/rp2040-hal/latest/rp2040_hal/usb/index.html#enumeration-issue-with-small-ep0-max-packet-size

iostat commented 1 year ago

Actually might not be that exact issue -- but basically you need to increase the max size of one of the buffers so that the descriptors can actually fit in the response. Lemme see if I can dig it up for you

iostat commented 1 year ago

https://github.com/rust-embedded-community/usb-device/blob/master/Cargo.toml#L22

See if enabling that feature in usb-device helps. You may have to enable it transitively unless you're depending on usb-device directly.

It's been a while since I did this but it would look something like:

# Cargo.toml
usb-device = { version = "...", features = ["control-buffer-256"] }

# or, eg, for rp-pico:

rp-pico = { version = "...", features = [..., "usb-device/control-buffer-256"] }

hope that helps!

mangelajo commented 1 year ago

Hey @iostat thanks for the quick response, I am using a stm32f411, and I tried control-buffer-256, but no luck...

[dependencies]
cortex-m = "0.7.7"
cortex-m-rt = "0.7.3"
stm32f4xx-hal = { version = "0.15.0", features = ["stm32f411", "usb_fs", "rt"] }
stm32f4 = { version = "0.15.1", features = ["stm32f411", "rt"]}
panic-halt= "0.2.0"
usb-device = { version="0.2.9", features = ["control-buffer-256"] }

I am trying this way:


    let usb_bus = unsafe {UsbBus::new(usb_periph, &mut EP_MEMORY )};

    let mut serial = SerialPort::new(&usb_bus);
    let mut serial2 = SerialPort::new(&usb_bus);
    let mut dfu = DfuRuntimeClass::new(&usb_bus, DFUBootloader);

    let mut usb_dev = UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x2b23, 0x1012))
        .manufacturer("Red Hat Inc.")
        .product("Jumpstarter")
        .serial_number(get_serial_str())
        .device_release(0x0001) 
        .self_powered(false)
        .max_power(250)
        .max_packet_size_0(64)
        .device_class(USB_CLASS_CDC)
        .build();

I guess I will need to setup my st-link and openocd and find where this is crashing :D

mangelajo commented 1 year ago

I am working on this https://github.com/mangelajo/jumpstarter#jumpstarter to enable setting up embedded software / SoC images to be tested in github/gitlab pipelines.

And I was thinking of setting up two ports per board, one for commands, and another one to talk to the embedded target.

mangelajo commented 1 year ago

Ok, so I got here:

Breakpoint 1, jumpstarter::__cortex_m_rt_main () at src/main.rs:91
91      let mut serial = SerialPort::new(&usb_bus);
(gdb) n
halted: PC: 0x080095d2
halted: PC: 0x080102e0
halted: PC: 0x080102e4
92      let mut serial2 = SerialPort::new(&usb_bus);
(gdb) n
halted: PC: 0x080095d2
^C
Program received signal SIGINT, Interrupt.
0x0801c17e in core::sync::atomic::compiler_fence (order=core::sync::atomic::Ordering::SeqCst)
    at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/sync/atomic.rs:3445
3445    /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/sync/atomic.rs: No such file or directory.
(gdb) bt
#0  0x0801c17e in core::sync::atomic::compiler_fence (order=core::sync::atomic::Ordering::SeqCst)
    at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/sync/atomic.rs:3445
#1  0x0801c176 in panic_halt::panic (_info=0x2001ecbc) at src/lib.rs:33
#2  0x0801c2e6 in core::panicking::panic_fmt () at library/core/src/panicking.rs:64
#3  0x0801c3e4 in core::result::unwrap_failed () at library/core/src/result.rs:1790
#4  0x080115a8 in core::result::Result<usb_device::endpoint::Endpoint<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>, usb_device::endpoint::In>, usb_device::UsbError>::expect<usb_device::endpoint::Endpoint<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>, usb_device::endpoint::In>, usb_device::UsbError> (self=...,
    msg=...) at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/result.rs:1069
#5  0x08010cd6 in usb_device::bus::UsbBusAllocator<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>>::bulk<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>, usb_device::endpoint::In> (self=0x2001f6ac, max_packet_size=64) at /home/majopela/.cargo/registry/src/github.com-1ecc6299db9ec823/usb-device-0.2.9/src/bus.rs:259
#6  0x0800d580 in usbd_serial::cdc_acm::CdcAcmClass<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>>::new<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>> (alloc=0x2001f6ac, max_packet_size=64) at /home/majopela/.cargo/registry/src/github.com-1ecc6299db9ec823/usbd-serial-0.1.1/src/cdc_acm.rs:60
#7  0x08009686 in usbd_serial::serial_port::SerialPort<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>, usbd_serial::buffer::DefaultBufferStore, usbd_serial::buffer::DefaultBufferStore>::new_with_store<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>, usbd_serial::buffer::DefaultBufferStore, usbd_serial::buffer::DefaultBufferStore> (alloc=0x2001f6ac, read_store=..., write_store=...) at /home/majopela/.cargo/registry/src/github.com-1ecc6299db9ec823/usbd-serial-0.1.1/src/serial_port.rs:70
#8  0x08009662 in usbd_serial::serial_port::SerialPort<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>, usbd_serial::buffer::DefaultBufferStore, usbd_serial::buffer::DefaultBufferStore>::new<synopsys_usb_otg::bus::UsbBus<stm32f4xx_hal::otg_fs::USB>> (alloc=0x2001f6ac)
    at /home/majopela/.cargo/registry/src/github.com-1ecc6299db9ec823/usbd-serial-0.1.1/src/serial_port.rs:52
 26     /// Allocates a bulk endpoint.
 25     ///
 24     /// # Arguments
 23     ///
 22     /// * `max_packet_size` - Maximum packet size in bytes. Must be one of 8, 16, 32 or 64.
 21     ///
 20     /// # Panics
 19     ///
 18     /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
 17     /// feasibly recoverable.
 16     #[inline]
 15     pub fn bulk<D: EndpointDirection>(&self, max_packet_size: u16) -> Endpoint<'_, B, D> {
 14         self.alloc(None, EndpointType::Bulk, max_packet_size, 0)
 13             .expect("alloc_ep failed")
 12     }

I have increased endpoint memory to 2K https://github.com/redhat-et/jumpstarter/blob/ab76f7f5003eb72ff029e203703f7239002979d4/test-harness/firmware/application/src/main.rs#L23

Am I doing it right? or should I add 2 ports in a different way?

I'll keep debugging

mangelajo commented 1 year ago

I suspect the driver (/the device) is running out of endpoints, I see it only has 4 endpoints in each direction for this model in particular :-/

But Isn't it only 2 endpoints per Serial? (IN+OUT?) or are there more?

iostat commented 1 year ago

I suspect the driver (/the device) is running out of endpoints, I see it only has 4 endpoints in each direction for this model in particular :-/

Unfortunately, that appears to be case. CDC-ACM needs /three/ endpoints per serial port: data in/out as you mentioned, as well as a control in-endpoint (distinct from EP0 used for overall enumeration/control.)

iostat commented 1 year ago

@eldruin Thanks for the merge!

@mangelajo I'd consider an F446 for your use case. As far as I'm aware that's the only one in the F4 family with more than 6 hard endpoints in the USB peripheral. It comes in similar 64- and 100-pin packages as the F411, so I suspect there might be substantial pin-compatibility, if not even a straight drop-in.

mangelajo commented 1 year ago

Thanks for the hint! I will have a look.

I will also consider the F412 compatible and has 6 endpoints instead of 4. For the F412 it seems to compile at least, I also tried to run it on the F411 just in case but of course the extra endpoints are silent :-) at least enumeration works .

On Mon, 17 Jul 2023 at 13:41 Ilya Ostrovskiy @.***> wrote:

@eldruin https://github.com/eldruin Thanks for the merge!

@mangelajo https://github.com/mangelajo I'd consider an F446 for your use case. As far as I'm aware that's the only one in the F4 family with more than 6 hard endpoints in the USB peripheral. It comes in similar 64- and 100-pin packages as the F411, so I suspect there might be substantial pin/compatibility.

— Reply to this email directly, view it on GitHub https://github.com/rust-embedded-community/usbd-serial/pull/31#issuecomment-1637958364, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAI7G4QQ3WQV256SAKPUEDDXQUQGLANCNFSM6AAAAAASLKRERE . You are receiving this because you were mentioned.Message ID: @.***>

-- Miguel Ángel Ajo @mangel_ajo https://twitter.com/mangel_ajo Office of CTO, Emerging Technologies, EDGE team.