From d7842b4b32e5aca1b3ed431cc50c126c6425728e Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 12 Sep 2024 18:00:42 +0100 Subject: [PATCH 1/7] Make sure that the aperture scatterguard position is set correctly and add tests --- src/dodal/devices/aperturescatterguard.py | 20 ++++++------- .../unit_tests/test_aperture_scatterguard.py | 30 +++++++++++++++++-- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 90597d532f..00f84686a2 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -220,15 +220,15 @@ async def _safe_move_within_datacollection_range( self.aperture.y.set(aperture_y), self.aperture.z.set(aperture_z), ) - return - await asyncio.gather( - self.aperture.x.set(aperture_x), - self.aperture.y.set(aperture_y), - self.aperture.z.set(aperture_z), - ) + else: + await asyncio.gather( + self.aperture.x.set(aperture_x), + self.aperture.y.set(aperture_y), + self.aperture.z.set(aperture_z), + ) - await asyncio.gather( - self.scatterguard.x.set(scatterguard_x), - self.scatterguard.y.set(scatterguard_y), - ) + await asyncio.gather( + self.scatterguard.x.set(scatterguard_x), + self.scatterguard.y.set(scatterguard_y), + ) await self.selected_aperture.set(value) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 3e51cbb415..e63ccb3fd9 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -139,10 +139,14 @@ def _assert_patched_ap_sg_has_call( def _assert_position_in_reading( - reading: dict[str, Any], position: AperturePosition, device_name: str + reading: dict[str, Any], + aperture: ApertureValue, + position: AperturePosition, + device_name: str, ): if position.radius is not None: assert reading[f"{device_name}-radius"]["value"] == position.radius + assert reading[f"{device_name}-selected_aperture"]["value"] == aperture assert reading[f"{device_name}-aperture-x"]["value"] == position.aperture_x assert reading[f"{device_name}-aperture-y"]["value"] == position.aperture_y assert reading[f"{device_name}-aperture-z"]["value"] == position.aperture_z @@ -307,6 +311,9 @@ async def test_aperture_positions_robot_load_unsafe( await ap_sg.get_current_aperture_position() +@pytest.mark.skip( + "Curently not working, see https://github.com/DiamondLightSource/dodal/issues/782" +) async def test_given_aperture_not_set_through_device_but_motors_in_position_when_device_read_then_position_returned( aperture_in_medium_pos: ApertureScatterguard, aperture_positions: dict[ApertureValue, AperturePosition], @@ -315,20 +322,32 @@ async def test_given_aperture_not_set_through_device_but_motors_in_position_when assert isinstance(reading, dict) _assert_position_in_reading( reading, + ApertureValue.MEDIUM, aperture_positions[ApertureValue.MEDIUM], aperture_in_medium_pos.name, ) +@pytest.mark.parametrize( + "aperture", + [ + ApertureValue.SMALL, + ApertureValue.MEDIUM, + ApertureValue.LARGE, + ApertureValue.ROBOT_LOAD, + ], +) async def test_when_aperture_set_and_device_read_then_position_returned( + aperture: ApertureValue, aperture_in_medium_pos: ApertureScatterguard, aperture_positions: dict[ApertureValue, AperturePosition], ): - await aperture_in_medium_pos.set(ApertureValue.MEDIUM) + await aperture_in_medium_pos.set(aperture) reading = await aperture_in_medium_pos.read() _assert_position_in_reading( reading, - aperture_positions[ApertureValue.MEDIUM], + aperture, + aperture_positions[aperture], aperture_in_medium_pos.name, ) @@ -347,6 +366,11 @@ async def test_ap_sg_in_runengine( assert await ap.z.user_readback.get_value() == test_loc.aperture_z assert await sg.x.user_readback.get_value() == test_loc.scatterguard_x assert await sg.y.user_readback.get_value() == test_loc.scatterguard_y + assert ( + await aperture_in_medium_pos.selected_aperture.get_value() + == ApertureValue.SMALL + ) + assert await aperture_in_medium_pos.radius.get_value() == 20 async def test_ap_sg_descriptor( From 6a87461bdc0b65b2dfcee6a43be8bd941752d52c Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Sat, 14 Sep 2024 15:53:45 +0100 Subject: [PATCH 2/7] Add helper signal that is backed by some function --- docs/reference/device-standards.rst | 83 +++++++++++++++++++++++++++-- pull_request_template.md | 2 +- src/dodal/common/signal_utils.py | 49 +++++++++++++++++ 3 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 src/dodal/common/signal_utils.py diff --git a/docs/reference/device-standards.rst b/docs/reference/device-standards.rst index 3ed95d3723..df80d40bb1 100644 --- a/docs/reference/device-standards.rst +++ b/docs/reference/device-standards.rst @@ -21,6 +21,17 @@ should think about where to place them in the following order: This is in an effort to avoid duplication across facilities/beamlines. +Device Best Practices +---------------------------- + +Ophyd-async directory contains a flowchart_ for a simplified decision tree about what interfaces +should a given device implement. In addition to this the following guidelines are strongly recommended: + +#. Devices should contain only the PV suffixes that are generic for any instance of the device. See `PV Suffixes`_ +#. Anything in a device that is expected to be set externally should be a signal. See `Use of signals`_ +#. Devices should not hold state, when they are read they should read the hardware. See `Holding State`_ + + PV Suffixes ----------- @@ -86,16 +97,78 @@ Instead you should make a soft signal: class MyDevice(Device): def __init__(self): - self.param = create_soft_signal(str, "", self.name) + self.param = soft_signal_rw(str) my_device = MyDevice() def my_plan(): yield from bps.mv(my_device.param, "new_value") -Ophyd Devices best practices ----------------------------- -Ophyd-async directory contains a flowchart_ for a simplified decision tree about what interfaces -should a given device implement. +Holding State +------------- + +Devices should avoid holding state as much as possible. Ophyd devices are mostly trying to reflect the state of hardware and so when the device is read that hardware should be read. + +If the device holds the state itself it is likely to not reflect the real hardware if: +* The device has just been initialised +* The hardware has changed independently e.g. via EPICS directly +* The hardware has failed to do what the device expected + +For example, if I have a device that I would like to treat as moving in/out based on an underlying axis then it would be incorrect to implement it like this: + +.. code-block:: python + + class InOut(Enum): + IN = 0 + OUT = 0 + + class MyDevice(Device): + def __init__(self): + self.underlying_motor = Motor("MOTOR") + with self.add_children_as_readables(): + self.in_out, self._in_out_setter = soft_signal_r_and_setter(InOut) + + + @AsyncStatus.wrap + async def set(self, value: InOut): + if value == InOut.IN: + await self.underlying_motor.set(100) + else: + await self.underlying_motor.set(0) + self._in_out_setter(value) + +While this may appear to work fine during normal operation the state of in_out is only ever updated if the ophyd device is set. It is incorrect to assume that underlying_motor only changes +based on this and so this has the issues listed above. Instead you should make sure to update in_out whenever the device is read e.g. + +.. code-block:: python + + class InOut(Enum): + IN = 0 + OUT = 0 + + class MyDevice(Device): + def __init__(self): + self.underlying_motor = Motor("MOTOR") + with self.add_children_as_readables(): + self.in_out = create_hardware_backed_soft_signal(InOut, self._get_in_out_from_hardware) + + async def _get_in_out_from_hardware(self): + current_position = await self.underlying_motor.get_value() + if isclose(current_position, 0): + return InOut.IN + elif isclose(current_position, 100): + return InOut.OUT + else: + raise ValueError() + + + @AsyncStatus.wrap + async def set(self, value: InOut): + if value == InOut.IN: + await self.underlying_motor.set(100) + else: + await self.underlying_motor.set(0) + + .. _flowchart: https://blueskyproject.io/ophyd-async/main/how-to/choose-interfaces-for-devices.html diff --git a/pull_request_template.md b/pull_request_template.md index 66a25a9b8d..810483a997 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -6,6 +6,6 @@ Fixes #ISSUE ### Checks for reviewer - [ ] Would the PR title make sense to a scientist on a set of release notes -- [ ] If a new device has been added does it follow the [standards](https://github.com/DiamondLightSource/dodal/wiki/Device-Standards) +- [ ] If a new device has been added does it follow the [standards](https://diamondlightsource.github.io/dodal/main/reference/device-standards.html) - [ ] If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly - [ ] Have the connection tests for the relevant beamline(s) been run via `dodal connect ${BEAMLINE}` diff --git a/src/dodal/common/signal_utils.py b/src/dodal/common/signal_utils.py new file mode 100644 index 0000000000..4d6e29c086 --- /dev/null +++ b/src/dodal/common/signal_utils.py @@ -0,0 +1,49 @@ +from collections.abc import Callable, Coroutine +from typing import Any, TypeVar + +from bluesky.protocols import Reading +from event_model.documents.event_descriptor import DataKey +from ophyd_async.core import SignalR, SoftSignalBackend +from ophyd_async.core._soft_signal_backend import SignalMetadata + +T = TypeVar("T") + + +class HarwareBackedSoftSignalBackend(SoftSignalBackend[T]): + def __init__( + self, + get_from_hardware_func: Callable[[], Coroutine[Any, Any, T]], + *args, + **kwargs, + ) -> None: + self.get_from_hardware_func = get_from_hardware_func + super().__init__(*args, **kwargs) + + async def _update_value(self): + new_value = await self.get_from_hardware_func() + await self.put(new_value) + + async def get_datakey(self, source: str) -> DataKey: + await self._update_value() + return await super().get_datakey(source) + + async def get_reading(self) -> Reading: + await self._update_value() + return await super().get_reading() + + async def get_value(self) -> T: + await self._update_value() + return await super().get_value() + + +def create_hardware_backed_soft_signal( + datatype: type[T], + get_from_hardware_func: Callable[[], Coroutine[Any, Any, T]], + units: str | None = None, +): + metadata = SignalMetadata(units=units, precision=None) + return SignalR( + backend=HarwareBackedSoftSignalBackend( + get_from_hardware_func, datatype, metadata=metadata + ) + ) From 4babae08b5c35db963b1abd023bc6c8a4c84c6a7 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Sat, 14 Sep 2024 15:54:40 +0100 Subject: [PATCH 3/7] Use a hardware backed signal for the aperturescatterguard --- src/dodal/devices/aperturescatterguard.py | 25 +++---- .../unit_tests/test_aperture_scatterguard.py | 68 ++++++++++++------- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 00f84686a2..6e4f7afe02 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -8,11 +8,11 @@ AsyncStatus, HintedSignal, StandardReadable, - soft_signal_rw, ) from pydantic import BaseModel, Field from dodal.common.beamlines.beamline_parameters import GDABeamlineParameters +from dodal.common.signal_utils import create_hardware_backed_soft_signal from dodal.devices.aperture import Aperture from dodal.devices.scatterguard import Scatterguard @@ -105,7 +105,9 @@ def __init__( ) -> None: self.aperture = Aperture(prefix + "-MO-MAPT-01:") self.scatterguard = Scatterguard(prefix + "-MO-SCAT-01:") - self.radius = soft_signal_rw(float, units="µm") + self.radius = create_hardware_backed_soft_signal( + float, self._get_current_radius, units="µm" + ) self._loaded_positions = loaded_positions self._tolerances = tolerances self.add_readables( @@ -119,7 +121,9 @@ def __init__( ], ) with self.add_children_as_readables(HintedSignal): - self.selected_aperture = soft_signal_rw(ApertureValue) + self.selected_aperture = create_hardware_backed_soft_signal( + ApertureValue, self.get_current_aperture_position + ) super().__init__(name) @@ -136,9 +140,6 @@ async def set(self, value: ApertureValue): @AsyncStatus.wrap async def _set_raw_unsafe(self, position: AperturePosition): """Accept the risks and move in an unsafe way. Collisions possible.""" - if position.radius is not None: - await self.radius.set(position.radius) - aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = ( position.values ) @@ -150,11 +151,6 @@ async def _set_raw_unsafe(self, position: AperturePosition): self.scatterguard.x.set(scatterguard_x), self.scatterguard.y.set(scatterguard_y), ) - try: - value = await self.get_current_aperture_position() - self.selected_aperture.set(value) - except InvalidApertureMove: - self.selected_aperture.set(None) # type: ignore async def get_current_aperture_position(self) -> ApertureValue: """ @@ -176,6 +172,10 @@ async def get_current_aperture_position(self) -> ApertureValue: raise InvalidApertureMove("Current aperture/scatterguard state unrecognised") + async def _get_current_radius(self) -> float | None: + current_value = await self.get_current_aperture_position() + return self._loaded_positions[current_value].radius + async def _safe_move_within_datacollection_range( self, position: AperturePosition, value: ApertureValue ): @@ -203,8 +203,6 @@ async def _safe_move_within_datacollection_range( ) current_ap_y = await self.aperture.y.user_readback.get_value() - if position.radius is not None: - await self.radius.set(position.radius) aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = ( position.values @@ -231,4 +229,3 @@ async def _safe_move_within_datacollection_range( self.scatterguard.x.set(scatterguard_x), self.scatterguard.y.set(scatterguard_y), ) - await self.selected_aperture.set(value) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index e63ccb3fd9..732df7d815 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -8,6 +8,7 @@ from bluesky.run_engine import RunEngine from ophyd_async.core import ( DeviceCollector, + callback_on_mock_put, get_mock_put, set_mock_value, ) @@ -115,6 +116,7 @@ async def aperture_in_medium_pos_w_call_log( await ap_sg._set_raw_unsafe(aperture_positions[ApertureValue.MEDIUM]) set_mock_value(ap_sg.aperture.medium, 1) + yield ap_sg, call_log @@ -144,9 +146,6 @@ def _assert_position_in_reading( position: AperturePosition, device_name: str, ): - if position.radius is not None: - assert reading[f"{device_name}-radius"]["value"] == position.radius - assert reading[f"{device_name}-selected_aperture"]["value"] == aperture assert reading[f"{device_name}-aperture-x"]["value"] == position.aperture_x assert reading[f"{device_name}-aperture-y"]["value"] == position.aperture_y assert reading[f"{device_name}-aperture-z"]["value"] == position.aperture_z @@ -244,19 +243,27 @@ def set_underlying_motors(ap_sg: ApertureScatterguard, position: AperturePositio motor.set(pos) -async def test_aperture_positions_large(ap_sg: ApertureScatterguard): - set_mock_value(ap_sg.aperture.large, 1) - assert await ap_sg.get_current_aperture_position() == ApertureValue.LARGE - - -async def test_aperture_positions_medium(ap_sg: ApertureScatterguard): - set_mock_value(ap_sg.aperture.medium, 1) - assert await ap_sg.get_current_aperture_position() == ApertureValue.MEDIUM - - -async def test_aperture_positions_small(ap_sg: ApertureScatterguard): - set_mock_value(ap_sg.aperture.small, 1) - assert await ap_sg.get_current_aperture_position() == ApertureValue.SMALL +@pytest.mark.parametrize( + "read_pv, aperture", + [ + ("large", ApertureValue.LARGE), + ("medium", ApertureValue.MEDIUM), + ("small", ApertureValue.SMALL), + ], +) +async def test_aperture_positions( + ap_sg: ApertureScatterguard, + aperture_positions: dict[ApertureValue, AperturePosition], + read_pv: str, + aperture: ApertureValue, +): + set_mock_value(getattr(ap_sg.aperture, read_pv), 1) + reading = await ap_sg.read() + assert isinstance(reading, dict) + assert ( + reading[f"{ap_sg.name}-radius"]["value"] == aperture_positions[aperture].radius + ) + assert reading[f"{ap_sg.name}-selected_aperture"]["value"] == aperture async def test_aperture_positions_robot_load( @@ -268,7 +275,12 @@ async def test_aperture_positions_robot_load( set_mock_value(ap_sg.aperture.small, 0) robot_load = aperture_positions[ApertureValue.ROBOT_LOAD] await ap_sg.aperture.y.set(robot_load.aperture_y) - assert await ap_sg.get_current_aperture_position() == ApertureValue.ROBOT_LOAD + reading = await ap_sg.read() + assert isinstance(reading, dict) + assert reading[f"{ap_sg.name}-radius"]["value"] == 0.0 + assert ( + reading[f"{ap_sg.name}-selected_aperture"]["value"] == ApertureValue.ROBOT_LOAD + ) async def test_aperture_positions_robot_load_within_tolerance( @@ -282,7 +294,12 @@ async def test_aperture_positions_robot_load_within_tolerance( set_mock_value(ap_sg.aperture.medium, 0) set_mock_value(ap_sg.aperture.small, 0) await ap_sg.aperture.y.set(robot_load_ap_y + tolerance) - assert await ap_sg.get_current_aperture_position() == ApertureValue.ROBOT_LOAD + reading = await ap_sg.read() + assert isinstance(reading, dict) + assert reading[f"{ap_sg.name}-radius"]["value"] == 0.0 + assert ( + reading[f"{ap_sg.name}-selected_aperture"]["value"] == ApertureValue.ROBOT_LOAD + ) async def test_aperture_positions_robot_load_outside_tolerance( @@ -297,7 +314,7 @@ async def test_aperture_positions_robot_load_outside_tolerance( set_mock_value(ap_sg.aperture.small, 0) await ap_sg.aperture.y.set(robot_load_ap_y + tolerance) with pytest.raises(InvalidApertureMove): - await ap_sg.get_current_aperture_position() + await ap_sg.read() async def test_aperture_positions_robot_load_unsafe( @@ -308,12 +325,9 @@ async def test_aperture_positions_robot_load_unsafe( set_mock_value(ap_sg.aperture.small, 0) await ap_sg.aperture.y.set(50.0) with pytest.raises(InvalidApertureMove): - await ap_sg.get_current_aperture_position() + await ap_sg.read() -@pytest.mark.skip( - "Curently not working, see https://github.com/DiamondLightSource/dodal/issues/782" -) async def test_given_aperture_not_set_through_device_but_motors_in_position_when_device_read_then_position_returned( aperture_in_medium_pos: ApertureScatterguard, aperture_positions: dict[ApertureValue, AperturePosition], @@ -360,6 +374,14 @@ async def test_ap_sg_in_runengine( ap = aperture_in_medium_pos.aperture sg = aperture_in_medium_pos.scatterguard test_loc = aperture_positions[ApertureValue.SMALL] + + def set_small_readback_pv(value, *args, **kwargs): + set_mock_value(ap.small, 1) + set_mock_value(ap.medium, 0) + set_mock_value(ap.y.user_readback, value) + + callback_on_mock_put(ap.y.user_setpoint, set_small_readback_pv) + RE(bps.abs_set(aperture_in_medium_pos, ApertureValue.SMALL, wait=True)) assert await ap.x.user_readback.get_value() == test_loc.aperture_x assert await ap.y.user_readback.get_value() == test_loc.aperture_y From 0d8531ba0fc7e68de8d6eddde17c9911952b862c Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 18 Sep 2024 11:34:48 +0100 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: DiamondJoseph <53935796+DiamondJoseph@users.noreply.github.com> --- src/dodal/common/signal_utils.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/dodal/common/signal_utils.py b/src/dodal/common/signal_utils.py index 4d6e29c086..4049a17ce9 100644 --- a/src/dodal/common/signal_utils.py +++ b/src/dodal/common/signal_utils.py @@ -23,10 +23,6 @@ async def _update_value(self): new_value = await self.get_from_hardware_func() await self.put(new_value) - async def get_datakey(self, source: str) -> DataKey: - await self._update_value() - return await super().get_datakey(source) - async def get_reading(self) -> Reading: await self._update_value() return await super().get_reading() @@ -40,8 +36,9 @@ def create_hardware_backed_soft_signal( datatype: type[T], get_from_hardware_func: Callable[[], Coroutine[Any, Any, T]], units: str | None = None, + precision: int | None = None, ): - metadata = SignalMetadata(units=units, precision=None) + metadata = SignalMetadata(units=units, precision=precision) return SignalR( backend=HarwareBackedSoftSignalBackend( get_from_hardware_func, datatype, metadata=metadata From 5bece25ea99b6fb08cbce2bde18c7b0486726c90 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 18 Sep 2024 11:33:42 +0100 Subject: [PATCH 5/7] Make get_current_aperture_position private --- src/dodal/devices/aperturescatterguard.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 6e4f7afe02..a924777d19 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -122,7 +122,7 @@ def __init__( ) with self.add_children_as_readables(HintedSignal): self.selected_aperture = create_hardware_backed_soft_signal( - ApertureValue, self.get_current_aperture_position + ApertureValue, self._get_current_aperture_position ) super().__init__(name) @@ -152,7 +152,7 @@ async def _set_raw_unsafe(self, position: AperturePosition): self.scatterguard.y.set(scatterguard_y), ) - async def get_current_aperture_position(self) -> ApertureValue: + async def _get_current_aperture_position(self) -> ApertureValue: """ Returns the current aperture position using readback values for SMALL, MEDIUM, LARGE. ROBOT_LOAD position defined when @@ -173,7 +173,7 @@ async def get_current_aperture_position(self) -> ApertureValue: raise InvalidApertureMove("Current aperture/scatterguard state unrecognised") async def _get_current_radius(self) -> float | None: - current_value = await self.get_current_aperture_position() + current_value = await self._get_current_aperture_position() return self._loaded_positions[current_value].radius async def _safe_move_within_datacollection_range( From a0d7ab00e401706493d9a14a4f1cab03d20ec27c Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 18 Sep 2024 12:35:54 +0100 Subject: [PATCH 6/7] Fix linting --- src/dodal/common/signal_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dodal/common/signal_utils.py b/src/dodal/common/signal_utils.py index 4049a17ce9..bf5c158989 100644 --- a/src/dodal/common/signal_utils.py +++ b/src/dodal/common/signal_utils.py @@ -2,7 +2,6 @@ from typing import Any, TypeVar from bluesky.protocols import Reading -from event_model.documents.event_descriptor import DataKey from ophyd_async.core import SignalR, SoftSignalBackend from ophyd_async.core._soft_signal_backend import SignalMetadata From 854c430d7526351a1ece12814a45aeef86a69a9d Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 20 Sep 2024 16:49:19 +0100 Subject: [PATCH 7/7] Add some documentation pointing to the future cleaner derived signals --- docs/reference/device-standards.rst | 2 +- src/dodal/common/signal_utils.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/reference/device-standards.rst b/docs/reference/device-standards.rst index df80d40bb1..21913f4873 100644 --- a/docs/reference/device-standards.rst +++ b/docs/reference/device-standards.rst @@ -169,6 +169,6 @@ based on this and so this has the issues listed above. Instead you should make s else: await self.underlying_motor.set(0) - +This will be simplified by https://github.com/bluesky/ophyd-async/issues/525 .. _flowchart: https://blueskyproject.io/ophyd-async/main/how-to/choose-interfaces-for-devices.html diff --git a/src/dodal/common/signal_utils.py b/src/dodal/common/signal_utils.py index bf5c158989..544d92b68d 100644 --- a/src/dodal/common/signal_utils.py +++ b/src/dodal/common/signal_utils.py @@ -37,6 +37,14 @@ def create_hardware_backed_soft_signal( units: str | None = None, precision: int | None = None, ): + """Creates a soft signal that, when read will call the function passed into + `get_from_hardware_func` and return this. + + This will allow you to make soft signals derived from arbitrary hardware signals. + However, calling subscribe on this signal does not give you a sensible value and + the signal is currently read only. See https://github.com/bluesky/ophyd-async/issues/525 + for a more full solution. + """ metadata = SignalMetadata(units=units, precision=precision) return SignalR( backend=HarwareBackedSoftSignalBackend(