From 4765f3f3264ce6c8b6cbaa2ac2155b72529d4b5c Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Mon, 19 Jan 2026 16:36:14 +0000 Subject: [PATCH 1/2] Add historical ADRs --- ...oller-initialization-on-main-event-loop.md | 215 +++++++++++++++ .../0004-backend-to-transport-refactoring.md | 85 ++++++ .../0005-background-thread-removal.md | 252 ++++++++++++++++++ .../0006-controller-api-abstraction-layer.md | 150 +++++++++++ .../decisions/0007-transport-consolidation.md | 247 +++++++++++++++++ ...ansport-dependencies-as-optional-extras.md | 191 +++++++++++++ .../0009-handler-to-attribute-io-pattern.md | 170 ++++++++++++ .../decisions/0010-subcontroller-removal.md | 197 ++++++++++++++ .../0011-controller-vector-implementation.md | 200 ++++++++++++++ 9 files changed, 1707 insertions(+) create mode 100644 docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md create mode 100644 docs/explanations/decisions/0004-backend-to-transport-refactoring.md create mode 100644 docs/explanations/decisions/0005-background-thread-removal.md create mode 100644 docs/explanations/decisions/0006-controller-api-abstraction-layer.md create mode 100644 docs/explanations/decisions/0007-transport-consolidation.md create mode 100644 docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md create mode 100644 docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md create mode 100644 docs/explanations/decisions/0010-subcontroller-removal.md create mode 100644 docs/explanations/decisions/0011-controller-vector-implementation.md diff --git a/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md b/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md new file mode 100644 index 000000000..261b14379 --- /dev/null +++ b/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md @@ -0,0 +1,215 @@ +# 3. Controller Initialization on Main Event Loop + +Date: 2024-08-01 + +**Related:** [PR #49](https://github.com/DiamondLightSource/FastCS/pull/49) + +## Status + +Accepted + +## Context + +In the original FastCS architecture, the Backend class initialization pattern made it difficult for controllers to perform async setup operations on the main event loop before their API was exposed through the Mapping layer. + +**Original Architecture - Separate Initialization:** + +The AsyncioBackend used composition, creating separate components and manually orchestrating initialization: + +```python +class AsyncioBackend: + def __init__(self, mapping: Mapping): + self._mapping = mapping + + def run_interactive_session(self): + dispatcher = AsyncioDispatcher() + backend = Backend(self._mapping, dispatcher.loop) + + # Manual initialization sequence + backend.link_process_tasks() + backend.run_initial_tasks() + backend.start_scan_tasks() +``` + +The Backend was a thin wrapper that accepted a pre-created Mapping: + +```python +class Backend: + def __init__(self, mapping: Mapping, loop: asyncio.AbstractEventLoop): + self._mapping = mapping + self._loop = loop +``` + +The system needed a way to: +- Allow controllers to run async initialization code on the main event loop +- Ensure initialization happens before the Mapping is created +- Provide a consistent Backend base class with a clear lifecycle +- Centralize initialization orchestration in one place + +## Decision + +We redesigned the Backend class to own the entire initialization lifecycle, including creating the event loop and running controller initialization on it before creating the Mapping. + +### New Architecture + +**Backend - Lifecycle Orchestrator:** + +```python +class Backend: + def __init__( + self, controller: Controller, loop: asyncio.AbstractEventLoop | None = None + ): + # 1. Create AsyncioDispatcher with optional loop + self._dispatcher = AsyncioDispatcher(loop) + self._loop = self._dispatcher.loop + self._controller = controller + + # 2. Add connect to initial tasks + self._initial_tasks.append(controller.connect) + + # 3. KEY CHANGE: Run controller.initialise() on main event loop + # BEFORE creating the Mapping + asyncio.run_coroutine_threadsafe( + self._controller.initialise(), self._loop + ).result() + + # 4. Create Mapping AFTER controller is initialized + self._mapping = Mapping(self._controller) + self._link_process_tasks() + + # 5. Build context dictionary for subclasses + self._context.update({ + "dispatcher": self._dispatcher, + "controller": self._controller, + "mapping": self._mapping, + }) + + def run(self): + self._run_initial_tasks() + self._start_scan_tasks() + self._run() # Subclass-specific implementation +``` + +### Key Changes + +1. **Controller Initialization on Main Event Loop:** + - Added `initialise()` method to Controller class as an async initialization hook + - Backend calls `asyncio.run_coroutine_threadsafe(controller.initialise(), self._loop).result()` + - This happens BEFORE the Mapping is created, allowing controllers to set up async resources + +2. **Backend Accepts Controller, Not Mapping:** + - **Before:** `Backend(mapping: Mapping, loop: AbstractEventLoop)` + - **After:** `Backend(controller: Controller, loop: AbstractEventLoop | None = None)` + - Backend now owns creating the Mapping after initialization + +3. **Inheritance-Based Architecture:** + - **Before:** AsyncioBackend used composition, creating Backend separately + - **After:** All backends inherit from Backend base class + - Subclasses implement `_run()` for protocol-specific execution + +4. **Centralized Lifecycle:** + - Backend orchestrates: `initialise()` → create Mapping → `connect()` → scan tasks → `_run()` + - Clear, documented initialization sequence + - Consistent across all backend types + +5. **Unified Context Management:** + - Backend maintains `_context` dictionary with dispatcher, controller, and mapping + - Passed to subclass `_run()` implementations + - Consistent context across all backends + +### Controller Lifecycle + +```python +class Controller(BaseController): + async def initialise(self) -> None: + """Called on main event loop BEFORE Mapping creation. + Override to perform async setup (database connections, etc.)""" + pass + + async def connect(self) -> None: + """Called as initial task AFTER Mapping creation. + Override to establish device connections.""" + pass +``` + +### Benefits + +- **Async Initialization Support:** Controllers can now perform async setup on the main + event loop before their API is exposed. This allows introspecting devices to create + Attributes and Methods dynamically. + +- **Predictable Lifecycle:** Clear initialization sequence: initialise → Mapping → connect → scan + +- **Better Separation of Concerns:** + - Backend: Infrastructure and lifecycle management + - Subclasses: Protocol-specific `_run()` implementation + - Controller: Domain logic and initialization + +- **Cleaner API:** Backends accept Controller directly, not pre-created Mapping + +## Consequences + +### Technical Changes + +- 358 insertions, 217 deletions across 13 files +- Restructured `src/fastcs/backend.py` with new lifecycle orchestration +- Added `initialise()` method to `src/fastcs/controller.py` +- Updated all backend implementations to inherit from Backend: + - `src/fastcs/backends/asyncio_backend.py` + - `src/fastcs/backends/epics/backend.py` + - `src/fastcs/backends/tango/backend.py` +- Updated corresponding IOC/runtime implementations: + - `src/fastcs/backends/epics/ioc.py` + - `src/fastcs/backends/tango/dsr.py` + +### Migration Impact + +For controller developers: +1. Can now override `initialise()` for async setup on main event loop +2. `connect()` continues to work as before for device connection logic +3. Clear lifecycle: `initialise()` runs first, then `connect()` as initial task + +For backend developers: +1. Backends now inherit from Backend base class +2. Constructor signature changed: accepts Controller, not Mapping +3. Implement `_run()` method instead of managing full initialization +4. Access `self._context` for dispatcher, controller, and mapping + +Example transformation: +```python +# Before +class EpicsBackend: + def __init__(self, mapping: Mapping): + dispatcher = AsyncioDispatcher() + backend = Backend(mapping, dispatcher.loop) + backend.run_initial_tasks() + +# After +class EpicsBackend(Backend): + def __init__(self, controller: Controller, pv_prefix: str): + super().__init__(controller) # Handles initialization + self._ioc = EpicsIOC(pv_prefix, self._mapping) + + def _run(self): + self._ioc.run(self._dispatcher, self._context) +``` + +### Architectural Impact + +This established a clear, consistent initialization pattern across all FastCS backends: + +``` +Backend.__init__: + 1. Create AsyncioDispatcher (main event loop) + 2. Run controller.initialise() on main event loop ← NEW + 3. Create Mapping from controller + 4. Link process tasks + 5. Prepare context + +Backend.run(): + 6. Run initial tasks (controller.connect(), etc.) + 7. Start scan tasks + 8. Call subclass._run() for protocol-specific execution +``` + +The addition of `controller.initialise()` on the main event loop enabled controllers to perform complex async setup operations before their API was exposed, while maintaining a clean separation between framework infrastructure and protocol implementations. diff --git a/docs/explanations/decisions/0004-backend-to-transport-refactoring.md b/docs/explanations/decisions/0004-backend-to-transport-refactoring.md new file mode 100644 index 000000000..0dee7c5b8 --- /dev/null +++ b/docs/explanations/decisions/0004-backend-to-transport-refactoring.md @@ -0,0 +1,85 @@ +# 4. Rename Backend to Transport + +Date: 2024-11-29 + +**Related:** [PR #67](https://github.com/DiamondLightSource/FastCS/pull/67) + +## Status + +Accepted + +## Context + +In the original FastCS architecture, the term "backend" was used to describe both: +1. The overall framework/system that managed controllers (the "backend system") +2. The specific communication protocol implementations (EPICS CA, PVA, REST, Tango) + +**Original Architecture:** +- There was a `Backend` base class that protocol implementations inherited from +- Protocol-specific classes like `EpicsBackend`, `RestBackend`, `TangoBackend` extended `Backend` +- Users worked directly with these inherited backend classes +- This created tight coupling between the core framework and protocol implementations + +This dual usage of "backend" created confusion for developers and users: +- It was unclear whether "backend" referred to the framework itself or the protocol layer +- The terminology didn't clearly differentiate between the abstract framework and the underlying communication mechanisms +- The inheritance pattern made it difficult to compose multiple transports or swap them dynamically + +## Decision + +We renamed "backend" to "transport" for all protocol/communication implementations to clearly differentiate them from the abstract framework. + +This refactoring involved: + +1. **Terminology Change:** + - `backends/` directory → `transport/` directory + - `EpicsBackend` → `EpicsTransport` + - `RestBackend` → `RestTransport` + - `TangoBackend` → `TangoTransport` + +2. **Architectural Improvements:** + - Introduced `TransportAdapter` abstract base class defining the contract for all transports: + - `run()` - Start the transport + - `create_docs()` - Generate documentation + - `create_gui()` - Generate GUI metadata + - Split transport configuration into separate `options` modules + - Added adapter pattern with template methods for consistency + +3. **Plugin Architecture (Composition over Inheritance):** + - **Before:** `Backend` base class with `EpicsBackend`, `RestBackend`, etc. inheriting from it + - **After:** `FastCS` core class that accepts `Transport` implementations as plugins + - Transports are now passed to `FastCS` rather than being subclasses of a framework class + - This enables flexible composition, runtime transport selection, and loose coupling + +4. **Public API Restructuring:** + - Introduced `FastCS` class as the programmatic interface for running controllers with transports + - Added `launch()` function as the primary entry point for initializing controllers + - Cleaned up import namespace to add structure to the public API + +The term "backend" was reserved for referring to the overall FastCS framework/system, while "transport" specifically refers to protocol implementations (EPICS CA, PVA, REST, GraphQL, Tango). + +## Consequences + +- **Clearer Terminology:** The separation between framework (backend) and protocol layer (transport) is now explicit and unambiguous +- **Consistent Architecture:** All transports follow the adapter pattern with a standardized interface +- **Better Separation of Concerns:** Transport configuration is separated from transport implementation via options modules +- **Improved Extensibility:** Adding new transport protocols is more straightforward with the adapter pattern +- **Reduced Dependency Coupling:** Transport options can be imported without including heavy transport dependencies +- **Clearer Public API:** The `launch()` function and `FastCS` class provide clear entry points + +### Migration Path + +For users migrating from the old API: +1. Replace all `backend` imports with `transport` imports +2. Update class names (e.g., `EpicsBackend` → `EpicsTransport`) +3. Prefer using the new `launch()` function instead of directly instantiating transports +4. Update configuration to use transport-specific options dataclasses + +### Technical Impact + +- 681 insertions, 321 deletions across the codebase +- All transport implementations refactored to follow adapter pattern +- Transport options moved to separate modules for cleaner dependency management +- Mapping class functionality integrated into Controller + +This decision established a clearer architectural foundation for FastCS, making it easier for contributors to understand the system's layers and for users to reason about how their controllers interact with different communication protocols. diff --git a/docs/explanations/decisions/0005-background-thread-removal.md b/docs/explanations/decisions/0005-background-thread-removal.md new file mode 100644 index 000000000..62e8c01d9 --- /dev/null +++ b/docs/explanations/decisions/0005-background-thread-removal.md @@ -0,0 +1,252 @@ +# 5. Remove Background Thread from Backend + +Date: 2025-01-24 + +**Related:** [PR #98](https://github.com/DiamondLightSource/FastCS/pull/98) + +## Status + +Accepted + +## Context + +The original FastCS Backend implementation used `asyncio.run_coroutine_threadsafe()` to execute controller operations on a background event loop thread managed by `AsyncioDispatcher`. This pattern was inherited from EPICS softIOC's threading model. + +**Original Architecture - Background Thread:** + +```python +class Backend: + def __init__( + self, + controller: Controller, + loop: asyncio.AbstractEventLoop | None = None, + ): + # Create dispatcher with its own thread + self.dispatcher = AsyncioDispatcher(loop) + self._loop = self.dispatcher.loop # Runs in background thread + + # Run initialization on background thread + asyncio.run_coroutine_threadsafe( + self._controller.initialise(), self._loop + ).result() # Block until complete + + def _run_initial_futures(self): + for coro in self._initial_coros: + # Schedule on background thread, block main thread + future = asyncio.run_coroutine_threadsafe(coro(), self._loop) + future.result() + + def start_scan_futures(self): + # Store Future objects from background thread + self._scan_futures = { + asyncio.run_coroutine_threadsafe(coro(), self._loop) + for coro in _get_scan_coros(self._controller) + } +``` + +This approach created a threading model where: +- Main thread creates Backend +- AsyncioDispatcher starts background thread with event loop +- Controller operations scheduled from main thread to background thread +- Main thread blocks waiting for background thread results via `.result()` + +**Problems with Background Thread:** + +1. **Thread Safety Complexity:** Managing state across thread boundaries introduced race conditions and required careful synchronization: + - Controller state accessed from both threads + - Attribute updates needed thread-safe mechanisms + - Future cancellation had timing issues + +2. **Blocking Main Thread:** Despite using async, the main thread blocked waiting for background thread operations: + ```python + future = asyncio.run_coroutine_threadsafe(coro(), self._loop) + future.result() # Main thread blocks here + ``` + This defeated the purpose of async programming + +3. **Complex Lifecycle Management:** Starting and stopping the background thread added complexity: + - Ensuring thread cleanup in `__del__` + - Managing Future objects for scan tasks + - Cancelling futures vs cancelling tasks had different semantics + +4. **Difficult to Test:** Background threads made tests: + - Non-deterministic (race conditions) + - Harder to debug (multiple call stacks) + - Slower (thread startup overhead) + +5. **Unnecessary for Most Transports:** Only EPICS CA softIOC actually required a background thread (due to its C library's threading requirements). Other transports (REST, GraphQL, Tango, PVA) could run entirely async: + - REST/GraphQL use async frameworks (FastAPI, Strawberry) + - EPICS PVA is pure Python async + +6. **Future vs Task Confusion:** Using `Future` objects from `run_coroutine_threadsafe()` instead of `Task` objects from `create_task()` created API inconsistencies + +The system needed a simpler concurrency model that: +- Runs all async code on a single event loop +- Uses native async/await throughout +- Eliminates thread synchronization complexity +- Allows transports to manage their own threading if needed + +## Decision + +We removed the background thread from Backend, making it fully async and allowing transports to manage their own threading requirements if needed. + +### New Architecture + +**Async-Only Backend:** + +```python +class Backend: + def __init__( + self, + controller: Controller, + loop: asyncio.AbstractEventLoop, # Now required, not created + ): + self._loop = loop # Caller's event loop + self._controller = controller + + self._initial_coros = [controller.connect] + self._scan_tasks: set[asyncio.Task] = set() # Tasks, not Futures + + # Run initialization on PROVIDED event loop (not background thread) + loop.run_until_complete(self._controller.initialise()) + self._link_process_tasks() + + async def serve(self): + """Fully async - no blocking""" + await self._run_initial_coros() + await self._start_scan_tasks() + + async def _run_initial_coros(self): + """Direct await - no threading""" + for coro in self._initial_coros: + await coro() + + async def _start_scan_tasks(self): + """Create tasks on same event loop""" + self._scan_tasks = { + self._loop.create_task(coro()) + for coro in _get_scan_coros(self._controller) + } +``` + +### Key Changes + +1. **No AsyncioDispatcher:** + - Removed `self.dispatcher = AsyncioDispatcher(loop)` + - Backend now receives event loop from caller + - No background thread creation + +2. **Synchronous Initialization:** + - **Before:** `asyncio.run_coroutine_threadsafe().result()` (cross-thread blocking) + - **After:** `loop.run_until_complete()` (same-thread blocking) + - Initialization happens before event loop starts running + +3. **Async serve() Method:** + - **Before:** `run()` method that scheduled futures + - **After:** `async serve()` method using native async/await + - Can be composed with other async operations + +4. **Tasks Instead of Futures:** + - **Before:** `asyncio.run_coroutine_threadsafe()` returns `Future` + - **After:** `loop.create_task()` returns `Task` + - Consistent with async best practices + +5. **Transport-Managed Threading:** + - EPICS CA transport creates its own AsyncioDispatcher when needed + - Other transports run purely async + - Each transport decides its threading model + +### Benefits + +- **Simplified Concurrency Model:** Single event loop for most operations, no cross-thread coordination + +- **True Async/Await:** Native Python async patterns throughout, no blocking `.result()` calls + +- **Better Testability:** Deterministic execution, easier to debug, faster tests + +- **Clearer Responsibility:** Transports that need threads manage them explicitly + +- **Task-Based API:** Consistent use of `Task` objects with cancellation support + +- **Composability:** `async serve()` can be composed with other async operations + +- **Performance:** Eliminated thread creation overhead for transports that don't need it + +## Consequences + +The Tango transport still requires being run on in a background thread because it does +not allow an event loop to be passed it. It always creates its own and then the issues +with coroutines being called from the wrong event loop persist. + +### Technical Changes + +- 769 insertions, 194 deletions across 27 files +- Removed `AsyncioDispatcher` from `src/fastcs/backend.py` +- Changed `Backend.__init__()` to accept event loop (not create it) +- Changed `Backend.run()` to `async Backend.serve()` +- Updated initialization from `run_coroutine_threadsafe()` to `run_until_complete()` +- Changed scan task management from `Future` to `Task` objects +- Updated all transport adapters: + - `src/fastcs/transport/epics/adapter.py` - Now creates dispatcher + - `src/fastcs/transport/graphql/adapter.py` - Pure async + - `src/fastcs/transport/rest/adapter.py` - Pure async + - `src/fastcs/transport/tango/adapter.py` - Updated async handling +- Added benchmarking tests in `tests/benchmarking/` +- Updated `src/fastcs/launch.py` to handle async serve + +### Migration Impact + +For transport developers: + +**Before (Background thread in Backend):** +```python +class MyTransport(TransportAdapter): + def __init__(self, controller_api, loop): + # Backend created its own thread + self._backend = Backend(controller) + + def run(self): + self._backend.run() # Synchronous + # Do transport-specific work +``` + +**After (Async serve):** +```python +class MyTransport(Transport): + def connect(self, controller_api, loop): + # Pass loop to Backend + self._backend = Backend(controller, loop) + + async def serve(self): + await self._backend.serve() # Async + # Do transport-specific work +``` + +For transports needing threads (like EPICS CA): + +```python +class EpicsCATransport(Transport): + def __init__(self): + # Transport creates its own dispatcher + self._dispatcher = AsyncioDispatcher() + + def connect(self, controller_api, loop): + # Use dispatcher's loop for Backend + self._backend = Backend(controller, self._dispatcher.loop) + + async def serve(self): + # Bridge to threaded environment + await self._backend.serve() +``` + +### Performance Impact + +Benchmarking showed: +- Faster startup (no thread creation for most transports) +- Reduced memory overhead (no extra thread stack) +- More predictable timing (no thread scheduling delays) +- Better CPU utilization (single event loop) + +### Architectural Impact + +The removal of the background thread simplified FastCS's concurrency model, making it more predictable, testable, and performant while still supporting transports that require threading (like EPICS CA) through explicit transport-level thread management. diff --git a/docs/explanations/decisions/0006-controller-api-abstraction-layer.md b/docs/explanations/decisions/0006-controller-api-abstraction-layer.md new file mode 100644 index 000000000..a313931eb --- /dev/null +++ b/docs/explanations/decisions/0006-controller-api-abstraction-layer.md @@ -0,0 +1,150 @@ +# 6. Create ControllerAPI Abstraction Layer + +Date: 2025-03-10 + +**Related:** [PR #87](https://github.com/DiamondLightSource/FastCS/pull/87) + +## Status + +Accepted + +## Context + +In the original FastCS architecture, transport implementations (EPICS CA, PVA, REST, GraphQL, Tango) directly accessed `Controller` and `SubController` instances to extract attributes, methods, and metadata for serving over their respective protocols. + +**Original Architecture - Direct Controller Access:** +- Each transport was responsible for traversing the controller and its sub-controllers to find attributes and methods +- Transports had direct access to controller internals, allowing them to modify things they shouldn't +- Each transport implemented its own custom traversal logic +- No static, immutable view of what a controller exposes + +**Problems with Direct Coupling:** + +1. **Tight Coupling:** Transports were tightly coupled to the internal structure of `Controller` and `SubController` classes, making it difficult to evolve the controller implementation without breaking transports + +2. **Code Duplication:** Every transport re-implemented similar logic to: + - Traverse the controller/sub-controller tree + - Discover attributes and methods + - Build hierarchical structures for their protocol + - Handle naming and path construction + +3. **No Encapsulation:** Transports had direct access to mutable controller state, making it possible to inadvertently modify things they shouldn't be able to change + +4. **No Static View:** Controllers expose their API dynamically during traversal rather than providing a complete, static snapshot after initialization + +The system needed a way to: +- Provide a complete, static view of the controller API once it's fully initialized +- Prevent transports from having direct access to mutable controller internals +- Decouple transport implementations from controller internals +- Provide a consistent view of controller APIs across all transports +- Centralize the logic for extracting and organizing controller metadata + +## Decision + +We introduced `ControllerAPI` as an abstraction layer that sits between `Controller` instances and transport implementations. + +### New Architecture + +**ControllerAPI - Transport-Independent Representation:** +```python +@dataclass +class ControllerAPI: + path: list[str] + attributes: dict[str, Attribute] + command_methods: dict[str, Command] + put_methods: dict[str, Put] + scan_methods: dict[str, Scan] + sub_apis: dict[str, "ControllerAPI"] + description: str | None + + def walk_api(self) -> Iterator["ControllerAPI"]: + """Traverse the entire API tree""" +``` + +### Key Changes + +1. **API Extraction Centralized:** + - A single `create_controller_api()` function extracts the API from a Controller + - This function handles all traversal, method discovery, and hierarchy building + - All transports use the same API extraction logic + +2. **Transports Accept ControllerAPI:** + - **Before:** `EpicsCAIOC(pv_prefix, controller: Controller)` + - **After:** `EpicsCAIOC(pv_prefix, controller_api: ControllerAPI)` + - Transports no longer have direct access to mutable Controller internals + - ControllerAPI provides a static, read-only view of the controller's complete API + +3. **Complete API After Initialization:** + - Once a controller is fully initialized, its entire API (all sub-controllers, attributes, and methods) is exposed in `ControllerAPI` + - Transports receive a complete snapshot rather than having to traverse dynamically + - Changes to the controller after initialization don't affect the ControllerAPI + +4. **Hierarchical API Structure:** + - ControllerAPI contains nested `sub_apis` for sub-controllers, forming a tree structure + - `walk_api()` method provides depth-first traversal + - Each level includes its path for proper naming + +5. **Complete Method Discovery:** + - Separate dictionaries for command_methods, put_methods, scan_methods + - Methods discovered and organized during API creation + - Transports receive ready-to-use method metadata + +### Benefits: + +- **Static View After Initialization:** Once a controller is fully initialized, its entire API (all sub-controllers, attributes, and methods) is exposed in a complete, static snapshot + +- **Encapsulation:** Transports cannot directly access or modify mutable controller internals - they work with a read-only ControllerAPI. The only attributes and methods themselves are mutable. + +- **Separation of Concerns:** + - Controllers focus on device logic + - ControllerAPI handles API representation + - Transports focus on protocol implementation + +- **Single Source of Truth:** One canonical representation of what a controller exposes + +- **Reduced Code Duplication:** Traversal and extraction logic written once + +- **Testability:** Controllers can be tested with mock ControllerAPIs, transports can be tested with synthetic APIs + +- **Evolution Independence:** Controller internals can change without affecting transports as long as API extraction is updated + +## Consequences + +### Technical Changes + +- 1,009 insertions, 568 deletions across 32 files +- Created `src/fastcs/controller_api.py` with ControllerAPI dataclass +- Updated all transport adapters to accept ControllerAPI instead of Controller: + - `src/fastcs/transport/epics/ca/adapter.py` + - `src/fastcs/transport/epics/pva/adapter.py` + - `src/fastcs/transport/graphQL/adapter.py` + - `src/fastcs/transport/rest/adapter.py` + - `src/fastcs/transport/tango/adapter.py` +- Refactored `src/fastcs/cs_methods.py` for method extraction +- Updated `src/fastcs/backend.py` to create ControllerAPI before passing to transports +- All transport IOC/server implementations updated to use ControllerAPI + +### Migration Impact + +For transport developers: +1. Update transport constructors to accept `ControllerAPI` instead of `Controller` +2. Replace direct attribute/method access with ControllerAPI dictionary lookups +3. Use `walk_api()` for tree traversal instead of custom logic +4. Remove custom extraction code in favor of ControllerAPI structure + +For controller developers: +- No changes required - controllers continue to work as before +- ControllerAPI is created automatically by the backend + +### Architectural Impact + +This established a clean layered architecture: +``` +Controllers (domain logic) + ↓ +ControllerAPI (canonical representation) + ↓ +Transports (protocol implementation) +``` + +The ControllerAPI became the primary contract between the FastCS core and transport implementations, ensuring all transports have a consistent, complete view of controller capabilities. diff --git a/docs/explanations/decisions/0007-transport-consolidation.md b/docs/explanations/decisions/0007-transport-consolidation.md new file mode 100644 index 000000000..375571213 --- /dev/null +++ b/docs/explanations/decisions/0007-transport-consolidation.md @@ -0,0 +1,247 @@ +# 7. Merge TransportAdapter and TransportOptions into Transport + +Date: 2025-09-29 + +**Related:** [PR #220](https://github.com/DiamondLightSource/FastCS/pull/220) + +## Status + +Accepted + +## Context + +Following the Backend-to-Transport refactoring (ADR-0004), FastCS transports were implemented using two separate class hierarchies: `TransportAdapter` for the transport implementation and separate `*Options` classes for configuration. + +**Original Architecture - Two-Class Pattern:** + +Each transport required two classes: + +```python +# Configuration class +@dataclass +class EpicsPVAOptions: + pv_prefix: str + gui_dir: Path | None = None + +# Implementation class +class EpicsPVATransport(TransportAdapter): + def __init__( + self, + controller_api: ControllerAPI, + options: EpicsPVAOptions, + ): + self._controller_api = controller_api + self._options = options + # ... setup code + + async def run(self): + # ... transport-specific code +``` + +**Usage Pattern:** +```python +# Create options +options = EpicsPVAOptions(pv_prefix="MY:DEVICE:") + +# Pass options to FastCS +fastcs = FastCS(controller, [options]) + +# FastCS internally: +# - Matched options type +# - Created corresponding TransportAdapter +# - Passed options to adapter +``` + +**Problems with Two-Class Pattern:** + +1. **API Surface Duplication:** Two classes per transport (5 transports = 10 classes) when a single class could suffice. + +2. **Complex Instantiation Logic:** FastCS needed pattern matching logic to create the right adapter from options: + ```python + match option: + case EpicsPVAOptions(): + transport = EpicsPVATransport(controller_api, option) + case EpicsCAOptions(): + transport = EpicsCATransport(controller_api, loop, option) + # ... 5 more cases + ``` + +3. **Inconsistent Constructor Signatures:** Different transports had different initialization requirements: + - Some needed `loop`, others didn't + - Some needed `controller_api` immediately, others deferred it + - Options were passed separately, requiring attribute access: `self._options.pv_prefix` + +4. **Unnecessary Abstraction Layer:** The Options classes were simple dataclasses with no behavior - they existed solely to carry configuration to the adapter + +The system needed a simpler, more direct approach where transport configuration and implementation were unified. + +## Decision + +We merged `TransportAdapter` and `*Options` classes into a single `Transport` base class that combines configuration and implementation. + +### New Architecture + +**Unified Transport Class:** + +```python +@dataclass +class EpicsPVATransport(Transport): + epicspva: EpicsIOCOptions + + def connect( + self, controller_api: ControllerAPI, loop: asyncio.AbstractEventLoop + ) -> None: + self._controller_api = controller_api + self._loop = loop + self._ioc = EpicsPVAIOC( + pv_prefix=self.epicspva.pv_prefix, + controller_api=controller_api, + ) + + async def serve(self) -> None: + await self._ioc.run(self.epicspva) +``` + +### Key Changes + +1. **Single Class per Transport:** + - Merged adapter and options into one dataclass + - Configuration fields are direct attributes + - Implementation methods are instance methods + +2. **Standardized Initialization:** + - All transports use `@dataclass` for configuration + - `connect()` receives controller_api and loop (deferred initialization) + - `serve()` runs the transport server + - Consistent signature across all transports + +3. **Simplified FastCS:** + ```python + # Before: Options → Adapter matching logic + for option in transport_options: + match option: + case EpicsPVAOptions(): + transport = EpicsPVATransport(controller_api, option) + + # After: Direct transport instances + for transport in transports: + transport.connect(controller_api, loop) # Unified interface + ``` + +4. **Better Type Safety:** + - `Transport.union()` creates Union type of all transports + - Used for CLI type hints and validation + - Automatically updated when new transports are added + +5. **Cleaner Imports:** + ```python + # Before + from fastcs.transport.epics.pva.options import EpicsPVAOptions + options = EpicsPVAOptions(pv_prefix="MY:DEVICE:") + + # After + from fastcs.transports.epics.pva import EpicsPVATransport + transport = EpicsPVATransport(epicspva=EpicsIOCOptions(pv_prefix="MY:DEVICE:")) + ``` + +### Benefits + +- **Reduced API Surface:** 5 classes instead of 10 (one Transport per protocol, not one Adapter + one Options) + +- **Simpler Mental Model:** Configuration and implementation in one place + +- **Consistent Interface:** All transports follow same initialization pattern + +- **Less Boilerplate:** No pattern matching needed in FastCS + +- **Easier Maintenance:** Transport parameters defined once in dataclass fields + +- **Better Type Safety:** Automatic Union type generation for all transports + +- **Cleaner Usage:** Direct instantiation of transport with configuration + +## Consequences + +### Technical Changes + +- 87 insertions, 87 deletions across 16 files (net neutral, massive simplification) +- Renamed `src/fastcs/transport/adapter.py` → `src/fastcs/transport/transport.py` +- Merged all `*Options` classes into corresponding `*Transport` classes +- Simplified `src/fastcs/launch.py` by removing pattern matching logic +- Updated all transport implementations: + - `src/fastcs/transport/epics/ca/adapter.py` + - `src/fastcs/transport/epics/pva/adapter.py` + - `src/fastcs/transport/graphql/adapter.py` + - `src/fastcs/transport/rest/adapter.py` + - `src/fastcs/transport/tango/adapter.py` +- Updated all tests and examples + +### Migration Impact + +For transport users: + +**Before (Options pattern):** +```python +from fastcs.transport.epics.pva.options import EpicsPVAOptions +from fastcs import FastCS + +options = EpicsPVAOptions( + pv_prefix="MY:DEVICE:", + gui_dir=Path("gui") +) + +fastcs = FastCS(controller, [options]) +``` + +**After (Transport pattern):** +```python +from fastcs.transports.epics.pva import EpicsPVATransport +from fastcs.transports.epics import EpicsIOCOptions +from fastcs import FastCS + +transport = EpicsPVATransport( + epicspva=EpicsIOCOptions( + pv_prefix="MY:DEVICE:", + gui_dir=Path("gui") + ) +) + +fastcs = FastCS(controller, [transport]) +``` + +For transport developers: + +**Before (Two classes):** +```python +@dataclass +class MyOptions: + param1: str + param2: int = 42 + +class MyTransport(TransportAdapter): + def __init__(self, controller_api: ControllerAPI, options: MyOptions): + self._options = options + # Setup using self._options.param1 + + async def run(self): + # Implementation +``` + +**After (One class):** +```python +@dataclass +class MyTransport(Transport): + param1: str + param2: int = 42 + + def connect(self, controller_api: ControllerAPI, loop: asyncio.AbstractEventLoop): + self._controller_api = controller_api + # Setup using self.param1 directly + + async def serve(self): + # Implementation +``` + +### Architectural Impact + +This decision established a cleaner, more direct API for transport implementations, reducing complexity while maintaining full functionality. The unified Transport class became the single abstraction for all protocol implementations in FastCS. diff --git a/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md b/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md new file mode 100644 index 000000000..066568054 --- /dev/null +++ b/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md @@ -0,0 +1,191 @@ +# 8. Split Transport Dependencies into Optional Extras + +Date: 2025-09-30 + +**Related:** [PR #221](https://github.com/DiamondLightSource/FastCS/pull/221) + +## Status + +Accepted + +## Context + +FastCS supports multiple transport protocols for exposing controller APIs: EPICS CA, EPICS PVA, Tango, REST, and GraphQL. Originally, all transport dependencies were required dependencies, meaning they were always installed regardless of which transports users actually needed. + +**Original Architecture - All Dependencies Required:** + +```toml +# pyproject.toml +[project] +dependencies = [ + "fastapi[standard]", # For REST + "numpy", + "pydantic", + "pvi~=0.11.0", # For EPICS GUI generation + "pytango", # For Tango + "softioc>=4.5.0", # For EPICS CA + "strawberry-graphql", # For GraphQL + "p4p", # For EPICS PVA + "IPython", + "ruamel.yaml", +] +``` + +This meant every FastCS installation included: +- Python bindings for EPICS CA (softioc) +- Python bindings for EPICS PVA (p4p) +- Python bindings for Tango (pytango) +- Web frameworks (FastAPI, Strawberry GraphQL) +- GUI generation tools (pvi) + +**Problems with Required Dependencies:** + +The system needed a way to: +- Install only the dependencies for transports actually being used +- Allow minimal installations for simple use cases +- Make dependency relationships explicit and documented +- Support "install everything" for development +- Reduce installation failures for unused transports + +## Decision + +We split transport dependencies into optional extras in `pyproject.toml`, allowing users to install only what they need. + +### New Architecture + +**Core Dependencies (Minimal):** + +```toml +[project] +dependencies = [ + "pydantic", # Core data validation + "numpy", # Numeric types + "ruamel.yaml", # YAML config parsing + "IPython", # Interactive shell +] +``` + +**Transport-Specific Extras:** + +```toml +[project.optional-dependencies] +# Individual transports +epicsca = ["pvi~=0.11.0", "softioc>=4.5.0"] +epicspva = ["p4p", "pvi~=0.11.0"] +tango = ["pytango"] +graphql = ["strawberry-graphql", "uvicorn[standard]>=0.12.0"] +rest = ["fastapi[standard]", "numpy", "uvicorn[standard]>=0.12.0"] + +# Convenience groups +epics = ["fastcs[epicsca]", "fastcs[epicspva]"] +all = ["fastcs[epics]", "fastcs[tango]", "fastcs[graphql]", "fastcs[rest]"] + +# Development and demos +demo = ["tickit~=0.4.3"] +dev = [ + "fastcs[all]", # Dev installs everything + "fastcs[demo]", + # ... test tools, docs tools, etc. +] +``` + +### Installation Examples + +**1. Minimal Installation (Core only):** +```bash +pip install fastcs +# Only: pydantic, numpy, ruamel.yaml, IPython +``` + +**2. Single Transport:** +```bash +pip install fastcs[epicspva] # EPICS PVA only +pip install fastcs[rest] # REST API only +pip install fastcs[tango] # Tango only +``` + +**3. Multiple Transports:** +```bash +pip install fastcs[epics,rest] # EPICS CA + PVA + REST +``` + +**4. All Transports:** +```bash +pip install fastcs[all] # Everything +``` + +**5. Development:** +```bash +pip install fastcs[dev] # All transports + dev tools +``` + +### Key Benefits + +1. **Clearer Documentation:** + - Extras make dependencies explicit: `pip install fastcs[epicspva]` + - Users understand what each transport needs + - Self-documenting installation commands + +2. **Development Convenience:** + - `fastcs[dev]` installs everything for development + - `fastcs[all]` installs all transports without dev tools + - Clear separation of concerns + +## Consequences + +### Technical Changes + +- Updated `pyproject.toml`: + - Moved transport dependencies from `dependencies` to `optional-dependencies` + - Created extras: `epicsca`, `epicspva`, `tango`, `graphql`, `rest` + - Created convenience groups: `epics`, `all`, `dev`, `demo` + - Kept only core dependencies in main `dependencies` list + +### Migration Impact + +For existing users: + +**Before:** +```bash +pip install fastcs # Gets everything +``` + +**After:** +```bash +# Option 1: Get everything (same as before) +pip install fastcs[all] + +# Option 2: Get only what you need +pip install fastcs[epicspva] +pip install fastcs[rest,epics] +``` + +For developers: + +**Before:** +```bash +pip install -e .[dev] # Got everything +``` + +**After:** +```bash +pip install -e .[dev] # Still gets everything (includes [all]) +``` + +### Architectural Impact + +This decision established a flexible dependency model: + +``` +fastcs (core) + ├── fastcs[epicsca] → softioc, pvi + ├── fastcs[epicspva] → p4p, pvi + ├── fastcs[tango] → pytango + ├── fastcs[graphql] → strawberry-graphql, uvicorn + ├── fastcs[rest] → fastapi, uvicorn + ├── fastcs[epics] → [epicsca] + [epicspva] + ├── fastcs[all] → all transports + └── fastcs[dev] → [all] + dev tools +``` + +The split into optional extras aligned with FastCS's philosophy of supporting multiple transports while keeping the core lightweight. Users can now install exactly what they need, reducing friction and improving deployment efficiency, while developers can still easily install everything with `fastcs[dev]`. diff --git a/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md b/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md new file mode 100644 index 000000000..ff77d4fb8 --- /dev/null +++ b/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md @@ -0,0 +1,170 @@ +# 9. Replace Handler with AttributeIO/AttributeIORef Pattern + +Date: 2025-10-03 + +**Related:** [PR #218](https://github.com/DiamondLightSource/FastCS/pull/218) + +## Status + +Accepted + +## Context + +In the original FastCS architecture, the `Handler` pattern was used to manage attribute I/O operations. The design had several classes: + +**Original Handler Architecture:** +- `AttrHandlerR` previously `Updater` - Protocol for reading/updating attribute values +- `AttrHandlerW` previously `Sender` - Protocol for writing/setting attribute values +- `AttrHandlerRW` previously `Handler` - Combined read-write handler +- `SimpleAttrHandler` - Basic implementation for internal parameters + +**Limitations of the Handler Pattern:** + +1. **Handler Instance per Attribute:** Every attribute needed its own Handler instance because that's where the specification connecting the attribute to a unique resource lived. This created: + - Heavy memory overhead for controllers with many attributes + - Redundant Handler instances when multiple attributes used the same I/O pattern + - Difficulty sharing I/O logic across attributes + +2. **Circular Reference Loop:** The architecture created circular dependencies: + - Controller → Attributes (controller owns attributes) + - Attributes → Handlers (each attribute has a handler) + - Handlers → Controller (handlers need controller reference to communicate with device) + +3. **Tight Coupling to Controllers:** Handlers needed direct references to Controllers, coupling I/O logic to the controller structure rather than just to the underlying connections (e.g., hardware interfaces, network connections) + +4. **Mixed Concerns:** Handlers combined resource specification (what to connect to) with I/O behavior (how to read/write), making both harder to reason about + +The system needed a more flexible way to: +- Share a single AttributeIO instance across multiple attributes +- Use lightweight AttributeIORef instances to specify resource connections per-attribute +- Break the circular dependency chain +- Validate that Controllers have exactly one AttributeIO to handle each Attribute +- Separate resource specification from I/O behavior + +## Decision + +We replaced the `Handler` pattern with a two-component system: `AttributeIO` for behavior and `AttributeIORef` for configuration. + +### New Architecture + +1. **AttributeIORef - Lightweight Resource Specification:** +```python +@dataclass(kw_only=True) +class AttributeIORef: + update_period: float | None = None +``` + - Lightweight dataclass specifying resource connection details per-attribute + - Can be subclassed to add fields like resource names, register addresses, etc. + - Multiple attributes can have their own AttributeIORef instances + - Dynamically connected to a single AttributeIO instance at runtime + +2. **AttributeIO - Shared I/O Behavior:** +```python +class AttributeIO(Generic[T, AttributeIORefT]): + async def update(self, attr: AttrR[T, AttributeIORefT]) -> None: + raise NotImplementedError() + + async def send(self, attr: AttrRW[T, AttributeIORefT], value: T) -> None: + raise NotImplementedError() +``` + - Single instance per Controller can handle multiple Attributes + - Generic class parameterized by data type `T` and reference type `AttributeIORefT` + - Receives the AttributeIORef for each attribute to know which resource to access + - Only needs to know about connections (e.g. resource name, hardware interface) + +3. **Parameterized Attributes:** + - Attributes are now parameterized with `AttributeIORef` types + - `AttrR[T, AttributeIORefT]` - Read-only attribute with typed I/O reference + - `AttrRW[T, AttributeIORefT]` - Read-write attribute with typed I/O reference + - Type system ensures matching between AttributeIO and AttributeIORef + +4. **Initialization Validation:** + - Controller validates at initialization that it has exactly one AttributeIO to handle each Attribute + - Ensures all attributes are properly connected before the serving the Controller API + +### Key Improvements + +- **Breaks Circular Dependencies:** AttributeIO only needs connections, not Controllers +- **Memory Efficiency:** Single AttributeIO instance serves many Attributes +- **Separation of Concerns:** + - AttributeIORef: lightweight resource specification (what to connect to) + - AttributeIO: shared I/O behavior (how to read/write) +- **Validated Coverage:** Initialization ensures every Attribute has an AttributeIO handler +- **Type Safety:** Generic types ensure AttributeIO and AttributeIORef match +- **Extensibility:** Easy to create custom Ref types with resource-specific fields + +## Consequences + +### Technical Changes + +- 519 insertions, 290 deletions across 20 files +- Created new files: + - `src/fastcs/attribute_io.py` - AttributeIO base class + - `src/fastcs/attribute_io_ref.py` - AttributeIORef base class +- Updated attribute system to use generic parameterization +- Refactored all tests to use new pattern +- Updated callbacks to `AttrUpdateCallback` and `AttrSetCallback` + +### Migration Impact + +Users and developers needed to: + +**Before (Handler pattern - one instance per attribute):** +```python +# Temperature controller that communicates via TCP/IP +class TempControllerHandler(AttrHandlerRW): + def __init__(self, register_name: str, controller: TempController): + self.register_name = register_name + self.connection = connection + self.update_period = 0.2 + self.controller = controller + + async def initialise(self, controller): + self.controller_ref = controller # Creates circular dependency + + async def update(self, attr): + # Each attribute needs its own handler instance + query = f"{self.register_name}?" + response = await self.connection.send_query(f"{query}\r\n") + value = float(response.strip()) + await attr.set(value) + + async def put(self, attr, value): + command = f"{self.register_name}={value}" + await self.connection.send_command(f"{command}\r\n") + +controller = TempController() +ramp_rate = AttrRW(Float(), handler=TempControllerHandler("R", controller)) +power = AttrR(Float(), handler=TempControllerHandler("P", controller)) +setpoint = AttrRW(Float(), handler=TempControllerHandler("S", controller)) +``` + +**After (AttributeIO pattern - shared instance):** +```python +@dataclass +class TempControllerIORef(AttributeIORef): + name: str # Register name like "R", "P", "S" + update_period: float = 0.2 + +class TempControllerAttributeIO(AttributeIO[float, TempControllerIORef]): + def __init__(self, connection: IPConnection): + self.connection = connection + + async def update(self, attr: AttrR[float, TempControllerIORef]) -> None: + query = f"{attr.io_ref.name}?" + response = await self.connection.send_query(f"{query}\r\n") + value = float(response.strip()) + await attr.update(value) + + async def send(self, attr: AttrRW[float, TempControllerIORef], value: float) -> None: + command = f"{attr.io_ref.name}={value}" + await self.connection.send_command(f"{command}\r\n") + +connection = IPConnection() +temp_io = TempControllerAttributeIO(connection) +ramp_rate = AttrRW(Float(), io_ref=TempControllerIORef(name="R")) +power = AttrR(Float(), io_ref=TempControllerIORef(name="P")) +setpoint = AttrRW(Float(), io_ref=TempControllerIORef(name="S")) +``` + +This decision established a more flexible, type-safe foundation for attribute I/O operations, enabling better extensibility and maintainability for controller implementations. diff --git a/docs/explanations/decisions/0010-subcontroller-removal.md b/docs/explanations/decisions/0010-subcontroller-removal.md new file mode 100644 index 000000000..8a20646c3 --- /dev/null +++ b/docs/explanations/decisions/0010-subcontroller-removal.md @@ -0,0 +1,197 @@ +# 10. Remove SubController Class + +Date: 2025-10-01 + +**Related:** [PR #222](https://github.com/DiamondLightSource/FastCS/pull/222) + +## Status + +Accepted + +## Context + +FastCS originally provided two separate classes for building controller hierarchies: `Controller` for top-level device controllers and `SubController` for nested components within a controller. + +**Original Architecture - Separate Classes:** + +```python +class Controller(BaseController): + """Top-level controller for a device.""" + def __init__(self, description: str | None = None) -> None: + super().__init__(description=description) + async def connect(self) -> None: + pass + +class SubController(BaseController): + """A subordinate to a Controller for managing a subset of a device.""" + root_attribute: Attribute | None = None + def __init__(self, description: str | None = None) -> None: + super().__init__(description=description) +``` + +The key distinctions were +- `SubController` had a `root_attribute` property for exposing a single attribute to its + parent +- Only `Controller` could be used as the root of the hierarchy + +Type annotations enforced the hierarchy: + +```python +# In BaseController +def register_sub_controller(self, name: str, sub_controller: SubController): + """Only accepts SubController instances""" + ... +``` + +**Usage Pattern:** +```python +class TemperatureRampController(SubController): # Must use SubController + start = AttrRW(Int()) + end = AttrRW(Int()) + +class TemperatureController(Controller): # Must use Controller + def __init__(self): + super().__init__() + ramp = TemperatureRampController() + self.register_sub_controller("Ramp1", ramp) +``` + +**Problems with Two-Class Hierarchy:** + +1. **Unnecessary Type Coupling:** The classes were functionally identical. Having two classes created artificial separation without meaningful functional benefit. + +2. **Design-Time Commitment:** Developers had to choose `Controller` vs `SubController` at class definition time, before knowing all the contexts where the component might be used. A component designed as standalone might later need to become nested, forcing inheritance changes. + +3. **Reduced Reusability:** Controllers written as `SubController` couldn't be used as top-level controllers without changing their base class, and vice versa. This coupling hurt composition flexibility. + +The system needed a way to support hierarchical composition without committing to hierarchy at class definition time. + +## Decision + +We unified `Controller` and `SubController` into a single `Controller` class that can be used in both top-level and nested contexts. + +### New Architecture + +**Unified Controller Class:** + +```python +class Controller(BaseController): + """Controller for a device or device component.""" + + root_attribute: Attribute | None = None # NOW on Controller + + def __init__(self, description: str | None = None) -> None: + super().__init__(description=description) + + async def connect(self) -> None: + pass +``` + +**Updated BaseController Type Annotations:** + +```python +# In BaseController +def add_sub_controller(self, name: str, sub_controller: Controller): + """Now accepts any Controller instance""" + ... + +def get_sub_controllers(self) -> dict[str, Controller]: + """Returns Controller instances""" + return self.__sub_controller_tree +``` + +### Key Changes + +1. **Single Unified Class:** + - Removed `SubController` class entirely + - Moved `root_attribute` property to `Controller` + - Controller can now be used in any context + +2. **Usage-Based Hierarchy:** + - Whether a Controller is "top-level" or "nested" is determined by how it's used, not by its class + - Same Controller class can be instantiated standalone or nested in another Controller + +3. **Flexible Composition:** + - Any Controller can be added as a sub-controller to another Controller + - No inheritance changes needed to repurpose a controller + - Composition determined at instantiation, not class definition + +4. **Simplified API:** + - One class to understand and import + - Consistent patterns across all controller definitions + - Type annotations simplified throughout codebase + +### Migration Pattern: + +**Before (Two Classes):** +```python +from fastcs import Controller, SubController + +class RampController(SubController): # Forced to use SubController + start = AttrRW(Int()) + end = AttrRW(Int()) + +class TempController(Controller): # Forced to use Controller + def __init__(self): + super().__init__() + self.register_sub_controller("Ramp1", RampController()) +``` + +**After (One Class):** +```python +from fastcs import Controller + +class RampController(Controller): # Just use Controller + start = AttrRW(Int()) + end = AttrRW(Int()) + +class TempController(Controller): # Just use Controller + def __init__(self): + super().__init__() + self.add_sub_controller("Ramp1", RampController()) +``` + +`RampController` could then be used as a the root controller. + +### Benefits + +- **Composition over Inheritance:** Hierarchy determined by usage, not class definition + +- **Increased Reusability:** Controllers can be used in any context without refactoring + +- **Simpler Mental Model:** One class for all controller use cases + +- **Reduced Coupling:** No design-time commitment to hierarchy level + +- **Easier Evolution:** Controllers can start standalone and be nested later without code changes + +- **Consistent API:** Single class reduces cognitive load for developers + +## Consequences + +### Technical Changes + +- 30 insertions, 43 deletions +- Removed `SubController` class from `src/fastcs/controller.py` +- Added `root_attribute` property to `Controller` class +- Updated type annotations throughout codebase: `SubController` → `Controller` +- Updated method signatures in `src/fastcs/controllers/base_controller.py` +- Renamed `register_sub_controller` to `add_sub_controller` for consistency +- Updated documentation and examples +- Updated all test files to use unified `Controller` class + +### Migration Impact + +For controller developers: +1. Replace all `class MyController(SubController)` with `class MyController(Controller)` +2. Replace `register_sub_controller()` calls with `add_sub_controller()` +3. No functional changes required - same composition patterns work + +For existing codebases: +- Simple find-and-replace migration: `SubController` → `Controller` +- Backward compatible: composition patterns unchanged +- Controllers previously written as `Controller` or `SubController` now uniformly use `Controller` + +### Architectural Impact + +This decision reinforced the composition-over-inheritance principle in FastCS. The removal of `SubController` flattened the inheritance hierarchy while maintaining full composition capabilities, making FastCS controllers more flexible and easier to compose in different contexts. diff --git a/docs/explanations/decisions/0011-controller-vector-implementation.md b/docs/explanations/decisions/0011-controller-vector-implementation.md new file mode 100644 index 000000000..dcec6dd03 --- /dev/null +++ b/docs/explanations/decisions/0011-controller-vector-implementation.md @@ -0,0 +1,200 @@ +# 11. Introduce ControllerVector for Indexed Sub-Controllers + +Date: 2025-11-10 + +**Related:** [PR #192](https://github.com/DiamondLightSource/FastCS/pull/192) + +## Status + +Accepted + +## Context + +After removing the `SubController` class (ADR-0010), FastCS had a unified `Controller` class that could be used in both top-level and nested contexts. However, a common use case remained unaddressed: managing collections of identical sub-controllers distinguished only by an integer index. + +**Common Use Case - Multiple Identical Components:** + +Many devices have multiple identical components that need individual control: +- Multi-axis motion stages (X, Y, Z axes) +- Multi-channel power supplies (Channel 1, 2, 3, ...) +- Temperature controller ramp profiles (Ramp 1, 2, 3, ...) +- Camera ROI regions (ROI 0, 1, 2, ...) + +**Original Approach - Manual Registration:** + +Before ControllerVector, developers had to manually register each indexed controller: + +```python +class MotorController(Controller): + position = AttrRW(Float()) + velocity = AttrRW(Float()) + +class StageController(Controller): + def __init__(self, num_axes: int): + super().__init__() + self._motors: list[MotorController] = [] + + # Manual registration for each axis + for i in range(num_axes): + motor = MotorController() + self._motors.append(motor) + self.add_sub_controller(f"Axis{i}", motor) # String-based naming +``` + +**Problems with Manual Registration:** + +1. **No Collection Semantics:** Controllers registered with string names (`"Axis0"`, `"Axis1"`) didn't behave like a natural collection. No iteration, indexing, or length operations. + +2. **String-Based Indexing:** Accessing controllers required string manipulation: `controller.sub_controllers["Axis0"]` instead of natural indexing: `controller.axes[0]` + +The system needed a dedicated pattern for indexed collections that: +- Provides natural collection semantics +- Enforces integer-based indexing +- Generates a clear PVI structure for clients and UI generation +- Reduces boilerplate for common patterns + +## Decision + +We introduced `ControllerVector`, a specialized controller type for managing collections of indexed sub-controllers with dict-like semantics. + +### Key Features + +1. **MutableMapping Interface:** + - Implements `__getitem__`, `__setitem__`, `__iter__`, `__len__` + - Provides dict-like access: `vector[0]`, `vector[1]`, etc. + - Supports iteration: `for index, controller in vector.items()` + +2. **Type-Safe Integer Indexing:** + - Keys must be integers (enforced by type hints and runtime checks) + - Values must be Controller instances + - Clear error messages for type violations + +3. **Non-Contiguous Indices:** + - Indices don't need to be sequential: `{1: motor1, 5: motor5, 10: motor10}` is valid + - Useful for sparse collections or specific numbering schemes + +4. **Inherits from BaseController:** + - Can have its own attributes alongside indexed sub-controllers + - Can be nested within other controllers + +### Usage Pattern + +**With ControllerVector:** + +```python +class MotorController(Controller): + position = AttrRW(Float()) + velocity = AttrRW(Float()) + +class AxesVector(ControllerVector): + """Vector of motor axes with shared attributes""" + enabled = AttrRW(Bool()) # Shared attribute for all axes + +class StageController(Controller): + def __init__(self, num_axes: int): + super().__init__() + + # Create vector with indexed motors + motors = {i: MotorController() for i in range(num_axes)} + self.axes = AxesVector(motors, description="Motor axes") + + # Natural collection access + first_axis = self.axes[0] + for i, motor in self.axes.items(): + print(f"Motor {i}: {motor}") +``` + +**Alternative Inline Usage:** + +```python +class StageController(Controller): + def __init__(self, num_axes: int): + super().__init__() + + # Direct ControllerVector instantiation + self.axes = ControllerVector( + {i: MotorController() for i in range(num_axes)}, + description="Motor axes" + ) +``` + +### Benefits + +- **Natural Collection Semantics:** Dict-like interface provides familiar indexing, iteration, and length operations + +- **Consistency:** Integer-only keys prevent naming inconsistencies + +- **Clear Intent:** ControllerVector explicitly signals "collection of identical components" + +- **Sparse Collections:** Non-contiguous indices support flexible numbering schemes + +## Consequences + +### Technical Changes + +- 408 insertions, 315 deletions across 12 files +- Created `src/fastcs/controllers/controller_vector.py` with ControllerVector class +- Updated `src/fastcs/controller_api.py` to handle ControllerVector +- Enhanced EPICS PVI generation in `src/fastcs/transport/epics/pva/pvi.py` +- Simplified `src/fastcs/transport/epics/pva/pvi_tree.py` (207 lines removed) +- Updated EPICS CA and PVA IOC implementations +- Updated test examples to demonstrate ControllerVector usage + +### Migration Impact + +For controller developers with indexed collections: + +**Before (Manual registration):** +```python +class Controller: + def __init__(self): + super().__init__() + for i in range(4): + channel = ChannelController() + self.add_sub_controller(f"Ch{i}", channel) + + # Access via string keys + first = self.sub_controllers["Ch0"] +``` + +**After (ControllerVector):** +```python +class Controller: + def __init__(self): + super().__init__() + self.channels = ControllerVector( + {i: ChannelController() for i in range(4)}, + description="Power supply channels" + ) + + # Natural dict-like access + first = self.channels[0] + for i, channel in self.channels.items(): + ... +``` + +### Design Patterns Enabled + +**1. Subclassed Vectors with Shared Attributes:** +```python +class ChannelVector(ControllerVector): + master_enable = AttrRW(Bool()) # Shared across all channels + + def __init__(self, num_channels: int): + channels = {i: ChannelController(i) for i in range(num_channels)} + super().__init__(channels, description="Power channels") +``` + +**2. Sparse Indexing:** +```python +# Non-contiguous indices for specific addressing schemes +rois = ControllerVector({ + 1: ROIController(), + 5: ROIController(), + 10: ROIController() +}) +``` + +### Architectural Impact + +The introduction of ControllerVector established a clear pattern for managing collections of identical components, reducing boilerplate while improving type safety and UI generation capabilities. From 9b5725b9a6d55169a826159c42b4656004974cf7 Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Tue, 20 Jan 2026 13:26:22 +0000 Subject: [PATCH 2/2] Second pass --- ...oller-initialization-on-main-event-loop.md | 206 ++------------- .../0004-backend-to-transport-refactoring.md | 82 ++---- .../0005-background-thread-removal.md | 235 +++--------------- .../0006-controller-api-abstraction-layer.md | 165 +++--------- .../decisions/0007-transport-consolidation.md | 218 ++-------------- ...ansport-dependencies-as-optional-extras.md | 181 +++----------- .../0009-handler-to-attribute-io-pattern.md | 76 ++---- .../decisions/0010-subcontroller-removal.md | 171 ++----------- .../0011-controller-vector-implementation.md | 194 ++++----------- 9 files changed, 249 insertions(+), 1279 deletions(-) diff --git a/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md b/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md index 261b14379..f412d2f90 100644 --- a/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md +++ b/docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md @@ -10,206 +10,34 @@ Accepted ## Context -In the original FastCS architecture, the Backend class initialization pattern made it difficult for controllers to perform async setup operations on the main event loop before their API was exposed through the Mapping layer. - -**Original Architecture - Separate Initialization:** - -The AsyncioBackend used composition, creating separate components and manually orchestrating initialization: - -```python -class AsyncioBackend: - def __init__(self, mapping: Mapping): - self._mapping = mapping - - def run_interactive_session(self): - dispatcher = AsyncioDispatcher() - backend = Backend(self._mapping, dispatcher.loop) - - # Manual initialization sequence - backend.link_process_tasks() - backend.run_initial_tasks() - backend.start_scan_tasks() -``` - -The Backend was a thin wrapper that accepted a pre-created Mapping: - -```python -class Backend: - def __init__(self, mapping: Mapping, loop: asyncio.AbstractEventLoop): - self._mapping = mapping - self._loop = loop -``` - -The system needed a way to: -- Allow controllers to run async initialization code on the main event loop -- Ensure initialization happens before the Mapping is created -- Provide a consistent Backend base class with a clear lifecycle -- Centralize initialization orchestration in one place +The Backend accepts a pre-created Mapping with no async initialization hook for controllers. Each backend subclass manually orchestrates initialization steps, leading to inconsistent lifecycle patterns. Controllers need a way to perform async setup before their API is exposed to transports. ## Decision -We redesigned the Backend class to own the entire initialization lifecycle, including creating the event loop and running controller initialization on it before creating the Mapping. - -### New Architecture - -**Backend - Lifecycle Orchestrator:** - -```python -class Backend: - def __init__( - self, controller: Controller, loop: asyncio.AbstractEventLoop | None = None - ): - # 1. Create AsyncioDispatcher with optional loop - self._dispatcher = AsyncioDispatcher(loop) - self._loop = self._dispatcher.loop - self._controller = controller - - # 2. Add connect to initial tasks - self._initial_tasks.append(controller.connect) - - # 3. KEY CHANGE: Run controller.initialise() on main event loop - # BEFORE creating the Mapping - asyncio.run_coroutine_threadsafe( - self._controller.initialise(), self._loop - ).result() - - # 4. Create Mapping AFTER controller is initialized - self._mapping = Mapping(self._controller) - self._link_process_tasks() - - # 5. Build context dictionary for subclasses - self._context.update({ - "dispatcher": self._dispatcher, - "controller": self._controller, - "mapping": self._mapping, - }) - - def run(self): - self._run_initial_tasks() - self._start_scan_tasks() - self._run() # Subclass-specific implementation -``` - -### Key Changes - -1. **Controller Initialization on Main Event Loop:** - - Added `initialise()` method to Controller class as an async initialization hook - - Backend calls `asyncio.run_coroutine_threadsafe(controller.initialise(), self._loop).result()` - - This happens BEFORE the Mapping is created, allowing controllers to set up async resources - -2. **Backend Accepts Controller, Not Mapping:** - - **Before:** `Backend(mapping: Mapping, loop: AbstractEventLoop)` - - **After:** `Backend(controller: Controller, loop: AbstractEventLoop | None = None)` - - Backend now owns creating the Mapping after initialization - -3. **Inheritance-Based Architecture:** - - **Before:** AsyncioBackend used composition, creating Backend separately - - **After:** All backends inherit from Backend base class - - Subclasses implement `_run()` for protocol-specific execution +Move initialisation logic into Backend so that it: +- Creates the event loop +- Runs `controller.initialise()` before creating the Mapping +- Creates the Mapping from the initialized controller +- Runs initial tasks including `controller.connect()` +- Delegates to transport-specific implementations -4. **Centralized Lifecycle:** - - Backend orchestrates: `initialise()` → create Mapping → `connect()` → scan tasks → `_run()` - - Clear, documented initialization sequence - - Consistent across all backend types - -5. **Unified Context Management:** - - Backend maintains `_context` dictionary with dispatcher, controller, and mapping - - Passed to subclass `_run()` implementations - - Consistent context across all backends - -### Controller Lifecycle - -```python -class Controller(BaseController): - async def initialise(self) -> None: - """Called on main event loop BEFORE Mapping creation. - Override to perform async setup (database connections, etc.)""" - pass - - async def connect(self) -> None: - """Called as initial task AFTER Mapping creation. - Override to establish device connections.""" - pass -``` - -### Benefits - -- **Async Initialization Support:** Controllers can now perform async setup on the main - event loop before their API is exposed. This allows introspecting devices to create - Attributes and Methods dynamically. - -- **Predictable Lifecycle:** Clear initialization sequence: initialise → Mapping → connect → scan - -- **Better Separation of Concerns:** - - Backend: Infrastructure and lifecycle management - - Subclasses: Protocol-specific `_run()` implementation - - Controller: Domain logic and initialization - -- **Cleaner API:** Backends accept Controller directly, not pre-created Mapping +Controller then has two hooks: `initialise()` for pre-API setup (hardware introspection, dynamic attribute creation) and `connect()` for post-API connection logic. ## Consequences -### Technical Changes +The new design the initialisation of the application and makes the API for writing controllers and transports simpler and more flexible. -- 358 insertions, 217 deletions across 13 files -- Restructured `src/fastcs/backend.py` with new lifecycle orchestration -- Added `initialise()` method to `src/fastcs/controller.py` -- Updated all backend implementations to inherit from Backend: - - `src/fastcs/backends/asyncio_backend.py` - - `src/fastcs/backends/epics/backend.py` - - `src/fastcs/backends/tango/backend.py` -- Updated corresponding IOC/runtime implementations: - - `src/fastcs/backends/epics/ioc.py` - - `src/fastcs/backends/tango/dsr.py` - -### Migration Impact +### Migration Pattern For controller developers: -1. Can now override `initialise()` for async setup on main event loop -2. `connect()` continues to work as before for device connection logic -3. Clear lifecycle: `initialise()` runs first, then `connect()` as initial task - -For backend developers: -1. Backends now inherit from Backend base class -2. Constructor signature changed: accepts Controller, not Mapping -3. Implement `_run()` method instead of managing full initialization -4. Access `self._context` for dispatcher, controller, and mapping -Example transformation: ```python -# Before -class EpicsBackend: - def __init__(self, mapping: Mapping): - dispatcher = AsyncioDispatcher() - backend = Backend(mapping, dispatcher.loop) - backend.run_initial_tasks() - -# After -class EpicsBackend(Backend): - def __init__(self, controller: Controller, pv_prefix: str): - super().__init__(controller) # Handles initialization - self._ioc = EpicsIOC(pv_prefix, self._mapping) - - def _run(self): - self._ioc.run(self._dispatcher, self._context) -``` - -### Architectural Impact - -This established a clear, consistent initialization pattern across all FastCS backends: - -``` -Backend.__init__: - 1. Create AsyncioDispatcher (main event loop) - 2. Run controller.initialise() on main event loop ← NEW - 3. Create Mapping from controller - 4. Link process tasks - 5. Prepare context +class MyController(Controller): + async def initialise(self) -> None: + # Async setup before API creation (introspect hardware, create attributes) + await self._hardware.connect() -Backend.run(): - 6. Run initial tasks (controller.connect(), etc.) - 7. Start scan tasks - 8. Call subclass._run() for protocol-specific execution + async def connect(self) -> None: + # Async setup after API creation (establish connections) + await self._hardware.initialize() ``` - -The addition of `controller.initialise()` on the main event loop enabled controllers to perform complex async setup operations before their API was exposed, while maintaining a clean separation between framework infrastructure and protocol implementations. diff --git a/docs/explanations/decisions/0004-backend-to-transport-refactoring.md b/docs/explanations/decisions/0004-backend-to-transport-refactoring.md index 0dee7c5b8..48ad3b15f 100644 --- a/docs/explanations/decisions/0004-backend-to-transport-refactoring.md +++ b/docs/explanations/decisions/0004-backend-to-transport-refactoring.md @@ -10,76 +10,46 @@ Accepted ## Context -In the original FastCS architecture, the term "backend" was used to describe both: -1. The overall framework/system that managed controllers (the "backend system") -2. The specific communication protocol implementations (EPICS CA, PVA, REST, Tango) +The original FastCS architecture used the term "backend" ambiguously to describe both the overall framework that managed controllers and the specific communication protocol implementations (EPICS CA, PVA, REST, Tango). This dual usage created confusion: -**Original Architecture:** -- There was a `Backend` base class that protocol implementations inherited from -- Protocol-specific classes like `EpicsBackend`, `RestBackend`, `TangoBackend` extended `Backend` -- Users worked directly with these inherited backend classes -- This created tight coupling between the core framework and protocol implementations - -This dual usage of "backend" created confusion for developers and users: - It was unclear whether "backend" referred to the framework itself or the protocol layer - The terminology didn't clearly differentiate between the abstract framework and the underlying communication mechanisms - The inheritance pattern made it difficult to compose multiple transports or swap them dynamically ## Decision -We renamed "backend" to "transport" for all protocol/communication implementations to clearly differentiate them from the abstract framework. - -This refactoring involved: - -1. **Terminology Change:** - - `backends/` directory → `transport/` directory - - `EpicsBackend` → `EpicsTransport` - - `RestBackend` → `RestTransport` - - `TangoBackend` → `TangoTransport` - -2. **Architectural Improvements:** - - Introduced `TransportAdapter` abstract base class defining the contract for all transports: - - `run()` - Start the transport - - `create_docs()` - Generate documentation - - `create_gui()` - Generate GUI metadata - - Split transport configuration into separate `options` modules - - Added adapter pattern with template methods for consistency +Rename "backend" to "transport" for all protocol/communication implementations to clearly differentiate them from the framework. -3. **Plugin Architecture (Composition over Inheritance):** - - **Before:** `Backend` base class with `EpicsBackend`, `RestBackend`, etc. inheriting from it - - **After:** `FastCS` core class that accepts `Transport` implementations as plugins - - Transports are now passed to `FastCS` rather than being subclasses of a framework class - - This enables flexible composition, runtime transport selection, and loose coupling +The term "backend" can now refer to the overall FastCS framework/system, while "transport" specifically refers to protocol implementations (EPICS CA, PVA, REST, GraphQL, Tango). FastCS accepts Transport implementations as plugins, enabling flexible composition and loose coupling. -4. **Public API Restructuring:** - - Introduced `FastCS` class as the programmatic interface for running controllers with transports - - Added `launch()` function as the primary entry point for initializing controllers - - Cleaned up import namespace to add structure to the public API - -The term "backend" was reserved for referring to the overall FastCS framework/system, while "transport" specifically refers to protocol implementations (EPICS CA, PVA, REST, GraphQL, Tango). +Key architectural changes: +- Introduce `TransportAdapter` abstract base class with standardized interface +- Move to composition-based architecture where transports are passed to `FastCS` rather than being subclasses +- Introduce `FastCS` class as the programmatic interface for running controllers with transports +- Add `launch()` function as the primary entry point for initializing controllers ## Consequences -- **Clearer Terminology:** The separation between framework (backend) and protocol layer (transport) is now explicit and unambiguous -- **Consistent Architecture:** All transports follow the adapter pattern with a standardized interface -- **Better Separation of Concerns:** Transport configuration is separated from transport implementation via options modules -- **Improved Extensibility:** Adding new transport protocols is more straightforward with the adapter pattern -- **Reduced Dependency Coupling:** Transport options can be imported without including heavy transport dependencies -- **Clearer Public API:** The `launch()` function and `FastCS` class provide clear entry points +### Benefits -### Migration Path +- **Clear Terminology:** The separation between framework (backend) and protocol layer (transport) is now explicit +- **Consistent Architecture:** All transports follow the adapter pattern with a standardized interface +- **Flexible Composition:** Transports can be added, removed, or swapped at runtime +- **Improved Extensibility:** Adding new transport protocols is straightforward with the adapter pattern -For users migrating from the old API: -1. Replace all `backend` imports with `transport` imports -2. Update class names (e.g., `EpicsBackend` → `EpicsTransport`) -3. Prefer using the new `launch()` function instead of directly instantiating transports -4. Update configuration to use transport-specific options dataclasses +### Migration Pattern -### Technical Impact +**Before (Inheritance hierarchy):** +```python +class EpicsBackend(Backend): + def run(self): + # Protocol-specific implementation -- 681 insertions, 321 deletions across the codebase -- All transport implementations refactored to follow adapter pattern -- Transport options moved to separate modules for cleaner dependency management -- Mapping class functionality integrated into Controller +fastcs = FastCS(controller) # Tightly coupled to framework +``` -This decision established a clearer architectural foundation for FastCS, making it easier for contributors to understand the system's layers and for users to reason about how their controllers interact with different communication protocols. +**After (Composition with Transport plugins):** +```python +transport = EpicsTransport(controller_api) +fastcs = FastCS(controller, [transport]) +``` diff --git a/docs/explanations/decisions/0005-background-thread-removal.md b/docs/explanations/decisions/0005-background-thread-removal.md index 62e8c01d9..bb1fa92b7 100644 --- a/docs/explanations/decisions/0005-background-thread-removal.md +++ b/docs/explanations/decisions/0005-background-thread-removal.md @@ -10,243 +10,76 @@ Accepted ## Context -The original FastCS Backend implementation used `asyncio.run_coroutine_threadsafe()` to execute controller operations on a background event loop thread managed by `AsyncioDispatcher`. This pattern was inherited from EPICS softIOC's threading model. +The current FastCS Backend implementation uses `asyncio.run_coroutine_threadsafe()` to execute controller operations on a background event loop thread managed by `AsyncioDispatcher`. This threading model creates several problems: -**Original Architecture - Background Thread:** +- **Thread Safety Complexity:** Managing state across thread boundaries introduced race conditions and required careful synchronization +- **Blocked Main Thread:** Despite using async, the main thread blocked waiting for background thread results via `.result()` +- **Complex Lifecycle Management:** Starting and stopping the background thread added complexity +- **Difficult to Test:** Background threads made tests non-deterministic and harder to debug +- **Unnecessary for Most Transports:** Only EPICS CA softIOC actually needed a background thread; other transports (REST, GraphQL, PVA) could run entirely async -```python -class Backend: - def __init__( - self, - controller: Controller, - loop: asyncio.AbstractEventLoop | None = None, - ): - # Create dispatcher with its own thread - self.dispatcher = AsyncioDispatcher(loop) - self._loop = self.dispatcher.loop # Runs in background thread - - # Run initialization on background thread - asyncio.run_coroutine_threadsafe( - self._controller.initialise(), self._loop - ).result() # Block until complete - - def _run_initial_futures(self): - for coro in self._initial_coros: - # Schedule on background thread, block main thread - future = asyncio.run_coroutine_threadsafe(coro(), self._loop) - future.result() - - def start_scan_futures(self): - # Store Future objects from background thread - self._scan_futures = { - asyncio.run_coroutine_threadsafe(coro(), self._loop) - for coro in _get_scan_coros(self._controller) - } -``` - -This approach created a threading model where: -- Main thread creates Backend -- AsyncioDispatcher starts background thread with event loop -- Controller operations scheduled from main thread to background thread -- Main thread blocks waiting for background thread results via `.result()` - -**Problems with Background Thread:** - -1. **Thread Safety Complexity:** Managing state across thread boundaries introduced race conditions and required careful synchronization: - - Controller state accessed from both threads - - Attribute updates needed thread-safe mechanisms - - Future cancellation had timing issues - -2. **Blocking Main Thread:** Despite using async, the main thread blocked waiting for background thread operations: - ```python - future = asyncio.run_coroutine_threadsafe(coro(), self._loop) - future.result() # Main thread blocks here - ``` - This defeated the purpose of async programming - -3. **Complex Lifecycle Management:** Starting and stopping the background thread added complexity: - - Ensuring thread cleanup in `__del__` - - Managing Future objects for scan tasks - - Cancelling futures vs cancelling tasks had different semantics - -4. **Difficult to Test:** Background threads made tests: - - Non-deterministic (race conditions) - - Harder to debug (multiple call stacks) - - Slower (thread startup overhead) - -5. **Unnecessary for Most Transports:** Only EPICS CA softIOC actually required a background thread (due to its C library's threading requirements). Other transports (REST, GraphQL, Tango, PVA) could run entirely async: - - REST/GraphQL use async frameworks (FastAPI, Strawberry) - - EPICS PVA is pure Python async - -6. **Future vs Task Confusion:** Using `Future` objects from `run_coroutine_threadsafe()` instead of `Task` objects from `create_task()` created API inconsistencies - -The system needed a simpler concurrency model that: -- Runs all async code on a single event loop -- Uses native async/await throughout -- Eliminates thread synchronization complexity -- Allows transports to manage their own threading if needed +The system needs a simpler concurrency model that uses native async/await patterns and allows transports to manage their own threading if needed. ## Decision -We removed the background thread from Backend, making it fully async and allowing transports to manage their own threading requirements if needed. - -### New Architecture - -**Async-Only Backend:** - -```python -class Backend: - def __init__( - self, - controller: Controller, - loop: asyncio.AbstractEventLoop, # Now required, not created - ): - self._loop = loop # Caller's event loop - self._controller = controller - - self._initial_coros = [controller.connect] - self._scan_tasks: set[asyncio.Task] = set() # Tasks, not Futures - - # Run initialization on PROVIDED event loop (not background thread) - loop.run_until_complete(self._controller.initialise()) - self._link_process_tasks() - - async def serve(self): - """Fully async - no blocking""" - await self._run_initial_coros() - await self._start_scan_tasks() - - async def _run_initial_coros(self): - """Direct await - no threading""" - for coro in self._initial_coros: - await coro() - - async def _start_scan_tasks(self): - """Create tasks on same event loop""" - self._scan_tasks = { - self._loop.create_task(coro()) - for coro in _get_scan_coros(self._controller) - } -``` - -### Key Changes - -1. **No AsyncioDispatcher:** - - Removed `self.dispatcher = AsyncioDispatcher(loop)` - - Backend now receives event loop from caller - - No background thread creation - -2. **Synchronous Initialization:** - - **Before:** `asyncio.run_coroutine_threadsafe().result()` (cross-thread blocking) - - **After:** `loop.run_until_complete()` (same-thread blocking) - - Initialization happens before event loop starts running +Remove the background thread from Backend, making it fully async, while allowing specific transports to use a background thread if required. Tango does require this because it does not accept an event loop to run on. Backend now accepts an event loop from the caller and uses native async/await throughout. Transports that need threading (like EPICS CA) manage their own threading explicitly. -3. **Async serve() Method:** - - **Before:** `run()` method that scheduled futures - - **After:** `async serve()` method using native async/await - - Can be composed with other async operations +Key architectural changes: +- Backend receives event loop from caller (no background dispatcher) +- Initialization uses `loop.run_until_complete()` instead of cross-thread scheduling +- Backend exposes `async serve()` method using native async/await patterns +- Scan tasks use `Task` objects from `loop.create_task()` instead of `Future` objects +- Transports that need threading create their own `AsyncioDispatcher` when needed -4. **Tasks Instead of Futures:** - - **Before:** `asyncio.run_coroutine_threadsafe()` returns `Future` - - **After:** `loop.create_task()` returns `Task` - - Consistent with async best practices - -5. **Transport-Managed Threading:** - - EPICS CA transport creates its own AsyncioDispatcher when needed - - Other transports run purely async - - Each transport decides its threading model +## Consequences ### Benefits -- **Simplified Concurrency Model:** Single event loop for most operations, no cross-thread coordination - +- **Simpler Concurrency Model:** Single event loop for most operations, no cross-thread coordination needed - **True Async/Await:** Native Python async patterns throughout, no blocking `.result()` calls - -- **Better Testability:** Deterministic execution, easier to debug, faster tests - -- **Clearer Responsibility:** Transports that need threads manage them explicitly - -- **Task-Based API:** Consistent use of `Task` objects with cancellation support - +- **Better Testability:** Deterministic execution, easier to debug, no thread scheduling delays +- **Clearer Responsibility:** Transports explicitly manage threading they need +- **Task-Based API:** Consistent use of `Task` objects with standard cancellation support - **Composability:** `async serve()` can be composed with other async operations -- **Performance:** Eliminated thread creation overhead for transports that don't need it - -## Consequences - -The Tango transport still requires being run on in a background thread because it does -not allow an event loop to be passed it. It always creates its own and then the issues -with coroutines being called from the wrong event loop persist. - -### Technical Changes - -- 769 insertions, 194 deletions across 27 files -- Removed `AsyncioDispatcher` from `src/fastcs/backend.py` -- Changed `Backend.__init__()` to accept event loop (not create it) -- Changed `Backend.run()` to `async Backend.serve()` -- Updated initialization from `run_coroutine_threadsafe()` to `run_until_complete()` -- Changed scan task management from `Future` to `Task` objects -- Updated all transport adapters: - - `src/fastcs/transport/epics/adapter.py` - Now creates dispatcher - - `src/fastcs/transport/graphql/adapter.py` - Pure async - - `src/fastcs/transport/rest/adapter.py` - Pure async - - `src/fastcs/transport/tango/adapter.py` - Updated async handling -- Added benchmarking tests in `tests/benchmarking/` -- Updated `src/fastcs/launch.py` to handle async serve - -### Migration Impact - -For transport developers: +### Migration Pattern **Before (Background thread in Backend):** ```python class MyTransport(TransportAdapter): def __init__(self, controller_api, loop): - # Backend created its own thread - self._backend = Backend(controller) + self._backend = Backend(controller) # Creates background thread def run(self): self._backend.run() # Synchronous - # Do transport-specific work + +# Client code +asyncio.run_coroutine_threadsafe(coro(), self._loop) # Cross-thread scheduling +future.result() # Main thread blocks ``` -**After (Async serve):** +**After (Async-only Backend):** ```python class MyTransport(Transport): def connect(self, controller_api, loop): - # Pass loop to Backend - self._backend = Backend(controller, loop) + self._backend = Backend(controller, loop) # Use caller's loop async def serve(self): - await self._backend.serve() # Async - # Do transport-specific work -``` + await self._backend.serve() # Native async -For transports needing threads (like EPICS CA): +# Client code +await self._backend.serve() # Direct await, no threading +``` +**For transports needing threads (EPICS CA):** ```python class EpicsCATransport(Transport): def __init__(self): - # Transport creates its own dispatcher - self._dispatcher = AsyncioDispatcher() + self._dispatcher = AsyncioDispatcher() # Transport manages its own thread def connect(self, controller_api, loop): - # Use dispatcher's loop for Backend self._backend = Backend(controller, self._dispatcher.loop) async def serve(self): - # Bridge to threaded environment - await self._backend.serve() + await self._backend.serve() # Bridge to threaded environment ``` - -### Performance Impact - -Benchmarking showed: -- Faster startup (no thread creation for most transports) -- Reduced memory overhead (no extra thread stack) -- More predictable timing (no thread scheduling delays) -- Better CPU utilization (single event loop) - -### Architectural Impact - -The removal of the background thread simplified FastCS's concurrency model, making it more predictable, testable, and performant while still supporting transports that require threading (like EPICS CA) through explicit transport-level thread management. diff --git a/docs/explanations/decisions/0006-controller-api-abstraction-layer.md b/docs/explanations/decisions/0006-controller-api-abstraction-layer.md index a313931eb..e372bbe96 100644 --- a/docs/explanations/decisions/0006-controller-api-abstraction-layer.md +++ b/docs/explanations/decisions/0006-controller-api-abstraction-layer.md @@ -10,141 +10,58 @@ Accepted ## Context -In the original FastCS architecture, transport implementations (EPICS CA, PVA, REST, GraphQL, Tango) directly accessed `Controller` and `SubController` instances to extract attributes, methods, and metadata for serving over their respective protocols. +Transports currently access `Controller` instances directly to extract attributes, methods, and metadata for serving over their protocols. This tight coupling creates a few problems: -**Original Architecture - Direct Controller Access:** -- Each transport was responsible for traversing the controller and its sub-controllers to find attributes and methods -- Transports had direct access to controller internals, allowing them to modify things they shouldn't -- Each transport implemented its own custom traversal logic -- No static, immutable view of what a controller exposes - -**Problems with Direct Coupling:** - -1. **Tight Coupling:** Transports were tightly coupled to the internal structure of `Controller` and `SubController` classes, making it difficult to evolve the controller implementation without breaking transports - -2. **Code Duplication:** Every transport re-implemented similar logic to: - - Traverse the controller/sub-controller tree - - Discover attributes and methods - - Build hierarchical structures for their protocol - - Handle naming and path construction - -3. **No Encapsulation:** Transports had direct access to mutable controller state, making it possible to inadvertently modify things they shouldn't be able to change - -4. **No Static View:** Controllers expose their API dynamically during traversal rather than providing a complete, static snapshot after initialization - -The system needed a way to: -- Provide a complete, static view of the controller API once it's fully initialized -- Prevent transports from having direct access to mutable controller internals -- Decouple transport implementations from controller internals -- Provide a consistent view of controller APIs across all transports -- Centralize the logic for extracting and organizing controller metadata +- **Tight Coupling:** Transports are coupled to internal Controller structure, making evolution difficult +- **Code Duplication:** Every transport re-implemented similar traversal logic for discovering attributes and methods +- **No Encapsulation:** Transports had direct access to mutable controller state +- **No Static View:** No complete, immutable snapshot of controller API after initialization ## Decision -We introduced `ControllerAPI` as an abstraction layer that sits between `Controller` instances and transport implementations. - -### New Architecture - -**ControllerAPI - Transport-Independent Representation:** -```python -@dataclass -class ControllerAPI: - path: list[str] - attributes: dict[str, Attribute] - command_methods: dict[str, Command] - put_methods: dict[str, Put] - scan_methods: dict[str, Scan] - sub_apis: dict[str, "ControllerAPI"] - description: str | None - - def walk_api(self) -> Iterator["ControllerAPI"]: - """Traverse the entire API tree""" -``` - -### Key Changes - -1. **API Extraction Centralized:** - - A single `create_controller_api()` function extracts the API from a Controller - - This function handles all traversal, method discovery, and hierarchy building - - All transports use the same API extraction logic - -2. **Transports Accept ControllerAPI:** - - **Before:** `EpicsCAIOC(pv_prefix, controller: Controller)` - - **After:** `EpicsCAIOC(pv_prefix, controller_api: ControllerAPI)` - - Transports no longer have direct access to mutable Controller internals - - ControllerAPI provides a static, read-only view of the controller's complete API - -3. **Complete API After Initialization:** - - Once a controller is fully initialized, its entire API (all sub-controllers, attributes, and methods) is exposed in `ControllerAPI` - - Transports receive a complete snapshot rather than having to traverse dynamically - - Changes to the controller after initialization don't affect the ControllerAPI - -4. **Hierarchical API Structure:** - - ControllerAPI contains nested `sub_apis` for sub-controllers, forming a tree structure - - `walk_api()` method provides depth-first traversal - - Each level includes its path for proper naming +Introduce `ControllerAPI` as an abstraction layer that provides transports with a complete, static, read-only representation of a controller's capabilities after initialization. -5. **Complete Method Discovery:** - - Separate dictionaries for command_methods, put_methods, scan_methods - - Methods discovered and organized during API creation - - Transports receive ready-to-use method metadata +All transports now work with `ControllerAPI` instead of direct `Controller` access. A single `create_controller_api()` function handles all API extraction, replaces custom traversal logic in each transport. -### Benefits: - -- **Static View After Initialization:** Once a controller is fully initialized, its entire API (all sub-controllers, attributes, and methods) is exposed in a complete, static snapshot - -- **Encapsulation:** Transports cannot directly access or modify mutable controller internals - they work with a read-only ControllerAPI. The only attributes and methods themselves are mutable. - -- **Separation of Concerns:** - - Controllers focus on device logic - - ControllerAPI handles API representation - - Transports focus on protocol implementation - -- **Single Source of Truth:** One canonical representation of what a controller exposes - -- **Reduced Code Duplication:** Traversal and extraction logic written once - -- **Testability:** Controllers can be tested with mock ControllerAPIs, transports can be tested with synthetic APIs - -- **Evolution Independence:** Controller internals can change without affecting transports as long as API extraction is updated +Key architectural changes: +- `ControllerAPI` dataclass represents the complete, hierarchical structure of what a controller exposes +- Separate dictionaries for attributes, command_methods, put_methods, and scan_methods +- `walk_api()` method provides depth-first traversal of the API tree +- Backend creates ControllerAPI once during initialization, before passing to transports ## Consequences -### Technical Changes +### Benefits -- 1,009 insertions, 568 deletions across 32 files -- Created `src/fastcs/controller_api.py` with ControllerAPI dataclass -- Updated all transport adapters to accept ControllerAPI instead of Controller: - - `src/fastcs/transport/epics/ca/adapter.py` - - `src/fastcs/transport/epics/pva/adapter.py` - - `src/fastcs/transport/graphQL/adapter.py` - - `src/fastcs/transport/rest/adapter.py` - - `src/fastcs/transport/tango/adapter.py` -- Refactored `src/fastcs/cs_methods.py` for method extraction -- Updated `src/fastcs/backend.py` to create ControllerAPI before passing to transports -- All transport IOC/server implementations updated to use ControllerAPI +- **Encapsulation:** Transports work with read-only API, cannot modify controller internals +- **Single Source of Truth:** One canonical representation of controller capabilities +- **Reduced Code Duplication:** Traversal and extraction logic written once, used by all transports +- **Separation of Concerns:** Controllers focus on device logic, ControllerAPI handles representation, transports focus on protocol +- **Testability:** Transports can be tested with synthetic ControllerAPIs; controllers tested independently +- **Evolution Independence:** Controller internals can change without affecting transports -### Migration Impact +### Migration Pattern -For transport developers: -1. Update transport constructors to accept `ControllerAPI` instead of `Controller` -2. Replace direct attribute/method access with ControllerAPI dictionary lookups -3. Use `walk_api()` for tree traversal instead of custom logic -4. Remove custom extraction code in favor of ControllerAPI structure - -For controller developers: -- No changes required - controllers continue to work as before -- ControllerAPI is created automatically by the backend - -### Architectural Impact - -This established a clean layered architecture: -``` -Controllers (domain logic) - ↓ -ControllerAPI (canonical representation) - ↓ -Transports (protocol implementation) +**Before (Direct Controller access):** +```python +class EpicsCAIOC: + def __init__(self, pv_prefix: str, controller: Controller): + # Each transport traverses controller itself + for attr_name in dir(controller): + attr = getattr(controller, attr_name) + if isinstance(attr, Attribute): + self._create_pv(f"{pv_prefix}{attr_name}", attr) ``` -The ControllerAPI became the primary contract between the FastCS core and transport implementations, ensuring all transports have a consistent, complete view of controller capabilities. +**After (ControllerAPI abstraction):** +```python +class EpicsCAIOC: + def __init__(self, pv_prefix: str, controller_api: ControllerAPI): + # Transport receives ready-to-use API structure + for attr_name, attr in controller_api.attributes.items(): + self._create_pv(f"{pv_prefix}{attr_name}", attr) + + # Walk sub-controllers using standard method + for sub_api in controller_api.walk_api(): + # Process sub-controllers with consistent structure +``` diff --git a/docs/explanations/decisions/0007-transport-consolidation.md b/docs/explanations/decisions/0007-transport-consolidation.md index 375571213..5248d73ac 100644 --- a/docs/explanations/decisions/0007-transport-consolidation.md +++ b/docs/explanations/decisions/0007-transport-consolidation.md @@ -10,209 +10,42 @@ Accepted ## Context -Following the Backend-to-Transport refactoring (ADR-0004), FastCS transports were implemented using two separate class hierarchies: `TransportAdapter` for the transport implementation and separate `*Options` classes for configuration. +FastCS transports are implemented using two separate classes: `TransportAdapter` for implementation and separate `*Options` classes for configuration. This pattern requires: -**Original Architecture - Two-Class Pattern:** - -Each transport required two classes: - -```python -# Configuration class -@dataclass -class EpicsPVAOptions: - pv_prefix: str - gui_dir: Path | None = None - -# Implementation class -class EpicsPVATransport(TransportAdapter): - def __init__( - self, - controller_api: ControllerAPI, - options: EpicsPVAOptions, - ): - self._controller_api = controller_api - self._options = options - # ... setup code - - async def run(self): - # ... transport-specific code -``` - -**Usage Pattern:** -```python -# Create options -options = EpicsPVAOptions(pv_prefix="MY:DEVICE:") - -# Pass options to FastCS -fastcs = FastCS(controller, [options]) - -# FastCS internally: -# - Matched options type -# - Created corresponding TransportAdapter -# - Passed options to adapter -``` - -**Problems with Two-Class Pattern:** - -1. **API Surface Duplication:** Two classes per transport (5 transports = 10 classes) when a single class could suffice. - -2. **Complex Instantiation Logic:** FastCS needed pattern matching logic to create the right adapter from options: - ```python - match option: - case EpicsPVAOptions(): - transport = EpicsPVATransport(controller_api, option) - case EpicsCAOptions(): - transport = EpicsCATransport(controller_api, loop, option) - # ... 5 more cases - ``` - -3. **Inconsistent Constructor Signatures:** Different transports had different initialization requirements: - - Some needed `loop`, others didn't - - Some needed `controller_api` immediately, others deferred it - - Options were passed separately, requiring attribute access: `self._options.pv_prefix` - -4. **Unnecessary Abstraction Layer:** The Options classes were simple dataclasses with no behavior - they existed solely to carry configuration to the adapter - -The system needed a simpler, more direct approach where transport configuration and implementation were unified. +- Two classes per transport +- Pattern matching logic in FastCS to create the right adapter from options +- Inconsistent constructor signatures across transports +- Redundant Options classes that only carry configuration data ## Decision -We merged `TransportAdapter` and `*Options` classes into a single `Transport` base class that combines configuration and implementation. +Merge `TransportAdapter` and `*Options` classes into a single `Transport` dataclass that combines configuration and implementation. -### New Architecture +All transports should follow a unified pattern: configuration fields are dataclass attributes, and `connect()` and `serve()` methods handle initialization and execution. FastCS accepts Transport instances directly. -**Unified Transport Class:** - -```python -@dataclass -class EpicsPVATransport(Transport): - epicspva: EpicsIOCOptions - - def connect( - self, controller_api: ControllerAPI, loop: asyncio.AbstractEventLoop - ) -> None: - self._controller_api = controller_api - self._loop = loop - self._ioc = EpicsPVAIOC( - pv_prefix=self.epicspva.pv_prefix, - controller_api=controller_api, - ) - - async def serve(self) -> None: - await self._ioc.run(self.epicspva) -``` +Key architectural changes: +- All transports use `@dataclass` decorator combining configuration and implementation +- Standardized `connect(controller_api, loop)` method for deferred initialization +- Standardized `async serve()` method for running the transport +- Removed pattern matching logic from FastCS +- Configuration fields are direct attributes (not nested in options object) -### Key Changes - -1. **Single Class per Transport:** - - Merged adapter and options into one dataclass - - Configuration fields are direct attributes - - Implementation methods are instance methods - -2. **Standardized Initialization:** - - All transports use `@dataclass` for configuration - - `connect()` receives controller_api and loop (deferred initialization) - - `serve()` runs the transport server - - Consistent signature across all transports - -3. **Simplified FastCS:** - ```python - # Before: Options → Adapter matching logic - for option in transport_options: - match option: - case EpicsPVAOptions(): - transport = EpicsPVATransport(controller_api, option) - - # After: Direct transport instances - for transport in transports: - transport.connect(controller_api, loop) # Unified interface - ``` - -4. **Better Type Safety:** - - `Transport.union()` creates Union type of all transports - - Used for CLI type hints and validation - - Automatically updated when new transports are added - -5. **Cleaner Imports:** - ```python - # Before - from fastcs.transport.epics.pva.options import EpicsPVAOptions - options = EpicsPVAOptions(pv_prefix="MY:DEVICE:") - - # After - from fastcs.transports.epics.pva import EpicsPVATransport - transport = EpicsPVATransport(epicspva=EpicsIOCOptions(pv_prefix="MY:DEVICE:")) - ``` +## Consequences ### Benefits -- **Reduced API Surface:** 5 classes instead of 10 (one Transport per protocol, not one Adapter + one Options) - +- **Reduced API Surface:** 5 classes instead of 10 (one Transport per protocol) - **Simpler Mental Model:** Configuration and implementation in one place - - **Consistent Interface:** All transports follow same initialization pattern - - **Less Boilerplate:** No pattern matching needed in FastCS - - **Easier Maintenance:** Transport parameters defined once in dataclass fields +- **Better Type Safety:** Consistent constructor signatures across all transports -- **Better Type Safety:** Automatic Union type generation for all transports - -- **Cleaner Usage:** Direct instantiation of transport with configuration +### Migration Pattern -## Consequences - -### Technical Changes - -- 87 insertions, 87 deletions across 16 files (net neutral, massive simplification) -- Renamed `src/fastcs/transport/adapter.py` → `src/fastcs/transport/transport.py` -- Merged all `*Options` classes into corresponding `*Transport` classes -- Simplified `src/fastcs/launch.py` by removing pattern matching logic -- Updated all transport implementations: - - `src/fastcs/transport/epics/ca/adapter.py` - - `src/fastcs/transport/epics/pva/adapter.py` - - `src/fastcs/transport/graphql/adapter.py` - - `src/fastcs/transport/rest/adapter.py` - - `src/fastcs/transport/tango/adapter.py` -- Updated all tests and examples - -### Migration Impact - -For transport users: - -**Before (Options pattern):** -```python -from fastcs.transport.epics.pva.options import EpicsPVAOptions -from fastcs import FastCS - -options = EpicsPVAOptions( - pv_prefix="MY:DEVICE:", - gui_dir=Path("gui") -) - -fastcs = FastCS(controller, [options]) -``` - -**After (Transport pattern):** -```python -from fastcs.transports.epics.pva import EpicsPVATransport -from fastcs.transports.epics import EpicsIOCOptions -from fastcs import FastCS - -transport = EpicsPVATransport( - epicspva=EpicsIOCOptions( - pv_prefix="MY:DEVICE:", - gui_dir=Path("gui") - ) -) - -fastcs = FastCS(controller, [transport]) -``` - -For transport developers: - -**Before (Two classes):** +**Before (Options + Adapter pattern):** ```python +# Configuration separate from implementation @dataclass class MyOptions: param1: str @@ -222,13 +55,11 @@ class MyTransport(TransportAdapter): def __init__(self, controller_api: ControllerAPI, options: MyOptions): self._options = options # Setup using self._options.param1 - - async def run(self): - # Implementation ``` -**After (One class):** +**After (Unified Transport):** ```python +# Configuration and implementation unified @dataclass class MyTransport(Transport): param1: str @@ -237,11 +68,4 @@ class MyTransport(Transport): def connect(self, controller_api: ControllerAPI, loop: asyncio.AbstractEventLoop): self._controller_api = controller_api # Setup using self.param1 directly - - async def serve(self): - # Implementation ``` - -### Architectural Impact - -This decision established a cleaner, more direct API for transport implementations, reducing complexity while maintaining full functionality. The unified Transport class became the single abstraction for all protocol implementations in FastCS. diff --git a/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md b/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md index 066568054..a7555f13b 100644 --- a/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md +++ b/docs/explanations/decisions/0008-transport-dependencies-as-optional-extras.md @@ -10,182 +10,59 @@ Accepted ## Context -FastCS supports multiple transport protocols for exposing controller APIs: EPICS CA, EPICS PVA, Tango, REST, and GraphQL. Originally, all transport dependencies were required dependencies, meaning they were always installed regardless of which transports users actually needed. - -**Original Architecture - All Dependencies Required:** - -```toml -# pyproject.toml -[project] -dependencies = [ - "fastapi[standard]", # For REST - "numpy", - "pydantic", - "pvi~=0.11.0", # For EPICS GUI generation - "pytango", # For Tango - "softioc>=4.5.0", # For EPICS CA - "strawberry-graphql", # For GraphQL - "p4p", # For EPICS PVA - "IPython", - "ruamel.yaml", -] -``` - -This meant every FastCS installation included: -- Python bindings for EPICS CA (softioc) -- Python bindings for EPICS PVA (p4p) -- Python bindings for Tango (pytango) -- Web frameworks (FastAPI, Strawberry GraphQL) -- GUI generation tools (pvi) - -**Problems with Required Dependencies:** +Currently all transport dependencies are installed regardless of which transports users actually needed. -The system needed a way to: -- Install only the dependencies for transports actually being used -- Allow minimal installations for simple use cases -- Make dependency relationships explicit and documented -- Support "install everything" for development -- Reduce installation failures for unused transports +Problems with required dependencies: +- Minimal installations bloated by unused transport dependencies +- Unclear dependency relationships for each transport +- No way to install just the core FastCS functionality ## Decision -We split transport dependencies into optional extras in `pyproject.toml`, allowing users to install only what they need. +Split transport dependencies into optional extras in `pyproject.toml`, allowing users to install only what they need. -### New Architecture +The core FastCS package now requires only essential dependencies (pydantic, numpy, ruamel.yaml, IPython). Each transport is available as an optional extra, with convenience groups like `[all]`, `[epics]`, and `[dev]` for common installation patterns. -**Core Dependencies (Minimal):** +Key architectural changes: +- Core dependencies: pydantic, numpy, ruamel.yaml, IPython +- Individual transport extras: `[epicsca]`, `[epicspva]`, `[tango]`, `[graphql]`, `[rest]` +- Convenience groups: `[epics]`, `[all]`, `[dev]`, `[demo]` +- Each transport declares its own dependencies explicitly -```toml -[project] -dependencies = [ - "pydantic", # Core data validation - "numpy", # Numeric types - "ruamel.yaml", # YAML config parsing - "IPython", # Interactive shell -] -``` +## Consequences -**Transport-Specific Extras:** - -```toml -[project.optional-dependencies] -# Individual transports -epicsca = ["pvi~=0.11.0", "softioc>=4.5.0"] -epicspva = ["p4p", "pvi~=0.11.0"] -tango = ["pytango"] -graphql = ["strawberry-graphql", "uvicorn[standard]>=0.12.0"] -rest = ["fastapi[standard]", "numpy", "uvicorn[standard]>=0.12.0"] - -# Convenience groups -epics = ["fastcs[epicsca]", "fastcs[epicspva]"] -all = ["fastcs[epics]", "fastcs[tango]", "fastcs[graphql]", "fastcs[rest]"] - -# Development and demos -demo = ["tickit~=0.4.3"] -dev = [ - "fastcs[all]", # Dev installs everything - "fastcs[demo]", - # ... test tools, docs tools, etc. -] -``` +### Benefits + +- **Minimal Core Installation:** Users can install FastCS core without transport dependencies +- **Explicit Dependency Relationships:** Each transport declares what it needs +- **Flexible Installation:** Users choose exactly what they need: `pip install fastcs[epicspva,rest]` +- **Development Convenience:** `pip install fastcs[dev]` includes everything for development +- **Clear Documentation:** Installation commands are self-documenting -### Installation Examples +### Installation Patterns -**1. Minimal Installation (Core only):** +**Minimal (core only):** ```bash pip install fastcs -# Only: pydantic, numpy, ruamel.yaml, IPython ``` -**2. Single Transport:** +**Single transport:** ```bash -pip install fastcs[epicspva] # EPICS PVA only -pip install fastcs[rest] # REST API only -pip install fastcs[tango] # Tango only +pip install fastcs[epicspva] # EPICS PVA +pip install fastcs[rest] # REST API ``` -**3. Multiple Transports:** +**Multiple transports:** ```bash pip install fastcs[epics,rest] # EPICS CA + PVA + REST ``` -**4. All Transports:** -```bash -pip install fastcs[all] # Everything -``` - -**5. Development:** -```bash -pip install fastcs[dev] # All transports + dev tools -``` - -### Key Benefits - -1. **Clearer Documentation:** - - Extras make dependencies explicit: `pip install fastcs[epicspva]` - - Users understand what each transport needs - - Self-documenting installation commands - -2. **Development Convenience:** - - `fastcs[dev]` installs everything for development - - `fastcs[all]` installs all transports without dev tools - - Clear separation of concerns - -## Consequences - -### Technical Changes - -- Updated `pyproject.toml`: - - Moved transport dependencies from `dependencies` to `optional-dependencies` - - Created extras: `epicsca`, `epicspva`, `tango`, `graphql`, `rest` - - Created convenience groups: `epics`, `all`, `dev`, `demo` - - Kept only core dependencies in main `dependencies` list - -### Migration Impact - -For existing users: - -**Before:** -```bash -pip install fastcs # Gets everything -``` - -**After:** +**All transports:** ```bash -# Option 1: Get everything (same as before) pip install fastcs[all] - -# Option 2: Get only what you need -pip install fastcs[epicspva] -pip install fastcs[rest,epics] -``` - -For developers: - -**Before:** -```bash -pip install -e .[dev] # Got everything ``` -**After:** +**Development:** ```bash -pip install -e .[dev] # Still gets everything (includes [all]) +pip install fastcs[dev] ``` - -### Architectural Impact - -This decision established a flexible dependency model: - -``` -fastcs (core) - ├── fastcs[epicsca] → softioc, pvi - ├── fastcs[epicspva] → p4p, pvi - ├── fastcs[tango] → pytango - ├── fastcs[graphql] → strawberry-graphql, uvicorn - ├── fastcs[rest] → fastapi, uvicorn - ├── fastcs[epics] → [epicsca] + [epicspva] - ├── fastcs[all] → all transports - └── fastcs[dev] → [all] + dev tools -``` - -The split into optional extras aligned with FastCS's philosophy of supporting multiple transports while keeping the core lightweight. Users can now install exactly what they need, reducing friction and improving deployment efficiency, while developers can still easily install everything with `fastcs[dev]`. diff --git a/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md b/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md index ff77d4fb8..756224233 100644 --- a/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md +++ b/docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md @@ -10,31 +10,27 @@ Accepted ## Context -In the original FastCS architecture, the `Handler` pattern was used to manage attribute I/O operations. The design had several classes: +Currently the `Handler` pattern is used to manage attribute I/O operations. This design has several classes: -**Original Handler Architecture:** - `AttrHandlerR` previously `Updater` - Protocol for reading/updating attribute values - `AttrHandlerW` previously `Sender` - Protocol for writing/setting attribute values - `AttrHandlerRW` previously `Handler` - Combined read-write handler - `SimpleAttrHandler` - Basic implementation for internal parameters -**Limitations of the Handler Pattern:** +There are a few limitations with this architecture: -1. **Handler Instance per Attribute:** Every attribute needed its own Handler instance because that's where the specification connecting the attribute to a unique resource lived. This created: - - Heavy memory overhead for controllers with many attributes - - Redundant Handler instances when multiple attributes used the same I/O pattern - - Difficulty sharing I/O logic across attributes +1. **Handler Instance per Attribute:** Every attribute needed its own Handler instance because that's where the specification connecting the attribute to a unique resource live is defined. This means redundant Handler instances when multiple attributes use the same I/O pattern -2. **Circular Reference Loop:** The architecture created circular dependencies: +2. **Circular Reference Loop:** The architecture has circular dependencies: - Controller → Attributes (controller owns attributes) - Attributes → Handlers (each attribute has a handler) - Handlers → Controller (handlers need controller reference to communicate with device) -3. **Tight Coupling to Controllers:** Handlers needed direct references to Controllers, coupling I/O logic to the controller structure rather than just to the underlying connections (e.g., hardware interfaces, network connections) +3. **Tight Coupling to Controllers:** Handlers need direct references to Controllers, coupling I/O logic to the controller structure rather than just to the underlying connections (e.g., hardware interfaces, network connections) -4. **Mixed Concerns:** Handlers combined resource specification (what to connect to) with I/O behavior (how to read/write), making both harder to reason about +4. **Mixed Concerns:** Handlers combine resource specification (what to connect to) with I/O behavior (how to read/write), making both harder to reason about -The system needed a more flexible way to: +The system needs a more flexible way to: - Share a single AttributeIO instance across multiple attributes - Use lightweight AttributeIORef instances to specify resource connections per-attribute - Break the circular dependency chain @@ -43,34 +39,21 @@ The system needed a more flexible way to: ## Decision -We replaced the `Handler` pattern with a two-component system: `AttributeIO` for behavior and `AttributeIORef` for configuration. +Replace the `Handler` pattern with a two-component system: `AttributeIO` for behavior and `AttributeIORef` for configuration. -### New Architecture +Key architectural changes: -1. **AttributeIORef - Lightweight Resource Specification:** -```python -@dataclass(kw_only=True) -class AttributeIORef: - update_period: float | None = None -``` - - Lightweight dataclass specifying resource connection details per-attribute +1. **AttributeIORef** - Lightweight resource specification per-attribute: + - Lightweight dataclass specifying resource connection details - Can be subclassed to add fields like resource names, register addresses, etc. - - Multiple attributes can have their own AttributeIORef instances + - Attributes have unique AttributeIORef instances - Dynamically connected to a single AttributeIO instance at runtime -2. **AttributeIO - Shared I/O Behavior:** -```python -class AttributeIO(Generic[T, AttributeIORefT]): - async def update(self, attr: AttrR[T, AttributeIORefT]) -> None: - raise NotImplementedError() - - async def send(self, attr: AttrRW[T, AttributeIORefT], value: T) -> None: - raise NotImplementedError() -``` - - Single instance per Controller can handle multiple Attributes +2. **AttributeIO** - Shared I/O behavior: + - Single instance per Controller handles multiple Attributes - Generic class parameterized by data type `T` and reference type `AttributeIORefT` - - Receives the AttributeIORef for each attribute to know which resource to access - - Only needs to know about connections (e.g. resource name, hardware interface) + - Accesses the AttributeIORef from the attribute to know which resource to access + - Only needs to know about connections, not controllers 3. **Parameterized Attributes:** - Attributes are now parameterized with `AttributeIORef` types @@ -80,34 +63,13 @@ class AttributeIO(Generic[T, AttributeIORefT]): 4. **Initialization Validation:** - Controller validates at initialization that it has exactly one AttributeIO to handle each Attribute - - Ensures all attributes are properly connected before the serving the Controller API - -### Key Improvements - -- **Breaks Circular Dependencies:** AttributeIO only needs connections, not Controllers -- **Memory Efficiency:** Single AttributeIO instance serves many Attributes -- **Separation of Concerns:** - - AttributeIORef: lightweight resource specification (what to connect to) - - AttributeIO: shared I/O behavior (how to read/write) -- **Validated Coverage:** Initialization ensures every Attribute has an AttributeIO handler -- **Type Safety:** Generic types ensure AttributeIO and AttributeIORef match -- **Extensibility:** Easy to create custom Ref types with resource-specific fields + - Ensures all attributes are properly connected before serving the Controller API ## Consequences -### Technical Changes - -- 519 insertions, 290 deletions across 20 files -- Created new files: - - `src/fastcs/attribute_io.py` - AttributeIO base class - - `src/fastcs/attribute_io_ref.py` - AttributeIORef base class -- Updated attribute system to use generic parameterization -- Refactored all tests to use new pattern -- Updated callbacks to `AttrUpdateCallback` and `AttrSetCallback` - ### Migration Impact -Users and developers needed to: +Users and developers need to: **Before (Handler pattern - one instance per attribute):** ```python @@ -166,5 +128,3 @@ ramp_rate = AttrRW(Float(), io_ref=TempControllerIORef(name="R")) power = AttrR(Float(), io_ref=TempControllerIORef(name="P")) setpoint = AttrRW(Float(), io_ref=TempControllerIORef(name="S")) ``` - -This decision established a more flexible, type-safe foundation for attribute I/O operations, enabling better extensibility and maintainability for controller implementations. diff --git a/docs/explanations/decisions/0010-subcontroller-removal.md b/docs/explanations/decisions/0010-subcontroller-removal.md index 8a20646c3..a0d4e5ade 100644 --- a/docs/explanations/decisions/0010-subcontroller-removal.md +++ b/docs/explanations/decisions/0010-subcontroller-removal.md @@ -10,120 +10,34 @@ Accepted ## Context -FastCS originally provided two separate classes for building controller hierarchies: `Controller` for top-level device controllers and `SubController` for nested components within a controller. +FastCS provides two separate classes for building controller hierarchies: `Controller` for top-level controllers and `SubController` for nested components. This has become a purely philosophical distinction and now just adds limitations for now benefit: -**Original Architecture - Separate Classes:** - -```python -class Controller(BaseController): - """Top-level controller for a device.""" - def __init__(self, description: str | None = None) -> None: - super().__init__(description=description) - async def connect(self) -> None: - pass - -class SubController(BaseController): - """A subordinate to a Controller for managing a subset of a device.""" - root_attribute: Attribute | None = None - def __init__(self, description: str | None = None) -> None: - super().__init__(description=description) -``` - -The key distinctions were -- `SubController` had a `root_attribute` property for exposing a single attribute to its - parent -- Only `Controller` could be used as the root of the hierarchy - -Type annotations enforced the hierarchy: - -```python -# In BaseController -def register_sub_controller(self, name: str, sub_controller: SubController): - """Only accepts SubController instances""" - ... -``` - -**Usage Pattern:** -```python -class TemperatureRampController(SubController): # Must use SubController - start = AttrRW(Int()) - end = AttrRW(Int()) - -class TemperatureController(Controller): # Must use Controller - def __init__(self): - super().__init__() - ramp = TemperatureRampController() - self.register_sub_controller("Ramp1", ramp) -``` - -**Problems with Two-Class Hierarchy:** - -1. **Unnecessary Type Coupling:** The classes were functionally identical. Having two classes created artificial separation without meaningful functional benefit. - -2. **Design-Time Commitment:** Developers had to choose `Controller` vs `SubController` at class definition time, before knowing all the contexts where the component might be used. A component designed as standalone might later need to become nested, forcing inheritance changes. - -3. **Reduced Reusability:** Controllers written as `SubController` couldn't be used as top-level controllers without changing their base class, and vice versa. This coupling hurt composition flexibility. - -The system needed a way to support hierarchical composition without committing to hierarchy at class definition time. +- **Design-Time Commitment:** Developers had to choose class at definition time, before knowing all contexts where components might be used +- **Reduced Reusability:** A component designed as `SubController` couldn't be reused as a top-level controller without changing its base class ## Decision -We unified `Controller` and `SubController` into a single `Controller` class that can be used in both top-level and nested contexts. - -### New Architecture +Unify `Controller` and `SubController` into a single `Controller` class that can be used in both top-level and nested contexts. Whether a Controller is "top-level" or "nested" is now determined by how it is used, not by its class. -**Unified Controller Class:** - -```python -class Controller(BaseController): - """Controller for a device or device component.""" +Key architectural changes: +- Remove `SubController` class entirely +- Move `root_attribute` property to `Controller` +- Rename `register_sub_controller()` to `add_sub_controller()` for consistency +- Any Controller instance can now be nested in any other Controller - root_attribute: Attribute | None = None # NOW on Controller - - def __init__(self, description: str | None = None) -> None: - super().__init__(description=description) - - async def connect(self) -> None: - pass -``` - -**Updated BaseController Type Annotations:** - -```python -# In BaseController -def add_sub_controller(self, name: str, sub_controller: Controller): - """Now accepts any Controller instance""" - ... - -def get_sub_controllers(self) -> dict[str, Controller]: - """Returns Controller instances""" - return self.__sub_controller_tree -``` - -### Key Changes - -1. **Single Unified Class:** - - Removed `SubController` class entirely - - Moved `root_attribute` property to `Controller` - - Controller can now be used in any context - -2. **Usage-Based Hierarchy:** - - Whether a Controller is "top-level" or "nested" is determined by how it's used, not by its class - - Same Controller class can be instantiated standalone or nested in another Controller +## Consequences -3. **Flexible Composition:** - - Any Controller can be added as a sub-controller to another Controller - - No inheritance changes needed to repurpose a controller - - Composition determined at instantiation, not class definition +### Benefits -4. **Simplified API:** - - One class to understand and import - - Consistent patterns across all controller definitions - - Type annotations simplified throughout codebase +- **Composition over Inheritance:** Hierarchy determined by usage, not class definition +- **Increased Reusability:** Controllers work in any context without refactoring +- **Simpler Mental Model:** One class for all controller use cases +- **Reduced Coupling:** No design-time commitment to hierarchy level +- **Easier Evolution:** Controllers can start standalone and be nested later -### Migration Pattern: +### Migration Pattern -**Before (Two Classes):** +**Before (Two classes):** ```python from fastcs import Controller, SubController @@ -137,7 +51,7 @@ class TempController(Controller): # Forced to use Controller self.register_sub_controller("Ramp1", RampController()) ``` -**After (One Class):** +**After (One class):** ```python from fastcs import Controller @@ -149,49 +63,6 @@ class TempController(Controller): # Just use Controller def __init__(self): super().__init__() self.add_sub_controller("Ramp1", RampController()) -``` - -`RampController` could then be used as a the root controller. - -### Benefits - -- **Composition over Inheritance:** Hierarchy determined by usage, not class definition - -- **Increased Reusability:** Controllers can be used in any context without refactoring -- **Simpler Mental Model:** One class for all controller use cases - -- **Reduced Coupling:** No design-time commitment to hierarchy level - -- **Easier Evolution:** Controllers can start standalone and be nested later without code changes - -- **Consistent API:** Single class reduces cognitive load for developers - -## Consequences - -### Technical Changes - -- 30 insertions, 43 deletions -- Removed `SubController` class from `src/fastcs/controller.py` -- Added `root_attribute` property to `Controller` class -- Updated type annotations throughout codebase: `SubController` → `Controller` -- Updated method signatures in `src/fastcs/controllers/base_controller.py` -- Renamed `register_sub_controller` to `add_sub_controller` for consistency -- Updated documentation and examples -- Updated all test files to use unified `Controller` class - -### Migration Impact - -For controller developers: -1. Replace all `class MyController(SubController)` with `class MyController(Controller)` -2. Replace `register_sub_controller()` calls with `add_sub_controller()` -3. No functional changes required - same composition patterns work - -For existing codebases: -- Simple find-and-replace migration: `SubController` → `Controller` -- Backward compatible: composition patterns unchanged -- Controllers previously written as `Controller` or `SubController` now uniformly use `Controller` - -### Architectural Impact - -This decision reinforced the composition-over-inheritance principle in FastCS. The removal of `SubController` flattened the inheritance hierarchy while maintaining full composition capabilities, making FastCS controllers more flexible and easier to compose in different contexts. +# RampController can now be used as a top-level controller or nested +``` diff --git a/docs/explanations/decisions/0011-controller-vector-implementation.md b/docs/explanations/decisions/0011-controller-vector-implementation.md index dcec6dd03..677171a7b 100644 --- a/docs/explanations/decisions/0011-controller-vector-implementation.md +++ b/docs/explanations/decisions/0011-controller-vector-implementation.md @@ -10,191 +10,81 @@ Accepted ## Context -After removing the `SubController` class (ADR-0010), FastCS had a unified `Controller` class that could be used in both top-level and nested contexts. However, a common use case remained unaddressed: managing collections of identical sub-controllers distinguished only by an integer index. - -**Common Use Case - Multiple Identical Components:** - -Many devices have multiple identical components that need individual control: -- Multi-axis motion stages (X, Y, Z axes) -- Multi-channel power supplies (Channel 1, 2, 3, ...) -- Temperature controller ramp profiles (Ramp 1, 2, 3, ...) -- Camera ROI regions (ROI 0, 1, 2, ...) - -**Original Approach - Manual Registration:** - -Before ControllerVector, developers had to manually register each indexed controller: +Many devices have multiple identical components that need individual control: multi-axis motion stages, multi-channel power supplies, camera ROI regions, etc. Before ControllerVector, developers manually registered each indexed controller with string-based names: ```python -class MotorController(Controller): - position = AttrRW(Float()) - velocity = AttrRW(Float()) - -class StageController(Controller): - def __init__(self, num_axes: int): - super().__init__() - self._motors: list[MotorController] = [] - - # Manual registration for each axis - for i in range(num_axes): - motor = MotorController() - self._motors.append(motor) - self.add_sub_controller(f"Axis{i}", motor) # String-based naming +for i in range(num_axes): + motor = MotorController() + self.add_sub_controller(f"Axis{i}", motor) # String-based naming ``` -**Problems with Manual Registration:** - -1. **No Collection Semantics:** Controllers registered with string names (`"Axis0"`, `"Axis1"`) didn't behave like a natural collection. No iteration, indexing, or length operations. - -2. **String-Based Indexing:** Accessing controllers required string manipulation: `controller.sub_controllers["Axis0"]` instead of natural indexing: `controller.axes[0]` - -The system needed a dedicated pattern for indexed collections that: -- Provides natural collection semantics -- Enforces integer-based indexing -- Generates a clear PVI structure for clients and UI generation -- Reduces boilerplate for common patterns +This approach lacks collection semantics. Accessing controllers requires string manipulation (`controller.sub_controllers["Axis0"]`) and heuristics to test if an attribute is numerical, rather than natural indexing (`controller.axes[0]`). ## Decision -We introduced `ControllerVector`, a specialized controller type for managing collections of indexed sub-controllers with dict-like semantics. - -### Key Features - -1. **MutableMapping Interface:** - - Implements `__getitem__`, `__setitem__`, `__iter__`, `__len__` - - Provides dict-like access: `vector[0]`, `vector[1]`, etc. - - Supports iteration: `for index, controller in vector.items()` - -2. **Type-Safe Integer Indexing:** - - Keys must be integers (enforced by type hints and runtime checks) - - Values must be Controller instances - - Clear error messages for type violations - -3. **Non-Contiguous Indices:** - - Indices don't need to be sequential: `{1: motor1, 5: motor5, 10: motor10}` is valid - - Useful for sparse collections or specific numbering schemes - -4. **Inherits from BaseController:** - - Can have its own attributes alongside indexed sub-controllers - - Can be nested within other controllers +Introduce `ControllerVector`, a specialized controller type for managing collections of indexed sub-controllers with dict-like semantics. -### Usage Pattern +ControllerVector implements `MutableMapping` with integer-only keys, providing natural indexing, iteration, and length operations. It supports non-contiguous indices and can have shared attributes alongside the indexed sub-controllers. -**With ControllerVector:** - -```python -class MotorController(Controller): - position = AttrRW(Float()) - velocity = AttrRW(Float()) - -class AxesVector(ControllerVector): - """Vector of motor axes with shared attributes""" - enabled = AttrRW(Bool()) # Shared attribute for all axes - -class StageController(Controller): - def __init__(self, num_axes: int): - super().__init__() - - # Create vector with indexed motors - motors = {i: MotorController() for i in range(num_axes)} - self.axes = AxesVector(motors, description="Motor axes") - - # Natural collection access - first_axis = self.axes[0] - for i, motor in self.axes.items(): - print(f"Motor {i}: {motor}") -``` - -**Alternative Inline Usage:** - -```python -class StageController(Controller): - def __init__(self, num_axes: int): - super().__init__() +Key architectural changes: +- `ControllerVector` implements `__getitem__`, `__setitem__`, `__iter__`, `__len__` +- Keys enforced to be integers only +- Supports sparse indexing: `{1: motor1, 5: motor5, 10: motor10}` +- Can be subclassed to add shared attributes +- Inherits from `BaseController` for full integration with controller hierarchy - # Direct ControllerVector instantiation - self.axes = ControllerVector( - {i: MotorController() for i in range(num_axes)}, - description="Motor axes" - ) -``` +## Consequences ### Benefits -- **Natural Collection Semantics:** Dict-like interface provides familiar indexing, iteration, and length operations - +- **Natural Collection Semantics:** Dict-like interface provides familiar indexing and iteration - **Consistency:** Integer-only keys prevent naming inconsistencies - - **Clear Intent:** ControllerVector explicitly signals "collection of identical components" - - **Sparse Collections:** Non-contiguous indices support flexible numbering schemes +- **Type Safety:** Integer indexing enforced by type hints and runtime checks +- **Shared Attributes:** Can add attributes to the vector itself, separate from indexed components -## Consequences - -### Technical Changes - -- 408 insertions, 315 deletions across 12 files -- Created `src/fastcs/controllers/controller_vector.py` with ControllerVector class -- Updated `src/fastcs/controller_api.py` to handle ControllerVector -- Enhanced EPICS PVI generation in `src/fastcs/transport/epics/pva/pvi.py` -- Simplified `src/fastcs/transport/epics/pva/pvi_tree.py` (207 lines removed) -- Updated EPICS CA and PVA IOC implementations -- Updated test examples to demonstrate ControllerVector usage - -### Migration Impact - -For controller developers with indexed collections: +### Migration Pattern **Before (Manual registration):** ```python -class Controller: - def __init__(self): +class StageController(Controller): + def __init__(self, num_axes: int): super().__init__() - for i in range(4): - channel = ChannelController() - self.add_sub_controller(f"Ch{i}", channel) + for i in range(num_axes): + motor = MotorController() + self.add_sub_controller(f"Axis{i}", motor) # Access via string keys - first = self.sub_controllers["Ch0"] + first = self.sub_controllers["Axis0"] ``` **After (ControllerVector):** ```python -class Controller: - def __init__(self): +class StageController(Controller): + def __init__(self, num_axes: int): super().__init__() - self.channels = ControllerVector( - {i: ChannelController() for i in range(4)}, - description="Power supply channels" + self.axes = ControllerVector( + {i: MotorController() for i in range(num_axes)}, + description="Motor axes" ) # Natural dict-like access - first = self.channels[0] - for i, channel in self.channels.items(): - ... + first = self.axes[0] + for i, motor in self.axes.items(): + print(f"Motor {i}: {motor}") ``` -### Design Patterns Enabled - -**1. Subclassed Vectors with Shared Attributes:** +**With Shared Attributes:** ```python -class ChannelVector(ControllerVector): - master_enable = AttrRW(Bool()) # Shared across all channels - - def __init__(self, num_channels: int): - channels = {i: ChannelController(i) for i in range(num_channels)} - super().__init__(channels, description="Power channels") -``` +class AxesVector(ControllerVector): + enabled = AttrRW(Bool()) # Shared across all axes -**2. Sparse Indexing:** -```python -# Non-contiguous indices for specific addressing schemes -rois = ControllerVector({ - 1: ROIController(), - 5: ROIController(), - 10: ROIController() -}) +class StageController(Controller): + def __init__(self, num_axes: int): + super().__init__() + self.axes = AxesVector( + {i: MotorController() for i in range(num_axes)}, + description="Motor axes" + ) ``` - -### Architectural Impact - -The introduction of ControllerVector established a clear pattern for managing collections of identical components, reducing boilerplate while improving type safety and UI generation capabilities.