From 43054dc0e84699df88087fd2cadbffe5471f9a33 Mon Sep 17 00:00:00 2001 From: Dawn Date: Tue, 29 Sep 2026 11:14:45 +0200 Subject: [PATCH 1/3] fix(gui): wait for hw lock before auto-centering after mount The server publishes the new sample while the mount still holds the hw lock, and /alc/center_loop takes that lock non-blocking, so the click fired on the sample-id change was rejected as busy and never retried. Arm on mount, fire on the first non-busy status tick instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../gui/panels/data_collection_settings.py | 22 +++++++++++++----- .../unit/gui/test_data_collection_settings.py | 23 +++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/aare/gui/panels/data_collection_settings.py b/src/aare/gui/panels/data_collection_settings.py index fc833c2c..6929d5de 100644 --- a/src/aare/gui/panels/data_collection_settings.py +++ b/src/aare/gui/panels/data_collection_settings.py @@ -195,6 +195,8 @@ class DataCollectionSettings(QFrame): # startup must not trigger an auto-centering (hardware would move # uninvited on every restart). self._status_seen = False + # Set on mount, fired once the hw lock frees (see _track_sample). + self._center_after_mount_pending = False self._tab_bar.currentChanged.connect(self._stack.setCurrentIndex) self._tab_bar.currentChanged.connect(self._on_tab_changed) @@ -242,23 +244,31 @@ 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() + ) self._status_seen = True + # The server publishes the new sample while the mount still holds the + # hw lock, and /alc/center_loop refuses (non-blocking lock) while busy, + # so firing on the id change alone was silently rejected. Wait for idle. + 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(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..0a7990bd 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -353,6 +353,29 @@ def test_auto_center_fires_on_mount_only_when_armed(settings_panel): assert len(clicks) == 3 +def test_auto_center_waits_for_mount_to_release_hw_lock(settings_panel): + # Server shows the new sample while the mount still holds the hw lock; + # center_loop would be refused then, so the click must wait for idle. + clicks = [] + settings_panel.find_tip.clicked.connect(lambda: clicks.append(1)) + settings_panel.auto_center_after_mount.setChecked(True) + settings_panel._track_sample(1) + settings_panel._track_sample(None) + + settings_panel._track_sample(2, busy=True) + settings_panel._track_sample(2, busy=True) + assert not clicks + settings_panel._track_sample(2, busy=False) + assert len(clicks) == 1 + settings_panel._track_sample(2, busy=False) + assert len(clicks) == 1 + + # Unmount before the lock frees drops the pending centering. + settings_panel._track_sample(3, busy=True) + settings_panel._track_sample(None, busy=False) + assert len(clicks) == 1 + + # --------------------------------------------------------------------------- # Pending color while typing (movestate property, committed on Enter/focus-out) # --------------------------------------------------------------------------- -- 2.54.0 From f49f34cd27572ec2662f412f84452b66dce7fd0b Mon Sep 17 00:00:00 2001 From: Dawn Date: Tue, 29 Sep 2026 11:19:24 +0200 Subject: [PATCH 2/3] fix(gui): skip mount auto-centering during queue automation Automation centers each sample itself and frees the hw lock between samples; waiting for idle made the GUI fire center_loop in that gap, which could steal the lock from the next /scan/auto and stop the queue. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/aare/gui/main_window.py | 3 +++ .../gui/panels/data_collection_settings.py | 14 +++++++++++++- .../unit/gui/test_data_collection_settings.py | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) 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 6929d5de..184b1030 100644 --- a/src/aare/gui/panels/data_collection_settings.py +++ b/src/aare/gui/panels/data_collection_settings.py @@ -197,6 +197,7 @@ class DataCollectionSettings(QFrame): self._status_seen = False # Set on mount, fired once the hw lock frees (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) @@ -258,7 +259,9 @@ class DataCollectionSettings(QFrame): self._sample_id = db_id self._tab_bar.setCurrentIndex(0) self._center_after_mount_pending = ( - self._status_seen and self.auto_center_after_mount.isChecked() + 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 the mount still holds the @@ -270,6 +273,15 @@ class DataCollectionSettings(QFrame): # main_window) instead of a second signal path. self.find_tip.click() + @Slot(bool) + def set_automation_running(self, running: bool): + # Queue automation centers each sample itself and frees the hw lock + # between samples; a GUI centering in that gap could steal the lock + # from the next /scan/auto and stop the queue. + self._automation_running = running + if running: + self._center_after_mount_pending = False + @Slot(int) def _on_tab_changed(self, idx: int): self.bounding_box.setVisible(idx == 0) diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index 0a7990bd..307c228d 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -376,6 +376,25 @@ def test_auto_center_waits_for_mount_to_release_hw_lock(settings_panel): assert len(clicks) == 1 +def test_auto_center_skipped_during_queue_automation(settings_panel): + # Automation centers itself and frees the lock between samples; a GUI + # centering there could steal the lock from the next /scan/auto. + clicks = [] + settings_panel.find_tip.clicked.connect(lambda: clicks.append(1)) + settings_panel.auto_center_after_mount.setChecked(True) + settings_panel._track_sample(1) + + settings_panel.set_automation_running(True) + settings_panel._track_sample(2, busy=True) + settings_panel._track_sample(2, busy=False) + assert not clicks + + settings_panel.set_automation_running(False) + settings_panel._track_sample(3, busy=True) + settings_panel._track_sample(3, busy=False) + assert len(clicks) == 1 + + # --------------------------------------------------------------------------- # Pending color while typing (movestate property, committed on Enter/focus-out) # --------------------------------------------------------------------------- -- 2.54.0 From 039c56a9e979633d1c835b982d57b552cf0e0ec3 Mon Sep 17 00:00:00 2001 From: Dawn Date: Tue, 29 Sep 2026 11:25:11 +0200 Subject: [PATCH 3/3] fix(gui): gate mount auto-centering on in-flight /scan/auto, not queue flag Pause flips the queue's running flag while the current sample is still being processed, so a mount seen after Pause armed a centering on an already handled sample. Track the /scan/auto request instead; a counter because the queue sends the next scan before the done signal reaches us. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/aare/gui/main_window.py | 5 ++-- .../gui/panels/data_collection_settings.py | 26 ++++++++++++------- .../unit/gui/test_data_collection_settings.py | 17 +++++++++--- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/aare/gui/main_window.py b/src/aare/gui/main_window.py index daa6d885..94ac0c2d 100644 --- a/src/aare/gui/main_window.py +++ b/src/aare/gui/main_window.py @@ -745,9 +745,6 @@ 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) @@ -1027,6 +1024,7 @@ class MainWindow(QMainWindow): ) self.job_list_panel.auto_scan.connect(self.daq.automated_scan) + self.job_list_panel.auto_scan.connect(self.data_collection.auto_scan_started) self.job_list_panel.unmount.connect(self.daq.unmount) self.job_list_panel.unmount_then_park_and_dry.connect(self.daq.unmount_then_park_and_dry) @@ -1151,6 +1149,7 @@ class MainWindow(QMainWindow): self.daq.automated_scan_done.connect(self.job_list_panel.automated_scan_done) self.daq.automated_scan_done.connect(self._mark_scan_result) + self.daq.automated_scan_done.connect(self.data_collection.auto_scan_finished) self.daq.automation_critical_failure.connect(self._on_automation_critical_failure) self.daq.manual_collection_critical_failure.connect( self._on_manual_collection_critical_failure diff --git a/src/aare/gui/panels/data_collection_settings.py b/src/aare/gui/panels/data_collection_settings.py index 184b1030..8f7d982d 100644 --- a/src/aare/gui/panels/data_collection_settings.py +++ b/src/aare/gui/panels/data_collection_settings.py @@ -197,7 +197,7 @@ class DataCollectionSettings(QFrame): self._status_seen = False # Set on mount, fired once the hw lock frees (see _track_sample). self._center_after_mount_pending = False - self._automation_running = False + self._auto_scans_in_flight = 0 self._tab_bar.currentChanged.connect(self._stack.setCurrentIndex) self._tab_bar.currentChanged.connect(self._on_tab_changed) @@ -261,7 +261,7 @@ class DataCollectionSettings(QFrame): self._center_after_mount_pending = ( self._status_seen and self.auto_center_after_mount.isChecked() - and not self._automation_running + and not self._auto_scans_in_flight ) self._status_seen = True # The server publishes the new sample while the mount still holds the @@ -273,14 +273,20 @@ class DataCollectionSettings(QFrame): # main_window) instead of a second signal path. self.find_tip.click() - @Slot(bool) - def set_automation_running(self, running: bool): - # Queue automation centers each sample itself and frees the hw lock - # between samples; a GUI centering in that gap could steal the lock - # from the next /scan/auto and stop the queue. - self._automation_running = running - if running: - self._center_after_mount_pending = False + # Queue automation centers each sample itself and frees the hw lock + # between samples; a GUI centering in that gap could steal the lock from + # the next /scan/auto. Tracks the request, not the queue's running flag, + # because Pause flips that flag while the current sample is still running. + # A counter, since the queue sends the next scan before the settings panel + # sees the previous one's done signal. + @Slot() + def auto_scan_started(self): + self._auto_scans_in_flight += 1 + self._center_after_mount_pending = False + + @Slot() + def auto_scan_finished(self): + self._auto_scans_in_flight = max(0, self._auto_scans_in_flight - 1) @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 307c228d..0020a565 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -376,7 +376,7 @@ def test_auto_center_waits_for_mount_to_release_hw_lock(settings_panel): assert len(clicks) == 1 -def test_auto_center_skipped_during_queue_automation(settings_panel): +def test_auto_center_skipped_while_auto_scan_in_flight(settings_panel): # Automation centers itself and frees the lock between samples; a GUI # centering there could steal the lock from the next /scan/auto. clicks = [] @@ -384,14 +384,25 @@ def test_auto_center_skipped_during_queue_automation(settings_panel): settings_panel.auto_center_after_mount.setChecked(True) settings_panel._track_sample(1) - settings_panel.set_automation_running(True) + # Queue sends the next scan before the settings panel sees the previous one + # finish (signal order in main_window): must still count as in flight. + settings_panel.auto_scan_started() + settings_panel.auto_scan_started() + settings_panel.auto_scan_finished() settings_panel._track_sample(2, busy=True) settings_panel._track_sample(2, busy=False) assert not clicks - settings_panel.set_automation_running(False) + # Pause mid-sample: the current scan still runs to completion, so no + # centering until its request is done and a later manual mount happens. settings_panel._track_sample(3, busy=True) + settings_panel.auto_scan_finished() settings_panel._track_sample(3, busy=False) + assert not clicks + + settings_panel._track_sample(None) + settings_panel._track_sample(4, busy=True) + settings_panel._track_sample(4, busy=False) assert len(clicks) == 1 -- 2.54.0