Stabilization of frame communication layer. - #27
Merged
Conversation
…zero bytes after.
There was a problem hiding this comment.
Pull request overview
This PR stabilizes the serial framing/communication layer (with emphasis on mixed binary + framed traffic) by introducing a dedicated frame parsing state machine and expanding test coverage around frame boundary detection and file transfers.
Changes:
- Replaced ad-hoc serial receive parsing in
SerialPortwith a newFrameParserstate machine. - Extended framing helpers (
ResponseFrame) with start/end-of-frame detection/validation and added corresponding tests. - Updated hardware/file-transfer tests and examples to use
FreeWilias a context manager; added stress-styleget_filetest coverage and a binary fixture.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| freewili/serialport.py | Switches RX handling to FrameParser; adds verbose logging/TRACE support and adjusts send timing. |
| freewili/frame_parser.py | New frame parsing state machine to separate response frames vs. binary data. |
| freewili/framing.py | Adds start/end-of-frame detection & validation helpers used by the parser. |
| freewili/safe_reponse_frame_dict.py | Extracts thread-safe response-frame storage into its own module. |
| freewili/fw_serial.py | Improves response-frame timeout error context; tightens file download write logic. |
| tests/test_frame_parser.py | Adds unit tests for FrameParser behavior across framed and binary data. |
| tests/test_framing.py | Adds tests for framing start/end validation helpers. |
| tests/test_get_file.py | Refactors context usage; adds multi-download stress test and improves callback test. |
| tests/test_fw.py / tests/test_hw.py / tests/test_hw_io.py / tests/test_i2c.py | Refactors tests to use context manager patterns for device lifecycle. |
| tests/assets/invalid.fwi | Adds binary fixture for file transfer stress testing. |
| examples/*.py | Refactors examples to use fw = ...; with fw: pattern. |
| .vscode/launch.json | Adds a debug configuration for running pytest with logging enabled. |
Comments suppressed due to low confidence (1)
freewili/serialport.py:275
- The serial port is now opened with exclusive=False. This can allow multiple processes/threads to open the same port simultaneously, which can cause data corruption or hard-to-debug IO errors. If the previous exclusive=True was intentional, consider keeping it as the default and making exclusivity configurable per SerialPort instance (or documenting why exclusivity must be disabled).
self.logger.debug(f"[{time.time() - start_time:.3f}] Opening {self._port}...")
serial_port = Serial(
self._port,
baudrate=self._baudrate,
timeout=0.001,
exclusive=False,
rtscts=False,
xonxoff=False,
dsrdtr=False,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
This update provides much needed fixes to the communication layer, especially around binary data transfers.