Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ADR for procedural device definition #12

Merged
merged 3 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,40 @@ During this process, the folder structure should incrementally be changed to
│ │ ├── __init__.py
│ │ ├── backends
│ │ │ ├── __init__.py
│ │ │ ├── _aioca.py
│ │ │ └── _p4p.py
│ │ │ ├── signal_backend.py
│ │ │ └── sim.py
│ │ ├── devices
│ │ ├── signals
│ │ │ ├── __init__.py
│ │ │ ├── device_collector.py
│ │ │ └── ...
│ │ ├── epicsdemo
│ │ │ └── ...
│ │ ├── signal.py
│ │ ├── async_status.py
│ │ ├── device_collector.py
│ │ └── utils.py
│ └── devices
│ ├── epics
│ └── tango
│ ├── epics
│ │ ├── backends
│ │ │ ├── __init__.py
│ │ │ ├── _p4p.py
│ │ │ └── _aioca.py
│ │ ├── areadetector
│ │ │ ├── __init__.py
│ │ │ ├── ad_driver.py
│ │ │ └── ...
│ │ ├── signal
│ │ │ └── ...
│ │ └── motion
│ │ ├── __init__.py
│ │ └── motor.py
│ └── panda
│ └── ...
├── tests
│ ├── core
│ │ └── ...
│ └── devices
│ └── epics
└── ...

The `__init__.py` files of each submodule (core, devices.epics and devices.tango) will
The `__init__.py` files of each submodule (core, epics, panda and eventually tango) will
be modified such that end users experience little disruption to how they use Ophyd Async.
For such users, `from ophyd.v2.core import ...` can be replaced with
`from ophyd_async.core import ...`.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
6. Procedural Device Definitions
================================

Date: 2023-09-11

Status
------

Accepted

Context
-------

Ophyd creates devices in a declarative way:

.. code-block:: python

class Sensor(Device):
mode = Component(EpicsSignal, "Mode", kind="config")
value = Component(EpicsSignalRO, "Value", kind="hinted")

This means when you make ``device = OldSensor(pv_prefix)`` then some metaclass
magic will call ``EpicsSignal(pv_prefix + "Mode", kind="config")`` and make it
available as ``device.mode``.

ophyd-async could convert this approach to use type hints instead of metaclasses:

.. code-block:: python

from typing import Annotated as A

class Sensor(EpicsDevice):
mode: A[SignalRW, CONFIG, pv_suffix("Mode")]
value: A[SignalR, READ, pv_suffix("Value")]

The superclass init could then read all the type hints and instantiate them with
the correct SignalBackends.

Alternatively it could use a procedural approach and be explicit about where the
arguments are passed at the cost of greater verbosity:

.. code-block:: python

class Sensor(StandardReadable):
def __init__(self, prefix: str, name="") -> None:
self.value = epics_signal_r(float, prefix + "Value")
self.mode = epics_signal_rw(EnergyMode, prefix + "Mode")
# Set name and signals for read() and read_configuration()
self.set_readable_signals(read=[self.value], config=[self.mode])
super().__init__(name=name)

The procedural approach to creating child Devices is:

.. code-block:: python

class SensorGroup(Device):
def __init__(self, prefix: str, num: int, name: Optional[str]=None):
self.sensors = DeviceVector(
{i: Sensor(f"{prefix}:CHAN{i}" for i in range(1, num+1))}
)
super().__init__(name=name)

We have not been able to come up with a declarative approach that can describe
the ``SensorGroup`` example in a succinct way.

Decision
--------

Type safety and readability are regarded above velocity, and magic should be
minimized. With this in mind we will stick with the procedural approach for now.
We may find a less verbose way of doing ``set_readable_signals`` by using a
context manager and overriding setattr in the future:

.. code-block:: python

with self.signals_added_to(READ):
self.value = epics_signal_r(float, prefix + "Value")
with self.signals_added_to(CONFIG):
self.mode = epics_signal_rw(EnergyMode, prefix + "Mode")

If someone comes up with a way to write ``SensorGroup`` in a declarative
and readable way then we may revisit this.

Consequences
------------

Ophyd and ophyd-async Devices will look less alike, but ophyd-async should be
learnable for beginners.
2 changes: 1 addition & 1 deletion docs/examples/ad_demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from bluesky.utils import ProgressBarManager, register_transform
from ophyd.v2.core import DeviceCollector

from ophyd_async.devices import areadetector
from ophyd_async.epics.areadetector import areadetector

# Create a run engine, with plotting, progressbar and transform
RE = RunEngine({}, call_returns_result=True)
Expand Down
2 changes: 1 addition & 1 deletion docs/user/examples/epics_demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from ophyd import Component, Device, EpicsSignal, EpicsSignalRO

from ophyd_async.core import epicsdemo
from ophyd_async.core.device_collector import DeviceCollector
from ophyd_async.core.devices.device_collector import DeviceCollector

# Create a run engine, with plotting, progressbar and transform
RE = RunEngine({}, call_returns_result=True)
Expand Down
3 changes: 1 addition & 2 deletions docs/user/reference/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ This is the internal API reference for ophyd_async
ophyd_async.core.backends
ophyd_async.core.devices
ophyd_async.core.epicsdemo
ophyd_async.core.signals
ophyd_async.core.signal
ophyd_async.core.async_status
ophyd_async.core.device_collector
ophyd_async.core.utils
84 changes: 0 additions & 84 deletions other_licenses/pyepics

This file was deleted.

14 changes: 2 additions & 12 deletions src/ophyd_async/core/__init__.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
from .async_status import AsyncStatus
from .backends import SignalBackend, SimSignalBackend
from .device_collector import DeviceCollector
from .devices import (
Device,
DeviceCollector,
DeviceVector,
StandardReadable,
connect_children,
get_device_children,
name_children,
)
from .signals import (
EpicsTransport,
from .signal import (
Signal,
SignalR,
SignalRW,
SignalW,
SignalX,
epics_signal_r,
epics_signal_rw,
epics_signal_w,
epics_signal_x,
observe_value,
set_and_wait_for_value,
set_sim_callback,
Expand Down Expand Up @@ -50,16 +45,11 @@
"connect_children",
"get_device_children",
"name_children",
"EpicsTransport",
"Signal",
"SignalR",
"SignalW",
"SignalRW",
"SignalX",
"epics_signal_r",
"epics_signal_w",
"epics_signal_rw",
"epics_signal_x",
"observe_value",
"set_and_wait_for_value",
"set_sim_callback",
Expand Down
2 changes: 2 additions & 0 deletions src/ophyd_async/core/devices/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
from .device import Device, connect_children, get_device_children, name_children
from .device_collector import DeviceCollector
from .device_vector import DeviceVector
from .standard_readable import StandardReadable

__all__ = [
"Device",
"DeviceCollector",
"connect_children",
"get_device_children",
"name_children",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

from bluesky.run_engine import call_in_bluesky_event_loop

from .devices import Device
from .utils import NotConnected
from ..utils import NotConnected
from .device import Device


class DeviceCollector:
Expand Down
2 changes: 1 addition & 1 deletion src/ophyd_async/core/devices/standard_readable.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from bluesky.protocols import Configurable, Descriptor, Readable, Reading, Stageable

from ..async_status import AsyncStatus
from ..signals import SignalR
from ..signal import SignalR
from ..utils import merge_gathered_dicts
from .device import Device

Expand Down
11 changes: 2 additions & 9 deletions src/ophyd_async/core/epicsdemo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,8 @@
import numpy as np
from bluesky.protocols import Movable, Stoppable

from ophyd_async.core import (
AsyncStatus,
Device,
StandardReadable,
epics_signal_r,
epics_signal_rw,
epics_signal_x,
observe_value,
)
from ophyd_async.core import AsyncStatus, Device, StandardReadable, observe_value
from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw, epics_signal_x


class EnergyMode(Enum):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
Subscribable,
)

from ..async_status import AsyncStatus
from ..backends.signal_backend import SignalBackend
from ..backends.sim import SimSignalBackend
from ..devices import Device
from ..utils import DEFAULT_TIMEOUT, Callback, ReadingValueCallback, T
from .async_status import AsyncStatus
from .backends.signal_backend import SignalBackend
from .backends.sim import SimSignalBackend
from .devices import Device
from .utils import DEFAULT_TIMEOUT, Callback, ReadingValueCallback, T

_sim_backends: Dict[Signal, SimSignalBackend] = {}

Expand Down
Loading
Loading