diff --git a/software/control/acquisition_yaml_loader.py b/software/control/acquisition_yaml_loader.py index eb50ac3a2..812ac6950 100644 --- a/software/control/acquisition_yaml_loader.py +++ b/software/control/acquisition_yaml_loader.py @@ -70,7 +70,9 @@ class RecordZStackYAMLData: recording_channel: Optional[Dict] = None fps: float = 10.0 duration_s: float = 1.0 - recording_z_offset_um: float = 0.0 + recording_bottom_z_offset_um: float = 0.0 + recording_nz: int = 1 + recording_dz_um: float = 1.0 zstack_enabled: bool = False zstack_channels: List[Dict] = field(default_factory=list) @@ -110,7 +112,9 @@ def _parse_record_zstack_yaml_data(data: dict, acq: dict) -> RecordZStackYAMLDat recording_channel=recording.get("channel"), fps=recording.get("fps", 10.0), duration_s=recording.get("duration_s", 1.0), - recording_z_offset_um=recording.get("z_offset_um", 0.0), + recording_bottom_z_offset_um=recording.get("bottom_z_offset_um", 0.0), + recording_nz=recording.get("nz", 1), + recording_dz_um=recording.get("dz_um", 1.0), zstack_enabled=z_stack.get("enabled", False), zstack_channels=z_stack.get("channels", []), z_min_um=z_stack.get("z_min_um", -3.0), diff --git a/software/control/core/record_zstack_controller.py b/software/control/core/record_zstack_controller.py index 0d88a88fb..64a19d31f 100644 --- a/software/control/core/record_zstack_controller.py +++ b/software/control/core/record_zstack_controller.py @@ -30,6 +30,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)] + + def _build_objective_info(objective_store, camera) -> dict: """Build the informational `objective:` YAML section. @@ -92,7 +105,9 @@ def _save_record_zstack_yaml( "channel": _serialize_for_yaml(params.recording_channel) if params.recording_channel else None, "fps": params.fps, "duration_s": params.duration_s, - "z_offset_um": params.recording_z_offset_um, + "bottom_z_offset_um": params.recording_bottom_z_offset_um, + "nz": params.recording_Nz, + "dz_um": params.recording_dz_um, }, "z_stack": { "enabled": params.zstack_enabled, @@ -139,7 +154,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..4866fefaf 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,32 @@ 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, 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 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_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 +383,92 @@ 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) + + # 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: + 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. 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( + 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 +480,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 +492,22 @@ 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()'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( - 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}" ) @@ -538,60 +607,60 @@ 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 + 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 - 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) - - 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/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/control/widgets.py b/software/control/widgets.py index b309937cc..536e5ba47 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -16979,6 +16979,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: @@ -16998,6 +17000,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." @@ -17527,41 +17533,102 @@ def _build_recording_group(self) -> QFrame: vbox.addWidget(self.recording_channel_table) - # Row 1: FPS | Duration | Z-offset - fps_row = QHBoxLayout() - fps_row.setSpacing(4) - - fps_row.addWidget(QLabel("FPS:")) + # 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(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(78) - fps_row.addWidget(self.entry_fps) + self.entry_fps.setFixedWidth(self._recording_col1_entry_w) + self.fps_row.addWidget(self.entry_fps) - fps_row.addSpacing(4) - fps_row.addWidget(QLabel("Dur:")) + 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(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) - self.entry_duration.setMaximumWidth(65) - 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(70) - fps_row.addWidget(self.entry_recording_z_offset) - - fps_row.addStretch(1) - vbox.addLayout(fps_row) + 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.setToolTip("Number of recording planes per FOV") + + self.label_recording_dz = QLabel("dz:") + self.entry_recording_dz = QDoubleSpinBox() + self.entry_recording_dz.setRange(0.05, 100.0) + 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.setToolTip("Plane spacing (shown when Nz > 1)") + + self.label_recording_bottom_z = QLabel("Z offset:") + 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") + + # Wire up dz visibility (Nz), Z-offset visibility (Laser AF — the + # 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() # Collapse the whole phase's fields when unchecked (Qt's checkable # QGroupBox only grays them out; a plain QCheckBox has no built-in @@ -17834,6 +17901,85 @@ 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), + 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), 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) + 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:") + 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 ) -> None: @@ -18082,7 +18228,9 @@ def _apply_yaml_settings(self, yaml_data) -> None: self._recording_illum_spin, self.entry_fps, self.entry_duration, - self.entry_recording_z_offset, + self.entry_recording_Nz, + self.entry_recording_bottom_z, + self.entry_recording_dz, self.checkbox_zstack, self.entry_zmin, self.entry_zmax, @@ -18117,7 +18265,9 @@ def _apply_yaml_settings(self, yaml_data) -> None: ) self.entry_fps.setValue(yaml_data.fps) self.entry_duration.setValue(yaml_data.duration_s) - self.entry_recording_z_offset.setValue(yaml_data.recording_z_offset_um) + self.entry_recording_Nz.setValue(yaml_data.recording_nz) + self.entry_recording_dz.setValue(yaml_data.recording_dz_um) + self.entry_recording_bottom_z.setValue(yaml_data.recording_bottom_z_offset_um) self.checkbox_zstack.setChecked(yaml_data.zstack_enabled) for name in list(self._zstack_channel_names): @@ -18172,6 +18322,12 @@ def _apply_yaml_settings(self, yaml_data) -> None: # in its stale "inactive" styling even after the checkbox/frame # visibility above are updated to reflect the loaded YAML. self._update_tab_styles() + # entry_recording_Nz's valueChanged and checkbox_laser_af's toggled + # signals were both blocked above too, so the normal + # _update_recording_planes_ui-driven dz/bottom-Z visibility refresh + # didn't fire. Unlike _on_time_toggled, _update_recording_planes_ui + # has no stored-state side effects, so it's safe to call directly. + self._update_recording_planes_ui() self._update_zstack_planes_label() self._update_scan_regions() @@ -18212,6 +18368,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): @@ -18275,7 +18433,13 @@ 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(), + # 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(), zstack_channels=zstack_channels, z_min_um=self.entry_zmin.value(), @@ -18291,6 +18455,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()). @@ -18318,6 +18485,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/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/control/test_acquisition_yaml_loader.py b/software/tests/control/test_acquisition_yaml_loader.py index 7c25e13b0..0e3d10319 100644 --- a/software/tests/control/test_acquisition_yaml_loader.py +++ b/software/tests/control/test_acquisition_yaml_loader.py @@ -294,7 +294,9 @@ def test_parse_record_zstack_yaml_select_wells(self, tmp_path): name: BF LED matrix full fps: 15.0 duration_s: 2.0 - z_offset_um: 1.5 + bottom_z_offset_um: 1.5 + nz: 3 + dz_um: 0.8 z_stack: enabled: true channels: @@ -326,7 +328,9 @@ def test_parse_record_zstack_yaml_select_wells(self, tmp_path): assert result.recording_channel == {"name": "BF LED matrix full"} assert result.fps == 15.0 assert result.duration_s == 2.0 - assert result.recording_z_offset_um == 1.5 + assert result.recording_bottom_z_offset_um == 1.5 + assert result.recording_nz == 3 + assert result.recording_dz_um == 0.8 assert result.zstack_enabled is True assert result.zstack_channels == [{"name": "Fluorescence 488 nm Ex"}] assert result.z_min_um == -2.0 diff --git a/software/tests/core/test_record_zstack_worker.py b/software/tests/core/test_record_zstack_worker.py index dcf37aa9a..0aa4b3b6e 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, @@ -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 @@ -756,6 +829,213 @@ def test_config_snapshot_dedupes_channels_by_name(tmp_path): 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 + + +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() + + +# --------------------------------------------------------------------------- +# 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" + + def test_build_objective_info_reads_objective_and_camera(): from unittest.mock import MagicMock from control.core.record_zstack_controller import _build_objective_info @@ -811,7 +1091,9 @@ def test_save_record_zstack_yaml_writes_full_schema(tmp_path): recording_channel=channel, fps=15.0, duration_s=2.0, - recording_z_offset_um=1.5, + recording_bottom_z_offset_um=1.5, + recording_Nz=3, + recording_dz_um=0.8, zstack_enabled=True, zstack_channels=[channel], z_min_um=-2.0, @@ -842,7 +1124,9 @@ def test_save_record_zstack_yaml_writes_full_schema(tmp_path): assert data["recording"]["channel"]["name"] == "BF LED matrix full" assert data["recording"]["fps"] == 15.0 assert data["recording"]["duration_s"] == 2.0 - assert data["recording"]["z_offset_um"] == 1.5 + assert data["recording"]["bottom_z_offset_um"] == 1.5 + assert data["recording"]["nz"] == 3 + assert data["recording"]["dz_um"] == 0.8 assert data["z_stack"]["enabled"] is True assert len(data["z_stack"]["channels"]) == 1 assert data["z_stack"]["z_min_um"] == -2.0 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 diff --git a/software/tests/test_record_zstack_widget.py b/software/tests/test_record_zstack_widget.py index 63451975c..0462e138b 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 @@ -662,6 +663,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 # --------------------------------------------------------------------------- @@ -1471,7 +1659,9 @@ def test_apply_yaml_settings_round_trips_all_fields(qtbot, simulated_widget_deps }, fps=25.0, duration_s=3.0, - recording_z_offset_um=2.0, + recording_bottom_z_offset_um=2.0, + recording_nz=3, + recording_dz_um=1.5, zstack_enabled=True, zstack_channels=[ { @@ -1499,7 +1689,9 @@ def test_apply_yaml_settings_round_trips_all_fields(qtbot, simulated_widget_deps assert w._recording_illumination() == pytest.approx(60.0) assert w.entry_fps.value() == pytest.approx(25.0) assert w.entry_duration.value() == pytest.approx(3.0) - assert w.entry_recording_z_offset.value() == pytest.approx(2.0) + assert w.entry_recording_bottom_z.value() == pytest.approx(2.0) + assert w.entry_recording_Nz.value() == 3 + assert w.entry_recording_dz.value() == pytest.approx(1.5) assert w.checkbox_zstack.isChecked() is True assert w._zstack_channel_names == ["Fluorescence 488 nm Ex"] assert w._get_zstack_row_values("Fluorescence 488 nm Ex") == pytest.approx((80.0, 0.5, 40.0)) @@ -1849,6 +2041,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 @@ -1858,8 +2054,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) @@ -1891,6 +2091,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) @@ -1898,3 +2104,103 @@ def test_full_save_load_round_trip_preserves_settings(qtbot, simulated_widget_de assert ch2.exposure_time == pytest.approx(ch1.exposure_time) assert ch2.analog_gain == pytest.approx(ch1.analog_gain) assert ch2.illumination_intensity == pytest.approx(ch1.illumination_intensity) + + +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 + + +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_offset_caption_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) + + # 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) + 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 + assert params.recording_Nz == 3 + assert params.recording_dz_um == 4.0 + + +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) + 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) + + 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