From 676e259d7349b5e0443564093a4d102cdea9b0f0 Mon Sep 17 00:00:00 2001 From: deadprogram Date: Sat, 25 Jul 2026 21:59:31 +0200 Subject: [PATCH 1/2] flash: fix retry corruption for compressed writes and cap ESP32 baud rate After a timeout during a compressed flash write, the retry's SLIP frame (starting with 0xC0) would terminate the stub's partial frame and then deliver the full retry data in the same write. The stub processed both as separate commands: it emitted a stale error for the truncated frame, then silently decompressed and wrote the retry block then advancing the zlib decompressor state. The host only read the stale error, flushed the real ACK, and retried with the same block, which the decompressor could no longer handle, causing all subsequent retries to hang. Fix by sending a bare 0xC0 to terminate the partial frame as a separate write, then waiting 50ms for the stale error response to arrive before flushing and retrying. This keeps the retry data out of the cleanup sequence and preserves consistent decompressor state. Also: - Add MaxUARTFlashBaud field to chipDef; set to 230400 for ESP32 to prevent UART RX FIFO overflow on USB-UART bridges (CH340, CP2102) - Increase flash block retries from 3 to 5 to provide headroom after the stale-frame cleanup cycle consumes one attempt Signed-off-by: deadprogram --- pkg/espflasher/chip.go | 7 +++ pkg/espflasher/flasher.go | 76 +++++++++++++++++++++++++-------- pkg/espflasher/protocol.go | 10 +++++ pkg/espflasher/protocol_test.go | 2 + pkg/espflasher/target_esp32.go | 1 + pkg/espflasher/version.go | 2 +- 6 files changed, 79 insertions(+), 19 deletions(-) diff --git a/pkg/espflasher/chip.go b/pkg/espflasher/chip.go index f453d7d..df98608 100644 --- a/pkg/espflasher/chip.go +++ b/pkg/espflasher/chip.go @@ -111,6 +111,13 @@ type chipDef struct { // CHANGE_BAUD command (0x0F). ESP32+ ROMs support this; ESP8266 does not. ROMHasChangeBaud bool + // MaxUARTFlashBaud caps the flash baud rate for UART bridge connections. + // The ESP32 ROM disables interrupts during flash page writes (because + // the SPI bus is shared with the cache), causing the 128-byte UART RX + // FIFO on common USB-UART bridges (CH340, CP2102) to overflow at high + // baud rates. Set to 0 for no cap (native USB chips have flow control). + MaxUARTFlashBaud int + // SPIMISODLenOffs is the register offset for the MISO data bit length // register (relative to SPIRegBase). On ESP32-S2 and newer, MISO/MOSI // lengths are in dedicated registers. On ESP8266 and ESP32, these are 0 diff --git a/pkg/espflasher/flasher.go b/pkg/espflasher/flasher.go index 7442599..74bbcf3 100644 --- a/pkg/espflasher/flasher.go +++ b/pkg/espflasher/flasher.go @@ -159,6 +159,7 @@ type connection interface { eraseRegion(offset, size uint32) error readFlash(offset, size uint32, progress ProgressFunc) ([]byte, error) flushInput() + terminatePartialFrame() isStub() bool setUSB(v bool) setSupportsEncryptedFlash(v bool) @@ -578,6 +579,13 @@ func (f *Flasher) FlashImage(data []byte, offset uint32, progress ProgressFunc) return fmt.Errorf("attach flash: %w", err) } + // Cap baud rate for UART-bridge chips prone to FIFO overflow. + if f.chip != nil && f.chip.MaxUARTFlashBaud > 0 && !f.usesUSB && + f.opts.FlashBaudRate > f.chip.MaxUARTFlashBaud { + f.logf("Limiting flash baud rate to %d (UART bridge on %s)", f.chip.MaxUARTFlashBaud, f.chip.Name) + f.opts.FlashBaudRate = f.chip.MaxUARTFlashBaud + } + // Optionally switch to higher baud rate (not supported by ESP8266 ROM) canChangeBaud := f.chip == nil || f.chip.ROMHasChangeBaud || f.conn.isStub() if canChangeBaud && f.opts.FlashBaudRate > 0 && f.opts.FlashBaudRate != f.opts.BaudRate { @@ -641,6 +649,13 @@ func (f *Flasher) FlashImages(images []ImagePart, progress ProgressFunc) error { f.opts.FlashMode = "dout" } + // Cap baud rate for UART-bridge chips prone to FIFO overflow. + if f.chip != nil && f.chip.MaxUARTFlashBaud > 0 && !f.usesUSB && + f.opts.FlashBaudRate > f.chip.MaxUARTFlashBaud { + f.logf("Limiting flash baud rate to %d (UART bridge on %s)", f.chip.MaxUARTFlashBaud, f.chip.Name) + f.opts.FlashBaudRate = f.chip.MaxUARTFlashBaud + } + // Optionally switch to higher baud rate (not supported by ESP8266 ROM) canChangeBaud := f.chip == nil || f.chip.ROMHasChangeBaud || f.conn.isStub() if canChangeBaud && f.opts.FlashBaudRate > 0 && f.opts.FlashBaudRate != f.opts.BaudRate { @@ -935,13 +950,17 @@ func (f *Flasher) EraseRegion(offset, size uint32, progress ProgressFunc) error const eraseProgressInterval = 500 * time.Millisecond // flashBlockRetries is the number of attempts for each flash data block write. -// At high baud rates (460800+), USB-UART bridges occasionally lose bytes during -// transmission, causing the stub to receive a truncated or corrupted SLIP frame. -// The stub detects this (bad data length or bad checksum) and responds with a -// clean error, leaving it ready for a resend. We retry only for these -// serial-integrity errors; device-side failures (SPI errors, inflate errors) -// are not retried. -const flashBlockRetries = 3 +// USB-UART bridges occasionally lose bytes during transmission, causing the stub +// to receive a truncated or corrupted SLIP frame. After a timeout, the first +// retry's leading 0xC0 terminates the stub's partial frame, often producing a +// stale "bad data length" error that consumes one retry for cleanup. With 5 +// retries we get at least 3 clean attempts after any such cleanup cycle. +const flashBlockRetries = 5 + +// flashBlockRetryDelay is the delay between flash block retries, allowing +// in-flight stale responses from the stub to arrive and be flushed before +// the next attempt. +const flashBlockRetryDelay = 50 * time.Millisecond // tickErase runs work (a blocking erase call) while emitting synthetic ETA // progress updates against est every interval, until work returns. progress @@ -1301,13 +1320,23 @@ func (f *Flasher) logf(format string, args ...interface{}) { // // 2. TimeoutError: the stub never responded, likely because bytes were lost // during UART transmission, leaving the stub waiting for the rest of an -// incomplete SLIP frame. On resend, the leading 0xC0 terminates the stub's -// partial frame; the stub may then emit a stale error response for the -// truncated frame before processing our retry, which is handled by the -// next retry iteration. +// incomplete SLIP frame. +// +// After a timeout, the stub may be holding a partial frame. To clean up +// without corrupting the decompressor state (which is critical for compressed +// flash writes), we: +// 1. Send a bare SLIP end byte (0xC0) to terminate the stub's partial frame. +// 2. Wait briefly for the stub to emit a stale error response for the +// truncated data. +// 3. Flush the serial RX buffer to discard that stale response. +// 4. Retry with a clean serial state. +// +// This prevents the retry data from being concatenated with the partial-frame +// termination in a single write, which would cause the stub to process both +// the cleanup and the retry as separate commands — advancing the decompressor +// state on the "invisible" second command while we read the stale error from +// the first. // -// Between retries the serial RX buffer and SLIP reader are flushed so stale -// responses from partial-frame cleanup don't corrupt subsequent reads. // Device-side failures (SPI errors, inflate errors) are NOT retried. func (f *Flasher) retryFlashBlock(seq, numBlocks uint32, writeFn func() error) error { var err error @@ -1322,17 +1351,28 @@ func (f *Flasher) retryFlashBlock(seq, numBlocks uint32, writeFn func() error) e if attempt < flashBlockRetries-1 { f.logf("Warning: block %d/%d write failed (attempt %d/%d): %v — retrying", seq, numBlocks, attempt+1, flashBlockRetries, err) - // Flush stale data so the retry starts with a clean serial state. - // After a timeout the stub may still be holding a partial frame; - // our next send's leading 0xC0 will terminate it, producing a - // stale error response that the flush on the FOLLOWING iteration - // (if needed) will clear. + + // If this was a timeout, the stub is likely holding a partial SLIP + // frame. Send a bare 0xC0 to terminate it cleanly, then wait for + // the stale error response before flushing and retrying. + if isTimeoutError(err) { + f.conn.terminatePartialFrame() + } + + // Wait for any stale responses to arrive, then flush them. + time.Sleep(flashBlockRetryDelay) f.conn.flushInput() } } return err } +// isTimeoutError returns true if err is a TimeoutError. +func isTimeoutError(err error) bool { + var te *TimeoutError + return errors.As(err, &te) +} + // isRetryableFlashError returns true if the error is a transient serial-link // issue (data corruption or loss) that can be recovered by resending. func isRetryableFlashError(err error) bool { diff --git a/pkg/espflasher/protocol.go b/pkg/espflasher/protocol.go index 778e8cb..12fbd2e 100644 --- a/pkg/espflasher/protocol.go +++ b/pkg/espflasher/protocol.go @@ -712,6 +712,16 @@ func (c *conn) flushInput() { c.reader.reset() } +// terminatePartialFrame sends a bare SLIP end byte (0xC0) to terminate any +// partial frame the stub may be holding. This is used during flash write retry +// cleanup: after a timeout, the stub is likely waiting for the rest of an +// incomplete SLIP frame. Sending 0xC0 alone (without any command data) makes +// the stub process the truncated frame and emit an error response, without +// inadvertently feeding it our retry data as a second command in the same write. +func (c *conn) terminatePartialFrame() { + c.port.Write([]byte{slipEnd}) //nolint:errcheck +} + // waitForStubFlashWrite sleeps to let the stub's flash write post-process // complete before the caller sends the next command. See stubFlashPageDelay // for the full rationale. When the ROM bootloader is active (no stub), the diff --git a/pkg/espflasher/protocol_test.go b/pkg/espflasher/protocol_test.go index 89abd76..71c514f 100644 --- a/pkg/espflasher/protocol_test.go +++ b/pkg/espflasher/protocol_test.go @@ -626,6 +626,8 @@ func (m *mockConnection) flushInput() { } } +func (m *mockConnection) terminatePartialFrame() {} + func (m *mockConnection) readFlash(offset, size uint32, progress ProgressFunc) ([]byte, error) { if m.readFlashFunc != nil { return m.readFlashFunc(offset, size, progress) diff --git a/pkg/espflasher/target_esp32.go b/pkg/espflasher/target_esp32.go index 10a4c34..16610bb 100644 --- a/pkg/espflasher/target_esp32.go +++ b/pkg/espflasher/target_esp32.go @@ -56,6 +56,7 @@ var defESP32 = &chipDef{ ROMHasCompressedFlash: true, ROMHasChangeBaud: true, + MaxUARTFlashBaud: 230400, FlashFrequency: map[string]byte{ "80m": 0xF, diff --git a/pkg/espflasher/version.go b/pkg/espflasher/version.go index 53100bd..641414f 100644 --- a/pkg/espflasher/version.go +++ b/pkg/espflasher/version.go @@ -1,4 +1,4 @@ package espflasher // Version is the current version of the espflasher library. -const Version = "0.7.1" +const Version = "0.8.0-dev" From bd34fa1935e5da1b12511c2b68866782381ad9f3 Mon Sep 17 00:00:00 2001 From: deadprogram Date: Sun, 26 Jul 2026 12:09:01 +0200 Subject: [PATCH 2/2] protocol: retry serial Write and Drain on EINTR On Linux, signals such as SIGWINCH (terminal resize) can interrupt write(2) and tcdrain(3) on serial file descriptors, causing them to return EINTR before any bytes are transferred. The go.bug.st/serial library does not retry internally, so the error bubbles up as "interrupted system call" and aborts the flash. Wrap port.Write and port.Drain in sendCommand with EINTR retry loops so transient signal interruptions are handled transparently. Fixes a "send command 0x12: interrupted system call" failure observed during compressed flash download on ESP32-C3 (USB-JTAG/Serial). Signed-off-by: deadprogram --- pkg/espflasher/protocol.go | 39 +++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/pkg/espflasher/protocol.go b/pkg/espflasher/protocol.go index 12fbd2e..e46b760 100644 --- a/pkg/espflasher/protocol.go +++ b/pkg/espflasher/protocol.go @@ -3,8 +3,10 @@ package espflasher import ( "bytes" "encoding/binary" + "errors" "fmt" "io" + "syscall" "time" "go.bug.st/serial" @@ -172,12 +174,12 @@ func (c *conn) sendCommand(opcode byte, data []byte, chk uint32) error { if end > len(frame) { end = len(frame) } - if _, err := c.port.Write(frame[off:end]); err != nil { + if err := writeRetryEINTR(c.port, frame[off:end]); err != nil { return err } } } else { - if _, err := c.port.Write(frame); err != nil { + if err := writeRetryEINTR(c.port, frame); err != nil { return err } } @@ -194,7 +196,38 @@ func (c *conn) sendCommand(opcode byte, data []byte, chk uint32) error { // ensures each frame is committed to the USB-UART bridge before we // proceed, adding a small but deterministic delay that gives the stub // more time between consecutive commands. - return c.port.Drain() + return drainRetryEINTR(c.port) +} + +// writeRetryEINTR calls port.Write, retrying transparently if interrupted by a +// signal (EINTR). On Linux, signals such as SIGWINCH can interrupt write(2) on +// serial file descriptors before any bytes are transferred. +func writeRetryEINTR(port serial.Port, data []byte) error { + for { + _, err := port.Write(data) + if err == nil { + return nil + } + if errors.Is(err, syscall.EINTR) { + continue + } + return err + } +} + +// drainRetryEINTR calls port.Drain, retrying transparently if interrupted by a +// signal (EINTR). On Linux, tcdrain(3) can be interrupted by any signal. +func drainRetryEINTR(port serial.Port) error { + for { + err := port.Drain() + if err == nil { + return nil + } + if errors.Is(err, syscall.EINTR) { + continue + } + return err + } } // commandResponse represents a parsed response from the ESP device.