Skip to content

io: keep a cancelled read's bytes, and release a device a failed setup claimed - #1201

Open
BioCam wants to merge 2 commits into
fix/ftdi-executor-offloadfrom
io-cancellation-safety
Open

io: keep a cancelled read's bytes, and release a device a failed setup claimed#1201
BioCam wants to merge 2 commits into
fix/ftdi-executor-offloadfrom
io-cancellation-safety

Conversation

@BioCam

@BioCam BioCam commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Builds on #1199, based on that branch, so this is the two commits on top.

Moving the blocking calls onto the executor makes them cancellable for the first time. asyncio.wait_for around a read on io.ftdi used to be a no-op, because the call never suspended and so always ran to completion; now the timeout fires. Cancelling the task does not stop the worker thread though: the transfer finishes, its bytes land in a future nobody is waiting on, and they are dropped. Those bytes are already out of the chip's buffer, so the next read picks up after them, part-way through the response the driver was assembling. Nothing raises where the loss happens, so it surfaces later as a checksum failure or a reply matched to the wrong command. stop_shaking() in AgilentBioTekPlateReaderBase already has this shape: it cancels _shaking_task, which may be inside send_command.

Separately, USB.setup's new failure path clears self.dev after _setup_sync may already have claimed the device, and stop() returns early on dev is None, so nothing ever disposes it.

  • FTDI.read shields the executor call and keeps what it produced in a small buffer that the next read is served from. A read issued while one is in flight waits for that one instead of starting a second.
  • FTDI.write shields too, so a cancelled write reaches the device in full rather than leaving it holding a partial command, or none, with no way for the caller to tell which.
  • FTDI.usb_purge_rx_buffer, FTDI.usb_reset and FTDI.stop drop the buffer and anything in flight, since all three mean the caller has declared that data stale.
  • FTDICommand assigns data and takes module, like SerialCommand and USBCommand already do. It declared data without assigning it, so every FTDICommand was written out without its bytes, and rebuilding one from a capture file raised on module. FTDIValidator.read also compared the recorded hex string's length against a byte count, so no recorded read could match. FTDIValidator can now replay a recorded capture; wiring the validate() entry point back up, and widening FTDIValidator.__init__ to accept what FTDI.serialize() returns, are separate and not touched here.
  • FTDI.readline is assembled from single-byte reads, with a terminator and a timeout. pylibftdi's own raises TypeError unless the device was opened in text mode and FTDI.setup opens it in byte mode, so the inherited one could only ever raise. No backend on io.ftdi calls it; the fifteen readline callers in the tree are all on io.serial or io.socket.
  • USB.setup disposes the device before clearing it.

Behaviour: no change to a read or write that runs to completion, which is every caller in the tree today. A cancelled read still raises, but no longer costs the bytes already in flight; a cancelled write now always completes. FTDI.readline gains two optional arguments and returns an assembled line instead of raising. Captures from io.ftdi gain the data field they always declared; no existing io.ftdi capture file was replayable, so nothing regresses. io.serial, io.hid and io.usb reads have the same cancellation shape, pre-dating #1199 since they were already offloaded, and are left alone here.

Tests: FTDICancellationTests (a cancelled read's bytes reach the next read and are captured once, a cancelled write still reaches the device, a purge discards held bytes, a text-mode device still reads, a second concurrent reader backs off), FTDICaptureTests (a capture replays through FTDIValidator), FTDIReadlineTests (terminator, timeout, empty terminator), and USBSetupFailureTests (a failed setup disposes the claimed device). All twelve fail against #1199 alone. ruff format, ruff check --select I, ruff check and mypy pylabrobot/io --check-untyped-defs are clean; the full suite passes (2144 passed, 2 skipped, 162 subtests).

🤖 Generated with Claude Code

BioCam and others added 2 commits August 13, 2026 14:52
…ure needs to replay

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@BioCam
BioCam requested a review from rickwierenga August 13, 2026 14:03
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.

1 participant