Skip to content

Fix ESP32 flash read errors by correcting stub protocol issues - #114

Closed
Jason2866 wants to merge 1 commit into
mainfrom
agents/fix-esp32-flash-read-error
Closed

Fix ESP32 flash read errors by correcting stub protocol issues#114
Jason2866 wants to merge 1 commit into
mainfrom
agents/fix-esp32-flash-read-error

Conversation

@Jason2866

@Jason2866 Jason2866 commented Aug 26, 2026

Copy link
Copy Markdown
Owner

This pull request addresses multiple issues in the ESP32 flash read implementation that were causing timeouts and read failures. The changes made are as follows:

  • Corrected Parameter Interpretation: The stub was incorrectly interpreting the maxInFlight parameter as a packet count instead of bytes. This led to overwhelming the USB buffer. The fix converts maxInFlight from bytes to the appropriate packet count before sending it to the stub.

  • ACK Logic Update: The acknowledgment (ACK) logic was modified to send an ACK after every packet, aligning it with the behavior of the Python esptool. This change improves flow control and prevents packet loss.

  • MD5 Hash Handling: The stub was sending an MD5 hash after each chunk, which was not being read by the host, leading to data corruption in subsequent reads. The implementation now includes reading and discarding the MD5 hash to prevent this issue.

  • Abort Frame Correction: The abort frame sent during errors was changed from a zero-length SLIP frame to a one-byte frame, ensuring that the stub correctly breaks out of its loop on error.

These changes collectively enhance the reliability of the flash read process for the ESP32, resolving the timeout and data corruption issues previously encountered.

Summary by CodeRabbit

  • Bug Fixes

    • Improved flash-read reliability by correctly managing packet acknowledgments and error recovery.
    • Ensured trailing verification data is properly consumed after each flash chunk.
    • Improved handling of read failures with an explicit transfer-abort signal.
  • Chores

    • Removed support modules for legacy device targets and console output formatting.
    • Removed unused stream transformation utilities for line breaks and timestamps.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ef3cc92-56e6-40b6-97d2-2521762480b4

📥 Commits

Reviewing files that changed from the base of the PR and between bb380ef and f5cfd79.

📒 Files selected for processing (18)
  • js/modules/esp32-BRKoi17y.js
  • js/modules/esp32c2-Btgr_lwh.js
  • js/modules/esp32c3-CHKfoI8W.js
  • js/modules/esp32c5-BDW4KtLo.js
  • js/modules/esp32c6-il8tTxAG.js
  • js/modules/esp32c61-thKzxBGf.js
  • js/modules/esp32h2-CxoUHv_P.js
  • js/modules/esp32p4-D3jLP-jY.js
  • js/modules/esp32p4r3-CqI71ojR.js
  • js/modules/esp32s2-iX3WoDbg.js
  • js/modules/esp32s3-DGwDVIgz.js
  • js/modules/esp32s31-Ef8NLP2_.js
  • js/modules/esp8266-CUwxJpGa.js
  • js/modules/esptool.js
  • js/util/console-color.js
  • js/util/line-break-transformer.js
  • js/util/timestamp-transformer.js
  • src/esp_loader.ts
💤 Files with no reviewable changes (16)
  • js/util/line-break-transformer.js
  • js/modules/esp32s3-DGwDVIgz.js
  • js/modules/esp32c2-Btgr_lwh.js
  • js/util/console-color.js
  • js/modules/esp32-BRKoi17y.js
  • js/modules/esp32s2-iX3WoDbg.js
  • js/util/timestamp-transformer.js
  • js/modules/esp32p4r3-CqI71ojR.js
  • js/modules/esp32c5-BDW4KtLo.js
  • js/modules/esp32c61-thKzxBGf.js
  • js/modules/esp8266-CUwxJpGa.js
  • js/modules/esp32c6-il8tTxAG.js
  • js/modules/esp32h2-CxoUHv_P.js
  • js/modules/esp32c3-CHKfoI8W.js
  • js/modules/esp32p4-D3jLP-jY.js
  • js/modules/esp32s31-Ef8NLP2_.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

readFlash now uses packet-based flow control, sends non-empty abort frames, acknowledges each packet, and consumes trailing MD5 packets. Generated ESP firmware modules and console and stream transformer utilities were removed.

Changes

Flash read protocol

Layer / File(s) Summary
Read request and abort handling
src/esp_loader.ts
The ESP_READ_FLASH request now receives a packet count derived from maxInFlight / blockSize. Read failures now send a one-byte SLIP abort frame.
Packet acknowledgment and MD5 handling
src/esp_loader.ts
The loader acknowledges every packet, removes byte-threshold tracking, and consumes each chunk’s trailing 16-byte MD5 packet.
Generated modules and stream utilities
js/modules/*, js/util/*
Generated ESP firmware modules, ColoredConsole, LineBreakTransformer, and TimestampTransformer were removed with their exports.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f5cfd

The PR updates ESP32 flash-read protocol handling to improve reliability, and no actionable merge-blocking risk remains based on the supplied evidence.

Poem

A rabbit checks each packet in flight

ACKs hop cleanly through the night
MD5 trails are tucked away
Abort frames help when streams go astray
Firmware modules fade from sight

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and clearly summarizes the main change: correcting ESP32 flash-read stub protocol handling to fix errors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agents/fix-esp32-flash-read-error

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Jason2866 Jason2866 closed this Aug 26, 2026
@Jason2866
Jason2866 deleted the agents/fix-esp32-flash-read-error branch September 1, 2026 21:35
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

Successfully merging this pull request may close these issues.

1 participant