From 5bb366cc2f39bdd9a2c861d0fc561141fd1838b0 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:20:37 +0100 Subject: [PATCH 01/16] Add Controller.id lifecycle for multi-controller foundation Introduce a stable per-controller identifier set once by the launcher between __init__ and initialise(). Reading id before it is set raises a RuntimeError, and setting twice raises. __repr__ surfaces the id once set, and create_api_and_tasks now seeds the root ControllerAPI path with [id] so sub-APIs become [id, sub]. Backwards compatible: when id is unset (existing single-controller launcher path), the API path remains empty and behaviour is unchanged. Part of #353. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/controllers/controller.py | 28 +++++++++++++++- tests/test_multi_controller.py | 50 ++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/test_multi_controller.py diff --git a/src/fastcs/controllers/controller.py b/src/fastcs/controllers/controller.py index fe1b4258..f5183ccd 100755 --- a/src/fastcs/controllers/controller.py +++ b/src/fastcs/controllers/controller.py @@ -22,6 +22,32 @@ def __init__( ) -> None: super().__init__(description=description, ios=ios) self._connected = False + self._id: str | None = None + + @property + def id(self) -> str: + """Stable identifier set once by the launcher between ``__init__`` and + ``initialise()``. Reading before set is a programming error.""" + if self._id is None: + raise RuntimeError( + f"Controller {type(self).__name__} id has not been set yet" + ) + return self._id + + def set_id(self, id: str) -> None: + """Set this controller's stable identifier. May only be called once.""" + if self._id is not None: + raise RuntimeError( + f"Controller {type(self).__name__} id is already set to " + f"{self._id!r}; cannot reset to {id!r}" + ) + self._id = id + + def __repr__(self): + base = super().__repr__() + if self._id is None: + return base + return f"{base[:-1]}, id={self._id!r})" def add_sub_controller(self, name: str, sub_controller: BaseController): if name.isdigit(): @@ -70,7 +96,7 @@ def create_api_and_tasks( tuple[ControllerAPI, list[ScanCallback], list[ScanCallback]] """ - controller_api = self._build_api([]) + controller_api = self._build_api([self._id] if self._id is not None else []) scan_dict: dict[float, list[ScanCallback]] = defaultdict(list) initial_coros: list[ScanCallback] = [] diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py new file mode 100644 index 00000000..8e082419 --- /dev/null +++ b/tests/test_multi_controller.py @@ -0,0 +1,50 @@ +"""Tests for the multi-controller foundation slice (#353). + +These tests exercise the public Controller.id lifecycle. +""" + +import pytest + +from fastcs.controllers import Controller + + +class _IdController(Controller): + pass + + +def test_id_raises_before_set(): + controller = _IdController() + with pytest.raises(RuntimeError, match="id"): + _ = controller.id + + +def test_id_returns_value_after_set(): + controller = _IdController() + controller.set_id("foo") + assert controller.id == "foo" + + +def test_set_id_twice_raises(): + controller = _IdController() + controller.set_id("foo") + with pytest.raises(RuntimeError, match="already"): + controller.set_id("bar") + + +def test_repr_includes_id_when_set(): + controller = _IdController() + assert "id=" not in repr(controller) + controller.set_id("foo") + assert "id='foo'" in repr(controller) + + +def test_controller_api_path_uses_id(): + controller = _IdController() + sub = _IdController() + controller.add_sub_controller("Sub", sub) + controller.set_id("X") + + api, _, _ = controller.create_api_and_tasks() + + assert api.path == ["X"] + assert api.sub_apis["Sub"].path == ["X", "Sub"] From 0d4a32965713e6baadd72a9df5e2437d4503f899 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:20:48 +0100 Subject: [PATCH 02/16] Add pv_prefix_from_path utility for path-based PV derivation Pure utility that derives an EPICS PV prefix from a controller path: the first segment (controller id) is used verbatim, while later segments are converted snake_case -> PascalCase. EPICS adopts this in #354 to replace the existing root-prefix-plus-pascalled-path approach for multi-controller IOCs. D2 module of #353. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/transports/epics/util.py | 11 +++++++++++ tests/transports/epics/test_pv_prefix.py | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 tests/transports/epics/test_pv_prefix.py diff --git a/src/fastcs/transports/epics/util.py b/src/fastcs/transports/epics/util.py index 4695939e..cfc79720 100644 --- a/src/fastcs/transports/epics/util.py +++ b/src/fastcs/transports/epics/util.py @@ -2,5 +2,16 @@ from fastcs.util import snake_to_pascal +def pv_prefix_from_path(path: list[str]) -> str: + """Derive an EPICS PV prefix from a controller path. + + The first segment (the controller id) is used verbatim; later segments are + converted snake_case → PascalCase. Joined with ':'. + """ + if not path: + raise ValueError("Cannot derive a PV prefix from an empty path") + return ":".join([path[0]] + [snake_to_pascal(node) for node in path[1:]]) + + def controller_pv_prefix(prefix: str, controller_api: ControllerAPI) -> str: return ":".join([prefix] + [snake_to_pascal(node) for node in controller_api.path]) diff --git a/tests/transports/epics/test_pv_prefix.py b/tests/transports/epics/test_pv_prefix.py new file mode 100644 index 00000000..5528afb2 --- /dev/null +++ b/tests/transports/epics/test_pv_prefix.py @@ -0,0 +1,23 @@ +import pytest + +from fastcs.transports.epics.util import pv_prefix_from_path + + +def test_pv_prefix_single_segment_verbatim(): + assert pv_prefix_from_path(["my-id"]) == "my-id" + + +def test_pv_prefix_keeps_root_verbatim_and_pascals_remainder(): + assert pv_prefix_from_path(["my-id", "sub_widget"]) == "my-id:SubWidget" + + +def test_pv_prefix_pascals_every_non_root_segment(): + assert ( + pv_prefix_from_path(["root_id", "sub_widget", "inner_thing"]) + == "root_id:SubWidget:InnerThing" + ) + + +def test_pv_prefix_empty_path_raises(): + with pytest.raises(ValueError, match="empty"): + pv_prefix_from_path([]) From 893487db09f0f355009951a3b5aed27d37e8fcdf Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:21:02 +0100 Subject: [PATCH 03/16] Add validate_rest_id for REST controller id validation Reject controller ids that aren't safe in a REST URL path: empty or containing characters outside the loosest URL-safe set ([A-Za-z0-9_-]+). The error message includes the offending id so startup failures are unambiguous. Hookup into RestTransport.connect follows in the multi-controller routing slice. D3-REST module of #353. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/transports/rest/util.py | 15 +++++++++++++++ tests/transports/rest/test_id_validator.py | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/transports/rest/test_id_validator.py diff --git a/src/fastcs/transports/rest/util.py b/src/fastcs/transports/rest/util.py index 550d7294..c869a0d9 100644 --- a/src/fastcs/transports/rest/util.py +++ b/src/fastcs/transports/rest/util.py @@ -1,9 +1,24 @@ +import re + import numpy as np from fastcs.datatypes import Bool, DataType, DType_T, Enum, Float, Int, String, Waveform REST_ALLOWED_DATATYPES = (Bool, DataType, Enum, Float, Int, String) +_REST_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + +def validate_rest_id(id: str) -> None: + """Reject controller ids that wouldn't be safe in a REST URL path.""" + if not id: + raise ValueError("Controller id is empty; ids must be non-empty") + if not _REST_ID_RE.fullmatch(id): + raise ValueError( + f"Controller id {id!r} is not a valid REST id; " + "only alphanumerics, '-' and '_' are allowed" + ) + def convert_datatype(datatype: DataType[DType_T]) -> type[DType_T]: """Converts a datatype to a rest serialisable type.""" diff --git a/tests/transports/rest/test_id_validator.py b/tests/transports/rest/test_id_validator.py new file mode 100644 index 00000000..fd7336b8 --- /dev/null +++ b/tests/transports/rest/test_id_validator.py @@ -0,0 +1,22 @@ +import pytest + +from fastcs.transports.rest.util import validate_rest_id + + +def test_validate_rest_id_accepts_alnum_dash_underscore(): + validate_rest_id("my-id_42") # should not raise + + +def test_validate_rest_id_rejects_empty(): + with pytest.raises(ValueError, match="empty"): + validate_rest_id("") + + +def test_validate_rest_id_rejects_path_separator(): + with pytest.raises(ValueError, match="bad/id"): + validate_rest_id("bad/id") + + +def test_validate_rest_id_rejects_space(): + with pytest.raises(ValueError, match="bad id"): + validate_rest_id("bad id") From 6dd956de1f708fecc5382e8c8aa76ebe829ccaa9 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:22:33 +0100 Subject: [PATCH 04/16] Unify Transport.connect signature on list[ControllerAPI] Every transport's connect() now takes list[ControllerAPI] uniformly. The existing single-controller transports (EPICS CA, EPICS PVA, GraphQL, Tango, REST) accept a list-of-one via a shared _expect_single helper and behave as before. FastCS.serve passes [self.controller_api]. True multi-controller support per transport will be wired in subsequent slices. This is a pure refactor: existing tests are updated to the new list-of-one call shape, no behaviour changes for any transport. Part of #353. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/control_system.py | 2 +- src/fastcs/transports/epics/ca/transport.py | 5 ++-- src/fastcs/transports/epics/pva/transport.py | 5 ++-- src/fastcs/transports/graphql/transport.py | 5 ++-- src/fastcs/transports/rest/transport.py | 5 ++-- src/fastcs/transports/tango/transport.py | 5 ++-- src/fastcs/transports/transport.py | 24 +++++++++++++++++--- tests/transports/graphQL/test_graphql.py | 2 +- tests/transports/rest/test_rest.py | 2 +- tests/transports/tango/test_dsr.py | 2 +- 10 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/fastcs/control_system.py b/src/fastcs/control_system.py index 39b7fd2e..209147cf 100644 --- a/src/fastcs/control_system.py +++ b/src/fastcs/control_system.py @@ -110,7 +110,7 @@ async def serve(self, interactive: bool = True) -> None: coros: list[Coroutine] = [] for transport in self._transports: - transport.connect(controller_api=self.controller_api, loop=self._loop) + transport.connect(controller_apis=[self.controller_api], loop=self._loop) coros.append(transport.serve()) common_context = context.keys() & transport.context.keys() if common_context: diff --git a/src/fastcs/transports/epics/ca/transport.py b/src/fastcs/transports/epics/ca/transport.py index cc51b5d6..8d65a376 100644 --- a/src/fastcs/transports/epics/ca/transport.py +++ b/src/fastcs/transports/epics/ca/transport.py @@ -14,7 +14,7 @@ from fastcs.transports.epics.ca.ioc import EpicsCAIOC from fastcs.transports.epics.docs import EpicsDocs from fastcs.transports.epics.gui import EpicsGUI -from fastcs.transports.transport import Transport +from fastcs.transports.transport import Transport, _expect_single @dataclass @@ -30,9 +30,10 @@ class EpicsCATransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ) -> None: + controller_api = _expect_single(controller_apis, "EpicsCATransport") self._controller_api = controller_api self._loop = loop self._pv_prefix = self.epicsca.pv_prefix diff --git a/src/fastcs/transports/epics/pva/transport.py b/src/fastcs/transports/epics/pva/transport.py index 5aff4591..6529c932 100644 --- a/src/fastcs/transports/epics/pva/transport.py +++ b/src/fastcs/transports/epics/pva/transport.py @@ -10,7 +10,7 @@ ) from fastcs.transports.epics.docs import EpicsDocs from fastcs.transports.epics.pva.gui import PvaEpicsGUI -from fastcs.transports.transport import Transport +from fastcs.transports.transport import Transport, _expect_single from .ioc import P4PIOC @@ -25,9 +25,10 @@ class EpicsPVATransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ) -> None: + controller_api = _expect_single(controller_apis, "EpicsPVATransport") self._controller_api = controller_api self._pv_prefix = self.epicspva.pv_prefix self._ioc = P4PIOC(self.epicspva.pv_prefix, controller_api) diff --git a/src/fastcs/transports/graphql/transport.py b/src/fastcs/transports/graphql/transport.py index a590c062..b813b759 100644 --- a/src/fastcs/transports/graphql/transport.py +++ b/src/fastcs/transports/graphql/transport.py @@ -2,7 +2,7 @@ from dataclasses import dataclass, field from fastcs.controllers import ControllerAPI -from fastcs.transports.transport import Transport +from fastcs.transports.transport import Transport, _expect_single from .graphql import GraphQLServer from .options import GraphQLServerOptions @@ -16,9 +16,10 @@ class GraphQLTransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): + controller_api = _expect_single(controller_apis, "GraphQLTransport") self._server = GraphQLServer(controller_api) async def serve(self) -> None: diff --git a/src/fastcs/transports/rest/transport.py b/src/fastcs/transports/rest/transport.py index d89a89b9..8e10531b 100644 --- a/src/fastcs/transports/rest/transport.py +++ b/src/fastcs/transports/rest/transport.py @@ -2,7 +2,7 @@ from dataclasses import dataclass, field from fastcs.controllers import ControllerAPI -from fastcs.transports.transport import Transport +from fastcs.transports.transport import Transport, _expect_single from .options import RestServerOptions from .rest import RestServer @@ -16,9 +16,10 @@ class RestTransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): + controller_api = _expect_single(controller_apis, "RestTransport") self._server = RestServer(controller_api) async def serve(self) -> None: diff --git a/src/fastcs/transports/tango/transport.py b/src/fastcs/transports/tango/transport.py index 6ce3b102..80e0f1e5 100644 --- a/src/fastcs/transports/tango/transport.py +++ b/src/fastcs/transports/tango/transport.py @@ -2,7 +2,7 @@ from dataclasses import dataclass, field from fastcs.controllers import ControllerAPI -from fastcs.transports.transport import Transport +from fastcs.transports.transport import Transport, _expect_single from .dsr import TangoDSR, TangoDSROptions @@ -15,9 +15,10 @@ class TangoTransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): + controller_api = _expect_single(controller_apis, "TangoTransport") self._dsr = TangoDSR(controller_api, loop) async def serve(self) -> None: diff --git a/src/fastcs/transports/transport.py b/src/fastcs/transports/transport.py index 705880ce..056dedf2 100644 --- a/src/fastcs/transports/transport.py +++ b/src/fastcs/transports/transport.py @@ -25,12 +25,15 @@ def union(cls): @abstractmethod def connect( - self, controller_api: ControllerAPI, loop: asyncio.AbstractEventLoop + self, + controller_apis: list[ControllerAPI], + loop: asyncio.AbstractEventLoop, ) -> None: """Connect the ``Transport`` to the control system - The `ControllerAPI` should be exposed over the transport. The provided event - loop should be used where required instead of creating a new one. + Each `ControllerAPI` in ``controller_apis`` should be exposed over the + transport. The provided event loop should be used where required instead of + creating a new one. """ pass @@ -53,3 +56,18 @@ async def serve(self) -> None: """ pass + + +def _expect_single( + controller_apis: list[ControllerAPI], transport_name: str +) -> ControllerAPI: + """Temporary helper for transports that only support one controller per process. + + True multi-controller support will be added per transport in subsequent slices. + """ + if len(controller_apis) != 1: + raise NotImplementedError( + f"{transport_name} does not yet support multiple controllers; " + f"got {len(controller_apis)}" + ) + return controller_apis[0] diff --git a/tests/transports/graphQL/test_graphql.py b/tests/transports/graphQL/test_graphql.py index 24f1c7c1..5e113f4a 100644 --- a/tests/transports/graphQL/test_graphql.py +++ b/tests/transports/graphQL/test_graphql.py @@ -66,7 +66,7 @@ def nest_response(path: list[str], value: Any) -> dict: def create_test_client(gql_controller_api: AssertableControllerAPI) -> TestClient: graphql_transport = GraphQLTransport() - graphql_transport.connect(gql_controller_api, asyncio.AbstractEventLoop()) + graphql_transport.connect([gql_controller_api], asyncio.AbstractEventLoop()) return TestClient(graphql_transport._server._app) diff --git a/tests/transports/rest/test_rest.py b/tests/transports/rest/test_rest.py index 2be0b21b..80af6698 100644 --- a/tests/transports/rest/test_rest.py +++ b/tests/transports/rest/test_rest.py @@ -32,7 +32,7 @@ def rest_controller_api(class_mocker: MockerFixture): def create_test_client(rest_controller_api: ControllerAPI) -> TestClient: rest_transport = RestTransport() - rest_transport.connect(rest_controller_api, asyncio.AbstractEventLoop()) + rest_transport.connect([rest_controller_api], asyncio.AbstractEventLoop()) return TestClient(rest_transport._server._app) diff --git a/tests/transports/tango/test_dsr.py b/tests/transports/tango/test_dsr.py index 3b599c62..5d44ff7a 100644 --- a/tests/transports/tango/test_dsr.py +++ b/tests/transports/tango/test_dsr.py @@ -49,7 +49,7 @@ def tango_controller_api(class_mocker: MockerFixture) -> AssertableControllerAPI def create_test_context(tango_controller_api: AssertableControllerAPI): tango_transport = TangoTransport() tango_transport.connect( - tango_controller_api, + [tango_controller_api], asyncio.get_event_loop(), ) device = tango_transport._dsr._device From 0b2e19854ea3ef65f5ee4180023d8f226afb529f Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:23:35 +0100 Subject: [PATCH 05/16] Route REST routes per controller id; reject illegal ids at connect RestServer now accepts list[ControllerAPI] and adds attribute and command routes for each. RestTransport hooks validate_rest_id into connect() so illegal ids fail fast with a clear startup error. Existing path-based routing already prefixes routes with controller_api.path[0], so once Controller.set_id seeds the API path, REST URLs become GET /{id}/{sub}/{attr} for free. Two new tests in tests/test_multi_controller.py cover routing two distinct ids in one process and rejecting an id with an illegal character. Part of #353. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/transports/rest/rest.py | 11 +++--- src/fastcs/transports/rest/transport.py | 9 +++-- tests/test_multi_controller.py | 52 ++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/fastcs/transports/rest/rest.py b/src/fastcs/transports/rest/rest.py index e1005ac0..189d67c2 100644 --- a/src/fastcs/transports/rest/rest.py +++ b/src/fastcs/transports/rest/rest.py @@ -20,16 +20,17 @@ class RestServer: - """A Rest Server which handles a controller""" + """A Rest Server which handles one or more controllers.""" - def __init__(self, controller_api: ControllerAPI): - self._controller_api = controller_api + def __init__(self, controller_apis: list[ControllerAPI]): + self._controller_apis = controller_apis self._app = self._create_app() def _create_app(self): app = FastAPI() - _add_attribute_api_routes(app, self._controller_api) - _add_command_api_routes(app, self._controller_api) + for controller_api in self._controller_apis: + _add_attribute_api_routes(app, controller_api) + _add_command_api_routes(app, controller_api) return app diff --git a/src/fastcs/transports/rest/transport.py b/src/fastcs/transports/rest/transport.py index 8e10531b..250e1335 100644 --- a/src/fastcs/transports/rest/transport.py +++ b/src/fastcs/transports/rest/transport.py @@ -2,10 +2,11 @@ from dataclasses import dataclass, field from fastcs.controllers import ControllerAPI -from fastcs.transports.transport import Transport, _expect_single +from fastcs.transports.transport import Transport from .options import RestServerOptions from .rest import RestServer +from .util import validate_rest_id @dataclass @@ -19,8 +20,10 @@ def connect( controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - controller_api = _expect_single(controller_apis, "RestTransport") - self._server = RestServer(controller_api) + for api in controller_apis: + if api.path: + validate_rest_id(api.path[0]) + self._server = RestServer(controller_apis) async def serve(self) -> None: await self._server.serve(self.rest) diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 8e082419..6caed608 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -1,17 +1,32 @@ """Tests for the multi-controller foundation slice (#353). -These tests exercise the public Controller.id lifecycle. +These tests exercise the public Controller.id lifecycle and +multi-controller REST routing through RestTransport. """ +import asyncio + import pytest +from fastapi.testclient import TestClient +from fastcs.attributes import AttrR from fastcs.controllers import Controller +from fastcs.datatypes import Int +from fastcs.transports.rest.transport import RestTransport class _IdController(Controller): pass +class _OneAttrController(Controller): + foo = AttrR(Int()) + + +class _OtherAttrController(Controller): + bar = AttrR(Int()) + + def test_id_raises_before_set(): controller = _IdController() with pytest.raises(RuntimeError, match="id"): @@ -48,3 +63,38 @@ def test_controller_api_path_uses_id(): assert api.path == ["X"] assert api.sub_apis["Sub"].path == ["X", "Sub"] + + +def _api_with_id(controller_class: type[Controller], id: str): + controller = controller_class() + controller.set_id(id) + api, _, _ = controller.create_api_and_tasks() + return api + + +def test_rest_transport_routes_two_controllers_by_id(): + api1 = _api_with_id(_OneAttrController, "alpha") + api2 = _api_with_id(_OtherAttrController, "beta") + + loop = asyncio.new_event_loop() + try: + transport = RestTransport() + transport.connect([api1, api2], loop) + + with TestClient(transport._server._app) as client: + assert client.get("/alpha/foo").status_code == 200 + assert client.get("/beta/bar").status_code == 200 + finally: + loop.close() + + +def test_rest_transport_rejects_illegal_id_at_connect(): + api = _api_with_id(_OneAttrController, "bad/id") + + loop = asyncio.new_event_loop() + try: + transport = RestTransport() + with pytest.raises(ValueError, match="bad/id"): + transport.connect([api], loop) + finally: + loop.close() From 011bb68325de8b87ac8299cfbdd03c59bb8a9618 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:43:23 +0100 Subject: [PATCH 06/16] Add multi-class launch() with dict-by-id controllers schema `launch()` now accepts either a single Controller class or a list of classes; the generated `fastcs.yaml` schema replaces the top-level `controller:` key with a dict of `controllers:` keyed by id. Each value carries a `type:` discriminator (defaults to the class `__name__`, overridable via `type_name: ClassVar[str]`) and an optional `controller:` options block. Single-class registration may omit `type:` via a default. Duplicate ids are rejected at YAML load time by ruamel's safe loader. Wiring through `FastCS` for >1 controller lands in the next slice; for now multi-entry configs validate cleanly but the run command exits with a clear LaunchError pointing at the deferred work. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/demo/controller.yaml | 12 +- src/fastcs/demo/schema.json | 145 ++++++++++++++++-------- src/fastcs/launch.py | 195 ++++++++++++++++++++++---------- tests/data/config.yaml | 6 +- tests/data/schema.json | 29 ++++- tests/test_launch.py | 156 +++++++++++++++++++++++-- 6 files changed, 421 insertions(+), 122 deletions(-) diff --git a/src/fastcs/demo/controller.yaml b/src/fastcs/demo/controller.yaml index 0df1fa41..fe75dfe3 100644 --- a/src/fastcs/demo/controller.yaml +++ b/src/fastcs/demo/controller.yaml @@ -1,9 +1,11 @@ # yaml-language-server: $schema=schema.json -controller: - ip_settings: - ip: "localhost" - port: 25565 - num_ramp_controllers: 4 +controllers: + TEMPERATURE: + controller: + ip_settings: + ip: "localhost" + port: 25565 + num_ramp_controllers: 4 transport: - graphql: host: localhost diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index a147c921..17f173d2 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -1,18 +1,34 @@ { "$defs": { - "EpicsCAOptions": { + "EpicsCATransport": { "properties": { + "epicsca": { + "$ref": "#/$defs/EpicsIOCOptions" + }, "docs": { - "$ref": "#/$defs/EpicsDocsOptions" + "anyOf": [ + { + "$ref": "#/$defs/EpicsDocsOptions" + }, + { + "type": "null" + } + ], + "default": null }, "gui": { - "$ref": "#/$defs/EpicsGUIOptions" - }, - "ioc": { - "$ref": "#/$defs/EpicsIOCOptions" + "anyOf": [ + { + "$ref": "#/$defs/EpicsGUIOptions" + }, + { + "type": "null" + } + ], + "default": null } }, - "title": "EpicsCAOptions", + "title": "EpicsCATransport", "type": "object" }, "EpicsDocsOptions": { @@ -80,28 +96,35 @@ "title": "EpicsIOCOptions", "type": "object" }, - "EpicsPVAOptions": { + "EpicsPVATransport": { "properties": { + "epicspva": { + "$ref": "#/$defs/EpicsIOCOptions" + }, "docs": { - "$ref": "#/$defs/EpicsDocsOptions" + "anyOf": [ + { + "$ref": "#/$defs/EpicsDocsOptions" + }, + { + "type": "null" + } + ], + "default": null }, "gui": { - "$ref": "#/$defs/EpicsGUIOptions" - }, - "ioc": { - "$ref": "#/$defs/EpicsIOCOptions" - } - }, - "title": "EpicsPVAOptions", - "type": "object" - }, - "GraphQLOptions": { - "properties": { - "gql": { - "$ref": "#/$defs/GraphQLServerOptions" + "anyOf": [ + { + "$ref": "#/$defs/EpicsGUIOptions" + }, + { + "type": "null" + } + ], + "default": null } }, - "title": "GraphQLOptions", + "title": "EpicsPVATransport", "type": "object" }, "GraphQLServerOptions": { @@ -125,6 +148,15 @@ "title": "GraphQLServerOptions", "type": "object" }, + "GraphQLTransport": { + "properties": { + "graphql": { + "$ref": "#/$defs/GraphQLServerOptions" + } + }, + "title": "GraphQLTransport", + "type": "object" + }, "IPConnectionSettings": { "properties": { "ip": { @@ -141,15 +173,6 @@ "title": "IPConnectionSettings", "type": "object" }, - "RestOptions": { - "properties": { - "rest": { - "$ref": "#/$defs/RestServerOptions" - } - }, - "title": "RestOptions", - "type": "object" - }, "RestServerOptions": { "properties": { "host": { @@ -171,6 +194,15 @@ "title": "RestServerOptions", "type": "object" }, + "RestTransport": { + "properties": { + "rest": { + "$ref": "#/$defs/RestServerOptions" + } + }, + "title": "RestTransport", + "type": "object" + }, "TangoDSROptions": { "properties": { "dev_name": { @@ -192,16 +224,35 @@ "title": "TangoDSROptions", "type": "object" }, - "TangoOptions": { + "TangoTransport": { "properties": { "tango": { "$ref": "#/$defs/TangoDSROptions" } }, - "title": "TangoOptions", + "title": "TangoTransport", + "type": "object" + }, + "TemperatureControllerEntry": { + "additionalProperties": false, + "properties": { + "type": { + "const": "TemperatureController", + "default": "TemperatureController", + "title": "Type", + "type": "string" + }, + "controller": { + "$ref": "#/$defs/TemperatureControllerSettings" + } + }, + "required": [ + "controller" + ], + "title": "TemperatureControllerEntry", "type": "object" }, - "TempControllerSettings": { + "TemperatureControllerSettings": { "properties": { "num_ramp_controllers": { "title": "Num Ramp Controllers", @@ -215,32 +266,36 @@ "num_ramp_controllers", "ip_settings" ], - "title": "TempControllerSettings", + "title": "TemperatureControllerSettings", "type": "object" } }, "additionalProperties": false, "properties": { - "controller": { - "$ref": "#/$defs/TempControllerSettings" + "controllers": { + "additionalProperties": { + "$ref": "#/$defs/TemperatureControllerEntry" + }, + "title": "Controllers", + "type": "object" }, "transport": { "items": { "anyOf": [ { - "$ref": "#/$defs/EpicsPVAOptions" + "$ref": "#/$defs/EpicsCATransport" }, { - "$ref": "#/$defs/EpicsCAOptions" + "$ref": "#/$defs/EpicsPVATransport" }, { - "$ref": "#/$defs/TangoOptions" + "$ref": "#/$defs/GraphQLTransport" }, { - "$ref": "#/$defs/RestOptions" + "$ref": "#/$defs/RestTransport" }, { - "$ref": "#/$defs/GraphQLOptions" + "$ref": "#/$defs/TangoTransport" } ] }, @@ -249,9 +304,9 @@ } }, "required": [ - "controller", + "controllers", "transport" ], - "title": "TempController", + "title": "TemperatureController", "type": "object" } diff --git a/src/fastcs/launch.py b/src/fastcs/launch.py index 99874cb9..cf6a8674 100644 --- a/src/fastcs/launch.py +++ b/src/fastcs/launch.py @@ -2,10 +2,10 @@ import inspect import json from pathlib import Path -from typing import Annotated, Any, Optional, get_type_hints +from typing import Annotated, Any, Literal, Optional, Union, get_type_hints import typer -from pydantic import BaseModel, ValidationError, create_model +from pydantic import BaseModel, Field, ValidationError, create_model from ruamel.yaml import YAML from fastcs import __version__ @@ -25,53 +25,75 @@ def launch( - controller_class: type[Controller], + controller_classes: type[Controller] | list[type[Controller]], version: str | None = None, ) -> None: """ Serves as an entry point for starting FastCS applications. - By utilizing type hints in a Controller's __init__ method, this + By utilizing type hints in each Controller's __init__ method, this function provides a command-line interface to describe and gather the required configuration before instantiating the application. Args: - controller_class (type[Controller]): The FastCS Controller to instantiate. - It must have a type-hinted __init__ method and no more than 2 arguments. - version (Optional[str]): The version of the FastCS Controller. - Optional + controller_classes: One or more FastCS Controller classes to make + available for instantiation. Each must have a type-hinted + __init__ method and no more than 2 arguments. The chosen class + for each id is selected by a ``type`` discriminator in the + config; when a single class is registered, ``type`` may be + omitted. + version (Optional[str]): The version of the FastCS application. Raises: - LaunchError: If the class's __init__ is not as expected - - Example of the expected Controller implementation: - class MyController(Controller): - def __init__(self, my_arg: MyControllerOptions) -> None: - ... + LaunchError: If a class's __init__ is not as expected. Typical usage: if __name__ == "__main__": - launch(MyController) + launch(MyController) # single class + launch([MyControllerA, MyControllerB]) # multi-class + """ + _launch(controller_classes, version)() + + +def _normalise_classes( + controller_classes: type[Controller] | list[type[Controller]], +) -> list[type[Controller]]: + if isinstance(controller_classes, list): + if not controller_classes: + raise LaunchError("launch() requires at least one Controller class") + return controller_classes + return [controller_classes] + + +def _discriminator(controller_class: type[Controller]) -> str: + """Type discriminator used in fastcs.yaml under each entry's ``type:`` key. + + Defaults to the class ``__name__`` and may be overridden by setting + ``type_name: ClassVar[str]`` on the Controller class. """ - _launch(controller_class, version)() + return getattr(controller_class, "type_name", controller_class.__name__) def _launch( - controller_class: type[Controller], + controller_classes: type[Controller] | list[type[Controller]], version: str | None = None, ) -> typer.Typer: - fastcs_options = _extract_options_model(controller_class) + classes = _normalise_classes(controller_classes) + fastcs_options = _build_options_model(classes) + type_map = {_discriminator(cls): cls for cls in classes} + app_name = classes[0].__name__ if len(classes) == 1 else "FastCS" launch_typer = typer.Typer() class LaunchContext: - def __init__(self, controller_class, fastcs_options): - self.controller_class = controller_class + def __init__(self, classes, fastcs_options, type_map): + self.classes = classes self.fastcs_options = fastcs_options + self.type_map = type_map def version_callback(value: bool): if value: if version: - print(f"{controller_class.__name__}: {version}") + print(f"{app_name}: {version}") print(f"FastCS: {__version__}") raise typer.Exit() @@ -83,27 +105,22 @@ def main( "--version", callback=version_callback, is_eager=True, - help=f"Display the {controller_class.__name__} version.", + help=f"Display the {app_name} version.", ), ): - ctx.obj = LaunchContext( - controller_class, - fastcs_options, - ) + ctx.obj = LaunchContext(classes, fastcs_options, type_map) - @launch_typer.command(help=f"Produce json schema for a {controller_class.__name__}") + @launch_typer.command(help=f"Produce json schema for a {app_name}") def schema(ctx: typer.Context): system_schema = ctx.obj.fastcs_options.model_json_schema() print(json.dumps(system_schema, indent=2)) - @launch_typer.command(help=f"Start up a {controller_class.__name__}") + @launch_typer.command(help=f"Start up a {app_name}") def run( ctx: typer.Context, config: Annotated[ Path, - typer.Argument( - help=f"A yaml file matching the {controller_class.__name__} schema" - ), + typer.Argument(help=f"A yaml file matching the {app_name} schema"), ], log_level: Annotated[LogLevel, typer.Option()] = LogLevel.INFO, graylog_endpoint: Annotated[ @@ -128,15 +145,13 @@ def run( ), ] = None, ): - """ - Start the controller - """ + """Start the controllers""" configure_logging( log_level, graylog_endpoint, graylog_static_fields, graylog_env_fields ) - controller_class = ctx.obj.controller_class fastcs_options = ctx.obj.fastcs_options + type_map = ctx.obj.type_map yaml = YAML(typ="safe") options_yaml = yaml.load(config) @@ -153,13 +168,19 @@ def run( raise LaunchError("Failed to validate config") from e - if hasattr(instance_options, "controller"): - controller = controller_class(instance_options.controller) - else: - controller = controller_class() + controllers = _instantiate_controllers(instance_options.controllers, type_map) + + if len(controllers) > 1: + raise LaunchError( + "Multi-controller execution is not yet wired through FastCS; " + "this lands in the next slice of issue #353. " + "Configure exactly one entry under `controllers:` for now." + ) instance = FastCS( - controller, instance_options.transport, loop=asyncio.get_event_loop() + controllers[0], + instance_options.transport, + loop=asyncio.get_event_loop(), ) instance.run() @@ -167,15 +188,43 @@ def run( return launch_typer -def _extract_options_model(controller_class: type[Controller]) -> type[BaseModel]: +def _instantiate_controllers( + controllers_options: dict[str, Any], + type_map: dict[str, type[Controller]], +) -> list[Controller]: + """Instantiate each entry under `controllers:` and stamp its id. + + Each value in ``controllers_options`` is a dynamically-built Pydantic + model whose fields are unknown to the type checker; the discriminator + and optional controller options block are accessed by name at runtime. + """ + controllers: list[Controller] = [] + for id, entry in controllers_options.items(): + cls = type_map[entry.type] + if hasattr(entry, "controller"): + controller = cls(entry.controller) + else: + controller = cls() + controller.set_id(id) + controllers.append(controller) + return controllers + + +def _build_entry_model(controller_class: type[Controller]) -> type[BaseModel]: + """Build a Pydantic model for one entry under `controllers:`. + + Each entry has a ``type`` discriminator literal and, for Controllers + whose ``__init__`` accepts a typed options argument, a ``controller`` + options block. + """ sig = inspect.signature(controller_class.__init__) args = inspect.getfullargspec(controller_class.__init__)[0] + discriminator = _discriminator(controller_class) + + fields: dict[str, Any] = {"type": (Literal[discriminator], discriminator)} + if len(args) == 1: - fastcs_options = create_model( - f"{controller_class.__name__}", - transport=(list[Transport.union()], ...), - __config__={"extra": "forbid"}, - ) + pass elif len(args) == 2: hints = get_type_hints(controller_class.__init__) if "return" in hints: @@ -187,22 +236,52 @@ def _extract_options_model(controller_class: type[Controller]) -> type[BaseModel f"Expected typehinting in '{controller_class.__name__}" f".__init__' but received {sig}. Add a typehint for `{args[-1]}`." ) - fastcs_options = create_model( - f"{controller_class.__name__}", - controller=(options_type, ...), - transport=(list[Transport.union()], ...), - __config__={"extra": "forbid"}, - ) + fields["controller"] = (options_type, ...) else: raise LaunchError( f"Expected no more than 2 arguments for '{controller_class.__name__}" f".__init__' but received {len(args)} as `{sig}`" ) - return fastcs_options + + return create_model( + f"{controller_class.__name__}Entry", + __config__={"extra": "forbid"}, + **fields, + ) + + +def _build_options_model( + controller_classes: list[type[Controller]], +) -> type[BaseModel]: + """Build the top-level Pydantic model for fastcs.yaml. + + `controllers:` is a dict keyed by id. Each value is either the single + registered class's entry model (in which case ``type`` is optional via + its default) or a discriminated union over all registered classes + (selected by the entry's ``type:`` field). + """ + entries = [_build_entry_model(cls) for cls in controller_classes] + + if len(entries) == 1: + entry_value_type: Any = entries[0] + title = controller_classes[0].__name__ + else: + entry_value_type = Annotated[ + Union[tuple(entries)], Field(discriminator="type") # noqa: UP007 + ] + title = "FastCS" + + return create_model( + title, + __config__={"extra": "forbid"}, + controllers=(dict[str, entry_value_type], ...), + transport=(list[Transport.union()], ...), + ) -def get_controller_schema(target: type[Controller]) -> dict[str, Any]: - """Gets schema for a give controller for serialisation.""" - options_model = _extract_options_model(target) - target_schema = options_model.model_json_schema() - return target_schema +def get_controller_schema( + target: type[Controller] | list[type[Controller]], +) -> dict[str, Any]: + """Gets schema for given controller class(es) for serialisation.""" + options_model = _build_options_model(_normalise_classes(target)) + return options_model.model_json_schema() diff --git a/tests/data/config.yaml b/tests/data/config.yaml index 8c7d3ac3..3d19562c 100644 --- a/tests/data/config.yaml +++ b/tests/data/config.yaml @@ -1,4 +1,8 @@ # yaml-language-server: $schema=schema.json +controllers: + device-1: + controller: + name: controller-name transport: - epicsca: {} docs: {} @@ -9,5 +13,3 @@ transport: - rest: {} - tango: {} - graphql: {} -controller: - name: controller-name diff --git a/tests/data/schema.json b/tests/data/schema.json index 8609adf3..3282cf65 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -157,6 +157,25 @@ "title": "GraphQLTransport", "type": "object" }, + "IsHintedEntry": { + "additionalProperties": false, + "properties": { + "type": { + "const": "IsHinted", + "default": "IsHinted", + "title": "Type", + "type": "string" + }, + "controller": { + "$ref": "#/$defs/SomeConfig" + } + }, + "required": [ + "controller" + ], + "title": "IsHintedEntry", + "type": "object" + }, "RestServerOptions": { "properties": { "host": { @@ -233,8 +252,12 @@ }, "additionalProperties": false, "properties": { - "controller": { - "$ref": "#/$defs/SomeConfig" + "controllers": { + "additionalProperties": { + "$ref": "#/$defs/IsHintedEntry" + }, + "title": "Controllers", + "type": "object" }, "transport": { "items": { @@ -261,7 +284,7 @@ } }, "required": [ - "controller", + "controllers", "transport" ], "title": "IsHinted", diff --git a/tests/test_launch.py b/tests/test_launch.py index a482f61a..681d8263 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -1,9 +1,10 @@ import json import os from dataclasses import dataclass +from typing import ClassVar, Literal import pytest -from pydantic import create_model +from pydantic import ValidationError, create_model from pytest_mock import MockerFixture from ruamel.yaml import YAML from typer.testing import CliRunner @@ -13,7 +14,12 @@ from fastcs.controllers import Controller from fastcs.datatypes import Int from fastcs.exceptions import LaunchError -from fastcs.launch import _launch, get_controller_schema, launch +from fastcs.launch import ( + _build_options_model, + _launch, + get_controller_schema, + launch, +) from fastcs.transports import Transport @@ -22,6 +28,11 @@ class SomeConfig: name: str +@dataclass +class OtherConfig: + address: str + + class SingleArg(Controller): def __init__(self): super().__init__() @@ -44,14 +55,32 @@ def __init__(self, arg: SomeConfig, too_many): super().__init__() +class OtherHinted(Controller): + def __init__(self, arg: OtherConfig) -> None: + super().__init__() + + +class Aliased(Controller): + type_name: ClassVar[str] = "aliased-controller" + + def __init__(self, arg: SomeConfig) -> None: + super().__init__() + + runner = CliRunner() def test_single_arg_schema(): + entry_model = create_model( + "SingleArgEntry", + __config__={"extra": "forbid"}, + type=(Literal["SingleArg"], "SingleArg"), + ) target_model = create_model( "SingleArg", - transport=(list[Transport.union()], ...), __config__={"extra": "forbid"}, + controllers=(dict[str, entry_model], ...), + transport=(list[Transport.union()], ...), ) target_dict = target_model.model_json_schema() @@ -64,11 +93,17 @@ def test_single_arg_schema(): def test_is_hinted_schema(data): + entry_model = create_model( + "IsHintedEntry", + __config__={"extra": "forbid"}, + type=(Literal["IsHinted"], "IsHinted"), + controller=(SomeConfig, ...), + ) target_model = create_model( "IsHinted", - controller=(SomeConfig, ...), - transport=(list[Transport.union()], ...), __config__={"extra": "forbid"}, + controllers=(dict[str, entry_model], ...), + transport=(list[Transport.union()], ...), ) target_dict = target_model.model_json_schema() @@ -79,10 +114,6 @@ def test_is_hinted_schema(data): assert result_dict == target_dict - # # store a schema to use for debugging - # with open(data / "schema.json", mode="w") as f: - # json.dump(result_dict, f, indent=2) - def test_not_hinted_schema(): error = ( @@ -162,3 +193,110 @@ def test_error_if_identical_context_in_transports(mocker: MockerFixture, data): result = runner.invoke(app, ["run", str(data / "config.yaml")]) assert isinstance(result.exception, RuntimeError) assert "Duplicate context keys found" in result.exception.args[0] + + +def _controllers(instance) -> dict: + """Read the dynamically-defined `controllers` mapping off a validated model.""" + return instance.controllers # type: ignore[attr-defined] + + +def test_single_class_omits_type(): + """Single-class registration may omit `type:` under each controller entry.""" + options_model = _build_options_model([IsHinted]) + instance = options_model.model_validate( + { + "controllers": {"my-id": {"controller": {"name": "x"}}}, + "transport": [{"rest": {}}], + } + ) + entry = _controllers(instance)["my-id"] + assert entry.type == "IsHinted" + assert entry.controller.name == "x" + + +def test_multi_class_discriminator(): + """Multi-class registration uses `type:` to pick the matching entry.""" + options_model = _build_options_model([IsHinted, OtherHinted]) + instance = options_model.model_validate( + { + "controllers": { + "first": {"type": "IsHinted", "controller": {"name": "a"}}, + "second": {"type": "OtherHinted", "controller": {"address": "b"}}, + }, + "transport": [{"rest": {}}], + } + ) + + first = _controllers(instance)["first"] + second = _controllers(instance)["second"] + assert first.type == "IsHinted" + assert isinstance(first.controller, SomeConfig) + assert second.type == "OtherHinted" + assert isinstance(second.controller, OtherConfig) + + +def test_multi_class_unknown_type_rejected(): + options_model = _build_options_model([IsHinted, OtherHinted]) + with pytest.raises(ValidationError): + options_model.model_validate( + { + "controllers": {"x": {"type": "Unknown", "controller": {"name": "a"}}}, + "transport": [{"rest": {}}], + } + ) + + +def test_type_name_override(): + """`type_name: ClassVar[str]` overrides the default `__name__` discriminator.""" + options_model = _build_options_model([Aliased, OtherHinted]) + instance = options_model.model_validate( + { + "controllers": { + "x": {"type": "aliased-controller", "controller": {"name": "n"}}, + }, + "transport": [{"rest": {}}], + } + ) + assert _controllers(instance)["x"].type == "aliased-controller" + + +def test_duplicate_id_rejected_at_yaml_load(tmp_path): + """ruamel YAML rejects duplicate mapping keys, so duplicate ids cannot + survive parsing; this is the natural source of duplicate-id rejection.""" + cfg = tmp_path / "dup.yaml" + cfg.write_text( + "controllers:\n" + " same:\n" + " controller: {name: a}\n" + " same:\n" + " controller: {name: b}\n" + "transport:\n" + " - rest: {}\n" + ) + yaml = YAML(typ="safe") + with pytest.raises(Exception, match="duplicate key"): + yaml.load(cfg) + + +def test_multi_controller_run_errors_until_fastcs_supports_list( + mocker: MockerFixture, tmp_path +): + """Until FastCS itself takes list[Controller] in the next slice, running a + multi-entry config raises a clear LaunchError.""" + mocker.patch("fastcs.launch.FastCS.run") + cfg = tmp_path / "multi.yaml" + cfg.write_text( + "controllers:\n" + " one:\n" + " type: IsHinted\n" + " controller: {name: a}\n" + " two:\n" + " type: OtherHinted\n" + " controller: {address: b}\n" + "transport:\n" + " - rest: {}\n" + ) + app = _launch([IsHinted, OtherHinted]) + result = runner.invoke(app, ["run", str(cfg)]) + assert isinstance(result.exception, LaunchError) + assert "Multi-controller execution" in str(result.exception) From d4ff267e78c9a2626c2f5d95d7e21bd091c06bb2 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 14:58:12 +0100 Subject: [PATCH 07/16] Wire FastCS multi-controller end-to-end (REST) FastCS.__init__ now accepts Controller | Sequence[Controller]; serve() loops initialise/post_initialise/connect/disconnect over every controller, builds list[ControllerAPI], and hands the full list to each transport.connect(). IPython context exposes parallel dicts (controllers, controller_apis) keyed by controller id (falling back to class name when no id is set), and the startup log line lists controller ids. fastcs.controller_api singular accessor is replaced with the controller_apis list. The temporary >1-controller LaunchError stub in launch._launch.run is removed; multi-entry configs are wired through FastCS directly. Single Controller direct construction continues to work via the union arg, so docs snippets are unchanged. A new end-to-end test in tests/test_multi_controller.py drives FastCS.serve with two id-tagged controllers and a RestTransport, asserts all four lifecycle hooks fire on each, and verifies // routing plus combined OpenAPI through TestClient. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/control_system.py | 78 ++++++++++++++++++++++++++-------- src/fastcs/launch.py | 9 +--- tests/test_control_system.py | 4 +- tests/test_launch.py | 19 +++++---- tests/test_multi_controller.py | 75 +++++++++++++++++++++++++++++++- 5 files changed, 148 insertions(+), 37 deletions(-) diff --git a/src/fastcs/control_system.py b/src/fastcs/control_system.py index 209147cf..cc5e5c82 100644 --- a/src/fastcs/control_system.py +++ b/src/fastcs/control_system.py @@ -7,7 +7,7 @@ from IPython.terminal.embed import InteractiveShellEmbed -from fastcs.controllers import Controller +from fastcs.controllers import Controller, ControllerAPI from fastcs.logging import logger from fastcs.methods import ScanCallback from fastcs.tracer import Tracer @@ -16,25 +16,49 @@ tracer = Tracer() +def _context_key(controller: Controller) -> str: + """Key used for a controller in IPython context dicts. + + Falls back to the class name when no id has been set so direct-construction + callers (without launch()) still get a sensible key. + """ + try: + return controller.id + except RuntimeError: + return controller.__class__.__name__ + + class FastCS: """Entrypoint for a FastCS application. - This class takes a `Controller`, creates asyncio tasks to run its update loops and - builds its API to serve over the given `Transport`s. + This class takes one or more `Controller`s, creates asyncio tasks to run their + update loops and builds their APIs to serve over the given `Transport`s. Args: - controller: The controller to serve in the control system + controllers: The controller(s) to serve in the control system. Accepts + either a single ``Controller`` or a sequence of them. transports: A list of transports to serve the API over loop: Optional event loop to run the control system in """ def __init__( self, - controller: Controller, + controllers: Controller | Sequence[Controller], transports: Sequence[Transport], loop: asyncio.AbstractEventLoop | None = None, ): - self._controller = controller + if isinstance(controllers, Controller): + controllers = [controllers] + self._controllers: list[Controller] = list(controllers) + if not self._controllers: + raise ValueError("FastCS requires at least one controller") + keys = [_context_key(c) for c in self._controllers] + if len(set(keys)) != len(keys): + duplicates = sorted({k for k in keys if keys.count(k) > 1}) + raise ValueError( + f"FastCS received controllers with duplicate context keys " + f"{duplicates}; ensure each controller has a unique id" + ) self._transports = transports self._loop = loop or asyncio.get_event_loop() @@ -42,6 +66,7 @@ def __init__( self._initial_coros: list[ScanCallback] = [] self._scan_tasks: set[asyncio.Task] = set() + self.controller_apis: list[ControllerAPI] = [] def run(self, interactive: bool = True): """Run the application @@ -93,16 +118,25 @@ async def serve(self, interactive: bool = True) -> None: interactive: Whether to create an interactive IPython shell """ - await self._controller.initialise() - self._controller.post_initialise() - - self.controller_api, self._scan_coros, self._initial_coros = ( - self._controller.create_api_and_tasks() - ) + for controller in self._controllers: + await controller.initialise() + controller.post_initialise() + + self.controller_apis = [] + self._scan_coros = [] + self._initial_coros = [] + for controller in self._controllers: + api, scan_coros, initial_coros = controller.create_api_and_tasks() + self.controller_apis.append(api) + self._scan_coros.extend(scan_coros) + self._initial_coros.extend(initial_coros) context = { - "controller": self._controller, - "controller_api": self.controller_api, + "controllers": {_context_key(c): c for c in self._controllers}, + "controller_apis": { + _context_key(c): api + for c, api in zip(self._controllers, self.controller_apis, strict=True) + }, "transports": [ transport.__class__.__name__ for transport in self._transports ], @@ -110,7 +144,7 @@ async def serve(self, interactive: bool = True) -> None: coros: list[Coroutine] = [] for transport in self._transports: - transport.connect(controller_apis=[self.controller_api], loop=self._loop) + transport.connect(controller_apis=self.controller_apis, loop=self._loop) coros.append(transport.serve()) common_context = context.keys() & transport.context.keys() if common_context: @@ -134,11 +168,12 @@ async def block_forever(): logger.info( "Starting FastCS", - controller=self._controller, + controllers=[_context_key(c) for c in self._controllers], transports=f"[{', '.join(str(t) for t in self._transports)}]", ) - await self._controller.connect() + for controller in self._controllers: + await controller.connect() await self._run_initial_coros() await self._start_scan_tasks() @@ -151,7 +186,14 @@ async def block_forever(): finally: logger.info("Shutting down FastCS") self._stop_scan_tasks() - await self._controller.disconnect() + for controller in self._controllers: + try: + await controller.disconnect() + except Exception: + logger.exception( + "Exception during disconnect", + controller=_context_key(controller), + ) async def _interactive_shell(self, context: dict[str, Any]): """Spawn interactive shell in another thread and wait for it to complete.""" diff --git a/src/fastcs/launch.py b/src/fastcs/launch.py index cf6a8674..f6232dbd 100644 --- a/src/fastcs/launch.py +++ b/src/fastcs/launch.py @@ -170,15 +170,8 @@ def run( controllers = _instantiate_controllers(instance_options.controllers, type_map) - if len(controllers) > 1: - raise LaunchError( - "Multi-controller execution is not yet wired through FastCS; " - "this lands in the next slice of issue #353. " - "Configure exactly one entry under `controllers:` for now." - ) - instance = FastCS( - controllers[0], + controllers, instance_options.transport, loop=asyncio.get_event_loop(), ) diff --git a/tests/test_control_system.py b/tests/test_control_system.py index c231b136..ca151cc0 100644 --- a/tests/test_control_system.py +++ b/tests/test_control_system.py @@ -53,8 +53,8 @@ async def do_nothing_static(self): await controller.do_nothing_static() await controller.do_nothing_dynamic() - await fastcs.controller_api.command_methods["do_nothing_static"]() - await fastcs.controller_api.command_methods["do_nothing_dynamic"]() + await fastcs.controller_apis[0].command_methods["do_nothing_static"]() + await fastcs.controller_apis[0].command_methods["do_nothing_dynamic"]() @pytest.mark.asyncio diff --git a/tests/test_launch.py b/tests/test_launch.py index 681d8263..df9dc222 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -11,6 +11,7 @@ from fastcs import __version__ from fastcs.attributes import AttrR +from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.datatypes import Int from fastcs.exceptions import LaunchError @@ -179,7 +180,7 @@ def test_error_if_identical_context_in_transports(mocker: MockerFixture, data): mocker.patch( "fastcs.transports.Transport.context", new_callable=mocker.PropertyMock, - return_value={"controller": "test"}, + return_value={"controllers": "test"}, ) mocker.patch( "fastcs.transports.epics.pva.transport.EpicsPVATransport.serve", @@ -278,11 +279,10 @@ def test_duplicate_id_rejected_at_yaml_load(tmp_path): yaml.load(cfg) -def test_multi_controller_run_errors_until_fastcs_supports_list( - mocker: MockerFixture, tmp_path -): - """Until FastCS itself takes list[Controller] in the next slice, running a - multi-entry config raises a clear LaunchError.""" +def test_multi_controller_run_reaches_fastcs(mocker: MockerFixture, tmp_path): + """Multi-entry config is wired through FastCS, which receives both + controllers in the order they appear under `controllers:`.""" + init_spy = mocker.spy(FastCS, "__init__") mocker.patch("fastcs.launch.FastCS.run") cfg = tmp_path / "multi.yaml" cfg.write_text( @@ -298,5 +298,8 @@ def test_multi_controller_run_errors_until_fastcs_supports_list( ) app = _launch([IsHinted, OtherHinted]) result = runner.invoke(app, ["run", str(cfg)]) - assert isinstance(result.exception, LaunchError) - assert "Multi-controller execution" in str(result.exception) + assert result.exit_code == 0, result.output + init_spy.assert_called_once() + controllers_arg = init_spy.call_args.args[1] + assert [c.id for c in controllers_arg] == ["one", "two"] + assert [type(c) for c in controllers_arg] == [IsHinted, OtherHinted] diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 6caed608..d468cffd 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -1,15 +1,18 @@ """Tests for the multi-controller foundation slice (#353). These tests exercise the public Controller.id lifecycle and -multi-controller REST routing through RestTransport. +multi-controller REST routing through RestTransport, up to the +end-to-end FastCS.serve() lifecycle with two controllers. """ import asyncio import pytest from fastapi.testclient import TestClient +from pytest_mock import MockerFixture from fastcs.attributes import AttrR +from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.datatypes import Int from fastcs.transports.rest.transport import RestTransport @@ -98,3 +101,73 @@ def test_rest_transport_rejects_illegal_id_at_connect(): transport.connect([api], loop) finally: loop.close() + + +class _LifecycleController(Controller): + """Records lifecycle hook calls for end-to-end assertions.""" + + foo = AttrR(Int()) + + def __init__(self): + super().__init__() + self.connected = False + self.initialised = False + self.post_initialised = False + + async def initialise(self): + self.initialised = True + + def post_initialise(self): + self.post_initialised = True + + async def connect(self): + self.connected = True + + async def disconnect(self): + self.connected = False + + +class _OtherLifecycleController(_LifecycleController): + bar = AttrR(Int()) + + +@pytest.mark.asyncio +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_id("alpha") + b = _OtherLifecycleController() + b.set_id("beta") + + transport = RestTransport() + # Stop RestTransport from binding to a real port; we exercise the FastAPI + # app directly through TestClient. + mocker.patch.object(RestTransport, "serve", new=lambda self: asyncio.sleep(3600)) + + fastcs = FastCS([a, b], [transport], asyncio.get_event_loop()) + task = asyncio.create_task(fastcs.serve(interactive=False)) + try: + await asyncio.sleep(0.1) + + for controller in (a, b): + assert controller.initialised + assert controller.post_initialised + assert controller.connected + + with TestClient(transport._server._app) as client: + assert client.get("/alpha/foo").status_code == 200 + assert client.get("/beta/foo").status_code == 200 + assert client.get("/beta/bar").status_code == 200 + + openapi = client.get("/openapi.json").json() + paths = set(openapi["paths"]) + assert "/alpha/foo" in paths + assert "/beta/foo" in paths + assert "/beta/bar" in paths + finally: + task.cancel() + await asyncio.sleep(0.1) + + for controller in (a, b): + assert not controller.connected From c8adb33bc29483b4b3f1a93f83ac84bf02e1161b Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 15:38:39 +0100 Subject: [PATCH 08/16] EPICS CA multi-root softioc with id-based PV prefix EpicsCATransport now hosts every configured controller in a single softioc, with each controller's id used verbatim as its PV prefix. EpicsCAIOC takes list[ControllerAPI] and loops the existing record/PVI/command builders per controller, deriving each prefix from pv_prefix_from_path(api.path) (the D2 utility introduced in #353). EpicsCATransport.connect drops _expect_single in favour of true multi-controller; validate_ca_id runs at connect time and rejects ids with illegal characters as well as setups whose longest derivable PV prefix already exceeds the 60-character EPICS limit. EpicsIOCOptions and its pv_prefix field are deleted. EpicsCAOptions and EpicsPVAOptions empty placeholders preserve epicsca: / epicspva: as fastcs.yaml discriminator keys (Pydantic union resolution is positional, so a unique field name per transport is still load-bearing). EpicsGUI no longer takes a separate prefix argument; PVs derive from the controller path. PVA temporarily continues via _expect_single but adopts pv_prefix_from_path so it gets the same id-based prefix and no longer needs EpicsIOCOptions; full PVA multi-root work lands in #355. tests/test_multi_controller.py grows a CA two-controllers-no-clash scenario and a CA id-validation fail-fast case. tests/example_softioc, tests/example_p4p_ioc, tests/benchmarking/controller, tests/conftest, test_initial_value, test_p4p, test_softioc, test_gui, test_pva_gui and the AssertableControllerAPI fixture all migrate to id-based naming (controllers set their id, transports take no prefix). Demo controller.yaml and both regenerated schema.json files reflect the new EpicsCAOptions / EpicsPVAOptions schemas and removal of pv_prefix. The 13 docs/snippets are exercised by tests/test_docs_snippets.py via runpy, so they migrate in this commit too to keep the suite green at every commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/snippets/dynamic.py | 3 +- docs/snippets/static04.py | 3 +- docs/snippets/static05.py | 4 +- docs/snippets/static06.py | 4 +- docs/snippets/static07.py | 4 +- docs/snippets/static08.py | 4 +- docs/snippets/static09.py | 4 +- docs/snippets/static10.py | 4 +- docs/snippets/static11.py | 4 +- docs/snippets/static12.py | 4 +- docs/snippets/static13.py | 4 +- docs/snippets/static14.py | 4 +- docs/snippets/static15.py | 4 +- src/fastcs/demo/controller.yaml | 3 +- src/fastcs/demo/schema.json | 21 ++++---- src/fastcs/transports/__init__.py | 3 +- src/fastcs/transports/epics/__init__.py | 3 +- src/fastcs/transports/epics/ca/ioc.py | 54 +++++++------------ src/fastcs/transports/epics/ca/transport.py | 32 ++++++----- src/fastcs/transports/epics/ca/util.py | 32 +++++++++++ src/fastcs/transports/epics/gui.py | 8 ++- src/fastcs/transports/epics/options.py | 17 ++++-- src/fastcs/transports/epics/pva/ioc.py | 15 +++--- src/fastcs/transports/epics/pva/transport.py | 12 +++-- src/fastcs/transports/epics/util.py | 5 -- tests/assertable_controller.py | 10 +++- tests/benchmarking/controller.py | 9 ++-- tests/conftest.py | 2 +- tests/data/schema.json | 21 ++++---- tests/example_p4p_ioc.py | 6 +-- tests/example_softioc.py | 10 ++-- tests/test_multi_controller.py | 54 +++++++++++++++++-- tests/transports/epics/ca/test_ca_util.py | 24 +++++++++ tests/transports/epics/ca/test_gui.py | 45 ++++++++-------- .../transports/epics/ca/test_initial_value.py | 4 +- tests/transports/epics/ca/test_softioc.py | 27 +++++----- tests/transports/epics/pva/test_p4p.py | 6 +-- tests/transports/epics/pva/test_pva_gui.py | 26 ++++----- 38 files changed, 296 insertions(+), 203 deletions(-) diff --git a/docs/snippets/dynamic.py b/docs/snippets/dynamic.py index 65d58497..24c93649 100644 --- a/docs/snippets/dynamic.py +++ b/docs/snippets/dynamic.py @@ -16,7 +16,6 @@ from fastcs.controllers import Controller from fastcs.datatypes import Bool, DataType, Float, Int, String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca import EpicsCATransport @@ -139,7 +138,7 @@ async def initialise(self): await self._connection.close() -epics_ca = EpicsCATransport(epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport() connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static04.py b/docs/snippets/static04.py index 6200b78f..b605d54d 100644 --- a/docs/snippets/static04.py +++ b/docs/snippets/static04.py @@ -2,7 +2,6 @@ from fastcs.controllers import Controller from fastcs.datatypes import String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.transport import EpicsCATransport @@ -10,7 +9,7 @@ class TemperatureController(Controller): device_id = AttrR(String()) -epics_ca = EpicsCATransport(epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport() fastcs = FastCS(TemperatureController(), [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static05.py b/docs/snippets/static05.py index 1a395197..29ed005e 100644 --- a/docs/snippets/static05.py +++ b/docs/snippets/static05.py @@ -4,7 +4,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport @@ -15,7 +15,7 @@ class TemperatureController(Controller): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) fastcs = FastCS(TemperatureController(), [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static06.py b/docs/snippets/static06.py index 4cd77fdd..16fac5d2 100644 --- a/docs/snippets/static06.py +++ b/docs/snippets/static06.py @@ -5,7 +5,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport @@ -25,7 +25,7 @@ async def connect(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static07.py b/docs/snippets/static07.py index 12b7333b..84f9fdd5 100644 --- a/docs/snippets/static07.py +++ b/docs/snippets/static07.py @@ -7,7 +7,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -47,7 +47,7 @@ async def connect(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static08.py b/docs/snippets/static08.py index beb0f49b..62151342 100644 --- a/docs/snippets/static08.py +++ b/docs/snippets/static08.py @@ -7,7 +7,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import Float, String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -53,7 +53,7 @@ async def connect(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static09.py b/docs/snippets/static09.py index dbe6a5ee..c72fe78f 100644 --- a/docs/snippets/static09.py +++ b/docs/snippets/static09.py @@ -7,7 +7,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import Float, String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -60,7 +60,7 @@ async def connect(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static10.py b/docs/snippets/static10.py index 5da5add7..76b40c58 100644 --- a/docs/snippets/static10.py +++ b/docs/snippets/static10.py @@ -7,7 +7,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import Float, Int, String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -78,7 +78,7 @@ async def connect(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static11.py b/docs/snippets/static11.py index beba164e..430e7d45 100644 --- a/docs/snippets/static11.py +++ b/docs/snippets/static11.py @@ -8,7 +8,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import Enum, Float, Int, String from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -85,7 +85,7 @@ async def connect(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static12.py b/docs/snippets/static12.py index 2b668166..6edb3313 100644 --- a/docs/snippets/static12.py +++ b/docs/snippets/static12.py @@ -10,7 +10,7 @@ from fastcs.datatypes import Enum, Float, Int, String from fastcs.launch import FastCS from fastcs.methods import scan -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -98,7 +98,7 @@ async def update_voltages(self): gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static13.py b/docs/snippets/static13.py index b454288e..5a7cb0a4 100644 --- a/docs/snippets/static13.py +++ b/docs/snippets/static13.py @@ -11,7 +11,7 @@ from fastcs.datatypes import Enum, Float, Int, String from fastcs.launch import FastCS from fastcs.methods import command, scan -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -106,7 +106,7 @@ async def disable_all(self) -> None: gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static14.py b/docs/snippets/static14.py index fb794b19..e928f7fc 100644 --- a/docs/snippets/static14.py +++ b/docs/snippets/static14.py @@ -12,7 +12,7 @@ from fastcs.launch import FastCS from fastcs.logging import configure_logging, logger from fastcs.methods import command, scan -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -112,7 +112,7 @@ async def disable_all(self) -> None: gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static15.py b/docs/snippets/static15.py index 0246d2cb..ac8350fe 100644 --- a/docs/snippets/static15.py +++ b/docs/snippets/static15.py @@ -12,7 +12,7 @@ from fastcs.launch import FastCS from fastcs.logging import LogLevel, configure_logging, logger from fastcs.methods import command, scan -from fastcs.transports.epics import EpicsGUIOptions, EpicsIOCOptions +from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport NumberT = TypeVar("NumberT", int, float) @@ -115,7 +115,7 @@ async def disable_all(self) -> None: gui_options = EpicsGUIOptions( output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" ) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/src/fastcs/demo/controller.yaml b/src/fastcs/demo/controller.yaml index fe75dfe3..e5c61368 100644 --- a/src/fastcs/demo/controller.yaml +++ b/src/fastcs/demo/controller.yaml @@ -11,8 +11,7 @@ transport: host: localhost port: 8083 log_level: info - - epicsca: - pv_prefix: GARYDEMO + - epicsca: {} gui: title: Temperature Controller Demo output_path: ./demo.bob diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index 17f173d2..e0a6b845 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -1,9 +1,14 @@ { "$defs": { + "EpicsCAOptions": { + "properties": {}, + "title": "EpicsCAOptions", + "type": "object" + }, "EpicsCATransport": { "properties": { "epicsca": { - "$ref": "#/$defs/EpicsIOCOptions" + "$ref": "#/$defs/EpicsCAOptions" }, "docs": { "anyOf": [ @@ -85,21 +90,15 @@ "title": "EpicsGUIOptions", "type": "object" }, - "EpicsIOCOptions": { - "properties": { - "pv_prefix": { - "default": "MY-DEVICE-PREFIX", - "title": "Pv Prefix", - "type": "string" - } - }, - "title": "EpicsIOCOptions", + "EpicsPVAOptions": { + "properties": {}, + "title": "EpicsPVAOptions", "type": "object" }, "EpicsPVATransport": { "properties": { "epicspva": { - "$ref": "#/$defs/EpicsIOCOptions" + "$ref": "#/$defs/EpicsPVAOptions" }, "docs": { "anyOf": [ diff --git a/src/fastcs/transports/__init__.py b/src/fastcs/transports/__init__.py index c5bc22b5..835ada3e 100644 --- a/src/fastcs/transports/__init__.py +++ b/src/fastcs/transports/__init__.py @@ -2,9 +2,10 @@ try: from .epics.ca.transport import EpicsCATransport as EpicsCATransport + from .epics.options import EpicsCAOptions as EpicsCAOptions from .epics.options import EpicsDocsOptions as EpicsDocsOptions from .epics.options import EpicsGUIOptions as EpicsGUIOptions - from .epics.options import EpicsIOCOptions as EpicsIOCOptions + from .epics.options import EpicsPVAOptions as EpicsPVAOptions except ImportError: pass diff --git a/src/fastcs/transports/epics/__init__.py b/src/fastcs/transports/epics/__init__.py index 0dad2e91..37d9c8f5 100644 --- a/src/fastcs/transports/epics/__init__.py +++ b/src/fastcs/transports/epics/__init__.py @@ -1,4 +1,5 @@ from .docs import EpicsDocsOptions as EpicsDocsOptions +from .options import EpicsCAOptions as EpicsCAOptions from .options import EpicsGUIFormat as EpicsGUIFormat from .options import EpicsGUIOptions as EpicsGUIOptions -from .options import EpicsIOCOptions as EpicsIOCOptions +from .options import EpicsPVAOptions as EpicsPVAOptions diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 3f510a1b..dfa4ab5f 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -12,34 +12,30 @@ from fastcs.methods import Command from fastcs.tracer import Tracer from fastcs.transports.epics.ca.util import ( + EPICS_MAX_NAME_LENGTH, _make_in_record, _make_out_record, cast_from_epics_type, cast_to_epics_type, ) -from fastcs.transports.epics.util import controller_pv_prefix +from fastcs.transports.epics.util import pv_prefix_from_path from fastcs.util import snake_to_pascal -EPICS_MAX_NAME_LENGTH = 60 - - tracer = Tracer() class EpicsCAIOC: - """A softioc which handles a controller""" + """A softioc which handles one or more controllers.""" - def __init__( - self, - pv_prefix: str, - controller_api: ControllerAPI, - ): - self._controller_api = controller_api - _add_pvi_info(f"{pv_prefix}:PVI") - _add_sub_controller_pvi_info(pv_prefix, controller_api) + def __init__(self, controller_apis: list[ControllerAPI]): + self._controller_apis = controller_apis + for controller_api in controller_apis: + root_pv_prefix = pv_prefix_from_path(controller_api.path) + _add_pvi_info(f"{root_pv_prefix}:PVI") + _add_sub_controller_pvi_info(controller_api) - _create_and_link_attribute_pvs(pv_prefix, controller_api) - _create_and_link_command_pvs(pv_prefix, controller_api) + _create_and_link_attribute_pvs(controller_api) + _create_and_link_command_pvs(controller_api) def run( self, @@ -95,18 +91,12 @@ def _add_pvi_info( record.add_info("Q:group", q_group) -def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): - """Add PVI references from controller to its sub controllers, recursively. - - Args: - pv_prefix: PV Prefix of IOC - parent: Controller to add PVI refs for - - """ - parent_pvi = f"{controller_pv_prefix(pv_prefix, parent)}:PVI" +def _add_sub_controller_pvi_info(parent: ControllerAPI): + """Add PVI references from controller to its sub controllers, recursively.""" + parent_pvi = f"{pv_prefix_from_path(parent.path)}:PVI" for child in parent.sub_apis.values(): - child_pvi = f"{controller_pv_prefix(pv_prefix, child)}:PVI" + child_pvi = f"{pv_prefix_from_path(child.path)}:PVI" child_name = ( f"__{child.path[-1]}" # Sub-Controller of ControllerVector if child.path[-1].isdigit() @@ -115,14 +105,12 @@ def _add_sub_controller_pvi_info(pv_prefix: str, parent: ControllerAPI): _add_pvi_info(child_pvi, parent_pvi, child_name.lower()) - _add_sub_controller_pvi_info(pv_prefix, child) + _add_sub_controller_pvi_info(child) -def _create_and_link_attribute_pvs( - root_pv_prefix: str, root_controller_api: ControllerAPI -) -> None: +def _create_and_link_attribute_pvs(root_controller_api: ControllerAPI) -> None: for controller_api in root_controller_api.walk_api(): - pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) + pv_prefix = pv_prefix_from_path(controller_api.path) for attr_name, attribute in controller_api.attributes.items(): if ( @@ -210,11 +198,9 @@ async def set_setpoint_without_process(value: DType_T): attribute.add_sync_setpoint_callback(set_setpoint_without_process) -def _create_and_link_command_pvs( - root_pv_prefix: str, root_controller_api: ControllerAPI -) -> None: +def _create_and_link_command_pvs(root_controller_api: ControllerAPI) -> None: for controller_api in root_controller_api.walk_api(): - pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) + pv_prefix = pv_prefix_from_path(controller_api.path) for attr_name, method in controller_api.command_methods.items(): pv_name = snake_to_pascal(attr_name) diff --git a/src/fastcs/transports/epics/ca/transport.py b/src/fastcs/transports/epics/ca/transport.py index 8d65a376..6728dc77 100644 --- a/src/fastcs/transports/epics/ca/transport.py +++ b/src/fastcs/transports/epics/ca/transport.py @@ -7,22 +7,24 @@ from fastcs.controllers import ControllerAPI from fastcs.logging import logger from fastcs.transports.epics import ( + EpicsCAOptions, EpicsDocsOptions, EpicsGUIOptions, - EpicsIOCOptions, ) from fastcs.transports.epics.ca.ioc import EpicsCAIOC +from fastcs.transports.epics.ca.util import validate_ca_id from fastcs.transports.epics.docs import EpicsDocs from fastcs.transports.epics.gui import EpicsGUI -from fastcs.transports.transport import Transport, _expect_single +from fastcs.transports.epics.util import pv_prefix_from_path +from fastcs.transports.transport import Transport @dataclass class EpicsCATransport(Transport): """Channel access transport.""" - epicsca: EpicsIOCOptions = field(default_factory=EpicsIOCOptions) - """Options for the IOC""" + epicsca: EpicsCAOptions = field(default_factory=EpicsCAOptions) + """CA-specific options. Currently empty; present as the YAML discriminator.""" docs: EpicsDocsOptions | None = None """Options for the docs""" gui: EpicsGUIOptions | None = None @@ -33,21 +35,23 @@ def connect( controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ) -> None: - controller_api = _expect_single(controller_apis, "EpicsCATransport") - self._controller_api = controller_api + for api in controller_apis: + validate_ca_id(api) + self._controller_apis = controller_apis self._loop = loop - self._pv_prefix = self.epicsca.pv_prefix - self._ioc = EpicsCAIOC(self.epicsca.pv_prefix, controller_api) + self._pv_prefixes = [pv_prefix_from_path(api.path) for api in controller_apis] + self._ioc = EpicsCAIOC(controller_apis) - if self.docs is not None: - EpicsDocs(self._controller_api).create_docs(self.docs) + for api in controller_apis: + if self.docs is not None: + EpicsDocs(api).create_docs(self.docs) - if self.gui is not None: - EpicsGUI(self._controller_api, self._pv_prefix).create_gui(self.gui) + if self.gui is not None: + EpicsGUI(api).create_gui(self.gui) async def serve(self) -> None: """Serve `ControllerAPI` over EPICS Channel Access""" - logger.info("Running IOC", pv_prefix=self._pv_prefix) + logger.info("Running IOC", pv_prefixes=self._pv_prefixes) self._ioc.run(self._loop) @property @@ -60,4 +64,4 @@ def context(self) -> dict[str, Any]: } def __repr__(self): - return f"EpicsCATransport({self._pv_prefix})" + return f"EpicsCATransport({self._pv_prefixes})" diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index b5892a47..0aeae58d 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -1,4 +1,5 @@ import enum +import re from collections.abc import Callable from dataclasses import asdict from typing import Any @@ -7,8 +8,39 @@ from softioc.pythonSoftIoc import RecordWrapper from fastcs.attributes import AttrR, AttrRW, AttrW +from fastcs.controllers import ControllerAPI from fastcs.datatypes import Bool, DataType, DType_T, Enum, Float, Int, String, Waveform from fastcs.exceptions import FastCSError +from fastcs.transports.epics.util import pv_prefix_from_path + +EPICS_MAX_NAME_LENGTH = 60 + +_CA_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + +def validate_ca_id(controller_api: ControllerAPI) -> None: + """Reject controller ids that wouldn't be safe in an EPICS CA PV name. + + Rejects ids with characters outside ``[A-Za-z0-9_-]`` and rejects setups + where the longest derivable PV prefix already exceeds the 60-character + EPICS PV name limit. + """ + id = controller_api.path[0] + if not _CA_ID_RE.fullmatch(id): + raise ValueError( + f"Controller id {id!r} is not a valid EPICS CA id; " + "only alphanumerics, '-' and '_' are allowed" + ) + longest_prefix = max( + len(pv_prefix_from_path(api.path)) for api in controller_api.walk_api() + ) + if longest_prefix > EPICS_MAX_NAME_LENGTH: + raise ValueError( + f"Controller id {id!r} produces a PV prefix of " + f"{longest_prefix} characters, which exceeds the EPICS " + f"{EPICS_MAX_NAME_LENGTH}-character PV name limit" + ) + _MBB_FIELD_PREFIXES = ( "ZR", diff --git a/src/fastcs/transports/epics/gui.py b/src/fastcs/transports/epics/gui.py index fbdf336d..6e79ea09 100644 --- a/src/fastcs/transports/epics/gui.py +++ b/src/fastcs/transports/epics/gui.py @@ -36,6 +36,7 @@ from fastcs.logging import logger from fastcs.methods import Command from fastcs.transports.epics.options import EpicsGUIFormat, EpicsGUIOptions +from fastcs.transports.epics.util import pv_prefix_from_path from fastcs.util import snake_to_pascal @@ -44,14 +45,11 @@ class EpicsGUI: command_value = "1" - def __init__(self, controller_api: ControllerAPI, pv_prefix: str) -> None: + def __init__(self, controller_api: ControllerAPI) -> None: self._controller_api = controller_api - self._pv_prefix = pv_prefix def _get_pv(self, attr_path: list[str], name: str): - attr_prefix = ":".join( - [self._pv_prefix] + [snake_to_pascal(node) for node in attr_path] - ) + attr_prefix = pv_prefix_from_path(attr_path) return f"{attr_prefix}:{snake_to_pascal(name)}" def _get_read_widget(self, attribute: Attribute) -> ReadWidgetUnion | None: diff --git a/src/fastcs/transports/epics/options.py b/src/fastcs/transports/epics/options.py index 029d634c..f0e37ddf 100644 --- a/src/fastcs/transports/epics/options.py +++ b/src/fastcs/transports/epics/options.py @@ -28,7 +28,18 @@ class EpicsGUIOptions: @dataclass -class EpicsIOCOptions: - """Epics IOC options for use in both CA and PVA transports.""" +class EpicsCAOptions: + """Channel-Access-specific options. - pv_prefix: str = "MY-DEVICE-PREFIX" + Currently empty: present so ``epicsca:`` survives in fastcs.yaml as the + transport discriminator key. Reserved for future CA-specific knobs. + """ + + +@dataclass +class EpicsPVAOptions: + """PVAccess-specific options. + + Currently empty: present so ``epicspva:`` survives in fastcs.yaml as the + transport discriminator key. Reserved for future PVA-specific knobs. + """ diff --git a/src/fastcs/transports/epics/pva/ioc.py b/src/fastcs/transports/epics/pva/ioc.py index 0c7ea4b7..4d3fada0 100644 --- a/src/fastcs/transports/epics/pva/ioc.py +++ b/src/fastcs/transports/epics/pva/ioc.py @@ -4,21 +4,19 @@ from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controllers import ControllerAPI -from fastcs.transports.epics.util import controller_pv_prefix +from fastcs.transports.epics.util import pv_prefix_from_path from fastcs.util import snake_to_pascal from ._pv_handlers import make_command_pv, make_shared_read_pv, make_shared_write_pv from .pvi import add_pvi_info -async def parse_attributes( - root_pv_prefix: str, root_controller_api: ControllerAPI -) -> StaticProvider: +async def parse_attributes(root_controller_api: ControllerAPI) -> StaticProvider: """Parses `Attribute` s into p4p signals in handlers.""" - provider = StaticProvider(root_pv_prefix) + provider = StaticProvider(pv_prefix_from_path(root_controller_api.path)) for controller_api in root_controller_api.walk_api(): - pv_prefix = controller_pv_prefix(root_pv_prefix, controller_api) + pv_prefix = pv_prefix_from_path(controller_api.path) add_pvi_info( provider=provider, pv_prefix=pv_prefix, @@ -52,12 +50,11 @@ async def parse_attributes( class P4PIOC: """A P4P IOC which handles a controller""" - def __init__(self, pv_prefix: str, controller_api: ControllerAPI): - self.pv_prefix = pv_prefix + def __init__(self, controller_api: ControllerAPI): self.controller_api = controller_api async def run(self): - provider = await parse_attributes(self.pv_prefix, self.controller_api) + provider = await parse_attributes(self.controller_api) endless_event = asyncio.Event() with Server([provider]): diff --git a/src/fastcs/transports/epics/pva/transport.py b/src/fastcs/transports/epics/pva/transport.py index 6529c932..31c019a2 100644 --- a/src/fastcs/transports/epics/pva/transport.py +++ b/src/fastcs/transports/epics/pva/transport.py @@ -6,10 +6,11 @@ from fastcs.transports.epics import ( EpicsDocsOptions, EpicsGUIOptions, - EpicsIOCOptions, + EpicsPVAOptions, ) from fastcs.transports.epics.docs import EpicsDocs from fastcs.transports.epics.pva.gui import PvaEpicsGUI +from fastcs.transports.epics.util import pv_prefix_from_path from fastcs.transports.transport import Transport, _expect_single from .ioc import P4PIOC @@ -19,7 +20,8 @@ class EpicsPVATransport(Transport): """PV access transport.""" - epicspva: EpicsIOCOptions = field(default_factory=EpicsIOCOptions) + epicspva: EpicsPVAOptions = field(default_factory=EpicsPVAOptions) + """PVA-specific options. Currently empty; present as the YAML discriminator.""" docs: EpicsDocsOptions | None = None gui: EpicsGUIOptions | None = None @@ -30,14 +32,14 @@ def connect( ) -> None: controller_api = _expect_single(controller_apis, "EpicsPVATransport") self._controller_api = controller_api - self._pv_prefix = self.epicspva.pv_prefix - self._ioc = P4PIOC(self.epicspva.pv_prefix, controller_api) + self._pv_prefix = pv_prefix_from_path(controller_api.path) + self._ioc = P4PIOC(controller_api) if self.docs is not None: EpicsDocs(self._controller_api).create_docs(self.docs) if self.gui is not None: - PvaEpicsGUI(self._controller_api, self._pv_prefix).create_gui(self.gui) + PvaEpicsGUI(self._controller_api).create_gui(self.gui) async def serve(self) -> None: """Serve `ControllerAPI` over EPICS PVAccess""" diff --git a/src/fastcs/transports/epics/util.py b/src/fastcs/transports/epics/util.py index cfc79720..c4b0b156 100644 --- a/src/fastcs/transports/epics/util.py +++ b/src/fastcs/transports/epics/util.py @@ -1,4 +1,3 @@ -from fastcs.controllers import ControllerAPI from fastcs.util import snake_to_pascal @@ -11,7 +10,3 @@ def pv_prefix_from_path(path: list[str]) -> str: if not path: raise ValueError("Cannot derive a PV prefix from an empty path") return ":".join([path[0]] + [snake_to_pascal(node) for node in path[1:]]) - - -def controller_pv_prefix(prefix: str, controller_api: ControllerAPI) -> str: - return ":".join([prefix] + [snake_to_pascal(node) for node in controller_api.path]) diff --git a/tests/assertable_controller.py b/tests/assertable_controller.py index 1b1c6f60..c5791613 100644 --- a/tests/assertable_controller.py +++ b/tests/assertable_controller.py @@ -69,15 +69,21 @@ async def counter(self): class AssertableControllerAPI(ControllerAPI): - def __init__(self, controller: Controller, mocker: MockerFixture) -> None: + def __init__( + self, + controller: Controller, + mocker: MockerFixture, + path: list[str] | None = None, + ) -> None: super().__init__() self.mocker = mocker self.command_method_spys: dict[str, MockType] = {} # Build a ControllerAPI from the given Controller - controller_api = controller._build_api([]) + controller_api = controller._build_api(path or []) # Copy its fields + self.path = controller_api.path self.attributes = controller_api.attributes self.command_methods = controller_api.command_methods self.scan_methods = controller_api.scan_methods diff --git a/tests/benchmarking/controller.py b/tests/benchmarking/controller.py index 7ffc2238..3056da8f 100644 --- a/tests/benchmarking/controller.py +++ b/tests/benchmarking/controller.py @@ -4,7 +4,6 @@ from fastcs.attributes import AttrR, AttrW from fastcs.controllers import Controller from fastcs.datatypes import Bool, Int -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.transport import EpicsCATransport from fastcs.transports.rest.options import RestServerOptions from fastcs.transports.rest.transport import RestTransport @@ -20,12 +19,12 @@ class MyTestController(Controller): def run(): transport_options = [ RestTransport(rest=RestServerOptions(port=8090)), - EpicsCATransport( - epicsca=EpicsIOCOptions(pv_prefix="BENCHMARK-DEVICE"), - ), + EpicsCATransport(), TangoTransport(tango=TangoDSROptions(dev_name="MY/BENCHMARK/DEVICE")), ] - instance = FastCS(MyTestController(), transport_options, asyncio.get_event_loop()) + controller = MyTestController() + controller.set_id("BENCHMARK-DEVICE") + instance = FastCS(controller, transport_options, asyncio.get_event_loop()) instance.run() diff --git a/tests/conftest.py b/tests/conftest.py index 5da18951..a2be77aa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -114,7 +114,7 @@ def write(self, s): # type: ignore try: sys.stdout = QueueWriter(stdout_queue) - run_ioc(pv_prefix=pv_prefix) + run_ioc(id=pv_prefix) except Exception as e: error_queue.put(e) diff --git a/tests/data/schema.json b/tests/data/schema.json index 3282cf65..a5f5b54a 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -1,9 +1,14 @@ { "$defs": { + "EpicsCAOptions": { + "properties": {}, + "title": "EpicsCAOptions", + "type": "object" + }, "EpicsCATransport": { "properties": { "epicsca": { - "$ref": "#/$defs/EpicsIOCOptions" + "$ref": "#/$defs/EpicsCAOptions" }, "docs": { "anyOf": [ @@ -85,21 +90,15 @@ "title": "EpicsGUIOptions", "type": "object" }, - "EpicsIOCOptions": { - "properties": { - "pv_prefix": { - "default": "MY-DEVICE-PREFIX", - "title": "Pv Prefix", - "type": "string" - } - }, - "title": "EpicsIOCOptions", + "EpicsPVAOptions": { + "properties": {}, + "title": "EpicsPVAOptions", "type": "object" }, "EpicsPVATransport": { "properties": { "epicspva": { - "$ref": "#/$defs/EpicsIOCOptions" + "$ref": "#/$defs/EpicsPVAOptions" }, "docs": { "anyOf": [ diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index b6dc843a..8cba9bf3 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -9,7 +9,6 @@ from fastcs.datatypes import Bool, DType_T, Enum, Float, Int, Table, Waveform from fastcs.launch import FastCS from fastcs.methods import command, scan -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.pva import EpicsPVATransport @@ -87,10 +86,11 @@ async def i(self): j: AttrR = AttrR(Int()) -def run(pv_prefix="P4P_TEST_DEVICE"): +def run(id="P4P_TEST_DEVICE"): simple_attribute_io = SimpleAttributeIO() - p4p_options = EpicsPVATransport(epicspva=EpicsIOCOptions(pv_prefix=pv_prefix)) + p4p_options = EpicsPVATransport() controller = ParentController(ios=[simple_attribute_io]) + controller.set_id(id) class ChildVector(ControllerVector): vector_attribute: AttrR = AttrR(Int()) diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 86eac91e..2443102f 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -5,7 +5,6 @@ from fastcs.controllers import Controller, ControllerVector from fastcs.datatypes import Int from fastcs.methods import command -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.transport import EpicsCATransport, EpicsGUIOptions @@ -22,8 +21,9 @@ async def d(self): pass -def run(pv_prefix="SOFTIOC_TEST_DEVICE"): +def run(id="SOFTIOC_TEST_DEVICE"): controller = ParentController() + controller.set_id(id) vector = ControllerVector({i: ChildController() for i in range(2)}) controller.add_sub_controller("ChildVector", vector) gui_options = EpicsGUIOptions( @@ -31,11 +31,7 @@ def run(pv_prefix="SOFTIOC_TEST_DEVICE"): ) fastcs = FastCS( controller, - [ - EpicsCATransport( - epicsca=EpicsIOCOptions(pv_prefix=pv_prefix), gui=gui_options - ) - ], + [EpicsCATransport(gui=gui_options)], ) fastcs.run(interactive=False) diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index d468cffd..fd4ed9b7 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -1,8 +1,5 @@ -"""Tests for the multi-controller foundation slice (#353). - -These tests exercise the public Controller.id lifecycle and -multi-controller REST routing through RestTransport, up to the -end-to-end FastCS.serve() lifecycle with two controllers. +"""Tests for the multi-controller foundation slice (#353) and per-transport +multi-root extensions (#354+). """ import asyncio @@ -15,6 +12,7 @@ from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.datatypes import Int +from fastcs.transports.epics.ca.transport import EpicsCATransport from fastcs.transports.rest.transport import RestTransport @@ -103,6 +101,52 @@ def test_rest_transport_rejects_illegal_id_at_connect(): loop.close() +def test_ca_transport_routes_two_controllers_with_distinct_pv_prefixes(mocker): + """One softioc serves N controllers; each id is its verbatim PV prefix.""" + util_builder = mocker.patch("fastcs.transports.epics.ca.util.builder") + mocker.patch("fastcs.transports.epics.ca.ioc.builder") + + api1 = _api_with_id(_OneAttrController, "ALPHA") + api2 = _api_with_id(_OtherAttrController, "BETA") + + loop = asyncio.new_event_loop() + try: + transport = EpicsCATransport() + transport.connect([api1, api2], loop) + finally: + loop.close() + + # Each controller's record lands under its verbatim id, no clash. + util_builder.longIn.assert_any_call( + "ALPHA:Foo", + DESC=mocker.ANY, + EGU=mocker.ANY, + LOPR=mocker.ANY, + HOPR=mocker.ANY, + initial_value=mocker.ANY, + ) + util_builder.longIn.assert_any_call( + "BETA:Bar", + DESC=mocker.ANY, + EGU=mocker.ANY, + LOPR=mocker.ANY, + HOPR=mocker.ANY, + initial_value=mocker.ANY, + ) + + +def test_ca_transport_rejects_illegal_id_at_connect(): + api = _api_with_id(_OneAttrController, "bad/id") + + loop = asyncio.new_event_loop() + try: + transport = EpicsCATransport() + with pytest.raises(ValueError, match="bad/id"): + transport.connect([api], loop) + finally: + loop.close() + + class _LifecycleController(Controller): """Records lifecycle hook calls for end-to-end assertions.""" diff --git a/tests/transports/epics/ca/test_ca_util.py b/tests/transports/epics/ca/test_ca_util.py index f484fca9..463a17dc 100644 --- a/tests/transports/epics/ca/test_ca_util.py +++ b/tests/transports/epics/ca/test_ca_util.py @@ -2,10 +2,12 @@ import pytest +from fastcs.controllers import ControllerAPI from fastcs.datatypes import Bool, Enum, Float, Int, String from fastcs.transports.epics.ca.util import ( cast_from_epics_type, cast_to_epics_type, + validate_ca_id, ) @@ -131,3 +133,25 @@ def test_cast_from_epics_type(datatype, from_epics, result): def test_cast_from_epics_validations(datatype, input): with pytest.raises(ValueError): cast_from_epics_type(datatype, input) + + +@pytest.mark.parametrize("id", ["DEVICE", "my-id", "name_1", "ABC-123_xyz"]) +def test_validate_ca_id_accepts_valid(id): + validate_ca_id(ControllerAPI(path=[id])) + + +@pytest.mark.parametrize("id", ["bad/id", "with space", "colons:in:id", ""]) +def test_validate_ca_id_rejects_illegal_characters(id): + with pytest.raises(ValueError, match="EPICS CA id"): + validate_ca_id(ControllerAPI(path=[id])) + + +def test_validate_ca_id_rejects_overlong_prefix(): + deep_path = ["A" * 50, "deeper_sub_controller_path"] + with pytest.raises(ValueError, match="exceeds the EPICS"): + validate_ca_id( + ControllerAPI( + path=deep_path[:1], + sub_apis={"sub": ControllerAPI(path=deep_path)}, + ) + ) diff --git a/tests/transports/epics/ca/test_gui.py b/tests/transports/epics/ca/test_gui.py index 72931b86..77be76dc 100644 --- a/tests/transports/epics/ca/test_gui.py +++ b/tests/transports/epics/ca/test_gui.py @@ -25,11 +25,11 @@ def test_get_pv(): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) - assert gui._get_pv([], "A") == "DEVICE:A" - assert gui._get_pv(["B"], "C") == "DEVICE:B:C" - assert gui._get_pv(["D", "E"], "F") == "DEVICE:D:E:F" + assert gui._get_pv(["DEVICE"], "A") == "DEVICE:A" + assert gui._get_pv(["DEVICE", "B"], "C") == "DEVICE:B:C" + assert gui._get_pv(["DEVICE", "D", "E"], "F") == "DEVICE:D:E:F" @pytest.mark.parametrize( @@ -44,10 +44,10 @@ def test_get_pv(): ], ) def test_get_attribute_component_r(datatype, widget): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) - assert gui._get_attribute_component([], "Attr", AttrR(datatype)) == SignalR( - name="Attr", read_pv="Attr", read_widget=widget + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrR(datatype)) == SignalR( + name="Attr", read_pv="DEVICE:Attr", read_widget=widget ) @@ -58,9 +58,9 @@ def test_get_attribute_component_r(datatype, widget): ], ) def test_get_attribute_component_r_signal_none(datatype): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) - assert gui._get_attribute_component([], "Attr", AttrR(datatype)) is None + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrR(datatype)) is None @pytest.mark.parametrize( @@ -74,32 +74,33 @@ def test_get_attribute_component_r_signal_none(datatype): ], ) def test_get_attribute_component_w(datatype, widget): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) - assert gui._get_attribute_component([], "Attr", AttrW(datatype)) == SignalW( - name="Attr", write_pv="Attr", write_widget=widget + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrW(datatype)) == SignalW( + name="Attr", write_pv="DEVICE:Attr", write_widget=widget ) def test_get_attribute_component_none(mocker): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) mocker.patch.object(gui, "_get_read_widget", return_value=None) mocker.patch.object(gui, "_get_write_widget", return_value=None) - assert gui._get_attribute_component([], "Attr", AttrR(Int())) is None - assert gui._get_attribute_component([], "Attr", AttrW(Int())) is None - assert gui._get_attribute_component([], "Attr", AttrRW(Int())) is None + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrR(Int())) is None + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrW(Int())) is None + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrRW(Int())) is None def test_get_write_widget_none(): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) assert ( gui._get_write_widget(attribute=AttrR(Waveform(array_dtype=np.int32))) is None ) -def test_get_components(controller_api): - gui = EpicsGUI(controller_api, "DEVICE") +def test_get_components(controller): + controller_api = controller._build_api(["DEVICE"]) + gui = EpicsGUI(controller_api) components = gui.extract_api_components(controller_api) assert components == [ @@ -172,7 +173,7 @@ def test_get_components_none(mocker): """Test that if _get_attribute_component returns none it is skipped""" controller_api = ControllerAPI() - gui = EpicsGUI(controller_api, "DEVICE") + gui = EpicsGUI(controller_api) mocker.patch.object(gui, "_get_attribute_component", return_value=None) components = gui.extract_api_components(controller_api) @@ -181,9 +182,9 @@ def test_get_components_none(mocker): def test_get_command_component(): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) - component = gui._get_command_component([], "Command") + component = gui._get_command_component(["DEVICE"], "Command") assert isinstance(component, SignalX) assert component.write_widget == ButtonPanel(actions={"Command": "1"}) diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index 727cb0d7..23c0976e 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -9,7 +9,6 @@ from fastcs.controllers import Controller from fastcs.datatypes import Bool, Enum, Float, Int, String, Waveform from fastcs.launch import FastCS -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.transport import EpicsCATransport @@ -53,9 +52,10 @@ async def test_initial_values_set_in_ca(mocker): loop = asyncio.get_event_loop() controller = InitialValuesController() + controller.set_id(pv_prefix) fastcs = FastCS( controller, - [EpicsCATransport(epicsca=EpicsIOCOptions(pv_prefix=pv_prefix))], + [EpicsCATransport()], loop, ) diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index d695abab..dda38d69 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -19,7 +19,6 @@ from fastcs.methods import Command from fastcs.transports.epics.ca import EpicsCATransport from fastcs.transports.epics.ca.ioc import ( - EPICS_MAX_NAME_LENGTH, EpicsCAIOC, _add_attr_pvi_info, _add_pvi_info, @@ -28,6 +27,7 @@ _create_and_link_write_pv, ) from fastcs.transports.epics.ca.util import ( + EPICS_MAX_NAME_LENGTH, _make_in_record, _make_out_record, ) @@ -273,7 +273,7 @@ class EpicsController(MyTestController): @pytest.fixture() def epics_controller_api(class_mocker: MockerFixture): - return AssertableControllerAPI(EpicsController(), class_mocker) + return AssertableControllerAPI(EpicsController(), class_mocker, path=[DEVICE]) def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): @@ -284,7 +284,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): "fastcs.transports.epics.ca.ioc._add_sub_controller_pvi_info" ) - EpicsCAIOC(DEVICE, epics_controller_api) + EpicsCAIOC([epics_controller_api]) # Check records are created util_builder.boolIn.assert_called_once_with( @@ -386,7 +386,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): # Check info tags are added add_pvi_info.assert_called_once_with(f"{DEVICE}:PVI") - add_sub_controller_pvi_info.assert_called_once_with(DEVICE, epics_controller_api) + add_sub_controller_pvi_info.assert_called_once_with(epics_controller_api) def test_add_pvi_info(mocker: MockerFixture): @@ -456,12 +456,12 @@ def test_add_pvi_info_with_parent(mocker: MockerFixture): def test_add_sub_controller_pvi_info(mocker: MockerFixture): add_pvi_info = mocker.patch("fastcs.transports.epics.ca.ioc._add_pvi_info") parent_api = mocker.MagicMock() - parent_api.path = [] + parent_api.path = [DEVICE] child_api = mocker.MagicMock() - child_api.path = ["Child"] + child_api.path = [DEVICE, "Child"] parent_api.sub_apis = {"d": child_api} - _add_sub_controller_pvi_info(DEVICE, parent_api) + _add_sub_controller_pvi_info(parent_api) add_pvi_info.assert_called_once_with( f"{DEVICE}:Child:PVI", f"{DEVICE}:PVI", "child" @@ -503,12 +503,14 @@ class ControllerLongNames(Controller): 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) + long_name_controller_api = AssertableControllerAPI( + ControllerLongNames(), mocker, path=[DEVICE] + ) 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" assert long_name_controller_api.attributes["attr_rw_short_name"].enabled assert long_name_controller_api.attributes[long_attr_name].enabled - EpicsCAIOC(DEVICE, long_name_controller_api) + EpicsCAIOC([long_name_controller_api]) assert long_name_controller_api.attributes["attr_rw_short_name"].enabled assert not long_name_controller_api.attributes[long_attr_name].enabled @@ -585,21 +587,22 @@ def test_long_pv_names_discarded(mocker: MockerFixture): def test_non_1d_waveforms_discarded(mocker: MockerFixture): api = ControllerAPI( + path=[DEVICE], attributes={ "waveform_0d": AttrR(Waveform(np.int32, shape=())), "waveform_1d": AttrR(Waveform(np.int32, shape=(10,))), "waveform_2d": AttrR(Waveform(np.int32, shape=(10, 2))), "waveform_3d": AttrR(Waveform(np.int32, shape=(10, 2, 3))), - } + }, ) create_mock = mocker.patch( "fastcs.transports.epics.ca.ioc._create_and_link_read_pv" ) - EpicsCAIOC("DEVICE", api) + EpicsCAIOC([api]) create_mock.assert_called_once_with( - "DEVICE", "Waveform1d", "waveform_1d", api.attributes["waveform_1d"] + DEVICE, "Waveform1d", "waveform_1d", api.attributes["waveform_1d"] ) diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index 3a1a06ac..a54e69dc 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -18,7 +18,6 @@ from fastcs.datatypes import Bool, Enum, Float, Int, String, Table, Waveform from fastcs.launch import FastCS from fastcs.methods import command -from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.pva.transport import EpicsPVATransport @@ -211,9 +210,8 @@ async def test_numeric_alarms(p4p_subprocess: tuple[str, Queue]): def make_fastcs(pv_prefix: str, controller: Controller) -> FastCS: - return FastCS( - controller, [EpicsPVATransport(epicspva=EpicsIOCOptions(pv_prefix=pv_prefix))] - ) + controller.set_id(pv_prefix) + return FastCS(controller, [EpicsPVATransport()]) def test_read_signal_set(): diff --git a/tests/transports/epics/pva/test_pva_gui.py b/tests/transports/epics/pva/test_pva_gui.py index f5403d2a..4a753608 100644 --- a/tests/transports/epics/pva/test_pva_gui.py +++ b/tests/transports/epics/pva/test_pva_gui.py @@ -29,26 +29,26 @@ ], ) def test_pva_get_attribute_component_r(datatype, widget): - gui = EpicsGUI(ControllerAPI(), "DEVICE") + gui = EpicsGUI(ControllerAPI()) - assert gui._get_attribute_component([], "Attr", AttrR(datatype)) == SignalR( - name="Attr", read_pv="Attr", read_widget=widget + assert gui._get_attribute_component(["DEVICE"], "Attr", AttrR(datatype)) == SignalR( + name="Attr", read_pv="DEVICE:Attr", read_widget=widget ) def test_get_pv_in_pva(): - gui = PvaEpicsGUI(ControllerAPI(), "DEVICE") + gui = PvaEpicsGUI(ControllerAPI()) - assert gui._get_pv([], "A") == "pva://DEVICE:A" - assert gui._get_pv(["B"], "C") == "pva://DEVICE:B:C" - assert gui._get_pv(["D", "E"], "F") == "pva://DEVICE:D:E:F" + assert gui._get_pv(["DEVICE"], "A") == "pva://DEVICE:A" + assert gui._get_pv(["DEVICE", "B"], "C") == "pva://DEVICE:B:C" + assert gui._get_pv(["DEVICE", "D", "E"], "F") == "pva://DEVICE:D:E:F" def test_get_attribute_component_table_write(): - gui = PvaEpicsGUI(ControllerAPI(), "DEVICE") + gui = PvaEpicsGUI(ControllerAPI()) attribute_component = gui._get_attribute_component( - [], + ["DEVICE"], "Table", AttrW( Table( @@ -71,10 +71,10 @@ def test_get_attribute_component_table_write(): def test_get_attribute_component_table_read(): - gui = PvaEpicsGUI(ControllerAPI(), "DEVICE") + gui = PvaEpicsGUI(ControllerAPI()) attribute_component = gui._get_attribute_component( - [], + ["DEVICE"], "Table", AttrR( Table( @@ -97,9 +97,9 @@ def test_get_attribute_component_table_read(): def test_get_command_component(): - gui = PvaEpicsGUI(ControllerAPI(), "DEVICE") + gui = PvaEpicsGUI(ControllerAPI()) - component = gui._get_command_component([], "Command") + component = gui._get_command_component(["DEVICE"], "Command") assert isinstance(component, SignalX) assert component.write_widget == ButtonPanel(actions={"Command": "true"}) From c1b95a29a8055342793bfc216954b08f909e8bdc Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 15:38:48 +0100 Subject: [PATCH 09/16] Update EPICS multi-transport docs for id-based prefix multiple-transports.md and launch-framework.md still showed EpicsIOCOptions(pv_prefix=...) in their Python and YAML examples. Replace those with the id-based shape: controllers set their id (or inherit it from the YAML controllers: dict key), and EpicsCATransport / EpicsPVATransport take no prefix argument. The prose follows the API that landed in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/how-to/launch-framework.md | 19 +++++++++++-------- docs/how-to/multiple-transports.md | 26 ++++++++++++-------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/docs/how-to/launch-framework.md b/docs/how-to/launch-framework.md index e3f210fb..8ebdf4db 100644 --- a/docs/how-to/launch-framework.md +++ b/docs/how-to/launch-framework.md @@ -58,16 +58,20 @@ Create a YAML configuration file matching the schema: ```yaml # device_config.yaml -controller: - ip_address: "192.168.1.100" - port: 25565 - timeout: 10.0 +controllers: + DEVICE: + controller: + ip_address: "192.168.1.100" + port: 25565 + timeout: 10.0 transport: - - epicsca: - pv_prefix: "DEVICE" + - epicsca: {} ``` +The key under `controllers:` (here `DEVICE`) is the controller id, used +verbatim as the EPICS PV prefix and as the REST route prefix. + Run with: ```bash @@ -98,8 +102,7 @@ Transports are configured in the `transport` section as a list: ```yaml transport: # EPICS Channel Access - - epicsca: - pv_prefix: "DEVICE" + - epicsca: {} gui: output_path: "opis/device.bob" title: "Device Control" diff --git a/docs/how-to/multiple-transports.md b/docs/how-to/multiple-transports.md index 62634009..05f68361 100644 --- a/docs/how-to/multiple-transports.md +++ b/docs/how-to/multiple-transports.md @@ -10,17 +10,17 @@ Pass a list of transports to `FastCS`: from fastcs.control_system import FastCS from fastcs.transports import ( EpicsCATransport, - EpicsIOCOptions, GraphQLTransport, RestTransport, ) controller = MyController() +controller.set_id("DEVICE") # PV prefix for EPICS / route prefix for REST fastcs = FastCS( controller, [ - EpicsCATransport(epicsca=EpicsIOCOptions(pv_prefix="DEVICE")), + EpicsCATransport(), RestTransport(), GraphQLTransport(), ] @@ -52,17 +52,18 @@ Each transport has its own options: ### EPICS Channel Access +The PV prefix is the controller's id (set via `controller.set_id(...)` or +auto-set by `launch()` from the YAML key). + ```python from pathlib import Path from fastcs.transports import ( EpicsCATransport, EpicsDocsOptions, EpicsGUIOptions, - EpicsIOCOptions, ) epics_ca = EpicsCATransport( - epicsca=EpicsIOCOptions(pv_prefix="DEVICE"), gui=EpicsGUIOptions( output_path=Path(".") / "device.bob", title="Device Control", @@ -76,11 +77,9 @@ epics_ca = EpicsCATransport( ### EPICS PV Access ```python -from fastcs.transports import EpicsPVATransport, EpicsIOCOptions +from fastcs.transports import EpicsPVATransport -epics_pva = EpicsPVATransport( - epicspva=EpicsIOCOptions(pv_prefix="DEVICE"), -) +epics_pva = EpicsPVATransport() ``` ### REST @@ -134,25 +133,24 @@ from pathlib import Path from fastcs.transports import ( EpicsCATransport, EpicsGUIOptions, - EpicsIOCOptions, EpicsPVATransport, ) +controller.set_id("DEVICE") + fastcs = FastCS( controller, [ EpicsCATransport( - epicsca=EpicsIOCOptions(pv_prefix="DEVICE"), gui=EpicsGUIOptions(output_path=Path(".") / "device.bob"), ), - EpicsPVATransport( - epicspva=EpicsIOCOptions(pv_prefix="DEVICE"), - ), + EpicsPVATransport(), ] ) ``` -Both transports share the same PV prefix and expose identical PVs. +Both transports derive the same PV prefix from the controller's id and +expose identical PVs. ## YAML Configuration From 43956050cdcbb8819821cbef0ff06eb197629d48 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 16:14:58 +0100 Subject: [PATCH 10/16] EPICS PVA multi-root with N PVI roots EpicsPVATransport now hosts every configured controller in one p4p server, with each controller's id used verbatim as its PV prefix. P4PIOC takes list[ControllerAPI] and builds one StaticProvider per controller via the existing parse_attributes helper, so each controller gets an independent :PVI root with no super-parent (per the PRD). EpicsPVATransport.connect drops _expect_single in favour of true multi-controller; validate_pva_id runs at connect time and rejects ids with illegal characters as well as setups whose longest derivable PV prefix already exceeds the 60-character EPICS limit. validate_pva_id mirrors validate_ca_id and lives in transports/epics/ pva/util.py to keep id validation a per-transport concern. To share the 60-char constant without a cross-transport import, EPICS_MAX_NAME_LENGTH moves up from ca/util.py to epics/util.py; ca/util.py re-imports it so existing ca.util consumers (ca/ioc.py, test_softioc) are unaffected. tests/test_multi_controller.py grows a PVA two-controllers-distinct-PVI scenario (asserts each StaticProvider exposes its own root) and a PVA id-validation fail-fast case. test_pva_util.py mirrors test_ca_util.py's validator coverage. test_p4p.py::test_pvi_grouping shortens its UUID id to 8 hex chars so the deepest derived prefix (:AdditionalChild:ChildChild) no longer trips the new 60-char check. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/transports/epics/ca/util.py | 4 +-- src/fastcs/transports/epics/pva/ioc.py | 17 ++++++++--- src/fastcs/transports/epics/pva/transport.py | 25 ++++++++------- src/fastcs/transports/epics/pva/util.py | 30 ++++++++++++++++++ src/fastcs/transports/epics/util.py | 3 ++ tests/test_multi_controller.py | 32 ++++++++++++++++++++ tests/transports/epics/pva/test_p4p.py | 4 ++- tests/transports/epics/pva/test_pva_util.py | 26 ++++++++++++++++ 8 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 src/fastcs/transports/epics/pva/util.py create mode 100644 tests/transports/epics/pva/test_pva_util.py diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index 0aeae58d..049ad74a 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -11,9 +11,7 @@ from fastcs.controllers import ControllerAPI from fastcs.datatypes import Bool, DataType, DType_T, Enum, Float, Int, String, Waveform from fastcs.exceptions import FastCSError -from fastcs.transports.epics.util import pv_prefix_from_path - -EPICS_MAX_NAME_LENGTH = 60 +from fastcs.transports.epics.util import EPICS_MAX_NAME_LENGTH, pv_prefix_from_path _CA_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") diff --git a/src/fastcs/transports/epics/pva/ioc.py b/src/fastcs/transports/epics/pva/ioc.py index 4d3fada0..d165cf64 100644 --- a/src/fastcs/transports/epics/pva/ioc.py +++ b/src/fastcs/transports/epics/pva/ioc.py @@ -48,14 +48,21 @@ async def parse_attributes(root_controller_api: ControllerAPI) -> StaticProvider class P4PIOC: - """A P4P IOC which handles a controller""" + """A P4P IOC which handles one or more controllers. - def __init__(self, controller_api: ControllerAPI): - self.controller_api = controller_api + Each controller gets its own `StaticProvider` so it exposes an independent + `:PVI` root with no super-parent. + """ + + def __init__(self, controller_apis: list[ControllerAPI]): + self._controller_apis = controller_apis + + async def _build_providers(self) -> list[StaticProvider]: + return [await parse_attributes(api) for api in self._controller_apis] async def run(self): - provider = await parse_attributes(self.controller_api) + providers = await self._build_providers() endless_event = asyncio.Event() - with Server([provider]): + with Server(providers): await endless_event.wait() diff --git a/src/fastcs/transports/epics/pva/transport.py b/src/fastcs/transports/epics/pva/transport.py index 31c019a2..327da8c0 100644 --- a/src/fastcs/transports/epics/pva/transport.py +++ b/src/fastcs/transports/epics/pva/transport.py @@ -10,8 +10,9 @@ ) from fastcs.transports.epics.docs import EpicsDocs from fastcs.transports.epics.pva.gui import PvaEpicsGUI +from fastcs.transports.epics.pva.util import validate_pva_id from fastcs.transports.epics.util import pv_prefix_from_path -from fastcs.transports.transport import Transport, _expect_single +from fastcs.transports.transport import Transport from .ioc import P4PIOC @@ -30,21 +31,23 @@ def connect( controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ) -> None: - controller_api = _expect_single(controller_apis, "EpicsPVATransport") - self._controller_api = controller_api - self._pv_prefix = pv_prefix_from_path(controller_api.path) - self._ioc = P4PIOC(controller_api) + for api in controller_apis: + validate_pva_id(api) + self._controller_apis = controller_apis + self._pv_prefixes = [pv_prefix_from_path(api.path) for api in controller_apis] + self._ioc = P4PIOC(controller_apis) - if self.docs is not None: - EpicsDocs(self._controller_api).create_docs(self.docs) + for api in controller_apis: + if self.docs is not None: + EpicsDocs(api).create_docs(self.docs) - if self.gui is not None: - PvaEpicsGUI(self._controller_api).create_gui(self.gui) + if self.gui is not None: + PvaEpicsGUI(api).create_gui(self.gui) async def serve(self) -> None: """Serve `ControllerAPI` over EPICS PVAccess""" - logger.info("Running IOC", pv_prefix=self._pv_prefix) + logger.info("Running IOC", pv_prefixes=self._pv_prefixes) await self._ioc.run() def __repr__(self): - return f"EpicsPVATransport({self._pv_prefix})" + return f"EpicsPVATransport({self._pv_prefixes})" diff --git a/src/fastcs/transports/epics/pva/util.py b/src/fastcs/transports/epics/pva/util.py new file mode 100644 index 00000000..070927c6 --- /dev/null +++ b/src/fastcs/transports/epics/pva/util.py @@ -0,0 +1,30 @@ +import re + +from fastcs.controllers import ControllerAPI +from fastcs.transports.epics.util import EPICS_MAX_NAME_LENGTH, pv_prefix_from_path + +_PVA_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + +def validate_pva_id(controller_api: ControllerAPI) -> None: + """Reject controller ids that wouldn't be safe in an EPICS PVA PV name. + + Rejects ids with characters outside ``[A-Za-z0-9_-]`` and rejects setups + where the longest derivable PV prefix already exceeds the 60-character + EPICS PV name limit. + """ + id = controller_api.path[0] + if not _PVA_ID_RE.fullmatch(id): + raise ValueError( + f"Controller id {id!r} is not a valid EPICS PVA id; " + "only alphanumerics, '-' and '_' are allowed" + ) + longest_prefix = max( + len(pv_prefix_from_path(api.path)) for api in controller_api.walk_api() + ) + if longest_prefix > EPICS_MAX_NAME_LENGTH: + raise ValueError( + f"Controller id {id!r} produces a PV prefix of " + f"{longest_prefix} characters, which exceeds the EPICS " + f"{EPICS_MAX_NAME_LENGTH}-character PV name limit" + ) diff --git a/src/fastcs/transports/epics/util.py b/src/fastcs/transports/epics/util.py index c4b0b156..5a35eb35 100644 --- a/src/fastcs/transports/epics/util.py +++ b/src/fastcs/transports/epics/util.py @@ -1,5 +1,8 @@ from fastcs.util import snake_to_pascal +EPICS_MAX_NAME_LENGTH = 60 +"""Maximum length of an EPICS PV name, enforced by both CA and PVA transports.""" + def pv_prefix_from_path(path: list[str]) -> str: """Derive an EPICS PV prefix from a controller path. diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index fd4ed9b7..40eb9627 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -13,6 +13,7 @@ from fastcs.controllers import Controller from fastcs.datatypes import Int from fastcs.transports.epics.ca.transport import EpicsCATransport +from fastcs.transports.epics.pva.transport import EpicsPVATransport from fastcs.transports.rest.transport import RestTransport @@ -147,6 +148,37 @@ def test_ca_transport_rejects_illegal_id_at_connect(): loop.close() +@pytest.mark.asyncio +async def test_pva_transport_serves_two_controllers_with_distinct_pvi_roots(): + """One p4p server hosts N controllers; each gets its own ``:PVI`` root with no + super-parent.""" + api1 = _api_with_id(_OneAttrController, "ALPHA") + api2 = _api_with_id(_OtherAttrController, "BETA") + + transport = EpicsPVATransport() + transport.connect([api1, api2], asyncio.get_event_loop()) + + providers = await transport._ioc._build_providers() + pv_names = {name for provider in providers for name in provider.keys()} + + assert "ALPHA:PVI" in pv_names + assert "BETA:PVI" in pv_names + assert "ALPHA:Foo" in pv_names + assert "BETA:Bar" in pv_names + + +def test_pva_transport_rejects_illegal_id_at_connect(): + api = _api_with_id(_OneAttrController, "bad/id") + + loop = asyncio.new_event_loop() + try: + transport = EpicsPVATransport() + with pytest.raises(ValueError, match="bad/id"): + transport.connect([api], loop) + finally: + loop.close() + + class _LifecycleController(Controller): """Records lifecycle hook calls for end-to-end assertions.""" diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index a54e69dc..4041f78c 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -298,7 +298,9 @@ class SomeController(Controller): controller.additional_child = sub_controller sub_controller.child_child = ChildChildController() - pv_prefix = str(uuid4()) + # 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) ctxt = ThreadContext("pva") diff --git a/tests/transports/epics/pva/test_pva_util.py b/tests/transports/epics/pva/test_pva_util.py new file mode 100644 index 00000000..f97bd8ed --- /dev/null +++ b/tests/transports/epics/pva/test_pva_util.py @@ -0,0 +1,26 @@ +import pytest + +from fastcs.controllers import ControllerAPI +from fastcs.transports.epics.pva.util import validate_pva_id + + +@pytest.mark.parametrize("id", ["DEVICE", "my-id", "name_1", "ABC-123_xyz"]) +def test_validate_pva_id_accepts_valid(id): + validate_pva_id(ControllerAPI(path=[id])) + + +@pytest.mark.parametrize("id", ["bad/id", "with space", "colons:in:id", ""]) +def test_validate_pva_id_rejects_illegal_characters(id): + with pytest.raises(ValueError, match="EPICS PVA id"): + validate_pva_id(ControllerAPI(path=[id])) + + +def test_validate_pva_id_rejects_overlong_prefix(): + deep_path = ["A" * 50, "deeper_sub_controller_path"] + with pytest.raises(ValueError, match="exceeds the EPICS"): + validate_pva_id( + ControllerAPI( + path=deep_path[:1], + sub_apis={"sub": ControllerAPI(path=deep_path)}, + ) + ) From 5317473f771c4069148d3da08fc449a15aed4c13 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 20:07:17 +0100 Subject: [PATCH 11/16] Use literal markup for P4PIOC docstring refs Sphinx is configured with `nitpicky = True` and `--fail-on-warning`, so single-backticks in the new P4PIOC docstring (`StaticProvider`, `:PVI`) were treated as :any: cross-references and failed to resolve. Switch to double-backticks so they're inline literals instead. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/fastcs/transports/epics/pva/ioc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fastcs/transports/epics/pva/ioc.py b/src/fastcs/transports/epics/pva/ioc.py index d165cf64..5b2e2961 100644 --- a/src/fastcs/transports/epics/pva/ioc.py +++ b/src/fastcs/transports/epics/pva/ioc.py @@ -50,8 +50,8 @@ async def parse_attributes(root_controller_api: ControllerAPI) -> StaticProvider class P4PIOC: """A P4P IOC which handles one or more controllers. - Each controller gets its own `StaticProvider` so it exposes an independent - `:PVI` root with no super-parent. + Each controller gets its own ``StaticProvider`` so it exposes an independent + ``:PVI`` root with no super-parent. """ def __init__(self, controller_apis: list[ControllerAPI]): From e5217785695505ae85635822a4b47f10f0131dff Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Tue, 5 May 2026 20:18:23 +0100 Subject: [PATCH 12/16] GraphQL combined schema with id-keyed top-level Query fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the GraphQL transport into the multi-controller foundation: - New `validate_graphql_id` enforces GraphQL `Name` syntax (the most restrictive of FastCS's transports — drives the lowest-common-denominator id-naming guidance for users mixing transports). - `GraphQLServer` now accepts `list[ControllerAPI]` and assembles a single combined schema with one top-level Query (and Mutation, where applicable) field per controller id. Sub-API type names are path-joined to keep two controllers' identically-named sub-controllers from clashing in the schema. - `GraphQLTransport.connect` validates ids fail-fast at startup. - `tests/test_multi_controller.py` gains a two-controller combined-schema scenario and a per-transport id-validation case. - The single-controller transport test is updated to namespace its queries under a controller id, matching the new contract; a latent bug in its `nest_mutation` helper (recursing through `nest_query`) is fixed in passing. - `docs/how-to/multiple-transports.md` adds a charset table and notes GraphQL as the lowest common denominator for cross-transport ids. --- docs/how-to/multiple-transports.md | 19 ++++++ src/fastcs/transports/graphql/graphql.py | 68 ++++++++++++++-------- src/fastcs/transports/graphql/transport.py | 9 ++- src/fastcs/transports/graphql/util.py | 21 +++++++ tests/test_multi_controller.py | 43 ++++++++++++++ tests/transports/graphQL/test_graphql.py | 25 ++++---- 6 files changed, 147 insertions(+), 38 deletions(-) create mode 100644 src/fastcs/transports/graphql/util.py diff --git a/docs/how-to/multiple-transports.md b/docs/how-to/multiple-transports.md index 05f68361..6a0941d7 100644 --- a/docs/how-to/multiple-transports.md +++ b/docs/how-to/multiple-transports.md @@ -30,6 +30,25 @@ fastcs.run() All transports run concurrently, exposing the same controller API. +## Choosing controller ids across transports + +Each transport derives its addressing from the controller's id (the PV prefix +for EPICS, the URL prefix for REST, the top-level Query field for GraphQL), +and each enforces its own charset at startup. + +| Transport | Allowed id charset | +|-----------|--------------------| +| EPICS CA | `[A-Za-z0-9_-]+`, plus the 60-char PV name limit | +| EPICS PVA | `[A-Za-z0-9_-]+` | +| REST | `[A-Za-z0-9_-]+` | +| GraphQL | `[A-Za-z_][A-Za-z0-9_]*` (GraphQL `Name`: no hyphens, no leading digit) | + +If you serve the same controller through multiple transports, use the +intersection — a leading letter or underscore followed by letters, digits and +underscores. GraphQL is the lowest common denominator: an id like `dev-01` +will start an EPICS or REST transport happily but fail fast when GraphQL is +added. + ## Available Transports | Transport | Protocol | Install Extra | Primary Use Case | diff --git a/src/fastcs/transports/graphql/graphql.py b/src/fastcs/transports/graphql/graphql.py index 8bc2fa32..f52593dc 100644 --- a/src/fastcs/transports/graphql/graphql.py +++ b/src/fastcs/transports/graphql/graphql.py @@ -17,18 +17,42 @@ class GraphQLServer: - """A GraphQL server which handles a controller""" + """A GraphQL server which serves one combined schema for N controllers. - def __init__(self, controller_api: ControllerAPI): - self._controller_api = controller_api + Each top-level controller is exposed as a Query (and, where applicable, + Mutation) field keyed by the controller's id, so a single endpoint serves + every configured device. + """ + + def __init__(self, controller_apis: list[ControllerAPI]): + self._controller_apis = controller_apis self._app = self._create_app() def _create_app(self) -> GraphQL: - api = GraphQLAPI(self._controller_api) - schema = api.create_schema() - app = GraphQL(schema) + queries: list[StrawberryField] = [] + mutations: list[StrawberryField] = [] + for controller_api in self._controller_apis: + id = controller_api.path[0] + sub_tree = GraphQLAPI(controller_api) + if sub_tree.queries: + queries.append( + _wrap_as_field(id, create_type(f"{id}Query", sub_tree.queries)) + ) + if sub_tree.mutations: + mutations.append( + _wrap_as_field(id, create_type(f"{id}Mutation", sub_tree.mutations)) + ) + + if not queries: + raise FastCSError( + "Can't create GraphQL transport from ControllerAPIs with no read " + "attributes" + ) - return app + query = create_type("Query", queries) + mutation = create_type("Mutation", mutations) if mutations else None + schema = strawberry.Schema(query=query, mutation=mutation) + return GraphQL(schema) async def serve(self, options: GraphQLServerOptions | None = None) -> None: options = options or GraphQLServerOptions() @@ -48,7 +72,11 @@ async def serve(self, options: GraphQLServerOptions | None = None) -> None: class GraphQLAPI: - """A Strawberry API built dynamically from a `ControllerAPI`""" + """A Strawberry sub-tree built dynamically from a single `ControllerAPI`. + + Produces the per-controller queries and mutations; the combined top-level + schema is assembled by `GraphQLServer`. + """ def __init__(self, controller_api: ControllerAPI): self.queries: list[StrawberryField] = [] @@ -87,34 +115,26 @@ 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) + field_name = controller_api.path[-1] + # Type name is path-joined so subs sharing a local name across two + # top-level controllers produce distinct GraphQL types. + type_stem = "_".join(controller_api.path) child_tree = GraphQLAPI(controller_api) if child_tree.queries: self.queries.append( _wrap_as_field( - name, create_type(f"{name}Query", child_tree.queries) + field_name, + create_type(f"{type_stem}_Query", child_tree.queries), ) ) if child_tree.mutations: self.mutations.append( _wrap_as_field( - name, create_type(f"{name}Mutation", child_tree.mutations) + field_name, + create_type(f"{type_stem}_Mutation", child_tree.mutations), ) ) - def create_schema(self) -> strawberry.Schema: - """Create a Strawberry Schema to load into a GraphQL application.""" - if not self.queries: - raise FastCSError( - "Can't create GraphQL transport from ControllerAPI with no read " - "attributes" - ) - - query = create_type("Query", self.queries) - mutation = create_type("Mutation", self.mutations) if self.mutations else None - - return strawberry.Schema(query=query, mutation=mutation) - def _wrap_attr_set( attr_name: str, attribute: AttrW[DType_T] diff --git a/src/fastcs/transports/graphql/transport.py b/src/fastcs/transports/graphql/transport.py index b813b759..951ad85f 100644 --- a/src/fastcs/transports/graphql/transport.py +++ b/src/fastcs/transports/graphql/transport.py @@ -2,10 +2,11 @@ from dataclasses import dataclass, field from fastcs.controllers import ControllerAPI -from fastcs.transports.transport import Transport, _expect_single +from fastcs.transports.transport import Transport from .graphql import GraphQLServer from .options import GraphQLServerOptions +from .util import validate_graphql_id @dataclass @@ -19,8 +20,10 @@ def connect( controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - controller_api = _expect_single(controller_apis, "GraphQLTransport") - self._server = GraphQLServer(controller_api) + for api in controller_apis: + if api.path: + validate_graphql_id(api.path[0]) + self._server = GraphQLServer(controller_apis) async def serve(self) -> None: await self._server.serve(self.graphql) diff --git a/src/fastcs/transports/graphql/util.py b/src/fastcs/transports/graphql/util.py new file mode 100644 index 00000000..7244aded --- /dev/null +++ b/src/fastcs/transports/graphql/util.py @@ -0,0 +1,21 @@ +import re + +_GRAPHQL_ID_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + + +def validate_graphql_id(id: str) -> None: + """Reject controller ids that aren't valid GraphQL field names. + + GraphQL ``Name`` syntax is the most restrictive of FastCS's transports: + only letters, digits and underscores are allowed, and the first character + cannot be a digit. This drives the lowest-common-denominator id-naming + guidance for users mixing transports. + """ + if not id: + raise ValueError("Controller id is empty; ids must be non-empty") + if not _GRAPHQL_ID_RE.fullmatch(id): + raise ValueError( + f"Controller id {id!r} is not a valid GraphQL id; " + "must match GraphQL Name syntax (letters, digits, underscores; " + "no leading digit)" + ) diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 40eb9627..86fa795d 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -14,6 +14,7 @@ from fastcs.datatypes import Int from fastcs.transports.epics.ca.transport import EpicsCATransport from fastcs.transports.epics.pva.transport import EpicsPVATransport +from fastcs.transports.graphql.transport import GraphQLTransport from fastcs.transports.rest.transport import RestTransport @@ -179,6 +180,48 @@ def test_pva_transport_rejects_illegal_id_at_connect(): loop.close() +def test_graphql_transport_combines_two_controllers_under_id_keyed_query(): + """One GraphQL endpoint exposes a single combined schema with one + top-level Query field per controller id.""" + from fastapi.testclient import TestClient + + api1 = _api_with_id(_OneAttrController, "alpha") + api2 = _api_with_id(_OtherAttrController, "beta") + + loop = asyncio.new_event_loop() + try: + transport = GraphQLTransport() + transport.connect([api1, api2], loop) + + # Strawberry's ASGI app doesn't implement the lifespan protocol, so + # construct the TestClient without `with` to skip startup events. + client = TestClient(transport._server._app) + response = client.post( + "/graphql", + json={"query": "{ alpha { foo } beta { bar } }"}, + ) + assert response.status_code == 200 + assert response.json()["data"] == { + "alpha": {"foo": 0}, + "beta": {"bar": 0}, + } + finally: + loop.close() + + +def test_graphql_transport_rejects_illegal_id_at_connect(): + # Hyphens are valid for REST/EPICS but not for GraphQL field names. + api = _api_with_id(_OneAttrController, "bad-id") + + loop = asyncio.new_event_loop() + try: + transport = GraphQLTransport() + with pytest.raises(ValueError, match="bad-id"): + transport.connect([api], loop) + finally: + loop.close() + + class _LifecycleController(Controller): """Records lifecycle hook calls for end-to-end assertions.""" diff --git a/tests/transports/graphQL/test_graphql.py b/tests/transports/graphQL/test_graphql.py index 5e113f4a..46d2fc8a 100644 --- a/tests/transports/graphQL/test_graphql.py +++ b/tests/transports/graphQL/test_graphql.py @@ -26,9 +26,12 @@ class GraphQLController(MyTestController): read_string = AttrRW(String()) +_GQL_ID = "device" + + @pytest.fixture(scope="class") def gql_controller_api(class_mocker: MockerFixture): - return AssertableControllerAPI(GraphQLController(), class_mocker) + return AssertableControllerAPI(GraphQLController(), class_mocker, path=[_GQL_ID]) def nest_query(path: list[str]) -> str: @@ -47,7 +50,7 @@ def nest_mutation(path: list[str], value: Any) -> str: field = queue.pop(0) if queue: - nesting = nest_query(queue) + nesting = nest_mutation(queue, value) return f"{field} {{ {nesting} }} " else: return f"{field}(value: {json.dumps(value)})" @@ -79,7 +82,7 @@ def test_read_int( self, gql_controller_api: AssertableControllerAPI, test_client: TestClient ): expect = 0 - path = ["readInt"] + path = [_GQL_ID, "readInt"] query = f"query {{ {nest_query(path)} }}" with gql_controller_api.assert_read_here(["read_int"]): response = test_client.post("/graphql", json={"query": query}) @@ -90,7 +93,7 @@ def test_read_write_int( self, gql_controller_api: AssertableControllerAPI, test_client: TestClient ): expect = 0 - path = ["readWriteInt"] + path = [_GQL_ID, "readWriteInt"] query = f"query {{ {nest_query(path)} }}" with gql_controller_api.assert_read_here(["read_write_int"]): response = test_client.post("/graphql", json={"query": query}) @@ -108,7 +111,7 @@ def test_read_write_float( self, gql_controller_api: AssertableControllerAPI, test_client: TestClient ): expect = 0 - path = ["readWriteFloat"] + path = [_GQL_ID, "readWriteFloat"] query = f"query {{ {nest_query(path)} }}" with gql_controller_api.assert_read_here(["read_write_float"]): response = test_client.post("/graphql", json={"query": query}) @@ -126,7 +129,7 @@ def test_read_bool( self, gql_controller_api: AssertableControllerAPI, test_client: TestClient ): expect = False - path = ["readBool"] + path = [_GQL_ID, "readBool"] query = f"query {{ {nest_query(path)} }}" with gql_controller_api.assert_read_here(["read_bool"]): response = test_client.post("/graphql", json={"query": query}) @@ -137,7 +140,7 @@ def test_write_bool( self, gql_controller_api: AssertableControllerAPI, test_client: TestClient ): value = True - path = ["writeBool"] + path = [_GQL_ID, "writeBool"] mutation = f"mutation {{ {nest_mutation(path, value)} }}" with gql_controller_api.assert_write_here(["write_bool"]): response = test_client.post("/graphql", json={"query": mutation}) @@ -149,19 +152,19 @@ def test_go( ): test_client = create_test_client(gql_controller_api) - path = ["go"] + path = [_GQL_ID, "go"] mutation = f"mutation {{ {nest_query(path)} }}" with gql_controller_api.assert_execute_here(["go"]): response = test_client.post("/graphql", json={"query": mutation}) assert response.status_code == 200 - assert response.json()["data"] == {path[-1]: True} + assert response.json()["data"] == nest_response(path, True) def test_read_child1( self, gql_controller_api: AssertableControllerAPI, test_client: TestClient ): expect = 0 - path = ["SubController01", "readInt"] + path = [_GQL_ID, "SubController01", "readInt"] query = f"query {{ {nest_query(path)} }}" with gql_controller_api.assert_read_here(["SubController01", "read_int"]): response = test_client.post("/graphql", json={"query": query}) @@ -170,7 +173,7 @@ def test_read_child1( def test_read_child2(self, gql_controller_api, test_client: TestClient): expect = 0 - path = ["SubController02", "readInt"] + path = [_GQL_ID, "SubController02", "readInt"] query = f"query {{ {nest_query(path)} }}" with gql_controller_api.assert_read_here(["SubController02", "read_int"]): response = test_client.post("/graphql", json={"query": query}) From fc8e710bab13fb2f45b921ef12021d428e5b04f0 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Wed, 6 May 2026 10:56:39 +0100 Subject: [PATCH 13/16] GUI/docs emission: per-id files plus index file (#358) D4 of #351 lands as a single transport-level module. Both EPICS transports now invoke `emit_gui_files(controller_apis, options, builder)` once with the full controller list, replacing the per-controller loop that wrote everything to the same file. The module produces: - One screen/docs file per controller at `output_dir/{id}.{ext}`, preserving the order in which controllers were declared in `fastcs.yaml`. - An index file at the root of `output_dir` -- emitted even for a single controller, so the file layout is stable as the controller count changes. The GUI index uses pvi's `DLSFormatter` directly (rather than the convenience `format_index` wrapper) so that `DeviceRef.name` can be coerced to satisfy pvi's `PascalStr` constraint when controller ids legitimately start with a digit (e.g. UUID-flavoured test prefixes). The docs side mirrors the GUI shape with a minimal markdown emitter -- just enough to lift `EpicsDocs.create_docs` off its prior no-op stub. Knock-on schema/option changes: - `EpicsGUIOptions.output_path` (single file) becomes `output_dir` (directory); ditto `EpicsDocsOptions.path` -> `output_dir`. The per-controller filename is derived from the controller id. - The bundled demo, the 13 docs snippets, `tests/example_softioc.py`, the multi-transport how-to and the `launch-framework` how-to all migrate to the new field name. Both `schema.json` files are regenerated. - `EpicsGUI` loses its `create_gui()` file-writing entry point in favour of a smaller `build_device(title) -> Device` helper that the emission module composes per controller. - `tests/data/config.yaml` drops its `gui: {}` / `docs: {}` blocks -- they were schema-fixture noise that now leaks generated files into the repo CWD when the launcher tests exercise `connect()`. Tests: - New `tests/transports/epics/test_emission.py` (D4 unit tests): per-id files plus index for single and multi-controller cases, declaration order preservation, missing-output-dir creation, PVA builder propagation, and the digit-leading-id coercion. - `tests/transports/epics/ca/test_gui.py` gains a transport-level assertion that the index file is generated alongside per-controller files (per #358 acceptance criteria). - `tests/test_multi_controller.py` gains a CA scenario that drives `EpicsCATransport.connect` end-to-end and asserts both per-id and index files for GUI and docs land in their configured `output_dir`. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/how-to/launch-framework.md | 2 +- docs/how-to/multiple-transports.md | 6 +- docs/snippets/static05.py | 4 +- docs/snippets/static06.py | 4 +- docs/snippets/static07.py | 4 +- docs/snippets/static08.py | 4 +- docs/snippets/static09.py | 4 +- docs/snippets/static10.py | 4 +- docs/snippets/static11.py | 4 +- docs/snippets/static12.py | 4 +- docs/snippets/static13.py | 4 +- docs/snippets/static14.py | 4 +- docs/snippets/static15.py | 4 +- src/fastcs/demo/controller.yaml | 2 +- src/fastcs/demo/schema.json | 17 +- src/fastcs/transports/epics/__init__.py | 2 +- src/fastcs/transports/epics/ca/transport.py | 11 +- src/fastcs/transports/epics/docs.py | 20 +-- src/fastcs/transports/epics/emission.py | 154 +++++++++++++++++ src/fastcs/transports/epics/gui.py | 18 +- src/fastcs/transports/epics/options.py | 7 +- src/fastcs/transports/epics/pva/transport.py | 11 +- tests/data/config.yaml | 4 - tests/data/schema.json | 17 +- tests/example_softioc.py | 4 +- tests/test_multi_controller.py | 33 ++++ tests/transports/epics/ca/test_gui.py | 32 +++- tests/transports/epics/test_emission.py | 165 +++++++++++++++++++ 28 files changed, 448 insertions(+), 101 deletions(-) create mode 100644 src/fastcs/transports/epics/emission.py create mode 100644 tests/transports/epics/test_emission.py diff --git a/docs/how-to/launch-framework.md b/docs/how-to/launch-framework.md index 8ebdf4db..c2d10869 100644 --- a/docs/how-to/launch-framework.md +++ b/docs/how-to/launch-framework.md @@ -104,7 +104,7 @@ transport: # EPICS Channel Access - epicsca: {} gui: - output_path: "opis/device.bob" + output_dir: "opis" title: "Device Control" # REST API diff --git a/docs/how-to/multiple-transports.md b/docs/how-to/multiple-transports.md index 6a0941d7..6d8ffc8f 100644 --- a/docs/how-to/multiple-transports.md +++ b/docs/how-to/multiple-transports.md @@ -84,11 +84,11 @@ from fastcs.transports import ( epics_ca = EpicsCATransport( gui=EpicsGUIOptions( - output_path=Path(".") / "device.bob", + output_dir=Path("./opis"), title="Device Control", ), docs=EpicsDocsOptions( - output_path=Path(".") / "device.csv", + output_dir=Path("./reference"), ), ) ``` @@ -161,7 +161,7 @@ fastcs = FastCS( controller, [ EpicsCATransport( - gui=EpicsGUIOptions(output_path=Path(".") / "device.bob"), + gui=EpicsGUIOptions(output_dir=Path("./opis")), ), EpicsPVATransport(), ] diff --git a/docs/snippets/static05.py b/docs/snippets/static05.py index 29ed005e..99518071 100644 --- a/docs/snippets/static05.py +++ b/docs/snippets/static05.py @@ -12,9 +12,7 @@ class TemperatureController(Controller): device_id = AttrR(String()) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) fastcs = FastCS(TemperatureController(), [epics_ca]) diff --git a/docs/snippets/static06.py b/docs/snippets/static06.py index 16fac5d2..657d7c0c 100644 --- a/docs/snippets/static06.py +++ b/docs/snippets/static06.py @@ -22,9 +22,7 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static07.py b/docs/snippets/static07.py index 84f9fdd5..3dd2b3e8 100644 --- a/docs/snippets/static07.py +++ b/docs/snippets/static07.py @@ -44,9 +44,7 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static08.py b/docs/snippets/static08.py index 62151342..d90fc6d1 100644 --- a/docs/snippets/static08.py +++ b/docs/snippets/static08.py @@ -50,9 +50,7 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static09.py b/docs/snippets/static09.py index c72fe78f..9a265e60 100644 --- a/docs/snippets/static09.py +++ b/docs/snippets/static09.py @@ -57,9 +57,7 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(connection_settings), [epics_ca]) diff --git a/docs/snippets/static10.py b/docs/snippets/static10.py index 76b40c58..4a84c883 100644 --- a/docs/snippets/static10.py +++ b/docs/snippets/static10.py @@ -75,9 +75,7 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static11.py b/docs/snippets/static11.py index 430e7d45..817dac43 100644 --- a/docs/snippets/static11.py +++ b/docs/snippets/static11.py @@ -82,9 +82,7 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static12.py b/docs/snippets/static12.py index 6edb3313..00c13b53 100644 --- a/docs/snippets/static12.py +++ b/docs/snippets/static12.py @@ -95,9 +95,7 @@ async def update_voltages(self): await controller.voltage.update(float(voltages[index])) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static13.py b/docs/snippets/static13.py index 5a7cb0a4..bac4f04b 100644 --- a/docs/snippets/static13.py +++ b/docs/snippets/static13.py @@ -103,9 +103,7 @@ async def disable_all(self) -> None: await asyncio.sleep(0.1) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) diff --git a/docs/snippets/static14.py b/docs/snippets/static14.py index e928f7fc..8600c7bf 100644 --- a/docs/snippets/static14.py +++ b/docs/snippets/static14.py @@ -109,9 +109,7 @@ async def disable_all(self) -> None: configure_logging() -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) diff --git a/docs/snippets/static15.py b/docs/snippets/static15.py index ac8350fe..4867acb9 100644 --- a/docs/snippets/static15.py +++ b/docs/snippets/static15.py @@ -112,9 +112,7 @@ async def disable_all(self) -> None: configure_logging(LogLevel.TRACE) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) diff --git a/src/fastcs/demo/controller.yaml b/src/fastcs/demo/controller.yaml index e5c61368..d8ca1b92 100644 --- a/src/fastcs/demo/controller.yaml +++ b/src/fastcs/demo/controller.yaml @@ -14,4 +14,4 @@ transport: - epicsca: {} gui: title: Temperature Controller Demo - output_path: ./demo.bob + output_dir: . diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index e0a6b845..f20662ed 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -38,10 +38,15 @@ }, "EpicsDocsOptions": { "properties": { - "path": { + "output_dir": { "default": ".", "format": "path", - "title": "Path", + "title": "Output Dir", + "type": "string" + }, + "title": { + "default": "FastCS Devices", + "title": "Title", "type": "string" }, "depth": { @@ -71,10 +76,10 @@ }, "EpicsGUIOptions": { "properties": { - "output_path": { - "default": "output.bob", + "output_dir": { + "default": ".", "format": "path", - "title": "Output Path", + "title": "Output Dir", "type": "string" }, "file_format": { @@ -82,7 +87,7 @@ "default": ".bob" }, "title": { - "default": "Simple Device", + "default": "FastCS Devices", "title": "Title", "type": "string" } diff --git a/src/fastcs/transports/epics/__init__.py b/src/fastcs/transports/epics/__init__.py index 37d9c8f5..0165c497 100644 --- a/src/fastcs/transports/epics/__init__.py +++ b/src/fastcs/transports/epics/__init__.py @@ -1,5 +1,5 @@ -from .docs import EpicsDocsOptions as EpicsDocsOptions from .options import EpicsCAOptions as EpicsCAOptions +from .options import EpicsDocsOptions as EpicsDocsOptions from .options import EpicsGUIFormat as EpicsGUIFormat from .options import EpicsGUIOptions as EpicsGUIOptions from .options import EpicsPVAOptions as EpicsPVAOptions diff --git a/src/fastcs/transports/epics/ca/transport.py b/src/fastcs/transports/epics/ca/transport.py index 6728dc77..be2ff283 100644 --- a/src/fastcs/transports/epics/ca/transport.py +++ b/src/fastcs/transports/epics/ca/transport.py @@ -13,7 +13,7 @@ ) from fastcs.transports.epics.ca.ioc import EpicsCAIOC from fastcs.transports.epics.ca.util import validate_ca_id -from fastcs.transports.epics.docs import EpicsDocs +from fastcs.transports.epics.emission import emit_docs_files, emit_gui_files from fastcs.transports.epics.gui import EpicsGUI from fastcs.transports.epics.util import pv_prefix_from_path from fastcs.transports.transport import Transport @@ -42,12 +42,11 @@ def connect( self._pv_prefixes = [pv_prefix_from_path(api.path) for api in controller_apis] self._ioc = EpicsCAIOC(controller_apis) - for api in controller_apis: - if self.docs is not None: - EpicsDocs(api).create_docs(self.docs) + if self.docs is not None: + emit_docs_files(controller_apis, self.docs) - if self.gui is not None: - EpicsGUI(api).create_gui(self.gui) + if self.gui is not None: + emit_gui_files(controller_apis, self.gui, EpicsGUI) async def serve(self) -> None: """Serve `ControllerAPI` over EPICS Channel Access""" diff --git a/src/fastcs/transports/epics/docs.py b/src/fastcs/transports/epics/docs.py index ced56172..42260554 100644 --- a/src/fastcs/transports/epics/docs.py +++ b/src/fastcs/transports/epics/docs.py @@ -1,14 +1,12 @@ -from fastcs.controllers import ControllerAPI +"""Re-exports kept for backwards-compatible imports. -from .options import EpicsDocsOptions +Per-controller docs file emission lives in +:py:mod:`fastcs.transports.epics.emission`; transports call +:py:func:`fastcs.transports.epics.emission.emit_docs_files` directly with +the full ``list[ControllerAPI]``. +""" +from fastcs.transports.epics.emission import emit_docs_files +from fastcs.transports.epics.options import EpicsDocsOptions -class EpicsDocs: - """For creating docs in the EPICS transports.""" - - def __init__(self, controller_apis: ControllerAPI) -> None: - self._controller_apis = controller_apis - - def create_docs(self, options: EpicsDocsOptions | None = None) -> None: - if options is None: - options = EpicsDocsOptions() +__all__ = ["emit_docs_files", "EpicsDocsOptions"] diff --git a/src/fastcs/transports/epics/emission.py b/src/fastcs/transports/epics/emission.py new file mode 100644 index 00000000..f60cca2d --- /dev/null +++ b/src/fastcs/transports/epics/emission.py @@ -0,0 +1,154 @@ +"""Per-controller GUI / docs file emission for the EPICS transports. + +D4 of #351: each transport produces ``output_dir/{id}{ext}`` for every +configured controller plus an ``index{ext}`` file linking to them. The +index is always emitted -- even for a single controller -- so the file +layout is stable as the number of controllers changes. Controllers +appear in the index in the order they were declared in ``fastcs.yaml``, +which is the order in which ``controller_apis`` is passed in. +""" + +from collections.abc import Callable + +from pvi._format.dls import DLSFormatter # type: ignore +from pvi.device import Device, DeviceRef, enforce_pascal_case # type: ignore + +from fastcs.controllers import ControllerAPI +from fastcs.logging import logger +from fastcs.transports.epics.gui import EpicsGUI +from fastcs.transports.epics.options import ( + EpicsDocsOptions, + EpicsGUIFormat, + EpicsGUIOptions, +) + +GUIBuilder = Callable[[ControllerAPI], EpicsGUI] +"""Per-flavour ``EpicsGUI`` constructor (CA bare PV / PVA ``pva://`` PV).""" + +INDEX_STEM = "index" + + +def _coerce_pascal_name(controller_id: str) -> str: + """Coerce an arbitrary controller id into a valid pvi ``PascalStr``. + + pvi's :py:class:`DeviceRef` constrains ``name`` to ``PascalStr`` (regex + ``^([A-Z][a-z0-9]*)*$``). FastCS controller ids range over the looser + union of every transport's charset and may legitimately start with a + digit (e.g. UUID-flavoured test prefixes), so we prepend ``X`` when + needed before delegating to ``pvi.device.enforce_pascal_case``. + """ + candidate = enforce_pascal_case(controller_id) + if candidate and not candidate[0].isupper(): + candidate = "X" + candidate + return candidate + + +def emit_gui_files( + controller_apis: list[ControllerAPI], + options: EpicsGUIOptions, + gui_builder: GUIBuilder, +) -> None: + """Write ``{id}{ext}`` per controller plus an index file in ``output_dir``. + + ``gui_builder`` lets each EPICS transport pick its PV-flavoured GUI + builder (CA's ``EpicsGUI`` writes bare PVs; PVA's ``PvaEpicsGUI`` + prefixes them with ``pva://``). An index file with one ``DeviceRef`` + button per controller sits at the root of ``output_dir`` regardless + of how many controllers there are; controllers appear in the order + they were declared in ``fastcs.yaml`` (i.e. the order of + ``controller_apis``). + """ + if options.file_format is EpicsGUIFormat.edl: + logger.warning("FastCS may not support all widgets in .edl screens") + + ext = options.file_format.value + output_dir = options.output_dir + output_dir.mkdir(parents=True, exist_ok=True) + + formatter = DLSFormatter() + + refs: list[DeviceRef] = [] + for api in controller_apis: + controller_id = api.path[0] + ui_filename = f"{controller_id}{ext}" + controller_path = (output_dir / ui_filename).resolve() + + device = gui_builder(api).build_device(options.title) + formatter.format(device, controller_path) + + refs.append( + DeviceRef( + name=_coerce_pascal_name(controller_id), + label=controller_id, + pv=controller_id.upper(), + ui=ui_filename, + macros={}, + ) + ) + + index_path = (output_dir / f"{INDEX_STEM}{ext}").resolve() + formatter.format(Device(label=options.title, children=refs), index_path) + + +DOCS_EXT = ".md" + + +def emit_docs_files( + controller_apis: list[ControllerAPI], + options: EpicsDocsOptions, +) -> None: + """Write ``{id}.md`` per controller plus ``index.md`` in ``output_dir``. + + Each per-controller file lists the controller's attributes and + commands as a flat reference page. The index links to each one in + declaration order, mirroring the GUI emission. + """ + output_dir = options.output_dir + output_dir.mkdir(parents=True, exist_ok=True) + + for api in controller_apis: + controller_id = api.path[0] + path = output_dir / f"{controller_id}{DOCS_EXT}" + 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)) + + +def _render_index_md(controller_apis: list[ControllerAPI], title: str) -> str: + lines = [f"# {title}", ""] + for api in controller_apis: + controller_id = api.path[0] + lines.append(f"- [{controller_id}]({controller_id}{DOCS_EXT})") + lines.append("") + return "\n".join(lines) + + +def _render_controller_md( + api: ControllerAPI, depth: int | None, _level: int = 0 +) -> str: + if depth is not None and _level > depth: + return "" + + heading = "#" * (_level + 1) + title = ":".join(api.path) if api.path else "(root)" + lines = [f"{heading} {title}", ""] + + if api.attributes: + lines.append(f"{heading}# Attributes") + lines.append("") + for name in api.attributes: + lines.append(f"- `{name}`") + lines.append("") + + if api.command_methods: + lines.append(f"{heading}# Commands") + lines.append("") + for name in api.command_methods: + lines.append(f"- `{name}`") + lines.append("") + + for sub_api in api.sub_apis.values(): + lines.append(_render_controller_md(sub_api, depth, _level + 1)) + + return "\n".join(lines).rstrip() + "\n" diff --git a/src/fastcs/transports/epics/gui.py b/src/fastcs/transports/epics/gui.py index 6e79ea09..882a83a0 100644 --- a/src/fastcs/transports/epics/gui.py +++ b/src/fastcs/transports/epics/gui.py @@ -1,4 +1,3 @@ -from pvi._format.dls import DLSFormatter # type: ignore from pvi.device import ( LED, ArrayTrace, @@ -35,7 +34,6 @@ ) from fastcs.logging import logger from fastcs.methods import Command -from fastcs.transports.epics.options import EpicsGUIFormat, EpicsGUIOptions from fastcs.transports.epics.util import pv_prefix_from_path from fastcs.util import snake_to_pascal @@ -145,21 +143,9 @@ def _get_command_component(self, attr_path: list[str], name: str): write_widget=ButtonPanel(actions={name: self.command_value}), ) - def create_gui(self, options: EpicsGUIOptions | None = None) -> None: - if options is None: - options = EpicsGUIOptions() - - if options.file_format is EpicsGUIFormat.edl: - logger.warning("FastCS may not support all widgets in .edl screens") - - assert options.output_path.suffix == options.file_format.value - options.output_path.parent.mkdir(parents=True, exist_ok=True) - + def build_device(self, title: str) -> Device: components = self.extract_api_components(self._controller_api) - device = Device(label=options.title, children=components) - - formatter = DLSFormatter() - formatter.format(device, options.output_path.resolve()) + return Device(label=title, children=components) def extract_api_components(self, controller_api: ControllerAPI) -> Tree: components: Tree = [] diff --git a/src/fastcs/transports/epics/options.py b/src/fastcs/transports/epics/options.py index f0e37ddf..55946400 100644 --- a/src/fastcs/transports/epics/options.py +++ b/src/fastcs/transports/epics/options.py @@ -7,7 +7,8 @@ class EpicsDocsOptions: """Docs options for EPICS.""" - path: Path = Path(".") + output_dir: Path = Path(".") + title: str = "FastCS Devices" depth: int | None = None @@ -22,9 +23,9 @@ class EpicsGUIFormat(Enum): class EpicsGUIOptions: """Epics GUI options for use in both CA and PVA transports.""" - output_path: Path = Path(".") / "output.bob" + output_dir: Path = Path(".") file_format: EpicsGUIFormat = EpicsGUIFormat.bob - title: str = "Simple Device" + title: str = "FastCS Devices" @dataclass diff --git a/src/fastcs/transports/epics/pva/transport.py b/src/fastcs/transports/epics/pva/transport.py index 327da8c0..d84ff98b 100644 --- a/src/fastcs/transports/epics/pva/transport.py +++ b/src/fastcs/transports/epics/pva/transport.py @@ -8,7 +8,7 @@ EpicsGUIOptions, EpicsPVAOptions, ) -from fastcs.transports.epics.docs import EpicsDocs +from fastcs.transports.epics.emission import emit_docs_files, emit_gui_files from fastcs.transports.epics.pva.gui import PvaEpicsGUI from fastcs.transports.epics.pva.util import validate_pva_id from fastcs.transports.epics.util import pv_prefix_from_path @@ -37,12 +37,11 @@ def connect( self._pv_prefixes = [pv_prefix_from_path(api.path) for api in controller_apis] self._ioc = P4PIOC(controller_apis) - for api in controller_apis: - if self.docs is not None: - EpicsDocs(api).create_docs(self.docs) + if self.docs is not None: + emit_docs_files(controller_apis, self.docs) - if self.gui is not None: - PvaEpicsGUI(api).create_gui(self.gui) + if self.gui is not None: + emit_gui_files(controller_apis, self.gui, PvaEpicsGUI) async def serve(self) -> None: """Serve `ControllerAPI` over EPICS PVAccess""" diff --git a/tests/data/config.yaml b/tests/data/config.yaml index 3d19562c..d7f04f3e 100644 --- a/tests/data/config.yaml +++ b/tests/data/config.yaml @@ -5,11 +5,7 @@ controllers: name: controller-name transport: - epicsca: {} - docs: {} - gui: {} - epicspva: {} - docs: {} - gui: {} - rest: {} - tango: {} - graphql: {} diff --git a/tests/data/schema.json b/tests/data/schema.json index a5f5b54a..fae8d15a 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -38,10 +38,15 @@ }, "EpicsDocsOptions": { "properties": { - "path": { + "output_dir": { "default": ".", "format": "path", - "title": "Path", + "title": "Output Dir", + "type": "string" + }, + "title": { + "default": "FastCS Devices", + "title": "Title", "type": "string" }, "depth": { @@ -71,10 +76,10 @@ }, "EpicsGUIOptions": { "properties": { - "output_path": { - "default": "output.bob", + "output_dir": { + "default": ".", "format": "path", - "title": "Output Path", + "title": "Output Dir", "type": "string" }, "file_format": { @@ -82,7 +87,7 @@ "default": ".bob" }, "title": { - "default": "Simple Device", + "default": "FastCS Devices", "title": "Title", "type": "string" } diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 2443102f..89687832 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -26,9 +26,7 @@ def run(id="SOFTIOC_TEST_DEVICE"): controller.set_id(id) 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" - ) + gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Vector") fastcs = FastCS( controller, [EpicsCATransport(gui=gui_options)], diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 86fa795d..3ef5a206 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -3,6 +3,7 @@ """ import asyncio +from pathlib import Path import pytest from fastapi.testclient import TestClient @@ -12,7 +13,9 @@ from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.datatypes import Int +from fastcs.transports.epics import EpicsDocsOptions, EpicsGUIOptions from fastcs.transports.epics.ca.transport import EpicsCATransport +from fastcs.transports.epics.emission import INDEX_STEM from fastcs.transports.epics.pva.transport import EpicsPVATransport from fastcs.transports.graphql.transport import GraphQLTransport from fastcs.transports.rest.transport import RestTransport @@ -137,6 +140,36 @@ def test_ca_transport_routes_two_controllers_with_distinct_pv_prefixes(mocker): ) +def test_ca_transport_emits_per_controller_gui_and_docs_files(mocker, tmp_path: Path): + """#358: GUI/docs emission writes ``output_dir/{id}.bob`` per controller + plus an index file alongside, even via the transport-level wiring.""" + mocker.patch("fastcs.transports.epics.ca.util.builder") + mocker.patch("fastcs.transports.epics.ca.ioc.builder") + + api1 = _api_with_id(_OneAttrController, "ALPHA") + api2 = _api_with_id(_OtherAttrController, "BETA") + + gui_dir = tmp_path / "opis" + docs_dir = tmp_path / "reference" + + loop = asyncio.new_event_loop() + try: + transport = EpicsCATransport( + gui=EpicsGUIOptions(output_dir=gui_dir), + docs=EpicsDocsOptions(output_dir=docs_dir), + ) + transport.connect([api1, api2], loop) + finally: + loop.close() + + assert (gui_dir / "ALPHA.bob").is_file() + assert (gui_dir / "BETA.bob").is_file() + assert (gui_dir / f"{INDEX_STEM}.bob").is_file() + assert (docs_dir / "ALPHA.md").is_file() + assert (docs_dir / "BETA.md").is_file() + assert (docs_dir / f"{INDEX_STEM}.md").is_file() + + def test_ca_transport_rejects_illegal_id_at_connect(): api = _api_with_id(_OneAttrController, "bad/id") diff --git a/tests/transports/epics/ca/test_gui.py b/tests/transports/epics/ca/test_gui.py index 77be76dc..8f4ce4da 100644 --- a/tests/transports/epics/ca/test_gui.py +++ b/tests/transports/epics/ca/test_gui.py @@ -1,3 +1,5 @@ +from pathlib import Path + import numpy as np import pytest from pvi.device import ( @@ -19,9 +21,11 @@ from tests.util import ColourEnum from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.controllers import ControllerAPI +from fastcs.controllers import Controller, ControllerAPI from fastcs.datatypes import Bool, Enum, Float, Int, String, Waveform +from fastcs.transports.epics.emission import INDEX_STEM, emit_gui_files from fastcs.transports.epics.gui import EpicsGUI +from fastcs.transports.epics.options import EpicsGUIOptions def test_get_pv(): @@ -188,3 +192,29 @@ def test_get_command_component(): assert isinstance(component, SignalX) assert component.write_widget == ButtonPanel(actions={"Command": "1"}) + + +class _A(Controller): + foo = AttrR(Int()) + + +class _B(Controller): + bar = AttrR(Int()) + + +def _api_with_id(cls, name): + c = cls() + c.set_id(name) + api, _, _ = c.create_api_and_tasks() + return api + + +def test_emit_gui_writes_index_alongside_per_controller_files(tmp_path: Path): + """#358 acceptance criteria: per-id .bob files and an index file.""" + apis = [_api_with_id(_A, "alpha"), _api_with_id(_B, "beta")] + + emit_gui_files(apis, EpicsGUIOptions(output_dir=tmp_path), EpicsGUI) + + assert (tmp_path / "alpha.bob").is_file() + assert (tmp_path / "beta.bob").is_file() + assert (tmp_path / f"{INDEX_STEM}.bob").is_file() diff --git a/tests/transports/epics/test_emission.py b/tests/transports/epics/test_emission.py new file mode 100644 index 00000000..005c6f0c --- /dev/null +++ b/tests/transports/epics/test_emission.py @@ -0,0 +1,165 @@ +"""D4 unit tests for per-controller GUI / docs file emission (#358).""" + +from pathlib import Path + +import pytest + +from fastcs.attributes import AttrR +from fastcs.controllers import Controller +from fastcs.datatypes import Int +from fastcs.transports.epics.emission import ( + DOCS_EXT, + INDEX_STEM, + emit_docs_files, + emit_gui_files, +) +from fastcs.transports.epics.gui import EpicsGUI +from fastcs.transports.epics.options import ( + EpicsDocsOptions, + EpicsGUIFormat, + EpicsGUIOptions, +) +from fastcs.transports.epics.pva.gui import PvaEpicsGUI + + +class _Alpha(Controller): + foo = AttrR(Int()) + + +class _Beta(Controller): + bar = AttrR(Int()) + + +def _api_with_id(controller_class: type[Controller], id: str): + controller = controller_class() + controller.set_id(id) + api, _, _ = controller.create_api_and_tasks() + return api + + +@pytest.fixture +def two_apis(): + return [_api_with_id(_Alpha, "alpha"), _api_with_id(_Beta, "beta")] + + +@pytest.fixture +def one_api(): + return [_api_with_id(_Alpha, "alpha")] + + +# --- GUI emission ---------------------------------------------------------- + + +def test_gui_emits_per_controller_files_and_index(two_apis, tmp_path: Path): + options = EpicsGUIOptions(output_dir=tmp_path) + emit_gui_files(two_apis, options, EpicsGUI) + + assert (tmp_path / "alpha.bob").is_file() + assert (tmp_path / "beta.bob").is_file() + assert (tmp_path / f"{INDEX_STEM}.bob").is_file() + + +def test_gui_emits_index_even_for_single_controller(one_api, tmp_path: Path): + """Stable file layout: index is emitted regardless of controller count.""" + emit_gui_files(one_api, EpicsGUIOptions(output_dir=tmp_path), EpicsGUI) + + assert (tmp_path / "alpha.bob").is_file() + assert (tmp_path / f"{INDEX_STEM}.bob").is_file() + + +def test_gui_index_lists_controllers_in_declaration_order(two_apis, tmp_path: Path): + """User story 23: index buttons follow YAML declaration order.""" + emit_gui_files(two_apis, EpicsGUIOptions(output_dir=tmp_path), EpicsGUI) + + text = (tmp_path / f"{INDEX_STEM}.bob").read_text() + alpha_pos = text.index("alpha.bob") + beta_pos = text.index("beta.bob") + assert alpha_pos < beta_pos + + +def test_gui_index_reverses_when_apis_reversed(two_apis, tmp_path: Path): + """If declaration order flips, the index order flips too.""" + emit_gui_files( + list(reversed(two_apis)), + EpicsGUIOptions(output_dir=tmp_path), + EpicsGUI, + ) + + text = (tmp_path / f"{INDEX_STEM}.bob").read_text() + assert text.index("beta.bob") < text.index("alpha.bob") + + +def test_gui_creates_missing_output_dir(two_apis, tmp_path: Path): + nested = tmp_path / "deep" / "opis" + emit_gui_files(two_apis, EpicsGUIOptions(output_dir=nested), EpicsGUI) + + assert (nested / "alpha.bob").is_file() + assert (nested / f"{INDEX_STEM}.bob").is_file() + + +def test_gui_index_tolerates_digit_leading_id(tmp_path: Path): + """pvi's index DeviceRef requires PascalStr; ids starting with a digit + (e.g. UUID-flavoured test prefixes) get coerced safely instead of + blowing up at validation time.""" + api = _api_with_id(_Alpha, "120e1b648d9a4ec8a2ae2bf33cf3e1ee") + emit_gui_files([api], EpicsGUIOptions(output_dir=tmp_path), EpicsGUI) + + assert (tmp_path / "120e1b648d9a4ec8a2ae2bf33cf3e1ee.bob").is_file() + assert (tmp_path / f"{INDEX_STEM}.bob").is_file() + + +def test_gui_uses_pva_builder_for_pva_transport(two_apis, tmp_path: Path): + """PVA transport's GUI builder writes ``pva://`` PV prefixes.""" + emit_gui_files(two_apis, EpicsGUIOptions(output_dir=tmp_path), PvaEpicsGUI) + + assert "pva://alpha:Foo" in (tmp_path / "alpha.bob").read_text() + assert "pva://beta:Bar" in (tmp_path / "beta.bob").read_text() + + +def test_gui_respects_file_format(two_apis, tmp_path: Path): + """The configured file format propagates to per-controller and index files.""" + options = EpicsGUIOptions(output_dir=tmp_path, file_format=EpicsGUIFormat.bob) + emit_gui_files(two_apis, options, EpicsGUI) + + assert (tmp_path / "alpha.bob").is_file() + assert (tmp_path / f"{INDEX_STEM}.bob").is_file() + + +# --- docs emission --------------------------------------------------------- + + +def test_docs_emits_per_controller_files_and_index(two_apis, tmp_path: Path): + emit_docs_files(two_apis, EpicsDocsOptions(output_dir=tmp_path)) + + assert (tmp_path / f"alpha{DOCS_EXT}").is_file() + assert (tmp_path / f"beta{DOCS_EXT}").is_file() + assert (tmp_path / f"{INDEX_STEM}{DOCS_EXT}").is_file() + + +def test_docs_emits_index_even_for_single_controller(one_api, tmp_path: Path): + emit_docs_files(one_api, EpicsDocsOptions(output_dir=tmp_path)) + + assert (tmp_path / f"alpha{DOCS_EXT}").is_file() + assert (tmp_path / f"{INDEX_STEM}{DOCS_EXT}").is_file() + + +def test_docs_index_lists_controllers_in_declaration_order(two_apis, tmp_path: Path): + emit_docs_files(two_apis, EpicsDocsOptions(output_dir=tmp_path)) + + index_text = (tmp_path / f"{INDEX_STEM}{DOCS_EXT}").read_text() + assert index_text.index("alpha") < index_text.index("beta") + + +def test_docs_per_controller_file_lists_attributes(two_apis, tmp_path: Path): + emit_docs_files(two_apis, EpicsDocsOptions(output_dir=tmp_path)) + + assert "foo" in (tmp_path / f"alpha{DOCS_EXT}").read_text() + assert "bar" in (tmp_path / f"beta{DOCS_EXT}").read_text() + + +def test_docs_creates_missing_output_dir(two_apis, tmp_path: Path): + nested = tmp_path / "deep" / "reference" + emit_docs_files(two_apis, EpicsDocsOptions(output_dir=nested)) + + assert (nested / f"alpha{DOCS_EXT}").is_file() + assert (nested / f"{INDEX_STEM}{DOCS_EXT}").is_file() From f2c7cef9ab7a9b1ac6738821df8eac65442ebbc1 Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Wed, 6 May 2026 11:33:56 +0100 Subject: [PATCH 14/16] Fix tutorial emphasize-lines after #358 snippet collapse The #358 commit migrated the docs/snippets EpicsGUIOptions(...) calls from a 3-line to a 1-line form via ruff format, shifting every line below `gui_options = ...` up by 2 in static05/06/10/14/15.py. The tutorial's `:emphasize-lines:` references for those snippets in docs/tutorials/static-drivers.md were left pointing at the old positions, which made `sphinx -W` fail with two "line number spec is out of range" warnings on static05 and static10. Update each affected range so the highlighted lines correspond to the same code as before. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/tutorials/static-drivers.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/tutorials/static-drivers.md b/docs/tutorials/static-drivers.md index 31c34592..f28f9b60 100644 --- a/docs/tutorials/static-drivers.md +++ b/docs/tutorials/static-drivers.md @@ -129,7 +129,7 @@ options to the transport options and generate a `demo.bob` file to use with Phoe :class: dropdown, hint :::{literalinclude} /snippets/static05.py -:emphasize-lines: 1,7,16-20,22 +:emphasize-lines: 1,7,15-18,20 ::: :::: @@ -155,7 +155,7 @@ The simulator control connection is on port 25565. :class: dropdown, hint :::{literalinclude} /snippets/static06.py -:emphasize-lines: 4,15-22,29-30 +:emphasize-lines: 4,15-22,27-28 ::: :::: @@ -288,7 +288,7 @@ are, which is used to register the correct number of ramp controllers with the p :class: dropdown, hint :::{literalinclude} /snippets/static10.py -:emphasize-lines: 10,28,32,35,44,48-56,64,70-74,85 +:emphasize-lines: 10,28,32,35,44,48-56,64,70-74,83 ::: :::: @@ -419,7 +419,7 @@ logger for `TemperatureControllerAttributeIO` to log the commands it sends. :class: dropdown, hint :::{literalinclude} /snippets/static14.py -:emphasize-lines: 13,48,110,117 +:emphasize-lines: 13,48,110,115 ::: :::: @@ -444,7 +444,7 @@ visible. :class: dropdown, hint :::{literalinclude} /snippets/static15.py -:emphasize-lines: 13,49-51,120 +:emphasize-lines: 13,49-51,118 ::: :::: From f6600bc98c493811862d4c14fe85278b575fc36d Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Wed, 6 May 2026 12:28:41 +0100 Subject: [PATCH 15/16] Demo two controllers, rename controller.yaml -> fastcs.yaml, migration guide Bundled demo now hosts two TemperatureController instances (MAIN/AUX) on distinct ports so the multi-controller feature is visible end-to-end. The demo simulation grows a second TempController on port 25566 with its own sink to drive the AUX controller; the main one stays on 25565. Renames src/fastcs/demo/controller.yaml -> src/fastcs/demo/fastcs.yaml. The launcher already takes the config path as a CLI argument, so nothing hard-codes the new name. .vscode/launch.json updated; schema.json unchanged (the dict-by-id form was already in place). docs/how-to/launch-framework.md examples migrate to fastcs.yaml and gain a "Hosting multiple controllers" section. New docs/how-to/migrate-to-multi-controller.md covers the breaking-change manual migration steps: file rename, controller: -> controllers:{id} dict, EpicsIOCOptions.pv_prefix removal, type: discriminator with single-class inference, GUI/docs output_dir rename. Fixes #359 --- .vscode/launch.json | 2 +- docs/how-to/launch-framework.md | 51 +++++++- docs/how-to/migrate-to-multi-controller.md | 122 ++++++++++++++++++ .../demo/{controller.yaml => fastcs.yaml} | 8 +- .../demo/simulation/temp_controller.yaml | 25 +++- 5 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 docs/how-to/migrate-to-multi-controller.md rename src/fastcs/demo/{controller.yaml => fastcs.yaml} (73%) diff --git a/.vscode/launch.json b/.vscode/launch.json index 88030120..22bceb95 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -41,7 +41,7 @@ "module": "fastcs.demo", "args": [ "run", - "${workspaceFolder:FastCS}/src/fastcs/demo/controller.yaml", + "${workspaceFolder:FastCS}/src/fastcs/demo/fastcs.yaml", "--log-level", "TRACE", // "--graylog-endpoint", diff --git a/docs/how-to/launch-framework.md b/docs/how-to/launch-framework.md index c2d10869..10267cce 100644 --- a/docs/how-to/launch-framework.md +++ b/docs/how-to/launch-framework.md @@ -54,10 +54,12 @@ if __name__ == "__main__": ## YAML Configuration Files -Create a YAML configuration file matching the schema: +Create a YAML configuration file matching the schema. The conventional +filename is `fastcs.yaml`, but any filename works — `run` takes the path +as an argument: ```yaml -# device_config.yaml +# fastcs.yaml controllers: DEVICE: controller: @@ -75,9 +77,48 @@ verbatim as the EPICS PV prefix and as the REST route prefix. Run with: ```bash -python my_driver.py run device_config.yaml +python my_driver.py run fastcs.yaml ``` +### Hosting multiple controllers + +`controllers:` is a dict, so a single application can host more than one +controller. Each entry needs a unique id (the dict key); together with the +optional `type:` discriminator it selects which class to instantiate. + +When `launch()` was given a single Controller class, `type:` may be +omitted — it defaults to that class's discriminator. The bundled demo +(`python -m fastcs.demo run src/fastcs/demo/fastcs.yaml`) hosts two +`DeviceController` instances on different ports to exercise this case +end-to-end: + +```yaml +# fastcs.yaml +controllers: + MAIN: + controller: + ip_address: "192.168.1.100" + port: 25565 + timeout: 10.0 + AUX: + controller: + ip_address: "192.168.1.101" + port: 25565 + timeout: 10.0 + +transport: + - epicsca: {} +``` + +When more than one class is registered with +`launch([ClassA, ClassB])`, every entry must carry an explicit +`type:` key naming the class. + +The transport list is shared across all controllers: each transport sees +the full set, and uses the per-entry id as the addressing prefix +(EPICS PV prefix, REST route prefix, GraphQL top-level Query field, Tango +device name segment). + ## Schema Generation Generate JSON schema for the configuration yaml: @@ -124,10 +165,10 @@ The `run` command includes logging options: ```bash # Set log level -python my_driver.py run config.yaml --log-level debug +python my_driver.py run fastcs.yaml --log-level debug # Send logs to Graylog -python my_driver.py run config.yaml \ +python my_driver.py run fastcs.yaml \ --graylog-endpoint "graylog.example.com:12201" \ --graylog-static-fields "app=my_driver,env=prod" ``` diff --git a/docs/how-to/migrate-to-multi-controller.md b/docs/how-to/migrate-to-multi-controller.md new file mode 100644 index 00000000..47361868 --- /dev/null +++ b/docs/how-to/migrate-to-multi-controller.md @@ -0,0 +1,122 @@ +# Migrate to Multi-Controller FastCS + +FastCS now supports more than one top-level Controller per application. +The launch-framework config schema, the EPICS option dataclasses, and +the bundled demo all changed shape to accommodate this. This guide +covers the manual migration steps for an existing FastCS app. + +## 1. Rename `controller.yaml` → `fastcs.yaml` + +The bundled demo's config file moved from +`src/fastcs/demo/controller.yaml` to `src/fastcs/demo/fastcs.yaml`. The +name `fastcs.yaml` is now the recommended convention for application +configs, but the launcher does not hard-code it — `python -m my_driver +run ` still accepts any path. If you rely on the demo path +explicitly (e.g. in a `launch.json` debug config), update it. + +## 2. `controller:` → `controllers: { : ... }` + +The top-level singular `controller:` block is gone. Replace it with a +dict keyed by controller id: + +```yaml +# Before +controller: + ip_address: "192.168.1.100" + port: 25565 + +transport: + - epicsca: {} +``` + +```yaml +# After +controllers: + DEVICE: # id — used as the addressing prefix + controller: # nested options block (same fields as before) + ip_address: "192.168.1.100" + port: 25565 + +transport: + - epicsca: {} +``` + +The dict key (here `DEVICE`) is the controller id. It is used verbatim +as the EPICS PV prefix, the REST route prefix, the GraphQL top-level +Query field, and the Tango device-name segment. See +[Run Multiple Transports Simultaneously](multiple-transports.md) for +the per-transport id charset rules — GraphQL's `[A-Za-z_][A-Za-z0-9_]*` +is the lowest common denominator. + +To host more than one controller, add more dict entries. Duplicate ids +are rejected at config-load time. + +## 3. Drop `pv_prefix` from `EpicsIOCOptions` + +`EpicsIOCOptions` and its `pv_prefix` field are removed. The PV prefix +is now derived from the controller id, so a transport block that used +to look like: + +```yaml +# Before +transport: + - epicsca: + pv_prefix: DEVICE +``` + +becomes: + +```yaml +# After +transport: + - epicsca: {} +``` + +The same applies to PVA. If you construct transports in Python rather +than via YAML, replace `EpicsCATransport(epicsca=EpicsIOCOptions( +pv_prefix="DEVICE"))` with `EpicsCATransport()` plus +`controller.set_id("DEVICE")` (or set the id from the YAML key when +using `launch()`). + +## 4. `type:` discriminator and single-class inference + +Each entry under `controllers:` carries a `type:` discriminator that +names the Controller class to instantiate. When `launch()` is called +with a single class, `type:` may be omitted — it defaults to that +class's discriminator (the class `__name__`, or +`type_name: ClassVar[str]` on the class if set). When `launch()` is +called with more than one class, every entry must carry an explicit +`type:`. + +```yaml +# Two-class app: launch([Lakeshore, Eurotherm]) +controllers: + CRYO: + type: Lakeshore + controller: + ip_address: "192.168.1.100" + OVEN: + type: Eurotherm + controller: + ip_address: "192.168.1.101" + +transport: + - epicsca: {} +``` + +## 5. Direct `FastCS(...)` usage is unchanged for the single-controller case + +If you instantiate `FastCS` directly rather than via `launch()`, the +single-controller form `FastCS(controller, transports)` still works. +For multi-controller, pass a sequence: +`FastCS([controller_a, controller_b], transports)`. Each Controller +must have had `set_id(...)` called before being handed to `FastCS`. + +## 6. GUI/docs emission output is now a directory + +`EpicsGUIOptions.output_path` (single file) was renamed to +`output_dir` (directory). `EpicsDocsOptions.path` likewise renamed to +`output_dir`. Per-controller files (`.bob`, `.md`) plus an +`index.` are written into the directory — even when only one +controller is configured. Update any YAML or Python that set the old +field names. diff --git a/src/fastcs/demo/controller.yaml b/src/fastcs/demo/fastcs.yaml similarity index 73% rename from src/fastcs/demo/controller.yaml rename to src/fastcs/demo/fastcs.yaml index d8ca1b92..7c65bfaa 100644 --- a/src/fastcs/demo/controller.yaml +++ b/src/fastcs/demo/fastcs.yaml @@ -1,11 +1,17 @@ # yaml-language-server: $schema=schema.json controllers: - TEMPERATURE: + MAIN: controller: ip_settings: ip: "localhost" port: 25565 num_ramp_controllers: 4 + AUX: + controller: + ip_settings: + ip: "localhost" + port: 25566 + num_ramp_controllers: 2 transport: - graphql: host: localhost diff --git a/src/fastcs/demo/simulation/temp_controller.yaml b/src/fastcs/demo/simulation/temp_controller.yaml index 682ce636..497485a6 100644 --- a/src/fastcs/demo/simulation/temp_controller.yaml +++ b/src/fastcs/demo/simulation/temp_controller.yaml @@ -4,7 +4,7 @@ value: 42.0 - type: fastcs.demo.simulation.device.TempController - name: tempcont + name: tempcont_main inputs: flux: component: source @@ -12,10 +12,29 @@ num_ramp_controllers: 4 default_start: 10 default_end: 50 + port: 25565 + +- type: fastcs.demo.simulation.device.TempController + name: tempcont_aux + inputs: + flux: + component: source + port: value + num_ramp_controllers: 2 + default_start: 5 + default_end: 30 + port: 25566 + +- type: tickit.devices.sink.Sink + name: sink_main + inputs: + flux: + component: tempcont_main + port: flux - type: tickit.devices.sink.Sink - name: sink + name: sink_aux inputs: flux: - component: tempcont + component: tempcont_aux port: flux From 207d68d8628a39d6c853b91624d4c6776e848fbf Mon Sep 17 00:00:00 2001 From: Giles Knap Date: Wed, 6 May 2026 13:00:57 +0100 Subject: [PATCH 16/16] Tango multi-device per controller with id in device name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TangoTransport.connect now accepts list[ControllerAPI] without the _expect_single guard. validate_tango_id (in transports/tango/util.py) runs per-id at connect, rejecting characters outside [A-Za-z0-9_-] with a clear startup error. New helpers tango_dev_class_name (id sanitised into a valid Python class name: hyphens to underscores, leading-digit prefix X) and tango_dev_name(id, dsr_instance) build the three-segment "//" device name with the id as the leading segment. TangoDSR holds list[type] _devices, one Tango device class per controller, and registers them via a single tango.server.run() call. The DSR's server name is fixed to FASTCS_TANGO_SERVER_NAME ("FastCS") so a multi-class server has a single identity independent of any one controller's class. _collect_dev_attributes / _collect_dev_commands now use the path relative to the device root (path[len(root):]) so the leading id is the device name and not also folded into attribute names — preserves single-controller attribute/command naming. dev_name dropped from TangoDSROptions; the device name is fully derived from the id and dsr_instance now, mirroring the EpicsIOCOptions.pv_prefix removal in #354. register_dev grows an optional server_name arg (defaults to dev_class for back-compat) and a new register_controller_devs helper batch-registers every controller under FASTCS_TANGO_SERVER_NAME. Tests: - tests/transports/tango/test_tango_util.py: validator + helper unit coverage including hyphen replacement and leading-digit prefix. - tests/test_multi_controller.py: two-controllers-distinct-devices scenario + id-validation fail-fast on bad-char ids. - tests/transports/tango/test_dsr.py: existing single-controller coverage updated for path=["DEVICE"] root and the _devices list. - tests/conftest.py register_device fixture and tests/benchmarking/controller.py migrated off the dropped dev_name. - Both schema.json files regenerated. docs/how-to/multiple-transports.md gains a Tango row in the charset table (the existing PVA row also gains the 60-char note in passing) and shows the dsr_instance-only options. The PVA-row drive-by closes half of #366. Fixes #357 --- docs/how-to/multiple-transports.md | 9 ++- src/fastcs/demo/schema.json | 5 -- src/fastcs/transports/tango/dsr.py | 78 +++++++++++++++++------ src/fastcs/transports/tango/options.py | 1 - src/fastcs/transports/tango/transport.py | 8 ++- src/fastcs/transports/tango/util.py | 37 +++++++++++ tests/benchmarking/controller.py | 3 +- tests/conftest.py | 13 ++-- tests/data/schema.json | 5 -- tests/test_multi_controller.py | 48 ++++++++++++++ tests/transports/tango/test_dsr.py | 4 +- tests/transports/tango/test_tango_util.py | 50 +++++++++++++++ 12 files changed, 217 insertions(+), 44 deletions(-) create mode 100644 tests/transports/tango/test_tango_util.py diff --git a/docs/how-to/multiple-transports.md b/docs/how-to/multiple-transports.md index 6d8ffc8f..14dc4aef 100644 --- a/docs/how-to/multiple-transports.md +++ b/docs/how-to/multiple-transports.md @@ -39,8 +39,9 @@ and each enforces its own charset at startup. | Transport | Allowed id charset | |-----------|--------------------| | EPICS CA | `[A-Za-z0-9_-]+`, plus the 60-char PV name limit | -| EPICS PVA | `[A-Za-z0-9_-]+` | +| EPICS PVA | `[A-Za-z0-9_-]+`, plus the 60-char PV name limit | | REST | `[A-Za-z0-9_-]+` | +| Tango | `[A-Za-z0-9_-]+` | | GraphQL | `[A-Za-z_][A-Za-z0-9_]*` (GraphQL `Name`: no hyphens, no leading digit) | If you serve the same controller through multiple transports, use the @@ -137,11 +138,15 @@ from fastcs.transports import TangoTransport, TangoDSROptions tango = TangoTransport( tango=TangoDSROptions( - device_name="test/device/1", + dsr_instance="MY_SERVER_INSTANCE", ), ) ``` +The Tango device name for each controller is derived from its id — +`{id}/{dev_class}/{dsr_instance}`. The id forms the leading device-name +segment, so multiple controllers in one DSR get distinct device names. + ## EPICS CA + PVA Together Run both EPICS protocols simultaneously: diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index f20662ed..7187a07b 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -209,11 +209,6 @@ }, "TangoDSROptions": { "properties": { - "dev_name": { - "default": "MY/DEVICE/NAME", - "title": "Dev Name", - "type": "string" - }, "dsr_instance": { "default": "MY_SERVER_INSTANCE", "title": "Dsr Instance", diff --git a/src/fastcs/transports/tango/dsr.py b/src/fastcs/transports/tango/dsr.py index 7350be2a..93aaab1c 100644 --- a/src/fastcs/transports/tango/dsr.py +++ b/src/fastcs/transports/tango/dsr.py @@ -16,8 +16,12 @@ cast_to_tango_type, get_server_metadata_from_attribute, get_server_metadata_from_datatype, + tango_dev_class_name, + tango_dev_name, ) +FASTCS_TANGO_SERVER_NAME = "FastCS" + def _wrap_updater_fget( attr_name: str, @@ -60,8 +64,11 @@ def _collect_dev_attributes( root_controller_api: ControllerAPI, loop: asyncio.AbstractEventLoop ) -> dict[str, Any]: collection: dict[str, Any] = {} + root_depth = len(root_controller_api.path) for controller_api in root_controller_api.walk_api(): - path = controller_api.path + # Path relative to the device root: the controller id (path[0]) is the + # Tango device name, not part of attribute names. + path = controller_api.path[root_depth:] for attr_name, attribute in controller_api.attributes.items(): attr_name = attr_name.title().replace("_", "") @@ -124,8 +131,9 @@ def _collect_dev_commands( loop: asyncio.AbstractEventLoop, ) -> dict[str, Any]: collection: dict[str, Any] = {} + root_depth = len(root_controller_api.path) for controller_api in root_controller_api.walk_api(): - path = controller_api.path + path = controller_api.path[root_depth:] for name, method in controller_api.command_methods.items(): cmd_name = name.title().replace("_", "") @@ -168,30 +176,36 @@ def _collect_dsr_args(options: TangoDSROptions) -> list[str]: class TangoDSR: - """For controlling a controller with tango""" + """Hosts one Tango device per controller in a single Device Server. + + Each controller in ``controller_apis`` becomes its own Tango device class, + named after the controller's id, with ``{id}/{dev_class}/{dsr_instance}`` as + its three-segment Tango device name. + """ def __init__( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - self._controller_api = controller_api + self._controller_apis = controller_apis self._loop = loop - self.dev_class = self._controller_api.__class__.__name__ - self._device = self._create_device() + self._devices: list[type] = [ + self._create_device(api) for api in controller_apis + ] - def _create_device(self): + def _create_device(self, controller_api: ControllerAPI) -> type: class_dict: dict = { - **_collect_dev_attributes(self._controller_api, self._loop), - **_collect_dev_commands(self._controller_api, self._loop), - **_collect_dev_properties(self._controller_api), - **_collect_dev_init(self._controller_api), - **_collect_dev_flags(self._controller_api), + **_collect_dev_attributes(controller_api, self._loop), + **_collect_dev_commands(controller_api, self._loop), + **_collect_dev_properties(controller_api), + **_collect_dev_init(controller_api), + **_collect_dev_flags(controller_api), } class_bases = (server.Device,) - pytango_class = type(self.dev_class, class_bases, class_dict) - return pytango_class + dev_class = tango_dev_class_name(controller_api.path[0]) + return type(dev_class, class_bases, class_dict) def run(self, options: TangoDSROptions | None = None) -> None: if options is None: @@ -200,18 +214,28 @@ def run(self, options: TangoDSROptions | None = None) -> None: dsr_args = _collect_dsr_args(options) server.run( - (self._device,), - [self.dev_class, options.dsr_instance, *dsr_args], + tuple(self._devices), + [FASTCS_TANGO_SERVER_NAME, options.dsr_instance, *dsr_args], green_mode=server.GreenMode.Asyncio, ) -def register_dev(dev_name: str, dev_class: str, dsr_instance: str) -> None: - """Register a device instance in the tango server.""" +def register_dev( + dev_name: str, + dev_class: str, + dsr_instance: str, + server_name: str | None = None, +) -> None: + """Register a device instance in the tango server. + + ``server_name`` defaults to ``dev_class`` for backward compatibility with + callers from before multi-controller support. For FastCS-hosted multi-class + DSRs, pass ``server_name=FASTCS_TANGO_SERVER_NAME``. + """ dev_info = DbDevInfo() dev_info.name = dev_name dev_info._class = dev_class # noqa: SLF001 - dev_info.server = f"{dev_class}/{dsr_instance}" + dev_info.server = f"{server_name or dev_class}/{dsr_instance}" db = Database() db.delete_device(dev_name) # Remove existing device entry @@ -224,3 +248,17 @@ def register_dev(dev_name: str, dev_class: str, dsr_instance: str) -> None: print(f" - Device: {read_dev_info.name}") print(f" - Class: {read_dev_info.class_name}") print(f" - Device server: {read_dev_info.ds_full_name}") + + +def register_controller_devs( + controller_apis: list[ControllerAPI], options: TangoDSROptions +) -> None: + """Register every controller's Tango device under the FastCS server name.""" + for api in controller_apis: + id = api.path[0] + register_dev( + dev_name=tango_dev_name(id, options.dsr_instance), + dev_class=tango_dev_class_name(id), + dsr_instance=options.dsr_instance, + server_name=FASTCS_TANGO_SERVER_NAME, + ) diff --git a/src/fastcs/transports/tango/options.py b/src/fastcs/transports/tango/options.py index 26f65410..463e8c4d 100644 --- a/src/fastcs/transports/tango/options.py +++ b/src/fastcs/transports/tango/options.py @@ -3,6 +3,5 @@ @dataclass class TangoDSROptions: - dev_name: str = "MY/DEVICE/NAME" dsr_instance: str = "MY_SERVER_INSTANCE" debug: bool = False diff --git a/src/fastcs/transports/tango/transport.py b/src/fastcs/transports/tango/transport.py index 80e0f1e5..4ad9186c 100644 --- a/src/fastcs/transports/tango/transport.py +++ b/src/fastcs/transports/tango/transport.py @@ -2,9 +2,10 @@ from dataclasses import dataclass, field from fastcs.controllers import ControllerAPI -from fastcs.transports.transport import Transport, _expect_single +from fastcs.transports.transport import Transport from .dsr import TangoDSR, TangoDSROptions +from .util import validate_tango_id @dataclass @@ -18,8 +19,9 @@ def connect( controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - controller_api = _expect_single(controller_apis, "TangoTransport") - self._dsr = TangoDSR(controller_api, loop) + for api in controller_apis: + validate_tango_id(api.path[0]) + self._dsr = TangoDSR(controller_apis, loop) async def serve(self) -> None: coro = asyncio.to_thread(self._dsr.run, self.tango) diff --git a/src/fastcs/transports/tango/util.py b/src/fastcs/transports/tango/util.py index 61c287e8..9a82f264 100644 --- a/src/fastcs/transports/tango/util.py +++ b/src/fastcs/transports/tango/util.py @@ -1,3 +1,4 @@ +import re from dataclasses import asdict from typing import Any @@ -18,6 +19,42 @@ TANGO_ALLOWED_DATATYPES = (Bool, DataType, Enum, Float, Int, String, Waveform) +_TANGO_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + +def validate_tango_id(id: str) -> None: + """Reject controller ids that wouldn't be safe in a Tango device-name segment.""" + if not id: + raise ValueError("Controller id is empty; ids must be non-empty") + if not _TANGO_ID_RE.fullmatch(id): + raise ValueError( + f"Controller id {id!r} is not a valid Tango id; " + "only alphanumerics, '-' and '_' are allowed" + ) + + +def tango_dev_class_name(id: str) -> str: + """Map a controller id to a valid Python class name for a Tango device class. + + Hyphens are replaced with underscores; a leading digit is prefixed with ``X``. + Assumes ``id`` has already been accepted by ``validate_tango_id``. + """ + sanitized = id.replace("-", "_") + if sanitized[0].isdigit(): + sanitized = "X" + sanitized + return sanitized + + +def tango_dev_name(id: str, dsr_instance: str) -> str: + """Build the three-segment Tango device name for a controller. + + The id forms the leading segment, followed by the per-id Tango device class + and the DSR instance name. Assumes ``id`` has been accepted by + ``validate_tango_id``. + """ + return f"{id}/{tango_dev_class_name(id)}/{dsr_instance}" + + DATATYPE_FIELD_TO_SERVER_FIELD = { "units": "unit", "min": "min_value", diff --git a/tests/benchmarking/controller.py b/tests/benchmarking/controller.py index 3056da8f..a83048d4 100644 --- a/tests/benchmarking/controller.py +++ b/tests/benchmarking/controller.py @@ -7,7 +7,6 @@ from fastcs.transports.epics.ca.transport import EpicsCATransport from fastcs.transports.rest.options import RestServerOptions from fastcs.transports.rest.transport import RestTransport -from fastcs.transports.tango.options import TangoDSROptions from fastcs.transports.tango.transport import TangoTransport @@ -20,7 +19,7 @@ def run(): transport_options = [ RestTransport(rest=RestServerOptions(port=8090)), EpicsCATransport(), - TangoTransport(tango=TangoDSROptions(dev_name="MY/BENCHMARK/DEVICE")), + TangoTransport(), ] controller = MyTestController() controller.set_id("BENCHMARK-DEVICE") diff --git a/tests/conftest.py b/tests/conftest.py index a2be77aa..04d9cfcf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,7 +22,8 @@ from fastcs.datatypes import Bool, Float, Int, String from fastcs.logging import configure_logging, logger from fastcs.logging._logging import LogLevel -from fastcs.transports.tango.dsr import register_dev +from fastcs.transports.tango.dsr import FASTCS_TANGO_SERVER_NAME, register_dev +from fastcs.transports.tango.util import tango_dev_class_name, tango_dev_name from tests.assertable_controller import MyTestAttributeIORef, MyTestController from tests.example_p4p_ioc import run as _run_p4p_ioc from tests.example_softioc import run as _run_softioc @@ -199,12 +200,16 @@ def register_device(): if not os.getenv("TANGO_HOST"): raise RuntimeError("TANGO_HOST not defined") + benchmark_id = "BENCHMARK-DEVICE" + dsr_instance = "MY_SERVER_INSTANCE" + for attempt in range(1, attempts + 1): try: register_dev( - dev_name="MY/BENCHMARK/DEVICE", - dev_class="TestController", - dsr_instance="MY_SERVER_INSTANCE", + dev_name=tango_dev_name(benchmark_id, dsr_instance), + dev_class=tango_dev_class_name(benchmark_id), + dsr_instance=dsr_instance, + server_name=FASTCS_TANGO_SERVER_NAME, ) break except Exception: diff --git a/tests/data/schema.json b/tests/data/schema.json index fae8d15a..9dea040a 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -225,11 +225,6 @@ }, "TangoDSROptions": { "properties": { - "dev_name": { - "default": "MY/DEVICE/NAME", - "title": "Dev Name", - "type": "string" - }, "dsr_instance": { "default": "MY_SERVER_INSTANCE", "title": "Dsr Instance", diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 3ef5a206..21341581 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -255,6 +255,54 @@ def test_graphql_transport_rejects_illegal_id_at_connect(): loop.close() +def test_tango_transport_builds_one_device_per_controller_with_id_in_name(): + """One Tango DSR hosts N devices; each id forms the leading segment of its + three-segment device name and a unique device class is built per controller.""" + from fastcs.transports.tango.dsr import FASTCS_TANGO_SERVER_NAME + from fastcs.transports.tango.transport import TangoTransport + from fastcs.transports.tango.util import tango_dev_class_name, tango_dev_name + + api1 = _api_with_id(_OneAttrController, "ALPHA") + api2 = _api_with_id(_OtherAttrController, "BETA") + + loop = asyncio.new_event_loop() + try: + transport = TangoTransport() + transport.connect([api1, api2], loop) + finally: + loop.close() + + # Two distinct Tango device classes, one per controller, named after the id. + devices = transport._dsr._devices + assert len(devices) == 2 + assert [d.__name__ for d in devices] == ["ALPHA", "BETA"] + + # Device names embed the id as the leading segment. + instance = transport.tango.dsr_instance + assert tango_dev_name("ALPHA", instance) == f"ALPHA/ALPHA/{instance}" + assert tango_dev_name("BETA", instance) == f"BETA/BETA/{instance}" + + # FastCS-hosted DSRs use a fixed server name independent of controller class + # so a multi-class server has a single identity. + assert FASTCS_TANGO_SERVER_NAME == "FastCS" + assert tango_dev_class_name("ALPHA") == "ALPHA" + assert tango_dev_class_name("BETA") == "BETA" + + +def test_tango_transport_rejects_illegal_id_at_connect(): + api = _api_with_id(_OneAttrController, "bad/id") + + loop = asyncio.new_event_loop() + try: + from fastcs.transports.tango.transport import TangoTransport + + transport = TangoTransport() + with pytest.raises(ValueError, match="bad/id"): + transport.connect([api], loop) + finally: + loop.close() + + class _LifecycleController(Controller): """Records lifecycle hook calls for end-to-end assertions.""" diff --git a/tests/transports/tango/test_dsr.py b/tests/transports/tango/test_dsr.py index 5d44ff7a..61a8f73e 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) + return AssertableControllerAPI(TangoController(), class_mocker, path=["DEVICE"]) def create_test_context(tango_controller_api: AssertableControllerAPI): @@ -52,7 +52,7 @@ def create_test_context(tango_controller_api: AssertableControllerAPI): [tango_controller_api], asyncio.get_event_loop(), ) - device = tango_transport._dsr._device + device = tango_transport._dsr._devices[0] # https://tango-controls.readthedocs.io/projects/pytango/en/v9.5.1/testing/test_context.html with DeviceTestContext(device, debug=0) as proxy: yield proxy diff --git a/tests/transports/tango/test_tango_util.py b/tests/transports/tango/test_tango_util.py new file mode 100644 index 00000000..96fe9a6a --- /dev/null +++ b/tests/transports/tango/test_tango_util.py @@ -0,0 +1,50 @@ +import pytest + +from fastcs.transports.tango.util import ( + tango_dev_class_name, + tango_dev_name, + validate_tango_id, +) + + +class TestValidateTangoId: + @pytest.mark.parametrize( + "id", + ["DEVICE", "DEV-1", "dev_1", "ALPHA", "BENCHMARK-DEVICE", "0LEAD"], + ) + def test_accepts_valid_ids(self, id: str): + validate_tango_id(id) + + def test_rejects_empty(self): + with pytest.raises(ValueError, match="empty"): + validate_tango_id("") + + @pytest.mark.parametrize( + "id", + ["bad/id", "bad id", "bad.id", "bad:id", "bad!id"], + ) + def test_rejects_illegal_chars(self, id: str): + with pytest.raises(ValueError, match=id): + validate_tango_id(id) + + +class TestTangoDevClassName: + def test_passes_through_valid_python_identifiers(self): + assert tango_dev_class_name("DEVICE") == "DEVICE" + assert tango_dev_class_name("dev_1") == "dev_1" + + def test_replaces_hyphens_with_underscores(self): + assert tango_dev_class_name("BENCHMARK-DEVICE") == "BENCHMARK_DEVICE" + + def test_prefixes_leading_digit_with_x(self): + assert tango_dev_class_name("0LEAD") == "X0LEAD" + assert tango_dev_class_name("1-2") == "X1_2" + + +class TestTangoDevName: + def test_three_segments_with_id_leading(self): + assert tango_dev_name("DEVICE", "INST") == "DEVICE/DEVICE/INST" + assert ( + tango_dev_name("BENCHMARK-DEVICE", "MY_INST") + == "BENCHMARK-DEVICE/BENCHMARK_DEVICE/MY_INST" + )