Skip to content

Commit 2d8c879

Browse files
committed
Third pass
1 parent 9b5725b commit 2d8c879

7 files changed

+19
-100
lines changed

docs/explanations/decisions/0003-controller-initialization-on-main-event-loop.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ Move initialisation logic into Backend so that it:
1919
- Runs `controller.initialise()` before creating the Mapping
2020
- Creates the Mapping from the initialized controller
2121
- Runs initial tasks including `controller.connect()`
22-
- Delegates to transport-specific implementations
2322

2423
Controller then has two hooks: `initialise()` for pre-API setup (hardware introspection, dynamic attribute creation) and `connect()` for post-API connection logic.
2524

2625
## Consequences
2726

28-
The new design the initialisation of the application and makes the API for writing controllers and transports simpler and more flexible.
27+
The new design the initialisation of the application is easier to understand and makes the API for writing controllers and transports simpler and more flexible.
2928

3029
### Migration Pattern
3130

docs/explanations/decisions/0005-background-thread-removal.md

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,11 @@ Accepted
1010

1111
## Context
1212

13-
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:
14-
15-
- **Thread Safety Complexity:** Managing state across thread boundaries introduced race conditions and required careful synchronization
16-
- **Blocked Main Thread:** Despite using async, the main thread blocked waiting for background thread results via `.result()`
17-
- **Complex Lifecycle Management:** Starting and stopping the background thread added complexity
18-
- **Difficult to Test:** Background threads made tests non-deterministic and harder to debug
19-
- **Unnecessary for Most Transports:** Only EPICS CA softIOC actually needed a background thread; other transports (REST, GraphQL, PVA) could run entirely async
20-
21-
The system needs a simpler concurrency model that uses native async/await patterns and allows transports to manage their own threading if needed.
13+
The FastCS Backend implementation uses `asyncio.run_coroutine_threadsafe()` to execute controller operations on a background event loop thread managed by `AsyncioDispatcher`. The system needs a simpler concurrency model that uses native async/await patterns and allows transports to manage their own threading if needed.
2214

2315
## Decision
2416

25-
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.
17+
Remove the background thread from Backend, making it fully async, while allowing specific transports to use a background thread if required. Backend should accept an event loop from the caller and use native async/await throughout. Transports that need threading (like Tango) manage their own threading explicitly.
2618

2719
Key architectural changes:
2820
- Backend receives event loop from caller (no background dispatcher)
@@ -36,50 +28,5 @@ Key architectural changes:
3628
### Benefits
3729

3830
- **Simpler Concurrency Model:** Single event loop for most operations, no cross-thread coordination needed
39-
- **True Async/Await:** Native Python async patterns throughout, no blocking `.result()` calls
40-
- **Better Testability:** Deterministic execution, easier to debug, no thread scheduling delays
41-
- **Clearer Responsibility:** Transports explicitly manage threading they need
4231
- **Task-Based API:** Consistent use of `Task` objects with standard cancellation support
4332
- **Composability:** `async serve()` can be composed with other async operations
44-
45-
### Migration Pattern
46-
47-
**Before (Background thread in Backend):**
48-
```python
49-
class MyTransport(TransportAdapter):
50-
def __init__(self, controller_api, loop):
51-
self._backend = Backend(controller) # Creates background thread
52-
53-
def run(self):
54-
self._backend.run() # Synchronous
55-
56-
# Client code
57-
asyncio.run_coroutine_threadsafe(coro(), self._loop) # Cross-thread scheduling
58-
future.result() # Main thread blocks
59-
```
60-
61-
**After (Async-only Backend):**
62-
```python
63-
class MyTransport(Transport):
64-
def connect(self, controller_api, loop):
65-
self._backend = Backend(controller, loop) # Use caller's loop
66-
67-
async def serve(self):
68-
await self._backend.serve() # Native async
69-
70-
# Client code
71-
await self._backend.serve() # Direct await, no threading
72-
```
73-
74-
**For transports needing threads (EPICS CA):**
75-
```python
76-
class EpicsCATransport(Transport):
77-
def __init__(self):
78-
self._dispatcher = AsyncioDispatcher() # Transport manages its own thread
79-
80-
def connect(self, controller_api, loop):
81-
self._backend = Backend(controller, self._dispatcher.loop)
82-
83-
async def serve(self):
84-
await self._backend.serve() # Bridge to threaded environment
85-
```

docs/explanations/decisions/0006-controller-api-abstraction-layer.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ Accepted
1010

1111
## Context
1212

13-
Transports currently access `Controller` instances directly to extract attributes, methods, and metadata for serving over their protocols. This tight coupling creates a few problems:
13+
Transports currently access `Controller` instances directly to extract attributes, methods, and metadata for serving over their protocols. This creates a few problems:
1414

1515
- **Tight Coupling:** Transports are coupled to internal Controller structure, making evolution difficult
1616
- **Code Duplication:** Every transport re-implemented similar traversal logic for discovering attributes and methods
17-
- **No Encapsulation:** Transports had direct access to mutable controller state
17+
- **No Encapsulation:** Transports have direct access to mutable controller state
1818
- **No Static View:** No complete, immutable snapshot of controller API after initialization
1919

2020
## Decision
@@ -27,7 +27,7 @@ Key architectural changes:
2727
- `ControllerAPI` dataclass represents the complete, hierarchical structure of what a controller exposes
2828
- Separate dictionaries for attributes, command_methods, put_methods, and scan_methods
2929
- `walk_api()` method provides depth-first traversal of the API tree
30-
- Backend creates ControllerAPI once during initialization, before passing to transports
30+
- Backend creates ControllerAPI during initialization and passes to transports
3131

3232
## Consequences
3333

docs/explanations/decisions/0007-transport-consolidation.md

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,8 @@ All transports should follow a unified pattern: configuration fields are datacla
2525

2626
Key architectural changes:
2727
- All transports use `@dataclass` decorator combining configuration and implementation
28-
- Standardized `connect(controller_api, loop)` method for deferred initialization
29-
- Standardized `async serve()` method for running the transport
30-
- Removed pattern matching logic from FastCS
31-
- Configuration fields are direct attributes (not nested in options object)
28+
- Standardized `connect(controller_api, loop)` method to load controller API into transport
29+
- Standardized `async serve()` method for running the transport service
3230

3331
## Consequences
3432

@@ -37,9 +35,6 @@ Key architectural changes:
3735
- **Reduced API Surface:** 5 classes instead of 10 (one Transport per protocol)
3836
- **Simpler Mental Model:** Configuration and implementation in one place
3937
- **Consistent Interface:** All transports follow same initialization pattern
40-
- **Less Boilerplate:** No pattern matching needed in FastCS
41-
- **Easier Maintenance:** Transport parameters defined once in dataclass fields
42-
- **Better Type Safety:** Consistent constructor signatures across all transports
4338

4439
### Migration Pattern
4540

docs/explanations/decisions/0009-handler-to-attribute-io-pattern.md

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,17 @@ Currently the `Handler` pattern is used to manage attribute I/O operations. This
1919

2020
There are a few limitations with this architecture:
2121

22-
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
22+
1. **Handler Instance per Attribute:** Every attribute needs its own Handler instance because that's where the specification connecting the attribute to a unique resource is defined. This means redundant Handler instances when multiple attributes use the same I/O pattern.
2323

24-
2. **Circular Reference Loop:** The architecture has circular dependencies:
24+
2. **Circular Reference Loop:**
2525
- Controller → Attributes (controller owns attributes)
2626
- Attributes → Handlers (each attribute has a handler)
2727
- Handlers → Controller (handlers need controller reference to communicate with device)
2828

2929
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)
3030

31-
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
32-
3331
The system needs a more flexible way to:
34-
- Share a single AttributeIO instance across multiple attributes
35-
- Use lightweight AttributeIORef instances to specify resource connections per-attribute
3632
- Break the circular dependency chain
37-
- Validate that Controllers have exactly one AttributeIO to handle each Attribute
3833
- Separate resource specification from I/O behavior
3934

4035
## Decision
@@ -45,7 +40,7 @@ Key architectural changes:
4540

4641
1. **AttributeIORef** - Lightweight resource specification per-attribute:
4742
- Lightweight dataclass specifying resource connection details
48-
- Can be subclassed to add fields like resource names, register addresses, etc.
43+
- Containers static fields like resource names, register addresses, etc.
4944
- Attributes have unique AttributeIORef instances
5045
- Dynamically connected to a single AttributeIO instance at runtime
5146

@@ -59,7 +54,7 @@ Key architectural changes:
5954
- Attributes are now parameterized with `AttributeIORef` types
6055
- `AttrR[T, AttributeIORefT]` - Read-only attribute with typed I/O reference
6156
- `AttrRW[T, AttributeIORefT]` - Read-write attribute with typed I/O reference
62-
- Type system ensures matching between AttributeIO and AttributeIORef
57+
- Type system and programmatic validation ensures matching between AttributeIO and AttributeIORef
6358

6459
4. **Initialization Validation:**
6560
- Controller validates at initialization that it has exactly one AttributeIO to handle each Attribute
@@ -69,9 +64,7 @@ Key architectural changes:
6964

7065
### Migration Impact
7166

72-
Users and developers need to:
73-
74-
**Before (Handler pattern - one instance per attribute):**
67+
**Before:**
7568
```python
7669
# Temperature controller that communicates via TCP/IP
7770
class TempControllerHandler(AttrHandlerRW):
@@ -101,7 +94,7 @@ power = AttrR(Float(), handler=TempControllerHandler("P", controller))
10194
setpoint = AttrRW(Float(), handler=TempControllerHandler("S", controller))
10295
```
10396

104-
**After (AttributeIO pattern - shared instance):**
97+
**After:**
10598
```python
10699
@dataclass
107100
class TempControllerIORef(AttributeIORef):

docs/explanations/decisions/0010-subcontroller-removal.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ Accepted
1010

1111
## Context
1212

13-
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:
13+
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 no benefit:
1414

15-
- **Design-Time Commitment:** Developers had to choose class at definition time, before knowing all contexts where components might be used
16-
- **Reduced Reusability:** A component designed as `SubController` couldn't be reused as a top-level controller without changing its base class
15+
- **Design-Time Commitment:** Developers have to choose class at definition time, before knowing all contexts where components might be used
16+
- **Reduced Reusability:** A component designed as `SubController` can't be reused as a top-level controller without changing its base class
1717

1818
## Decision
1919

@@ -22,7 +22,6 @@ Unify `Controller` and `SubController` into a single `Controller` class that can
2222
Key architectural changes:
2323
- Remove `SubController` class entirely
2424
- Move `root_attribute` property to `Controller`
25-
- Rename `register_sub_controller()` to `add_sub_controller()` for consistency
2625
- Any Controller instance can now be nested in any other Controller
2726

2827
## Consequences

docs/explanations/decisions/0011-controller-vector-implementation.md

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Accepted
1010

1111
## Context
1212

13-
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:
13+
Many devices have multiple identical components that need individual control: multi-axis motion stages, multi-channel power supplies, multiple file writers, etc. Developers must manually register each indexed controller with string-based names and check for this pattern throughout the code to perform different logic
1414

1515
```python
1616
for i in range(num_axes):
@@ -24,7 +24,7 @@ This approach lacks collection semantics. Accessing controllers requires string
2424

2525
Introduce `ControllerVector`, a specialized controller type for managing collections of indexed sub-controllers with dict-like semantics.
2626

27-
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.
27+
`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.
2828

2929
Key architectural changes:
3030
- `ControllerVector` implements `__getitem__`, `__setitem__`, `__iter__`, `__len__`
@@ -74,17 +74,3 @@ class StageController(Controller):
7474
for i, motor in self.axes.items():
7575
print(f"Motor {i}: {motor}")
7676
```
77-
78-
**With Shared Attributes:**
79-
```python
80-
class AxesVector(ControllerVector):
81-
enabled = AttrRW(Bool()) # Shared across all axes
82-
83-
class StageController(Controller):
84-
def __init__(self, num_axes: int):
85-
super().__init__()
86-
self.axes = AxesVector(
87-
{i: MotorController() for i in range(num_axes)},
88-
description="Motor axes"
89-
)
90-
```

0 commit comments

Comments
 (0)