From f478d7a7f9cac02c009478a61d6973b5d717ef64 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 03:07:30 -0400 Subject: [PATCH 01/15] feat(multi-plane): params + pure plane-offset/validation helpers Co-Authored-By: Claude Fable 5 --- .../control/core/record_zstack_controller.py | 18 ++++++++++++++- software/control/core/record_zstack_worker.py | 2 +- software/control/widgets.py | 8 ++++++- .../tests/core/test_record_zstack_worker.py | 22 ++++++++++++++++--- software/tests/test_record_zstack_widget.py | 11 ++++++++++ 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/software/control/core/record_zstack_controller.py b/software/control/core/record_zstack_controller.py index f3b560886..261fff172 100644 --- a/software/control/core/record_zstack_controller.py +++ b/software/control/core/record_zstack_controller.py @@ -25,6 +25,19 @@ def zstack_offsets_um(z_min_um: float, z_max_um: float, step_um: float) -> List[ return [round(z_min_um + i * step_um, 6) for i in range(zstack_plane_count(z_min_um, z_max_um, step_um))] +def recording_plane_offsets_um(bottom_um: float, nz: int, dz_um: float) -> List[float]: + """Offsets (µm, relative to the z reference) of the Nz recording planes. + + Plane j sits at bottom_um + j*dz_um. Nz=1 is the single-plane case and + ignores dz_um (the widget hides dz entirely for Nz=1). + """ + if nz < 1: + raise ValueError("require Nz >= 1") + if nz > 1 and dz_um <= 0: + raise ValueError("require dz > 0 when Nz > 1") + return [round(bottom_um + j * dz_um, 6) for j in range(nz)] + + @dataclass class RecordZStackAcquisitionParameters: base_path: str @@ -37,7 +50,10 @@ class RecordZStackAcquisitionParameters: recording_channel: Optional[AcquisitionChannel] = None fps: float = 10.0 duration_s: float = 1.0 - recording_z_offset_um: float = 0.0 + # Recording planes: plane j at z_ref + recording_bottom_z_offset_um + j*recording_dz_um. + recording_bottom_z_offset_um: float = 0.0 + recording_Nz: int = 1 + recording_dz_um: float = 1.0 # z-stack phase zstack_enabled: bool = False zstack_channels: List[AcquisitionChannel] = field(default_factory=list) diff --git a/software/control/core/record_zstack_worker.py b/software/control/core/record_zstack_worker.py index 7d1cec2dc..4016bda56 100644 --- a/software/control/core/record_zstack_worker.py +++ b/software/control/core/record_zstack_worker.py @@ -354,7 +354,7 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: self._select_config(rec_channel) # Move to z_ref + recording offset. - self._move_z_to_offset(z_ref, self.params.recording_z_offset_um) + self._move_z_to_offset(z_ref, self.params.recording_bottom_z_offset_um) # Size the dataset, pacing, and time metadata from the fps the camera can # actually deliver: a camera clamped below the requested rate (exposure diff --git a/software/control/widgets.py b/software/control/widgets.py index 5f678d5f2..9373d59b5 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -16958,6 +16958,8 @@ def _validate_record_zstack_params( zstack_channel_names: List[str], use_laser_af: bool, laser_af_has_reference: bool, + recording_nz: int = 1, + recording_dz_um: float = 1.0, ) -> Optional[str]: """Return an error string describing the first validation failure, or None if valid.""" if not base_path: @@ -16977,6 +16979,10 @@ def _validate_record_zstack_params( return f"Recording: fps×duration yields 0 frames (fps={fps}, duration={duration_s}s). Increase one or both." if not recording_channel_name: return "A channel must be chosen for the Recording phase." + if recording_nz < 1: + return "Recording Nz must be at least 1." + if recording_nz > 1 and recording_dz_um <= 0: + return "Recording dz must be > 0 when Nz > 1." if zstack_enabled: if z_max <= z_min: return "Z-Stack: z_max must be greater than z_min." @@ -17787,7 +17793,7 @@ def _make_channel_base(name: str) -> AcquisitionChannel: recording_channel=recording_channel, fps=self.entry_fps.value(), duration_s=self.entry_duration.value(), - recording_z_offset_um=self.entry_recording_z_offset.value(), + recording_bottom_z_offset_um=self.entry_recording_z_offset.value(), zstack_enabled=self.checkbox_zstack.isChecked(), zstack_channels=zstack_channels, z_min_um=self.entry_zmin.value(), diff --git a/software/tests/core/test_record_zstack_worker.py b/software/tests/core/test_record_zstack_worker.py index 8f83e367a..da0c8f751 100644 --- a/software/tests/core/test_record_zstack_worker.py +++ b/software/tests/core/test_record_zstack_worker.py @@ -108,7 +108,7 @@ def test_record_zstack_worker_smoke(tmp_path): recording_channel=recording_channel, fps=fps, duration_s=duration_s, - recording_z_offset_um=0.0, + recording_bottom_z_offset_um=0.0, zstack_enabled=True, zstack_channels=zstack_channels, z_min_um=-1.0, @@ -246,7 +246,7 @@ def test_record_zstack_controller_smoke(tmp_path): recording_channel=recording_channel, fps=fps, duration_s=duration_s, - recording_z_offset_um=0.0, + recording_bottom_z_offset_um=0.0, zstack_enabled=True, zstack_channels=zstack_channels, z_min_um=-1.0, @@ -364,7 +364,7 @@ def _build_worker_harness(tmp_path, recording_enabled, zstack_enabled, zstack_ch recording_channel=channels[0], fps=10.0, duration_s=0.2, - recording_z_offset_um=0.0, + recording_bottom_z_offset_um=0.0, zstack_enabled=zstack_enabled, zstack_channels=list(channels[zstack_channel_slice]) if zstack_enabled else [], z_min_um=0.0, @@ -754,3 +754,19 @@ def test_config_snapshot_dedupes_channels_by_name(tmp_path): snapshot = yaml.safe_load(open(Path(tmp_path) / params.experiment_id / "acquisition_channels.yaml")) names = [c["name"] for c in snapshot["channels"]] assert len(names) == len(set(names)), f"duplicate channel names in snapshot: {names}" + + +def test_recording_plane_offsets(): + from control.core.record_zstack_controller import recording_plane_offsets_um + + assert recording_plane_offsets_um(-2.0, 3, 4.0) == [-2.0, 2.0, 6.0] + assert recording_plane_offsets_um(5.0, 1, 1.0) == [5.0] # Nz=1: dz irrelevant + + +def test_recording_plane_offsets_validation(): + from control.core.record_zstack_controller import recording_plane_offsets_um + + with pytest.raises(ValueError): + recording_plane_offsets_um(0.0, 0, 1.0) # Nz < 1 + with pytest.raises(ValueError): + recording_plane_offsets_um(0.0, 2, 0.0) # dz <= 0 with Nz > 1 diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 40b808833..7cc5d7781 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1061,3 +1061,14 @@ def test_refresh_channel_list_warns_on_silent_selection_swap(qtbot, simulated_wi assert any( prev in rec.message and "Only Channel" in rec.message for rec in caplog.records ), f"no warning about the recording selection changing from {prev!r}" + + +def test_validate_helper_recording_nz_and_dz(): + params = _base_params(recording_enabled=True, recording_nz=0) + assert _validate_record_zstack_params(**params) is not None # Nz < 1 + params = _base_params(recording_enabled=True, recording_nz=3, recording_dz_um=0.0) + assert _validate_record_zstack_params(**params) is not None # dz <= 0 with Nz > 1 + params = _base_params(recording_enabled=True, recording_nz=1, recording_dz_um=0.0) + assert _validate_record_zstack_params(**params) is None # Nz=1: dz irrelevant + params = _base_params(recording_enabled=True, recording_nz=3, recording_dz_um=0.5) + assert _validate_record_zstack_params(**params) is None From efe75a1a89d8022b4c77289208689e2494a43c8e Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 03:15:45 -0400 Subject: [PATCH 02/15] feat(multi-plane): extra _squid attrs passthrough on ZarrAcquisitionConfig Co-Authored-By: Claude Fable 5 --- software/control/core/zarr_writer.py | 6 ++++ software/tests/core/test_streaming_capture.py | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/software/control/core/zarr_writer.py b/software/control/core/zarr_writer.py index e89221a0c..52288b982 100644 --- a/software/control/core/zarr_writer.py +++ b/software/control/core/zarr_writer.py @@ -73,6 +73,9 @@ class ZarrAcquisitionConfig: compression: ZarrCompression = ZarrCompression.FAST is_hcs: bool = True # Default to HCS (5D); non-HCS uses 6D with FOV dimension plate_name: str = "plate" + # Optional extra keys merged into the "_squid" attributes at metadata-write + # time (e.g. per-plane recording metadata: plane_index, plane_z_offset_um). + extra_squid_attrs: Optional[Dict[str, object]] = None @property def ndim(self) -> int: @@ -621,6 +624,9 @@ def _write_zarr_metadata(self) -> None: }, } + if config.extra_squid_attrs: + zattrs["_squid"].update(config.extra_squid_attrs) + # Write metadata to zarr.json attributes (strict Zarr v3 compliance) # For HCS, output_path is the array path ({fov}/0), but OME-NGFF metadata # should be at the parent group level ({fov}/zarr.json), not the array level. diff --git a/software/tests/core/test_streaming_capture.py b/software/tests/core/test_streaming_capture.py index 9b9fbb8f8..440c4faae 100644 --- a/software/tests/core/test_streaming_capture.py +++ b/software/tests/core/test_streaming_capture.py @@ -795,3 +795,36 @@ def stop_streaming(self): src2.start(lambda f: None) src2.stop() assert cam2.mode_calls == 1 and cam2.rate_calls == 1 # default behavior unchanged + + +def test_zarr_config_extra_squid_attrs_written(tmp_path): + """Per-plane recording metadata rides in _squid via extra_squid_attrs.""" + import json + import pytest as _pytest + + _pytest.importorskip("tensorstore") + from control.core.zarr_writer import ZarrAcquisitionConfig, ZarrWriter + + cfg = ZarrAcquisitionConfig( + output_path=str(tmp_path / "plane.ome.zarr"), + shape=(2, 1, 1, 4, 4), + dtype=np.uint16, + pixel_size_um=1.0, + z_step_um=None, + time_increment_s=0.1, + channel_names=["BF"], + channel_colors=["#FFFFFF"], + channel_wavelengths=[None], + is_hcs=False, + extra_squid_attrs={"plane_index": 2, "plane_z_offset_um": 6.0, "n_planes": 3}, + ) + w = ZarrWriter(cfg) + w.initialize() + w.write_frame(np.zeros((4, 4), np.uint16), t=0, c=0, z=0) + w.finalize() + + meta = json.load(open(tmp_path / "plane.ome.zarr" / "zarr.json")) + squid_attrs = meta["attributes"]["_squid"] + assert squid_attrs["plane_index"] == 2 + assert squid_attrs["plane_z_offset_um"] == 6.0 + assert squid_attrs["n_planes"] == 3 From 18a75fa962bfbb14b99191b61da915f84a6233a4 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 11:56:06 -0400 Subject: [PATCH 03/15] feat(multi-plane): per-plane recording loop, paths, and metadata in RecordZStackWorker Co-Authored-By: Claude Fable 5 --- software/control/core/record_zstack_worker.py | 158 ++++++++++++------ .../tests/core/test_record_zstack_worker.py | 69 ++++++++ 2 files changed, 175 insertions(+), 52 deletions(-) diff --git a/software/control/core/record_zstack_worker.py b/software/control/core/record_zstack_worker.py index 4016bda56..92b47c083 100644 --- a/software/control/core/record_zstack_worker.py +++ b/software/control/core/record_zstack_worker.py @@ -44,6 +44,7 @@ from control.core.record_zstack_controller import ( RecordZStackAcquisitionParameters, frame_count, + recording_plane_offsets_um, zstack_offsets_um, zstack_plane_count, ) @@ -344,24 +345,27 @@ def _laser_af_has_reference(self) -> bool: # ---------------------------------------------------------------- record def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: - """Record a continuous stream to a per-FOV recording zarr, then restore Z. - - Returns the number of frames emitted. + """Record per-plane continuous streams for this FOV, then restore Z. + + Plane j records at ``z_ref + recording_bottom_z_offset_um + j*recording_dz_um`` + (one Zarr store per plane; Nz=1 keeps the historical single-plane layout + byte-identical to before per-plane recording existed). The recording + channel, achievable fps, dataset frame count, and illumination are all + established once per FOV and shared across every plane. Returns the + total number of frames emitted across all planes. """ # Apply the recording channel (exposure/gain/illumination settings). rec_channel = self.params.recording_channel if rec_channel is not None: self._select_config(rec_channel) - # Move to z_ref + recording offset. - self._move_z_to_offset(z_ref, self.params.recording_bottom_z_offset_um) - # Size the dataset, pacing, and time metadata from the fps the camera can # actually deliver: a camera clamped below the requested rate (exposure # limit, PRECISE_FRAMERATE max) can never fill fps*duration frames within # duration seconds — the run would stall to the timeout and leave the # trailing planes blank. The mode switch happens first because toupcam - # resets its frame-rate strategy on mode change. + # resets its frame-rate strategy on mode change. This is established + # once per FOV (not per plane): every plane records at the same fps. self.camera.set_acquisition_mode(CameraAcquisitionMode.CONTINUOUS) effective_fps = self.params.fps try: @@ -374,13 +378,83 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: effective_fps = achievable_fps except Exception: log.exception("set_frame_rate probe failed; assuming the requested fps") - + if effective_fps != self.params.fps: + # The probe requested the original fps; align the camera hint with + # the clamped rate every plane will actually pace/size against. + # Each plane's ContinuousFrameSource is already_configured=True and + # will not repeat this call. + try: + self.camera.set_frame_rate(effective_fps) + except Exception: + log.exception("failed to re-apply clamped frame rate") T = max(1, frame_count(effective_fps, self.params.duration_s)) - out = self._recording_path(t_idx, region_id, fov_idx) + + offsets = recording_plane_offsets_um( + self.params.recording_bottom_z_offset_um, self.params.recording_Nz, self.params.recording_dz_um + ) + n_planes = len(offsets) + + # The CONTINUOUS stream does not gate illumination per-frame, and + # set_microscope_mode only energizes illumination when live (we are not + # live here). Illumination stays on across all planes of this FOV (no + # flicker, fewer MCU commands); _record_one_plane assumes it is already + # energized. + total_emitted = 0 + self.liveController.turn_on_illumination() + try: + for plane_idx, plane_offset_um in enumerate(offsets): + if plane_idx > 0 and self.abort_requested_fn(): + log.info(f"abort requested; skipping recording planes {plane_idx}..{n_planes - 1}") + break + self._move_z_to_offset(z_ref, plane_offset_um) + total_emitted += self._record_one_plane( + t_idx, region_id, fov_idx, plane_idx, n_planes, plane_offset_um, effective_fps, T + ) + finally: + self.liveController.turn_off_illumination() + + # Restore the camera to a software-trigger-friendly state and Z reference. + try: + self.camera.set_acquisition_mode(CameraAcquisitionMode.SOFTWARE_TRIGGER) + except Exception: + log.exception("Failed to restore software-trigger acquisition mode after recording") + self.stage.move_z_to(z_ref) + self.wait_till_operation_is_completed() + self._sleep(SCAN_STABILIZATION_TIME_MS_Z / 1000) + return total_emitted + + def _record_one_plane( + self, + t_idx: int, + region_id, + fov_idx: int, + plane_idx: int, + n_planes: int, + plane_offset_um: float, + effective_fps: float, + T: int, + ) -> int: + """Record one plane's continuous stream to its own Zarr store. + + Assumes illumination is already on, the fps/mode/T setup already ran in + ``record()``, and the stage is already at the plane. Returns the number + of frames emitted for this plane. + """ + rec_channel = self.params.recording_channel + out = self._recording_path(t_idx, region_id, fov_idx, plane_idx=plane_idx, n_planes=n_planes) y, x, dtype = self._frame_shape rec_channel_name = rec_channel.name if rec_channel is not None else "REC" rec_color = rec_channel.display_color if rec_channel is not None else "#FFFFFF" + # Nz=1 keeps extra_squid_attrs=None so single-plane output metadata is + # byte-identical to before per-plane recording existed. + extra_attrs = None + if n_planes > 1: + extra_attrs = { + "plane_index": plane_idx, + "plane_z_offset_um": plane_offset_um, + "n_planes": n_planes, + } cfg = ZarrAcquisitionConfig( output_path=out, shape=(T, 1, 1, y, x), @@ -392,14 +466,8 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: channel_colors=[rec_color], channel_wavelengths=[None], is_hcs=False, + extra_squid_attrs=extra_attrs, ) - if effective_fps != self.params.fps: - # The probe requested the original fps; align the camera hint with - # the clamped rate we will actually pace/size against. - try: - self.camera.set_frame_rate(effective_fps) - except Exception: - log.exception("failed to re-apply clamped frame rate") writer = RecordingWriter(cfg) cap = StreamingCapture( ContinuousFrameSource(self.camera, effective_fps, already_configured=True), @@ -410,35 +478,21 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: ) # Generous timeout: enough to gather T frames even at a slow effective rate. timeout = self.params.duration_s * 3 + 5 - - # The CONTINUOUS stream does not gate illumination per-frame, and - # set_microscope_mode only energizes illumination when live (we are not - # live here). Turn illumination on for the whole recording, off in finally, - # so the recorded frames are not dark. - self.liveController.turn_on_illumination() - try: - emitted = cap.run(timeout=timeout) - finally: - self.liveController.turn_off_illumination() - log.info(f"recording done t={t_idx} region={region_id} fov={fov_idx}: {emitted}/{T} frames") - - # Restore the camera to a software-trigger-friendly state and Z reference. - try: - self.camera.set_acquisition_mode(CameraAcquisitionMode.SOFTWARE_TRIGGER) - except Exception: - log.exception("Failed to restore software-trigger acquisition mode after recording") - self.stage.move_z_to(z_ref) - self.wait_till_operation_is_completed() - self._sleep(SCAN_STABILIZATION_TIME_MS_Z / 1000) - # Fail fast on write errors OR a wedged drain thread: either way the - # cause is almost always systematic (full disk, stalled mount), so - # continuing would burn the timeout at every remaining FOV producing - # blank data. A wedged finalize returns before errors are countable, - # which is why write_error_count alone is not sufficient. - # run() catches this, aborts, and signals finished. + emitted = cap.run(timeout=timeout) + log.info( + f"recording done t={t_idx} region={region_id} fov={fov_idx} " + f"plane={plane_idx + 1}/{n_planes}: {emitted}/{T} frames" + ) + # Fail fast on write errors OR a wedged drain thread OR dropped frames: + # either way the cause is almost always systematic (full disk, stalled + # mount), so continuing would burn the timeout at every remaining + # plane/FOV producing blank data. A wedged finalize returns before + # errors are countable, which is why write_error_count alone is not + # sufficient. record() lets this propagate; run() catches it, aborts, + # and signals finished. if writer.write_error_count > 0 or writer.finalize_wedged or writer.dropped_count > 0: raise RuntimeError( - f"recording failed at t={t_idx} region={region_id} fov={fov_idx} " + f"recording failed at t={t_idx} region={region_id} fov={fov_idx} plane={plane_idx} " f"(write errors={writer.write_error_count}, dropped={writer.dropped_count}, " f"drain wedged={writer.finalize_wedged}); store sealed incomplete: {out}" ) @@ -583,15 +637,15 @@ def _probe_frame_shape(self) -> Tuple[int, int, np.dtype]: dtype = np.uint16 return int(height), int(width), np.dtype(dtype) - def _recording_path(self, t_idx: int, region_id, fov_idx: int) -> str: - """Per-(t, region, fov) recording dataset path under {experiment}/recording.""" - return os.path.join( - self.experiment_path, - "recording", - f"t{t_idx}", - str(region_id), - f"fov_{fov_idx}.ome.zarr", - ) + def _recording_path(self, t_idx: int, region_id, fov_idx: int, plane_idx: int = 0, n_planes: int = 1) -> str: + """Per-(t, region, fov[, plane]) recording dataset path under {experiment}/recording. + + Single-plane recordings (n_planes == 1) keep the historical + ``fov_{k}.ome.zarr`` name; multi-plane recordings get one store per + plane: ``fov_{k}_z{j}.ome.zarr``. + """ + fov_name = f"fov_{fov_idx}.ome.zarr" if n_planes == 1 else f"fov_{fov_idx}_z{plane_idx}.ome.zarr" + return os.path.join(self.experiment_path, "recording", f"t{t_idx}", str(region_id), fov_name) def _zstack_dir(self, region_id) -> str: """Directory passed as ``current_path`` to the inherited capture path. diff --git a/software/tests/core/test_record_zstack_worker.py b/software/tests/core/test_record_zstack_worker.py index da0c8f751..db538a735 100644 --- a/software/tests/core/test_record_zstack_worker.py +++ b/software/tests/core/test_record_zstack_worker.py @@ -770,3 +770,72 @@ def test_recording_plane_offsets_validation(): recording_plane_offsets_um(0.0, 0, 1.0) # Nz < 1 with pytest.raises(ValueError): recording_plane_offsets_um(0.0, 2, 0.0) # dz <= 0 with Nz > 1 + + +def test_recording_path_plane_naming(): + from types import SimpleNamespace + + from control.core.record_zstack_worker import RecordZStackWorker + + fake_self = SimpleNamespace(experiment_path="/exp") + single = RecordZStackWorker._recording_path(fake_self, 0, "B2", 1) + multi = RecordZStackWorker._recording_path(fake_self, 0, "B2", 1, plane_idx=2, n_planes=3) + assert single.endswith("recording/t0/B2/fov_1.ome.zarr") # Nz=1 keeps today's name + assert multi.endswith("recording/t0/B2/fov_1_z2.ome.zarr") + + +def test_record_multi_plane_writes_one_zarr_per_plane(tmp_path): + """Nz=3 → three per-plane recordings per FOV with correct names, shapes, + and plane metadata; Nz=1 filename compatibility is covered by the smoke test.""" + pytest.importorskip("tensorstore") + import json + + scope, live_controller, channels, worker, aborted = _build_worker_harness( + tmp_path, recording_enabled=True, zstack_enabled=False + ) + worker.params.recording_Nz = 3 + worker.params.recording_dz_um = 2.0 + worker.params.recording_bottom_z_offset_um = -2.0 + + worker.run() + assert not aborted["v"] + + rec_dir = Path(tmp_path) / "state_restore" / "recording" / "t0" / "A1" + names = sorted(p.name for p in rec_dir.glob("*.ome.zarr")) + assert names == ["fov_0_z0.ome.zarr", "fov_0_z1.ome.zarr", "fov_0_z2.ome.zarr"] + + for j, name in enumerate(names): + meta = json.load(open(rec_dir / name / "zarr.json")) + squid_attrs = meta["attributes"]["_squid"] + assert squid_attrs["plane_index"] == j + assert squid_attrs["n_planes"] == 3 + assert abs(squid_attrs["plane_z_offset_um"] - (-2.0 + j * 2.0)) < 1e-9 + assert squid_attrs["acquisition_complete"] is True + assert squid_attrs["shape"][1:3] == [1, 1] # (T,1,1,Y,X) per plane + + +def test_record_multi_plane_abort_between_planes(tmp_path): + """Abort after the first plane completes → exactly one plane file, no error, + run() still finishes (state restore + done marker).""" + pytest.importorskip("tensorstore") + + scope, live_controller, channels, worker, aborted = _build_worker_harness( + tmp_path, recording_enabled=True, zstack_enabled=False + ) + worker.params.recording_Nz = 3 + worker.params.recording_dz_um = 1.0 + + orig = worker._record_one_plane + + def record_then_abort(*args, **kwargs): + emitted = orig(*args, **kwargs) + aborted["v"] = True # user presses Stop right after plane 0 finishes + return emitted + + worker._record_one_plane = record_then_abort + worker.run() + + rec_dir = Path(tmp_path) / "state_restore" / "recording" / "t0" / "A1" + names = sorted(p.name for p in rec_dir.glob("*.ome.zarr")) + assert names == ["fov_0_z0.ome.zarr"], f"expected only plane 0 after abort, got {names}" + assert (Path(tmp_path) / "state_restore" / ".done").exists() From ebd488001bcb90e6b79f302f95d7f3d84f991b80 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 12:07:26 -0400 Subject: [PATCH 04/15] feat(multi-plane): Bottom Z / Nz / dz recording fields with dz hidden at Nz=1 Co-Authored-By: Claude Fable 5 --- software/control/widgets.py | 93 ++++++++++++++++++--- software/tests/test_record_zstack_widget.py | 62 ++++++++++++++ 2 files changed, 143 insertions(+), 12 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index 9373d59b5..e21d39ee6 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17294,7 +17294,7 @@ def _build_recording_group(self) -> QGroupBox: table_row.addStretch(1) vbox.addLayout(table_row) - # Row 1: FPS | Duration | Z-offset + # Row 1: FPS | Duration fps_row = QHBoxLayout() fps_row.setSpacing(4) @@ -17317,19 +17317,67 @@ def _build_recording_group(self) -> QGroupBox: self.entry_duration.setMaximumWidth(90) fps_row.addWidget(self.entry_duration) - fps_row.addSpacing(4) - fps_row.addWidget(QLabel("Z-offset:")) - self.entry_recording_z_offset = QDoubleSpinBox() - self.entry_recording_z_offset.setRange(-1000, 1000) - self.entry_recording_z_offset.setValue(0.0) - self.entry_recording_z_offset.setSuffix(" μm") - self.entry_recording_z_offset.setKeyboardTracking(False) - self.entry_recording_z_offset.setMaximumWidth(95) - fps_row.addWidget(self.entry_recording_z_offset) - fps_row.addStretch(1) vbox.addLayout(fps_row) + # Row 2: Bottom Z | Nz | dz (dz hidden when Nz == 1) + plane_row = QHBoxLayout() + plane_row.setSpacing(4) + + plane_row.addWidget(QLabel("Bottom Z:")) + self.entry_recording_bottom_z = QDoubleSpinBox() + self.entry_recording_bottom_z.setRange(-500.0, 500.0) + self.entry_recording_bottom_z.setDecimals(1) + self.entry_recording_bottom_z.setSingleStep(0.5) + self.entry_recording_bottom_z.setValue(0.0) + self.entry_recording_bottom_z.setSuffix(" µm") + self.entry_recording_bottom_z.setKeyboardTracking(False) + self.entry_recording_bottom_z.setMaximumWidth(95) + self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") + plane_row.addWidget(self.entry_recording_bottom_z) + + plane_row.addSpacing(4) + plane_row.addWidget(QLabel("Nz:")) + self.entry_recording_Nz = QSpinBox() + self.entry_recording_Nz.setRange(1, 100) + self.entry_recording_Nz.setValue(1) + self.entry_recording_Nz.setKeyboardTracking(False) + self.entry_recording_Nz.setMaximumWidth(70) + self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") + plane_row.addWidget(self.entry_recording_Nz) + + plane_row.addSpacing(4) + self.label_recording_dz = QLabel("dz:") + plane_row.addWidget(self.label_recording_dz) + self.entry_recording_dz = QDoubleSpinBox() + self.entry_recording_dz.setRange(0.05, 100.0) + self.entry_recording_dz.setDecimals(2) + self.entry_recording_dz.setSingleStep(0.5) + self.entry_recording_dz.setValue(1.0) + self.entry_recording_dz.setSuffix(" µm") + self.entry_recording_dz.setKeyboardTracking(False) + self.entry_recording_dz.setMaximumWidth(95) + self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") + plane_row.addWidget(self.entry_recording_dz) + + plane_row.addStretch(1) + vbox.addLayout(plane_row) + + # Row 3: computed planes/time summary + planes_row = QHBoxLayout() + planes_row.setSpacing(4) + self.label_recording_planes = QLabel("") + planes_row.addWidget(self.label_recording_planes) + planes_row.addStretch(1) + vbox.addLayout(planes_row) + + # Wire up recording plane count / dz-visibility updates + self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) + self.entry_recording_dz.valueChanged.connect(self._update_recording_planes_ui) + self.entry_recording_bottom_z.valueChanged.connect(self._update_recording_planes_ui) + self.entry_duration.valueChanged.connect(self._update_recording_planes_ui) + self._update_recording_planes_ui() + return grp def _build_zstack_group(self) -> QGroupBox: @@ -17492,6 +17540,23 @@ def _update_zstack_planes_label(self) -> None: except Exception: self.label_zstack_planes.setText("-- planes") + def _update_recording_planes_ui(self) -> None: + """Toggle dz visibility (hidden entirely when Nz == 1 — user requirement) + and refresh the computed planes/time summary label.""" + nz = self.entry_recording_Nz.value() + multi = nz > 1 + self.label_recording_dz.setVisible(multi) + self.entry_recording_dz.setVisible(multi) + bottom = self.entry_recording_bottom_z.value() + per_fov_s = nz * self.entry_duration.value() + if multi: + top = bottom + (nz - 1) * self.entry_recording_dz.value() + self.label_recording_planes.setText( + f"{nz} planes: {bottom:+.1f} … {top:+.1f} µm rel. reference — {per_fov_s:.1f} s/FOV" + ) + else: + self.label_recording_planes.setText(f"1 plane @ {bottom:+.1f} µm — {per_fov_s:.1f} s/FOV") + def _add_zstack_channel_row( self, name: str, exposure: float = 50.0, gain: float = 0.0, illumination: float = 50.0 ) -> None: @@ -17726,6 +17791,8 @@ def validate(self) -> Optional[str]: zstack_channel_names=list(self._zstack_channel_names), use_laser_af=self.checkbox_laser_af.isChecked(), laser_af_has_reference=self._laser_af_has_reference(), + recording_nz=self.entry_recording_Nz.value(), + recording_dz_um=self.entry_recording_dz.value(), ) def build_parameters(self): @@ -17793,7 +17860,9 @@ def _make_channel_base(name: str) -> AcquisitionChannel: recording_channel=recording_channel, fps=self.entry_fps.value(), duration_s=self.entry_duration.value(), - recording_bottom_z_offset_um=self.entry_recording_z_offset.value(), + recording_bottom_z_offset_um=self.entry_recording_bottom_z.value(), + recording_Nz=self.entry_recording_Nz.value(), + recording_dz_um=self.entry_recording_dz.value(), zstack_enabled=self.checkbox_zstack.isChecked(), zstack_channels=zstack_channels, z_min_um=self.entry_zmin.value(), diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 7cc5d7781..2e31028e1 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1072,3 +1072,65 @@ def test_validate_helper_recording_nz_and_dz(): assert _validate_record_zstack_params(**params) is None # Nz=1: dz irrelevant params = _base_params(recording_enabled=True, recording_nz=3, recording_dz_um=0.5) assert _validate_record_zstack_params(**params) is None + + +def test_recording_dz_hidden_when_nz_is_one(qtbot, simulated_widget_deps): + """dz must be HIDDEN (not just disabled) when Nz == 1 — user requirement.""" + from control.widgets import RecordZStackMultiPointWidget + + w = RecordZStackMultiPointWidget(**simulated_widget_deps) + qtbot.addWidget(w) + + assert w.entry_recording_Nz.value() == 1 + assert w.entry_recording_dz.isHidden() + assert w.label_recording_dz.isHidden() + + w.entry_recording_Nz.setValue(3) + assert not w.entry_recording_dz.isHidden() + assert not w.label_recording_dz.isHidden() + + w.entry_recording_Nz.setValue(1) + assert w.entry_recording_dz.isHidden() + assert w.label_recording_dz.isHidden() + + +def test_recording_planes_label_and_build_parameters(qtbot, simulated_widget_deps): + from control.widgets import RecordZStackMultiPointWidget + + w = RecordZStackMultiPointWidget(**simulated_widget_deps) + qtbot.addWidget(w) + w.lineEdit_savingDir.setText("/tmp/test") + w.checkbox_recording.setChecked(True) + w.entry_duration.setValue(2.0) + w.entry_recording_bottom_z.setValue(-2.0) + w.entry_recording_Nz.setValue(3) + w.entry_recording_dz.setValue(4.0) + + # Label reflects the span and the per-FOV time cost (3 planes x 2 s). + text = w.label_recording_planes.text() + assert "3 planes" in text + assert "6.0 s" in text + + params = w.build_parameters() + assert params.recording_bottom_z_offset_um == -2.0 + assert params.recording_Nz == 3 + assert params.recording_dz_um == 4.0 + + +def test_validate_wires_recording_nz_dz(qtbot, simulated_widget_deps): + """Widget.validate() must reject dz<=0 when Nz>1 (delegated to the helper).""" + from control.widgets import RecordZStackMultiPointWidget + + w = RecordZStackMultiPointWidget(**simulated_widget_deps) + qtbot.addWidget(w) + w.lineEdit_savingDir.setText("/tmp/test") + w.checkbox_recording.setChecked(True) + w.checkbox_zstack.setChecked(False) + w.entry_recording_Nz.setValue(3) + w.entry_recording_dz.setValue(4.0) + assert w.validate() is None + # The spinbox minimum prevents dz<=0 in the UI, so drive the helper wiring + # check through build-parameters-level values instead: Nz spin min is 1, + # dz spin min is > 0 — validate() passing Nz/dz through is what's under test. + w.entry_recording_Nz.setValue(1) + assert w.validate() is None From bec545db9aea9af4273d02637cd76cc4d9e2cc8f Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 12:38:58 -0400 Subject: [PATCH 05/15] fix(multi-plane): move plane-0 Z move before illumination; restore camera/Z on fail-fast path; harden validate-wiring test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final whole-branch review findings on feat/multi-plane-recording: - record(): move to the first plane BEFORE turning illumination on (pre-branch parity — illumination was previously off during the Z move + settle; the multi-plane refactor had regressed this to illuminate during every FOV's first Z move, causing extra photobleaching). - record(): widen the try/finally so the camera-mode + stage-Z restore also runs when a plane raises the fail-fast RuntimeError (previously only ran on the normal path, leaving the stage at the plane offset on error). - test_validate_wires_recording_nz_dz: spinbox minimums make invalid Nz/dz unreachable, so the test asserted nothing beyond validate() returning None. Now monkeypatches _validate_record_zstack_params to capture and assert the actual recording_nz/recording_dz_um kwargs it receives. - Updated record()/_record_one_plane docstrings and comments to describe the new control flow. --- software/control/core/record_zstack_worker.py | 59 +++++---- .../tests/core/test_record_zstack_worker.py | 122 ++++++++++++++++++ software/tests/test_record_zstack_widget.py | 31 +++-- 3 files changed, 182 insertions(+), 30 deletions(-) diff --git a/software/control/core/record_zstack_worker.py b/software/control/core/record_zstack_worker.py index 92b47c083..649c251ec 100644 --- a/software/control/core/record_zstack_worker.py +++ b/software/control/core/record_zstack_worker.py @@ -350,9 +350,14 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: Plane j records at ``z_ref + recording_bottom_z_offset_um + j*recording_dz_um`` (one Zarr store per plane; Nz=1 keeps the historical single-plane layout byte-identical to before per-plane recording existed). The recording - channel, achievable fps, dataset frame count, and illumination are all - established once per FOV and shared across every plane. Returns the - total number of frames emitted across all planes. + channel, achievable fps, and dataset frame count are all established + once per FOV and shared across every plane. The stage moves to the + first plane with illumination OFF (matching the pre-multi-plane + behavior), illumination then turns on for the remainder of the FOV + (not toggled between planes), and the camera-mode + stage-Z restore + always runs on the way out — normal completion, an abort between + planes, or a plane raising the fail-fast ``RuntimeError`` below. + Returns the total number of frames emitted across all planes. """ # Apply the recording channel (exposure/gain/illumination settings). rec_channel = self.params.recording_channel @@ -394,33 +399,42 @@ def record(self, t_idx: int, region_id, fov_idx: int, z_ref: float) -> int: ) n_planes = len(offsets) - # The CONTINUOUS stream does not gate illumination per-frame, and - # set_microscope_mode only energizes illumination when live (we are not - # live here). Illumination stays on across all planes of this FOV (no - # flicker, fewer MCU commands); _record_one_plane assumes it is already - # energized. + # Move to the first plane with illumination OFF (matches the + # pre-multi-plane behavior: the sample must not be illuminated during + # the Z move + settle). The CONTINUOUS stream does not gate + # illumination per-frame, and set_microscope_mode only energizes + # illumination when live (we are not live here), so illumination is + # turned on explicitly here and stays on across all planes of this FOV + # (no flicker between planes, fewer MCU commands); _record_one_plane + # assumes it is already energized. total_emitted = 0 + self._move_z_to_offset(z_ref, offsets[0]) self.liveController.turn_on_illumination() try: for plane_idx, plane_offset_um in enumerate(offsets): - if plane_idx > 0 and self.abort_requested_fn(): - log.info(f"abort requested; skipping recording planes {plane_idx}..{n_planes - 1}") - break - self._move_z_to_offset(z_ref, plane_offset_um) + if plane_idx > 0: + if self.abort_requested_fn(): + log.info(f"abort requested; skipping recording planes {plane_idx}..{n_planes - 1}") + break + self._move_z_to_offset(z_ref, plane_offset_um) total_emitted += self._record_one_plane( t_idx, region_id, fov_idx, plane_idx, n_planes, plane_offset_um, effective_fps, T ) finally: self.liveController.turn_off_illumination() - - # Restore the camera to a software-trigger-friendly state and Z reference. - try: - self.camera.set_acquisition_mode(CameraAcquisitionMode.SOFTWARE_TRIGGER) - except Exception: - log.exception("Failed to restore software-trigger acquisition mode after recording") - self.stage.move_z_to(z_ref) - self.wait_till_operation_is_completed() - self._sleep(SCAN_STABILIZATION_TIME_MS_Z / 1000) + # Restore the camera to a software-trigger-friendly state and Z + # reference. Runs on every exit path — normal completion, the + # abort break above, and a plane raising the fail-fast + # RuntimeError below — so the stage is never left at a plane + # offset. Exceptions here are only logged so they don't mask an + # in-flight RuntimeError from _record_one_plane. + try: + self.camera.set_acquisition_mode(CameraAcquisitionMode.SOFTWARE_TRIGGER) + except Exception: + log.exception("Failed to restore software-trigger acquisition mode after recording") + self.stage.move_z_to(z_ref) + self.wait_till_operation_is_completed() + self._sleep(SCAN_STABILIZATION_TIME_MS_Z / 1000) return total_emitted def _record_one_plane( @@ -488,7 +502,8 @@ def _record_one_plane( # mount), so continuing would burn the timeout at every remaining # plane/FOV producing blank data. A wedged finalize returns before # errors are countable, which is why write_error_count alone is not - # sufficient. record() lets this propagate; run() catches it, aborts, + # sufficient. record()'s finally still restores illumination/camera + # mode/stage-Z before this propagates; run() then catches it, aborts, # and signals finished. if writer.write_error_count > 0 or writer.finalize_wedged or writer.dropped_count > 0: raise RuntimeError( diff --git a/software/tests/core/test_record_zstack_worker.py b/software/tests/core/test_record_zstack_worker.py index db538a735..c445d0601 100644 --- a/software/tests/core/test_record_zstack_worker.py +++ b/software/tests/core/test_record_zstack_worker.py @@ -839,3 +839,125 @@ def record_then_abort(*args, **kwargs): names = sorted(p.name for p in rec_dir.glob("*.ome.zarr")) assert names == ["fov_0_z0.ome.zarr"], f"expected only plane 0 after abort, got {names}" assert (Path(tmp_path) / "state_restore" / ".done").exists() + + +# --------------------------------------------------------------------------- +# Final-review-fix tests: illumination timing + fail-fast restore +# --------------------------------------------------------------------------- + + +def test_record_moves_to_first_plane_before_illumination(tmp_path): + """Finding 1: the first plane's Z move must happen BEFORE illumination + turns on (pre-branch parity — the sample must not be illuminated during + the Z move + settle). Later planes move while illumination is already on + (it is not toggled between planes), and it turns off only after the last + plane.""" + pytest.importorskip("tensorstore") + + scope, live_controller, channels, worker, aborted = _build_worker_harness( + tmp_path, recording_enabled=True, zstack_enabled=False + ) + worker.params.recording_Nz = 3 + worker.params.recording_dz_um = 1.0 + + events = [] + call_count = {"n": 0} + + def fake_record_one_plane(*args, **kwargs): + call_count["n"] += 1 + return 0 + + worker._record_one_plane = fake_record_one_plane + + orig_move = worker._move_z_to_offset + + def move_spy(z_ref, offset): + events.append(("move", offset)) + return orig_move(z_ref, offset) + + worker._move_z_to_offset = move_spy + + orig_on = live_controller.turn_on_illumination + + def on_spy(): + events.append(("illum_on",)) + return orig_on() + + live_controller.turn_on_illumination = on_spy + + orig_off = live_controller.turn_off_illumination + + def off_spy(): + events.append(("illum_off",)) + return orig_off() + + live_controller.turn_off_illumination = off_spy + + z_ref = scope.stage.get_pos().z_mm + total = worker.record(0, "A1", 0, z_ref) + + assert call_count["n"] == 3, "all three planes must still be recorded" + assert total == 0 + assert events == [ + ("move", 0.0), + ("illum_on",), + ("move", 1.0), + ("move", 2.0), + ("illum_off",), + ], f"unexpected event order: {events}" + + +def test_record_restores_camera_mode_and_stage_z_when_plane_raises(tmp_path): + """Finding 2: when _record_one_plane raises (fail-fast), the post-loop + camera-mode restore + stage-Z restore + settle must still run (the stage + must not be left at a plane offset), illumination must still turn off, and + the RuntimeError must still propagate.""" + pytest.importorskip("tensorstore") + from squid.abc import CameraAcquisitionMode + + scope, live_controller, channels, worker, aborted = _build_worker_harness( + tmp_path, recording_enabled=True, zstack_enabled=False + ) + worker.params.recording_Nz = 3 + worker.params.recording_dz_um = 1.0 + + events = [] + call_count = {"n": 0} + + def fake_record_one_plane(*args, **kwargs): + call_count["n"] += 1 + if call_count["n"] == 2: + raise RuntimeError("simulated fail-fast") + return 0 + + worker._record_one_plane = fake_record_one_plane + + orig_off = live_controller.turn_off_illumination + + def off_spy(): + events.append("illum_off") + return orig_off() + + live_controller.turn_off_illumination = off_spy + + orig_move_z_to = scope.stage.move_z_to + restore_calls = [] + + def move_z_to_spy(z_mm): + restore_calls.append(z_mm) + return orig_move_z_to(z_mm) + + scope.stage.move_z_to = move_z_to_spy + + z_ref = scope.stage.get_pos().z_mm + + with pytest.raises(RuntimeError, match="simulated fail-fast"): + worker.record(0, "A1", 0, z_ref) + + assert events == ["illum_off"], "illumination must still be turned off on the raising path" + assert restore_calls and restore_calls[-1] == pytest.approx( + z_ref + ), "stage-Z restore must still run after _record_one_plane raises" + assert ( + scope.camera.get_acquisition_mode() == CameraAcquisitionMode.SOFTWARE_TRIGGER + ), "camera acquisition mode must be restored to SOFTWARE_TRIGGER after a raising plane" diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 2e31028e1..ce863e764 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1117,8 +1117,14 @@ def test_recording_planes_label_and_build_parameters(qtbot, simulated_widget_dep assert params.recording_dz_um == 4.0 -def test_validate_wires_recording_nz_dz(qtbot, simulated_widget_deps): - """Widget.validate() must reject dz<=0 when Nz>1 (delegated to the helper).""" +def test_validate_wires_recording_nz_dz(qtbot, simulated_widget_deps, monkeypatch): + """Widget.validate() must forward the recording_nz/recording_dz_um spinbox + values to the helper. The spinbox minimums (Nz>=1, dz>0) make invalid + values unreachable through the UI, so asserting only ``validate() is + None`` would still pass even if those kwargs were silently dropped from + the call. Intercept the helper instead and assert the actual kwargs it + receives.""" + import control.widgets as widgets_mod from control.widgets import RecordZStackMultiPointWidget w = RecordZStackMultiPointWidget(**simulated_widget_deps) @@ -1128,9 +1134,18 @@ def test_validate_wires_recording_nz_dz(qtbot, simulated_widget_deps): w.checkbox_zstack.setChecked(False) w.entry_recording_Nz.setValue(3) w.entry_recording_dz.setValue(4.0) - assert w.validate() is None - # The spinbox minimum prevents dz<=0 in the UI, so drive the helper wiring - # check through build-parameters-level values instead: Nz spin min is 1, - # dz spin min is > 0 — validate() passing Nz/dz through is what's under test. - w.entry_recording_Nz.setValue(1) - assert w.validate() is None + + real_validate = widgets_mod._validate_record_zstack_params + captured = {} + + def spy(**kwargs): + captured.update(kwargs) + return real_validate(**kwargs) + + monkeypatch.setattr(widgets_mod, "_validate_record_zstack_params", spy) + + result = w.validate() + + assert result is None + assert captured.get("recording_nz") == 3 + assert captured.get("recording_dz_um") == 4.0 From a2d37fa2f926dc7f5e8a27a8df2186dbc52d50d2 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 12:59:05 -0400 Subject: [PATCH 06/15] =?UTF-8?q?refactor(multi-plane):=20compact=20record?= =?UTF-8?q?ing=20UI=20=E2=80=94=20Nz=20leads=20FPS/Dur=20row,=20dynamic=20?= =?UTF-8?q?Z-offset=20caption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nz moves to the FPS/Duration row (with a divider) and the computed plane-summary label is removed; the offset caption reads "Z offset" at Nz=1 and "Bottom Z offset" at Nz>1, on one row with dz. Co-Authored-By: Claude Fable 5 --- software/control/widgets.py | 65 +++++++++------------ software/tests/test_record_zstack_widget.py | 11 ++-- 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index e21d39ee6..06d2a9296 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17294,10 +17294,29 @@ def _build_recording_group(self) -> QGroupBox: table_row.addStretch(1) vbox.addLayout(table_row) - # Row 1: FPS | Duration + # Row 1: Nz | FPS + Duration | computed planes/time summary + def _vline() -> QFrame: + line = QFrame() + line.setFrameShape(QFrame.VLine) + line.setFrameShadow(QFrame.Sunken) + return line + fps_row = QHBoxLayout() fps_row.setSpacing(4) + fps_row.addWidget(QLabel("Nz:")) + self.entry_recording_Nz = QSpinBox() + self.entry_recording_Nz.setRange(1, 100) + self.entry_recording_Nz.setValue(1) + self.entry_recording_Nz.setKeyboardTracking(False) + self.entry_recording_Nz.setMaximumWidth(70) + self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") + fps_row.addWidget(self.entry_recording_Nz) + + fps_row.addSpacing(4) + fps_row.addWidget(_vline()) + fps_row.addSpacing(4) + fps_row.addWidget(QLabel("FPS:")) self.entry_fps = QDoubleSpinBox() self.entry_fps.setRange(0.1, 1000) @@ -17320,11 +17339,13 @@ def _build_recording_group(self) -> QGroupBox: fps_row.addStretch(1) vbox.addLayout(fps_row) - # Row 2: Bottom Z | Nz | dz (dz hidden when Nz == 1) + # Row 2: Z offset (caption becomes "Bottom Z offset" when Nz > 1) | dz + # (dz hidden when Nz == 1) plane_row = QHBoxLayout() plane_row.setSpacing(4) - plane_row.addWidget(QLabel("Bottom Z:")) + self.label_recording_bottom_z = QLabel("Z offset:") + plane_row.addWidget(self.label_recording_bottom_z) self.entry_recording_bottom_z = QDoubleSpinBox() self.entry_recording_bottom_z.setRange(-500.0, 500.0) self.entry_recording_bottom_z.setDecimals(1) @@ -17336,16 +17357,6 @@ def _build_recording_group(self) -> QGroupBox: self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") plane_row.addWidget(self.entry_recording_bottom_z) - plane_row.addSpacing(4) - plane_row.addWidget(QLabel("Nz:")) - self.entry_recording_Nz = QSpinBox() - self.entry_recording_Nz.setRange(1, 100) - self.entry_recording_Nz.setValue(1) - self.entry_recording_Nz.setKeyboardTracking(False) - self.entry_recording_Nz.setMaximumWidth(70) - self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") - plane_row.addWidget(self.entry_recording_Nz) - plane_row.addSpacing(4) self.label_recording_dz = QLabel("dz:") plane_row.addWidget(self.label_recording_dz) @@ -17363,19 +17374,8 @@ def _build_recording_group(self) -> QGroupBox: plane_row.addStretch(1) vbox.addLayout(plane_row) - # Row 3: computed planes/time summary - planes_row = QHBoxLayout() - planes_row.setSpacing(4) - self.label_recording_planes = QLabel("") - planes_row.addWidget(self.label_recording_planes) - planes_row.addStretch(1) - vbox.addLayout(planes_row) - - # Wire up recording plane count / dz-visibility updates + # Wire up dz-visibility / caption updates (both driven by Nz alone) self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) - self.entry_recording_dz.valueChanged.connect(self._update_recording_planes_ui) - self.entry_recording_bottom_z.valueChanged.connect(self._update_recording_planes_ui) - self.entry_duration.valueChanged.connect(self._update_recording_planes_ui) self._update_recording_planes_ui() return grp @@ -17542,20 +17542,11 @@ def _update_zstack_planes_label(self) -> None: def _update_recording_planes_ui(self) -> None: """Toggle dz visibility (hidden entirely when Nz == 1 — user requirement) - and refresh the computed planes/time summary label.""" - nz = self.entry_recording_Nz.value() - multi = nz > 1 + and switch the Z-offset caption between single- and multi-plane wording.""" + multi = self.entry_recording_Nz.value() > 1 self.label_recording_dz.setVisible(multi) self.entry_recording_dz.setVisible(multi) - bottom = self.entry_recording_bottom_z.value() - per_fov_s = nz * self.entry_duration.value() - if multi: - top = bottom + (nz - 1) * self.entry_recording_dz.value() - self.label_recording_planes.setText( - f"{nz} planes: {bottom:+.1f} … {top:+.1f} µm rel. reference — {per_fov_s:.1f} s/FOV" - ) - else: - self.label_recording_planes.setText(f"1 plane @ {bottom:+.1f} µm — {per_fov_s:.1f} s/FOV") + self.label_recording_bottom_z.setText("Bottom Z offset:" if multi else "Z offset:") def _add_zstack_channel_row( self, name: str, exposure: float = 50.0, gain: float = 0.0, illumination: float = 50.0 diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index ce863e764..84c51cee0 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1094,7 +1094,7 @@ def test_recording_dz_hidden_when_nz_is_one(qtbot, simulated_widget_deps): assert w.label_recording_dz.isHidden() -def test_recording_planes_label_and_build_parameters(qtbot, simulated_widget_deps): +def test_recording_offset_caption_and_build_parameters(qtbot, simulated_widget_deps): from control.widgets import RecordZStackMultiPointWidget w = RecordZStackMultiPointWidget(**simulated_widget_deps) @@ -1106,10 +1106,11 @@ def test_recording_planes_label_and_build_parameters(qtbot, simulated_widget_dep w.entry_recording_Nz.setValue(3) w.entry_recording_dz.setValue(4.0) - # Label reflects the span and the per-FOV time cost (3 planes x 2 s). - text = w.label_recording_planes.text() - assert "3 planes" in text - assert "6.0 s" in text + # Caption uses multi-plane wording when Nz > 1 and reverts at Nz == 1. + assert w.label_recording_bottom_z.text() == "Bottom Z offset:" + w.entry_recording_Nz.setValue(1) + assert w.label_recording_bottom_z.text() == "Z offset:" + w.entry_recording_Nz.setValue(3) params = w.build_parameters() assert params.recording_bottom_z_offset_um == -2.0 From d29cb3e6264afd73517ead08abbe29a7bfa92aa3 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 13:05:21 -0400 Subject: [PATCH 07/15] =?UTF-8?q?refactor(multi-plane):=20single-row=20rec?= =?UTF-8?q?ording=20controls=20=E2=80=94=20Nz,=20Z=20offset,=20dz=20|=20FP?= =?UTF-8?q?S,=20Dur?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- software/control/widgets.py | 67 +++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index 06d2a9296..d3c72478a 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17294,7 +17294,9 @@ def _build_recording_group(self) -> QGroupBox: table_row.addStretch(1) vbox.addLayout(table_row) - # Row 1: Nz | FPS + Duration | computed planes/time summary + # Single row: Nz | Z offset (+ dz when Nz > 1) | FPS + Duration. + # dz is hidden when Nz == 1, and the offset caption switches between + # "Z offset" (Nz == 1) and "Bottom Z offset" (Nz > 1). def _vline() -> QFrame: line = QFrame() line.setFrameShape(QFrame.VLine) @@ -17313,6 +17315,34 @@ def _vline() -> QFrame: self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") fps_row.addWidget(self.entry_recording_Nz) + fps_row.addSpacing(4) + self.label_recording_bottom_z = QLabel("Z offset:") + fps_row.addWidget(self.label_recording_bottom_z) + self.entry_recording_bottom_z = QDoubleSpinBox() + self.entry_recording_bottom_z.setRange(-500.0, 500.0) + self.entry_recording_bottom_z.setDecimals(1) + self.entry_recording_bottom_z.setSingleStep(0.5) + self.entry_recording_bottom_z.setValue(0.0) + self.entry_recording_bottom_z.setSuffix(" µm") + self.entry_recording_bottom_z.setKeyboardTracking(False) + self.entry_recording_bottom_z.setMaximumWidth(95) + self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") + fps_row.addWidget(self.entry_recording_bottom_z) + + fps_row.addSpacing(4) + self.label_recording_dz = QLabel("dz:") + fps_row.addWidget(self.label_recording_dz) + self.entry_recording_dz = QDoubleSpinBox() + self.entry_recording_dz.setRange(0.05, 100.0) + self.entry_recording_dz.setDecimals(2) + self.entry_recording_dz.setSingleStep(0.5) + self.entry_recording_dz.setValue(1.0) + self.entry_recording_dz.setSuffix(" µm") + self.entry_recording_dz.setKeyboardTracking(False) + self.entry_recording_dz.setMaximumWidth(95) + self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") + fps_row.addWidget(self.entry_recording_dz) + fps_row.addSpacing(4) fps_row.addWidget(_vline()) fps_row.addSpacing(4) @@ -17339,41 +17369,6 @@ def _vline() -> QFrame: fps_row.addStretch(1) vbox.addLayout(fps_row) - # Row 2: Z offset (caption becomes "Bottom Z offset" when Nz > 1) | dz - # (dz hidden when Nz == 1) - plane_row = QHBoxLayout() - plane_row.setSpacing(4) - - self.label_recording_bottom_z = QLabel("Z offset:") - plane_row.addWidget(self.label_recording_bottom_z) - self.entry_recording_bottom_z = QDoubleSpinBox() - self.entry_recording_bottom_z.setRange(-500.0, 500.0) - self.entry_recording_bottom_z.setDecimals(1) - self.entry_recording_bottom_z.setSingleStep(0.5) - self.entry_recording_bottom_z.setValue(0.0) - self.entry_recording_bottom_z.setSuffix(" µm") - self.entry_recording_bottom_z.setKeyboardTracking(False) - self.entry_recording_bottom_z.setMaximumWidth(95) - self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") - plane_row.addWidget(self.entry_recording_bottom_z) - - plane_row.addSpacing(4) - self.label_recording_dz = QLabel("dz:") - plane_row.addWidget(self.label_recording_dz) - self.entry_recording_dz = QDoubleSpinBox() - self.entry_recording_dz.setRange(0.05, 100.0) - self.entry_recording_dz.setDecimals(2) - self.entry_recording_dz.setSingleStep(0.5) - self.entry_recording_dz.setValue(1.0) - self.entry_recording_dz.setSuffix(" µm") - self.entry_recording_dz.setKeyboardTracking(False) - self.entry_recording_dz.setMaximumWidth(95) - self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") - plane_row.addWidget(self.entry_recording_dz) - - plane_row.addStretch(1) - vbox.addLayout(plane_row) - # Wire up dz-visibility / caption updates (both driven by Nz alone) self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) self._update_recording_planes_ui() From 7762b1bd0e3737c3094b94ff02604a54257e2870 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 14:54:28 -0400 Subject: [PATCH 08/15] refactor(multi-plane): integer FPS/Dur spinboxes, drop fps suffix Co-Authored-By: Claude Fable 5 --- software/control/widgets.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index d3c72478a..48e3fd645 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17349,9 +17349,9 @@ def _vline() -> QFrame: fps_row.addWidget(QLabel("FPS:")) self.entry_fps = QDoubleSpinBox() - self.entry_fps.setRange(0.1, 1000) + self.entry_fps.setRange(1, 1000) + self.entry_fps.setDecimals(0) self.entry_fps.setValue(10.0) - self.entry_fps.setSuffix(" fps") self.entry_fps.setKeyboardTracking(False) self.entry_fps.setMaximumWidth(100) fps_row.addWidget(self.entry_fps) @@ -17359,7 +17359,8 @@ def _vline() -> QFrame: fps_row.addSpacing(4) fps_row.addWidget(QLabel("Dur:")) self.entry_duration = QDoubleSpinBox() - self.entry_duration.setRange(0.01, 3600) + self.entry_duration.setRange(1, 3600) + self.entry_duration.setDecimals(0) self.entry_duration.setValue(1.0) self.entry_duration.setSuffix(" s") self.entry_duration.setKeyboardTracking(False) From 26925c15ed30875e5171fdd32fa4defc28fb0453 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sat, 4 Jul 2026 15:18:09 -0400 Subject: [PATCH 09/15] feat(multi-plane): show recording Z offset only when Laser AF is enabled; slim spinbox widths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without Laser AF there is no reference plane to offset from — the field (and caption) hide, and build_parameters emits 0.0 so a hidden stale value cannot shift the recording planes. Co-Authored-By: Claude Fable 5 --- software/control/widgets.py | 30 ++++++++++++++------- software/tests/test_record_zstack_widget.py | 11 ++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index 48e3fd645..ff8e1a73f 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17311,7 +17311,7 @@ def _vline() -> QFrame: self.entry_recording_Nz.setRange(1, 100) self.entry_recording_Nz.setValue(1) self.entry_recording_Nz.setKeyboardTracking(False) - self.entry_recording_Nz.setMaximumWidth(70) + self.entry_recording_Nz.setMaximumWidth(55) self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") fps_row.addWidget(self.entry_recording_Nz) @@ -17325,7 +17325,7 @@ def _vline() -> QFrame: self.entry_recording_bottom_z.setValue(0.0) self.entry_recording_bottom_z.setSuffix(" µm") self.entry_recording_bottom_z.setKeyboardTracking(False) - self.entry_recording_bottom_z.setMaximumWidth(95) + self.entry_recording_bottom_z.setMaximumWidth(85) self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") fps_row.addWidget(self.entry_recording_bottom_z) @@ -17339,7 +17339,7 @@ def _vline() -> QFrame: self.entry_recording_dz.setValue(1.0) self.entry_recording_dz.setSuffix(" µm") self.entry_recording_dz.setKeyboardTracking(False) - self.entry_recording_dz.setMaximumWidth(95) + self.entry_recording_dz.setMaximumWidth(85) self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") fps_row.addWidget(self.entry_recording_dz) @@ -17353,7 +17353,7 @@ def _vline() -> QFrame: self.entry_fps.setDecimals(0) self.entry_fps.setValue(10.0) self.entry_fps.setKeyboardTracking(False) - self.entry_fps.setMaximumWidth(100) + self.entry_fps.setMaximumWidth(60) fps_row.addWidget(self.entry_fps) fps_row.addSpacing(4) @@ -17364,14 +17364,16 @@ def _vline() -> QFrame: self.entry_duration.setValue(1.0) self.entry_duration.setSuffix(" s") self.entry_duration.setKeyboardTracking(False) - self.entry_duration.setMaximumWidth(90) + self.entry_duration.setMaximumWidth(75) fps_row.addWidget(self.entry_duration) fps_row.addStretch(1) vbox.addLayout(fps_row) - # Wire up dz-visibility / caption updates (both driven by Nz alone) + # Wire up dz visibility (Nz), Z-offset visibility (Laser AF — the offset + # is relative to the AF reference plane), and the offset caption (Nz). self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) + self.checkbox_laser_af.toggled.connect(self._update_recording_planes_ui) self._update_recording_planes_ui() return grp @@ -17537,11 +17539,17 @@ def _update_zstack_planes_label(self) -> None: self.label_zstack_planes.setText("-- planes") def _update_recording_planes_ui(self) -> None: - """Toggle dz visibility (hidden entirely when Nz == 1 — user requirement) - and switch the Z-offset caption between single- and multi-plane wording.""" + """Toggle dz visibility (hidden entirely when Nz == 1 — user requirement), + show the Z-offset field only when Laser AF is enabled (the offset is + relative to the AF reference plane; without AF the user positions Z + directly), and switch the offset caption between single- and + multi-plane wording.""" multi = self.entry_recording_Nz.value() > 1 self.label_recording_dz.setVisible(multi) self.entry_recording_dz.setVisible(multi) + use_af = self.checkbox_laser_af.isChecked() + self.label_recording_bottom_z.setVisible(use_af) + self.entry_recording_bottom_z.setVisible(use_af) self.label_recording_bottom_z.setText("Bottom Z offset:" if multi else "Z offset:") def _add_zstack_channel_row( @@ -17847,7 +17855,11 @@ def _make_channel_base(name: str) -> AcquisitionChannel: recording_channel=recording_channel, fps=self.entry_fps.value(), duration_s=self.entry_duration.value(), - recording_bottom_z_offset_um=self.entry_recording_bottom_z.value(), + # The Z-offset field is hidden without Laser AF (no AF reference + # plane to offset from) — a hidden value must not silently apply. + recording_bottom_z_offset_um=( + self.entry_recording_bottom_z.value() if self.checkbox_laser_af.isChecked() else 0.0 + ), recording_Nz=self.entry_recording_Nz.value(), recording_dz_um=self.entry_recording_dz.value(), zstack_enabled=self.checkbox_zstack.isChecked(), diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 84c51cee0..83d5c0bd2 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1106,6 +1106,17 @@ def test_recording_offset_caption_and_build_parameters(qtbot, simulated_widget_d w.entry_recording_Nz.setValue(3) w.entry_recording_dz.setValue(4.0) + # Without Laser AF there is no reference plane to offset from: the field + # is hidden and its (stale) value must not reach the parameters. + assert not w.checkbox_laser_af.isChecked() + assert w.entry_recording_bottom_z.isHidden() + assert w.label_recording_bottom_z.isHidden() + assert w.build_parameters().recording_bottom_z_offset_um == 0.0 + + w.checkbox_laser_af.setChecked(True) + assert not w.entry_recording_bottom_z.isHidden() + assert not w.label_recording_bottom_z.isHidden() + # Caption uses multi-plane wording when Nz > 1 and reverts at Nz == 1. assert w.label_recording_bottom_z.text() == "Bottom Z offset:" w.entry_recording_Nz.setValue(1) From 0fea746328affaba03c265f5bf720d88a85f7f33 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sun, 5 Jul 2026 02:58:27 -0400 Subject: [PATCH 10/15] test(record-zstack): cover Nz/dz/bottom-Z fields in the settings round-trip test test_full_save_load_round_trip_preserves_settings previously ran with Laser AF off, which forces recording_bottom_z_offset_um to 0.0 in build_parameters() and left recording_Nz/recording_dz_um at their defaults -- so the multi-plane recording fields introduced by this branch were never actually exercised by the round trip. Enable Laser AF and set Nz/dz/bottom-Z to distinctive values so the save->parse->apply cycle is verified for these fields too. --- software/tests/test_record_zstack_widget.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 346deaee8..86632a353 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -1757,6 +1757,10 @@ def test_full_save_load_round_trip_preserves_settings(qtbot, simulated_widget_de - recording channel and its numeric settings (exposure_time, analog_gain, illumination_intensity) - z-stack channels and their numeric settings - acquisition parameters (FPS, duration, z-range, XY mode) + - multi-plane recording fields (Laser AF on, Nz, dz, bottom-Z offset) -- build_parameters() + forces recording_bottom_z_offset_um to 0.0 when Laser AF is off (see + _update_recording_planes_ui), so Laser AF must be enabled for the bottom-Z value to + round-trip meaningfully. """ from control.core.record_zstack_controller import _save_record_zstack_yaml from control.acquisition_yaml_loader import parse_acquisition_yaml @@ -1766,8 +1770,12 @@ def test_full_save_load_round_trip_preserves_settings(qtbot, simulated_widget_de qtbot.addWidget(w1) w1.lineEdit_savingDir.setText(str(tmp_path)) w1.checkbox_recording.setChecked(True) + w1.checkbox_laser_af.setChecked(True) w1.entry_fps.setValue(30.0) w1.entry_duration.setValue(4.0) + w1.entry_recording_Nz.setValue(3) + w1.entry_recording_dz.setValue(2.5) + w1.entry_recording_bottom_z.setValue(-4.0) w1.checkbox_zstack.setChecked(True) w1._add_zstack_channel_row("Fluorescence 488 nm Ex", exposure=80.0, gain=1.0, illumination=45.0) w1.entry_zmin.setValue(-5.0) @@ -1799,6 +1807,12 @@ def test_full_save_load_round_trip_preserves_settings(qtbot, simulated_widget_de # Recording channel (enabled above, so should be non-None) assert params2.recording_channel == params1.recording_channel + # Multi-plane recording fields (Laser AF on, Nz/dz/bottom-Z offset) + assert params2.use_laser_af == params1.use_laser_af + assert params2.recording_Nz == params1.recording_Nz + assert params2.recording_dz_um == pytest.approx(params1.recording_dz_um) + assert params2.recording_bottom_z_offset_um == pytest.approx(params1.recording_bottom_z_offset_um) + # Z-stack channels: check channel names and numeric settings assert [c.name for c in params2.zstack_channels] == [c.name for c in params1.zstack_channels] assert len(params2.zstack_channels) == len(params1.zstack_channels) From c31e2230bbd6f1e9a0891059978364b03ec36309 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sun, 5 Jul 2026 15:22:19 -0400 Subject: [PATCH 11/15] feat(multi-plane): confirm recording plane summary before starting acquisition Restores the plane-count/Z-range/per-FOV-duration summary that a prior UI change removed (it was an inline label, dropped for being too wide) as a QMessageBox.question shown right after validate() succeeds and before the acquisition actually starts, giving the user one last look. Answering No aborts the start and re-unchecks the button; the dialog is skipped entirely when the Recording phase is disabled. The summary's Z offset uses the same effective-value logic as build_parameters() (0.0 when Laser AF is off) so it can never show a stale value from the hidden entry_recording_bottom_z field that doesn't match what actually runs. Also fixes entry_recording_bottom_z's tooltip, which stayed frozen at "Bottom plane offset relative to the Z reference" even after Nz dropped to 1 and the caption switched to "Z offset:" -- it now tracks the same multi/ single wording as the label. --- software/control/widgets.py | 30 ++++ software/tests/test_record_zstack_widget.py | 188 ++++++++++++++++++++ 2 files changed, 218 insertions(+) diff --git a/software/control/widgets.py b/software/control/widgets.py index 803180814..f57055fc0 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17803,6 +17803,9 @@ def _update_recording_planes_ui(self) -> None: self.label_recording_bottom_z.setVisible(use_af) self.entry_recording_bottom_z.setVisible(use_af) self.label_recording_bottom_z.setText("Bottom Z offset:" if multi else "Z offset:") + self.entry_recording_bottom_z.setToolTip( + "Bottom plane offset relative to the Z reference" if multi else "Offset relative to the Z reference" + ) def _add_zstack_channel_row( self, name: str, exposure: float = 50.0, gain: float = 0.0, illumination: float = 50.0 @@ -18269,6 +18272,9 @@ def toggle_acquisition(self, pressed: bool) -> None: On start (pressed=True): - validate(); show QMessageBox.warning and un-check button on failure. + - if Recording is enabled, show a QMessageBox.question confirming the + plane-count/Z-range/per-FOV-duration summary; un-check button and + abort if the user answers No. - emit signal_acquisition_started(True) so gui_hcs can lock down the UI. - call recordZStackController.run_acquisition(self.build_parameters()). @@ -18296,6 +18302,30 @@ def toggle_acquisition(self, pressed: bool) -> None: QMessageBox.warning(self, "Invalid Parameters", error) return + # One last look at the recording plane summary before starting. + # bottom must use the same effective-value logic as build_parameters() + # (the field is hidden and stale when Laser AF is off, so a hidden + # value must not silently apply -- see the comment there). + if self.checkbox_recording.isChecked(): + nz = self.entry_recording_Nz.value() + bottom = self.entry_recording_bottom_z.value() if self.checkbox_laser_af.isChecked() else 0.0 + per_fov_s = nz * self.entry_duration.value() + if nz > 1: + top = bottom + (nz - 1) * self.entry_recording_dz.value() + summary = f"{nz} planes: {bottom:+.1f} … {top:+.1f} µm rel. reference — {per_fov_s:.1f} s/FOV" + else: + summary = f"1 plane @ {bottom:+.1f} µm — {per_fov_s:.1f} s/FOV" + reply = QMessageBox.question( + self, + "Confirm Recording", + f"Recording: {summary}\n\nStart acquisition?", + QMessageBox.Yes | QMessageBox.No, + QMessageBox.Yes, + ) + if reply != QMessageBox.Yes: + self.btn_startAcquisition.setChecked(False) + return + # Refresh the per-well FOV grid before building parameters so the # scan regions reflect the current overlap/shape/region-size settings. self._update_scan_regions() diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 86632a353..23a04fef5 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -25,6 +25,7 @@ from unittest.mock import MagicMock, patch import pytest +from qtpy.QtWidgets import QMessageBox from control.widgets import _validate_record_zstack_params @@ -583,6 +584,193 @@ def test_toggle_acquisition_stop_calls_request_abort(qtbot, simulated_widget_dep ctrl.run_acquisition.assert_not_called() +# --------------------------------------------------------------------------- +# Confirmation dialog: recording plane summary shown before Start proceeds +# --------------------------------------------------------------------------- + + +def _make_valid_recording_widget(qtbot, deps, *, laser_af=False): + """Return a widget pre-configured for a valid recording-only acquisition. + + Recording is enabled, Z-Stack is disabled. If laser_af is True, a stub + laser_autofocus_controller reporting a reference plane is wired in and + checkbox_laser_af is checked (needed for validate() to pass when Laser AF + is on) -- the caller is then free to set entry_recording_bottom_z. + """ + from control.widgets import RecordZStackMultiPointWidget + + deps = dict(deps) + if laser_af: + laser_ctrl = MagicMock() + laser_ctrl.laser_af_properties.has_reference = True + deps["laser_autofocus_controller"] = laser_ctrl + + w = RecordZStackMultiPointWidget(**deps) + qtbot.addWidget(w) + w.lineEdit_savingDir.setText("/tmp/test_confirm") + w.checkbox_zstack.setChecked(False) + w.checkbox_recording.setChecked(True) + if laser_af: + w.checkbox_laser_af.setChecked(True) + return w + + +def test_toggle_acquisition_confirm_single_plane_summary_and_yes_proceeds(qtbot, simulated_widget_deps): + """Nz == 1: the confirmation dialog shows the single-plane summary and + answering Yes lets the acquisition start.""" + ctrl = _make_stub_controller() + simulated_widget_deps["recordZStackController"] = ctrl + + w = _make_valid_recording_widget(qtbot, simulated_widget_deps, laser_af=True) + w.entry_recording_Nz.setValue(1) + w.entry_recording_bottom_z.setValue(-1.5) + w.entry_duration.setValue(2.0) + + expected = f"1 plane @ {-1.5:+.1f} µm — {1 * 2.0:.1f} s/FOV" + + captured = {} + + def fake_question(parent, title, text, *args, **kwargs): + captured["title"] = title + captured["text"] = text + return QMessageBox.Yes + + with patch("control.widgets.QMessageBox.question", side_effect=fake_question): + w.toggle_acquisition(True) + + assert expected in captured["text"] + ctrl.run_acquisition.assert_called_once() + + +def test_toggle_acquisition_confirm_multi_plane_summary_and_yes_proceeds(qtbot, simulated_widget_deps): + """Nz > 1: the confirmation dialog shows the multi-plane summary with the + correct computed top (bottom + (nz - 1) * dz) and answering Yes proceeds.""" + ctrl = _make_stub_controller() + simulated_widget_deps["recordZStackController"] = ctrl + + w = _make_valid_recording_widget(qtbot, simulated_widget_deps, laser_af=True) + w.entry_recording_Nz.setValue(4) + w.entry_recording_bottom_z.setValue(-3.0) + w.entry_recording_dz.setValue(2.0) + # entry_duration has 0 decimals (whole seconds only) -- use a value that + # can't be affected by rounding. + w.entry_duration.setValue(3.0) + + nz, bottom, dz, duration = 4, -3.0, 2.0, 3.0 + top = bottom + (nz - 1) * dz + per_fov_s = nz * duration + expected = f"{nz} planes: {bottom:+.1f} … {top:+.1f} µm rel. reference — {per_fov_s:.1f} s/FOV" + + captured = {} + + def fake_question(parent, title, text, *args, **kwargs): + captured["text"] = text + return QMessageBox.Yes + + with patch("control.widgets.QMessageBox.question", side_effect=fake_question): + w.toggle_acquisition(True) + + assert expected in captured["text"] + ctrl.run_acquisition.assert_called_once() + + +def test_toggle_acquisition_confirm_uses_zero_offset_when_laser_af_off(qtbot, simulated_widget_deps): + """Laser AF OFF: the summary's bottom must be 0.0, not whatever stale value + is left sitting in entry_recording_bottom_z from when Laser AF was on. + + This mirrors build_parameters()'s effective-value logic + (entry_recording_bottom_z.value() if checkbox_laser_af.isChecked() else 0.0) + -- the dialog must not show a number that doesn't match what actually runs. + """ + ctrl = _make_stub_controller() + simulated_widget_deps["recordZStackController"] = ctrl + + laser_ctrl = MagicMock() + laser_ctrl.laser_af_properties.has_reference = True + simulated_widget_deps["laser_autofocus_controller"] = laser_ctrl + + w = _make_valid_recording_widget(qtbot, simulated_widget_deps, laser_af=True) + # Leave a nonzero stale value in the field, then turn Laser AF back off. + w.entry_recording_bottom_z.setValue(7.5) + w.checkbox_laser_af.setChecked(False) + w.entry_recording_Nz.setValue(1) + w.entry_duration.setValue(1.0) + + assert w.entry_recording_bottom_z.value() == pytest.approx(7.5) # stale value still sitting there + assert not w.checkbox_laser_af.isChecked() + + captured = {} + + def fake_question(parent, title, text, *args, **kwargs): + captured["text"] = text + return QMessageBox.Yes + + with patch("control.widgets.QMessageBox.question", side_effect=fake_question): + w.toggle_acquisition(True) + + assert "+0.0" in captured["text"] + assert "+7.5" not in captured["text"] + ctrl.run_acquisition.assert_called_once() + + +def test_toggle_acquisition_confirm_no_aborts_start(qtbot, simulated_widget_deps): + """Answering No to the confirmation dialog must NOT start the acquisition, + and must leave the Start button unchecked.""" + ctrl = _make_stub_controller() + simulated_widget_deps["recordZStackController"] = ctrl + + w = _make_valid_recording_widget(qtbot, simulated_widget_deps) + w.btn_startAcquisition.setChecked(True) + + with patch("control.widgets.QMessageBox.question", return_value=QMessageBox.No): + w.toggle_acquisition(True) + + ctrl.run_acquisition.assert_not_called() + assert not w.btn_startAcquisition.isChecked() + + +def test_toggle_acquisition_no_confirm_when_recording_disabled(qtbot, simulated_widget_deps): + """When the Recording phase is not enabled, no confirmation dialog should + appear at all -- Start proceeds straight through as before.""" + ctrl = _make_stub_controller() + simulated_widget_deps["recordZStackController"] = ctrl + + w = _make_valid_widget(qtbot, simulated_widget_deps) # zstack-only, recording off + assert not w.checkbox_recording.isChecked() + + def fail_if_called(*args, **kwargs): + raise AssertionError("QMessageBox.question must not be called when recording is disabled") + + with patch("control.widgets.QMessageBox.question", side_effect=fail_if_called): + w.toggle_acquisition(True) + + ctrl.run_acquisition.assert_called_once() + + +def test_recording_bottom_z_tooltip_matches_caption(qtbot, simulated_widget_deps): + """The entry_recording_bottom_z tooltip must track the same multi/single + wording as its label caption (label_recording_bottom_z), so it doesn't go + stale (e.g. still saying "Bottom plane offset" when Nz == 1 and the label + already reverted to "Z offset:").""" + from control.widgets import RecordZStackMultiPointWidget + + w = RecordZStackMultiPointWidget(**simulated_widget_deps) + qtbot.addWidget(w) + + # Default is Nz == 1 (single plane). + assert w.entry_recording_Nz.value() == 1 + assert w.label_recording_bottom_z.text() == "Z offset:" + assert w.entry_recording_bottom_z.toolTip() == "Offset relative to the Z reference" + + w.entry_recording_Nz.setValue(3) + assert w.label_recording_bottom_z.text() == "Bottom Z offset:" + assert w.entry_recording_bottom_z.toolTip() == "Bottom plane offset relative to the Z reference" + + w.entry_recording_Nz.setValue(1) + assert w.label_recording_bottom_z.text() == "Z offset:" + assert w.entry_recording_bottom_z.toolTip() == "Offset relative to the Z reference" + + # --------------------------------------------------------------------------- # New tests added by fix-batch3 # --------------------------------------------------------------------------- From 2da4a8a168af326c044652cb18b4fed1fe8cfd62 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sun, 5 Jul 2026 16:06:38 -0400 Subject: [PATCH 12/15] refactor(multi-plane): FPS/Duration lead the recording row; Bottom-Z offset moves to its own row Live-testing feedback: FPS/Duration should come first, and the Bottom-Z offset field (visible only when Laser AF is on) shouldn't share a row with Nz/dz/FPS/Dur since it shifts that row's width when it appears/disappears. --- software/control/widgets.py | 99 +++++++++++---------- software/tests/test_record_zstack_widget.py | 6 +- 2 files changed, 56 insertions(+), 49 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index f57055fc0..b28b4c332 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17479,9 +17479,12 @@ def _build_recording_group(self) -> QGroupBox: vbox.addWidget(self.recording_channel_table) - # Single row: Nz | Z offset (+ dz when Nz > 1) | FPS + Duration. - # dz is hidden when Nz == 1, and the offset caption switches between - # "Z offset" (Nz == 1) and "Bottom Z offset" (Nz > 1). + # Row 1: FPS + Duration | Nz (+ dz when Nz > 1). + # Row 2 (own row, shown only when Laser AF is on): Z/Bottom-Z offset — + # kept off the main row so it doesn't shift Nz/dz/FPS/Dur when it + # appears/disappears. dz is hidden when Nz == 1, and the offset + # caption switches between "Z offset" (Nz == 1) and "Bottom Z offset" + # (Nz > 1). def _vline() -> QFrame: line = QFrame() line.setFrameShape(QFrame.VLine) @@ -17491,6 +17494,30 @@ def _vline() -> QFrame: fps_row = QHBoxLayout() fps_row.setSpacing(4) + fps_row.addWidget(QLabel("FPS:")) + self.entry_fps = QDoubleSpinBox() + self.entry_fps.setRange(1, 1000) + self.entry_fps.setDecimals(0) + self.entry_fps.setValue(10.0) + self.entry_fps.setKeyboardTracking(False) + self.entry_fps.setMaximumWidth(60) + fps_row.addWidget(self.entry_fps) + + fps_row.addSpacing(4) + fps_row.addWidget(QLabel("Dur:")) + self.entry_duration = QDoubleSpinBox() + self.entry_duration.setRange(1, 3600) + self.entry_duration.setDecimals(0) + self.entry_duration.setValue(1.0) + self.entry_duration.setSuffix(" s") + self.entry_duration.setKeyboardTracking(False) + self.entry_duration.setMaximumWidth(75) + fps_row.addWidget(self.entry_duration) + + fps_row.addSpacing(4) + fps_row.addWidget(_vline()) + fps_row.addSpacing(4) + fps_row.addWidget(QLabel("Nz:")) self.entry_recording_Nz = QSpinBox() self.entry_recording_Nz.setRange(1, 100) @@ -17500,20 +17527,6 @@ def _vline() -> QFrame: self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") fps_row.addWidget(self.entry_recording_Nz) - fps_row.addSpacing(4) - self.label_recording_bottom_z = QLabel("Z offset:") - fps_row.addWidget(self.label_recording_bottom_z) - self.entry_recording_bottom_z = QDoubleSpinBox() - self.entry_recording_bottom_z.setRange(-500.0, 500.0) - self.entry_recording_bottom_z.setDecimals(1) - self.entry_recording_bottom_z.setSingleStep(0.5) - self.entry_recording_bottom_z.setValue(0.0) - self.entry_recording_bottom_z.setSuffix(" µm") - self.entry_recording_bottom_z.setKeyboardTracking(False) - self.entry_recording_bottom_z.setMaximumWidth(85) - self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") - fps_row.addWidget(self.entry_recording_bottom_z) - fps_row.addSpacing(4) self.label_recording_dz = QLabel("dz:") fps_row.addWidget(self.label_recording_dz) @@ -17528,35 +17541,32 @@ def _vline() -> QFrame: self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") fps_row.addWidget(self.entry_recording_dz) - fps_row.addSpacing(4) - fps_row.addWidget(_vline()) - fps_row.addSpacing(4) - - fps_row.addWidget(QLabel("FPS:")) - self.entry_fps = QDoubleSpinBox() - self.entry_fps.setRange(1, 1000) - self.entry_fps.setDecimals(0) - self.entry_fps.setValue(10.0) - self.entry_fps.setKeyboardTracking(False) - self.entry_fps.setMaximumWidth(60) - fps_row.addWidget(self.entry_fps) - - fps_row.addSpacing(4) - fps_row.addWidget(QLabel("Dur:")) - self.entry_duration = QDoubleSpinBox() - self.entry_duration.setRange(1, 3600) - self.entry_duration.setDecimals(0) - self.entry_duration.setValue(1.0) - self.entry_duration.setSuffix(" s") - self.entry_duration.setKeyboardTracking(False) - self.entry_duration.setMaximumWidth(75) - fps_row.addWidget(self.entry_duration) - fps_row.addStretch(1) vbox.addLayout(fps_row) - # Wire up dz visibility (Nz), Z-offset visibility (Laser AF — the offset - # is relative to the AF reference plane), and the offset caption (Nz). + # Bottom/Z offset gets its own row so it doesn't shift the row above + # when it appears/disappears with Laser AF. + bottom_z_row = QHBoxLayout() + bottom_z_row.setSpacing(4) + self.label_recording_bottom_z = QLabel("Z offset:") + bottom_z_row.addWidget(self.label_recording_bottom_z) + self.entry_recording_bottom_z = QDoubleSpinBox() + self.entry_recording_bottom_z.setRange(-500.0, 500.0) + self.entry_recording_bottom_z.setDecimals(1) + self.entry_recording_bottom_z.setSingleStep(0.5) + self.entry_recording_bottom_z.setValue(0.0) + self.entry_recording_bottom_z.setSuffix(" µm") + self.entry_recording_bottom_z.setKeyboardTracking(False) + self.entry_recording_bottom_z.setMaximumWidth(85) + self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") + bottom_z_row.addWidget(self.entry_recording_bottom_z) + bottom_z_row.addStretch(1) + self.recording_bottom_z_frame = QFrame() + self.recording_bottom_z_frame.setLayout(bottom_z_row) + vbox.addWidget(self.recording_bottom_z_frame) + + # Wire up dz visibility (Nz), Z-offset row visibility (Laser AF — the + # offset is relative to the AF reference plane), and the offset caption (Nz). self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) self.checkbox_laser_af.toggled.connect(self._update_recording_planes_ui) self._update_recording_planes_ui() @@ -17800,8 +17810,7 @@ def _update_recording_planes_ui(self) -> None: self.label_recording_dz.setVisible(multi) self.entry_recording_dz.setVisible(multi) use_af = self.checkbox_laser_af.isChecked() - self.label_recording_bottom_z.setVisible(use_af) - self.entry_recording_bottom_z.setVisible(use_af) + self.recording_bottom_z_frame.setVisible(use_af) self.label_recording_bottom_z.setText("Bottom Z offset:" if multi else "Z offset:") self.entry_recording_bottom_z.setToolTip( "Bottom plane offset relative to the Z reference" if multi else "Offset relative to the Z reference" diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 23a04fef5..660ac58bf 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -2056,13 +2056,11 @@ def test_recording_offset_caption_and_build_parameters(qtbot, simulated_widget_d # Without Laser AF there is no reference plane to offset from: the field # is hidden and its (stale) value must not reach the parameters. assert not w.checkbox_laser_af.isChecked() - assert w.entry_recording_bottom_z.isHidden() - assert w.label_recording_bottom_z.isHidden() + assert w.recording_bottom_z_frame.isHidden() assert w.build_parameters().recording_bottom_z_offset_um == 0.0 w.checkbox_laser_af.setChecked(True) - assert not w.entry_recording_bottom_z.isHidden() - assert not w.label_recording_bottom_z.isHidden() + assert not w.recording_bottom_z_frame.isHidden() # Caption uses multi-plane wording when Nz > 1 and reverts at Nz == 1. assert w.label_recording_bottom_z.text() == "Bottom Z offset:" From 177f71a6d87c35cc24f29482b2a6420671fcc644 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sun, 5 Jul 2026 17:19:48 -0400 Subject: [PATCH 13/15] refactor(multi-plane): align Bottom-Z offset row under Nz via QGridLayout Live-testing feedback: the offset row should line up with Nz, not start flush-left. Switched the recording-controls block from two independent QHBoxLayouts to one QGridLayout so the offset shares Nz's columns and aligns exactly regardless of font/platform metrics. --- software/control/widgets.py | 62 +++++++++------------ software/tests/test_record_zstack_widget.py | 6 +- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index b28b4c332..00f3e8e33 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17479,32 +17479,32 @@ def _build_recording_group(self) -> QGroupBox: vbox.addWidget(self.recording_channel_table) - # Row 1: FPS + Duration | Nz (+ dz when Nz > 1). - # Row 2 (own row, shown only when Laser AF is on): Z/Bottom-Z offset — - # kept off the main row so it doesn't shift Nz/dz/FPS/Dur when it - # appears/disappears. dz is hidden when Nz == 1, and the offset - # caption switches between "Z offset" (Nz == 1) and "Bottom Z offset" - # (Nz > 1). + # Grid, not two independent HBoxes: FPS/Duration | Nz (+ dz when Nz > 1) + # on row 0, with the Z/Bottom-Z offset on row 1 sharing Nz's columns + # (5-6) so it lines up under Nz exactly, regardless of font/platform + # metrics, rather than an HBox row starting flush-left. dz is hidden + # when Nz == 1; the offset caption switches between "Z offset" + # (Nz == 1) and "Bottom Z offset" (Nz > 1). def _vline() -> QFrame: line = QFrame() line.setFrameShape(QFrame.VLine) line.setFrameShadow(QFrame.Sunken) return line - fps_row = QHBoxLayout() - fps_row.setSpacing(4) + recording_grid = QGridLayout() + recording_grid.setHorizontalSpacing(4) + recording_grid.setVerticalSpacing(2) - fps_row.addWidget(QLabel("FPS:")) + recording_grid.addWidget(QLabel("FPS:"), 0, 0) self.entry_fps = QDoubleSpinBox() self.entry_fps.setRange(1, 1000) self.entry_fps.setDecimals(0) self.entry_fps.setValue(10.0) self.entry_fps.setKeyboardTracking(False) self.entry_fps.setMaximumWidth(60) - fps_row.addWidget(self.entry_fps) + recording_grid.addWidget(self.entry_fps, 0, 1) - fps_row.addSpacing(4) - fps_row.addWidget(QLabel("Dur:")) + recording_grid.addWidget(QLabel("Dur:"), 0, 2) self.entry_duration = QDoubleSpinBox() self.entry_duration.setRange(1, 3600) self.entry_duration.setDecimals(0) @@ -17512,24 +17512,21 @@ def _vline() -> QFrame: self.entry_duration.setSuffix(" s") self.entry_duration.setKeyboardTracking(False) self.entry_duration.setMaximumWidth(75) - fps_row.addWidget(self.entry_duration) + recording_grid.addWidget(self.entry_duration, 0, 3) - fps_row.addSpacing(4) - fps_row.addWidget(_vline()) - fps_row.addSpacing(4) + recording_grid.addWidget(_vline(), 0, 4) - fps_row.addWidget(QLabel("Nz:")) + recording_grid.addWidget(QLabel("Nz:"), 0, 5) self.entry_recording_Nz = QSpinBox() self.entry_recording_Nz.setRange(1, 100) self.entry_recording_Nz.setValue(1) self.entry_recording_Nz.setKeyboardTracking(False) self.entry_recording_Nz.setMaximumWidth(55) self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") - fps_row.addWidget(self.entry_recording_Nz) + recording_grid.addWidget(self.entry_recording_Nz, 0, 6) - fps_row.addSpacing(4) self.label_recording_dz = QLabel("dz:") - fps_row.addWidget(self.label_recording_dz) + recording_grid.addWidget(self.label_recording_dz, 0, 7) self.entry_recording_dz = QDoubleSpinBox() self.entry_recording_dz.setRange(0.05, 100.0) self.entry_recording_dz.setDecimals(2) @@ -17539,17 +17536,13 @@ def _vline() -> QFrame: self.entry_recording_dz.setKeyboardTracking(False) self.entry_recording_dz.setMaximumWidth(85) self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") - fps_row.addWidget(self.entry_recording_dz) + recording_grid.addWidget(self.entry_recording_dz, 0, 8) - fps_row.addStretch(1) - vbox.addLayout(fps_row) + recording_grid.setColumnStretch(9, 1) - # Bottom/Z offset gets its own row so it doesn't shift the row above - # when it appears/disappears with Laser AF. - bottom_z_row = QHBoxLayout() - bottom_z_row.setSpacing(4) + # Row 1: Z/Bottom-Z offset, placed in Nz's own columns (5-6). self.label_recording_bottom_z = QLabel("Z offset:") - bottom_z_row.addWidget(self.label_recording_bottom_z) + recording_grid.addWidget(self.label_recording_bottom_z, 1, 5) self.entry_recording_bottom_z = QDoubleSpinBox() self.entry_recording_bottom_z.setRange(-500.0, 500.0) self.entry_recording_bottom_z.setDecimals(1) @@ -17559,13 +17552,11 @@ def _vline() -> QFrame: self.entry_recording_bottom_z.setKeyboardTracking(False) self.entry_recording_bottom_z.setMaximumWidth(85) self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") - bottom_z_row.addWidget(self.entry_recording_bottom_z) - bottom_z_row.addStretch(1) - self.recording_bottom_z_frame = QFrame() - self.recording_bottom_z_frame.setLayout(bottom_z_row) - vbox.addWidget(self.recording_bottom_z_frame) + recording_grid.addWidget(self.entry_recording_bottom_z, 1, 6) - # Wire up dz visibility (Nz), Z-offset row visibility (Laser AF — the + vbox.addLayout(recording_grid) + + # Wire up dz visibility (Nz), Z-offset visibility (Laser AF — the # offset is relative to the AF reference plane), and the offset caption (Nz). self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) self.checkbox_laser_af.toggled.connect(self._update_recording_planes_ui) @@ -17810,7 +17801,8 @@ def _update_recording_planes_ui(self) -> None: self.label_recording_dz.setVisible(multi) self.entry_recording_dz.setVisible(multi) use_af = self.checkbox_laser_af.isChecked() - self.recording_bottom_z_frame.setVisible(use_af) + self.label_recording_bottom_z.setVisible(use_af) + self.entry_recording_bottom_z.setVisible(use_af) self.label_recording_bottom_z.setText("Bottom Z offset:" if multi else "Z offset:") self.entry_recording_bottom_z.setToolTip( "Bottom plane offset relative to the Z reference" if multi else "Offset relative to the Z reference" diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 660ac58bf..23a04fef5 100644 --- a/software/tests/test_record_zstack_widget.py +++ b/software/tests/test_record_zstack_widget.py @@ -2056,11 +2056,13 @@ def test_recording_offset_caption_and_build_parameters(qtbot, simulated_widget_d # Without Laser AF there is no reference plane to offset from: the field # is hidden and its (stale) value must not reach the parameters. assert not w.checkbox_laser_af.isChecked() - assert w.recording_bottom_z_frame.isHidden() + assert w.entry_recording_bottom_z.isHidden() + assert w.label_recording_bottom_z.isHidden() assert w.build_parameters().recording_bottom_z_offset_um == 0.0 w.checkbox_laser_af.setChecked(True) - assert not w.recording_bottom_z_frame.isHidden() + assert not w.entry_recording_bottom_z.isHidden() + assert not w.label_recording_bottom_z.isHidden() # Caption uses multi-plane wording when Nz > 1 and reverts at Nz == 1. assert w.label_recording_bottom_z.text() == "Bottom Z offset:" From 621aae7b8776b33ff9cd59d0b621d7b53a1c45a7 Mon Sep 17 00:00:00 2001 From: Hongquan Li Date: Sun, 5 Jul 2026 21:52:54 -0400 Subject: [PATCH 14/15] refactor(multi-plane): dynamic recording-row reflow with aligned columns Live-testing feedback across several rounds landed on: FPS/Duration always lead row 1; Nz/dz/offset append to row 1 too as long as at most 2 of them are visible, but move to their own row 2 (as a group) only when all 3 are shown at once (Nz>1 AND Laser AF on) -- that's too many fields for one row. When on row 2, Nz/dz get temporarily widened to match FPS/Dur's widths so the columns align exactly (both rows share the same left margin, so equal leading widths are sufficient -- no grid needed). Otherwise they use their own compact widths sized to the actual label/spinbox content, fixing an earlier pass that forced the wider shared width unconditionally and left visible dead space in the common single-row case. Also: dz now shows 1 decimal instead of 2, and both rows share one uniform inter-item spacing instead of a larger explicit gap between field groups. --- software/control/widgets.py | 160 ++++++++++++++++++++++++++---------- 1 file changed, 118 insertions(+), 42 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index 20cfc43c2..7839a6f3b 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -17482,70 +17482,86 @@ def _build_recording_group(self) -> QGroupBox: vbox.addWidget(self.recording_channel_table) - # Grid, not two independent HBoxes: FPS/Duration | Nz (+ dz when Nz > 1) - # on row 0, with the Z/Bottom-Z offset on row 1 sharing Nz's columns - # (5-6) so it lines up under Nz exactly, regardless of font/platform - # metrics, rather than an HBox row starting flush-left. dz is hidden - # when Nz == 1; the offset caption switches between "Z offset" - # (Nz == 1) and "Bottom Z offset" (Nz > 1). - def _vline() -> QFrame: - line = QFrame() - line.setFrameShape(QFrame.VLine) - line.setFrameShadow(QFrame.Sunken) - return line - - recording_grid = QGridLayout() - recording_grid.setHorizontalSpacing(4) - recording_grid.setVerticalSpacing(2) - - recording_grid.addWidget(QLabel("FPS:"), 0, 0) + # Row 1 (fixed): FPS + Duration. + # Nz/dz/offset reflow dynamically: they append to row 1 (after + # Duration) whenever at most 2 of them are visible, but move down to + # their own row 2 together when all 3 are visible at once (Nz>1 AND + # Laser AF on) — that's too many fields for one row. See + # _rebuild_recording_rows, called from _update_recording_planes_ui + # whenever Nz or Laser AF change. + # FPS/Dur always use their own compact widths (fit the actual text, + # not shared with anything). Nz/dz normally use their OWN, smaller + # compact widths too — they only borrow FPS's/Dur's (wider) widths + # when _rebuild_recording_rows puts them on row 2 for alignment; + # forcing the wider shared width unconditionally (as an earlier pass + # here did) left visible dead space after "Nz:"/"dz:" even in the + # common single-row case, where nothing needs to align with them. + self._recording_col1_label_w = 30 # fits "FPS:" + self._recording_col1_entry_w = 50 # fits FPS's spinbox + self._recording_col2_label_w = 28 # fits "Dur:" + self._recording_col2_entry_w = 64 # fits Duration's spinbox + self._recording_nz_label_w = 24 # fits "Nz:" on its own + self._recording_nz_entry_w = 44 # fits Nz's spinbox on its own + self._recording_dz_label_w = 22 # fits "dz:" on its own + self._recording_dz_entry_w = 78 # fits dz's spinbox+suffix on its own + + self.fps_row = QHBoxLayout() + self.fps_row.setSpacing(4) + + label_fps = QLabel("FPS:") + label_fps.setFixedWidth(self._recording_col1_label_w) + self.fps_row.addWidget(label_fps) self.entry_fps = QDoubleSpinBox() self.entry_fps.setRange(1, 1000) self.entry_fps.setDecimals(0) self.entry_fps.setValue(10.0) self.entry_fps.setKeyboardTracking(False) - self.entry_fps.setMaximumWidth(60) - recording_grid.addWidget(self.entry_fps, 0, 1) + self.entry_fps.setFixedWidth(self._recording_col1_entry_w) + self.fps_row.addWidget(self.entry_fps) - recording_grid.addWidget(QLabel("Dur:"), 0, 2) + label_dur = QLabel("Dur:") + label_dur.setFixedWidth(self._recording_col2_label_w) + self.fps_row.addWidget(label_dur) self.entry_duration = QDoubleSpinBox() self.entry_duration.setRange(1, 3600) self.entry_duration.setDecimals(0) self.entry_duration.setValue(1.0) self.entry_duration.setSuffix(" s") self.entry_duration.setKeyboardTracking(False) - self.entry_duration.setMaximumWidth(75) - recording_grid.addWidget(self.entry_duration, 0, 3) - - recording_grid.addWidget(_vline(), 0, 4) - - recording_grid.addWidget(QLabel("Nz:"), 0, 5) + self.entry_duration.setFixedWidth(self._recording_col2_entry_w) + self.fps_row.addWidget(self.entry_duration) + + # Fixed prefix length (FPS label/spin, spacing, Dur label/spin) that + # _rebuild_recording_rows must never remove. + self._fps_row_fixed_count = self.fps_row.count() + vbox.addLayout(self.fps_row) + + self.z_row = QHBoxLayout() + self.z_row.setSpacing(4) + vbox.addLayout(self.z_row) + + # Movable widgets — not added to either row here; _rebuild_recording_rows + # (called once at the end of this method, and again on every Nz/Laser-AF + # change) places them AND sets their widths (compact normally, matched + # to FPS/Dur only when on row 2). + self.label_recording_Nz = QLabel("Nz:") self.entry_recording_Nz = QSpinBox() self.entry_recording_Nz.setRange(1, 100) self.entry_recording_Nz.setValue(1) self.entry_recording_Nz.setKeyboardTracking(False) - self.entry_recording_Nz.setMaximumWidth(55) self.entry_recording_Nz.setToolTip("Number of recording planes per FOV") - recording_grid.addWidget(self.entry_recording_Nz, 0, 6) self.label_recording_dz = QLabel("dz:") - recording_grid.addWidget(self.label_recording_dz, 0, 7) self.entry_recording_dz = QDoubleSpinBox() self.entry_recording_dz.setRange(0.05, 100.0) - self.entry_recording_dz.setDecimals(2) + self.entry_recording_dz.setDecimals(1) self.entry_recording_dz.setSingleStep(0.5) self.entry_recording_dz.setValue(1.0) self.entry_recording_dz.setSuffix(" µm") self.entry_recording_dz.setKeyboardTracking(False) - self.entry_recording_dz.setMaximumWidth(85) self.entry_recording_dz.setToolTip("Plane spacing (shown when Nz > 1)") - recording_grid.addWidget(self.entry_recording_dz, 0, 8) - recording_grid.setColumnStretch(9, 1) - - # Row 1: Z/Bottom-Z offset, placed in Nz's own columns (5-6). self.label_recording_bottom_z = QLabel("Z offset:") - recording_grid.addWidget(self.label_recording_bottom_z, 1, 5) self.entry_recording_bottom_z = QDoubleSpinBox() self.entry_recording_bottom_z.setRange(-500.0, 500.0) self.entry_recording_bottom_z.setDecimals(1) @@ -17555,12 +17571,10 @@ def _vline() -> QFrame: self.entry_recording_bottom_z.setKeyboardTracking(False) self.entry_recording_bottom_z.setMaximumWidth(85) self.entry_recording_bottom_z.setToolTip("Bottom plane offset relative to the Z reference") - recording_grid.addWidget(self.entry_recording_bottom_z, 1, 6) - - vbox.addLayout(recording_grid) # Wire up dz visibility (Nz), Z-offset visibility (Laser AF — the - # offset is relative to the AF reference plane), and the offset caption (Nz). + # offset is relative to the AF reference plane), the offset caption + # (Nz), and the row reflow (both). self.entry_recording_Nz.valueChanged.connect(self._update_recording_planes_ui) self.checkbox_laser_af.toggled.connect(self._update_recording_planes_ui) self._update_recording_planes_ui() @@ -17798,8 +17812,8 @@ def _update_recording_planes_ui(self) -> None: """Toggle dz visibility (hidden entirely when Nz == 1 — user requirement), show the Z-offset field only when Laser AF is enabled (the offset is relative to the AF reference plane; without AF the user positions Z - directly), and switch the offset caption between single- and - multi-plane wording.""" + directly), switch the offset caption between single- and multi-plane + wording, and reflow Nz/dz/offset between row 1 and row 2.""" multi = self.entry_recording_Nz.value() > 1 self.label_recording_dz.setVisible(multi) self.entry_recording_dz.setVisible(multi) @@ -17810,6 +17824,68 @@ def _update_recording_planes_ui(self) -> None: self.entry_recording_bottom_z.setToolTip( "Bottom plane offset relative to the Z reference" if multi else "Offset relative to the Z reference" ) + self._rebuild_recording_rows(multi, use_af) + + def _rebuild_recording_rows(self, multi: bool, use_af: bool) -> None: + """Reflow Nz/dz/offset between fps_row (FPS/Duration's row) and their + own z_row. + + At most 2 of {Nz, dz, offset} are ever shown together unless both + Nz > 1 and Laser AF are on — in that 3-item case they move to z_row + as a group instead of overcrowding fps_row. Widgets are detached and + re-added (not just hidden) so an inactive row takes no vertical + space at all, rather than an empty-but-present row. + """ + + def _clear(layout: QHBoxLayout) -> None: + while layout.count(): + item = layout.takeAt(0) + if item.widget() is None: # spacing/stretch item, not a widget + del item + + while self.fps_row.count() > self._fps_row_fixed_count: + item = self.fps_row.takeAt(self.fps_row.count() - 1) + if item.widget() is None: + del item + _clear(self.z_row) + + use_second_row = multi and use_af + target = self.z_row if use_second_row else self.fps_row + + # Compact widths normally; only widen to match FPS/Dur when Nz/dz + # actually sit below them on row 2, so the common single-row case + # never carries dead space sized for an alignment it isn't doing. + self.label_recording_Nz.setFixedWidth( + self._recording_col1_label_w if use_second_row else self._recording_nz_label_w + ) + self.entry_recording_Nz.setFixedWidth( + self._recording_col1_entry_w if use_second_row else self._recording_nz_entry_w + ) + self.label_recording_dz.setFixedWidth( + self._recording_col2_label_w if use_second_row else self._recording_dz_label_w + ) + self.entry_recording_dz.setFixedWidth( + self._recording_col2_entry_w if use_second_row else self._recording_dz_entry_w + ) + + target.addWidget(self.label_recording_Nz) + target.addWidget(self.entry_recording_Nz) + + if multi: + target.addWidget(self.label_recording_dz) + target.addWidget(self.entry_recording_dz) + + if use_af: + target.addWidget(self.label_recording_bottom_z) + target.addWidget(self.entry_recording_bottom_z) + + target.addStretch(1) + if use_second_row: + # fps_row didn't receive the movable widgets in this mode, but it + # still needs its own trailing stretch — otherwise Qt has nothing + # to absorb the row's leftover width and stretches FPS/Duration's + # label-to-spinbox gap apart instead. + self.fps_row.addStretch(1) def _add_zstack_channel_row( self, name: str, exposure: float = 50.0, gain: float = 0.0, illumination: float = 50.0 From 0becc4e5042ef59774ab312d2a33e59910c01bba Mon Sep 17 00:00:00 2001 From: You Yan Date: Mon, 6 Jul 2026 13:07:43 -0700 Subject: [PATCH 15/15] fix(record-zstack): probe frame shape in CONTINUOUS mode so software-trigger live can't break recording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting a recording while Live ran in SOFTWARE-trigger mode aborted the run on real hardware. _probe_frame_shape() sized the recording dataset by sending a software trigger, but Live's last trigger was still outstanding (get_ready_for_trigger() == False), so send_trigger() was rejected ("trigger too early"). The probe then fell back to get_resolution() — the raw binned sensor size (3104x2084) — while the camera delivers cropped 2084x2084 frames, so every write_frame failed with a dimension-alignment error and the store sealed incomplete, fail-fasting the whole acquisition. Probe in CONTINUOUS mode instead — the same mode recording uses — and read one free-run frame (no send_trigger, so trigger readiness can't block it); the probed shape is then exactly what recording writes. On a genuine probe failure, abort with a clear error instead of sizing from get_resolution() and grinding out a blank, incomplete recording. Also fix SimulatedCamera.read_camera_frame() to wait for a frame (matching the real camera / AbstractCamera contract) rather than returning None before the first CONTINUOUS frame — otherwise the continuous probe gets no frame in sim. Verified on a real ITR3CMOS26000KMA: the continuous probe returns (2084, 2084) (matching delivery) vs get_resolution()'s (3104, 2084), and get_ready_for_trigger() is False right after a software trigger (reproducing the reject). New tests cover the probe using CONTINUOUS/never triggering and aborting (not get_resolution) on no frame. Co-Authored-By: Claude Opus 4.8 --- software/control/core/record_zstack_worker.py | 62 ++++++++-------- software/squid/camera/utils.py | 15 +++- .../tests/core/test_record_zstack_worker.py | 73 +++++++++++++++++++ 3 files changed, 118 insertions(+), 32 deletions(-) diff --git a/software/control/core/record_zstack_worker.py b/software/control/core/record_zstack_worker.py index 649c251ec..4866fefaf 100644 --- a/software/control/core/record_zstack_worker.py +++ b/software/control/core/record_zstack_worker.py @@ -607,50 +607,50 @@ def _move_z_to_offset(self, z_ref: float, offset_um: float) -> None: self._sleep(SCAN_STABILIZATION_TIME_MS_Z / 1000) def _probe_frame_shape(self) -> Tuple[int, int, np.dtype]: - """Return (Y, X, dtype) for the recording dataset from one processed frame. - - ``get_resolution()`` reports the sensor/binned size, but frames delivered - to callbacks pass through ``_process_raw_frame`` (software crop, rotation, - ROI). On cameras where the two differ, a dataset sized from - ``get_resolution()`` makes every ``write_frame`` fail — a blank recording. - Capture one real frame and size the dataset from it; fall back to - ``get_resolution()`` only if the probe capture fails. + """Return (Y, X, dtype) for the recording dataset from one real processed frame. + + Frames delivered to callbacks pass through ``_process_raw_frame`` (software + crop, rotation, ROI), so their shape can differ from ``get_resolution()`` + (the raw binned sensor size). Sizing the dataset from the sensor size makes + every ``write_frame`` fail on cropped cameras — a blank recording. + + Probe in CONTINUOUS mode — the same mode the recording phase uses — and read + one free-run frame, so the probed shape is exactly what recording writes. + CONTINUOUS needs no ``send_trigger``, so a Live view left in SOFTWARE-trigger + mode (with an outstanding trigger that makes ``get_ready_for_trigger`` reject + an immediate trigger) can't block the probe — the failure that previously + dropped it onto the wrong ``get_resolution`` size and failed every write. """ frame = None try: - self.camera.set_acquisition_mode(CameraAcquisitionMode.SOFTWARE_TRIGGER) + self.camera.set_acquisition_mode(CameraAcquisitionMode.CONTINUOUS) self.camera.start_streaming() - self.camera.send_trigger() cam_frame = self.camera.read_camera_frame() if cam_frame is not None: frame = cam_frame.frame except Exception: - log.exception("probe-frame capture failed; falling back to get_resolution()") + log.exception("probe-frame capture failed") finally: try: self.camera.stop_streaming() except Exception: log.exception("failed to stop streaming after probe frame") - if frame is not None: - if frame.ndim != 2: - # A color frame (Y, X, 3) would silently produce a 2-D dataset - # that every write then fails against — reject it up front. - raise ValueError( - f"recording supports monochrome frames only; camera delivered shape {frame.shape} " - f"(set the camera to a mono pixel format for the recording phase)" - ) - return int(frame.shape[0]), int(frame.shape[1]), frame.dtype - log.warning("sizing recording dataset from get_resolution(); may mismatch delivered frames") - width, height = self.camera.get_resolution() - # Map pixel format to numpy dtype (MONO8 -> uint8, else uint16). - try: - from squid.config import CameraPixelFormat - - fmt = self.camera.get_pixel_format() - dtype = np.uint8 if fmt == CameraPixelFormat.MONO8 else np.uint16 - except Exception: - dtype = np.uint16 - return int(height), int(width), np.dtype(dtype) + if frame is None: + # No usable frame: sizing from get_resolution() (raw sensor size) would + # mismatch the cropped delivery and fail every write, so abort with a + # clear reason instead of grinding out a blank, incomplete recording. + raise RuntimeError( + "recording could not capture a probe frame to size the dataset " + "(camera delivered no frame in CONTINUOUS mode); aborting recording" + ) + if frame.ndim != 2: + # A color frame (Y, X, 3) would silently produce a 2-D dataset that + # every write then fails against — reject it up front. + raise ValueError( + f"recording supports monochrome frames only; camera delivered shape {frame.shape} " + f"(set the camera to a mono pixel format for the recording phase)" + ) + return int(frame.shape[0]), int(frame.shape[1]), frame.dtype def _recording_path(self, t_idx: int, region_id, fov_idx: int, plane_idx: int = 0, n_planes: int = 1) -> str: """Per-(t, region, fov[, plane]) recording dataset path under {experiment}/recording. diff --git a/software/squid/camera/utils.py b/software/squid/camera/utils.py index 7fe9fc484..055b93127 100644 --- a/software/squid/camera/utils.py +++ b/software/squid/camera/utils.py @@ -346,7 +346,20 @@ def get_is_streaming(self): return self._streaming_thread and self._streaming_thread.is_alive() @debug_log - def read_camera_frame(self) -> CameraFrame: + def read_camera_frame(self) -> Optional[CameraFrame]: + # Match the real camera / AbstractCamera contract: wait for a frame rather + # than returning whatever _current_frame happens to be right now. In + # CONTINUOUS mode _current_frame is None until the streaming thread produces + # the first frame, so an immediate return would hand callers None (e.g. the + # recording frame-shape probe, which then can't size its dataset). The + # software-trigger path is unaffected: send_trigger() sets _current_frame + # synchronously before the read, so this returns immediately. Bounded by a + # timeout so a stopped/idle stream returns (the last frame or None) instead + # of blocking forever. + timeout_s = (self._exposure_time_ms / 1000.0) * 1.02 + 4 + deadline = time.time() + timeout_s + while self._current_frame is None and time.time() < deadline: + time.sleep(0.001) return self._current_frame @debug_log diff --git a/software/tests/core/test_record_zstack_worker.py b/software/tests/core/test_record_zstack_worker.py index c7f677ac0..0aa4b3b6e 100644 --- a/software/tests/core/test_record_zstack_worker.py +++ b/software/tests/core/test_record_zstack_worker.py @@ -661,6 +661,79 @@ def read_camera_frame(self): RecordZStackWorker._probe_frame_shape(fake_self) +def test_probe_frame_shape_uses_continuous_and_never_triggers(): + """Regression: starting a recording with Live in SOFTWARE-trigger mode left + an outstanding trigger, so the old probe's send_trigger() was rejected + ("trigger too early") and it fell back to the wrong get_resolution() size, + failing every write on cropped cameras. The probe must run in CONTINUOUS + mode (what recording uses) and never call send_trigger, so trigger state + can't block it, and must size from the delivered (cropped) frame.""" + from types import SimpleNamespace + + import numpy as np + + from control.core.record_zstack_worker import RecordZStackWorker + from squid.abc import CameraAcquisitionMode + + processed = np.zeros((2084, 2084), dtype=np.uint16) # cropped delivered shape + modes = [] + + class _FakeCamera: + def get_resolution(self): + return (3104, 2084) # raw binned sensor (width, height) — must NOT be used + + def set_acquisition_mode(self, mode): + modes.append(mode) + + def start_streaming(self): + pass + + def stop_streaming(self): + pass + + def send_trigger(self, *args, **kwargs): + raise AssertionError("probe must not send a trigger in CONTINUOUS mode") + + def read_camera_frame(self): + return SimpleNamespace(frame=processed) + + fake_self = SimpleNamespace(camera=_FakeCamera()) + y, x, dtype = RecordZStackWorker._probe_frame_shape(fake_self) + + assert (y, x) == (2084, 2084), f"expected cropped delivered shape, got ({y}, {x})" + assert dtype == np.uint16 + assert CameraAcquisitionMode.CONTINUOUS in modes, "probe must set CONTINUOUS mode" + + +def test_probe_frame_shape_aborts_when_no_frame_delivered(): + """If the camera delivers no probe frame, the probe must raise (aborting the + run with a clear error) rather than sizing from get_resolution() — the raw + sensor size, which makes every write fail on cropped cameras (a blank run).""" + from types import SimpleNamespace + + from control.core.record_zstack_worker import RecordZStackWorker + + class _FakeCamera: + def get_resolution(self): + return (3104, 2084) + + def set_acquisition_mode(self, mode): + pass + + def start_streaming(self): + pass + + def stop_streaming(self): + pass + + def read_camera_frame(self): + return None + + fake_self = SimpleNamespace(camera=_FakeCamera()) + with pytest.raises(RuntimeError, match="probe"): + RecordZStackWorker._probe_frame_shape(fake_self) + + def test_record_fails_fast_on_dropped_frames(tmp_path, monkeypatch): """R2: sustained backpressure drops are the systematic slow-disk condition the fail-fast was written for — record() must abort the acquisition, not