Skip to content

parse_fw_string: hold the firmware reply parser once, in protocol/text/framing - #1215

Merged
BioCam merged 1 commit into
PyLabRobot:mainfrom
BioCam:hamilton-protocol-text-framing
Aug 24, 2026
Merged

parse_fw_string: hold the firmware reply parser once, in protocol/text/framing#1215
BioCam merged 1 commit into
PyLabRobot:mainfrom
BioCam:hamilton-protocol-text-framing

Conversation

@BioCam

@BioCam BioCam commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

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 pylabrobot/hamilton/protocol/text/framing.py holding parse_fw_string and parse_firmware_version_date, moved verbatim.
  • hamilton/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. The only change is that there is now one copy rather than two.

Tests: adds framing_tests.py - parse_fw_string across the identifier, the three field types, the repeated-field list form, and a missing field raising; parse_firmware_version_date against 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 ones STAR_tests.py already exercises, so coverage moves with the code rather than being invented. ruff format, ruff check --select I, ruff check, and mypy pylabrobot --check-untyped-defs are clean; the full suite passes.

Nothing here touches hamilton/transport/tcp, so it does not overlap #1195.

🤖 Generated with Claude Code

…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>
@BioCam
BioCam merged commit 338199d into PyLabRobot:main Aug 24, 2026
21 checks passed
@BioCam
BioCam deleted the hamilton-protocol-text-framing branch August 24, 2026 17:58
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.

2 participants