-
Notifications
You must be signed in to change notification settings - Fork 151
Read from read loop remove generator logic #228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Download the artifacts for this pull request: |
|
can confirm fixed on Chrome Version 143.0.7499.110 |
|
yeah seems fine on an s2 with same chrome version win11. |
|
I tried this patch on my problem ESP32-S3. Unfortunately, it does not work. The failure is that the code fails to detect the S3s entry to Waiting for download state, even though the S3 really did enter that state and issued the appropriate message. Then the serial port gets a bunch of Buffer Overflows and never recovers. I think it is a race condition between the serial port setup and the timing of the messages. This branch https://github.com/MitchBradley/esptool-js/tree/timedRead works with all of the ESP32s and ESP32-S3s that I have tested so far. Every read goes through a low-level "timedRead()" function whose timeout ensures that the reader cannot stay locked forever. It retries all possible non-fatal errors that cause the loss of the reader, by getting a new reader per the example code in core WebSerial documentation. Fatal errors that cannot be recovered cause a throw. The SLIP reader and a new "readLine(timeout)" function are built on top of a buffering layer that sits above timedRead(). readLine() is used for parsing/detecting the entry to download mode, thus avoiding a regex that spans multiple lines, and avoiding the possible race condition when those initial bytes were read all at once. |
|
Hi @MitchBradley Could you share which board are you using ? Using ESP32-S3-DevKitC-1 seems to work for me with this branch but maybe 3rd party boards would work differently. Also does it happen when using baud rate 115200 ? Your approach is closer to the previous implementation without async generators which is cool but I have no way to test these changes unfortunately as I can't reproduce myself with Espressif boards. |
Fix #224
Description
Using yield and async generator is not efficient logic for reading the serial port.
This PR introduces
readloopas an always reading to a buffer and SLIP parsing from buffer. Seems to be more reliable.This pull request refactors the serial communication logic to simplify data reading and improve reliability. The main changes focus on removing asynchronous generators in favor of simpler async functions, introducing a new
sleeputility, and enhancing debugging and buffer management. The updates affect both theesploader.tsandwebserial.tsfiles, resulting in more maintainable and understandable code.Serial Communication Refactor:
Transportwith a simpler async function, removingnewReadand generator-basedreadLoop. The newreadmethod now waits for data with a timeout and processes available bytes in a straightforward loop. (src/webserial.ts) [1] [2]transport.read().next()andtransport.newRead()inESPLoaderto use the newtransport.read()interface, simplifying packet reading and improving consistency. (src/esploader.ts) [1] [2] [3] [4]Utility and Buffer Management:
sleeputility function inutil.tsand replaced all internal_sleepusages with this shared function. (src/util.ts,src/esploader.ts,src/webserial.ts) [1] [2] [3] [4]peek()method toTransportfor safely inspecting the buffer contents without consuming data, improving boot log detection logic. (src/webserial.ts,src/esploader.ts) [1] [2]Debugging and Logging Enhancements:
src/esploader.ts) [1] [2]Bug Fixes and Logic Improvements:
src/webserial.ts) [1] [2] [3]src/esploader.ts)These changes collectively make the serial communication code easier to understand, more robust, and better suited for debugging and extension.
Testing
Manual testing using
examples/typescriptcommands.Checklist
Before submitting a Pull Request, please ensure the following: