feat: Squid laser engine — status polling, GUI tab, acquisition gating#536
Merged
Conversation
Add LaserChannelState, TcmModuleInfo, LaserChannelInfo, SquidLaserEngineStatus, and SquidLaserEngineError to serial_peripherals.py, plus 11 unit tests covering ready/error/display_state logic including the dual-TCM 55x channel. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add module-level helpers _parse_status_packet and _build_command_packet to control/serial_peripherals.py, decoding the firmware 'S' status packet (big-endian centidegree int16 fields, 6 TCM blocks, ΔT and hi-temp setpoints) into SquidLaserEngineStatus, and building command packets with little-endian CRC32 + 0x0A 0x0D framing. Tests in tests/control/test_squid_laser_engine.py cover all-active, temperature parsing, negative ΔT setpoint inversion, 55x dual-module mapping, laser TTL, warming-up state, truncated/wrong-cmd payload rejection, and build-command framing for query/wake/sleep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…mport, add edge-case tests)
Adds the shared _SquidLaserEngineBase (QObject) holding all wake/sleep and wait-until-ready logic, plus a SquidLaserEngine_Simulation that synthesizes status on a tick thread for headless tests. Subclass hooks (_send_query, _send_wake, _send_sleep, start, close) keep the surface identical to the real engine added in the next task. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the production SquidLaserEngine class alongside the existing simulation. Two daemon threads: a query thread that sends 'Q' every query_interval_s and a receive thread that frames \x0A\x0D-terminated packets, verifies CRC32, and dispatches status frames to _publish_status. Wake/sleep commands route through a shared serial-write lock; serial exceptions trigger _signal_connection_lost and stop both threads. Tests inject a _FakeSerial via the _test_serial constructor hook and cover periodic Q transmission, status parsing, wake/sleep packet shape, CRC-mismatch drop accounting, and connection_lost emission on SerialException (using qtbot to pump the cross-thread signal).
- Use _running.is_set() for close() idempotency; drop redundant _closed flag. - Remove unused progress_cb parameter from wait_until_ready (YAGNI). - Skip status emission in simulator when no module state changed (dirty flag). - Avoid redundant dict writes in held-state and wake/sleep paths. - Make crc_mismatch_count / parse_failure_count read-only properties. - Capture engine reference at dialog show-time so close-time disconnect hits the right object. - Tighten the dialog-disconnect except clause to TypeError only so a RuntimeError from a destroyed C++ object can't be silently swallowed. - Drop a defensive try/except around wake_up() in Live mode that was catching nothing real (wake_up itself handles serial errors internally). - Export LASER_CHANNEL_ORDER from serial_peripherals so the widget can reuse the canonical channel order instead of duplicating it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class integration for the Cephla Squid laser engine (real + simulation), including background status polling, a dedicated GUI status tab, and readiness-based acquisition gating to prevent dim images when channels are not yet ACTIVE.
Changes:
- Introduces
SquidLaserEngine/SquidLaserEngine_Simulationwith packet parsing/building, status signals, andwait_until_readygating. - Adds a Laser Engine GUI tab and integrates engine lifecycle into
MicroscopeAddons(start/wake on prepare; close on shutdown). - Adds readiness behavior: live-mode warn-only flow and multipoint acquisition gating with a modal progress dialog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| software/control/serial_peripherals.py | Adds Squid laser engine protocol/types, real + simulation controllers, background threads, and readiness wait logic. |
| software/control/laser_engine_widget.py | New GUI widget/tab to display per-channel status and allow Wake/Sleep actions. |
| software/control/microscope.py | Creates/starts/closes the laser engine addon under USE_SQUID_LASER_ENGINE. |
| software/control/core/live_controller.py | Adds a warn-only laser-engine readiness check and a Qt signal for user-facing warnings. |
| software/control/gui_hcs.py | Wires in Laser Engine tab, shows readiness modal during acquisition gating, and surfaces live warnings. |
| software/control/core/multi_point_worker.py | Gates each timepoint on laser-engine readiness via wait_until_ready. |
| software/control/core/multi_point_utils.py | Extends controller callbacks with laser-engine waiting/ready hooks. |
| software/control/_def.py | Adds configuration flags/timeouts for Squid laser engine integration. |
| software/tests/control/test_squid_laser_engine.py | New unit tests covering protocol parsing/building, simulation, fake-serial round-trips, and wait logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Shared logic for the real and simulation engines. | ||
|
|
||
| Subclasses must implement: _send_query, _send_wake(channel_index), | ||
| _send_sleep(channel_index), is_connection_lost. |
| if not ok: | ||
| if self.abort_requested_fn(): | ||
| return # outer abort check handles this | ||
| raise RuntimeError("Laser engine did not reach ready state within timeout; aborting acquisition") |
Comment on lines
203
to
208
| fluidics: Optional[Fluidics] = None, | ||
| piezo_stage: Optional[PiezoStage] = None, | ||
| sci_microscopy_led_array: Optional[SciMicroscopyLEDArray] = None, | ||
| squid_laser_engine: Optional["serial_peripherals.SquidLaserEngine"] = None, | ||
| ): | ||
| self.xlight: Optional[serial_peripherals.XLight] = xlight |
Comment on lines
+1867
to
+1872
| def _on_live_controller_warning(self, message: str) -> None: | ||
| """Surface a non-fatal warning from LiveController (e.g. laser engine | ||
| not yet ready). Logs and shows a non-blocking message box.""" | ||
| self.log.warning(message) | ||
| QMessageBox.warning(self, "Laser engine", message) | ||
|
|
- Rename `_SquidLaserEngineBase` → `SquidLaserEngineBase`. The shared base is referenced from `microscope.py`'s public type hint, so the leading underscore was misleading. Also corrected the docstring to list the actual subclass hooks (start, close, _send_query, _send_wake, _send_sleep) — `is_connection_lost` is provided by the base. - Type `MicroscopeAddons.squid_laser_engine` against the base class so the simulation variant is covered too (was annotated only against the real engine). - Replace the misleading "did not reach ready state within timeout" error in the acquisition gate with a branched message: distinguish connection-lost vs timeout, and include the channel keys being waited on. - Make the live-mode warning genuinely non-blocking: use `QMessageBox(...).show()` instead of the modal `QMessageBox.warning(...)`. Drop repeats within 5s so a flapping engine doesn't stack dialogs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ate line - Move the Laser Engine tab to the end of the camera tab area (after Focus Map) instead of between Piezo and NL5. - Replace the QTableWidget with one QLabel per channel — simpler markup, monospaced text, per-state inline color span. The table was overkill for 5 fixed rows. - Drop the "Last update: X s ago" line and its 1s QTimer. The status_updated signal already drives all repaints; the age display was noise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…string format spec)
- Format the channel-keys list in the laser-engine error message with comma-join instead of repr-of-list, so the message reads "channel(s) 470, 55x" instead of "channel(s) ['470', '55x']". - Drop a noise comment on _format_temp. - Trim the GUI-thread serial.write justification to one line. - Type _channel_lines as Dict[str, QLabel] instead of bare dict. - Set Qt.WA_DeleteOnClose on the live-warning QMessageBox and reuse a stored reference, so a flapping engine can't accumulate boxes that the user hasn't dismissed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…class Drops SQUID_LASER_ENGINE_QUERY_INTERVAL_S and SQUID_LASER_ENGINE_READY_TIMEOUT_S from control/_def.py — neither was machine-specific config that warranted a global flag. The defaults now live as class attributes on SquidLaserEngineBase (DEFAULT_QUERY_INTERVAL_S, READY_TIMEOUT_S), still overridable per-instance via the constructor and per-call via wait_until_ready's timeout_s arg. MicroscopeAddons no longer threads the query interval through to the engine constructor — the default applies. The acquisition gate reads the timeout off the engine instance instead of the global. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves the ~660 lines of laser-engine code (status types, parser/builder, SquidLaserEngineBase, SquidLaserEngine, SquidLaserEngine_Simulation, SquidLaserEngineError, public LASER_CHANNEL_ORDER) out of control/serial_peripherals.py and into a dedicated control/squid_laser_engine.py. Updates the three importers (control/microscope.py, control/laser_engine_widget.py, tests/control/test_squid_laser_engine.py) to pull from the new module. serial_peripherals.py loses its now-unused IntEnum/dataclass/crc32/QObject/Signal /Iterable/List imports. Also reverts control/_def.py to ship-default values (USE_SQUID_LASER_ENGINE=False, SQUID_LASER_ENGINE_SN=None) — the prior commit accidentally captured a local hardware-testing edit. Local users continue to flip these to True+real-SN as needed; we just don't commit those changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WA_DeleteOnClose tears down the Qt C++ side when the user clicks Ok, leaving the Python reference stale. The next warning calls prior.close() on the dead wrapper and raises RuntimeError. - Wrap the prior.close() in try/except RuntimeError so a dead wrapper doesn't crash the warning path. - Connect to the destroyed signal so we clear the reference proactively when the C++ object goes away.
…race
When close() clears _running and closes the port, an in-flight
self._serial.read(1) in the receive loop can race the fd cleanup.
pyserial 3.5 raises a TypeError ('NoneType' cannot be interpreted as
an integer') from inside os.read because self.fd has been nulled out —
not the SerialException/OSError we were catching.
Broaden the except to any Exception, then check _running to distinguish
shutdown (exit quietly) from a real disconnect (log + signal
connection_lost). Same fix applied to _write_packet for symmetry.
Tests still green; no behavior change on the live-runtime disconnect
path because that path leaves _running set when the read fails.
Comment on lines
1
to
8
| import abc | ||
| import threading | ||
|
|
||
| import serial | ||
| from serial.tools import list_ports | ||
| import time | ||
| from typing import Tuple, Optional | ||
| from typing import Callable, Optional, Tuple | ||
| import struct |
Comment on lines
+1905
to
+1910
| # 0/0 → indeterminate progress. No Cancel button — the regular Abort button still works. | ||
| self._laser_engine_dialog = QProgressDialog(msg, "", 0, 0, self) | ||
| self._laser_engine_dialog.setWindowTitle("Laser engine warming up") | ||
| self._laser_engine_dialog.setCancelButton(None) | ||
| self._laser_engine_dialog.setWindowModality(Qt.ApplicationModal) | ||
| # Live label updates as new status comes in. Capture the engine reference here |
Comment on lines
+531
to
+540
| def close(self) -> None: | ||
| if not self._running.is_set(): | ||
| return | ||
| self._running.clear() | ||
| # Close the port before joining so any blocking read() unblocks. | ||
| if self._serial is not None: | ||
| try: | ||
| self._serial.close() | ||
| except Exception: | ||
| self._log.exception("Error closing serial port") |
Comment on lines
+592
to
+606
| def _receive_loop(self) -> None: | ||
| msg = bytearray() | ||
| while self._running.is_set(): | ||
| try: | ||
| chunk = self._serial.read(1) | ||
| except Exception as e: | ||
| if not self._running.is_set(): | ||
| return # shutdown race against close() | ||
| self._log.error(f"SquidLaserEngine read failed: {e}") | ||
| self._signal_connection_lost(str(e)) | ||
| self._running.clear() | ||
| return | ||
| if not chunk: | ||
| continue | ||
| byte = chunk[0] |
Comment on lines
+370
to
+372
| def force_connection_lost(self, message: str = "simulated drop") -> None: | ||
| self._signal_connection_lost(message) | ||
|
|
- serial_peripherals.py: drop now-unused `threading`, `Callable`, `Optional`, `Tuple`, `struct` imports left over after the laser-engine extraction. - gui_hcs.py: make the per-timepoint laser-engine progress dialog non-modal (with WindowStaysOnTopHint) instead of ApplicationModal — so the main-window Abort button stays reachable while the wait dialog is up. The docstring and PR description both said abort still worked, but ApplicationModal blocked it. - squid_laser_engine.SquidLaserEngine.close: hold _serial_lock around the serial.close() so a concurrent _write_packet can't race the shutdown. - squid_laser_engine.SquidLaserEngine._receive_loop: read whole frames with serial.read_until(b"\\x0a\\x0d", size=2048) instead of byte-by- byte read(1). Drops the syscall count from ~76/sec to ~1/sec at the default 1 Hz query cadence; size=2048 gives the same garbage-resync bound as the old defensive 1024-byte clamp. - squid_laser_engine.SquidLaserEngine_Simulation.force_connection_lost: also clear _running so the simulator stops ticking after a forced disconnect, mirroring the real engine's behavior on I/O failure. - _FakeSerial test helper: add read_until that loops self.read(1) so the existing monkeypatch-based connection_lost test still triggers.
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.
Summary
Adds bidirectional USB-serial communication with the Cephla Squid laser engine (Teensy 4.1 + 5 lasers + 6 TCM modules):
SquidLaserEngine(real) andSquidLaserEngine_Simulationclasses incontrol/serial_peripherals.py— two background threads (1 Hz query + receive) emittingstatus_updated(SquidLaserEngineStatus)andconnection_lost(str)Qt signals.MicroscopeAddonsintegration: builds the engine whenUSE_SQUID_LASER_ENGINE=True, callsstart()then non-blockingwake_up_all()inprepare_for_use, closes idempotently inMicroscope.close().LiveController.start_live()if the channel about to be used is not yet ACTIVE — fireswake_up()on the side and surfaces the warning via a newsignal_warningand aQMessageBox.warningslot.MultiPointWorker.run(). If the channels needed for this acquisition aren't ready, the worker thread blocks onwait_until_readywhile the main thread shows a non-cancelable modalQProgressDialogwith live per-channel state. The regular Abort button still cancels the wait via the existing abort flag. Re-checked every timepoint so long-dtsleep gaps are handled correctly.Behind the new
USE_SQUID_LASER_ENGINEflag in_def.py(defaultFalse); existing setups are unaffected.Test plan
python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py— 1247 passed (matches baseline) + 41 new tests intests/control/test_squid_laser_engine.pycovering the protocol parser/builder, simulation state machine, fake-serial round-trips, andwait_until_readybehaviour (happy path, timeout, cancel, ERROR, connection-lost).black --config pyproject.toml --check .— clean.USE_SQUID_LASER_ENGINE = True, runpython3 main_hcs.py --simulation, confirm the tab appears, Wake-All / Sleep-All toggle states in the table, Live warns when a channel is sleeping, and a multipoint acquisition shows the modal then proceeds.SQUID_LASER_ENGINE_SNto the USB serial number, confirm status polls at 1 Hz and wake/sleep commands take effect.Notes
405,470,55x,638,730with55xdisplayed in the middle of the list. Optical aliases (488→'470', 545/550/555/561→'55x', 640→'638', 735/750→'730') are mapped automatically bychannel_keys_for_wavelengths.LightSource/IlluminationControllerabstraction (LDI/CELESTA path) yet — that refactor is a follow-up. This PR is purely additive: status polling, GUI surfacing, and acquisition gating. Existing TTL-based laser triggering is unchanged.USE_SQUID_LASER_ENGINE=Truebut the device can't be found,MicroscopeAddons.prepare_for_usepropagates the exception and the app fails to boot — intentional, so the configuration error surfaces immediately.🤖 Generated with Claude Code