Fix serial file transfer on macOS and 3rd party serial tool - #3248
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the USB serial (CDC-style) implementation to behave correctly with USB 2.0 high-speed bulk requirements and with host tools (notably macOS and some third‑party serial utilities) that are stricter about packet sizing, ZLP termination, and CDC control requests.
Changes:
- Make high-speed bulk endpoints compliant by using 512-byte
wMaxPacketSizeand matching transfer buffer sizes. - Fix TX/RX transfer robustness by ensuring USB DMA buffers outlive the scheduling call (avoid stack-backed buffers) and by increasing bulk OUT queue depth.
- Improve host interoperability by enabling ZLP for bulk IN, treating configuration/line coding as “channel opened”, and returning a proper
GET_LINE_CODINGpayload.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| firmware/application/usb_serial_shell_filesystem.cpp | Avoid stack overflow in binary SD write by moving the USB bulk buffer to static storage. |
| firmware/application/usb_serial_host_to_device.hpp | Stop redefining bulk buffer size locally; rely on the shared USB serial header. |
| firmware/application/usb_serial_host_to_device.cpp | Mark ISR-written transfer metadata as volatile for thread-context polling. |
| firmware/application/usb_serial_endpoints.c | Document bulk IN queue constraint; increase bulk OUT queue depth to allow more in-flight reads. |
| firmware/application/usb_serial_device_to_host.h | Increase bulk buffer / serial buffer sizing defaults to 512 to match HS bulk max packet. |
| firmware/application/usb_serial_device_to_host.c | Replace stack TX buffer with static ping-pong buffers suitable for DMA lifetime. |
| firmware/application/usb_serial_descriptor.c | Correct HS bulk max packet size (512) and fix endpoint descriptor comments/interval. |
| firmware/application/usb_serial_cdc.c | Improve USB IRQ wakeup reliability; enable bulk IN ZLP; return/track CDC line coding; open channel earlier for tools that don’t assert DTR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifndef USBSERIAL_BUFFERS_SIZE | ||
| #define USBSERIAL_BUFFERS_SIZE 128 | ||
| #define USBSERIAL_BUFFERS_SIZE 512 | ||
| #endif |
There was a problem hiding this comment.
i tested it and it behaves stable at this val.
this is a safe trade off, we have so many code that can stackoverflow anyway. and not worth to have the buffer static to waist RAM.
|
|
||
| size_t size = (size_t)strtol(argv[0], NULL, 10); | ||
|
|
||
| chprintf(chp, "send %d bytes\r\n", size); |
There was a problem hiding this comment.
it is safe that max number of here could only be 100000, it is impossible to greater than INT_MAX and I believe the cast is not worth for more asm instructions it brings, i guess 1 more tick
IMPORTANT
Merge this firstly, and merge portapack-mayhem/MayhemHub#108 after next mayhem FW release.
Brief description of what you did
Proof that your changes work
In discord channels
🖥️ Proof it compiles
Space remaining in flash ROM: 5044 bytes ( 0.5 %)
📱 Proof of testing on a real device
maintainer bypass
📡 Proof against a real emitter/receiver (if applicable)
maintainer bypass
📚 Wiki documentation commitment
Checklist