From 338f2e4980e35104f6c79445c566056427dbfcdf Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 27 Aug 2025 15:01:02 +0000 Subject: [PATCH 1/4] feat: raise error if attr name in reserved bluesky protocol names --- src/ophyd_async/core/_device.py | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/ophyd_async/core/_device.py b/src/ophyd_async/core/_device.py index 354cb72b93..cae699e08c 100644 --- a/src/ophyd_async/core/_device.py +++ b/src/ophyd_async/core/_device.py @@ -70,6 +70,39 @@ async def connect_real(self, device: Device, timeout: float, force_reconnect: bo await wait_for_connection(**coros) +DEVICE_RESERVED_ATTRS = { + "add_callback", + "exception", + "done", + "success", + "name", + "collect_asset_docs", + "get_index", + "read_configuration", + "describe_configuration", + "trigger", + "prepare", + "read", + "describe", + "describe_collect", + "collect", + "collect_pages", + "set", + "locate", + "kickoff", + "complete", + "stage", + "unstage", + "pause", + "resume", + "stop", + "subscribe", + "clear_sub", + "check_value", + "hints", +} + + class Device(HasName): """Common base class for all Ophyd Async Devices. @@ -152,6 +185,11 @@ def __setattr__(self, name: str, value: Any) -> None: # ...hence not doing an isinstance check for attributes we # know not to be Devices elif name not in _not_device_attrs and isinstance(value, Device): + if name in DEVICE_RESERVED_ATTRS: + raise NameError( + f"`{name}` is used in one of the bluesky protocols. " + f"Please use `{name}_` instead." + ) value.parent = self self._child_devices[name] = value # And if the name is set, then set the name of all children, From c3eb27ffbf672ed46f3a37a17e4aed9653b01c43 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 27 Aug 2025 15:01:28 +0000 Subject: [PATCH 2/4] tests: add test to check error is raised if protocol name used in attr --- tests/core/test_device.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/core/test_device.py b/tests/core/test_device.py index 37342c1174..0eb56ec4ab 100644 --- a/tests/core/test_device.py +++ b/tests/core/test_device.py @@ -16,6 +16,7 @@ soft_signal_rw, wait_for_connection, ) +from ophyd_async.core._device import DEVICE_RESERVED_ATTRS # noqa: PLC2701 from ophyd_async.epics import motor from ophyd_async.plan_stubs import ensure_connected @@ -67,6 +68,19 @@ def get_source(self) -> str: return self.signal_ref().source +@pytest.mark.parametrize("attr_name", DEVICE_RESERVED_ATTRS) +def test_attr_in_bluesky_protocols(attr_name): + class DeviceWithProtocolName(Device): + def __init__(self, name: str = "") -> None: + super().__init__(name) + # use setattr so we can inject the name dynamically + setattr(self, attr_name, soft_signal_rw(int, name="foo")) + + expected_msg = f"Please use `{attr_name}_` instead" + with pytest.raises(NameError, match=expected_msg): + DeviceWithProtocolName("bar") + + async def test_device_connect_missing_connector() -> None: # Create an instance of Device without calling __init__ device = object.__new__(Device) From 4893a3a25e0905c69f8e54166140b84bdc8f9fb7 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Tue, 2 Sep 2025 09:15:48 +0000 Subject: [PATCH 3/4] refactor: amend logic flow in setattr --- src/ophyd_async/core/_device.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/ophyd_async/core/_device.py b/src/ophyd_async/core/_device.py index cae699e08c..83952f8c01 100644 --- a/src/ophyd_async/core/_device.py +++ b/src/ophyd_async/core/_device.py @@ -71,10 +71,6 @@ async def connect_real(self, device: Device, timeout: float, force_reconnect: bo DEVICE_RESERVED_ATTRS = { - "add_callback", - "exception", - "done", - "success", "name", "collect_asset_docs", "get_index", @@ -176,7 +172,14 @@ def set_name(self, name: str, *, child_name_separator: str | None = None) -> Non def __setattr__(self, name: str, value: Any) -> None: # Bear in mind that this function is called *a lot*, so # we need to make sure nothing expensive happens in it... - if name == "parent": + if name in _not_device_attrs: + pass + elif name in DEVICE_RESERVED_ATTRS: + raise NameError( + f"`{name}` is used in one of the bluesky protocols. " + f"Please use `{name}_` instead." + ) + elif name == "parent": if self.parent not in (value, None): raise TypeError( f"Cannot set the parent of {self} to be {value}: " @@ -184,12 +187,7 @@ def __setattr__(self, name: str, value: Any) -> None: ) # ...hence not doing an isinstance check for attributes we # know not to be Devices - elif name not in _not_device_attrs and isinstance(value, Device): - if name in DEVICE_RESERVED_ATTRS: - raise NameError( - f"`{name}` is used in one of the bluesky protocols. " - f"Please use `{name}_` instead." - ) + elif isinstance(value, Device): value.parent = self self._child_devices[name] = value # And if the name is set, then set the name of all children, From f21e8efe628e5e6990ea9db4119d9b82945f020c Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Tue, 2 Sep 2025 13:35:45 +0000 Subject: [PATCH 4/4] chore: append trailing underscore to trigger signals --- src/ophyd_async/epics/adcore/_core_io.py | 2 +- src/ophyd_async/fastcs/eiger/_eiger_io.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ophyd_async/epics/adcore/_core_io.py b/src/ophyd_async/epics/adcore/_core_io.py index 687a1cfd75..fff904e334 100644 --- a/src/ophyd_async/epics/adcore/_core_io.py +++ b/src/ophyd_async/epics/adcore/_core_io.py @@ -243,7 +243,7 @@ class NDPluginCBIO(NDPluginBaseIO): pre_count: A[SignalRW[int], PvSuffix.rbv("PreCount")] post_count: A[SignalRW[int], PvSuffix.rbv("PostCount")] preset_trigger_count: A[SignalRW[int], PvSuffix.rbv("PresetTriggerCount")] - trigger: A[SignalRW[bool], PvSuffix.rbv("Trigger")] + trigger_: A[SignalRW[bool], PvSuffix.rbv("Trigger")] capture: A[SignalRW[bool], PvSuffix.rbv("Capture")] flush_on_soft_trg: A[ SignalRW[NDCBFlushOnSoftTrgMode], PvSuffix.rbv("FlushOnSoftTrg") diff --git a/src/ophyd_async/fastcs/eiger/_eiger_io.py b/src/ophyd_async/fastcs/eiger/_eiger_io.py index e9c2fb677a..1d3ebc8ada 100644 --- a/src/ophyd_async/fastcs/eiger/_eiger_io.py +++ b/src/ophyd_async/fastcs/eiger/_eiger_io.py @@ -40,7 +40,7 @@ class EigerDetectorIO(Device): omega_increment: SignalRW[float] arm: SignalX disarm: SignalX - trigger: SignalX + trigger_: SignalX class EigerDriverIO(Device):