feat(hamilton): add Prep device over TCP transport - #1196
Open
cmoscy wants to merge 32 commits into
Open
Conversation
Replace the thin post-PyLabRobot#1000 stub with the HOI/HARP session client, command layer, wire types, and introspection stack so Prep/Nimbus can build on it.
Python 3.9 binds asyncio.Lock to the current loop at Socket construction, so HamiltonTCPClient cannot be created in sync TestCase methods. Use IsolatedAsyncioTestCase for those cases.
Plain Prep package with channels/head8/gripper over HamiltonTCPClient, kwargs for LH params, local op types, PrepDeck, and liquid_class_resolver.
…pick Expose resource-aware plate drop with deck reassignment, arm-level pick_up_at_location, and optional resource_width on pick_up_resource so notebooks no longer compute grip geometry by hand.
Delete prep/standard op dataclasses; peers take TipSpots/wells + vols with kwargs instead of constructing op objects.
Queue intents before device commands and commit or roll back from ChannelSuccesses so labware trackers stay consistent for deck and visualizer updates without vendor-specific bookkeeping.
Replace private mounted-tip maps with TipTrackers and call shared finalize after pick/drop and aspirate/dispense so spot, well, and mount state update from command outcomes.
Move to mount XY at traverse height before PrepPickUpTool, matching tip pickup, so tool engage does not swoop down from an arbitrary pose.
Show deck-rooted visualizer updates for tip spots and well fills, with status prints for channel mount state during dual-channel transfer.
Capture tip/volume tracking through dual-channel and 8MPH sections, link the notebook under Hamilton Prep docs, add channels volume-tracker coverage, and apply ruff formatting on the related Prep modules.
Drop field(kw_only=...) (3.10+), default dest in __post_init__, and declare dest on constructors that pass it. Also fix liquid-class test typing, gripper mount isinstance, and head8 test formatting for CI.
Those investigation notebooks are not in the PR; listing them in conf.py leaked local-only paths into shared docs config. Demo wiring stays via hamilton/prep/index.md.
…connect _send_raw retried by re-writing the command on connection errors. TimeoutError and OSError were both treated as retryable, and the read uses a 300s default timeout, so a slow motion command that timed out on the read was physically executed twice. Reconnection now belongs to the caller: stop() then setup(). No other transport in the repo self-heals a connection, and re-establishing the session cannot report whether the in-flight command completed. Session state is scoped to the connected session and fully reset by setup(): client id, sequence numbers, instrument addresses and the object registry all survived a reconnect before. setup() on a live client now raises instead of leaking the socket.
…eclared data wire_type_of() resolves an Annotated alias or bare WireType in one place, replacing four ad-hoc __metadata__ probes across messages.py and wire_types.py. TCPCommand now declares Response and uses_physical_channels as ClassVars. Response replaces a hasattr() dispatch; uses_physical_channels replaces duck-typing that inferred channel semantics by looking for a "channel" attribute on the first element of any list field. Device peers declare the flag; the transport no longer guesses. get_log_params reads dataclass fields instead of walking the __init__ signature and probing self. assert isinstance on wire-derived responses became real raises: asserts are stripped under python -O, which is exactly when a malformed frame most needs to fail loudly.
TCP-side concurrency semantics are not established for this protocol, so only one command is in flight at a time. _transact holds the lock across sequence-number allocation, build, write and the terminal read, then releases it before the response is decoded. That boundary is load-bearing: error enrichment resolves interface and method names through introspection, which sends further commands through _transact, so a lock spanning the whole of _send_raw would deadlock on the first firmware error. A test covers that path and fails with a timeout under the naive design.
Reading used to happen inline inside a command, which meant events were only observed while a command was in flight, and any non-ACK non-EVENT frame was accepted as the current command's response regardless of origin. A response arriving late, after a timeout, silently became the next command's answer. The reader owns the socket from the end of setup() until stop(). It dispatches events continuously, skips ACKs, and hands the terminal frame to the waiting command. Frames arriving with nothing waiting are dropped and logged rather than queued. The handoff is a single slot, not a keyed map: the command lock already allows one command in flight, and whether the device echoes the request sequence number is unverified. Mismatches are logged, so the pairing can be confirmed from real traffic before a keyed demux is built on it. Reads stay inline during the init and registration handshake, which exchanges Registration frames rather than commands. A reader that dies on a live connection fails the waiting command instead of hanging it.
…outing The transport had no coverage of connection lifecycle, retry, event dispatch or concurrency. Adds the regressions for the behaviour the preceding commits established: a command is written exactly once when the read fails, I/O on a disconnected client names setup(), setup() refuses to run twice, session state and sequence numbers reset between sessions, commands do not interleave, error enrichment does not deadlock against the command lock, events arrive between commands, unmatched frames are dropped rather than misdelivered, and a dead reader fails the waiting command.
Found against MLPrep firmware. Shortly after registration the device sends a HARP protocol-2 frame carrying options and no HOI body. _read_one_message routed every protocol-2 frame to CommandResponse, which unpacks a HOI header that is not there, so the reader died and every later command failed with "reader is not running". Inline reading never hit this because nothing read the socket between commands. _read_one_message now returns None for a frame with no routable message, and the reader skips unparseable frames instead of terminating. Frames are length-prefixed and consumed whole, so skipping one cannot desynchronize the stream. Also records what the same session established: the device echoes the request address and sequence number on every response (26/26), so the mismatch warning now documents a real invariant rather than an open question.
Turning a firmware error into a readable message asks the device for interface and method names. When those queries fail too, the failure was enriched the same way, which enriched again: a device answering STATUS_EXCEPTION to every request produced a RecursionError after 55 reads rather than an HoiError. Pre-dates this branch; reproduced identically at 4e71a71 and confirmed to need a device that fails Interface-0 queries, which is why healthy hardware never showed it. It surfaces exactly when the error message matters most. Enrichment is now non-re-entrant, guarded by a ContextVar so concurrent callers keep their own state. A nested entry falls back to HC_RESULT_PROTOCOL and terse addressing, so the caller still gets a real HoiError naming the failing address, interface and action.
…ilton-tcp-transport
Reconciles the Prep peers with the transport changes: - PrepClient drops auto_reconnect / max_reconnect_attempts; recovery is stop() then setup(). Its _send_raw override and PrepChatterboxClient lose the ensure_connection parameter. - PREP_ERROR_CODES now imports from pylabrobot.hamilton.prep.error_tables, following the table move on the transport branch. - PrepCommand derives uses_physical_channels from its per-channel StructArray fields. The transport no longer infers channel semantics, but Prep declares over a hundred command types whose per-channel-ness follows from their wire shape, so the device layer derives it rather than repeating a flag on each. Verified to classify all 57 constructible Prep commands identically to the previous transport-level logic. - TCPCommand keeps protocol / interface_id / command_id / action_code as plain class attributes rather than ClassVar: PrepStatusRequest redeclares command_id and interface_id as per-instance dataclass fields by design, which ClassVar forbids.
…able Declaring protocol, interface_id, command_id and the action configuration as ClassVar forbids a subclass from redeclaring them as per-instance dataclass fields. Prep's PrepStatusRequest does exactly that by design, carrying command_id and interface_id on the instance so one class can serve many firmware methods. The ClassVar conversion was incidental to removing attribute probing and was not needed for it; Response and uses_physical_channels remain ClassVar because nothing overrides them per instance.
ruff check --select I, which CI runs as part of make format-check.
…waits Three reader tests queued response frames before sending the command. The reader could drain them first and correctly drop them as unmatched, leaving the command waiting until its timeout. Whether that happened depended on task scheduling: they passed on 3.14 and failed on 3.11, so CI would have caught them intermittently at best. Frames are now fed after the command registers its pending response, with an explicit wait for that registration. Also asserts the stale-frame test really did consume the frame it is about to prove was dropped, rather than passing because the reader had not run yet. Sorts imports after the error-table move, which CI checks via ruff check --select I in make format-check.
The typos CI check rejects 'unparseable'; the rest of the codebase already uses 'unparsable'.
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
2 participants
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
Stacked on #1195.
Adds a plain
Prepdevice on the Hamilton TCP transport (channels/head8/ gripper), plus PrepDeck, liquid-class resolver, shared tip/volume finalize helpers, and a basic demo wired into the Hamilton docs.Resource-facing APIs (no LiquidHandler/Backend) are intentionally not included.