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

panic! in embassy-usb-synopsys-otg on second serial connection (BUT only if defmt-trace level debugging not enabled) #3493

Open
paulc opened this issue Nov 2, 2024 · 2 comments

Comments

@paulc
Copy link

paulc commented Nov 2, 2024

I had a strange issue running the usb_serial example on an STM32F401 where the serial device would work fine for the first connection from the host, but if I disconnected the terminal session and then re-connected the driver would panic with the following message.

95.600067 [panic_probe::print_defmt::print] lib.rs:104  ERROR panicked at /Users/paulc/Sync/Development/embedded-rust/embassy/embassy-usb-synopsys-otg/src/lib.rs:46:9:
assertion failed: ep_num < ep_count

Very strangely, if I enabled the defmt feature in embassy-stm32 crate and set DEFMT_LOG=trace the problem would go away. As an aside, enabling the defmt feature in the embassy-usb-synopsys-otg results in lots of compile errors with defmt::assert! (see #3492) (constant function errors) which makes debugging a bit tricky.

It looks like the driver is getting an unexpected response from let status = r.grxstsp().read() (status.epnum() was 12 rather than 1 when the error showed up in my testing).

The patch below fixes this by replacing the assert!(ep_num < ep_count) with a test which skips the packet rather than panic-ing.

I suspect that there is a more fundamental issue here but but this does at least fix the panic! issue.

diff --git a/embassy-usb-synopsys-otg/src/lib.rs b/embassy-usb-synopsys-otg/src/lib.rs
index f90403936..3197817ee 100644
--- a/embassy-usb-synopsys-otg/src/lib.rs
+++ b/embassy-usb-synopsys-otg/src/lib.rs
@@ -43,7 +43,16 @@ pub unsafe fn on_interrupt<const MAX_EP_COUNT: usize>(
         let ep_num = status.epnum() as usize;
         let len = status.bcnt() as usize;
 
-        assert!(ep_num < ep_count);
+        if ep_num >= ep_count {
+            // Removed `assert!(ep_num < ep_count);` as this fails for second serial connection UNLESS
+            // defmt-trace level debugging is enabled in embassy-stm32 (note that the defmt feature
+            // doesnt work in embassy-usb-synopsys-otg as defmt::assert! generates multiple errors)
+            //
+            // I'm guessing this may be some sort of timing issue so instead of panic-ing skip the
+            // invalid packet :‑(
+            error!("Skipping Invalid Packet (ep_num >= ep_count): ep_num: {} ep_count: {}", ep_num, ep_count);
+            continue;
+        }
 
         match status.pktstsd() {
             vals::Pktstsd::SETUP_DATA_RX => {
@vDorst
Copy link
Contributor

vDorst commented Nov 3, 2024

I see these panics to with this #3212 (comment) usb audio. But every reset, pressing the reset butten, the device/software behaves differently.

@votrungchi
Copy link
Contributor

I think it related my issue #3459

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

3 participants