Repository navigation
fix(ibidi): release multiplexer serial port deterministically on shutdown - #20
Merged
Merged
Conversation
…down After a PycroFlow restart the ibidi MultiFlOW could not reconnect without physically replugging USB/power. The port was only closed via GC-dependent __del__: SystemService.close() never disconnected the fluid system, and disconnect_fluid() closed only the Hamilton bus (ha.disconnect()), not the ibidi's separate serial link. Dropping a live USB-CDC handle (possibly mid-command) can also leave the firmware wedged until a physical replug. - SystemService.close() now calls disconnect_fluid() - disconnect_fluid() calls LegacyArchitecture.close() before tearing down the Hamilton bus and dropping the reference - new LegacyArchitecture.close(): deterministic, idempotent; stops the spill sensor and closes the multiplexer - IbidiMultiplexer.close() flushes + clears serial buffers before closing (graceful, not abrupt) - IbidiMultiplexer opens with exclusive=True (POSIX lock; no-op on Windows) - hermetic tests for all four paths Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses code-review findings on the shutdown path: - IbidiMultiplexer.close() no longer calls flush() — pyserial flush blocks until output drains (termios.tcdrain / out_waiting busy-wait), which on an unresponsive device (the exact restart scenario) would hang the entire shutdown, uncatchable by the try/except. Output is now discarded via reset_output_buffer instead. close() also runs under self._lock so it can't swap the handle out from under an in-flight _command on another thread. - LegacyArchitecture.close() releases each resource independently: a failure in _stop_spill_sensor() must not skip mux.close(), since the _closed guard allows only one attempt and would otherwise strand the ibidi port. - tests updated (flush must NOT be called) + new regression: spill-sensor failure still releases the port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Problem
On the rig, after restarting PycroFlow the ibidi MultiFlOW multiplexer could not be reconnected without physically unplugging/replugging the USB and/or power cables.
Root cause
The multiplexer's serial port was only ever closed through garbage-collection-dependent
__del__hooks, which are not guaranteed to run at interpreter exit:SystemService.close()(called fromcloseEvent) didstop_all_moves()+close_imaging()— it never disconnected the fluid system.disconnect_fluid()only closed the Hamilton bus (ha.disconnect()); the ibidi is a separate serial link that was never explicitly closed, and then thefluid_systemreference was dropped → close fell to GC.Two failure modes follow, and the fix addresses both:
Changes
SystemService.close()now callsdisconnect_fluid().disconnect_fluid()callsLegacyArchitecture.close()before tearing down the Hamilton bus and dropping the reference.LegacyArchitecture.close()— deterministic, idempotent counterpart to__del__; stops the spill sensor and closes the multiplexer.IbidiMultiplexer.close()flushes + clears serial buffers before closing (graceful, not abrupt).IbidiMultiplexeropens withexclusive=True(POSIX lock; no-op on Windows, where COM ports are already exclusive) so a stale handle fails the next open loudly.Tests
Hermetic (fake serial / MagicMock), all passing: flush-before-close ordering + idempotency; port release via
LegacyArchitecture.close();SystemService.close()disconnects the fluid architecture. (The unrelatedtest_placeholder_when_monet_absentfailure is pre-existing ondevelop.)Which cable actually fixes the reconnect: USB-only replug → OS/handle (this PR is sufficient); needs power cycle → firmware wedge, in which case a follow-up to leave the device idle (and possibly issue a reset command) on close is warranted.
🤖 Generated with Claude Code