From f1ad58ad229c267fb192da991c75e9e74ed03b72 Mon Sep 17 00:00:00 2001 From: You Yan Date: Thu, 18 Jun 2026 20:57:55 -0700 Subject: [PATCH 01/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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 From 3981e562f4cf2ad61f8893bb539a9a1dcffaccf6 Mon Sep 17 00:00:00 2001 From: You Yan Date: Sun, 19 Jul 2026 21:31:10 -0700 Subject: [PATCH 16/17] =?UTF-8?q?refactor:=20drop=20the=20Offset=5Fum=20co?= =?UTF-8?q?lumn=20=E2=80=94=20per-region=20offsets=20are=20session-only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persisting offsets in the focus-points CSV coupled them to a laser-AF reference the file does not identify, so an import could silently apply offsets captured against a different reference plane (PR #562 review, finding 6). Rather than embedding a reference identity in the CSV, stop persisting offsets entirely: export writes the original four columns, import reads points only (an Offset_um column from earlier builds of this branch is ignored), and importing points drops any captured offsets, since they belong to the replaced points. Offsets are captured per session in the Focus Map tab. Co-Authored-By: Claude Fable 5 --- software/control/widgets.py | 54 ++++--------------- software/docs/per-region-laser-af-offset.md | 8 +-- .../test_per_region_laser_af_offset.py | 39 +++++++------- 3 files changed, 36 insertions(+), 65 deletions(-) diff --git a/software/control/widgets.py b/software/control/widgets.py index bb471c506..efc41e8e2 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -10922,20 +10922,16 @@ def _match_by_region_box(self): 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"]) + writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_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]) + writer.writerow([region_id, x, y, z]) def _read_focus_points_csv(self, file_path): - """Parse a focus-points CSV. Returns (points, offsets). + """Parse a focus-points CSV into a list of (region_id, x, y, z) points. - 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. + Unknown columns are ignored. 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) @@ -10946,7 +10942,6 @@ def _read_focus_points_csv(self, file_path): 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: @@ -10961,16 +10956,7 @@ def _read_focus_points_csv(self, file_path): 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: - offset_val = float(row[offset_idx]) - except ValueError: - 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 + return points def export_focus_points(self): """Export focus points to a CSV file""" @@ -10991,7 +10977,7 @@ def export_focus_points(self): QMessageBox.critical(self, "Export Error", f"Failed to export focus points: {str(e)}") def import_focus_points(self): - """Import focus points (and optional per-region offsets) from a CSV file""" + """Import focus points from a CSV file""" file_path, _ = QFileDialog.getOpenFileName(self, "Import Focus Points", "", "CSV Files (*.csv);;All Files (*)") if not file_path: return @@ -11000,7 +10986,7 @@ def import_focus_points(self): # 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) + imported_points = self._read_focus_points_csv(file_path) # If by_region is checked, validate regions if self.by_region_checkbox.isChecked(): @@ -11022,31 +11008,13 @@ def import_focus_points(self): # 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 + # Replace existing points with the imported ones. Captured laser-AF offsets + # belong to the replaced points, so they are dropped (re-capture per region). self.focus_points = imported_points - self.region_laser_af_offsets = imported_offsets + self._clear_region_offsets() self.update_point_list() self.update_focus_point_display() - self.status_label.setText(f"Imported {len(imported_points)} focus points{offset_note}") + self.status_label.setText(f"Imported {len(imported_points)} focus points") except ValueError as e: QMessageBox.warning(self, "Invalid Format", str(e)) except Exception as e: diff --git a/software/docs/per-region-laser-af-offset.md b/software/docs/per-region-laser-af-offset.md index 03596ba1e..b24fdf8ac 100644 --- a/software/docs/per-region-laser-af-offset.md +++ b/software/docs/per-region-laser-af-offset.md @@ -44,10 +44,10 @@ reference plane. 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). + offset; regenerating the focus grid or importing focus points from CSV clears them. +- **Offsets are not saved**: the focus-points CSV export contains only the points. + Offsets live in the current session — re-record them after importing points or + restarting the software. - **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 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 1c2f53216..33a16f20c 100644 --- a/software/tests/control/test_per_region_laser_af_offset.py +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -286,30 +286,35 @@ def test_reference_change_no_status_when_nothing_to_clear(): w.status_label.setText.assert_not_called() -def test_csv_roundtrip_includes_offsets(tmp_path): +def test_csv_roundtrip_points_only(tmp_path): + # Offsets are session-only state: export writes just the focus points, never the offsets. 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 + offsets={"A1": 7.0}, ) src._write_focus_points_csv = FocusMapWidget._write_focus_points_csv.__get__(src) path = str(tmp_path / "fp.csv") src._write_focus_points_csv(path) + with open(path) as f: + header = f.readline().strip() + assert header == "Region_ID,X_mm,Y_mm,Z_um" + dst = _FMStub() dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) - points, offsets = dst._read_focus_points_csv(path) + points = 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") +def test_csv_read_ignores_extra_columns(tmp_path): + # Files exported by builds that wrote an Offset_um column still import fine; + # the extra column is ignored. + path = tmp_path / "with_offsets.csv" + path.write_text("Region_ID,X_mm,Y_mm,Z_um,Offset_um\nA1,1.0,2.0,0.5,7.0\n") dst = _FMStub() dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) - points, offsets = dst._read_focus_points_csv(str(path)) + points = 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): @@ -322,20 +327,18 @@ 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): +def test_csv_read_skips_nonfinite_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 + "Region_ID,X_mm,Y_mm,Z_um\n" + "A1,1.0,2.0,0.5\n" # good + "C3,nan,2.0,0.5\n" # NaN coordinate -> whole row dropped + "D4,1.0,inf,0.5\n" # inf coordinate -> whole row dropped ) 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 + points = dst._read_focus_points_csv(str(path)) + assert [p[0] for p in points] == ["A1"] # --------------------------------------------------------------------------- From c2a15fcc2edccc6ddd1ed1e3a5394b810d48ad40 Mon Sep 17 00:00:00 2001 From: You Yan Date: Sun, 19 Jul 2026 21:39:51 -0700 Subject: [PATCH 17/17] docs: move the plan + design docs to the AI-docs repo Planning documents rot in-tree (both already cite line numbers earlier commits shifted); repo convention routes them to the AI-docs repo, where they now live under Squid/in-progress/. The user guide stays. Co-Authored-By: Claude Fable 5 --- .../2026-06-18-per-region-laser-af-offset.md | 831 ------------------ ...06-18-per-region-laser-af-offset-design.md | 246 ------ 2 files changed, 1077 deletions(-) delete mode 100644 software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md delete mode 100644 software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.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 deleted file mode 100644 index 29d87dbd7..000000000 --- a/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md +++ /dev/null @@ -1,831 +0,0 @@ -# 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. 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 deleted file mode 100644 index 880c2cd49..000000000 --- a/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md +++ /dev/null @@ -1,246 +0,0 @@ -# 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.