Split out common AD file plugin logic into core writer class, create …

…ADTiffWriter (#606)

* Starting to work on ad tiff writer

* Continue working on tiff writer

* Further work on tiff writer, existing tests now passing.

* Remove functions moved to superclas from hdf writer

* Significant re-org and simplification of ad classes

* Ruff formatting

* Modify ad sim classes to reflect new superclasses

* Modify vimba and kinetix classes

* Modify aravis and pilatus classes

* Update all tests to make sure they still pass with changes

* Some cleanup

* Changes to standard detector to account for controller/writer types in typing

* Significant changes to base detector, controller, and writer classes

* Update detector and controller classes to reflect changes

* Make sure panda standard det uses new type hints

* Most tests passing

* Revert change in test that was resolved by pydantic version update

* Remove debugging prints

* Linter fixes

* Fix linter error

* Move creation of writer outside of base AreaDetector class init per review

* Make sure we don't wait for capture to be done!

* Allow for specifying whether or not to use fileio signals for dataset description

* Revert "Allow for specifying whether or not to use fileio signals for dataset description"

This reverts commit 488d7eb.

* Fix linter errors, remove unused enum

* Change from return to await to conform to return type

* Apply more suggestions from review

* Replace instances of DeviceCollector to init_devices

* Update src/ophyd_async/epics/adaravis/_aravis_controller.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Update src/ophyd_async/epics/adaravis/_aravis_controller.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Update src/ophyd_async/epics/adcore/_core_writer.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Update src/ophyd_async/epics/adcore/_core_logic.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Update src/ophyd_async/epics/adcore/_core_detector.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Update src/ophyd_async/epics/adcore/_core_detector.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Update src/ophyd_async/epics/adcore/_core_writer.py

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

* Fix all tests aside from 3.12 unawaited coro after applying suggestions

* Resolve unawaited coro error on 3.12

* Remove unused typevar

---------

Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>

src/ophyd_async/core/__init__.py

@@ -1,5 +1,6 @@
 from ._detector import (
     DetectorController,
+    DetectorControllerT,
     DetectorTrigger,
     DetectorWriter,
     StandardDetector,
@@ -81,6 +82,7 @@
 
 __all__ = [
     "DetectorController",
+    "DetectorControllerT",
     "DetectorTrigger",
     "DetectorWriter",
     "StandardDetector",

src/ophyd_async/core/_detector.py

@@ -5,10 +5,15 @@
 from abc import ABC, abstractmethod
 from collections.abc import AsyncGenerator, AsyncIterator, Callable, Iterator, Sequence
 from functools import cached_property
+from typing import (
+    Generic,
+    TypeVar,
+)
 
 from bluesky.protocols import (
     Collectable,
     Flyable,
+    Hints,
     Preparable,
     Reading,
     Stageable,
@@ -87,7 +92,7 @@ def get_deadtime(self, exposure: float | None) -> float:
         """For a given exposure, how long should the time between exposures be"""
 
     @abstractmethod
-    async def prepare(self, trigger_info: TriggerInfo):
+    async def prepare(self, trigger_info: TriggerInfo) -> None:
         """
         Do all necessary steps to prepare the detector for triggers.
 
@@ -157,6 +162,16 @@ def collect_stream_docs(self, indices_written: int) -> AsyncIterator[StreamAsset
     async def close(self) -> None:
         """Close writer, blocks until I/O is complete"""
 
+    @property
+    def hints(self) -> Hints:
+        return {}
+
+
+# Add type var for controller so we can define
+# StandardDetector[KinetixController, ADWriter] for example
+DetectorControllerT = TypeVar("DetectorControllerT", bound=DetectorController)
+DetectorWriterT = TypeVar("DetectorWriterT", bound=DetectorWriter)
+
 
 class StandardDetector(
     Device,
@@ -168,6 +183,7 @@ class StandardDetector(
     Flyable,
     Collectable,
     WritesStreamAssets,
+    Generic[DetectorControllerT, DetectorWriterT],
 ):
     """
     Useful detector base class for step and fly scanning detectors.
@@ -176,8 +192,8 @@ class StandardDetector(
 
     def __init__(
         self,
-        controller: DetectorController,
-        writer: DetectorWriter,
+        controller: DetectorControllerT,
+        writer: DetectorWriterT,
         config_sigs: Sequence[SignalR] = (),
         name: str = "",
         connector: DeviceConnector | None = None,
@@ -211,19 +227,11 @@ def __init__(
         self._initial_frame: int = 0
         super().__init__(name, connector=connector)
 
-    @property
-    def controller(self) -> DetectorController:
-        return self._controller
-
-    @property
-    def writer(self) -> DetectorWriter:
-        return self._writer
-
     @AsyncStatus.wrap
     async def stage(self) -> None:
         # Disarm the detector, stop file writing.
         await self._check_config_sigs()
-        await asyncio.gather(self.writer.close(), self.controller.disarm())
+        await asyncio.gather(self._writer.close(), self._controller.disarm())
         self._trigger_info = None
 
     async def _check_config_sigs(self):
@@ -244,7 +252,7 @@ async def _check_config_sigs(self):
     @AsyncStatus.wrap
     async def unstage(self) -> None:
         # Stop data writing.
-        await asyncio.gather(self.writer.close(), self.controller.disarm())
+        await asyncio.gather(self._writer.close(), self._controller.disarm())
 
     async def read_configuration(self) -> dict[str, Reading]:
         return await merge_gathered_dicts(sig.read() for sig in self._config_sigs)
@@ -274,12 +282,12 @@ async def trigger(self) -> None:
         assert self._trigger_info
         assert self._trigger_info.trigger is DetectorTrigger.INTERNAL
         # Arm the detector and wait for it to finish.
-        indices_written = await self.writer.get_indices_written()
-        await self.controller.arm()
-        await self.controller.wait_for_idle()
+        indices_written = await self._writer.get_indices_written()
+        await self._controller.arm()
+        await self._controller.wait_for_idle()
         end_observation = indices_written + 1
 
-        async for index in self.writer.observe_indices_written(
+        async for index in self._writer.observe_indices_written(
             DEFAULT_TIMEOUT
             + (self._trigger_info.livetime or 0)
             + (self._trigger_info.deadtime or 0)
@@ -308,9 +316,9 @@ async def prepare(self, value: TriggerInfo) -> None:
                 value.deadtime
             ), "Deadtime must be supplied when in externally triggered mode"
         if value.deadtime:
-            required = self.controller.get_deadtime(value.livetime)
+            required = self._controller.get_deadtime(value.livetime)
             assert required <= value.deadtime, (
-                f"Detector {self.controller} needs at least {required}s deadtime, "
+                f"Detector {self._controller} needs at least {required}s deadtime, "
                 f"but trigger logic provides only {value.deadtime}s"
             )
         self._trigger_info = value
@@ -319,12 +327,12 @@ async def prepare(self, value: TriggerInfo) -> None:
             if isinstance(self._trigger_info.number_of_triggers, list)
             else [self._trigger_info.number_of_triggers]
         )
-        self._initial_frame = await self.writer.get_indices_written()
+        self._initial_frame = await self._writer.get_indices_written()
         self._describe, _ = await asyncio.gather(
-            self.writer.open(value.multiplier), self.controller.prepare(value)
+            self._writer.open(value.multiplier), self._controller.prepare(value)
         )
         if value.trigger != DetectorTrigger.INTERNAL:
-            await self.controller.arm()
+            await self._controller.arm()
             self._fly_start = time.monotonic()
 
     @AsyncStatus.wrap
@@ -343,7 +351,7 @@ async def kickoff(self):
     @WatchableAsyncStatus.wrap
     async def complete(self):
         assert self._trigger_info
-        indices_written = self.writer.observe_indices_written(
+        indices_written = self._writer.observe_indices_written(
             self._trigger_info.frame_timeout
             or (
                 DEFAULT_TIMEOUT
@@ -372,7 +380,7 @@ async def complete(self):
                 self._completable_frames = 0
                 self._frames_to_complete = 0
                 self._number_of_triggers_iter = None
-                await self.controller.wait_for_idle()
+                await self._controller.wait_for_idle()
 
     async def describe_collect(self) -> dict[str, DataKey]:
         return self._describe
@@ -384,9 +392,13 @@ async def collect_asset_docs(
         # The index is optional, and provided for fly scans, however this needs to be
         # retrieved for step scans.
         if index is None:
-            index = await self.writer.get_indices_written()
-        async for doc in self.writer.collect_stream_docs(index):
+            index = await self._writer.get_indices_written()
+        async for doc in self._writer.collect_stream_docs(index):
             yield doc
 
     async def get_index(self) -> int:
-        return await self.writer.get_indices_written()
+        return await self._writer.get_indices_written()
+
+    @property
+    def hints(self) -> Hints:
+        return self._writer.hints

src/ophyd_async/core/_flyer.py

@@ -1,5 +1,5 @@
 from abc import ABC, abstractmethod
-from typing import Generic
+from typing import Any, Generic
 
 from bluesky.protocols import Flyable, Preparable, Stageable
 
@@ -10,7 +10,7 @@
 
 class FlyerController(ABC, Generic[T]):
     @abstractmethod
-    async def prepare(self, value: T):
+    async def prepare(self, value: T) -> Any:
         """Move to the start of the flyscan"""
 
     @abstractmethod

src/ophyd_async/epics/adaravis/__init__.py

@@ -1,9 +1,11 @@
 from ._aravis import AravisDetector
 from ._aravis_controller import AravisController
-from ._aravis_io import AravisDriverIO
+from ._aravis_io import AravisDriverIO, AravisTriggerMode, AravisTriggerSource
 
 __all__ = [
     "AravisDetector",
     "AravisController",
     "AravisDriverIO",
+    "AravisTriggerMode",
+    "AravisTriggerSource",
 ]

src/ophyd_async/epics/adaravis/_aravis.py

@@ -1,61 +1,47 @@
-from typing import get_args
+from collections.abc import Sequence
 
-from bluesky.protocols import HasHints, Hints
-
-from ophyd_async.core import PathProvider, StandardDetector
+from ophyd_async.core import PathProvider
+from ophyd_async.core._signal import SignalR
 from ophyd_async.epics import adcore
 
 from ._aravis_controller import AravisController
 from ._aravis_io import AravisDriverIO
 
 
-class AravisDetector(StandardDetector, HasHints):
+class AravisDetector(adcore.AreaDetector[AravisController]):
     """
     Ophyd-async implementation of an ADAravis Detector.
     The detector may be configured for an external trigger on a GPIO port,
     which must be done prior to preparing the detector
     """
 
-    _controller: AravisController
-    _writer: adcore.ADHDFWriter
-
     def __init__(
         self,
         prefix: str,
         path_provider: PathProvider,
         drv_suffix="cam1:",
-        hdf_suffix="HDF1:",
-        name="",
+        writer_cls: type[adcore.ADWriter] = adcore.ADHDFWriter,
+        fileio_suffix: str | None = None,
+        name: str = "",
         gpio_number: AravisController.GPIO_NUMBER = 1,
+        config_sigs: Sequence[SignalR] = (),
+        plugins: dict[str, adcore.NDPluginBaseIO] | None = None,
     ):
-        self.drv = AravisDriverIO(prefix + drv_suffix)
-        self.hdf = adcore.NDFileHDFIO(prefix + hdf_suffix)
+        driver = AravisDriverIO(prefix + drv_suffix)
+        controller = AravisController(driver, gpio_number=gpio_number)
+
+        writer = writer_cls.with_io(
+            prefix,
+            path_provider,
+            dataset_source=driver,
+            fileio_suffix=fileio_suffix,
+            plugins=plugins,
+        )
 
         super().__init__(
-            AravisController(self.drv, gpio_number=gpio_number),
-            adcore.ADHDFWriter(
-                self.hdf,
-                path_provider,
-                lambda: self.name,
-                adcore.ADBaseDatasetDescriber(self.drv),
-            ),
-            config_sigs=(self.drv.acquire_time,),
+            controller=controller,
+            writer=writer,
+            plugins=plugins,
             name=name,
+            config_sigs=config_sigs,
         )
-
-    def get_external_trigger_gpio(self):
-        return self._controller.gpio_number
-
-    def set_external_trigger_gpio(self, gpio_number: AravisController.GPIO_NUMBER):
-        supported_gpio_numbers = get_args(AravisController.GPIO_NUMBER)
-        if gpio_number not in supported_gpio_numbers:
-            raise ValueError(
-                f"{self.__class__.__name__} only supports the following GPIO "
-                f"indices: {supported_gpio_numbers} but was asked to "
-                f"use {gpio_number}"
-            )
-        self._controller.gpio_number = gpio_number
-
-    @property
-    def hints(self) -> Hints:
-        return self._writer.hints

src/ophyd_async/epics/adaravis/_aravis_controller.py

@@ -2,11 +2,8 @@
 from typing import Literal
 
 from ophyd_async.core import (
-    AsyncStatus,
-    DetectorController,
     DetectorTrigger,
     TriggerInfo,
-    set_and_wait_for_value,
 )
 from ophyd_async.epics import adcore
 
@@ -18,13 +15,17 @@
 _HIGHEST_POSSIBLE_DEADTIME = 1961e-6
 
 
-class AravisController(DetectorController):
+class AravisController(adcore.ADBaseController[AravisDriverIO]):
     GPIO_NUMBER = Literal[1, 2, 3, 4]
 
-    def __init__(self, driver: AravisDriverIO, gpio_number: GPIO_NUMBER) -> None:
-        self._drv = driver
+    def __init__(
+        self,
+        driver: AravisDriverIO,
+        good_states: frozenset[adcore.DetectorState] = adcore.DEFAULT_GOOD_STATES,
+        gpio_number: GPIO_NUMBER = 1,
+    ) -> None:
+        super().__init__(driver, good_states=good_states)
         self.gpio_number = gpio_number
-        self._arm_status: AsyncStatus | None = None
 
     def get_deadtime(self, exposure: float | None) -> float:
         return _HIGHEST_POSSIBLE_DEADTIME
@@ -35,25 +36,18 @@ async def prepare(self, trigger_info: TriggerInfo):
         else:
             image_mode = adcore.ImageMode.MULTIPLE
         if (exposure := trigger_info.livetime) is not None:
-            await self._drv.acquire_time.set(exposure)
+            asyncio.gather(self.driver.acquire_time.set(exposure))
 
         trigger_mode, trigger_source = self._get_trigger_info(trigger_info.trigger)
         # trigger mode must be set first and on it's own!
-        await self._drv.trigger_mode.set(trigger_mode)
+        await self.driver.trigger_mode.set(trigger_mode)
 
         await asyncio.gather(
-            self._drv.trigger_source.set(trigger_source),
-            self._drv.num_images.set(trigger_info.total_number_of_triggers),
-            self._drv.image_mode.set(image_mode),
+            self.driver.trigger_source.set(trigger_source),
+            self.driver.num_images.set(trigger_info.total_number_of_triggers),
+            self.driver.image_mode.set(image_mode),
         )
 
-    async def arm(self):
-        self._arm_status = await set_and_wait_for_value(self._drv.acquire, True)
-
-    async def wait_for_idle(self):
-        if self._arm_status:
-            await self._arm_status
-
     def _get_trigger_info(
         self, trigger: DetectorTrigger
     ) -> tuple[AravisTriggerMode, AravisTriggerSource]:
@@ -72,6 +66,3 @@ def _get_trigger_info(
             return AravisTriggerMode.OFF, AravisTriggerSource.FREERUN
         else:
             return (AravisTriggerMode.ON, f"Line{self.gpio_number}")  # type: ignore
-
-    async def disarm(self):
-        await adcore.stop_busy_record(self._drv.acquire, False, timeout=1)