From 2bdacddde2559c4d91597c23c09ba1e92922538f Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Tue, 10 Nov 2026 12:18:06 +0000 Subject: [PATCH] Add plugins_with_proc for the areaDetector process plugin adcore.plugins_with_proc() builds the "proc" plugin from proc_suffix and adds it to a detector's plugins, rejecting a "proc" passed in plugins. The seven vendor detectors (Andor2, Aravis, Kinetix, Merlin, Pilatus, SimDetector, Vimba) and ContAcqDetector use it and pass the process plugin to their trigger logic as before. Includes the changelog entry, the how-to line and the test for the reserved name. Assisted-by: Claude Opus 5.5 --- CHANGELOG.md | 7 ++++ docs/how-to/implement-ad-detector.md | 9 ++--- src/ophyd_async/epics/adandor.py | 11 +++--- src/ophyd_async/epics/adaravis.py | 11 +++--- src/ophyd_async/epics/adcore/__init__.py | 3 +- src/ophyd_async/epics/adcore/_detector.py | 35 +++++++++++++++---- src/ophyd_async/epics/adkinetix.py | 11 +++--- src/ophyd_async/epics/admerlin.py | 11 +++--- src/ophyd_async/epics/adpilatus.py | 11 +++--- src/ophyd_async/epics/adsimdetector.py | 11 +++--- src/ophyd_async/epics/advimba.py | 11 +++--- .../unit_tests/epics/adcore/test_detector.py | 8 +++++ 12 files changed, 100 insertions(+), 39 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000000..3e4b5f36a9 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,7 @@ +# Changelog + +## Unreleased + +### Added + +- `plugins_with_proc()` adds the process plugin made from a `proc_suffix` to a detector's `plugins`, and raises `ValueError` if `plugins` already has a `"proc"`. Every areaDetector vendor detector and `ContAcqDetector` uses it, so `"proc"` is reserved: pass `proc_suffix` instead. diff --git a/docs/how-to/implement-ad-detector.md b/docs/how-to/implement-ad-detector.md index 232dc9748e..93b0e4c638 100644 --- a/docs/how-to/implement-ad-detector.md +++ b/docs/how-to/implement-ad-detector.md @@ -68,7 +68,7 @@ Most areaDetectors can use the standard [](#adcore.ADAcquireLogic) which handles ## Add a Detector that puts it all together -Now you need to make an [](#adcore.AreaDetector) subclass that uses your IO and Trigger Logic with the standard Arm Logic and Writer classes that come with ADCore. The `__init__` method should: +Now you need to make an [](#adcore.AreaDetector) subclass that uses your IO and Trigger Logic with the standard acquire logic and data logic that come with ADCore. The `__init__` method should: 1. Create the driver IO instance 2. Create instances of your logic classes 3. Call `super().__init__()` passing the driver and logic instances to the baseclass @@ -77,6 +77,7 @@ The constructor parameters should include: - `prefix`: The PV prefix for the driver and plugins - `*writer_factories`: Zero or more [](#adcore.ADWriterFactory) instances (e.g. `ADWriterFactory.hdf(path_provider)`); omit entirely to skip file writing - `driver_suffix`: A PV suffix for the driver, defaulting to `"cam1:"` +- `proc_suffix`: An optional PV suffix for a process plugin, which the detector creates with [](#adcore.plugins_with_proc) and gives to its trigger logic - `plugins`: An optional mapping of {`name`: [](#adcore.NDPluginBaseIO)} for additional plugins - `config_sigs`: Additional signals to report in configuration (beyond the standard acquire_time and acquire_period) - `name`: An optional name for the device @@ -90,7 +91,7 @@ For example, for ADAravis: The `AreaDetector` baseclass will: - Store the driver as `self.driver` -- Call `add_detector_logics()` to register your trigger and arm logic +- Call `add_detector_logics()` to register your trigger and acquire logic - Create and register a data logic for file writing for each factory in `writer_factories` - Add configuration signals (driver.acquire_time, driver.acquire_period, and any you specify) - Store any plugins as attributes on the detector @@ -256,9 +257,9 @@ And test internal triggering: You have now made a detector, and can import and create it like this: ```python -from ophyd_async.epics import adaravis +from ophyd_async.epics import adaravis, adcore -det = adaravis.AravisDetector("PREFIX:", path_provider) +det = adaravis.AravisDetector("PREFIX:", adcore.ADWriterFactory.hdf(path_provider)) ``` The detector will now support: diff --git a/src/ophyd_async/epics/adandor.py b/src/ophyd_async/epics/adandor.py index 5e1feef409..95d4c7963c 100644 --- a/src/ophyd_async/epics/adandor.py +++ b/src/ophyd_async/epics/adandor.py @@ -21,6 +21,7 @@ AreaDetector, NDPluginBaseIO, NDProcessIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -97,7 +98,9 @@ class AndorDetector(AreaDetector[Andor2DriverIO]): :param writer_factories: Factories for file writer plugins and their data logics :param driver_suffix: Suffix for the driver PV, defaults to "cam1:" :param proc_suffix: If provided, an NDProcessIO plugin is created at this suffix - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -113,14 +116,14 @@ def __init__( name: str = "", ) -> None: driver = Andor2DriverIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver), - trigger_logic=Andor2TriggerLogic(driver, proc_plugin), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=Andor2TriggerLogic(driver, proc), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/adaravis.py b/src/ophyd_async/epics/adaravis.py index 695c78e886..2a0d89d176 100644 --- a/src/ophyd_async/epics/adaravis.py +++ b/src/ophyd_async/epics/adaravis.py @@ -23,6 +23,7 @@ AreaDetector, NDPluginBaseIO, NDProcessIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -104,7 +105,9 @@ class AravisDetector(AreaDetector[AravisDriverIO]): :param override_deadtime: If provided, this value is used for deadtime instead of looking up based on camera model. - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -121,14 +124,14 @@ def __init__( name: str = "", ) -> None: driver = AravisDriverIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver), - trigger_logic=AravisTriggerLogic(driver, proc_plugin, override_deadtime), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=AravisTriggerLogic(driver, proc, override_deadtime), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/adcore/__init__.py b/src/ophyd_async/epics/adcore/__init__.py index 08dbd0836d..27a079424b 100644 --- a/src/ophyd_async/epics/adcore/__init__.py +++ b/src/ophyd_async/epics/adcore/__init__.py @@ -11,7 +11,7 @@ NDArrayDescription, PluginSignalDataLogic, ) -from ._detector import AreaDetector, ContAcqDetector +from ._detector import AreaDetector, ContAcqDetector, plugins_with_proc from ._io import ( NDROIIO, ADBaseColorMode, @@ -102,6 +102,7 @@ # Detector "AreaDetector", "ContAcqDetector", + "plugins_with_proc", # NDAttributes "NDAttributeDataType", "NDAttributePvDbrType", diff --git a/src/ophyd_async/epics/adcore/_detector.py b/src/ophyd_async/epics/adcore/_detector.py index e44e6ec5bf..71d500ca6c 100644 --- a/src/ophyd_async/epics/adcore/_detector.py +++ b/src/ophyd_async/epics/adcore/_detector.py @@ -82,6 +82,29 @@ def get_plugin( return plugin +def plugins_with_proc( + plugins: Mapping[str, NDPluginBaseIO] | None, + prefix: str, + proc_suffix: str | None, +) -> tuple[dict[str, NDPluginBaseIO], NDProcessIO | None]: + """Add a process plugin made from proc_suffix to plugins. + + :param plugins: The caller's plugins, which must not include "proc" + :param prefix: The detector's PV prefix + :param proc_suffix: The process plugin's PV suffix, if it has one + :returns: The plugins with "proc" added, and the process plugin + """ + plugins = dict(plugins or {}) + if "proc" in plugins: + msg = "Pass proc_suffix to add a process plugin, not plugins['proc']" + raise ValueError(msg) + proc = None + if proc_suffix: + proc = NDProcessIO(prefix + proc_suffix) + plugins["proc"] = proc + return plugins, proc + + class ContAcqDetector(AreaDetector[ADBaseIOT]): """Create an continuously acquiring AreaDetector instance. @@ -93,7 +116,9 @@ class ContAcqDetector(AreaDetector[ADBaseIOT]): :param proc_suffix: Suffix for the NDProcessIO plugin PV, if any, defaults to None :param num_frames_to_skip: Incoming frames to skip before each CB gate opening, defaults to 0 - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -113,16 +138,14 @@ def __init__( ) -> None: driver = driver_cls(prefix + driver_suffix) cb_plugin = NDCircularBuffIO(prefix + cb_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADContAcqAcquireLogic(driver, cb_plugin, num_frames_to_skip), - trigger_logic=ADContAcqTriggerLogic(driver, cb_plugin, proc_plugin), - plugins=(plugins or {}) - | {"cb": cb_plugin} - | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=ADContAcqTriggerLogic(driver, cb_plugin, proc), + plugins={**plugins, "cb": cb_plugin}, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/adkinetix.py b/src/ophyd_async/epics/adkinetix.py index ca855bb581..bca7aa36d4 100644 --- a/src/ophyd_async/epics/adkinetix.py +++ b/src/ophyd_async/epics/adkinetix.py @@ -22,6 +22,7 @@ ADWriterFactory, AreaDetector, NDPluginBaseIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -99,7 +100,9 @@ class KinetixDetector(AreaDetector[KinetixDriverIO]): :param writer_factories: Factories for file writer plugins and their data logics :param driver_suffix: Suffix for the driver PV, defaults to "cam1:" :param proc_suffix: If provided, an NDProcessIO plugin is created at this suffix - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -115,14 +118,14 @@ def __init__( name: str = "", ) -> None: driver = KinetixDriverIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver), - trigger_logic=KinetixTriggerLogic(driver, proc_plugin), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=KinetixTriggerLogic(driver, proc), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/admerlin.py b/src/ophyd_async/epics/admerlin.py index 0ace7baa64..c6a09295ef 100644 --- a/src/ophyd_async/epics/admerlin.py +++ b/src/ophyd_async/epics/admerlin.py @@ -26,6 +26,7 @@ AreaDetector, NDPluginBaseIO, NDProcessIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -124,7 +125,9 @@ class MerlinDetector(AreaDetector[MerlinDriverIO]): :param writer_factories: Factories for file writer plugins and their data logics :param driver_suffix: Suffix for the driver PV, defaults to "cam1:" :param proc_suffix: If provided, an NDProcessIO plugin is created at this suffix - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -140,14 +143,14 @@ def __init__( name: str = "", ) -> None: driver = MerlinDriverIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver), - trigger_logic=MerlinTriggerLogic(driver, proc_plugin), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=MerlinTriggerLogic(driver, proc), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/adpilatus.py b/src/ophyd_async/epics/adpilatus.py index a5209a0276..a6cd11ab87 100644 --- a/src/ophyd_async/epics/adpilatus.py +++ b/src/ophyd_async/epics/adpilatus.py @@ -25,6 +25,7 @@ AreaDetector, NDPluginBaseIO, NDProcessIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -145,7 +146,9 @@ class PilatusDetector(AreaDetector[PilatusDriverIO]): `PilatusTriggerMode.MULT_TRIGGER` for one image per trigger pulse. :param driver_suffix: Suffix for the driver PV, defaults to "cam1:" :param proc_suffix: If provided, an NDProcessIO plugin is created at this suffix - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -163,16 +166,16 @@ def __init__( name: str = "", ) -> None: driver = PilatusDriverIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver, driver_armed_signal=driver.armed), trigger_logic=PilatusTriggerLogic( - driver, readout_time, proc_plugin, edge_trigger_mode=edge_trigger_mode + driver, readout_time, proc, edge_trigger_mode=edge_trigger_mode ), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/adsimdetector.py b/src/ophyd_async/epics/adsimdetector.py index c73b6fd2ce..ad412452e9 100644 --- a/src/ophyd_async/epics/adsimdetector.py +++ b/src/ophyd_async/epics/adsimdetector.py @@ -18,6 +18,7 @@ AreaDetector, NDPluginBaseIO, NDProcessIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -55,7 +56,9 @@ class SimDetector(AreaDetector[ADBaseIO]): :param writer_factories: Factories for file writer plugins and their data logics :param driver_suffix: Suffix for the driver PV, defaults to "cam1:" :param proc_suffix: If provided, an NDProcessIO plugin is created at this suffix - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -71,14 +74,14 @@ def __init__( name: str = "", ) -> None: driver = ADBaseIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver), - trigger_logic=SimDetectorTriggerLogic(driver, proc_plugin), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=SimDetectorTriggerLogic(driver, proc), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/src/ophyd_async/epics/advimba.py b/src/ophyd_async/epics/advimba.py index f6afd41ba9..1f74af96e4 100644 --- a/src/ophyd_async/epics/advimba.py +++ b/src/ophyd_async/epics/advimba.py @@ -26,6 +26,7 @@ AreaDetector, NDPluginBaseIO, NDProcessIO, + plugins_with_proc, prepare_exposures, prepare_filter, trigger_info_from_driver, @@ -151,7 +152,9 @@ class VimbaDetector(AreaDetector[VimbaDriverIO]): If provided, this value is used for deadtime instead of looking up based on camera model. :param proc_suffix: If provided, an NDProcessIO plugin is created at this suffix - :param plugins: Additional areaDetector plugins to include + :param plugins: + Additional areaDetector plugins to include. "proc" is reserved: use + proc_suffix instead. :param config_sigs: Additional signals to include in configuration :param name: Name for the detector device """ @@ -168,14 +171,14 @@ def __init__( name: str = "", ) -> None: driver = VimbaDriverIO(prefix + driver_suffix) - proc_plugin = NDProcessIO(prefix + proc_suffix) if proc_suffix else None + plugins, proc = plugins_with_proc(plugins, prefix, proc_suffix) super().__init__( driver, prefix, *writer_factories, acquire_logic=ADAcquireLogic(driver), - trigger_logic=VimbaTriggerLogic(driver, proc_plugin, override_deadtime), - plugins=(plugins or {}) | ({"proc": proc_plugin} if proc_plugin else {}), + trigger_logic=VimbaTriggerLogic(driver, proc, override_deadtime), + plugins=plugins, config_sigs=config_sigs, name=name, ) diff --git a/tests/unit_tests/epics/adcore/test_detector.py b/tests/unit_tests/epics/adcore/test_detector.py index 6b9edb2633..ac6215311d 100644 --- a/tests/unit_tests/epics/adcore/test_detector.py +++ b/tests/unit_tests/epics/adcore/test_detector.py @@ -177,3 +177,11 @@ async def test_get_ndarray_resource_info_unsupported_color_mode( match=r"^Unsupported ColorMode Bayer! Only Mono and RGB1 are supported\.$", ): await det.prepare(TriggerInfo()) + + +@pytest.mark.parametrize("proc_suffix", [None, "PROC:"]) +def test_plugins_proc_is_reserved_for_proc_suffix(proc_suffix: str | None): + with pytest.raises(ValueError, match=r"Pass proc_suffix to add a process plugin"): + adcore.plugins_with_proc( + {"proc": adcore.NDProcessIO("PREFIX:PROC:")}, "PREFIX:", proc_suffix + )