parse_fw_string: hold the firmware reply parser once, in protocol/text/framing - #1215
Merged
Merged
Conversation
…text/framing` `parse_star_fw_string` exists twice: once in `hamilton/transport/usb/protocol.py`, once in `legacy/liquid_handling/backends/hamilton/STAR_backend.py`. The two are identical - 117 lines each, no differing lines ignoring comments and docstrings - so a fix to the reply grammar has to be made in both, and a fix to one is silently not a fix to the other. The grammar is also not a property of USB. The same shape comes back from a STAR over USB and from a tilt module over a serial line, and the parser is a pure function of two strings: no state, no I/O, no device. Keeping it inside the transport that happens to carry it is what let the second copy appear. - Adds `protocol/text/framing.py` holding `parse_fw_string` and `parse_firmware_version_date`, moved verbatim. - `transport/usb/protocol.py` re-exports both under their old names, so `parse_star_fw_string` and `parse_star_firmware_version_date` keep working from where they are imported today. - `STAR_backend.py` drops its copy and imports the one implementation, still exporting `parse_star_fw_string` for its own callers and tests. Behaviour: none. The implementation is unchanged, both old names resolve to the same object as the new ones, and nothing that imports them needs to move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rickwierenga
approved these changes
Aug 24, 2026
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.
parse_star_fw_stringexists twice: once inhamilton/transport/usb/protocol.py, once inlegacy/liquid_handling/backends/hamilton/STAR_backend.py. The two are identical - 117 lines each, no differing lines ignoring comments and docstrings - so a fix to the reply grammar has to be made in both, and a fix to one is silently not a fix to the other.The grammar is also not a property of USB. The same shape comes back from a STAR over USB and from a tilt module over a serial line, and the parser is a pure function of two strings: no state, no I/O, no device. Keeping it inside the transport that happens to carry it is what let the second copy appear.
pylabrobot/hamilton/protocol/text/framing.pyholdingparse_fw_stringandparse_firmware_version_date, moved verbatim.hamilton/transport/usb/protocol.pyre-exports both under their old names, soparse_star_fw_stringandparse_star_firmware_version_datekeep working from where they are imported today.STAR_backend.pydrops its copy and imports the one implementation, still exportingparse_star_fw_stringfor its own callers and tests.Behaviour: none. The implementation is unchanged, both old names resolve to the same object as the new ones, and nothing that imports them needs to move. The only change is that there is now one copy rather than two.
Tests: adds
framing_tests.py-parse_fw_stringacross the identifier, the three field types, the repeated-field list form, and a missing field raising;parse_firmware_version_dateagainst a master and a 96-head reply; and one assertion that both old names still import and are the same object. The parser cases are the onesSTAR_tests.pyalready exercises, so coverage moves with the code rather than being invented.ruff format,ruff check --select I,ruff check, andmypy pylabrobot --check-untyped-defsare clean; the full suite passes.Nothing here touches
hamilton/transport/tcp, so it does not overlap #1195.🤖 Generated with Claude Code