feat(transport): Add USB-Serial/JTAG and transport layer selection support #29
Conversation
👋 Hello radimkarnis, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
There was a problem hiding this comment.
Pull request overview
This PR introduces transport layer abstraction to support multiple communication interfaces (UART and USB-Serial/JTAG). The changes enable the stub to automatically detect and configure the appropriate transport layer at runtime.
- Adds transport layer abstraction with runtime selection between UART and USB-Serial/JTAG
- Implements USB-Serial/JTAG interrupt handler alongside existing UART handler
- Refactors SLIP layer to use configurable TX function pointer instead of hardcoded UART calls
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/transport.h | Declares transport initialization and interrupt handler interfaces |
| src/transport.c | Implements transport detection logic and configures interrupt handlers for both UART and USB-Serial/JTAG |
| src/slip.h | Adds public API for registering transport-specific TX functions |
| src/slip.c | Replaces direct UART calls with configurable function pointer |
| src/main.c | Refactors to use new transport abstraction by removing UART-specific initialization |
| src/CMakeLists.txt | Adds transport.c to build |
| esp-stub-lib | Updates submodule to version with USB-Serial/JTAG support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stub_lib_usb_serial_jtag_get_intr_flags(); | ||
|
|
||
| while (stub_lib_usb_serial_jtag_is_data_available()) { | ||
| slip_recv_byte(stub_lib_usb_serial_jtag_read_rxfifo_byte()); |
There was a problem hiding this comment.
The USB-Serial/JTAG interrupt handler processes all available bytes without checking for frame completion or errors. This differs from the UART handler (lines 21-29) which breaks on slip_is_frame_complete() or slip_is_frame_error(). This inconsistency could lead to buffer overflow or incorrect frame processing. Add the same frame completion check as in the UART handler.
| slip_recv_byte(stub_lib_usb_serial_jtag_read_rxfifo_byte()); | |
| slip_recv_byte(stub_lib_usb_serial_jtag_read_rxfifo_byte()); | |
| // Cannot process more bytes until frame is processed | |
| if (slip_is_frame_complete() || slip_is_frame_error()) { | |
| break; | |
| } |
| void stub_transport_init(void) | ||
| { | ||
| if (stub_lib_usb_serial_jtag_is_active()) { | ||
| stub_lib_usb_serial_jtag_rominit_intr_attach(17, usb_jtag_serial_interrupt_handler, USB_SERIAL_JTAG_SERIAL_OUT_RECV_PKT_INT_ENA); |
There was a problem hiding this comment.
The magic number 17 for the interrupt number should be defined as a named constant (e.g., USB_SERIAL_JTAG_INTR_NUM) to improve code clarity and maintainability, similar to how UART_NUM_0 and 5 are used for UART configuration.
| slip_set_tx_fn(stub_lib_usb_serial_jtag_tx_one_char); | ||
| } else { | ||
| stub_lib_uart_wait_idle(UART_NUM_0); // Wait until ROM sends response to last command | ||
| stub_lib_uart_rominit_intr_attach(UART_NUM_0, 5, uart_rx_interrupt_handler, UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT); |
There was a problem hiding this comment.
The magic number 5 for the UART interrupt number should be defined as a named constant (e.g., UART_INTR_NUM) to improve code clarity and maintainability.
radimkarnis
force-pushed
the
feat/usb_serial_jtag
branch
from
58ee6b7 to
c69f5f9
Compare
Description
Related
Testing
TODOs