From f1ad58ad229c267fb192da991c75e9e74ed03b72 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 20:57:55 -0700 Subject: [PATCH 01/15] docs: design spec for per-region laser-AF offset (focus-map constant-z + laser AF) Capture a per-well laser-AF displacement offset at each focus point and drive laser AF to that per-region target during acquisition, instead of the single global reference plane. Active only when Reflection AF + Use Focus Map + the new mode checkbox are all on. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...06-18-per-region-laser-af-offset-design.md | 246 ++++++++++++++++++ 1 file changed, 246 insertions(+) create mode 100644 software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md diff --git a/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md b/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md new file mode 100644 index 000000000..880c2cd49 --- /dev/null +++ b/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md @@ -0,0 +1,246 @@ +# Per-region laser-AF offset (focus-map constant-z + laser AF) + +- **Date:** 2026-06-18 +- **Status:** Approved (design) +- **Scope:** `software/` tree + +## Summary + +Let the user combine laser autofocus (laser AF) with a constant-z focus map so +that each well/region is focused at its **own offset relative to the single +global laser-AF reference plane**, rather than all wells being driven to that one +shared plane. + +Workflow: the user sets the laser-AF reference once, defines one focus point per +well (the existing focus-map "constant + Fit by Region, 1×1" case), and at each +well navigates to the desired focus z and records it. At that moment the system +captures the laser-AF displacement — `measure_displacement()` = `(x − +x_reference)·pixel_to_um` — and stores it as that region's offset. During +acquisition, laser AF at every FOV in that well drives the stage to the well's +stored offset (`move_to_target(offset)`) instead of to the global reference +(`move_to_target(0)`). + +This "pins" each well to its capture-time relationship with the reference plane; +laser AF then maintains that pin against z drift, including across time-lapse +timepoints (because the existing per-FOV laser-AF behavior is unchanged — only +its *target* changes). + +## Motivation + +Today the two subsystems can both be enabled, but they fight: + +- The focus map bakes an **absolute** stage z into every FOV + (`multi_point_controller.py:755-765`). Absolute z does not track sample/stage + drift over a long acquisition. +- Laser AF runs per FOV and drives the spot back to **one** global reference + plane (`move_to_target(0)` hardcoded at `multi_point_worker.py:1158`). It has + no per-well notion, so it overrides the focus map's per-well z by snapping + every well to the same plane. + +There is no way to say "well A1 should be focused 3 µm above the reference plane, +well B2 at −1 µm, and keep them there as the system drifts." This feature adds +exactly that, reusing existing machinery. + +## Background: current behavior (verified against code) + +### Laser AF — `control/core/laser_auto_focus_controller.py` +- Single global reference: `laser_af_properties.x_reference` / `has_reference`, + set once via `set_reference()` (line 395). `signal_reference_changed` is emitted + on change (line 464). +- `measure_displacement()` (line 302) returns `(x − x_reference)·pixel_to_um` in + µm, or `float('nan')` on failure (lines 319/335/339). +- `move_to_target(target_um)` (line 346): measures current displacement, then + moves `um_to_move = target_um − current_displacement_um` (line 372) so the spot + lands at `target_um` of displacement from reference. **Already supports an + arbitrary target** — no controller change needed. + - Returns `False` (soft failure, no raise) if: no reference (355), NaN + displacement (362), `|measured displacement| > laser_af_range` (366), or + cross-correlation mismatch (378). Note the range guard checks the *measured* + displacement, not `(measured − target)`. + +### Focus map — `control/core/core.py` (`FocusMap`) + `control/widgets.py` (`FocusMapWidget`) +- `FocusMapWidget` (widgets.py:10371) holds `focus_points: list[(region_id, x, y, + z)]` (10386). Capture points: `add_current_point` (10596) and + `update_current_z` (10664). Grid generation (`generate_grid`) replaces + `focus_points`. +- The "1 point per well" case is `method="constant"` + `Fit by Region` + 1×1 grid. + `fit_surface` (10680) validates `focus_regions == scan_regions` when by-region + (10688-10697) and enforces 1×1 for constant (10699-10705). +- On acquisition start the controller bakes absolute z into every FOV + (`multi_point_controller.py:755-765` via `scanCoordinates.update_fov_z_level`). + This stays as the **coarse pre-position** in the new mode (unchanged). + +### Per-channel z-offset — `control/core/multi_point_worker.py` +- `_apply_channel_z_offset` (1220) applies each channel's `z_offset_um` **relative + to wherever laser AF left the stage**, gated on `apply_channel_offset AND + do_reflection_af AND af_succeeded`, tracked in `_current_z_offset_um`, reset per + z-level (1256). Because it is relative to the laser-AF-anchored z, it composes + additively with the new per-region target **with no math change**. + +### How they combine today +Both `set_focus_map(focusMap)` and `set_reflection_af_flag(True)` can be set in +one run. Per-FOV z precedence: base z → focus-map absolute z (baked) → laser AF +relative correction to global plane (overrides focus-map z) → per-channel offset. +So the focus-map per-well z is effectively discarded the moment laser AF runs. + +## Goal / non-goals + +**Goal:** a per-region µm offset, captured in the Focus Map panel and applied as +the laser-AF target during acquisition, active only when explicitly enabled and +only for the constant 1-point-per-well focus map. + +**Non-goals (YAGNI):** +- No change to laser-AF per-FOV scheduling or timepoint behavior — laser AF runs + exactly as today; only its target changes. +- No combining offsets with non-constant (spline/RBF surface) focus maps. +- No "measure once per well and reuse for all FOVs" optimization. +- No change to `move_to_target`, to the focus-map z-baking, or to + `_apply_channel_z_offset` internals. + +## Design + +### State / data model +- A new dict `region_laser_af_offsets: dict[str, float]` (region_id → offset µm). +- **Owned by** `FocusMapWidget` (parallel to `focus_points`). Rationale: the + offset is a laser-AF concern captured in the Focus Map panel, not surface-fit + geometry, so it stays out of the `FocusMap` core class. (Approved over the + alternative of co-locating it with `FocusMap.region_surface_fits`.) +- Threaded to the worker via `AcquisitionParameters` (the focus map itself is not + passed to the worker; its z is pre-baked, but offsets must reach + `perform_autofocus`). + +### Capture — `FocusMapWidget` +1. Add a `laserAutofocusController` parameter to `FocusMapWidget.__init__` + (widgets.py:10374), passed from `gui_hcs.py:976` as + `self.laserAutofocusController` (which is `Optional` and may be `None` when + `SUPPORT_LASER_AUTOFOCUS` is off — already defined by gui_hcs.py:647, before + line 976). Store as `self.laserAutofocusController`. +2. New instance state: `self.region_laser_af_offsets: dict[str, float] = {}` and a + `self.capture_laser_af_offset_enabled: bool = False` flag. +3. Capture helper `_capture_region_offset(region_id)`: + - No-op (and clear any stale entry for that region) unless + `capture_laser_af_offset_enabled` is true, the controller is not `None`, and + `laser_af_properties.has_reference` is true. + - Call `measure_displacement()`. If `NaN` → warn (status label + log), do **not** + store. If `|offset| > laser_af_properties.laser_af_range` → warn the well will + fail AF at acquisition, but still store (user may re-target). + - Else store `region_laser_af_offsets[region_id] = offset`. +4. Call `_capture_region_offset(region_id)` at the end of `add_current_point` + (10633-10636) and `update_current_z` (10664-10670). +5. Keep offsets in sync with the focus-point lifecycle: + - `remove_current_point` (10640): drop the region's offset if that was its last + point. + - `generate_grid` (replaces `focus_points`) and any clear path: clear + `region_laser_af_offsets`. + - `import_focus_points`: load offsets if present (see Persistence). + +### Activation / UI +- New checkbox **in the Focus Map panel** (the shared `FocusMapWidget`), e.g. + `self.checkbox_perRegionLaserAFOffset = QCheckBox("Per-region laser AF offset")`, + added in `setup_ui` (near 10456). Its `toggled` sets + `self.capture_laser_af_offset_enabled` and updates the status hint. +- Enable-state: enabled only when (a) the panel is enabled (the panel is already + toggled by `checkbox_useFocusMap.toggled → focusMapWidget.setEnabled`, at + widgets.py:6154 and 7698) **and** (b) Reflection AF is on. Add a + `FocusMapWidget.set_reflection_af_available(bool)` method that stores the flag and + re-derives the checkbox `setEnabled`. Connect **both** multipoint widgets' + `checkbox_withReflectionAutofocus.toggled` to it (alongside the existing + connections at widgets.py:6156-6157 and 7695-7696), mirroring the + `_update_apply_channel_offset_enable_state` precedent. Set the initial state + during each widget's init (next to 6191 / 7745). +- When the checkbox is disabled or unchecked, capture is off and (see below) no + offsets are pushed to the controller — so behavior is identical to today. + +### Acquisition apply +1. `MultiPointController` (multi_point_controller.py): + - New field `self.region_laser_af_offsets: dict[str, float] = {}` near line 228. + - New setter `set_region_laser_af_offsets(self, offsets)` near line 415, + mirroring `set_focus_map`. + - In `build_params` (921), pass `region_laser_af_offsets=self.region_laser_af_offsets` + into the new `AcquisitionParameters` field. +2. `AcquisitionParameters` (multi_point_utils.py:29): add + `region_laser_af_offsets: Dict[str, float]` (default `field(default_factory=dict)` + if the dataclass uses defaults; otherwise add to the constructor call in + `build_params` and to the field list). +3. `MultiPointWorker` (multi_point_worker.py): unpack + `self.region_laser_af_offsets = params.region_laser_af_offsets` in `__init__` + (next to where `self.do_reflection_af` is set, ~line 120). +4. `perform_autofocus` (multi_point_worker.py:1158): replace + `self.laser_auto_focus_controller.move_to_target(0)` with + `self.laser_auto_focus_controller.move_to_target(self.region_laser_af_offsets.get(region_id, 0.0))`. + `region_id` is already a parameter (1133). Log the target used; a missing + region defaults to `0.0` (current behavior). +5. GUI gating — both acquisition-start blocks. The mode is active only when **all + three** hold: `checkbox_useFocusMap.isChecked()` **and** + `checkbox_withReflectionAutofocus.isChecked()` **and** + `focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked()`. (A Qt checkbox + stays checked while disabled, so checking only the mode box is not sufficient — + all three must be verified at start.) + - Flexible path (widgets.py:6480-6493): inside the existing + `if self.checkbox_useFocusMap.isChecked():` block, when the other two + conditions also hold, call + `self.multipointController.set_region_laser_af_offsets(self.focusMapWidget.region_laser_af_offsets)`; + in every other branch call `set_region_laser_af_offsets({})`. + - Wellplate path (widgets.py:8830-8849): same addition. + - Optionally warn at start if the mode is on but some scan regions lack an + offset (those FOVs fall back to target 0). + +### Correctness / edge cases +- **Reference invalidation:** connect `laserAutofocusController.signal_reference_changed` + (laser_auto_focus_controller.py:464) to a `FocusMapWidget` slot that clears + `region_laser_af_offsets` and shows a status message. This covers the auto-reset + on z-range edits (`widgets.py:6285` calls `set_reference`), which otherwise would + silently invalidate every captured offset. +- **No controller / feature off:** all capture is a clean no-op when the controller + is `None` (`SUPPORT_LASER_AUTOFOCUS` off) or the mode checkbox is off. +- **NaN / no reference at capture:** never store; warn. +- **Range at acquisition:** unchanged `move_to_target` range guard; an out-of-range + measured displacement returns `False` → that FOV's AF fails (existing path: + `_laser_af_failures += 1`, per-channel offset skipped). Documented, not changed. +- **Composition with per-channel offset:** unchanged; additive by construction. + +### Persistence +- Extend `export_focus_points` to write an optional `offset_um` column and + `import_focus_points` to read it when present (back-compatible: files without the + column import as before, with empty offsets). Keep this minimal; it is the only + persistence added. + +## File-by-file change list + +| File | Change | +|---|---| +| `control/gui_hcs.py:976` | Pass `self.laserAutofocusController` to `FocusMapWidget(...)`. | +| `control/widgets.py` (`FocusMapWidget`, ~10371-10720) | New ctor param + state; `checkbox_perRegionLaserAFOffset`; `_capture_region_offset`; hook `add_current_point`/`update_current_z`/`remove_current_point`/`generate_grid`/import-export; `set_reflection_af_available`; `signal_reference_changed` handler. | +| `control/widgets.py` (FlexibleMultiPointWidget) | Connect `checkbox_withReflectionAutofocus.toggled` → `focusMapWidget.set_reflection_af_available` (~6156); init enable state (~6191); push offsets at acquisition start (~6480-6493). | +| `control/widgets.py` (WellplateMultiPointWidget) | Same connections (~7695), init (~7745), and push (~8830-8849). | +| `control/core/multi_point_controller.py` | `region_laser_af_offsets` field (~228); `set_region_laser_af_offsets` (~415); pass through `build_params` (~921). | +| `control/core/multi_point_utils.py:29` | Add `region_laser_af_offsets: Dict[str, float]` to `AcquisitionParameters`. | +| `control/core/multi_point_worker.py` | Unpack field in `__init__` (~120); use per-region target at `perform_autofocus` (1158). | + +## Testing + +Use simulated backends (`tests/tools.py`, `tests/control/conftest.py`): +- **Capture:** with a stub laser-AF controller returning a known displacement, + `add_current_point`/`update_current_z` store the offset per region; `NaN` + and no-reference cases store nothing; controller `None` is a no-op. +- **Apply:** `MultiPointWorker.perform_autofocus(region_id, fov)` calls + `move_to_target` with the region's offset (and `0.0` for an unmapped region). + Mock `laser_auto_focus_controller.move_to_target` and assert the argument. +- **Invalidation:** emitting `signal_reference_changed` clears stored offsets. +- **Plumbing:** `set_region_laser_af_offsets` → `build_params` → + `AcquisitionParameters` → worker field round-trips. +- **Regression:** mode off / unmapped regions → `move_to_target(0)` (no behavior + change). Per-channel offset math unchanged when a region offset is present. + +Run: `python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py` +plus `black --config pyproject.toml --check .` + +## Risks + +- Two multipoint widgets share one `FocusMapWidget`; the reflection-AF enable + wiring must be connected from **both** (only one is active at a time). +- The laser-AF range guard is against measured-from-reference displacement, so a + large per-well offset plus drift can exceed `laser_af_range` and silently fail AF + for that FOV. Mitigated by the capture-time `|offset| > laser_af_range` warning. +- `measure_displacement()` toggles the AF laser and adds latency at each capture; + acceptable because capture is a manual, per-well setup action. From 001d7e9e23c67f33686d4ee125ded7a6ebff4b54 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:05:36 -0700 Subject: [PATCH 02/15] docs: implementation plan for per-region laser-AF offset Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-18-per-region-laser-af-offset.md | 831 ++++++++++++++++++ 1 file changed, 831 insertions(+) create mode 100644 software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md diff --git a/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md b/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md new file mode 100644 index 000000000..29d87dbd7 --- /dev/null +++ b/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md @@ -0,0 +1,831 @@ +# Per-region laser-AF offset Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let each well/region be focused at its own offset from the single global laser-AF reference plane, by capturing that offset at the focus point and driving laser AF to it during acquisition (instead of always to displacement 0). + +**Architecture:** A `region_laser_af_offsets: dict[region_id → µm]` is captured in `FocusMapWidget` when the user records a focus point (one per well, the constant-z case), threaded to `MultiPointController` → `AcquisitionParameters` → `MultiPointWorker`, and consumed by changing the single `move_to_target(0)` call in `perform_autofocus` to `move_to_target(offset_for_region)`. The mode is gated behind an explicit checkbox that requires both Reflection AF and Use Focus Map. The focus map's absolute z stays as the coarse pre-position (no change to z-baking); the per-channel z-offset still composes additively because it is relative to wherever laser AF lands. + +**Tech Stack:** Python 3, PyQt5, pytest. Hardware-abstraction via `squid/` simulated backends. No new dependencies. + +Full design: `docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md`. + +## Global Constraints + +- Black line length is **120** (`pyproject.toml`); run `black --config pyproject.toml .` before each commit. +- Logging: GUI status via `self.status_label.setText(...)` (existing `FocusMapWidget` pattern); worker logs via `self._log`. +- Tests run under pytest/pytest-qt; CI command: `python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py`. +- Branch for this work: `per-region-laser-af-offset` (already created off `master`, design spec already committed). +- Offset is **µm displacement from the global laser-AF reference**, equal to `LaserAutofocusController.measure_displacement()` at the focus z. Empty dict → current behavior (target 0 everywhere). +- Mode is active only when ALL of: `checkbox_useFocusMap` ON, `checkbox_withReflectionAutofocus` ON, and `FocusMapWidget.checkbox_perRegionLaserAFOffset` ON (a Qt checkbox stays checked while disabled — verify all three at acquisition start). +- All feature unit tests live in `tests/control/test_per_region_laser_af_offset.py`. + +--- + +### Task 1: Backend — thread offsets through params and apply them in the worker + +**Files:** +- Modify: `control/core/multi_point_utils.py:1` (import) and `:28-65` (`AcquisitionParameters`) +- Modify: `control/core/multi_point_controller.py:228` (field), `:415` (setter), `:935-961` (`build_params`) +- Modify: `control/core/multi_point_worker.py:120-122` (unpack), `:1157-1158` (apply) +- Test: `tests/control/test_per_region_laser_af_offset.py` + +**Interfaces:** +- Produces: `AcquisitionParameters.region_laser_af_offsets: Dict[str, float]` (default `{}`); `MultiPointController.set_region_laser_af_offsets(offsets)`; `MultiPointWorker.region_laser_af_offsets`; `MultiPointWorker.perform_autofocus(region_id, fov)` now targets the region's offset. +- Consumes (existing): `MultiPointWorker.laser_auto_focus_controller.move_to_target(target_um)` (already supports arbitrary targets). + +- [ ] **Step 1: Write the failing test (dataclass field + default)** + +Create `tests/control/test_per_region_laser_af_offset.py`: + +```python +"""Unit tests for the per-region laser-AF offset feature. + +Backend tests construct minimal MultiPointWorker-/FocusMapWidget-shaped stubs and +call the real methods in isolation, mirroring tests/control/test_MultiPointWorker_offsets.py. +""" + +import math +from dataclasses import fields +from unittest.mock import MagicMock + +from control.core.multi_point_utils import AcquisitionParameters +from control.core.multi_point_worker import MultiPointWorker + + +def test_acquisition_parameters_has_region_offsets_field(): + names = {f.name for f in fields(AcquisitionParameters)} + assert "region_laser_af_offsets" in names + + +def test_region_offsets_default_factory_is_empty_dict(): + fld = next(f for f in fields(AcquisitionParameters) if f.name == "region_laser_af_offsets") + assert fld.default_factory() == {} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: FAIL — `region_laser_af_offsets` not in fields (and `StopIteration`/AttributeError on the second test). + +- [ ] **Step 3: Add the field to `AcquisitionParameters`** + +In `control/core/multi_point_utils.py`, change line 1 import: + +```python +from dataclasses import dataclass, field +``` + +Then add, immediately after the `xy_mode` field (currently line 65) inside `AcquisitionParameters`: + +```python + # Per-region laser-AF target offsets (µm from the global laser-AF reference plane), + # keyed by region_id. Empty unless the focus-map + laser-AF combined mode is active, + # in which case each FOV in a region targets that region's offset instead of 0. + region_laser_af_offsets: Dict[str, float] = field(default_factory=dict) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (2 passed). + +- [ ] **Step 5: Write the failing test (apply path)** + +Append to `tests/control/test_per_region_laser_af_offset.py`: + +```python +class _AFStub: + """MultiPointWorker-ish object with just what perform_autofocus's laser-AF branch reads.""" + + def __init__(self, offsets, move_result=True): + self.do_reflection_af = True + self.region_laser_af_offsets = offsets + self._log = MagicMock() + self.laser_auto_focus_controller = MagicMock() + self.laser_auto_focus_controller.move_to_target.return_value = move_result + self._laser_af_successes = 0 + self._laser_af_failures = 0 + # Only touched on the exception path: + self.base_path = "/tmp" + self.experiment_ID = "exp" + self.time_point = 0 + + perform_autofocus = MultiPointWorker.perform_autofocus + + +def test_perform_autofocus_uses_region_offset(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("A1", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(5.0) + assert w._laser_af_successes == 1 + + +def test_perform_autofocus_defaults_to_zero_for_unmapped_region(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("B2", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + + +def test_perform_autofocus_failure_increments_and_returns_false(): + w = _AFStub({"A1": 5.0}, move_result=False) + assert w.perform_autofocus("A1", 0) is False + assert w._laser_af_failures == 1 +``` + +- [ ] **Step 6: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k perform_autofocus -v` +Expected: FAIL — `move_to_target` called with `0`, not `5.0` (current code hardcodes `move_to_target(0)`). + +- [ ] **Step 7: Apply the per-region target in `perform_autofocus`** + +In `control/core/multi_point_worker.py`, replace lines 1157-1158: + +```python + try: + af_succeeded = self.laser_auto_focus_controller.move_to_target(0) +``` + +with: + +```python + try: + target_um = self.region_laser_af_offsets.get(region_id, 0.0) + self._log.info(f"laser AF target for region '{region_id}': {target_um:.2f} µm") + af_succeeded = self.laser_auto_focus_controller.move_to_target(target_um) +``` + +- [ ] **Step 8: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (5 passed). + +- [ ] **Step 9: Wire the plumbing (controller field, setter, build_params, worker unpack)** + +In `control/core/multi_point_controller.py`, after line 228 (`self.focus_map = None`): + +```python + self.region_laser_af_offsets = {} +``` + +After `set_focus_map` (currently lines 415-416), add: + +```python + def set_region_laser_af_offsets(self, offsets): + # region_id -> µm offset from the global laser-AF reference plane. Empty dict means + # every FOV targets the reference (displacement 0), i.e. current behavior. + self.region_laser_af_offsets = dict(offsets) if offsets else {} +``` + +In `build_params`, inside the `AcquisitionParameters(...)` call (after `xy_mode=self.xy_mode,` at line 960): + +```python + region_laser_af_offsets=self.region_laser_af_offsets, +``` + +In `control/core/multi_point_worker.py`, after line 122 (`self.apply_channel_offset = acquisition_parameters.apply_channel_offset`): + +```python + self.region_laser_af_offsets = acquisition_parameters.region_laser_af_offsets +``` + +- [ ] **Step 10: Verify import smoke + format + full backend test** + +Run: `python3 -c "import control.core.multi_point_controller, control.core.multi_point_worker, control.core.multi_point_utils"` +Expected: no error. +Run: `black --config pyproject.toml control/core/multi_point_utils.py control/core/multi_point_controller.py control/core/multi_point_worker.py tests/control/test_per_region_laser_af_offset.py` +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (5 passed). + +- [ ] **Step 11: Commit** + +```bash +git add control/core/multi_point_utils.py control/core/multi_point_controller.py control/core/multi_point_worker.py tests/control/test_per_region_laser_af_offset.py +git commit -m "feat: apply per-region laser-AF target offset in multipoint worker + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 2: Capture logic in `FocusMapWidget` + +**Files:** +- Modify: `control/core/__init__`? No. Modify: `control/widgets.py` (`FocusMapWidget`, `__init__` ~10374-10392; helpers added near 10670) +- Test: `tests/control/test_per_region_laser_af_offset.py` + +**Interfaces:** +- Produces: `FocusMapWidget.region_laser_af_offsets: dict[str, float]`; `FocusMapWidget.capture_laser_af_offset_enabled: bool`; methods `_capture_region_offset(region_id)`, `_clear_region_offsets()`, `_sync_offsets_to_focus_points()`, `_on_laser_af_reference_changed(has_reference)`. +- Consumes: `self.laserAutofocusController` (may be `None`); its `.measure_displacement() -> float` and `.laser_af_properties.has_reference` / `.laser_af_range`. + +- [ ] **Step 1: Write the failing test (capture matrix + clear + sync)** + +Append to `tests/control/test_per_region_laser_af_offset.py`: + +```python +from control.widgets import FocusMapWidget + + +def _laser_controller(displacement, has_reference=True, laser_af_range=200.0): + c = MagicMock() + c.laser_af_properties.has_reference = has_reference + c.laser_af_properties.laser_af_range = laser_af_range + c.measure_displacement.return_value = displacement + return c + + +class _FMStub: + """FocusMapWidget-ish object exposing just what the capture/persistence helpers read.""" + + def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets=None): + self.capture_laser_af_offset_enabled = enabled + self.laserAutofocusController = controller + self.focus_points = focus_points if focus_points is not None else [] + self.region_laser_af_offsets = offsets if offsets is not None else {} + self.status_label = MagicMock() + + _capture_region_offset = FocusMapWidget._capture_region_offset + _clear_region_offsets = FocusMapWidget._clear_region_offsets + _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points + + +def test_capture_stores_displacement_when_enabled(): + w = _FMStub(controller=_laser_controller(3.5)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 3.5} + + +def test_capture_noop_when_mode_disabled(): + ctrl = _laser_controller(3.5) + w = _FMStub(enabled=False, controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_noop_when_no_controller(): + w = _FMStub(controller=None) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + + +def test_capture_noop_when_no_reference(): + ctrl = _laser_controller(3.5, has_reference=False) + w = _FMStub(controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_does_not_store_nan(): + w = _FMStub(controller=_laser_controller(float("nan"))) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_capture_stores_but_warns_when_out_of_range(): + w = _FMStub(controller=_laser_controller(500.0, laser_af_range=200.0)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 500.0} + assert w.status_label.setText.called + + +def test_capture_replaces_stale_entry_when_disabled(): + # Re-capturing with mode off must not leave a stale value for that region. + w = _FMStub(enabled=False, controller=_laser_controller(3.5), offsets={"A1": 9.0}) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_clear_region_offsets(): + w = _FMStub(offsets={"A1": 1.0, "B2": 2.0}) + w._clear_region_offsets() + assert w.region_laser_af_offsets == {} + + +def test_sync_drops_orphaned_offsets(): + w = _FMStub(focus_points=[("A1", 0.0, 0.0, 1.0)], offsets={"A1": 1.0, "B2": 2.0}) + w._sync_offsets_to_focus_points() + assert w.region_laser_af_offsets == {"A1": 1.0} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k "capture or clear or sync" -v` +Expected: FAIL — `AttributeError: type object 'FocusMapWidget' has no attribute '_capture_region_offset'`. + +- [ ] **Step 3: Add capture state to `FocusMapWidget.__init__`** + +In `control/widgets.py`, change the constructor signature (line 10374): + +```python + def __init__(self, stage: AbstractStage, navigationViewer, scanCoordinates, focusMap, laserAutofocusController=None): +``` + +After `self.focusMap = focusMap` (line 10383) add: + +```python + self.laserAutofocusController = laserAutofocusController +``` + +After `self.enabled = False # toggled when focus map enabled for next acquisition` (line 10387) add: + +```python + # Per-region laser-AF offsets (µm from the global laser-AF reference), keyed by + # region_id. Captured at each focus point when the combined mode is enabled. + self.region_laser_af_offsets = {} + self.capture_laser_af_offset_enabled = False + self._reflection_af_available = False +``` + +At the end of `__init__` (after `self.add_margin = True`, line 10392) add: + +```python + if self.laserAutofocusController is not None: + self.laserAutofocusController.signal_reference_changed.connect(self._on_laser_af_reference_changed) +``` + +- [ ] **Step 4: Add the capture/clear/sync helper methods** + +In `control/widgets.py`, add these methods to `FocusMapWidget` (e.g. immediately after `get_region_points_dict`, around line 10678): + +```python + def _capture_region_offset(self, region_id): + """Record the laser-AF displacement at the current z as this region's offset. + + No-op (and clears any stale entry for region_id) unless the combined mode is + enabled, a laser-AF controller exists, and a reference is set. Never stores NaN + (failed spot detection). Warns — but still stores — when the offset exceeds the + laser-AF range, since that region would fail AF during acquisition. + """ + self.region_laser_af_offsets.pop(region_id, None) + if not self.capture_laser_af_offset_enabled or self.laserAutofocusController is None: + return + if not self.laserAutofocusController.laser_af_properties.has_reference: + self.status_label.setText("Laser AF reference not set — per-region offset not captured") + return + offset_um = self.laserAutofocusController.measure_displacement() + if math.isnan(offset_um): + self.status_label.setText(f"Laser AF spot not detected — offset not captured for {region_id}") + return + laser_af_range = self.laserAutofocusController.laser_af_properties.laser_af_range + if abs(offset_um) > laser_af_range: + self.status_label.setText( + f"Warning: region {region_id} offset {offset_um:.1f} µm exceeds laser AF range " + f"({laser_af_range:.1f} µm); it may fail AF during acquisition" + ) + self.region_laser_af_offsets[region_id] = offset_um + + def _clear_region_offsets(self): + self.region_laser_af_offsets = {} + + def _sync_offsets_to_focus_points(self): + """Drop offsets for regions that no longer have any focus point.""" + live_regions = {rid for rid, _, _, _ in self.focus_points} + for rid in list(self.region_laser_af_offsets.keys()): + if rid not in live_regions: + self.region_laser_af_offsets.pop(rid, None) + + def _on_laser_af_reference_changed(self, has_reference): + # The reference plane moved; previously-captured offsets are relative to the old + # reference and are now meaningless. Clear them so they cannot be applied stale. + if self.region_laser_af_offsets: + self._clear_region_offsets() + self.status_label.setText("Laser AF reference changed — captured per-region offsets cleared") +``` + +- [ ] **Step 5: Hook capture into the focus-point lifecycle** + +In `add_current_point`, after `self.navigationViewer.register_focus_point(pos.x_mm, pos.y_mm)` (line 10636) add: + +```python + self._capture_region_offset(region_id) +``` + +In `update_current_z`, after `self.update_point_list()` (line 10670) add: + +```python + self._capture_region_offset(region_id) +``` + +Replace `remove_current_point` (lines 10640-10645) with: + +```python + def remove_current_point(self): + index = self.point_combo.currentIndex() + if 0 <= index < len(self.focus_points): + self.focus_points.pop(index) + self._sync_offsets_to_focus_points() + self.update_point_list() + self.update_focus_point_display() +``` + +In `generate_grid`, after `self.focus_points.clear()` (line 10570) add: + +```python + self._clear_region_offsets() +``` + +- [ ] **Step 6: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k "capture or clear or sync" -v` +Expected: PASS (9 passed). + +- [ ] **Step 7: Format + full feature test** + +Run: `black --config pyproject.toml control/widgets.py tests/control/test_per_region_laser_af_offset.py` +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (14 passed total). + +- [ ] **Step 8: Commit** + +```bash +git add control/widgets.py tests/control/test_per_region_laser_af_offset.py +git commit -m "feat: capture per-region laser-AF offset in FocusMapWidget + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 3: Persist offsets in the focus-points CSV + +**Files:** +- Modify: `control/widgets.py` (`FocusMapWidget.export_focus_points` 10729-10754, `import_focus_points` 10756-10826; add two pure helpers) +- Test: `tests/control/test_per_region_laser_af_offset.py` + +**Interfaces:** +- Produces: `FocusMapWidget._write_focus_points_csv(file_path)`, `FocusMapWidget._read_focus_points_csv(file_path) -> (points, offsets)`. + +- [ ] **Step 1: Write the failing test (CSV round-trip incl. offsets + back-compat)** + +Append to `tests/control/test_per_region_laser_af_offset.py`: + +```python +def test_csv_roundtrip_includes_offsets(tmp_path): + src = _FMStub( + focus_points=[("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)], + offsets={"A1": 7.0}, # B2 intentionally has no offset + ) + src._write_focus_points_csv = FocusMapWidget._write_focus_points_csv.__get__(src) + path = str(tmp_path / "fp.csv") + src._write_focus_points_csv(path) + + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(path) + assert points == [("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)] + assert offsets == {"A1": 7.0} + + +def test_csv_read_back_compat_without_offset_column(tmp_path): + path = tmp_path / "legacy.csv" + path.write_text("Region_ID,X_mm,Y_mm,Z_um\nA1,1.0,2.0,0.5\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(str(path)) + assert points == [("A1", 1.0, 2.0, 0.5)] + assert offsets == {} + + +def test_csv_read_rejects_missing_required_columns(tmp_path): + path = tmp_path / "bad.csv" + path.write_text("Region_ID,X_mm\nA1,1.0\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + import pytest + + with pytest.raises(ValueError): + dst._read_focus_points_csv(str(path)) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k csv -v` +Expected: FAIL — `AttributeError: ... has no attribute '_write_focus_points_csv'`. + +- [ ] **Step 3: Add the pure CSV helpers** + +In `control/widgets.py`, add to `FocusMapWidget` (near the export/import methods): + +```python + def _write_focus_points_csv(self, file_path): + with open(file_path, "w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_um", "Offset_um"]) + for region_id, x, y, z in self.focus_points: + offset = self.region_laser_af_offsets.get(region_id, "") + writer.writerow([region_id, x, y, z, offset]) + + def _read_focus_points_csv(self, file_path): + """Parse a focus-points CSV. Returns (points, offsets). + + points: list of (region_id, x, y, z). offsets: {region_id: float} for rows that + carry a non-empty Offset_um (column optional, for back-compat). Raises ValueError + if any required column is missing. + """ + points = [] + offsets = {} + with open(file_path, "r", newline="") as csvfile: + reader = csv.reader(csvfile) + header = next(reader) + required_columns = ["Region_ID", "X_mm", "Y_mm", "Z_um"] + if not all(col in header for col in required_columns): + raise ValueError(f"CSV file must contain columns: {', '.join(required_columns)}") + region_idx = header.index("Region_ID") + x_idx = header.index("X_mm") + y_idx = header.index("Y_mm") + z_idx = header.index("Z_um") + offset_idx = header.index("Offset_um") if "Offset_um" in header else None + for row in reader: + if len(row) >= 4: + try: + region_id = str(row[region_idx]) + x = float(row[x_idx]) + y = float(row[y_idx]) + z = float(row[z_idx]) + except (ValueError, IndexError): + continue + points.append((region_id, x, y, z)) + if offset_idx is not None and offset_idx < len(row) and row[offset_idx] != "": + try: + offsets[region_id] = float(row[offset_idx]) + except ValueError: + pass + return points, offsets +``` + +- [ ] **Step 4: Route the dialog methods through the helpers** + +Replace the body of `export_focus_points` (the `try`/`except` block, lines 10741-10754) with: + +```python + try: + self._write_focus_points_csv(file_path) + self.status_label.setText(f"Exported {len(self.focus_points)} points to {file_path}") + except Exception as e: + QMessageBox.critical(self, "Export Error", f"Failed to export focus points: {str(e)}") +``` + +Replace the **entire** `import_focus_points` method (lines 10756-10826) with this version, which routes parsing through the helper and loads the offsets (the by-region validation flow is preserved verbatim): + +```python + def import_focus_points(self): + """Import focus points (and optional per-region offsets) from a CSV file""" + file_path, _ = QFileDialog.getOpenFileName(self, "Import Focus Points", "", "CSV Files (*.csv);;All Files (*)") + if not file_path: + return + + try: + imported_points, imported_offsets = self._read_focus_points_csv(file_path) + except ValueError as e: + QMessageBox.warning(self, "Invalid Format", str(e)) + return + except Exception as e: + QMessageBox.critical(self, "Import Error", f"Failed to import focus points: {str(e)}") + return + + # If by_region is checked, validate regions + if self.by_region_checkbox.isChecked(): + scan_regions = set(self.scanCoordinates.region_centers.keys()) + focus_regions = set(region_id for region_id, _, _, _ in imported_points) + if not focus_regions == scan_regions: + response = QMessageBox.warning( + self, + "Region Mismatch", + f"The imported focus points have regions: {', '.join(sorted(focus_regions))}\n\n" + f"Current scan has regions: {', '.join(sorted(scan_regions))}\n\n" + "Import anyway (disable 'By Region') or cancel?", + QMessageBox.Ok | QMessageBox.Cancel, + QMessageBox.Cancel, + ) + if response == QMessageBox.Cancel: + return + else: + # User chose to continue, uncheck by_region + self.by_region_checkbox.setChecked(False) + + # Clear existing points and add imported ones + self.focus_points = imported_points + self.region_laser_af_offsets = imported_offsets + self.update_point_list() + self.update_focus_point_display() + self.status_label.setText(f"Imported {len(imported_points)} focus points") +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k csv -v` +Expected: PASS (3 passed). + +- [ ] **Step 6: Format + full feature test + import smoke** + +Run: `black --config pyproject.toml control/widgets.py tests/control/test_per_region_laser_af_offset.py` +Run: `python3 -c "import control.widgets"` +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (17 passed total). + +- [ ] **Step 7: Commit** + +```bash +git add control/widgets.py tests/control/test_per_region_laser_af_offset.py +git commit -m "feat: persist per-region laser-AF offsets in focus-points CSV + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 4: GUI wiring — checkbox, controller handle, multipoint connections, acquisition push + +This task is GUI integration (Qt-heavy); it is verified by a `--simulation` smoke test rather than unit tests. Implement all sub-steps, then run the smoke test before committing. + +**Files:** +- Modify: `control/widgets.py` (`FocusMapWidget.setup_ui` ~10456-10459, `make_connections` ~10486; new handlers; FlexibleMultiPointWidget `make_connections` ~6154 + init ~6188 + acquisition block 6480-6484; WellplateMultiPointWidget `make_connections` ~7693 + init ~7742 + acquisition block 8827-8838) +- Modify: `control/gui_hcs.py:976` (pass the controller) + +**Interfaces:** +- Consumes: `FocusMapWidget.set_reflection_af_available(bool)`, `FocusMapWidget.checkbox_perRegionLaserAFOffset`, `FocusMapWidget.region_laser_af_offsets`, `MultiPointController.set_region_laser_af_offsets`. + +- [ ] **Step 1: Add the checkbox to `FocusMapWidget.setup_ui`** + +In `control/widgets.py`, after `settings_layout.addWidget(self.by_region_checkbox)` (line 10458) add: + +```python + self.checkbox_perRegionLaserAFOffset = QCheckBox("Per-region laser AF offset") + self.checkbox_perRegionLaserAFOffset.setChecked(False) + self.checkbox_perRegionLaserAFOffset.setEnabled(False) + self.checkbox_perRegionLaserAFOffset.setToolTip( + "With laser AF and focus map both on: capture each region's offset from the laser AF " + "reference at its focus point, and drive laser AF to that per-region target during acquisition." + ) + settings_layout.addWidget(self.checkbox_perRegionLaserAFOffset) +``` + +- [ ] **Step 2: Wire the checkbox + add handlers** + +In `FocusMapWidget.make_connections`, after `self.fit_method_combo.currentTextChanged.connect(self._match_by_region_box)` (line 10486) add: + +```python + self.checkbox_perRegionLaserAFOffset.toggled.connect(self._on_per_region_offset_toggled) +``` + +Add these methods to `FocusMapWidget` (near the capture helpers from Task 2): + +```python + def _on_per_region_offset_toggled(self, checked): + self.capture_laser_af_offset_enabled = checked + if not checked: + self._clear_region_offsets() + + def set_reflection_af_available(self, available): + # The per-region offset only makes sense with laser AF on. Disable + uncheck (and + # via the toggle handler, clear) the checkbox when reflection AF is off — mirrors + # the per-channel offset checkbox behavior in the multipoint widgets. + self._reflection_af_available = bool(available) + self.checkbox_perRegionLaserAFOffset.setEnabled(self._reflection_af_available) + if not self._reflection_af_available and self.checkbox_perRegionLaserAFOffset.isChecked(): + self.checkbox_perRegionLaserAFOffset.setChecked(False) +``` + +- [ ] **Step 3: Pass the laser-AF controller into the widget** + +In `control/gui_hcs.py`, replace the `FocusMapWidget(...)` construction (lines 976-978): + +```python + self.focusMapWidget = widgets.FocusMapWidget( + self.stage, self.navigationViewer, self.scanCoordinates, core.FocusMap(), self.laserAutofocusController + ) +``` + +(`self.laserAutofocusController` is defined at gui_hcs.py:647 — `Optional`, `None` when `SUPPORT_LASER_AUTOFOCUS` is off — so this is safe; the widget tolerates `None`.) + +- [ ] **Step 4: Connect Reflection-AF availability from both multipoint widgets** + +In FlexibleMultiPointWidget `make_connections`, after line 6157 (`...connect(self._update_apply_channel_offset_enable_state)`) add: + +```python + self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) +``` + +After line 6188 (`self._update_apply_channel_offset_enable_state(self.checkbox_withReflectionAutofocus.isChecked())`) add: + +```python + self.focusMapWidget.set_reflection_af_available(self.checkbox_withReflectionAutofocus.isChecked()) +``` + +In WellplateMultiPointWidget `make_connections`, after line 7693 add the same `set_reflection_af_available` connect line, and after line 7742 add the same init call. + +- [ ] **Step 5: Push offsets at acquisition start (Flexible path)** + +In `control/widgets.py`, replace the Flexible focus-map block (lines 6480-6484): + +```python + if self.checkbox_useFocusMap.isChecked(): + self.focusMapWidget.fit_surface() + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + else: + self.multipointController.set_focus_map(None) +``` + +with: + +```python + if self.checkbox_useFocusMap.isChecked(): + self.focusMapWidget.fit_surface() + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + if ( + self.checkbox_withReflectionAutofocus.isChecked() + and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() + ): + self.multipointController.set_region_laser_af_offsets(self.focusMapWidget.region_laser_af_offsets) + else: + self.multipointController.set_region_laser_af_offsets({}) + else: + self.multipointController.set_focus_map(None) + self.multipointController.set_region_laser_af_offsets({}) +``` + +- [ ] **Step 6: Push offsets at acquisition start (Wellplate path)** + +Replace the Wellplate focus-map block (lines 8827-8838): + +```python + if self.checkbox_useFocusMap.isChecked(): + # Try to fit the surface + if self.focusMapWidget.fit_surface(): + # If fit successful, set the surface fitter in controller + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + else: + QMessageBox.warning(self, "Warning", "Failed to fit focus surface") + self.btn_startAcquisition.setChecked(False) + return + else: + # If checkbox not checked, set surface fitter to None + self.multipointController.set_focus_map(None) +``` + +with: + +```python + if self.checkbox_useFocusMap.isChecked(): + # Try to fit the surface + if self.focusMapWidget.fit_surface(): + # If fit successful, set the surface fitter in controller + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + if ( + self.checkbox_withReflectionAutofocus.isChecked() + and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() + ): + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.region_laser_af_offsets + ) + else: + self.multipointController.set_region_laser_af_offsets({}) + else: + QMessageBox.warning(self, "Warning", "Failed to fit focus surface") + self.btn_startAcquisition.setChecked(False) + return + else: + # If checkbox not checked, set surface fitter to None + self.multipointController.set_focus_map(None) + self.multipointController.set_region_laser_af_offsets({}) +``` + +- [ ] **Step 7: Format + import smoke + full suite** + +Run: `black --config pyproject.toml control/widgets.py control/gui_hcs.py` +Run: `python3 -c "import control.widgets, control.gui_hcs"` +Expected: no error. +Run: `python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py -q` +Expected: PASS (no regressions; the 17 feature tests included). + +- [ ] **Step 8: Manual `--simulation` smoke test** + +Run: `python3 main_hcs.py --simulation` +Verify by hand: +1. With Reflection AF OFF, the "Per-region laser AF offset" checkbox in the Focus Map panel is disabled. +2. Turn Reflection AF ON → checkbox becomes enabled. Turn it OFF → checkbox disables and unchecks. +3. With Use Focus Map ON + Reflection AF ON + the new checkbox ON, set a laser-AF reference, define one focus point per region (constant + Fit by Region, 1×1), and confirm Add/Update-Z populates `focusMapWidget.region_laser_af_offsets` (visible via the status label messages / a debugger). +4. Re-setting the laser-AF reference clears the captured offsets (status label message). +5. Export then import a focus-points CSV and confirm the `Offset_um` column round-trips. + +Note any deviations; none should occur. + +- [ ] **Step 9: Commit** + +```bash +git add control/widgets.py control/gui_hcs.py +git commit -m "feat: per-region laser-AF offset GUI wiring (checkbox + acquisition push) + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Notes for the implementer + +- Line numbers are from the current `per-region-laser-af-offset` branch HEAD; if a prior task shifted lines, re-locate by the quoted code, not the number. +- Do not touch `move_to_target`, the focus-map z-baking (`multi_point_controller.py:755-765`), or `_apply_channel_z_offset` — composition is by construction (the per-channel offset is relative to wherever laser AF lands, which now includes the per-region target). +- The third multipoint widget that uses `_ApplyChannelOffsetMixin` (widgets.py ~9433) has no focus map, so it needs no changes. From 2f24e0a1108f89a0cb4b6d0818f467ab5cb5933b Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:09:42 -0700 Subject: [PATCH 03/15] feat: apply per-region laser-AF target offset in multipoint worker Co-Authored-By: Claude Opus 4.8 (1M context) --- .../control/core/multi_point_controller.py | 7 +++ software/control/core/multi_point_utils.py | 7 ++- software/control/core/multi_point_worker.py | 5 +- .../test_per_region_laser_af_offset.py | 60 +++++++++++++++++++ 4 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 software/tests/control/test_per_region_laser_af_offset.py diff --git a/software/control/core/multi_point_controller.py b/software/control/core/multi_point_controller.py index 680c22b8d..01d822d22 100644 --- a/software/control/core/multi_point_controller.py +++ b/software/control/core/multi_point_controller.py @@ -226,6 +226,7 @@ def __init__( self.overlap_percent = 10.0 # FOV overlap percentage self.focus_map = None + self.region_laser_af_offsets = {} self.gen_focus_map = False self.focus_map_storage = [] self.already_using_fmap = False @@ -415,6 +416,11 @@ def set_gen_focus_map_flag(self, flag): def set_focus_map(self, focusMap): self.focus_map = focusMap # None if dont use focusMap + def set_region_laser_af_offsets(self, offsets): + # region_id -> µm offset from the global laser-AF reference plane. Empty dict means + # every FOV targets the reference (displacement 0), i.e. current behavior. + self.region_laser_af_offsets = dict(offsets) if offsets else {} + def set_base_path(self, path): self.base_path = path @@ -958,6 +964,7 @@ def build_params(self, scan_position_information: ScanPositionInformation) -> Ac plate_num_rows=plate_num_rows, plate_num_cols=plate_num_cols, xy_mode=self.xy_mode, + region_laser_af_offsets=self.region_laser_af_offsets, ) def _on_acquisition_completed(self): diff --git a/software/control/core/multi_point_utils.py b/software/control/core/multi_point_utils.py index 11157e1f9..79dbbb68c 100644 --- a/software/control/core/multi_point_utils.py +++ b/software/control/core/multi_point_utils.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import List, Tuple, Dict, Optional, Callable, TYPE_CHECKING from control.core.job_processing import CaptureInfo @@ -64,6 +64,11 @@ class AcquisitionParameters: # XY mode for determining scan type xy_mode: str = "Current Position" # "Current Position", "Select Wells", "Manual", "Load Coordinates" + # Per-region laser-AF target offsets (µm from the global laser-AF reference plane), + # keyed by region_id. Empty unless the focus-map + laser-AF combined mode is active, + # in which case each FOV in a region targets that region's offset instead of 0. + region_laser_af_offsets: Dict[str, float] = field(default_factory=dict) + @dataclass class OverallProgressUpdate: diff --git a/software/control/core/multi_point_worker.py b/software/control/core/multi_point_worker.py index 9c834ed83..7d197d026 100644 --- a/software/control/core/multi_point_worker.py +++ b/software/control/core/multi_point_worker.py @@ -120,6 +120,7 @@ def __init__( self.do_reflection_af = acquisition_parameters.do_reflection_autofocus self.use_piezo = acquisition_parameters.use_piezo self.apply_channel_offset = acquisition_parameters.apply_channel_offset + self.region_laser_af_offsets = acquisition_parameters.region_laser_af_offsets self.display_resolution_scaling = acquisition_parameters.display_resolution_scaling self.experiment_ID = acquisition_parameters.experiment_ID @@ -1155,7 +1156,9 @@ def perform_autofocus(self, region_id, fov): # value, NOT by raising — both paths must mark the FOV's AF as failed or # the per-channel z-offset gate would apply offsets from an unanchored z. try: - af_succeeded = self.laser_auto_focus_controller.move_to_target(0) + target_um = self.region_laser_af_offsets.get(region_id, 0.0) + self._log.info(f"laser AF target for region '{region_id}': {target_um:.2f} µm") + af_succeeded = self.laser_auto_focus_controller.move_to_target(target_um) except Exception as e: file_ID = f"{region_id}_focus_camera.bmp" saving_path = os.path.join(self.base_path, self.experiment_ID, str(self.time_point), file_ID) diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py new file mode 100644 index 000000000..0a8db423f --- /dev/null +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -0,0 +1,60 @@ +"""Unit tests for the per-region laser-AF offset feature. + +Backend tests construct minimal MultiPointWorker-/FocusMapWidget-shaped stubs and +call the real methods in isolation, mirroring tests/control/test_MultiPointWorker_offsets.py. +""" + +import math +from dataclasses import fields +from unittest.mock import MagicMock + +from control.core.multi_point_utils import AcquisitionParameters +from control.core.multi_point_worker import MultiPointWorker + + +def test_acquisition_parameters_has_region_offsets_field(): + names = {f.name for f in fields(AcquisitionParameters)} + assert "region_laser_af_offsets" in names + + +def test_region_offsets_default_factory_is_empty_dict(): + fld = next(f for f in fields(AcquisitionParameters) if f.name == "region_laser_af_offsets") + assert fld.default_factory() == {} + + +class _AFStub: + """MultiPointWorker-ish object with just what perform_autofocus's laser-AF branch reads.""" + + def __init__(self, offsets, move_result=True): + self.do_reflection_af = True + self.region_laser_af_offsets = offsets + self._log = MagicMock() + self.laser_auto_focus_controller = MagicMock() + self.laser_auto_focus_controller.move_to_target.return_value = move_result + self._laser_af_successes = 0 + self._laser_af_failures = 0 + # Only touched on the exception path: + self.base_path = "/tmp" + self.experiment_ID = "exp" + self.time_point = 0 + + perform_autofocus = MultiPointWorker.perform_autofocus + + +def test_perform_autofocus_uses_region_offset(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("A1", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(5.0) + assert w._laser_af_successes == 1 + + +def test_perform_autofocus_defaults_to_zero_for_unmapped_region(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("B2", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + + +def test_perform_autofocus_failure_increments_and_returns_false(): + w = _AFStub({"A1": 5.0}, move_result=False) + assert w.perform_autofocus("A1", 0) is False + assert w._laser_af_failures == 1 From 8d046b3a34b194943e73adbdb166f80e987fe296 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:14:30 -0700 Subject: [PATCH 04/15] feat: capture per-region laser-AF offset in FocusMapWidget Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/widgets.py | 60 ++++++++++++- .../test_per_region_laser_af_offset.py | 86 +++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index 1607b1f4b..d4b859954 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -10368,7 +10368,9 @@ def _is_valid_column_value(self, column_name: str, value: int) -> bool: class FocusMapWidget(QFrame): """Widget for managing focus map points and surface fitting""" - def __init__(self, stage: AbstractStage, navigationViewer, scanCoordinates, focusMap): + def __init__( + self, stage: AbstractStage, navigationViewer, scanCoordinates, focusMap, laserAutofocusController=None + ): super().__init__() self.setFrameStyle(QFrame.Panel | QFrame.Raised) self._allow_updating_focus_points_on_signal = True @@ -10378,16 +10380,25 @@ def __init__(self, stage: AbstractStage, navigationViewer, scanCoordinates, focu self.navigationViewer = navigationViewer self.scanCoordinates = scanCoordinates self.focusMap = focusMap + self.laserAutofocusController = laserAutofocusController # Store focus points in widget self.focus_points = [] # list of (region_id, x, y, z) tuples self.enabled = False # toggled when focus map enabled for next acquisition + # Per-region laser-AF offsets (µm from the global laser-AF reference), keyed by + # region_id. Captured at each focus point when the combined mode is enabled. + self.region_laser_af_offsets = {} + self.capture_laser_af_offset_enabled = False + self._reflection_af_available = False self.setup_ui() self.make_connections() self.setEnabled(False) self.add_margin = True # margin for focus grid makes it smaller, but will avoid points at the borders + if self.laserAutofocusController is not None: + self.laserAutofocusController.signal_reference_changed.connect(self._on_laser_af_reference_changed) + def setup_ui(self): """Create and arrange UI components""" self.layout = QVBoxLayout(self) @@ -10568,6 +10579,7 @@ def generate_grid(self, rows=4, cols=4): if self.enabled: self.point_combo.blockSignals(True) self.focus_points.clear() + self._clear_region_offsets() self.navigationViewer.clear_focus_points() self.status_label.setText(" ") current_z = self.stage.get_pos().z_mm @@ -10631,6 +10643,7 @@ def add_current_point(self): self.focus_points.append((region_id, pos.x_mm, pos.y_mm, pos.z_mm)) self.update_point_list() self.navigationViewer.register_focus_point(pos.x_mm, pos.y_mm) + self._capture_region_offset(region_id) else: QMessageBox.warning(self, "Region Error", "Could not determine a valid region for this focus point.") @@ -10638,6 +10651,7 @@ def remove_current_point(self): index = self.point_combo.currentIndex() if 0 <= index < len(self.focus_points): self.focus_points.pop(index) + self._sync_offsets_to_focus_points() self.update_point_list() self.update_focus_point_display() @@ -10665,6 +10679,7 @@ def update_current_z(self): region_id, x, y, _ = self.focus_points[index] self.focus_points[index] = (region_id, x, y, new_z) self.update_point_list() + self._capture_region_offset(region_id) def get_region_points_dict(self): points_dict = {} @@ -10674,6 +10689,49 @@ def get_region_points_dict(self): points_dict[region_id].append((x, y, z)) return points_dict + def _capture_region_offset(self, region_id): + """Record the laser-AF displacement at the current z as this region's offset. + + No-op (and clears any stale entry for region_id) unless the combined mode is + enabled, a laser-AF controller exists, and a reference is set. Never stores NaN + (failed spot detection). Warns — but still stores — when the offset exceeds the + laser-AF range, since that region would fail AF during acquisition. + """ + self.region_laser_af_offsets.pop(region_id, None) + if not self.capture_laser_af_offset_enabled or self.laserAutofocusController is None: + return + if not self.laserAutofocusController.laser_af_properties.has_reference: + self.status_label.setText("Laser AF reference not set — per-region offset not captured") + return + offset_um = self.laserAutofocusController.measure_displacement() + if math.isnan(offset_um): + self.status_label.setText(f"Laser AF spot not detected — offset not captured for {region_id}") + return + laser_af_range = self.laserAutofocusController.laser_af_properties.laser_af_range + if abs(offset_um) > laser_af_range: + self.status_label.setText( + f"Warning: region {region_id} offset {offset_um:.1f} µm exceeds laser AF range " + f"({laser_af_range:.1f} µm); it may fail AF during acquisition" + ) + self.region_laser_af_offsets[region_id] = offset_um + + def _clear_region_offsets(self): + self.region_laser_af_offsets = {} + + def _sync_offsets_to_focus_points(self): + """Drop offsets for regions that no longer have any focus point.""" + live_regions = {rid for rid, _, _, _ in self.focus_points} + for rid in list(self.region_laser_af_offsets.keys()): + if rid not in live_regions: + self.region_laser_af_offsets.pop(rid, None) + + def _on_laser_af_reference_changed(self, has_reference): + # The reference plane moved; previously-captured offsets are relative to the old + # reference and are now meaningless. Clear them so they cannot be applied stale. + if self.region_laser_af_offsets: + self._clear_region_offsets() + self.status_label.setText("Laser AF reference changed — captured per-region offsets cleared") + def fit_surface(self): try: method = self.fit_method_combo.currentText() diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index 0a8db423f..e850cb76b 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -58,3 +58,89 @@ def test_perform_autofocus_failure_increments_and_returns_false(): w = _AFStub({"A1": 5.0}, move_result=False) assert w.perform_autofocus("A1", 0) is False assert w._laser_af_failures == 1 + + +from control.widgets import FocusMapWidget + + +def _laser_controller(displacement, has_reference=True, laser_af_range=200.0): + c = MagicMock() + c.laser_af_properties.has_reference = has_reference + c.laser_af_properties.laser_af_range = laser_af_range + c.measure_displacement.return_value = displacement + return c + + +class _FMStub: + """FocusMapWidget-ish object exposing just what the capture/persistence helpers read.""" + + def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets=None): + self.capture_laser_af_offset_enabled = enabled + self.laserAutofocusController = controller + self.focus_points = focus_points if focus_points is not None else [] + self.region_laser_af_offsets = offsets if offsets is not None else {} + self.status_label = MagicMock() + + _capture_region_offset = FocusMapWidget._capture_region_offset + _clear_region_offsets = FocusMapWidget._clear_region_offsets + _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points + + +def test_capture_stores_displacement_when_enabled(): + w = _FMStub(controller=_laser_controller(3.5)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 3.5} + + +def test_capture_noop_when_mode_disabled(): + ctrl = _laser_controller(3.5) + w = _FMStub(enabled=False, controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_noop_when_no_controller(): + w = _FMStub(controller=None) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + + +def test_capture_noop_when_no_reference(): + ctrl = _laser_controller(3.5, has_reference=False) + w = _FMStub(controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_does_not_store_nan(): + w = _FMStub(controller=_laser_controller(float("nan"))) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_capture_stores_but_warns_when_out_of_range(): + w = _FMStub(controller=_laser_controller(500.0, laser_af_range=200.0)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 500.0} + assert w.status_label.setText.called + + +def test_capture_replaces_stale_entry_when_disabled(): + # Re-capturing with mode off must not leave a stale value for that region. + w = _FMStub(enabled=False, controller=_laser_controller(3.5), offsets={"A1": 9.0}) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_clear_region_offsets(): + w = _FMStub(offsets={"A1": 1.0, "B2": 2.0}) + w._clear_region_offsets() + assert w.region_laser_af_offsets == {} + + +def test_sync_drops_orphaned_offsets(): + w = _FMStub(focus_points=[("A1", 0.0, 0.0, 1.0)], offsets={"A1": 1.0, "B2": 2.0}) + w._sync_offsets_to_focus_points() + assert w.region_laser_af_offsets == {"A1": 1.0} From dba03413b97ad02ebd42e895aa285185f9a6987c Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:18:15 -0700 Subject: [PATCH 05/15] fix: move test import to top; cover laser-AF reference invalidation Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test_per_region_laser_af_offset.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index e850cb76b..c4350f90d 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -10,6 +10,7 @@ from control.core.multi_point_utils import AcquisitionParameters from control.core.multi_point_worker import MultiPointWorker +from control.widgets import FocusMapWidget def test_acquisition_parameters_has_region_offsets_field(): @@ -60,9 +61,6 @@ def test_perform_autofocus_failure_increments_and_returns_false(): assert w._laser_af_failures == 1 -from control.widgets import FocusMapWidget - - def _laser_controller(displacement, has_reference=True, laser_af_range=200.0): c = MagicMock() c.laser_af_properties.has_reference = has_reference @@ -84,6 +82,7 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= _capture_region_offset = FocusMapWidget._capture_region_offset _clear_region_offsets = FocusMapWidget._clear_region_offsets _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points + _on_laser_af_reference_changed = FocusMapWidget._on_laser_af_reference_changed def test_capture_stores_displacement_when_enabled(): @@ -125,6 +124,7 @@ def test_capture_stores_but_warns_when_out_of_range(): w._capture_region_offset("A1") assert w.region_laser_af_offsets == {"A1": 500.0} assert w.status_label.setText.called + assert "exceeds" in w.status_label.setText.call_args[0][0] def test_capture_replaces_stale_entry_when_disabled(): @@ -144,3 +144,16 @@ def test_sync_drops_orphaned_offsets(): w = _FMStub(focus_points=[("A1", 0.0, 0.0, 1.0)], offsets={"A1": 1.0, "B2": 2.0}) w._sync_offsets_to_focus_points() assert w.region_laser_af_offsets == {"A1": 1.0} + + +def test_reference_change_clears_offsets_and_sets_status(): + w = _FMStub(offsets={"A1": 1.0}) + w._on_laser_af_reference_changed(True) + assert w.region_laser_af_offsets == {} + assert w.status_label.setText.called + + +def test_reference_change_no_status_when_nothing_to_clear(): + w = _FMStub() + w._on_laser_af_reference_changed(True) + w.status_label.setText.assert_not_called() From 9b0f221875245e4c13a6df65fb3985b7c1ad2400 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:21:42 -0700 Subject: [PATCH 06/15] feat: persist per-region laser-AF offsets in focus-points CSV Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/widgets.py | 152 +++++++++--------- .../test_per_region_laser_af_offset.py | 37 +++++ 2 files changed, 116 insertions(+), 73 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index d4b859954..a4ccd0ab4 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -10784,6 +10784,51 @@ def _match_by_region_box(self): if self.fit_method_combo.currentText() == "constant": self.by_region_checkbox.setChecked(True) + def _write_focus_points_csv(self, file_path): + with open(file_path, "w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_um", "Offset_um"]) + for region_id, x, y, z in self.focus_points: + offset = self.region_laser_af_offsets.get(region_id, "") + writer.writerow([region_id, x, y, z, offset]) + + def _read_focus_points_csv(self, file_path): + """Parse a focus-points CSV. Returns (points, offsets). + + points: list of (region_id, x, y, z). offsets: {region_id: float} for rows that + carry a non-empty Offset_um (column optional, for back-compat). Raises ValueError + if any required column is missing. + """ + points = [] + offsets = {} + with open(file_path, "r", newline="") as csvfile: + reader = csv.reader(csvfile) + header = next(reader) + required_columns = ["Region_ID", "X_mm", "Y_mm", "Z_um"] + if not all(col in header for col in required_columns): + raise ValueError(f"CSV file must contain columns: {', '.join(required_columns)}") + region_idx = header.index("Region_ID") + x_idx = header.index("X_mm") + y_idx = header.index("Y_mm") + z_idx = header.index("Z_um") + offset_idx = header.index("Offset_um") if "Offset_um" in header else None + for row in reader: + if len(row) >= 4: + try: + region_id = str(row[region_idx]) + x = float(row[x_idx]) + y = float(row[y_idx]) + z = float(row[z_idx]) + except (ValueError, IndexError): + continue + points.append((region_id, x, y, z)) + if offset_idx is not None and offset_idx < len(row) and row[offset_idx] != "": + try: + offsets[region_id] = float(row[offset_idx]) + except ValueError: + pass + return points, offsets + def export_focus_points(self): """Export focus points to a CSV file""" if not self.focus_points: @@ -10797,91 +10842,52 @@ def export_focus_points(self): file_path += ".csv" try: - with open(file_path, "w", newline="") as csvfile: - writer = csv.writer(csvfile) - # Write header - writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_um"]) - - # Write data - for region_id, x, y, z in self.focus_points: - writer.writerow([region_id, x, y, z]) - + self._write_focus_points_csv(file_path) self.status_label.setText(f"Exported {len(self.focus_points)} points to {file_path}") - except Exception as e: QMessageBox.critical(self, "Export Error", f"Failed to export focus points: {str(e)}") def import_focus_points(self): - """Import focus points from a CSV file""" + """Import focus points (and optional per-region offsets) from a CSV file""" file_path, _ = QFileDialog.getOpenFileName(self, "Import Focus Points", "", "CSV Files (*.csv);;All Files (*)") - if not file_path: return try: - # Read the CSV file - imported_points = [] - with open(file_path, "r", newline="") as csvfile: - reader = csv.reader(csvfile) - header = next(reader) # Skip header row - - # Validate header - required_columns = ["Region_ID", "X_mm", "Y_mm", "Z_um"] - if not all(col in header for col in required_columns): - QMessageBox.warning( - self, "Invalid Format", f"CSV file must contain columns: {', '.join(required_columns)}" - ) - return - - # Get column indices - region_idx = header.index("Region_ID") - x_idx = header.index("X_mm") - y_idx = header.index("Y_mm") - z_idx = header.index("Z_um") - - # Read data - for row in reader: - if len(row) >= 4: - try: - region_id = str(row[region_idx]) - x = float(row[x_idx]) - y = float(row[y_idx]) - z = float(row[z_idx]) - imported_points.append((region_id, x, y, z)) - except (ValueError, IndexError): - continue - - # If by_region is checked, validate regions - if self.by_region_checkbox.isChecked(): - scan_regions = set(self.scanCoordinates.region_centers.keys()) - focus_regions = set(region_id for region_id, _, _, _ in imported_points) - - if not focus_regions == scan_regions: - response = QMessageBox.warning( - self, - "Region Mismatch", - f"The imported focus points have regions: {', '.join(sorted(focus_regions))}\n\n" - f"Current scan has regions: {', '.join(sorted(scan_regions))}\n\n" - "Import anyway (disable 'By Region') or cancel?", - QMessageBox.Ok | QMessageBox.Cancel, - QMessageBox.Cancel, - ) - - if response == QMessageBox.Cancel: - return - else: - # User chose to continue, uncheck by_region - self.by_region_checkbox.setChecked(False) - - # Clear existing points and add imported ones - self.focus_points = imported_points - self.update_point_list() - self.update_focus_point_display() - - self.status_label.setText(f"Imported {len(imported_points)} focus points") - + imported_points, imported_offsets = self._read_focus_points_csv(file_path) + except ValueError as e: + QMessageBox.warning(self, "Invalid Format", str(e)) + return except Exception as e: QMessageBox.critical(self, "Import Error", f"Failed to import focus points: {str(e)}") + return + + # If by_region is checked, validate regions + if self.by_region_checkbox.isChecked(): + scan_regions = set(self.scanCoordinates.region_centers.keys()) + focus_regions = set(region_id for region_id, _, _, _ in imported_points) + if not focus_regions == scan_regions: + response = QMessageBox.warning( + self, + "Region Mismatch", + f"The imported focus points have regions: {', '.join(sorted(focus_regions))}\n\n" + f"Current scan has regions: {', '.join(sorted(scan_regions))}\n\n" + "Import anyway (disable 'By Region') or cancel?", + QMessageBox.Ok | QMessageBox.Cancel, + QMessageBox.Cancel, + ) + if response == QMessageBox.Cancel: + return + else: + # User chose to continue, uncheck by_region + self.by_region_checkbox.setChecked(False) + + # Clear existing points and add imported ones + self.focus_points = imported_points + self.region_laser_af_offsets = imported_offsets + self.update_point_list() + self.update_focus_point_display() + self.status_label.setText(f"Imported {len(imported_points)} focus points") def on_regions_updated(self): if not self._allow_updating_focus_points_on_signal: diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index c4350f90d..3ac14a332 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -157,3 +157,40 @@ def test_reference_change_no_status_when_nothing_to_clear(): w = _FMStub() w._on_laser_af_reference_changed(True) w.status_label.setText.assert_not_called() + + +def test_csv_roundtrip_includes_offsets(tmp_path): + src = _FMStub( + focus_points=[("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)], + offsets={"A1": 7.0}, # B2 intentionally has no offset + ) + src._write_focus_points_csv = FocusMapWidget._write_focus_points_csv.__get__(src) + path = str(tmp_path / "fp.csv") + src._write_focus_points_csv(path) + + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(path) + assert points == [("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)] + assert offsets == {"A1": 7.0} + + +def test_csv_read_back_compat_without_offset_column(tmp_path): + path = tmp_path / "legacy.csv" + path.write_text("Region_ID,X_mm,Y_mm,Z_um\nA1,1.0,2.0,0.5\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(str(path)) + assert points == [("A1", 1.0, 2.0, 0.5)] + assert offsets == {} + + +def test_csv_read_rejects_missing_required_columns(tmp_path): + path = tmp_path / "bad.csv" + path.write_text("Region_ID,X_mm\nA1,1.0\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + import pytest + + with pytest.raises(ValueError): + dst._read_focus_points_csv(str(path)) From 99098f6fa06cd1c7fba3e6fd9db78ac542c05374 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:29:14 -0700 Subject: [PATCH 07/15] feat: per-region laser-AF offset GUI wiring (checkbox + acquisition push) Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/gui_hcs.py | 2 +- software/control/widgets.py | 45 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index 361f76836..e1578949e 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -974,7 +974,7 @@ def load_widgets(self): self.wellSelectionWidget = widgets.Well1536SelectionWidget(self.wellplateFormatWidget) self.scanCoordinates.add_well_selector(self.wellSelectionWidget) self.focusMapWidget = widgets.FocusMapWidget( - self.stage, self.navigationViewer, self.scanCoordinates, core.FocusMap() + self.stage, self.navigationViewer, self.scanCoordinates, core.FocusMap(), self.laserAutofocusController ) if SUPPORT_LASER_AUTOFOCUS: diff --git a/software/control/widgets.py b/software/control/widgets.py index a4ccd0ab4..a8879897e 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -6152,6 +6152,7 @@ def setup_connections(self): self.checkbox_withAutofocus.toggled.connect(self.multipointController.set_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self.multipointController.set_reflection_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self._update_apply_channel_offset_enable_state) + self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) self.checkbox_usePiezo.toggled.connect(self.multipointController.set_use_piezo) self.checkbox_skipSaving.toggled.connect(self.multipointController.set_skip_saving) self.btn_setSavingDir.clicked.connect(self.set_saving_dir) @@ -6186,6 +6187,7 @@ def setup_connections(self): self.toggle_z_range_controls(False) self.multipointController.set_use_piezo(self.checkbox_usePiezo.isChecked()) self._update_apply_channel_offset_enable_state(self.checkbox_withReflectionAutofocus.isChecked()) + self.focusMapWidget.set_reflection_af_available(self.checkbox_withReflectionAutofocus.isChecked()) def setup_layout(self): self.grid = QVBoxLayout() @@ -6477,8 +6479,16 @@ def toggle_acquisition(self, pressed): if self.checkbox_useFocusMap.isChecked(): self.focusMapWidget.fit_surface() self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + if ( + self.checkbox_withReflectionAutofocus.isChecked() + and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() + ): + self.multipointController.set_region_laser_af_offsets(self.focusMapWidget.region_laser_af_offsets) + else: + self.multipointController.set_region_laser_af_offsets({}) else: self.multipointController.set_focus_map(None) + self.multipointController.set_region_laser_af_offsets({}) # Set acquisition parameters self.multipointController.set_deltaZ(self.entry_deltaZ.value()) @@ -7691,6 +7701,7 @@ def add_components(self): self.checkbox_withAutofocus.toggled.connect(self.multipointController.set_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self.multipointController.set_reflection_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self._update_apply_channel_offset_enable_state) + self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) self.checkbox_genAFMap.toggled.connect(self.multipointController.set_gen_focus_map_flag) self.checkbox_useFocusMap.toggled.connect(self.focusMapWidget.setEnabled) self.checkbox_useFocusMap.toggled.connect(self.multipointController.set_manual_focus_map_flag) @@ -7740,6 +7751,7 @@ def add_components(self): self.checkbox_withAutofocus.toggled.connect(self.save_multipoint_widget_config_to_cache) self.checkbox_withReflectionAutofocus.toggled.connect(self.save_multipoint_widget_config_to_cache) self._update_apply_channel_offset_enable_state(self.checkbox_withReflectionAutofocus.isChecked()) + self.focusMapWidget.set_reflection_af_available(self.checkbox_withReflectionAutofocus.isChecked()) def enable_manual_ROI(self): self._mosaic_layers_initialized = True @@ -8829,6 +8841,15 @@ def toggle_acquisition(self, pressed): if self.focusMapWidget.fit_surface(): # If fit successful, set the surface fitter in controller self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + if ( + self.checkbox_withReflectionAutofocus.isChecked() + and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() + ): + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.region_laser_af_offsets + ) + else: + self.multipointController.set_region_laser_af_offsets({}) else: QMessageBox.warning(self, "Warning", "Failed to fit focus surface") self.btn_startAcquisition.setChecked(False) @@ -8836,6 +8857,7 @@ def toggle_acquisition(self, pressed): else: # If checkbox not checked, set surface fitter to None self.multipointController.set_focus_map(None) + self.multipointController.set_region_laser_af_offsets({}) self.multipointController.set_deltaZ(self.entry_deltaZ.value()) self.multipointController.set_NZ(self.entry_NZ.value()) @@ -10464,6 +10486,14 @@ def setup_ui(self): self.by_region_checkbox = QCheckBox("Fit by Region") self.by_region_checkbox.setChecked(False) settings_layout.addWidget(self.by_region_checkbox) + self.checkbox_perRegionLaserAFOffset = QCheckBox("Per-region laser AF offset") + self.checkbox_perRegionLaserAFOffset.setChecked(False) + self.checkbox_perRegionLaserAFOffset.setEnabled(False) + self.checkbox_perRegionLaserAFOffset.setToolTip( + "With laser AF and focus map both on: capture each region's offset from the laser AF " + "reference at its focus point, and drive laser AF to that per-region target during acquisition." + ) + settings_layout.addWidget(self.checkbox_perRegionLaserAFOffset) self.layout.addLayout(settings_layout) # Status label - reserve space even when hidden @@ -10492,6 +10522,7 @@ def make_connections(self): # Connect fitting method change self.fit_method_combo.currentTextChanged.connect(self._match_by_region_box) + self.checkbox_perRegionLaserAFOffset.toggled.connect(self._on_per_region_offset_toggled) def update_point_list(self): """Update point selection combo showing grid coordinates for points""" @@ -10715,6 +10746,20 @@ def _capture_region_offset(self, region_id): ) self.region_laser_af_offsets[region_id] = offset_um + def _on_per_region_offset_toggled(self, checked): + self.capture_laser_af_offset_enabled = checked + if not checked: + self._clear_region_offsets() + + def set_reflection_af_available(self, available): + # The per-region offset only makes sense with laser AF on. Disable + uncheck (and + # via the toggle handler, clear) the checkbox when reflection AF is off — mirrors + # the per-channel offset checkbox behavior in the multipoint widgets. + self._reflection_af_available = bool(available) + self.checkbox_perRegionLaserAFOffset.setEnabled(self._reflection_af_available) + if not self._reflection_af_available and self.checkbox_perRegionLaserAFOffset.isChecked(): + self.checkbox_perRegionLaserAFOffset.setChecked(False) + def _clear_region_offsets(self): self.region_laser_af_offsets = {} From 10b247573ed955c10e746fedd26bd37c7b0a1a50 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:33:37 -0700 Subject: [PATCH 08/15] fix: set region_laser_af_offsets on perform_autofocus test stub The Task 1 change made perform_autofocus read self.region_laser_af_offsets; the pre-existing _af_stub in test_MultiPointWorker_offsets.py did not define it, so two perform_autofocus tests regressed. The real worker sets it in __init__; this fixes only the stale test stub. Co-Authored-By: Claude Opus 4.8 (1M context) --- software/tests/control/test_MultiPointWorker_offsets.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/software/tests/control/test_MultiPointWorker_offsets.py b/software/tests/control/test_MultiPointWorker_offsets.py index c0ad996ec..da604eff5 100644 --- a/software/tests/control/test_MultiPointWorker_offsets.py +++ b/software/tests/control/test_MultiPointWorker_offsets.py @@ -360,6 +360,9 @@ def _af_stub(*, move_to_target_result=True, move_to_target_raises=False): w.laser_auto_focus_controller.move_to_target.return_value = move_to_target_result w._laser_af_successes = 0 w._laser_af_failures = 0 + # perform_autofocus reads this per-region target map (empty -> target 0.0, the + # pre-existing behavior); the real worker sets it in __init__. + w.region_laser_af_offsets = {} w.perform_autofocus = MultiPointWorker.perform_autofocus.__get__(w) return w From ef783d9721998ec0e29d29dd38294d886456bfdb Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:37:29 -0700 Subject: [PATCH 09/15] test: cover FocusMapWidget reflection-AF availability and offset-toggle logic Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test_per_region_laser_af_offset.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index 3ac14a332..d8eb28800 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -78,11 +78,15 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= self.focus_points = focus_points if focus_points is not None else [] self.region_laser_af_offsets = offsets if offsets is not None else {} self.status_label = MagicMock() + self._reflection_af_available = False + self.checkbox_perRegionLaserAFOffset = MagicMock() _capture_region_offset = FocusMapWidget._capture_region_offset _clear_region_offsets = FocusMapWidget._clear_region_offsets _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points _on_laser_af_reference_changed = FocusMapWidget._on_laser_af_reference_changed + _on_per_region_offset_toggled = FocusMapWidget._on_per_region_offset_toggled + set_reflection_af_available = FocusMapWidget.set_reflection_af_available def test_capture_stores_displacement_when_enabled(): @@ -194,3 +198,52 @@ def test_csv_read_rejects_missing_required_columns(tmp_path): with pytest.raises(ValueError): dst._read_focus_points_csv(str(path)) + + +# --------------------------------------------------------------------------- +# _on_per_region_offset_toggled +# --------------------------------------------------------------------------- + + +def test_toggle_on_sets_capture_enabled(): + w = _FMStub(enabled=False) + w._on_per_region_offset_toggled(True) + assert w.capture_laser_af_offset_enabled is True + + +def test_toggle_off_sets_capture_disabled_and_clears_offsets(): + w = _FMStub(enabled=True, offsets={"A1": 1.0, "B2": 2.0}) + w._on_per_region_offset_toggled(False) + assert w.capture_laser_af_offset_enabled is False + assert w.region_laser_af_offsets == {} + + +# --------------------------------------------------------------------------- +# set_reflection_af_available +# --------------------------------------------------------------------------- + + +def test_set_reflection_af_unavailable_when_checkbox_checked(): + w = _FMStub() + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + w.set_reflection_af_available(False) + assert w._reflection_af_available is False + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_once_with(False) + w.checkbox_perRegionLaserAFOffset.setChecked.assert_called_once_with(False) + + +def test_set_reflection_af_unavailable_when_checkbox_unchecked_no_setchecked(): + w = _FMStub() + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w.set_reflection_af_available(False) + assert w._reflection_af_available is False + w.checkbox_perRegionLaserAFOffset.setChecked.assert_not_called() + + +def test_set_reflection_af_available_enables_checkbox_no_setchecked(): + w = _FMStub() + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w.set_reflection_af_available(True) + assert w._reflection_af_available is True + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_once_with(True) + w.checkbox_perRegionLaserAFOffset.setChecked.assert_not_called() From dfcde42e80bcb1b9577406bf715d088878cd449f Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 21:52:55 -0700 Subject: [PATCH 10/15] fix: scope per-region AF offset to constant focus map; clear stale offsets per run Co-Authored-By: Claude Opus 4.8 (1M context) --- .../control/core/multi_point_controller.py | 7 ++ software/control/widgets.py | 25 +++++-- .../test_per_region_laser_af_offset.py | 65 +++++++++++++++++-- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/software/control/core/multi_point_controller.py b/software/control/core/multi_point_controller.py index 01d822d22..6aa5b90df 100644 --- a/software/control/core/multi_point_controller.py +++ b/software/control/core/multi_point_controller.py @@ -848,6 +848,13 @@ def finish_fn(): acquisition_params = self.build_params(scan_position_information=scan_position_information) + # build_params has captured the per-region laser-AF offsets into + # acquisition_params for THIS run; clear the controller's copy now so a later + # acquisition from an entry point that never pushes offsets (e.g. fluidics or + # the control server) cannot inherit stale targets. Rebinding (not mutating) + # leaves the copy already handed to acquisition_params intact. + self.region_laser_af_offsets = {} + # Gather objective and camera info for YAML current_objective = self.objectiveStore.current_objective objective_dict = self.objectiveStore.objectives_dict.get(current_objective, {}) diff --git a/software/control/widgets.py b/software/control/widgets.py index a8879897e..46a93bd19 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -6152,6 +6152,7 @@ def setup_connections(self): self.checkbox_withAutofocus.toggled.connect(self.multipointController.set_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self.multipointController.set_reflection_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self._update_apply_channel_offset_enable_state) + # FocusMapWidget is shared by both multipoint tabs; enable-state is "last toggle wins" — harmless because each acquisition-start re-reads its own checkbox_withReflectionAutofocus.isChecked(). self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) self.checkbox_usePiezo.toggled.connect(self.multipointController.set_use_piezo) self.checkbox_skipSaving.toggled.connect(self.multipointController.set_skip_saving) @@ -7701,6 +7702,7 @@ def add_components(self): self.checkbox_withAutofocus.toggled.connect(self.multipointController.set_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self.multipointController.set_reflection_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self._update_apply_channel_offset_enable_state) + # FocusMapWidget is shared by both multipoint tabs; enable-state is "last toggle wins" — harmless because each acquisition-start re-reads its own checkbox_withReflectionAutofocus.isChecked(). self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) self.checkbox_genAFMap.toggled.connect(self.multipointController.set_gen_focus_map_flag) self.checkbox_useFocusMap.toggled.connect(self.focusMapWidget.setEnabled) @@ -10522,6 +10524,8 @@ def make_connections(self): # Connect fitting method change self.fit_method_combo.currentTextChanged.connect(self._match_by_region_box) + self.fit_method_combo.currentTextChanged.connect(self._update_per_region_offset_enabled) + self.by_region_checkbox.toggled.connect(self._update_per_region_offset_enabled) self.checkbox_perRegionLaserAFOffset.toggled.connect(self._on_per_region_offset_toggled) def update_point_list(self): @@ -10752,12 +10756,23 @@ def _on_per_region_offset_toggled(self, checked): self._clear_region_offsets() def set_reflection_af_available(self, available): - # The per-region offset only makes sense with laser AF on. Disable + uncheck (and - # via the toggle handler, clear) the checkbox when reflection AF is off — mirrors - # the per-channel offset checkbox behavior in the multipoint widgets. self._reflection_af_available = bool(available) - self.checkbox_perRegionLaserAFOffset.setEnabled(self._reflection_af_available) - if not self._reflection_af_available and self.checkbox_perRegionLaserAFOffset.isChecked(): + self._update_per_region_offset_enabled() + + def _update_per_region_offset_enabled(self, *args): + # The per-region laser-AF offset is only valid for the constant, one-point-per-well + # focus map (method="constant" + Fit by Region) while laser AF is active. A + # non-constant surface has many points per region, so a single per-region offset + # cannot represent it — keep the checkbox unavailable there. Disabling also unchecks + # (which clears captured offsets via the toggle handler) so a stale opt-in cannot + # survive a mode switch. (*args absorbs the value emitted by the combo/checkbox signals.) + enabled = ( + self._reflection_af_available + and self.fit_method_combo.currentText() == "constant" + and self.by_region_checkbox.isChecked() + ) + self.checkbox_perRegionLaserAFOffset.setEnabled(enabled) + if not enabled and self.checkbox_perRegionLaserAFOffset.isChecked(): self.checkbox_perRegionLaserAFOffset.setChecked(False) def _clear_region_offsets(self): diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index d8eb28800..eabca23a7 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -4,7 +4,7 @@ call the real methods in isolation, mirroring tests/control/test_MultiPointWorker_offsets.py. """ -import math +import pytest from dataclasses import fields from unittest.mock import MagicMock @@ -80,6 +80,10 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= self.status_label = MagicMock() self._reflection_af_available = False self.checkbox_perRegionLaserAFOffset = MagicMock() + self.fit_method_combo = MagicMock() + self.fit_method_combo.currentText.return_value = "constant" + self.by_region_checkbox = MagicMock() + self.by_region_checkbox.isChecked.return_value = True _capture_region_offset = FocusMapWidget._capture_region_offset _clear_region_offsets = FocusMapWidget._clear_region_offsets @@ -87,6 +91,7 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= _on_laser_af_reference_changed = FocusMapWidget._on_laser_af_reference_changed _on_per_region_offset_toggled = FocusMapWidget._on_per_region_offset_toggled set_reflection_af_available = FocusMapWidget.set_reflection_af_available + _update_per_region_offset_enabled = FocusMapWidget._update_per_region_offset_enabled def test_capture_stores_displacement_when_enabled(): @@ -194,7 +199,6 @@ def test_csv_read_rejects_missing_required_columns(tmp_path): path.write_text("Region_ID,X_mm\nA1,1.0\n") dst = _FMStub() dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) - import pytest with pytest.raises(ValueError): dst._read_focus_points_csv(str(path)) @@ -219,7 +223,7 @@ def test_toggle_off_sets_capture_disabled_and_clears_offsets(): # --------------------------------------------------------------------------- -# set_reflection_af_available +# set_reflection_af_available / _update_per_region_offset_enabled # --------------------------------------------------------------------------- @@ -228,7 +232,7 @@ def test_set_reflection_af_unavailable_when_checkbox_checked(): w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True w.set_reflection_af_available(False) assert w._reflection_af_available is False - w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_once_with(False) + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) w.checkbox_perRegionLaserAFOffset.setChecked.assert_called_once_with(False) @@ -241,9 +245,60 @@ def test_set_reflection_af_unavailable_when_checkbox_unchecked_no_setchecked(): def test_set_reflection_af_available_enables_checkbox_no_setchecked(): + # With method="constant" and by_region=True (stub defaults), reflection AF on → enabled. w = _FMStub() w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False w.set_reflection_af_available(True) assert w._reflection_af_available is True - w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_once_with(True) + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(True) w.checkbox_perRegionLaserAFOffset.setChecked.assert_not_called() + + +# --------------------------------------------------------------------------- +# constant-mode gating (Fix B) +# --------------------------------------------------------------------------- + + +def test_update_per_region_offset_disabled_when_method_is_spline(): + """setEnabled(False) even if reflection AF is on, when method != constant.""" + w = _FMStub() + w._reflection_af_available = True + w.fit_method_combo.currentText.return_value = "spline" + w.by_region_checkbox.isChecked.return_value = True + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) + + +def test_update_per_region_offset_disabled_when_by_region_unchecked(): + """setEnabled(False) when by_region is False, even with method=constant and AF on.""" + w = _FMStub() + w._reflection_af_available = True + w.fit_method_combo.currentText.return_value = "constant" + w.by_region_checkbox.isChecked.return_value = False + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) + + +def test_update_per_region_offset_enabled_when_all_conditions_met(): + """setEnabled(True) only when all three conditions hold.""" + w = _FMStub() + w._reflection_af_available = True + w.fit_method_combo.currentText.return_value = "constant" + w.by_region_checkbox.isChecked.return_value = True + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(True) + + +def test_update_per_region_offset_unchecks_when_conditions_not_met_and_was_checked(): + """When conditions fail and checkbox was checked, setChecked(False) must be called.""" + w = _FMStub() + w._reflection_af_available = True + w.fit_method_combo.currentText.return_value = "rbf" + w.by_region_checkbox.isChecked.return_value = True + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) + w.checkbox_perRegionLaserAFOffset.setChecked.assert_called_once_with(False) From 2511938df3917fc50c88e9d29d656d8099d55b47 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 22:21:47 -0700 Subject: [PATCH 11/15] refactor: simplify per-region laser-AF offset code - guard offset capture with isfinite (matches capture_current_z_offset) instead of isnan - extract FocusMapWidget.get_offsets_for_acquisition() to DRY the gating across both multipoint widgets (one source of truth for the reflection-AF + checkbox condition) - _sync_offsets_to_focus_points via dict comprehension; dict(offsets or {}) in the setter - only log the per-FOV laser-AF target when non-zero (no new noise on the default path) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../control/core/multi_point_controller.py | 2 +- software/control/core/multi_point_worker.py | 3 +- software/control/widgets.py | 43 +++++++++++-------- .../test_per_region_laser_af_offset.py | 21 +++++++++ 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/software/control/core/multi_point_controller.py b/software/control/core/multi_point_controller.py index 6aa5b90df..50ec99f55 100644 --- a/software/control/core/multi_point_controller.py +++ b/software/control/core/multi_point_controller.py @@ -419,7 +419,7 @@ def set_focus_map(self, focusMap): def set_region_laser_af_offsets(self, offsets): # region_id -> µm offset from the global laser-AF reference plane. Empty dict means # every FOV targets the reference (displacement 0), i.e. current behavior. - self.region_laser_af_offsets = dict(offsets) if offsets else {} + self.region_laser_af_offsets = dict(offsets or {}) def set_base_path(self, path): self.base_path = path diff --git a/software/control/core/multi_point_worker.py b/software/control/core/multi_point_worker.py index 7d197d026..4259e9901 100644 --- a/software/control/core/multi_point_worker.py +++ b/software/control/core/multi_point_worker.py @@ -1157,7 +1157,8 @@ def perform_autofocus(self, region_id, fov): # the per-channel z-offset gate would apply offsets from an unanchored z. try: target_um = self.region_laser_af_offsets.get(region_id, 0.0) - self._log.info(f"laser AF target for region '{region_id}': {target_um:.2f} µm") + if target_um: + self._log.info(f"laser AF target for region '{region_id}': {target_um:.2f} µm") af_succeeded = self.laser_auto_focus_controller.move_to_target(target_um) except Exception as e: file_ID = f"{region_id}_focus_camera.bmp" diff --git a/software/control/widgets.py b/software/control/widgets.py index 46a93bd19..700a6e409 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -6480,13 +6480,9 @@ def toggle_acquisition(self, pressed): if self.checkbox_useFocusMap.isChecked(): self.focusMapWidget.fit_surface() self.multipointController.set_focus_map(self.focusMapWidget.focusMap) - if ( - self.checkbox_withReflectionAutofocus.isChecked() - and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() - ): - self.multipointController.set_region_laser_af_offsets(self.focusMapWidget.region_laser_af_offsets) - else: - self.multipointController.set_region_laser_af_offsets({}) + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.get_offsets_for_acquisition(self.checkbox_withReflectionAutofocus.isChecked()) + ) else: self.multipointController.set_focus_map(None) self.multipointController.set_region_laser_af_offsets({}) @@ -8843,15 +8839,11 @@ def toggle_acquisition(self, pressed): if self.focusMapWidget.fit_surface(): # If fit successful, set the surface fitter in controller self.multipointController.set_focus_map(self.focusMapWidget.focusMap) - if ( - self.checkbox_withReflectionAutofocus.isChecked() - and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() - ): - self.multipointController.set_region_laser_af_offsets( - self.focusMapWidget.region_laser_af_offsets + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.get_offsets_for_acquisition( + self.checkbox_withReflectionAutofocus.isChecked() ) - else: - self.multipointController.set_region_laser_af_offsets({}) + ) else: QMessageBox.warning(self, "Warning", "Failed to fit focus surface") self.btn_startAcquisition.setChecked(False) @@ -10739,7 +10731,9 @@ def _capture_region_offset(self, region_id): self.status_label.setText("Laser AF reference not set — per-region offset not captured") return offset_um = self.laserAutofocusController.measure_displacement() - if math.isnan(offset_um): + # measure_displacement() returns float('nan') on a soft failure; guard with isfinite + # (matching capture_current_z_offset) so an invalid reading is never stored. + if offset_um is None or not math.isfinite(offset_um): self.status_label.setText(f"Laser AF spot not detected — offset not captured for {region_id}") return laser_af_range = self.laserAutofocusController.laser_af_properties.laser_af_range @@ -10781,9 +10775,20 @@ def _clear_region_offsets(self): def _sync_offsets_to_focus_points(self): """Drop offsets for regions that no longer have any focus point.""" live_regions = {rid for rid, _, _, _ in self.focus_points} - for rid in list(self.region_laser_af_offsets.keys()): - if rid not in live_regions: - self.region_laser_af_offsets.pop(rid, None) + self.region_laser_af_offsets = { + rid: offset for rid, offset in self.region_laser_af_offsets.items() if rid in live_regions + } + + def get_offsets_for_acquisition(self, reflection_af_active): + """Per-region laser-AF offsets to apply for an acquisition, or {} when the combined + mode is inactive (reflection AF off, or the per-region checkbox unchecked). + + Centralizes the reflection-AF/checkbox gating so both multipoint widgets — and any + future acquisition entry point — share one source of truth. + """ + if reflection_af_active and self.checkbox_perRegionLaserAFOffset.isChecked(): + return dict(self.region_laser_af_offsets) + return {} def _on_laser_af_reference_changed(self, has_reference): # The reference plane moved; previously-captured offsets are relative to the old diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index eabca23a7..4f2e830e4 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -92,6 +92,7 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= _on_per_region_offset_toggled = FocusMapWidget._on_per_region_offset_toggled set_reflection_af_available = FocusMapWidget.set_reflection_af_available _update_per_region_offset_enabled = FocusMapWidget._update_per_region_offset_enabled + get_offsets_for_acquisition = FocusMapWidget.get_offsets_for_acquisition def test_capture_stores_displacement_when_enabled(): @@ -302,3 +303,23 @@ def test_update_per_region_offset_unchecks_when_conditions_not_met_and_was_check w._update_per_region_offset_enabled() w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) w.checkbox_perRegionLaserAFOffset.setChecked.assert_called_once_with(False) + + +def test_get_offsets_for_acquisition_returns_offsets_when_active(): + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + result = w.get_offsets_for_acquisition(reflection_af_active=True) + assert result == {"A1": 3.0} + assert result is not w.region_laser_af_offsets # returns a copy, not the live dict + + +def test_get_offsets_for_acquisition_empty_when_reflection_af_off(): + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + assert w.get_offsets_for_acquisition(reflection_af_active=False) == {} + + +def test_get_offsets_for_acquisition_empty_when_checkbox_off(): + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + assert w.get_offsets_for_acquisition(reflection_af_active=True) == {} From 686dea3518f7988a6f1e8e35defa3f4a799e58df Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 22:32:03 -0700 Subject: [PATCH 12/15] fix: decouple per-region AF offset checkbox from per-tab Reflection AF (PR #562 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review: the FocusMapWidget is shared by all three multipoint tabs, so wiring each tab's Reflection AF checkbox into a shared set_reflection_af_available was 'last toggle wins' — an inactive tab could disable/uncheck the per-region checkbox (clearing captured offsets) and the acquisition gate reads the shared checkbox, silently dropping the feature. - Enable the per-region checkbox purely on the shared focus-map controls (constant + Fit by Region); remove set_reflection_af_available and its per-tab wiring. The 'requires Reflection AF' rule is now enforced only at acquisition by get_offsets_for_acquisition, using the RUNNING tab's own checkbox. - Stop clearing captured offsets on uncheck (data-loss vector); offsets are cleared only on reference change or focus-point edits, and the gate returns {} while unchecked, so retaining them is safe. - Update tests accordingly. Also add docs/per-region-laser-af-offset.md (user instructions). Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/widgets.py | 37 +++++------ software/docs/per-region-laser-af-offset.md | 61 +++++++++++++++++++ .../test_per_region_laser_af_offset.py | 57 ++++------------- 3 files changed, 86 insertions(+), 69 deletions(-) create mode 100644 software/docs/per-region-laser-af-offset.md diff --git a/software/control/widgets.py b/software/control/widgets.py index 700a6e409..83461c2b8 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -6152,8 +6152,6 @@ def setup_connections(self): self.checkbox_withAutofocus.toggled.connect(self.multipointController.set_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self.multipointController.set_reflection_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self._update_apply_channel_offset_enable_state) - # FocusMapWidget is shared by both multipoint tabs; enable-state is "last toggle wins" — harmless because each acquisition-start re-reads its own checkbox_withReflectionAutofocus.isChecked(). - self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) self.checkbox_usePiezo.toggled.connect(self.multipointController.set_use_piezo) self.checkbox_skipSaving.toggled.connect(self.multipointController.set_skip_saving) self.btn_setSavingDir.clicked.connect(self.set_saving_dir) @@ -6188,7 +6186,6 @@ def setup_connections(self): self.toggle_z_range_controls(False) self.multipointController.set_use_piezo(self.checkbox_usePiezo.isChecked()) self._update_apply_channel_offset_enable_state(self.checkbox_withReflectionAutofocus.isChecked()) - self.focusMapWidget.set_reflection_af_available(self.checkbox_withReflectionAutofocus.isChecked()) def setup_layout(self): self.grid = QVBoxLayout() @@ -7698,8 +7695,6 @@ def add_components(self): self.checkbox_withAutofocus.toggled.connect(self.multipointController.set_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self.multipointController.set_reflection_af_flag) self.checkbox_withReflectionAutofocus.toggled.connect(self._update_apply_channel_offset_enable_state) - # FocusMapWidget is shared by both multipoint tabs; enable-state is "last toggle wins" — harmless because each acquisition-start re-reads its own checkbox_withReflectionAutofocus.isChecked(). - self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) self.checkbox_genAFMap.toggled.connect(self.multipointController.set_gen_focus_map_flag) self.checkbox_useFocusMap.toggled.connect(self.focusMapWidget.setEnabled) self.checkbox_useFocusMap.toggled.connect(self.multipointController.set_manual_focus_map_flag) @@ -7749,7 +7744,6 @@ def add_components(self): self.checkbox_withAutofocus.toggled.connect(self.save_multipoint_widget_config_to_cache) self.checkbox_withReflectionAutofocus.toggled.connect(self.save_multipoint_widget_config_to_cache) self._update_apply_channel_offset_enable_state(self.checkbox_withReflectionAutofocus.isChecked()) - self.focusMapWidget.set_reflection_af_available(self.checkbox_withReflectionAutofocus.isChecked()) def enable_manual_ROI(self): self._mosaic_layers_initialized = True @@ -10405,7 +10399,6 @@ def __init__( # region_id. Captured at each focus point when the combined mode is enabled. self.region_laser_af_offsets = {} self.capture_laser_af_offset_enabled = False - self._reflection_af_available = False self.setup_ui() self.make_connections() @@ -10745,26 +10738,24 @@ def _capture_region_offset(self, region_id): self.region_laser_af_offsets[region_id] = offset_um def _on_per_region_offset_toggled(self, checked): + # Toggling only gates whether NEW captures happen; it deliberately does not clear + # already-captured offsets. Those are cleared only when the laser-AF reference + # changes or the focus points change. get_offsets_for_acquisition returns {} while + # unchecked, so retaining them here is safe and avoids losing data on an incidental + # uncheck (e.g. switching focus-map method). self.capture_laser_af_offset_enabled = checked - if not checked: - self._clear_region_offsets() - - def set_reflection_af_available(self, available): - self._reflection_af_available = bool(available) - self._update_per_region_offset_enabled() def _update_per_region_offset_enabled(self, *args): # The per-region laser-AF offset is only valid for the constant, one-point-per-well - # focus map (method="constant" + Fit by Region) while laser AF is active. A - # non-constant surface has many points per region, so a single per-region offset - # cannot represent it — keep the checkbox unavailable there. Disabling also unchecks - # (which clears captured offsets via the toggle handler) so a stale opt-in cannot - # survive a mode switch. (*args absorbs the value emitted by the combo/checkbox signals.) - enabled = ( - self._reflection_af_available - and self.fit_method_combo.currentText() == "constant" - and self.by_region_checkbox.isChecked() - ) + # focus map (method="constant" + Fit by Region): a non-constant surface has many + # points per region, so a single per-region offset cannot represent it. Enable is + # driven ONLY by these two SHARED focus-map controls — deliberately NOT by any + # multipoint tab's Reflection AF checkbox, because this one widget is shared across + # all tabs and a per-tab toggle would clobber the others ("last toggle wins"). + # Whether laser AF is actually active is enforced at acquisition by + # get_offsets_for_acquisition, using the running tab's own checkbox. + # (*args absorbs the value emitted by the combo/checkbox signals.) + enabled = self.fit_method_combo.currentText() == "constant" and self.by_region_checkbox.isChecked() self.checkbox_perRegionLaserAFOffset.setEnabled(enabled) if not enabled and self.checkbox_perRegionLaserAFOffset.isChecked(): self.checkbox_perRegionLaserAFOffset.setChecked(False) diff --git a/software/docs/per-region-laser-af-offset.md b/software/docs/per-region-laser-af-offset.md new file mode 100644 index 000000000..447219ad9 --- /dev/null +++ b/software/docs/per-region-laser-af-offset.md @@ -0,0 +1,61 @@ +# Per-region laser-AF offset + +Focus each well/region at its **own height relative to the laser-AF reference plane**, +while laser autofocus keeps that height stable against drift over a long acquisition. + +Normally, when laser reflection autofocus (laser AF) is on, every field of view is driven +back to the **single** laser-AF reference plane. This feature instead lets each region +keep the focus offset you recorded for it — useful when different wells need slightly +different focus relative to the reference (e.g. sample-height differences across a plate). + +## When it applies + +The feature is active only when **all** of these are true: + +- Laser AF (**Reflection AF**) is enabled for the acquisition. +- A **focus map** is used, set to **`constant`** method with **`Fit by Region`** checked + (i.e. one focus point per well/region — the "constant-z" case). +- The **`Per-region laser AF offset`** checkbox (in the Focus Map tab) is checked. + +If any of these is off, behavior is unchanged: laser AF drives every FOV to the single +reference plane. + +## How to use it + +1. **Set the laser-AF reference** once (the usual "Set Reference" step), with the sample at + a representative focus. All per-region offsets are measured relative to this plane. +2. Open the **Focus Map** tab. Set **Fitting Method** to `constant` and check **Fit by + Region**. The **`Per-region laser AF offset`** checkbox now becomes available — check it. +3. For **each well/region**, add one focus point: + - Navigate to the region and bring it into the focus you want. + - Click **Add** (or select the region's point and click **Update Z**). + - The current laser-AF displacement is recorded as that region's offset. The status line + reports problems (no reference set, spot not detected, or an offset larger than the + laser-AF range — that region may fail AF during the run). +4. In the multipoint panel, enable **Reflection AF** and **Use Focus Map**. +5. Start the acquisition. At each FOV, laser AF drives the stage to that region's recorded + offset instead of to the shared reference plane. + +## Good to know + +- **Re-setting the laser-AF reference clears all captured offsets** — they were measured + against the old reference and are no longer valid. Re-record them after changing the + reference. +- **Changing focus points keeps offsets in sync**: removing a region's point drops its + offset; regenerating the focus grid clears them. +- **Save / reuse**: Export the focus points to CSV — the file includes an `Offset_um` + column. Importing restores both the points and their offsets. Older CSV files without + that column still import (with no offsets). +- **Per-channel Z-offsets still apply** on top of the per-region offset, exactly as before. +- The offset value is the laser-AF displacement (µm) from the reference plane at the focus + point, so focus tracks the reference as the sample drifts — even across time-lapse + timepoints. + +## Troubleshooting + +- **The `Per-region laser AF offset` checkbox is greyed out.** It only enables for a + `constant` + `Fit by Region` focus map. Pick those in the Focus Map tab. +- **Offsets don't seem to apply during the run.** Confirm **Reflection AF** is enabled in + the multipoint panel you started the run from, and that the checkbox is still checked. +- **A region keeps failing autofocus.** Its offset may exceed the laser-AF range; re-record + it closer to the reference plane, or move the reference. diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index 4f2e830e4..f84aec9f8 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -78,7 +78,6 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= self.focus_points = focus_points if focus_points is not None else [] self.region_laser_af_offsets = offsets if offsets is not None else {} self.status_label = MagicMock() - self._reflection_af_available = False self.checkbox_perRegionLaserAFOffset = MagicMock() self.fit_method_combo = MagicMock() self.fit_method_combo.currentText.return_value = "constant" @@ -90,7 +89,6 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points _on_laser_af_reference_changed = FocusMapWidget._on_laser_af_reference_changed _on_per_region_offset_toggled = FocusMapWidget._on_per_region_offset_toggled - set_reflection_af_available = FocusMapWidget.set_reflection_af_available _update_per_region_offset_enabled = FocusMapWidget._update_per_region_offset_enabled get_offsets_for_acquisition = FocusMapWidget.get_offsets_for_acquisition @@ -216,54 +214,26 @@ def test_toggle_on_sets_capture_enabled(): assert w.capture_laser_af_offset_enabled is True -def test_toggle_off_sets_capture_disabled_and_clears_offsets(): +def test_toggle_off_disables_capture_but_retains_offsets(): + # Unchecking only stops new captures; it must NOT discard already-captured offsets + # (those are cleared only on reference change or focus-point edits). The acquisition + # gate returns {} while unchecked, so retaining them is safe and avoids data loss when + # an unrelated state change (e.g. another tab) incidentally unchecks the box. w = _FMStub(enabled=True, offsets={"A1": 1.0, "B2": 2.0}) w._on_per_region_offset_toggled(False) assert w.capture_laser_af_offset_enabled is False - assert w.region_laser_af_offsets == {} - - -# --------------------------------------------------------------------------- -# set_reflection_af_available / _update_per_region_offset_enabled -# --------------------------------------------------------------------------- - - -def test_set_reflection_af_unavailable_when_checkbox_checked(): - w = _FMStub() - w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True - w.set_reflection_af_available(False) - assert w._reflection_af_available is False - w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) - w.checkbox_perRegionLaserAFOffset.setChecked.assert_called_once_with(False) - - -def test_set_reflection_af_unavailable_when_checkbox_unchecked_no_setchecked(): - w = _FMStub() - w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False - w.set_reflection_af_available(False) - assert w._reflection_af_available is False - w.checkbox_perRegionLaserAFOffset.setChecked.assert_not_called() - - -def test_set_reflection_af_available_enables_checkbox_no_setchecked(): - # With method="constant" and by_region=True (stub defaults), reflection AF on → enabled. - w = _FMStub() - w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False - w.set_reflection_af_available(True) - assert w._reflection_af_available is True - w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(True) - w.checkbox_perRegionLaserAFOffset.setChecked.assert_not_called() + assert w.region_laser_af_offsets == {"A1": 1.0, "B2": 2.0} # --------------------------------------------------------------------------- -# constant-mode gating (Fix B) +# _update_per_region_offset_enabled — gated ONLY on the shared focus-map controls +# (constant + Fit by Region), NOT on any tab's Reflection AF checkbox, so a tab +# toggle cannot clobber the shared widget. Reflection AF is enforced at acquisition. # --------------------------------------------------------------------------- def test_update_per_region_offset_disabled_when_method_is_spline(): - """setEnabled(False) even if reflection AF is on, when method != constant.""" w = _FMStub() - w._reflection_af_available = True w.fit_method_combo.currentText.return_value = "spline" w.by_region_checkbox.isChecked.return_value = True w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False @@ -272,9 +242,7 @@ def test_update_per_region_offset_disabled_when_method_is_spline(): def test_update_per_region_offset_disabled_when_by_region_unchecked(): - """setEnabled(False) when by_region is False, even with method=constant and AF on.""" w = _FMStub() - w._reflection_af_available = True w.fit_method_combo.currentText.return_value = "constant" w.by_region_checkbox.isChecked.return_value = False w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False @@ -282,10 +250,8 @@ def test_update_per_region_offset_disabled_when_by_region_unchecked(): w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) -def test_update_per_region_offset_enabled_when_all_conditions_met(): - """setEnabled(True) only when all three conditions hold.""" +def test_update_per_region_offset_enabled_when_constant_and_by_region(): w = _FMStub() - w._reflection_af_available = True w.fit_method_combo.currentText.return_value = "constant" w.by_region_checkbox.isChecked.return_value = True w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False @@ -294,9 +260,8 @@ def test_update_per_region_offset_enabled_when_all_conditions_met(): def test_update_per_region_offset_unchecks_when_conditions_not_met_and_was_checked(): - """When conditions fail and checkbox was checked, setChecked(False) must be called.""" + """When conditions fail and the checkbox was checked, it must be unchecked.""" w = _FMStub() - w._reflection_af_available = True w.fit_method_combo.currentText.return_value = "rbf" w.by_region_checkbox.isChecked.return_value = True w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True From 172d30552d78a74cfff8058e793154b8b10620a3 Mon Sep 17 00:00:00 2001 From: You Yan Date: Fri, 19 Jun 2026 06:10:28 -0700 Subject: [PATCH 13/15] feat: rename checkbox to 'Laser AF Offset' and show captured offset on status line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Focus Map checkbox label 'Per-region laser AF offset' -> 'Laser AF Offset' (internal symbols unchanged). - On a successful in-range capture, the focus-map status line now shows the recorded offset (e.g. 'Region A1: Laser AF offset +2.30 µm') when Add/Update Z runs. - Doc + test updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/widgets.py | 4 +++- software/docs/per-region-laser-af-offset.md | 13 +++++++------ .../control/test_per_region_laser_af_offset.py | 8 ++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index 83461c2b8..34643e159 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -10473,7 +10473,7 @@ def setup_ui(self): self.by_region_checkbox = QCheckBox("Fit by Region") self.by_region_checkbox.setChecked(False) settings_layout.addWidget(self.by_region_checkbox) - self.checkbox_perRegionLaserAFOffset = QCheckBox("Per-region laser AF offset") + self.checkbox_perRegionLaserAFOffset = QCheckBox("Laser AF Offset") self.checkbox_perRegionLaserAFOffset.setChecked(False) self.checkbox_perRegionLaserAFOffset.setEnabled(False) self.checkbox_perRegionLaserAFOffset.setToolTip( @@ -10735,6 +10735,8 @@ def _capture_region_offset(self, region_id): f"Warning: region {region_id} offset {offset_um:.1f} µm exceeds laser AF range " f"({laser_af_range:.1f} µm); it may fail AF during acquisition" ) + else: + self.status_label.setText(f"Region {region_id}: Laser AF offset {offset_um:+.2f} µm") self.region_laser_af_offsets[region_id] = offset_um def _on_per_region_offset_toggled(self, checked): diff --git a/software/docs/per-region-laser-af-offset.md b/software/docs/per-region-laser-af-offset.md index 447219ad9..2011f0b50 100644 --- a/software/docs/per-region-laser-af-offset.md +++ b/software/docs/per-region-laser-af-offset.md @@ -15,7 +15,7 @@ The feature is active only when **all** of these are true: - Laser AF (**Reflection AF**) is enabled for the acquisition. - A **focus map** is used, set to **`constant`** method with **`Fit by Region`** checked (i.e. one focus point per well/region — the "constant-z" case). -- The **`Per-region laser AF offset`** checkbox (in the Focus Map tab) is checked. +- The **`Laser AF Offset`** checkbox (in the Focus Map tab) is checked. If any of these is off, behavior is unchanged: laser AF drives every FOV to the single reference plane. @@ -25,13 +25,14 @@ reference plane. 1. **Set the laser-AF reference** once (the usual "Set Reference" step), with the sample at a representative focus. All per-region offsets are measured relative to this plane. 2. Open the **Focus Map** tab. Set **Fitting Method** to `constant` and check **Fit by - Region**. The **`Per-region laser AF offset`** checkbox now becomes available — check it. + Region**. The **`Laser AF Offset`** checkbox now becomes available — check it. 3. For **each well/region**, add one focus point: - Navigate to the region and bring it into the focus you want. - Click **Add** (or select the region's point and click **Update Z**). - - The current laser-AF displacement is recorded as that region's offset. The status line - reports problems (no reference set, spot not detected, or an offset larger than the - laser-AF range — that region may fail AF during the run). + - The current laser-AF displacement is recorded as that region's offset and shown on the + status line (e.g. `Region A1: Laser AF offset +2.30 µm`). Problems are reported there + too: no reference set, spot not detected, or an offset larger than the laser-AF range + (that region may fail AF during the run). 4. In the multipoint panel, enable **Reflection AF** and **Use Focus Map**. 5. Start the acquisition. At each FOV, laser AF drives the stage to that region's recorded offset instead of to the shared reference plane. @@ -53,7 +54,7 @@ reference plane. ## Troubleshooting -- **The `Per-region laser AF offset` checkbox is greyed out.** It only enables for a +- **The `Laser AF Offset` checkbox is greyed out.** It only enables for a `constant` + `Fit by Region` focus map. Pick those in the Focus Map tab. - **Offsets don't seem to apply during the run.** Confirm **Reflection AF** is enabled in the multipoint panel you started the run from, and that the checkbox is still checked. diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index f84aec9f8..5d03aaed6 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -99,6 +99,14 @@ def test_capture_stores_displacement_when_enabled(): assert w.region_laser_af_offsets == {"A1": 3.5} +def test_capture_shows_offset_on_status_line(): + # On a successful in-range capture, the focus-map status line reports the offset. + w = _FMStub(controller=_laser_controller(2.3)) + w._capture_region_offset("A1") + msg = w.status_label.setText.call_args[0][0] + assert "A1" in msg and "2.3" in msg + + def test_capture_noop_when_mode_disabled(): ctrl = _laser_controller(3.5) w = _FMStub(enabled=False, controller=ctrl) From 3c8fd2546227971a86b3294a474a3518ddb7d2f3 Mon Sep 17 00:00:00 2001 From: You Yan Date: Fri, 19 Jun 2026 12:55:04 -0700 Subject: [PATCH 14/15] fix: suspend main live during Update-Z laser-AF offset capture Capturing a per-region offset calls measure_displacement(), which toggles the AF laser over the microcontroller serial link and waits; a running main-camera live stream queues triggers on the same link, contends, times out, and returns NaN ("spot not detected"). Mirror LiveControlWidget.capture_current_z_offset: stop the main live around the measurement and restart it after (no signal, so the user is not yanked to the Live tab). FocusMapWidget now receives the main liveController. Co-Authored-By: Claude Opus 4.8 (1M context) --- software/control/gui_hcs.py | 7 +++++- software/control/widgets.py | 25 +++++++++++++++++-- software/docs/per-region-laser-af-offset.md | 7 +++--- .../test_per_region_laser_af_offset.py | 21 +++++++++++++++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index e1578949e..b443520fe 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -974,7 +974,12 @@ def load_widgets(self): self.wellSelectionWidget = widgets.Well1536SelectionWidget(self.wellplateFormatWidget) self.scanCoordinates.add_well_selector(self.wellSelectionWidget) self.focusMapWidget = widgets.FocusMapWidget( - self.stage, self.navigationViewer, self.scanCoordinates, core.FocusMap(), self.laserAutofocusController + self.stage, + self.navigationViewer, + self.scanCoordinates, + core.FocusMap(), + self.laserAutofocusController, + self.liveController, ) if SUPPORT_LASER_AUTOFOCUS: diff --git a/software/control/widgets.py b/software/control/widgets.py index 34643e159..c0519962d 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -10379,7 +10379,13 @@ class FocusMapWidget(QFrame): """Widget for managing focus map points and surface fitting""" def __init__( - self, stage: AbstractStage, navigationViewer, scanCoordinates, focusMap, laserAutofocusController=None + self, + stage: AbstractStage, + navigationViewer, + scanCoordinates, + focusMap, + laserAutofocusController=None, + liveController=None, ): super().__init__() self.setFrameStyle(QFrame.Panel | QFrame.Raised) @@ -10391,6 +10397,9 @@ def __init__( self.scanCoordinates = scanCoordinates self.focusMap = focusMap self.laserAutofocusController = laserAutofocusController + # Main imaging live controller — suspended around laser-AF offset captures (see + # _capture_region_offset). May be None when laser AF is unsupported. + self.liveController = liveController # Store focus points in widget self.focus_points = [] # list of (region_id, x, y, z) tuples @@ -10723,7 +10732,19 @@ def _capture_region_offset(self, region_id): if not self.laserAutofocusController.laser_af_properties.has_reference: self.status_label.setText("Laser AF reference not set — per-region offset not captured") return - offset_um = self.laserAutofocusController.measure_displacement() + # Suspend the main camera's live stream during the measurement: laser AF toggles the + # AF laser via the microcontroller and waits for completion, while live keeps queuing + # triggers on the same serial link; the contention can time out and return NaN. Mirror + # LiveControlWidget.capture_current_z_offset. start_live() alone (no signal) resumes the + # stream without yanking the user to the Live tab. + was_live = self.liveController is not None and self.liveController.is_live + try: + if was_live: + self.liveController.stop_live() + offset_um = self.laserAutofocusController.measure_displacement() + finally: + if was_live: + self.liveController.start_live() # measure_displacement() returns float('nan') on a soft failure; guard with isfinite # (matching capture_current_z_offset) so an invalid reading is never stored. if offset_um is None or not math.isfinite(offset_um): diff --git a/software/docs/per-region-laser-af-offset.md b/software/docs/per-region-laser-af-offset.md index 2011f0b50..03596ba1e 100644 --- a/software/docs/per-region-laser-af-offset.md +++ b/software/docs/per-region-laser-af-offset.md @@ -30,9 +30,10 @@ reference plane. - Navigate to the region and bring it into the focus you want. - Click **Add** (or select the region's point and click **Update Z**). - The current laser-AF displacement is recorded as that region's offset and shown on the - status line (e.g. `Region A1: Laser AF offset +2.30 µm`). Problems are reported there - too: no reference set, spot not detected, or an offset larger than the laser-AF range - (that region may fail AF during the run). + status line (e.g. `Region A1: Laser AF offset +2.30 µm`). The live view briefly pauses + while the reading is taken, then resumes. Problems are reported on the status line too: + no reference set, spot not detected, or an offset larger than the laser-AF range (that + region may fail AF during the run). 4. In the multipoint panel, enable **Reflection AF** and **Use Focus Map**. 5. Start the acquisition. At each FOV, laser AF drives the stage to that region's recorded offset instead of to the shared reference plane. diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index 5d03aaed6..23b29f6c2 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -72,12 +72,14 @@ def _laser_controller(displacement, has_reference=True, laser_af_range=200.0): class _FMStub: """FocusMapWidget-ish object exposing just what the capture/persistence helpers read.""" - def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets=None): + def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets=None, live=False): self.capture_laser_af_offset_enabled = enabled self.laserAutofocusController = controller self.focus_points = focus_points if focus_points is not None else [] self.region_laser_af_offsets = offsets if offsets is not None else {} self.status_label = MagicMock() + self.liveController = MagicMock() + self.liveController.is_live = live self.checkbox_perRegionLaserAFOffset = MagicMock() self.fit_method_combo = MagicMock() self.fit_method_combo.currentText.return_value = "constant" @@ -107,6 +109,23 @@ def test_capture_shows_offset_on_status_line(): assert "A1" in msg and "2.3" in msg +def test_capture_suspends_main_live_during_measurement(): + # measure_displacement contends with the main live stream on the microcontroller serial + # link; capture must stop live around the measurement and restore it after. + w = _FMStub(controller=_laser_controller(2.3), live=True) + w._capture_region_offset("A1") + w.liveController.stop_live.assert_called_once() + w.liveController.start_live.assert_called_once() + assert w.region_laser_af_offsets == {"A1": 2.3} + + +def test_capture_does_not_toggle_live_when_not_live(): + w = _FMStub(controller=_laser_controller(2.3), live=False) + w._capture_region_offset("A1") + w.liveController.stop_live.assert_not_called() + w.liveController.start_live.assert_not_called() + + def test_capture_noop_when_mode_disabled(): ctrl = _laser_controller(3.5) w = _FMStub(enabled=False, controller=ctrl) From 9d33d5492bc4a41a9ae1adb55f75a6186cac8be7 Mon Sep 17 00:00:00 2001 From: You Yan Date: Sun, 5 Jul 2026 18:54:20 -0700 Subject: [PATCH 15/15] fix: address PR #562 review findings (per-region laser-AF offset) Independently verified all 8 findings; fixed all of them. F1 (BLOCKER): move_to_target(nonzero) always failed spot-alignment verification (its crop is fixed at x_reference) and reverted z at every FOV, and suppressed the per-channel offset via the af_succeeded gate. Now anchor closed-loop with move_to_target(0.0) (verification valid), then apply the offset as an OPEN-LOOP relative move (new LaserAutofocusController.apply_relative_offset_um). F2 (leak): offsets were pushed before the GUI disk/RAM checks and cleared only late in run_acquisition, so an abort stranded them on the shared controller for fluidics/TCP runs. Now run_acquisition consumes-and-clears up front (threaded into build_params), and both widgets push offsets only after all pre-flight checks pass. F3: benign objective/profile reload re-emits signal_reference_changed unchanged; the listener now clears only when x_reference actually differs from the one offsets were captured against. F4: edit_current_point now drops the region's (now-stale) offset and prompts re-capture. F5: capture failures now log (widget has a logger) and get_offsets_for_acquisition warns at run start about scan regions missing an offset. F6: CSV import only applies offsets when a reference is set, records the reference the offsets are tied to, and notes offsets in the status message. F7: whole import body re-wrapped so any failure shows a dialog (not a silent half-update). F8: _read_focus_points_csv rejects non-finite coordinates and offsets. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/laser_auto_focus_controller.py | 13 ++ .../control/core/multi_point_controller.py | 25 ++- software/control/core/multi_point_worker.py | 11 +- software/control/widgets.py | 196 +++++++++++++----- .../test_per_region_laser_af_offset.py | 130 +++++++++++- 5 files changed, 301 insertions(+), 74 deletions(-) diff --git a/software/control/core/laser_auto_focus_controller.py b/software/control/core/laser_auto_focus_controller.py index c18dc5a31..88ed56aff 100644 --- a/software/control/core/laser_auto_focus_controller.py +++ b/software/control/core/laser_auto_focus_controller.py @@ -392,6 +392,19 @@ def _move_z(self, um_to_move: float) -> None: else: self.stage.move_z(um_to_move / 1000) + def apply_relative_offset_um(self, offset_um: float) -> None: + """Open-loop relative Z move of ``offset_um`` (displacement µm, 1:1 with Z), with NO + spot re-verification. + + Intended to run right after ``move_to_target(0.0)`` has anchored — and verified — the + spot at the reference plane, to then place the sample at a target displacement from + that reference. Spot-alignment verification (``_verify_spot_alignment``) always crops + at ``x_reference`` and would fail for a deliberately-displaced spot, so it must NOT be + used to reach a nonzero target; this method deliberately skips it. No-op for offset 0. + """ + if offset_um: + self._move_z(offset_um) + def set_reference(self) -> bool: """Set the current spot position as the reference position. diff --git a/software/control/core/multi_point_controller.py b/software/control/core/multi_point_controller.py index 50ec99f55..92c667c4a 100644 --- a/software/control/core/multi_point_controller.py +++ b/software/control/core/multi_point_controller.py @@ -680,6 +680,13 @@ def get_estimated_mosaic_ram_bytes(self) -> int: return mosaic_width * mosaic_height * bytes_per_pixel * num_channels def run_acquisition(self, acquire_current_fov=False): + # Consume the per-region laser-AF offsets for THIS run and clear the sticky controller + # copy up-front. Any early return below — or a prior GUI abort that pushed offsets but + # never reached run_acquisition (e.g. aborted on the disk/RAM dialog) — then cannot leak + # them into a later acquisition from an entry point that never sets them (fluidics + # widget, TCP control server). + run_region_laser_af_offsets = self.region_laser_af_offsets + self.region_laser_af_offsets = {} if not self.validate_acquisition_settings(): # emit acquisition finished signal to re-enable the UI self.callbacks.signal_acquisition_finished() @@ -846,14 +853,10 @@ def finish_fn(): updated_callbacks = dataclasses.replace(self.callbacks, signal_acquisition_finished=finish_fn) - acquisition_params = self.build_params(scan_position_information=scan_position_information) - - # build_params has captured the per-region laser-AF offsets into - # acquisition_params for THIS run; clear the controller's copy now so a later - # acquisition from an entry point that never pushes offsets (e.g. fluidics or - # the control server) cannot inherit stale targets. Rebinding (not mutating) - # leaves the copy already handed to acquisition_params intact. - self.region_laser_af_offsets = {} + acquisition_params = self.build_params( + scan_position_information=scan_position_information, + region_laser_af_offsets=run_region_laser_af_offsets, + ) # Gather objective and camera info for YAML current_objective = self.objectiveStore.current_objective @@ -931,7 +934,9 @@ def finish_fn(): self._memory_monitor.stop() self._memory_monitor = None - def build_params(self, scan_position_information: ScanPositionInformation) -> AcquisitionParameters: + def build_params( + self, scan_position_information: ScanPositionInformation, region_laser_af_offsets: Optional[dict] = None + ) -> AcquisitionParameters: # Determine plate dimensions from wellplate format if available plate_num_rows = 8 # Default for 96-well plate_num_cols = 12 @@ -971,7 +976,7 @@ def build_params(self, scan_position_information: ScanPositionInformation) -> Ac plate_num_rows=plate_num_rows, plate_num_cols=plate_num_cols, xy_mode=self.xy_mode, - region_laser_af_offsets=self.region_laser_af_offsets, + region_laser_af_offsets=region_laser_af_offsets if region_laser_af_offsets is not None else {}, ) def _on_acquisition_completed(self): diff --git a/software/control/core/multi_point_worker.py b/software/control/core/multi_point_worker.py index 4259e9901..018417747 100644 --- a/software/control/core/multi_point_worker.py +++ b/software/control/core/multi_point_worker.py @@ -1157,9 +1157,14 @@ def perform_autofocus(self, region_id, fov): # the per-channel z-offset gate would apply offsets from an unanchored z. try: target_um = self.region_laser_af_offsets.get(region_id, 0.0) - if target_um: - self._log.info(f"laser AF target for region '{region_id}': {target_um:.2f} µm") - af_succeeded = self.laser_auto_focus_controller.move_to_target(target_um) + # Anchor closed-loop at the reference (displacement 0), where spot-alignment + # verification is valid, then apply the per-region offset as an OPEN-LOOP + # relative move. Driving move_to_target() straight to a nonzero displacement + # would always fail verification (its crop is fixed at x_reference) and revert. + af_succeeded = self.laser_auto_focus_controller.move_to_target(0.0) + if af_succeeded and target_um: + self._log.info(f"applying per-region laser AF offset for region '{region_id}': {target_um:+.2f} µm") + self.laser_auto_focus_controller.apply_relative_offset_um(target_um) except Exception as e: file_ID = f"{region_id}_focus_camera.bmp" saving_path = os.path.join(self.base_path, self.experiment_ID, str(self.time_point), file_ID) diff --git a/software/control/widgets.py b/software/control/widgets.py index c0519962d..bb471c506 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -6477,12 +6477,8 @@ def toggle_acquisition(self, pressed): if self.checkbox_useFocusMap.isChecked(): self.focusMapWidget.fit_surface() self.multipointController.set_focus_map(self.focusMapWidget.focusMap) - self.multipointController.set_region_laser_af_offsets( - self.focusMapWidget.get_offsets_for_acquisition(self.checkbox_withReflectionAutofocus.isChecked()) - ) else: self.multipointController.set_focus_map(None) - self.multipointController.set_region_laser_af_offsets({}) # Set acquisition parameters self.multipointController.set_deltaZ(self.entry_deltaZ.value()) @@ -6523,6 +6519,14 @@ def toggle_acquisition(self, pressed): self.signal_acquisition_started.emit(True) self.signal_acquisition_shape.emit(self.entry_NZ.value(), self.entry_deltaZ.value()) + # Push per-region laser-AF offsets only now that all pre-flight checks have passed, + # so a disk/RAM abort above cannot strand them on the shared controller for a later run. + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.get_offsets_for_acquisition(self.checkbox_withReflectionAutofocus.isChecked()) + if self.checkbox_useFocusMap.isChecked() + else {} + ) + # Start coordinate-based acquisition self.multipointController.run_acquisition() else: @@ -8833,11 +8837,6 @@ def toggle_acquisition(self, pressed): if self.focusMapWidget.fit_surface(): # If fit successful, set the surface fitter in controller self.multipointController.set_focus_map(self.focusMapWidget.focusMap) - self.multipointController.set_region_laser_af_offsets( - self.focusMapWidget.get_offsets_for_acquisition( - self.checkbox_withReflectionAutofocus.isChecked() - ) - ) else: QMessageBox.warning(self, "Warning", "Failed to fit focus surface") self.btn_startAcquisition.setChecked(False) @@ -8845,7 +8844,6 @@ def toggle_acquisition(self, pressed): else: # If checkbox not checked, set surface fitter to None self.multipointController.set_focus_map(None) - self.multipointController.set_region_laser_af_offsets({}) self.multipointController.set_deltaZ(self.entry_deltaZ.value()) self.multipointController.set_NZ(self.entry_NZ.value()) @@ -8881,6 +8879,14 @@ def toggle_acquisition(self, pressed): # Update UI to show acquisition is running self._set_ui_acquisition_running(self.entry_NZ.value(), self.entry_deltaZ.value()) + # Push per-region laser-AF offsets only now that all pre-flight checks have passed, + # so a disk/RAM abort above cannot strand them on the shared controller for a later run. + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.get_offsets_for_acquisition(self.checkbox_withReflectionAutofocus.isChecked()) + if self.checkbox_useFocusMap.isChecked() + else {} + ) + # Start acquisition self.multipointController.run_acquisition() @@ -10390,6 +10396,7 @@ def __init__( super().__init__() self.setFrameStyle(QFrame.Panel | QFrame.Raised) self._allow_updating_focus_points_on_signal = True + self._log = squid.logging.get_logger(self.__class__.__name__) # Store controllers self.stage = stage @@ -10408,6 +10415,10 @@ def __init__( # region_id. Captured at each focus point when the combined mode is enabled. self.region_laser_af_offsets = {} self.capture_laser_af_offset_enabled = False + # The laser-AF reference (x_reference) the current offsets were measured against. + # Used to distinguish a genuine re-reference (which invalidates offsets) from the + # benign, unchanged re-emit of signal_reference_changed on every config reload. + self._offsets_reference_x = None self.setup_ui() self.make_connections() @@ -10594,6 +10605,13 @@ def edit_current_point(self): new_y = y_spin.value() new_z = z_spin.value() / 1000 # Convert μm back to mm for storage self.focus_points[index] = (region_id, new_x, new_y, new_z) + # The point moved but the stage isn't there to re-measure, so any captured + # laser-AF offset is now stale — drop it (do NOT blind-capture) so it can't + # override the edited z at acquisition. Prompt the user to re-capture. + if self.region_laser_af_offsets.pop(region_id, None) is not None: + msg = f"Region {region_id}: edited — laser AF offset cleared; re-capture with Update Z" + self.status_label.setText(msg) + self._log.info(msg) self.update_point_list() self.update_focus_point_display() @@ -10721,16 +10739,21 @@ def get_region_points_dict(self): def _capture_region_offset(self, region_id): """Record the laser-AF displacement at the current z as this region's offset. - No-op (and clears any stale entry for region_id) unless the combined mode is - enabled, a laser-AF controller exists, and a reference is set. Never stores NaN - (failed spot detection). Warns — but still stores — when the offset exceeds the - laser-AF range, since that region would fail AF during acquisition. + Called after a focus point's z is (re)set (Add / Update Z). The region's previous + offset is dropped up-front: the point's z just changed, so any prior offset is stale + and only a fresh, valid reading may replace it. On any failure (mode off / no + controller / no reference / bad reading) the region is left WITHOUT an offset — it + falls back to the reference plane at acquisition — and the reason is both shown on the + status line and logged (get_offsets_for_acquisition also warns at run start about + regions missing an offset). Warns but still stores when the offset exceeds range. """ self.region_laser_af_offsets.pop(region_id, None) if not self.capture_laser_af_offset_enabled or self.laserAutofocusController is None: return if not self.laserAutofocusController.laser_af_properties.has_reference: - self.status_label.setText("Laser AF reference not set — per-region offset not captured") + msg = f"Laser AF reference not set — offset not captured for region {region_id}" + self.status_label.setText(msg) + self._log.warning(msg) return # Suspend the main camera's live stream during the measurement: laser AF toggles the # AF laser via the microcontroller and waits for completion, while live keeps queuing @@ -10748,17 +10771,24 @@ def _capture_region_offset(self, region_id): # measure_displacement() returns float('nan') on a soft failure; guard with isfinite # (matching capture_current_z_offset) so an invalid reading is never stored. if offset_um is None or not math.isfinite(offset_um): - self.status_label.setText(f"Laser AF spot not detected — offset not captured for {region_id}") + msg = f"Laser AF spot not detected — offset not captured for region {region_id}" + self.status_label.setText(msg) + self._log.warning(msg) return laser_af_range = self.laserAutofocusController.laser_af_properties.laser_af_range if abs(offset_um) > laser_af_range: - self.status_label.setText( + msg = ( f"Warning: region {region_id} offset {offset_um:.1f} µm exceeds laser AF range " f"({laser_af_range:.1f} µm); it may fail AF during acquisition" ) + self.status_label.setText(msg) + self._log.warning(msg) else: self.status_label.setText(f"Region {region_id}: Laser AF offset {offset_um:+.2f} µm") + self._log.info(f"Captured laser AF offset for region {region_id}: {offset_um:+.2f} µm") self.region_laser_af_offsets[region_id] = offset_um + # Remember which reference these offsets were measured against (see _on_laser_af_reference_changed). + self._offsets_reference_x = self.laserAutofocusController.laser_af_properties.x_reference def _on_per_region_offset_toggled(self, checked): # Toggling only gates whether NEW captures happen; it deliberately does not clear @@ -10785,6 +10815,7 @@ def _update_per_region_offset_enabled(self, *args): def _clear_region_offsets(self): self.region_laser_af_offsets = {} + self._offsets_reference_x = None def _sync_offsets_to_focus_points(self): """Drop offsets for regions that no longer have any focus point.""" @@ -10800,16 +10831,41 @@ def get_offsets_for_acquisition(self, reflection_af_active): Centralizes the reflection-AF/checkbox gating so both multipoint widgets — and any future acquisition entry point — share one source of truth. """ - if reflection_af_active and self.checkbox_perRegionLaserAFOffset.isChecked(): - return dict(self.region_laser_af_offsets) - return {} + if not (reflection_af_active and self.checkbox_perRegionLaserAFOffset.isChecked()): + return {} + offsets = dict(self.region_laser_af_offsets) + # Flag scan regions with no captured offset (they fall back to the reference plane), + # so a silently-missing well is visible at run start rather than only diagnosable later. + try: + scan_regions = set(self.scanCoordinates.region_centers.keys()) + except AttributeError: + scan_regions = set() + missing = sorted(str(r) for r in scan_regions if r not in offsets) + if missing: + msg = ( + f"Laser AF Offset: {len(missing)} region(s) without a captured offset will use " + f"the reference plane: {', '.join(missing)}" + ) + self.status_label.setText(msg) + self._log.warning(msg) + return offsets def _on_laser_af_reference_changed(self, has_reference): - # The reference plane moved; previously-captured offsets are relative to the old - # reference and are now meaningless. Clear them so they cannot be applied stale. - if self.region_laser_af_offsets: - self._clear_region_offsets() - self.status_label.setText("Laser AF reference changed — captured per-region offsets cleared") + # signal_reference_changed is re-emitted on every config reload (objective/profile + # switch) with an UNCHANGED reference, so clear only when the reference actually differs + # from the one the current offsets were measured against — otherwise a benign reload + # would silently wipe valid offsets. + if not self.region_laser_af_offsets: + return + current_x = None + if has_reference and self.laserAutofocusController is not None: + current_x = self.laserAutofocusController.laser_af_properties.x_reference + if current_x == self._offsets_reference_x: + return + self._clear_region_offsets() + msg = "Laser AF reference changed — captured per-region offsets cleared" + self.status_label.setText(msg) + self._log.info(msg) def fit_surface(self): try: @@ -10900,12 +10956,20 @@ def _read_focus_points_csv(self, file_path): z = float(row[z_idx]) except (ValueError, IndexError): continue + # Reject non-finite coordinates: float('nan')/'inf' parse fine but blow up + # later in FOV pixel mapping (round(nan)/round(inf)). + if not (math.isfinite(x) and math.isfinite(y) and math.isfinite(z)): + continue points.append((region_id, x, y, z)) if offset_idx is not None and offset_idx < len(row) and row[offset_idx] != "": try: - offsets[region_id] = float(row[offset_idx]) + offset_val = float(row[offset_idx]) except ValueError: - pass + offset_val = None + # Skip non-finite offsets — a NaN/inf target would make laser AF fail + # for every FOV in that region. + if offset_val is not None and math.isfinite(offset_val): + offsets[region_id] = offset_val return points, offsets def export_focus_points(self): @@ -10932,41 +10996,61 @@ def import_focus_points(self): if not file_path: return + # Whole body guarded so a failure anywhere (parse, region-mismatch dialog, state + # assignment, display refresh) surfaces a dialog rather than only logging via the Qt + # excepthook and leaving the widget silently half-updated. try: imported_points, imported_offsets = self._read_focus_points_csv(file_path) + + # If by_region is checked, validate regions + if self.by_region_checkbox.isChecked(): + scan_regions = set(self.scanCoordinates.region_centers.keys()) + focus_regions = set(region_id for region_id, _, _, _ in imported_points) + if not focus_regions == scan_regions: + response = QMessageBox.warning( + self, + "Region Mismatch", + f"The imported focus points have regions: {', '.join(sorted(focus_regions))}\n\n" + f"Current scan has regions: {', '.join(sorted(scan_regions))}\n\n" + "Import anyway (disable 'By Region') or cancel?", + QMessageBox.Ok | QMessageBox.Cancel, + QMessageBox.Cancel, + ) + if response == QMessageBox.Cancel: + return + else: + # User chose to continue, uncheck by_region + self.by_region_checkbox.setChecked(False) + + # Imported offsets are relative to the laser-AF reference they were captured + # against, which the CSV does not record. Only apply them if a reference is set + # now, and flag that they must match it (there is no way to verify automatically). + offset_note = "" + if imported_offsets: + has_reference = ( + self.laserAutofocusController is not None + and self.laserAutofocusController.laser_af_properties.has_reference + ) + if has_reference: + self._offsets_reference_x = self.laserAutofocusController.laser_af_properties.x_reference + offset_note = ( + f"; {len(imported_offsets)} laser AF offset(s) loaded — verify they match " + f"the current laser AF reference" + ) + else: + imported_offsets = {} + offset_note = "; laser AF offsets ignored (no reference set)" + + # Clear existing points and add imported ones + self.focus_points = imported_points + self.region_laser_af_offsets = imported_offsets + self.update_point_list() + self.update_focus_point_display() + self.status_label.setText(f"Imported {len(imported_points)} focus points{offset_note}") except ValueError as e: QMessageBox.warning(self, "Invalid Format", str(e)) - return except Exception as e: QMessageBox.critical(self, "Import Error", f"Failed to import focus points: {str(e)}") - return - - # If by_region is checked, validate regions - if self.by_region_checkbox.isChecked(): - scan_regions = set(self.scanCoordinates.region_centers.keys()) - focus_regions = set(region_id for region_id, _, _, _ in imported_points) - if not focus_regions == scan_regions: - response = QMessageBox.warning( - self, - "Region Mismatch", - f"The imported focus points have regions: {', '.join(sorted(focus_regions))}\n\n" - f"Current scan has regions: {', '.join(sorted(scan_regions))}\n\n" - "Import anyway (disable 'By Region') or cancel?", - QMessageBox.Ok | QMessageBox.Cancel, - QMessageBox.Cancel, - ) - if response == QMessageBox.Cancel: - return - else: - # User chose to continue, uncheck by_region - self.by_region_checkbox.setChecked(False) - - # Clear existing points and add imported ones - self.focus_points = imported_points - self.region_laser_af_offsets = imported_offsets - self.update_point_list() - self.update_focus_point_display() - self.status_label.setText(f"Imported {len(imported_points)} focus points") def on_regions_updated(self): if not self._allow_updating_focus_points_on_signal: diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py index 23b29f6c2..1c2f53216 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -8,7 +8,8 @@ from dataclasses import fields from unittest.mock import MagicMock -from control.core.multi_point_utils import AcquisitionParameters +from control.core.multi_point_utils import AcquisitionParameters, ScanPositionInformation +from control.core.multi_point_controller import MultiPointController from control.core.multi_point_worker import MultiPointWorker from control.widgets import FocusMapWidget @@ -18,6 +19,48 @@ def test_acquisition_parameters_has_region_offsets_field(): assert "region_laser_af_offsets" in names +class _BuildParamsStub: + """MultiPointController-shaped stub exercising build_params' offset threading in isolation.""" + + def __init__(self): + self.scanCoordinates = object() # no 'format' attr -> default plate dims, no wellplate lookup + self._log = MagicMock() + self.experiment_ID = "exp" + self.base_path = "/tmp" + self.selected_configurations = [] + self.timestamp_acquisition_started = 0.0 + self.NX = self.NY = self.NZ = self.Nt = 1 + self.deltaX = self.deltaY = self.deltaZ = self.deltat = 0.0 + self.do_autofocus = False + self.do_reflection_af = True + self.apply_channel_offset = True + self.use_piezo = False + self.display_resolution_scaling = 1.0 + self.z_stacking_config = "FROM CENTER" + self.z_range = (0.0, 0.0) + self.use_fluidics = False + self.skip_saving = False + self.xy_mode = "Current Position" + + build_params = MultiPointController.build_params + + +def _empty_scan_position_information(): + return ScanPositionInformation(scan_region_coords_mm=[], scan_region_names=[], scan_region_fov_coords_mm={}) + + +def test_build_params_threads_region_offsets(): + params = _BuildParamsStub().build_params(_empty_scan_position_information(), region_laser_af_offsets={"A1": 4.0}) + assert params.region_laser_af_offsets == {"A1": 4.0} + + +def test_build_params_defaults_offsets_to_empty_when_not_passed(): + # run_acquisition clears self.region_laser_af_offsets up-front and passes the snapshot + # explicitly; a None/absent argument must yield {} (no leaked sticky state). + params = _BuildParamsStub().build_params(_empty_scan_position_information()) + assert params.region_laser_af_offsets == {} + + def test_region_offsets_default_factory_is_empty_dict(): fld = next(f for f in fields(AcquisitionParameters) if f.name == "region_laser_af_offsets") assert fld.default_factory() == {} @@ -43,9 +86,13 @@ def __init__(self, offsets, move_result=True): def test_perform_autofocus_uses_region_offset(): + # Anchor closed-loop at the reference (move_to_target(0.0)), then apply the per-region + # offset open-loop — driving move_to_target to a nonzero displacement would fail the + # reference-crop verification and revert. w = _AFStub({"A1": 5.0}) assert w.perform_autofocus("A1", 0) is True - w.laser_auto_focus_controller.move_to_target.assert_called_once_with(5.0) + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + w.laser_auto_focus_controller.apply_relative_offset_um.assert_called_once_with(5.0) assert w._laser_af_successes == 1 @@ -53,11 +100,15 @@ def test_perform_autofocus_defaults_to_zero_for_unmapped_region(): w = _AFStub({"A1": 5.0}) assert w.perform_autofocus("B2", 0) is True w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + # No offset for B2 → no open-loop move. + w.laser_auto_focus_controller.apply_relative_offset_um.assert_not_called() -def test_perform_autofocus_failure_increments_and_returns_false(): +def test_perform_autofocus_offset_not_applied_when_anchor_fails(): + # If anchoring at the reference fails, the open-loop offset must NOT be applied. w = _AFStub({"A1": 5.0}, move_result=False) assert w.perform_autofocus("A1", 0) is False + w.laser_auto_focus_controller.apply_relative_offset_um.assert_not_called() assert w._laser_af_failures == 1 @@ -78,6 +129,8 @@ def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets= self.focus_points = focus_points if focus_points is not None else [] self.region_laser_af_offsets = offsets if offsets is not None else {} self.status_label = MagicMock() + self._log = MagicMock() + self._offsets_reference_x = None self.liveController = MagicMock() self.liveController.is_live = live self.checkbox_perRegionLaserAFOffset = MagicMock() @@ -181,13 +234,52 @@ def test_sync_drops_orphaned_offsets(): assert w.region_laser_af_offsets == {"A1": 1.0} -def test_reference_change_clears_offsets_and_sets_status(): - w = _FMStub(offsets={"A1": 1.0}) +def test_capture_sets_offsets_reference_x_on_success(): + ctrl = _laser_controller(3.5) + ctrl.laser_af_properties.x_reference = 123.0 + w = _FMStub(controller=ctrl) + w._capture_region_offset("A1") + assert w._offsets_reference_x == 123.0 + + +def test_capture_failure_logs_warning(): + # A failed re-capture (spot not detected) drops the offset AND logs a warning — not just + # a transient status label — so silent data loss is visible. + w = _FMStub(controller=_laser_controller(float("nan")), offsets={"A1": 4.0}) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + assert w._log.warning.called + + +def test_reference_change_clears_offsets_when_reference_actually_changed(): + ctrl = _laser_controller(0.0) + ctrl.laser_af_properties.x_reference = 200.0 # reference is now different + w = _FMStub(controller=ctrl, offsets={"A1": 1.0}) + w._offsets_reference_x = 100.0 # offsets were captured against the OLD reference w._on_laser_af_reference_changed(True) assert w.region_laser_af_offsets == {} assert w.status_label.setText.called +def test_reference_change_keeps_offsets_on_benign_reemit(): + # A config reload re-emits signal_reference_changed with the SAME reference; offsets must survive. + ctrl = _laser_controller(0.0) + ctrl.laser_af_properties.x_reference = 100.0 + w = _FMStub(controller=ctrl, offsets={"A1": 1.0}) + w._offsets_reference_x = 100.0 # same reference the offsets were captured against + w._on_laser_af_reference_changed(True) + assert w.region_laser_af_offsets == {"A1": 1.0} + w.status_label.setText.assert_not_called() + + +def test_reference_lost_clears_offsets(): + ctrl = _laser_controller(0.0, has_reference=False) + w = _FMStub(controller=ctrl, offsets={"A1": 1.0}) + w._offsets_reference_x = 100.0 + w._on_laser_af_reference_changed(False) + assert w.region_laser_af_offsets == {} + + def test_reference_change_no_status_when_nothing_to_clear(): w = _FMStub() w._on_laser_af_reference_changed(True) @@ -230,6 +322,22 @@ def test_csv_read_rejects_missing_required_columns(tmp_path): dst._read_focus_points_csv(str(path)) +def test_csv_read_skips_nonfinite_offsets_and_coords(tmp_path): + path = tmp_path / "bad.csv" + path.write_text( + "Region_ID,X_mm,Y_mm,Z_um,Offset_um\n" + "A1,1.0,2.0,0.5,3.0\n" # good + "B2,1.0,2.0,0.5,nan\n" # good coords, NaN offset -> point kept, offset rejected + "C3,nan,2.0,0.5,4.0\n" # NaN coordinate -> whole row dropped + "D4,1.0,2.0,0.5,inf\n" # inf offset -> point kept, offset rejected + ) + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(str(path)) + assert [p[0] for p in points] == ["A1", "B2", "D4"] # C3 dropped (non-finite coord) + assert offsets == {"A1": 3.0} # NaN/inf offsets rejected + + # --------------------------------------------------------------------------- # _on_per_region_offset_toggled # --------------------------------------------------------------------------- @@ -315,3 +423,15 @@ def test_get_offsets_for_acquisition_empty_when_checkbox_off(): w = _FMStub(offsets={"A1": 3.0}) w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False assert w.get_offsets_for_acquisition(reflection_af_active=True) == {} + + +def test_get_offsets_warns_about_regions_missing_offset(): + # A scan region with no captured offset falls back to the reference plane — warn at run + # start so a silently-missing well is visible. + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + w.scanCoordinates = MagicMock() + w.scanCoordinates.region_centers.keys.return_value = ["A1", "B2"] + result = w.get_offsets_for_acquisition(reflection_af_active=True) + assert result == {"A1": 3.0} + assert w._log.warning.called # B2 has no offset