refactor: Eiger refactoring, fix test and add docs.

csaxs_bec/devices/jungfraujoch/README.MD

@@ -34,5 +34,15 @@ More information about the JungfrauJoch and API client can be found at: (https:/
 A thin wrapper for the JungfrauJoch API client is provided in the [jungfrau_joch_client](./jungfrau_joch_client.py). 
 Details about the specific integration are provided in the code. 
 
-## Eiger debugging
-For debugging the Eiger hardware, please contact the detector group for support.
+
+## Eiger implementation
+The Eiger detector integration is provided in the [eiger.py](./eiger.py) module. It provides a base integration for both Eiger 1.5M and Eiger 9M detectors.
+Logic specific to each detector is implemented in the respective modules:
+- [eiger_1_5m.py](./eiger_1_5m.py)
+- [eiger_9m.py](./eiger_9m.py)
+
+With the current implementation, the detector initialization should be done by a beamline scientist through the JungfrauJoch WEB UI by choosing the 
+appropriate detector (1.5M or 9M) before loading the device config with BEC. BEC will check upon connecting if the selected detector matches the expected one.
+A preview stream for images is also provided which is forwarded and accessible through the `preview_image` signal.
+
+For more specific details, please check the code documentation. 

csaxs_bec/devices/jungfraujoch/eiger.py

@@ -114,6 +114,7 @@ class Eiger(PSIDeviceBase):
         self._readout_time = readout_time
         self._full_path = ""
         self._num_triggers = 0
+        self._wait_for_on_complete = 20  # seconds
         if self.device_manager is not None:
             self.device_manager: DeviceManagerDS
 
@@ -351,7 +352,7 @@ class Eiger(PSIDeviceBase):
         def wait_for_complete():
             start_time = time.time()
             # NOTE: This adjust the time (s) that should be waited for completion of the scan.
-            timeout = 20  #
+            timeout = self._wait_for_on_complete
             while time.time() - start_time < timeout:
                 if self.jfj_client.wait_for_idle(timeout=1, raise_on_timeout=False):
                     # TODO: Once available, add check for
@@ -372,7 +373,7 @@ class Eiger(PSIDeviceBase):
             statistics: MeasurementStatistics = self.jfj_client.api.statistics_data_collection_get(
                 _request_timeout=5
             )
-            broker_status = self.jfj_client.jjf_state
+            broker_status = self.jfj_client.jfj_status
             raise TimeoutError(
                 f"Timeout after waiting for device {self.name} to complete for {time.time()-start_time:.2f}s \n \n"
                 f"Broker status: \n{yaml.dump(broker_status.to_dict(), indent=4)} \n \n"

csaxs_bec/devices/jungfraujoch/jungfrau_joch_client.py

@@ -66,7 +66,7 @@ class JungfrauJochClient:
         self._parent_name = parent.name if parent else self.__class__.__name__
 
     @property
-    def jjf_state(self) -> BrokerStatus:
+    def jfj_status(self) -> BrokerStatus:
         """Broker status of JungfrauJoch."""
         response = self.api.status_get()
         return BrokerStatus(**response.to_dict())
@@ -83,7 +83,7 @@ class JungfrauJochClient:
     # pylint: disable=missing-function-docstring
     @property
     def detector_state(self) -> DetectorState:
-        return DetectorState(self.jjf_state.state)
+        return DetectorState(self.jfj_status.state)
 
     def connect_and_initialise(self, timeout: int = 10) -> None:
         """

tests/tests_devices/test_eiger.py

@@ -5,6 +5,7 @@ from time import time
 from typing import TYPE_CHECKING, Generator
 from unittest import mock
 
+import numpy as np
 import pytest
 from bec_lib.messages import FileMessage, ScanStatusMessage
 from jfjoch_client.models.broker_status import BrokerStatus
@@ -78,7 +79,7 @@ def detector_list(request) -> Generator[DetectorList, None, None]:
             ),
             DetectorListElement(
                 id=2,
-                description="EIGER 8.5M (tmp)",
+                description="EIGER 9M",
                 serial_number="123456",
                 base_ipv4_addr="192.168.0.1",
                 udp_interface_count=1,
@@ -103,7 +104,11 @@ def eiger_1_5m(mock_scan_info) -> Generator[Eiger1_5M, None, None]:
     name = "eiger_1_5m"
     dev = Eiger1_5M(name=name, beam_center=(256, 256), detector_distance=100.0)
     dev.scan_info.msg = mock_scan_info
-    yield dev
+    try:
+        yield dev
+    finally:
+        if dev._destroyed is False:
+            dev.destroy()
 
 
 @pytest.fixture(scope="function")
@@ -113,7 +118,19 @@ def eiger_9m(mock_scan_info) -> Generator[Eiger9M, None, None]:
     name = "eiger_9m"
     dev = Eiger9M(name=name)
     dev.scan_info.msg = mock_scan_info
-    yield dev
+    try:
+        yield dev
+    finally:
+        if dev._destroyed is False:
+            dev.destroy()
+
+
+def test_eiger_wait_for_connection(eiger_1_5m, eiger_9m):
+    """Test the wait_for_connection metho is calling status_get on the JFJ API client."""
+    for eiger in (eiger_1_5m, eiger_9m):
+        with mock.patch.object(eiger.jfj_client.api, "status_get") as mock_status_get:
+            eiger.wait_for_connection(timeout=1)
+            mock_status_get.assert_called_once_with(_request_timeout=1)
 
 
 @pytest.mark.parametrize("detector_state", ["Idle", "Inactive"])
@@ -141,7 +158,7 @@ def test_eiger_1_5m_on_connected(eiger_1_5m, detector_list, detector_state):
         else:
             eiger.on_connected()
             assert mock_set_det.call_args == mock.call(
-                DetectorSettings(frame_time_us=500, timing=DetectorTiming.TRIGGER), timeout=10
+                DetectorSettings(frame_time_us=500, timing=DetectorTiming.TRIGGER), timeout=5
             )
             assert mock_file_writer.call_args == mock.call(
                 file_writer_settings=FileWriterSettings(
@@ -179,7 +196,7 @@ def test_eiger_9m_on_connected(eiger_9m, detector_list, detector_state):
         else:
             eiger.on_connected()
             assert mock_set_det.call_args == mock.call(
-                DetectorSettings(frame_time_us=500, timing=DetectorTiming.TRIGGER), timeout=10
+                DetectorSettings(frame_time_us=500, timing=DetectorTiming.TRIGGER), timeout=5
             )
             assert mock_file_writer.call_args == mock.call(
                 file_writer_settings=FileWriterSettings(
@@ -216,11 +233,39 @@ def test_eiger_on_stop(eiger_1_5m):
         stop_event.wait(timeout=5)  # Thread should be killed from task_handler
 
 
+def test_eiger_on_destroy(eiger_1_5m):
+    """Test the on_destroy logic of the Eiger detector. This is equivalent for 9M and 1_5M."""
+    eiger = eiger_1_5m
+    start_event = threading.Event()
+    stop_event = threading.Event()
+
+    def tmp_task():
+        start_event.set()
+        try:
+            while True:
+                time.sleep(0.1)
+        finally:
+            stop_event.set()
+
+    eiger.task_handler.submit_task(tmp_task)
+    start_event.wait(timeout=5)
+
+    with (
+        mock.patch.object(eiger.jfj_preview_client, "stop") as mock_jfj_preview_client_stop,
+        mock.patch.object(eiger.jfj_client, "stop") as mock_jfj_client_stop,
+    ):
+        eiger.on_destroy()
+        mock_jfj_preview_client_stop.assert_called_once()
+        mock_jfj_client_stop.assert_called_once()
+        stop_event.wait(timeout=5)
+
+
 @pytest.mark.timeout(25)
 @pytest.mark.parametrize("raise_timeout", [True, False])
 def test_eiger_on_complete(eiger_1_5m, raise_timeout):
     """Test the on_complete logic of the Eiger detector. This is equivalent for 9M and 1_5M."""
     eiger = eiger_1_5m
+    eiger._wait_for_on_complete = 1  # reduce wait time for testing
 
     callback_completed_event = threading.Event()
 
@@ -230,7 +275,7 @@ def test_eiger_on_complete(eiger_1_5m, raise_timeout):
 
     unblock_wait_for_idle = threading.Event()
 
-    def mock_wait_for_idle(timeout: int, request_timeout: float):
+    def mock_wait_for_idle(timeout: float, raise_on_timeout: bool) -> bool:
         if unblock_wait_for_idle.wait(timeout):
             if raise_timeout:
                 return False
@@ -238,11 +283,18 @@ def test_eiger_on_complete(eiger_1_5m, raise_timeout):
         return False
 
     with (
+        mock.patch.object(
+            eiger.jfj_client.api, "status_get", return_value=BrokerStatus(state="Idle")
+        ),
         mock.patch.object(eiger.jfj_client, "wait_for_idle", side_effect=mock_wait_for_idle),
         mock.patch.object(
             eiger.jfj_client.api,
             "statistics_data_collection_get",
-            return_value=MeasurementStatistics(run_number=1),
+            return_value=MeasurementStatistics(
+                run_number=1,
+                images_collected=eiger.scan_info.msg.num_points
+                * eiger.scan_info.msg.scan_parameters["frames_per_trigger"],
+            ),
         ),
     ):
         status = eiger.complete()
@@ -284,7 +336,7 @@ def test_eiger_file_event_callback(eiger_1_5m, tmp_path):
     assert file_msg.hinted_h5_entries == {"data": "entry/data/data"}
 
 
-def test_eiger_on_sage(eiger_1_5m):
+def test_eiger_on_stage(eiger_1_5m):
     """Test the on_stage and on_unstage logic of the Eiger detector. This is equivalent for 9M and 1_5M."""
     eiger = eiger_1_5m
     scan_msg = eiger.scan_info.msg
@@ -316,3 +368,35 @@ def test_eiger_on_sage(eiger_1_5m):
         )
         assert mock_start.call_args == mock.call(settings=data_settings)
         assert eiger.staged is Staged.yes
+
+
+def test_eiger_set_det_distance_test_beam_center(eiger_1_5m):
+    """Test the set_detector_distance and set_beam_center methods. Equivalent for 9M and 1_5M."""
+    eiger = eiger_1_5m
+    old_distance = eiger.detector_distance
+    new_distance = old_distance + 100
+    old_beam_center = eiger.beam_center
+    new_beam_center = (old_beam_center[0] + 20, old_beam_center[1] + 50)
+    eiger.set_detector_distance(new_distance)
+    assert eiger.detector_distance == new_distance
+    eiger.set_beam_center(x=new_beam_center[0], y=new_beam_center[1])
+    assert eiger.beam_center == new_beam_center
+    with pytest.raises(ValueError):
+        eiger.set_beam_center(x=-10, y=100)  # Cannot set negative beam center
+    with pytest.raises(ValueError):
+        eiger.detector_distance = -50  # Cannot set negative detector distance
+
+
+def test_eiger_preview_callback(eiger_1_5m):
+    """Preview callback test for the Eiger detector. This is equivalent for 9M and 1_5M."""
+    eiger = eiger_1_5m
+    # NOTE: I don't find models for the CBOR messages used by JFJ, currently using a dummay dict.
+    # Please adjust once the proper model is found.
+    for msg_type in ["start", "end", "image", "calibration", "metadata"]:
+        msg = {"type": msg_type, "data": {"default": np.array([[1, 2], [3, 4]])}}
+        with mock.patch.object(eiger.preview_image, "put") as mock_preview_put:
+            eiger._preview_callback(msg)
+            if msg_type == "image":
+                mock_preview_put.assert_called_once_with(msg["data"]["default"])
+            else:
+                mock_preview_put.assert_not_called()