Merge branch 'main' into bimorph-mirrors

.github/workflows/_tox.yml

@@ -20,5 +20,8 @@ jobs:
       - name: Install python packages
         uses: ./.github/actions/install_requirements
 
+      - name: Run import linter
+        run: lint-imports
+
       - name: Run tox
         run: tox -e ${{ inputs.tox }}

.pre-commit-config.yaml

@@ -22,3 +22,11 @@ repos:
         entry: ruff format --force-exclude
         types: [python]
         require_serial: true
+
+      - id: import-contracts
+        name: Ensure import directionality
+        pass_filenames: false
+        language: system
+        entry: lint-imports
+        types: [python]
+        require_serial: false

conftest.py

@@ -53,9 +53,9 @@ def patched_open(*args, **kwargs):
         requested_path = Path(args[0])
         if requested_path.is_absolute():
             for p in BANNED_PATHS:
-                assert not requested_path.is_relative_to(
-                    p
-                ), f"Attempt to open {requested_path} from inside a unit test"
+                assert not requested_path.is_relative_to(p), (
+                    f"Attempt to open {requested_path} from inside a unit test"
+                )
         return unpatched_open(*args, **kwargs)
 
     with patch("builtins.open", side_effect=patched_open):

docs/explanations/reviews.md

@@ -51,6 +51,6 @@ New members should be regular contributors to dodal and should have been "shadow
 - Adherence to the review standards above as well as the [repository standards](../reference/standards.rst) and [device standards](../reference/device-standards.rst).
 - Independent (i.e. not just to satisfy a reviewer) motivation to make sure all code in dodal is well-tested. Use of unit and system tests as appropriate. Clear effort made to keep test coverage high (>90%). 
 - Advanced understanding of bluesky and ophyd-async including concepts and best practices, such as how to appropriately split logic between devices/plans/callbacks.
-- Humility in the use of reviewing as a tool in a way that balances the need to preserve quality with the need for progress. Appropriate use of the Must/Should/Could/Nit system documented above is helpful in showing this, as it gives the reviewer the opportunity to weight their comments in terms of project impact. They should also demonstrate similar humiliary as a reviewee.
+- Humility in the use of reviewing as a tool in a way that balances the need to preserve quality with the need for progress. Appropriate use of the Must/Should/Could/Nit system documented above is helpful in showing this, as it gives the reviewer the opportunity to weight their comments in terms of project impact. They should also demonstrate similar humility as a reviewee.
 
 Additionally, they should be regularly raising issues in the repository and demonstrating the ability to write well formed issues, with well defined acceptance criteria, that are understandable without large amounts of context.

pyproject.toml

@@ -44,6 +44,7 @@ requires-python = ">=3.10"
 dev = [
     "black",
     "diff-cover",
+    "import-linter",
     "mypy",
     # Commented out due to dependency version conflict with pydantic 1.x
     # "copier",
@@ -181,3 +182,17 @@ lint.select = [
 # Remove this line to forbid private member access in tests
 "tests/**/*" = ["SLF001"]
 "system_tests/**/*" = ["SLF001"]
+
+[tool.importlinter]
+root_package = "dodal"
+
+[[tool.importlinter.contracts]]
+name = "Common cannot import from beamlines"
+type = "forbidden"
+source_modules = ["dodal.common"]
+forbidden_modules = ["dodal.beamlines"]
+
+[[tool.importlinter.contracts]]
+name = "Enforce import order"
+type = "layers"
+layers = ["dodal.plans", "dodal.beamlines", "dodal.devices"]

src/dodal/beamlines/i03.py

@@ -40,8 +40,13 @@
 from dodal.devices.webcam import Webcam
 from dodal.devices.xbpm_feedback import XBPMFeedback
 from dodal.devices.xspress3.xspress3 import Xspress3
-from dodal.devices.zebra import Zebra
-from dodal.devices.zebra_controlled_shutter import ZebraShutter
+from dodal.devices.zebra.zebra import Zebra
+from dodal.devices.zebra.zebra_constants_mapping import (
+    ZebraMapping,
+    ZebraSources,
+    ZebraTTLOutputs,
+)
+from dodal.devices.zebra.zebra_controlled_shutter import ZebraShutter
 from dodal.devices.zocalo import ZocaloResults
 from dodal.log import set_beamline as set_log_beamline
 from dodal.utils import BeamlinePrefix, get_beamline_name, skip_device
@@ -58,6 +63,12 @@
 
 set_path_provider(PandASubpathProvider())
 
+I03_ZEBRA_MAPPING = ZebraMapping(
+    outputs=ZebraTTLOutputs(TTL_DETECTOR=1, TTL_SHUTTER=2, TTL_XSPRESS3=3, TTL_PANDA=4),
+    sources=ZebraSources(),
+    AND_GATE_FOR_AUTO_SHUTTER=2,
+)
+
 
 def aperture_scatterguard(
     wait_for_connection: bool = True,
@@ -368,6 +379,7 @@ def zebra(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -
         "-EA-ZEBRA-01:",
         wait_for_connection,
         fake_with_ophyd_sim,
+        mapping=I03_ZEBRA_MAPPING,
     )
 
 

src/dodal/beamlines/i04.py

@@ -28,8 +28,13 @@
 from dodal.devices.thawer import Thawer
 from dodal.devices.undulator import Undulator
 from dodal.devices.xbpm_feedback import XBPMFeedback
-from dodal.devices.zebra import Zebra
-from dodal.devices.zebra_controlled_shutter import ZebraShutter
+from dodal.devices.zebra.zebra import Zebra
+from dodal.devices.zebra.zebra_constants_mapping import (
+    ZebraMapping,
+    ZebraSources,
+    ZebraTTLOutputs,
+)
+from dodal.devices.zebra.zebra_controlled_shutter import ZebraShutter
 from dodal.log import set_beamline as set_log_beamline
 from dodal.utils import BeamlinePrefix, get_beamline_name, skip_device
 
@@ -47,6 +52,11 @@
 set_log_beamline(BL)
 set_utils_beamline(BL)
 
+I04_ZEBRA_MAPPING = ZebraMapping(
+    outputs=(ZebraTTLOutputs(TTL_DETECTOR=1, TTL_FAST_SHUTTER=2, TTL_XSPRESS3=3)),
+    sources=ZebraSources(),
+)
+
 
 def smargon(
     wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
@@ -341,6 +351,7 @@ def zebra(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -
         "-EA-ZEBRA-01:",
         wait_for_connection,
         fake_with_ophyd_sim,
+        mapping=I04_ZEBRA_MAPPING,
     )
 
 

src/dodal/beamlines/i13_1.py

@@ -8,7 +8,8 @@
     set_path_provider,
 )
 from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline
-from dodal.common.visit import StaticVisitPathProvider
+from dodal.common.visit import LocalDirectoryServiceClient, StaticVisitPathProvider
+from dodal.devices.i13_1.merlin import Merlin
 from dodal.devices.motors import XYZPositioner
 from dodal.log import set_beamline as set_log_beamline
 from dodal.utils import get_beamline_name
@@ -19,7 +20,8 @@
 set_path_provider(
     StaticVisitPathProvider(
         BL,
-        Path("/data/2024/cm37257-4/"),  # latest commissioning visit
+        Path("/dls/i13-1/data/2024/cm37257-5/tmp/"),  # latest commissioning visit
+        client=LocalDirectoryServiceClient(),
     )
 )
 
@@ -64,3 +66,19 @@ def side_camera(
         wait=wait_for_connection,
         fake=fake_with_ophyd_sim,
     )
+
+
+def merlin(
+    wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
+) -> Merlin:
+    return device_instantiation(
+        Merlin,
+        prefix="BL13J-EA-DET-04:",
+        name="merlin",
+        bl_prefix=False,
+        drv_suffix="CAM:",
+        hdf_suffix="HDF5:",
+        path_provider=get_path_provider(),
+        wait=wait_for_connection,
+        fake=fake_with_ophyd_sim,
+    )

src/dodal/beamlines/i24.py

@@ -16,7 +16,12 @@
 from dodal.devices.i24.vgonio import VerticalGoniometer
 from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.oav_parameters import OAVConfig
-from dodal.devices.zebra import Zebra
+from dodal.devices.zebra.zebra import Zebra
+from dodal.devices.zebra.zebra_constants_mapping import (
+    ZebraMapping,
+    ZebraSources,
+    ZebraTTLOutputs,
+)
 from dodal.log import set_beamline as set_log_beamline
 from dodal.utils import get_beamline_name, skip_device
 
@@ -29,6 +34,11 @@
 set_log_beamline(BL)
 set_utils_beamline(BL)
 
+I24_ZEBRA_MAPPING = ZebraMapping(
+    outputs=ZebraTTLOutputs(TTL_EIGER=1, TTL_PILATUS=2, TTL_FAST_SHUTTER=4),
+    sources=ZebraSources(),
+)
+
 
 def attenuator(
     wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
@@ -191,6 +201,7 @@ def zebra(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -
         "-EA-ZEBRA-01:",
         wait_for_connection,
         fake_with_ophyd_sim,
+        mapping=I24_ZEBRA_MAPPING,
     )
 
 

src/dodal/beamlines/training_rig.py

@@ -1,6 +1,7 @@
 from pathlib import Path
 
 from ophyd_async.epics.adaravis import AravisDetector
+from ophyd_async.fastcs.panda import HDFPanda
 
 from dodal.common.beamlines.beamline_utils import (
     device_factory,
@@ -33,7 +34,7 @@
 set_path_provider(
     StaticVisitPathProvider(
         BL,
-        Path("/exports/mybeamline/data"),
+        Path("/data"),
         client=LocalDirectoryServiceClient(),
     )
 )
@@ -52,3 +53,11 @@ def det() -> AravisDetector:
         drv_suffix="DET:",
         hdf_suffix=HDF5_PREFIX,
     )
+
+
+@device_factory()
+def panda() -> HDFPanda:
+    return HDFPanda(
+        prefix=f"{PREFIX.beamline_prefix}-MO-PANDA-01:",
+        path_provider=get_path_provider(),
+    )

src/dodal/common/crystal_metadata.py

@@ -55,7 +55,7 @@ def make_crystal_metadata_from_material(
     d_spacing = d_spacing_param or CrystalMetadata.calculate_default_d_spacing(
         material.value.lattice_parameter, reflection_plane
     )
-    assert all(
-        isinstance(i, int) and i > 0 for i in reflection_plane
-    ), "Reflection plane indices must be positive integers"
+    assert all(isinstance(i, int) and i > 0 for i in reflection_plane), (
+        "Reflection plane indices must be positive integers"
+    )
     return CrystalMetadata(usage, material.value.name, reflection_plane, d_spacing)

src/dodal/common/udc_directory_provider.py

@@ -46,7 +46,9 @@ async def update(self, *, directory: Path, suffix: str = "", **kwargs):
         self._filename_provider.suffix = suffix
 
     def __call__(self, device_name: str | None = None) -> PathInfo:
-        assert self._output_directory, "Directory unknown for PandA to write into, update() needs to be called at least once"
+        assert self._output_directory, (
+            "Directory unknown for PandA to write into, update() needs to be called at least once"
+        )
         return PathInfo(
             directory_path=self._output_directory,
             filename=self._filename_provider(device_name),

src/dodal/devices/attenuator/attenuator.py

@@ -106,7 +106,7 @@ def __init__(
         with self.add_children_as_readables():
             self.filters: DeviceVector[FilterMotor] = DeviceVector(
                 {
-                    index: FilterMotor(f"{prefix}MP{index+1}:", filter, name)
+                    index: FilterMotor(f"{prefix}MP{index + 1}:", filter, name)
                     for index, filter in enumerate(filter_selection)
                 }
             )

src/dodal/devices/eiger_odin.py

@@ -91,10 +91,10 @@ def check_frames_dropped(self) -> tuple[bool, str]:
     def wait_for_no_errors(self, timeout) -> dict[SubscriptionStatus, str]:
         errors = {}
         for node_number, node_pv in enumerate(self.nodes):
-            errors[
-                await_value(node_pv.error_status, False, timeout)
-            ] = f"Filewriter {node_number} is in an error state with error message\
+            errors[await_value(node_pv.error_status, False, timeout)] = (
+                f"Filewriter {node_number} is in an error state with error message\
                      - {node_pv.error_message.get()}"
+            )
 
         return errors
 

src/dodal/devices/flux.py

@@ -1,7 +1,14 @@
-from ophyd import Component, Device, EpicsSignalRO, Kind
+from ophyd_async.core import (
+    StandardReadable,
+    StandardReadableFormat,
+)
+from ophyd_async.epics.core import epics_signal_r
 
 
-class Flux(Device):
+class Flux(StandardReadable):
     """Simple device to get the flux reading"""
 
-    flux_reading = Component(EpicsSignalRO, "SAMP", kind=Kind.hinted)
+    def __init__(self, prefix: str, name="") -> None:
+        with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL):
+            self.flux_reading = epics_signal_r(float, prefix + "SAMP")
+            super().__init__(name=name)

src/dodal/devices/i13_1/merlin.py

@@ -0,0 +1,33 @@
+from ophyd_async.core import PathProvider, StandardDetector
+from ophyd_async.epics import adcore
+
+from dodal.devices.i13_1.merlin_controller import MerlinController
+from dodal.devices.i13_1.merlin_io import MerlinDriverIO
+
+
+class Merlin(StandardDetector):
+    _controller: MerlinController
+    _writer: adcore.ADHDFWriter
+
+    def __init__(
+        self,
+        prefix: str,
+        path_provider: PathProvider,
+        drv_suffix="CAM:",
+        hdf_suffix="HDF:",
+        name: str = "",
+    ):
+        self.drv = MerlinDriverIO(prefix + drv_suffix)
+        self.hdf = adcore.NDFileHDFIO(prefix + hdf_suffix)
+
+        super().__init__(
+            MerlinController(self.drv),
+            adcore.ADHDFWriter(
+                self.hdf,
+                path_provider,
+                lambda: self.name,
+                adcore.ADBaseDatasetDescriber(self.drv),
+            ),
+            config_sigs=(self.drv.acquire_period, self.drv.acquire_time),
+            name=name,
+        )

src/dodal/devices/i13_1/merlin_controller.py

@@ -0,0 +1,52 @@
+import asyncio
+import logging
+
+from ophyd_async.core import (
+    DEFAULT_TIMEOUT,
+    AsyncStatus,
+    DetectorController,
+    TriggerInfo,
+)
+from ophyd_async.epics import adcore
+
+from dodal.devices.i13_1.merlin_io import MerlinDriverIO, MerlinImageMode
+
+
+class MerlinController(DetectorController):
+    def __init__(
+        self,
+        driver: MerlinDriverIO,
+        good_states: frozenset[adcore.DetectorState] = adcore.DEFAULT_GOOD_STATES,
+    ) -> None:
+        self.driver = driver
+        self.good_states = good_states
+        self.frame_timeout: float = 0
+        self._arm_status: AsyncStatus | None = None
+        for drv_child in self.driver.children():
+            logging.debug(drv_child)
+
+    def get_deadtime(self, exposure: float | None) -> float:
+        return 0.002
+
+    async def prepare(self, trigger_info: TriggerInfo):
+        self.frame_timeout = (
+            DEFAULT_TIMEOUT + await self.driver.acquire_time.get_value()
+        )
+        await asyncio.gather(
+            self.driver.num_images.set(trigger_info.total_number_of_triggers),
+            self.driver.image_mode.set(MerlinImageMode.MULTIPLE),
+        )
+
+    async def arm(self):
+        self._arm_status = await adcore.start_acquiring_driver_and_ensure_status(
+            self.driver, good_states=self.good_states, timeout=self.frame_timeout
+        )
+
+    async def wait_for_idle(self):
+        if self._arm_status:
+            await self._arm_status
+
+    async def disarm(self):
+        # We can't use caput callback as we already used it in arm() and we can't have
+        # 2 or they will deadlock
+        await adcore.stop_busy_record(self.driver.acquire, False, timeout=1)