fix(eiger): let each model keep its own readout time, and make it configurable

Readout is a property of the detector, not a beamline constant: a 9M has more
modules than a 1.5M, and the Falcon needs 3 ms against the delay generator's
200 us. The per-model constants therefore stay, with comments saying the
duplication is deliberate so nobody consolidates them again. They all hold 2e-4
today only because no measured per-model value exists yet.

Writing a test for the deviceConfig override exposed that it never worked. Both
subclasses passed readout_time to super() while also forwarding **kwargs, so
supplying it raised "got multiple values for keyword argument" -- and through
BEC it never even got that far, because readout_time was not a named parameter
of the subclass signature and the device server drops config keys it cannot see
(the same rule behind the recent prefix incident). Both subclasses now name it
with the model constant as default, and a test asserts the signatures keep it.

Also documents frame_time_us in DetectorSettings as required-but-ignored for the
Eiger, and warns that its 500 is microseconds while every other time in the
module is seconds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLnmUurqcNd1FiDY5M2uZr

csaxs_bec/devices/jungfraujoch/eiger.py

@@ -78,6 +78,11 @@ logger = bec_logger.logger
 # the trigger period allows.
 MIN_EXP_TIME = 500e-6  # s
 EIGER_READOUT_TIME = 2e-4  # s -- matches DDG2 DEFAULT_READOUT_TIMES["ab"]
+# NOTE Each model declares its own value and passes it here; that duplication is
+# deliberate, since readout is a property of the detector (a 9M has more modules than a
+# 1.5M, and the Falcon needs 3 ms). They all happen to be 2e-4 today only because no
+# measured per-model number exists yet. This constant is the fallback for a model that
+# does not state one; deviceConfig's readout_time overrides it per deployment.
 
 
 class EigerError(Exception):
@@ -303,6 +308,10 @@ class Eiger(PSIDeviceBase):
         # self.jfj_client.connect_and_initialise(timeout=10)
 
         # Setup Detector settings, here we may also set the energy already as this might be time consuming
+        # NOTE frame_time_us is required by the API but ignored for the Eiger ("For EIGER
+        # detector this is default frame time, not used otherwise"); the exposure comes
+        # from image_time_us in DatasetSettings. The 500 is a placeholder -- it is NOT the
+        # readout time, and it is in microseconds, unlike every other time in this module.
         settings = DetectorSettings(frame_time_us=int(500), timing=DetectorTiming.TRIGGER)
         self.jfj_client.set_detector_settings(settings, timeout=5)
 

csaxs_bec/devices/jungfraujoch/eiger_1_5m.py

@@ -11,9 +11,11 @@ from typing import TYPE_CHECKING
 
 from csaxs_bec.devices.jungfraujoch.eiger import Eiger
 
-# Gap between the end of an acquisition and the next trigger; must match the
-# delay generator's gap (DDG2 DEFAULT_READOUT_TIMES["ab"]).
-EIGER1_5M_READOUT_TIME = 2e-4  # s
+# Gap this model needs between the end of an acquisition and the next trigger. Kept
+# per model on purpose -- readout is a detector property, not a beamline constant. The
+# delay generator's gap (DDG2 DEFAULT_READOUT_TIMES["ab"]) must be at least as large as
+# the largest value among the detectors in a scan, or the slowest one overruns.
+EIGER1_5M_READOUT_TIME = 2e-4  # s -- placeholder until a measured value exists
 DETECTOR_NAME = "EIGER 1.5M"
 
 
@@ -42,12 +44,13 @@ class Eiger1_5M(Eiger):
         beam_center: tuple[float, float] = (0.0, 0.0),
         scan_info: ScanInfo = None,
         device_manager: DeviceManagerDS = None,
+        readout_time: float = EIGER1_5M_READOUT_TIME,
         **kwargs,
     ) -> None:
         super().__init__(
             name=name,
             detector_name=DETECTOR_NAME,
-            readout_time=EIGER1_5M_READOUT_TIME,
+            readout_time=readout_time,
             detector_distance=detector_distance,
             beam_center=beam_center,
             scan_info=scan_info,

csaxs_bec/devices/jungfraujoch/eiger_9m.py

@@ -20,9 +20,11 @@ if TYPE_CHECKING:  # pragma no cover
     from bec_lib.devicemanager import ScanInfo
     from bec_server.device_server.device_server import DeviceManagerDS
 
-# Gap between the end of an acquisition and the next trigger; must match the
-# delay generator's gap (DDG2 DEFAULT_READOUT_TIMES["ab"]).
-EIGER9M_READOUT_TIME = 2e-4  # s
+# Gap this model needs between the end of an acquisition and the next trigger. Kept
+# per model on purpose -- readout is a detector property, not a beamline constant. The
+# delay generator's gap (DDG2 DEFAULT_READOUT_TIMES["ab"]) must be at least as large as
+# the largest value among the detectors in a scan, or the slowest one overruns.
+EIGER9M_READOUT_TIME = 2e-4  # s -- placeholder until a measured value exists
 DETECTOR_NAME = "EIGER 9M"  # "EIGER 9M""
 
 
@@ -46,12 +48,13 @@ class Eiger9M(Eiger):
         beam_center: tuple[float, float] = (0.0, 0.0),
         scan_info: ScanInfo = None,
         device_manager: DeviceManagerDS = None,
+        readout_time: float = EIGER9M_READOUT_TIME,
         **kwargs,
     ) -> None:
         super().__init__(
             name=name,
             detector_name=DETECTOR_NAME,
-            readout_time=EIGER9M_READOUT_TIME,
+            readout_time=readout_time,
             detector_distance=detector_distance,
             beam_center=beam_center,
             scan_info=scan_info,

tests/tests_devices/test_eiger.py

@@ -1,4 +1,5 @@
 # pylint: skip-file
+import inspect
 import os
 import threading
 import time
@@ -23,8 +24,8 @@ from ophyd_devices.utils.psi_device_base_utils import DeviceStatus
 
 from csaxs_bec.devices.jungfraujoch.eiger import EigerError
 from csaxs_bec.devices.jungfraujoch.eiger import EIGER_READOUT_TIME, MIN_EXP_TIME
-from csaxs_bec.devices.jungfraujoch.eiger_1_5m import Eiger1_5M
-from csaxs_bec.devices.jungfraujoch.eiger_9m import Eiger9M
+from csaxs_bec.devices.jungfraujoch.eiger_1_5m import EIGER1_5M_READOUT_TIME, Eiger1_5M
+from csaxs_bec.devices.jungfraujoch.eiger_9m import EIGER9M_READOUT_TIME, Eiger9M
 
 if TYPE_CHECKING:  # pragma no cover
     from bec_lib.messages import FileMessage
@@ -537,3 +538,36 @@ def test_eiger_minimum_exposure_check_is_unchanged(eiger_1_5m):
     ):
         with pytest.raises(ValueError):
             eiger.stage()
+
+
+def test_eiger_models_may_declare_their_own_readout_time(eiger_1_5m, eiger_9m):
+    """Readout is a detector property, so each model states its own value.
+
+    They coincide today, but nothing should force them to: a 9M has more modules
+    than a 1.5M, and the Falcon needs 3 ms.
+    """
+    assert eiger_1_5m._readout_time == EIGER1_5M_READOUT_TIME
+    assert eiger_9m._readout_time == EIGER9M_READOUT_TIME
+
+
+def test_eiger_readout_time_is_overridable_per_deployment(mock_scan_info):
+    """deviceConfig's readout_time wins over the model's default."""
+    dev = Eiger1_5M(
+        name="eiger_1_5m", beam_center=(256, 256), detector_distance=100.0, readout_time=1.5e-3
+    )
+    try:
+        assert dev._readout_time == 1.5e-3
+    finally:
+        dev.destroy()
+
+
+def test_eiger_subclasses_name_readout_time_for_the_device_server():
+    """deviceConfig keys reach a device only if the class signature names them.
+
+    bec_server intersects the config keys with the named parameters of the class
+    (device_server/devices/devicemanager.py), so a subclass that only forwards
+    **kwargs silently drops readout_time -- and passing it explicitly used to raise
+    'got multiple values for keyword argument'.
+    """
+    for cls in (Eiger1_5M, Eiger9M):
+        assert "readout_time" in inspect.signature(cls).parameters