From 3409d01efdd75e778675d7fb1d9cfa4e70b604ea Mon Sep 17 00:00:00 2001 From: Dawn Date: Tue, 29 Sep 2026 12:14:30 +0200 Subject: [PATCH] fix(gui): fire mount auto-centering on the first idle status tick The "apply after mounting" checkbox clicked Auto Centering as soon as the status poll showed a new sample id. /sample/mount still holds the hw lock at that point, /alc/center_loop takes the same lock non-blocking, and the resulting BeamlineBusyException is a quiet error the GUI swallows. So the automatic click never did anything; only a later manual click worked. Arm on the sample-id change and click on the first status tick with busy=False, which is exactly what the user did by hand. Skip arming while queue automation runs: it mounts and centers on its own and frees the lock between /scan/auto calls, so a GUI centering in that gap would steal the lock from the next scan. Pause during a scan's mount phase can still arm that one sample, but Pause ends in an unmount, so the window is a ms-wide lock gap sampled at 2 Hz; accepted, noted in the code. Supersedes the three-commit approach on fix/auto-center-after-mount-wait-idle (PR 243). Co-Authored-By: Claude Fable 5.1 --- src/aare/gui/main_window.py | 3 ++ .../gui/panels/data_collection_settings.py | 39 ++++++++++++++++--- .../unit/gui/test_data_collection_settings.py | 33 ++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/aare/gui/main_window.py b/src/aare/gui/main_window.py index 67fecf58..daa6d885 100644 --- a/src/aare/gui/main_window.py +++ b/src/aare/gui/main_window.py @@ -745,6 +745,9 @@ class MainWindow(QMainWindow): self.automation_progress_panel.set_running ) self.job_list_panel.automation_running_changed.connect(self._on_automation_running_changed) + self.job_list_panel.automation_running_changed.connect( + self.data_collection.set_automation_running + ) self.automation_progress_panel.set_samples_in_queue( len(self.job_list_panel.table_model.samples) diff --git a/src/aare/gui/panels/data_collection_settings.py b/src/aare/gui/panels/data_collection_settings.py index fc833c2c..59be7f06 100644 --- a/src/aare/gui/panels/data_collection_settings.py +++ b/src/aare/gui/panels/data_collection_settings.py @@ -195,6 +195,9 @@ class DataCollectionSettings(QFrame): # startup must not trigger an auto-centering (hardware would move # uninvited on every restart). self._status_seen = False + # Armed on mount, fired on the first idle status tick (see _track_sample). + self._center_after_mount_pending = False + self._automation_running = False self._tab_bar.currentChanged.connect(self._stack.setCurrentIndex) self._tab_bar.currentChanged.connect(self._on_tab_changed) @@ -242,23 +245,47 @@ class DataCollectionSettings(QFrame): self.raster.update_daq_status(s) self.screening.update_daq_status(s) self.simple.update_daq_status(s) - self._track_sample(None if s.sample is None else s.sample.db_id) + self._track_sample(None if s.sample is None else s.sample.db_id, s.busy) - def _track_sample(self, db_id: int | None): + def _track_sample(self, db_id: int | None, busy: bool = False): # Separate from update_daq_status so tests can drive mount detection # without building a full DAQStatusModel. if db_id is None: # Forget on unmount so remounting the same sample counts as a # fresh mount below. self._sample_id = None + self._center_after_mount_pending = False elif db_id != self._sample_id: self._sample_id = db_id self._tab_bar.setCurrentIndex(0) - if self._status_seen and self.auto_center_after_mount.isChecked(): - # Reuse the button wiring (clicked -> daq.center_loop in - # main_window) instead of a second signal path. - self.find_tip.click() + self._center_after_mount_pending = ( + self._status_seen + and self.auto_center_after_mount.isChecked() + and not self._automation_running + ) self._status_seen = True + # The server publishes the new sample while /sample/mount still holds + # the hw lock, and /alc/center_loop takes that lock non-blocking, so a + # click on the id change alone was rejected as busy (a quiet error the + # GUI swallows). Wait for the first idle tick instead: same as the + # user waiting a bit and clicking. + if self._center_after_mount_pending and not busy: + self._center_after_mount_pending = False + # Reuse the button wiring (clicked -> daq.center_loop in + # main_window) instead of a second signal path. + self.find_tip.click() + + @Slot(bool) + def set_automation_running(self, running: bool): + # Queue automation mounts and centers each sample itself and frees the + # hw lock between /scan/auto calls; a GUI centering in that gap would + # steal the lock from the next scan and stop the queue. + # ponytail: Pause drops the flag while the current scan may still be + # in its mount phase -> at most one extra centering on that sample. + # Track in-flight /scan/auto instead if that ever matters. + self._automation_running = running + if running: + self._center_after_mount_pending = False @Slot(int) def _on_tab_changed(self, idx: int): diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index 252566c5..28046cce 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -353,6 +353,39 @@ def test_auto_center_fires_on_mount_only_when_armed(settings_panel): assert len(clicks) == 3 +def test_auto_center_waits_for_hw_lock_after_mount(settings_panel): + clicks = [] + settings_panel.find_tip.clicked.connect(lambda: clicks.append(1)) + settings_panel.auto_center_after_mount.setChecked(True) + settings_panel._track_sample(None) + + # The mount still holds the hw lock when the sample id first shows up: + # /alc/center_loop would be refused, so no click yet. + settings_panel._track_sample(1, busy=True) + settings_panel._track_sample(1, busy=True) + assert not clicks + # First idle tick fires it, exactly once. + settings_panel._track_sample(1, busy=False) + settings_panel._track_sample(1, busy=False) + assert len(clicks) == 1 + + # Unmount while still pending drops the request. + settings_panel._track_sample(None) + settings_panel._track_sample(2, busy=True) + settings_panel._track_sample(None, busy=False) + assert len(clicks) == 1 + + # Queue automation mounts and centers on its own: never fire in its gaps. + settings_panel.set_automation_running(True) + settings_panel._track_sample(3, busy=True) + settings_panel._track_sample(3, busy=False) + assert len(clicks) == 1 + settings_panel.set_automation_running(False) + settings_panel._track_sample(None) + settings_panel._track_sample(4, busy=False) + assert len(clicks) == 2 + + # --------------------------------------------------------------------------- # Pending color while typing (movestate property, committed on Enter/focus-out) # ---------------------------------------------------------------------------