Repository navigation
Accept a single validated ROM SYNC reply on USB-Serial-JTAG devices - #1084
Massimiliano-solutiontech wants to merge 1 commit into
Conversation
efe537e to
820d1e7
Compare
SergioGasquez
left a comment
SergioGasquez
left a comment
There was a problem hiding this comment.
Hi! Thanks for the contribution, here are some comments
| reset_strategy.reset(&mut self.serial)?; | ||
|
|
||
| if self.is_native_usb_sync_profile() { | ||
| sleep(NATIVE_USB_SYNC_SETTLE); |
There was a problem hiding this comment.
I would drop this unless you have an scenario where VirtualHere/SEH fails without it
| } else { | ||
| Err(Error::RomError(Box::new(RomError::new( | ||
| CommandType::Sync, | ||
| RomErrorKind::InvalidMessage, |
There was a problem hiding this comment.
We are losing ROM error detail and always returning InvalidMessage.
|
|
||
| ### Fixed | ||
|
|
||
| - Connecting to USB-Serial-JTAG devices (`303a:1001`) no longer requires all eight ROM `SYNC` replies; one strictly validated reply is accepted and the duplicates are drained, which fixes connecting through USB-over-IP passthrough such as VirtualHere or SEH UTN |
There was a problem hiding this comment.
| - Connecting to USB-Serial-JTAG devices (`303a:1001`) no longer requires all eight ROM `SYNC` replies; one strictly validated reply is accepted and the duplicates are drained, which fixes connecting through USB-over-IP passthrough such as VirtualHere or SEH UTN | |
| - Connecting to USB-Serial-JTAG devices (`303a:1001`) no longer requires all eight ROM `SYNC` replies; one strictly validated reply is accepted and the duplicates are drained, which fixes connecting through USB-over-IP passthrough such as VirtualHere or SEH UTN (#1084) |
| self.write_command(Command::Sync)?; | ||
| self.serial.flush()?; | ||
|
|
||
| let first_deadline = std::time::Instant::now() + NATIVE_USB_SYNC_TIMEOUT; |
There was a problem hiding this comment.
NATIVE_USB_SYNC_TIMEOUT makes each sync attempt wait up to 1 s for the first reply. Before, it was 100 ms (SYNC_TIMEOUT). begin() runs 7 connect attempts with 5 syncs each. So when the ROM doesn't answer (the reset didn't enter download mode, the wrong port was picked, or an app is running), the time spent waiting before ConnectionFailed goes from about 3.5 s to about 35 s. This hits every chip on 303a:1001, not only remote links.
Could you keep the first-reply wait at SYNC_TIMEOUT and only start the drain once a valid reply arrives? If remote links need a longer first-reply wait, could you share the first-reply latency you measured over VirtualHere so we can choose a value from that?
| Ok(()) | ||
| } | ||
|
|
||
| fn is_native_usb_sync_profile(&self) -> bool { |
There was a problem hiding this comment.
Reset strategy selection and is_using_usb_serial_jtag() only check the PID, while this gate also checks the VID. So a xxxx:1001 port would get the USB-JTAG reset but the old sync. Could you reuse is_using_usb_serial_jtag() here? Please also rename native_usb_* to usb_serial_jtag_*, because "native USB" also covers USB-OTG (S2/S3/P4), which this path doesn't handle.
The ESP ROM answers one
SYNCcommand with eight identical replies, andConnection::sync()requires all of them. When an ESP32-C6's USB-Serial-JTAG port is used through USB-over-IP passthrough (VirtualHere, SEH UTN) on macOS, most of the burst never reaches the host. The CDC-ACM driver keeps a single 64-byte IN request queued, and every refill costs a network round trip. With VirtualHere, exactly 2 of 8 replies arrived in 100/100 attempts, so espflash can't connect.For ports whose USB VID/PID is
303a:1001,sync()now:SYNC;0x20120707and both status bytes. Stub-style replies and malformed frames are rejected;Other VID/PIDs keep the current loop unchanged. Tests cover frame validation, the VID/PID gate, split reads, malformed escapes, and two valid replies followed by a truncated duplicate tail.
Over VirtualHere this is necessary but not sufficient. The currently bundled C6 stub then fails to start (timeout after Using flash stub), and with the v1.3.0 ESP Flasher Stub from #1060 flashing works (see the table below).
Tested (macOS 26.7, Apple M1 Max):
board-info0/5invalid headercontinuously)cargo xtask run-tests --chip esp32c6, local C6FH4A side effect on directly attached boards: when the chip has no valid bootloader (a blank board, or partway through the hardware suite) the ROM floods the port, and the current loop needs ~2.7 s to find eight clean replies. The validated first-reply path connects in 0.83 s, which halves the suite's runtime on a C6FH4.
The extra ~0.1 s per connect on local boards is the post-reset settle. I can drop it if you'd prefer: it was added for remote links, and the drain logic doesn't depend on it.
cargo test -p espflash --lib: 46 passed, 1 ignored.cargo +nightly fmt --checkclean, no clippy warnings insrc/.