diff --git a/.github/CONTRIBUTING.rst b/.github/CONTRIBUTING.rst index fe19bae11b..36ea1e20b3 100644 --- a/.github/CONTRIBUTING.rst +++ b/.github/CONTRIBUTING.rst @@ -24,6 +24,7 @@ Code coverage While 100% code coverage does not make a library bug-free, it significantly reduces the number of easily caught bugs! Please make sure coverage remains the same or is improved by a pull request! + Developer guide --------------- diff --git a/docs/user/examples/epics_demo.py b/docs/user/examples/epics_demo.py index 862377e73d..0f0b1bfe6f 100644 --- a/docs/user/examples/epics_demo.py +++ b/docs/user/examples/epics_demo.py @@ -33,4 +33,5 @@ class OldSensor(Device): # Create ophyd-async devices with DeviceCollector(): det = demo.Sensor(pv_prefix) + det_group = demo.SensorGroup(pv_prefix) samp = demo.SampleStage(pv_prefix) diff --git a/docs/user/how-to/compound-devices.rst b/docs/user/how-to/compound-devices.rst new file mode 100644 index 0000000000..4722b58f9a --- /dev/null +++ b/docs/user/how-to/compound-devices.rst @@ -0,0 +1,44 @@ +.. note:: + + Ophyd async is included on a provisional basis until the v1.0 release and + may change API on minor release numbers before then + +Compound Devices Together +========================= + +Assembly +-------- + +Compound assemblies can be used to group Devices into larger logical Devices: + +.. literalinclude:: ../../../src/ophyd_async/epics/demo/__init__.py + :pyobject: SampleStage + +This applies prefixes on construction: + +- SampleStage is passed a prefix like ``DEVICE:`` +- SampleStage.x will append its prefix ``X:`` to get ``DEVICE:X:`` +- SampleStage.x.velocity will append its suffix ``Velocity`` to get + ``DEVICE:X:Velocity`` + +If SampleStage is further nested in another Device another layer of prefix nesting would occur + +.. note:: + + SampleStage does not pass any signals into its superclass init. This means + that its ``read()`` method will return an empty dictionary. This means you + can ``rd sample_stage.x``, but not ``rd sample_stage``. + + +Grouping by Index +----------------- + +Sometimes, it makes sense to group devices by number, say an array of sensors: + +.. literalinclude:: ../../../src/ophyd_async/epics/demo/__init__.py + :pyobject: SensorGroup + +:class:`~ophyd-async.core.DeviceVector` allows writing maintainable, arbitrary-length device groups instead of fixed classes for each possible grouping. A :class:`~ophyd-async.core.DeviceVector` can be accessed via indices, for example: ``my_sensor_group.sensors[2]``. Here ``sensors`` is a dictionary with integer indices rather than a list so that the most semantically sensible indices may be used, the sensor group above may be 1-indexed, for example, because the sensors' datasheet calls them "sensor 1", "sensor 2" etc. + +.. note:: + The :class:`~ophyd-async.core.DeviceVector` adds an extra level of nesting to the device tree compared to static components like ``sensor_1``, ``sensor_2`` etc. so the behavior is not completely equivalent. diff --git a/docs/user/how-to/make-a-simple-device.rst b/docs/user/how-to/make-a-simple-device.rst index 86b112d307..dcad73bb99 100644 --- a/docs/user/how-to/make-a-simple-device.rst +++ b/docs/user/how-to/make-a-simple-device.rst @@ -64,27 +64,3 @@ completes. This co-routine is wrapped in a timeout handler, and passed to an `AsyncStatus` which will start executing it as soon as the Run Engine adds a callback to it. The ``stop()`` method then pokes a PV if the move needs to be interrupted. - -Assembly --------- - -Compound assemblies can be used to group Devices into larger logical Devices: - -.. literalinclude:: ../../../src/ophyd_async/epics/demo/__init__.py - :pyobject: SampleStage - -This applies prefixes on construction: - -- SampleStage is passed a prefix like ``DEVICE:`` -- SampleStage.x will append its prefix ``X:`` to get ``DEVICE:X:`` -- SampleStage.x.velocity will append its suffix ``Velocity`` to get - ``DEVICE:X:Velocity`` - -If SampleStage is further nested in another Device another layer of prefix -nesting would occur - -.. note:: - - SampleStage does not pass any signals into its superclass init. This means - that its ``read()`` method will return an empty dictionary. This means you - can ``rd sample_stage.x``, but not ``rd sample_stage``. diff --git a/docs/user/index.rst b/docs/user/index.rst index c6ef7fdd03..bc6f84e684 100644 --- a/docs/user/index.rst +++ b/docs/user/index.rst @@ -32,6 +32,7 @@ side-bar. :maxdepth: 1 how-to/make-a-simple-device + how-to/compound-devices how-to/write-tests-for-devices how-to/run-container diff --git a/src/ophyd_async/core/device.py b/src/ophyd_async/core/device.py index 4733a1bc2c..07145ae8f2 100644 --- a/src/ophyd_async/core/device.py +++ b/src/ophyd_async/core/device.py @@ -82,6 +82,14 @@ async def connect(self, sim: bool = False, timeout: float = DEFAULT_TIMEOUT): class DeviceVector(Dict[int, VT], Device): + """ + Defines device components with indices. + + In the below example, foos becomes a dictionary on the parent device + at runtime, so parent.foos[2] returns a FooDevice. For example usage see + :class:`~ophyd_async.epics.demo.DynamicSensorGroup` + """ + def children(self) -> Generator[Tuple[str, Device], None, None]: for attr_name, attr in self.items(): if isinstance(attr, Device): diff --git a/src/ophyd_async/core/signal.py b/src/ophyd_async/core/signal.py index 5bb73cb42d..567694425e 100644 --- a/src/ophyd_async/core/signal.py +++ b/src/ophyd_async/core/signal.py @@ -278,15 +278,15 @@ async def assert_value(signal: SignalR[T], value: T) -> None: ---------- signal: Call get_valueand and compare it with expected value. - value: - the expected value from the signal. + value: + the expected value from the signal. Notes ----- Example usage:: await assert_reading(signal, value) - - + + """ assert await signal.get_value() == value diff --git a/src/ophyd_async/epics/demo/__init__.py b/src/ophyd_async/epics/demo/__init__.py index 73833c1731..2df6af3e4b 100644 --- a/src/ophyd_async/epics/demo/__init__.py +++ b/src/ophyd_async/epics/demo/__init__.py @@ -14,7 +14,13 @@ import numpy as np from bluesky.protocols import Movable, Stoppable -from ophyd_async.core import AsyncStatus, Device, StandardReadable, observe_value +from ophyd_async.core import ( + AsyncStatus, + Device, + DeviceVector, + StandardReadable, + observe_value, +) from ..signal.signal import epics_signal_r, epics_signal_rw, epics_signal_x @@ -43,6 +49,19 @@ def __init__(self, prefix: str, name="") -> None: super().__init__(name=name) +class SensorGroup(StandardReadable): + def __init__(self, prefix: str, name: str = "", sensor_count: int = 3) -> None: + self.sensors = DeviceVector( + {i: Sensor(f"{prefix}{i}:") for i in range(1, sensor_count + 1)} + ) + + # Makes read() produce the values of all sensors + self.set_readable_signals( + read=[sensor.value for sensor in self.sensors.values()], + ) + super().__init__(name) + + class Mover(StandardReadable, Movable, Stoppable): """A demo movable that moves based on velocity""" @@ -135,11 +154,22 @@ def start_ioc_subprocess() -> str: pv_prefix = "".join(random.choice(string.ascii_uppercase) for _ in range(12)) + ":" here = Path(__file__).absolute().parent args = [sys.executable, "-m", "epicscorelibs.ioc"] + + # Create standalone sensor args += ["-m", f"P={pv_prefix}"] args += ["-d", str(here / "sensor.db")] - for suff in "XY": - args += ["-m", f"P={pv_prefix}{suff}:"] + + # Create sensor group + for suffix in ["1", "2", "3"]: + args += ["-m", f"P={pv_prefix}{suffix}:"] + args += ["-d", str(here / "sensor.db")] + + # Create X and Y motors + for suffix in ["X", "Y"]: + args += ["-m", f"P={pv_prefix}{suffix}:"] args += ["-d", str(here / "mover.db")] + + # Start IOC process = subprocess.Popen( args, stdin=subprocess.PIPE, diff --git a/src/ophyd_async/epics/pvi/pvi.py b/src/ophyd_async/epics/pvi/pvi.py index dc211250db..37dc70c5ed 100644 --- a/src/ophyd_async/epics/pvi/pvi.py +++ b/src/ophyd_async/epics/pvi/pvi.py @@ -1,7 +1,7 @@ import re from dataclasses import dataclass -from inspect import isclass from typing import ( + Any, Callable, Dict, FrozenSet, @@ -44,6 +44,18 @@ def _strip_number_from_string(string: str) -> Tuple[str, Optional[int]]: return name, number +def _split_subscript(tp: T) -> Union[Tuple[Any, Tuple[Any]], Tuple[T, None]]: + """Split a subscripted type into the its origin and args. + + If `tp` is not a subscripted type, then just return the type and None as args. + + """ + if get_origin(tp) is not None: + return get_origin(tp), get_args(tp) + + return tp, None + + def _strip_union(field: Union[Union[T], T]) -> T: if get_origin(field) is Union: args = get_args(field) @@ -115,86 +127,70 @@ def _parse_type( ): if common_device_type: # pre-defined type - device_type = _strip_union(common_device_type) - is_device_vector, device_type = _strip_device_vector(device_type) - - if ((origin := get_origin(device_type)) and issubclass(origin, Signal)) or ( - isclass(device_type) and issubclass(device_type, Signal) - ): - # if device_type is of the form `Signal` or `Signal[type]` - is_signal = True - signal_dtype = get_args(device_type)[0] - else: - is_signal = False - signal_dtype = None + device_cls = _strip_union(common_device_type) + is_device_vector, device_cls = _strip_device_vector(device_cls) + device_cls, device_args = _split_subscript(device_cls) + assert issubclass(device_cls, Device) + + is_signal = issubclass(device_cls, Signal) + signal_dtype = device_args[0] if device_args is not None else None elif is_pvi_table: # is a block, we can make it a DeviceVector if it ends in a number is_device_vector = number_suffix is not None is_signal = False signal_dtype = None - device_type = Device + device_cls = Device else: # is a signal, signals aren't stored in DeviceVectors unless # they're defined as such in the common_device_type is_device_vector = False is_signal = True signal_dtype = None - device_type = Signal + device_cls = Signal - return is_device_vector, is_signal, signal_dtype, device_type + return is_device_vector, is_signal, signal_dtype, device_cls def _sim_common_blocks(device: Device, stripped_type: Optional[Type] = None): device_t = stripped_type or type(device) - for sub_name, sub_device_t in get_type_hints(device_t).items(): - if sub_name in ("_name", "parent"): - continue + sub_devices = ( + (field, field_type) + for field, field_type in get_type_hints(device_t).items() + if field not in ("_name", "parent") + ) + + for device_name, device_cls in sub_devices: + device_cls = _strip_union(device_cls) + is_device_vector, device_cls = _strip_device_vector(device_cls) + device_cls, device_args = _split_subscript(device_cls) + assert issubclass(device_cls, Device) - # we'll take the first type in the union which isn't NoneType - sub_device_t = _strip_union(sub_device_t) - is_device_vector, sub_device_t = _strip_device_vector(sub_device_t) - is_signal = ( - (origin := get_origin(sub_device_t)) and issubclass(origin, Signal) - ) or (issubclass(sub_device_t, Signal)) + is_signal = issubclass(device_cls, Signal) + signal_dtype = device_args[0] if device_args is not None else None - # TODO: worth coming back to all this code once 3.9 is gone and we can use - # match statments: https://github.com/bluesky/ophyd-async/issues/180 if is_device_vector: if is_signal: - signal_type = args[0] if (args := get_args(sub_device_t)) else None - sub_device_1 = sub_device_t(SimSignalBackend(signal_type, sub_name)) - sub_device_2 = sub_device_t(SimSignalBackend(signal_type, sub_name)) - sub_device = DeviceVector( - { - 1: sub_device_1, - 2: sub_device_2, - } - ) + sub_device_1 = device_cls(SimSignalBackend(signal_dtype, device_name)) + sub_device_2 = device_cls(SimSignalBackend(signal_dtype, device_name)) + sub_device = DeviceVector({1: sub_device_1, 2: sub_device_2}) else: - sub_device = DeviceVector( - { - 1: sub_device_t(), - 2: sub_device_t(), - } - ) + sub_device = DeviceVector({1: device_cls(), 2: device_cls()}) + + for sub_device_in_vector in sub_device.values(): + _sim_common_blocks(sub_device_in_vector, stripped_type=device_cls) + for value in sub_device.values(): value.parent = sub_device - - elif is_signal: - signal_type = args[0] if (args := get_args(sub_device_t)) else None - sub_device = sub_device_t(SimSignalBackend(signal_type, sub_name)) else: - sub_device = sub_device_t() - - if not is_signal: - if is_device_vector: - for sub_device_in_vector in sub_device.values(): - _sim_common_blocks(sub_device_in_vector, stripped_type=sub_device_t) + if is_signal: + sub_device = device_cls(SimSignalBackend(signal_dtype, device_name)) else: - _sim_common_blocks(sub_device, stripped_type=sub_device_t) + sub_device = device_cls() + + _sim_common_blocks(sub_device, stripped_type=device_cls) - setattr(device, sub_name, sub_device) + setattr(device, device_name, sub_device) sub_device.parent = device diff --git a/tests/epics/demo/test_demo.py b/tests/epics/demo/test_demo.py index 5b8847e842..832143de99 100644 --- a/tests/epics/demo/test_demo.py +++ b/tests/epics/demo/test_demo.py @@ -1,6 +1,7 @@ import asyncio +import subprocess from typing import Dict -from unittest.mock import ANY, Mock, call +from unittest.mock import ANY, Mock, call, patch import pytest from bluesky.protocols import Reading @@ -19,7 +20,7 @@ @pytest.fixture -async def sim_mover(): +async def sim_mover() -> demo.Mover: async with DeviceCollector(sim=True): sim_mover = demo.Mover("BLxxI-MO-TABLE-01:X:") # Signals connected here @@ -28,17 +29,27 @@ async def sim_mover(): set_sim_value(sim_mover.units, "mm") set_sim_value(sim_mover.precision, 3) set_sim_value(sim_mover.velocity, 1) - yield sim_mover + return sim_mover @pytest.fixture -async def sim_sensor(): +async def sim_sensor() -> demo.Sensor: async with DeviceCollector(sim=True): sim_sensor = demo.Sensor("SIM:SENSOR:") # Signals connected here assert sim_sensor.name == "sim_sensor" - yield sim_sensor + return sim_sensor + + +@pytest.fixture +async def sim_sensor_group() -> demo.SensorGroup: + async with DeviceCollector(sim=True): + sim_sensor_group = demo.SensorGroup("SIM:SENSOR:") + # Signals connected here + + assert sim_sensor_group.name == "sim_sensor_group" + return sim_sensor_group class Watcher: @@ -224,3 +235,71 @@ def my_plan(): with pytest.raises(RuntimeError, match="Will deadlock run engine if run in a plan"): RE(my_plan()) + + +async def test_dynamic_sensor_group_disconnected(): + with pytest.raises(NotConnected): + async with DeviceCollector(timeout=0.1): + sim_sensor_group_dynamic = demo.SensorGroup("SIM:SENSOR:") + + assert sim_sensor_group_dynamic.name == "sim_sensor_group_dynamic" + + +async def test_dynamic_sensor_group_read_and_describe( + sim_sensor_group: demo.SensorGroup, +): + set_sim_value(sim_sensor_group.sensors[1].value, 0.0) + set_sim_value(sim_sensor_group.sensors[2].value, 0.5) + set_sim_value(sim_sensor_group.sensors[3].value, 1.0) + + await sim_sensor_group.stage() + description = await sim_sensor_group.describe() + reading = await sim_sensor_group.read() + await sim_sensor_group.unstage() + + assert description == { + "sim_sensor_group-sensors-1-value": { + "dtype": "number", + "shape": [], + "source": "sim://SIM:SENSOR:1:Value", + }, + "sim_sensor_group-sensors-2-value": { + "dtype": "number", + "shape": [], + "source": "sim://SIM:SENSOR:2:Value", + }, + "sim_sensor_group-sensors-3-value": { + "dtype": "number", + "shape": [], + "source": "sim://SIM:SENSOR:3:Value", + }, + } + assert reading == { + "sim_sensor_group-sensors-1-value": { + "alarm_severity": 0, + "timestamp": ANY, + "value": 0.0, + }, + "sim_sensor_group-sensors-2-value": { + "alarm_severity": 0, + "timestamp": ANY, + "value": 0.5, + }, + "sim_sensor_group-sensors-3-value": { + "alarm_severity": 0, + "timestamp": ANY, + "value": 1.0, + }, + } + + +@patch("ophyd_async.epics.demo.subprocess.Popen") +async def test_ioc_starts(mock_popen: Mock): + demo.start_ioc_subprocess() + mock_popen.assert_called_once_with( + ANY, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True, + )