From a426cb44ee2cc2ebe2e7c81bbcf569eb148c6bff Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Wed, 20 May 2026 13:33:34 +0000 Subject: [PATCH 01/11] refactor(controllers): constructor-pass path, drop set_path() Closes #373. `Controller.path` is now a keyword-only ``__init__`` parameter on ``BaseController`` (default ``[]``) and is threaded through user code rather than seeded post-construction. The launcher builds each root with ``cls(options, path=[entry.id])`` (or ``cls(path=[entry.id])`` when no options); a parent constructs its subs with the full path already baked in (e.g. ``Sub(path=self.path + [name])``). ``BaseController.set_path()`` is removed entirely. ``add_sub_controller(name, sub)`` no longer mutates ``sub._path`` -- it sanity-asserts ``sub.path == parent.path + [name]`` and rejects the call with a clear error if the caller forgot to thread the path. ``_build_api()`` reads ``self._path`` directly, eliminating the parent-side path argument and the recursive re-prefix it required. Custom ``Controller.__init__`` (root and sub-controller alike) must now accept ``path`` and forward it to ``super().__init__``. Demo controllers and doc snippets are updated to the new shape; tests either construct with ``path=[...]`` from the start or move the id declaration above the controller construction so sub paths can be threaded explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/snippets/dynamic.py | 16 ++--- docs/snippets/static04.py | 3 +- docs/snippets/static05.py | 3 +- docs/snippets/static06.py | 7 +-- docs/snippets/static07.py | 7 +-- docs/snippets/static08.py | 9 +-- docs/snippets/static09.py | 9 +-- docs/snippets/static10.py | 22 ++++--- docs/snippets/static11.py | 22 ++++--- docs/snippets/static12.py | 22 ++++--- docs/snippets/static13.py | 22 ++++--- docs/snippets/static14.py | 22 ++++--- docs/snippets/static15.py | 22 ++++--- src/fastcs/controllers/base_controller.py | 27 ++++---- src/fastcs/controllers/controller.py | 6 +- src/fastcs/controllers/controller_vector.py | 4 +- src/fastcs/demo/controllers.py | 29 +++++++-- src/fastcs/launch.py | 15 ++--- tests/assertable_controller.py | 16 ++--- tests/benchmarking/controller.py | 3 +- tests/conftest.py | 2 +- tests/example_p4p_ioc.py | 25 +++++--- tests/example_softioc.py | 9 ++- tests/test_controllers.py | 59 +++++++++++------- tests/test_launch.py | 29 ++++----- tests/test_multi_controller.py | 18 +++--- tests/transports/epics/ca/test_gui.py | 10 +-- .../transports/epics/ca/test_initial_value.py | 3 +- tests/transports/epics/ca/test_softioc.py | 4 +- tests/transports/epics/pva/test_p4p.py | 62 ++++++++----------- tests/transports/epics/test_emission.py | 3 +- tests/transports/graphQL/test_graphql.py | 2 +- tests/transports/tango/test_dsr.py | 2 +- 33 files changed, 289 insertions(+), 225 deletions(-) diff --git a/docs/snippets/dynamic.py b/docs/snippets/dynamic.py index 7dde6dfe..f6e6b997 100644 --- a/docs/snippets/dynamic.py +++ b/docs/snippets/dynamic.py @@ -98,9 +98,11 @@ def __init__( index: int, parameters: dict[str, TemperatureControllerParameter], io: TemperatureControllerAttributeIO, + *, + path=None, ): self._parameters = parameters - super().__init__(f"Ramp{index}", ios=[io]) + super().__init__(f"Ramp{index}", ios=[io], path=path) async def initialise(self): for name, attribute in create_attributes(self._parameters).items(): @@ -108,12 +110,12 @@ async def initialise(self): class TemperatureController(Controller): - def __init__(self, settings: IPConnectionSettings): + def __init__(self, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() self._io = TemperatureControllerAttributeIO(self._connection) - super().__init__(ios=[self._io]) + super().__init__(ios=[self._io], path=path) async def connect(self): await self._connection.connect(self._ip_settings) @@ -129,19 +131,19 @@ async def initialise(self): self.add_attribute(name, attribute) for idx, ramp_parameters in enumerate(ramps_api): + name = f"Ramp{idx + 1:02d}" ramp_controller = TemperatureRampController( - idx + 1, ramp_parameters, self._io + idx + 1, ramp_parameters, self._io, path=self.path + [name] ) await ramp_controller.initialise() - self.add_sub_controller(f"Ramp{idx + 1:02d}", ramp_controller) + self.add_sub_controller(name, ramp_controller) await self._connection.close() epics_ca = EpicsCATransport() connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) diff --git a/docs/snippets/static04.py b/docs/snippets/static04.py index 345794ea..d30d055b 100644 --- a/docs/snippets/static04.py +++ b/docs/snippets/static04.py @@ -10,8 +10,7 @@ class TemperatureController(Controller): epics_ca = EpicsCATransport() -controller = TemperatureController() -controller.set_path(["DEMO"]) +controller = TemperatureController(path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static05.py b/docs/snippets/static05.py index 0d3b610a..df6ccc4f 100644 --- a/docs/snippets/static05.py +++ b/docs/snippets/static05.py @@ -14,8 +14,7 @@ class TemperatureController(Controller): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) -controller = TemperatureController() -controller.set_path(["DEMO"]) +controller = TemperatureController(path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static06.py b/docs/snippets/static06.py index 269e3309..9696e98b 100644 --- a/docs/snippets/static06.py +++ b/docs/snippets/static06.py @@ -12,8 +12,8 @@ class TemperatureController(Controller): device_id = AttrR(String()) - def __init__(self, settings: IPConnectionSettings): - super().__init__() + def __init__(self, settings: IPConnectionSettings, *, path=None): + super().__init__(path=path) self._ip_settings = settings self._connection = IPConnection() @@ -25,8 +25,7 @@ async def connect(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) diff --git a/docs/snippets/static07.py b/docs/snippets/static07.py index cac5549d..9478c50d 100644 --- a/docs/snippets/static07.py +++ b/docs/snippets/static07.py @@ -34,11 +34,11 @@ async def update(self, attr: AttrR[NumberT, IDAttributeIORef]): class TemperatureController(Controller): device_id = AttrR(String(), io_ref=IDAttributeIORef()) - def __init__(self, settings: IPConnectionSettings): + def __init__(self, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[IDAttributeIO(self._connection)]) + super().__init__(ios=[IDAttributeIO(self._connection)], path=path) async def connect(self): await self._connection.connect(self._ip_settings) @@ -47,8 +47,7 @@ async def connect(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static08.py b/docs/snippets/static08.py index 5382fa3c..6370b29a 100644 --- a/docs/snippets/static08.py +++ b/docs/snippets/static08.py @@ -40,11 +40,13 @@ class TemperatureController(Controller): device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - def __init__(self, settings: IPConnectionSettings): + def __init__(self, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) async def connect(self): await self._connection.connect(self._ip_settings) @@ -53,8 +55,7 @@ async def connect(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static09.py b/docs/snippets/static09.py index dc5bb9d5..de039920 100644 --- a/docs/snippets/static09.py +++ b/docs/snippets/static09.py @@ -47,11 +47,13 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, settings: IPConnectionSettings): + def __init__(self, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) async def connect(self): await self._connection.connect(self._ip_settings) @@ -60,8 +62,7 @@ async def connect(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static10.py b/docs/snippets/static10.py index 24f6d523..5b0c5e0d 100644 --- a/docs/snippets/static10.py +++ b/docs/snippets/static10.py @@ -47,10 +47,12 @@ class TemperatureRampController(Controller): start = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="S")) end = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="E")) - def __init__(self, index: int, connection: IPConnection) -> None: + def __init__(self, index: int, connection: IPConnection, *, path=None) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(connection, suffix)], + path=path, ) @@ -59,17 +61,22 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, ramp_count: int, settings: IPConnectionSettings): + def __init__(self, ramp_count: int, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self._connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) async def connect(self): await self._connection.connect(self._ip_settings) @@ -78,8 +85,7 @@ async def connect(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(4, connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static11.py b/docs/snippets/static11.py index e07ed5a4..026dfab4 100644 --- a/docs/snippets/static11.py +++ b/docs/snippets/static11.py @@ -54,10 +54,12 @@ class TemperatureRampController(Controller): end = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="E")) enabled = AttrRW(Enum(OnOffEnum), io_ref=TemperatureControllerAttributeIORef("N")) - def __init__(self, index: int, connection: IPConnection) -> None: + def __init__(self, index: int, connection: IPConnection, *, path=None) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(connection, suffix)], + path=path, ) @@ -66,17 +68,22 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, ramp_count: int, settings: IPConnectionSettings): + def __init__(self, ramp_count: int, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self._connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) async def connect(self): await self._connection.connect(self._ip_settings) @@ -85,8 +92,7 @@ async def connect(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(4, connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static12.py b/docs/snippets/static12.py index 9f1f2af1..3f1a9abf 100644 --- a/docs/snippets/static12.py +++ b/docs/snippets/static12.py @@ -59,10 +59,12 @@ class TemperatureRampController(Controller): actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: + def __init__(self, index: int, connection: IPConnection, *, path=None) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(connection, suffix)], + path=path, ) @@ -71,17 +73,22 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, ramp_count: int, settings: IPConnectionSettings): + def __init__(self, ramp_count: int, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self._connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) async def connect(self): await self._connection.connect(self._ip_settings) @@ -98,8 +105,7 @@ async def update_voltages(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(4, connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static13.py b/docs/snippets/static13.py index b2036c66..a5347175 100644 --- a/docs/snippets/static13.py +++ b/docs/snippets/static13.py @@ -60,10 +60,12 @@ class TemperatureRampController(Controller): actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: + def __init__(self, index: int, connection: IPConnection, *, path=None) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(connection, suffix)], + path=path, ) @@ -72,17 +74,22 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, ramp_count: int, settings: IPConnectionSettings): + def __init__(self, ramp_count: int, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self._connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) async def connect(self): await self._connection.connect(self._ip_settings) @@ -106,8 +113,7 @@ async def disable_all(self) -> None: gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(4, connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static14.py b/docs/snippets/static14.py index f54c93d3..a4a39b2c 100644 --- a/docs/snippets/static14.py +++ b/docs/snippets/static14.py @@ -63,10 +63,12 @@ class TemperatureRampController(Controller): actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: + def __init__(self, index: int, connection: IPConnection, *, path=None) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(connection, suffix)], + path=path, ) @@ -75,17 +77,22 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, ramp_count: int, settings: IPConnectionSettings): + def __init__(self, ramp_count: int, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self._connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) async def connect(self): await self._connection.connect(self._ip_settings) @@ -113,8 +120,7 @@ async def disable_all(self) -> None: epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) -controller = TemperatureController(4, connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(4, connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static15.py b/docs/snippets/static15.py index aa2d53a9..0ebacc3d 100644 --- a/docs/snippets/static15.py +++ b/docs/snippets/static15.py @@ -66,10 +66,12 @@ class TemperatureRampController(Controller): actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: + def __init__(self, index: int, connection: IPConnection, *, path=None) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(connection, suffix)], + path=path, ) @@ -78,17 +80,22 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, ramp_count: int, settings: IPConnectionSettings): + def __init__(self, ramp_count: int, settings: IPConnectionSettings, *, path=None): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__( + ios=[TemperatureControllerAttributeIO(self._connection)], path=path + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self._connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) async def connect(self): await self._connection.connect(self._ip_settings) @@ -116,8 +123,7 @@ async def disable_all(self) -> None: epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) -controller = TemperatureController(4, connection_settings) -controller.set_path(["DEMO"]) +controller = TemperatureController(4, connection_settings, path=["DEMO"]) fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index 8b29ed6d..d4fdbead 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -39,9 +39,10 @@ class BaseController(Tracer): def __init__( self, - path: list[str] | None = None, description: str | None = None, ios: Sequence[AnyAttributeIO] | None = None, + *, + path: list[str] | None = None, ) -> None: super().__init__() @@ -49,7 +50,7 @@ def __init__( # Use the argument over the one class defined description. self.description = description - self._path: list[str] = path or [] + self._path: list[str] = list(path) if path else [] # Internal state that should not be accessed directly by base classes self.__attributes: dict[str, Attribute] = {} @@ -287,14 +288,6 @@ def path(self) -> list[str]: """Path prefix of attributes, recursively including parent Controllers.""" return self._path - def set_path(self, path: list[str]): - if self._path: - raise ValueError(f"sub controller is already registered under {self.path}") - - self._path = path - for attribute in self.__attributes.values(): - attribute.set_path(path) - def _check_for_name_clash(self, name: str): namespaces = { "attribute": self.__attributes, @@ -356,7 +349,13 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): f"'{sub_controller.__class__.__name__}'." ) - sub_controller.set_path(self.path + [name]) + expected = self.path + [name] + if sub_controller.path != expected: + raise ValueError( + f"Sub controller path {sub_controller.path} does not match " + f"parent path {self.path} + [{name!r}]. Construct the sub " + f"controller with `path={expected!r}`." + ) self.__sub_controllers[name] = sub_controller super().__setattr__(name, sub_controller) @@ -406,14 +405,14 @@ def add_scan(self, name: str, scan: Scan): def scan_methods(self) -> dict[str, Scan]: return self.__scan_methods - def _build_api(self, path: list[str]) -> ControllerAPI: + def _build_api(self) -> ControllerAPI: return ControllerAPI( - path=path, + path=self._path, attributes=self.attributes, command_methods=self.command_methods, scan_methods=self.scan_methods, sub_apis={ - name: sub_controller._build_api(path + [name]) # noqa: SLF001 + name: sub_controller._build_api() # noqa: SLF001 for name, sub_controller in self.sub_controllers.items() }, description=self.description, diff --git a/src/fastcs/controllers/controller.py b/src/fastcs/controllers/controller.py index a6c72602..c95f3fea 100755 --- a/src/fastcs/controllers/controller.py +++ b/src/fastcs/controllers/controller.py @@ -19,8 +19,10 @@ def __init__( self, description: str | None = None, ios: Sequence[AnyAttributeIO] | None = None, + *, + path: list[str] | None = None, ) -> None: - super().__init__(description=description, ios=ios) + super().__init__(description=description, ios=ios, path=path) self._connected = False def add_sub_controller(self, name: str, sub_controller: BaseController): @@ -70,7 +72,7 @@ def create_api_and_tasks( tuple[ControllerAPI, list[ScanCallback], list[ScanCallback]] """ - controller_api = self._build_api(self._path) + controller_api = self._build_api() scan_dict: dict[float, list[ScanCallback]] = defaultdict(list) initial_coros: list[ScanCallback] = [] diff --git a/src/fastcs/controllers/controller_vector.py b/src/fastcs/controllers/controller_vector.py index 11925827..f4e70844 100755 --- a/src/fastcs/controllers/controller_vector.py +++ b/src/fastcs/controllers/controller_vector.py @@ -19,8 +19,10 @@ def __init__( children: Mapping[int, Controller_T], description: str | None = None, ios: Sequence[AnyAttributeIO] | None = None, + *, + path: list[str] | None = None, ) -> None: - super().__init__(description=description, ios=ios) + super().__init__(description=description, ios=ios, path=path) self._children: dict[int, Controller_T] = {} for index, child in children.items(): self[index] = child diff --git a/src/fastcs/demo/controllers.py b/src/fastcs/demo/controllers.py index 5926fc8c..08c5c433 100755 --- a/src/fastcs/demo/controllers.py +++ b/src/fastcs/demo/controllers.py @@ -71,20 +71,29 @@ class TemperatureController(Controller): power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef(name="P")) voltages = AttrR(Waveform(np.int32, shape=(4,))) - def __init__(self, settings: TemperatureControllerSettings) -> None: + def __init__( + self, + settings: TemperatureControllerSettings, + *, + path: list[str] | None = None, + ) -> None: self.connection = IPConnection() self.suffix = "" super().__init__( - ios=[TemperatureControllerAttributeIO(self.connection, self.suffix)] + ios=[TemperatureControllerAttributeIO(self.connection, self.suffix)], + path=path, ) self._settings = settings self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, settings.num_ramp_controllers + 1): - controller = TemperatureRampController(index, self.connection) + name = f"R{index}" + controller = TemperatureRampController( + index, self.connection, path=self.path + [name] + ) self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.add_sub_controller(name, controller) @command() async def cancel_all(self) -> None: @@ -138,9 +147,17 @@ class TemperatureRampController(Controller): actual = AttrR(Float(prec=3), io_ref=TemperatureControllerAttributeIORef(name="A")) voltage = AttrR(Float(prec=3)) - def __init__(self, index: int, conn: IPConnection) -> None: + def __init__( + self, + index: int, + conn: IPConnection, + *, + path: list[str] | None = None, + ) -> None: suffix = f"{index:02d}" super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(conn, suffix)] + f"Ramp{suffix}", + ios=[TemperatureControllerAttributeIO(conn, suffix)], + path=path, ) self.connection = conn diff --git a/src/fastcs/launch.py b/src/fastcs/launch.py index 7bbe7726..baca0c61 100644 --- a/src/fastcs/launch.py +++ b/src/fastcs/launch.py @@ -198,15 +198,15 @@ def run( def _instantiate_controllers( controllers_options: list[Any], ) -> list[Controller]: - """Instantiate each entry under `controllers:` and seed its path. + """Instantiate each entry under `controllers:` with its path seeded. Each item in ``controllers_options`` is a dynamically-built Pydantic model that exposes ``id``, ``type`` and the controller's options fields inlined as siblings. The originating Controller class and its options-type are looked up in ``_ENTRY_REGISTRY`` (populated by - ``_build_entry_model``). The entry's ``id`` is seeded into the - controller's ``_path`` via ``set_path([id])`` so that - ``ControllerAPI.path`` is rooted at the YAML id. + ``_build_entry_model``). The entry's ``id`` is passed as ``path=[id]`` + to the controller constructor so that ``ControllerAPI.path`` is rooted + at the YAML id. """ seen_ids: set[str] = set() duplicates: list[str] = [] @@ -229,10 +229,11 @@ def _instantiate_controllers( for name in entry_cls.model_fields if name not in ("id", "type") } - controller = registered.cls(registered.options_type(**field_values)) + controller = registered.cls( + registered.options_type(**field_values), path=[entry.id] + ) else: - controller = registered.cls() - controller.set_path([entry.id]) + controller = registered.cls(path=[entry.id]) controllers.append(controller) return controllers diff --git a/tests/assertable_controller.py b/tests/assertable_controller.py index c5791613..3e61432d 100644 --- a/tests/assertable_controller.py +++ b/tests/assertable_controller.py @@ -32,19 +32,20 @@ async def send(self, attr: AttrW[DType_T, MyTestAttributeIORef], value: DType_T) class TestSubController(Controller): read_int: AttrR = AttrR(Int(), io_ref=MyTestAttributeIORef()) - def __init__(self) -> None: - super().__init__(ios=[test_attribute_io]) + def __init__(self, *, path: list[str] | None = None) -> None: + super().__init__(ios=[test_attribute_io], path=path) class MyTestController(Controller): - def __init__(self) -> None: - super().__init__(ios=[test_attribute_io]) + def __init__(self, *, path: list[str] | None = None) -> None: + super().__init__(ios=[test_attribute_io], path=path) self._sub_controllers: list[TestSubController] = [] for index in range(1, 3): - controller = TestSubController() + name = f"SubController{index:02d}" + controller = TestSubController(path=self.path + [name]) self._sub_controllers.append(controller) - self.add_sub_controller(f"SubController{index:02d}", controller) + self.add_sub_controller(name, controller) initialised = False count = 0 @@ -73,7 +74,6 @@ def __init__( self, controller: Controller, mocker: MockerFixture, - path: list[str] | None = None, ) -> None: super().__init__() @@ -81,7 +81,7 @@ def __init__( self.command_method_spys: dict[str, MockType] = {} # Build a ControllerAPI from the given Controller - controller_api = controller._build_api(path or []) + controller_api = controller._build_api() # Copy its fields self.path = controller_api.path self.attributes = controller_api.attributes diff --git a/tests/benchmarking/controller.py b/tests/benchmarking/controller.py index fc2d187e..94f0b60f 100644 --- a/tests/benchmarking/controller.py +++ b/tests/benchmarking/controller.py @@ -21,8 +21,7 @@ def run(): EpicsCATransport(), TangoTransport(), ] - controller = MyTestController() - controller.set_path(["BENCHMARK-DEVICE"]) + controller = MyTestController(path=["BENCHMARK-DEVICE"]) instance = FastCS(controller, transport_options, asyncio.get_event_loop()) instance.run() diff --git a/tests/conftest.py b/tests/conftest.py index 818c7d17..ebd081c7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,7 +57,7 @@ def controller(): @pytest.fixture def controller_api(controller): - return controller._build_api([]) + return controller._build_api() DATA_PATH = Path(__file__).parent / "data" diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index 95cc8e70..bf1494d7 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -43,16 +43,16 @@ class ParentController(Controller): io_ref=SimpleAttributeIORef(), ) - def __init__(self, description=None, ios=None): - super().__init__(description, ios) + def __init__(self, description=None, ios=None, *, path=None): + super().__init__(description, ios, path=path) class ChildController(Controller): fail_on_next_e = True c: AttrW = AttrW(Int(), io_ref=SimpleAttributeIORef()) - def __init__(self, description=None, ios=None): - super().__init__(description, ios) + def __init__(self, description=None, ios=None, *, path=None): + super().__init__(description, ios, path=path) @command() async def d(self): @@ -89,25 +89,30 @@ async def i(self): def run(id="P4P_TEST_DEVICE"): simple_attribute_io = SimpleAttributeIO() p4p_options = EpicsPVATransport() - controller = ParentController(ios=[simple_attribute_io]) - controller.set_path([id]) + controller = ParentController(ios=[simple_attribute_io], path=[id]) class ChildVector(ControllerVector): vector_attribute: AttrR = AttrR(Int()) - def __init__(self, children, description=None): - super().__init__(children, description) + def __init__(self, children, description=None, *, path=None): + super().__init__(children, description, path=path) + child_path = [id, "child"] sub_controller = ChildVector( { 1: ChildController( - description="some sub controller", ios=[simple_attribute_io] + description="some sub controller", + ios=[simple_attribute_io], + path=child_path + ["1"], ), 2: ChildController( - description="another sub controller", ios=[simple_attribute_io] + description="another sub controller", + ios=[simple_attribute_io], + path=child_path + ["2"], ), }, description="some child vector", + path=child_path, ) controller.add_sub_controller("child", sub_controller) diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 5d00ef4d..7749af72 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -22,9 +22,12 @@ async def d(self): def run(id="SOFTIOC_TEST_DEVICE"): - controller = ParentController() - controller.set_path([id]) - vector = ControllerVector({i: ChildController() for i in range(2)}) + controller = ParentController(path=[id]) + vector_path = [id, "ChildVector"] + vector = ControllerVector( + {i: ChildController(path=vector_path + [str(i)]) for i in range(2)}, + path=vector_path, + ) controller.add_sub_controller("ChildVector", vector) gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Vector") fastcs = FastCS( diff --git a/tests/test_controllers.py b/tests/test_controllers.py index 5d5dfb4e..288ab773 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -11,8 +11,8 @@ def test_controller_nesting(): controller = Controller() - sub_controller = Controller() - sub_sub_controller = Controller() + sub_controller = Controller(path=["a"]) + sub_sub_controller = Controller(path=["a", "b"]) controller.a = sub_controller sub_controller.b = sub_sub_controller @@ -23,15 +23,17 @@ def test_controller_nesting(): assert sub_controller.sub_controllers == {"b": sub_sub_controller} with pytest.raises(ValueError, match=r"Cannot add sub controller"): - controller.a = Controller() + controller.a = Controller(path=["a"]) - with pytest.raises(ValueError, match=r"already registered"): + # sub_controller has path=["a"], so adding it under name "c" violates the + # parent.path + [name] invariant. + with pytest.raises(ValueError, match=r"does not match parent path"): controller.c = sub_controller class SomeSubController(Controller): - def __init__(self): - super().__init__() + def __init__(self, *, path=None): + super().__init__(path=path) sub_attribute = AttrR(Int()) @@ -43,8 +45,8 @@ class SomeController(Controller): equal_attr = AttrR(Int()) annotated_and_equal_attr: AttrR[int] = AttrR(Int()) - def __init__(self, sub_controller: Controller): - super().__init__() + def __init__(self, sub_controller: Controller, *, path=None): + super().__init__(path=path) self.attr_on_object = AttrR(Int()) @@ -55,7 +57,7 @@ def __init__(self, sub_controller: Controller): def test_attribute_parsing(): - sub_controller = SomeSubController() + sub_controller = SomeSubController(path=["sub_controller"]) controller = SomeController(sub_controller) assert set(controller.attributes.keys()) == { @@ -86,13 +88,17 @@ async def noop() -> None: "member_name, member_value, expected_error", [ ("attr", AttrR(Float()), r"Cannot add attribute"), - ("attr", Controller(), r"Cannot add sub controller"), + ("attr", Controller(path=["attr"]), r"Cannot add sub controller"), ("attr", Command(noop), r"Cannot add command"), ("sub_controller", AttrR(Int()), r"Cannot add attribute"), - ("sub_controller", Controller(), r"Cannot add sub controller"), + ( + "sub_controller", + Controller(path=["sub_controller"]), + r"Cannot add sub controller", + ), ("sub_controller", Command(noop), r"Cannot add command"), ("cmd", AttrR(Int()), r"Cannot add attribute"), - ("cmd", Controller(), r"Cannot add sub controller"), + ("cmd", Controller(path=["cmd"]), r"Cannot add sub controller"), ("cmd", Command(noop), r"Cannot add command"), ], ) @@ -105,7 +111,7 @@ class ConflictingController(Controller): def __init__(self): super().__init__() - self.sub_controller = Controller() + self.sub_controller = Controller(path=["sub_controller"]) controller = ConflictingController() @@ -114,7 +120,7 @@ def __init__(self): def test_controller_raises_error_if_passed_numeric_sub_controller_name(): - sub_controller = SomeSubController() + sub_controller = SomeSubController(path=["sub_controller"]) controller = SomeController(sub_controller) with pytest.raises(ValueError, match="Numeric-only names are not allowed"): @@ -122,15 +128,19 @@ def test_controller_raises_error_if_passed_numeric_sub_controller_name(): def test_controller_vector_raises_error_if_add_sub_controller_called(): - controller_vector = ControllerVector({i: SomeSubController() for i in range(2)}) + controller_vector = ControllerVector( + {i: SomeSubController(path=[str(i)]) for i in range(2)} + ) with pytest.raises(NotImplementedError, match="Use __setitem__ instead"): - controller_vector.add_sub_controller("subcontroller", SomeSubController()) + controller_vector.add_sub_controller( + "subcontroller", SomeSubController(path=["subcontroller"]) + ) def test_controller_vector_indexing(): - controller = SomeSubController() - another_controller = SomeSubController() + controller = SomeSubController(path=["10"]) + another_controller = SomeSubController(path=["1"]) controller_vector = ControllerVector({1: another_controller}) controller_vector[10] = controller assert controller_vector.sub_controllers["10"] == controller @@ -142,14 +152,17 @@ def test_controller_vector_indexing(): def test_controller_vector_delitem_raises_exception(): - controller = SomeSubController() + controller = SomeSubController(path=["1"]) 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()} + sub_controllers = { + 1: SomeSubController(path=["1"]), + 2: SomeSubController(path=["2"]), + } controller_vector = ControllerVector(sub_controllers) for index, child in controller_vector.items(): @@ -207,9 +220,9 @@ class HintedController(Controller): controller._validate_type_hints() with pytest.raises(RuntimeError, match="does not match defined type"): - controller.add_sub_controller("child", Controller()) + controller.add_sub_controller("child", Controller(path=["child"])) - controller.add_sub_controller("child", SomeSubController()) + controller.add_sub_controller("child", SomeSubController(path=["child"])) controller._validate_type_hints() @@ -249,7 +262,7 @@ async def scan_nothing(self): pass controller = MyTestController() - api = controller._build_api([]) + api = controller._build_api() assert api.description == controller.description assert list(api.attributes) == ["attr1", "attr2"] diff --git a/tests/test_launch.py b/tests/test_launch.py index 17e935f4..a9a75ebb 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -35,37 +35,37 @@ class OtherConfig: class SingleArg(Controller): - def __init__(self): - super().__init__() + def __init__(self, *, path=None): + super().__init__(path=path) class NotHinted(Controller): - def __init__(self, arg): - super().__init__() + def __init__(self, arg, *, path=None): + super().__init__(path=path) class IsHinted(Controller): read = AttrR(Int()) - def __init__(self, arg: SomeConfig) -> None: - super().__init__() + def __init__(self, arg: SomeConfig, *, path=None) -> None: + super().__init__(path=path) class ManyArgs(Controller): - def __init__(self, arg: SomeConfig, too_many): - super().__init__() + def __init__(self, arg: SomeConfig, too_many, *, path=None): + super().__init__(path=path) class OtherHinted(Controller): - def __init__(self, arg: OtherConfig) -> None: - super().__init__() + def __init__(self, arg: OtherConfig, *, path=None) -> None: + super().__init__(path=path) class Aliased(Controller): type_name: ClassVar[str] = "aliased-controller" - def __init__(self, arg: SomeConfig) -> None: - super().__init__() + def __init__(self, arg: SomeConfig, *, path=None) -> None: + super().__init__(path=path) runner = CliRunner() @@ -121,7 +121,7 @@ def test_is_hinted_schema(data): def test_not_hinted_schema(): error = ( "Expected typehinting in 'NotHinted.__init__' but received " - "(self, arg). Add a typehint for `arg`." + "(self, arg, *, path=None). Add a typehint for `arg`." ) with pytest.raises(LaunchError) as exc_info: @@ -133,7 +133,8 @@ def test_over_defined_schema(): error = ( "" "Expected no more than 2 arguments for 'ManyArgs.__init__' " - "but received 3 as `(self, arg: tests.test_launch.SomeConfig, too_many)`" + "but received 3 as `(self, arg: tests.test_launch.SomeConfig, too_many, " + "*, path=None)`" ) with pytest.raises(LaunchError) as exc_info: diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 9e27f62e..5e1c67f0 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -34,10 +34,9 @@ class _OtherAttrController(Controller): def test_controller_api_path_uses_id(): - controller = _IdController() - sub = _IdController() + controller = _IdController(path=["X"]) + sub = _IdController(path=["X", "Sub"]) controller.add_sub_controller("Sub", sub) - controller.set_path(["X"]) api, _, _ = controller.create_api_and_tasks() @@ -46,8 +45,7 @@ def test_controller_api_path_uses_id(): def _api_with_id(controller_class: type[Controller], id: str): - controller = controller_class() - controller.set_path([id]) + controller = controller_class(path=[id]) api, _, _ = controller.create_api_and_tasks() return api @@ -305,8 +303,8 @@ class _LifecycleController(Controller): foo = AttrR(Int()) - def __init__(self): - super().__init__() + def __init__(self, *, path: list[str] | None = None): + super().__init__(path=path) self.connected = False self.initialised = False self.post_initialised = False @@ -332,10 +330,8 @@ class _OtherLifecycleController(_LifecycleController): async def test_fastcs_serves_two_controllers_end_to_end(mocker: MockerFixture): """FastCS.serve drives lifecycle on every controller and routes REST traffic per-id; combined OpenAPI describes both prefixes.""" - a = _LifecycleController() - a.set_path(["alpha"]) - b = _OtherLifecycleController() - b.set_path(["beta"]) + a = _LifecycleController(path=["alpha"]) + b = _OtherLifecycleController(path=["beta"]) transport = RestTransport() # Stop RestTransport from binding to a real port; we exercise the FastAPI diff --git a/tests/transports/epics/ca/test_gui.py b/tests/transports/epics/ca/test_gui.py index 46e000e1..13ad03d1 100644 --- a/tests/transports/epics/ca/test_gui.py +++ b/tests/transports/epics/ca/test_gui.py @@ -102,8 +102,11 @@ def test_get_write_widget_none(): ) -def test_get_components(controller): - controller_api = controller._build_api(["DEVICE"]) +def test_get_components(): + from tests.conftest import BackendTestController + + controller = BackendTestController(path=["DEVICE"]) + controller_api = controller._build_api() gui = EpicsGUI(controller_api) components = gui.extract_api_components(controller_api) @@ -203,8 +206,7 @@ class _B(Controller): def _api_with_id(cls, name): - c = cls() - c.set_path([name]) + c = cls(path=[name]) api, _, _ = c.create_api_and_tasks() return api diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index b78090dd..44f72767 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -51,8 +51,7 @@ async def test_initial_values_set_in_ca(mocker): pv_prefix = "SOFTIOC_INITIAL_DEVICE" loop = asyncio.get_event_loop() - controller = InitialValuesController() - controller.set_path([pv_prefix]) + controller = InitialValuesController(path=[pv_prefix]) fastcs = FastCS( controller, [EpicsCATransport()], diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 2ad7a601..1beb3cf5 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -273,7 +273,7 @@ class EpicsController(MyTestController): @pytest.fixture() def epics_controller_api(class_mocker: MockerFixture): - return AssertableControllerAPI(EpicsController(), class_mocker, path=[DEVICE]) + return AssertableControllerAPI(EpicsController(path=[DEVICE]), class_mocker) def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): @@ -504,7 +504,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): util_builder = mocker.patch("fastcs.transports.epics.ca.util.builder") ioc_builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") long_name_controller_api = AssertableControllerAPI( - ControllerLongNames(), mocker, path=[DEVICE] + ControllerLongNames(path=[DEVICE]), mocker ) long_attr_name = "attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name" long_rw_name = "attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_RBV" diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index 947ade79..163df31e 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -209,8 +209,7 @@ async def test_numeric_alarms(p4p_subprocess: tuple[str, Queue]): a_monitor.close() -def make_fastcs(pv_prefix: str, controller: Controller) -> FastCS: - controller.set_path([pv_prefix]) +def make_fastcs(controller: Controller) -> FastCS: return FastCS(controller, [EpicsPVATransport()]) @@ -219,9 +218,9 @@ class SomeController(Controller): a: AttrRW = AttrRW(Int(max=400_000, max_alarm=40_000)) b: AttrR = AttrR(Float(min=-1, min_alarm=-0.5, prec=2)) - controller = SomeController() pv_prefix = str(uuid4()) - fastcs = make_fastcs(pv_prefix, controller) + controller = SomeController(path=[pv_prefix]) + fastcs = make_fastcs(controller) ctxt = ThreadContext("pva") @@ -272,36 +271,27 @@ class SomeController(Controller): another_attr_1000: AttrRW = AttrRW(Int()) a_third_attr: AttrW = AttrW(Int()) - controller = SomeController() - - sub_controller_vector = ControllerVector({i: ChildController() for i in range(3)}) + # Short id keeps the deepest prefix (`:AdditionalChild:ChildChild`) + # under the 60-char EPICS PV name limit enforced by validate_pva_id. + pv_prefix = uuid4().hex[:8] + controller = SomeController(path=[pv_prefix]) + child_path = [pv_prefix, "child"] + sub_controller_vector = ControllerVector( + {i: ChildController(path=child_path + [str(i)]) for i in range(3)}, + path=child_path, + ) controller.add_sub_controller("child", sub_controller_vector) - sub_controller = ChildController() - controller.child0 = sub_controller - sub_controller.child_child = ChildChildController() - - sub_controller = ChildController() - controller.child1 = sub_controller - sub_controller.child_child = ChildChildController() - - sub_controller = ChildController() - controller.child2 = sub_controller - sub_controller.child_child = ChildChildController() - - sub_controller = ChildController() - controller.another_child = sub_controller - sub_controller.child_child = ChildChildController() - - sub_controller = ChildController() - controller.additional_child = sub_controller - sub_controller.child_child = ChildChildController() + for name in ("child0", "child1", "child2", "another_child", "additional_child"): + sub_path = [pv_prefix, name] + sub_controller = ChildController(path=sub_path) + setattr(controller, name, sub_controller) + sub_controller.child_child = ChildChildController( + path=sub_path + ["child_child"] + ) - # Short id keeps the deepest prefix (`:AdditionalChild:ChildChild`) - # under the 60-char EPICS PV name limit enforced by validate_pva_id. - pv_prefix = uuid4().hex[:8] - fastcs = make_fastcs(pv_prefix, controller) + fastcs = make_fastcs(controller) ctxt = ThreadContext("pva") @@ -411,9 +401,9 @@ class SomeController(Controller): some_table: AttrRW = AttrRW(Table(table_columns)) some_enum: AttrRW = AttrRW(Enum(AnEnum)) - controller = SomeController() pv_prefix = str(uuid4()) - fastcs = make_fastcs(pv_prefix, controller) + controller = SomeController(path=[pv_prefix]) + fastcs = make_fastcs(controller) ctxt = ThreadContext("pva", nt=False) @@ -550,9 +540,9 @@ async def some_task(): asyncio.create_task(some_task()) - controller = SomeController() pv_prefix = str(uuid4()) - fastcs = make_fastcs(pv_prefix, controller) + controller = SomeController(path=[pv_prefix]) + fastcs = make_fastcs(controller) expected_error_string = ( "RuntimeError: Received request to run command but it is " "already in progress. Maybe the command should spawn an asyncio task?" @@ -619,9 +609,9 @@ class SomeController(Controller): async def command_runs_for_a_while(self): await asyncio.sleep(0.2) - controller = SomeController() pv_prefix = str(uuid4()) - fastcs = make_fastcs(pv_prefix, controller) + controller = SomeController(path=[pv_prefix]) + fastcs = make_fastcs(controller) command_runs_for_a_while_times = [] async def put_pvs(): diff --git a/tests/transports/epics/test_emission.py b/tests/transports/epics/test_emission.py index b4e22365..f1164277 100644 --- a/tests/transports/epics/test_emission.py +++ b/tests/transports/epics/test_emission.py @@ -32,8 +32,7 @@ class _Beta(Controller): def _api_with_id(controller_class: type[Controller], name: str): - controller = controller_class() - controller.set_path([name]) + controller = controller_class(path=[name]) api, _, _ = controller.create_api_and_tasks() return api diff --git a/tests/transports/graphQL/test_graphql.py b/tests/transports/graphQL/test_graphql.py index 46d2fc8a..1fd66acc 100644 --- a/tests/transports/graphQL/test_graphql.py +++ b/tests/transports/graphQL/test_graphql.py @@ -31,7 +31,7 @@ class GraphQLController(MyTestController): @pytest.fixture(scope="class") def gql_controller_api(class_mocker: MockerFixture): - return AssertableControllerAPI(GraphQLController(), class_mocker, path=[_GQL_ID]) + return AssertableControllerAPI(GraphQLController(path=[_GQL_ID]), class_mocker) def nest_query(path: list[str]) -> str: diff --git a/tests/transports/tango/test_dsr.py b/tests/transports/tango/test_dsr.py index 61a8f73e..3e7dfd74 100644 --- a/tests/transports/tango/test_dsr.py +++ b/tests/transports/tango/test_dsr.py @@ -43,7 +43,7 @@ class TangoController(MyTestController): @pytest.fixture(scope="class") def tango_controller_api(class_mocker: MockerFixture) -> AssertableControllerAPI: - return AssertableControllerAPI(TangoController(), class_mocker, path=["DEVICE"]) + return AssertableControllerAPI(TangoController(path=["DEVICE"]), class_mocker) def create_test_context(tango_controller_api: AssertableControllerAPI): From 63ffd94d1d1e6b6d4b2c5d62cfe7acc421e96ade Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Thu, 21 May 2026 16:48:26 +0100 Subject: [PATCH 02/11] Remove path pre-validation from add_sub_controller It decouples sub-controller's path assignment from controller registration. --- src/fastcs/controllers/base_controller.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index d4fdbead..380f6466 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -349,13 +349,6 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): f"'{sub_controller.__class__.__name__}'." ) - expected = self.path + [name] - if sub_controller.path != expected: - raise ValueError( - f"Sub controller path {sub_controller.path} does not match " - f"parent path {self.path} + [{name!r}]. Construct the sub " - f"controller with `path={expected!r}`." - ) self.__sub_controllers[name] = sub_controller super().__setattr__(name, sub_controller) From 4228a7ddcb435770a6b89f86584e669d7976d67b Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Fri, 22 May 2026 13:06:34 +0000 Subject: [PATCH 03/11] test(controllers): drop stale path-mismatch assertion The path pre-validation in `add_sub_controller` was removed in 63ffd94d to decouple sub-controller path assignment from registration; the `does not match parent path` ValueError no longer fires. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_controllers.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index 288ab773..8f74d71b 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -25,11 +25,6 @@ def test_controller_nesting(): with pytest.raises(ValueError, match=r"Cannot add sub controller"): controller.a = Controller(path=["a"]) - # sub_controller has path=["a"], so adding it under name "c" violates the - # parent.path + [name] invariant. - with pytest.raises(ValueError, match=r"does not match parent path"): - controller.c = sub_controller - class SomeSubController(Controller): def __init__(self, *, path=None): From df6ea2a66f1ba2a0eb014181ae323ea00d33f985 Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Tue, 26 May 2026 16:51:56 +0000 Subject: [PATCH 04/11] chore: update pvi dependency to 0.14.0b1 Update fastcs to use pvi 0.14.0b1 beta which includes: - Group label preservation through SubScreen reconstructions - Flattened Grid group handling for INLINE controller hoisting - Improved support for dynamic GroupLayout modes --- pyproject.toml | 4 ++-- uv.lock | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f178a375..8150e229 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,8 +34,8 @@ requires-python = ">=3.11" [project.optional-dependencies] demo = ["tickit~=0.4.3"] -epicsca = ["pvi~=0.12.0", "softioc>=4.5.0"] -epicspva = ["p4p", "pvi~=0.12.0"] +epicsca = ["pvi>=0.14.0b1", "softioc>=4.5.0"] +epicspva = ["p4p", "pvi>=0.14.0b1"] epics = ["fastcs[epicsca]", "fastcs[epicspva]"] tango = ["pytango"] graphql = ["strawberry-graphql", "uvicorn[standard]>=0.12.0"] diff --git a/uv.lock b/uv.lock index 9a6d3c72..0dd48ef8 100644 --- a/uv.lock +++ b/uv.lock @@ -1002,8 +1002,8 @@ requires-dist = [ { name = "numpy" }, { name = "numpy", marker = "extra == 'rest'" }, { name = "p4p", marker = "extra == 'epicspva'" }, - { name = "pvi", marker = "extra == 'epicsca'", specifier = "~=0.12.0" }, - { name = "pvi", marker = "extra == 'epicspva'", specifier = "~=0.12.0" }, + { name = "pvi", marker = "extra == 'epicsca'", specifier = ">=0.14.0b1" }, + { name = "pvi", marker = "extra == 'epicspva'", specifier = ">=0.14.0b1" }, { name = "pydantic" }, { name = "pygelf" }, { name = "pytango", marker = "extra == 'tango'" }, @@ -2147,7 +2147,7 @@ wheels = [ [[package]] name = "pvi" -version = "0.12.0" +version = "0.14.0b1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "jinja2" }, @@ -2157,9 +2157,9 @@ dependencies = [ { name = "ruamel-yaml" }, { name = "typer" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/fc/91/3808872d96de8dac33069bf97f94f6ff430841ceaa8398db122ffe9d4244/pvi-0.12.0.tar.gz", hash = "sha256:1fb288dd863f31c276c0e119b57a36788f2b6c2e1324f88f1fac2c8ae1a8bb03", size = 189219, upload-time = "2025-12-22T14:11:02.545Z" } +sdist = { url = "https://files.pythonhosted.org/packages/8d/73/dae6a9497057f54ed3d7ce38b025c2dd6a5a718afaf143cb6acc185b8d23/pvi-0.14.0b1.tar.gz", hash = "sha256:8be53fdc4262c94b3a658d39f8d93a377e80d6194780f7948c3e92aa5a171918", size = 281599, upload-time = "2026-05-26T16:06:15.685Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/5e/b9/ffdd156df74460300c7e0483662670ed2826b7c09c59ba6a3a1b8c83b05d/pvi-0.12.0-py3-none-any.whl", hash = "sha256:b578a513fbbfe2368623eb6a75d84daf8b3176d4e46d3bc506f0ee3a1dbbfd80", size = 57072, upload-time = "2025-12-22T14:11:01.254Z" }, + { url = "https://files.pythonhosted.org/packages/f4/a0/eed716a7bf50917c8a7bf02f1cdf13cfb129dfbe03889c16dd4dad0b42da/pvi-0.14.0b1-py3-none-any.whl", hash = "sha256:239ad57c038219be061e7fc83382f0402e39a5f69289f056881f692df37fe7b6", size = 58195, upload-time = "2026-05-26T16:06:14.39Z" }, ] [[package]] From 08caae50a657b59be6d9f9755b5dab5dbe18c7c3 Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Tue, 26 May 2026 10:16:18 +0000 Subject: [PATCH 05/11] fix: preserve original sub-controller name as Group display label The name passed to Group() in extract_api_components() must satisfy the PascalCase constraint required by PVI ('^([A-Z][a-z0-9]*)*$'). When a controller's path segment contains characters outside that set (hyphens, digits in certain positions, etc.) the name has to be sanitised before it can be used as the Group key. PVI then derives the button/screen label from that sanitised name via to_title_case(), producing something unreadable (e.g. 'Bl 04i Ea E1rio 01') rather than the intended string. ControllerAPI.path already holds the original, unsanitised path segments. Pass api.path[-1] as the optional label= field on the Group so that PVI's Component.get_label() returns the original name verbatim instead of falling back to to_title_case(name). This is backwards-compatible: label defaults to None, so controllers whose path is empty are unaffected and existing behaviour is preserved. --- src/fastcs/transports/epics/gui.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fastcs/transports/epics/gui.py b/src/fastcs/transports/epics/gui.py index 882a83a0..42715d99 100644 --- a/src/fastcs/transports/epics/gui.py +++ b/src/fastcs/transports/epics/gui.py @@ -156,6 +156,7 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: components.append( Group( name=snake_to_pascal(name), + label=api.path[-1] if api.path else None, layout=SubScreen(), children=self.extract_api_components(api), ) From dc06881dd2ec3ce88191cbda181d504e45916e6f Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Tue, 26 May 2026 10:44:12 +0000 Subject: [PATCH 06/11] feat: add GroupLayout to control sub-controller GUI layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a GroupLayout enum (SUBSCREEN | INLINE) that lets callers specify, per sub-controller, how its children should be presented on the parent screen. SUBSCREEN (default) — children appear on a separate screen opened by a navigate button, preserving the previous behaviour. INLINE — children are rendered as an inline Grid block directly on the parent screen, without an extra navigation level. The enum is decoupled from PVI: it lives in fastcs.controllers, which has no dependency on any transport. The EPICS GUI layer (gui.py) resolves it to the appropriate pvi.device layout object when building the component tree. Usage: from fastcs.controllers import Controller, GroupLayout class MyController(Controller): group_layout = GroupLayout.INLINE # class-level default # or per-instance: ctrl = MyController(group_layout=GroupLayout.INLINE) parent.add_sub_controller("Ctrl", ctrl) --- src/fastcs/controllers/__init__.py | 1 + src/fastcs/controllers/base_controller.py | 9 ++++++++- src/fastcs/controllers/controller.py | 7 +++++-- src/fastcs/controllers/controller_api.py | 17 +++++++++++++++++ src/fastcs/transports/epics/gui.py | 4 +++- 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/fastcs/controllers/__init__.py b/src/fastcs/controllers/__init__.py index b982292d..ef92090b 100644 --- a/src/fastcs/controllers/__init__.py +++ b/src/fastcs/controllers/__init__.py @@ -1,4 +1,5 @@ from .base_controller import BaseController as BaseController from .controller import Controller as Controller from .controller_api import ControllerAPI as ControllerAPI +from .controller_api import GroupLayout as GroupLayout from .controller_vector import ControllerVector as ControllerVector diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index 380f6466..6d6da837 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -12,7 +12,7 @@ ) from fastcs.attributes import AnyAttributeIO, Attribute, AttrR, AttrW, HintedAttribute -from fastcs.controllers.controller_api import ControllerAPI +from fastcs.controllers.controller_api import ControllerAPI, GroupLayout from fastcs.logging import logger from fastcs.methods import Command, Method, Scan, UnboundCommand, UnboundScan from fastcs.tracer import Tracer @@ -36,6 +36,8 @@ class BaseController(Tracer): # behaviour of instantiated controllers root_attribute: Attribute | None = None description: str | None = None + group_layout: GroupLayout = GroupLayout.SUBSCREEN + """How this controller's children are laid out on a parent screen.""" def __init__( self, @@ -43,6 +45,7 @@ def __init__( ios: Sequence[AnyAttributeIO] | None = None, *, path: list[str] | None = None, + group_layout: GroupLayout | None = None, ) -> None: super().__init__() @@ -50,6 +53,9 @@ def __init__( # Use the argument over the one class defined description. self.description = description + if group_layout is not None: + self.group_layout = group_layout + self._path: list[str] = list(path) if path else [] # Internal state that should not be accessed directly by base classes @@ -409,4 +415,5 @@ def _build_api(self) -> ControllerAPI: for name, sub_controller in self.sub_controllers.items() }, description=self.description, + group_layout=self.group_layout, ) diff --git a/src/fastcs/controllers/controller.py b/src/fastcs/controllers/controller.py index c95f3fea..8ee5485a 100755 --- a/src/fastcs/controllers/controller.py +++ b/src/fastcs/controllers/controller.py @@ -6,7 +6,7 @@ from fastcs.attributes.attr_r import AttrR from fastcs.attributes.attribute_io_ref import AttributeIORef from fastcs.controllers.base_controller import BaseController -from fastcs.controllers.controller_api import ControllerAPI +from fastcs.controllers.controller_api import ControllerAPI, GroupLayout from fastcs.logging import logger from fastcs.methods import ScanCallback from fastcs.util import ONCE @@ -21,8 +21,11 @@ def __init__( ios: Sequence[AnyAttributeIO] | None = None, *, path: list[str] | None = None, + group_layout: GroupLayout | None = None, ) -> None: - super().__init__(description=description, ios=ios, path=path) + super().__init__( + description=description, ios=ios, path=path, group_layout=group_layout + ) self._connected = False def add_sub_controller(self, name: str, sub_controller: BaseController): diff --git a/src/fastcs/controllers/controller_api.py b/src/fastcs/controllers/controller_api.py index 044e6b83..9072ed1c 100644 --- a/src/fastcs/controllers/controller_api.py +++ b/src/fastcs/controllers/controller_api.py @@ -1,10 +1,25 @@ from collections.abc import Iterator from dataclasses import dataclass, field +from enum import StrEnum, auto from fastcs.attributes import Attribute from fastcs.methods import Command, Scan +class GroupLayout(StrEnum): + """How a sub-controller's children are displayed in the generated GUI. + + ``SUBSCREEN`` (default) — children appear on a separate screen opened by a + navigate button on the parent screen. + + ``INLINE`` — children are rendered as an inline grid block directly on the + parent screen, without an extra navigation level. + """ + + SUBSCREEN = auto() + INLINE = auto() + + @dataclass class ControllerAPI: """Attributes, Methods and sub APIs of a Controller to expose in a Transport @@ -26,6 +41,8 @@ class ControllerAPI: """The `ControllerAPI` s of the sub controllers of the `Controller`""" description: str | None = None """A description to display in the `Transport` layer""" + group_layout: GroupLayout = GroupLayout.SUBSCREEN + """How this controller's children are laid out when rendered inside a parent screen.""" def walk_api(self) -> Iterator["ControllerAPI"]: """Walk through all the nested `ControllerAPI` s of this `ControllerAPI`. diff --git a/src/fastcs/transports/epics/gui.py b/src/fastcs/transports/epics/gui.py index 42715d99..f469838f 100644 --- a/src/fastcs/transports/epics/gui.py +++ b/src/fastcs/transports/epics/gui.py @@ -24,6 +24,7 @@ from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW from fastcs.controllers import ControllerAPI +from fastcs.controllers.controller_api import GroupLayout from fastcs.datatypes import ( Bool, Enum, @@ -153,11 +154,12 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: for name, api in controller_api.sub_apis.items(): if name.isdigit(): name = f"{controller_api.path[-1]}{name}" + layout = Grid() if api.group_layout is GroupLayout.INLINE else SubScreen() components.append( Group( name=snake_to_pascal(name), label=api.path[-1] if api.path else None, - layout=SubScreen(), + layout=layout, children=self.extract_api_components(api), ) ) From b99958227109ae3e36ea34ec5683854aefd387a7 Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Tue, 26 May 2026 16:49:46 +0000 Subject: [PATCH 07/11] fix: sanitize PVA field names and improve GUI title defaults Sanitize child controller names for PVA field names by replacing non-alphanumeric characters (e.g. hyphens from beamline PV prefix formats like 'BL04I-EA-E1RIO-01') with underscores. This prevents P4P validation errors. Also make EpicsGUIOptions.title optional (defaults to None) and derive a meaningful default from the controller's path when not explicitly provided, improving upon the hardcoded 'FastCS Devices' placeholder. This allows GUI screens and docs to display the controller ID as the title when no explicit title is set. Updates schema.json files to reflect the title field now accepting null values. --- src/fastcs/demo/schema.json | 13 ++++++++--- src/fastcs/transports/epics/ca/ioc.py | 6 ++++- src/fastcs/transports/epics/emission.py | 29 ++++++++++++++++++++++--- src/fastcs/transports/epics/options.py | 3 ++- tests/data/schema.json | 13 ++++++++--- 5 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index 7fc6150e..ea9fef16 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -89,9 +89,16 @@ "default": ".bob" }, "title": { - "default": "FastCS Devices", - "title": "Title", - "type": "string" + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Title" } }, "title": "EpicsGUIOptions", diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 7ac5026d..4722a10c 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -1,4 +1,5 @@ import asyncio +import re from typing import Any, Literal from softioc import builder, softioc @@ -101,8 +102,11 @@ def _add_sub_controller_pvi_info(parent: ControllerAPI): if child.path[-1].isdigit() else child.path[-1] ) + # Sanitize for PVA field names: replace any non-alphanumeric, non-underscore + # character (e.g. hyphens from beamline PV prefixes) with underscores. + child_pvi_field = re.sub(r"[^a-zA-Z0-9_]", "_", child_name).lower() - _add_pvi_info(child_pvi, parent_pvi, child_name.lower()) + _add_pvi_info(child_pvi, parent_pvi, child_pvi_field) _add_sub_controller_pvi_info(child) diff --git a/src/fastcs/transports/epics/emission.py b/src/fastcs/transports/epics/emission.py index f90652f9..17382b26 100644 --- a/src/fastcs/transports/epics/emission.py +++ b/src/fastcs/transports/epics/emission.py @@ -93,7 +93,11 @@ def emit_gui_files( ui_filename = f"{name}{ext}" controller_path = (output_dir / ui_filename).resolve() - device = gui_builder(api).build_device(options.title) + device = gui_builder(api).build_device( + options.title + if options.title is not None + else (api.path[0] if api.path else "FastCS Devices") + ) formatter.format(device, controller_path) refs.append( @@ -107,7 +111,16 @@ def emit_gui_files( ) index_path = (output_dir / f"{INDEX_STEM}{ext}").resolve() - formatter.format(Device(label=options.title, children=refs), index_path) + index_title = ( + options.title + if options.title is not None + else ( + controller_apis[0].path[0] + if controller_apis and controller_apis[0].path + else "FastCS Devices" + ) + ) + formatter.format(Device(label=index_title, children=refs), index_path) DOCS_EXT = ".md" @@ -132,7 +145,17 @@ def emit_docs_files( path.write_text(_render_controller_md(api, options.depth)) index_path = output_dir / f"{INDEX_STEM}{DOCS_EXT}" - index_path.write_text(_render_index_md(controller_apis, options.title)) + index_path.write_text( + _render_index_md( + controller_apis, + options.title + or ( + controller_apis[0].path[0] + if controller_apis and controller_apis[0].path + else "FastCS Devices" + ), + ) + ) def _render_index_md(controller_apis: list[ControllerAPI], title: str) -> str: diff --git a/src/fastcs/transports/epics/options.py b/src/fastcs/transports/epics/options.py index 03821a0f..dcef9ca9 100644 --- a/src/fastcs/transports/epics/options.py +++ b/src/fastcs/transports/epics/options.py @@ -28,7 +28,8 @@ class EpicsGUIOptions: output_dir: Path = Path(".") file_format: EpicsGUIFormat = EpicsGUIFormat.bob - title: str = "FastCS Devices" + title: str | None = None + """Screen title. Defaults to the controller id when not provided.""" @dataclass diff --git a/tests/data/schema.json b/tests/data/schema.json index 5e9d800f..41a02501 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -89,9 +89,16 @@ "default": ".bob" }, "title": { - "default": "FastCS Devices", - "title": "Title", - "type": "string" + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "title": "Title" } }, "title": "EpicsGUIOptions", From 267462b281afe1e7fecd337ff311d13eaf7327f3 Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Tue, 26 May 2026 23:02:18 +0100 Subject: [PATCH 08/11] minor code corrections suggested by coderabbitai --- pyproject.toml | 8 ++++---- src/fastcs/controllers/controller_api.py | 3 ++- src/fastcs/transports/epics/emission.py | 3 ++- uv.lock | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8150e229..a2b372f6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,8 +5,8 @@ build-backend = "setuptools.build_meta" [project] name = "fastcs" authors = [ - {name = "Martin Gaughran", email = "martin.gaughran@diamond.ac.uk"}, - {name = "Gary Yendell", email = "gary.yendell@diamond.ac.uk"}, + { name = "Martin Gaughran", email = "martin.gaughran@diamond.ac.uk" }, + { name = "Gary Yendell", email = "gary.yendell@diamond.ac.uk" }, ] classifiers = [ "Development Status :: 3 - Alpha", @@ -34,8 +34,8 @@ requires-python = ">=3.11" [project.optional-dependencies] demo = ["tickit~=0.4.3"] -epicsca = ["pvi>=0.14.0b1", "softioc>=4.5.0"] -epicspva = ["p4p", "pvi>=0.14.0b1"] +epicsca = ["pvi==0.14.0b1", "softioc>=4.5.0"] +epicspva = ["p4p", "pvi==0.14.0b1"] epics = ["fastcs[epicsca]", "fastcs[epicspva]"] tango = ["pytango"] graphql = ["strawberry-graphql", "uvicorn[standard]>=0.12.0"] diff --git a/src/fastcs/controllers/controller_api.py b/src/fastcs/controllers/controller_api.py index 9072ed1c..6ac8b23e 100644 --- a/src/fastcs/controllers/controller_api.py +++ b/src/fastcs/controllers/controller_api.py @@ -42,7 +42,8 @@ class ControllerAPI: description: str | None = None """A description to display in the `Transport` layer""" group_layout: GroupLayout = GroupLayout.SUBSCREEN - """How this controller's children are laid out when rendered inside a parent screen.""" + """How this controller's children are laid out when rendered \ + inside a parent screen.""" def walk_api(self) -> Iterator["ControllerAPI"]: """Walk through all the nested `ControllerAPI` s of this `ControllerAPI`. diff --git a/src/fastcs/transports/epics/emission.py b/src/fastcs/transports/epics/emission.py index 17382b26..d03f2e12 100644 --- a/src/fastcs/transports/epics/emission.py +++ b/src/fastcs/transports/epics/emission.py @@ -149,7 +149,8 @@ def emit_docs_files( _render_index_md( controller_apis, options.title - or ( + if options.title is not None + else ( controller_apis[0].path[0] if controller_apis and controller_apis[0].path else "FastCS Devices" diff --git a/uv.lock b/uv.lock index 0dd48ef8..a2db4065 100644 --- a/uv.lock +++ b/uv.lock @@ -1002,8 +1002,8 @@ requires-dist = [ { name = "numpy" }, { name = "numpy", marker = "extra == 'rest'" }, { name = "p4p", marker = "extra == 'epicspva'" }, - { name = "pvi", marker = "extra == 'epicsca'", specifier = ">=0.14.0b1" }, - { name = "pvi", marker = "extra == 'epicspva'", specifier = ">=0.14.0b1" }, + { name = "pvi", marker = "extra == 'epicsca'", specifier = "==0.14.0b1" }, + { name = "pvi", marker = "extra == 'epicspva'", specifier = "==0.14.0b1" }, { name = "pydantic" }, { name = "pygelf" }, { name = "pytango", marker = "extra == 'tango'" }, From cd544f783c5e1170f6fd4d55efdd57bd6661b92d Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Wed, 27 May 2026 11:17:43 +0000 Subject: [PATCH 09/11] fix: Resolve type checking error in EpicsGUI.extract_api_components - Change return type from Tree to ResolvedTree to match Group.children parameter - Add cast() to handle list[ComponentUnion | Include] to ResolvedTree conversion - The method never appends Include objects, so the cast is semantically safe - Fixes pyright error: Type parameter variance issue with Include in Tree union This resolves the type checking failure when running 'tox -p' without affecting the runtime behavior of the code. --- src/fastcs/transports/epics/gui.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/fastcs/transports/epics/gui.py b/src/fastcs/transports/epics/gui.py index f469838f..dd966933 100644 --- a/src/fastcs/transports/epics/gui.py +++ b/src/fastcs/transports/epics/gui.py @@ -1,3 +1,5 @@ +from typing import cast + from pvi.device import ( LED, ArrayTrace, @@ -8,6 +10,7 @@ Grid, Group, ReadWidgetUnion, + ResolvedTree, SignalR, SignalRW, SignalW, @@ -148,7 +151,7 @@ def build_device(self, title: str) -> Device: components = self.extract_api_components(self._controller_api) return Device(label=title, children=components) - def extract_api_components(self, controller_api: ControllerAPI) -> Tree: + def extract_api_components(self, controller_api: ControllerAPI) -> ResolvedTree: components: Tree = [] for name, api in controller_api.sub_apis.items(): @@ -206,4 +209,4 @@ def extract_api_components(self, controller_api: ControllerAPI) -> Tree: for name, children in groups.items(): components.append(Group(name=name, layout=Grid(), children=children)) - return components + return cast(ResolvedTree, components) From babb1cfda054862fdbe0f821937f128dd7805f5a Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Wed, 27 May 2026 11:26:52 +0000 Subject: [PATCH 10/11] Fix missing colon separator (as spotted by coderabbitai) --- tests/transports/epics/pva/test_p4p.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index 163df31e..3fa3cbf1 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -309,7 +309,7 @@ class SomeController(Controller): 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 + f"{pv_prefix}:Child:0:ChildChild:PVI", child_child_child_controller_pvi.append ) serve = asyncio.ensure_future(fastcs.serve(interactive=False)) From 8eb8a114d2a56c2eef5678c576e70f81ab89953e Mon Sep 17 00:00:00 2001 From: Gregory Gay Date: Wed, 27 May 2026 11:46:01 +0000 Subject: [PATCH 11/11] test: Add coverage tests for BaseController group_layout and launch no-options path Add two targeted tests to improve code coverage: 1. test_controller_with_group_layout() - Tests that the group_layout parameter is properly initialized in BaseController.__init__() when explicitly passed and that it defaults to GroupLayout.SUBSCREEN. Covers line 57 in base_controller.py. 2. test_launch_single_arg_no_options() - Tests the launch system's handling of controllers that take no configuration options. Exercises the code path where registered.cls() is called with only path=[entry.id]. Covers line 236 in launch.py. These tests address CodeCov patch coverage report gaps. --- tests/test_controllers.py | 11 ++++++++++- tests/test_launch.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index 8f74d71b..c80a1e7e 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -4,7 +4,7 @@ import pytest from fastcs.attributes import AttrR, AttrRW -from fastcs.controllers import Controller, ControllerVector +from fastcs.controllers import Controller, ControllerVector, GroupLayout from fastcs.datatypes import Enum, Float, Int from fastcs.methods import Command, Scan, command, scan @@ -290,3 +290,12 @@ async def failing_scan(self): task.cancel() with pytest.raises(asyncio.CancelledError): await task + + +def test_controller_with_group_layout(): + """Test that group_layout parameter is properly set on a controller.""" + controller = Controller(group_layout=GroupLayout.INLINE) + assert controller.group_layout == GroupLayout.INLINE + + controller_default = Controller() + assert controller_default.group_layout == GroupLayout.SUBSCREEN diff --git a/tests/test_launch.py b/tests/test_launch.py index a9a75ebb..e278dd45 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -322,3 +322,31 @@ def test_multi_controller_run_reaches_fastcs(mocker: MockerFixture, tmp_path): controllers_arg = init_spy.call_args.args[1] assert [c.path[0] for c in controllers_arg] == ["one", "two"] assert [type(c) for c in controllers_arg] == [IsHinted, OtherHinted] + + +def test_launch_single_arg_no_options(mocker: MockerFixture, tmp_path): + """Test launching a controller that takes no options (no type hints). + + This exercises the code path where a controller doesn't expect options, + ensuring registered.cls() is called with only path=[entry.id]. + """ + init_spy = mocker.spy(FastCS, "__init__") + mocker.patch("fastcs.launch.FastCS.run") + + cfg = tmp_path / "single_arg.yaml" + cfg.write_text( + "controllers:\n" + " - id: my-controller\n" + " type: tests.SingleArg\n" + "transport:\n" + " - rest: {}\n" + ) + app = _launch(SingleArg) + result = runner.invoke(app, ["run", str(cfg)]) + assert result.exit_code == 0, result.output + + init_spy.assert_called_once() + controllers_arg = init_spy.call_args.args[1] + assert len(controllers_arg) == 1 + assert isinstance(controllers_arg[0], SingleArg) + assert controllers_arg[0].path == ["my-controller"]