fix(eiger): log JungfrauJoch cancel and flag a Dectris server left armed
CI for csaxs_bec / test (push) Failing after 3m15s
CI for csaxs_bec / test (push) Failing after 3m15s
On 2026-10-10 an interlock abort left the Eiger2 Si 500K Dectris server armed (state ready, stream acquire), and every re-init then failed with "Detector busy" until the fibres were re-seated. The cancel path logged nothing on success, so it was not possible to tell what happened. - stop() logs a successful cancel with its reason, then the Dectris status - recover_from_error() adds a hint to the error when Dectris is still armed - on_stop logs that it was called - tests: block cancel to the production JungfrauJoch host from fixtures Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1MYT2ByPys8XL6BdfApAF
This commit is contained in:
1 parent
53f74c67f0
commit
d56a2a6d47
3 files changed
+163
-9
No files matched your search
@@ -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):
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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": "<TimeoutError: x>"}, "unknown"),
|
||||
({"detector/state": "idle", "stream": {"state": "ready"}}, None),
|
||||
({"detector/state": "<TimeoutError: x>", "stream": "<TimeoutError: x>"}, 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]
|
||||
Reference in new issue
Block a user