diff --git a/.vscode/launch.json b/.vscode/launch.json index 88030120e..22bceb95d 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/conf.py b/docs/conf.py index 88ced3f26..3ee2ad966 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -98,7 +98,6 @@ ("py:class", "fastcs.logging._graylog.GraylogStaticFields"), ("py:class", "fastcs.logging._graylog.GraylogEnvFields"), ("py:obj", "fastcs.control_system.build_controller_api"), - ("py:obj", "fastcs.transports.epics.util.controller_pv_prefix"), ("docutils", "fastcs.demo.controllers.TemperatureControllerSettings"), # TypeVar without docstrings still give warnings ("py:class", "strawberry.schema.schema.Schema"), diff --git a/docs/how-to/launch-framework.md b/docs/how-to/launch-framework.md index e3f210fb8..bbc0951fd 100644 --- a/docs/how-to/launch-framework.md +++ b/docs/how-to/launch-framework.md @@ -54,26 +54,75 @@ 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 -controller: - ip_address: "192.168.1.100" - port: 25565 - timeout: 10.0 +# fastcs.yaml +controllers: + DEVICE: + type: DeviceController + ip_address: "192.168.1.100" + port: 25565 + timeout: 10.0 transport: - - epicsca: - pv_prefix: "DEVICE" + - epicsca: {} ``` +Every entry carries a required `type:` discriminator that names the +Controller class to instantiate. The remaining fields under each id +come straight from that class's `__init__` options type +(`DeviceSettings` here). + +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 -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); the `type:` +discriminator selects which class to instantiate. For example, two +`DeviceController` instances on different IPs sharing a single transport +list: + +```yaml +# fastcs.yaml +controllers: + MAIN: + type: DeviceController + ip_address: "192.168.1.100" + port: 25565 + timeout: 10.0 + AUX: + type: DeviceController + ip_address: "192.168.1.101" + port: 25565 + timeout: 10.0 + +transport: + - epicsca: {} ``` +When more than one class is registered with `launch([ClassA, ClassB])`, +each entry's `type:` selects between them. + +For a real working example, see `src/fastcs/demo/fastcs.yaml`, which +hosts two `TemperatureController` instances and can be run with +`python -m fastcs.demo run src/fastcs/demo/fastcs.yaml`. + +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: @@ -86,9 +135,11 @@ Use this schema for IDE autocompletion in YAML files: ```yaml # yaml-language-server: $schema=schema.json -controller: - ip_address: "192.168.1.100" - # ... IDE will provide autocompletion +controllers: + DEVICE: + type: DeviceController + ip_address: "192.168.1.100" + # ... IDE will provide autocompletion ``` ## Transport Configuration @@ -98,10 +149,9 @@ 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" + output_dir: "opis" title: "Device Control" # REST API @@ -121,10 +171,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 000000000..ca97f3950 --- /dev/null +++ b/docs/how-to/migrate-to-multi-controller.md @@ -0,0 +1,118 @@ +# 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 + type: DeviceController # required discriminator + 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 is required on every entry + +Each entry under `controllers:` carries a required `type:` discriminator +that names the Controller class to instantiate. The discriminator value +is the class `__name__`, or `type_name: ClassVar[str]` on the class if +set. The same rule applies whether `launch()` is called with a single +class or with several — `type:` is never optional. + +```yaml +# Two-class app: launch([Lakeshore, Eurotherm]) +controllers: + CRYO: + type: Lakeshore + ip_address: "192.168.1.100" + OVEN: + type: Eurotherm + 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/docs/how-to/multiple-transports.md b/docs/how-to/multiple-transports.md index 626340096..14dc4aef3 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(), ] @@ -30,6 +30,26 @@ 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_-]+`, 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 +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 | @@ -52,23 +72,24 @@ 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", + output_dir=Path("./opis"), title="Device Control", ), docs=EpicsDocsOptions( - output_path=Path(".") / "device.csv", + output_dir=Path("./reference"), ), ) ``` @@ -76,11 +97,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 @@ -119,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: @@ -134,25 +157,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"), + gui=EpicsGUIOptions(output_dir=Path("./opis")), ), + 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 diff --git a/docs/snippets/dynamic.py b/docs/snippets/dynamic.py index 65d584978..46cee1171 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,9 +138,11 @@ 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]) +controller = TemperatureController(connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static04.py b/docs/snippets/static04.py index 6200b78f3..e590b35ff 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,8 +9,10 @@ class TemperatureController(Controller): device_id = AttrR(String()) -epics_ca = EpicsCATransport(epicsca=EpicsIOCOptions(pv_prefix="DEMO")) -fastcs = FastCS(TemperatureController(), [epics_ca]) +epics_ca = EpicsCATransport() +controller = TemperatureController() +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static05.py b/docs/snippets/static05.py index 1a3951974..630f0a84f 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 @@ -12,11 +12,11 @@ class TemperatureController(Controller): device_id = AttrR(String()) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) -fastcs = FastCS(TemperatureController(), [epics_ca]) +gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") +epics_ca = EpicsCATransport(gui=gui_options) +controller = TemperatureController() +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static06.py b/docs/snippets/static06.py index 4cd77fdd7..7b7af6d41 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 @@ -22,12 +22,12 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": diff --git a/docs/snippets/static07.py b/docs/snippets/static07.py index 12b7333bb..068109e28 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) @@ -44,12 +44,12 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static08.py b/docs/snippets/static08.py index beb0f49b9..45806bb03 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) @@ -50,12 +50,12 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static09.py b/docs/snippets/static09.py index dbe6a5ee8..c380b715d 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) @@ -57,12 +57,12 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static10.py b/docs/snippets/static10.py index 5da5add73..5cf7eb5ab 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) @@ -75,12 +75,12 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(4, connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static11.py b/docs/snippets/static11.py index beba164ed..4032837e7 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) @@ -82,12 +82,12 @@ async def connect(self): await self._connection.connect(self._ip_settings) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(4, connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static12.py b/docs/snippets/static12.py index 2b6681663..4de327b4a 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) @@ -95,12 +95,12 @@ async def update_voltages(self): await controller.voltage.update(float(voltages[index])) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(4, connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static13.py b/docs/snippets/static13.py index b454288e9..e2812e338 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) @@ -103,12 +103,12 @@ async def disable_all(self) -> None: await asyncio.sleep(0.1) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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]) +controller = TemperatureController(4, connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static14.py b/docs/snippets/static14.py index fb794b199..e60bb276e 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) @@ -109,13 +109,13 @@ async def disable_all(self) -> None: configure_logging() -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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) -fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) +controller = TemperatureController(4, connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static15.py b/docs/snippets/static15.py index 0246d2cb0..1c5bf2587 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) @@ -112,13 +112,13 @@ async def disable_all(self) -> None: configure_logging(LogLevel.TRACE) -gui_options = EpicsGUIOptions( - output_path=Path(".") / "demo.bob", title="Demo Temperature Controller" -) -epics_ca = EpicsCATransport(gui=gui_options, epicsca=EpicsIOCOptions(pv_prefix="DEMO")) +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) -fastcs = FastCS(TemperatureController(4, connection_settings), [epics_ca]) +controller = TemperatureController(4, connection_settings) +controller.set_id("DEMO") +fastcs = FastCS(controller, [epics_ca]) if __name__ == "__main__": fastcs.run() diff --git a/docs/tutorials/static-drivers.md b/docs/tutorials/static-drivers.md index 31c345922..f28f9b603 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 ::: :::: diff --git a/src/fastcs/control_system.py b/src/fastcs/control_system.py index 39b7fd2e5..cc5e5c829 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_api=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/controllers/controller.py b/src/fastcs/controllers/controller.py index fe1b4258c..f5183ccde 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/src/fastcs/demo/controller.yaml b/src/fastcs/demo/controller.yaml deleted file mode 100644 index 0df1fa41d..000000000 --- a/src/fastcs/demo/controller.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# yaml-language-server: $schema=schema.json -controller: - ip_settings: - ip: "localhost" - port: 25565 - num_ramp_controllers: 4 -transport: - - graphql: - host: localhost - port: 8083 - log_level: info - - epicsca: - pv_prefix: GARYDEMO - gui: - title: Temperature Controller Demo - output_path: ./demo.bob diff --git a/src/fastcs/demo/fastcs.yaml b/src/fastcs/demo/fastcs.yaml new file mode 100644 index 000000000..474be4651 --- /dev/null +++ b/src/fastcs/demo/fastcs.yaml @@ -0,0 +1,23 @@ +# yaml-language-server: $schema=schema.json +controllers: + MAIN: + type: TemperatureController + ip_settings: + ip: "localhost" + port: 25565 + num_ramp_controllers: 4 + AUX: + type: TemperatureController + ip_settings: + ip: "localhost" + port: 25566 + num_ramp_controllers: 2 +transport: + - graphql: + host: localhost + port: 8083 + log_level: info + - epicsca: {} + gui: + title: Temperature Controller Demo + output_dir: . diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index a147c9210..7187a07b6 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -1,26 +1,52 @@ { "$defs": { "EpicsCAOptions": { + "properties": {}, + "title": "EpicsCAOptions", + "type": "object" + }, + "EpicsCATransport": { "properties": { + "epicsca": { + "$ref": "#/$defs/EpicsCAOptions" + }, "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": { "properties": { - "path": { + "output_dir": { "default": ".", "format": "path", - "title": "Path", + "title": "Output Dir", + "type": "string" + }, + "title": { + "default": "FastCS Devices", + "title": "Title", "type": "string" }, "depth": { @@ -50,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": { @@ -61,7 +87,7 @@ "default": ".bob" }, "title": { - "default": "Simple Device", + "default": "FastCS Devices", "title": "Title", "type": "string" } @@ -69,39 +95,40 @@ "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" }, - "EpicsPVAOptions": { + "EpicsPVATransport": { "properties": { + "epicspva": { + "$ref": "#/$defs/EpicsPVAOptions" + }, "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 +152,15 @@ "title": "GraphQLServerOptions", "type": "object" }, + "GraphQLTransport": { + "properties": { + "graphql": { + "$ref": "#/$defs/GraphQLServerOptions" + } + }, + "title": "GraphQLTransport", + "type": "object" + }, "IPConnectionSettings": { "properties": { "ip": { @@ -141,15 +177,6 @@ "title": "IPConnectionSettings", "type": "object" }, - "RestOptions": { - "properties": { - "rest": { - "$ref": "#/$defs/RestServerOptions" - } - }, - "title": "RestOptions", - "type": "object" - }, "RestServerOptions": { "properties": { "host": { @@ -171,13 +198,17 @@ "title": "RestServerOptions", "type": "object" }, + "RestTransport": { + "properties": { + "rest": { + "$ref": "#/$defs/RestServerOptions" + } + }, + "title": "RestTransport", + "type": "object" + }, "TangoDSROptions": { "properties": { - "dev_name": { - "default": "MY/DEVICE/NAME", - "title": "Dev Name", - "type": "string" - }, "dsr_instance": { "default": "MY_SERVER_INSTANCE", "title": "Dsr Instance", @@ -192,16 +223,35 @@ "title": "TangoDSROptions", "type": "object" }, - "TangoOptions": { + "TangoTransport": { "properties": { "tango": { "$ref": "#/$defs/TangoDSROptions" } }, - "title": "TangoOptions", + "title": "TangoTransport", "type": "object" }, - "TempControllerSettings": { + "TemperatureControllerEntry": { + "additionalProperties": false, + "properties": { + "type": { + "const": "TemperatureController", + "default": "TemperatureController", + "title": "Type", + "type": "string" + }, + "controller": { + "$ref": "#/$defs/TemperatureControllerSettings" + } + }, + "required": [ + "controller" + ], + "title": "TemperatureControllerEntry", + "type": "object" + }, + "TemperatureControllerSettings": { "properties": { "num_ramp_controllers": { "title": "Num Ramp Controllers", @@ -215,32 +265,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 +303,9 @@ } }, "required": [ - "controller", + "controllers", "transport" ], - "title": "TempController", + "title": "TemperatureController", "type": "object" } diff --git a/src/fastcs/demo/simulation/temp_controller.yaml b/src/fastcs/demo/simulation/temp_controller.yaml index 682ce636b..497485a6e 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 diff --git a/src/fastcs/launch.py b/src/fastcs/launch.py index 99874cb9d..7b625f0ba 100644 --- a/src/fastcs/launch.py +++ b/src/fastcs/launch.py @@ -1,11 +1,12 @@ import asyncio +import dataclasses 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__ @@ -24,54 +25,86 @@ from fastcs.transports import Transport +@dataclasses.dataclass(frozen=True) +class _RegisteredClass: + cls: type[Controller] + expects_options: bool + options_type: Any + + +_ENTRY_REGISTRY: dict[type[BaseModel], _RegisteredClass] = {} +"""Maps each dynamically-built Entry model class to its originating +Controller class and whether it expects an options arg (with the +options-type, if any). Populated by ``_build_entry_model`` and read by +``_instantiate_controllers``.""" + + 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 required ``type`` discriminator + in the config. + 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) + 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, fastcs_options): self.fastcs_options = fastcs_options 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 +116,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(fastcs_options) - @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,14 +156,11 @@ 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 yaml = YAML(typ="safe") @@ -153,13 +178,12 @@ 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) instance = FastCS( - controller, instance_options.transport, loop=asyncio.get_event_loop() + controllers, + instance_options.transport, + loop=asyncio.get_event_loop(), ) instance.run() @@ -167,42 +191,153 @@ def run( return launch_typer -def _extract_options_model(controller_class: type[Controller]) -> type[BaseModel]: +def _instantiate_controllers( + controllers_options: dict[str, Any], +) -> list[Controller]: + """Instantiate each entry under `controllers:` and stamp its id. + + Each value in ``controllers_options`` is a dynamically-built Pydantic + model that exposes ``type`` plus the controller's options fields + inlined as siblings. The originating Controller class and its + options-type are looked up in ``_ENTRY_REGISTRY`` (populated by + ``_build_entry_model``). + """ + controllers: list[Controller] = [] + for id, entry in controllers_options.items(): + entry_cls: type[BaseModel] = type(entry) + registered = _ENTRY_REGISTRY[entry_cls] + if registered.expects_options: + field_values = { + name: getattr(entry, name) + for name in entry_cls.model_fields + if name != "type" + } + controller = registered.cls(registered.options_type(**field_values)) + else: + controller = registered.cls() + controller.set_id(id) + controllers.append(controller) + return controllers + + +def _options_field_definitions(options_type: type) -> dict[str, tuple[Any, Any]]: + """Field-by-field definitions for inlining ``options_type`` into an Entry. + + Returns a mapping suitable for splatting into ``create_model``. Supports + pydantic ``BaseModel`` and (stdlib or pydantic) dataclasses; raises + ``LaunchError`` for anything else. + """ + if isinstance(options_type, type) and issubclass(options_type, BaseModel): + return { + name: (field.annotation, field) + for name, field in options_type.model_fields.items() + } + if dataclasses.is_dataclass(options_type): + hints = get_type_hints(options_type) + result: dict[str, tuple[Any, Any]] = {} + for f in dataclasses.fields(options_type): + annotation = hints.get(f.name, f.type) + if f.default is not dataclasses.MISSING: + default: Any = f.default + elif f.default_factory is not dataclasses.MISSING: + default = Field(default_factory=f.default_factory) + else: + default = ... + result[f.name] = (annotation, default) + return result + raise LaunchError( + f"Cannot inline options type {options_type!r}: expected a dataclass " + f"or pydantic BaseModel." + ) + + +def _build_entry_model(controller_class: type[Controller]) -> type[BaseModel]: + """Build a Pydantic model for one entry under `controllers:`. + + Each entry exposes a ``type`` discriminator literal alongside the + options-type's fields, inlined as siblings (no nested ``controller:`` + block). The Controller class and its options-type are recorded in + ``_ENTRY_REGISTRY`` for use by ``_instantiate_controllers``. + """ sig = inspect.signature(controller_class.__init__) args = inspect.getfullargspec(controller_class.__init__)[0] + discriminator = _discriminator(controller_class) + + fields: dict[str, Any] = {"type": (Literal[discriminator], ...)} + expects_options = False + options_type: Any = None + if len(args) == 1: - fastcs_options = create_model( - f"{controller_class.__name__}", - transport=(list[Transport.union()], ...), - __config__={"extra": "forbid"}, - ) + pass elif len(args) == 2: + expects_options = True hints = get_type_hints(controller_class.__init__) - if "return" in hints: - del hints["return"] - if hints: - options_type = list(hints.values())[-1] - else: + hints.pop("return", None) + if not hints: raise LaunchError( 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"}, - ) + options_type = list(hints.values())[-1] + options_fields = _options_field_definitions(options_type) + if "type" in options_fields: + raise LaunchError( + f"Options type {options_type.__name__} for " + f"{controller_class.__name__} declares a 'type' field, which " + f"collides with the launch-framework discriminator key." + ) + fields.update(options_fields) 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 + + entry_model = create_model( + f"{controller_class.__name__}Entry", + __config__={"extra": "forbid"}, + **fields, + ) + _ENTRY_REGISTRY[entry_model] = _RegisteredClass( + cls=controller_class, + expects_options=expects_options, + options_type=options_type, + ) + return entry_model + + +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 or a discriminated union over all + registered classes; in both cases the entry's ``type:`` field is + required and names the controller class. + """ + 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/src/fastcs/transports/__init__.py b/src/fastcs/transports/__init__.py index c5bc22b5d..835ada3ed 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 0dad2e91e..0165c497e 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 EpicsDocsOptions as EpicsDocsOptions 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 3f510a1b7..7ac5026dd 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -17,29 +17,24 @@ cast_from_epics_type, cast_to_epics_type, ) -from fastcs.transports.epics.util import controller_pv_prefix +from fastcs.transports.epics.util import EPICS_MAX_NAME_LENGTH, 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 +90,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 +104,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 +197,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 cc51b5d66..be2ff2833 100644 --- a/src/fastcs/transports/epics/ca/transport.py +++ b/src/fastcs/transports/epics/ca/transport.py @@ -7,13 +7,15 @@ 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.docs import EpicsDocs +from fastcs.transports.epics.ca.util import validate_ca_id +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 @@ -21,8 +23,8 @@ 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 @@ -30,23 +32,25 @@ class EpicsCATransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ) -> None: - 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) + emit_docs_files(controller_apis, self.docs) if self.gui is not None: - EpicsGUI(self._controller_api, self._pv_prefix).create_gui(self.gui) + emit_gui_files(controller_apis, self.gui, EpicsGUI) 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 @@ -59,4 +63,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 b5892a470..6a3e6dd83 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,23 @@ 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 validate_epics_pv_id + +_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. + """ + validate_epics_pv_id(controller_api, transport_label="EPICS CA id", id_re=_CA_ID_RE) + _MBB_FIELD_PREFIXES = ( "ZR", diff --git a/src/fastcs/transports/epics/docs.py b/src/fastcs/transports/epics/docs.py deleted file mode 100644 index ced56172d..000000000 --- a/src/fastcs/transports/epics/docs.py +++ /dev/null @@ -1,14 +0,0 @@ -from fastcs.controllers import ControllerAPI - -from .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() diff --git a/src/fastcs/transports/epics/emission.py b/src/fastcs/transports/epics/emission.py new file mode 100644 index 000000000..2d6a576e7 --- /dev/null +++ b/src/fastcs/transports/epics/emission.py @@ -0,0 +1,174 @@ +"""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 ( # type: ignore + NON_PASCAL_CHARS_RE, + Device, + DeviceRef, + enforce_pascal_case, +) + +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``. + + Some otherwise-valid transport ids (the EPICS CA validator accepts + ``[A-Za-z0-9_-]+`` so ``"___"`` and ``"-"`` are legal) contain no + Pascal-usable characters at all. ``enforce_pascal_case`` strips + everything and then unconditionally indexes ``s[0]``, raising + ``IndexError`` on the empty string. We fail fast with a clearer + ``ValueError`` instead -- a silent fallback would produce nonsense + GUI names that the user can't trace back to the offending id. + """ + stripped = NON_PASCAL_CHARS_RE.sub("", controller_id) + if not stripped: + raise ValueError( + f"Controller id {controller_id!r} contains no characters usable " + "in a Pascal-case name; pick an id with at least one ASCII " + "letter or digit" + ) + candidate = enforce_pascal_case(controller_id) + if 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, + 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 fbdf336d3..882a83a02 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,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 +43,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: @@ -147,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 029d634c6..55946400d 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,13 +23,24 @@ 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 -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 0c7ea4b7c..5b2e29611 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, @@ -50,15 +48,21 @@ async def parse_attributes( class P4PIOC: - """A P4P IOC which handles a controller""" + """A P4P IOC which handles one or more controllers. - def __init__(self, pv_prefix: str, controller_api: ControllerAPI): - self.pv_prefix = pv_prefix - 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.pv_prefix, 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 5aff45916..d84ff98b7 100644 --- a/src/fastcs/transports/epics/pva/transport.py +++ b/src/fastcs/transports/epics/pva/transport.py @@ -6,10 +6,12 @@ from fastcs.transports.epics import ( EpicsDocsOptions, EpicsGUIOptions, - EpicsIOCOptions, + 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 from fastcs.transports.transport import Transport from .ioc import P4PIOC @@ -19,29 +21,32 @@ 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 def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ) -> None: - self._controller_api = controller_api - self._pv_prefix = self.epicspva.pv_prefix - self._ioc = P4PIOC(self.epicspva.pv_prefix, 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) + emit_docs_files(controller_apis, self.docs) if self.gui is not None: - PvaEpicsGUI(self._controller_api, self._pv_prefix).create_gui(self.gui) + emit_gui_files(controller_apis, self.gui, PvaEpicsGUI) 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 000000000..624e378fe --- /dev/null +++ b/src/fastcs/transports/epics/pva/util.py @@ -0,0 +1,18 @@ +import re + +from fastcs.controllers import ControllerAPI +from fastcs.transports.epics.util import validate_epics_pv_id + +_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. + """ + validate_epics_pv_id( + controller_api, transport_label="EPICS PVA id", id_re=_PVA_ID_RE + ) diff --git a/src/fastcs/transports/epics/util.py b/src/fastcs/transports/epics/util.py index 4695939e2..24b7d3da1 100644 --- a/src/fastcs/transports/epics/util.py +++ b/src/fastcs/transports/epics/util.py @@ -1,6 +1,49 @@ +import re + from fastcs.controllers import ControllerAPI 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. + + 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 validate_epics_pv_id( + controller_api: ControllerAPI, + *, + transport_label: str, + id_re: re.Pattern[str], +) -> None: + """Reject controller ids that wouldn't be safe in an EPICS PV name. -def controller_pv_prefix(prefix: str, controller_api: ControllerAPI) -> str: - return ":".join([prefix] + [snake_to_pascal(node) for node in controller_api.path]) + Rejects ids with characters not matched by ``id_re`` and rejects setups + where the longest derivable PV prefix already exceeds the + ``EPICS_MAX_NAME_LENGTH`` PV name limit. ``transport_label`` is the + transport-specific tag (e.g. ``"EPICS CA id"``) that appears in the + illegal-characters error message. + """ + id = controller_api.path[0] + if not id_re.fullmatch(id): + raise ValueError( + f"Controller id {id!r} is not a valid {transport_label}; " + "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/graphql/graphql.py b/src/fastcs/transports/graphql/graphql.py index 8bc2fa322..f52593dca 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 a590c062f..23eaa8088 100644 --- a/src/fastcs/transports/graphql/transport.py +++ b/src/fastcs/transports/graphql/transport.py @@ -6,6 +6,7 @@ from .graphql import GraphQLServer from .options import GraphQLServerOptions +from .util import validate_graphql_id @dataclass @@ -16,10 +17,12 @@ class GraphQLTransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - self._server = GraphQLServer(controller_api) + for api in controller_apis: + 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 000000000..7244adedd --- /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/src/fastcs/transports/rest/rest.py b/src/fastcs/transports/rest/rest.py index e1005ac0c..189d67c29 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 d89a89b9e..250e13358 100644 --- a/src/fastcs/transports/rest/transport.py +++ b/src/fastcs/transports/rest/transport.py @@ -6,6 +6,7 @@ from .options import RestServerOptions from .rest import RestServer +from .util import validate_rest_id @dataclass @@ -16,10 +17,13 @@ class RestTransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - 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/src/fastcs/transports/rest/util.py b/src/fastcs/transports/rest/util.py index 550d72941..c869a0d9f 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/src/fastcs/transports/tango/dsr.py b/src/fastcs/transports/tango/dsr.py index 7350be2af..93aaab1c8 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 26f654102..463e8c4d3 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 6ce3b102b..f2861822d 100644 --- a/src/fastcs/transports/tango/transport.py +++ b/src/fastcs/transports/tango/transport.py @@ -5,6 +5,7 @@ from fastcs.transports.transport import Transport from .dsr import TangoDSR, TangoDSROptions +from .util import tango_dev_class_name, validate_tango_id @dataclass @@ -15,10 +16,23 @@ class TangoTransport(Transport): def connect( self, - controller_api: ControllerAPI, + controller_apis: list[ControllerAPI], loop: asyncio.AbstractEventLoop, ): - self._dsr = TangoDSR(controller_api, loop) + seen: dict[str, str] = {} + for api in controller_apis: + id = api.path[0] + validate_tango_id(id) + cls_name = tango_dev_class_name(id) + if cls_name in seen: + raise ValueError( + f"Controller ids {seen[cls_name]!r} and {id!r} both sanitise " + f"to Tango device-class name {cls_name!r}; pick one variant " + "(hyphens and underscores are not distinguishable in Tango " + "class names)" + ) + seen[cls_name] = id + 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 61c287e8f..9a82f264c 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/src/fastcs/transports/transport.py b/src/fastcs/transports/transport.py index 705880cec..056dedf2e 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/assertable_controller.py b/tests/assertable_controller.py index 1b1c6f608..c57916134 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 7ffc2238a..a83048d47 100644 --- a/tests/benchmarking/controller.py +++ b/tests/benchmarking/controller.py @@ -4,11 +4,9 @@ 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 -from fastcs.transports.tango.options import TangoDSROptions from fastcs.transports.tango.transport import TangoTransport @@ -20,12 +18,12 @@ class MyTestController(Controller): def run(): transport_options = [ RestTransport(rest=RestServerOptions(port=8090)), - EpicsCATransport( - epicsca=EpicsIOCOptions(pv_prefix="BENCHMARK-DEVICE"), - ), - TangoTransport(tango=TangoDSROptions(dev_name="MY/BENCHMARK/DEVICE")), + EpicsCATransport(), + TangoTransport(), ] - 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 5da18951b..04d9cfcfa 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 @@ -114,7 +115,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) @@ -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/config.yaml b/tests/data/config.yaml index 8c7d3ac3d..d31a0e79d 100644 --- a/tests/data/config.yaml +++ b/tests/data/config.yaml @@ -1,13 +1,11 @@ # yaml-language-server: $schema=schema.json +controllers: + device-1: + type: IsHinted + name: controller-name transport: - epicsca: {} - docs: {} - gui: {} - epicspva: {} - docs: {} - gui: {} - rest: {} - tango: {} - graphql: {} -controller: - name: controller-name diff --git a/tests/data/schema.json b/tests/data/schema.json index 8609adf33..69f4be8ef 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": [ @@ -33,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": { @@ -66,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": { @@ -77,7 +87,7 @@ "default": ".bob" }, "title": { - "default": "Simple Device", + "default": "FastCS Devices", "title": "Title", "type": "string" } @@ -85,21 +95,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": [ @@ -157,6 +161,26 @@ "title": "GraphQLTransport", "type": "object" }, + "IsHintedEntry": { + "additionalProperties": false, + "properties": { + "type": { + "const": "IsHinted", + "title": "Type", + "type": "string" + }, + "name": { + "title": "Name", + "type": "string" + } + }, + "required": [ + "type", + "name" + ], + "title": "IsHintedEntry", + "type": "object" + }, "RestServerOptions": { "properties": { "host": { @@ -187,26 +211,8 @@ "title": "RestTransport", "type": "object" }, - "SomeConfig": { - "properties": { - "name": { - "title": "Name", - "type": "string" - } - }, - "required": [ - "name" - ], - "title": "SomeConfig", - "type": "object" - }, "TangoDSROptions": { "properties": { - "dev_name": { - "default": "MY/DEVICE/NAME", - "title": "Dev Name", - "type": "string" - }, "dsr_instance": { "default": "MY_SERVER_INSTANCE", "title": "Dsr Instance", @@ -233,8 +239,12 @@ }, "additionalProperties": false, "properties": { - "controller": { - "$ref": "#/$defs/SomeConfig" + "controllers": { + "additionalProperties": { + "$ref": "#/$defs/IsHintedEntry" + }, + "title": "Controllers", + "type": "object" }, "transport": { "items": { @@ -261,7 +271,7 @@ } }, "required": [ - "controller", + "controllers", "transport" ], "title": "IsHinted", diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index b6dc843a7..8cba9bf3d 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 86eac91ed..896878325 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,20 +21,15 @@ 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( - output_path=Path(".") / "demo.bob", title="Demo Vector" - ) + gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Vector") 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_control_system.py b/tests/test_control_system.py index c231b136b..ca151cc02 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 a482f61a1..d03b1eb7e 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -1,19 +1,26 @@ 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 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 -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 +29,11 @@ class SomeConfig: name: str +@dataclass +class OtherConfig: + address: str + + class SingleArg(Controller): def __init__(self): super().__init__() @@ -44,14 +56,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"], ...), + ) 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 +94,17 @@ def test_single_arg_schema(): def test_is_hinted_schema(data): + entry_model = create_model( + "IsHintedEntry", + __config__={"extra": "forbid"}, + type=(Literal["IsHinted"], ...), + name=(str, ...), + ) 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 +115,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 = ( @@ -148,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", @@ -162,3 +194,121 @@ 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_requires_type(): + """`type:` is mandatory on every controllers entry, even when only one + Controller class is registered.""" + options_model = _build_options_model([IsHinted]) + with pytest.raises(ValidationError): + options_model.model_validate( + { + "controllers": {"my-id": {"name": "x"}}, + "transport": [{"rest": {}}], + } + ) + + instance = options_model.model_validate( + { + "controllers": {"my-id": {"type": "IsHinted", "name": "x"}}, + "transport": [{"rest": {}}], + } + ) + entry = _controllers(instance)["my-id"] + assert entry.type == "IsHinted" + assert entry.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", "name": "a"}, + "second": {"type": "OtherHinted", "address": "b"}, + }, + "transport": [{"rest": {}}], + } + ) + + first = _controllers(instance)["first"] + second = _controllers(instance)["second"] + assert first.type == "IsHinted" + assert first.name == "a" + assert second.type == "OtherHinted" + assert second.address == "b" + + +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", "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", "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" + " name: a\n" + " same:\n" + " 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_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( + "controllers:\n" + " one:\n" + " type: IsHinted\n" + " name: a\n" + " two:\n" + " type: OtherHinted\n" + " address: b\n" + "transport:\n" + " - rest: {}\n" + ) + app = _launch([IsHinted, OtherHinted]) + result = runner.invoke(app, ["run", str(cfg)]) + assert result.exit_code == 0, result.output + init_spy.assert_called_once() + controllers_arg = init_spy.call_args.args[1] + assert [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 new file mode 100644 index 000000000..018c79c5b --- /dev/null +++ b/tests/test_multi_controller.py @@ -0,0 +1,396 @@ +"""Tests for the multi-controller foundation slice (#353) and per-transport +multi-root extensions (#354+). +""" + +import asyncio +from pathlib import Path + +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.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 + + +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"): + _ = 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"] + + +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() + + +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_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") + + loop = asyncio.new_event_loop() + try: + transport = EpicsCATransport() + with pytest.raises(ValueError, match="bad/id"): + transport.connect([api], loop) + finally: + 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() + + +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() + + +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() + + +def test_tango_transport_rejects_post_sanitisation_class_name_collision(): + """#371: hyphens and underscores both sanitise to underscores in Tango device + class names, so ``DEV-1`` and ``DEV_1`` would silently override each other in + ``tango.server.run``. Detect and fail fast at connect.""" + api1 = _api_with_id(_OneAttrController, "DEV-1") + api2 = _api_with_id(_OtherAttrController, "DEV_1") + + loop = asyncio.new_event_loop() + try: + from fastcs.transports.tango.transport import TangoTransport + + transport = TangoTransport() + with pytest.raises(ValueError) as excinfo: + transport.connect([api1, api2], loop) + finally: + loop.close() + + message = str(excinfo.value) + # Both colliding ids appear in the message so callers can spot the pair. + assert "'DEV-1'" in message + assert "'DEV_1'" in message + + +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 diff --git a/tests/transports/epics/ca/test_ca_util.py b/tests/transports/epics/ca/test_ca_util.py index f484fca93..463a17dcd 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 72931b862..8f4ce4da2 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,17 +21,19 @@ 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(): - 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 +48,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 +62,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 +78,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 +177,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 +186,35 @@ 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"}) + + +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/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index 727cb0d7d..23c0976e1 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 d695abab5..2ad7a6015 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, @@ -31,6 +30,7 @@ _make_in_record, _make_out_record, ) +from fastcs.transports.epics.util import EPICS_MAX_NAME_LENGTH DEVICE = "DEVICE" @@ -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 3a1a06aca..4041f78c1 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(): @@ -300,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_gui.py b/tests/transports/epics/pva/test_pva_gui.py index f5403d2ab..4a753608e 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"}) 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 000000000..f97bd8edc --- /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)}, + ) + ) diff --git a/tests/transports/epics/test_emission.py b/tests/transports/epics/test_emission.py new file mode 100644 index 000000000..a1b10fd56 --- /dev/null +++ b/tests/transports/epics/test_emission.py @@ -0,0 +1,206 @@ +"""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, + _coerce_pascal_name, + 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_index_uses_controller_id_verbatim(two_apis, tmp_path: Path): + """#368: index DeviceRef pv must match the IOC prefix verbatim (no case fold).""" + emit_gui_files(two_apis, EpicsGUIOptions(output_dir=tmp_path), EpicsGUI) + + text = (tmp_path / f"{INDEX_STEM}.bob").read_text() + assert "alpha" in text + assert "beta" in text + assert "ALPHA" not in text + assert "BETA" not in 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() + + +# --- _coerce_pascal_name fail-fast (#369) ---------------------------------- + + +@pytest.mark.parametrize("bad_id", ["___", "-", "_-_", "----"]) +def test_coerce_pascal_name_rejects_all_punctuation_ids(bad_id: str): + """Ids that strip to empty would IndexError inside ``enforce_pascal_case``. + + The EPICS CA validator accepts ``[A-Za-z0-9_-]+`` so ids like ``"___"`` + and ``"-"`` reach GUI emission. We raise ``ValueError`` rather than + falling back to a generated name so the offending id surfaces in the + error message instead of producing a nonsense PascalStr the user + can't trace. + """ + with pytest.raises(ValueError, match=repr(bad_id)) as excinfo: + _coerce_pascal_name(bad_id) + + assert "Pascal-case" in str(excinfo.value) + + +def test_coerce_pascal_name_accepts_digit_leading_id(): + """Happy path: digit-leading ids are still coerced via ``X`` prefix.""" + assert _coerce_pascal_name("120e1b6") == "X120e1b6" + + +def test_coerce_pascal_name_accepts_normal_pascal_id(): + """Happy path: ordinary Pascal-friendly ids round-trip cleanly.""" + assert _coerce_pascal_name("alpha") == "Alpha" diff --git a/tests/transports/epics/test_pv_prefix.py b/tests/transports/epics/test_pv_prefix.py new file mode 100644 index 000000000..5528afb29 --- /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([]) diff --git a/tests/transports/graphQL/test_graphql.py b/tests/transports/graphQL/test_graphql.py index 24f1c7c16..46d2fc8a9 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)})" @@ -66,7 +69,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) @@ -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}) diff --git a/tests/transports/rest/test_id_validator.py b/tests/transports/rest/test_id_validator.py new file mode 100644 index 000000000..fd7336b89 --- /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") diff --git a/tests/transports/rest/test_rest.py b/tests/transports/rest/test_rest.py index 2be0b21b9..80af6698b 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 3b599c624..61a8f73ef 100644 --- a/tests/transports/tango/test_dsr.py +++ b/tests/transports/tango/test_dsr.py @@ -43,16 +43,16 @@ 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): tango_transport = TangoTransport() tango_transport.connect( - tango_controller_api, + [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 000000000..96fe9a6a3 --- /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" + )