Improve mpremote socket and rfc2217 connections#19062
Draft
Josverl wants to merge 23 commits intomicropython:masterfrom
Draft
Improve mpremote socket and rfc2217 connections#19062Josverl wants to merge 23 commits intomicropython:masterfrom
Josverl wants to merge 23 commits intomicropython:masterfrom
Conversation
closes: micropython#18657 (cherry picked from commit a953024) Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
fixes: micropython#18658 (cherry picked from commit dd6e0f7) Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Adds a Modern Windows console implementation that can write raw UTF-8 bytes directly (like POSIX) Improve the Legacy consol to use an incremental decoder to handle split UTF-8 sequences within writes. fixes micropython#15228 Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19062 +/- ##
==========================================
+ Coverage 98.38% 98.46% +0.08%
==========================================
Files 171 176 +5
Lines 22298 22802 +504
==========================================
+ Hits 21937 22453 +516
+ Misses 361 349 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Code size report: |
RemoteCommand.wr_bytes(): Now correctly encodes strings to UTF-8 bytes before calculating length. Based on the suggestion by @felixdoer Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Reduces code duplication while allowing different configurations. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Add regressions tests for utf-8 and unicode. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
allows for skipping of some potentially destructive tests. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
The MicroPython Unix port sends only `\n`. With attr[1] = 0, the terminal doesn't translate `\n` to `\r\n`, causing a staircase effect. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
…ercept class. This fixes an error with mounting over socket:// connections. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
pyserial's RFC2217 uses a background thread to process telnet data from the socket into an internal buffer. When mpremote's REPL loop used select() on the socket, it signaled "ready" when data arrived, but inWaiting() returned 0 because the background thread hadn't processed the data yet. This caused a race condition: 1. select() returns because socket has data 2. inWaiting() returns 0 (background thread hasn't processed yet) 3. REPL loop skips reading 4. Next iteration: select() returns, now inWaiting() > 0 5. Finally reads the data Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com> (cherry picked from commit 1ab8ba3)
fixes micropython#15228 Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com> (cherry picked from commit f104330)
Needs to be split out in more detail Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
- Update .gitignore to include coverage data files. - Modify pyproject.toml to append coverage data during pytest runs. - Enhance README.md with instructions for running tests with coverage. - Update run-mpremote-tests.sh to support coverage collection. - Add test_basic.sh and test_basic.sh.exp for device connection tests. - Extend test_mount.sh with additional remote actions and comments. - Improve get_devices function in test_repl_newlines.py to support MPREMOTE_DEVICE environment variable. Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
…tils module Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <jos_verlinde@hotmail.com>
…les. Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
fe618bf to
9d9637a
Compare
Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
Member
|
This is great, but it's hard to review because it does so many things at once
Yes! I think the first thing would be to split out the testing changes, a PR that only updates the existing tests and doesn't add any new features or new tests. |
Contributor
Author
|
Absolutely. This was intended to provide insights into a possible approach to testing Mpremote in CI, and across more platforms as asked by @projectgus
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes multiple issues with mpremote when connecting to socket and RFC2217 targets, and adds comprehensive pytest-based testing infrastructure.
possibly this PR should be split into multiple PRs:
Key improvements:
Fixed RFC2217 REPL race conditions: pyserial's RFC2217 uses a background thread to process telnet data. The REPL loop's
select()would signal ready when socket data arrived, butinWaiting()returned 0 because the background thread hadn't processed the data yet, causing delayed or missed input.Fixed socket connection support: Added missing
in_waitingproperty toSerialInterceptwhich was causing errors when usingmpremote mountover socket:// connections.Improved REPL newline handling: Fixed staircase effect when connecting to Unix port by properly configuring terminal output flags (OPOST | ONLCR) for NL → CR-NL translation.
Enhanced
waitchar()for network connections: Improved handling of file descriptors for socket-based connections and RFC2217 (which doesn't implementfileno()).Better error handling on disconnect: Device disconnection messages now properly display for RFC2217 and socket connections, even when command-line parameters are provided.
Optimized network performance: Improved SerialTransport performance for network connections.
Testing infrastructure:
Testing
Tested on the following configurations:
/dev/ttyACM0,/dev/ttyUSB0socket://localhost:2218rfc2217://localhost:2217In order to enable testing on Windows - and for other tests that require some level of mocking to be created I have addedd tests using the pytest framework.
The new pytest suite includes 548+ test assertions across multiple test files covering:
test_unicode.py,test_unicode_filenames.pytest_equals_sign.pytest_windows_console_utf8.pytest_utils.pytest_repl_newlines.pyTo run all tests:
Trade-offs and Alternatives
I have chosen to use the
pytestframework over stdlib'sunittestas:conftest.py) elegantly handles complex setup/teardown of device connections and test scenariosI have not ported the existing bash based tests to pytest, but it should be possible to have the pytest test runner discover and run the existing "test_*.sh" files as part of as seperate PR
Generative AI
I used generative AI tools when creating this PR, but a human has checked the
code and is responsible for the code and the description above.