DiamondLightSource / DiamondLightSource/fastcs

PRD: Multiple top-level Controllers with per-controller ids (#120)

Open
#351 0 comments 0 reactions 0 assignees View on GitHub
enhancement needs design python
Dominant language
Python
Stars
6
Forks
8
Avg merge
2d 12h
Merged PRs (30d)
3

Description

## Problem Statement

A FastCS application can currently expose only a single root Controller. Real beamline devices and orchestration patterns — PandA blocks, CATio with one ECAT master plus N ERIO peer controllers, a PMAC plus per-axis peer controllers — need multiple independent root Controllers in one process, each with its own stable PV prefix that survives axis moves, configuration reloads, and software restarts. Today, the only way to host more than one device per FastCS application is to spin up multiple processes, which fragments archiving, GUI builds, and operator workflows across IOCs.

## Solution

A FastCS application can host multiple top-level Controllers in a single process. Each Controller carries an `id` chosen by the controls engineer. That `id` becomes the first segment of every transport path:

- For EPICS CA and PVA, it is the PV prefix.
- For REST, it is the leading URL path segment.
- For GraphQL, it is the top-level Query field key.
- For Tango, it is the leading attribute-name segment.

The configuration file is renamed from `controller.yaml` to `fastcs.yaml` and uses a dict-keyed-by-id form for the new `controllers:` block. Each entry chooses its Controller class via a `type:` discriminator. A single transport entry per kind (`epicsca:`, `epicspva:`, `rest:`, `graphql:`, `tango:`) serves all configured controllers from one process. The `id` is used verbatim in every transport — there is no silent mangling — and each transport validates the `id` against its own naming rules at startup, failing fast on illegal characters.

```yaml
controllers:
BLxxI-EA-ECAT-01:
type: ECATController
controller: { ... }
BLxxI-EA-ERIO-01:
type: ERIOController
controller: { ... }
transport:
- epicsca:
- epicspva:
- rest:
```

When only one Controller class is registered with `launch()`, the `type:` field can be omitted.

## User Stories

1. As a beamline engineer, I want to host multiple controllers in one IOC, so that archiving, GUI builds, and discovery tooling stay co-located.
2. As a controls engineer, I want each controller's PV prefix to be a deliberate string I choose, so that downstream archiving and clients have a stable reference.
3. As a controls engineer, I want to host multiple different Controller classes in the same process, so that I can model coupled hardware (e.g. an ECAT master plus its ERIO peers) coherently.
4. As a controls engineer, I want to spin up several independent instances of the same Controller class with different ids and connection settings, so that I can serve a multi-coupler bus from one application.
5. As a Controller author with one device, I want the new YAML to be only mildly more verbose than today's single-controller form, so that the simple case is not penalised.
6. As a Controller author with one Controller class registered, I want to omit the `type:` field, so that my config stays concise.
7. As an application author with multiple Controller classes, I want each YAML entry to declare which class it is, so that the launcher instantiates the right one.
8. As an application author, I want to register all my Controller classes once at the launch site, so that I have a single place to update when adding new controller types.
9. As a Controller author, I want to read `self.id` in `initialise()` and beyond, so that I can include it in log messages and device-state queries.
10. As a Controller author, I want a clear error if I try to read `self.id` from `__init__`, so that I can fix the timing without confusion.
11. As a Controller author, I want `controller.id` to be set exactly once by the launcher, so that I cannot accidentally rebind it.
12. As a Phoebus user, I want each controller to expose its own `:PVI` root, so that I can browse each device independently with my existing PVI tools.
13. As a beamline operator, I want a generated index screen with one button per controller, so that I can launch each device's screen from one place.
14. As a beamline operator, I want each controller's screen at `output_dir/{id}.bob`, so that I can open it directly without going through the index.
15. As a maintainer, I want generated docs split into one file per controller, so that documentation diffs track the code that changed.
16. As a controls engineer, I want each transport to fail fast at startup if my `id` violates its naming rules, so that I find the problem before the IOC tries to start.
17. As a controls engineer, I want my `id` used verbatim in every transport with no silent name mangling, so that I can correlate names across logs, PVs, URLs, and GraphQL queries.
18. As a controls engineer mixing EPICS and GraphQL transports, I want clear documentation that my `id` must satisfy both transports' rules, so that I plan around the lowest-common-denominator naming.
19. As a REST client author, I want `GET /{id}/{sub}/{attr}` routes for each controller, so that I can address each device by its id.
20. As a GraphQL client author, I want one combined schema with one Query field per controller id, so that I can query all devices from one endpoint.
21. As a Tango client author, I want each controller exposed as its own Tango device with its id in the device name, so that existing Tango tooling works.
22. As a controls engineer, I want my `controllers:` block to be a YAML dict keyed by id, so that duplicate ids are caught at YAML parse time without manual checks.
23. As a beamline operator viewing the index screen, I want controllers to appear in the order I declared them in YAML, so that the screen layout is predictable.
24. As an operator, I want every log line emitted by a controller to surface its id, so that I can filter logs by device.
25. As a developer debugging at the IPython shell embedded in FastCS, I want a `controllers` dict keyed by id, so that I can poke at each controller individually.
26. As a developer debugging at the IPython shell, I want `controllers["X"].api` to give me the transport-visible projection of that controller, so that I do not have to maintain a separate `controller_apis` lookup.
27. As a REST client integrator, I want one OpenAPI schema that describes all controllers, so that I can codegen one client.
28. As a YAML author, I want `launch ... schema` to emit an accurate JSON schema for the new dict-by-id options, so that my IDE provides correct completions.
29. As a new fastcs user, I want the bundled demo and docs to use the `fastcs.yaml` filename, so that I can copy the convention.
30. As a new fastcs user, I want the bundled demo to host two controllers, so that I can see the multi-controller feature working out of the box.
31. As an author of an existing FastCS application, I want clear migration guidance and a changelog entry, so that I can move from `controller.yaml` to `fastcs.yaml` with confidence.
32. As a controls engineer using EPICS, I want a clear error when my id plus path lengths exceed the EPICS 60-character PV name limit, so that I can shorten names before deploying.
33. As a developer reading logs, I want `repr(controller)` to include the id when set, so that crashes and tracebacks tell me which controller is involved.
34. As a controls engineer using only one transport (e.g. only EPICS), I want to use idiomatic naming for that transport (e.g. hyphenated PV-style ids) without being forced into a lowest-common-denominator charset.

## Implementation Decisions

- Multi-controller runtime model uses an explicit list of root Controllers; no synthetic wrapper Controller is introduced.
- The `controllers:` block in `fastcs.yaml` is a dict keyed by id. Each value carries a `type:` discriminator and a `controller:` options block.
- `launch()` accepts a list of Controller classes. The type discriminator value defaults to the class's `__name__`; an optional `type_name: ClassVar[str]` on the class overrides this.
- When only one class is registered with `launch()`, `type:` may be omitted in YAML and is inferred.
- `Controller` instances gain an `id` attribute set by the launcher after `__init__` returns and before `initialise()`. Reading `id` before it has been set raises a clear runtime error. Setting it twice raises.
- The `id` becomes the first segment of every `ControllerAPI.path`. Sub-controller paths continue to be produced by recursive API construction.
- All transports adopt a uniform `connect(controller_apis: list[ControllerAPI], loop)` interface.
- Each transport validates the `id` against its own naming rules at connect time and raises a clear startup error on illegal characters. There is no silent name mangling. Mixing transports with incompatible naming rules forces a lowest-common-denominator id; this is documented.
- EPICS CA and PVA host all configured controllers in one softioc with N independent PVI roots. There is no super-parent PVI record. The `EpicsIOCOptions` dataclass and its `pv_prefix` field are removed entirely.
- EPICS path-to-PV mapping treats `path[0]` (the id) verbatim; subsequent path segments continue to go through snake-to-Pascal conversion. The previous separate `prefix` parameter to the PV-prefix utility is removed.
- The GraphQL transport synthesises one combined schema with N top-level Query fields keyed by id.
- The REST and Tango transports iterate per-controller for route and device construction respectively.
- GUI emission produces a directory containing one file per controller plus an index file linking to them, using pvi's `format_index`. The index file is always emitted, even when there is one controller, so the file layout is stable as the number of controllers changes.
- Docs emission mirrors GUI: one file per controller in the configured output directory.
- The IPython shell context exposes `controllers: dict[id, Controller]`. There is no parallel `controller_apis` dict; instead, each `Controller` instance gains an `api` back-pointer to its `ControllerAPI`, set by `FastCS` after API build.
- Logging surfaces controller ids in the FastCS startup line and in `Controller.__repr__` when the id has been set.
- The demo YAML is renamed from `controller.yaml` to `fastcs.yaml`. The launcher does not hard-code the filename.
- Deep modules extracted for testability:
- **D1** Controller registry / discriminator resolution (build the dynamic Pydantic model, instantiate Controllers from YAML, set ids, with single-class type inference and `type_name` overrides).
- **D2** PV-prefix derivation utility (path[0] verbatim, path[1:] snake-to-Pascal).
- **D3** Per-transport id validators (one per transport).
- **D4** GUI/docs file emission (per-id files plus index file).
- ADR considerations: setting `id` after `__init__` and before `initialise()` is consistent with ADR 0003 (controller initialisation on the main event loop). Adding a `Controller.api` back-pointer mildly relaxes ADR 0006's strict view-only abstraction; the relaxation is intentional, framed as a debugging affordance, and worth documenting in a follow-up ADR if the team agrees.

## Testing Decisions

- Tests cover external behaviour: `ControllerAPI` shapes, file-system outputs, transport-visible names, error messages, and lifecycle event ordering. Implementation details (private attributes, internal traversal order beyond what the spec promises) are not asserted.
- Existing tests migrate to the new schema in the same commit as the change to the underlying interface, so each commit ships with a green test suite. There is no separate "fix tests" commit.
- Each of the four deep modules (D1–D4) gets its own unit tests in the commit that introduces it.
- A new file `tests/test_multi_controller.py` covers five end-to-end multi-controller scenarios:
1. Two controllers wire into one IOC with distinct PV prefixes and no clash.
2. `controller.api.path == [id]` for the root, `[id, sub]` for sub-controllers.
3. Index file is emitted alongside per-id files.
4. `controller.id` is available from `initialise()` onward and raises if read in `__init__`.
5. `set_id` raises if called twice.
- `tests/test_launch.py` gains targeted cases for type discriminator resolution, single-class type inference, and duplicate-id rejection (handled naturally by Pydantic's `dict[str, ...]` parsing).
- Each transport test file gains a per-transport id-validation case demonstrating the fail-fast behaviour at `connect()` time.
- The existing GUI test gains a case asserting that the index file is generated alongside per-controller files.
- Prior art followed: `tests/test_launch.py` for options-model shape; `tests/test_controllers.py` for sub-controller registration and path mechanics; `tests/transports/epics/ca/test_softioc_system.py` for end-to-end IOC behaviour.
- The `conftest.pv_prefix` fixture is migrated to id-based naming as part of the EPICS multi-root commit.

## Out of Scope

- Tutorial rewrite ("more sexy code"). Tracked separately for a follow-up PR.
- Useful defaults such as a default CA + PVA transport pair when `transport:` is omitted. Tracked separately.
- A super-PVI parent record listing all controllers from one well-known PV. Deliberately omitted in this PR; revisit if discovery tooling demands it.
- Automated migration tooling for users moving from `controller.yaml` to `fastcs.yaml`. Migration is manual and documented in the changelog.
- Per-controller GUI / docs overrides (different titles, output paths, file formats per id). The single transport-level options apply to all controllers.
- Per-transport id aliases (different `id` per transport for the same controller). Explicitly rejected to keep the contract simple.
- Multi-process FastCS deployments. This PRD addresses multi-controller within one process only.

## Further Notes

- Implementation lands as a single PR with eight commits (prep `controller_pv_prefix` rework; prep Controller.id mechanics; transport base + REST + Tango + per-transport validators; EPICS multi-root internals; GraphQL combined schema; GUI / docs emission; launch / config; demo). Each commit ships green and carries the unit tests for the code it introduces.
- The repository does not currently have a `needs-triage` label. This issue is tagged `enhancement`, `python`, and `needs design` instead.
- Source design discussion: #120. A summary of the design pass, including the switch from list-with-id to dict-keyed-by-id, has been left as a comment on that issue for visibility to @coretl.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.