From bb39833d83f2951e12facaa3f13e170c28084cc9 Mon Sep 17 00:00:00 2001 From: menzel Date: Tue, 1 Sep 2026 14:54:00 +0200 Subject: [PATCH] feat(ddg2): make the detector-trigger readout time configurable The gap between consecutive detector triggers is exp_time minus a readout time that was a module constant fixed at 0.2 ms. Some detectors need more: FalconcSAXS declares MIN_READOUT = 3 ms, fifteen times longer. Nothing reconciled the two -- DDG2 does not know which detectors are in the scan, and the falcon only validates its exposure time, never the gap -- so a detector that cannot keep up silently dropped frames. There was also no way to change it. on_stage recomputes the pulse width from the constant on every scan, so a value set by hand from the client did not survive to the next acquisition. The readout times are now per-instance, settable two ways: a readout_times key in deviceConfig for a per-deployment default, and set_readout_times() via USER_ACCESS for a change between scans. Raising the value widens the gap and shortens the exposure by the same amount; burst_period stays at exp_time, so the frame rate is unaffected. Channel pair 'ab' is the one multiplexed to the detectors and normally the only one worth changing. A value that would exceed the exposure time is rejected, and a non-default value is logged at on_stage so a scan taken with a widened gap is recoverable from the logs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KLnmUurqcNd1FiDY5M2uZr --- .../epics/delay_generator_csaxs/ddg_2.py | 93 ++++++++++++++++++- .../test_delay_generator_csaxs.py | 62 +++++++++++++ 2 files changed, 151 insertions(+), 4 deletions(-) diff --git a/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_2.py b/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_2.py index 0141d5dc..5ee38fe5 100644 --- a/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_2.py +++ b/csaxs_bec/devices/epics/delay_generator_csaxs/ddg_2.py @@ -22,7 +22,10 @@ Burst mode is enabled: - Burst period is set to the exposure time. """ +from __future__ import annotations + import time +from typing import TYPE_CHECKING from bec_lib.logger import bec_logger from bec_server.scan_server.scans.scan_base import ScanInfo as ScanServerScanInfo @@ -42,6 +45,9 @@ from csaxs_bec.devices.epics.delay_generator_csaxs.delay_generator_csaxs import ) from csaxs_bec.devices.utils.utils import fetch_scan_info +if TYPE_CHECKING: # pragma: no cover + from bec_lib.devicemanager import DeviceManagerBase, ScanInfo + logger = bec_logger.logger ######################## @@ -113,6 +119,77 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS): distributes the trigger to the detectors. The DDG2 is triggered by the DDG1 through the EXT/EN channel. """ + # Methods exposed to the BEC client, i.e. callable as dev.ddg2.() + USER_ACCESS = ["get_readout_times", "set_readout_times"] + + def __init__( + self, + name: str, + prefix: str = "", + scan_info: ScanInfo | None = None, + device_manager: DeviceManagerBase | None = None, + readout_times: dict[str, float] | None = None, + **kwargs, + ): + """ + Args: + readout_times: per-channel-pair readout times in seconds, merged over + DEFAULT_READOUT_TIMES. Only the pairs given are overridden, e.g. + ``{"ab": 3.0e-3}``. See set_readout_times() for what this controls. + """ + super().__init__( + name=name, prefix=prefix, scan_info=scan_info, device_manager=device_manager, **kwargs + ) + self._readout_times: dict[str, float] = dict(DEFAULT_READOUT_TIMES) + if readout_times: + self.set_readout_times(**readout_times) + + def get_readout_times(self) -> dict[str, float]: + """Return the readout times per channel pair, in seconds. + + Returns: + dict[str, float]: a copy of the effective readout times. + """ + return dict(self._readout_times) + + def set_readout_times(self, **channels: float) -> dict[str, float]: + """Set the readout time of one or more channel pairs, in seconds. + + The readout time is subtracted from the exposure time to obtain the pulse + width, while the burst period stays at the exposure time. Raising it therefore + widens the gap between consecutive detector triggers and shortens the exposure + by the same amount; the frame rate is unaffected. + + This matters for detectors that need a minimum dead time between frames. The + default of 0.2 ms is shorter than some detectors can handle -- FalconcSAXS, for + instance, declares MIN_READOUT = 3 ms -- and a value that is too short shows up + as silently dropped frames rather than as an error. + + Channel pair 'ab' is the one multiplexed to the detectors, so it is normally the + only one worth changing. The new value applies from the next on_stage, i.e. the + next scan; it does not alter a scan already staged. + + Args: + **channels: channel pair to readout time in seconds, e.g. ``ab=3.0e-3``. + + Returns: + dict[str, float]: the effective readout times after the update. + + Raises: + ValueError: for an unknown channel pair or a non-positive value. + """ + for channel, value in channels.items(): + if channel not in self._readout_times: + raise ValueError( + f"Unknown channel pair {channel!r}; expected one of " + f"{sorted(self._readout_times)}" + ) + if value <= 0: + raise ValueError(f"readout time for {channel!r} must be > 0, got {value}") + self._readout_times[channel] = float(value) + logger.info(f"DDG {self.name} readout times set to {self._readout_times}") + return self.get_readout_times() + def on_init(self) -> None: """Initialize the device""" self.scan_parameters: ScanServerScanInfo | None = None @@ -193,9 +270,10 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS): # NOTE Check if the exposure time is longer than all readout times. # Raise a ValueError if requested exposure time is too short. - if any(exp_time <= rt for rt in DEFAULT_READOUT_TIMES.values()): + if any(exp_time <= rt for rt in self._readout_times.values()): raise ValueError( - f"Exposure time {exp_time} is too short for the readout times {DEFAULT_READOUT_TIMES}" + f"Exposure time {exp_time} is too short for the readout times " + f"{self._readout_times}" ) ######################################### @@ -208,8 +286,15 @@ class DDG2(PSIDeviceBase, DelayGeneratorCSAXS): if self.burst_period.get() != exp_time: self.burst_period.put(exp_time) - # Calculate the pulse width for the channel pair 'ab' - burst_pulse_width = exp_time - DEFAULT_READOUT_TIMES["ab"] + # Calculate the pulse width for the channel pair 'ab'. The readout time is the + # gap left between consecutive triggers; see set_readout_times(). + burst_pulse_width = exp_time - self._readout_times["ab"] + if self._readout_times["ab"] != DEFAULT_READOUT_TIMES["ab"]: + logger.info( + f"DDG {self.name} using non-default readout time " + f"{self._readout_times['ab']}s for 'ab' (default " + f"{DEFAULT_READOUT_TIMES['ab']}s); pulse width {burst_pulse_width}s" + ) # Trigger detectors with delay 0, and pulse width = exp_time - readout_time self.set_delay_pairs(channel="ab", delay=0, width=burst_pulse_width) diff --git a/tests/tests_devices/test_delay_generator_csaxs.py b/tests/tests_devices/test_delay_generator_csaxs.py index a484dae3..19810450 100644 --- a/tests/tests_devices/test_delay_generator_csaxs.py +++ b/tests/tests_devices/test_delay_generator_csaxs.py @@ -613,3 +613,65 @@ def test_ddg2_on_stop(mock_ddg2: DDG2): # mock_ddg2.burst_mode.put(1) # Enable burst mode # mock_ddg2.stop() # assert mock_ddg2.burst_mode.get() == 0 # Burst mode is disabled + + +def test_ddg2_readout_times_default(mock_ddg2: DDG2): + """Without an override the module defaults are in force.""" + assert mock_ddg2.get_readout_times() == DDG2_DEFAULT_READOUT_TIMES + # A copy, so a caller cannot mutate the device's state by accident. + mock_ddg2.get_readout_times()["ab"] = 1.0 + assert mock_ddg2.get_readout_times()["ab"] == DDG2_DEFAULT_READOUT_TIMES["ab"] + + +def test_ddg2_set_readout_times_validates(mock_ddg2: DDG2): + """Unknown pairs and non-positive values are rejected before they reach a scan.""" + with pytest.raises(ValueError): + mock_ddg2.set_readout_times(xy=1e-3) + with pytest.raises(ValueError): + mock_ddg2.set_readout_times(ab=0) + with pytest.raises(ValueError): + mock_ddg2.set_readout_times(ab=-1e-3) + assert mock_ddg2.get_readout_times() == DDG2_DEFAULT_READOUT_TIMES + + +def test_ddg2_set_readout_times_widens_the_gap(mock_ddg2: DDG2): + """A raised readout time shortens the pulse; the burst period is unchanged.""" + ddg = mock_ddg2 + exp_time = 0.1 + frames_per_trigger = 10 + ddg.on_connected() + ddg.scan_info.msg.info["exp_time"] = exp_time + ddg.scan_info.msg.info["frames_per_trigger"] = frames_per_trigger + + ddg.set_readout_times(ab=3e-3) + assert ddg.get_readout_times()["ab"] == 3e-3 + # Other pairs are untouched. + assert ddg.get_readout_times()["cd"] == DDG2_DEFAULT_READOUT_TIMES["cd"] + + ddg.stage() + assert np.isclose(ddg.ab.width.get(), exp_time - 3e-3) + assert ddg.burst_period.get() == exp_time # frame rate unaffected + ddg.unstage() + + +def test_ddg2_readout_times_from_device_config(): + """deviceConfig can set it per deployment, merged over the defaults.""" + with patched_device( + DDG2, name="ddg2", prefix="test_ddg2:", readout_times={"ab": 3e-3}, _mock_pv_initial_value=0 + ) as ddg: + try: + assert ddg.get_readout_times()["ab"] == 3e-3 + assert ddg.get_readout_times()["gh"] == DDG2_DEFAULT_READOUT_TIMES["gh"] + finally: + ddg.destroy() + + +def test_ddg2_too_long_readout_time_is_caught_at_stage(mock_ddg2: DDG2): + """A readout time exceeding the exposure must fail loudly, not emit a negative pulse.""" + ddg = mock_ddg2 + ddg.on_connected() + ddg.scan_info.msg.info["exp_time"] = 1e-3 + ddg.scan_info.msg.info["frames_per_trigger"] = 1 + ddg.set_readout_times(ab=3e-3) + with pytest.raises(ValueError): + ddg.stage()