diff --git a/csaxs_bec/devices/jungfraujoch/eiger.py b/csaxs_bec/devices/jungfraujoch/eiger.py index 4053b888..b5dec40c 100644 --- a/csaxs_bec/devices/jungfraujoch/eiger.py +++ b/csaxs_bec/devices/jungfraujoch/eiger.py @@ -365,7 +365,7 @@ class Eiger(PSIDeviceBase): """ start_time = time.time() logger.debug(f"On connected called for {self.name}") - self.jfj_client.stop(request_timeout=3) + self.jfj_client.stop(request_timeout=3, reason="on_connected") # Check which detector is selected # Get available detectors @@ -446,7 +446,7 @@ class Eiger(PSIDeviceBase): f"automatically, please initialise it in the WEB UI: {self._host}. {exc}" ) from exc if self.jfj_client.detector_state != DetectorState.IDLE: - self.jfj_client.stop(request_timeout=0.5) + self.jfj_client.stop(request_timeout=0.5, reason="on_stage, detector not IDLE") self.jfj_client.wait_for_idle(timeout=5) # TODO Reactivate @@ -641,7 +641,8 @@ class Eiger(PSIDeviceBase): def on_stop(self) -> None: """Called when the device is stopped.""" - self.jfj_client.stop(request_timeout=0.5) + logger.info(f"Device {self.name} on_stop called, cancelling JungfrauJoch acquisition.") + self.jfj_client.stop(request_timeout=0.5, reason="on_stop") self.task_handler.shutdown() def on_destroy(self): diff --git a/csaxs_bec/devices/jungfraujoch/jungfrau_joch_client.py b/csaxs_bec/devices/jungfraujoch/jungfrau_joch_client.py index 42837676..2e99453b 100644 --- a/csaxs_bec/devices/jungfraujoch/jungfrau_joch_client.py +++ b/csaxs_bec/devices/jungfraujoch/jungfrau_joch_client.py @@ -175,13 +175,24 @@ class JungfrauJochClient: f"re-initialising the detector (recovery #{self._recoveries} since startup)." ) # Capture the detector server state before the re-initialisation wipes it. - self.log_dectris_status(f"before re-init (recovery #{self._recoveries})") + dectris_status = self.log_dectris_status( + f"before re-init (recovery #{self._recoveries})" + ) try: self._initialize(timeout) except Exception as exc: + hint = "" + series = self._dectris_still_armed(dectris_status) + if series is not None: + hint = ( + f" Dectris detector server is still armed (state ready, series {series}) " + "from a previous acquisition that was not cancelled. JungfrauJoch cannot " + "re-initialise until it is disarmed. Disarm it on the Dectris server, or " + "re-seat the detector fibres." + ) raise JungfrauJochClientError( f"Error on device {self._parent_name}: re-initialisation after detector " - f"error failed: {exc}" + f"error failed: {exc}{hint}" ) from exc state = self.detector_state if state != DetectorState.IDLE: @@ -192,7 +203,7 @@ class JungfrauJochClient: logger.info(f"Device {self._parent_name} re-initialised and back in IDLE state.") return True - def log_dectris_status(self, reason: str, timeout: float = 2) -> None: + def log_dectris_status(self, reason: str, timeout: float = 2) -> dict | None: """ Log the status endpoints of the Dectris detector server (Simplon API) as a single warning line of compact JSON. Only read-only GET requests are made. Does nothing if no dectris_url @@ -202,9 +213,12 @@ class JungfrauJochClient: Args: reason (str): Why the status is logged, included in the log line. timeout (float): Timeout in seconds of each HTTP request. Requests run in parallel. + + Returns: + dict | None: The logged status values, None if disabled or the status could not be read. """ if self._dectris_url is None: - return + return None try: base = self._dectris_url urls = { @@ -229,8 +243,30 @@ class JungfrauJochClient: f"Dectris detector server status for device {self._parent_name} ({reason}): " f"{json.dumps(results, separators=(',', ':'), default=str)}" ) + return results except Exception as exc: logger.warning(f"Could not log Dectris detector server status ({reason}): {exc}") + return None + + @staticmethod + def _dectris_still_armed(results: dict | None) -> str | None: + """ + Check a status returned by log_dectris_status for a detector server that is still armed, + i.e. a series was started but never finished or cancelled. JungfrauJoch then refuses to + initialise with "Detector busy". + + Returns: + str | None: The series id of the armed series ("unknown" if not available), None if + the detector is not armed or the status is not available. + """ + if not isinstance(results, dict): + return None + stream = results.get("stream") + stream_state = stream.get("state") if isinstance(stream, dict) else None + if results.get("detector/state") != "ready" and stream_state != "acquire": + return None + series = results.get("detector/series_unique_id") + return series if isinstance(series, str) and not series.startswith("<") else "unknown" @staticmethod def _api_error_message(exc: Exception) -> str | None: @@ -336,12 +372,21 @@ class JungfrauJochClient: f"{yaml.dump(settings, indent=4)}. \nTraceback: {content}." ) - def stop(self, request_timeout: float = 0.5) -> None: - """Stop the acquisition, this only logs errors and is not raising.""" + def stop(self, request_timeout: float = 0.5, reason: str = "") -> None: + """ + Stop the acquisition, this only logs errors and is not raising. After the cancel, the + Dectris detector server status is logged (if configured) to show whether it disarmed. + + Args: + request_timeout (float): Timeout in seconds for the cancel request. + reason (str): Why the acquisition is stopped, included in the log lines. + """ def _stop_call(self): try: self.api.cancel_post_with_http_info() # (_request_timeout=request_timeout) + logger.info(f"Cancel sent to JungfrauJoch for device {self._parent_name} ({reason}).") + self.log_dectris_status(f"after cancel ({reason})") except requests.exceptions.Timeout: content = traceback.format_exc() logger.error( diff --git a/tests/tests_devices/test_eiger.py b/tests/tests_devices/test_eiger.py index afc3c192..06469cf4 100644 --- a/tests/tests_devices/test_eiger.py +++ b/tests/tests_devices/test_eiger.py @@ -167,6 +167,24 @@ def eiger_9m(mock_scan_info) -> Generator[Eiger9M, None, None]: dev.destroy() +@pytest.fixture(autouse=True) +def no_live_jfjoch_cancel(): + """ + The devices point to the production JungfrauJoch host, and destroy() -> on_stop() sends a + cancel from a background thread. Block it so tests never cancel a live acquisition, nor read + the Dectris detector server afterwards. Tests patching the client instance still override this. + """ + with mock.patch( + "jfjoch_client.api.default_api.DefaultApi.cancel_post_with_http_info", + side_effect=ConnectionError("tests must not reach JungfrauJoch"), + ): + yield + # Stop threads started by device teardown must finish while the patch is active. + for thread in threading.enumerate(): + if thread.name == "stop_jungfraujoch_thread": + thread.join(timeout=5) + + @pytest.fixture(scope="function") def eiger2_si_500k(mock_scan_info) -> Generator[Eiger2Si500K, None, None]: """Fixture for the Eiger 2 Si 500K device. @@ -997,3 +1015,93 @@ def test_eiger_on_complete_logs_dectris_status_in_error_state(eiger2_si_500k): with pytest.raises(EigerError): status.wait(timeout=5) mock_log.assert_called_once() + + +def test_jfj_client_log_dectris_status_returns_results(): + disabled = _jfj_client() + assert disabled.log_dectris_status("test") is None + + client = JungfrauJochClient(host="http://localhost:1", dectris_url="http://dcu") + response = mock.Mock() + response.json.return_value = {"value": "idle"} + with ( + mock.patch( + "csaxs_bec.devices.jungfraujoch.jungfrau_joch_client.requests.get", + return_value=response, + ), + mock.patch("csaxs_bec.devices.jungfraujoch.jungfrau_joch_client.logger"), + ): + results = client.log_dectris_status("test") + assert results["detector/state"] == "idle" + + +@pytest.mark.parametrize( + "results, expected", + [ + ({"detector/state": "ready", "detector/series_unique_id": "S1"}, "S1"), + ({"detector/state": "idle", "stream": {"state": "acquire"}}, "unknown"), + ({"detector/state": "ready", "detector/series_unique_id": ""}, "unknown"), + ({"detector/state": "idle", "stream": {"state": "ready"}}, None), + ({"detector/state": "", "stream": ""}, None), + ({}, None), + (None, None), + ], +) +def test_jfj_client_dectris_still_armed(results, expected): + assert JungfrauJochClient._dectris_still_armed(results) == expected + + +@pytest.mark.parametrize( + "dectris_status, hint_expected", + [ + ( + { + "detector/state": "ready", + "detector/series_unique_id": "01M4J4FPRPZ2J7M6MW2YSA27VG", + "stream": {"state": "acquire"}, + }, + True, + ), + ({"detector/state": "idle", "stream": {"state": "ready"}}, False), + (None, False), + ], +) +def test_jfj_client_recover_from_error_hints_armed_dectris(dectris_status, hint_expected): + """A failed re-init tells the operator when the Dectris server is still armed.""" + client = JungfrauJochClient(host="http://localhost:1", dectris_url="http://dcu") + with ( + mock.patch.object( + JungfrauJochClient, + "detector_state", + new_callable=mock.PropertyMock, + return_value=DetectorState.ERROR, + ), + mock.patch.object(client, "log_dectris_status", return_value=dectris_status), + mock.patch.object(client.api, "initialize_post"), + mock.patch.object( + client, "wait_for_idle", side_effect=JungfrauJochClientError("Detector busy") + ), + ): + with pytest.raises(JungfrauJochClientError, match="Detector busy") as exc_info: + client.recover_from_error() + message = str(exc_info.value) + assert ("still armed" in message) is hint_expected + assert ("01M4J4FPRPZ2J7M6MW2YSA27VG" in message) is hint_expected + + +def test_jfj_client_stop_logs_cancel_and_dectris_status(): + client = JungfrauJochClient(host="http://localhost:1", dectris_url="http://dcu") + calls = mock.Mock() + with ( + mock.patch.object(client.api, "cancel_post_with_http_info", calls.cancel), + mock.patch.object(client, "log_dectris_status", calls.log), + mock.patch("csaxs_bec.devices.jungfraujoch.jungfrau_joch_client.logger") as mock_logger, + mock.patch( + "csaxs_bec.devices.jungfraujoch.jungfrau_joch_client.threading.Thread", + side_effect=lambda target, args, **kwargs: mock.Mock(start=lambda: target(*args)), + ), + ): + client.stop(reason="on_stop") + assert [c[0] for c in calls.mock_calls] == ["cancel", "log"] + calls.log.assert_called_once_with("after cancel (on_stop)") + assert "on_stop" in mock_logger.info.call_args[0][0]