diff --git a/src/aare/gui/main_window.py b/src/aare/gui/main_window.py index 21e12fc4..c8d95864 100644 --- a/src/aare/gui/main_window.py +++ b/src/aare/gui/main_window.py @@ -234,6 +234,9 @@ class MainWindow(QMainWindow): self._remote_close_reason: str | None = None self._remote_close_banner_active: bool = False self._latest_daq_status: DAQStatusModel | None = None + # Queue head awaiting removal: popped only once /status shows it is no + # longer on the gonio, so a failed unmount never loses a sample. + self._pending_unmount_pop: int | None = None self._tutorial_event_bus = TutorialEventBus(self) self._tutorial_text_resolver = DictionaryTextResolver(MANUAL_MOUNT_TUTORIAL) @@ -1094,6 +1097,7 @@ class MainWindow(QMainWindow): self.job_list_panel.viewer_track_online.connect(self.viewer.load_online) self.sample_logic.sample_changed.connect(self.data_collection.file_path_panel.update_sample) + self.sample_logic.sample_changed.connect(self._on_mounted_sample_changed) self.daq.update.connect(self.beamline.omega_panel.update_daq_status) self.daq.update.connect(self.beamline.smargon_panel.update_daq_status) @@ -2163,19 +2167,39 @@ class MainWindow(QMainWindow): return None def _mount_next_from_queue(self) -> None: - """Manual step-through: mount queue head; if head already on gonio, pop it and mount the following one. - Pops only after /status confirms mount, so a failed robot move never loses a sample.""" + """Manual step-through: mount queue head; if head already on gonio, mount the following one. + + The mounted head stays in the queue until /status confirms it left the + gonio (see ``_on_mounted_sample_changed``): a failed unmount must not + drop the sample that is still physically mounted. + """ queue = self.job_list_panel.table_model.samples mounted = getattr(self._latest_daq_status, "sample", None) - if queue and mounted is not None and mounted.db_id == queue[0].db_id: - self.job_list_panel.table_model.remove_sample(mounted.db_id) - queue = self.job_list_panel.table_model.samples - if not queue: - self._on_manual_unmount_requested() # or no-op; your call - return - self._on_manual_mount_requested(queue[0]) + head_mounted = bool(queue) and mounted is not None and mounted.db_id == queue[0].db_id + target = queue[1] if head_mounted else (queue[0] if queue else None) + if target is None: + sent = self._on_manual_unmount_requested() + else: + sent = self._on_manual_mount_requested(target) + if sent and head_mounted: + self._pending_unmount_pop = queue[0].db_id - def _on_manual_mount_requested(self, sample, reference: bool = False) -> None: + @Slot(object) + def _on_mounted_sample_changed(self, sample) -> None: + """Pop the pending queue head once /status shows it is off the gonio. + + Intentionally not cleared on ``operation_failed``: a busy/duplicate + POST error must not cancel the pop for an exchange still in flight. + """ + pending = self._pending_unmount_pop + if pending is None: + return + if sample is not None and sample.db_id == pending: + return + self._pending_unmount_pop = None + self.job_list_panel.remove_samples([pending]) + + def _on_manual_mount_requested(self, sample, reference: bool = False) -> bool: """Pre-check the hutch before sending a manual mount to the server. Gives the user an immediate pop-up if the door is open / alarm active, @@ -2188,10 +2212,11 @@ class MainWindow(QMainWindow): QMessageBox.critical(self, "Mounting Failed", reason) except Exception: logger.exception("Failed to show mount-blocked popup") - return + return False self.daq.mount(sample, reference) + return True - def _on_manual_unmount_requested(self) -> None: + def _on_manual_unmount_requested(self) -> bool: """Block a manual unmount if the hutch isn't ready (robot can't move).""" reason = self._hutch_blocks_mount() if reason is not None: @@ -2200,8 +2225,9 @@ class MainWindow(QMainWindow): QMessageBox.critical(self, "Unmounting Failed", reason) except Exception: logger.exception("Failed to show unmount-blocked popup") - return + return False self.daq.unmount() + return True def _precondition_ok(self) -> bool: """Run the shared ring-current / shutter / door 'continue?' check. diff --git a/tests/unit/gui/test_main_window.py b/tests/unit/gui/test_main_window.py index 36b718f3..c7f2e3ab 100644 --- a/tests/unit/gui/test_main_window.py +++ b/tests/unit/gui/test_main_window.py @@ -16,7 +16,7 @@ def mock_ui_state(): yield mock -def test_main_window_init(qtbot, mock_ui_state, daq_status_factory): +def test_main_window_init(qtbot, mock_ui_state, daq_status_factory, sample_info): with ( patch("requests.get") as mock_get, patch("aare.gui.main_window.DAQWorker"), @@ -117,6 +117,24 @@ def test_main_window_init(qtbot, mock_ui_state, daq_status_factory): win._on_manual_unmount_requested() unmount_mock.assert_called_once() + # Mount next: the mounted head stays queued until /status confirms it + # left the gonio, so a failed unmount never drops it from the queue. + head = sample_info + nxt = sample_info.model_copy(update={"db_id": 2, "sample_name": "sample2"}) + win.job_list_panel.table_model.updateData([head, nxt]) + mount_mock = cast(MagicMock, win.daq.mount) + win.update_daq_status(daq_status_factory(sample=head)) + win.sample_logic.update_daq_status(daq_status_factory(sample=head)) + with patch.object(win, "_hutch_blocks_mount", return_value=None): + win._mount_next_from_queue() + mount_mock.assert_called_once_with(nxt, False) + queued = lambda: [s.db_id for s in win.job_list_panel.table_model.samples] + assert queued() == [1, 2], "head must stay queued until unmount confirmed" + win.sample_logic.update_daq_status(daq_status_factory(sample=head)) + assert queued() == [1, 2], "head still mounted (unmount failed) -> keep it" + win.sample_logic.update_daq_status(daq_status_factory(sample=nxt)) + assert queued() == [2], "exchange confirmed by /status -> head popped" + # Motion watch: only the robot station switches to the combined # beamline view. Moving no longer does (users kept losing the sample # camera on short gonio moves), and busy alone never does — Sample