Merge branch 'main' into 772_beamline_specific_zebra_constants

.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

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/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/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/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)

src/dodal/devices/i13_1/merlin_io.py

@@ -0,0 +1,17 @@
+from ophyd_async.core import StrictEnum
+from ophyd_async.epics import adcore
+from ophyd_async.epics.core import epics_signal_rw_rbv
+
+
+class MerlinImageMode(StrictEnum):
+    SINGLE = "Single"
+    MULTIPLE = "Multiple"
+    CONTINUOUS = "Continuous"
+    THRESHOLD = "Threshold"
+    BACKGROUND = "Background"
+
+
+class MerlinDriverIO(adcore.ADBaseIO):
+    def __init__(self, prefix: str, name: str = "") -> None:
+        super().__init__(prefix, name)
+        self.image_mode = epics_signal_rw_rbv(MerlinImageMode, prefix + "ImageMode")

src/dodal/devices/p45.py

@@ -1,44 +1,55 @@
-from ophyd import Component as Cpt
-from ophyd import EpicsMotor, MotorBundle
-from ophyd.areadetector.base import ADComponent as Cpt
+from ophyd_async.core import StandardReadable
+from ophyd_async.epics.motor import Motor
 
 
-class SampleY(MotorBundle):
+class SampleY(StandardReadable):
     """
     Motors for controlling the sample's y position and stretch in the y axis.
     """
 
-    base = Cpt(EpicsMotor, "CS:Y")
-    stretch = Cpt(EpicsMotor, "CS:Y:STRETCH")
-    top = Cpt(EpicsMotor, "Y:TOP")
-    bottom = Cpt(EpicsMotor, "Y:BOT")
+    def __init__(self, prefix: str, name="") -> None:
+        with self.add_children_as_readables():
+            self.base = Motor(prefix + "CS:Y")
+            self.stretch = Motor(prefix + "CS:Y:STRETCH")
+            self.top = Motor(prefix + "Y:TOP")
+            self.bottom = Motor(prefix + "Y:BOT")
+            super().__init__(name=name)
 
 
-class SampleTheta(MotorBundle):
+class SampleTheta(StandardReadable):
     """
     Motors for controlling the sample's theta position and skew
     """
 
-    base = Cpt(EpicsMotor, "THETA:POS")
-    skew = Cpt(EpicsMotor, "THETA:SKEW")
-    top = Cpt(EpicsMotor, "THETA:TOP")
-    bottom = Cpt(EpicsMotor, "THETA:BOT")
+    def __init__(self, prefix: str, name="") -> None:
+        with self.add_children_as_readables():
+            self.base = Motor(prefix + "THETA:POS")
+            self.skew = Motor(prefix + "THETA:SKEW")
+            self.top = Motor(prefix + "THETA:TOP")
+            self.bottom = Motor(prefix + "THETA:BOT")
+            super().__init__(name=name)
 
 
-class TomoStageWithStretchAndSkew(MotorBundle):
+class TomoStageWithStretchAndSkew(StandardReadable):
     """
     Grouping of motors for the P45 tomography stage
     """
 
-    x = Cpt(EpicsMotor, "X")
-    y = Cpt(SampleY, "")
-    theta = Cpt(SampleTheta, "")
+    def __init__(self, prefix: str, name="") -> None:
+        with self.add_children_as_readables():
+            self.x = Motor(prefix + "X")
+            self.y = SampleY(prefix)
+            self.theta = SampleTheta(prefix)
+            super().__init__(name=name)
 
 
-class Choppers(MotorBundle):
+class Choppers(StandardReadable):
     """
     Grouping for the P45 chopper motors
     """
 
-    x = Cpt(EpicsMotor, "ENDAT")
-    y = Cpt(EpicsMotor, "BISS")
+    def __init__(self, prefix: str, name="") -> None:
+        with self.add_children_as_readables():
+            self.x = Motor(prefix + "ENDAT")
+            self.y = Motor(prefix + "BISS")
+            super().__init__(name=name)

src/dodal/devices/s4_slit_gaps.py

@@ -1,8 +1,12 @@
-from ophyd import Component, Device, EpicsMotor
+from ophyd_async.core import StandardReadable
+from ophyd_async.epics.motor import Motor
 
 
-class S4SlitGaps(Device):
+class S4SlitGaps(StandardReadable):
     """Note that the S4 slits have a different PV fromat to other beamline slits"""
 
-    xgap = Component(EpicsMotor, "XGAP")
-    ygap = Component(EpicsMotor, "YGAP")
+    def __init__(self, prefix: str, name="") -> None:
+        with self.add_children_as_readables():
+            self.xgap = Motor(prefix + "XGAP")
+            self.ygap = Motor(prefix + "YGAP")
+            super().__init__(name=name)