From d14f19438ba9e34c305eb24d4addd41f35335ac3 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 15 Aug 2024 14:19:23 +0100 Subject: [PATCH 01/11] Add PVs for fastcs eiger --- src/ophyd_async/epics/eiger/_eiger_io.py | 35 ++++++++++++++++++++++++ src/ophyd_async/epics/eiger/eiger.py | 10 +++++++ 2 files changed, 45 insertions(+) create mode 100644 src/ophyd_async/epics/eiger/_eiger_io.py create mode 100644 src/ophyd_async/epics/eiger/eiger.py diff --git a/src/ophyd_async/epics/eiger/_eiger_io.py b/src/ophyd_async/epics/eiger/_eiger_io.py new file mode 100644 index 000000000..ce8295ddc --- /dev/null +++ b/src/ophyd_async/epics/eiger/_eiger_io.py @@ -0,0 +1,35 @@ +from ophyd_async.core import Device +from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw_rbv, epics_signal_x + + +class EigerDriverIO(Device): + def __init__(self, prefix: str, name: str = "") -> None: + self.bit_depth = epics_signal_r( + int, f"{prefix}BitDepthReadout" + ) # Should be enum + self.stale_parameters = epics_signal_r(bool, f"{prefix}StaleParameters") + self.state = epics_signal_r(str, f"{prefix}DetectorState") # Should be enum + self.roi_mode = epics_signal_rw_rbv(str, f"{prefix}RoiMode") # Should be bool + + self.acquire_time = epics_signal_rw_rbv(float, f"{prefix}CountTime") + self.acquire_period = epics_signal_rw_rbv(float, f"{prefix}FrameTime") + + self.num_images = epics_signal_rw_rbv(int, f"{prefix}Nimages") + self.num_triggers = epics_signal_rw_rbv(int, f"{prefix}Ntrigger") + + self.trigger_mode = epics_signal_rw_rbv(str, f"{prefix}TriggerMode") + + self.arm = epics_signal_x(f"{prefix}Arm") + self.disarm = epics_signal_x(f"{prefix}Disarm") + self.abort = epics_signal_x(f"{prefix}Abort") + + self.beam_centre_x = epics_signal_rw_rbv(float, f"{prefix}BeamCenterX") + self.beam_centre_y = epics_signal_rw_rbv(float, f"{prefix}BeamCenterY") + + self.det_distance = epics_signal_rw_rbv(float, f"{prefix}DetectorDistance") + self.omega_start = epics_signal_rw_rbv(float, f"{prefix}OmegaStart") + self.omega_increment = epics_signal_rw_rbv(float, f"{prefix}OmegaIncrement") + + self.photon_energy = epics_signal_rw_rbv(float, f"{prefix}PhotonEnergy") + + super().__init__(name) diff --git a/src/ophyd_async/epics/eiger/eiger.py b/src/ophyd_async/epics/eiger/eiger.py new file mode 100644 index 000000000..f3625f2aa --- /dev/null +++ b/src/ophyd_async/epics/eiger/eiger.py @@ -0,0 +1,10 @@ +from bluesky.run_engine import RunEngine + +from ophyd_async.core import DeviceCollector + +from ._eiger_io import EigerDriverIO + +RE = RunEngine() + +with DeviceCollector(): + driver = EigerDriverIO("EIGER-455:") From eca5850e5c916aa57d4f3773dd4a47af289ee931 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 15 Aug 2024 15:45:11 +0100 Subject: [PATCH 02/11] Add odin PVs --- src/ophyd_async/epics/eiger/_odin_io.py | 61 +++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/ophyd_async/epics/eiger/_odin_io.py diff --git a/src/ophyd_async/epics/eiger/_odin_io.py b/src/ophyd_async/epics/eiger/_odin_io.py new file mode 100644 index 000000000..b3042fa30 --- /dev/null +++ b/src/ophyd_async/epics/eiger/_odin_io.py @@ -0,0 +1,61 @@ +from enum import Enum + +from ophyd_async.core import Device, DeviceVector +from ophyd_async.epics.signal import ( + epics_signal_r, + epics_signal_rw_rbv, + epics_signal_w, + epics_signal_x, +) + + +class Writing(str, Enum): + ON = "ON" + OFF = "OFF" + + +class OdinNode(Device): + def __init__(self, prefix: str, name: str = "") -> None: + self.writing = epics_signal_r(Writing, f"{prefix}HDF:Writing") + + # Cannot find: + # FPClearErrors + # FPErrorState_RBV + # FPErrorMessage_RBV + + self.connected = epics_signal_r( + bool, f"{prefix}Connected" + ) # Assuming this is both FPProcessConnected_RBV and FRProcessConnected_RBV + + # Usually assert that FramesTimedOut_RBV and FramesDropped_RBV are 0 + # , do these exist or do we want to assert something else? + + super().__init__(name) + + +class Odin(Device): + def __init__(self, prefix: str, name: str = "") -> None: + self.nodes = DeviceVector({i: OdinNode(f"{prefix}FP{i}:") for i in range(4)}) + + self.capture = epics_signal_x(f"{prefix}ConfigHdfWrite") + self.num_captured = epics_signal_r(int, f"{prefix}FramesWritten") + self.num_to_capture = epics_signal_w(int, f"{prefix}Frames") + + self.start_timeout = epics_signal_rw_rbv(int, f"{prefix}TimeoutTimerPeriod") + + self.image_height = epics_signal_rw_rbv(int, f"{prefix}DatasetDataDims0") + self.image_width = epics_signal_rw_rbv(int, f"{prefix}DatasetDataDims1") + + self.num_row_chunks = epics_signal_rw_rbv(int, f"{prefix}DatasetDataChunks1") + self.num_col_chunks = epics_signal_rw_rbv(int, f"{prefix}DatasetDataChunks2") + + self.file_path = epics_signal_rw_rbv(str, f"{prefix}ConfigHdfFilePath") + self.file_name = epics_signal_rw_rbv(str, f"{prefix}ConfigHdfFilePrefix") + + self.acquisition_id = epics_signal_rw_rbv( + str, f"{prefix}ConfigHdfAcquisitionId" + ) + + self.data_type = epics_signal_rw_rbv(str, f"{prefix}DatasetDataDatatype") + + super().__init__(name) From 2c4d50932c514561366ac8c62de62dfaaedb39b4 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 15 Aug 2024 16:15:39 +0100 Subject: [PATCH 03/11] Add ability to change photon energy --- src/ophyd_async/epics/eiger/__init__.py | 0 .../epics/eiger/{eiger.py => _eiger.py} | 3 +- src/ophyd_async/epics/eiger/_eiger_io.py | 37 +++++++++++++++---- tests/epics/eiger/test_eiger_io.py | 32 ++++++++++++++++ 4 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 src/ophyd_async/epics/eiger/__init__.py rename src/ophyd_async/epics/eiger/{eiger.py => _eiger.py} (74%) create mode 100644 tests/epics/eiger/test_eiger_io.py diff --git a/src/ophyd_async/epics/eiger/__init__.py b/src/ophyd_async/epics/eiger/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/ophyd_async/epics/eiger/eiger.py b/src/ophyd_async/epics/eiger/_eiger.py similarity index 74% rename from src/ophyd_async/epics/eiger/eiger.py rename to src/ophyd_async/epics/eiger/_eiger.py index f3625f2aa..2b675c973 100644 --- a/src/ophyd_async/epics/eiger/eiger.py +++ b/src/ophyd_async/epics/eiger/_eiger.py @@ -1,8 +1,7 @@ from bluesky.run_engine import RunEngine from ophyd_async.core import DeviceCollector - -from ._eiger_io import EigerDriverIO +from ophyd_async.epics.eiger._eiger_io import EigerDriverIO RE = RunEngine() diff --git a/src/ophyd_async/epics/eiger/_eiger_io.py b/src/ophyd_async/epics/eiger/_eiger_io.py index ce8295ddc..a9e858b25 100644 --- a/src/ophyd_async/epics/eiger/_eiger_io.py +++ b/src/ophyd_async/epics/eiger/_eiger_io.py @@ -1,15 +1,37 @@ -from ophyd_async.core import Device +from enum import Enum + +from ophyd_async.core import AsyncStatus, Device from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw_rbv, epics_signal_x +class EigerTriggerMode(str, Enum): + internal = "ints" + edge = "exts" + gate = "exte" + + +class PhotonEnergy(Device): + """Changing photon energy takes some time so only do so if the current energy is + outside the tolerance.""" + + def __init__(self, prefix, tolerance=0.1, name: str = "") -> None: + self._photon_energy = epics_signal_rw_rbv(float, f"{prefix}PhotonEnergy") + self.tolerance = tolerance + super().__init__(name) + + @AsyncStatus.wrap + async def set(self, value): + current_energy = await self._photon_energy.get_value() + if abs(current_energy - value) > self.tolerance: + await self._photon_energy.set(value) + + class EigerDriverIO(Device): def __init__(self, prefix: str, name: str = "") -> None: - self.bit_depth = epics_signal_r( - int, f"{prefix}BitDepthReadout" - ) # Should be enum + self.bit_depth = epics_signal_r(int, f"{prefix}BitDepthReadout") self.stale_parameters = epics_signal_r(bool, f"{prefix}StaleParameters") - self.state = epics_signal_r(str, f"{prefix}DetectorState") # Should be enum - self.roi_mode = epics_signal_rw_rbv(str, f"{prefix}RoiMode") # Should be bool + self.state = epics_signal_r(str, f"{prefix}DetectorState") + self.roi_mode = epics_signal_rw_rbv(str, f"{prefix}RoiMode") self.acquire_time = epics_signal_rw_rbv(float, f"{prefix}CountTime") self.acquire_period = epics_signal_rw_rbv(float, f"{prefix}FrameTime") @@ -17,6 +39,7 @@ def __init__(self, prefix: str, name: str = "") -> None: self.num_images = epics_signal_rw_rbv(int, f"{prefix}Nimages") self.num_triggers = epics_signal_rw_rbv(int, f"{prefix}Ntrigger") + # Ideally this will be a EigerTriggerMode, see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 self.trigger_mode = epics_signal_rw_rbv(str, f"{prefix}TriggerMode") self.arm = epics_signal_x(f"{prefix}Arm") @@ -30,6 +53,6 @@ def __init__(self, prefix: str, name: str = "") -> None: self.omega_start = epics_signal_rw_rbv(float, f"{prefix}OmegaStart") self.omega_increment = epics_signal_rw_rbv(float, f"{prefix}OmegaIncrement") - self.photon_energy = epics_signal_rw_rbv(float, f"{prefix}PhotonEnergy") + self.photon_energy = PhotonEnergy(prefix) super().__init__(name) diff --git a/tests/epics/eiger/test_eiger_io.py b/tests/epics/eiger/test_eiger_io.py new file mode 100644 index 000000000..e94180b28 --- /dev/null +++ b/tests/epics/eiger/test_eiger_io.py @@ -0,0 +1,32 @@ +from pytest import fixture + +from ophyd_async.core import DeviceCollector, get_mock_put, set_mock_value +from ophyd_async.epics.eiger._eiger_io import PhotonEnergy + + +@fixture +def photon_energy(RE): + with DeviceCollector(mock=True): + photon_energy = PhotonEnergy("") + return photon_energy + + +async def test_given_energy_within_tolerance_when_photon_energy_set_then_pv_unchanged( + photon_energy, +): + initial_energy = 10 + set_mock_value(photon_energy._photon_energy, initial_energy) + await photon_energy.set(10.002) + get_mock_put(photon_energy._photon_energy).assert_not_called() + assert (await photon_energy._photon_energy.get_value()) == initial_energy + + +async def test_given_energy_outside_tolerance_when_photon_energy_set_then_pv_changed( + photon_energy, +): + initial_energy = 10 + new_energy = 15 + set_mock_value(photon_energy._photon_energy, initial_energy) + await photon_energy.set(new_energy) + get_mock_put(photon_energy._photon_energy).assert_called_once() + assert (await photon_energy._photon_energy.get_value()) == new_energy From aebc98c48c0adf01bb5dc5ed4fa43d1b1b79f26b Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 19 Aug 2024 12:00:42 +0100 Subject: [PATCH 04/11] Add basic device and system test --- pyproject.toml | 2 +- src/ophyd_async/epics/eiger/__init__.py | 9 ++ src/ophyd_async/epics/eiger/_eiger.py | 35 ++++++-- .../epics/eiger/_eiger_controller.py | 48 ++++++++++ src/ophyd_async/epics/eiger/_eiger_io.py | 8 +- src/ophyd_async/epics/eiger/_odin_io.py | 81 +++++++++++++++-- system_tests/epics/eiger/README.md | 8 ++ .../epics/eiger/start_iocs_and_run_tests.sh | 15 ++++ system_tests/epics/eiger/test_eiger_system.py | 88 +++++++++++++++++++ 9 files changed, 278 insertions(+), 16 deletions(-) create mode 100644 src/ophyd_async/epics/eiger/_eiger_controller.py create mode 100644 system_tests/epics/eiger/README.md create mode 100755 system_tests/epics/eiger/start_iocs_and_run_tests.sh create mode 100644 system_tests/epics/eiger/test_eiger_system.py diff --git a/pyproject.toml b/pyproject.toml index 83dcb9ca2..3e3aa76de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -138,7 +138,7 @@ commands = [tool.ruff] -src = ["src", "tests"] +src = ["src", "tests", "system_tests"] line-length = 88 lint.select = [ "C4", # flake8-comprehensions - https://beta.ruff.rs/docs/rules/#flake8-comprehensions-c4 diff --git a/src/ophyd_async/epics/eiger/__init__.py b/src/ophyd_async/epics/eiger/__init__.py index e69de29bb..829d768de 100644 --- a/src/ophyd_async/epics/eiger/__init__.py +++ b/src/ophyd_async/epics/eiger/__init__.py @@ -0,0 +1,9 @@ +from ._eiger import EigerDetector +from ._eiger_controller import EigerController +from ._eiger_io import EigerDriverIO + +__all__ = [ + "EigerDetector", + "EigerController", + "EigerDriverIO", +] diff --git a/src/ophyd_async/epics/eiger/_eiger.py b/src/ophyd_async/epics/eiger/_eiger.py index 2b675c973..a81580691 100644 --- a/src/ophyd_async/epics/eiger/_eiger.py +++ b/src/ophyd_async/epics/eiger/_eiger.py @@ -1,9 +1,32 @@ -from bluesky.run_engine import RunEngine +from ophyd_async.core import PathProvider, StandardDetector -from ophyd_async.core import DeviceCollector -from ophyd_async.epics.eiger._eiger_io import EigerDriverIO +from ._eiger_controller import EigerController +from ._eiger_io import EigerDriverIO +from ._odin_io import Odin, OdinWriter -RE = RunEngine() -with DeviceCollector(): - driver = EigerDriverIO("EIGER-455:") +class EigerDetector(StandardDetector): + """ + Ophyd-async implementation of an ADKinetix Detector. + https://github.com/NSLS-II/ADKinetix + """ + + _controller: EigerController + _writer: Odin + + def __init__( + self, + prefix: str, + path_provider: PathProvider, + drv_suffix="-EA-EIGER-01:", + hdf_suffix="-EA-ODIN-01:", + name="", + ): + self.drv = EigerDriverIO(prefix + drv_suffix) + self.odin = Odin(prefix + hdf_suffix + "FP:") + + super().__init__( + EigerController(self.drv), + OdinWriter(path_provider, lambda: "", self.odin), + name=name, + ) diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py new file mode 100644 index 000000000..2219ad028 --- /dev/null +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -0,0 +1,48 @@ +import asyncio +from typing import Optional + +from ophyd_async.core import ( + AsyncStatus, + DetectorControl, + DetectorTrigger, + set_and_wait_for_other_value, +) + +from ._eiger_io import EigerDriverIO, EigerTriggerMode + +EIGER_TRIGGER_MODE_MAP = { + DetectorTrigger.internal: EigerTriggerMode.internal, + DetectorTrigger.constant_gate: EigerTriggerMode.gate, + DetectorTrigger.variable_gate: EigerTriggerMode.gate, + DetectorTrigger.edge_trigger: EigerTriggerMode.edge, +} + + +class EigerController(DetectorControl): + def __init__( + self, + driver: EigerDriverIO, + ) -> None: + self._drv = driver + + def get_deadtime(self, exposure: float) -> float: + return 0.0001 + + @AsyncStatus.wrap + async def arm( + self, + num: int, + trigger: DetectorTrigger = DetectorTrigger.internal, + exposure: Optional[float] = None, + ): + await asyncio.gather( + self._drv.trigger_mode.set(EIGER_TRIGGER_MODE_MAP[trigger]), + self._drv.num_images.set(num), + self._drv.acquire_time.set(exposure), + self._drv.acquire_period.set(exposure), + ) + + await set_and_wait_for_other_value(self._drv.arm, 1, self._drv.state, "ready") + + async def disarm(self): + await self._drv.disarm.set(1) diff --git a/src/ophyd_async/epics/eiger/_eiger_io.py b/src/ophyd_async/epics/eiger/_eiger_io.py index a9e858b25..1df3c8626 100644 --- a/src/ophyd_async/epics/eiger/_eiger_io.py +++ b/src/ophyd_async/epics/eiger/_eiger_io.py @@ -1,7 +1,7 @@ from enum import Enum from ophyd_async.core import AsyncStatus, Device -from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw_rbv, epics_signal_x +from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw_rbv, epics_signal_w class EigerTriggerMode(str, Enum): @@ -42,9 +42,9 @@ def __init__(self, prefix: str, name: str = "") -> None: # Ideally this will be a EigerTriggerMode, see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 self.trigger_mode = epics_signal_rw_rbv(str, f"{prefix}TriggerMode") - self.arm = epics_signal_x(f"{prefix}Arm") - self.disarm = epics_signal_x(f"{prefix}Disarm") - self.abort = epics_signal_x(f"{prefix}Abort") + self.arm = epics_signal_w(int, f"{prefix}Arm") + self.disarm = epics_signal_w(int, f"{prefix}Disarm") + self.abort = epics_signal_w(int, f"{prefix}Abort") self.beam_centre_x = epics_signal_rw_rbv(float, f"{prefix}BeamCenterX") self.beam_centre_y = epics_signal_rw_rbv(float, f"{prefix}BeamCenterY") diff --git a/src/ophyd_async/epics/eiger/_odin_io.py b/src/ophyd_async/epics/eiger/_odin_io.py index b3042fa30..15ff4d87b 100644 --- a/src/ophyd_async/epics/eiger/_odin_io.py +++ b/src/ophyd_async/epics/eiger/_odin_io.py @@ -1,11 +1,22 @@ +import asyncio from enum import Enum - -from ophyd_async.core import Device, DeviceVector +from typing import AsyncGenerator, AsyncIterator, Dict + +from bluesky.protocols import DataKey, StreamAsset + +from ophyd_async.core import ( + DEFAULT_TIMEOUT, + DetectorWriter, + Device, + DeviceVector, + NameProvider, + PathProvider, + observe_value, +) from ophyd_async.epics.signal import ( epics_signal_r, epics_signal_rw_rbv, epics_signal_w, - epics_signal_x, ) @@ -37,9 +48,9 @@ class Odin(Device): def __init__(self, prefix: str, name: str = "") -> None: self.nodes = DeviceVector({i: OdinNode(f"{prefix}FP{i}:") for i in range(4)}) - self.capture = epics_signal_x(f"{prefix}ConfigHdfWrite") + self.capture = epics_signal_w(Writing, f"{prefix}ConfigHdfWrite") self.num_captured = epics_signal_r(int, f"{prefix}FramesWritten") - self.num_to_capture = epics_signal_w(int, f"{prefix}Frames") + self.num_to_capture = epics_signal_rw_rbv(int, f"{prefix}ConfigHdfFrames") self.start_timeout = epics_signal_rw_rbv(int, f"{prefix}TimeoutTimerPeriod") @@ -59,3 +70,63 @@ def __init__(self, prefix: str, name: str = "") -> None: self.data_type = epics_signal_rw_rbv(str, f"{prefix}DatasetDataDatatype") super().__init__(name) + + +class OdinWriter(DetectorWriter): + def __init__( + self, + path_provider: PathProvider, + name_provider: NameProvider, + odin_driver: Odin, + ) -> None: + self._drv = odin_driver + self._path_provider = path_provider + self._name_provider = name_provider + super().__init__() + + async def open(self, multiplier: int = 1) -> Dict[str, DataKey]: + info = self._path_provider(device_name=self._name_provider()) + + await asyncio.gather( + self._drv.file_path.set(str(info.directory_path)), + self._drv.file_name.set(info.filename), + self._drv.data_type.set("uint16"), # TODO: get from eiger + self._drv.num_to_capture.set(1), + ) + + # await wait_for_value(self._drv.acquisition_id, info.filename, DEFAULT_TIMEOUT) + + await self._drv.capture.set(Writing.ON) + + return await self._describe() + + async def _describe(self) -> Dict[str, DataKey]: + """ + Return a describe based on the datasets PV + """ + # TODO: fill this in properly + return { + "data": DataKey( + source="", + shape=[], + dtype="number", + dtype_numpy=" AsyncGenerator[int, None]: + async for num_captured in observe_value(self._drv.num_captured, timeout): + yield num_captured + + async def get_indices_written(self) -> int: + return await self._drv.num_captured.get_value() + + def collect_stream_docs(self, indices_written: int) -> AsyncIterator[StreamAsset]: + # TODO + pass + + async def close(self) -> None: + await self._drv.capture.set(Writing.OFF) diff --git a/system_tests/epics/eiger/README.md b/system_tests/epics/eiger/README.md new file mode 100644 index 000000000..8080a5339 --- /dev/null +++ b/system_tests/epics/eiger/README.md @@ -0,0 +1,8 @@ +This system test runs against the eiger tickit sim. To run it: + +0. Ensure you have disabled SELinux (https://dev-portal.diamond.ac.uk/guide/containers/tutorials/podman/#enable-use-of-vscode-features) +1. Run `podman run --rm -it -v /dev/shm:/dev/shm -v /tmp:/tmp --net=host ghcr.io/diamondlightsource/eiger-detector-runtime:1.16.0beta5` this will bring up the simulator itself. +2. In a separate terminal load a python environment with `ophyd-async` in it +3. `cd system_tests/epics/eiger` and `./start_iocs_and_run_tests.sh` + + diff --git a/system_tests/epics/eiger/start_iocs_and_run_tests.sh b/system_tests/epics/eiger/start_iocs_and_run_tests.sh new file mode 100755 index 000000000..cc5b92bfd --- /dev/null +++ b/system_tests/epics/eiger/start_iocs_and_run_tests.sh @@ -0,0 +1,15 @@ + +host=$(hostname | tr -cd '[:digit:]') +export eiger_ioc=EIGER-$host +export odin_ioc=ODIN-$host + +mkdir /tmp/opi + +podman run --rm -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc + +podman run --rm -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc + +sleep 1 + +pytest . + diff --git a/system_tests/epics/eiger/test_eiger_system.py b/system_tests/epics/eiger/test_eiger_system.py new file mode 100644 index 000000000..c166fc92f --- /dev/null +++ b/system_tests/epics/eiger/test_eiger_system.py @@ -0,0 +1,88 @@ +import asyncio +import os +from pathlib import Path + +import h5py +import pytest +from bluesky.run_engine import RunEngine + +from ophyd_async.core import ( + DetectorTrigger, + Device, + DeviceCollector, + StaticPathProvider, + TriggerInfo, +) +from ophyd_async.epics.eiger import EigerDetector +from ophyd_async.epics.signal import epics_signal_rw + +EIGER_PREFIX = os.environ["eiger_ioc"] + ":" +ODIN_PREFIX = os.environ["odin_ioc"] + ":" +SAVE_PATH = "/tmp" + + +class SetupDevice(Device): + """Holds PVs that we would either expect to be initially set and + never change or to be externally set in prod.""" + + def __init__(self, eiger_prefix: str, odin_prefix: str) -> None: + self.trigger = epics_signal_rw(int, f"{eiger_prefix}Trigger") + self.header_detail = epics_signal_rw(str, f"{eiger_prefix}HeaderDetail") + self.compression = epics_signal_rw(str, f"{odin_prefix}DatasetDataCompression") + self.frames_per_block = epics_signal_rw( + int, f"{odin_prefix}ProcessFramesPerBlock" + ) + self.blocks_per_file = epics_signal_rw( + int, f"{odin_prefix}ProcessBlocksPerFile" + ) + super().__init__("") + + +@pytest.fixture +def RE(): + return RunEngine() + + +@pytest.fixture +async def setup_device(RE): + async with DeviceCollector(): + device = SetupDevice(EIGER_PREFIX, ODIN_PREFIX + "FP:") + await asyncio.gather( + device.header_detail.set("all"), + device.compression.set("BSLZ4"), + device.frames_per_block.set(1000), + device.blocks_per_file.set(1), + ) + + return device + + +@pytest.fixture +async def test_eiger(RE) -> EigerDetector: + provider = StaticPathProvider(lambda: "test_eiger", Path(SAVE_PATH)) + async with DeviceCollector(): + test_eiger = EigerDetector("", provider, EIGER_PREFIX, ODIN_PREFIX) + + return test_eiger + + +async def test_trigger_saves_file(test_eiger: EigerDetector, setup_device: SetupDevice): + single_shot = TriggerInfo( + frame_timeout=None, + number=1, + trigger=DetectorTrigger.internal, + deadtime=None, + livetime=None, + ) + + await test_eiger.stage() + await test_eiger.prepare(single_shot) + # Need to work out what the hold up is in prepare so we cant do this straight away. + # File path propogation? + await asyncio.sleep(0.5) + await setup_device.trigger.set(1) + await asyncio.sleep(0.5) # Need to work out when it's actually finished writing + + with h5py.File(SAVE_PATH + "/test_eiger_000001.h5") as f: + assert "data" in f.keys() + assert len(f["data"]) == 1 From 0078df27091f2974cc9111cd1f12a4021266ef02 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 19 Aug 2024 18:10:56 +0100 Subject: [PATCH 05/11] Add unit tests for eiger controller --- src/ophyd_async/core/_signal.py | 2 +- src/ophyd_async/epics/eiger/_eiger.py | 3 +- .../epics/eiger/_eiger_controller.py | 22 +++-- src/ophyd_async/epics/eiger/_eiger_io.py | 2 +- src/ophyd_async/epics/eiger/_odin_io.py | 44 ++++----- tests/epics/eiger/test_eiger_controller.py | 91 +++++++++++++++++++ 6 files changed, 128 insertions(+), 36 deletions(-) create mode 100644 tests/epics/eiger/test_eiger_controller.py diff --git a/src/ophyd_async/core/_signal.py b/src/ophyd_async/core/_signal.py index 3fbcde573..7b0827be7 100644 --- a/src/ophyd_async/core/_signal.py +++ b/src/ophyd_async/core/_signal.py @@ -562,7 +562,7 @@ async def wait_for_value( async def set_and_wait_for_other_value( - set_signal: SignalRW[T], + set_signal: SignalW[T], set_value: T, read_signal: SignalR[S], read_value: S, diff --git a/src/ophyd_async/epics/eiger/_eiger.py b/src/ophyd_async/epics/eiger/_eiger.py index a81580691..1fce9d4b2 100644 --- a/src/ophyd_async/epics/eiger/_eiger.py +++ b/src/ophyd_async/epics/eiger/_eiger.py @@ -7,8 +7,7 @@ class EigerDetector(StandardDetector): """ - Ophyd-async implementation of an ADKinetix Detector. - https://github.com/NSLS-II/ADKinetix + Ophyd-async implementation of an Eiger Detector. """ _controller: EigerController diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index 2219ad028..2bea0c8d0 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -2,6 +2,7 @@ from typing import Optional from ophyd_async.core import ( + DEFAULT_TIMEOUT, AsyncStatus, DetectorControl, DetectorTrigger, @@ -35,14 +36,23 @@ async def arm( trigger: DetectorTrigger = DetectorTrigger.internal, exposure: Optional[float] = None, ): - await asyncio.gather( - self._drv.trigger_mode.set(EIGER_TRIGGER_MODE_MAP[trigger]), + coros = [ + self._drv.trigger_mode.set(EIGER_TRIGGER_MODE_MAP[trigger].value), self._drv.num_images.set(num), - self._drv.acquire_time.set(exposure), - self._drv.acquire_period.set(exposure), + ] + if exposure is not None: + coros.extend( + [ + self._drv.acquire_time.set(exposure), + self._drv.acquire_period.set(exposure), + ] + ) + await asyncio.gather(*coros) + + # TODO: Detector state should be an enum see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 + await set_and_wait_for_other_value( + self._drv.arm, 1, self._drv.state, "ready", timeout=DEFAULT_TIMEOUT ) - await set_and_wait_for_other_value(self._drv.arm, 1, self._drv.state, "ready") - async def disarm(self): await self._drv.disarm.set(1) diff --git a/src/ophyd_async/epics/eiger/_eiger_io.py b/src/ophyd_async/epics/eiger/_eiger_io.py index 1df3c8626..988c571ae 100644 --- a/src/ophyd_async/epics/eiger/_eiger_io.py +++ b/src/ophyd_async/epics/eiger/_eiger_io.py @@ -39,7 +39,7 @@ def __init__(self, prefix: str, name: str = "") -> None: self.num_images = epics_signal_rw_rbv(int, f"{prefix}Nimages") self.num_triggers = epics_signal_rw_rbv(int, f"{prefix}Ntrigger") - # Ideally this will be a EigerTriggerMode, see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 + # TODO: Should be EigerTriggerMode enum, see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 self.trigger_mode = epics_signal_rw_rbv(str, f"{prefix}TriggerMode") self.arm = epics_signal_w(int, f"{prefix}Arm") diff --git a/src/ophyd_async/epics/eiger/_odin_io.py b/src/ophyd_async/epics/eiger/_odin_io.py index 15ff4d87b..46d988e96 100644 --- a/src/ophyd_async/epics/eiger/_odin_io.py +++ b/src/ophyd_async/epics/eiger/_odin_io.py @@ -2,7 +2,8 @@ from enum import Enum from typing import AsyncGenerator, AsyncIterator, Dict -from bluesky.protocols import DataKey, StreamAsset +from bluesky.protocols import StreamAsset +from event_model.documents.event_descriptor import DataKey from ophyd_async.core import ( DEFAULT_TIMEOUT, @@ -28,18 +29,7 @@ class Writing(str, Enum): class OdinNode(Device): def __init__(self, prefix: str, name: str = "") -> None: self.writing = epics_signal_r(Writing, f"{prefix}HDF:Writing") - - # Cannot find: - # FPClearErrors - # FPErrorState_RBV - # FPErrorMessage_RBV - - self.connected = epics_signal_r( - bool, f"{prefix}Connected" - ) # Assuming this is both FPProcessConnected_RBV and FRProcessConnected_RBV - - # Usually assert that FramesTimedOut_RBV and FramesDropped_RBV are 0 - # , do these exist or do we want to assert something else? + self.connected = epics_signal_r(bool, f"{prefix}Connected") super().__init__(name) @@ -90,27 +80,29 @@ async def open(self, multiplier: int = 1) -> Dict[str, DataKey]: await asyncio.gather( self._drv.file_path.set(str(info.directory_path)), self._drv.file_name.set(info.filename), - self._drv.data_type.set("uint16"), # TODO: get from eiger + self._drv.data_type.set( + "uint16" + ), # TODO: Get from eiger https://github.com/bluesky/ophyd-async/issues/529 + # TODO: Where should I actually be setting this + # The arm of the detector controller? This then breaks the seperation self._drv.num_to_capture.set(1), ) - # await wait_for_value(self._drv.acquisition_id, info.filename, DEFAULT_TIMEOUT) - await self._drv.capture.set(Writing.ON) return await self._describe() async def _describe(self) -> Dict[str, DataKey]: - """ - Return a describe based on the datasets PV - """ - # TODO: fill this in properly + data_shape = await asyncio.gather( + self._drv.image_height.get_value(), self._drv.image_width.get_value() + ) + return { "data": DataKey( - source="", - shape=[], - dtype="number", - dtype_numpy=" int: return await self._drv.num_captured.get_value() def collect_stream_docs(self, indices_written: int) -> AsyncIterator[StreamAsset]: - # TODO - pass + # TODO: Correctly return stream https://github.com/bluesky/ophyd-async/issues/530 + raise NotImplementedError() async def close(self) -> None: await self._drv.capture.set(Writing.OFF) diff --git a/tests/epics/eiger/test_eiger_controller.py b/tests/epics/eiger/test_eiger_controller.py new file mode 100644 index 000000000..c081358d0 --- /dev/null +++ b/tests/epics/eiger/test_eiger_controller.py @@ -0,0 +1,91 @@ +from unittest.mock import ANY, patch + +from pytest import fixture, raises + +from ophyd_async.core import ( + DeviceCollector, + callback_on_mock_put, + get_mock_put, + set_mock_value, +) +from ophyd_async.epics.eiger._eiger_controller import EigerController +from ophyd_async.epics.eiger._eiger_io import EigerDriverIO + +DriverAndController = tuple[EigerDriverIO, EigerController] + + +@fixture +def eiger_driver_and_controller_no_arm(RE) -> DriverAndController: + with DeviceCollector(mock=True): + driver = EigerDriverIO("") + controller = EigerController(driver) + + return driver, controller + + +@fixture +def eiger_driver_and_controller( + eiger_driver_and_controller_no_arm: DriverAndController, +) -> DriverAndController: + driver, controller = eiger_driver_and_controller_no_arm + + def become_ready_on_arm(*args, **kwargs): + if args[0] == 1: + set_mock_value(driver.state, "ready") + + callback_on_mock_put(driver.arm, become_ready_on_arm) + + return driver, controller + + +async def test_when_arm_with_exposure_then_time_and_period_set( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + test_exposure = 0.002 + await controller.arm(10, exposure=test_exposure) + assert (await driver.acquire_period.get_value()) == test_exposure + assert (await driver.acquire_time.get_value()) == test_exposure + + +async def test_when_arm_with_no_exposure_then_arm_set_correctly( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + await controller.arm(10, exposure=None) + get_mock_put(driver.arm).assert_called_once_with(1, wait=ANY, timeout=ANY) + + +async def test_when_arm_with_number_of_images_then_number_of_images_set_correctly( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + test_number_of_images = 40 + await controller.arm(test_number_of_images, exposure=None) + get_mock_put(driver.num_images).assert_called_once_with( + test_number_of_images, wait=ANY, timeout=ANY + ) + + +@patch("ophyd_async.epics.eiger._eiger_controller.DEFAULT_TIMEOUT", 0.1) +async def test_given_detector_fails_to_go_ready_when_arm_called_then_fails( + eiger_driver_and_controller_no_arm: DriverAndController, +): + driver, controller = eiger_driver_and_controller_no_arm + with raises(TimeoutError): + await controller.arm(10) + + +async def test_when_disarm_called_on_controller_then_disarm_called_on_driver( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + await controller.disarm() + get_mock_put(driver.disarm).assert_called_once_with(1, wait=ANY, timeout=ANY) + + +async def test_when_get_deadtime_called_then_returns_expected_deadtime( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + assert controller.get_deadtime(0) == 0.0001 From 9df23f233b206d57679b1a2e95e5f2e0a6aef0d9 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 19 Aug 2024 18:44:15 +0100 Subject: [PATCH 06/11] Add tests for odin --- src/ophyd_async/epics/eiger/_eiger.py | 2 +- tests/epics/eiger/test_eiger_detector.py | 12 ++++ tests/epics/eiger/test_odin_io.py | 75 ++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 tests/epics/eiger/test_eiger_detector.py create mode 100644 tests/epics/eiger/test_odin_io.py diff --git a/src/ophyd_async/epics/eiger/_eiger.py b/src/ophyd_async/epics/eiger/_eiger.py index 1fce9d4b2..062f68079 100644 --- a/src/ophyd_async/epics/eiger/_eiger.py +++ b/src/ophyd_async/epics/eiger/_eiger.py @@ -26,6 +26,6 @@ def __init__( super().__init__( EigerController(self.drv), - OdinWriter(path_provider, lambda: "", self.odin), + OdinWriter(path_provider, lambda: self.name, self.odin), name=name, ) diff --git a/tests/epics/eiger/test_eiger_detector.py b/tests/epics/eiger/test_eiger_detector.py new file mode 100644 index 000000000..5ae603f8b --- /dev/null +++ b/tests/epics/eiger/test_eiger_detector.py @@ -0,0 +1,12 @@ +from unittest.mock import MagicMock + +from ophyd_async.core import DeviceCollector +from ophyd_async.epics.eiger import EigerDetector + + +def test_when_detector_initialised_then_driver_and_odin_have_expected_prefixes(RE): + with DeviceCollector(mock=True): + detector = EigerDetector("BL03I", MagicMock()) + + assert "BL03I-EA-EIGER-01:" in detector.drv.arm.source + assert "BL03I-EA-ODIN-01:FP:" in detector.odin.acquisition_id.source diff --git a/tests/epics/eiger/test_odin_io.py b/tests/epics/eiger/test_odin_io.py new file mode 100644 index 000000000..bdb0223cd --- /dev/null +++ b/tests/epics/eiger/test_odin_io.py @@ -0,0 +1,75 @@ +from pathlib import Path +from unittest.mock import ANY, MagicMock + +import pytest + +from ophyd_async.core import DeviceCollector, get_mock_put, set_mock_value +from ophyd_async.epics.eiger._odin_io import Odin, OdinWriter, Writing + +OdinDriverAndWriter = tuple[Odin, OdinWriter] + + +@pytest.fixture +def odin_driver_and_writer(RE) -> OdinDriverAndWriter: + with DeviceCollector(mock=True): + driver = Odin("") + writer = OdinWriter(MagicMock(), lambda: "odin", driver) + return driver, writer + + +async def test_when_open_called_then_file_correctly_set( + odin_driver_and_writer: OdinDriverAndWriter, +): + driver, writer = odin_driver_and_writer + path_info = writer._path_provider.return_value + expected_path = "/tmp" + expected_filename = "filename.h5" + path_info.directory_path = Path(expected_path) + path_info.filename = expected_filename + + await writer.open() + + get_mock_put(driver.file_path).assert_called_once_with( + expected_path, wait=ANY, timeout=ANY + ) + get_mock_put(driver.file_name).assert_called_once_with( + expected_filename, wait=ANY, timeout=ANY + ) + + +async def test_when_open_called_then_all_expected_signals_set( + odin_driver_and_writer: OdinDriverAndWriter, +): + driver, writer = odin_driver_and_writer + await writer.open() + + get_mock_put(driver.data_type).assert_called_once_with( + "uint16", wait=ANY, timeout=ANY + ) + get_mock_put(driver.num_to_capture).assert_called_once_with( + 1, wait=ANY, timeout=ANY + ) + + get_mock_put(driver.capture).assert_called_once_with( + Writing.ON, wait=ANY, timeout=ANY + ) + + +async def test_given_data_shape_set_when_open_called_then_describe_has_correct_shape( + odin_driver_and_writer: OdinDriverAndWriter, +): + driver, writer = odin_driver_and_writer + set_mock_value(driver.image_width, 1024) + set_mock_value(driver.image_height, 768) + description = await writer.open() + assert description["data"]["shape"] == [768, 1024] + + +async def test_when_closed_then_data_capture_turned_off( + odin_driver_and_writer: OdinDriverAndWriter, +): + driver, writer = odin_driver_and_writer + await writer.close() + get_mock_put(driver.capture).assert_called_once_with( + Writing.OFF, wait=ANY, timeout=ANY + ) From c19cccad75bef9cdf619e28e3edbd98d2300378b Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 27 Aug 2024 17:39:40 +0100 Subject: [PATCH 07/11] Clean up the containers after running them --- .../epics/eiger/start_iocs_and_run_tests.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/system_tests/epics/eiger/start_iocs_and_run_tests.sh b/system_tests/epics/eiger/start_iocs_and_run_tests.sh index cc5b92bfd..ae9cdcec7 100755 --- a/system_tests/epics/eiger/start_iocs_and_run_tests.sh +++ b/system_tests/epics/eiger/start_iocs_and_run_tests.sh @@ -5,11 +5,21 @@ export odin_ioc=ODIN-$host mkdir /tmp/opi -podman run --rm -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc +if command -v docker &> /dev/null; then + DOCKER_COMMAND=docker +else + DOCKER_COMMAND=podman +fi -podman run --rm -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc +echo "Using $DOCKER_COMMAND" + +$DOCKER_COMMAND run --rm --name=$eiger_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc + +$DOCKER_COMMAND run --rm --name=$odin_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc sleep 1 pytest . +podman kill $eiger_ioc +podman kill $odin_ioc From 7c2d990a81ac51852af80e222bedfb8050d2c2cf Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 27 Aug 2024 17:53:10 +0100 Subject: [PATCH 08/11] Write files continuously and close odin on unstage --- src/ophyd_async/epics/eiger/_odin_io.py | 13 +++++++------ .../epics/eiger/start_iocs_and_run_tests.sh | 8 ++++---- system_tests/epics/eiger/test_eiger_system.py | 1 + 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/ophyd_async/epics/eiger/_odin_io.py b/src/ophyd_async/epics/eiger/_odin_io.py index 46d988e96..0d1b3516d 100644 --- a/src/ophyd_async/epics/eiger/_odin_io.py +++ b/src/ophyd_async/epics/eiger/_odin_io.py @@ -13,11 +13,12 @@ NameProvider, PathProvider, observe_value, + set_and_wait_for_value, ) from ophyd_async.epics.signal import ( epics_signal_r, + epics_signal_rw, epics_signal_rw_rbv, - epics_signal_w, ) @@ -38,7 +39,9 @@ class Odin(Device): def __init__(self, prefix: str, name: str = "") -> None: self.nodes = DeviceVector({i: OdinNode(f"{prefix}FP{i}:") for i in range(4)}) - self.capture = epics_signal_w(Writing, f"{prefix}ConfigHdfWrite") + self.capture = epics_signal_rw( + Writing, f"{prefix}Writing", f"{prefix}ConfigHdfWrite" + ) self.num_captured = epics_signal_r(int, f"{prefix}FramesWritten") self.num_to_capture = epics_signal_rw_rbv(int, f"{prefix}ConfigHdfFrames") @@ -83,9 +86,7 @@ async def open(self, multiplier: int = 1) -> Dict[str, DataKey]: self._drv.data_type.set( "uint16" ), # TODO: Get from eiger https://github.com/bluesky/ophyd-async/issues/529 - # TODO: Where should I actually be setting this - # The arm of the detector controller? This then breaks the seperation - self._drv.num_to_capture.set(1), + self._drv.num_to_capture.set(0), ) await self._drv.capture.set(Writing.ON) @@ -121,4 +122,4 @@ def collect_stream_docs(self, indices_written: int) -> AsyncIterator[StreamAsset raise NotImplementedError() async def close(self) -> None: - await self._drv.capture.set(Writing.OFF) + await set_and_wait_for_value(self._drv.capture, Writing.OFF) diff --git a/system_tests/epics/eiger/start_iocs_and_run_tests.sh b/system_tests/epics/eiger/start_iocs_and_run_tests.sh index ae9cdcec7..4bb9f7b53 100755 --- a/system_tests/epics/eiger/start_iocs_and_run_tests.sh +++ b/system_tests/epics/eiger/start_iocs_and_run_tests.sh @@ -13,13 +13,13 @@ fi echo "Using $DOCKER_COMMAND" -$DOCKER_COMMAND run --rm --name=$eiger_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc +#$DOCKER_COMMAND run --rm --name=$eiger_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc -$DOCKER_COMMAND run --rm --name=$odin_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc +#$DOCKER_COMMAND run --rm --name=$odin_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc sleep 1 pytest . -podman kill $eiger_ioc -podman kill $odin_ioc +#podman kill $eiger_ioc +#podman kill $odin_ioc diff --git a/system_tests/epics/eiger/test_eiger_system.py b/system_tests/epics/eiger/test_eiger_system.py index c166fc92f..dbcb23836 100644 --- a/system_tests/epics/eiger/test_eiger_system.py +++ b/system_tests/epics/eiger/test_eiger_system.py @@ -82,6 +82,7 @@ async def test_trigger_saves_file(test_eiger: EigerDetector, setup_device: Setup await asyncio.sleep(0.5) await setup_device.trigger.set(1) await asyncio.sleep(0.5) # Need to work out when it's actually finished writing + await test_eiger.unstage() with h5py.File(SAVE_PATH + "/test_eiger_000001.h5") as f: assert "data" in f.keys() From 6574add3b7047175b4ae6b381e9fa34cb45fdf3c Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 27 Aug 2024 22:53:18 +0100 Subject: [PATCH 09/11] Allow specifying a photon energy during prepare --- src/ophyd_async/epics/eiger/__init__.py | 8 ++--- src/ophyd_async/epics/eiger/_eiger.py | 14 +++++++- .../epics/eiger/_eiger_controller.py | 7 ++++ src/ophyd_async/epics/eiger/_eiger_io.py | 20 ++--------- system_tests/epics/eiger/test_eiger_system.py | 21 +++++++----- tests/epics/eiger/test_eiger_controller.py | 23 +++++++++++++ tests/epics/eiger/test_eiger_detector.py | 34 ++++++++++++++++--- tests/epics/eiger/test_eiger_io.py | 32 ----------------- tests/epics/eiger/test_odin_io.py | 2 +- 9 files changed, 90 insertions(+), 71 deletions(-) delete mode 100644 tests/epics/eiger/test_eiger_io.py diff --git a/src/ophyd_async/epics/eiger/__init__.py b/src/ophyd_async/epics/eiger/__init__.py index 829d768de..bf9368859 100644 --- a/src/ophyd_async/epics/eiger/__init__.py +++ b/src/ophyd_async/epics/eiger/__init__.py @@ -1,9 +1,5 @@ -from ._eiger import EigerDetector +from ._eiger import EigerDetector, EigerTriggerInfo from ._eiger_controller import EigerController from ._eiger_io import EigerDriverIO -__all__ = [ - "EigerDetector", - "EigerController", - "EigerDriverIO", -] +__all__ = ["EigerDetector", "EigerController", "EigerDriverIO", "EigerTriggerInfo"] diff --git a/src/ophyd_async/epics/eiger/_eiger.py b/src/ophyd_async/epics/eiger/_eiger.py index 062f68079..e7d60786e 100644 --- a/src/ophyd_async/epics/eiger/_eiger.py +++ b/src/ophyd_async/epics/eiger/_eiger.py @@ -1,10 +1,17 @@ -from ophyd_async.core import PathProvider, StandardDetector +from pydantic import Field + +from ophyd_async.core import AsyncStatus, PathProvider, StandardDetector +from ophyd_async.core._detector import TriggerInfo from ._eiger_controller import EigerController from ._eiger_io import EigerDriverIO from ._odin_io import Odin, OdinWriter +class EigerTriggerInfo(TriggerInfo): + energy_ev: float = Field(gt=0) + + class EigerDetector(StandardDetector): """ Ophyd-async implementation of an Eiger Detector. @@ -29,3 +36,8 @@ def __init__( OdinWriter(path_provider, lambda: self.name, self.odin), name=name, ) + + @AsyncStatus.wrap + async def prepare(self, value: EigerTriggerInfo) -> None: + await self._controller.set_energy(value.energy_ev) + await super().prepare(value) diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index 2bea0c8d0..14aa3231e 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -29,6 +29,13 @@ def __init__( def get_deadtime(self, exposure: float) -> float: return 0.0001 + async def set_energy(self, energy: float, tolerance: float = 0.1): + """Changing photon energy takes some time so only do so if the current energy is + outside the tolerance.""" + current_energy = await self._drv.photon_energy.get_value() + if abs(current_energy - energy) > tolerance: + await self._drv.photon_energy.set(energy) + @AsyncStatus.wrap async def arm( self, diff --git a/src/ophyd_async/epics/eiger/_eiger_io.py b/src/ophyd_async/epics/eiger/_eiger_io.py index 988c571ae..1df672592 100644 --- a/src/ophyd_async/epics/eiger/_eiger_io.py +++ b/src/ophyd_async/epics/eiger/_eiger_io.py @@ -1,6 +1,6 @@ from enum import Enum -from ophyd_async.core import AsyncStatus, Device +from ophyd_async.core import Device from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw_rbv, epics_signal_w @@ -10,22 +10,6 @@ class EigerTriggerMode(str, Enum): gate = "exte" -class PhotonEnergy(Device): - """Changing photon energy takes some time so only do so if the current energy is - outside the tolerance.""" - - def __init__(self, prefix, tolerance=0.1, name: str = "") -> None: - self._photon_energy = epics_signal_rw_rbv(float, f"{prefix}PhotonEnergy") - self.tolerance = tolerance - super().__init__(name) - - @AsyncStatus.wrap - async def set(self, value): - current_energy = await self._photon_energy.get_value() - if abs(current_energy - value) > self.tolerance: - await self._photon_energy.set(value) - - class EigerDriverIO(Device): def __init__(self, prefix: str, name: str = "") -> None: self.bit_depth = epics_signal_r(int, f"{prefix}BitDepthReadout") @@ -53,6 +37,6 @@ def __init__(self, prefix: str, name: str = "") -> None: self.omega_start = epics_signal_rw_rbv(float, f"{prefix}OmegaStart") self.omega_increment = epics_signal_rw_rbv(float, f"{prefix}OmegaIncrement") - self.photon_energy = PhotonEnergy(prefix) + self.photon_energy = epics_signal_rw_rbv(float, f"{prefix}PhotonEnergy") super().__init__(name) diff --git a/system_tests/epics/eiger/test_eiger_system.py b/system_tests/epics/eiger/test_eiger_system.py index dbcb23836..8d20c1a70 100644 --- a/system_tests/epics/eiger/test_eiger_system.py +++ b/system_tests/epics/eiger/test_eiger_system.py @@ -11,13 +11,10 @@ Device, DeviceCollector, StaticPathProvider, - TriggerInfo, ) -from ophyd_async.epics.eiger import EigerDetector +from ophyd_async.epics.eiger import EigerDetector, EigerTriggerInfo from ophyd_async.epics.signal import epics_signal_rw -EIGER_PREFIX = os.environ["eiger_ioc"] + ":" -ODIN_PREFIX = os.environ["odin_ioc"] + ":" SAVE_PATH = "/tmp" @@ -38,15 +35,20 @@ def __init__(self, eiger_prefix: str, odin_prefix: str) -> None: super().__init__("") +@pytest.fixture +def ioc_prefixes(): + return os.environ["eiger_ioc"] + ":", os.environ["odin_ioc"] + ":" + + @pytest.fixture def RE(): return RunEngine() @pytest.fixture -async def setup_device(RE): +async def setup_device(RE, ioc_prefixes): async with DeviceCollector(): - device = SetupDevice(EIGER_PREFIX, ODIN_PREFIX + "FP:") + device = SetupDevice(ioc_prefixes[0], ioc_prefixes[1] + "FP:") await asyncio.gather( device.header_detail.set("all"), device.compression.set("BSLZ4"), @@ -58,21 +60,22 @@ async def setup_device(RE): @pytest.fixture -async def test_eiger(RE) -> EigerDetector: +async def test_eiger(RE, ioc_prefixes) -> EigerDetector: provider = StaticPathProvider(lambda: "test_eiger", Path(SAVE_PATH)) async with DeviceCollector(): - test_eiger = EigerDetector("", provider, EIGER_PREFIX, ODIN_PREFIX) + test_eiger = EigerDetector("", provider, ioc_prefixes[0], ioc_prefixes[1]) return test_eiger async def test_trigger_saves_file(test_eiger: EigerDetector, setup_device: SetupDevice): - single_shot = TriggerInfo( + single_shot = EigerTriggerInfo( frame_timeout=None, number=1, trigger=DetectorTrigger.internal, deadtime=None, livetime=None, + energy_ev=10000, ) await test_eiger.stage() diff --git a/tests/epics/eiger/test_eiger_controller.py b/tests/epics/eiger/test_eiger_controller.py index c081358d0..70f307816 100644 --- a/tests/epics/eiger/test_eiger_controller.py +++ b/tests/epics/eiger/test_eiger_controller.py @@ -89,3 +89,26 @@ async def test_when_get_deadtime_called_then_returns_expected_deadtime( ): driver, controller = eiger_driver_and_controller assert controller.get_deadtime(0) == 0.0001 + + +async def test_given_energy_within_tolerance_when_photon_energy_set_then_pv_unchanged( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + initial_energy = 10 + set_mock_value(driver.photon_energy, initial_energy) + await controller.set_energy(10.002) + get_mock_put(driver.photon_energy).assert_not_called() + assert (await driver.photon_energy.get_value()) == initial_energy + + +async def test_given_energy_outside_tolerance_when_photon_energy_set_then_pv_changed( + eiger_driver_and_controller: DriverAndController, +): + driver, controller = eiger_driver_and_controller + initial_energy = 10 + new_energy = 15 + set_mock_value(driver.photon_energy, initial_energy) + await controller.set_energy(new_energy) + get_mock_put(driver.photon_energy).assert_called_once() + assert (await driver.photon_energy.get_value()) == new_energy diff --git a/tests/epics/eiger/test_eiger_detector.py b/tests/epics/eiger/test_eiger_detector.py index 5ae603f8b..33cfef805 100644 --- a/tests/epics/eiger/test_eiger_detector.py +++ b/tests/epics/eiger/test_eiger_detector.py @@ -1,12 +1,38 @@ -from unittest.mock import MagicMock +from unittest.mock import ANY, AsyncMock, MagicMock -from ophyd_async.core import DeviceCollector -from ophyd_async.epics.eiger import EigerDetector +import pytest +from ophyd_async.core import DetectorTrigger, DeviceCollector, get_mock_put +from ophyd_async.epics.eiger import EigerDetector, EigerTriggerInfo -def test_when_detector_initialised_then_driver_and_odin_have_expected_prefixes(RE): + +@pytest.fixture +def detector(RE): with DeviceCollector(mock=True): detector = EigerDetector("BL03I", MagicMock()) + return detector + +def test_when_detector_initialised_then_driver_and_odin_have_expected_prefixes( + detector, +): assert "BL03I-EA-EIGER-01:" in detector.drv.arm.source assert "BL03I-EA-ODIN-01:FP:" in detector.odin.acquisition_id.source + + +async def test_when_prepared_with_energy_then_energy_set_on_detector(detector): + detector.controller.arm = AsyncMock() + await detector.prepare( + EigerTriggerInfo( + frame_timeout=None, + number=1, + trigger=DetectorTrigger.internal, + deadtime=None, + livetime=None, + energy_ev=10000, + ) + ) + + get_mock_put(detector.drv.photon_energy).assert_called_once_with( + 10000, wait=ANY, timeout=ANY + ) diff --git a/tests/epics/eiger/test_eiger_io.py b/tests/epics/eiger/test_eiger_io.py deleted file mode 100644 index e94180b28..000000000 --- a/tests/epics/eiger/test_eiger_io.py +++ /dev/null @@ -1,32 +0,0 @@ -from pytest import fixture - -from ophyd_async.core import DeviceCollector, get_mock_put, set_mock_value -from ophyd_async.epics.eiger._eiger_io import PhotonEnergy - - -@fixture -def photon_energy(RE): - with DeviceCollector(mock=True): - photon_energy = PhotonEnergy("") - return photon_energy - - -async def test_given_energy_within_tolerance_when_photon_energy_set_then_pv_unchanged( - photon_energy, -): - initial_energy = 10 - set_mock_value(photon_energy._photon_energy, initial_energy) - await photon_energy.set(10.002) - get_mock_put(photon_energy._photon_energy).assert_not_called() - assert (await photon_energy._photon_energy.get_value()) == initial_energy - - -async def test_given_energy_outside_tolerance_when_photon_energy_set_then_pv_changed( - photon_energy, -): - initial_energy = 10 - new_energy = 15 - set_mock_value(photon_energy._photon_energy, initial_energy) - await photon_energy.set(new_energy) - get_mock_put(photon_energy._photon_energy).assert_called_once() - assert (await photon_energy._photon_energy.get_value()) == new_energy diff --git a/tests/epics/eiger/test_odin_io.py b/tests/epics/eiger/test_odin_io.py index bdb0223cd..1ef9c5944 100644 --- a/tests/epics/eiger/test_odin_io.py +++ b/tests/epics/eiger/test_odin_io.py @@ -47,7 +47,7 @@ async def test_when_open_called_then_all_expected_signals_set( "uint16", wait=ANY, timeout=ANY ) get_mock_put(driver.num_to_capture).assert_called_once_with( - 1, wait=ANY, timeout=ANY + 0, wait=ANY, timeout=ANY ) get_mock_put(driver.capture).assert_called_once_with( From a650bbe57b1daddb5d1c63bcf479f9cebaa72da6 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 27 Aug 2024 23:03:45 +0100 Subject: [PATCH 10/11] Add reference for deadtime --- src/ophyd_async/epics/eiger/_eiger_controller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index 14aa3231e..fa37bbeba 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -27,6 +27,7 @@ def __init__( self._drv = driver def get_deadtime(self, exposure: float) -> float: + # See https://media.dectris.com/filer_public/30/14/3014704e-5f3b-43ba-8ccf-8ef720e60d2a/240202_usermanual_eiger2.pdf return 0.0001 async def set_energy(self, energy: float, tolerance: float = 0.1): From 76ad527d69e43819bb569afdf1b5077dac0e460b Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 2 Sep 2024 14:21:54 +0100 Subject: [PATCH 11/11] Fix commented out running of container --- system_tests/epics/eiger/start_iocs_and_run_tests.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/system_tests/epics/eiger/start_iocs_and_run_tests.sh b/system_tests/epics/eiger/start_iocs_and_run_tests.sh index 4bb9f7b53..ae9cdcec7 100755 --- a/system_tests/epics/eiger/start_iocs_and_run_tests.sh +++ b/system_tests/epics/eiger/start_iocs_and_run_tests.sh @@ -13,13 +13,13 @@ fi echo "Using $DOCKER_COMMAND" -#$DOCKER_COMMAND run --rm --name=$eiger_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc +$DOCKER_COMMAND run --rm --name=$eiger_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/eiger-fastcs:0.1.0beta5 ioc $eiger_ioc -#$DOCKER_COMMAND run --rm --name=$odin_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc +$DOCKER_COMMAND run --rm --name=$odin_ioc -dt --net=host -v /tmp/opi/:/epics/opi ghcr.io/diamondlightsource/odin-fastcs:0.2.0beta2 ioc $odin_ioc sleep 1 pytest . -#podman kill $eiger_ioc -#podman kill $odin_ioc +podman kill $eiger_ioc +podman kill $odin_ioc