fix(navigation): keep planned-coordinate grid fixed during acquisition#563
Merged
Alpaca233 merged 1 commit intoJun 24, 2026
Conversation
Alpaca233
force-pushed
the
fix/live-scan-grid-follows-during-acquisition
branch
from
7a98274 to
90e8a0b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a navigation-viewer regression where the planned-coordinate “live scan grid” (orange overlay) incorrectly re-centers on stage motion during acquisitions, by preventing duplicate Qt signal connections and enforcing a runtime guard during acquisition.
Changes:
- Make
HighContentScreeningGui.toggle_live_scan_grididempotent to avoid accumulating duplicateposition_after_moveconnections. - Make
WellplateMultiPointWidget.update_live_coordinatesa no-op whilemultipointController.acquisition_in_progress()is true. - Add regression tests covering acquisition behavior and idempotent connect/disconnect behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| software/tests/control/test_live_scan_grid.py | Adds regression tests for live scan grid behavior during acquisition and for idempotent signal wiring. |
| software/control/widgets.py | Prevents live-grid updates during acquisition by early-returning in update_live_coordinates. |
| software/control/gui_hcs.py | Prevents duplicate Qt signal connections by making toggle_live_scan_grid idempotent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
+36
| def _build_gui(qtbot, monkeypatch): | ||
| monkeypatch.setattr(QMessageBox, "question", _confirm_exit) | ||
| scope = control.microscope.Microscope.build_from_global_config(True) | ||
| win = control.gui_hcs.HighContentScreeningGui(microscope=scope, is_simulation=True) | ||
| qtbot.add_widget(win) |
The orange "live scan grid" (planned-coordinate overlay) re-centers on the current stage position via WellplateMultiPointWidget.update_live_coordinates, which is wired to MovementUpdater.position_after_move only while manually navigating. It is meant to be disconnected for the duration of an acquisition. Two defects let it follow the stage during a scan: 1. toggle_live_scan_grid(on=True) connected the signal unconditionally. PyQt removes only one connection per disconnect(), so any double-enable left a dangling connection that the single acquisition-start disconnect could not clear. Duplicates accumulated across acquisitions, so the grid started chasing the stage by the 2nd-3rd run. 2. update_live_coordinates never checked whether an acquisition was running, so a surviving connection redrew the grid on every stage move. Fix: make toggle_live_scan_grid idempotent (so duplicate connections can never form) and make update_live_coordinates a no-op while acquisition_in_progress(). The current-position FOV box uses a separate connection, so it still tracks the stage; only the planned grid is pinned. Adds regression tests covering both invariants. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Alpaca233
force-pushed
the
fix/live-scan-grid-follows-during-acquisition
branch
from
90e8a0b to
e3289a2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
During acquisition, the orange grid of planned FOV coordinates in the navigation viewer follows the current stage position instead of staying fixed on the well plate. It tends to appear by the 2nd–3rd acquisition, especially after switching between Current Position and Manual XY modes and changing magnification.
Root cause
The orange "live scan grid" is a positioning preview (
WellplateMultiPointWidget.update_live_coordinates→set_live_scan_coordinates) wired toMovementUpdater.position_after_moveonly while manually navigating. It is supposed to be disconnected for the duration of an acquisition (HighContentScreeningGui.toggleAcquisitionStart). Two defects defeat that:toggle_live_scan_grid(on=True)connected the signal unconditionally. PyQt removes only one connection perdisconnect(), so any double-enable left a dangling connection that the single acquisition-start disconnect could not clear. Duplicates accumulate across acquisitions — which is why it shows up by the 2nd–3rd run.update_live_coordinatesnever checked whether an acquisition was running, so a surviving connection redrew the grid on every stage move.Fix
toggle_live_scan_grididempotent (returns early if already in the requested state) so duplicate connections can never form.update_live_coordinatesa no-op whileacquisition_in_progress()— defense in depth that directly enforces the invariant.The current-position FOV box uses a separate
position_after_moveconnection, so it still tracks the stage during acquisition; only the planned grid is pinned.Tests
tests/control/test_live_scan_grid.py:test_live_scan_grid_does_not_follow_stage_during_acquisition— the grid stays put during acquisition, and still follows during manual navigation.test_toggle_live_scan_grid_is_idempotent— a redundant enable leaves no dangling connection after a single disable.Both fail before the fix and pass after. Existing GUI tests still pass.
🤖 Generated with Claude Code