From 7f0d4b7a18af9761892c2e2ef405f925ba851c84 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 15 Aug 2025 13:36:04 +0000 Subject: [PATCH 01/29] feat: implement initial subcontroller vector --- src/fastcs/controller.py | 67 +++++++++++++++++---- src/fastcs/controller_api.py | 4 +- src/fastcs/transport/epics/ca/ioc.py | 2 +- src/fastcs/transport/epics/gui.py | 12 ++-- src/fastcs/transport/epics/pva/pvi_tree.py | 27 +++++++-- src/fastcs/transport/graphql/graphql.py | 2 +- src/fastcs/transport/rest/rest.py | 4 +- src/fastcs/transport/tango/dsr.py | 7 ++- tests/example_p4p_ioc.py | 52 ++++++++++++----- tests/transport/epics/pva/test_p4p.py | 68 +++++++++++++++------- 10 files changed, 178 insertions(+), 67 deletions(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 56f224412..3d72c322e 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections import Counter -from collections.abc import Sequence +from collections.abc import Iterator, Mapping, MutableMapping, Sequence from copy import deepcopy from typing import get_type_hints @@ -22,7 +22,7 @@ class BaseController(Tracer): def __init__( self, - path: list[str] | None = None, + path: list[str | int] | None = None, description: str | None = None, ios: Sequence[AttributeIO[T, AttributeIORefT]] | None = None, ) -> None: @@ -35,8 +35,8 @@ def __init__( if not hasattr(self, "attributes"): self.attributes = {} - self._path: list[str] = path or [] - self.__sub_controller_tree: dict[str, Controller] = {} + self._path: list[str | int] = path or [] + self.__sub_controller_tree: dict[str | int, BaseController] = {} self._bind_attrs() @@ -71,11 +71,11 @@ def connect_attribute_ios(self) -> None: controller.connect_attribute_ios() @property - def path(self) -> list[str]: + def path(self) -> list[str | int]: """Path prefix of attributes, recursively including parent Controllers.""" return self._path - def set_path(self, path: list[str]): + def set_path(self, path: list[str | int]): if self._path: raise ValueError(f"sub controller is already registered under {self.path}") @@ -142,7 +142,7 @@ def add_attribute(self, name, attribute: Attribute): self.attributes[name] = attribute super().__setattr__(name, attribute) - def add_sub_controller(self, name: str, sub_controller: Controller): + def add_sub_controller(self, name: str | int, sub_controller: BaseController): if name in self.__sub_controller_tree.keys(): raise ValueError( f"Cannot add sub controller {name}. " @@ -156,18 +156,18 @@ def add_sub_controller(self, name: str, sub_controller: Controller): sub_controller.set_path(self.path + [name]) self.__sub_controller_tree[name] = sub_controller - super().__setattr__(name, sub_controller) + super().__setattr__(str(name), sub_controller) if isinstance(sub_controller.root_attribute, Attribute): - self.attributes[name] = sub_controller.root_attribute + self.attributes[str(name)] = sub_controller.root_attribute @property - def sub_controllers(self) -> dict[str, Controller]: + def sub_controllers(self) -> dict[str | int, BaseController]: return self.__sub_controller_tree def __repr__(self): name = self.__class__.__name__ - path = ".".join(self.path) or None + path = ".".join([str(p) for p in self.path]) or None sub_controllers = list(self.sub_controllers.keys()) or None return f"{name}(path={path}, sub_controllers={sub_controllers})" @@ -204,3 +204,48 @@ async def connect(self) -> None: async def disconnect(self) -> None: pass + + +class SubControllerVector(MutableMapping[int, Controller], Controller): + """A collection of SubControllers, with an arbitrary integer index. + An instance of this class can be registered with a parent ``Controller`` to include + it's children as part of a larger controller. Each child of the vector will keep + a string name of the vector. + """ + + def __init__( + self, children: Mapping[int, Controller], description: str | None = None + ) -> None: + self._children: dict[int, Controller] = {} + self.update(children) + super().__init__(description=description) + for index, child in children.items(): + self.add_sub_controller(index, child) + + def __getitem__(self, key: int) -> Controller: + return self._children[key] + + def __setitem__(self, key: int, value: Controller) -> None: + if not isinstance(key, int): + msg = f"Expected int, got {key}" + raise TypeError(msg) + if not isinstance(value, Controller): + msg = f"Expected Controller, got {value}" + raise TypeError(msg) + self._children[key] = value + + def __delitem__(self, key: int) -> None: + del self._children[key] + + def __iter__(self) -> Iterator[int]: + yield from self._children + + def __len__(self) -> int: + return len(self._children) + + def children(self) -> Iterator[tuple[str, Controller]]: + for key, child in self._children.items(): + yield str(key), child + + def __hash__(self): + return hash(id(self)) diff --git a/src/fastcs/controller_api.py b/src/fastcs/controller_api.py index 88b4c38ff..655588c31 100644 --- a/src/fastcs/controller_api.py +++ b/src/fastcs/controller_api.py @@ -17,12 +17,12 @@ class ControllerAPI: """Attributes, bound methods and sub APIs of a `Controller`""" - path: list[str] = field(default_factory=list) + path: list[str | int] = field(default_factory=list) """Path within controller tree (empty if this is the root)""" attributes: dict[str, Attribute] = field(default_factory=dict) command_methods: dict[str, Command] = field(default_factory=dict) scan_methods: dict[str, Scan] = field(default_factory=dict) - sub_apis: dict[str, "ControllerAPI"] = field(default_factory=dict) + sub_apis: dict[str | int, "ControllerAPI"] = field(default_factory=dict) """APIs of the sub controllers of the `Controller` this API was built from""" description: str | None = None diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index b65fc7404..0095c678b 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -113,7 +113,7 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): for child in parent.sub_apis.values(): child_pvi = f"{controller_pv_prefix(pv_prefix, child)}:PVI" - child_name = child.path[-1].lower() + child_name = str(child.path[-1]).lower() _add_pvi_info(child_pvi, parent_pvi, child_name) _add_sub_controller_pvi_info(pv_prefix, child) diff --git a/src/fastcs/transport/epics/gui.py b/src/fastcs/transport/epics/gui.py index f2a0be9ff..5c8542221 100644 --- a/src/fastcs/transport/epics/gui.py +++ b/src/fastcs/transport/epics/gui.py @@ -51,9 +51,9 @@ def __init__(self, controller_api: ControllerAPI, pv_prefix: str) -> None: self._controller_api = controller_api self._pv_prefix = pv_prefix - def _get_pv(self, attr_path: list[str], name: str): + def _get_pv(self, attr_path: list[str | int], name: str): attr_prefix = ":".join( - [self._pv_prefix] + [snake_to_pascal(node) for node in attr_path] + [self._pv_prefix] + [snake_to_pascal(str(node)) for node in attr_path] ) return f"{attr_prefix}:{snake_to_pascal(name)}" @@ -88,7 +88,7 @@ def _get_write_widget(self, fastcs_datatype: DataType) -> WriteWidgetUnion | Non raise FastCSError(f"Unsupported type {type(datatype)}: {datatype}") def _get_attribute_component( - self, attr_path: list[str], name: str, attribute: Attribute + self, attr_path: list[str | int], name: str, attribute: Attribute ) -> SignalR | SignalW | SignalRW | None: pv = self._get_pv(attr_path, name) name = snake_to_pascal(name) @@ -129,7 +129,7 @@ def _get_attribute_component( case _: raise FastCSError(f"Unsupported attribute type: {type(attribute)}") - def _get_command_component(self, attr_path: list[str], name: str): + def _get_command_component(self, attr_path: list[str | int], name: str): pv = self._get_pv(attr_path, name) name = snake_to_pascal(name) @@ -160,6 +160,8 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: components: Tree = [] for name, api in controller_api.sub_apis.items(): + if isinstance(name, int): + name = f"{controller_api.path[-1]}{name}" components.append( Group( name=snake_to_pascal(name), @@ -216,7 +218,7 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: class PvaEpicsGUI(EpicsGUI): """For creating gui in the PVA EPICS transport.""" - def _get_pv(self, attr_path: list[str], name: str): + def _get_pv(self, attr_path: list[str | int], name: str): return f"pva://{super()._get_pv(attr_path, name)}" def _get_read_widget(self, fastcs_datatype: DataType) -> ReadWidgetUnion | None: diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index e4f8726ff..26c379a88 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -83,14 +83,29 @@ def _make_p4p_raw_value(self) -> dict: stripped_leaf = pv_leaf.rstrip(":PVI") is_controller = stripped_leaf != pv_leaf pvi_name, number = _pv_to_pvi_name(stripped_leaf or pv_leaf) - if is_controller and number is not None: - if signal_info.access not in p4p_raw_value[pvi_name]: - p4p_raw_value[pvi_name][signal_info.access] = {} - p4p_raw_value[pvi_name][signal_info.access][f"v{number}"] = ( + if is_controller and number is not None and not pvi_name: + pattern = rf"(?:(?<=:)|^)([^:]+)(?=:{re.escape(str(number))}(?:[:]|$))" + match = re.search(pattern, signal_info.pv) + + if not match: + raise RuntimeError( + "Failed to extract parent SubControllerVector name " + f"from Subcontroller pv {signal_info.pv}" + ) + if ( + signal_info.access + not in p4p_raw_value[_pascal_to_snake(match.group(1))] + ): + p4p_raw_value[_pascal_to_snake(match.group(1))][ + signal_info.access + ] = {} + p4p_raw_value[_pascal_to_snake(match.group(1))][signal_info.access][ + f"v{number}" + ] = signal_info.pv + elif is_controller: + p4p_raw_value[_pascal_to_snake(stripped_leaf)][signal_info.access] = ( signal_info.pv ) - elif is_controller: - p4p_raw_value[pvi_name][signal_info.access] = signal_info.pv else: attr_pvi_name = f"{pvi_name}{'' if number is None else number}" p4p_raw_value[attr_pvi_name][signal_info.access] = signal_info.pv diff --git a/src/fastcs/transport/graphql/graphql.py b/src/fastcs/transport/graphql/graphql.py index 3560f659e..ffa7b533f 100644 --- a/src/fastcs/transport/graphql/graphql.py +++ b/src/fastcs/transport/graphql/graphql.py @@ -86,7 +86,7 @@ def _process_commands(self, controller_api: ControllerAPI): def _process_sub_apis(self, root_controller_api: ControllerAPI): """Recursively add fields from the queries and mutations of sub apis""" for controller_api in root_controller_api.sub_apis.values(): - name = "".join(controller_api.path) + name = "".join([str(node) for node in controller_api.path]) child_tree = GraphQLAPI(controller_api) if child_tree.queries: self.queries.append( diff --git a/src/fastcs/transport/rest/rest.py b/src/fastcs/transport/rest/rest.py index 66f7d3dee..3247b1309 100644 --- a/src/fastcs/transport/rest/rest.py +++ b/src/fastcs/transport/rest/rest.py @@ -101,7 +101,7 @@ async def attr_get() -> Any: # Must be any as response_model is set def _add_attribute_api_routes(app: FastAPI, root_controller_api: ControllerAPI) -> None: for controller_api in root_controller_api.walk_api(): - path = controller_api.path + path = [str(node) for node in controller_api.path] for attr_name, attribute in controller_api.attributes.items(): attr_name = attr_name.replace("_", "-") @@ -151,7 +151,7 @@ async def command() -> None: def _add_command_api_routes(app: FastAPI, root_controller_api: ControllerAPI) -> None: for controller_api in root_controller_api.walk_api(): - path = controller_api.path + path = [str(node) for node in controller_api.path] for name, method in root_controller_api.command_methods.items(): cmd_name = name.replace("_", "-") diff --git a/src/fastcs/transport/tango/dsr.py b/src/fastcs/transport/tango/dsr.py index 44d00a731..34193e4e0 100644 --- a/src/fastcs/transport/tango/dsr.py +++ b/src/fastcs/transport/tango/dsr.py @@ -61,7 +61,7 @@ def _collect_dev_attributes( ) -> dict[str, Any]: collection: dict[str, Any] = {} for controller_api in root_controller_api.walk_api(): - path = controller_api.path + path = [str(node) for node in controller_api.path] for attr_name, attribute in controller_api.attributes.items(): attr_name = attr_name.title().replace("_", "") @@ -109,7 +109,8 @@ def _wrap_command_f( ) -> Callable[..., Awaitable[None]]: async def _dynamic_f(tango_device: Device) -> None: tango_device.info_stream( - f"called {'_'.join(controller_api.path)} f method: {method_name}" + f"called {'_'.join([str(node) for node in controller_api.path])} " + f"f method: {method_name}" ) coro = method() @@ -125,7 +126,7 @@ def _collect_dev_commands( ) -> dict[str, Any]: collection: dict[str, Any] = {} for controller_api in root_controller_api.walk_api(): - path = controller_api.path + path = [str(node) for node in controller_api.path] for name, method in controller_api.command_methods.items(): cmd_name = name.title().replace("_", "") diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index 6196a30c8..a8f74928c 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -3,17 +3,22 @@ import numpy as np -from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controller import Controller +from fastcs.attributes import AttrHandlerW, AttrR, AttrRW, AttrW +from fastcs.controller import Controller, SubController, SubControllerVector from fastcs.datatypes import Bool, Enum, Float, Int, Table, Waveform from fastcs.launch import FastCS from fastcs.transport.epics.options import ( EpicsIOCOptions, ) -from fastcs.transport.epics.pva.transport import EpicsPVATransport +from fastcs.transport.epics.pva.options import EpicsPVAOptions from fastcs.wrappers import command, scan +class SimpleAttributeSetter(AttrHandlerW): + async def put(self, attr, value): + await attr.update_display_without_process(value) + + class FEnum(enum.Enum): A = 0 B = 1 @@ -25,29 +30,29 @@ class FEnum(enum.Enum): class ParentController(Controller): description = "some controller" a: AttrRW = AttrRW(Int(max=400_000, max_alarm=40_000)) - b: AttrW = AttrW(Float(min=-1, min_alarm=-0.5)) + b: AttrW = AttrW(Float(min=-1, min_alarm=-0.5), handler=SimpleAttributeSetter()) table: AttrRW = AttrRW( Table([("A", np.int32), ("B", "i"), ("C", "?"), ("D", np.float64)]) ) -class ChildController(Controller): +class ChildController(SubController): fail_on_next_e = True - c: AttrW = AttrW(Int()) + c: AttrW = AttrW(Int(), handler=SimpleAttributeSetter()) @command() async def d(self): print("D: RUNNING") await asyncio.sleep(0.1) print("D: FINISHED") - await self.j.update(self.j.get() + 1) + await self.j.set(self.j.get() + 1) e: AttrR = AttrR(Bool()) @scan(1) async def flip_flop(self): - await self.e.update(not self.e.get()) + await self.e.set(not self.e.get()) f: AttrRW = AttrRW(Enum(FEnum)) g: AttrRW = AttrRW(Waveform(np.int64, shape=(3,))) @@ -63,21 +68,36 @@ async def i(self): else: self.fail_on_next_e = True print("I: FINISHED") - await self.j.update(self.j.get() + 1) + await self.j.set(self.j.get() + 1) j: AttrR = AttrR(Int()) def run(pv_prefix="P4P_TEST_DEVICE"): + p4p_options = EpicsPVAOptions(pva_ioc=EpicsIOCOptions(pv_prefix=pv_prefix)) controller = ParentController() - controller.a.enable_tracing() - controller.child1 = ChildController(description="some sub controller") - controller.child2 = ChildController(description="another sub controller") - - fastcs = FastCS( - controller, [EpicsPVATransport(pva_ioc=EpicsIOCOptions(pv_prefix=pv_prefix))] + # controller.register_sub_controller( + # "Child1", ChildController(description="some sub controller") + # ) + # controller.register_sub_controller( + # "Child2", ChildController(description="another sub controller") + # ) + + class Vector(SubControllerVector): + int: AttrR = AttrR(Int()) + + sub_controller = Vector( + { + 1: ChildController(description="some sub controller"), + 2: ChildController(description="another sub controller"), + } ) - fastcs.run(interactive=False) + + controller.register_sub_controller("Child", sub_controller) + + fastcs = FastCS(controller, [p4p_options]) + fastcs.create_gui() + fastcs.run() if __name__ == "__main__": diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 1cb5f9f98..52b3e4f77 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -14,7 +14,7 @@ from p4p.nt import NTTable from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controller import Controller +from fastcs.controller import Controller, SubControllerVector from fastcs.datatypes import Bool, Enum, Float, Int, String, Table, Waveform from fastcs.launch import FastCS from fastcs.transport.epics.options import EpicsIOCOptions @@ -266,6 +266,15 @@ class SomeController(Controller): controller = SomeController() + sub_controller_vector = SubControllerVector( + {i: ChildController() for i in range(3)} + ) + + for _, child in sub_controller_vector.children(): + child.add_sub_controller("ChildChild", ChildChildController()) + + controller.add_sub_controller("Child", sub_controller_vector) + sub_controller = ChildController() controller.child0 = sub_controller sub_controller.child_child = ChildChildController() @@ -291,13 +300,21 @@ class SomeController(Controller): ctxt = ThreadContext("pva") - controller_pvi, child_controller_pvi, child_child_controller_pvi = [], [], [] + ( + controller_pvi, + child_vector_controller_pvi, + child_child_controller_pvi, + child_child_child_controller_pvi, + ) = [], [], [], [] controller_monitor = ctxt.monitor(f"{pv_prefix}:PVI", controller_pvi.append) - child_controller_monitor = ctxt.monitor( - f"{pv_prefix}:Child0:PVI", child_controller_pvi.append + child_vector_controller_monitor = ctxt.monitor( + f"{pv_prefix}:Child:PVI", child_vector_controller_pvi.append ) child_child_controller_monitor = ctxt.monitor( - f"{pv_prefix}:Child0:ChildChild:PVI", child_child_controller_pvi.append + f"{pv_prefix}:Child:0:PVI", child_child_controller_pvi.append + ) + child_child_child_controller_monitor = ctxt.monitor( + f"{pv_prefix}Child:0:ChildChild:PVI", child_child_child_controller_pvi.append ) serve = asyncio.ensure_future(fastcs.serve(interactive=False)) @@ -309,8 +326,9 @@ class SomeController(Controller): ... finally: controller_monitor.close() - child_controller_monitor.close() + child_vector_controller_monitor.close() child_child_controller_monitor.close() + child_child_child_controller_monitor.close() serve.cancel() assert len(controller_pvi) == 1 @@ -329,17 +347,22 @@ class SomeController(Controller): "another_attr1000": {"rw": f"{pv_prefix}:AnotherAttr1000"}, "a_third_attr": {"w": f"{pv_prefix}:AThirdAttr"}, "attr1": {"rw": f"{pv_prefix}:Attr1"}, - "child": { - "d": { - "v0": f"{pv_prefix}:Child0:PVI", - "v1": f"{pv_prefix}:Child1:PVI", - "v2": f"{pv_prefix}:Child2:PVI", - } + "child": {"d": f"{pv_prefix}:Child:PVI"}, + "child_attribute_same_name": { + "d": f"{pv_prefix}:ChildAttributeSameName:PVI", + "r": f"{pv_prefix}:ChildAttributeSameName", + "child": { + "d": { + "v0": f"{pv_prefix}:Child0:PVI", + "v1": f"{pv_prefix}:Child1:PVI", + "v2": f"{pv_prefix}:Child2:PVI", + } + }, }, }, } - assert len(child_controller_pvi) == 1 - assert child_controller_pvi[0].todict() == { + assert len(child_vector_controller_pvi) == 1 + assert child_vector_controller_pvi[0].todict() == { "alarm": {"message": "", "severity": 0, "status": 0}, "display": {"description": ""}, "timeStamp": { @@ -348,11 +371,13 @@ class SomeController(Controller): "userTag": 0, }, "value": { - "attr_c": {"w": f"{pv_prefix}:Child0:AttrC"}, - "attr_d": { - "w": f"{pv_prefix}:Child0:AttrD", + "child": { + "d": { + "v0": f"{pv_prefix}:Child:0:PVI", + "v1": f"{pv_prefix}:Child:1:PVI", + "v2": f"{pv_prefix}:Child:2:PVI", + }, }, - "child_child": {"d": f"{pv_prefix}:Child0:ChildChild:PVI"}, }, } assert len(child_child_controller_pvi) == 1 @@ -365,8 +390,11 @@ class SomeController(Controller): "userTag": 0, }, "value": { - "attr_e": {"rw": f"{pv_prefix}:Child0:ChildChild:AttrE"}, - "attr_f": {"r": f"{pv_prefix}:Child0:ChildChild:AttrF"}, + "attr_c": {"w": f"{pv_prefix}:Child:0:AttrC"}, + "attr_d": { + "w": f"{pv_prefix}:Child:0:AttrD", + }, + "child_child": {"d": f"{pv_prefix}:Child:0:ChildChild:PVI"}, }, } From 7591e8fa87700b98c18b882febab955753d92148 Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Thu, 21 Aug 2025 16:18:17 +0000 Subject: [PATCH 02/29] WIP Pass bool record *NAM fields --- src/fastcs/transport/epics/ca/util.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/fastcs/transport/epics/ca/util.py b/src/fastcs/transport/epics/ca/util.py index d526eb99a..fb882ee2a 100644 --- a/src/fastcs/transport/epics/ca/util.py +++ b/src/fastcs/transport/epics/ca/util.py @@ -91,6 +91,9 @@ def _verify_in_datatype(_, value): return value in datatype.names arguments["validate"] = _verify_in_datatype + case Bool(): + arguments["ZNAM"] = "False" + arguments["ONAM"] = "True" return arguments From d6bfed52557a19e092c2680bfaaed948d9141ad8 Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Thu, 21 Aug 2025 16:17:59 +0000 Subject: [PATCH 03/29] WIP Handle ints in CA pvi --- src/fastcs/transport/epics/ca/ioc.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 0095c678b..f29bdc251 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -113,9 +113,14 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): for child in parent.sub_apis.values(): child_pvi = f"{controller_pv_prefix(pv_prefix, child)}:PVI" - child_name = str(child.path[-1]).lower() + child_name = ( + f"{child.path[-2]}{child.path[-1]}" # Parent name + child idx + if isinstance(child.path[-1], int) + else str(child.path[-1]) # Child name + ) + + _add_pvi_info(child_pvi, parent_pvi, child_name.lower()) - _add_pvi_info(child_pvi, parent_pvi, child_name) _add_sub_controller_pvi_info(pv_prefix, child) From 98fdd7663f8dbc69cac6eb702f6d3782ec30bf2f Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 13:50:32 +0000 Subject: [PATCH 04/29] chore: fix typing due to integer in controller path --- src/fastcs/attributes.py | 6 +++--- src/fastcs/controller_api.py | 9 ++++++--- src/fastcs/transport/epics/util.py | 4 +++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/fastcs/attributes.py b/src/fastcs/attributes.py index 14eba97d0..c9f4c9e9d 100644 --- a/src/fastcs/attributes.py +++ b/src/fastcs/attributes.py @@ -74,7 +74,7 @@ def name(self) -> str: return self._name @property - def path(self) -> list[str]: + def path(self) -> list[int | str]: return self._path def add_update_datatype_callback( @@ -99,7 +99,7 @@ def set_name(self, name: str): self._name = name - def set_path(self, path: list[str]): + def set_path(self, path: list[int | str]): if self._path: raise RuntimeError( f"Attribute is already registered with a controller at {self._path}" @@ -109,7 +109,7 @@ def set_path(self, path: list[str]): def __repr__(self): name = self.__class__.__name__ - path = ".".join(self._path + [self._name]) or None + path = ".".join([str(node) for node in self._path] + [self._name]) or None datatype = self._datatype.__class__.__name__ return f"{name}(path={path}, datatype={datatype}, io_ref={self._io_ref})" diff --git a/src/fastcs/controller_api.py b/src/fastcs/controller_api.py index 655588c31..c6d661dfc 100644 --- a/src/fastcs/controller_api.py +++ b/src/fastcs/controller_api.py @@ -38,9 +38,12 @@ def walk_api(self) -> Iterator["ControllerAPI"]: yield from api.walk_api() def __repr__(self): - return f"""\ -ControllerAPI(path={self.path}, sub_apis=[{", ".join(self.sub_apis.keys())}])\ -""" + return ( + f"ControllerAPI(" + f"path={self.path}, " + f"sub_apis=[{', '.join(str(sub_api) for sub_api in self.sub_apis.keys())}]" + f")" + ) def get_scan_and_initial_coros( self, diff --git a/src/fastcs/transport/epics/util.py b/src/fastcs/transport/epics/util.py index 431afe4b8..5f37c8452 100644 --- a/src/fastcs/transport/epics/util.py +++ b/src/fastcs/transport/epics/util.py @@ -3,4 +3,6 @@ def controller_pv_prefix(prefix: str, controller_api: ControllerAPI) -> str: - return ":".join([prefix] + [snake_to_pascal(node) for node in controller_api.path]) + return ":".join( + [prefix] + [snake_to_pascal(str(node)) for node in controller_api.path] + ) From 57b411d0816bea9f082fca0dd3d10e9e52a1ce17 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 13:51:39 +0000 Subject: [PATCH 05/29] chore: amend p4p raw value logic to strip suffix --- src/fastcs/transport/epics/pva/pvi_tree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index 26c379a88..a4d73ba46 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -80,7 +80,7 @@ def _get_signal_infos(self) -> dict[str, _PviSignalInfo]: def _make_p4p_raw_value(self) -> dict: p4p_raw_value = defaultdict(dict) for pv_leaf, signal_info in self._get_signal_infos().items(): - stripped_leaf = pv_leaf.rstrip(":PVI") + stripped_leaf = pv_leaf.removesuffix(":PVI") is_controller = stripped_leaf != pv_leaf pvi_name, number = _pv_to_pvi_name(stripped_leaf or pv_leaf) if is_controller and number is not None and not pvi_name: From d520bbe6ac18943a88b7139d94933339f3e3374d Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 13:52:42 +0000 Subject: [PATCH 06/29] chore: update example iocs --- tests/example_p4p_ioc.py | 88 ++++++++++++++++++++++++---------------- tests/example_softioc.py | 21 +++++++--- 2 files changed, 70 insertions(+), 39 deletions(-) diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index a8f74928c..2600f95ce 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -1,22 +1,31 @@ import asyncio import enum +from dataclasses import dataclass import numpy as np -from fastcs.attributes import AttrHandlerW, AttrR, AttrRW, AttrW -from fastcs.controller import Controller, SubController, SubControllerVector -from fastcs.datatypes import Bool, Enum, Float, Int, Table, Waveform +from fastcs.attribute_io import AttributeIO +from fastcs.attribute_io_ref import AttributeIORef +from fastcs.attributes import AttrR, AttrRW, AttrW +from fastcs.controller import Controller, SubControllerVector +from fastcs.datatypes import Bool, Enum, Float, Int, T, Table, Waveform from fastcs.launch import FastCS from fastcs.transport.epics.options import ( EpicsIOCOptions, ) -from fastcs.transport.epics.pva.options import EpicsPVAOptions +from fastcs.transport.epics.pva.transport import EpicsPVATransport from fastcs.wrappers import command, scan -class SimpleAttributeSetter(AttrHandlerW): - async def put(self, attr, value): - await attr.update_display_without_process(value) +@dataclass +class SimpleAttributeIORef(AttributeIORef): + pass + + +class SimpleAttributeIO(AttributeIO[T, SimpleAttributeIORef]): + async def send(self, attr: AttrW[T, SimpleAttributeIORef], value): + if isinstance(attr, AttrRW): + await attr.update(value) class FEnum(enum.Enum): @@ -29,30 +38,39 @@ class FEnum(enum.Enum): class ParentController(Controller): description = "some controller" - a: AttrRW = AttrRW(Int(max=400_000, max_alarm=40_000)) - b: AttrW = AttrW(Float(min=-1, min_alarm=-0.5), handler=SimpleAttributeSetter()) + a: AttrRW = AttrRW( + Int(max=400_000, max_alarm=40_000), io_ref=SimpleAttributeIORef() + ) + b: AttrW = AttrW(Float(min=-1, min_alarm=-0.5), io_ref=SimpleAttributeIORef()) table: AttrRW = AttrRW( - Table([("A", np.int32), ("B", "i"), ("C", "?"), ("D", np.float64)]) + Table([("A", np.int32), ("B", "i"), ("C", "?"), ("D", np.float64)]), + io_ref=SimpleAttributeIORef(), ) + def __init__(self, description=None, ios=None): + super().__init__(description, ios) + -class ChildController(SubController): +class ChildController(Controller): fail_on_next_e = True - c: AttrW = AttrW(Int(), handler=SimpleAttributeSetter()) + c: AttrW = AttrW(Int(), io_ref=SimpleAttributeIORef()) + + def __init__(self, description=None, ios=None): + super().__init__(description, ios) @command() async def d(self): print("D: RUNNING") await asyncio.sleep(0.1) print("D: FINISHED") - await self.j.set(self.j.get() + 1) + await self.j.update(self.j.get() + 1) - e: AttrR = AttrR(Bool()) + e: AttrR = AttrR(Bool(), io_ref=SimpleAttributeIORef()) @scan(1) async def flip_flop(self): - await self.e.set(not self.e.get()) + await self.e.update(not self.e.get()) f: AttrRW = AttrRW(Enum(FEnum)) g: AttrRW = AttrRW(Waveform(np.int64, shape=(3,))) @@ -68,35 +86,37 @@ async def i(self): else: self.fail_on_next_e = True print("I: FINISHED") - await self.j.set(self.j.get() + 1) + await self.j.update(self.j.get() + 1) j: AttrR = AttrR(Int()) def run(pv_prefix="P4P_TEST_DEVICE"): - p4p_options = EpicsPVAOptions(pva_ioc=EpicsIOCOptions(pv_prefix=pv_prefix)) - controller = ParentController() - # controller.register_sub_controller( - # "Child1", ChildController(description="some sub controller") - # ) - # controller.register_sub_controller( - # "Child2", ChildController(description="another sub controller") - # ) - - class Vector(SubControllerVector): - int: AttrR = AttrR(Int()) - - sub_controller = Vector( + simple_attribute_io = SimpleAttributeIO() + p4p_options = EpicsPVATransport(pva_ioc=EpicsIOCOptions(pv_prefix=pv_prefix)) + controller = ParentController(ios=[simple_attribute_io]) + + class ChildVector(SubControllerVector): + vector_attribute: AttrR = AttrR(Int()) + + def __init__(self, children, description=None): + super().__init__(children, description) + + sub_controller = ChildVector( { - 1: ChildController(description="some sub controller"), - 2: ChildController(description="another sub controller"), - } + 1: ChildController( + description="some sub controller", ios=[simple_attribute_io] + ), + 2: ChildController( + description="another sub controller", ios=[simple_attribute_io] + ), + }, + description="some child vector", ) - controller.register_sub_controller("Child", sub_controller) + controller.add_sub_controller("Child", sub_controller) fastcs = FastCS(controller, [p4p_options]) - fastcs.create_gui() fastcs.run() diff --git a/tests/example_softioc.py b/tests/example_softioc.py index d5dea6eed..c833c8a5c 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -1,8 +1,10 @@ +from pathlib import Path + from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controller import Controller +from fastcs.controller import Controller, SubControllerVector from fastcs.datatypes import Int from fastcs.launch import FastCS -from fastcs.transport.epics.ca.transport import EpicsCATransport +from fastcs.transport.epics.ca.transport import EpicsCATransport, EpicsGUIOptions from fastcs.transport.epics.options import EpicsIOCOptions from fastcs.wrappers import command @@ -22,11 +24,20 @@ async def d(self): def run(pv_prefix="SOFTIOC_TEST_DEVICE"): controller = ParentController() - controller.child = ChildController() + vector = SubControllerVector({i: ChildController() for i in range(2)}) + controller.add_sub_controller("ChildVector", vector) + gui_options = EpicsGUIOptions( + output_path=Path(".") / "demo.bob", title="Demo Vector" + ) fastcs = FastCS( - controller, [EpicsCATransport(ca_ioc=EpicsIOCOptions(pv_prefix=pv_prefix))] + controller, + [ + EpicsCATransport( + ca_ioc=EpicsIOCOptions(pv_prefix=pv_prefix), gui=gui_options + ) + ], ) - fastcs.run(interactive=False) + fastcs.run(interactive=True) if __name__ == "__main__": From 016344f1b704f059474c4f2dbf70c4804a9b7892 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 13:53:15 +0000 Subject: [PATCH 07/29] tests: amend tests to use controller vetor --- .../transport/epics/ca/test_softioc_system.py | 23 ++++-- tests/transport/epics/pva/test_p4p.py | 72 +++++++++---------- 2 files changed, 54 insertions(+), 41 deletions(-) diff --git a/tests/transport/epics/ca/test_softioc_system.py b/tests/transport/epics/ca/test_softioc_system.py index 1d324dab9..ce893b183 100644 --- a/tests/transport/epics/ca/test_softioc_system.py +++ b/tests/transport/epics/ca/test_softioc_system.py @@ -16,16 +16,31 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert parent_pvi["value"] == { "a": {"r": f"{pv_prefix}:A"}, "b": {"r": f"{pv_prefix}:B_RBV", "w": f"{pv_prefix}:B"}, - "child": {"d": f"{pv_prefix}:Child:PVI"}, + "childvector": {"d": f"{pv_prefix}:ChildVector:PVI"}, } - child_pvi_pv = parent_pvi["value"]["child"]["d"] + child_vector_pvi_pv = parent_pvi["value"]["childvector"]["d"] + _child_vector_pvi = ctxt.get(child_vector_pvi_pv) + assert isinstance(_child_vector_pvi, Value) + _child_vector_pvi = _child_vector_pvi.todict() + assert all( + f in _child_vector_pvi for f in ("alarm", "display", "timeStamp", "value") + ) + assert _child_vector_pvi["display"] == { + "description": "The records in this controller" + } + assert _child_vector_pvi["value"] == { + "childvector0": {"d": f"{pv_prefix}:ChildVector:0:PVI"}, + "childvector1": {"d": f"{pv_prefix}:ChildVector:1:PVI"}, + } + + child_pvi_pv = _child_vector_pvi["value"]["childvector0"]["d"] _child_pvi = ctxt.get(child_pvi_pv) assert isinstance(_child_pvi, Value) child_pvi = _child_pvi.todict() assert all(f in child_pvi for f in ("alarm", "display", "timeStamp", "value")) assert child_pvi["display"] == {"description": "The records in this controller"} assert child_pvi["value"] == { - "c": {"w": f"{pv_prefix}:Child:C"}, - "d": {"x": f"{pv_prefix}:Child:D"}, + "c": {"w": f"{pv_prefix}:ChildVector:0:C"}, + "d": {"x": f"{pv_prefix}:ChildVector:0:D"}, } diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 52b3e4f77..e870491ad 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -35,32 +35,42 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): assert parent_pvi["value"] == { "a": {"rw": f"{pv_prefix}:A"}, "b": {"w": f"{pv_prefix}:B"}, - "child": { - "d": { - "v1": f"{pv_prefix}:Child1:PVI", - "v2": f"{pv_prefix}:Child2:PVI", - } - }, + "child": {"d": f"{pv_prefix}:Child:PVI"}, "table": { "rw": f"{pv_prefix}:Table", }, } - child_pvi_pv = parent_pvi["value"]["child"]["d"]["v1"] + child_vector_pvi_pv = parent_pvi["value"]["child"]["d"] + _child_vector_pvi = await ctxt.get(child_vector_pvi_pv) + assert isinstance(_child_vector_pvi, Value) + _child_vector_pvi = _child_vector_pvi.todict() + assert all( + f in _child_vector_pvi for f in ("alarm", "display", "timeStamp", "value") + ) + assert _child_vector_pvi["display"] == {"description": "some child vector"} + assert _child_vector_pvi["value"] == { + "vector_attribute": {"r": f"{pv_prefix}:Child:VectorAttribute"}, + "child": { + "d": {"v1": f"{pv_prefix}:Child:1:PVI", "v2": f"{pv_prefix}:Child:2:PVI"} + }, + } + + child_pvi_pv = _child_vector_pvi["value"]["child"]["d"]["v1"] _child_pvi = await ctxt.get(child_pvi_pv) assert isinstance(_child_pvi, Value) child_pvi = _child_pvi.todict() assert all(f in child_pvi for f in ("alarm", "display", "timeStamp", "value")) assert child_pvi["display"] == {"description": "some sub controller"} assert child_pvi["value"] == { - "c": {"w": f"{pv_prefix}:Child1:C"}, - "d": {"x": f"{pv_prefix}:Child1:D"}, - "e": {"r": f"{pv_prefix}:Child1:E"}, - "f": {"rw": f"{pv_prefix}:Child1:F"}, - "g": {"rw": f"{pv_prefix}:Child1:G"}, - "h": {"rw": f"{pv_prefix}:Child1:H"}, - "i": {"x": f"{pv_prefix}:Child1:I"}, - "j": {"r": f"{pv_prefix}:Child1:J"}, + "c": {"w": f"{pv_prefix}:Child:1:C"}, + "d": {"x": f"{pv_prefix}:Child:1:D"}, + "e": {"r": f"{pv_prefix}:Child:1:E"}, + "f": {"rw": f"{pv_prefix}:Child:1:F"}, + "g": {"rw": f"{pv_prefix}:Child:1:G"}, + "h": {"rw": f"{pv_prefix}:Child:1:H"}, + "i": {"x": f"{pv_prefix}:Child:1:I"}, + "j": {"r": f"{pv_prefix}:Child:1:J"}, } @@ -74,7 +84,7 @@ async def test_scan_method(p4p_subprocess: tuple[str, Queue]): # time for the p4p transport to update, broadcast, get. latency = 1e8 - e_monitor = ctxt.monitor(f"{pv_prefix}:Child1:E", e_values.put) + e_monitor = ctxt.monitor(f"{pv_prefix}:Child:1:E", e_values.put) try: # Throw away the value on the ioc setup so we can compare timestamps _ = await e_values.get() @@ -111,14 +121,14 @@ async def test_command_method(p4p_subprocess: tuple[str, Queue]): j_values = asyncio.Queue() ctxt = Context("pva") - d_monitor = ctxt.monitor(f"{pv_prefix}:Child1:D", d_values.put) - i_monitor = ctxt.monitor(f"{pv_prefix}:Child1:I", i_values.put) - j_monitor = ctxt.monitor(f"{pv_prefix}:Child1:J", j_values.put) + d_monitor = ctxt.monitor(f"{pv_prefix}:Child:1:D", d_values.put) + i_monitor = ctxt.monitor(f"{pv_prefix}:Child:1:I", i_values.put) + j_monitor = ctxt.monitor(f"{pv_prefix}:Child:1:J", j_values.put) try: j_initial_value = await j_values.get() assert (await d_values.get()).raw.value is False - await ctxt.put(f"{pv_prefix}:Child1:D", True) + await ctxt.put(f"{pv_prefix}:Child:1:D", True) assert (await d_values.get()).raw.value is True # D process hangs for 0.1s, so we wait slightly longer await asyncio.sleep(0.2) @@ -132,7 +142,7 @@ async def test_command_method(p4p_subprocess: tuple[str, Queue]): assert before_command_value["value"] is False assert before_command_value["alarm"]["severity"] == 0 assert before_command_value["alarm"]["message"] == "" - await ctxt.put(f"{pv_prefix}:Child1:I", True) + await ctxt.put(f"{pv_prefix}:Child:1:I", True) assert (await i_values.get()).raw.value is True await asyncio.sleep(0.2) @@ -146,7 +156,7 @@ async def test_command_method(p4p_subprocess: tuple[str, Queue]): assert j_values.empty() # Second run succeeds - await ctxt.put(f"{pv_prefix}:Child1:I", True) + await ctxt.put(f"{pv_prefix}:Child:1:I", True) assert (await i_values.get()).raw.value is True await asyncio.sleep(0.2) after_command_value = (await i_values.get()).raw @@ -270,9 +280,6 @@ class SomeController(Controller): {i: ChildController() for i in range(3)} ) - for _, child in sub_controller_vector.children(): - child.add_sub_controller("ChildChild", ChildChildController()) - controller.add_sub_controller("Child", sub_controller_vector) sub_controller = ChildController() @@ -348,17 +355,9 @@ class SomeController(Controller): "a_third_attr": {"w": f"{pv_prefix}:AThirdAttr"}, "attr1": {"rw": f"{pv_prefix}:Attr1"}, "child": {"d": f"{pv_prefix}:Child:PVI"}, - "child_attribute_same_name": { - "d": f"{pv_prefix}:ChildAttributeSameName:PVI", - "r": f"{pv_prefix}:ChildAttributeSameName", - "child": { - "d": { - "v0": f"{pv_prefix}:Child0:PVI", - "v1": f"{pv_prefix}:Child1:PVI", - "v2": f"{pv_prefix}:Child2:PVI", - } - }, - }, + "child0": {"d": f"{pv_prefix}:Child0:PVI"}, + "child1": {"d": f"{pv_prefix}:Child1:PVI"}, + "child2": {"d": f"{pv_prefix}:Child2:PVI"}, }, } assert len(child_vector_controller_pvi) == 1 @@ -394,7 +393,6 @@ class SomeController(Controller): "attr_d": { "w": f"{pv_prefix}:Child:0:AttrD", }, - "child_child": {"d": f"{pv_prefix}:Child:0:ChildChild:PVI"}, }, } From de036d78b171f0d6d5c24ba73fddfabb3a425bda Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 13:56:06 +0000 Subject: [PATCH 08/29] chore: amend controller api path type signature --- src/fastcs/control_system.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fastcs/control_system.py b/src/fastcs/control_system.py index 3f9fb4cde..828981b24 100644 --- a/src/fastcs/control_system.py +++ b/src/fastcs/control_system.py @@ -180,7 +180,9 @@ def build_controller_api(controller: Controller) -> ControllerAPI: return _build_controller_api(controller, []) -def _build_controller_api(controller: BaseController, path: list[str]) -> ControllerAPI: +def _build_controller_api( + controller: BaseController, path: list[str | int] +) -> ControllerAPI: scan_methods: dict[str, Scan] = {} command_methods: dict[str, Command] = {} for attr_name in dir(controller): From c8a6c0a373222b2366e0a80ab65c7049b85ff563 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 13:58:49 +0000 Subject: [PATCH 09/29] chore: add ios to vector init --- src/fastcs/controller.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 3d72c322e..ae2f2fceb 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -214,11 +214,14 @@ class SubControllerVector(MutableMapping[int, Controller], Controller): """ def __init__( - self, children: Mapping[int, Controller], description: str | None = None + self, + children: Mapping[int, Controller], + description: str | None = None, + ios: Sequence[AttributeIO[T, AttributeIORefT]] | None = None, ) -> None: self._children: dict[int, Controller] = {} self.update(children) - super().__init__(description=description) + super().__init__(description=description, ios=ios) for index, child in children.items(): self.add_sub_controller(index, child) From 480bab80f0a8ed742f3c3da3b8945ff3979e07d2 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 14:30:55 +0000 Subject: [PATCH 10/29] refactor: remove duplicate pvi tree layer for p4p --- src/fastcs/transport/epics/pva/pvi_tree.py | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index a4d73ba46..e09dd695f 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -84,24 +84,9 @@ def _make_p4p_raw_value(self) -> dict: is_controller = stripped_leaf != pv_leaf pvi_name, number = _pv_to_pvi_name(stripped_leaf or pv_leaf) if is_controller and number is not None and not pvi_name: - pattern = rf"(?:(?<=:)|^)([^:]+)(?=:{re.escape(str(number))}(?:[:]|$))" - match = re.search(pattern, signal_info.pv) - - if not match: - raise RuntimeError( - "Failed to extract parent SubControllerVector name " - f"from Subcontroller pv {signal_info.pv}" - ) - if ( - signal_info.access - not in p4p_raw_value[_pascal_to_snake(match.group(1))] - ): - p4p_raw_value[_pascal_to_snake(match.group(1))][ - signal_info.access - ] = {} - p4p_raw_value[_pascal_to_snake(match.group(1))][signal_info.access][ - f"v{number}" - ] = signal_info.pv + if signal_info.access not in p4p_raw_value[f"v{number}"]: + p4p_raw_value[f"v{number}"][signal_info.access] = {} + p4p_raw_value[f"v{number}"][signal_info.access] = signal_info.pv elif is_controller: p4p_raw_value[_pascal_to_snake(stripped_leaf)][signal_info.access] = ( signal_info.pv From d88276593e41270a787a613da15efaa86ec93256 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 14:31:20 +0000 Subject: [PATCH 11/29] test: amend p4p tests to use new pvi structure --- tests/transport/epics/pva/test_p4p.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index e870491ad..e3288ee45 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -51,12 +51,11 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): assert _child_vector_pvi["display"] == {"description": "some child vector"} assert _child_vector_pvi["value"] == { "vector_attribute": {"r": f"{pv_prefix}:Child:VectorAttribute"}, - "child": { - "d": {"v1": f"{pv_prefix}:Child:1:PVI", "v2": f"{pv_prefix}:Child:2:PVI"} - }, + "v1": {"d": f"{pv_prefix}:Child:1:PVI"}, + "v2": {"d": f"{pv_prefix}:Child:2:PVI"}, } - child_pvi_pv = _child_vector_pvi["value"]["child"]["d"]["v1"] + child_pvi_pv = _child_vector_pvi["value"]["v1"]["d"] _child_pvi = await ctxt.get(child_pvi_pv) assert isinstance(_child_pvi, Value) child_pvi = _child_pvi.todict() @@ -370,13 +369,9 @@ class SomeController(Controller): "userTag": 0, }, "value": { - "child": { - "d": { - "v0": f"{pv_prefix}:Child:0:PVI", - "v1": f"{pv_prefix}:Child:1:PVI", - "v2": f"{pv_prefix}:Child:2:PVI", - }, - }, + "v0": {"d": f"{pv_prefix}:Child:0:PVI"}, + "v1": {"d": f"{pv_prefix}:Child:1:PVI"}, + "v2": {"d": f"{pv_prefix}:Child:2:PVI"}, }, } assert len(child_child_controller_pvi) == 1 From 3e8e43afa588920e90f869ac51c636d00b5bfe08 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 14:44:49 +0000 Subject: [PATCH 12/29] chore: revert type signature for add_sub_controller --- src/fastcs/controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index ae2f2fceb..e34438b4f 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -142,7 +142,7 @@ def add_attribute(self, name, attribute: Attribute): self.attributes[name] = attribute super().__setattr__(name, attribute) - def add_sub_controller(self, name: str | int, sub_controller: BaseController): + def add_sub_controller(self, name: str | int, sub_controller: Controller): if name in self.__sub_controller_tree.keys(): raise ValueError( f"Cannot add sub controller {name}. " From a8805b90b56330a2e47b057c30d912336d9dace1 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 3 Nov 2025 15:27:14 +0000 Subject: [PATCH 13/29] chore: amend controller vector name --- src/fastcs/controller.py | 2 +- tests/example_p4p_ioc.py | 4 ++-- tests/example_softioc.py | 4 ++-- tests/transport/epics/pva/test_p4p.py | 6 ++---- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index e34438b4f..8eea59b32 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -206,7 +206,7 @@ async def disconnect(self) -> None: pass -class SubControllerVector(MutableMapping[int, Controller], Controller): +class ControllerVector(MutableMapping[int, Controller], Controller): """A collection of SubControllers, with an arbitrary integer index. An instance of this class can be registered with a parent ``Controller`` to include it's children as part of a larger controller. Each child of the vector will keep diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index 2600f95ce..1145ac82f 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -7,7 +7,7 @@ from fastcs.attribute_io import AttributeIO from fastcs.attribute_io_ref import AttributeIORef from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controller import Controller, SubControllerVector +from fastcs.controller import Controller, ControllerVector from fastcs.datatypes import Bool, Enum, Float, Int, T, Table, Waveform from fastcs.launch import FastCS from fastcs.transport.epics.options import ( @@ -96,7 +96,7 @@ def run(pv_prefix="P4P_TEST_DEVICE"): p4p_options = EpicsPVATransport(pva_ioc=EpicsIOCOptions(pv_prefix=pv_prefix)) controller = ParentController(ios=[simple_attribute_io]) - class ChildVector(SubControllerVector): + class ChildVector(ControllerVector): vector_attribute: AttrR = AttrR(Int()) def __init__(self, children, description=None): diff --git a/tests/example_softioc.py b/tests/example_softioc.py index c833c8a5c..ad40e5c69 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -1,7 +1,7 @@ from pathlib import Path from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controller import Controller, SubControllerVector +from fastcs.controller import Controller, ControllerVector from fastcs.datatypes import Int from fastcs.launch import FastCS from fastcs.transport.epics.ca.transport import EpicsCATransport, EpicsGUIOptions @@ -24,7 +24,7 @@ async def d(self): def run(pv_prefix="SOFTIOC_TEST_DEVICE"): controller = ParentController() - vector = SubControllerVector({i: ChildController() for i in range(2)}) + vector = ControllerVector({i: ChildController() for i in range(2)}) controller.add_sub_controller("ChildVector", vector) gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Vector" diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index e3288ee45..10ecda2b0 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -14,7 +14,7 @@ from p4p.nt import NTTable from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controller import Controller, SubControllerVector +from fastcs.controller import Controller, ControllerVector from fastcs.datatypes import Bool, Enum, Float, Int, String, Table, Waveform from fastcs.launch import FastCS from fastcs.transport.epics.options import EpicsIOCOptions @@ -275,9 +275,7 @@ class SomeController(Controller): controller = SomeController() - sub_controller_vector = SubControllerVector( - {i: ChildController() for i in range(3)} - ) + sub_controller_vector = ControllerVector({i: ChildController() for i in range(3)}) controller.add_sub_controller("Child", sub_controller_vector) From 09c98426344e3c74ccf4795a40409459061a7584 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 5 Nov 2025 10:49:19 +0000 Subject: [PATCH 14/29] refactor: move vector children up one tree level using controller api --- src/fastcs/transport/epics/pva/ioc.py | 2 +- src/fastcs/transport/epics/pva/pvi_tree.py | 55 ++++++++++++++++------ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index be20f27e9..f626b5fc5 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -37,7 +37,7 @@ async def parse_attributes( for controller_api in root_controller_api.walk_api(): pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) - pvi_tree.add_sub_device(pv_prefix, controller_api.description) + pvi_tree.add_sub_device(pv_prefix, controller_api.description, controller_api) for attr_name, attribute in controller_api.attributes.items(): full_pv_name = f"{pv_prefix}:{snake_to_pascal(attr_name)}" diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index 897e1ea13..0825d4232 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -8,6 +8,8 @@ from p4p.server import StaticProvider from p4p.server.asyncio import SharedPV +from fastcs.controller_api import ControllerAPI + from .types import p4p_alarm_states, p4p_timestamp_now AccessModeType = Literal["r", "w", "rw", "d", "x"] @@ -40,16 +42,19 @@ class PviDevice(dict[str, "PviDevice"]): """For creating a pvi structure in pva.""" pv_prefix: str + controller_api: ControllerAPI | None description: str | None device_signal_info: _PviSignalInfo | None def __init__( self, pv_prefix: str, + controller_api: ControllerAPI | None = None, description: str | None = None, device_signal_info: _PviSignalInfo | None = None, ): self.pv_prefix = pv_prefix + self.controller_api = controller_api self.description = description self.device_signal_info = device_signal_info @@ -82,17 +87,37 @@ def _make_p4p_raw_value(self) -> dict: for pv_leaf, signal_info in self._get_signal_infos().items(): stripped_leaf = pv_leaf.removesuffix(":PVI") is_controller = stripped_leaf != pv_leaf - pvi_name, number = _pv_to_pvi_name(stripped_leaf or pv_leaf) - if is_controller and number is not None and not pvi_name: - if signal_info.access not in p4p_raw_value[f"v{number}"]: - p4p_raw_value[f"v{number}"][signal_info.access] = {} - p4p_raw_value[f"v{number}"][signal_info.access] = signal_info.pv - elif is_controller: - p4p_raw_value[_pascal_to_snake(stripped_leaf)][signal_info.access] = ( - signal_info.pv - ) - else: - attr_pvi_name = f"{pvi_name}{'' if number is None else number}" + if is_controller and self.controller_api and not stripped_leaf.isdigit(): + sub_api = self.controller_api.sub_apis[stripped_leaf] + # Check if sub-device has sub-devices with numeric names + vector_children = [ + child for child in sub_api.sub_apis.keys() if isinstance(child, int) + ] + # Sub-device is a ControllerVector + if vector_children: + # Check if ControllerVector has a 'd' entry + if ( + signal_info.access + not in p4p_raw_value[_pascal_to_snake(stripped_leaf)] + ): + p4p_raw_value[_pascal_to_snake(stripped_leaf)][ + signal_info.access + ] = {} + # Group entries for all vector children + for vector_child in vector_children: + p4p_raw_value[_pascal_to_snake(stripped_leaf)][ + signal_info.access + ][ + f"v{vector_child}" + ] = f"{signal_info.pv.removesuffix(':PVI')}:{vector_child}:PVI" + # Sub-device is a Controller + else: + p4p_raw_value[_pascal_to_snake(stripped_leaf)][ + signal_info.access + ] = signal_info.pv + # Add attribute entry + elif not is_controller: + attr_pvi_name = f"{stripped_leaf}" p4p_raw_value[attr_pvi_name][signal_info.access] = signal_info.pv return p4p_raw_value @@ -179,17 +204,19 @@ def __init__(self, pv_prefix: str): self._pvi_tree_root: PviDevice = PviDevice(pv_prefix) def add_sub_device( - self, - device_pv: str, - description: str | None, + self, device_pv: str, description: str | None, controller_api: ControllerAPI ): if ":" not in device_pv: assert device_pv == self._pvi_tree_root.pv_prefix self._pvi_tree_root.description = description + self._pvi_tree_root.controller_api = controller_api else: self._pvi_tree_root.get_recursively( *device_pv.split(":")[1:] # To remove the prefix ).description = description + self._pvi_tree_root.get_recursively( + *device_pv.split(":")[1:] # To remove the prefix + ).controller_api = controller_api def add_signal( self, From c696a9714e9512c191d428afefa27de3ccf924fd Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 5 Nov 2025 13:25:01 +0000 Subject: [PATCH 15/29] refactor: amend pva structure to add intermediate ControllerVector level --- src/fastcs/transport/epics/ca/ioc.py | 4 +- src/fastcs/transport/epics/pva/pvi_tree.py | 55 +++++++++------------- 2 files changed, 24 insertions(+), 35 deletions(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index f29bdc251..429aae910 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -114,9 +114,9 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): for child in parent.sub_apis.values(): child_pvi = f"{controller_pv_prefix(pv_prefix, child)}:PVI" child_name = ( - f"{child.path[-2]}{child.path[-1]}" # Parent name + child idx + f"__{child.path[-1]}" # Sub-Controller of ControllerVector if isinstance(child.path[-1], int) - else str(child.path[-1]) # Child name + else str(child.path[-1]) ) _add_pvi_info(child_pvi, parent_pvi, child_name.lower()) diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index 0825d4232..c0d67390b 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -69,54 +69,44 @@ def get_recursively(self, *args: str) -> "PviDevice": d = d[arg] return d - def _get_signal_infos(self) -> dict[str, _PviSignalInfo]: - device_signal_infos: dict[str, _PviSignalInfo] = {} + def _get_signal_infos( + self, + ) -> dict[str, tuple[_PviSignalInfo, ControllerAPI | None]]: + device_signal_infos: dict[str, tuple[_PviSignalInfo, ControllerAPI | None]] = {} for sub_device_name, sub_device in self.items(): if sub_device: - device_signal_infos[f"{sub_device_name}:PVI"] = _PviSignalInfo( - pv=f"{sub_device.pv_prefix}:PVI", access="d" + device_signal_infos[f"{sub_device_name}:PVI"] = ( + _PviSignalInfo(pv=f"{sub_device.pv_prefix}:PVI", access="d"), + sub_device.controller_api, ) if sub_device.device_signal_info: - device_signal_infos[sub_device_name] = sub_device.device_signal_info + device_signal_infos[sub_device_name] = ( + sub_device.device_signal_info, + sub_device.controller_api, + ) return device_signal_infos def _make_p4p_raw_value(self) -> dict: p4p_raw_value = defaultdict(dict) - for pv_leaf, signal_info in self._get_signal_infos().items(): + for pv_leaf, signal_info_and_api in self._get_signal_infos().items(): + # Sub-controller api returned if current item is a Controller + signal_info, sub_controller_api = signal_info_and_api stripped_leaf = pv_leaf.removesuffix(":PVI") - is_controller = stripped_leaf != pv_leaf - if is_controller and self.controller_api and not stripped_leaf.isdigit(): - sub_api = self.controller_api.sub_apis[stripped_leaf] - # Check if sub-device has sub-devices with numeric names - vector_children = [ - child for child in sub_api.sub_apis.keys() if isinstance(child, int) - ] - # Sub-device is a ControllerVector - if vector_children: - # Check if ControllerVector has a 'd' entry - if ( - signal_info.access - not in p4p_raw_value[_pascal_to_snake(stripped_leaf)] - ): - p4p_raw_value[_pascal_to_snake(stripped_leaf)][ - signal_info.access - ] = {} - # Group entries for all vector children - for vector_child in vector_children: - p4p_raw_value[_pascal_to_snake(stripped_leaf)][ - signal_info.access - ][ - f"v{vector_child}" - ] = f"{signal_info.pv.removesuffix(':PVI')}:{vector_child}:PVI" - # Sub-device is a Controller + # Add Controller entry + if sub_controller_api: + # Sub-device of a ControllerVector + if isinstance(sub_controller_api.path[-1], int): + p4p_raw_value[f"__{int(stripped_leaf)}"][signal_info.access] = ( + signal_info.pv + ) else: p4p_raw_value[_pascal_to_snake(stripped_leaf)][ signal_info.access ] = signal_info.pv # Add attribute entry - elif not is_controller: + else: attr_pvi_name = f"{stripped_leaf}" p4p_raw_value[attr_pvi_name][signal_info.access] = signal_info.pv @@ -209,7 +199,6 @@ def add_sub_device( if ":" not in device_pv: assert device_pv == self._pvi_tree_root.pv_prefix self._pvi_tree_root.description = description - self._pvi_tree_root.controller_api = controller_api else: self._pvi_tree_root.get_recursively( *device_pv.split(":")[1:] # To remove the prefix From b27499b2e61fd5335b2b485ee427c46ca3a3078a Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 5 Nov 2025 13:44:50 +0000 Subject: [PATCH 16/29] refactor: revert path to str only and restrict add_sub_controller --- src/fastcs/attributes.py | 4 +-- src/fastcs/control_system.py | 4 +-- src/fastcs/controller.py | 38 +++++++++++++++++----- src/fastcs/controller_api.py | 4 +-- src/fastcs/transport/epics/ca/ioc.py | 2 +- src/fastcs/transport/epics/gui.py | 10 +++--- src/fastcs/transport/epics/pva/pvi_tree.py | 2 +- 7 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/fastcs/attributes.py b/src/fastcs/attributes.py index 7f6a5f673..4024484b4 100644 --- a/src/fastcs/attributes.py +++ b/src/fastcs/attributes.py @@ -74,7 +74,7 @@ def name(self) -> str: return self._name @property - def path(self) -> list[int | str]: + def path(self) -> list[str]: return self._path def add_update_datatype_callback( @@ -99,7 +99,7 @@ def set_name(self, name: str): self._name = name - def set_path(self, path: list[int | str]): + def set_path(self, path: list[str]): if self._path: raise RuntimeError( f"Attribute is already registered with a controller at {self._path}" diff --git a/src/fastcs/control_system.py b/src/fastcs/control_system.py index 40b8d0d0c..82dbf019d 100644 --- a/src/fastcs/control_system.py +++ b/src/fastcs/control_system.py @@ -180,9 +180,7 @@ def build_controller_api(controller: Controller) -> ControllerAPI: return _build_controller_api(controller, []) -def _build_controller_api( - controller: BaseController, path: list[str | int] -) -> ControllerAPI: +def _build_controller_api(controller: BaseController, path: list[str]) -> ControllerAPI: scan_methods: dict[str, Scan] = {} command_methods: dict[str, Command] = {} for attr_name in dir(controller): diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 8eea59b32..096bbd0a3 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -22,7 +22,7 @@ class BaseController(Tracer): def __init__( self, - path: list[str | int] | None = None, + path: list[str] | None = None, description: str | None = None, ios: Sequence[AttributeIO[T, AttributeIORefT]] | None = None, ) -> None: @@ -35,8 +35,8 @@ def __init__( if not hasattr(self, "attributes"): self.attributes = {} - self._path: list[str | int] = path or [] - self.__sub_controller_tree: dict[str | int, BaseController] = {} + self._path: list[str] = path or [] + self.__sub_controller_tree: dict[str, BaseController] = {} self._bind_attrs() @@ -71,11 +71,11 @@ def connect_attribute_ios(self) -> None: controller.connect_attribute_ios() @property - def path(self) -> list[str | int]: + def path(self) -> list[str]: """Path prefix of attributes, recursively including parent Controllers.""" return self._path - def set_path(self, path: list[str | int]): + def set_path(self, path: list[str]): if self._path: raise ValueError(f"sub controller is already registered under {self.path}") @@ -142,7 +142,9 @@ def add_attribute(self, name, attribute: Attribute): self.attributes[name] = attribute super().__setattr__(name, attribute) - def add_sub_controller(self, name: str | int, sub_controller: Controller): + def add_sub_controller( + self, name: str, sub_controller: Controller | ControllerVector + ): if name in self.__sub_controller_tree.keys(): raise ValueError( f"Cannot add sub controller {name}. " @@ -162,7 +164,7 @@ def add_sub_controller(self, name: str | int, sub_controller: Controller): self.attributes[str(name)] = sub_controller.root_attribute @property - def sub_controllers(self) -> dict[str | int, BaseController]: + def sub_controllers(self) -> dict[str, BaseController]: return self.__sub_controller_tree def __repr__(self): @@ -199,6 +201,16 @@ def __init__( ) -> None: super().__init__(description=description, ios=ios) + def add_sub_controller( + self, name: str, sub_controller: Controller | ControllerVector + ): + if name.isdigit(): + raise ValueError( + f"Cannot add sub controller {name}. " + "Numeric-only names are not allowed; use ControllerVector instead" + ) + return super().add_sub_controller(name, sub_controller) + async def connect(self) -> None: pass @@ -206,13 +218,15 @@ async def disconnect(self) -> None: pass -class ControllerVector(MutableMapping[int, Controller], Controller): +class ControllerVector(MutableMapping[int, Controller], BaseController): """A collection of SubControllers, with an arbitrary integer index. An instance of this class can be registered with a parent ``Controller`` to include it's children as part of a larger controller. Each child of the vector will keep a string name of the vector. """ + root_attribute: Attribute | None = None + def __init__( self, children: Mapping[int, Controller], @@ -223,7 +237,13 @@ def __init__( self.update(children) super().__init__(description=description, ios=ios) for index, child in children.items(): - self.add_sub_controller(index, child) + super().add_sub_controller(str(index), child) + + def add_sub_controller(self, *args, **kwargs): + raise NotImplementedError( + "Cannot add named sub controller to ControllerVector. " + "Use __setitem__ instead, for indexed sub controllers" + ) def __getitem__(self, key: int) -> Controller: return self._children[key] diff --git a/src/fastcs/controller_api.py b/src/fastcs/controller_api.py index c6d661dfc..eecc9f5a7 100644 --- a/src/fastcs/controller_api.py +++ b/src/fastcs/controller_api.py @@ -17,12 +17,12 @@ class ControllerAPI: """Attributes, bound methods and sub APIs of a `Controller`""" - path: list[str | int] = field(default_factory=list) + path: list[str] = field(default_factory=list) """Path within controller tree (empty if this is the root)""" attributes: dict[str, Attribute] = field(default_factory=dict) command_methods: dict[str, Command] = field(default_factory=dict) scan_methods: dict[str, Scan] = field(default_factory=dict) - sub_apis: dict[str | int, "ControllerAPI"] = field(default_factory=dict) + sub_apis: dict[str, "ControllerAPI"] = field(default_factory=dict) """APIs of the sub controllers of the `Controller` this API was built from""" description: str | None = None diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 429aae910..7a0d4f8fe 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -115,7 +115,7 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): child_pvi = f"{controller_pv_prefix(pv_prefix, child)}:PVI" child_name = ( f"__{child.path[-1]}" # Sub-Controller of ControllerVector - if isinstance(child.path[-1], int) + if child.path[-1].isdigit() else str(child.path[-1]) ) diff --git a/src/fastcs/transport/epics/gui.py b/src/fastcs/transport/epics/gui.py index 5c8542221..5f37e1c59 100644 --- a/src/fastcs/transport/epics/gui.py +++ b/src/fastcs/transport/epics/gui.py @@ -51,7 +51,7 @@ def __init__(self, controller_api: ControllerAPI, pv_prefix: str) -> None: self._controller_api = controller_api self._pv_prefix = pv_prefix - def _get_pv(self, attr_path: list[str | int], name: str): + def _get_pv(self, attr_path: list[str], name: str): attr_prefix = ":".join( [self._pv_prefix] + [snake_to_pascal(str(node)) for node in attr_path] ) @@ -88,7 +88,7 @@ def _get_write_widget(self, fastcs_datatype: DataType) -> WriteWidgetUnion | Non raise FastCSError(f"Unsupported type {type(datatype)}: {datatype}") def _get_attribute_component( - self, attr_path: list[str | int], name: str, attribute: Attribute + self, attr_path: list[str], name: str, attribute: Attribute ) -> SignalR | SignalW | SignalRW | None: pv = self._get_pv(attr_path, name) name = snake_to_pascal(name) @@ -129,7 +129,7 @@ def _get_attribute_component( case _: raise FastCSError(f"Unsupported attribute type: {type(attribute)}") - def _get_command_component(self, attr_path: list[str | int], name: str): + def _get_command_component(self, attr_path: list[str], name: str): pv = self._get_pv(attr_path, name) name = snake_to_pascal(name) @@ -160,7 +160,7 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: components: Tree = [] for name, api in controller_api.sub_apis.items(): - if isinstance(name, int): + if name.isdigit(): name = f"{controller_api.path[-1]}{name}" components.append( Group( @@ -218,7 +218,7 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: class PvaEpicsGUI(EpicsGUI): """For creating gui in the PVA EPICS transport.""" - def _get_pv(self, attr_path: list[str | int], name: str): + def _get_pv(self, attr_path: list[str], name: str): return f"pva://{super()._get_pv(attr_path, name)}" def _get_read_widget(self, fastcs_datatype: DataType) -> ReadWidgetUnion | None: diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index c0d67390b..f0f594423 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -97,7 +97,7 @@ def _make_p4p_raw_value(self) -> dict: # Add Controller entry if sub_controller_api: # Sub-device of a ControllerVector - if isinstance(sub_controller_api.path[-1], int): + if sub_controller_api.path[-1].isdigit(): p4p_raw_value[f"__{int(stripped_leaf)}"][signal_info.access] = ( signal_info.pv ) From ecaf5ec82621093e5e0da4b09ba6ddccc6bbc672 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 5 Nov 2025 15:13:32 +0000 Subject: [PATCH 17/29] tests: amend tests to use new vector children naming --- src/fastcs/transport/epics/pva/pvi_tree.py | 2 +- tests/transport/epics/ca/test_softioc_system.py | 6 +++--- tests/transport/epics/pva/test_p4p.py | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index f0f594423..6e66a6180 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -107,7 +107,7 @@ def _make_p4p_raw_value(self) -> dict: ] = signal_info.pv # Add attribute entry else: - attr_pvi_name = f"{stripped_leaf}" + attr_pvi_name = f"{_pascal_to_snake(stripped_leaf)}" p4p_raw_value[attr_pvi_name][signal_info.access] = signal_info.pv return p4p_raw_value diff --git a/tests/transport/epics/ca/test_softioc_system.py b/tests/transport/epics/ca/test_softioc_system.py index ce893b183..bda71d54d 100644 --- a/tests/transport/epics/ca/test_softioc_system.py +++ b/tests/transport/epics/ca/test_softioc_system.py @@ -30,11 +30,11 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): "description": "The records in this controller" } assert _child_vector_pvi["value"] == { - "childvector0": {"d": f"{pv_prefix}:ChildVector:0:PVI"}, - "childvector1": {"d": f"{pv_prefix}:ChildVector:1:PVI"}, + "__0": {"d": f"{pv_prefix}:ChildVector:0:PVI"}, + "__1": {"d": f"{pv_prefix}:ChildVector:1:PVI"}, } - child_pvi_pv = _child_vector_pvi["value"]["childvector0"]["d"] + child_pvi_pv = _child_vector_pvi["value"]["__0"]["d"] _child_pvi = ctxt.get(child_pvi_pv) assert isinstance(_child_pvi, Value) child_pvi = _child_pvi.todict() diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 10ecda2b0..0d85b116b 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -51,11 +51,11 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): assert _child_vector_pvi["display"] == {"description": "some child vector"} assert _child_vector_pvi["value"] == { "vector_attribute": {"r": f"{pv_prefix}:Child:VectorAttribute"}, - "v1": {"d": f"{pv_prefix}:Child:1:PVI"}, - "v2": {"d": f"{pv_prefix}:Child:2:PVI"}, + "__1": {"d": f"{pv_prefix}:Child:1:PVI"}, + "__2": {"d": f"{pv_prefix}:Child:2:PVI"}, } - child_pvi_pv = _child_vector_pvi["value"]["v1"]["d"] + child_pvi_pv = _child_vector_pvi["value"]["__1"]["d"] _child_pvi = await ctxt.get(child_pvi_pv) assert isinstance(_child_pvi, Value) child_pvi = _child_pvi.todict() @@ -367,9 +367,9 @@ class SomeController(Controller): "userTag": 0, }, "value": { - "v0": {"d": f"{pv_prefix}:Child:0:PVI"}, - "v1": {"d": f"{pv_prefix}:Child:1:PVI"}, - "v2": {"d": f"{pv_prefix}:Child:2:PVI"}, + "__0": {"d": f"{pv_prefix}:Child:0:PVI"}, + "__1": {"d": f"{pv_prefix}:Child:1:PVI"}, + "__2": {"d": f"{pv_prefix}:Child:2:PVI"}, }, } assert len(child_child_controller_pvi) == 1 From cad6c344925b8ed45c05025376f4a946e48a2f5f Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 5 Nov 2025 15:35:37 +0000 Subject: [PATCH 18/29] tests: add tests for restricted add_sub_controller method --- tests/test_controller.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_controller.py b/tests/test_controller.py index 0bb0df26d..0fbe96e05 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -1,7 +1,7 @@ import pytest from fastcs.attributes import AttrR -from fastcs.controller import Controller +from fastcs.controller import Controller, ControllerVector from fastcs.datatypes import Float, Int @@ -101,3 +101,18 @@ def __init__(self): ValueError, match=r"Cannot add attribute .* existing sub controller" ): controller.sub_controller = AttrR(Int()) # pyright: ignore[reportAttributeAccessIssue] + + +def test_controller_raises_error_if_passed_numeric_sub_controller_name(): + sub_controller = SomeSubController() + controller = SomeController(sub_controller) + + with pytest.raises(ValueError, match="Numeric-only names are not allowed"): + controller.add_sub_controller("30", sub_controller) + + +def test_controller_vector_raises_error_if_add_sub_controller_called(): + controller_vector = ControllerVector({i: SomeSubController() for i in range(2)}) + + with pytest.raises(NotImplementedError, match="Use __setitem__ instead"): + controller_vector.add_sub_controller("subcontroller", SomeSubController()) From c231cd9446ceeecb3f064a0b618d215660d76ea6 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 6 Nov 2025 14:10:34 +0000 Subject: [PATCH 19/29] chore: address review comments --- src/fastcs/attributes.py | 2 +- src/fastcs/controller.py | 35 +++++++++++----------- src/fastcs/controller_api.py | 2 +- src/fastcs/transport/epics/ca/ioc.py | 2 +- src/fastcs/transport/epics/gui.py | 2 +- src/fastcs/transport/epics/pva/pvi_tree.py | 1 - src/fastcs/transport/epics/util.py | 4 +-- src/fastcs/transport/graphql/graphql.py | 2 +- src/fastcs/transport/rest/rest.py | 4 +-- src/fastcs/transport/tango/dsr.py | 7 ++--- 10 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/fastcs/attributes.py b/src/fastcs/attributes.py index 4024484b4..331299577 100644 --- a/src/fastcs/attributes.py +++ b/src/fastcs/attributes.py @@ -109,7 +109,7 @@ def set_path(self, path: list[str]): def __repr__(self): name = self.__class__.__name__ - path = ".".join([str(node) for node in self._path] + [self._name]) or None + path = ".".join(self._path + [self._name]) or None datatype = self._datatype.__class__.__name__ return f"{name}(path={path}, datatype={datatype}, io_ref={self._io_ref})" diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 096bbd0a3..3586f5689 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -158,10 +158,10 @@ def add_sub_controller( sub_controller.set_path(self.path + [name]) self.__sub_controller_tree[name] = sub_controller - super().__setattr__(str(name), sub_controller) + super().__setattr__(name, sub_controller) if isinstance(sub_controller.root_attribute, Attribute): - self.attributes[str(name)] = sub_controller.root_attribute + self.attributes[name] = sub_controller.root_attribute @property def sub_controllers(self) -> dict[str, BaseController]: @@ -169,7 +169,7 @@ def sub_controllers(self) -> dict[str, BaseController]: def __repr__(self): name = self.__class__.__name__ - path = ".".join([str(p) for p in self.path]) or None + path = ".".join(self.path) or None sub_controllers = list(self.sub_controllers.keys()) or None return f"{name}(path={path}, sub_controllers={sub_controllers})" @@ -219,11 +219,8 @@ async def disconnect(self) -> None: class ControllerVector(MutableMapping[int, Controller], BaseController): - """A collection of SubControllers, with an arbitrary integer index. - An instance of this class can be registered with a parent ``Controller`` to include - it's children as part of a larger controller. Each child of the vector will keep - a string name of the vector. - """ + """A controller with a collection of identical sub controllers distinguished + by a numeric value""" root_attribute: Attribute | None = None @@ -239,14 +236,22 @@ def __init__( for index, child in children.items(): super().add_sub_controller(str(index), child) - def add_sub_controller(self, *args, **kwargs): + def add_sub_controller( + self, name: str, sub_controller: Controller | ControllerVector + ): raise NotImplementedError( "Cannot add named sub controller to ControllerVector. " - "Use __setitem__ instead, for indexed sub controllers" + "Use __setitem__ instead, for indexed sub controllers. " + "E.g., vector[1] = Controller()" ) def __getitem__(self, key: int) -> Controller: - return self._children[key] + try: + return self._children[key] + except KeyError as exception: + raise KeyError( + f"ControllerVector does not have Controller with key {key}" + ) from exception def __setitem__(self, key: int, value: Controller) -> None: if not isinstance(key, int): @@ -266,9 +271,5 @@ def __iter__(self) -> Iterator[int]: def __len__(self) -> int: return len(self._children) - def children(self) -> Iterator[tuple[str, Controller]]: - for key, child in self._children.items(): - yield str(key), child - - def __hash__(self): - return hash(id(self)) + def children(self) -> Iterator[tuple[int, Controller]]: + yield from self._children.items() diff --git a/src/fastcs/controller_api.py b/src/fastcs/controller_api.py index eecc9f5a7..7aa757a44 100644 --- a/src/fastcs/controller_api.py +++ b/src/fastcs/controller_api.py @@ -41,7 +41,7 @@ def __repr__(self): return ( f"ControllerAPI(" f"path={self.path}, " - f"sub_apis=[{', '.join(str(sub_api) for sub_api in self.sub_apis.keys())}]" + f"sub_apis=[{', '.join(sub_api for sub_api in self.sub_apis.keys())}]" f")" ) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 7a0d4f8fe..494484495 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -116,7 +116,7 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): child_name = ( f"__{child.path[-1]}" # Sub-Controller of ControllerVector if child.path[-1].isdigit() - else str(child.path[-1]) + else child.path[-1] ) _add_pvi_info(child_pvi, parent_pvi, child_name.lower()) diff --git a/src/fastcs/transport/epics/gui.py b/src/fastcs/transport/epics/gui.py index 5f37e1c59..a87893f39 100644 --- a/src/fastcs/transport/epics/gui.py +++ b/src/fastcs/transport/epics/gui.py @@ -53,7 +53,7 @@ def __init__(self, controller_api: ControllerAPI, pv_prefix: str) -> None: def _get_pv(self, attr_path: list[str], name: str): attr_prefix = ":".join( - [self._pv_prefix] + [snake_to_pascal(str(node)) for node in attr_path] + [self._pv_prefix] + [snake_to_pascal(node) for node in attr_path] ) return f"{attr_prefix}:{snake_to_pascal(name)}" diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py index 6e66a6180..0e2fe1728 100644 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ b/src/fastcs/transport/epics/pva/pvi_tree.py @@ -42,7 +42,6 @@ class PviDevice(dict[str, "PviDevice"]): """For creating a pvi structure in pva.""" pv_prefix: str - controller_api: ControllerAPI | None description: str | None device_signal_info: _PviSignalInfo | None diff --git a/src/fastcs/transport/epics/util.py b/src/fastcs/transport/epics/util.py index 5f37c8452..431afe4b8 100644 --- a/src/fastcs/transport/epics/util.py +++ b/src/fastcs/transport/epics/util.py @@ -3,6 +3,4 @@ def controller_pv_prefix(prefix: str, controller_api: ControllerAPI) -> str: - return ":".join( - [prefix] + [snake_to_pascal(str(node)) for node in controller_api.path] - ) + return ":".join([prefix] + [snake_to_pascal(node) for node in controller_api.path]) diff --git a/src/fastcs/transport/graphql/graphql.py b/src/fastcs/transport/graphql/graphql.py index ffa7b533f..3560f659e 100644 --- a/src/fastcs/transport/graphql/graphql.py +++ b/src/fastcs/transport/graphql/graphql.py @@ -86,7 +86,7 @@ def _process_commands(self, controller_api: ControllerAPI): def _process_sub_apis(self, root_controller_api: ControllerAPI): """Recursively add fields from the queries and mutations of sub apis""" for controller_api in root_controller_api.sub_apis.values(): - name = "".join([str(node) for node in controller_api.path]) + name = "".join(controller_api.path) child_tree = GraphQLAPI(controller_api) if child_tree.queries: self.queries.append( diff --git a/src/fastcs/transport/rest/rest.py b/src/fastcs/transport/rest/rest.py index 3247b1309..66f7d3dee 100644 --- a/src/fastcs/transport/rest/rest.py +++ b/src/fastcs/transport/rest/rest.py @@ -101,7 +101,7 @@ async def attr_get() -> Any: # Must be any as response_model is set def _add_attribute_api_routes(app: FastAPI, root_controller_api: ControllerAPI) -> None: for controller_api in root_controller_api.walk_api(): - path = [str(node) for node in controller_api.path] + path = controller_api.path for attr_name, attribute in controller_api.attributes.items(): attr_name = attr_name.replace("_", "-") @@ -151,7 +151,7 @@ async def command() -> None: def _add_command_api_routes(app: FastAPI, root_controller_api: ControllerAPI) -> None: for controller_api in root_controller_api.walk_api(): - path = [str(node) for node in controller_api.path] + path = controller_api.path for name, method in root_controller_api.command_methods.items(): cmd_name = name.replace("_", "-") diff --git a/src/fastcs/transport/tango/dsr.py b/src/fastcs/transport/tango/dsr.py index a5a41c01e..dc929a5f7 100644 --- a/src/fastcs/transport/tango/dsr.py +++ b/src/fastcs/transport/tango/dsr.py @@ -61,7 +61,7 @@ def _collect_dev_attributes( ) -> dict[str, Any]: collection: dict[str, Any] = {} for controller_api in root_controller_api.walk_api(): - path = [str(node) for node in controller_api.path] + path = controller_api.path for attr_name, attribute in controller_api.attributes.items(): attr_name = attr_name.title().replace("_", "") @@ -109,8 +109,7 @@ def _wrap_command_f( ) -> Callable[..., Awaitable[None]]: async def _dynamic_f(tango_device: Device) -> None: tango_device.info_stream( - f"called {'_'.join([str(node) for node in controller_api.path])} " - f"f method: {method_name}" + f"called {'_'.join(controller_api.path)} f method: {method_name}" ) coro = method() @@ -126,7 +125,7 @@ def _collect_dev_commands( ) -> dict[str, Any]: collection: dict[str, Any] = {} for controller_api in root_controller_api.walk_api(): - path = [str(node) for node in controller_api.path] + path = controller_api.path for name, method in controller_api.command_methods.items(): cmd_name = name.title().replace("_", "") From 62f3761cfe98145002c3b3c25027ae78014e21f4 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 12:16:17 +0000 Subject: [PATCH 20/29] chore: remove children() method from ControllerVector --- src/fastcs/controller.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 3586f5689..0065d3d5d 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -270,6 +270,3 @@ def __iter__(self) -> Iterator[int]: def __len__(self) -> int: return len(self._children) - - def children(self) -> Iterator[tuple[int, Controller]]: - yield from self._children.items() From c4431719b536f86959198b73c70623f32fc0693d Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 12:17:24 +0000 Subject: [PATCH 21/29] refactor: convert PviDevice and PviTree operations into flattened free functions --- src/fastcs/transport/epics/pva/ioc.py | 34 +--- src/fastcs/transport/epics/pva/pvi.py | 116 +++++++++++ src/fastcs/transport/epics/pva/pvi_tree.py | 222 --------------------- 3 files changed, 125 insertions(+), 247 deletions(-) create mode 100644 src/fastcs/transport/epics/pva/pvi.py delete mode 100644 src/fastcs/transport/epics/pva/pvi_tree.py diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index f626b5fc5..7950b6bc1 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -2,7 +2,7 @@ from p4p.server import Server, StaticProvider -from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controller_api import ControllerAPI from fastcs.transport.epics.util import controller_pv_prefix from fastcs.util import snake_to_pascal @@ -12,32 +12,20 @@ make_shared_read_pv, make_shared_write_pv, ) -from .pvi_tree import AccessModeType, PviTree - - -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)}") +from .pvi import add_pvi_info async def parse_attributes( root_pv_prefix: str, root_controller_api: ControllerAPI -) -> list[StaticProvider]: +) -> StaticProvider: """Parses `Attribute` s into p4p signals in handlers.""" - pvi_tree = PviTree(root_pv_prefix) provider = StaticProvider(root_pv_prefix) for controller_api in root_controller_api.walk_api(): pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) - - pvi_tree.add_sub_device(pv_prefix, controller_api.description, controller_api) + provider = add_pvi_info( + provider=provider, pv_prefix=pv_prefix, controller_api=controller_api + ) for attr_name, attribute in controller_api.attributes.items(): full_pv_name = f"{pv_prefix}:{snake_to_pascal(attr_name)}" @@ -47,23 +35,19 @@ async def parse_attributes( attribute_pv_rbv = make_shared_read_pv(attribute) provider.add(f"{full_pv_name}", attribute_pv) provider.add(f"{full_pv_name}_RBV", attribute_pv_rbv) - pvi_tree.add_signal(f"{full_pv_name}", "rw") case AttrR(): attribute_pv = make_shared_read_pv(attribute) provider.add(f"{full_pv_name}", attribute_pv) - pvi_tree.add_signal(f"{full_pv_name}", "r") case AttrW(): attribute_pv = make_shared_write_pv(attribute) provider.add(f"{full_pv_name}", attribute_pv) - pvi_tree.add_signal(f"{full_pv_name}", "w") for attr_name, method in controller_api.command_methods.items(): full_pv_name = f"{pv_prefix}:{snake_to_pascal(attr_name)}" command_pv = make_command_pv(method.fn) provider.add(f"{full_pv_name}", command_pv) - pvi_tree.add_signal(f"{full_pv_name}", "x") - return [provider, pvi_tree.make_provider()] + return provider class P4PIOC: @@ -74,8 +58,8 @@ def __init__(self, pv_prefix: str, controller_api: ControllerAPI): self.controller_api = controller_api async def run(self): - providers = await parse_attributes(self.pv_prefix, self.controller_api) + provider = await parse_attributes(self.pv_prefix, self.controller_api) endless_event = asyncio.Event() - with Server(providers): + with Server([provider]): await endless_event.wait() diff --git a/src/fastcs/transport/epics/pva/pvi.py b/src/fastcs/transport/epics/pva/pvi.py new file mode 100644 index 000000000..568f9c78e --- /dev/null +++ b/src/fastcs/transport/epics/pva/pvi.py @@ -0,0 +1,116 @@ +from collections import defaultdict +from typing import Literal + +from p4p import Type, Value +from p4p.nt.common import alarm, timeStamp +from p4p.server import StaticProvider +from p4p.server.asyncio import SharedPV + +from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW +from fastcs.controller_api import ControllerAPI +from fastcs.util import snake_to_pascal + +from .types import p4p_alarm_states, p4p_timestamp_now + +AccessModeType = Literal["r", "w", "rw", "d", "x"] + + +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, + controller_api: ControllerAPI, + description: str | None = None, +) -> StaticProvider: + provider.add( + f"{pv_prefix}:PVI", + SharedPV(initial=_make_p4p_value(pv_prefix, controller_api, description)), + ) + return provider + + +def _make_p4p_value( + pv_prefix: str, controller_api: ControllerAPI, description: str | None +) -> Value: + display = ( + {"display": {"description": description}} if description is not None else {} + ) # Defined here so the value can be (none) + + raw_value = _make_p4p_raw_value(pv_prefix, controller_api) + p4p_type = _make_type_for_raw_value(raw_value) + + try: + return Value( + p4p_type, + { + **p4p_alarm_states(), + **p4p_timestamp_now(), + **display, + "value": raw_value, + }, + ) + except KeyError as e: + raise ValueError(f"Failed to create p4p Value from {raw_value}") from e + + +def _make_p4p_raw_value(pv_prefix: str, controller_api: ControllerAPI) -> dict: + p4p_raw_value = defaultdict(dict) + # Sub-controller api returned if current item is a Controller + for pv_leaf, sub_controller_api in controller_api.sub_apis.items(): + # Add Controller entry + # Sub-device of a ControllerVector + controller_pvi_name = f"{snake_to_pascal(pv_leaf)}" + pv = f"{pv_prefix}:{controller_pvi_name}:PVI" + if sub_controller_api.path[-1].isdigit(): + p4p_raw_value[f"__{int(pv_leaf)}"]["d"] = pv + else: + p4p_raw_value[controller_pvi_name]["d"] = pv + for pv_leaf, attribute in controller_api.attributes.items(): + # Add attribute entry + attr_pvi_name = f"{snake_to_pascal(pv_leaf)}" + pv = f"{pv_prefix}:{attr_pvi_name}" + p4p_raw_value[attr_pvi_name][_attribute_to_access(attribute)] = pv + + return p4p_raw_value + + +def _make_type_for_raw_value(raw_value: dict) -> Type: + p4p_raw_type = [] + for pvi_group_name, access_to_field in raw_value.items(): + pvi_group_structure = [] + for access, field in access_to_field.items(): + if isinstance(field, str): + pvi_group_structure.append((access, "s")) + elif isinstance(field, dict): + pvi_group_structure.append( + ( + access, + ( + "S", + None, + [(v, "s") for v, _ in field.items()], + ), + ) + ) + + p4p_raw_type.append((pvi_group_name, ("S", "structure", pvi_group_structure))) + + return Type( + [ + ("alarm", alarm), + ("timeStamp", timeStamp), + ("display", ("S", None, [("description", "s")])), + ("value", ("S", "structure", p4p_raw_type)), + ] + ) diff --git a/src/fastcs/transport/epics/pva/pvi_tree.py b/src/fastcs/transport/epics/pva/pvi_tree.py deleted file mode 100644 index 0e2fe1728..000000000 --- a/src/fastcs/transport/epics/pva/pvi_tree.py +++ /dev/null @@ -1,222 +0,0 @@ -import re -from collections import defaultdict -from dataclasses import dataclass -from typing import Literal - -from p4p import Type, Value -from p4p.nt.common import alarm, timeStamp -from p4p.server import StaticProvider -from p4p.server.asyncio import SharedPV - -from fastcs.controller_api import ControllerAPI - -from .types import p4p_alarm_states, p4p_timestamp_now - -AccessModeType = Literal["r", "w", "rw", "d", "x"] - -PviName = str - - -@dataclass -class _PviSignalInfo: - """For storing a pv and it's access in pvi parsing.""" - - pv: str - access: AccessModeType - - -def _pascal_to_snake(name: str) -> str: - s1 = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", name) - return re.sub("([a-z0-9])([A-Z])", r"\1_\2", s1).lower() - - -def _pv_to_pvi_name(pv: str) -> tuple[str, int | None]: - leaf = pv.rsplit(":", maxsplit=1)[-1] - match = re.search(r"(\d+)$", leaf) - number = int(match.group(1)) if match else None - string_without_number = re.sub(r"\d+$", "", leaf) - return _pascal_to_snake(string_without_number), number - - -class PviDevice(dict[str, "PviDevice"]): - """For creating a pvi structure in pva.""" - - pv_prefix: str - description: str | None - device_signal_info: _PviSignalInfo | None - - def __init__( - self, - pv_prefix: str, - controller_api: ControllerAPI | None = None, - description: str | None = None, - device_signal_info: _PviSignalInfo | None = None, - ): - self.pv_prefix = pv_prefix - self.controller_api = controller_api - self.description = description - self.device_signal_info = device_signal_info - - def __missing__(self, key: str) -> "PviDevice": - new_device = PviDevice(pv_prefix=f"{self.pv_prefix}:{key}") - self[key] = new_device - return self[key] - - def get_recursively(self, *args: str) -> "PviDevice": - d = self - for arg in args: - d = d[arg] - return d - - def _get_signal_infos( - self, - ) -> dict[str, tuple[_PviSignalInfo, ControllerAPI | None]]: - device_signal_infos: dict[str, tuple[_PviSignalInfo, ControllerAPI | None]] = {} - - for sub_device_name, sub_device in self.items(): - if sub_device: - device_signal_infos[f"{sub_device_name}:PVI"] = ( - _PviSignalInfo(pv=f"{sub_device.pv_prefix}:PVI", access="d"), - sub_device.controller_api, - ) - if sub_device.device_signal_info: - device_signal_infos[sub_device_name] = ( - sub_device.device_signal_info, - sub_device.controller_api, - ) - - return device_signal_infos - - def _make_p4p_raw_value(self) -> dict: - p4p_raw_value = defaultdict(dict) - for pv_leaf, signal_info_and_api in self._get_signal_infos().items(): - # Sub-controller api returned if current item is a Controller - signal_info, sub_controller_api = signal_info_and_api - stripped_leaf = pv_leaf.removesuffix(":PVI") - # Add Controller entry - if sub_controller_api: - # Sub-device of a ControllerVector - if sub_controller_api.path[-1].isdigit(): - p4p_raw_value[f"__{int(stripped_leaf)}"][signal_info.access] = ( - signal_info.pv - ) - else: - p4p_raw_value[_pascal_to_snake(stripped_leaf)][ - signal_info.access - ] = signal_info.pv - # Add attribute entry - else: - attr_pvi_name = f"{_pascal_to_snake(stripped_leaf)}" - p4p_raw_value[attr_pvi_name][signal_info.access] = signal_info.pv - - return p4p_raw_value - - def _make_type_for_raw_value(self, raw_value: dict) -> Type: - p4p_raw_type = [] - for pvi_group_name, access_to_field in raw_value.items(): - pvi_group_structure = [] - for access, field in access_to_field.items(): - if isinstance(field, str): - pvi_group_structure.append((access, "s")) - elif isinstance(field, dict): - pvi_group_structure.append( - ( - access, - ( - "S", - None, - [(v, "s") for v, _ in field.items()], - ), - ) - ) - - p4p_raw_type.append( - (pvi_group_name, ("S", "structure", pvi_group_structure)) - ) - - return Type( - [ - ("alarm", alarm), - ("timeStamp", timeStamp), - ("display", ("S", None, [("description", "s")])), - ("value", ("S", "structure", p4p_raw_type)), - ] - ) - - def make_p4p_value(self) -> Value: - display = ( - {"display": {"description": self.description}} - if self.description is not None - else {} - ) # Defined here so the value can be (none) - - raw_value = self._make_p4p_raw_value() - p4p_type = self._make_type_for_raw_value(raw_value) - - try: - return Value( - p4p_type, - { - **p4p_alarm_states(), - **p4p_timestamp_now(), - **display, - "value": raw_value, - }, - ) - except KeyError as e: - raise ValueError(f"Failed to create p4p Value from {raw_value}") from e - - def make_provider( - self, - provider: StaticProvider | None = None, - ) -> StaticProvider: - if provider is None: - provider = StaticProvider("PVI") - - provider.add( - f"{self.pv_prefix}:PVI", - SharedPV(initial=self.make_p4p_value()), - ) - - for sub_device in self.values(): - if sub_device: - sub_device.make_provider(provider=provider) - - return provider - - -# TODO: This can be dramatically cleaned up after https://github.com/DiamondLightSource/FastCS/issues/122 -class PviTree: - """For storing pvi structures.""" - - def __init__(self, pv_prefix: str): - self._pvi_tree_root: PviDevice = PviDevice(pv_prefix) - - def add_sub_device( - self, device_pv: str, description: str | None, controller_api: ControllerAPI - ): - if ":" not in device_pv: - assert device_pv == self._pvi_tree_root.pv_prefix - self._pvi_tree_root.description = description - else: - self._pvi_tree_root.get_recursively( - *device_pv.split(":")[1:] # To remove the prefix - ).description = description - self._pvi_tree_root.get_recursively( - *device_pv.split(":")[1:] # To remove the prefix - ).controller_api = controller_api - - def add_signal( - self, - attribute_pv: str, - access: AccessModeType, - ): - leaf_device = self._pvi_tree_root.get_recursively(*attribute_pv.split(":")[1:]) - - if leaf_device.device_signal_info is not None: - raise ValueError(f"Tried to add the field '{attribute_pv}' twice.") - - leaf_device.device_signal_info = _PviSignalInfo(pv=attribute_pv, access=access) - - def make_provider(self) -> StaticProvider: - return self._pvi_tree_root.make_provider() From a7fd462c9bf28d976f1eb073ec185f90edcba77b Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 12:50:04 +0000 Subject: [PATCH 22/29] tests: amend cases and descriptin passing to fix tests --- src/fastcs/transport/epics/pva/ioc.py | 5 ++++- src/fastcs/transport/epics/pva/pvi.py | 13 +++++++------ tests/example_p4p_ioc.py | 2 +- tests/transport/epics/pva/test_p4p.py | 8 ++++---- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index 7950b6bc1..f865845bb 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -24,7 +24,10 @@ async def parse_attributes( for controller_api in root_controller_api.walk_api(): pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) provider = add_pvi_info( - provider=provider, pv_prefix=pv_prefix, controller_api=controller_api + provider=provider, + pv_prefix=pv_prefix, + controller_api=controller_api, + description=controller_api.description, ) for attr_name, attribute in controller_api.attributes.items(): diff --git a/src/fastcs/transport/epics/pva/pvi.py b/src/fastcs/transport/epics/pva/pvi.py index 568f9c78e..7b237c084 100644 --- a/src/fastcs/transport/epics/pva/pvi.py +++ b/src/fastcs/transport/epics/pva/pvi.py @@ -70,17 +70,18 @@ def _make_p4p_raw_value(pv_prefix: str, controller_api: ControllerAPI) -> dict: for pv_leaf, sub_controller_api in controller_api.sub_apis.items(): # Add Controller entry # Sub-device of a ControllerVector - controller_pvi_name = f"{snake_to_pascal(pv_leaf)}" - pv = f"{pv_prefix}:{controller_pvi_name}:PVI" + pv = f"{pv_prefix}:{snake_to_pascal(pv_leaf)}:PVI" if sub_controller_api.path[-1].isdigit(): p4p_raw_value[f"__{int(pv_leaf)}"]["d"] = pv else: - p4p_raw_value[controller_pvi_name]["d"] = pv + p4p_raw_value[pv_leaf]["d"] = pv for pv_leaf, attribute in controller_api.attributes.items(): # Add attribute entry - attr_pvi_name = f"{snake_to_pascal(pv_leaf)}" - pv = f"{pv_prefix}:{attr_pvi_name}" - p4p_raw_value[attr_pvi_name][_attribute_to_access(attribute)] = pv + pv = f"{pv_prefix}:{snake_to_pascal(pv_leaf)}" + p4p_raw_value[pv_leaf][_attribute_to_access(attribute)] = 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 return p4p_raw_value diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index 1145ac82f..878e955f9 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -114,7 +114,7 @@ def __init__(self, children, description=None): description="some child vector", ) - controller.add_sub_controller("Child", sub_controller) + controller.add_sub_controller("child", sub_controller) fastcs = FastCS(controller, [p4p_options]) fastcs.run() diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 0d85b116b..d26c6922f 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -277,7 +277,7 @@ class SomeController(Controller): sub_controller_vector = ControllerVector({i: ChildController() for i in range(3)}) - controller.add_sub_controller("Child", sub_controller_vector) + controller.add_sub_controller("child", sub_controller_vector) sub_controller = ChildController() controller.child0 = sub_controller @@ -347,10 +347,10 @@ class SomeController(Controller): "value": { "additional_child": {"d": f"{pv_prefix}:AdditionalChild:PVI"}, "another_child": {"d": f"{pv_prefix}:AnotherChild:PVI"}, - "another_attr0": {"rw": f"{pv_prefix}:AnotherAttr0"}, - "another_attr1000": {"rw": f"{pv_prefix}:AnotherAttr1000"}, + "another_attr_0": {"rw": f"{pv_prefix}:AnotherAttr0"}, + "another_attr_1000": {"rw": f"{pv_prefix}:AnotherAttr1000"}, "a_third_attr": {"w": f"{pv_prefix}:AThirdAttr"}, - "attr1": {"rw": f"{pv_prefix}:Attr1"}, + "attr_1": {"rw": f"{pv_prefix}:Attr1"}, "child": {"d": f"{pv_prefix}:Child:PVI"}, "child0": {"d": f"{pv_prefix}:Child0:PVI"}, "child1": {"d": f"{pv_prefix}:Child1:PVI"}, From a33962931f74d291eed5296b283f76289cccfba2 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 12:58:01 +0000 Subject: [PATCH 23/29] docs: add docstring to public function --- src/fastcs/transport/epics/pva/pvi.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fastcs/transport/epics/pva/pvi.py b/src/fastcs/transport/epics/pva/pvi.py index 7b237c084..72e555af2 100644 --- a/src/fastcs/transport/epics/pva/pvi.py +++ b/src/fastcs/transport/epics/pva/pvi.py @@ -33,6 +33,7 @@ def add_pvi_info( controller_api: ControllerAPI, description: str | None = None, ) -> StaticProvider: + """Add PVI information to given provider.""" provider.add( f"{pv_prefix}:PVI", SharedPV(initial=_make_p4p_value(pv_prefix, controller_api, description)), From edad64f7efcfcff8e4b47303b0dea926f2749738 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 14:27:41 +0000 Subject: [PATCH 24/29] refactor: add sub controller in ControllerVector setitem --- src/fastcs/controller.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 0065d3d5d..9edbd7f76 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -230,11 +230,10 @@ def __init__( description: str | None = None, ios: Sequence[AttributeIO[T, AttributeIORefT]] | None = None, ) -> None: - self._children: dict[int, Controller] = {} - self.update(children) super().__init__(description=description, ios=ios) + self._children: dict[int, Controller] = {} for index, child in children.items(): - super().add_sub_controller(str(index), child) + self[index] = child def add_sub_controller( self, name: str, sub_controller: Controller | ControllerVector @@ -261,9 +260,10 @@ def __setitem__(self, key: int, value: Controller) -> None: msg = f"Expected Controller, got {value}" raise TypeError(msg) self._children[key] = value + super().add_sub_controller(str(key), value) def __delitem__(self, key: int) -> None: - del self._children[key] + raise NotImplementedError("Cannot delete sub controller from ControllerVector.") def __iter__(self) -> Iterator[int]: yield from self._children From 249128ac7267c00cc6402fdf3ae1535121be1a55 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 14:28:03 +0000 Subject: [PATCH 25/29] tests: test setitem and delitem in ControllerVector --- tests/test_controller.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_controller.py b/tests/test_controller.py index 0fbe96e05..50d3e5c48 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -116,3 +116,19 @@ def test_controller_vector_raises_error_if_add_sub_controller_called(): with pytest.raises(NotImplementedError, match="Use __setitem__ instead"): controller_vector.add_sub_controller("subcontroller", SomeSubController()) + + +def test_controller_vector_indexing(): + controller = SomeSubController() + another_controller = SomeSubController() + controller_vector = ControllerVector({1: another_controller}) + controller_vector[10] = controller + assert controller_vector.sub_controllers["10"] == controller + assert controller_vector.sub_controllers["1"] == another_controller + + +def test_controller_vector_delitem_raises_exception(): + controller = SomeSubController() + controller_vector = ControllerVector({1: controller}) + with pytest.raises(NotImplementedError, match="Cannot delete"): + del controller_vector[1] From 06505fd5337b22f7005d40273a7bd0773f7958bc Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 14:41:07 +0000 Subject: [PATCH 26/29] tests: add tests for getitem and iter --- tests/test_controller.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_controller.py b/tests/test_controller.py index 50d3e5c48..4828c8ffb 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -124,7 +124,11 @@ def test_controller_vector_indexing(): controller_vector = ControllerVector({1: another_controller}) controller_vector[10] = controller assert controller_vector.sub_controllers["10"] == controller - assert controller_vector.sub_controllers["1"] == another_controller + assert controller_vector[1] == another_controller + assert len(controller_vector) == 2 + + with pytest.raises(KeyError): + _ = controller_vector[2] def test_controller_vector_delitem_raises_exception(): @@ -132,3 +136,11 @@ def test_controller_vector_delitem_raises_exception(): controller_vector = ControllerVector({1: controller}) with pytest.raises(NotImplementedError, match="Cannot delete"): del controller_vector[1] + + +def test_controller_vector_iter(): + sub_controllers = {1: SomeSubController(), 2: SomeSubController()} + controller_vector = ControllerVector(sub_controllers) + + for index, child in controller_vector.items(): + assert sub_controllers[index] == child From b0056c7d7848272bee473f345b9267de243c89ee Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 16:51:42 +0000 Subject: [PATCH 27/29] chore: address review comments --- src/fastcs/controller.py | 17 ++++------------- src/fastcs/controller_api.py | 2 +- src/fastcs/transport/epics/pva/pvi.py | 2 +- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/fastcs/controller.py b/src/fastcs/controller.py index 91ba401cf..8a71b49b0 100755 --- a/src/fastcs/controller.py +++ b/src/fastcs/controller.py @@ -17,6 +17,7 @@ class BaseController(Tracer): #: Attributes passed from the device at runtime. attributes: dict[str, Attribute] + root_attribute: Attribute | None = None description: str | None = None @@ -144,9 +145,7 @@ def add_attribute(self, name, attribute: Attribute): self.attributes[name] = attribute super().__setattr__(name, attribute) - def add_sub_controller( - self, name: str, sub_controller: Controller | ControllerVector - ): + def add_sub_controller(self, name: str, sub_controller: BaseController): if name in self.__sub_controller_tree.keys(): raise ValueError( f"Cannot add sub controller {sub_controller}. " @@ -196,8 +195,6 @@ class Controller(BaseController): such as generating a UI or creating parameters for a control system. """ - root_attribute: Attribute | None = None - def __init__( self, description: str | None = None, @@ -205,9 +202,7 @@ def __init__( ) -> None: super().__init__(description=description, ios=ios) - def add_sub_controller( - self, name: str, sub_controller: Controller | ControllerVector - ): + def add_sub_controller(self, name: str, sub_controller: BaseController): if name.isdigit(): raise ValueError( f"Cannot add sub controller {name}. " @@ -226,8 +221,6 @@ class ControllerVector(MutableMapping[int, Controller], BaseController): """A controller with a collection of identical sub controllers distinguished by a numeric value""" - root_attribute: Attribute | None = None - def __init__( self, children: Mapping[int, Controller], @@ -239,9 +232,7 @@ def __init__( for index, child in children.items(): self[index] = child - def add_sub_controller( - self, name: str, sub_controller: Controller | ControllerVector - ): + def add_sub_controller(self, name: str, sub_controller: BaseController): raise NotImplementedError( "Cannot add named sub controller to ControllerVector. " "Use __setitem__ instead, for indexed sub controllers. " diff --git a/src/fastcs/controller_api.py b/src/fastcs/controller_api.py index 7aa757a44..431105100 100644 --- a/src/fastcs/controller_api.py +++ b/src/fastcs/controller_api.py @@ -41,7 +41,7 @@ def __repr__(self): return ( f"ControllerAPI(" f"path={self.path}, " - f"sub_apis=[{', '.join(sub_api for sub_api in self.sub_apis.keys())}]" + f"sub_apis=[{', '.join(self.sub_apis.keys())}]" f")" ) diff --git a/src/fastcs/transport/epics/pva/pvi.py b/src/fastcs/transport/epics/pva/pvi.py index 72e555af2..d5defb767 100644 --- a/src/fastcs/transport/epics/pva/pvi.py +++ b/src/fastcs/transport/epics/pva/pvi.py @@ -70,9 +70,9 @@ def _make_p4p_raw_value(pv_prefix: str, controller_api: ControllerAPI) -> dict: # Sub-controller api returned if current item is a Controller for pv_leaf, sub_controller_api in controller_api.sub_apis.items(): # Add Controller entry - # Sub-device of a ControllerVector pv = f"{pv_prefix}:{snake_to_pascal(pv_leaf)}:PVI" if sub_controller_api.path[-1].isdigit(): + # Sub-device of a ControllerVector p4p_raw_value[f"__{int(pv_leaf)}"]["d"] = pv else: p4p_raw_value[pv_leaf]["d"] = pv From 8fb306558b4a59a8375b87b0e9d3b73b4d264fbe Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 16:56:48 +0000 Subject: [PATCH 28/29] chore: add todo for _attribute_to_access --- src/fastcs/transport/epics/pva/pvi.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fastcs/transport/epics/pva/pvi.py b/src/fastcs/transport/epics/pva/pvi.py index d5defb767..a3a7d4482 100644 --- a/src/fastcs/transport/epics/pva/pvi.py +++ b/src/fastcs/transport/epics/pva/pvi.py @@ -15,6 +15,7 @@ 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(): From 0245dcaedf409c811dd345357cbe5dffd575e413 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 10 Nov 2025 11:02:41 +0000 Subject: [PATCH 29/29] refactor: amend add_pvi_info to add to provider in place --- src/fastcs/transport/epics/pva/ioc.py | 2 +- src/fastcs/transport/epics/pva/pvi.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index f865845bb..7abb07ae9 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -23,7 +23,7 @@ async def parse_attributes( for controller_api in root_controller_api.walk_api(): pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) - provider = add_pvi_info( + add_pvi_info( provider=provider, pv_prefix=pv_prefix, controller_api=controller_api, diff --git a/src/fastcs/transport/epics/pva/pvi.py b/src/fastcs/transport/epics/pva/pvi.py index a3a7d4482..bb11defb9 100644 --- a/src/fastcs/transport/epics/pva/pvi.py +++ b/src/fastcs/transport/epics/pva/pvi.py @@ -33,13 +33,12 @@ def add_pvi_info( pv_prefix: str, controller_api: ControllerAPI, description: str | None = None, -) -> StaticProvider: +) -> None: """Add PVI information to given provider.""" provider.add( f"{pv_prefix}:PVI", SharedPV(initial=_make_p4p_value(pv_prefix, controller_api, description)), ) - return provider def _make_p4p_value(