diff --git a/docs/conf.py b/docs/conf.py index 4f74d36ee..335e94ad6 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -79,6 +79,7 @@ ("py:class", "asyncio.streams.StreamReader"), ("py:class", "asyncio.streams.StreamWriter"), ("py:class", "asyncio.locks.Event"), + ("py:class", "_asyncio.Task"), # Annoying error: # docstring of collections.abc.Callable:1: WARNING: # 'any' reference target not found: self [ref.any] @@ -91,6 +92,8 @@ ("py:class", "p4p.nt.NTTable"), # httpx and fastapi don't have intersphinx mappings ("py:class", "httpx.AsyncBaseTransport"), + ("py:class", "httpx.AsyncClient"), + ("py:class", "httpx.Response"), ("py:class", "fastapi.applications.FastAPI"), # Problems in FastCS itself ("py:class", "BaseController"), @@ -116,6 +119,8 @@ nitpick_ignore_regex = [ ("py:class", r"fastcs.*.DType_T"), ("py:class", r"fastcs.*.Numeric_T"), + ("py:class", r"fastcs.*.Connection_T"), + ("py:obj", r"fastcs.*.Connection_T"), ("py:obj", r"fastcs.*.DType_T"), (r"py:.*", r"fastcs\.demo.*"), (r"py:.*", r"tickit.*"), diff --git a/docs/explanations/connections.md b/docs/explanations/connections.md new file mode 100644 index 000000000..36afe87de --- /dev/null +++ b/docs/explanations/connections.md @@ -0,0 +1,308 @@ +# Connections + +A `Connection` is a link to hardware. It is small: a constructor, `connect`, `close` +and the IO methods a driver needs, plus an optional list of the connection types it +depends on. It holds no health state and has no hooks. + +Everything ongoing about the link belongs to its `Supervisor`: whether it is up, +reconnecting it and its retry budget, the scans of the controllers that use it, and +their `connected` attributes. There is one supervisor per connection, so connections, +not controllers, are the unit of failure and recovery. A tree of five sub controllers +behind one socket has one health state, one reconnect loop and one retry budget +between them. + +## Writing one + +Subclass `Connection`. Its constructor arguments are its device settings - they become +its block in `fastcs.yaml` - and its IO methods just talk to the device: + +```python +from fastcs.connections import Connection + + +class DetectorConnection(Connection): + def __init__(self, host: str, port: int = 8000) -> None: + self._base = f"http://{host}:{port}" + self._client: AsyncClient | None = None + + async def connect(self) -> None: + self._client = AsyncClient(base_url=self._base) + + async def close(self) -> None: + if self._client is not None: + await self._client.aclose() + self._client = None + + async def get(self, path: str): + response = await self._client.get(path) + response.raise_for_status() + return response.json()["value"] +``` + +There is no `try`/`except` in `get`. When something goes wrong, IO methods just raise, +and the supervisor decides what the exception means - see the boundary, below. + +`connect` means "make the link usable", not merely "open the socket": a device that +needs a mode set every time it comes back has that write here. `close` must tolerate +being called on a link that is already closed - the supervisor calls it before every +reconnect attempt. + +### The ones that come with FastCS + +Most drivers need none of the above. `IPConnection` and `SerialConnection` cover the +stream transports; `HTTPConnection` covers REST devices, with `get`, `get_bytes`, +`put` and the `request` underneath them, so a driver that only needs a different URL +layout writes that and nothing else: + +```python +class DetectorConnection(HTTPConnection): + # The detector wraps every value as {"value": ...}, which is the detector's + # convention rather than HTTP's - so this is the whole subclass. + async def get(self, path: str): + return (await super().get(path))["value"] +``` + +`SimConnection` is the base for a simulated device: it opens and closes trivially and +can never fail. A simulator is a *sibling* of the real transport rather than a +subclass - inheriting `SerialConnection` would inherit a serial handle it never opens. + +## Holding one + +A controller is given its connection; it never looks one up. The top-level controller +declares its connections as type-hinted constructor arguments, and passes each down to +the children that need it: + +```python +class DetectorController(Controller): + # Narrows the base class's connection so this controller's own code can call + # the methods of the connection it actually holds. + connection: DetectorConnection + + def __init__(self, connection: DetectorConnection) -> None: + self.connection = connection + super().__init__() + + async def build(self) -> None: + # Every connection is open by now, so this can ask the device what it has. + for parameter in await self.connection.get("detector/api/1.8.0/config/keys"): + ... # one attribute per reported key + + +class EigerController(Controller): + # The class named in fastcs.yaml. Its type hints declare the connections. + def __init__(self, eiger: DetectorConnection, odin: OdinConnection) -> None: + super().__init__() + self.EIGER_DET = DetectorController(eiger) + self.ODIN = OdinController(odin) +``` + +Because the parent passes connections in, the same class works for any number of +instances - a `MotorController` can be used for pitch and yaw in one tree - and naming +stays at the top level, where the schema can check it. + +What arrives in the constructor is not the connection itself but a **handle**: a +stand-in that passes as the declared type, for type checkers and `isinstance` alike, +and routes every `async` method call through the connection's supervisor. Authors +never create or see a handle type. Two handles to one connection need not be the same +object, so the framework compares the connections behind them. + +A controller's attributes talk only to its own connection - two devices means two +controllers. That is what lets each attribute belong to exactly one supervisor. A +controller that needs another device's behaviour calls a method on the controller +that owns it. A controller with no connection at all (a soft controller that only +groups others, or a `ControllerVector`) is never paused. + +Tests need none of this: a controller only needs something that behaves like its +connection type, so a test passes the connection itself, or a mock. + +## Configuring them + +`fastcs.yaml` names each connection under the controller's entry, by its constructor +argument name. The schema is generated from the type hints, so it knows exactly which +names are required and what settings each has: + +```yaml +controllers: + - id: EIGER + type: mydriver.EigerController + connections: + eiger: + settings: {host: eiger.local} + odin: + settings: {host: odin.local, port: 8888} + reconnect_attempts: 5 + reconnect_period: 2.0 +``` + +There is no `type:` - the type hint supplies it - and no `depends_on`, which is +declared on the Connection class. `reconnect_attempts` and `reconnect_period` are +framework settings, read by the supervisor: retry behaviour is a fact about a +deployment, not about a device class. + +An application that builds its own tree does what the launcher does - create each +connection, give it a supervisor, and pass the handle to the controller: + +```python +eiger = Supervisor(DetectorConnection("eiger.local"), name="eiger") +controller = EigerDetController(eiger.handle) +FastCS(controller, transports, supervisors=[eiger]).run() +``` + +See [](../how-to/launch-framework.md) for the configuration in full. + +## Startup + +The `ControllerRunner` owns the startup and shutdown sequence, and nothing ongoing: + +1. Open every connection, in dependency order. Any failure exits. +2. Walk the tree calling `build`, repeating over anything newly added until a pass + adds nothing. +3. **Seal** the tree: resolve dependencies into a fixed graph, hand each controller's + scans to its connection's supervisor, add the `connected` attributes, and freeze + the API. Nothing can be added to the tree after this. +4. Transports attach, before any values flow. +5. Call `setup` once across the whole tree, run the read-once reads, and hand over to + the supervisors. + +A connection that fails at startup stops the application, and the orchestrator +restarts it. Restarting at startup is cheap, and a partly built tree - clients connect +successfully and never find what they are looking for - is worse than none. + +## The boundary + +Every call through a handle passes its supervisor's boundary, which: + +- fails fast with `DisconnectedError`, doing no IO, if the link is already down, +- turns a link failure into `DisconnectedError`, marks the link down and wakes the + reconnect loop, +- passes a device error - the device said no - back to the caller unchanged. + +Someone has to tell "the link is gone" from "the device said no", and the boundary +can do it broadly because it only ever sees exceptions from connection IO. The +connection's `ConnectionPolicy` says which is which. By default: + +| What happened | Treated as | +| --------------------------------------------------- | ----------------------------- | +| Connection reset, broken pipe, refused, unreachable | Disconnected | +| EOF or an incomplete read | Disconnected | +| Device node vanished (DRA) | Disconnected | +| Timeouts | Disconnected after 3 in a row | +| Error reply, rejected value, HTTP error status | Passed to caller | +| Parse errors | Passed to caller | + +A device with its own way of saying "I'm offline", such as a special reply, may raise +`DisconnectedError` directly; that is optional. + +Because the supervisor sees the error where it starts, a getter with a broad +`try`/`except` cannot hide a dead link, and every caller is covered - getters, +setters, commands and spawned tasks alike. A put on a dead link fails back to the +client rather than being logged and swallowed. Calls a connection makes to its own +methods do not pass the handle, so each call from a controller is counted once. + +Detection is passive: a connection is only known to be down once something uses it. A +supervisor with no polling among its controllers warns about that at startup. + +## While it is up, and while it is not + +Each attribute and `@scan` method belongs to the supervisor of its controller's +connection, grouped by period, one task per period. A soft controller's periodic work +runs in a loop owned by the runner. So each connection is paused on its own: a +struggling Odin link cannot delay Eiger polls that share its period, and a child on a +healthy link keeps polling when its parent's link drops. + +When a link goes down, its supervisor pauses its scans, turns the `connected` +attribute of each controller holding it off, and marks every polled value it owns as +`Severity.INVALID`, so clients see stale values flagged rather than stale values that +look fine. Scans stay silent about `DisconnectedError`: the supervisor has already +said so. Other scan errors are logged once, then as a periodic reminder, until the +scan succeeds again. + +When it comes back, `connected` goes on, the read-once reads of the controllers +holding it run again - a rebooted device can come back with different values - and the +scans resume. `setup` does not run again: configuration a device needs every time it +comes back belongs in `connect`. + +Every controller has a read-only `connected` attribute, added at the seal. A soft +controller reports it as always on. It is an ordinary attribute, so every transport +publishes it with no changes. + +## Reconnecting + +``` +forever: + sleep until the link is marked down + + loop: + wait until every connection it depends on is up # attempts are not used + if one of them gave up: + stall and stop # only a restart can help + + close(), then connect() + + if it worked: + mark link up # connected on, read-once reads again, scans resume + back to sleeping + + attempts += 1 + if attempts == reconnect_attempts: + give up and stop # connected stays off; dependents stall + sleep reconnect_period +``` + +The supervisor logs each transition once: down, retrying, back up, giving up. + +### Policies + +Some failures are known never to recover, and retrying them is noise. The +`ConnectionPolicy` says which: a failed reconnect the policy calls terminal gives up +at once instead of burning the rest of the budget, and the "Giving up" log line says +why. A device node injected by a Kubernetes DRA claim is the case that motivated it - +the claim is made when the pod starts, so a node that has gone will not come back +without a restart: + +```python +from fastcs.connections import DRAPolicy, SerialConnection + + +class StageConnection(SerialConnection): + policy = DRAPolicy() +``` + +`DRAPolicy` is also *fatal*: rather than stall with its dependents serving stale +values, it asks the runner to shut the application down, so the orchestrator restarts +the pod and the claim is re-established. + +A policy is a set of independent settings rather than one class per behaviour - which +errors count as disconnection, the timeout count, which failures are terminal, and +whether that is fatal - so a slow DRA device is `DRAPolicy(timeout_count=10)`, not a +new class. It is a class attribute of the connection rather than a constructor +argument, so it does not appear in the connection's configuration, and a `Supervisor` +can be given a different one. + +### Dependencies + +Dependencies are declared by type, on the Connection class - a driver fact, written +once by whoever knows it: + +```python +class OdinConnection(HTTPConnection): + depends_on = [EigerConnection] +``` + +This reads as: wait for every connection of those types under the same top-level +controller. At startup, connections open in dependency order, so every Eiger before +any Odin. At the seal the type rules are resolved into a fixed graph between +instances, and checked for cycles; reconnect loops use that graph. + +All of them must be up: a connection layered over two links is no more usable with +one of them than with neither. While any is down, the dependent waits instead of +attempting, so its retry budget is not burnt against a dead dependency. If any one of +them gives up entirely, the dependent stalls, saying so, rather than retrying +something that cannot work. Scoped to one top-level controller there is normally one +connection of each type; where there are several, a dependent waits for all of them. + +## Shutdown + +Closing is a runner operation, not an author hook: every connection is closed in +reverse of the order it was opened, so anything layered over another is closed before +what it rides on. `setup` is not undone - devices keep their last configured state. diff --git a/docs/explanations/controllers.md b/docs/explanations/controllers.md index b697fca1a..f944ab256 100644 --- a/docs/explanations/controllers.md +++ b/docs/explanations/controllers.md @@ -7,23 +7,42 @@ FastCS provides three controller classes: `Controller`, `ControllerVector`, and `Controller` is the primary building block for FastCS drivers. It can serve two roles: -**Root controller:** passed directly to the `FastCS` launcher. In this role, FastCS -will call its lifecycle hooks and run the scan tasks it creates on the event loop. +**Root controller:** passed directly to the `FastCS` launcher. **Sub controller:** attached to a parent controller via `add_sub_controller()` or by -assigning it as an attribute. In this role, the sub controller's lifecycle hooks -(`connect`, `reconnect`, `initialise`, `disconnect`) are not called automatically by -FastCS. The parent controller is responsible for calling them as part of its own -lifecycle, if required. +assigning it as an attribute. + +The `ControllerRunner` owns the order of the startup sequence and calls the +lifecycle hooks of **every** controller in the tree, root and sub alike. A parent +never drives a child's lifecycle to compensate for sequencing. ### Lifecycle hooks | Method | Purpose | |---|---| -| `initialise` | Fill declared attributes, and add dynamic ones, before the API is built | -| `connect` | Open connection to device | -| `reconnect` | Re-open connection after scan error | -| `disconnect` | Release device resources before shutdown | +| `__init__` | Everything knowable without the device: settings, static attributes | +| `build` | Structure that depends on the device - fill declared attributes, and add dynamic attributes and sub controllers | +| `setup` | Hardware writes and checks, once the whole tree is built | + +The same question, three ways: + +| What do I need to answer this? | Where it goes | +|---|---| +| Nothing - settings and the class | `__init__` | +| The device | `build` | +| My children, connected | `setup` | + +There is no `connect`, `reconnect` or `disconnect` hook. Opening the link, reopening +it after a failure and closing it at shutdown belong to the connection's `Supervisor` +and the runner - see [connections](./connections.md). `setup` runs once only, not +again after a reconnect. + +After `build`, the tree is **sealed**: nothing can be added to it from `setup` on, +because the API a transport serves is a snapshot of the tree at that point. + +`build` runs with every connection already open, so a controller that has to ask the +device what it has - how many channels, which parameters - reads it there and creates +what it finds. Attributes are constructed in `__init__`, or declared as class-body type hints and created for you - see [](declaring-attributes.md) for which to use when. @@ -32,12 +51,15 @@ shared by every instance of the controller. ### Scan task behaviour -When used as the root controller, FastCS collects all `@scan` methods and readable -attributes whose `getter` is wrapped in `Polled`, across the whole controller -hierarchy, to be run as background tasks by FastCS. Scan tasks are gated on the -`_connected` flag: if a scan -raises an exception, `_connected` is set to `False` and tasks pause until `reconnect` -sets it back to `True`. +FastCS collects all `@scan` methods and readable attributes whose `getter` is wrapped +in `Polled`, across the whole controller hierarchy, to be run as background tasks. +Each controller's work belongs to the supervisor of its **connection**, and is paused +while that connection is down rather than polling a link that cannot answer. A +controller with no connection is never paused. + +A scan that raises is logged and retried. It does not decide whether the connection +is down - the connection's supervisor has already seen the exception, at the call +that raised it. ```python from fastcs.controllers import Controller @@ -46,62 +68,55 @@ from fastcs.methods import scan class TemperatureController(Controller): - def __init__(self, host, port): + connection: DeviceConnection + + def __init__(self, connection: DeviceConnection): + self.connection = connection super().__init__() - self._host, self._port = host, port self.temperature = AttrR(float, units="degC") self.setpoint = AttrRW(float, units="degC") - async def connect(self): - self._client = await DeviceClient.connect(self._host, self._port) - self._connected = True - - async def reconnect(self): - try: - self._client = await DeviceClient.connect(self._host, self._port) - self._connected = True - except Exception: - logger.error("Failed to reconnect") - - async def disconnect(self): - await self._client.close() - @scan(period=1.0) async def update_temperature(self): - value = await self._client.get_temperature() + # Paused with the connection: while it is down this does not run at all. + value = await self.connection.get_temperature() await self.temperature.update(value) ``` ### Using Controller as a sub controller When a `Controller` is nested inside another, it organises the driver into logical -sections and its attributes are exposed under a prefixed path. If the sub -controller also has connection logic, the parent must invoke it explicitly: +sections and its attributes are exposed under a prefixed path. A sub controller that +talks to the same device is given the *same* connection as its parent rather than +consulting it, so the two share one supervisor: ```python class ChannelController(Controller): - def __init__(self): + connection: DeviceConnection + + def __init__(self, connection: DeviceConnection): + self.connection = connection super().__init__() - self.value = AttrR(float) - async def connect(self): - ... - self._connected = True + self.value = AttrR(float) class RootController(Controller): + connection: DeviceConnection + channel: ChannelController - def __init__(self): + def __init__(self, device: DeviceConnection): + self.connection = device super().__init__() - self.channel = ChannelController() - - async def connect(self): - await self.channel.connect() - self._connected = True + self.channel = ChannelController(self.connection) ``` +A sub controller that talks to a *different* device is passed that device's +connection instead, which the top-level controller declares as another constructor +argument. Nothing is inferred from tree position. + ## ControllerVector `ControllerVector` is a convenience wrapper for a set of controllers of the same type, @@ -133,12 +148,6 @@ class RootController(Controller): {i: ChannelController() for i in range(num_channels)} ) - async def connect(self): - for channel in self.channels.values(): - await channel.connect() - - self._connected = True - async def update_all(self): for index, channel in self.channels.items(): value = await self._client.get_channel(index) @@ -158,7 +167,7 @@ Use `ControllerVector` when: - The device has a set of identical channels, axes, or modules identified by number - You need to iterate over sub controllers and perform the same action on each -- The number of instances may vary (e.g. determined at runtime during `initialise`) +- The number of instances may vary (e.g. determined at runtime during `build`) Use a plain `Controller` with named sub controllers when the sub controllers are distinct components with different types or roles. diff --git a/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md b/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md index 4f67b0528..8a011dafb 100644 --- a/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md +++ b/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md @@ -118,3 +118,73 @@ surface referenced by decision 13 of #388. responsibility**. 4. **Who owns reconnect?** The runner owns the whole lifecycle, including reconnect. + +## Amendment: connections own reconnect (#422) + +Question 4 above says the runner owns reconnect, and stops there. It left the +*subject* of reconnect implicit, and the first implementation took it to be the +controller — which cannot work once controllers share a link, because +reconnecting a controller that does not own its connection is a no-op. + +The subject is the **connection**. Connection state moves off `Controller` onto a +first-class `Connection` object: controllers hold one, several may hold the same +one, and it owns its own health, reconnect task and retry budget. See +[connections](../connections.md) for the shape, and the design attached to +[issue #422](https://github.com/DiamondLightSource/fastcs/issues/422). + +What this amends: + +- **`Controller.connect`/`reconnect`/`disconnect`/`_connected` are removed.** A + driver never touches a connected flag; `Connection.connect()` opens the link or + raises, and the framework decides what that means. +- **`initialise`/`post_initialise` become `build`/`setup`**, splitting "structure + that depends on the device" from "hardware writes once the tree is built". + `build` runs with the connection open, so a controller that must ask the device + what it has does so there. +- **The runner owns the startup order** — connections, then `build` to a fixpoint, + then `setup`, then tasks — and shutdown, closing connections in reverse. + +Points the review of #420 asked to settle, and how they land: + +- **`connection._connected = True` from outside.** There is a framework-only + `_set_connected()` next to `set_disconnected()`, so the flag has one owner. +- **What "fatal" means.** Not `sys.exit`: an embedded FastCS must survive it. The + runner sets `fatal_error` (an `asyncio.Event`) and records `fatal_reason`; + `FastCS.serve` raises it, and an embedder observes it instead. +- **`check()` skipped while IO is succeeding.** Dropped. There is no separate + health-check hook: a connection with any polling is proved alive by that + polling, and a device that needs a heartbeat gets a `@scan`, which is ordinary + driver code. A connection nothing polls is warned about at startup. +- **`depends_on` cycles.** Declared rather than derived, and detected at startup. +- **`reconnect_attempts` exhausting to a terminal state.** Kept as the design specifies + (default 10), and *not* propagated to dependents — the parent's give-up message + names what it blocks. Whether the default should instead be retry-forever is + left open; it is one constant. + +## Amendment: no introspection, and one HTTP connection (#424 review) + +The amendment above made `Connection` generic in what `connect()` returned, handed +that value to `build`, and compared it on every reconnect. All of it is removed, +per the spec attached to [this review +comment](https://github.com/DiamondLightSource/fastcs/pull/424#issuecomment-5604015457). + +- **Introspection is a larger design problem than it looked**, and gets its own + issue rather than riding along here. A controller that must ask the device what it + has still does - in `build`, against an open connection - but the framework neither + carries the answer nor compares it. The unsolved part is a connection whose + *existence* depends on introspection: it cannot be declared in `fastcs.yaml`, + because it does not exist at config-parse time. +- **Every connection is declared up front**, so the runner's list is exactly what the + launcher built. No tree walking to collect connections, no deduplication by + identity, no second sweep after the build phase - and `ControllerRunner` requires + its connections argument rather than falling back to the tree. +- **`HTTPConnection` joins `IPConnection` and `SerialConnection`.** `fastcs-eiger` + and `fastcs-odin` each hand-roll an HTTP client, on different libraries; one + framework class replaces both. httpx, because it is already a FastCS dependency. +- **`SimConnection` is the base for a simulated device**, a sibling of the real + transports rather than a subclass of one, selected by `type:` in `fastcs.yaml`. +- **`max_attempts` becomes `reconnect_attempts`**, pairing with `reconnect_period`. + +The consequence for `fatal_error` is that nothing in the framework sets it: its one +producer was the introspection mismatch. The channel is kept, because the problem it +solves - a background task that cannot usefully raise - has not gone anywhere. diff --git a/docs/explanations/decisions/0021-connection-recovery-policy.md b/docs/explanations/decisions/0021-connection-recovery-policy.md new file mode 100644 index 000000000..7bee5ad25 --- /dev/null +++ b/docs/explanations/decisions/0021-connection-recovery-policy.md @@ -0,0 +1,100 @@ +# 21. A connection holds its recovery policy rather than inheriting it + +Date: 2026-09-11 + +## Status + +Accepted, and amended by +[ADR 22](0022-connection-supervisors-and-declared-connections.md): `Recovery` is now +`ConnectionPolicy` and `DRANode` is `DRAPolicy`; the policy also classifies IO +failures, and `reconnect_period` and `reconnect_attempts` have moved off the +connection to its `Supervisor`, set per instance in `fastcs.yaml`. + +## Context + +A connection whose reconnect fails is retried every `reconnect_period` until +`reconnect_attempts` consecutive failures, then gives up and stalls its dependents +until the process restarts. For some failures that is the wrong shape. A device node +injected by a Kubernetes DRA claim - a USB/IP serial port, say - is established when +the pod starts; once it has gone it will not reappear in that pod, so every retry +is noise and the connection then sits there, apparently healthy, until someone +notices. The only fix is a pod restart. + +Commit `0f62b58` expressed this by inheritance. `Connection` gained two hooks, +`is_terminal(exc)` and `unrecoverable_reason()`, and `fastcs.connections.dra` added a +`DRADeviceMixin` that overrode them, requiring a driver to implement an abstract +`_node_path` property: + +```python +class DRASerialConnection(DRADeviceMixin, SerialConnection): + @property + def _node_path(self) -> str: + return self._settings.port +``` + +The runner never consulted either hook, so the mixin had no effect yet. Porting +`fastcs-ximc` to it surfaced four problems, all of them about inheritance rather than +the behaviour: + +1. **MRO order was a silent trap.** `class X(DRADeviceMixin, SerialConnection)` + worked; `class X(SerialConnection, DRADeviceMixin)` inherited + `Connection.is_terminal`, returned `False`, and the mixin did nothing - no error, + no warning, nothing in a diff to notice. +2. **A class per transport × policy.** Serial needed `DRASerialConnection`; a claimed + IP or HTTP device needed another, and a second policy would multiply them again. A + claimed node behaves identically behind any transport, so this was duplication + with no content. +3. **The policy could not change without changing the class.** Whether a node comes + from a DRA claim is a deployment fact, but inheritance fixed it at authoring time. +4. **The contract was a private abstract hook.** `_node_path` was what a driver had + to implement, and it appeared in no public signature. + +## Decision + +Replace the mixin with a policy object the connection holds. + +- `fastcs.connections.recovery` defines `Recovery`, the default: it calls no failure + terminal. It has `is_terminal(exc)`, `reason(connection)` for the log line, and + `is_fatal`, which says whether a terminal failure should bring the application + down. `DRANode` is the first subclass: `FileNotFoundError` is terminal, and fatal. + Policies are stateless, so one instance can be shared. +- `Connection.recovery` is a class attribute defaulting to `Recovery()`. A connection + class sets it, or it is assigned to one instance. It is deliberately not a + constructor argument: the launcher builds a connection's config schema from its + `__init__` signature, so an argument would appear in every connection's schema, + and choosing a policy in YAML would need a discriminated union. +- `Connection.is_terminal` and `Connection.unrecoverable_reason` are removed rather + than kept as delegating wrappers, which would give two ways to say one thing with + undefined precedence when a subclass both overrode a method and held a policy. +- `Connection.label` is a public property naming the device - the port, address or + base URL for the framework connections, and the class name otherwise. It replaces + `_node_path`. +- The `ControllerRunner` consults the policy on every failed reconnect. A terminal + failure gives up immediately, without spending the rest of the retry budget; if + the policy is fatal, the runner also reports it through `fatal_error`, which + `FastCS.serve` already turns into a clean shutdown. The first `connect` at startup + is not affected, since a failure there already aborts startup. +- `fastcs.connections.dra` is deleted. + +`reconnect_period` and `reconnect_attempts` stay on the connection. Whether a failure +is terminal is a device fact that no deployment should be able to contradict; the +period and the budget are what a site tunes, and are configurable per connection in +`fastcs.yaml`. Folding them into the policy would mean either a site could overrule +the device fact or the numbers stopped being configurable. + +## Consequences + +One policy serves any transport, and a connection is given one by assignment, so the +four problems above go away: there is no base-class order to get wrong, no class per +combination, a policy can be changed on an instance, and the contract a policy relies +on (`label`) is public. + +A DRA-claimed device whose node disappears now gives up on the first failed reconnect +and shuts the application down with a message saying why, so the orchestrator +restarts the pod and the claim is re-established. + +The policy is not selectable from `fastcs.yaml`; the connection class carries the +choice. That can be revisited if a deployment needs to change policy without +changing `type:`. A later extension could let a policy own the retry *schedule* +(for example a backoff computed from the connection's `reconnect_period`) while the +connection keeps owning the numbers; that is a separate decision. diff --git a/docs/explanations/decisions/0022-connection-supervisors-and-declared-connections.md b/docs/explanations/decisions/0022-connection-supervisors-and-declared-connections.md new file mode 100644 index 000000000..57b071c82 --- /dev/null +++ b/docs/explanations/decisions/0022-connection-supervisors-and-declared-connections.md @@ -0,0 +1,105 @@ +# 22. Connections are declared by type hints and owned by a Supervisor + +Date: 2026-09-28 + +## Status + +Accepted. Amends [ADR 21](0021-connection-recovery-policy.md). + +## Context + +Connections had grown several jobs, and several places that each did part of one. + +- **The contract lived inside `__init__` bodies.** A controller claimed a connection + with `connections.get("name", Type)`, possibly several levels down the tree. The + schema for `fastcs.yaml` could check each connection's settings, but not which names + were needed or what type each had to be, so the config validated against a shape + nobody had declared. Lookup by name also broke reuse: a `MotorController` that claims + `"motor"` cannot be used twice in one tree, for pitch and yaw. +- **Failure was reported twice, by the author.** A connection's IO had to call + `set_disconnected()` *and* raise. The hook informed the framework and the exception + informed nobody; a missing hook meant nothing ever reconnected, and nothing + complained. +- **Health lived on the connection, scheduling on the tree.** The runner built scan + loops per top-level controller and gated them on the root's connection. A sub + controller on a different connection kept polling its dead link, and was frozen when + the root's link dropped even if its own was healthy. A soft root gated nothing. +- **Clients could not see it.** The runner knew a link was down, but clients saw stale + values with no alarm. +- **Deployment facts and driver facts were mixed.** `depends_on` was written in every + deployment's YAML, although Odin needs Eiger whatever the deployment, while the retry + settings were constructor arguments on every connection class. + +## Decision + +- **Controllers receive connections; they never look them up.** The top-level + controller declares its connections as constructor arguments hinted with a + `Connection` type, and passes each down to the children that need it. The launcher + reads the hints, generates the schema for the entry's `connections:` block - one + required key per argument, no `type:` - builds each connection from its settings, and + passes it in. `Connections`, `connections.get` and `launch(connection_classes=...)` + are removed. +- **A `Supervisor` per connection owns everything ongoing about it:** health, the + reconnect loop and its retry budget, the scans of the controllers holding it, and + their `connected` attributes. The `ControllerRunner` owns only the startup and + shutdown sequence. The launcher creates the supervisors, since their handles must + exist before the controllers are constructed; an embedder creates them itself. +- **Controllers hold a handle, not the connection.** The handle passes as the + connection's type for type checkers and `isinstance`, and routes every `async` method + call through the supervisor's boundary, which fails fast with `DisconnectedError` + while the link is down, turns a link failure into `DisconnectedError` and marks the + link down, and passes a device error back unchanged. `set_disconnected()`, + `connected`, `wait_up()` and `wait_down()` are removed from `Connection`, which is + left with a constructor, `connect`, `close`, its IO methods, `depends_on` and + `policy`. +- **`ConnectionPolicy` classifies failures.** `Recovery` becomes `ConnectionPolicy`, a + frozen set of independent settings: which errors mean disconnection (by default + `OSError`, `EOFError` and httpx transport errors), which are timeouts and how many in + a row mean disconnection (3), which reconnect failures are terminal, and whether that + is fatal. `DRANode` becomes `DRAPolicy`. +- **Dependencies are declared by type, on the Connection class:** + `OdinConnection.depends_on = [EigerConnection]` means wait for every + `EigerConnection` under the same top-level controller. Connections open in that order + at startup; the rules are resolved into a fixed graph, checked for cycles, at the + seal. +- **Retry settings are framework keys in `fastcs.yaml`,** `reconnect_attempts` and + `reconnect_period` on each connection, read by the supervisor. +- **Scans are owned per connection.** Each attribute and `@scan` method belongs to the + supervisor of its controller's connection, grouped by period, and is paused while + that link is down. A soft controller's work runs in a loop owned by the runner. +- **Health is published as attributes.** Every controller gets a read-only + `connected` attribute at the seal, kept in step by its supervisor and always on for a + soft controller. When a link drops, every value its supervisor polls is republished + with `Severity.INVALID`, and a readback callback now fires when only the severity + changes. No transport changes. +- **Read-once reads re-run after every reconnect; `setup()` does not.** Configuration a + device needs every time it comes back belongs in `connect()`. +- **An explicit seal.** After the build phase, `connected` is added and the tree is + frozen: adding an attribute, method or sub controller afterwards raises, rather than + silently never reaching a transport. +- **A put on a dead link fails back to the caller** instead of being logged and + swallowed. + +## Consequences + +`fastcs.yaml` is fully schema-checked, and a controller class can be reused for any +number of instances. Driver IO needs no error handling for the link: nothing is left +to forget, and a getter that swallows an exception cannot hide a dead link, because the +supervisor saw it first. Each connection pauses and recovers on its own, which fixes +the root-gating bug, and clients see a dead link as `connected` off and invalid values. + +Every controller's API gains a `connected` attribute, so every transport serves one +more PV or field per controller. + +Deferred to later work, as the design proposed: + +- Introspection on reconnect - comparing what the device reports about itself with + what it reported at startup. +- Connections created during `build`, handed to the framework with an explicit `open()`. + Until then, a controller holding a connection no supervisor owns is rejected at the + seal. +- Push-based connections, whose failures never pass the boundary. +- Selecting a simulated connection from the YAML, and choosing a policy per instance + there. +- Several connections of one type in one entry wait on all of them; an explicit + per-instance override is not provided. diff --git a/docs/explanations/declaring-attributes.md b/docs/explanations/declaring-attributes.md index f99535f97..643d1dd6a 100644 --- a/docs/explanations/declaring-attributes.md +++ b/docs/explanations/declaring-attributes.md @@ -47,7 +47,7 @@ attribute from it: class OdinDetector(Controller): frames: AttrRW[int] - async def initialise(self) -> None: + async def build(self) -> None: for name, spec in await self._query_parameter_tree(): self.filler.fill_attribute( name, getter=spec.getter, setter=spec.setter, **spec.meta @@ -59,7 +59,7 @@ class OdinDetector(Controller): The hint is not a promise to build something later. `self.frames` **exists as soon as `__init__` returns** — as an `AttrRW[int]` with no IO yet — so the rest of `__init__` can reference it, hand it to a sibling, or subscribe to it. That -rule is what makes `initialise` safe to run in parallel across controllers: +rule is what makes `build` safe to run in parallel across controllers: only `__init__` is serial, and by the time it ends every attribute anything refers to is there. @@ -79,7 +79,7 @@ class EigerDetector(Controller): state: AttrR # enum built from the device's `allowed_values` ``` -FastCS cannot create that one, so it is a **promise** instead: introspection +FastCS cannot create that one, so it is a **promise** instead: `build` must add it with `add_attribute`, and `check_filled` fails if nothing did. The access mode is still checked — adding an `AttrW` where an `AttrR` was promised raises. @@ -104,8 +104,8 @@ own without FastCS knowing anything about it. `check_filled()` raises if anything the class body declared is missing, listing it by name. FastCS calls it across the whole controller tree after -`initialise`, so a driver that forgets cannot serve a half-built API; call it -yourself at the end of your own `initialise` to fail before anything else runs. +`build`, so a driver that forgets cannot serve a half-built API; call it +yourself at the end of your own `build` to fail before anything else runs. An `| None` hint is not required. ## Summary @@ -113,9 +113,9 @@ An `| None` hint is not required. | You know | Write | |---|---| | Everything about the attribute | `self.x = AttrRW(...)` in `__init__` | -| Its type, but not its IO or metadata | `x: AttrRW[int]` and fill it in `initialise` | -| Its access mode only | `x: AttrR` and `add_attribute` it in `initialise` | -| Nothing until the device answers | No declaration; `add_attribute` in `initialise` | +| Its type, but not its IO or metadata | `x: AttrRW[int]` and fill it in `build` | +| Its access mode only | `x: AttrR` and `add_attribute` it in `build` | +| Nothing until the device answers | No declaration; `add_attribute` in `build` | See [ADR 0013](decisions/0013-declarative-procedural-split-and-controller-filler.md) for why there is one declarative mechanism rather than two. diff --git a/docs/explanations/stable-interface.md b/docs/explanations/stable-interface.md index 52f9c4b21..58c50a69e 100644 --- a/docs/explanations/stable-interface.md +++ b/docs/explanations/stable-interface.md @@ -14,30 +14,41 @@ transports, no interactive shell. `FastCS` is a caller of it. ```python from fastcs.controllers import ControllerRunner -runner = ControllerRunner(controller) -apis = await runner.setup() # initialise, and build the ControllerAPIs -await runner.start() # connect, run initial tasks, start scanning +runner = ControllerRunner(controller, supervisors=[supervisor]) +apis = await runner.build() # open connections, build and seal the tree, build APIs +await runner.start() # setup, run initial tasks, start scanning ... -await runner.stop() # stop the tasks, disconnect +await runner.stop() # stop the tasks, close the connections ``` -- **`setup()`** runs `initialise()` and `post_initialise()` on each controller - and builds their `ControllerAPI`s. It exists as a separate step because - anything serving the controllers has to register its callbacks *before* the - first values are read, or it misses them. -- **`start()`** connects the controllers, runs the initial (`ONCE`) tasks, and - starts the periodic ones. It runs `setup()` first if you have not, so an - embedder that does not need the APIs in between can just call `start()`. -- **`stop()`** cancels the tasks and disconnects. +- **`build()`** opens every supervised `Connection`, walks the tree calling + `build()` on each controller, seals it, and builds their `ControllerAPI`s. It exists as + a separate step because anything serving the controllers has to register its + callbacks *before* the first values are read, or it misses them. +- **`start()`** runs `setup()` across the tree and the initial (`ONCE`) tasks, + then hands over to the supervisors, which run the periodic ones and a reconnect + loop per connection. It runs + `build()` first if you have not, so an embedder that does not need the APIs in + between can just call `start()`. +- **`stop()`** cancels the tasks and closes every connection, in reverse of the + order they were opened. **Idempotency is the caller's responsibility.** Starting a running runner, or stopping a stopped one, is not defined — an embedder whose own connect may run more than once has to keep track itself. -The runner also owns **reconnect**. A scan task whose callback raises marks its -controller disconnected and pauses rather than dying; the runner notices and -calls `Controller.reconnect()` until it comes back. This is deliberately not -left to each controller, so every controller recovers the same way. +**Reconnecting** belongs to each connection's `Supervisor`, not to the runner or a +controller: see [connections](./connections.md). Every call through a connection +handle passes its supervisor, which marks the link down when it fails and brings it +back at its own pace. This is deliberately not left to each controller, so every +connection recovers the same way and controllers sharing one recover together. + +**A fatal runner condition is observable rather than fatal to the process.** +`runner.fatal_error` is an `asyncio.Event` set when the runner cannot carry on — +a DRA device node that has gone away and will not come back, say — with +`runner.fatal_reason` carrying why. Nothing calls `sys.exit`, so an embedded +FastCS inside another process decides for itself what to do; `FastCS.serve` +raises it. ## Reading the structure: `ControllerAPI` diff --git a/docs/explanations/what-is-fastcs.md b/docs/explanations/what-is-fastcs.md index da10059e9..245c984d0 100644 --- a/docs/explanations/what-is-fastcs.md +++ b/docs/explanations/what-is-fastcs.md @@ -53,7 +53,7 @@ using `ControllerVector`. See [Controllers](controllers.md) for details. ## Writing a driver To create a driver, subclass `Controller`, declare attributes and commands as typed -class members, and implement lifecycle hooks (`connect`, `reconnect`, `disconnect`) and +class members, receive the connections it talks over as constructor arguments, and add periodic scan tasks. The [tutorials](../tutorials.md) walk through a complete example. ## Deploying as an application @@ -76,8 +76,10 @@ device logic does not require a running IOC or device server. **Multiple transports from a single driver** — the same controller can serve EPICS CA, EPICS PVA, Tango, REST, and GraphQL simultaneously or in any combination. -**Automatic reconnection** — if a scan task raises an exception, FastCS pauses all -tasks and calls `reconnect()` automatically, resuming once the connection is restored. +**Automatic reconnection** — every call into a connection passes its supervisor, which +notices when the link fails, pauses that connection's scans, flags its values as +invalid and reconnects it, resuming once it is restored. The driver writes no error +handling for it. **Auto-generated OPI screens** — EPICS transports generate CSS-Phoebus screen files from the controller's attribute metadata, with no additional effort from the driver diff --git a/docs/how-to/launch-framework.md b/docs/how-to/launch-framework.md index 7b9d811ee..281fa8d33 100644 --- a/docs/how-to/launch-framework.md +++ b/docs/how-to/launch-framework.md @@ -128,6 +128,69 @@ 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). +### Declaring connections + +A controller declares its connections as constructor arguments hinted with a +`Connection` type. Each one is configured under `connections:` in the controller's +entry, keyed by the argument name: + +```python +class MotorController(Controller): + connection: SerialConnection + + def __init__(self, motor: SerialConnection) -> None: + self.connection = motor + super().__init__() + + +launch(MotorController) +``` + +```yaml +# fastcs.yaml +controllers: + - id: PITCH + type: my_driver.MotorController + connections: + motor: + settings: + port: /dev/ttyS0 + - id: YAW + type: my_driver.MotorController + connections: + motor: + settings: + port: /dev/ttyS1 + reconnect_attempts: 5 + reconnect_period: 2.0 + +transport: + - epicsca: {} +``` + +The schema is generated from the type hints, so it checks everything: which names are +required, that nothing else is given, and each connection's own settings - the +arguments of its constructor. There is no `type:` under a connection, because the type +hint supplies it. `reconnect_attempts` and `reconnect_period` are framework settings, +read by the connection's supervisor rather than passed to the connection, and default +to 10 attempts one second apart. + +Each entry gets its own connections, so the two motors above get different links. +Before the controller is constructed, the launcher creates each connection from its +settings, gives it a `Supervisor`, and passes the supervisor's *handle* - a stand-in +that behaves like the connection - as the argument. Nothing is opened until the +application starts. See [connections](../explanations/connections.md) for what the +supervisor does with it. + +A connection that depends on another declares it on its class, not here - see +[connections](../explanations/connections.md). Dependencies resolve +within one entry, so sibling entries cannot share a connection or depend on each +other. A gateway device with several instruments behind one link is modelled as one +tree, with the gateway as the top-level controller. + +`connections` is a reserved name: an options type may not declare a field called +`connections`. + ## Schema Generation Generate JSON schema for the configuration yaml: @@ -204,7 +267,8 @@ FastCS: 0.12.0 The `launch()` function requires: -1. Controller `__init__` must have at most 2 arguments (including `self`) +1. Controller `__init__` may take at most one configuration argument, plus any + number of arguments hinted with a `Connection` type 2. If a configuration argument exists, it must have a type hint Using a dataclass or Pydantic model is recommended for the configuration type, as it enables JSON schema generation. Other type-hinted types will work, but will not produce a useful schema. @@ -225,6 +289,11 @@ class BadController(Controller): def __init__(self, settings): # Error: no type hint super().__init__() +# Valid - connections do not count towards the limit +class ConnectedController(Controller): + def __init__(self, motor: SerialConnection, settings: MySettings): + super().__init__() + # Invalid - too many arguments class TooManyArgs(Controller): def __init__(self, settings: MySettings, extra: str): # Error diff --git a/docs/how-to/update-attributes-from-device.md b/docs/how-to/update-attributes-from-device.md index f67dca33a..47391b209 100644 --- a/docs/how-to/update-attributes-from-device.md +++ b/docs/how-to/update-attributes-from-device.md @@ -165,8 +165,11 @@ class MultiChannelController(Controller): await channel.voltage.update(float(voltage)) ``` -The scan period (here `0.1` seconds) sets how often the batched query runs. Scans that -raise an exception will pause and wait for `reconnect()` to be called before resuming. +The scan period (here `0.1` seconds) sets how often the batched query runs. A scan that +raises is logged and tried again on the next period. If the failure was the connection +itself going down, the scan waits for the connection to come back up rather than +querying a dead link - the runner reopens it, and there is nothing for a driver to +call. See [](../explanations/connections.md). ### Scan as a cache for getters @@ -236,13 +239,13 @@ class SubscriptionController(Controller): super().__init__() self._client = subscription_client - async def connect(self): + async def setup(self): # Register an async callback that forwards updates into the attribute. + # `setup` runs once the whole tree is built and every connection is open. async def on_temperature_change(value: float) -> None: await self.temperature.update(value) await self._client.subscribe("temperature", on_temperature_change) - await super().connect() ``` If the library only supports synchronous callbacks, schedule the coroutine onto the diff --git a/docs/snippets/dynamic.py b/docs/snippets/dynamic.py index 4e7c00e72..7f7b56f03 100644 --- a/docs/snippets/dynamic.py +++ b/docs/snippets/dynamic.py @@ -4,7 +4,7 @@ from pydantic import BaseModel, ConfigDict, ValidationError from fastcs.attributes import Attribute, AttrR, AttrRW -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.datatypes import DType from fastcs.launch import FastCS @@ -80,17 +80,23 @@ async def setter(value, command=command, dtype=datatype): class TemperatureRampController(Controller): + connection: IPConnection + def __init__( self, index: int, parameters: dict[str, TemperatureControllerParameter], protocol: TemperatureProtocol, + connection: IPConnection, ): self._parameters = parameters self._protocol = protocol + # The same connection the parent holds, so this controller's polled + # attributes pause with it while it is down. + self.connection = connection super().__init__(f"Ramp{index}") - async def initialise(self): + async def build(self): for name, attribute in create_attributes( self._parameters, self._protocol ).items(): @@ -98,20 +104,20 @@ async def initialise(self): class TemperatureController(Controller): - def __init__(self, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) - - super().__init__() + connection: IPConnection - async def connect(self): - await self._connection.connect(self._ip_settings) + def __init__(self, connection: IPConnection): + # Opening it, and reopening it after a failure, is its supervisor's job. + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) - async def initialise(self): - await self.connect() + super().__init__() - api = json.loads((await self._connection.send_query("API?\r\n")).strip("\r\n")) + async def build(self): + # Runs with the connection already open. The ramp controllers added here get + # their own `build` called by the runner on a later pass, so there is no + # need - and no way - to drive their lifecycle from this one. + api = json.loads((await self.connection.send_query("API?\r\n")).strip("\r\n")) ramps_api = api.pop("Ramps") @@ -120,19 +126,19 @@ async def initialise(self): for idx, ramp_parameters in enumerate(ramps_api): ramp_controller = TemperatureRampController( - idx + 1, ramp_parameters, self._protocol + idx + 1, ramp_parameters, self._protocol, self.connection ) - await ramp_controller.initialise() self.add_sub_controller(f"Ramp{idx + 1:02d}", ramp_controller) - await self._connection.close() - epics_ca = EpicsCATransport() connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": diff --git a/docs/snippets/static06.py b/docs/snippets/static06.py index 98af3b41b..963aaac24 100644 --- a/docs/snippets/static06.py +++ b/docs/snippets/static06.py @@ -1,7 +1,7 @@ from pathlib import Path from fastcs.attributes import AttrR -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.transports.epics import EpicsGUIOptions @@ -9,24 +9,25 @@ class TemperatureController(Controller): + connection: IPConnection + device_id: AttrR[str] - def __init__(self, settings: IPConnectionSettings): + def __init__(self, connection: IPConnection): super().__init__() - self._ip_settings = settings - self._connection = IPConnection() - - async def connect(self): - await self._connection.connect(self._ip_settings) + self.connection = connection gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": diff --git a/docs/snippets/static07.py b/docs/snippets/static07.py index 2aea3bb76..62241c6e1 100644 --- a/docs/snippets/static07.py +++ b/docs/snippets/static07.py @@ -1,7 +1,7 @@ from pathlib import Path from fastcs.attributes import AttrR, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.transports.epics import EpicsGUIOptions @@ -9,28 +9,29 @@ class TemperatureController(Controller): - def __init__(self, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() + connection: IPConnection + + def __init__(self, connection: IPConnection): + self.connection = connection super().__init__() self.device_id = AttrR(str, getter=Polled(self._get_device_id, period=0.2)) async def _get_device_id(self) -> str: - response = await self._connection.send_query("ID?\r\n") + response = await self.connection.send_query("ID?\r\n") return response.strip("\r\n") - async def connect(self): - await self._connection.connect(self._ip_settings) - gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static08.py b/docs/snippets/static08.py index 95363380b..8a7047c9c 100644 --- a/docs/snippets/static08.py +++ b/docs/snippets/static08.py @@ -2,7 +2,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.transports.epics import EpicsGUIOptions @@ -27,10 +27,11 @@ async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: class TemperatureController(Controller): - def __init__(self, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -43,16 +44,16 @@ async def _get_device_id(self) -> str: async def _get_power(self) -> float: return await self._protocol.send_query("P", float) - async def connect(self): - await self._connection.connect(self._ip_settings) - gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static09.py b/docs/snippets/static09.py index 10c07b334..35c4dc990 100644 --- a/docs/snippets/static09.py +++ b/docs/snippets/static09.py @@ -2,7 +2,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.transports.epics import EpicsGUIOptions @@ -27,10 +27,11 @@ async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: class TemperatureController(Controller): - def __init__(self, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -54,16 +55,16 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static10.py b/docs/snippets/static10.py index e6ea3292d..72d3786f6 100644 --- a/docs/snippets/static10.py +++ b/docs/snippets/static10.py @@ -2,7 +2,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.transports.epics import EpicsGUIOptions @@ -27,8 +27,11 @@ async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: class TemperatureRampController(Controller): + connection: IPConnection + def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" + self.connection = connection self._protocol = TemperatureProtocol(connection, suffix) super().__init__(f"Ramp{suffix}") @@ -53,10 +56,11 @@ async def _set_end(self, value: int) -> None: class TemperatureController(Controller): - def __init__(self, ramp_count: int, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, ramp_count: int, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -70,7 +74,7 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + controller = TemperatureRampController(index, self.connection) self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) @@ -86,16 +90,16 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(4, supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static11.py b/docs/snippets/static11.py index 222d0cdda..0a615f993 100644 --- a/docs/snippets/static11.py +++ b/docs/snippets/static11.py @@ -3,7 +3,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.transports.epics import EpicsGUIOptions @@ -33,8 +33,11 @@ class OnOffEnum(enum.StrEnum): class TemperatureRampController(Controller): + connection: IPConnection + def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" + self.connection = connection self._protocol = TemperatureProtocol(connection, suffix) super().__init__(f"Ramp{suffix}") @@ -70,10 +73,11 @@ async def _set_enabled(self, value: OnOffEnum) -> None: class TemperatureController(Controller): - def __init__(self, ramp_count: int, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, ramp_count: int, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -87,7 +91,7 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + controller = TemperatureRampController(index, self.connection) self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) @@ -103,16 +107,16 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(4, supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static12.py b/docs/snippets/static12.py index db7ced42c..6e5f960b0 100644 --- a/docs/snippets/static12.py +++ b/docs/snippets/static12.py @@ -4,7 +4,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.methods import scan @@ -35,8 +35,11 @@ class OnOffEnum(enum.StrEnum): class TemperatureRampController(Controller): + connection: IPConnection + def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" + self.connection = connection self._protocol = TemperatureProtocol(connection, suffix) super().__init__(f"Ramp{suffix}") @@ -81,10 +84,11 @@ async def _get_actual(self) -> float: class TemperatureController(Controller): - def __init__(self, ramp_count: int, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, ramp_count: int, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -98,7 +102,7 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + controller = TemperatureRampController(index, self.connection) self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) @@ -114,13 +118,10 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - @scan(0.1) async def update_voltages(self): voltages = json.loads( - (await self._connection.send_query("V?\r\n")).strip("\r\n") + (await self.connection.send_query("V?\r\n")).strip("\r\n") ) for index, controller in enumerate(self._ramp_controllers): await controller.voltage.update(float(voltages[index])) @@ -129,9 +130,12 @@ async def update_voltages(self): gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(4, supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static13.py b/docs/snippets/static13.py index 420acd994..d9fa758c6 100644 --- a/docs/snippets/static13.py +++ b/docs/snippets/static13.py @@ -5,7 +5,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.methods import command, scan @@ -36,8 +36,11 @@ class OnOffEnum(enum.StrEnum): class TemperatureRampController(Controller): + connection: IPConnection + def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" + self.connection = connection self._protocol = TemperatureProtocol(connection, suffix) super().__init__(f"Ramp{suffix}") @@ -82,10 +85,11 @@ async def _get_actual(self) -> float: class TemperatureController(Controller): - def __init__(self, ramp_count: int, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, ramp_count: int, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -99,7 +103,7 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + controller = TemperatureRampController(index, self.connection) self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) @@ -115,13 +119,10 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - @scan(0.1) async def update_voltages(self): voltages = json.loads( - (await self._connection.send_query("V?\r\n")).strip("\r\n") + (await self.connection.send_query("V?\r\n")).strip("\r\n") ) for index, controller in enumerate(self._ramp_controllers): await controller.voltage.update(float(voltages[index])) @@ -137,9 +138,12 @@ async def disable_all(self) -> None: gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Temperature Controller") epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) -controller = TemperatureController(4, connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(4, supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static14.py b/docs/snippets/static14.py index 9e25a6418..d8cf1b618 100644 --- a/docs/snippets/static14.py +++ b/docs/snippets/static14.py @@ -5,7 +5,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.logging import configure_logging, logger @@ -40,8 +40,11 @@ class OnOffEnum(enum.StrEnum): class TemperatureRampController(Controller): + connection: IPConnection + def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" + self.connection = connection self._protocol = TemperatureProtocol(connection, suffix) super().__init__(f"Ramp{suffix}") @@ -86,10 +89,11 @@ async def _get_actual(self) -> float: class TemperatureController(Controller): - def __init__(self, ramp_count: int, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, ramp_count: int, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -103,7 +107,7 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + controller = TemperatureRampController(index, self.connection) self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) @@ -119,13 +123,10 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - @scan(0.1) async def update_voltages(self): voltages = json.loads( - (await self._connection.send_query("V?\r\n")).strip("\r\n") + (await self.connection.send_query("V?\r\n")).strip("\r\n") ) for index, controller in enumerate(self._ramp_controllers): await controller.voltage.update(float(voltages[index])) @@ -145,9 +146,12 @@ async def disable_all(self) -> None: epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) -controller = TemperatureController(4, connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(4, supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/snippets/static15.py b/docs/snippets/static15.py index ac1a1d0d9..c450da45b 100644 --- a/docs/snippets/static15.py +++ b/docs/snippets/static15.py @@ -5,7 +5,7 @@ from typing import TypeVar from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings, Supervisor from fastcs.controllers import Controller from fastcs.launch import FastCS from fastcs.logging import LogLevel, configure_logging, logger @@ -48,8 +48,11 @@ class OnOffEnum(enum.StrEnum): class TemperatureRampController(Controller): + connection: IPConnection + def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" + self.connection = connection self._protocol = TemperatureProtocol(connection, suffix) super().__init__(f"Ramp{suffix}") @@ -94,10 +97,11 @@ async def _get_actual(self) -> float: class TemperatureController(Controller): - def __init__(self, ramp_count: int, settings: IPConnectionSettings): - self._ip_settings = settings - self._connection = IPConnection() - self._protocol = TemperatureProtocol(self._connection) + connection: IPConnection + + def __init__(self, ramp_count: int, connection: IPConnection): + self.connection = connection + self._protocol = TemperatureProtocol(self.connection) super().__init__() @@ -111,7 +115,7 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): - controller = TemperatureRampController(index, self._connection) + controller = TemperatureRampController(index, self.connection) self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) @@ -127,13 +131,10 @@ async def _get_ramp_rate(self) -> float: async def _set_ramp_rate(self, value: float) -> None: await self._protocol.send_command("R", value, float) - async def connect(self): - await self._connection.connect(self._ip_settings) - @scan(0.1) async def update_voltages(self): voltages = json.loads( - (await self._connection.send_query("V?\r\n")).strip("\r\n") + (await self.connection.send_query("V?\r\n")).strip("\r\n") ) for index, controller in enumerate(self._ramp_controllers): await controller.voltage.update(float(voltages[index])) @@ -153,9 +154,12 @@ async def disable_all(self) -> None: epics_ca = EpicsCATransport(gui=gui_options) connection_settings = IPConnectionSettings("localhost", 25565) logger.info("Configuring connection settings", connection_settings=connection_settings) -controller = TemperatureController(4, connection_settings) +supervisor = Supervisor(IPConnection(connection_settings), name="temperature") +controller = TemperatureController(4, supervisor.handle) controller.set_path(["DEMO"]) -fastcs = FastCS(controller, [epics_ca]) +# The supervisor opens the connection, reconnects it if it drops, and hands the +# controller a handle that behaves like the IPConnection itself. +fastcs = FastCS(controller, [epics_ca], supervisors=[supervisor]) if __name__ == "__main__": fastcs.run() diff --git a/docs/tutorials/dynamic-drivers.md b/docs/tutorials/dynamic-drivers.md index ae55dc608..84a6552b8 100644 --- a/docs/tutorials/dynamic-drivers.md +++ b/docs/tutorials/dynamic-drivers.md @@ -31,14 +31,14 @@ or a description for the parameter. ## FastCS Initialisation -Specific `Controller` classes can optionally implement an async `initialise` method to +Specific `Controller` classes can optionally implement an async `build` method to perform any start up logic. The intention here is that the `__init__` method should be -minimal and the `initialise` method performs any long running calls, such as querying an +minimal and the `build` method performs any long running calls, such as querying an API, allowing FastCS to run these concurrently to reduce start times. Take the driver implementation from the previous tutorial and remove the statically defined `Attributes` and creation of sub controllers in `__init__`. Then -implement an `initialise` method to create these dynamically instead. +implement a `build` method to create these dynamically instead. Create a pydantic model to validate the response from the device @@ -56,12 +56,12 @@ construction time just like statically-declared ones do. :lines: 50-79 ::: -Update the controllers to not define attributes statically and implement initialise +Update the controllers to not define attributes statically and implement build methods to create these attributes dynamically, passing the shared `TemperatureProtocol` down to `create_attributes` so the dynamically-created getters/setters can use it. :::{literalinclude} /snippets/dynamic.py -:lines: 82-128 +:lines: 82-131 ::: TODO: Add `enabled` back in to `TemperatureRampController` and recreate `disable_all` to diff --git a/docs/tutorials/static-drivers.md b/docs/tutorials/static-drivers.md index 78ecf9eb5..d88b24df2 100644 --- a/docs/tutorials/static-drivers.md +++ b/docs/tutorials/static-drivers.md @@ -150,10 +150,24 @@ and set values. This is implemented with plain `getter`/`setter` callables passe its own getter/setter logic and connection, but there are some built in connection options. -Update the controller to create an `IPConnection` to communicate with the simulator over -TCP and implement a `connect` method that establishes the connection. The `connect` -method is called by the FastCS application at the appropriate time during start up to -ensure the connection is established before it is used. +Update the controller to take an `IPConnection`, which communicates with the simulator +over TCP, and store it. A controller is given its connection rather than creating it: a +driver never opens, closes or reconnects the connection itself. Declaring +`connection: IPConnection` on the class narrows the base class's connection so this +controller's own code can call `IPConnection`'s methods. + +The connection is created alongside the controller, and handed to a `Supervisor`, which +opens it at the appropriate time during start up, before anything uses it, and reopens it +if it drops. The controller is given the supervisor's *handle*: it behaves exactly like +the `IPConnection`, but every call through it passes the supervisor, which is how FastCS +knows when the link has gone - so the driver needs no error handling of its own. The +supervisor is handed to `FastCS` too, so the framework can open every connection before +it builds the controller tree. + +When an application is started from a `fastcs.yaml` with `launch`, all of this is done +for you: the connection's settings go in the entry's `connections:` block, under the name +of the controller's constructor argument (see +[the launch framework](../how-to/launch-framework.md)). :::{note} The simulator control connection is on port 25565. @@ -163,7 +177,7 @@ The simulator control connection is on port 25565. :class: dropdown, hint :::{literalinclude} /snippets/static06.py -:emphasize-lines: 4,15-22,27-28 +:emphasize-lines: 4,12,16,19,25-26,30 ::: :::: @@ -189,7 +203,7 @@ Passing the getter bare, as here, means it is called once at start up. Wrap it i :class: dropdown, hint :::{literalinclude} /snippets/static07.py -:emphasize-lines: 13-19,21-23 +:emphasize-lines: 12,15,19,21-23 ::: :::: @@ -232,7 +246,7 @@ constructor to perform the cast. :class: dropdown, hint :::{literalinclude} /snippets/static08.py -:emphasize-lines: 12,15-27,34,38-39,41-45 +:emphasize-lines: 12,15-27,35,39-40,42-46 ::: :::: @@ -256,7 +270,7 @@ The set commands do not return a response, so the setter uses `send_command` ins :class: dropdown, hint :::{literalinclude} /snippets/static09.py -:emphasize-lines: 4,40-45,53-57 +:emphasize-lines: 4,41-46,54-57 ::: :::: @@ -297,11 +311,15 @@ Create a `TemperatureRampController` with two `AttrRW`s for the ramp start and e to define how many ramps there are, which is used to register the correct number of ramp controllers with the parent. +Each ramp is given the *same* connection as its parent rather than one of its own, so +the whole tree has one supervisor between it: when the link drops, every ramp's polling +pauses with it and they all resume together. + ::::{admonition} Code 10 :class: dropdown, hint :::{literalinclude} /snippets/static10.py -:emphasize-lines: 30-53,57,73-77 +:emphasize-lines: 30,32-56,75-79 ::: :::: @@ -326,7 +344,7 @@ Add an `AttrRW` to the `TemperatureRampController`s with an `Enum` type, using a :class: dropdown, hint :::{literalinclude} /snippets/static11.py -:emphasize-lines: 1,31-33,48-53,67-71 +:emphasize-lines: 1,31-33,51-56,70-74 ::: :::: @@ -381,7 +399,7 @@ above. :class: dropdown, hint :::{literalinclude} /snippets/static12.py -:emphasize-lines: 11,56-58,78-82,123-129 +:emphasize-lines: 11,59-61,81-85,121-127 ::: :::: @@ -402,7 +420,7 @@ controller by calling `set` on each `enabled` attribute. :class: dropdown, hint :::{literalinclude} /snippets/static13.py -:emphasize-lines: 1,132-137 +:emphasize-lines: 1,133-138 ::: :::: @@ -434,7 +452,7 @@ inside `TemperatureProtocol.send_command` to log the commands it sends. :class: dropdown, hint :::{literalinclude} /snippets/static14.py -:emphasize-lines: 12,28,145,150 +:emphasize-lines: 12,28,146,154 ::: :::: @@ -462,7 +480,7 @@ is enabled the messages are visible. :class: dropdown, hint :::{literalinclude} /snippets/static15.py -:emphasize-lines: 12,14,21,34-36,41,125,153 +:emphasize-lines: 12,14,21,34-36,41,129,154 ::: :::: diff --git a/src/fastcs/attributes/attr_r.py b/src/fastcs/attributes/attr_r.py index 554cb7d19..7829279c1 100644 --- a/src/fastcs/attributes/attr_r.py +++ b/src/fastcs/attributes/attr_r.py @@ -379,7 +379,7 @@ async def update(self, value: DType_T | Update[DType_T]) -> None: self.log_event("Attribute set", value=repr(value), attribute=self) - _previous_value = self._value + _previous_value, _previous_severity = self._value, self._severity try: self._value = self.validate(value) except ValueError: @@ -397,10 +397,14 @@ async def update(self, value: DType_T | Update[DType_T]) -> None: } if self._readback_callbacks is not None: + # A value flagged invalid, or cleared again, is news even if the value + # itself has not changed. + changed = ( + not self.equal(self._value, _previous_value) + or self._severity is not _previous_severity + ) callbacks_to_call: list[AttrReadbackCallback[DType_T]] = [ - cb - for cb, always in self._readback_callbacks - if always or not self.equal(self._value, _previous_value) + cb for cb, always in self._readback_callbacks if always or changed ] try: await asyncio.gather(*[cb(self._value) for cb in callbacks_to_call]) diff --git a/src/fastcs/attributes/attr_rw.py b/src/fastcs/attributes/attr_rw.py index 5134b8bda..5fdb65954 100644 --- a/src/fastcs/attributes/attr_rw.py +++ b/src/fastcs/attributes/attr_rw.py @@ -23,6 +23,7 @@ Table, TableMeta, ) +from fastcs.exceptions import DisconnectedError from fastcs.logging import logger from fastcs.util import Controller_T @@ -242,6 +243,10 @@ async def set(self, value: DType_T) -> None: else: try: result = await self._setter(self._setpoint) + except DisconnectedError: + # Back to the client: the link is down, and its supervisor has + # already said so. + raise except Exception as e: logger.opt(exception=e).error( "Set failed", attribute=self, setpoint=self._setpoint diff --git a/src/fastcs/attributes/attr_w.py b/src/fastcs/attributes/attr_w.py index b0282f02c..fee46f513 100644 --- a/src/fastcs/attributes/attr_w.py +++ b/src/fastcs/attributes/attr_w.py @@ -22,6 +22,7 @@ Table, TableMeta, ) +from fastcs.exceptions import DisconnectedError from fastcs.logging import logger from fastcs.util import Controller_T @@ -231,6 +232,10 @@ async def set(self, value: DType_T) -> None: if self._setter is not None: try: result = await self._setter(self._setpoint) + except DisconnectedError: + # Back to the client: the link is down, and its supervisor has + # already said so. + raise except Exception as e: logger.opt(exception=e).error( "Set failed", attribute=self, setpoint=self._setpoint diff --git a/src/fastcs/connections/__init__.py b/src/fastcs/connections/__init__.py index 5001409a1..d87c88c3c 100644 --- a/src/fastcs/connections/__init__.py +++ b/src/fastcs/connections/__init__.py @@ -1,5 +1,16 @@ +from fastcs.exceptions import DisconnectedError as DisconnectedError + +from .connection import Connection as Connection +from .http_connection import HTTPConnection as HTTPConnection +from .http_connection import HTTPConnectionSettings as HTTPConnectionSettings from .ip_connection import IPConnection as IPConnection from .ip_connection import IPConnectionSettings as IPConnectionSettings from .ip_connection import StreamConnection as StreamConnection +from .policy import ConnectionPolicy as ConnectionPolicy +from .policy import DRAPolicy as DRAPolicy from .serial_connection import SerialConnection as SerialConnection from .serial_connection import SerialConnectionSettings as SerialConnectionSettings +from .sim_connection import SimConnection as SimConnection +from .supervisor import DEFAULT_RECONNECT_ATTEMPTS as DEFAULT_RECONNECT_ATTEMPTS +from .supervisor import DEFAULT_RECONNECT_PERIOD as DEFAULT_RECONNECT_PERIOD +from .supervisor import Supervisor as Supervisor diff --git a/src/fastcs/connections/connection.py b/src/fastcs/connections/connection.py new file mode 100644 index 000000000..b82fee80f --- /dev/null +++ b/src/fastcs/connections/connection.py @@ -0,0 +1,88 @@ +from __future__ import annotations + +from abc import ABC, abstractmethod +from collections.abc import Sequence +from typing import ClassVar + +from fastcs.connections.policy import ConnectionPolicy + + +class Connection(ABC): + """A link to hardware: a constructor, ``connect``, ``close`` and IO methods. + + A connection holds no health state and has no hooks. Its constructor arguments + are its device settings - they become its block in ``fastcs.yaml`` - and its IO + methods just talk to the device, raising when something goes wrong:: + + class EigerConnection(Connection): + def __init__(self, port: int) -> None: + self._port = port + + async def connect(self) -> None: ... + async def close(self) -> None: ... + + async def get(self, path: str) -> Any: ... + + Everything ongoing about the link - whether it is up, reconnecting, the scans + that use it - belongs to its `Supervisor`. A controller never holds the + connection itself but a *handle*: a stand-in that looks like the connection and + routes every call through the supervisor, which fails fast while the link is + down and tells a dead link from a device complaint by the exception raised (see + `ConnectionPolicy`). So IO methods need no ``try``/``except`` of their own. + + Calls a connection makes to its own methods do not pass through the handle, so + each call from a controller is counted exactly once. + + The framework keys per-connection state by identity, so a ``Connection`` must + never define ``__eq__``: two sockets with matching settings are two + connections, and an ``__eq__`` would silently collapse them. + """ + + depends_on: ClassVar[Sequence[type[Connection]]] = () + """Connection types this one needs before it can work. + + Read as: wait for every connection of these types under the same top-level + controller. Connections are opened in that order at startup, and a reconnect + waits for them to be up - and stalls if one of them gives up. A driver fact, + declared once here rather than in every deployment:: + + class OdinConnection(HTTPConnection): + depends_on = [EigerConnection] + """ + + policy: ClassVar[ConnectionPolicy] = ConnectionPolicy() + """Which failures mean the link is gone, and what to do when it cannot recover. + + A class attribute, not a constructor argument: a constructor argument would + appear in every connection's config schema. Policies are frozen, so the default + instance is shared. + """ + + @abstractmethod + async def connect(self) -> None: + """Open the link, or raise. + + This means "make the link usable", not merely "open the socket" - a device + that needs a mode set every time it comes back has that write here, rather + than in a controller's ``setup``, which runs once only. + """ + + @abstractmethod + async def close(self) -> None: + """Close the link. Called at shutdown and before every reconnect attempt. + + Must tolerate being called on a link that is already closed. + """ + + @property + def label(self) -> str: + """What to call this connection's device in a failure message. + + The device node or address where a connection knows one, and the class + name otherwise. Distinct from the name its supervisor logs, which comes + from config rather than from the device. + """ + return type(self).__name__ + + def __repr__(self) -> str: + return f"{type(self).__name__}({self.label})" diff --git a/src/fastcs/connections/http_connection.py b/src/fastcs/connections/http_connection.py new file mode 100644 index 000000000..0d8e400aa --- /dev/null +++ b/src/fastcs/connections/http_connection.py @@ -0,0 +1,102 @@ +from dataclasses import dataclass, field +from typing import Any + +from httpx import AsyncBaseTransport, AsyncClient, Response + +from fastcs.connections.connection import Connection +from fastcs.exceptions import DisconnectedError + + +@dataclass +class HTTPConnectionSettings: + host: str = "127.0.0.1" + port: int = 80 + scheme: str = "http" + timeout: float = 10.0 + headers: dict[str, str] = field(default_factory=dict) + + @property + def base_url(self) -> str: + return f"{self.scheme}://{self.host}:{self.port}" + + +class HTTPConnection(Connection): + """An HTTP connection. + + The settings are given at construction rather than to ``connect``, because the + framework opens and reopens the link without knowing anything about it. A + transport failure is raised as httpx raises it, and the default + `ConnectionPolicy` reads it as the link going down; an error status is the + device answering, and is passed back to the caller. + + One framework class rather than one per driver: every REST device does the same + few things, and a driver needing a verb or a response shape this does not cover + uses `request` rather than rolling its own client. + + Args: + settings: Where to connect to + + """ + + def __init__(self, settings: HTTPConnectionSettings | None = None) -> None: + self._settings = settings or HTTPConnectionSettings() + + self._transport: AsyncBaseTransport | None = None + """Set by a subclass to talk to an in-process ASGI app instead of a socket. + + Not a constructor argument: a connection's ``__init__`` signature is its + config schema, and a transport object is not config. It is how a driver + tests against its own simulator without a network. + """ + + self.__client: AsyncClient | None = None + + @property + def label(self) -> str: + return self._settings.base_url + + @property + def _client(self) -> AsyncClient: + if self.__client is None: + raise DisconnectedError( + "Need to call connect() before using HTTPConnection." + ) + + return self.__client + + async def connect(self) -> None: + self.__client = AsyncClient( + base_url=self._settings.base_url, + timeout=self._settings.timeout, + headers=self._settings.headers, + transport=self._transport, + ) + + async def close(self) -> None: + if self.__client is None: + return + + await self.__client.aclose() + self.__client = None + + async def get(self, path: str) -> Any: + """GET, returning the parsed JSON body.""" + return (await self.request("GET", path)).json() + + async def get_bytes(self, path: str) -> bytes: + """GET, returning the raw body. For frame and file data.""" + return (await self.request("GET", path)).content + + async def put(self, path: str, value: Any) -> Any: + """PUT a JSON value, returning the parsed JSON body if there is one.""" + response = await self.request("PUT", path, json=value) + return response.json() if response.content else None + + async def request(self, method: str, path: str, **kwargs) -> Response: + """Every request goes through here, so `get` and `put` behave alike.""" + response = await self._client.request(method, path, **kwargs) + + # A 4xx or 5xx is a device complaint - an `httpx.HTTPStatusError`, which the + # policy passes back to the caller without touching connection health. + response.raise_for_status() + return response diff --git a/src/fastcs/connections/ip_connection.py b/src/fastcs/connections/ip_connection.py index f021aa913..421fab160 100644 --- a/src/fastcs/connections/ip_connection.py +++ b/src/fastcs/connections/ip_connection.py @@ -1,15 +1,11 @@ import asyncio from dataclasses import dataclass +from fastcs.connections.connection import Connection +from fastcs.exceptions import DisconnectedError from fastcs.tracer import Tracer -class DisconnectedError(Exception): - """Raised if the ip connection is disconnected.""" - - pass - - @dataclass class IPConnectionSettings: ip: str = "127.0.0.1" @@ -46,12 +42,25 @@ async def close(self): await self.writer.wait_closed() -class IPConnection(Tracer): - """For connecting to an ip using a `StreamConnection`.""" +class IPConnection(Connection, Tracer): + """For connecting to an ip using a `StreamConnection`. + + The settings are given at construction rather than to ``connect``, because the + framework opens and reopens the link without knowing anything about it. + + Args: + settings: Where to connect to - def __init__(self): - super().__init__() - self.__connection = None + """ + + def __init__(self, settings: IPConnectionSettings | None = None) -> None: + Tracer.__init__(self) + self._settings = settings or IPConnectionSettings() + self.__connection: StreamConnection | None = None + + @property + def label(self) -> str: + return f"{self._settings.ip}:{self._settings.port}" @property def _connection(self) -> StreamConnection: @@ -60,8 +69,10 @@ def _connection(self) -> StreamConnection: return self.__connection - async def connect(self, settings: IPConnectionSettings): - reader, writer = await asyncio.open_connection(settings.ip, settings.port) + async def connect(self) -> None: + reader, writer = await asyncio.open_connection( + self._settings.ip, self._settings.port + ) self.__connection = StreamConnection(reader, writer) async def send_command(self, message: str) -> None: @@ -72,6 +83,16 @@ async def send_query(self, message: str) -> str: async with self._connection as connection: await connection.send_message(message) response = await connection.receive_response() + if not response: + # ``readline`` returns b"" at EOF, so a peer that closed the socket + # rather than answering looks like an empty reply. It is a dead + # link, and nothing else here would notice: the caller would get "" + # and fail to parse it, over and over, while the reconnect loop + # stayed idle. + raise DisconnectedError( + "Connection closed by peer while awaiting a response" + ) + self.log_event( "Received query response", query=message.strip(), @@ -79,7 +100,7 @@ async def send_query(self, message: str) -> str: ) return response - async def close(self): + async def close(self) -> None: if self.__connection is None: return diff --git a/src/fastcs/connections/policy.py b/src/fastcs/connections/policy.py new file mode 100644 index 000000000..e3be5a23b --- /dev/null +++ b/src/fastcs/connections/policy.py @@ -0,0 +1,134 @@ +"""How a connection's failures are read, and what to do about them. + +A policy a connection holds, rather than a base class it inherits: the transport +and what to do when it fails are chosen separately, so one policy serves any +connection and a connection can be given any policy. + +A policy is a set of independent settings rather than one class per behaviour, so +that a slow DRA device is `DRAPolicy` with a different timeout count, not a +``DRASlowPolicy``. +""" + +from __future__ import annotations + +import enum +from dataclasses import dataclass +from typing import TYPE_CHECKING + +import httpx + +from fastcs.exceptions import DisconnectedError + +if TYPE_CHECKING: + from fastcs.connections.connection import Connection + + +DEFAULT_TIMEOUT_COUNT = 3 +"""Consecutive timeouts that mean the link is gone, rather than the device slow.""" + +DEFAULT_TIMEOUT_ERRORS: tuple[type[BaseException], ...] = ( + TimeoutError, + httpx.TimeoutException, +) +"""A call that took too long. One is not a dead link; several in a row are.""" + +DEFAULT_DISCONNECTION_ERRORS: tuple[type[BaseException], ...] = ( + # Connection reset, broken pipe, refused, unreachable, a vanished device node, + # a serial port that went away - every one an OSError. + OSError, + # EOF, or a read cut short (asyncio.IncompleteReadError is an EOFError) + EOFError, + # Anything httpx could not complete at the transport level. Not an HTTP error + # status: that is the device answering. + httpx.TransportError, +) +"""What the link failing looks like, as opposed to the device saying no.""" + + +class Failure(enum.Enum): + """What an exception out of a connection's IO means.""" + + DISCONNECTED = "disconnected" + """The link is gone.""" + + TIMEOUT = "timeout" + """The call took too long. Disconnected once enough of them happen in a row.""" + + DEVICE = "device" + """The device answered, and said no. Passed back to the caller unchanged.""" + + +@dataclass(frozen=True) +class ConnectionPolicy: + """Keep retrying, and classify failures by the default rules. + + Assign one to a `Connection` class, or pass one to a `Supervisor`, to change + how its failures are read or what happens when it cannot recover. Frozen, so + one instance can be shared between any number of connections. + """ + + disconnection_errors: tuple[type[BaseException], ...] = DEFAULT_DISCONNECTION_ERRORS + """Exceptions from connection IO that mean the link is gone.""" + + timeout_errors: tuple[type[BaseException], ...] = DEFAULT_TIMEOUT_ERRORS + """Exceptions from connection IO that mean the call took too long. + + Checked before `disconnection_errors`, since a `TimeoutError` is an `OSError`. + """ + + timeout_count: int = DEFAULT_TIMEOUT_COUNT + """Consecutive timeouts that mark the link down.""" + + terminal_errors: tuple[type[BaseException], ...] = () + """Failures of `Connection.connect` that can never succeed in this process. + + A reconnect that fails with one of these gives up at once, rather than + spending the rest of its attempts on retries that cannot succeed. + """ + + fatal: bool = False + """Whether a terminal failure should bring the application down. + + A connection that has given up stalls its dependents and serves stale values + forever. When only a restart can fix the cause, saying so and exiting is + better than sitting there looking healthy. + """ + + def classify(self, exc: BaseException) -> Failure: + """What an exception raised by a connection's IO means.""" + if isinstance(exc, DisconnectedError): + # Raised on purpose, by a driver that recognised its device going away. + return Failure.DISCONNECTED + if isinstance(exc, self.timeout_errors): + return Failure.TIMEOUT + if isinstance(exc, self.disconnection_errors): + return Failure.DISCONNECTED + return Failure.DEVICE + + def is_terminal(self, exc: BaseException) -> bool: + """Whether a failed `Connection.connect` can never succeed in this process.""" + return isinstance(exc, self.terminal_errors) + + def reason(self, connection: Connection) -> str: + """Why it cannot recover, for the log line that ends the retry loop.""" + return f"{connection.label} cannot recover from this failure." + + +@dataclass(frozen=True) +class DRAPolicy(ConnectionPolicy): + """A device node injected by a Kubernetes DRA claim. + + The claim is established when the pod starts, so a node that has gone will + not reappear in it. Retrying cannot help and a pod restart can, so a missing + node is both terminal and fatal. + """ + + terminal_errors: tuple[type[BaseException], ...] = (FileNotFoundError,) + fatal: bool = True + + def reason(self, connection: Connection) -> str: + return ( + f"Device node {connection.label} has gone away. It comes from a " + "Kubernetes DRA claim and will not reappear in this pod. Restart " + "the pod to re-establish the claim." + ) diff --git a/src/fastcs/connections/serial_connection.py b/src/fastcs/connections/serial_connection.py index 65bbf6801..42d22faac 100644 --- a/src/fastcs/connections/serial_connection.py +++ b/src/fastcs/connections/serial_connection.py @@ -3,6 +3,8 @@ import aioserial +from fastcs.connections.connection import Connection + class NotOpenedError(Exception): """If the serial stream is not opened.""" @@ -16,15 +18,30 @@ class SerialConnectionSettings: baud: int = 115200 -class SerialConnection: - """A serial connection.""" +class SerialConnection(Connection): + """A serial connection. + + The settings are given at construction rather than to ``connect``, because the + framework opens and reopens the link without knowing anything about it. + + Args: + settings: Which port to open, and at what baud rate + + """ - def __init__(self): - self.stream = None + def __init__(self, settings: SerialConnectionSettings) -> None: + self._settings = settings self._lock = asyncio.Lock() + self.__stream: aioserial.AioSerial | None = None - async def connect(self, settings: SerialConnectionSettings) -> None: - self.__stream = aioserial.AioSerial(port=settings.port, baudrate=settings.baud) + @property + def label(self) -> str: + return self._settings.port + + async def connect(self) -> None: + self.__stream = aioserial.AioSerial( + port=self._settings.port, baudrate=self._settings.baud + ) @property def _stream(self) -> aioserial.AioSerial: @@ -52,5 +69,8 @@ async def _receive_response(self, size): async def close(self) -> None: async with self._lock: - self._stream.close() + if self.__stream is None: + return + + self.__stream.close() self.__stream = None diff --git a/src/fastcs/connections/sim_connection.py b/src/fastcs/connections/sim_connection.py new file mode 100644 index 000000000..2e4ff9b3b --- /dev/null +++ b/src/fastcs/connections/sim_connection.py @@ -0,0 +1,33 @@ +from fastcs.connections.connection import Connection +from fastcs.logging import logger + + +class SimConnection(Connection): + """Base for simulated connections. + + Opens and closes trivially and can never fail, so it is connected for the life of + the process and its reconnect loop idles forever. Subclasses implement whatever IO + methods their driver calls - there is no transport that can go away. + + A sibling of the real transports rather than a subclass of one: a simulator that + inherits `IPConnection` also inherits a stream handle it will never open and a + property that raises. Inheriting this instead means the only thing a driver + writes is the pretending:: + + class SimSerialConnection(SimConnection): + def __init__(self) -> None: + self._position = 0 + + async def send_query(self, message: bytes, response_size: int) -> bytes: + ... # canned responses + + Which one a controller gets is decided by whoever constructs its connection, not + by a magic port value or an environment check inside the driver. A test passes + the simulated one straight to the controller's constructor. + """ + + async def connect(self) -> None: + logger.info("[SIM] Connected", connection=type(self).__name__) + + async def close(self) -> None: + logger.info("[SIM] Disconnected", connection=type(self).__name__) diff --git a/src/fastcs/connections/supervisor.py b/src/fastcs/connections/supervisor.py new file mode 100644 index 000000000..a78072fbb --- /dev/null +++ b/src/fastcs/connections/supervisor.py @@ -0,0 +1,362 @@ +from __future__ import annotations + +import asyncio +import functools +import inspect +from collections.abc import Callable, Coroutine +from typing import Any, Generic, TypeVar, cast + +from fastcs.connections.connection import Connection +from fastcs.connections.policy import ConnectionPolicy, Failure +from fastcs.exceptions import DisconnectedError +from fastcs.logging import logger +from fastcs.scheduling import ScanSchedule + +DEFAULT_RECONNECT_PERIOD = 1.0 +"""Seconds a supervisor waits between reconnect attempts, unless configured.""" + +DEFAULT_RECONNECT_ATTEMPTS = 10 +"""Reconnect attempts a supervisor makes before giving up, unless configured.""" + +Connection_T = TypeVar("Connection_T", bound=Connection) + + +class Supervisor(Generic[Connection_T]): + """Owns everything ongoing about one connection. + + Its health, its reconnect loop and retry budget, the scans of the controllers + that hold it, and their ``connected`` attributes. A controller is given the + supervisor's `handle`, never the connection, so every call it makes passes the + supervisor's boundary, which: + + - fails fast with `DisconnectedError` if the link is already down, + - turns a link failure - see `ConnectionPolicy` - into `DisconnectedError`, + marks the link down and wakes the reconnect loop, + - passes a device error back to the caller unchanged. + + The launcher creates one per connection in ``fastcs.yaml``. An application that + builds its own tree creates them itself, before the controllers that take + their handles:: + + eiger = Supervisor(EigerConnection(port=8000), name="eiger") + controller = EigerController(eiger=eiger.handle) + FastCS(controller, transports, supervisors=[eiger]).run() + + Args: + connection: The connection to supervise. Nothing is opened yet. + name: What to call it in log messages; the connection's class by default + reconnect_attempts: Consecutive failed reconnect attempts before giving up + reconnect_period: Seconds between reconnect attempts + policy: How to read its failures; the connection class's ``policy`` by + default + + """ + + def __init__( + self, + connection: Connection_T, + *, + name: str | None = None, + reconnect_attempts: int = DEFAULT_RECONNECT_ATTEMPTS, + reconnect_period: float = DEFAULT_RECONNECT_PERIOD, + policy: ConnectionPolicy | None = None, + ) -> None: + self._connection = connection + self.name = name or type(connection).__name__ + self.reconnect_attempts = reconnect_attempts + self.reconnect_period = reconnect_period + self.policy = policy or type(connection).policy + + self._handle = cast(Connection_T, _Handle(self)) + + self._up = False + self._up_event = asyncio.Event() + self._down_event = asyncio.Event() + self._gave_up = asyncio.Event() + self._timeouts = 0 + + self.dependencies: list[Supervisor] = [] + """The supervisors this one waits for before reconnecting. Set at the seal.""" + + self.schedule = ScanSchedule() + """The work of the controllers holding this connection. Filled at the seal.""" + + self._on_fatal: Callable[[BaseException], None] | None = None + self._tasks: set[asyncio.Task] = set() + + @property + def connection(self) -> Connection_T: + """The connection itself. Calls made on it bypass the boundary.""" + return self._connection + + @property + def handle(self) -> Connection_T: + """What a controller holds in place of the connection. + + Passes as the connection's own type, for type checkers and ``isinstance`` + alike, and routes every ``async`` method call through this supervisor. + """ + return self._handle + + @property + def up(self) -> bool: + """Whether the link is believed to be usable.""" + return self._up + + @property + def gave_up(self) -> bool: + """Whether this supervisor has stopped trying. Only a restart helps now.""" + return self._gave_up.is_set() + + async def wait_up(self) -> None: + """Block until the link is up. Returns immediately if it already is.""" + await self._up_event.wait() + + # The boundary + + async def call( + self, + method: Callable[..., Coroutine[Any, Any, Any]], + *args: Any, + **kwargs: Any, + ) -> Any: + """Call one of the connection's IO methods through the boundary.""" + if not self._up: + raise DisconnectedError(f"{self.name} is disconnected") + + try: + result = await method(*args, **kwargs) + except Exception as exc: + failure = self.policy.classify(exc) + if failure is Failure.TIMEOUT: + self._timeouts += 1 + if self._timeouts < self.policy.timeout_count: + raise + elif failure is Failure.DEVICE: + # The device answered, so the link is alive. + self._timeouts = 0 + raise + + self.mark_down(exc) + if isinstance(exc, DisconnectedError): + raise + raise DisconnectedError(f"{self.name} is disconnected: {exc!r}") from exc + + self._timeouts = 0 + return result + + def mark_down(self, error: BaseException) -> None: + """Mark the link down and wake the reconnect loop. Idempotent.""" + if not self._up: + return + + self._up = False + self._timeouts = 0 + self._up_event.clear() + self._down_event.set() + logger.warning("Connection down", connection=self.name, error=repr(error)) + + # Lifecycle + + async def open(self) -> None: + """Open the link for the first time. Raises if it cannot be opened.""" + await self._connection.connect() + self._set_up() + + async def close(self) -> None: + """Close the link. Safe on a link that is already closed.""" + self._up = False + self._up_event.clear() + await self._connection.close() + + def start( + self, + loop: asyncio.AbstractEventLoop, + on_fatal: Callable[[BaseException], None], + ) -> None: + """Start the scan loops and the reconnect loop.""" + self._on_fatal = on_fatal + self._tasks = self.schedule.start(loop, self._up_event) + self._tasks.add(loop.create_task(self._reconnect_loop())) + + def stop(self) -> None: + """Cancel the scan loops and the reconnect loop.""" + for task in self._tasks: + if not task.done(): + task.cancel() + self._tasks.clear() + + def _set_up(self) -> None: + self._up = True + self._timeouts = 0 + self._down_event.clear() + self._up_event.set() + + # Recovery + + async def _reconnect_loop(self) -> None: + """Sleep until the link is marked down, then bring it back. + + One per connection, idle until that connection actually goes down, so a + healthy connection costs nothing and each retries at its own pace. + """ + while True: + await self._down_event.wait() + + await self.schedule.set_connected(False) + await self.schedule.flag_polled_invalid() + + if not await self._reconnect(): + return + + async def _reconnect(self) -> bool: + """Retry until the link is back. False if it never will be.""" + attempts = 0 + while True: + blocked = [d for d in self.dependencies if not d.up] + if blocked: + # Waiting is free: no attempt is made, so none is counted. A + # connection layered over two links is no more usable with one of + # them than with neither, so all of them must be up. + logger.info( + "Waiting on dependencies", + connection=self.name, + dependencies=[d.name for d in blocked], + ) + await _wait_for_all_up_or_any_gave_up(blocked) + + stalled = [d for d in blocked if d.gave_up] + if stalled: + # This connection cannot succeed, but it has spent nothing + # either. Say so, then stop: only a restart helps now. + logger.error( + "Stalled: dependency gave up", + connection=self.name, + dependencies=[d.name for d in stalled], + ) + return False + continue + + try: + await self._connection.close() # tolerates an already-closed link + await self._connection.connect() + except Exception as exc: + attempts += 1 + terminal = self.policy.is_terminal(exc) + if terminal or attempts >= self.reconnect_attempts: + self._give_up(exc, attempts, terminal) + return False + + if attempts == 1: + logger.opt(exception=exc).warning( + "Reconnect failed, retrying", + connection=self.name, + period=self.reconnect_period, + attempts=self.reconnect_attempts, + ) + await asyncio.sleep(self.reconnect_period) + continue + + self._set_up() + logger.info("Connection back up", connection=self.name) + await self.schedule.set_connected(True) + # A rebooted device can come back with different values, and a + # read-once read would otherwise stay stale forever. + await self.schedule.reread_once() + return True + + def _give_up(self, exc: Exception, attempts: int, terminal: bool) -> None: + # Terminal until the process restarts. Setting the event releases anything + # waiting on this connection, so dependents stall loudly instead of hanging. + self._gave_up.set() + logger.opt(exception=exc).error( + "Giving up", + connection=self.name, + attempts=attempts, + reason=self.policy.reason(self._connection) if terminal else None, + ) + if terminal and self.policy.fatal and self._on_fatal is not None: + # Only a restart can fix it, so ask for one rather than sit there + # looking healthy while serving stale values. + self._on_fatal(exc) + + def __repr__(self) -> str: + return f"Supervisor({self.name}, up={self._up})" + + +async def _wait_for_all_up_or_any_gave_up(supervisors: list[Supervisor]) -> None: + """Block until every one is up, or any one of them gives up. + + Waiting on recovery alone would hang forever once one gives up, so both are + awaited and whichever lands first wins. + """ + + async def all_up() -> None: + await asyncio.gather(*(s.wait_up() for s in supervisors)) + + waiters = {asyncio.ensure_future(all_up())} + waiters |= {asyncio.ensure_future(s._gave_up.wait()) for s in supervisors} # noqa: SLF001 + + try: + await asyncio.wait(waiters, return_when=asyncio.FIRST_COMPLETED) + finally: + for waiter in waiters: + waiter.cancel() + + +class _Handle: + """A stand-in for a connection that routes every call through its supervisor. + + ``__class__`` reports the connection's own type, so ``isinstance`` checks pass + and the handle can be passed anywhere the connection could. Only ``async`` + methods go through the boundary; anything else - a property, a sync helper, + tracing - is the connection's own, unchanged. + """ + + __slots__ = ("_supervisor",) + + def __init__(self, supervisor: Supervisor) -> None: + object.__setattr__(self, "_supervisor", supervisor) + + @property + def __class__(self) -> type: # pyright: ignore[reportIncompatibleMethodOverride] + return type(self._supervisor.connection) + + def __getattr__(self, name: str) -> Any: + supervisor: Supervisor = self._supervisor + value = getattr(supervisor.connection, name) + if not inspect.iscoroutinefunction(value): + return value + + @functools.wraps(value) + async def through_boundary(*args: Any, **kwargs: Any) -> Any: + return await supervisor.call(value, *args, **kwargs) + + return through_boundary + + def __setattr__(self, name: str, value: Any) -> None: + setattr(self._supervisor.connection, name, value) + + def __dir__(self) -> list[str]: + return dir(self._supervisor.connection) + + def __repr__(self) -> str: + return f"" + + +def connection_of(connection: Connection) -> Connection: + """The connection behind a handle, or the connection itself if it is not one. + + Two handles to one connection need not be the same object, so anything that + compares connections compares what this returns. + """ + if type(connection) is _Handle: + return connection._supervisor.connection # noqa: SLF001 + return connection + + +def supervisor_of(connection: Connection) -> Supervisor | None: + """The supervisor behind a handle, or ``None`` for a bare connection.""" + if type(connection) is _Handle: + return connection._supervisor # noqa: SLF001 + return None diff --git a/src/fastcs/control_system.py b/src/fastcs/control_system.py index 44467fe63..05b4e9b5e 100644 --- a/src/fastcs/control_system.py +++ b/src/fastcs/control_system.py @@ -7,6 +7,7 @@ from IPython.terminal.embed import InteractiveShellEmbed +from fastcs.connections import Supervisor from fastcs.controllers import Controller, ControllerAPI, ControllerRunner from fastcs.logging import logger from fastcs.tracer import Tracer @@ -38,6 +39,10 @@ class FastCS: 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 + supervisors: The supervisors of the connections the controllers were + given - one sequence per controller, or a single sequence. These are the + connections the runner opens and supervises; a tree of purely soft + controllers has none. """ def __init__( @@ -45,6 +50,7 @@ def __init__( controllers: Controller | Sequence[Controller], transports: Sequence[Transport], loop: asyncio.AbstractEventLoop | None = None, + supervisors: Sequence[Supervisor] | Sequence[Sequence[Supervisor]] = (), ): if isinstance(controllers, Controller): controllers = [controllers] @@ -61,7 +67,7 @@ def __init__( self._transports = transports self._loop = loop or asyncio.get_event_loop() - self._runner = ControllerRunner(self._controllers, self._loop) + self._runner = ControllerRunner(self._controllers, supervisors, self._loop) self.controller_apis: list[ControllerAPI] = [] def run(self, interactive: bool = True): @@ -95,10 +101,34 @@ async def serve(self, interactive: bool = True) -> None: interactive: Whether to create an interactive IPython shell """ + try: + coros = await self._start(interactive) + except BaseException: + # A failure during startup aborts: a partly built application is worse + # than none, because clients connect successfully and never find what + # they are looking for. Close whatever was opened on the way up, then + # let the caller - or the orchestrator - see why. + await self._runner.stop() + raise + + try: + await asyncio.gather(*coros) + except asyncio.CancelledError: + pass + except Exception: + logger.exception("Unhandled exception in serve") + finally: + logger.info("Shutting down FastCS") + await self._runner.stop() + if self._runner.fatal_reason is not None: + raise self._runner.fatal_reason + + async def _start(self, interactive: bool) -> list[Coroutine]: + """Bring the application up, and return what ``serve`` should await.""" # Build the APIs before wiring transports to them: a transport # registers its callbacks when it connects, and would miss the first # readback if the controllers had already started. - self.controller_apis = await self._runner.setup() + self.controller_apis = await self._runner.build() context = { "controllers": {_context_key(c): c for c in self._controllers}, @@ -143,15 +173,20 @@ async def block_forever(): await self._runner.start() - try: - await asyncio.gather(*coros) - except asyncio.CancelledError: - pass - except Exception: - logger.exception("Unhandled exception in serve") - finally: - logger.info("Shutting down FastCS") - await self._runner.stop() + # A fatal condition - a DRA device node that has gone away, say - happens in + # a background task, where a raise would be invisible. The runner records it + # instead, and this coroutine is where the process notices and comes down + # rather than serving stale values it can never refresh. Nothing calls + # ``sys.exit``, so an embedder sees an exception out of ``serve`` rather + # than losing its process. + async def fail_on_fatal() -> None: + await self._runner.fatal_error.wait() + assert self._runner.fatal_reason is not None + raise self._runner.fatal_reason + + coros.append(fail_on_fatal()) + + return coros 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/base_controller.py b/src/fastcs/controllers/base_controller.py index 162a68a61..66cb63bde 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -1,7 +1,7 @@ from __future__ import annotations from inspect import getattr_static -from typing import TypeVar +from typing import Any, TypeVar from fastcs.attributes import Attribute, UnboundAttr from fastcs.controllers.controller_api import ControllerAPI @@ -30,6 +30,31 @@ class BaseController(Tracer): root_attribute: Attribute | None = None description: str | None = None + connection: Any = None + """The link this controller does its IO over, if it has one. + + Set in ``__init__`` to the connection the controller was given - a handle from + the connection's `Supervisor`, which is what arrives in the constructor. A + controller's attributes talk only to this one connection - two devices means + two controllers - and several controllers may hold the same one, in which case + their scans are paused and resumed together, and the read-once reads of all of + them are repeated after it reconnects. A controller that needs another device's + behaviour calls a method on the controller that owns it. + + A controller with no connection (a soft grouping controller, or a + `ControllerVector`) is never paused, and reports ``connected`` as always on. + + Typed ``Any`` rather than ``Connection | None`` so that a driver can narrow it to + the connection it actually holds, and call that connection's own methods:: + + class TemperatureController(Controller): + connection: IPConnection + + A mutable attribute is invariant, so a driver cannot narrow a declared + ``Connection | None`` without a type checker objecting to every driver in + existence. + """ + def __init__( self, path: list[str] | None = None, @@ -42,6 +67,7 @@ def __init__( self.description = description self._path: list[str] = path or [] + self._sealed = False # Internal state that should not be accessed directly by base classes self.__attributes: dict[str, Attribute] = {} @@ -151,19 +177,67 @@ def __setattr__(self, name, value): else: super().__setattr__(name, value) - async def initialise(self): - """Hook for subclasses to dynamically add attributes before building the API""" + async def build(self): + """Hook for structure that depends on the device. + + Called by the framework once every connection is open, and before the tree + is sealed. Add the attributes and sub controllers that could + only be known by asking the device - the ones knowable without it belong in + ``__init__``, which is where a controller can be constructed and inspected in + a test with no hardware. + + The connection is open by the time this runs, so a controller that has to + ask the device what it has - how many channels, which parameters - reads it + here and creates what it finds. + + No hardware *writes* here. A device that needs a mode set before it can be + read has that write in `Connection.connect`, which means "make the link + usable" rather than merely "open the socket". + """ pass - def post_initialise(self): - """Hook to call after all attributes added, before serving the application""" - self.check_filled() + async def setup(self): + """Hook for hardware writes and checks, once the whole tree is built. + + Called by the framework after every controller's ``build`` has run and every + connection is open, so this can read and write across the tree. + + Runs once only - not again after a reconnect. Configuration a device needs + every time it comes back belongs in `Connection.connect`. + + No new attributes or sub controllers here: the tree is sealed by now. + """ + pass + + def seal(self) -> None: + """Freeze this controller and its sub controllers, recursively. + + Called by the framework once the build phase is over and the implicit + attributes are added. The API a transport serves is a snapshot of the tree + at that point, so anything added later would silently never reach one; + after the seal, adding it raises instead. + """ + self._sealed = True + for sub_controller in self.sub_controllers.values(): + sub_controller.seal() + + @property + def sealed(self) -> bool: + return self._sealed + + def _check_not_sealed(self) -> None: + if self._sealed: + raise RuntimeError( + f"Controller {'.'.join(self.path) or type(self).__name__} is sealed. " + "Add attributes, methods and sub controllers in `__init__` or " + "`build`, never after them." + ) def check_filled(self): """Check that every class-body declaration was provisioned, recursively. A driver may call ``self.filler.check_filled()`` itself at the end of - its own ``initialise``; the framework calls this afterwards so that a + its own ``build``; the framework calls this afterwards so that a controller which forgot to does not serve a half-built API. """ self.filler.check_filled() @@ -199,6 +273,8 @@ def _check_for_name_clash(self, name: str): ) def add_attribute(self, name, attr: Attribute): + self._check_not_sealed() + try: self._check_for_name_clash(name) except ValueError as exc: @@ -222,6 +298,8 @@ def attributes(self) -> dict[str, Attribute]: return self.__attributes def add_sub_controller(self, name: str, sub_controller: BaseController): + self._check_not_sealed() + try: self._check_for_name_clash(name) except ValueError as exc: @@ -282,6 +360,8 @@ def _check_against_declaration(self, name: str, member: object): ) def add_command(self, name: str, command: Command): + self._check_not_sealed() + try: self._check_for_name_clash(name) except ValueError as exc: @@ -303,6 +383,8 @@ def command_methods(self) -> dict[str, Command]: return self.__command_methods def add_scan(self, name: str, scan: Scan): + self._check_not_sealed() + try: self._check_for_name_clash(name) except ValueError as exc: diff --git a/src/fastcs/controllers/controller.py b/src/fastcs/controllers/controller.py index 0bee8d7d8..8d43b5a18 100755 --- a/src/fastcs/controllers/controller.py +++ b/src/fastcs/controllers/controller.py @@ -1,13 +1,5 @@ -import asyncio -from collections import defaultdict -from collections.abc import Sequence - -from fastcs.attributes.attr_r import AttrR from fastcs.controllers.base_controller import BaseController from fastcs.controllers.controller_api import ControllerAPI -from fastcs.logging import logger -from fastcs.methods import ScanCallback -from fastcs.util import ONCE class Controller(BaseController): @@ -18,7 +10,6 @@ def __init__( description: str | None = None, ) -> None: super().__init__(description=description) - self._connected = False def add_sub_controller(self, name: str, sub_controller: BaseController): if name.isdigit(): @@ -28,119 +19,6 @@ def add_sub_controller(self, name: str, sub_controller: BaseController): ) return super().add_sub_controller(name, sub_controller) - @property - def connected(self) -> bool: - """Whether the controller believes it can talk to its device. - - Set by `connect`/`reconnect`, and cleared when a scan task raises. The - `ControllerRunner` reads it to decide when to reconnect. - """ - return self._connected - - async def connect(self) -> None: - """Hook to perform initial connection to device - - This should set ``_connected`` to ``True`` if the connection was successful to - enable scan tasks. - - """ - self._connected = True - - async def reconnect(self): - """Hook to reconnect to device after an error - - This should set ``_connected`` to ``True`` if the connection was successful to - enable scan tasks. - - If the connection cannot be re-established it should log an error with the - reason. It should not raise an exception. - - """ - self._connected = True - - async def disconnect(self) -> None: - """Hook to tidy up resources before stopping the application""" - pass - - def create_api_and_tasks( - self, - ) -> tuple[ControllerAPI, list[ScanCallback], list[ScanCallback]]: - """Create api for transports tasks for FastCS backend - - Creates a tuple of - - The `ControllerAPI` for this controller - - Initial coroutines to be run once on startup - - Periodic coroutines to run as background tasks - - Returns: - tuple[ControllerAPI, list[ScanCallback], list[ScanCallback]] - - """ - controller_api = self._build_api(self._path) - - scan_dict: dict[float, list[ScanCallback]] = defaultdict(list) - initial_coros: list[ScanCallback] = [] - - for api in controller_api.walk_api(): - for method in api.scan_methods.values(): - if method.period is ONCE: - initial_coros.append(method.fn) - else: - scan_dict[method.period].append(method.fn) - - for attribute in api.attributes.values(): - if not (isinstance(attribute, AttrR) and attribute.has_getter()): - continue - - poll_period = attribute.poll_period - - async def poll_attribute(attribute: AttrR = attribute) -> None: - await attribute.poll() - - if poll_period is ONCE: - initial_coros.append(poll_attribute) - elif poll_period is not None: - scan_dict[poll_period].append(poll_attribute) - - periodic_scan_coros: list[ScanCallback] = [] - for period, methods in scan_dict.items(): - periodic_scan_coros.append(self._create_periodic_scan_coro(period, methods)) - - return controller_api, periodic_scan_coros, initial_coros - - def _create_periodic_scan_coro( - self, period: float, scans: Sequence[ScanCallback] - ) -> ScanCallback: - """Create a coroutine to run scans at a given period - - This returns a coroutine that runs scans at a given period. If an exception is - raised in a callback it is caught and the updates for the controller are - paused, waiting for `_connected` to be set back to true via the `reconnect` - method. - - Args: - period: The period to run the scans at - scans: A list of `ScanCallback` to run periodically - - Returns: - A wrapper `ScanCallback` that runs all of the callbacks at a given period - """ - - async def scan_coro() -> None: - while True: - if not self._connected: - await asyncio.sleep(1) - continue - - try: - await asyncio.gather( - asyncio.sleep(period), *[scan() for scan in scans] - ) - except Exception: - logger.exception("Exception in scan task", period=period) - self._connected = False - - await asyncio.sleep(1) # Wait so this message appears last - logger.error("Pausing scan tasks and waiting for reconnect") - - return scan_coro + def create_api(self) -> ControllerAPI: + """Create the `ControllerAPI` of this controller and its sub controllers.""" + return self._build_api(self._path) diff --git a/src/fastcs/controllers/filler.py b/src/fastcs/controllers/filler.py index 3a53495ad..88d57d628 100644 --- a/src/fastcs/controllers/filler.py +++ b/src/fastcs/controllers/filler.py @@ -13,11 +13,11 @@ class OdinDetector(Controller): frames: AttrRW[int] # exists as soon as __init__ returns - async def initialise(self) -> None: + async def build(self) -> None: self.filler.fill_attribute("frames", getter=..., setter=...) so ``self.frames`` can be referenced by the rest of ``__init__`` - the rule -ADR 0013 takes from ophyd-async, and what makes ``initialise`` safe to run in +ADR 0013 takes from ophyd-async, and what makes ``build`` safe to run in parallel across controllers. Filling provisions the IO and metadata on the attribute that is already there, so a reference taken during ``__init__`` stays valid. @@ -25,7 +25,7 @@ async def initialise(self) -> None: A hint that cannot say what it holds - ``state: AttrR``, where the datatype is an enum whose members only exist on the wire - is a **promise** instead: it is not created, and `ControllerFiller.check_filled` requires that introspection -added it by the time the controller is initialised. +added it by the time the controller is built. """ from __future__ import annotations diff --git a/src/fastcs/controllers/runner.py b/src/fastcs/controllers/runner.py index ead653e9e..d71fe8e68 100644 --- a/src/fastcs/controllers/runner.py +++ b/src/fastcs/controllers/runner.py @@ -1,40 +1,66 @@ import asyncio -from collections.abc import Sequence +from collections import deque +from collections.abc import Iterator, Sequence +from fastcs.attributes import AttrR +from fastcs.connections import Connection, Supervisor +from fastcs.connections.supervisor import connection_of, supervisor_of +from fastcs.controllers.base_controller import BaseController from fastcs.controllers.controller import Controller from fastcs.controllers.controller_api import ControllerAPI from fastcs.logging import logger -from fastcs.methods import ScanCallback +from fastcs.scheduling import ScanSchedule -RECONNECT_PERIOD = 1.0 -"""Seconds between checks for a controller that has dropped its connection""" +MAX_BUILD_PASSES = 32 +"""Passes the build phase makes before deciding the tree is not settling. + +A ``build`` that adds a sub controller whose ``build`` adds another needs one pass +per tier; a cap catches runaway construction rather than hanging. +""" + +CONNECTED_ATTRIBUTE = "connected" +"""The implicit attribute every controller gets at the seal. + +Whether the controller can talk to its device: kept in step with its connection by +that connection's `Supervisor`, and always on for a controller with no connection. +An ordinary attribute, so every transport publishes it with no changes. +""" class ControllerRunner: """Runs one or more `Controller` s, without serving them anywhere. - This owns the whole controller lifecycle - initialising, connecting, - running the initial and periodic tasks, reconnecting after a failure, and - tidying up - and nothing about how the controllers are presented. `FastCS` - uses it and adds transports on top; an embedded caller that only wants the + The runner owns the startup and shutdown sequence, and nothing ongoing about a + connection - that belongs to the connection's `Supervisor`. `FastCS` uses the + runner and adds transports on top; an embedded caller that only wants the controllers running can use it on its own:: - runner = ControllerRunner(controller) + eiger = Supervisor(EigerConnection(port=8000), name="eiger") + runner = ControllerRunner(EigerController(eiger.handle), [eiger]) await runner.start() ... await runner.stop() - Starting has two halves, because anything serving the controllers needs - their `ControllerAPI` before the first values are read: ``setup`` initialises - them and builds the APIs, and ``start`` connects and starts the tasks. - Calling ``start`` on its own does both. + Startup is: open every connection, in dependency order; call ``build`` across + the tree until it stops growing; seal the tree; then ``setup`` once, the + read-once reads, and hand over to the supervisors. Controllers never call + their own hooks to compensate for sequencing. + + Starting is in two halves, because anything serving the controllers needs their + `ControllerAPI` before the first values are read: ``build`` opens the + connections, builds and seals the tree and returns the APIs, and ``start`` does + the rest. Calling ``start`` on its own does both. - **Idempotency is the caller's responsibility.** Starting a running runner, - or stopping a stopped one, is not defined. + **A failure anywhere in startup aborts.** A partly built tree means an + application with a silently incomplete set of parameters, which is worse than no + application at all. The orchestrator owns the retry. Args: - controllers: The controller(s) to run. Accepts either a single - ``Controller`` or a sequence of them. + controllers: The top-level controller(s) to run + supervisors: The supervisors of the connections the controllers were given. + Either one sequence per top-level controller, or - for one controller, or + when the controllers should share one scope - a single sequence. A + connection's ``depends_on`` is resolved within its own scope. loop: Optional event loop to create the tasks in """ @@ -42,110 +68,374 @@ class ControllerRunner: def __init__( self, controllers: Controller | Sequence[Controller], + supervisors: Sequence[Supervisor] | Sequence[Sequence[Supervisor]] = (), loop: asyncio.AbstractEventLoop | None = None, ) -> None: + self._tasks: set[asyncio.Task] = set() + self._supervisors: list[Supervisor] = [] + if isinstance(controllers, Controller): controllers = [controllers] self._controllers: list[Controller] = list(controllers) + self._scopes = _scopes(supervisors, len(self._controllers)) + self._supervisors = [s for scope in self._scopes for s in scope] self._loop = loop + self._opened: list[Supervisor] = [] + self._soft = ScanSchedule() self._controller_apis: list[ControllerAPI] = [] - self._scan_coros: list[ScanCallback] = [] - self._initial_coros: list[ScanCallback] = [] - self._tasks: set[asyncio.Task] = set() + + self.fatal_error: asyncio.Event = asyncio.Event() + """Set when something has happened that the application cannot carry on from. + + A background task cannot usefully raise - nothing is awaiting it - and an + embedded FastCS must not call ``sys.exit``, so a fatal condition is reported + here instead. `FastCS` awaits it and shuts down; an embedder can do the same, + and read `fatal_reason` for what happened. + + Set by a supervisor whose reconnect fails terminally under a fatal + `ConnectionPolicy` - a DRA device node that has gone away, say - since only + a restart can fix that. + """ + + self.fatal_reason: BaseException | None = None + """Why `fatal_error` was set, if it was.""" @property def controller_apis(self) -> list[ControllerAPI]: - """The API of each controller. Empty until ``setup`` has run.""" + """The API of each controller. Empty until ``build`` has run.""" return self._controller_apis - async def setup(self) -> list[ControllerAPI]: - """Initialise the controllers and build their APIs. + @property + def supervisors(self) -> list[Supervisor]: + """The supervisors this runner starts and stops, in the order it opens them.""" + return list(self._opened or self._supervisors) + + async def build(self) -> list[ControllerAPI]: + """Open every connection, build and seal the tree, and create the APIs. - Runs before anything connects, so that a transport can be wired to the - APIs and catch the first readback. + Runs before anything is set up or scanned, so that a transport can be wired + to the APIs and catch the first readback. Returns: The API of each controller, in the order they were given """ - for controller in self._controllers: - await controller.initialise() - controller.post_initialise() + try: + return await self._open_build_and_seal() + except BaseException: + # Startup aborts, but the connections opened before the failure are + # still open, and nothing else would ever close them. + await self._close_connections() + raise + + async def _open_build_and_seal(self) -> list[ControllerAPI]: + dependencies = { + supervisor: dependencies + for scope in self._scopes + for supervisor, dependencies in _resolve_dependencies(scope).items() + } - 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) + for supervisor in _in_dependency_order(self._supervisors, dependencies): + self._opened.append(supervisor) + await supervisor.open() + + await self._build_phase() + + # Every connection exists by now, so the graph is fixed from here on and + # reconnect loops use it as it is. + for supervisor, resolved in dependencies.items(): + supervisor.dependencies = resolved + + self._seal() + self._controller_apis = [ + controller.create_api() for controller in self._controllers + ] return self._controller_apis async def start(self) -> None: - """Connect the controllers and start their tasks. + """Set the tree up and hand it over to the supervisors. - Runs ``setup`` first if it has not already run. + Runs ``build`` first if it has not already run. """ if not self._controller_apis: - await self.setup() + await self.build() - for controller in self._controllers: - await controller.connect() + try: + await self._setup_and_run() + except BaseException: + # As in ``build``: a ``setup`` or an initial read that raises leaves + # every connection open with nothing to close them. + await self.stop() + raise + + async def _setup_and_run(self) -> None: + for controller in self._walk_controllers(): + await controller.setup() + + self._warn_about_unpolled_connections() + + await self._soft.set_connected(True) + for supervisor in self._supervisors: + await supervisor.schedule.set_connected(supervisor.up) - for coro in self._initial_coros: - await coro() + await self._soft.read_once() + for supervisor in self._supervisors: + await supervisor.schedule.read_once() loop = self._loop or asyncio.get_event_loop() - self._tasks = {loop.create_task(coro()) for coro in self._scan_coros} - self._tasks |= { - loop.create_task(self._reconnect_loop(controller)) - for controller in self._controllers - } + self._tasks = self._soft.start(loop, gate=None) + for supervisor in self._supervisors: + supervisor.start(loop, on_fatal=self.fail) async def stop(self) -> None: - """Stop the tasks and disconnect the controllers.""" + """Stop every task and close every connection. + + Connections close in reverse of the order they opened, so anything layered + over another is closed before what it rides on. ``setup`` is not undone - + devices keep their last configured state. + """ self._cancel_tasks() + await self._close_connections() - for controller in self._controllers: + async def _close_connections(self) -> None: + for supervisor in reversed(self._opened): try: - await controller.disconnect() + await supervisor.close() except Exception: logger.exception( - "Exception during disconnect", controller=controller.path + "Exception while closing connection", connection=supervisor.name ) - async def _reconnect_loop(self, controller: Controller) -> None: - """Bring a controller back after its scan tasks hit an error. + def fail(self, error: BaseException) -> None: + """Report a condition the application cannot carry on from. + + Raising here would be invisible - this is called from a background task with + nothing awaiting it - and an embedded FastCS must not call ``sys.exit``, so + the failure is recorded and whatever is running the runner decides what to + do. + """ + if self.fatal_reason is None: + self.fatal_reason = error + self.fatal_error.set() + + # Build and seal + + async def _build_phase(self) -> None: + """Walk the tree top-down calling ``build``, to a fixpoint. - A scan task that raises marks its controller disconnected and pauses - rather than dying, so something has to try to bring it back. That is the - runner's job rather than the controller's, so that every controller - reconnects the same way whether or not its author thought about it. + A ``build`` may add sub controllers, which need building themselves, so the + walk repeats over anything newly added until a pass adds nothing. """ - while True: - await asyncio.sleep(RECONNECT_PERIOD) + built: set[int] = set() - if controller.connected: - continue + for _ in range(MAX_BUILD_PASSES): + pending = [c for c in self._walk_controllers() if id(c) not in built] + if not pending: + return - logger.info("Attempting to reconnect", controller=controller.path) + for controller in pending: + built.add(id(controller)) + await controller.build() + + raise RuntimeError( + f"Controller tree did not settle in {MAX_BUILD_PASSES} build passes. " + "A `build` that adds a sub controller on every pass never finishes." + ) + + def _seal(self) -> None: + """Hand each controller's work to its owner, add ``connected``, and freeze. + + Every class-body declaration must have been provisioned by now: the build + phase is over, so nothing else is going to fill one in. + """ + for controller in self._controllers: + controller.check_filled() + + held: set[int] = set() + for controller in self._walk_controllers(): + supervisor = self._supervisor_of(controller) + schedule = self._soft if supervisor is None else supervisor.schedule + if supervisor is not None: + held.add(id(supervisor)) + + connected = AttrR(bool, description="Whether the device link is up") try: - await controller.reconnect() - except Exception: - logger.exception("Reconnect failed", controller=controller.path) + controller.add_attribute(CONNECTED_ATTRIBUTE, connected) + except ValueError as exc: + raise RuntimeError( + f"Controller {_describe(controller)} already has a member named " + f"{CONNECTED_ATTRIBUTE!r}, which the framework adds to every " + "controller. Rename it." + ) from exc + + schedule.add_controller(controller) + schedule.add_connected_attribute(connected) + + for supervisor in self._supervisors: + if id(supervisor) not in held: + logger.warning( + "Connection declared but not held by any controller. It will " + "be opened and reconnected while doing nothing.", + connection=supervisor.name, + ) + + for controller in self._controllers: + controller.seal() + + def _supervisor_of(self, controller: BaseController) -> Supervisor | None: + """The supervisor that owns a controller's work, or ``None`` if it is soft.""" + connection: Connection | None = controller.connection + if connection is None: + return None + + supervisor = supervisor_of(connection) + if supervisor is not None and any(supervisor is s for s in self._supervisors): + return supervisor + + # A test, or an embedder, may hand a controller the connection itself. + # Nothing it calls passes the boundary, but its scans can still be paused. + underlying = connection_of(connection) + for supervisor in self._supervisors: + if supervisor.connection is underlying: + return supervisor + + raise RuntimeError( + f"Controller {_describe(controller)} holds a " + f"{type(underlying).__name__} the runner does not supervise, so it " + "would never be opened or reconnected. Give it a handle from a " + "`Supervisor` passed to the runner." + ) + + def _warn_about_unpolled_connections(self) -> None: + """Nothing detects a connection failing unless something uses it regularly. + + Phrased as fact rather than fault: an all-on-demand device is a legitimate + design, it just will not notice a failure until the next write. + """ + for supervisor in self._supervisors: + if not supervisor.schedule.has_polling: + logger.warning( + "Connection has no polled attribute or scan method among its " + "controllers, so nothing will detect it failing until the next " + "write.", + connection=supervisor.name, + ) + + # Helpers + + def _walk_controllers(self) -> Iterator[BaseController]: + """Every controller in the tree, level order.""" + queue: deque[BaseController] = deque(self._controllers) + while queue: + controller = queue.popleft() + yield controller + queue.extend(controller.sub_controllers.values()) def _cancel_tasks(self) -> None: - # ``Task.cancel`` does not raise - it returns whether the task was - # cancellable - so the guards the old FastCS._stop_scan_tasks wrapped - # this in never fired. for task in self._tasks: if not task.done(): task.cancel() - self._tasks.clear() + for supervisor in self._supervisors: + supervisor.stop() + def __del__(self): self._cancel_tasks() + + +def _describe(controller: BaseController) -> str: + return ".".join(controller.path) or type(controller).__name__ + + +def _scopes( + supervisors: Sequence[Supervisor] | Sequence[Sequence[Supervisor]], + controllers: int, +) -> list[list[Supervisor]]: + """The supervisors grouped by the top-level controller whose scope they are in.""" + flat = [each for each in supervisors if isinstance(each, Supervisor)] + grouped = [list(each) for each in supervisors if not isinstance(each, Supervisor)] + if flat and grouped: + raise ValueError("Give the runner supervisors, or groups of them, not both.") + + if not grouped: + scopes = [flat] + elif len(grouped) == controllers: + scopes = grouped + else: + raise ValueError( + f"Given {len(grouped)} groups of supervisors for {controllers} " + "controllers. Give one group per controller, or one group in all." + ) + + seen: set[int] = set() + for supervisor in (s for scope in scopes for s in scope): + if id(supervisor) in seen: + raise ValueError(f"{supervisor} was given to the runner twice.") + seen.add(id(supervisor)) + + return scopes + + +def _resolve_dependencies( + scope: list[Supervisor], +) -> dict[Supervisor, list[Supervisor]]: + """Resolve each connection's ``depends_on`` types to the instances in its scope. + + Every connection of a named type is waited for, so where there are several of + one type, a dependent waits for all of them. + + Raises: + ValueError: If the resolved graph has a cycle, which would leave every + connection in it waiting on the others forever + + """ + resolved = { + supervisor: [ + other + for other in scope + if other is not supervisor + and isinstance(other.connection, tuple(supervisor.connection.depends_on)) + ] + for supervisor in scope + } + + settled: set[int] = set() + + def visit(supervisor: Supervisor, path: list[Supervisor]) -> None: + if id(supervisor) in settled: + return + if any(supervisor is node for node in path): + cycle = " -> ".join(s.name for s in [*path, supervisor]) + raise ValueError(f"Cycle in connection dependencies: {cycle}") + for dependency in resolved[supervisor]: + visit(dependency, [*path, supervisor]) + settled.add(id(supervisor)) + + for supervisor in scope: + visit(supervisor, []) + + return resolved + + +def _in_dependency_order( + supervisors: list[Supervisor], dependencies: dict[Supervisor, list[Supervisor]] +) -> list[Supervisor]: + """Declaration order, except that a dependency comes before its dependent. + + Assumes the graph is acyclic - `_resolve_dependencies` has checked it. + """ + ordered: list[Supervisor] = [] + + def visit(supervisor: Supervisor) -> None: + if any(supervisor is done for done in ordered): + return + for dependency in dependencies[supervisor]: + visit(dependency) + ordered.append(supervisor) + + for supervisor in supervisors: + visit(supervisor) + return ordered diff --git a/src/fastcs/demo/README.md b/src/fastcs/demo/README.md index b6e5db9db..2c01f72fb 100644 --- a/src/fastcs/demo/README.md +++ b/src/fastcs/demo/README.md @@ -63,7 +63,10 @@ Notes: the lesson — "small & known → declare; large & self-describing → introspect" — and the REST sim also exercises an HTTP client backend the temp examples never touch, matching real downstream drivers (`fastcs-eiger`, `fastcs-secop`, - PandABlocks). + PandABlocks). The walk happens in `EigerDetector.build`, against an open + connection; `EigerConnection` is a plain `HTTPConnection` subclass that knows + the detector's URL layout and its `{"value": ...}` envelope, and nothing about + what the detector turns out to have. ## Baselines vs framework PRs diff --git a/src/fastcs/demo/eiger.py b/src/fastcs/demo/eiger.py index afebc0c7c..c72a6bf82 100644 --- a/src/fastcs/demo/eiger.py +++ b/src/fastcs/demo/eiger.py @@ -1,21 +1,24 @@ -"""Example 5 - introspectable controller: a cut-down Eiger over the fake REST sim. +"""Example 5 - dynamic controller: a cut-down Eiger over the fake REST sim. Half the attributes (``count_time``, ``state``) are declared as type hints and -checked by the current ``HintedAttribute`` introspection-validation mechanism; the -rest of the parameter tree is discovered at ``initialise()`` time by walking the -sim's ``keys`` endpoints and is added dynamically, with no static check. A device -that describes itself over the wire is exactly the case where introspection earns -its complexity - contrast with the (deliberately non-introspectable) SCPI/temperature -examples. +checked by the filler; the rest of the parameter tree is discovered by walking the +sim's ``keys`` endpoints in ``build`` and is added dynamically, with no static +check. A device that describes itself over the wire is exactly the case where +asking earns its complexity - contrast with the (deliberately self-describing-free) +SCPI/temperature examples. + +The walk happens in `EigerDetector.build`, which the framework calls once the +connection is open. `EigerConnection` is a plain `HTTPConnection` subclass: it knows +how to talk to the detector, and nothing about what the detector turns out to have. """ import enum -from dataclasses import dataclass -from typing import Any, cast +from typing import Any, NamedTuple, cast import httpx from fastcs.attributes import AttrR, AttrRW, Polled +from fastcs.connections import HTTPConnection, HTTPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import DType from fastcs.demo.simulation.eiger import API_PREFIX, Subsystem, ValueType @@ -30,161 +33,188 @@ # Poll period (seconds) for read-only status params that change on the device. UPDATE_PERIOD = 0.2 +SUBSYSTEMS: tuple[Subsystem, ...] = ("config", "status") -def _datatype(param: str, data: dict[str, Any]) -> type[DType]: + +class ParameterInfo(NamedTuple): + """What the device says about one of its parameters. + + Deliberately the *shape* of the parameter and not its value: the shape is what + ``build`` turns into an attribute, and the value changes every time it is read. + """ + + subsystem: Subsystem + name: str + value_type: ValueType + access_mode: str + allowed_values: tuple[str, ...] | None + + +def _datatype(info: ParameterInfo) -> type[DType]: """Build a datatype for a parameter from the metadata the device reports. A parameter that reports ``allowed_values`` is discrete, so it becomes an enum class built from those values. The members are only knowable over the wire, - which is exactly the case introspection exists for. + which is exactly the case a runtime walk exists for. """ - allowed_values = data.get("allowed_values") - if allowed_values is None: - return _DATATYPES[data["value_type"]] + if info.allowed_values is None: + return _DATATYPES[info.value_type] - name = "".join(part.title() for part in param.split("_")) + name = "".join(part.title() for part in info.name.split("_")) # The functional API builds a class; type checkers only see the instance signature. return cast( - type[enum.Enum], enum.Enum(name, {value: value for value in allowed_values}) + type[enum.Enum], + enum.Enum(name, {value: value for value in info.allowed_values}), ) -@dataclass -class EigerConnectionSettings: - base_url: str = "http://localhost:8000" +class EigerConnection(HTTPConnection): + """HTTP to the Eiger REST sim, and the one thing that knows when it is down. + Everything about being an HTTP connection - the client, and raising what httpx + raises when the transport fails - comes from `HTTPConnection`, and everything + about the link's health from its supervisor. What is here is only what is + Eiger's rather than HTTP's: the URL layout, and the ``{"value": ...}`` envelope + the detector wraps every parameter in. -class EigerConnection: - """Thin async HTTP client wrapper for the Eiger REST sim. + Args: + settings: Where the detector's REST API lives - A ``transport`` can be supplied to point directly at an in-process ASGI app - (e.g. in tests), bypassing the network entirely. """ - def __init__(self, transport: httpx.AsyncBaseTransport | None = None): - self._transport = transport - self._client: httpx.AsyncClient | None = None + def __init__(self, settings: HTTPConnectionSettings | None = None) -> None: + super().__init__(settings or HTTPConnectionSettings(port=8000)) - async def connect(self, settings: EigerConnectionSettings) -> None: - self._client = httpx.AsyncClient( - base_url=settings.base_url, transport=self._transport - ) + @classmethod + def in_process(cls, transport: httpx.AsyncBaseTransport) -> "EigerConnection": + """Talk directly to an in-process ASGI app, bypassing the network. - async def close(self) -> None: - if self._client is not None: - await self._client.aclose() - self._client = None + Not a constructor argument: the constructor's signature is the + connection's config schema, and a transport object is not config. + """ + connection = cls() + connection._transport = transport + return connection - @property - def client(self) -> httpx.AsyncClient: - if self._client is None: - raise RuntimeError("EigerConnection is not connected") - return self._client + async def get(self, path: str) -> Any: + """The detector wraps every parameter as ``{"value": ...}`` - unwrap it.""" + return (await super().get(path))["value"] async def keys(self, subsystem: Subsystem) -> list[str]: - response = await self.client.get(f"{API_PREFIX}/{subsystem}/keys") - response.raise_for_status() - return response.json() + """The parameter names one subsystem reports.""" + # The listing endpoint answers with a bare list rather than an envelope, + # so it goes through the un-overridden `get`. + return await super().get(f"{API_PREFIX}/{subsystem}/keys") - async def get(self, subsystem: Subsystem, param: str) -> dict: - response = await self.client.get(f"{API_PREFIX}/{subsystem}/{param}") - response.raise_for_status() - return response.json() + async def describe(self, subsystem: Subsystem, param: str) -> dict: + """The whole envelope for one parameter: its value *and* its metadata.""" + return await super().get(f"{API_PREFIX}/{subsystem}/{param}") - async def put(self, subsystem: Subsystem, param: str, value) -> None: - response = await self.client.put( - f"{API_PREFIX}/{subsystem}/{param}", json={"value": value} - ) - response.raise_for_status() + async def get_parameter(self, subsystem: Subsystem, param: str) -> Any: + return await self.get(f"{API_PREFIX}/{subsystem}/{param}") + + async def put_parameter(self, subsystem: Subsystem, param: str, value: Any) -> None: + await self.put(f"{API_PREFIX}/{subsystem}/{param}", {"value": value}) class EigerDetector(Controller): - """Cut-down Eiger controller: half declared, half introspected.""" + """Cut-down Eiger controller: half declared, half discovered at runtime.""" + + connection: EigerConnection - # Declared (checked): must exist, with this access mode and dtype, after - # initialise() introspects the parameter tree. ``state`` is discrete, and its - # enum class is built from the ``allowed_values`` the device reports, so there - # is no author-time type to hint - only the access mode can be pinned here. + # Declared (checked): must exist, with this access mode and dtype, once + # build() has turned what the device reports into attributes. ``state`` + # is discrete, and its enum class is built from the ``allowed_values`` the + # device reports, so there is no author-time type to hint - only the access + # mode can be pinned here. count_time: AttrRW[float] state: AttrR - # Derived (soft): built on top of the introspected ``state`` param. Declaring + # Derived (soft): built on top of the discovered ``state`` param. Declaring # ``state`` as a checked attribute is what lets us reference it in code and # publish something computed from it - here, whether the detector is idle. idle: AttrR[bool] - def __init__( - self, - settings: EigerConnectionSettings | None = None, - transport: httpx.AsyncBaseTransport | None = None, - ) -> None: - self.connection = EigerConnection(transport=transport) + def __init__(self, eiger: EigerConnection) -> None: + self.connection = eiger super().__init__() - self._settings = settings or EigerConnectionSettings() - def _getter(self, subsystem: Subsystem, param: str): async def get() -> Any: - data = await self.connection.get(subsystem, param) # No cast here - ``update`` validates against the datatype, which is the # one place a bad value from the device should be coerced or complained # about. - return data["value"] + return await self.connection.get_parameter(subsystem, param) return get def _setter(self, subsystem: Subsystem, param: str): async def put(value: Any) -> None: - await self.connection.put(subsystem, param, value) + await self.connection.put_parameter(subsystem, param, value) return put - async def connect(self) -> None: - await self.connection.connect(self._settings) - self._connected = True + async def _walk(self) -> list[ParameterInfo]: + """Ask the detector what it has. - async def disconnect(self) -> None: - await self.connection.close() - - async def initialise(self) -> None: - for subsystem in ("config", "status"): + The connection is open by the time ``build`` runs, so this is a plain + sequence of reads - the shape of the tree is whatever the device answers + with on this particular startup. + """ + parameters: list[ParameterInfo] = [] + for subsystem in SUBSYSTEMS: for param in await self.connection.keys(subsystem): - data = await self.connection.get(subsystem, param) - datatype = _datatype(param, data) - - if data["access_mode"] == "rw": - getter = self._getter(subsystem, param) - setter = self._setter(subsystem, param) - else: - # Read-only params are status values that change on the device, - # so poll them periodically rather than reading once. - getter = Polled( - self._getter(subsystem, param), period=UPDATE_PERIOD - ) - setter = None - - declaration = self.filler.declarations.get(param) - if declaration is not None and declaration.child is not None: - # A parameter the class body declared already exists as an - # unfilled attribute, so provision that one rather than - # adding a second of the same name. The filler checks the - # access mode and datatype the hint promised against what - # the device turned out to report. - self.filler.fill_attribute( - param, datatype=datatype, getter=getter, setter=setter - ) - elif setter is None: - self.add_attribute(param, AttrR(datatype, getter=getter)) - else: - self.add_attribute( - param, AttrRW(datatype, getter=getter, setter=setter) + data = await self.connection.describe(subsystem, param) + allowed_values = data.get("allowed_values") + parameters.append( + ParameterInfo( + subsystem=subsystem, + name=param, + value_type=data["value_type"], + access_mode=data["access_mode"], + allowed_values=( + None if allowed_values is None else tuple(allowed_values) + ), ) + ) + return parameters + + async def build(self) -> None: + """Turn what the device reports into attributes.""" + for parameter in await self._walk(): + datatype = _datatype(parameter) + getter = self._getter(parameter.subsystem, parameter.name) + setter = None + + if parameter.access_mode == "rw": + setter = self._setter(parameter.subsystem, parameter.name) + else: + # Read-only params are status values that change on the device, + # so poll them periodically rather than reading once. + getter = Polled(getter, period=UPDATE_PERIOD) + + declaration = self.filler.declarations.get(parameter.name) + if declaration is not None and declaration.child is not None: + # A parameter the class body declared already exists as an + # unfilled attribute, so provision that one rather than adding a + # second of the same name. The filler checks the access mode and + # datatype the hint promised against what the device turned out + # to report. + self.filler.fill_attribute( + parameter.name, datatype=datatype, getter=getter, setter=setter + ) + elif setter is None: + self.add_attribute(parameter.name, AttrR(datatype, getter=getter)) + else: + self.add_attribute( + parameter.name, AttrRW(datatype, getter=getter, setter=setter) + ) # Every hinted parameter should have turned up in the tree the device # reported. self.filler.check_filled() - # Keep the derived ``idle`` flag in sync with the introspected ``state``. + # Keep the derived ``idle`` flag in sync with the discovered ``state``. self.state.add_readback_callback(self._update_idle) async def _update_idle(self, state: enum.Enum) -> None: diff --git a/src/fastcs/demo/fastcs.yaml b/src/fastcs/demo/fastcs.yaml index 1784ef2f4..4cfbaa019 100644 --- a/src/fastcs/demo/fastcs.yaml +++ b/src/fastcs/demo/fastcs.yaml @@ -2,15 +2,21 @@ controllers: - id: MAIN type: fastcs.TemperatureController - ip_settings: - ip: "localhost" - port: 25565 + connections: + temperature: + settings: + ip: "localhost" + port: 25565 + reconnect_attempts: 5 + reconnect_period: 2.0 num_ramp_controllers: 4 - id: AUX type: fastcs.TemperatureController - ip_settings: - ip: "localhost" - port: 25566 + connections: + temperature: + settings: + ip: "localhost" + port: 25566 num_ramp_controllers: 2 transport: - graphql: diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index 210c36256..9db66ef5b 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -174,6 +174,36 @@ "title": "GraphQLTransport", "type": "object" }, + "IPConnectionConfig": { + "additionalProperties": false, + "properties": { + "reconnect_attempts": { + "default": 10, + "minimum": 1, + "title": "Reconnect Attempts", + "type": "integer" + }, + "reconnect_period": { + "default": 1.0, + "exclusiveMinimum": 0, + "title": "Reconnect Period", + "type": "number" + }, + "settings": { + "anyOf": [ + { + "$ref": "#/$defs/IPConnectionSettings" + }, + { + "type": "null" + } + ], + "default": null + } + }, + "title": "IPConnectionConfig", + "type": "object" + }, "IPConnectionSettings": { "properties": { "ip": { @@ -247,6 +277,19 @@ "title": "TangoTransport", "type": "object" }, + "TemperatureControllerConnections": { + "additionalProperties": false, + "properties": { + "temperature": { + "$ref": "#/$defs/IPConnectionConfig" + } + }, + "required": [ + "temperature" + ], + "title": "TemperatureControllerConnections", + "type": "object" + }, "TemperatureControllerEntry": { "additionalProperties": false, "properties": { @@ -259,19 +302,19 @@ "title": "Type", "type": "string" }, + "connections": { + "$ref": "#/$defs/TemperatureControllerConnections" + }, "num_ramp_controllers": { "title": "Num Ramp Controllers", "type": "integer" - }, - "ip_settings": { - "$ref": "#/$defs/IPConnectionSettings" } }, "required": [ "id", "type", - "num_ramp_controllers", - "ip_settings" + "connections", + "num_ramp_controllers" ], "title": "TemperatureControllerEntry", "type": "object" diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py index 89a7caabc..1a0b43904 100755 --- a/src/fastcs/demo/temperature_attr.py +++ b/src/fastcs/demo/temperature_attr.py @@ -31,7 +31,7 @@ import numpy as np from fastcs.attributes import AttrR, AttrRW, Polled -from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.connections import IPConnection from fastcs.controllers import Controller, ControllerVector from fastcs.datatypes import Array1D, DType_T from fastcs.logging import logger @@ -46,7 +46,6 @@ class OnOffEnum(enum.StrEnum): @dataclass class TemperatureControllerSettings: num_ramp_controllers: int - ip_settings: IPConnectionSettings class TemperatureProtocol: @@ -126,8 +125,19 @@ async def get_actual(self) -> float: class TemperatureController(Controller): - def __init__(self, settings: TemperatureControllerSettings) -> None: - self.connection = IPConnection() + # Narrows the base class's `Connection | None`, so this controller's own code + # can call the methods of the connection it actually holds. + connection: IPConnection + + def __init__( + self, temperature: IPConnection, settings: TemperatureControllerSettings + ) -> None: + # The type hint is the declaration: the launcher configures this argument + # under `connections: temperature:` in this entry, opens it, and supervises + # it. What arrives here is a handle that behaves like the IPConnection, and + # the ramps below are handed the same one, so the whole tree shares one + # link. Nothing here connects, reconnects or handles a dead socket. + self.connection = temperature self._settings = settings self._protocol = TemperatureProtocol(self.connection) @@ -156,22 +166,6 @@ async def cancel_all(self) -> None: # TODO: The requests all get concatenated and the sim doesn't handle it await asyncio.sleep(0.1) - async def connect(self) -> None: - await self.connection.connect(self._settings.ip_settings) - - async def reconnect(self): - try: - await self.connection.close() - await self.connection.connect(self._settings.ip_settings) - except BaseException: - logger.exception("Reconnect failed") - return - - self._connected = True - - async def close(self) -> None: - await self.connection.close() - @scan(0.1) async def update_voltages(self): voltages = await self._protocol.get_voltages() @@ -186,12 +180,13 @@ async def update_voltages(self): class TemperatureRampController(Controller): - def __init__(self, index: int, conn: IPConnection) -> None: - self._protocol = TemperatureRampProtocol(conn, index) + connection: IPConnection - super().__init__(f"Ramp{index:02d}") + def __init__(self, index: int, connection: IPConnection) -> None: + self.connection = connection + self._protocol = TemperatureRampProtocol(connection, index) - self.connection = conn + super().__init__(f"Ramp{index:02d}") # Datatypes inferred from the protocol methods' annotations - including the # enum, whose members come from OnOffEnum via get_enabled's return type. diff --git a/src/fastcs/exceptions.py b/src/fastcs/exceptions.py index 45fbad3dc..dc5a0e38e 100644 --- a/src/fastcs/exceptions.py +++ b/src/fastcs/exceptions.py @@ -6,3 +6,17 @@ class LaunchError(FastCSError): """For when there is an error in launching FastCS with the given transports and controller. """ + + +class DisconnectedError(ConnectionError): + """The link to a device is down, so the call could not be made. + + Raised by a connection's `Supervisor` rather than by driver code: every call + through a connection handle passes the supervisor, which fails fast while the + link is down and turns a link failure into this. A driver may raise it + directly for a device-specific "I'm offline" signal, such as a special reply, + and the supervisor treats that as a link failure too. + + A `ConnectionError`, and so an `OSError`, because that is what it is: the + transport is gone, not the device complaining. + """ diff --git a/src/fastcs/launch.py b/src/fastcs/launch.py index 7bbe7726e..a863e4489 100644 --- a/src/fastcs/launch.py +++ b/src/fastcs/launch.py @@ -10,6 +10,12 @@ from ruamel.yaml import YAML from fastcs import __version__ +from fastcs.connections import ( + DEFAULT_RECONNECT_ATTEMPTS, + DEFAULT_RECONNECT_PERIOD, + Connection, + Supervisor, +) from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.exceptions import LaunchError @@ -24,20 +30,38 @@ ) from fastcs.transports import Transport +CONNECTIONS_KEY = "connections" +"""The key under an entry that configures its controller's connections. + +One sub-key per connection-typed argument of the controller's ``__init__``, so the +schema knows exactly which names are required and what each one's settings are. +""" + +_ENTRY_KEYS = ("id", "type", CONNECTIONS_KEY) +"""Keys an entry model owns, rather than inlining from the options type.""" + +_FRAMEWORK_CONNECTION_KEYS = ("reconnect_attempts", "reconnect_period") +"""Keys of a connection's block read by its `Supervisor`, not the connection.""" + @dataclasses.dataclass(frozen=True) class _RegisteredClass: cls: type[Controller] - expects_options: bool + options_arg: str | None options_type: Any + connection_args: dict[str, type[Connection]] _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 +Controller class, the name and type of its options argument (if any), and its +connection-typed arguments. Populated by ``_build_entry_model`` and read by ``_instantiate_controllers``.""" +_CONNECTION_REGISTRY: dict[type[BaseModel], type[Connection]] = {} +"""Maps each dynamically-built connection model class to its Connection class. +Populated by ``_build_connection_model`` and read by ``_build_supervisor``.""" + def launch( controller_classes: type[Controller] | list[type[Controller]], @@ -53,9 +77,11 @@ def launch( Args: 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. + __init__ method taking no more than one options argument, plus any + number of arguments hinted with a `Connection` type - those are + configured under the entry's ``connections:`` block, one key per + argument name. 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: @@ -80,9 +106,9 @@ def _normalise_classes( def _discriminator(controller_class: type[Controller]) -> str: - """Type discriminator used in fastcs.yaml under each entry's ``type:`` key. + """Type discriminator used in fastcs.yaml under a ``type:`` key. - Defaults to ``.`` so that Controllers from + Defaults to ``.`` so that classes from independently-distributed packages cannot collide. May be overridden verbatim (no prefix added) by setting ``type_name: ClassVar[str]`` on the Controller class. @@ -182,12 +208,15 @@ def run( raise LaunchError("Failed to validate config") from e - controllers = _instantiate_controllers(instance_options.controllers) + controllers, supervisors = _instantiate_controllers( + instance_options.controllers + ) instance = FastCS( controllers, instance_options.transport, loop=asyncio.get_event_loop(), + supervisors=supervisors, ) instance.run() @@ -197,16 +226,25 @@ def run( def _instantiate_controllers( controllers_options: list[Any], -) -> list[Controller]: +) -> tuple[list[Controller], list[list[Supervisor]]]: """Instantiate each entry under `controllers:` and seed its path. Each item in ``controllers_options`` is a dynamically-built Pydantic - model that exposes ``id``, ``type`` and the controller's options fields - inlined as siblings. The originating Controller class and its - options-type are looked up in ``_ENTRY_REGISTRY`` (populated by - ``_build_entry_model``). The entry's ``id`` is seeded into the - controller's ``_path`` via ``set_path([id])`` so that - ``ControllerAPI.path`` is rooted at the YAML id. + model that exposes ``id``, ``type``, a ``connections`` block if the + controller takes any connections, and the controller's options fields + inlined as siblings. The originating Controller class and its arguments + are looked up in ``_ENTRY_REGISTRY`` (populated by ``_build_entry_model``). + The entry's ``id`` is seeded into the controller's ``_path`` via + ``set_path([id])`` so that ``ControllerAPI.path`` is rooted at the YAML id. + + Every connection is created, and given a `Supervisor`, before the controller + is constructed: the controller receives each supervisor's handle as the + argument of the same name. Nothing is opened yet. + + Returns: + The controllers, and the supervisors of each entry - which is the whole + set of connections the runner will supervise. + """ seen_ids: set[str] = set() duplicates: list[str] = [] @@ -220,21 +258,48 @@ def _instantiate_controllers( ) controllers: list[Controller] = [] + supervisors: list[list[Supervisor]] = [] for entry in controllers_options: entry_cls: type[BaseModel] = type(entry) registered = _ENTRY_REGISTRY[entry_cls] - if registered.expects_options: + + kwargs: dict[str, Any] = {} + entry_supervisors: list[Supervisor] = [] + for name in registered.connection_args: + config = getattr(getattr(entry, CONNECTIONS_KEY), name) + supervisor = _build_supervisor(f"{entry.id}.{name}", config) + entry_supervisors.append(supervisor) + kwargs[name] = supervisor.handle + + if registered.options_arg is not None: field_values = { name: getattr(entry, name) for name in entry_cls.model_fields - if name not in ("id", "type") + if name not in _ENTRY_KEYS } - controller = registered.cls(registered.options_type(**field_values)) - else: - controller = registered.cls() + kwargs[registered.options_arg] = registered.options_type(**field_values) + + controller = registered.cls(**kwargs) controller.set_path([entry.id]) controllers.append(controller) - return controllers + supervisors.append(entry_supervisors) + return controllers, supervisors + + +def _build_supervisor(name: str, config: BaseModel) -> Supervisor: + """Create one connection from its block, and the supervisor that owns it.""" + connection_class = _CONNECTION_REGISTRY[type(config)] + settings = { + field: getattr(config, field) + for field in type(config).model_fields + if field not in _FRAMEWORK_CONNECTION_KEYS + } + return Supervisor( + connection_class(**settings), + name=name, + reconnect_attempts=getattr(config, "reconnect_attempts"), # noqa: B009 + reconnect_period=getattr(config, "reconnect_period"), # noqa: B009 + ) def _options_field_definitions(options_type: type) -> dict[str, tuple[Any, Any]]: @@ -268,39 +333,152 @@ def _options_field_definitions(options_type: type) -> dict[str, tuple[Any, Any]] ) +def _parameter_default(parameter: inspect.Parameter) -> Any: + return ... if parameter.default is inspect.Parameter.empty else parameter.default + + +def _connection_field_definitions( + connection_class: type[Connection], +) -> dict[str, tuple[Any, Any]]: + """Field-by-field definitions for one connection, from its ``__init__``. + + A connection's settings are constructor arguments rather than an options + object, so its config block is built from the signature directly - the same + treatment `_options_field_definitions` gives an options type. + """ + if connection_class.__init__ is object.__init__: + return {} + + signature = inspect.signature(connection_class.__init__) + hints = get_type_hints(connection_class.__init__) + + fields: dict[str, tuple[Any, Any]] = {} + for name, parameter in signature.parameters.items(): + if name == "self": + continue + + if parameter.kind in ( + inspect.Parameter.VAR_POSITIONAL, + inspect.Parameter.VAR_KEYWORD, + ): + raise LaunchError( + f"Cannot build config for {connection_class.__name__}: " + f"`{parameter}` cannot be expressed as a config field." + ) + + if name not in hints: + raise LaunchError( + f"Expected typehinting in '{connection_class.__name__}" + f".__init__' but received {signature}. Add a typehint for `{name}`." + ) + fields[name] = (hints[name], _parameter_default(parameter)) + + return fields + + +_CONNECTION_MODELS: dict[type[Connection], type[BaseModel]] = {} +"""One config model per Connection class, however many entries use it.""" + + +def _build_connection_model(connection_class: type[Connection]) -> type[BaseModel]: + """Build a Pydantic model for one connection in an entry's ``connections:``. + + The connection's own settings, plus the framework keys its `Supervisor` reads. + There is no ``type:`` - the controller's type hint supplies it - and no + ``depends_on``, which is declared on the Connection class. + """ + if connection_class in _CONNECTION_MODELS: + return _CONNECTION_MODELS[connection_class] + + fields: dict[str, Any] = { + "reconnect_attempts": (int, Field(DEFAULT_RECONNECT_ATTEMPTS, ge=1)), + "reconnect_period": (float, Field(DEFAULT_RECONNECT_PERIOD, gt=0)), + } + for name, definition in _connection_field_definitions(connection_class).items(): + if name in fields: + raise LaunchError( + f"Connection {connection_class.__name__} takes a {name!r} argument, " + f"which collides with a launch-framework key." + ) + fields[name] = definition + + connection_model = create_model( + f"{connection_class.__name__}Config", + __config__={"extra": "forbid"}, + **fields, + ) + _CONNECTION_REGISTRY[connection_model] = connection_class + _CONNECTION_MODELS[connection_class] = connection_model + return connection_model + + +def _connections_model( + controller_class: type[Controller], connection_args: dict[str, type[Connection]] +) -> type[BaseModel]: + """The ``connections:`` block of an entry: one required key per argument.""" + fields: dict[str, Any] = { + name: (_build_connection_model(connection_class), ...) + for name, connection_class in connection_args.items() + } + return create_model( + f"{controller_class.__name__}Connections", + __config__={"extra": "forbid"}, + **fields, + ) + + +def _is_connection_type(hint: Any) -> bool: + return isinstance(hint, type) and issubclass(hint, Connection) + + def _build_entry_model(controller_class: type[Controller]) -> type[BaseModel]: """Build a Pydantic model for one entry under `controllers:`. - Each entry exposes ``id`` and 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``. + Each entry exposes ``id``, a ``type`` discriminator literal and - when the + controller takes any connections - a ``connections`` block, alongside the + options-type's fields inlined as siblings (no nested ``controller:`` block). + The Controller class and its arguments are recorded in ``_ENTRY_REGISTRY`` + for use by ``_instantiate_controllers``. + + An ``__init__`` argument hinted with a `Connection` type is a connection, and + its name is its key under ``connections:``. At most one other argument is + allowed, and it is the options type. """ sig = inspect.signature(controller_class.__init__) - args = inspect.getfullargspec(controller_class.__init__)[0] + args = inspect.getfullargspec(controller_class.__init__) + names = [*args.args[1:], *args.kwonlyargs] + hints = get_type_hints(controller_class.__init__) + hints.pop("return", None) discriminator = _discriminator(controller_class) + connection_args = { + name: hints[name] for name in names if _is_connection_type(hints.get(name)) + } + options_args = [name for name in names if name not in connection_args] + fields: dict[str, Any] = { "id": (str, ...), "type": (Literal[discriminator], ...), } - expects_options = False + if connection_args: + fields[CONNECTIONS_KEY] = ( + _connections_model(controller_class, connection_args), + ..., + ) + + options_arg: str | None = None options_type: Any = None - if len(args) == 1: - pass - elif len(args) == 2: - expects_options = True - hints = get_type_hints(controller_class.__init__) - hints.pop("return", None) - if not hints: + if len(options_args) == 1: + options_arg = options_args[0] + if options_arg not in hints: raise LaunchError( f"Expected typehinting in '{controller_class.__name__}" - f".__init__' but received {sig}. Add a typehint for `{args[-1]}`." + f".__init__' but received {sig}. Add a typehint for `{options_arg}`." ) - options_type = list(hints.values())[-1] + options_type = hints[options_arg] options_fields = _options_field_definitions(options_type) - for reserved in ("id", "type"): + for reserved in _ENTRY_KEYS: if reserved in options_fields: raise LaunchError( f"Options type {options_type.__name__} for " @@ -308,10 +486,11 @@ def _build_entry_model(controller_class: type[Controller]) -> type[BaseModel]: f"which collides with a launch-framework key." ) fields.update(options_fields) - else: + elif len(options_args) > 1: raise LaunchError( - f"Expected no more than 2 arguments for '{controller_class.__name__}" - f".__init__' but received {len(args)} as `{sig}`" + f"Expected no more than one options argument for " + f"'{controller_class.__name__}.__init__', besides its connections, " + f"but received {len(options_args)} as `{sig}`" ) entry_model = create_model( @@ -321,8 +500,9 @@ def _build_entry_model(controller_class: type[Controller]) -> type[BaseModel]: ) _ENTRY_REGISTRY[entry_model] = _RegisteredClass( cls=controller_class, - expects_options=expects_options, + options_arg=options_arg, options_type=options_type, + connection_args=connection_args, ) return entry_model diff --git a/src/fastcs/scheduling.py b/src/fastcs/scheduling.py new file mode 100644 index 000000000..7284b4c21 --- /dev/null +++ b/src/fastcs/scheduling.py @@ -0,0 +1,217 @@ +"""The periodic and read-once work of a group of controllers. + +Each `Supervisor` owns one of these for the controllers holding its connection, and +the `ControllerRunner` owns one for the soft controllers, which hold none. Grouping +the work by owner is what lets one connection's scans pause while it is down +without touching anybody else's. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import time +from collections import defaultdict +from collections.abc import Callable, Coroutine +from typing import TYPE_CHECKING, Any + +from fastcs.attributes import AttrR, Severity, Update +from fastcs.exceptions import DisconnectedError +from fastcs.logging import logger +from fastcs.util import ONCE + +if TYPE_CHECKING: + from fastcs.controllers import BaseController + +ScanCallback = Callable[[], Coroutine[None, None, None]] + +ERROR_SUMMARY_PERIOD = 60.0 +"""Seconds between reminders that a scan is still failing the same way.""" + + +async def flag_invalid(attribute: AttrR[Any]) -> None: + """Republish an attribute's last value marked as untrustworthy. + + The value and the time it was obtained are kept; only the severity changes, so + a client sees a stale value flagged rather than a made-up one. The next good + poll clears it. + """ + if attribute.severity is Severity.INVALID: + return + + await _publish( + attribute, + Update( + attribute.readback, + timestamp=attribute.timestamp, + severity=Severity.INVALID, + ), + ) + + +async def _publish(attribute: AttrR[Any], value: Any) -> None: + """Update an attribute on the framework's behalf, never raising. + + ``update`` has already logged whatever went wrong - a failing transport + callback, say - and a reconnect loop or scan task must not die of it. + """ + with contextlib.suppress(Exception): + await attribute.update(value) + + +class _ErrorReport: + """Rate-limited logging for one scan that keeps failing. + + The first failure is logged in full, then only a periodic reminder with a count + until the scan succeeds again - a device that rejects the same read every 0.1s + should not bury everything else in the log. + """ + + def __init__(self, event: str, **context: Any) -> None: + self._event = event + self._context = context + self._failures = 0 + self._last_logged = 0.0 + + def failed(self, exc: Exception) -> None: + now = time.monotonic() + if self._failures == 0: + logger.opt(exception=exc).error(self._event, **self._context) + self._last_logged = now + elif now - self._last_logged >= ERROR_SUMMARY_PERIOD: + logger.error( + f"{self._event} (still failing)", + failures=self._failures + 1, + error=repr(exc), + **self._context, + ) + self._last_logged = now + self._failures += 1 + + def succeeded(self) -> None: + if self._failures: + logger.info( + f"{self._event} (recovered)", failures=self._failures, **self._context + ) + self._failures = 0 + + +def _guarded_poll(attribute: AttrR[Any]) -> ScanCallback: + report = _ErrorReport("Poll failed", attribute=attribute) + + async def poll() -> None: + try: + await attribute.poll() + except DisconnectedError: + # Silent: the supervisor has already said the link is down. + await flag_invalid(attribute) + except Exception as exc: + report.failed(exc) + else: + report.succeeded() + + return poll + + +def _guarded_scan(scan: Callable[[], Coroutine[Any, Any, Any]]) -> ScanCallback: + report = _ErrorReport("Scan failed", fn=scan) + + async def run() -> None: + try: + await scan() + except DisconnectedError: + pass + except Exception as exc: + report.failed(exc) + else: + report.succeeded() + + return run + + +class ScanSchedule: + """The read-once reads, periodic scans and ``connected`` flags of some controllers. + + Periodic work is grouped by period, one task per period, and every callback is + guarded: a `DisconnectedError` flags the attribute that hit it and says nothing + more, and anything else is logged, rate limited, and retried next period. + """ + + def __init__(self) -> None: + self._initial: list[ScanCallback] = [] + self._periodic: dict[float, list[ScanCallback]] = defaultdict(list) + self._polled: list[AttrR[Any]] = [] + self._connected: list[AttrR[bool]] = [] + + def add_controller(self, controller: BaseController) -> None: + """Take on the scans and polled attributes of one controller.""" + for method in controller.scan_methods.values(): + if method.period is ONCE: + self._initial.append(method.__call__) + else: + self._periodic[method.period].append(_guarded_scan(method.__call__)) + + for attribute in controller.attributes.values(): + if not (isinstance(attribute, AttrR) and attribute.has_getter()): + continue + + period = attribute.poll_period + if period is ONCE: + self._initial.append(attribute.poll) + elif period is not None: + self._periodic[period].append(_guarded_poll(attribute)) + self._polled.append(attribute) + + def add_connected_attribute(self, attribute: AttrR[bool]) -> None: + """An attribute to keep in step with whether this work can run.""" + self._connected.append(attribute) + + @property + def has_polling(self) -> bool: + """Whether anything here runs periodically, and so would notice a failure.""" + return bool(self._periodic) + + @property + def periods(self) -> list[float]: + """The distinct periods of the periodic work, one task each.""" + return list(self._periodic) + + async def read_once(self) -> None: + """Run every read-once read and scan. Raises the first failure.""" + for callback in self._initial: + await callback() + + async def reread_once(self) -> None: + """Run every read-once read and scan again, logging rather than raising.""" + for callback in self._initial: + await _guarded_scan(callback)() + + async def set_connected(self, connected: bool) -> None: + for attribute in self._connected: + await _publish(attribute, connected) + + async def flag_polled_invalid(self) -> None: + """Mark every periodically read value as stale, since none can be read.""" + for attribute in self._polled: + await flag_invalid(attribute) + + def start( + self, loop: asyncio.AbstractEventLoop, gate: asyncio.Event | None + ) -> set[asyncio.Task]: + """Start one task per period. With a ``gate``, each waits while it is clear.""" + return { + loop.create_task(_run_periodically(period, callbacks, gate)) + for period, callbacks in self._periodic.items() + } + + +async def _run_periodically( + period: float, callbacks: list[ScanCallback], gate: asyncio.Event | None +) -> None: + while True: + if gate is not None: + await gate.wait() + + # Guarded, so this never raises; and sleeping alongside rather than after + # keeps the period steady however long the callbacks take. + await asyncio.gather(asyncio.sleep(period), *(cb() for cb in callbacks)) diff --git a/tests/assertable_controller.py b/tests/assertable_controller.py index 8299bff91..5c7993141 100644 --- a/tests/assertable_controller.py +++ b/tests/assertable_controller.py @@ -25,18 +25,12 @@ def __init__(self) -> None: self._sub_controllers.append(controller) self.add_sub_controller(f"SubController{index:02d}", controller) - initialised = False + built = False count = 0 - async def initialise(self) -> None: - await super().initialise() - self.initialised = True - - async def connect(self) -> None: - self._connected = True - - async def disconnect(self) -> None: - self._connected = False + async def build(self) -> None: + await super().build() + self.built = True @command() async def go(self): diff --git a/tests/demo/test_eiger.py b/tests/demo/test_eiger.py index 2b518c8bb..d9d0e0659 100644 --- a/tests/demo/test_eiger.py +++ b/tests/demo/test_eiger.py @@ -6,7 +6,9 @@ import pytest_asyncio from fastcs.attributes import AttrR, AttrRW -from fastcs.demo.eiger import UPDATE_PERIOD, EigerDetector +from fastcs.connections import Supervisor +from fastcs.controllers import ControllerRunner +from fastcs.demo.eiger import UPDATE_PERIOD, EigerConnection, EigerDetector from fastcs.demo.simulation.eiger import EigerParameter, create_eiger_sim_app from fastcs.util import ONCE @@ -14,15 +16,22 @@ SimState = dict[str, dict[str, EigerParameter]] +def _supervisor(app) -> Supervisor[EigerConnection]: + """What a `fastcs.yaml` would have built, pointed at the sim app.""" + return Supervisor( + EigerConnection.in_process(httpx.ASGITransport(app=app)), name="eiger" + ) + + @pytest_asyncio.fixture async def _eiger(): app = create_eiger_sim_app() - controller = EigerDetector(transport=httpx.ASGITransport(app=app)) - await controller.connect() - await controller.initialise() - controller.post_initialise() + supervisor = _supervisor(app) + controller = EigerDetector(supervisor.handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.build() yield controller, app.state.sim - await controller.disconnect() + await runner.stop() @pytest_asyncio.fixture @@ -119,10 +128,10 @@ async def test_temperature_oscillation_seen_via_subscribe(): # the controller's temperature attribute, subscribing for updates. app = create_eiger_sim_app() async with app.router.lifespan_context(app): - controller = EigerDetector(transport=httpx.ASGITransport(app=app)) - await controller.connect() - await controller.initialise() - controller.post_initialise() + supervisor = _supervisor(app) + controller = EigerDetector(supervisor.handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.build() temperature = controller.attributes["temperature"] assert isinstance(temperature, AttrR) @@ -139,6 +148,6 @@ async def record(value: float) -> None: await temperature.poll() await asyncio.sleep(0.2) - await controller.disconnect() + await runner.stop() assert len(set(seen)) > 1, f"temperature did not change: {seen}" diff --git a/tests/demo/test_temperature_attr.py b/tests/demo/test_temperature_attr.py index bab260065..6d6ff5f5c 100644 --- a/tests/demo/test_temperature_attr.py +++ b/tests/demo/test_temperature_attr.py @@ -4,7 +4,7 @@ import pytest from fastcs.attributes import AttrW -from fastcs.connections import IPConnectionSettings +from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import ControllerVector from fastcs.demo.temperature_attr import ( OnOffEnum, @@ -16,13 +16,12 @@ @pytest.fixture def controller() -> TemperatureController: - settings = TemperatureControllerSettings( - num_ramp_controllers=4, - ip_settings=IPConnectionSettings(ip="localhost", port=25565), + # A controller only needs something that behaves like its connection, so a + # test passes the connection itself, with no supervisor involved. + return TemperatureController( + IPConnection(IPConnectionSettings(ip="localhost", port=25565)), + TemperatureControllerSettings(num_ramp_controllers=4), ) - controller = TemperatureController(settings) - controller.post_initialise() - return controller @pytest.fixture diff --git a/tests/test_attr_decorator.py b/tests/test_attr_decorator.py index f5c5c1474..97eab6cad 100644 --- a/tests/test_attr_decorator.py +++ b/tests/test_attr_decorator.py @@ -1,4 +1,3 @@ -import asyncio from enum import Enum import numpy as np @@ -16,6 +15,7 @@ ) from fastcs.controllers import Controller from fastcs.datatypes import Array1D, Limits, NumericLimits +from fastcs.scheduling import ScanSchedule from fastcs.util import ONCE @@ -423,14 +423,14 @@ async def voltage(self) -> float: @pytest.mark.asyncio async def test_polled_attributes_are_scheduled(power_supply: PowerSupply): controller = power_supply - _, periodic, initial = controller.create_api_and_tasks() + schedule = ScanSchedule() + schedule.add_controller(controller) # ``serial`` is read once at connect; ``voltage`` is polled at 0.5s; # ``retries`` is never read on a schedule. - assert len(initial) == 1 - assert len(periodic) == 1 + assert schedule.periods == [0.5] - await asyncio.gather(*[coro() for coro in initial]) + await schedule.read_once() assert controller.serial.readback == "PSU-1" assert controller.retries.readback == 0 diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 0a0b0d061..16a7db717 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -400,7 +400,7 @@ class DemoParameterController(Controller): int_parameter: AttrRW float_parameter: AttrRW # hint to satisfy pyright - async def initialise(self): + async def build(self): self._connection = DummyConnection() await self._connection.connect() dtype_mapping = {"int": int, "float": float} @@ -454,7 +454,7 @@ async def setter(value, uri=uri): ) c = DemoParameterController() - await c.initialise() + await c.build() assert await c.ro_int_parameter.poll() == 10 assert await c.ro_int_parameter.poll() == 11 diff --git a/tests/test_connections.py b/tests/test_connections.py new file mode 100644 index 000000000..ee710f53b --- /dev/null +++ b/tests/test_connections.py @@ -0,0 +1,297 @@ +import asyncio +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest + +from fastcs.connections import ( + Connection, + ConnectionPolicy, + DisconnectedError, + DRAPolicy, + HTTPConnection, + HTTPConnectionSettings, + IPConnection, + IPConnectionSettings, + SerialConnection, + SerialConnectionSettings, + SimConnection, +) +from fastcs.connections.ip_connection import StreamConnection +from fastcs.connections.policy import Failure +from fastcs.connections.serial_connection import NotOpenedError + + +class OneConnection(Connection): + async def connect(self) -> None: ... + async def close(self) -> None: ... + + +# IPConnection + + +@pytest.mark.asyncio +async def test_ip_connect_opens_the_settings_it_was_given(): + connection = IPConnection(IPConnectionSettings(ip="192.0.2.1", port=1234)) + reader, writer = MagicMock(), MagicMock() + + with patch( + "asyncio.open_connection", AsyncMock(return_value=(reader, writer)) + ) as open_connection: + await connection.connect() + + open_connection.assert_awaited_once_with("192.0.2.1", 1234) + assert isinstance(connection._connection, StreamConnection) # noqa: SLF001 + + +@pytest.mark.asyncio +async def test_using_an_unopened_ip_connection_says_so(): + with pytest.raises(DisconnectedError, match="call connect"): + await IPConnection().send_command("ID?\r\n") + + +@pytest.mark.asyncio +async def test_a_command_that_hits_a_dead_socket_just_raises(): + """No error handling in the IO: the supervisor's boundary reads the exception.""" + connection = IPConnection() + stream = MagicMock() + stream.__aenter__ = AsyncMock(return_value=stream) + stream.__aexit__ = AsyncMock(return_value=False) + stream.send_message = AsyncMock(side_effect=ConnectionResetError) + connection._IPConnection__connection = stream # pyright: ignore[reportAttributeAccessIssue] + + with pytest.raises(ConnectionResetError): + await connection.send_command("R=1\r\n") + + +@pytest.mark.asyncio +async def test_a_command_is_sent_on_the_stream(): + connection = IPConnection() + stream = MagicMock() + stream.__aenter__ = AsyncMock(return_value=stream) + stream.__aexit__ = AsyncMock(return_value=False) + stream.send_message = AsyncMock() + connection._IPConnection__connection = stream # pyright: ignore[reportAttributeAccessIssue] + + await connection.send_command("R=1\r\n") + + stream.send_message.assert_awaited_once_with("R=1\r\n") + + +@pytest.mark.asyncio +async def test_stream_connection_reads_and_writes_lines(): + reader = asyncio.StreamReader() + reader.feed_data(b"ID=1\r\n") + writer = MagicMock() + writer.drain = AsyncMock() + writer.wait_closed = AsyncMock() + + stream = StreamConnection(reader, writer) + async with stream as held: + await held.send_message("ID?\r\n") + assert await held.receive_response() == "ID=1\r\n" + + writer.write.assert_called_once_with(b"ID?\r\n") + + await stream.close() + writer.close.assert_called_once() + + +# SerialConnection + + +@pytest.mark.asyncio +async def test_serial_connect_opens_the_settings_it_was_given(): + connection = SerialConnection( + SerialConnectionSettings(port="/dev/ttyS0", baud=9600) + ) + + with patch("aioserial.AioSerial") as aioserial: + await connection.connect() + + aioserial.assert_called_once_with(port="/dev/ttyS0", baudrate=9600) + + +@pytest.mark.asyncio +async def test_using_an_unopened_serial_connection_says_so(): + connection = SerialConnection(SerialConnectionSettings(port="/dev/ttyS0")) + + with pytest.raises(NotOpenedError, match="call connect"): + await connection.send_command(b"ID?\r\n") + + +@pytest.mark.asyncio +async def test_serial_round_trip(): + connection = SerialConnection(SerialConnectionSettings(port="/dev/ttyS0")) + stream = MagicMock() + stream.write_async = AsyncMock() + stream.read_async = AsyncMock(return_value=b"ID=1") + + with patch("aioserial.AioSerial", return_value=stream): + await connection.connect() + + await connection.send_command(b"R=1\r\n") + assert await connection.send_query(b"ID?\r\n", 4) == b"ID=1" + + await connection.close() + stream.close.assert_called_once() + # Closing an already-closed link is tolerated - the runner does it before + # every reconnect attempt. + await connection.close() + + +# SimConnection + + +@pytest.mark.asyncio +async def test_a_sim_connection_opens_and_closes_without_a_transport(): + """The pretending is all a driver writes: there is nothing here to fail.""" + + class SimDevice(SimConnection): + def __init__(self) -> None: + self.position = 0 + + async def move(self, steps: int) -> None: + self.position += steps + + connection = SimDevice() + await connection.connect() + + await connection.move(3) + assert connection.position == 3 + + await connection.close() + + +def test_a_sim_connection_is_a_sibling_of_the_real_transports(): + """Not a subclass of one: it would inherit a handle it never opens.""" + assert issubclass(SimConnection, Connection) + assert not issubclass(SimConnection, IPConnection | SerialConnection) + + +# ConnectionPolicy + + +@pytest.mark.parametrize( + "error", + [ + ConnectionResetError(), + BrokenPipeError(), + ConnectionRefusedError(), + OSError("Network is unreachable"), + EOFError(), + asyncio.IncompleteReadError(b"", 4), + FileNotFoundError("/dev/ttyACM0"), + httpx.ConnectError("refused"), + DisconnectedError("the device said it is offline"), + ], +) +def test_a_link_failure_is_a_disconnection(error: Exception): + assert ConnectionPolicy().classify(error) is Failure.DISCONNECTED + + +@pytest.mark.parametrize( + "error", [TimeoutError(), TimeoutError(), httpx.ReadTimeout("slow")] +) +def test_a_timeout_is_counted_rather_than_a_disconnection(error: Exception): + assert ConnectionPolicy().classify(error) is Failure.TIMEOUT + + +@pytest.mark.parametrize( + "error", + [ + ValueError("could not convert string to float: 'ERR'"), + httpx.HTTPStatusError( + "404", + request=httpx.Request("GET", "http://device"), + response=httpx.Response(404), + ), + RuntimeError("rejected"), + ], +) +def test_a_device_error_is_passed_to_the_caller(error: Exception): + assert ConnectionPolicy().classify(error) is Failure.DEVICE + + +def test_three_timeouts_in_a_row_mean_disconnected_by_default(): + assert ConnectionPolicy().timeout_count == 3 + + +def test_a_missing_device_node_is_terminal_for_a_dra_device(): + assert DRAPolicy().is_terminal(FileNotFoundError()) + + +@pytest.mark.parametrize("error", [TimeoutError(), OSError("I/O error")]) +def test_other_failures_are_not_terminal_for_a_dra_device(error: Exception): + assert not DRAPolicy().is_terminal(error) + + +def test_the_default_policy_never_gives_up_early(): + assert not ConnectionPolicy().is_terminal(FileNotFoundError()) + + +def test_the_default_policy_is_not_fatal(): + assert not ConnectionPolicy().fatal + + +def test_a_dra_device_is_fatal(): + """Only a pod restart can re-establish the claim, so it asks for one.""" + assert DRAPolicy().fatal + + +def test_a_policy_is_a_set_of_independent_settings(): + """A slow DRA device is a DRAPolicy with a different count, not a new class.""" + policy = DRAPolicy(timeout_count=10) + + assert policy.timeout_count == 10 + assert policy.is_terminal(FileNotFoundError()) + + +def test_every_connection_keeps_retrying_by_default(): + assert not OneConnection.policy.is_terminal(FileNotFoundError()) + + +def test_a_policy_is_set_on_the_class(): + class DRASerialConnection(SerialConnection): + policy = DRAPolicy() + + assert DRASerialConnection.policy.is_terminal(FileNotFoundError()) + + +def test_the_default_reason_names_the_device(): + connection = SerialConnection(SerialConnectionSettings(port="/dev/ttyACM0")) + + assert ConnectionPolicy().reason(connection) == ( + "/dev/ttyACM0 cannot recover from this failure." + ) + + +def test_the_dra_reason_names_the_device_node(): + connection = SerialConnection(SerialConnectionSettings(port="/dev/ttyACM0")) + + assert "/dev/ttyACM0" in DRAPolicy().reason(connection) + + +# label + + +def test_a_connection_is_labelled_by_its_class_unless_it_knows_its_device(): + assert OneConnection().label == "OneConnection" + + +def test_a_serial_connection_is_labelled_by_its_port(): + connection = SerialConnection(SerialConnectionSettings(port="/dev/ttyACM0")) + + assert connection.label == "/dev/ttyACM0" + + +def test_an_ip_connection_is_labelled_by_its_address(): + connection = IPConnection(IPConnectionSettings(ip="192.0.2.1", port=1234)) + + assert connection.label == "192.0.2.1:1234" + + +def test_an_http_connection_is_labelled_by_its_base_url(): + connection = HTTPConnection(HTTPConnectionSettings(host="192.0.2.1", port=8080)) + + assert connection.label == "http://192.0.2.1:8080" diff --git a/tests/test_control_system.py b/tests/test_control_system.py index 19e3215e7..6a53e9515 100644 --- a/tests/test_control_system.py +++ b/tests/test_control_system.py @@ -3,6 +3,7 @@ import pytest from fastcs.attributes import AttrR, NotPolled, Polled +from fastcs.connections import Connection, Supervisor from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.methods import Command, command @@ -30,7 +31,7 @@ class MyTestController(Controller): def __init__(self): super().__init__() - async def initialise(self): + async def build(self): async def do_nothing_dynamic() -> None: pass @@ -94,33 +95,68 @@ def __init__(self): assert controller.update_once.readback == 1 assert controller.update_never.readback == 0 - # One periodic scan task per distinct period, plus one reconnect watcher - assert len(fastcs._runner._scan_coros) == 1 - assert len(fastcs._runner._initial_coros) == 1 + # One periodic scan task per distinct period + assert fastcs._runner._soft.periods == [0.1] @pytest.mark.asyncio -async def test_controller_connect_disconnect(): - class MyTestController(Controller): - async def connect(self): - self.connect_called = True +async def test_serve_opens_and_closes_the_connection(): + """Opening and closing the link is the runner's job, not the controller's.""" - async def disconnect(self): - self.connect_called = False + class MyTestConnection(Connection): + def __init__(self): + self.open = False - controller = MyTestController() + async def connect(self) -> None: + self.open = True + + async def close(self) -> None: + self.open = False + + class MyTestController(Controller): + def __init__(self, connection): + self.connection = connection + super().__init__() + + connection = MyTestConnection() + supervisor = Supervisor(connection) + controller = MyTestController(supervisor.handle) loop = asyncio.get_event_loop() - fastcs = FastCS(controller, [], loop) + fastcs = FastCS(controller, [], loop, [supervisor]) task = asyncio.create_task(fastcs.serve(interactive=False)) - # connect is called at the start of serve + # The runner opens every connection at the start of serve await asyncio.sleep(0.1) - assert controller.connect_called + assert connection.open + connected = controller.attributes["connected"] + assert isinstance(connected, AttrR) and connected.readback task.cancel() - # disconnect is called at the end of serve + # ...and closes them at the end of it await asyncio.sleep(0.1) - assert not controller.connect_called + assert not connection.open + + +@pytest.mark.asyncio +async def test_a_fatal_runner_condition_comes_out_of_serve(): + """Not `sys.exit`: an embedded FastCS must be able to see this and decide. + + Reported rather than raised, because whatever hits it is a background task with + nothing awaiting it. + """ + + class MyTestController(Controller): + pass + + fastcs = FastCS(MyTestController(), [], asyncio.get_event_loop()) + + task = asyncio.create_task(fastcs.serve(interactive=False)) + await asyncio.sleep(0.1) + + fastcs._runner.fail(RuntimeError("the device is beyond saving")) + + with pytest.raises(RuntimeError, match="beyond saving"): + await asyncio.wait_for(task, timeout=5) diff --git a/tests/test_controller_filler.py b/tests/test_controller_filler.py index f52cd9a8e..68679f820 100644 --- a/tests/test_controller_filler.py +++ b/tests/test_controller_filler.py @@ -54,14 +54,14 @@ class Declared(Controller): async def test_attributes_added_without_a_hint(): # `fastcs-PandABlocks`/`fastcs-secop`: the whole tree comes off the wire. class Dynamic(Controller): - async def initialise(self) -> None: + async def build(self) -> None: self.add_attribute("discovered", AttrR(int)) controller = Dynamic() assert controller.attributes == {} - await controller.initialise() + await controller.build() assert set(controller.attributes) == {"discovered"} controller.check_filled() diff --git a/tests/test_controller_runner.py b/tests/test_controller_runner.py index b60bb7a64..edce47022 100644 --- a/tests/test_controller_runner.py +++ b/tests/test_controller_runner.py @@ -1,35 +1,75 @@ import asyncio import sys +from collections.abc import Iterator +from typing import Any import pytest -from fastcs.attributes import AttrR +from fastcs.attributes import AttrR, AttrRW, Polled, Severity +from fastcs.connections import ( + Connection, + DisconnectedError, + DRAPolicy, + Supervisor, +) from fastcs.controllers import Controller, ControllerRunner -from fastcs.controllers.runner import RECONNECT_PERIOD +from fastcs.controllers.runner import CONNECTED_ATTRIBUTE, MAX_BUILD_PASSES +from fastcs.logging import logger from fastcs.methods import scan from fastcs.util import ONCE +class FakeConnection(Connection): + """A connection that opens when told to, and records what was asked of it.""" + + def __init__(self) -> None: + self.fail_connect: Exception | None = None + self.fail_io: Exception | None = None + self.connects = 0 + self.closes = 0 + self.value = 1 + + async def connect(self) -> None: + self.connects += 1 + if self.fail_connect is not None: + raise self.fail_connect + + async def close(self) -> None: + self.closes += 1 + + async def read(self) -> int: + if self.fail_io is not None: + raise self.fail_io + return self.value + + async def write(self, value: int) -> None: + if self.fail_io is not None: + raise self.fail_io + self.value = value + + +class BaseLink(FakeConnection): + pass + + +class LayeredLink(FakeConnection): + depends_on = [BaseLink] + + class LifecycleController(Controller): """Records every lifecycle hook the runner is supposed to call.""" - def __init__(self): + def __init__(self, connection: FakeConnection | None = None): + self.connection = connection super().__init__() self.events: list[str] = [] self.count = AttrR(int) - async def initialise(self): - self.events.append("initialise") - - def post_initialise(self): - self.events.append("post_initialise") + async def build(self): + self.events.append("build") - async def connect(self): - self.events.append("connect") - await super().connect() - - async def disconnect(self): - self.events.append("disconnect") + async def setup(self): + self.events.append("setup") @scan(ONCE) async def read_once(self): @@ -37,42 +77,131 @@ async def read_once(self): await self.count.update(self.count.readback + 1) +class Polling(Controller): + """Polls its connection quickly, so it notices and recovers from failures.""" + + def __init__(self, connection: FakeConnection | None): + self.connection = connection + super().__init__() + self.value = AttrRW( + int, getter=Polled(self._read, period=0.001), setter=self._write + ) + self.polls = 0 + + async def _read(self) -> int: + self.polls += 1 + return await self.connection.read() + + async def _write(self, value: int) -> None: + await self.connection.write(value) + + +def supervise( + connection: FakeConnection, **kwargs: Any +) -> tuple[Supervisor[FakeConnection], FakeConnection]: + """A supervisor, and the handle a controller would be given.""" + supervisor = Supervisor(connection, name=type(connection).__name__, **kwargs) + return supervisor, supervisor.handle + + +def link_down(supervisor: Supervisor) -> None: + supervisor.mark_down(ConnectionResetError()) + + +async def eventually(predicate, timeout: float = 2) -> None: + async def wait() -> None: + while not predicate(): + await asyncio.sleep(0.001) + + await asyncio.wait_for(wait(), timeout=timeout) + + +@pytest.fixture +def log_records() -> Iterator[list[dict[str, Any]]]: + records: list[dict[str, Any]] = [] + handler = logger.add( + lambda message: records.append( + {"event": message.record["message"], **message.record["extra"]} + ), + level="DEBUG", + ) + yield records + logger.remove(handler) + + +# The lifecycle + + @pytest.mark.asyncio async def test_the_runner_drives_the_whole_lifecycle(): - controller = LifecycleController() - runner = ControllerRunner(controller) + supervisor, handle = supervise(FakeConnection()) + controller = LifecycleController(handle) + runner = ControllerRunner(controller, [supervisor]) + + await runner.start() + await runner.stop() + + assert controller.events == ["build", "setup", "initial"] + + +@pytest.mark.asyncio +async def test_start_opens_the_connection(): + supervisor, handle = supervise(FakeConnection()) + runner = ControllerRunner(LifecycleController(handle), [supervisor]) await runner.start() try: - assert controller.events == [ - "initialise", - "post_initialise", - "connect", - "initial", - ] - assert controller.count.readback == 1 + assert supervisor.up finally: await runner.stop() - assert controller.events[-1] == "disconnect" + +@pytest.mark.asyncio +async def test_stop_closes_the_connection(): + """Shutdown is a runner operation, not an author hook.""" + supervisor, handle = supervise(FakeConnection()) + runner = ControllerRunner(LifecycleController(handle), [supervisor]) + await runner.start() + + await runner.stop() + + assert supervisor.connection.closes == 1 @pytest.mark.asyncio -async def test_setup_builds_the_apis_before_anything_connects(): - """A transport is wired to the APIs between setup and start.""" +async def test_connections_open_before_anything_is_built(): + """A ``build`` runs against an open link, so it can ask the device questions.""" + supervisor, handle = supervise(FakeConnection()) + read_in_build: list[int] = [] + + class Asking(Controller): + def __init__(self, connection: FakeConnection): + self.connection = connection + super().__init__() + + async def build(self): + read_in_build.append(await self.connection.read()) + + runner = ControllerRunner(Asking(handle), [supervisor]) + await runner.build() + + assert read_in_build == [1] + + +@pytest.mark.asyncio +async def test_build_builds_the_apis_before_anything_is_set_up(): + """A transport is wired to the APIs between build and start.""" controller = LifecycleController() runner = ControllerRunner(controller) - apis = await runner.setup() + apis = await runner.build() - assert [api.path for api in apis] == [[]] assert "count" in apis[0].attributes - assert controller.events == ["initialise", "post_initialise"] - assert runner.controller_apis == apis + assert controller.events == ["build"] @pytest.mark.asyncio -async def test_start_sets_up_when_setup_has_not_run(): +async def test_start_builds_when_build_has_not_run(): runner = ControllerRunner(LifecycleController()) await runner.start() @@ -89,107 +218,782 @@ async def test_a_runner_takes_several_controllers(): await runner.start() try: - assert len(runner.controller_apis) == 2 assert all(controller.count.readback == 1 for controller in controllers) finally: await runner.stop() @pytest.mark.asyncio -async def test_stop_reports_a_failing_disconnect_without_raising(monkeypatch): - class UndisconnectableController(LifecycleController): - async def disconnect(self): - raise RuntimeError("no") +async def test_setup_runs_once_the_whole_tree_is_built(): + """A parent's ``setup`` can read a child that only exists after ``build``.""" + order: list[str] = [] - logged: list[tuple[str, BaseException | None]] = [] + class Child(Controller): + async def build(self): + order.append("child build") - def record_exception(event, **kwargs): - # ``logger.exception`` is called from the ``except`` block, so the - # exception it is reporting is the one currently being handled. - logged.append((event, sys.exc_info()[1])) + async def setup(self): + order.append("child setup") - monkeypatch.setattr("fastcs.controllers.runner.logger.exception", record_exception) + class Parent(Controller): + async def build(self): + order.append("parent build") + self.add_sub_controller("CHILD", Child()) - runner = ControllerRunner(UndisconnectableController()) + async def setup(self): + order.append("parent setup") + + runner = ControllerRunner(Parent()) await runner.start() + try: + assert order == ["parent build", "child build", "parent setup", "child setup"] + finally: + await runner.stop() - await runner.stop() - assert len(logged) == 1 - event, error = logged[0] - assert event == "Exception during disconnect" - assert isinstance(error, RuntimeError) - assert str(error) == "no" +@pytest.mark.asyncio +async def test_build_repeats_until_the_tree_stops_growing(): + class Tier(Controller): + def __init__(self, depth: int) -> None: + super().__init__() + self._depth = depth + + async def build(self): + if self._depth: + self.add_sub_controller("SUB", Tier(self._depth - 1)) + + root = Tier(3) + runner = ControllerRunner(root) + await runner.build() + + controller: Controller = root + for _ in range(3): + controller = controller.sub_controllers["SUB"] # type: ignore[assignment] + assert controller.sub_controllers == {} @pytest.mark.asyncio -async def test_the_runner_reconnects_a_controller_that_dropped_out(monkeypatch): - """Nothing else calls reconnect, so a paused controller would stay paused.""" - monkeypatch.setattr("fastcs.controllers.runner.RECONNECT_PERIOD", 0.01) +async def test_a_tree_that_never_settles_is_caught(): + class Runaway(Controller): + async def build(self): + self.add_sub_controller("SUB", Runaway()) - class DroppingController(LifecycleController): - reconnects = 0 + runner = ControllerRunner(Runaway()) - async def reconnect(self): - self.reconnects += 1 - await super().reconnect() + with pytest.raises(RuntimeError, match=f"{MAX_BUILD_PASSES} build passes"): + await runner.build() + + +@pytest.mark.asyncio +async def test_controllers_sharing_a_connection_open_it_once(): + supervisor, handle = supervise(FakeConnection()) + + class Parent(Controller): + def __init__(self): + super().__init__() + self.add_sub_controller("A", LifecycleController(handle)) + self.add_sub_controller("B", LifecycleController(handle)) + + runner = ControllerRunner(Parent(), [supervisor]) + await runner.build() + + assert supervisor.connection.connects == 1 + + +@pytest.mark.asyncio +async def test_a_connection_the_runner_does_not_supervise_is_rejected(): + """It would never be opened, and so never reconnected either.""" + + class LateConnector(Controller): + async def build(self): + self.add_sub_controller("LATE", LifecycleController(FakeConnection())) + + runner = ControllerRunner(LateConnector()) + + with pytest.raises(RuntimeError, match="does not supervise"): + await runner.build() + + +@pytest.mark.asyncio +async def test_a_controller_may_hold_the_connection_itself(): + """A test can hand over the raw connection, and it is still supervised.""" + supervisor, _ = supervise(FakeConnection(), reconnect_attempts=1000) + controller = Polling(supervisor.connection) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + supervisor.connection.fail_connect = RuntimeError("down") + link_down(supervisor) + await asyncio.sleep(0.01) + polls = controller.polls + await asyncio.sleep(0.01) + + assert controller.polls == polls + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_connection_no_controller_holds_is_warned_about(log_records): + supervisor, _ = supervise(FakeConnection()) + runner = ControllerRunner(LifecycleController(), [supervisor]) + + await runner.build() + try: + assert any( + "not held by any controller" in r["event"] + and r["connection"] == "FakeConnection" + for r in log_records + ) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_connection_nothing_polls_is_warned_about(log_records): + supervisor, handle = supervise(FakeConnection()) + runner = ControllerRunner(LifecycleController(handle), [supervisor]) - controller = DroppingController() - runner = ControllerRunner(controller) await runner.start() try: - assert controller.connected + assert any("no polled attribute" in r["event"] for r in log_records) + finally: + await runner.stop() - # What a scan task does when its callback raises - controller._connected = False - await asyncio.sleep(0.05) +@pytest.mark.asyncio +async def test_a_polled_connection_is_not_warned_about(log_records): + supervisor, handle = supervise(FakeConnection()) + runner = ControllerRunner(Polling(handle), [supervisor]) - assert controller.reconnects >= 1 - assert controller.connected + await runner.start() + try: + assert not any("no polled attribute" in r["event"] for r in log_records) finally: await runner.stop() +# The seal + + +@pytest.mark.asyncio +async def test_the_tree_is_sealed_after_build(): + controller = LifecycleController() + runner = ControllerRunner(controller) + await runner.build() + + with pytest.raises(RuntimeError, match="sealed"): + controller.add_attribute("late", AttrR(int)) + + +@pytest.mark.asyncio +async def test_a_sub_controller_is_sealed_too(): + child = LifecycleController() + parent = Controller() + parent.add_sub_controller("CHILD", child) + runner = ControllerRunner(parent) + await runner.build() + + with pytest.raises(RuntimeError, match="sealed"): + child.add_sub_controller("LATE", Controller()) + + @pytest.mark.asyncio -async def test_a_failing_reconnect_does_not_stop_the_runner(monkeypatch): - monkeypatch.setattr("fastcs.controllers.runner.RECONNECT_PERIOD", 0.01) +async def test_adding_in_setup_raises(): + class Late(Controller): + async def setup(self): + self.late = AttrR(int) + + runner = ControllerRunner(Late()) - class UnreconnectableController(LifecycleController): - attempts = 0 + with pytest.raises(RuntimeError, match="sealed"): + await runner.start() - async def reconnect(self): - self.attempts += 1 - raise RuntimeError("still down") - controller = UnreconnectableController() +@pytest.mark.asyncio +async def test_every_controller_gets_a_connected_attribute(): + supervisor, handle = supervise(FakeConnection()) + parent = Controller() + parent.add_sub_controller("CHILD", LifecycleController(handle)) + runner = ControllerRunner(parent, [supervisor]) + + apis = await runner.build() + + assert all(CONNECTED_ATTRIBUTE in api.attributes for api in apis[0].walk_api()) + + +@pytest.mark.asyncio +async def test_a_soft_controller_is_always_connected(): + controller = LifecycleController() runner = ControllerRunner(controller) + + await runner.start() + connected = controller.attributes[CONNECTED_ATTRIBUTE] + assert isinstance(connected, AttrR) + try: + assert connected.readback is True + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_connected_goes_off_when_the_link_drops(): + supervisor, handle = supervise(FakeConnection(), reconnect_period=0.001) + controller = LifecycleController(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + connected = controller.attributes[CONNECTED_ATTRIBUTE] + assert isinstance(connected, AttrR) + try: + supervisor.connection.fail_connect = RuntimeError("down") + link_down(supervisor) + + await eventually(lambda: connected.readback is False) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_connected_comes_back_on_with_the_link(): + supervisor, handle = supervise(FakeConnection(), reconnect_period=0.001) + controller = LifecycleController(handle) + runner = ControllerRunner(controller, [supervisor]) await runner.start() + connected = controller.attributes[CONNECTED_ATTRIBUTE] + assert isinstance(connected, AttrR) try: - controller._connected = False - await asyncio.sleep(0.05) + link_down(supervisor) + await asyncio.wait_for(supervisor.wait_up(), timeout=2) - # It keeps trying rather than dying on the first failure - assert controller.attempts > 1 - assert not controller.connected + await eventually(lambda: connected.readback is True) finally: await runner.stop() +@pytest.mark.asyncio +async def test_a_controller_with_its_own_connected_member_is_rejected(): + class Clashing(Controller): + def __init__(self): + super().__init__() + self.connected = AttrR(bool) + + runner = ControllerRunner(Clashing()) + + with pytest.raises(RuntimeError, match="already has a member named"): + await runner.build() + + +# Scans + + +@pytest.mark.asyncio +async def test_scans_pause_while_their_connection_is_down(): + supervisor, handle = supervise(FakeConnection(), reconnect_attempts=1000) + controller = Polling(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + supervisor.connection.fail_connect = RuntimeError("down") + link_down(supervisor) + await asyncio.sleep(0.01) + polls = controller.polls + await asyncio.sleep(0.02) + + assert controller.polls == polls + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_scans_resume_after_a_reconnect(): + supervisor, handle = supervise(FakeConnection(), reconnect_period=0.001) + controller = Polling(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + link_down(supervisor) + await asyncio.wait_for(supervisor.wait_up(), timeout=2) + polls = controller.polls + + await eventually(lambda: controller.polls > polls) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_child_keeps_scanning_when_its_parents_link_drops(): + """Each connection is paused on its own, not the whole tree on the root's.""" + root_supervisor, root_handle = supervise(BaseLink(), reconnect_attempts=1000) + child_supervisor, child_handle = supervise(FakeConnection()) + root = Polling(root_handle) + child = Polling(child_handle) + root.add_sub_controller("CHILD", child) + runner = ControllerRunner(root, [root_supervisor, child_supervisor]) + await runner.start() + try: + root_supervisor.connection.fail_connect = RuntimeError("down") + link_down(root_supervisor) + await asyncio.sleep(0.01) + polls = child.polls + + await eventually(lambda: child.polls > polls) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_child_pauses_when_its_own_link_drops_under_a_healthy_parent(): + root_supervisor, root_handle = supervise(BaseLink()) + child_supervisor, child_handle = supervise( + FakeConnection(), reconnect_attempts=1000 + ) + root = Polling(root_handle) + child = Polling(child_handle) + root.add_sub_controller("CHILD", child) + runner = ControllerRunner(root, [root_supervisor, child_supervisor]) + await runner.start() + try: + child_supervisor.connection.fail_connect = RuntimeError("down") + link_down(child_supervisor) + await asyncio.sleep(0.01) + polls = child.polls + await asyncio.sleep(0.02) + + assert child.polls == polls + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_soft_parent_does_not_gate_its_children(): + child_supervisor, child_handle = supervise(FakeConnection()) + root = Controller() + child = Polling(child_handle) + root.add_sub_controller("CHILD", child) + runner = ControllerRunner(root, [child_supervisor]) + await runner.start() + try: + polls = child.polls + + await eventually(lambda: child.polls > polls) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_poll_that_hits_a_dead_link_takes_the_link_down(): + supervisor, handle = supervise(FakeConnection(), reconnect_attempts=1000) + runner = ControllerRunner(Polling(handle), [supervisor]) + await runner.start() + try: + supervisor.connection.fail_connect = RuntimeError("down") + supervisor.connection.fail_io = ConnectionResetError() + + await eventually(lambda: not supervisor.up) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_polled_values_are_flagged_invalid_when_the_link_drops(): + supervisor, handle = supervise(FakeConnection(), reconnect_attempts=1000) + controller = Polling(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + supervisor.connection.fail_connect = RuntimeError("down") + link_down(supervisor) + + await eventually(lambda: controller.value.severity is Severity.INVALID) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_good_poll_after_a_reconnect_clears_the_flag(): + supervisor, handle = supervise( + FakeConnection(), reconnect_period=0.001, reconnect_attempts=10_000 + ) + controller = Polling(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + # Hold the reconnect off until the flag has been seen, or it can be set + # and cleared again between two checks + supervisor.connection.fail_connect = RuntimeError("down") + link_down(supervisor) + await eventually(lambda: controller.value.severity is Severity.INVALID) + + supervisor.connection.fail_connect = None + await eventually(lambda: controller.value.severity is Severity.NO_ALARM) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_read_once_reads_run_again_after_a_reconnect(): + """A rebooted device can come back with different values.""" + supervisor, handle = supervise(FakeConnection(), reconnect_period=0.001) + controller = LifecycleController(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + link_down(supervisor) + + await eventually(lambda: controller.count.readback == 2) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_setup_does_not_run_again_after_a_reconnect(): + supervisor, handle = supervise(FakeConnection(), reconnect_period=0.001) + controller = LifecycleController(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + link_down(supervisor) + await eventually(lambda: controller.count.readback == 2) + + assert controller.events.count("setup") == 1 + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_raising_scan_is_retried(): + calls = 0 + + class Failing(Controller): + @scan(0.001) + async def failing(self): + nonlocal calls + calls += 1 + raise RuntimeError("scan error") + + runner = ControllerRunner(Failing()) + await runner.start() + try: + await eventually(lambda: calls > 3) + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_scan_failing_the_same_way_is_logged_once(log_records): + calls = 0 + + class Failing(Controller): + @scan(0.001) + async def failing(self): + nonlocal calls + calls += 1 + raise RuntimeError("scan error") + + runner = ControllerRunner(Failing()) + await runner.start() + try: + await eventually(lambda: calls > 3) + + assert [r["event"] for r in log_records].count("Scan failed") == 1 + finally: + await runner.stop() + + +@pytest.mark.asyncio +async def test_a_put_on_a_dead_link_fails_back_to_the_caller(): + supervisor, handle = supervise(FakeConnection(), reconnect_attempts=1000) + controller = Polling(handle) + runner = ControllerRunner(controller, [supervisor]) + await runner.start() + try: + supervisor.connection.fail_connect = RuntimeError("down") + link_down(supervisor) + + with pytest.raises(DisconnectedError): + await controller.value.set(5) + finally: + await runner.stop() + + +# Dependencies, by type + + +@pytest.mark.asyncio +async def test_connections_are_opened_in_dependency_order(): + """However they were declared.""" + opened: list[str] = [] + + class RecordedBase(BaseLink): + async def connect(self) -> None: + opened.append("base") + + class RecordedLayered(LayeredLink): + async def connect(self) -> None: + opened.append("layered") + + layered, layered_handle = supervise(RecordedLayered()) + base, base_handle = supervise(RecordedBase()) + root = Controller() + root.add_sub_controller("LAYERED", LifecycleController(layered_handle)) + root.add_sub_controller("BASE", LifecycleController(base_handle)) + runner = ControllerRunner(root, [layered, base]) + + await runner.build() + + assert opened == ["base", "layered"] + + +@pytest.mark.asyncio +async def test_a_dependency_is_every_connection_of_the_type(): + first, _ = supervise(BaseLink()) + second, _ = supervise(BaseLink()) + layered, _ = supervise(LayeredLink()) + runner = ControllerRunner(LifecycleController(), [first, second, layered]) + + await runner.build() + + assert layered.dependencies == [first, second] + + +@pytest.mark.asyncio +async def test_a_subclass_satisfies_a_dependency(): + class SpecialBase(BaseLink): + pass + + base, _ = supervise(SpecialBase()) + layered, _ = supervise(LayeredLink()) + runner = ControllerRunner(LifecycleController(), [base, layered]) + + await runner.build() + + assert layered.dependencies == [base] + + +@pytest.mark.asyncio +async def test_a_dependency_with_no_instance_is_not_waited_for(): + layered, _ = supervise(LayeredLink()) + runner = ControllerRunner(LifecycleController(), [layered]) + + await runner.build() + + assert layered.dependencies == [] + + +@pytest.mark.asyncio +async def test_dependencies_are_resolved_within_one_top_level_controller(): + base_a, _ = supervise(BaseLink()) + layered_a, _ = supervise(LayeredLink()) + base_b, _ = supervise(BaseLink()) + runner = ControllerRunner( + [LifecycleController(), LifecycleController()], + [[base_a, layered_a], [base_b]], + ) + + await runner.build() + + assert layered_a.dependencies == [base_a] + + +@pytest.mark.asyncio +async def test_a_dependency_cycle_is_caught_at_startup(): + class Chicken(FakeConnection): + pass + + class Egg(FakeConnection): + depends_on = [Chicken] + + Chicken.depends_on = [Egg] + + chicken, _ = supervise(Chicken()) + egg, _ = supervise(Egg()) + runner = ControllerRunner(LifecycleController(), [chicken, egg]) + + with pytest.raises(ValueError, match="Cycle in connection dependencies"): + await runner.build() + + +@pytest.mark.asyncio +async def test_a_dependent_waits_for_its_dependency_to_reconnect(): + base, base_handle = supervise( + BaseLink(), reconnect_period=0.001, reconnect_attempts=1000 + ) + layered, layered_handle = supervise(LayeredLink(), reconnect_period=0.001) + root = Controller() + root.add_sub_controller("BASE", Polling(base_handle)) + root.add_sub_controller("LAYERED", Polling(layered_handle)) + runner = ControllerRunner(root, [base, layered]) + await runner.start() + try: + base.connection.fail_connect = RuntimeError("down") + link_down(base) + link_down(layered) + await asyncio.sleep(0.02) + + assert layered.connection.connects == 1 + finally: + await runner.stop() + + +# Supervisor groups + + +def test_one_group_per_controller_is_required_when_grouping(): + supervisor, _ = supervise(FakeConnection()) + + with pytest.raises(ValueError, match="groups of supervisors"): + ControllerRunner([LifecycleController(), LifecycleController()], [[supervisor]]) + + +def test_a_supervisor_given_twice_is_rejected(): + supervisor, _ = supervise(FakeConnection()) + + with pytest.raises(ValueError, match="twice"): + ControllerRunner(LifecycleController(), [supervisor, supervisor]) + + +# Stopping, and failing to start + + +@pytest.mark.asyncio +async def test_stop_closes_connections_in_reverse_of_opening(): + closed: list[str] = [] + + class RecordedBase(BaseLink): + async def close(self) -> None: + closed.append("base") + + class RecordedLayered(LayeredLink): + async def close(self) -> None: + closed.append("layered") + + base, _ = supervise(RecordedBase()) + layered, _ = supervise(RecordedLayered()) + runner = ControllerRunner(LifecycleController(), [layered, base]) + await runner.start() + + await runner.stop() + + assert closed == ["layered", "base"] + + +@pytest.mark.asyncio +async def test_stop_reports_a_failing_close_without_raising(monkeypatch): + class UncloseableConnection(FakeConnection): + async def close(self) -> None: + raise RuntimeError("no") + + logged: list[tuple[str, BaseException | None]] = [] + + def record_exception(event, **kwargs): + # ``logger.exception`` is called from the ``except`` block, so the + # exception it is reporting is the one currently being handled. + logged.append((event, sys.exc_info()[1])) + + monkeypatch.setattr("fastcs.controllers.runner.logger.exception", record_exception) + + supervisor, handle = supervise(UncloseableConnection()) + runner = ControllerRunner(LifecycleController(handle), [supervisor]) + await runner.start() + + await runner.stop() + + assert [(event, str(error)) for event, error in logged] == [ + ("Exception while closing connection", "no") + ] + + @pytest.mark.asyncio async def test_stop_cancels_the_tasks(): - controller = LifecycleController() - runner = ControllerRunner(controller) + supervisor, handle = supervise(FakeConnection()) + runner = ControllerRunner(Polling(handle), [supervisor]) await runner.start() - tasks = set(runner._tasks) - assert tasks + tasks = set(supervisor._tasks) # noqa: SLF001 await runner.stop() await asyncio.sleep(0) assert all(task.cancelled() or task.done() for task in tasks) - assert not runner._tasks -def test_reconnect_period_is_a_second_by_default(): - assert RECONNECT_PERIOD == 1.0 +@pytest.mark.asyncio +async def test_a_connection_that_fails_to_open_aborts_startup(): + supervisor, handle = supervise(FakeConnection()) + supervisor.connection.fail_connect = ConnectionRefusedError() + runner = ControllerRunner(LifecycleController(handle), [supervisor]) + + with pytest.raises(ConnectionRefusedError): + await runner.build() + + +@pytest.mark.asyncio +async def test_a_connection_that_fails_to_open_closes_those_opened_before_it(): + first, _ = supervise(FakeConnection()) + second, _ = supervise(FakeConnection()) + second.connection.fail_connect = ConnectionRefusedError() + runner = ControllerRunner(LifecycleController(), [first, second]) + + with pytest.raises(ConnectionRefusedError): + await runner.build() + + assert first.connection.closes == 1 + + +@pytest.mark.asyncio +async def test_a_failed_build_closes_what_it_opened(): + """Startup aborts, and no task exists yet for a later `stop` to clean up.""" + supervisor, handle = supervise(FakeConnection()) + + class Unbuildable(Controller): + def __init__(self): + self.connection = handle + super().__init__() + + async def build(self): + raise RuntimeError("cannot build") + + runner = ControllerRunner(Unbuildable(), [supervisor]) + + with pytest.raises(RuntimeError, match="cannot build"): + await runner.build() + + assert supervisor.connection.closes == 1 + + +@pytest.mark.asyncio +async def test_a_failed_setup_closes_what_it_opened(): + supervisor, handle = supervise(FakeConnection()) + + class Unsetuppable(Controller): + def __init__(self): + self.connection = handle + super().__init__() + + async def setup(self): + raise RuntimeError("cannot set up") + + runner = ControllerRunner(Unsetuppable(), [supervisor]) + + with pytest.raises(RuntimeError, match="cannot set up"): + await runner.start() + + assert supervisor.connection.closes == 1 + + +@pytest.mark.asyncio +async def test_a_fatal_failure_is_reported_by_the_runner(): + supervisor, handle = supervise( + FakeConnection(), reconnect_period=0.001, policy=DRAPolicy() + ) + runner = ControllerRunner(LifecycleController(handle), [supervisor]) + await runner.start() + try: + error = FileNotFoundError("/dev/ttyACM0") + supervisor.connection.fail_connect = error + link_down(supervisor) + + await asyncio.wait_for(runner.fatal_error.wait(), timeout=2) + + assert runner.fatal_reason is error + finally: + await runner.stop() diff --git a/tests/test_controllers.py b/tests/test_controllers.py index fb8575c21..d6f05db11 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -1,4 +1,3 @@ -import asyncio import enum import pytest @@ -371,30 +370,3 @@ async def scan_nothing(self): assert list(api.attributes) == ["attr1", "attr2"] assert list(api.command_methods) == ["do_nothing"] assert list(api.scan_methods) == ["scan_nothing"] - - -@pytest.mark.asyncio -async def test_scan_exception_sets_disconnected_and_reconnect_resumes(): - class MyTestController(Controller): - @scan(0.01) - async def failing_scan(self): - raise RuntimeError("scan error") - - controller = MyTestController() - controller.post_initialise() - _, scan_coros, _ = controller.create_api_and_tasks() - - controller._connected = True - task = asyncio.create_task(scan_coros[0]()) - - # Wait long enough for the scan to run and raise, setting _connected = False - await asyncio.sleep(0.1) - assert not controller._connected - - # Trigger reconnect - _connected resumes scan tasks - await controller.reconnect() - assert controller._connected - - task.cancel() - with pytest.raises(asyncio.CancelledError): - await task diff --git a/tests/test_http_connection.py b/tests/test_http_connection.py new file mode 100644 index 000000000..8cda1a591 --- /dev/null +++ b/tests/test_http_connection.py @@ -0,0 +1,131 @@ +import httpx +import pytest +import pytest_asyncio + +from fastcs.connections import ( + DisconnectedError, + HTTPConnection, + HTTPConnectionSettings, + Supervisor, +) + + +def _handler(request: httpx.Request) -> httpx.Response: + """A device that answers a couple of paths and complains about the rest.""" + if request.url.path == "/value": + return httpx.Response(200, json={"value": 1.5}) + if request.url.path == "/frame": + return httpx.Response(200, content=b"\x00\x01\x02") + if request.url.path == "/set": + return httpx.Response(200, json={"applied": True}) + if request.url.path == "/nothing": + return httpx.Response(204) + return httpx.Response(404, json={"error": "no such parameter"}) + + +class FakeDevice(HTTPConnection): + """Points the connection at an in-process handler rather than a socket.""" + + def __init__(self, handler=_handler) -> None: + super().__init__() + self._transport = httpx.MockTransport(handler) + + +@pytest_asyncio.fixture +async def device(): + connection = FakeDevice() + await connection.connect() + yield connection + await connection.close() + + +def test_settings_build_the_base_url(): + settings = HTTPConnectionSettings(host="detector", port=8080, scheme="https") + + assert settings.base_url == "https://detector:8080" + + +def test_the_defaults_are_a_local_http_device(): + assert HTTPConnectionSettings().base_url == "http://127.0.0.1:80" + + +@pytest.mark.asyncio +async def test_using_it_before_it_is_open_says_so(): + """The same failure as any other connection: not open is not a device fault.""" + with pytest.raises(DisconnectedError, match="connect"): + await FakeDevice().get("/value") + + +@pytest.mark.asyncio +async def test_get_returns_the_parsed_body(device: FakeDevice): + assert await device.get("/value") == {"value": 1.5} + + +@pytest.mark.asyncio +async def test_get_bytes_returns_the_raw_body(device: FakeDevice): + """Frame and file data is not JSON.""" + assert await device.get_bytes("/frame") == b"\x00\x01\x02" + + +@pytest.mark.asyncio +async def test_put_returns_the_body_when_there_is_one(device: FakeDevice): + assert await device.put("/set", {"value": 2.0}) == {"applied": True} + + +@pytest.mark.asyncio +async def test_put_returns_none_when_the_device_answers_with_nothing( + device: FakeDevice, +): + assert await device.put("/nothing", 1) is None + + +@pytest.mark.asyncio +async def test_an_error_status_is_a_device_complaint_not_a_dead_link(): + """A 404 is the device rejecting one parameter, so the link stays up.""" + supervisor = Supervisor(FakeDevice()) + await supervisor.open() + + with pytest.raises(httpx.HTTPStatusError): + await supervisor.handle.get("/missing") + + assert supervisor.up + + +@pytest.mark.asyncio +async def test_a_transport_failure_marks_the_connection_down(): + def refuse(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("connection refused", request=request) + + supervisor = Supervisor(FakeDevice(refuse)) + await supervisor.open() + + with pytest.raises(DisconnectedError): + await supervisor.handle.get("/value") + + assert not supervisor.up + + +@pytest.mark.asyncio +async def test_closing_twice_is_allowed(device: FakeDevice): + """`close` is called before every reconnect attempt, on whatever state.""" + await device.close() + await device.close() + + with pytest.raises(DisconnectedError): + await device.get("/value") + + +@pytest.mark.asyncio +async def test_a_subclass_reshapes_the_response_without_touching_health(): + """The Eiger case: the envelope is the device's convention, not HTTP's.""" + + class Unwrapping(FakeDevice): + async def get(self, path: str): + return (await super().get(path))["value"] + + connection = Unwrapping() + await connection.connect() + try: + assert await connection.get("/value") == 1.5 + finally: + await connection.close() diff --git a/tests/test_ip_connection.py b/tests/test_ip_connection.py index 3770174f1..3c0c3b8b0 100644 --- a/tests/test_ip_connection.py +++ b/tests/test_ip_connection.py @@ -2,7 +2,7 @@ import pytest -from fastcs.connections.ip_connection import IPConnection +from fastcs.connections import DisconnectedError, IPConnection, Supervisor @pytest.fixture @@ -50,3 +50,39 @@ async def test_close_connected_and_connection_reset(connection): await conn.close() assert conn._IPConnection__connection is None + + +def _answering(response: str) -> IPConnection: + conn = IPConnection() + mock_stream = MagicMock() + mock_stream.__aenter__ = AsyncMock(return_value=mock_stream) + mock_stream.__aexit__ = AsyncMock(return_value=False) + mock_stream.send_message = AsyncMock() + mock_stream.receive_response = AsyncMock(return_value=response) + conn._IPConnection__connection = mock_stream # pyright: ignore[reportAttributeAccessIssue] + # Already open, as the mock stream is + conn.connect = AsyncMock() + return conn + + +@pytest.mark.asyncio +async def test_a_peer_that_closes_instead_of_answering_marks_the_link_down(): + """``readline`` returns b"" at EOF, which is a dead link, not an empty reply.""" + supervisor = Supervisor(_answering("")) + await supervisor.open() + + with pytest.raises(DisconnectedError): + await supervisor.handle.send_query("ID?\r\n") + + # Without this the caller just gets "", fails to parse it, and retries forever + # while the reconnect loop stays idle. + assert not supervisor.up + + +@pytest.mark.asyncio +async def test_a_real_response_leaves_the_link_up(): + supervisor = Supervisor(_answering("ID=1\r\n")) + await supervisor.open() + + assert await supervisor.handle.send_query("ID?\r\n") == "ID=1\r\n" + assert supervisor.up diff --git a/tests/test_launch.py b/tests/test_launch.py index 987c6bff8..f5a46a914 100644 --- a/tests/test_launch.py +++ b/tests/test_launch.py @@ -11,11 +11,19 @@ from fastcs import __version__ from fastcs.attributes import AttrR +from fastcs.connections import ( + DEFAULT_RECONNECT_ATTEMPTS, + Connection, + HTTPConnection, + Supervisor, +) +from fastcs.connections.supervisor import connection_of, supervisor_of from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.exceptions import LaunchError from fastcs.launch import ( _build_options_model, + _instantiate_controllers, _launch, get_controller_schema, launch, @@ -67,6 +75,48 @@ def __init__(self, arg: SomeConfig) -> None: super().__init__() +@dataclass +class LinkSettings: + host: str + port: int = 22 + + +class FakeConnection(Connection): + def __init__(self, settings: LinkSettings) -> None: + self.settings = settings + + async def connect(self) -> None: ... + + async def close(self) -> None: ... + + +class OtherConnection(Connection): + def __init__(self, tag: str = "untagged") -> None: + self.tag = tag + + async def connect(self) -> None: ... + + async def close(self) -> None: ... + + +class NeedsConnections(Controller): + """The common shape: connections declared by type hints, and nothing else.""" + + def __init__(self, link: FakeConnection, other: OtherConnection) -> None: + super().__init__() + self.link = link + self.other = other + + +class NeedsBoth(Controller): + """A connection alongside an options object.""" + + def __init__(self, link: FakeConnection, arg: SomeConfig) -> None: + super().__init__() + self.link = link + self.arg = arg + + runner = CliRunner() @@ -131,8 +181,9 @@ def test_not_hinted_schema(): def test_over_defined_schema(): error = ( "" - "Expected no more than 2 arguments for 'ManyArgs.__init__' " - "but received 3 as `(self, arg: tests.test_launch.SomeConfig, too_many)`" + "Expected no more than one options argument for 'ManyArgs.__init__', " + "besides its connections, but received 2 as " + "`(self, arg: tests.test_launch.SomeConfig, too_many)`" ) with pytest.raises(LaunchError) as exc_info: @@ -320,3 +371,379 @@ def test_multi_controller_run_reaches_fastcs(mocker: MockerFixture, tmp_path): controllers_arg = init_spy.call_args.args[1] assert [c.path[0] for c in controllers_arg] == ["one", "two"] assert [type(c) for c in controllers_arg] == [IsHinted, OtherHinted] + + +# `connections:` in the config, and injection + +LINK = {"settings": {"host": "h"}} + + +def _build( + controllers: list[dict], classes=None +) -> tuple[list[Controller], list[list[Supervisor]]]: + """Validate a `controllers:` list and instantiate it, as ``run`` does.""" + options_model = _build_options_model(classes or [NeedsConnections]) + instance = options_model.model_validate( + {"controllers": controllers, "transport": [{"rest": {}}]} + ) + return _instantiate_controllers(_controllers(instance)) + + +def _validate(controllers: list[dict], classes=None) -> None: + options_model = _build_options_model(classes or [NeedsConnections]) + options_model.model_validate( + {"controllers": controllers, "transport": [{"rest": {}}]} + ) + + +def test_a_controller_receives_a_handle_for_each_connection(): + (controllers, _) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}}, + } + ] + ) + (controller,) = controllers + assert isinstance(controller, NeedsConnections) + + assert supervisor_of(controller.link) is not None + + +def test_a_handle_passes_as_the_hinted_type(): + (controllers, _) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}}, + } + ] + ) + (controller,) = controllers + assert isinstance(controller, NeedsConnections) + + assert isinstance(controller.link, FakeConnection) + + +def test_a_connection_is_built_from_its_settings(): + (controllers, _) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": { + "link": {"settings": {"host": "192.168.0.2", "port": 23}}, + "other": {"tag": "tagged"}, + }, + } + ] + ) + (controller,) = controllers + assert isinstance(controller, NeedsConnections) + + assert controller.link.settings == LinkSettings(host="192.168.0.2", port=23) + + +def test_the_supervisors_of_each_entry_are_returned_in_argument_order(): + (controllers, supervisors) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"other": {}, "link": LINK}, + } + ] + ) + (controller,) = controllers + assert isinstance(controller, NeedsConnections) + + assert supervisors == [ + [supervisor_of(controller.link), supervisor_of(controller.other)] + ] + + +def test_a_supervisor_is_named_after_its_entry_and_argument(): + (_, supervisors) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}}, + } + ] + ) + + assert [s.name for s in supervisors[0]] == ["x.link", "x.other"] + + +def test_reconnect_settings_are_read_by_the_supervisor(): + (_, supervisors) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": { + "link": {**LINK, "reconnect_attempts": 5, "reconnect_period": 2.0}, + "other": {}, + }, + } + ] + ) + link = supervisors[0][0] + + assert (link.reconnect_attempts, link.reconnect_period) == (5, 2.0) + + +def test_reconnect_settings_default_per_connection(): + (_, supervisors) = _build( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}}, + } + ] + ) + + assert supervisors[0][0].reconnect_attempts == DEFAULT_RECONNECT_ATTEMPTS + + +def test_connections_are_created_per_entry(): + """The same class used twice in one config gets two independent links.""" + (controllers, _) = _build( + [ + { + "id": "PITCH", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}}, + }, + { + "id": "YAW", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}}, + }, + ] + ) + first, second = controllers + assert isinstance(first, NeedsConnections) + assert isinstance(second, NeedsConnections) + + assert connection_of(first.link) is not connection_of(second.link) + + +def test_connections_alongside_an_options_object(): + (controllers, _) = _build( + [ + { + "id": "x", + "type": "tests.NeedsBoth", + "name": "a-name", + "connections": {"link": LINK}, + } + ], + classes=[NeedsBoth], + ) + (controller,) = controllers + assert isinstance(controller, NeedsBoth) + + assert controller.arg == SomeConfig(name="a-name") + + +def test_every_connection_is_required(): + with pytest.raises(ValidationError, match="other"): + _validate( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"link": LINK}, + } + ] + ) + + +def test_an_undeclared_connection_name_is_rejected(): + with pytest.raises(ValidationError, match="typo"): + _validate( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": {"link": LINK, "other": {}, "typo": {}}, + } + ] + ) + + +def test_an_unknown_connection_setting_is_rejected(): + with pytest.raises(ValidationError, match="depends_on"): + _validate( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": { + "link": {**LINK, "depends_on": "other"}, + "other": {}, + }, + } + ] + ) + + +def test_a_connection_type_is_not_configurable(): + """The type hint supplies it.""" + with pytest.raises(ValidationError, match="type"): + _validate( + [ + { + "id": "x", + "type": "tests.NeedsConnections", + "connections": { + "link": {**LINK, "type": "tests.FakeConnection"}, + "other": {}, + }, + } + ] + ) + + +def test_connections_for_a_controller_that_takes_none(): + with pytest.raises(ValidationError, match="connections"): + _validate( + [ + { + "id": "x", + "type": "tests.IsHinted", + "name": "n", + "connections": {"link": LINK}, + } + ], + classes=[IsHinted], + ) + + +def test_connections_is_a_reserved_options_field(): + @dataclass + class Colliding: + connections: str + + class Collides(Controller): + def __init__(self, arg: Colliding) -> None: + super().__init__() + + with pytest.raises(LaunchError, match="'connections' field"): + _build_options_model([Collides]) + + +def test_no_connections_block_for_a_controller_that_takes_none(): + schema = get_controller_schema(IsHinted) + entry = schema["$defs"]["IsHintedEntry"] + assert "connections" not in entry["properties"] + + +def test_the_connections_block_in_the_schema_names_each_argument(): + schema = get_controller_schema(NeedsConnections) + block = schema["$defs"]["NeedsConnectionsConnections"] + + assert block["required"] == ["link", "other"] + + +def test_the_framework_keys_are_in_a_connections_schema(): + schema = get_controller_schema(NeedsConnections) + connection = schema["$defs"]["FakeConnectionConfig"] + + assert {"reconnect_attempts", "reconnect_period"} <= set(connection["properties"]) + + +def test_a_connection_argument_without_a_type_hint(): + class Unhinted(Connection): + def __init__(self, settings) -> None: ... + + async def connect(self) -> None: ... + + async def close(self) -> None: ... + + class NeedsUnhinted(Controller): + def __init__(self, link: Unhinted) -> None: + super().__init__() + + with pytest.raises(LaunchError, match="Add a typehint for `settings`"): + _build_options_model([NeedsUnhinted]) + + +def test_a_connection_taking_star_args(): + class Starred(Connection): + def __init__(self, *settings: str) -> None: ... + + async def connect(self) -> None: ... + + async def close(self) -> None: ... + + class NeedsStarred(Controller): + def __init__(self, link: Starred) -> None: + super().__init__() + + with pytest.raises(LaunchError, match=r"`\*settings: str` cannot be expressed"): + _build_options_model([NeedsStarred]) + + +def test_a_connection_argument_colliding_with_a_framework_key(): + class Colliding(Connection): + def __init__(self, reconnect_period: float) -> None: ... + + async def connect(self) -> None: ... + + async def close(self) -> None: ... + + class NeedsColliding(Controller): + def __init__(self, link: Colliding) -> None: + super().__init__() + + with pytest.raises(LaunchError, match="collides with a launch-framework key"): + _build_options_model([NeedsColliding]) + + +def test_a_connection_with_no_constructor_has_only_framework_settings(): + class Bare(Connection): + async def connect(self) -> None: ... + + async def close(self) -> None: ... + + class NeedsBare(Controller): + def __init__(self, link: Bare) -> None: + super().__init__() + + schema = get_controller_schema(NeedsBare) + + assert set(schema["$defs"]["BareConfig"]["properties"]) == { + "reconnect_attempts", + "reconnect_period", + } + + +def test_the_framework_http_connection_is_usable_from_config(): + """A REST driver hints `HTTPConnection` and writes no connection at all.""" + + class NeedsHTTP(Controller): + def __init__(self, odin: HTTPConnection) -> None: + super().__init__() + self.connection = odin + + (controllers, _) = _build( + [ + { + "id": "OD", + "type": "tests.NeedsHTTP", + "connections": {"odin": {"settings": {"host": "odin", "port": 8888}}}, + } + ], + classes=[NeedsHTTP], + ) + + connection = connection_of(controllers[0].connection) + assert isinstance(connection, HTTPConnection) + assert connection._settings.base_url == "http://odin:8888" diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 7b561773f..c1488b1a9 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -10,6 +10,7 @@ from pytest_mock import MockerFixture from fastcs.attributes import AttrR +from fastcs.connections import Connection, Supervisor from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.transports.epics import EpicsDocsOptions, EpicsGUIOptions @@ -38,7 +39,7 @@ def test_controller_api_path_uses_id(): controller.add_sub_controller("Sub", sub) controller.set_path(["X"]) - api, _, _ = controller.create_api_and_tasks() + api = controller.create_api() assert api.path == ["X"] assert api.sub_apis["Sub"].path == ["X", "Sub"] @@ -47,7 +48,7 @@ def test_controller_api_path_uses_id(): def _api_with_id(controller_class: type[Controller], id: str): controller = controller_class() controller.set_path([id]) - api, _, _ = controller.create_api_and_tasks() + api = controller.create_api() return api @@ -299,28 +300,37 @@ class names, so ``DEV-1`` and ``DEV_1`` would silently override each other in assert "'DEV_1'" in message +class _LifecycleConnection(Connection): + """Records whether the runner opened and closed the link.""" + + def __init__(self): + self.open = False + + async def connect(self) -> None: + self.open = True + + async def close(self) -> None: + self.open = False + + class _LifecycleController(Controller): """Records lifecycle hook calls for end-to-end assertions.""" + connection: _LifecycleConnection + foo: AttrR[int] - def __init__(self): + def __init__(self, connection: _LifecycleConnection): + self.connection = connection super().__init__() - self.connect_called = False - self.initialised = False - self.post_initialised = False - - async def initialise(self): - self.initialised = True + self.built = False + self.set_up = False - def post_initialise(self): - self.post_initialised = True + async def build(self): + self.built = True - async def connect(self): - self.connect_called = True - - async def disconnect(self): - self.connect_called = False + async def setup(self): + self.set_up = True class _OtherLifecycleController(_LifecycleController): @@ -331,9 +341,11 @@ class _OtherLifecycleController(_LifecycleController): async def test_fastcs_serves_two_controllers_end_to_end(mocker: MockerFixture): """FastCS.serve drives lifecycle on every controller and routes REST traffic per-id; combined OpenAPI describes both prefixes.""" - a = _LifecycleController() + supervisor_a = Supervisor(_LifecycleConnection()) + a = _LifecycleController(supervisor_a.handle) a.set_path(["alpha"]) - b = _OtherLifecycleController() + supervisor_b = Supervisor(_LifecycleConnection()) + b = _OtherLifecycleController(supervisor_b.handle) b.set_path(["beta"]) transport = RestTransport() @@ -341,15 +353,18 @@ async def test_fastcs_serves_two_controllers_end_to_end(mocker: MockerFixture): # app directly through TestClient. mocker.patch.object(RestTransport, "serve", new=lambda self: asyncio.sleep(3600)) - fastcs = FastCS([a, b], [transport], asyncio.get_event_loop()) + # One group of supervisors per entry, as the launcher builds them. + fastcs = FastCS( + [a, b], [transport], asyncio.get_event_loop(), [[supervisor_a], [supervisor_b]] + ) 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.connect_called + assert controller.built + assert controller.set_up + assert controller.connection.open with TestClient(transport._server._app) as client: assert client.get("/alpha/foo").status_code == 200 @@ -369,4 +384,4 @@ async def test_fastcs_serves_two_controllers_end_to_end(mocker: MockerFixture): pass for controller in (a, b): - assert not controller.connect_called + assert not controller.connection.open diff --git a/tests/test_supervisor.py b/tests/test_supervisor.py new file mode 100644 index 000000000..db7e0a98c --- /dev/null +++ b/tests/test_supervisor.py @@ -0,0 +1,573 @@ +import asyncio +from collections.abc import Iterator +from typing import Any + +import pytest + +from fastcs.connections import ( + DEFAULT_RECONNECT_ATTEMPTS, + DEFAULT_RECONNECT_PERIOD, + Connection, + ConnectionPolicy, + DisconnectedError, + DRAPolicy, + Supervisor, +) +from fastcs.connections.supervisor import connection_of, supervisor_of +from fastcs.logging import logger + + +class FakeConnection(Connection): + """Opens when told to, and answers ``read`` until told to fail.""" + + def __init__(self) -> None: + self.fail_connect: Exception | None = None + self.failures_left: int | None = None + """With ``fail_connect``, how many connects fail before one succeeds.""" + self.fail_io: Exception | None = None + self.connects = 0 + self.closes = 0 + self.reads = 0 + + async def connect(self) -> None: + self.connects += 1 + if self.fail_connect is None: + return + if self.failures_left is not None: + if self.failures_left == 0: + return + self.failures_left -= 1 + raise self.fail_connect + + async def close(self) -> None: + self.closes += 1 + + async def read(self) -> int: + self.reads += 1 + if self.fail_io is not None: + raise self.fail_io + return 1 + + async def read_twice(self) -> int: + return await self.read() + await self.read() + + def describe(self) -> str: + return "a fake" + + +@pytest.fixture +def log_records() -> Iterator[list[dict[str, Any]]]: + records: list[dict[str, Any]] = [] + handler = logger.add( + lambda message: records.append( + {"event": message.record["message"], **message.record["extra"]} + ), + level="DEBUG", + ) + yield records + logger.remove(handler) + + +async def opened( + connection: FakeConnection | None = None, **kwargs: Any +) -> Supervisor[FakeConnection]: + supervisor = Supervisor(connection or FakeConnection(), **kwargs) + await supervisor.open() + return supervisor + + +def fail(supervisor: Supervisor[FakeConnection]) -> None: + """Drop the link, as a failing read would.""" + supervisor.mark_down(ConnectionResetError()) + + +# The handle + + +def test_a_handle_passes_as_the_connection_type(): + supervisor = Supervisor(FakeConnection()) + + assert isinstance(supervisor.handle, FakeConnection) + + +def test_a_handle_is_not_the_connection(): + supervisor = Supervisor(FakeConnection()) + + assert supervisor.handle is not supervisor.connection + + +def test_the_connection_behind_a_handle_is_found(): + supervisor = Supervisor(FakeConnection()) + + assert connection_of(supervisor.handle) is supervisor.connection + + +def test_a_bare_connection_is_its_own_connection(): + connection = FakeConnection() + + assert connection_of(connection) is connection + + +def test_the_supervisor_behind_a_handle_is_found(): + supervisor = Supervisor(FakeConnection()) + + assert supervisor_of(supervisor.handle) is supervisor + + +def test_a_bare_connection_has_no_supervisor(): + assert supervisor_of(FakeConnection()) is None + + +def test_sync_members_pass_straight_through_a_handle(): + supervisor = Supervisor(FakeConnection()) + + assert supervisor.handle.describe() == "a fake" + + +def test_setting_through_a_handle_sets_on_the_connection(): + supervisor = Supervisor(FakeConnection()) + + supervisor.handle.reads = 5 + + assert supervisor.connection.reads == 5 + + +@pytest.mark.asyncio +async def test_an_io_method_is_called_through_a_handle(): + supervisor = await opened() + + assert await supervisor.handle.read() == 1 + + +@pytest.mark.asyncio +async def test_nested_calls_are_counted_once(): + """A connection calling its own methods does not pass the handle again.""" + supervisor = await opened(policy=ConnectionPolicy(timeout_count=2)) + supervisor.connection.fail_io = TimeoutError() + + with pytest.raises(TimeoutError): + await supervisor.handle.read_twice() + + # One timeout counted, not two, so the link is still up + assert supervisor.up + + +def test_the_defaults(): + supervisor = Supervisor(FakeConnection()) + + assert supervisor.reconnect_attempts == DEFAULT_RECONNECT_ATTEMPTS + assert supervisor.reconnect_period == DEFAULT_RECONNECT_PERIOD + + +def test_the_policy_comes_from_the_connection_class(): + class DRAConnection(FakeConnection): + policy = DRAPolicy() + + assert Supervisor(DRAConnection()).policy == DRAPolicy() + + +def test_a_policy_given_to_the_supervisor_overrides_the_class(): + policy = ConnectionPolicy(timeout_count=10) + + assert Supervisor(FakeConnection(), policy=policy).policy is policy + + +def test_a_supervisor_is_named_after_its_connection_by_default(): + assert Supervisor(FakeConnection()).name == "FakeConnection" + + +# The boundary + + +@pytest.mark.asyncio +async def test_a_call_before_the_link_is_opened_fails_fast(): + supervisor = Supervisor(FakeConnection()) + + with pytest.raises(DisconnectedError): + await supervisor.handle.read() + + +@pytest.mark.asyncio +async def test_a_call_while_the_link_is_down_does_no_io(): + supervisor = await opened() + fail(supervisor) + + with pytest.raises(DisconnectedError): + await supervisor.handle.read() + + assert supervisor.connection.reads == 0 + + +@pytest.mark.asyncio +async def test_a_link_error_is_raised_as_disconnected(): + supervisor = await opened() + supervisor.connection.fail_io = ConnectionResetError() + + with pytest.raises(DisconnectedError): + await supervisor.handle.read() + + +@pytest.mark.asyncio +async def test_a_link_error_marks_the_link_down(): + supervisor = await opened() + supervisor.connection.fail_io = ConnectionResetError() + + with pytest.raises(DisconnectedError): + await supervisor.handle.read() + + assert not supervisor.up + + +@pytest.mark.asyncio +async def test_a_link_error_is_caught_even_if_the_caller_swallows_it(): + """The supervisor sees the error where it starts, so no getter can hide it.""" + supervisor = await opened() + supervisor.connection.fail_io = ConnectionResetError() + + try: + await supervisor.handle.read() + except Exception: + pass + + assert not supervisor.up + + +@pytest.mark.asyncio +async def test_a_device_error_is_passed_back_unchanged(): + supervisor = await opened() + supervisor.connection.fail_io = ValueError("rejected") + + with pytest.raises(ValueError, match="rejected"): + await supervisor.handle.read() + + +@pytest.mark.asyncio +async def test_a_device_error_leaves_the_link_up(): + supervisor = await opened() + supervisor.connection.fail_io = ValueError("rejected") + + with pytest.raises(ValueError): + await supervisor.handle.read() + + assert supervisor.up + + +@pytest.mark.asyncio +async def test_a_driver_may_say_its_device_is_offline(): + supervisor = await opened() + supervisor.connection.fail_io = DisconnectedError("device reports OFFLINE") + + with pytest.raises(DisconnectedError, match="OFFLINE"): + await supervisor.handle.read() + + assert not supervisor.up + + +@pytest.mark.asyncio +async def test_one_timeout_is_passed_back_as_it_is(): + supervisor = await opened() + supervisor.connection.fail_io = TimeoutError() + + with pytest.raises(TimeoutError): + await supervisor.handle.read() + + assert supervisor.up + + +@pytest.mark.asyncio +async def test_timeouts_in_a_row_mark_the_link_down(): + supervisor = await opened(policy=ConnectionPolicy(timeout_count=3)) + supervisor.connection.fail_io = TimeoutError() + for _ in range(2): + with pytest.raises(TimeoutError): + await supervisor.handle.read() + + with pytest.raises(DisconnectedError): + await supervisor.handle.read() + + +@pytest.mark.asyncio +async def test_a_good_call_resets_the_timeout_count(): + supervisor = await opened(policy=ConnectionPolicy(timeout_count=2)) + supervisor.connection.fail_io = TimeoutError() + with pytest.raises(TimeoutError): + await supervisor.handle.read() + supervisor.connection.fail_io = None + await supervisor.handle.read() + supervisor.connection.fail_io = TimeoutError() + + with pytest.raises(TimeoutError): + await supervisor.handle.read() + + assert supervisor.up + + +@pytest.mark.asyncio +async def test_going_down_is_logged_once(log_records): + supervisor = await opened() + supervisor.connection.fail_io = ConnectionResetError() + + for _ in range(3): + with pytest.raises(DisconnectedError): + await supervisor.handle.read() + + assert [r["event"] for r in log_records].count("Connection down") == 1 + + +# Reconnecting + + +async def started( + connection: FakeConnection | None = None, **kwargs: Any +) -> tuple[Supervisor[FakeConnection], list[BaseException]]: + supervisor = await opened(connection, **kwargs) + fatal: list[BaseException] = [] + supervisor.start(asyncio.get_event_loop(), on_fatal=fatal.append) + return supervisor, fatal + + +async def gave_up(supervisor: Supervisor) -> None: + async def wait() -> None: + while not supervisor.gave_up: + await asyncio.sleep(0.001) + + await asyncio.wait_for(wait(), timeout=2) + + +@pytest.mark.asyncio +async def test_a_link_that_dropped_is_reconnected(): + supervisor, _ = await started(reconnect_period=0.001) + try: + fail(supervisor) + + await asyncio.wait_for(supervisor.wait_up(), timeout=2) + + assert supervisor.connection.connects == 2 + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_the_link_is_closed_before_it_is_reopened(): + supervisor, _ = await started(reconnect_period=0.001) + try: + fail(supervisor) + + await asyncio.wait_for(supervisor.wait_up(), timeout=2) + + assert supervisor.connection.closes == 1 + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_failing_reconnect_gives_up_after_its_attempts(): + supervisor, _ = await started(reconnect_period=0.001, reconnect_attempts=3) + try: + supervisor.connection.fail_connect = RuntimeError("still down") + fail(supervisor) + + await gave_up(supervisor) + + # The initial open, then three attempts + assert supervisor.connection.connects == 4 + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_giving_up_is_terminal(): + supervisor, _ = await started(reconnect_period=0.001, reconnect_attempts=3) + try: + supervisor.connection.fail_connect = RuntimeError("still down") + fail(supervisor) + await gave_up(supervisor) + connects = supervisor.connection.connects + + await asyncio.sleep(0.05) + + assert supervisor.connection.connects == connects + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_clean_reconnect_restores_the_retry_budget(): + supervisor, _ = await started(reconnect_period=0.001, reconnect_attempts=3) + try: + supervisor.connection.fail_connect = RuntimeError("down") + for _ in range(2): + # Two of the three attempts fail each time, five in all + supervisor.connection.failures_left = 2 + fail(supervisor) + await asyncio.wait_for(supervisor.wait_up(), timeout=2) + + assert not supervisor.gave_up + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_terminal_failure_gives_up_without_spending_the_budget(): + supervisor, _ = await started( + reconnect_period=0.001, + reconnect_attempts=1000, + policy=DRAPolicy(fatal=False), + ) + try: + supervisor.connection.fail_connect = FileNotFoundError("/dev/ttyACM0") + fail(supervisor) + + await gave_up(supervisor) + + assert supervisor.connection.connects == 2 # the open, then one attempt + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_terminal_failure_is_logged_with_the_policys_reason(log_records): + supervisor, _ = await started(reconnect_period=0.001, policy=DRAPolicy()) + try: + supervisor.connection.fail_connect = FileNotFoundError("/dev/ttyACM0") + fail(supervisor) + + await gave_up(supervisor) + + giving_up = [r for r in log_records if r["event"] == "Giving up"] + assert giving_up[0]["reason"] == DRAPolicy().reason(supervisor.connection) + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_terminal_failure_under_a_fatal_policy_is_fatal(): + supervisor, fatal = await started(reconnect_period=0.001, policy=DRAPolicy()) + try: + error = FileNotFoundError("/dev/ttyACM0") + supervisor.connection.fail_connect = error + fail(supervisor) + + await gave_up(supervisor) + + assert fatal == [error] + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_terminal_failure_under_a_non_fatal_policy_only_stalls(): + supervisor, fatal = await started( + reconnect_period=0.001, policy=DRAPolicy(fatal=False) + ) + try: + supervisor.connection.fail_connect = FileNotFoundError("/dev/ttyACM0") + fail(supervisor) + + await gave_up(supervisor) + + assert fatal == [] + finally: + supervisor.stop() + + +@pytest.mark.asyncio +async def test_running_out_of_attempts_is_not_fatal(): + supervisor, fatal = await started( + reconnect_period=0.001, reconnect_attempts=2, policy=DRAPolicy() + ) + try: + supervisor.connection.fail_connect = OSError("I/O error") + fail(supervisor) + + await gave_up(supervisor) + + assert fatal == [] + finally: + supervisor.stop() + + +# Dependencies + + +@pytest.mark.asyncio +async def test_a_dependent_waits_rather_than_spending_its_attempts(): + base, _ = await started(reconnect_period=0.001, reconnect_attempts=1000) + layered, _ = await started(reconnect_period=0.001, reconnect_attempts=1) + layered.dependencies = [base] + try: + base.connection.fail_connect = RuntimeError("down") + fail(base) + fail(layered) + await asyncio.sleep(0.05) + + # Not attempted at all while its dependency is down + assert layered.connection.connects == 1 + + base.connection.fail_connect = None + await asyncio.wait_for(layered.wait_up(), timeout=2) + finally: + base.stop() + layered.stop() + + +@pytest.mark.asyncio +async def test_a_dependent_waits_for_every_dependency(): + """A connection over two links is no more usable with one than with neither.""" + first, _ = await started(reconnect_period=0.001, reconnect_attempts=1000) + second, _ = await started(reconnect_period=0.001, reconnect_attempts=1000) + layered, _ = await started(reconnect_period=0.001) + layered.dependencies = [first, second] + try: + for dependency in (first, second): + dependency.connection.fail_connect = RuntimeError("down") + fail(dependency) + fail(layered) + first.connection.fail_connect = None + await asyncio.wait_for(first.wait_up(), timeout=2) + await asyncio.sleep(0.05) + + assert layered.connection.connects == 1 + finally: + for supervisor in (first, second, layered): + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_dependent_stalls_when_a_dependency_gives_up(log_records): + healthy, _ = await started(reconnect_period=0.001) + doomed, _ = await started(reconnect_period=0.001, reconnect_attempts=1) + layered, _ = await started(reconnect_period=0.001) + layered.dependencies = [healthy, doomed] + try: + doomed.connection.fail_connect = RuntimeError("down for good") + fail(doomed) + fail(layered) + + await gave_up(doomed) + await asyncio.sleep(0.05) + + stalled = [r for r in log_records if r["event"].startswith("Stalled")] + assert stalled[0]["dependencies"] == [doomed.name] + finally: + for supervisor in (healthy, doomed, layered): + supervisor.stop() + + +@pytest.mark.asyncio +async def test_a_stalled_dependent_makes_no_attempt(): + doomed, _ = await started(reconnect_period=0.001, reconnect_attempts=1) + layered, _ = await started(reconnect_period=0.001) + layered.dependencies = [doomed] + try: + doomed.connection.fail_connect = RuntimeError("down for good") + fail(doomed) + fail(layered) + + await gave_up(doomed) + await asyncio.sleep(0.05) + + assert layered.connection.connects == 1 + finally: + doomed.stop() + layered.stop() diff --git a/tests/transports/epics/ca/test_gui.py b/tests/transports/epics/ca/test_gui.py index 997f60dd6..8a26ea29a 100644 --- a/tests/transports/epics/ca/test_gui.py +++ b/tests/transports/epics/ca/test_gui.py @@ -203,7 +203,7 @@ class _B(Controller): def _api_with_id(cls, name): c = cls() c.set_path([name]) - api, _, _ = c.create_api_and_tasks() + api = c.create_api() return api diff --git a/tests/transports/epics/ca/test_softioc_system.py b/tests/transports/epics/ca/test_softioc_system.py index fd1b1ddd0..04efd62af 100644 --- a/tests/transports/epics/ca/test_softioc_system.py +++ b/tests/transports/epics/ca/test_softioc_system.py @@ -18,6 +18,7 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): "b": {"r": f"{pv_prefix}:B_RBV", "w": f"{pv_prefix}:B"}, "clamped": {"r": f"{pv_prefix}:Clamped_RBV", "w": f"{pv_prefix}:Clamped"}, "childvector": {"d": f"{pv_prefix}:ChildVector:PVI"}, + "connected": {"r": f"{pv_prefix}:Connected"}, } child_vector_pvi_pv = parent_pvi["value"]["childvector"]["d"] @@ -33,6 +34,7 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert _child_vector_pvi["value"] == { "__0": {"d": f"{pv_prefix}:ChildVector:0:PVI"}, "__1": {"d": f"{pv_prefix}:ChildVector:1:PVI"}, + "connected": {"r": f"{pv_prefix}:ChildVector:Connected"}, } child_pvi_pv = _child_vector_pvi["value"]["__0"]["d"] @@ -44,8 +46,12 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert child_pvi["value"] == { "c": {"w": f"{pv_prefix}:ChildVector:0:C"}, "d": {"x": f"{pv_prefix}:ChildVector:0:D"}, + "connected": {"r": f"{pv_prefix}:ChildVector:0:Connected"}, } + # A soft controller reports connected as always on + assert ctxt.get(f"{pv_prefix}:Connected") == 1 + initial_value = ctxt.get(f"{pv_prefix}:Clamped_RBV") assert initial_value # Clamped initial value is truthy assert ctxt.get(f"{pv_prefix}:Clamped") == initial_value # Setpoint is synced diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index ff63015c8..c73389a69 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -20,6 +20,7 @@ from fastcs.datatypes import Array1D, Limits, NumericLimits, Table from fastcs.launch import FastCS from fastcs.methods import command +from fastcs.scheduling import ScanSchedule from fastcs.transports.epics.pva.transport import EpicsPVATransport @@ -37,6 +38,7 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): "a": {"rw": f"{pv_prefix}:A"}, "b": {"w": f"{pv_prefix}:B"}, "child": {"d": f"{pv_prefix}:Child:PVI"}, + "connected": {"r": f"{pv_prefix}:Connected"}, "table": { "rw": f"{pv_prefix}:Table", }, @@ -54,6 +56,7 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): "vector_attribute": {"r": f"{pv_prefix}:Child:VectorAttribute"}, "__1": {"d": f"{pv_prefix}:Child:1:PVI"}, "__2": {"d": f"{pv_prefix}:Child:2:PVI"}, + "connected": {"r": f"{pv_prefix}:Child:Connected"}, } child_pvi_pv = _child_vector_pvi["value"]["__1"]["d"] @@ -64,6 +67,7 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): assert child_pvi["display"] == {"description": "some sub controller"} assert child_pvi["value"] == { "c": {"w": f"{pv_prefix}:Child:1:C"}, + "connected": {"r": f"{pv_prefix}:Child:1:Connected"}, "clamped": {"rw": f"{pv_prefix}:Child:1:Clamped"}, "d": {"x": f"{pv_prefix}:Child:1:D"}, "e": {"r": f"{pv_prefix}:Child:1:E"}, @@ -388,6 +392,7 @@ def __init__(self) -> None: "child0": {"d": f"{pv_prefix}:Child0:PVI"}, "child1": {"d": f"{pv_prefix}:Child1:PVI"}, "child2": {"d": f"{pv_prefix}:Child2:PVI"}, + "connected": {"r": f"{pv_prefix}:Connected"}, }, } assert len(child_vector_controller_pvi) == 1 @@ -403,6 +408,7 @@ def __init__(self) -> None: "__0": {"d": f"{pv_prefix}:Child:0:PVI"}, "__1": {"d": f"{pv_prefix}:Child:1:PVI"}, "__2": {"d": f"{pv_prefix}:Child:2:PVI"}, + "connected": {"r": f"{pv_prefix}:Child:Connected"}, }, } assert len(child_child_controller_pvi) == 1 @@ -416,6 +422,7 @@ def __init__(self) -> None: }, "value": { "attr_c": {"w": f"{pv_prefix}:Child:0:AttrC"}, + "connected": {"r": f"{pv_prefix}:Child:0:Connected"}, "attr_d": { "w": f"{pv_prefix}:Child:0:AttrD", }, @@ -712,9 +719,10 @@ async def get_a(self) -> int: controller = SeedController() controller.set_path([str(uuid4())]) - await controller.initialise() - controller.post_initialise() - controller_api, _, initial_coros = controller.create_api_and_tasks() + await controller.build() + controller_api = controller.create_api() + schedule = ScanSchedule() + schedule.add_controller(controller) attribute = controller_api.attributes["a"] assert isinstance(attribute, AttrRW) @@ -734,8 +742,7 @@ async def wrapper(value): transport.connect(controller_apis=[controller_api], loop=asyncio.get_running_loop()) # Nothing has awaited transport.serve() at this point - as in FastCS.serve() - for coro in initial_coros: - await coro() + await schedule.read_once() assert attribute.setpoint == 10 assert published == [10] diff --git a/tests/transports/epics/test_emission.py b/tests/transports/epics/test_emission.py index c5bc2e095..27a446b5a 100644 --- a/tests/transports/epics/test_emission.py +++ b/tests/transports/epics/test_emission.py @@ -33,7 +33,7 @@ class _Beta(Controller): def _api_with_id(controller_class: type[Controller], name: str): controller = controller_class() controller.set_path([name]) - api, _, _ = controller.create_api_and_tasks() + api = controller.create_api() return api