Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can we handle read buffer overflow? #35

Open
hacknus opened this issue Jun 2, 2023 · 5 comments
Open

How can we handle read buffer overflow? #35

hacknus opened this issue Jun 2, 2023 · 5 comments

Comments

@hacknus
Copy link

hacknus commented Jun 2, 2023

I have an embedded project on an STM32F405 with USB serial configured and running with FreeRTOS (rust wrapper) as follows:

// Make USB serial device globally available
pub static G_USB_SERIAL: Mutex<RefCell<Option<SerialPort<UsbBus<USB>>>>> =
    Mutex::new(RefCell::new(None));

// Make USB device globally available
pub static G_USB_DEVICE: Mutex<RefCell<Option<UsbDevice<UsbBus<USB>>>>> =
    Mutex::new(RefCell::new(None));

pub unsafe fn usb_init(usb: USB) {
    static mut EP_MEMORY: [u32; 1024] = [0; 1024];
    static mut USB_BUS: Option<UsbBusAllocator<stm32f4xx_hal::otg_fs::UsbBusType>> = None;
    USB_BUS = Some(stm32f4xx_hal::otg_fs::UsbBusType::new(usb, &mut EP_MEMORY));
    let usb_bus = USB_BUS.as_ref().unwrap();
    let serial_port = SerialPort::new(&usb_bus);
    let usb_dev = UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x17c0, 0x28dd))
        .manufacturer("University of Bern")
        .product("Thermometry")
        .serial_number("IceLab814")
        .device_class(usbd_serial::USB_CLASS_CDC)
        .build();
    cortex_m::interrupt::free(|cs| {
        *G_USB_SERIAL.borrow(cs).borrow_mut() = Some(serial_port);
        *G_USB_DEVICE.borrow(cs).borrow_mut() = Some(usb_dev);
    });
}

pub fn usb_read(message: &mut [u8; 1024]) -> bool {
    cortex_m::interrupt::free(|cs| {
        *message = [0; 1024];
        return match G_USB_SERIAL.borrow(cs).borrow_mut().as_mut() {
            None => false,
            Some(serial) => match serial.read(message) {
                Ok(a) => if a < 1024 {
                    true
                } else {
                    false
                },
                Err(_) => false,
            },
        };
    })
}

#[interrupt]
#[allow(non_snake_case)]
fn OTG_FS() {
    cortex_m::interrupt::free(|cs| {
        match G_USB_DEVICE.borrow(cs).borrow_mut().as_mut() {
            None => {}
            Some(usb_dev) => {
                match G_USB_SERIAL.borrow(cs).borrow_mut().as_mut() {
                    None => {}
                    Some(serial) => {
                        // do this regularly to keep connection to USB host
                        usb_dev.poll(&mut [serial]);
                    }
                }
            }
        }
    });
}

initialized in the main function like this:

let mut dp = pac::Peripherals::take().unwrap();

let rcc = dp.RCC.constrain();

let clocks = rcc
    .cfgr
    .use_hse(8.MHz())
    .sysclk(48.MHz())
    .hclk(48.MHz())
    .require_pll48clk()
    .pclk1(24.MHz())
    .pclk2(24.MHz())
    .freeze();

let gpioa = dp.GPIOA.split();

// initialize usb
let usb = USB {
    usb_global: dp.OTG_FS_GLOBAL,
    usb_device: dp.OTG_FS_DEVICE,
    usb_pwrclk: dp.OTG_FS_PWRCLK,
    pin_dm: gpioa.pa11.into_alternate(),
    pin_dp: gpioa.pa12.into_alternate(),
    hclk: clocks.hclk(),
 };
unsafe {
     usb_init(usb);
     cortex_m::peripheral::NVIC::unmask(Interrupt::OTG_FS);
}

my USB task (stack size 2048) does something like this:

loop {
    let mut message_bytes = [0; 1024];
    usb_read(&mut message_bytes);
    // do something
    // print something over usb
}

I noticed that the SerialPort::new(&usb_bus) doc says: "Creates a new USB serial port with the provided UsbBus and 128 byte read/write buffers.". Whenever I send a message longer than 63 characters, the microcontroller gets stuck and requires a reset. How can I handle this case?

@hacknus
Copy link
Author

hacknus commented Jun 6, 2023

I stepped through with the debugger and it seems that the chip does not panic or hardfault, it gets stuck somewhere in the interrupt routine, however by stepping through, I'm not really able to observe where (very long debug output):

cortex_m::interrupt::free<synopsys_usb_otg::bus::{impl#2}::poll::{closure_env#0}<stm32f4xx_hal::otg_fs::USB>, usb_device::bus::PollResult> (f=...)
    at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\cortex-m-0.7.7\src/interrupt.rs:64
64          let r = f(unsafe { &CriticalSection::new() });
(gdb) s

Program stopped.
synopsys_usb_otg::bus::{impl#2}::poll::{closure#0}<stm32f4xx_hal::otg_fs::USB> (cs=0x2001ff9c) at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\synopsys-usb-otg-0.3.2\src/bus.rs:579
579                 let core_id = read_reg!(otg_global, regs.global(), CID);
(gdb) s

Program stopped.
synopsys_usb_otg::target::UsbRegisters::global (self=0x200013ec <thermometry_ctrl_rust::usb::usb_init::USB_BUS+420>)
    at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\synopsys-usb-otg-0.3.2/src\target.rs:51
51              unsafe { &*(self.0 as *const _) }
(gdb) s
halted: PC: 0x080042ee
synopsys_usb_otg::bus::{impl#2}::poll::{closure#0}<stm32f4xx_hal::otg_fs::USB> (cs=0x2001ff9c) at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\synopsys-usb-otg-0.3.2\src/bus.rs:579
579                 let core_id = read_reg!(otg_global, regs.global(), CID);
(gdb) s
synopsys_usb_otg::ral::register::RWRegister<u32>::read<u32> (self=<optimized out>) at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\synopsys-usb-otg-0.3.2/src\ral\register.rs:23
23              unsafe { ::core::ptr::read_volatile(self.register.get()) }
(gdb) s
core::ptr::read_volatile<u32> (src=<optimized out>) at /rustc/88fb1b922b047981fc0cfc62aa1418b4361ae72e/library/core/src/ptr/mod.rs:1552
1552    /rustc/88fb1b922b047981fc0cfc62aa1418b4361ae72e/library/core/src/ptr/mod.rs: No such file or directory.
(gdb) s
halted: PC: 0x080042f2
halted: PC: 0x080042f4
585                 if reset != 0 {
(gdb) s
halted: PC: 0x080042f6
halted: PC: 0x080043bc
595                 if enum_done != 0 {
(gdb) s
halted: PC: 0x080043be
halted: PC: 0x080043c0
636                 } else if wakeup != 0 {
(gdb) s
halted: PC: 0x080043c4
halted: PC: 0x080043c6
641                 } else if suspend != 0 {
(gdb) s
halted: PC: 0x080043c8
halted: PC: 0x080043ca
653                     if rxflvl != 0 {
(gdb) s
halted: PC: 0x080043cc
halted: PC: 0x0800449a
654                         let (epnum, data_size, status) = read_reg!(otg_global, regs.global(), GRXSTSR, EPNUM, BCNT, PKTSTS);
(gdb) s
synopsys_usb_otg::ral::register::RWRegister<u32>::read<u32> (self=0x5000001c) at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\synopsys-usb-otg-0.3.2/src\ral\register.rs:23
23              unsafe { ::core::ptr::read_volatile(self.register.get()) }
(gdb) s
core::ptr::read_volatile<u32> (src=0x5000001c) at /rustc/88fb1b922b047981fc0cfc62aa1418b4361ae72e/library/core/src/ptr/mod.rs:1552
1552    /rustc/88fb1b922b047981fc0cfc62aa1418b4361ae72e/library/core/src/ptr/mod.rs: No such file or directory.
(gdb) s
halted: PC: 0x0800449c
synopsys_usb_otg::bus::{impl#2}::poll::{closure#0}<stm32f4xx_hal::otg_fs::USB> (cs=0x2001ff9c) at C:\Users\ls21y493\.cargo\registry\src\index.crates.io-6f17d22bba15001f\synopsys-usb-otg-0.3.2\src/bus.rs:654
654                         let (epnum, data_size, status) = read_reg!(otg_global, regs.global(), GRXSTSR, EPNUM, BCNT, PKTSTS);

@AzazKamaz
Copy link

AzazKamaz commented Jun 19, 2024

Actually this crate has an internal buffer that is enough only for usb 1.1 mode which only allows small packets
Here it is:

pub struct DefaultBufferStore([u8; 128]);

With usb 2.0 packet size can go up to 512 bytes I guess, so just put some bigger number in there (for example 1024) and it would be good (at least helped for me last time I tried)

@AzazKamaz
Copy link

Or you can just use imxrt_usbd::Speed::LowFull instead of imxrt_usbd::Speed::High

@hacknus
Copy link
Author

hacknus commented Jun 19, 2024

@AzazKamaz thanks for the input! I am however not sure where I should place this import and/or adjust the buffer in my user-side code? Or would this require a PR / update in the library?

@AzazKamaz
Copy link

@hacknus so the easy way to just get it working reliably is to switch to lower speed. It is done on the user side when you init usbd. I'm not really sure if this is available in any of the implementations/mcus, I'm currently using imxrt. I changed it here: https://github.com/imxrt-rs/imxrt-hal/blob/0cb3b8d320e3d2a0e43304f4beca06efba261af1/examples/rtic_usb_serial.rs#L41

I made a patch just for me, maybe will upstream it later, it should fix the issue and get high speed work properly: https://github.com/AzazKamaz/usbd-serial/tree/30b6cff9906be68267fe0960ebfba5b4754c9c80

Also I'm not really sure if after this patch it will be able to act as a full speed device since settings are different... It should in theory do some dynamic configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants