From cf942088fe790625598b0b175969df8704e853fd Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Tue, 20 Jan 2026 15:55:57 +0000 Subject: [PATCH 1/2] Add access_mode property to Attribute classes Resolves #260 by adding an access_mode property to Attribute and its subclasses, replacing the need for the _attribute_to_access() free function. The property returns "r" for AttrR, "w" for AttrW, and "rw" for AttrRW, providing a cleaner API for determining attribute access modes. - Add AttributeAccessMode type definition to attribute.py - Make Attribute class abstract with abstract access_mode property - Implement access_mode property in AttrR, AttrW, and AttrRW - Export AttributeAccessMode from attributes module - Replace _attribute_to_access() function usage in PVA PVI code - Remove _attribute_to_access() function and TODO comment - Add unit test for the new access_mode property All tests and type checking pass. Co-Authored-By: Claude Haiku 4.5 --- src/fastcs/attributes/__init__.py | 1 + src/fastcs/attributes/attr_r.py | 7 ++++++- src/fastcs/attributes/attr_rw.py | 6 ++++++ src/fastcs/attributes/attr_w.py | 7 ++++++- src/fastcs/attributes/attribute.py | 13 ++++++++++-- src/fastcs/transports/epics/pva/pvi.py | 16 +------------- tests/test_attributes.py | 29 +++++++++++++++++++++++++- 7 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/fastcs/attributes/__init__.py b/src/fastcs/attributes/__init__.py index b25e66f72..d0f5e59f0 100644 --- a/src/fastcs/attributes/__init__.py +++ b/src/fastcs/attributes/__init__.py @@ -2,6 +2,7 @@ from .attr_rw import AttrRW as AttrRW from .attr_w import AttrW as AttrW from .attribute import Attribute as Attribute +from .attribute import AttributeAccessMode as AttributeAccessMode from .attribute_io import AnyAttributeIO as AnyAttributeIO from .attribute_io import AttributeIO as AttributeIO from .attribute_io_ref import AttributeIORef as AttributeIORef diff --git a/src/fastcs/attributes/attr_r.py b/src/fastcs/attributes/attr_r.py index aec7f5222..4b1e54c74 100644 --- a/src/fastcs/attributes/attr_r.py +++ b/src/fastcs/attributes/attr_r.py @@ -4,7 +4,7 @@ from collections.abc import Awaitable, Callable from typing import Any -from fastcs.attributes.attribute import Attribute +from fastcs.attributes.attribute import Attribute, AttributeAccessMode from fastcs.attributes.attribute_io_ref import AttributeIORefT from fastcs.attributes.util import AttrValuePredicate, PredicateEvent from fastcs.datatypes import DataType, DType_T @@ -47,6 +47,11 @@ def get(self) -> DType_T: """Get the cached value of the attribute.""" return self._value + @property + def access_mode(self) -> AttributeAccessMode: + """The access mode of this attribute.""" + return "r" + async def update(self, value: Any) -> None: """Update the value of the attibute diff --git a/src/fastcs/attributes/attr_rw.py b/src/fastcs/attributes/attr_rw.py index c8365373b..c60ba5aa0 100644 --- a/src/fastcs/attributes/attr_rw.py +++ b/src/fastcs/attributes/attr_rw.py @@ -1,5 +1,6 @@ from fastcs.attributes.attr_r import AttrR from fastcs.attributes.attr_w import AttrW +from fastcs.attributes.attribute import AttributeAccessMode from fastcs.attributes.attribute_io_ref import AttributeIORefT from fastcs.datatypes import DataType, DType_T from fastcs.logging import bind_logger @@ -25,6 +26,11 @@ def __init__( if io_ref is None: self.set_on_put_callback(self._internal_update) + @property + def access_mode(self) -> AttributeAccessMode: + """The access mode of this attribute.""" + return "rw" + async def _internal_update( self, attr: AttrW[DType_T, AttributeIORefT], value: DType_T ): diff --git a/src/fastcs/attributes/attr_w.py b/src/fastcs/attributes/attr_w.py index a9b2cff72..b710e2ed3 100644 --- a/src/fastcs/attributes/attr_w.py +++ b/src/fastcs/attributes/attr_w.py @@ -2,7 +2,7 @@ from collections.abc import Awaitable, Callable from typing import Any -from fastcs.attributes.attribute import Attribute +from fastcs.attributes.attribute import Attribute, AttributeAccessMode from fastcs.attributes.attribute_io_ref import AttributeIORefT from fastcs.datatypes import DataType, DType_T from fastcs.logging import bind_logger @@ -37,6 +37,11 @@ def __init__( self._sync_setpoint_callbacks: list[AttrSyncSetpointCallback[DType_T]] = [] """Callbacks to publish changes to the setpoint of the attribute""" + @property + def access_mode(self) -> AttributeAccessMode: + """The access mode of this attribute.""" + return "w" + async def put(self, setpoint: DType_T, sync_setpoint: bool = False) -> None: """Set the setpoint of the attribute diff --git a/src/fastcs/attributes/attribute.py b/src/fastcs/attributes/attribute.py index c737bd28d..ae6e3852d 100644 --- a/src/fastcs/attributes/attribute.py +++ b/src/fastcs/attributes/attribute.py @@ -1,5 +1,6 @@ +from abc import ABC, abstractmethod from collections.abc import Callable -from typing import Generic +from typing import Generic, Literal from fastcs.attributes.attribute_io_ref import AttributeIORefT from fastcs.datatypes import DataType, DType, DType_T @@ -8,8 +9,10 @@ logger = bind_logger(logger_name=__name__) +AttributeAccessMode = Literal["r", "w", "rw"] -class Attribute(Generic[DType_T, AttributeIORefT], Tracer): + +class Attribute(Generic[DType_T, AttributeIORefT], Tracer, ABC): """Base FastCS attribute. Instances of this class added to a ``Controller`` will be used by the FastCS class. @@ -74,6 +77,12 @@ def path(self) -> list[str]: def full_name(self) -> str: return ".".join(self._path + [self._name]) + @property + @abstractmethod + def access_mode(self) -> AttributeAccessMode: + """The access mode of this attribute.""" + ... + def add_update_datatype_callback( self, callback: Callable[[DataType[DType_T]], None] ) -> None: diff --git a/src/fastcs/transports/epics/pva/pvi.py b/src/fastcs/transports/epics/pva/pvi.py index 2823fe38c..5c381c05a 100644 --- a/src/fastcs/transports/epics/pva/pvi.py +++ b/src/fastcs/transports/epics/pva/pvi.py @@ -6,7 +6,6 @@ from p4p.server import StaticProvider from p4p.server.asyncio import SharedPV -from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW from fastcs.transports.controller_api import ControllerAPI from fastcs.util import snake_to_pascal @@ -15,19 +14,6 @@ AccessModeType = Literal["r", "w", "rw", "d", "x"] -# TODO: This should be removed after https://github.com/DiamondLightSource/FastCS/issues/260 -def _attribute_to_access(attribute: Attribute) -> AccessModeType: - match attribute: - case AttrRW(): - return "rw" - case AttrR(): - return "r" - case AttrW(): - return "w" - case _: - raise ValueError(f"Unknown attribute type {type(attribute)}") - - def add_pvi_info( provider: StaticProvider, pv_prefix: str, @@ -79,7 +65,7 @@ def _make_p4p_raw_value(pv_prefix: str, controller_api: ControllerAPI) -> dict: for pv_leaf, attribute in controller_api.attributes.items(): # Add attribute entry pv = f"{pv_prefix}:{snake_to_pascal(pv_leaf)}" - p4p_raw_value[pv_leaf][_attribute_to_access(attribute)] = pv + p4p_raw_value[pv_leaf][attribute.access_mode] = pv for pv_leaf, _ in controller_api.command_methods.items(): pv = f"{pv_prefix}:{snake_to_pascal(pv_leaf)}" p4p_raw_value[pv_leaf]["x"] = pv diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 41de437a4..e9fb5e668 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -6,11 +6,38 @@ import pytest from pytest_mock import MockerFixture -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import ( + AttributeAccessMode, + AttributeIO, + AttributeIORef, + AttrR, + AttrRW, + AttrW, +) from fastcs.controllers import Controller from fastcs.datatypes import Float, Int, String +def test_attribute_access_mode(): + """Test that attributes have the correct access_mode property.""" + attr_r = AttrR(String()) + attr_w = AttrW(String()) + attr_rw = AttrRW(String()) + + assert attr_r.access_mode == "r" + assert attr_w.access_mode == "w" + assert attr_rw.access_mode == "rw" + + # Verify type compatibility + mode_r: AttributeAccessMode = attr_r.access_mode + mode_w: AttributeAccessMode = attr_w.access_mode + mode_rw: AttributeAccessMode = attr_rw.access_mode + + assert isinstance(mode_r, str) + assert isinstance(mode_w, str) + assert isinstance(mode_rw, str) + + def test_attr_r(): attr = AttrR(String(), group="test group") From 664a900211d85fae8a935380649259b667cd9e67 Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Tue, 20 Jan 2026 16:13:12 +0000 Subject: [PATCH 2/2] De-duplicate docstring --- src/fastcs/attributes/attr_r.py | 1 - src/fastcs/attributes/attr_rw.py | 1 - src/fastcs/attributes/attr_w.py | 1 - 3 files changed, 3 deletions(-) diff --git a/src/fastcs/attributes/attr_r.py b/src/fastcs/attributes/attr_r.py index 4b1e54c74..ae67c6d11 100644 --- a/src/fastcs/attributes/attr_r.py +++ b/src/fastcs/attributes/attr_r.py @@ -49,7 +49,6 @@ def get(self) -> DType_T: @property def access_mode(self) -> AttributeAccessMode: - """The access mode of this attribute.""" return "r" async def update(self, value: Any) -> None: diff --git a/src/fastcs/attributes/attr_rw.py b/src/fastcs/attributes/attr_rw.py index c60ba5aa0..af6491a04 100644 --- a/src/fastcs/attributes/attr_rw.py +++ b/src/fastcs/attributes/attr_rw.py @@ -28,7 +28,6 @@ def __init__( @property def access_mode(self) -> AttributeAccessMode: - """The access mode of this attribute.""" return "rw" async def _internal_update( diff --git a/src/fastcs/attributes/attr_w.py b/src/fastcs/attributes/attr_w.py index b710e2ed3..4532364a5 100644 --- a/src/fastcs/attributes/attr_w.py +++ b/src/fastcs/attributes/attr_w.py @@ -39,7 +39,6 @@ def __init__( @property def access_mode(self) -> AttributeAccessMode: - """The access mode of this attribute.""" return "w" async def put(self, setpoint: DType_T, sync_setpoint: bool = False) -> None: