diff --git a/docs/contributor_guide/event-bus.md b/docs/contributor_guide/event-bus.md index 71a93d2e7e2..021f42c6a0b 100644 --- a/docs/contributor_guide/event-bus.md +++ b/docs/contributor_guide/event-bus.md @@ -22,14 +22,95 @@ activity performed to execute them. An instrumented method remains a no-op with respect to events unless an EventBus with at least one subscriber is active. Use one of these helpers: -- `@evented_operation(...)` for one public async method. -- `with event_operation(...):` for one logical operation implemented by several calls. +- `@evented_operation(...)` for a trivial projection of one public async method's invocation. +- `with event_operation(...):` for explicit semantic context assembled inside an operation. - `emit_event(...)` only for a meaningful state transition that is not an operation lifecycle. Use a low-level diagnostic event only when the transport boundary itself is useful to observe. Diagnostic events may inherit the enclosing semantic operation context, but do not define a new protocol-level action or resource-transfer meaning. +## Choosing an instrumentation style + +Prefer explicit `event_operation()` construction for new semantic frontend operations. It keeps +the event boundary and metadata next to the code that determines their meaning: + +```python +async def move_to_target(self, requested_target: str) -> None: + target = self.resolve_target(requested_target) + target_coordinate = self.coordinate_for_target(target) + + with event_operation( + "plate_mover.move_to_target", + device=resource_reference(self), + resources=[], + requested_target=requested_target, + target=target, + target_coordinate=coordinate_reference(target_coordinate), + ): + await self.backend.move_to(target_coordinate) +``` + +Compute metadata before entering the operation scope when doing so has no hardware side effects. +Perform operation validation inside the scope when validation failures are part of the operation's +lifecycle. Always enter the scope before issuing hardware commands so command failures emit the +correlated `.failed` event. If meaningful data is known only after successful execution, expose it +with `completed_data_factory`; return the full completed-event payload, including stable invocation +context: + +```python +operation_data = { + "device": resource_reference(self), + "resources": [resource_reference(plate)], +} +completion_data: dict[str, object] = {} +with event_operation( + "reader.read_plate", + **operation_data, + completed_data_factory=lambda: {**operation_data, **completion_data}, +): + result = await self.backend.read_plate(plate) + completion_data["result"] = result +``` + +The completion factory runs after the operation body has succeeded. Keep it pure, deterministic, +and non-throwing so event construction cannot turn a successful hardware operation into an +application failure. + +`@evented_operation(...)` remains appropriate when all event metadata is a simple, pure projection +of invocation arguments and pre-operation resource state. Its context factory is called with the +method's original `*args` and `**kwargs`; the decorator does not inspect, bind, normalize, or apply +defaults to the call. The context factory must therefore mirror the decorated method's calling +signature, including positional order, parameter kinds, defaults, and `**backend_kwargs`: + +```python +def _set_temperature_event_context( + self: "TemperatureController", + temperature: float, + passive: bool = False, +) -> dict[str, object]: + return { + "device": resource_reference(self), + "resources": [] if self.resource is None else [resource_reference(self.resource)], + "target_temperature": temperature, + "passive": passive, + } + + +@evented_operation("temperature_controller.set_temperature", _set_temperature_event_context) +async def set_temperature( + self, + temperature: float, + passive: bool = False, +) -> None: + ... +``` + +Do not use a decorator context factory when event meaning depends on validation, normalization, +resolved targets, derived values, hardware responses, or final resource state. Construct the event +explicitly inside the operation instead. In either style, context construction must be pure: it +must not command hardware, mutate resource state, or perform expensive I/O. + ## Event contract A `PLREvent` always contains: @@ -101,144 +182,16 @@ Avoid adding fields solely to simplify one consumer. An event should describe wh actually did; dashboards, logs, and integrations can derive their own views from the structured references. -## Operation templates - -### Machine lifecycle - -Use for `setup()` and `stop()` on a public PLR machine frontend. - -```python -@evented_operation( - "machine.setup", - lambda self, **_: { - "device": device_reference(self, name="plate_reader"), - "resources": [], - }, -) -async def setup(self, **backend_kwargs): - ... -``` - -If the machine frontend itself inherits `Resource`, use `resource_reference(self)` instead. -Do not pass arbitrary controller objects to `resource_reference()` merely because they expose a -`name` attribute. - -### Resource transfer - -Use for a plate, lid, carrier, or other resource transfer. The direct moved resource is listed in -`resources`; locations are named separately. - -```python -with event_operation( - "incubator.fetch_plate", - device=resource_reference(self), - resources=[resource_reference(plate)], - source=resource_reference(site), - destination=resource_reference(self.loading_tray), -): - await self.backend.fetch_plate_to_loading_tray(plate) -``` - -For pickup and drop, record the resource's invocation state in `.started`. A -`completed_data_factory` may capture its final assignment or pose for `.completed`. -For a pickup, capture `resource.parent` as `source` before the frontend unassigns the moved -resource. Omit `source` if the resource is not currently assigned; do not infer one from a caller -or a physical-deck assumption. - -### Liquid handling - -Use the direct operated containers in `resources`, plus one `liquid_operations` record per -channel. Each item should include the channel, direct resource reference, owning plate reference -when applicable, and `volume`. - -```python -{ - "device": resource_reference(liquid_handler), - "resources": [resource_reference(well)], - "liquid_operations": [{ - "channel": 0, - "resource": resource_reference(well), - "plate": resource_reference(well.parent), - "volume": 50.0, - }], -} -``` - -### Tip handling - -Report each direct `TipSpot`, `TipRack`, or `Trash` resource. For channelized tip actions, -include `tip_operations` with the channel and direct resource. - -```python -{ - "device": resource_reference(liquid_handler), - "resources": [resource_reference(tip_spot)], - "tip_operations": [{"channel": 0, "resource": resource_reference(tip_spot)}], -} -``` - -### Thermal and shaking operations - -Represent the issuing controller as `device`. A `ResourceHolder` controller should include its -currently loaded direct resource in `resources` when one is assigned at operation start. Do not -infer a resource from broader deck state. Event fields use PLR's -[default units](../user_guide/getting-started/units.md), so their names do not repeat those units: - -```python -{ - "device": resource_reference(controller), - "resources": [resource_reference(controller.resource)], # only when loaded - "target_temperature": 37.0, - "duration": 300.0, - "speed_rpm": 800.0, -} -``` - -The `speed_rpm` suffix is explicit because rotational speed differs from PLR's default linear -speed in millimeters per second. - -For a protocol-requested temperature dwell, use `temperature_controller.hold_temperature` with -`duration` and the controller's configured `target_temperature` when known. The operation -must not reissue `set_temperature()` or claim that an attached resource reached target -temperature. New direct vendor frontends should emit this same semantic operation independently; -they do not need to inherit from the legacy temperature-controller frontend. - -### Centrifuge and loader operations - -Use `centrifuge.spin` for one requested centrifuge cycle. Include every directly loaded -resource, including one bucket holder reference per loaded resource when the frontend exposes -individual buckets. Use explicit physical parameters: - -```python -{ - "device": device_reference(centrifuge, name=centrifuge.name), - "resources": [resource_reference(plate)], - "bucket_resources": [{ - "holder": resource_reference(bucket), - "resource": resource_reference(plate), - }], - "relative_centrifugal_force": 500.0, - "duration": 60.0, - "acceleration_fraction": 0.8, - "deceleration_fraction": 0.8, -} -``` - -`relative_centrifugal_force` is the dimensionless multiple of standard gravity conventionally -written as x g, which PLR defines as the default unit for relative centrifugal force; it is not a -mass in grams or a force in Newtons. - -Use `centrifuge_loader.load` and `centrifuge_loader.unload` for a loader's physical transfer -between its staging holder and a centrifuge bucket. List the direct plate in `resources`, and use -the actual staging holder and bucket as `source` and `destination`. +## Canonical operation schemas -### Arm/controller motion +The [Event Schema Registry](event-schemas.md) defines the canonical operation names, fields, +units, lifecycle-specific payloads, and resource semantics for every currently instrumented +frontend and diagnostic producer. Use an existing schema whenever the operation meaning matches. -Public controller operations should identify the controller in `device` and use PLR's default -units for motion arguments. Add a unit suffix only when a field deliberately differs from the -default, such as a percentage-based speed. Low-level controller operations can be useful to a -diagnostic listener, but higher-level resource-aware wrappers should emit the resource-transfer -events when an arm is actually approaching, picking up, moving, or dropping a PLR resource. +If a contribution introduces a genuinely new device or operation family, choose semantically +accurate conventions, add the proposed contract to the registry in the same contribution, and +treat maintainer review as establishing the standard for future implementations of that operation. +Do not create an undocumented vendor-local alias for an existing concept. ## Failure events @@ -261,15 +214,24 @@ When adding EventBus support to a frontend or driver: 1. Choose public semantic operation boundaries; do not decorate transport primitives by default. 2. Use a stable `.` name and one event scope per logical action. -3. Include `device` and direct `resources` in the context factory. -4. Preserve PLR resource semantics; use ancestry for context rather than substituting resources. -5. Use PLR's default units without repeating them in field names. Add a suffix only when a value +3. Prefer explicit `event_operation()` construction, especially when context includes validated, + normalized, resolved, derived, measured, or final-state values. +4. Use `@evented_operation(...)` only for a trivial invocation projection. Match the context + factory's complete calling signature to the decorated method; the decorator forwards the + original `*args` and `**kwargs` without binding or normalization. +5. Include `device` and direct `resources` in the operation context. +6. Preserve PLR resource semantics; use ancestry for context rather than substituting resources. +7. Use PLR's default units without repeating them in field names. Add a suffix only when a value deliberately uses a different unit or representation, such as `speed_rpm` or `speed_pct`. -6. Include `source` and `destination` only for actual resource transfers. -7. Add tests for `.started`, `.completed`, and `.failed`, including operation-ID correlation. -8. Verify no EventBus listener is required for normal device operation and that listener failures - cannot alter hardware control flow. +8. Include `source` and `destination` only for actual resource transfers. +9. Add tests for `.started`, `.completed`, and `.failed`, including operation-ID correlation. For + decorated methods, test both positional and keyword invocation when the public method supports + both. +10. Verify no EventBus listener is required for normal device operation and that listener failures + cannot alter hardware control flow. +11. Update the [Event Schema Registry](event-schemas.md) and user-guide implementation matrix when + adding a new operation, changing a payload, or instrumenting a new frontend. New or existing drivers should adopt these conventions incrementally at their public semantic API -boundaries. Update the user EventBus coverage reference when adding a newly instrumented frontend -or changing an emitted public operation. +boundaries. A new operation family establishes precedent for future devices, so its schema should +be reviewed as deliberately as its implementation. diff --git a/docs/contributor_guide/event-schemas.md b/docs/contributor_guide/event-schemas.md new file mode 100644 index 00000000000..337d48d9d52 --- /dev/null +++ b/docs/contributor_guide/event-schemas.md @@ -0,0 +1,324 @@ +# Event Schema Registry + +This registry defines canonical names and payload fields for EventBus records emitted by PLR. +It is the shared semantic contract between instrument frontends and event consumers. + +The registry describes operation families, not required capabilities. A frontend implements only +the operations that its hardware and public API support. When it does implement an operation that +already appears here, it should use the canonical operation name and field meanings below. + +For guidance on operation boundaries, instrumentation style, and testing, see the +[EventBus contributor guide](event-bus.md). For the exact frontend classes that currently emit +these operations, see [Current event coverage](../user_guide/machine-agnostic-features/event-bus.md#current-event-coverage). + +## Extending the registry + +When adding an event for a device or operation family that is not represented here: + +1. Prefer a device-independent semantic name over a vendor-specific name when the operation has a + clear cross-device meaning. +2. Reuse canonical fields from this registry when their meaning matches. Do not introduce a second + name for an existing concept. +3. Add new fields only when they describe information the PLR operation actually knows. +4. Document the proposed operation and fields in this registry in the same contribution as the + implementation. +5. Treat maintainer review of the new schema as establishing the convention for future frontends + that implement that operation family. + +Do not distort resource semantics or add presentation-only fields for a particular logger, +dashboard, or notification service. + +## Record classes + +### Semantic operation lifecycle + +A semantic operation uses this lifecycle: + +```text +..started +..completed +``` + +or, when it raises: + +```text +..started +..failed +``` + +Lifecycle records share `context.operation` and `context.operation_id`. A failed record preserves +the operation's invocation data and adds: + +| Field | Type | Meaning | +| --- | --- | --- | +| `error_type` | `str` | Exception class name. | +| `error_message` | `str` | String representation of the original exception. | + +Tables in this guide describe fields in `event.data`. A field marked **completed only** is added +only after successful execution. All other listed fields describe invocation state and remain +stable across the lifecycle unless an operation-specific note says otherwise. + +### State-transition records + +State records describe an instantaneous PLR model transition and do not use the +`started`/`completed`/`failed` lifecycle. Current examples are `resource.assigned` and +`resource.unassigned`. + +### Diagnostic records + +Diagnostic events describe controller or transport activity. They may use a lifecycle when the +underlying command has a meaningful request and response, but they are not semantic frontend +operations. When emitted inside a semantic operation, they inherit its event context. + +## Canonical common fields + +| Field | Type | Meaning | +| --- | --- | --- | +| `device` | `DeviceReference` or `ResourceReference` | Device or controller issuing the operation. | +| `resources` | `list[ResourceReference]` | Direct PLR resources acted on by the operation. Omit or use an empty list when none are known. | +| `source` | `ResourceReference` or `CoordinateReference` | Physical origin of a resource transfer. | +| `destination` | `ResourceReference` or `CoordinateReference` | Physical destination of a resource transfer. | +| `duration` | `float` | Requested duration in PLR's default time unit. | +| `timeout` | `float` | Requested timeout in PLR's default time unit. | +| `target_temperature` | `float` | Configured or requested controller target temperature. | +| `current_temperature` | `float` | Controller sensor reading observed by the operation. It is not a resource-temperature measurement unless explicitly documented otherwise. | +| `tolerance` | `float` | Allowed temperature difference in PLR's default temperature unit. | +| `volume` | `float` | Requested liquid volume in PLR's default volume unit. | + +Use `resource_reference()` for direct resources and resource endpoints. Its `ancestors` provide +structural context without replacing a `Well`, `TipSpot`, plate, or holder with a more convenient +display resource. Use `coordinate_reference()` for geometric endpoints and targets. + +Quantitative fields use PLR's [default units](../user_guide/getting-started/units.md). Add a suffix +only when the value deliberately uses a different representation, such as `speed_rpm` or +`speed_pct`. + +### Canonical vocabulary + +Use these names consistently across operation families: + +| Concept | Canonical field | Do not introduce aliases such as | +| --- | --- | --- | +| Requested elapsed time | `duration` | `time`, `duration_s`, `duration_sec`, `seconds` | +| Maximum wait | `timeout` | `timeout_s`, `wait_time` | +| Requested thermal setpoint | `target_temperature` | `temperature_target`, `set_temperature`, `target_temperature_c` | +| Observed controller temperature | `current_temperature` | `actual_temperature`, `measured_temperature`, `current_temperature_c` | +| Temperature acceptance range | `tolerance` | `temperature_tolerance`, `tolerance_c` | +| Relative centrifugal force | `relative_centrifugal_force` | `g`, `g_force`, `rcf` | +| Direct operated resources | `resources` | `plates`, `labware`, `items` | +| Transfer endpoints | `source`, `destination` | `from`, `to` | +| Liquid volume | `volume` | `vol`, `volume_ul` | +| Successful command result | `response` | `reply`, `result_data` | + +This vocabulary is semantic, not merely stylistic. For example, `target_temperature` is a +controller setpoint, while `current_temperature` is an observed controller reading. Do not use one +as an alias for the other. + +## Machine lifecycle + +| Operation | Fields | Notes | +| --- | --- | --- | +| `machine.setup` | `device`, `backend` | Initializes a generic machine frontend. | +| `machine.stop` | `device`, `backend` | Stops a generic machine frontend. | + +Vendor frontends may use their own component name, such as `precise_flex.setup`, while preserving +the same lifecycle meaning. + +## Resource-model state + +These are state-transition records rather than semantic operation lifecycles. + +| Event | Fields | Notes | +| --- | --- | --- | +| `resource.assigned` | `resource`, `parent`, `location` | Emitted after assignment. `location` is the child's relative `CoordinateReference`, or `None`. | +| `resource.unassigned` | `resource`, `previous_parent`, `previous_location` | Emitted after unassignment while preserving the former parent and relative location. | + +## Resource transfer + +### Incubators + +| Operation | Fields | Notes | +| --- | --- | --- | +| `incubator.fetch_plate` | `device`, `resources`, `source`, `destination` | `resources` contains the directly moved plate; endpoints describe the storage site and loading tray when known. | +| `incubator.take_in_plate` | `device`, `resources`, `source`, `destination` | Moves the loading-tray plate into storage. A requested selector such as `"random"` or `"smallest"` may identify an unresolved destination at invocation. | + +### Stackers + +| Operation | Fields | Notes | +| --- | --- | --- | +| `benchcel.downstack` | `device`, `resources`, `source`, `destination` | Moves the accessible plate from a stack to the loading tray. | +| `benchcel.upstack` | `device`, `resources`, `source`, `destination` | Moves the loading-tray plate onto a stack. | +| `benchcel.move_plate_between_stacks` | `device`, `resources`, `source`, `destination` | Moves the accessible plate between two stacks. | + +### Centrifuge loaders + +| Operation | Fields | Notes | +| --- | --- | --- | +| `centrifuge_loader.load` | `device`, `resources`, `source`, `destination` | Transfers the staging plate into the selected centrifuge bucket. | +| `centrifuge_loader.unload` | `device`, `resources`, `source`, `destination` | Transfers the selected bucket plate onto the staging holder. | + +### Liquid-handler resource movement + +| Operation | Fields | Notes | +| --- | --- | --- | +| `liquid_handler.resource_pickup` | `device`, `resources`, optional `source` | `resources` contains the directly picked-up resource. Capture `source` before successful pickup unassigns it. | +| `liquid_handler.resource_move` | `device`, `resources` | Moves the currently held resource without assigning it to a destination. | +| `liquid_handler.resource_drop` | `device`, `resources`, `destination` | Drops the currently held resource at a resource or geometric destination. | + +## Liquid handling + +### Channelized liquid operations + +| Operation | Fields | +| --- | --- | +| `liquid_handler.aspirate` | `device`, `resources`, `liquid_operations` | +| `liquid_handler.dispense` | `device`, `resources`, `liquid_operations` | + +`resources` contains the unique direct containers operated on. `liquid_operations` contains one +record per channel: + +| Field | Type | Meaning | +| --- | --- | --- | +| `channel` | `int` | Liquid-handler channel index. | +| `resource` | `ResourceReference` | Direct operated container, normally a well or trough. | +| `plate` | `ResourceReference` | Owning plate when one exists; otherwise the direct container. | +| `volume` | `float` | Requested channel volume. | + +### Channelized tip operations + +| Operation | Fields | +| --- | --- | +| `liquid_handler.tip_pickup` | `device`, `resources`, `tip_operations` | +| `liquid_handler.tip_drop` | `device`, `resources`, `tip_operations` | + +`resources` contains unique direct `TipSpot` or `Trash` resources. `tip_operations` contains one +record per channel with `channel` and direct `resource` fields. + +### 96-head tip operations + +| Operation | Fields | Notes | +| --- | --- | --- | +| `liquid_handler.tip_pickup_96` | `device`, `resources` | Direct resource is the operated `TipRack`. | +| `liquid_handler.tip_drop_96` | `device`, `resources` | Direct resource is the destination `TipRack` or `Trash`. | + +## Shaking and temperature control + +Controllers that are `ResourceHolder`s include their directly loaded resource in `resources` when +one is assigned at operation start. + +| Operation | Fields | Notes | +| --- | --- | --- | +| `shaker.shake` | `device`, optional `resources`, `speed_rpm`, optional `duration` | Omitted `duration` means shaking continues after the call returns. | +| `shaker.stop_shaking` | `device`, optional `resources` | Explicitly stops an indefinite shake. | +| `temperature_controller.set_temperature` | `device`, optional `resources`, `target_temperature`, `passive` | Records the requested target and cooling policy. | +| `temperature_controller.wait_for_temperature` | `device`, optional `resources`, `target_temperature`, `timeout`, `tolerance`; **completed only:** `current_temperature` | `current_temperature` is the final controller reading that satisfied tolerance. | +| `temperature_controller.hold_temperature` | `device`, optional `resources`, `duration`, optional `target_temperature` | Records a requested dwell without reissuing a setpoint or asserting that a resource reached temperature. | +| `temperature_controller.deactivate` | `device`, optional `resources`, optional `target_temperature` | Stops active temperature control. | + +## Centrifugation + +| Operation | Fields | Notes | +| --- | --- | --- | +| `centrifuge.spin` | `device`, `resources`, `bucket_resources`, `relative_centrifugal_force`, `duration`, `acceleration_fraction`, `deceleration_fraction` | Describes one requested spin cycle. | + +`resources` contains directly loaded resources only. Empty buckets are not represented. +`bucket_resources` preserves the association between each loaded resource and its holder: + +```python +{ + "holder": resource_reference(bucket), + "resource": resource_reference(plate), +} +``` + +`relative_centrifugal_force` is the dimensionless multiple of standard gravity conventionally +written as x g. Acceleration and deceleration are fractions of the device maximum. + +## Brooks PreciseFlex + +PreciseFlex currently exposes vendor-specific controller operations. These records describe the +controller command and geometric or joint target; a higher-level resource-aware integration should +emit separate resource-transfer operations when it knows the moved PLR resource. + +### Lifecycle and controller state + +| Operation | Fields | +| --- | --- | +| `precise_flex.setup` | `device`, `skip_home` | +| `precise_flex.stop` | `device` | +| `precise_flex.power_on` | `device` | +| `precise_flex.power_off` | `device` | +| `precise_flex.recover_from_fault` | `device` | +| `precise_flex.home` | `device` | +| `precise_flex.start_freedrive` | `device`, optional `free_axes` | +| `precise_flex.stop_freedrive` | `device` | +| `precise_flex.halt` | `device` | +| `precise_flex.park` | `device` | + +### Motion + +| Operation | Fields | +| --- | --- | +| `precise_flex.move_to_joint_position` | `device`, `target_joint_position`, optional `speed_pct` | +| `precise_flex.move_to_location` | `device`, `target`, optional `speed_pct` | +| `precise_flex.move_through_cartesian_poses` | `device`, `waypoint_count`, optional `start_target`, optional `end_target`, optional `speed_pct`, `blend` | +| `precise_flex.move_gripper` | `device`, `width`, `force_sensing` | +| `precise_flex.move_gripper_joint_position` | `device`, `gripper_joint_position`, `force_sensing` | +| `precise_flex.move_rail` | `device`, `rail_position` | +| `precise_flex.pick_up_at_joint_position` | `device`, `target_joint_position`, `resource_width`, `finger_speed_pct`, `grasp_force` | +| `precise_flex.drop_at_joint_position` | `device`, `target_joint_position`, `resource_width` | +| `precise_flex.pick_up_at_location` | `device`, `target`, `resource_width`, `finger_speed_pct`, `grasp_force` | +| `precise_flex.drop_at_location` | `device`, `target`, `resource_width` | + +`target_joint_position` maps axis names to positions. A Cartesian `target` contains a serialized +`location`, approach `direction`, optional elbow `orientation`, optional `wrist`, and optional +`rail_position`. Lengths use PLR's default unit, `grasp_force` uses the default force unit, and +percentage values use the `_pct` suffix. + +## Diagnostic transports and firmware + +### Serial, USB, and FTDI + +`io.read` and `io.write` are instantaneous diagnostic records: + +| Field | Meaning | +| --- | --- | +| `transport` | `"serial"`, `"usb"`, or `"ftdi"`. | +| `device` | Human-readable transport device name. | +| `device_id` | Port, serial number, or transport-specific identifier. | +| `data` | Decoded or hexadecimal transport payload. | + +### Hamilton firmware commands + +`firmware.command.started`, `firmware.command.completed`, and `firmware.command.failed` use: + +| Field | Lifecycle | Meaning | +| --- | --- | --- | +| `transport` | all | `"hamilton_usb"`. | +| `driver` | all | Hamilton driver class name. | +| `module` | all | Firmware module identifier. | +| `command` | all | Firmware command identifier. | +| `command_id` | all | Correlation identifier assigned by the driver. | +| `raw_command` | all | Full assembled command. | +| `response` | completed | Raw firmware response, if any. | +| `error_type`, `error_message` | failed | Original exception details. | + +### PreciseFlex firmware commands + +`precise_flex.firmware_command.started`, `precise_flex.firmware_command.completed`, and +`precise_flex.firmware_command.failed` use `device` and `command` in the full lifecycle, `response` +on completion, and `error_type` plus `error_message` on failure. + +## Schema compatibility + +Treat operation names and documented field meanings as public integration contracts: + +- Adding an optional field is normally backward compatible. +- Adding completion-only information is normally backward compatible when invocation fields stay + stable. +- Renaming a field, changing its units, replacing a direct resource with an ancestor, or changing + the meaning of an existing field is a compatibility change and requires explicit review. +- Vendor-specific extensions should not silently redefine a canonical cross-device field. + +When implementation and this registry diverge, update them together and add tests that assert the +canonical operation name, lifecycle, and payload fields. diff --git a/docs/contributor_guide/index.md b/docs/contributor_guide/index.md index 07890feabbf..8f4ac656fee 100644 --- a/docs/contributor_guide/index.md +++ b/docs/contributor_guide/index.md @@ -17,6 +17,7 @@ contributing-to-docs device-driver-guide device-registry event-bus +event-schemas ```
diff --git a/docs/user_guide/machine-agnostic-features/event-bus.md b/docs/user_guide/machine-agnostic-features/event-bus.md index 9a809687383..59d484df5f1 100644 --- a/docs/user_guide/machine-agnostic-features/event-bus.md +++ b/docs/user_guide/machine-agnostic-features/event-bus.md @@ -89,7 +89,7 @@ batch identifiers that PLR itself cannot know. from pylabrobot.events import event_context with use_event_bus(event_bus), event_context(run_id="run-42", batch_id="batch-2"): - await incubator.fetch_plate_to_loading_tray(site) + await incubator.fetch_plate_to_loading_tray("plate_1") ``` The values are inherited by nested PLR events. Keep this context application-specific; device @@ -110,11 +110,11 @@ diagnostic events preserve controller and transport activity for debugging. ## Current event coverage -EventBus adoption is incremental. The initial implementation instruments the following public +EventBus adoption is incremental. The current implementation instruments the following public frontends. Each listed semantic operation emits `started`, `completed`, and `failed` lifecycle events. -| Frontend | Operations | +| Frontend | Canonical semantic operations | | --- | --- | | `legacy.machines.Machine` | `machine.setup`, `machine.stop` | | `legacy.storage.Incubator` | `incubator.fetch_plate`, `incubator.take_in_plate` | @@ -133,7 +133,9 @@ Detailed operation references: - [Incubator](event-bus/incubator.md) - [LiquidHandler](event-bus/liquid-handler.md) - [Shaker and temperature controller](event-bus/thermal-and-shaking.md) +- [VSpin centrifuge and Access2 loader](../agilent/vspin/events.md) - [Diagnostic transports](event-bus/diagnostic-transports.md) +- [Canonical schema for every operation above](../../contributor_guide/event-schemas.md) ```{toctree} :hidden: @@ -173,7 +175,15 @@ serialized `Coordinate` in `target.location`; joint targets use axis-name-to-val `pick` and `drop` describe controller actions. A resource-aware wrapper should emit the separate resource-transfer event when it has PLR resource context. +### Agilent BenchCel + +`benchcel.downstack`, `benchcel.upstack`, and `benchcel.move_plate_between_stacks` include the +BenchCel in `device`, the directly moved plate in `resources`, and the actual PLR stack or loading +tray holders in `source` and `destination`. + ## More detail The [EventBus contributor guide](../../contributor_guide/event-bus.md) defines the stable naming, -resource, and test conventions for driver authors adding coverage. +resource, and test conventions for driver authors adding coverage. The +[Event Schema Registry](../../contributor_guide/event-schemas.md) defines canonical operation names +and payload fields. diff --git a/docs/user_guide/machine-agnostic-features/event-bus/thermal-and-shaking.md b/docs/user_guide/machine-agnostic-features/event-bus/thermal-and-shaking.md index e341aba46f7..f025a08d912 100644 --- a/docs/user_guide/machine-agnostic-features/event-bus/thermal-and-shaking.md +++ b/docs/user_guide/machine-agnostic-features/event-bus/thermal-and-shaking.md @@ -18,14 +18,18 @@ The instrumented `legacy.temperature_controlling.TemperatureController` frontend | Operation | Primary fields | | --- | --- | | `temperature_controller.set_temperature` | `device`, loaded `resources`, `target_temperature`, `passive` | -| `temperature_controller.wait_for_temperature` | `device`, loaded `resources`, `target_temperature`, `timeout`, `tolerance` | +| `temperature_controller.wait_for_temperature` | `device`, loaded `resources`, `target_temperature`, `timeout`, `tolerance`; completed event adds `current_temperature` | | `temperature_controller.hold_temperature` | `device`, loaded `resources`, `duration`, configured `target_temperature` when known | -| `temperature_controller.deactivate` | `device`, loaded `resources` | +| `temperature_controller.deactivate` | `device`, loaded `resources`, configured `target_temperature` when known | `hold_temperature` records a protocol-requested dwell while the controller remains at its existing configuration. It does not send a new temperature command and does not assert that a resource has reached the configured target. +For `wait_for_temperature`, `current_temperature` is the controller's final sensor reading that +satisfied the requested tolerance. It is emitted only on successful completion and does not imply +that a loaded resource itself reached that temperature. + If a resource is assigned when an operation starts, the event's `resources` contains that direct loaded resource. If the holder is empty, `resources` is omitted rather than inferred from surrounding deck state. diff --git a/pylabrobot/agilent/benchcel/benchcel.py b/pylabrobot/agilent/benchcel/benchcel.py index 9a59bd6fc3c..eb28cd7dcbc 100644 --- a/pylabrobot/agilent/benchcel/benchcel.py +++ b/pylabrobot/agilent/benchcel/benchcel.py @@ -506,31 +506,31 @@ def parse_current_position_response(frame: Frame, *, selector: int = 1) -> Curre def _downstack_event_context( - benchcel: "BenchCel4R", stack: ResourceStack, *, timeout: float = 30.0 + self: "BenchCel4R", stack: ResourceStack, *, timeout: float = 30.0 ) -> dict: plate = stack.get_top_item() if len(stack.children) > 0 else None return { - "device": resource_reference(benchcel), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], "source": resource_reference(stack), - "destination": resource_reference(benchcel.loading_tray), + "destination": resource_reference(self.loading_tray), } def _upstack_event_context( - benchcel: "BenchCel4R", stack: ResourceStack, *, timeout: float = 30.0 + self: "BenchCel4R", stack: ResourceStack, *, timeout: float = 30.0 ) -> dict: - plate = benchcel.loading_tray.resource + plate = self.loading_tray.resource return { - "device": resource_reference(benchcel), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], - "source": resource_reference(benchcel.loading_tray), + "source": resource_reference(self.loading_tray), "destination": resource_reference(stack), } def _stack_transfer_event_context( - benchcel: "BenchCel4R", + self: "BenchCel4R", source: ResourceStack, destination: ResourceStack, *, @@ -538,7 +538,7 @@ def _stack_transfer_event_context( ) -> dict: plate = source.get_top_item() if len(source.children) > 0 else None return { - "device": resource_reference(benchcel), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], "source": resource_reference(source), "destination": resource_reference(destination), diff --git a/pylabrobot/agilent/vspin/access2.py b/pylabrobot/agilent/vspin/access2.py index 441b3dcd4e9..6f1e2757662 100644 --- a/pylabrobot/agilent/vspin/access2.py +++ b/pylabrobot/agilent/vspin/access2.py @@ -17,24 +17,24 @@ logger = logging.getLogger(__name__) -def _loader_load_event_context(loader: "Access2") -> dict: - plate = loader.resource +def _loader_load_event_context(self: "Access2") -> dict: + plate = self.resource return { - "device": resource_reference(loader), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], - "source": resource_reference(loader), - "destination": resource_reference(loader._vspin.at_bucket), + "source": resource_reference(self), + "destination": resource_reference(self._vspin.at_bucket), } -def _loader_unload_event_context(loader: "Access2") -> dict: - bucket = loader._vspin.at_bucket +def _loader_unload_event_context(self: "Access2") -> dict: + bucket = self._vspin.at_bucket plate = None if bucket is None else bucket.resource return { - "device": resource_reference(loader), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], "source": resource_reference(bucket), - "destination": resource_reference(loader), + "destination": resource_reference(self), } diff --git a/pylabrobot/agilent/vspin/vspin.py b/pylabrobot/agilent/vspin/vspin.py index 1d21c1ea9aa..59169ec346a 100644 --- a/pylabrobot/agilent/vspin/vspin.py +++ b/pylabrobot/agilent/vspin/vspin.py @@ -56,8 +56,7 @@ def _save_vspin_calibrations(device_id, remainder: int): def _vspin_event_context( - vspin: "VSpin", - *, + self: "VSpin", g: float = 500, duration: float = 60, acceleration: float = 0.8, @@ -69,11 +68,11 @@ def _vspin_event_context( "holder": resource_reference(bucket), "resource": resource_reference(bucket.resource), } - for bucket in (vspin.bucket1, vspin.bucket2) + for bucket in (self.bucket1, self.bucket2) if bucket.resource is not None ] return { - "device": device_reference(vspin, name=vspin.name), + "device": device_reference(self, name=self.name), "resources": [bucket["resource"] for bucket in bucket_resources], "bucket_resources": bucket_resources, "relative_centrifugal_force": g, diff --git a/pylabrobot/agilent/vspin/vspin_tests.py b/pylabrobot/agilent/vspin/vspin_tests.py index 3fe6a9ee80b..b23470aaa99 100644 --- a/pylabrobot/agilent/vspin/vspin_tests.py +++ b/pylabrobot/agilent/vspin/vspin_tests.py @@ -72,6 +72,30 @@ async def test_spin_failure_emits_requested_parameters(self): self.assertEqual(events[0].context["operation_id"], events[1].context["operation_id"]) self.assertEqual(events[1].data["error_type"], "ValueError") + async def test_spin_accepts_positional_parameters_with_event_bus(self): + vspin = VSpin(name="centrifuge", device_id="test") + vspin.request_door_open = AsyncMock(return_value=False) # type: ignore[method-assign] + vspin.request_door_locked = AsyncMock(return_value=True) # type: ignore[method-assign] + vspin.request_bucket_locked = AsyncMock(return_value=False) # type: ignore[method-assign] + vspin.request_tachometer = AsyncMock(return_value=100000) # type: ignore[method-assign] + vspin.request_position = AsyncMock( # type: ignore[method-assign] + side_effect=[0, 10000000] + ) + vspin.request_home_position = AsyncMock(side_effect=[0, 1]) # type: ignore[method-assign] + vspin.send_command = AsyncMock(return_value=b"") # type: ignore[method-assign] + events: list[PLREvent] = [] + event_bus = EventBus() + event_bus.subscribe(events.append) + + with use_event_bus(event_bus): + await vspin.spin(500, 1, 0.5, 0.6) + + started = events[0] + self.assertEqual(started.data["relative_centrifugal_force"], 500) + self.assertEqual(started.data["duration"], 1) + self.assertEqual(started.data["acceleration_fraction"], 0.5) + self.assertEqual(started.data["deceleration_fraction"], 0.6) + class TestAccess2Events(unittest.IsolatedAsyncioTestCase): def setUp(self): diff --git a/pylabrobot/events/bus.py b/pylabrobot/events/bus.py index ebfce7a576f..5d143b63592 100644 --- a/pylabrobot/events/bus.py +++ b/pylabrobot/events/bus.py @@ -314,6 +314,13 @@ def evented_operation( The wrapper is a no-op when no listener is installed, preserving normal PLR performance and behaviour. Nested resource and transport events inherit the generated operation context. + + This helper deliberately forwards the decorated method's original ``*args`` and ``**kwargs`` + directly to ``context_factory``. Use it only when the event context is a simple projection of + invocation arguments and pre-operation resource state, and keep the factory's calling signature + aligned with the decorated method. For context that depends on validation, normalization, + derived values, hardware responses, or final state, construct an :func:`event_operation` + explicitly inside the method instead. """ def decorator(func: Callable[..., Awaitable[Any]]) -> Callable[..., Awaitable[Any]]: diff --git a/pylabrobot/events/bus_tests.py b/pylabrobot/events/bus_tests.py index 206fc53fd5d..6251ac3b0d9 100644 --- a/pylabrobot/events/bus_tests.py +++ b/pylabrobot/events/bus_tests.py @@ -170,3 +170,52 @@ async def action(): ) self.assertEqual(events[1].data["error_type"], "RuntimeError") self.assertEqual(events[1].context["operation_id"], events[0].context["operation_id"]) + + async def test_evented_operation_forwards_positional_and_keyword_calls_unchanged(self): + events: list[PLREvent] = [] + event_bus = EventBus() + event_bus.subscribe(events.append) + + context_calls: list[tuple[int, int, str, dict[str, object]]] = [] + + def context_factory( + value: int, + scale: int = 2, + *, + mode: str = "normal", + **backend_kwargs: object, + ) -> dict: + context_calls.append((value, scale, mode, backend_kwargs)) + return { + "value": value, + "scale": scale, + "mode": mode, + "backend_kwargs": backend_kwargs, + } + + @evented_operation("device.action", context_factory) + async def action( + value: int, + scale: int = 2, + *, + mode: str = "normal", + **backend_kwargs: object, + ) -> int: + return value * scale + + with use_event_bus(event_bus): + self.assertEqual(await action(3, 4, mode="fast", retries=1), 12) + self.assertEqual( + await action(value=3, scale=4, mode="fast", retries=1), + 12, + ) + + self.assertEqual( + context_calls, + [ + (3, 4, "fast", {"retries": 1}), + (3, 4, "fast", {"retries": 1}), + ], + ) + started_events = [event for event in events if event.name == "device.action.started"] + self.assertEqual(started_events[0].data, started_events[1].data) diff --git a/pylabrobot/legacy/liquid_handling/liquid_handler.py b/pylabrobot/legacy/liquid_handling/liquid_handler.py index 8e2be35139f..e010ee806a4 100644 --- a/pylabrobot/legacy/liquid_handling/liquid_handler.py +++ b/pylabrobot/legacy/liquid_handling/liquid_handler.py @@ -91,10 +91,15 @@ def _resource_pickup_event_context( - liquid_handler: "LiquidHandler", resource: Resource, **_: Any + self: "LiquidHandler", + resource: Resource, + offset: Coordinate = Coordinate.zero(), + pickup_distance_from_top: Optional[float] = None, + direction: GripDirection = GripDirection.FRONT, + **backend_kwargs: Any, ) -> Dict[str, Any]: context = { - "device": resource_reference(liquid_handler), + "device": resource_reference(self), "resources": [resource_reference(resource)], } if resource.parent is not None: @@ -102,7 +107,7 @@ def _resource_pickup_event_context( return context -def _picked_resource_event_context(liquid_handler: "LiquidHandler", **_: Any) -> Dict[str, Any]: +def _picked_resource_event_context(liquid_handler: "LiquidHandler") -> Dict[str, Any]: pickup = liquid_handler._resource_pickup return { "device": resource_reference(liquid_handler), @@ -111,11 +116,13 @@ def _picked_resource_event_context(liquid_handler: "LiquidHandler", **_: Any) -> def _resource_drop_event_context( - liquid_handler: "LiquidHandler", + self: "LiquidHandler", destination: Union[ResourceStack, ResourceHolder, Resource, Coordinate], - **_: Any, + offset: Coordinate = Coordinate.zero(), + direction: GripDirection = GripDirection.FRONT, + **backend_kwargs: Any, ) -> Dict[str, Any]: - context = _picked_resource_event_context(liquid_handler) + context = _picked_resource_event_context(self) if isinstance(destination, Resource): context["destination"] = resource_reference(destination) else: @@ -123,6 +130,16 @@ def _resource_drop_event_context( return context +def _resource_move_event_context( + self: "LiquidHandler", + to: Coordinate, + offset: Coordinate = Coordinate.zero(), + direction: Optional[GripDirection] = None, + **backend_kwargs: Any, +) -> Dict[str, Any]: + return _picked_resource_event_context(self) + + def _liquid_operation_plate(resource: Container) -> Resource: """Return the plate that owns a well, or the container itself when it is not plate-backed.""" @@ -144,16 +161,22 @@ def _safe_event_volume(value: Any) -> Any: def _liquid_operation_event_context( - liquid_handler: "LiquidHandler", + self: "LiquidHandler", resources: Sequence[Container], vols: Sequence[Any], use_channels: Optional[List[int]] = None, - **_: Any, + flow_rates: Optional[List[Optional[float]]] = None, + offsets: Optional[List[Coordinate]] = None, + liquid_height: Optional[List[Optional[float]]] = None, + blow_out_air_volume: Optional[List[Optional[float]]] = None, + spread: Literal["wide", "tight", "custom"] = "wide", + mix: Optional[List[Mix]] = None, + **backend_kwargs: Any, ) -> Dict[str, Any]: """Describe requested liquid operations using their directly operated containers.""" resource_list = list(resources) - channels = use_channels or liquid_handler._default_use_channels or list(range(len(resource_list))) + channels = use_channels or self._default_use_channels or list(range(len(resource_list))) operation_resources = resource_list if len(operation_resources) == 1 and len(channels) > 1: operation_resources = operation_resources * len(channels) @@ -182,7 +205,7 @@ def _liquid_operation_event_context( ) return { - "device": resource_reference(liquid_handler), + "device": resource_reference(self), "resources": unique_operation_resources, "liquid_operations": liquid_operations, } @@ -220,18 +243,53 @@ def _tip_operation_event_context( } -def _tip_rack_operation_event_context( - liquid_handler: "LiquidHandler", - tip_rack: Optional[TipRack] = None, - resource: Optional[Union[TipRack, Trash]] = None, - **_: Any, +def _tip_pickup_event_context( + self: "LiquidHandler", + tip_spots: List[TipSpot], + use_channels: Optional[List[int]] = None, + offsets: Optional[List[Coordinate]] = None, + **backend_kwargs: Any, +) -> Dict[str, Any]: + return _tip_operation_event_context(self, tip_spots, use_channels) + + +def _tip_drop_event_context( + self: "LiquidHandler", + tip_spots: Sequence[Union[TipSpot, Trash]], + use_channels: Optional[List[int]] = None, + offsets: Optional[List[Coordinate]] = None, + allow_nonzero_volume: bool = False, + **backend_kwargs: Any, ) -> Dict[str, Any]: - """Describe a 96-head operation by its directly operated rack or trash resource.""" + return _tip_operation_event_context(self, tip_spots, use_channels) + + +def _tip_rack_pickup_event_context( + self: "LiquidHandler", + tip_rack: TipRack, + offset: Coordinate = Coordinate.zero(), + **backend_kwargs: Any, +) -> Dict[str, Any]: + """Describe a 96-head pickup by its directly operated tip rack.""" - operation_resource = tip_rack if tip_rack is not None else resource return { - "device": resource_reference(liquid_handler), - "resources": [] if operation_resource is None else [resource_reference(operation_resource)], + "device": resource_reference(self), + "resources": [resource_reference(tip_rack)], + } + + +def _tip_rack_drop_event_context( + self: "LiquidHandler", + resource: Union[TipRack, Trash], + offset: Coordinate = Coordinate.zero(), + allow_nonzero_volume: bool = False, + **backend_kwargs: Any, +) -> Dict[str, Any]: + """Describe a 96-head drop by its directly operated rack or trash resource.""" + + return { + "device": resource_reference(self), + "resources": [resource_reference(resource)], } @@ -584,7 +642,7 @@ def get_picked_up_resource(self) -> Optional[Resource]: return None return self._resource_pickup.resource - @evented_operation("liquid_handler.tip_pickup", _tip_operation_event_context) + @evented_operation("liquid_handler.tip_pickup", _tip_pickup_event_context) @need_setup_finished async def pick_up_tips( self, @@ -734,7 +792,7 @@ def get_mounted_tips(self) -> List[Optional[Tip]]: """ return [tracker.get_tip() if tracker.has_tip else None for tracker in self.head.values()] - @evented_operation("liquid_handler.tip_drop", _tip_operation_event_context) + @evented_operation("liquid_handler.tip_drop", _tip_drop_event_context) @need_setup_finished async def drop_tips( self, @@ -1599,7 +1657,7 @@ async def use_tips( else: await self.return_tips(use_channels=channels) - @evented_operation("liquid_handler.tip_pickup_96", _tip_rack_operation_event_context) + @evented_operation("liquid_handler.tip_pickup_96", _tip_rack_pickup_event_context) async def pick_up_tips96( self, tip_rack: TipRack, @@ -1669,7 +1727,7 @@ async def pick_up_tips96( tip_spot.tracker.commit() self.head96[i].commit() - @evented_operation("liquid_handler.tip_drop_96", _tip_rack_operation_event_context) + @evented_operation("liquid_handler.tip_drop_96", _tip_rack_drop_event_context) async def drop_tips96( self, resource: Union[TipRack, Trash], @@ -2250,7 +2308,7 @@ async def pick_up_resource( self._state_updated() - @evented_operation("liquid_handler.resource_move", _picked_resource_event_context) + @evented_operation("liquid_handler.resource_move", _resource_move_event_context) async def move_picked_up_resource( self, to: Coordinate, diff --git a/pylabrobot/legacy/liquid_handling/liquid_handler_tests.py b/pylabrobot/legacy/liquid_handling/liquid_handler_tests.py index 663d6fb2983..f1fc47945d1 100644 --- a/pylabrobot/legacy/liquid_handling/liquid_handler_tests.py +++ b/pylabrobot/legacy/liquid_handling/liquid_handler_tests.py @@ -732,6 +732,111 @@ async def test_96_head_tip_operations_emit_direct_rack_resources(self): self.assertEqual(events[0].context["resources"][0]["name"], self.tip_rack.name) self.assertEqual(events[0].context["resources"][0]["type"], "TipRack") + async def _exercise_evented_operations_with_argument_style( + self, *, use_keywords: bool + ) -> list[PLREvent]: + tip_spot = self.tip_rack.get_item("A1") + well = self.plate.get_item("A1") + well.tracker.set_volume(10) + zero = Coordinate.zero() + destination = ResourceHolder("destination", size_x=200, size_y=200, size_z=0) + self.deck.assign_child_resource( + destination, location=Coordinate(600, 100, 0), ignore_collision=True + ) + events: list[PLREvent] = [] + event_bus = EventBus() + event_bus.subscribe(events.append) + + with use_event_bus(event_bus): + if use_keywords: + await self.lh.pick_up_tips(tip_spots=[tip_spot], use_channels=[0], offsets=[zero]) + await self.lh.aspirate( + resources=[well], + vols=[1], + use_channels=[0], + flow_rates=[None], + offsets=[zero], + liquid_height=[None], + blow_out_air_volume=[None], + spread="wide", + mix=None, + ) + await self.lh.dispense( + resources=[well], + vols=[1], + use_channels=[0], + flow_rates=[None], + offsets=[zero], + liquid_height=[None], + blow_out_air_volume=[None], + spread="wide", + mix=None, + ) + await self.lh.drop_tips( + tip_spots=[tip_spot], + use_channels=[0], + offsets=[zero], + allow_nonzero_volume=False, + ) + await self.lh.pick_up_tips96(tip_rack=self.tip_rack, offset=zero) + await self.lh.drop_tips96(resource=self.tip_rack, offset=zero, allow_nonzero_volume=False) + await self.lh.pick_up_resource( + resource=self.plate, + offset=zero, + pickup_distance_from_top=5.0, + direction=GripDirection.FRONT, + ) + await self.lh.move_picked_up_resource( + to=Coordinate(500, 100, 100), offset=zero, direction=GripDirection.FRONT + ) + await self.lh.drop_resource( + destination=destination, offset=zero, direction=GripDirection.FRONT + ) + else: + await self.lh.pick_up_tips([tip_spot], [0], [zero]) + await self.lh.aspirate([well], [1], [0], [None], [zero], [None], [None], "wide", None) + await self.lh.dispense([well], [1], [0], [None], [zero], [None], [None], "wide", None) + await self.lh.drop_tips([tip_spot], [0], [zero], False) + await self.lh.pick_up_tips96(self.tip_rack, zero) + await self.lh.drop_tips96(self.tip_rack, zero, False) + await self.lh.pick_up_resource(self.plate, zero, 5.0, GripDirection.FRONT) + await self.lh.move_picked_up_resource(Coordinate(500, 100, 100), zero, GripDirection.FRONT) + await self.lh.drop_resource(destination, zero, GripDirection.FRONT) + + return events + + async def test_evented_operations_accept_positional_arguments(self): + events = await self._exercise_evented_operations_with_argument_style(use_keywords=False) + self._assert_evented_operation_argument_test_events(events) + + async def test_evented_operations_accept_keyword_arguments(self): + events = await self._exercise_evented_operations_with_argument_style(use_keywords=True) + self._assert_evented_operation_argument_test_events(events) + + def _assert_evented_operation_argument_test_events(self, events: list[PLREvent]) -> None: + started = [event for event in events if event.name.endswith(".started")] + self.assertEqual( + [event.name for event in started], + [ + "liquid_handler.tip_pickup.started", + "liquid_handler.aspirate.started", + "liquid_handler.dispense.started", + "liquid_handler.tip_drop.started", + "liquid_handler.tip_pickup_96.started", + "liquid_handler.tip_drop_96.started", + "liquid_handler.resource_pickup.started", + "liquid_handler.resource_move.started", + "liquid_handler.resource_drop.started", + ], + ) + self.assertEqual(started[0].context["tip_operations"][0]["channel"], 0) + self.assertEqual(started[1].context["liquid_operations"][0]["volume"], 1.0) + self.assertEqual(started[4].context["resources"][0]["name"], self.tip_rack.name) + self.assertEqual(started[5].context["resources"][0]["name"], self.tip_rack.name) + self.assertEqual(started[6].context["resources"][0]["name"], self.plate.name) + self.assertEqual(started[7].context["resources"][0]["name"], self.plate.name) + self.assertEqual(started[8].context["destination"]["name"], "destination") + async def test_return_tips(self): tip_spot = self.tip_rack.get_item("A1") tip = tip_spot.get_tip() diff --git a/pylabrobot/legacy/machines/machine.py b/pylabrobot/legacy/machines/machine.py index 55470a41155..f44ba2b770b 100644 --- a/pylabrobot/legacy/machines/machine.py +++ b/pylabrobot/legacy/machines/machine.py @@ -19,7 +19,7 @@ _R = TypeVar("_R", bound=Awaitable[Any]) -def _machine_event_context(machine: "Machine", **_: Any) -> dict: +def _machine_event_context(machine: "Machine") -> dict: device = ( resource_reference(machine) if isinstance(machine, Resource) @@ -31,6 +31,14 @@ def _machine_event_context(machine: "Machine", **_: Any) -> dict: } +def _machine_setup_event_context(self: "Machine", **backend_kwargs: Any) -> dict: + return _machine_event_context(self) + + +def _machine_stop_event_context(self: "Machine") -> dict: + return _machine_event_context(self) + + def need_setup_finished(func: Callable[_P, _R]) -> Callable[_P, _R]: """Decorator for methods that require the machine to be set up. @@ -74,13 +82,13 @@ def deserialize(cls, data: dict): data_copy["backend"] = backend return cls(**data_copy) - @evented_operation("machine.setup", _machine_event_context) + @evented_operation("machine.setup", _machine_setup_event_context) async def setup(self, **backend_kwargs): await self.backend.setup(**backend_kwargs) self._setup_finished = True @need_setup_finished - @evented_operation("machine.stop", _machine_event_context) + @evented_operation("machine.stop", _machine_stop_event_context) async def stop(self): await self.backend.stop() self._setup_finished = False diff --git a/pylabrobot/legacy/shaking/shaker.py b/pylabrobot/legacy/shaking/shaker.py index a831a0e5cd5..ed36711d8e0 100644 --- a/pylabrobot/legacy/shaking/shaker.py +++ b/pylabrobot/legacy/shaking/shaker.py @@ -8,24 +8,32 @@ from .backend import ShakerBackend -def _shaker_event_context( - shaker: "Shaker", - speed: Optional[float] = None, +def _shaker_controller_event_context(self: "Shaker") -> dict[str, Any]: + context: dict[str, Any] = {"device": resource_reference(self)} + if self.resource is not None: + context["resources"] = [resource_reference(self.resource)] + return context + + +def _shake_event_context( + self: "Shaker", + speed: float, duration: Optional[float] = None, - **_: Any, + **backend_kwargs: Any, ) -> dict[str, Any]: """Describe a shaker operation and its directly loaded resource, when present.""" - context: dict[str, Any] = {"device": resource_reference(shaker)} - if shaker.resource is not None: - context["resources"] = [resource_reference(shaker.resource)] - if speed is not None: - context["speed_rpm"] = float(speed) + context = _shaker_controller_event_context(self) + context["speed_rpm"] = float(speed) if duration is not None: context["duration"] = float(duration) return context +def _stop_shaking_event_context(self: "Shaker", **backend_kwargs: Any) -> dict[str, Any]: + return _shaker_controller_event_context(self) + + class Shaker(ResourceHolder, Machine): """A shaker machine""" @@ -53,7 +61,7 @@ def __init__( Machine.__init__(self, backend=backend) self.backend: ShakerBackend = backend # fix type - @evented_operation("shaker.shake", _shaker_event_context) + @evented_operation("shaker.shake", _shake_event_context) async def shake(self, speed: float, duration: Optional[float] = None, **backend_kwargs): """Shake the shaker at the given speed @@ -73,7 +81,7 @@ async def shake(self, speed: float, duration: Optional[float] = None, **backend_ if self.backend.supports_locking: await self.backend.unlock_plate() - @evented_operation("shaker.stop_shaking", _shaker_event_context) + @evented_operation("shaker.stop_shaking", _stop_shaking_event_context) async def stop_shaking(self, **backend_kwargs): await self.backend.stop_shaking(**backend_kwargs) diff --git a/pylabrobot/legacy/storage/incubator.py b/pylabrobot/legacy/storage/incubator.py index 9a58b090d4a..1e4894d90b3 100644 --- a/pylabrobot/legacy/storage/incubator.py +++ b/pylabrobot/legacy/storage/incubator.py @@ -21,29 +21,33 @@ class NoFreeSiteError(Exception): pass -def _fetch_plate_event_context(incubator: "Incubator", plate_name: str, **_: object) -> dict: +def _fetch_plate_event_context( + self: "Incubator", plate_name: str, **backend_kwargs: object +) -> dict: try: - site = incubator.get_site_by_plate_name(plate_name) + site = self.get_site_by_plate_name(plate_name) plate = site.resource except ResourceNotFoundError: site = None plate = None return { - "device": resource_reference(incubator), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], "source": resource_reference(site), - "destination": resource_reference(incubator.loading_tray), + "destination": resource_reference(self.loading_tray), } def _take_in_plate_event_context( - incubator: "Incubator", site: Union[PlateHolder, Literal["random", "smallest"]], **_: object + self: "Incubator", + site: Union[PlateHolder, Literal["random", "smallest"]], + **backend_kwargs: object, ) -> dict: - plate = incubator.loading_tray.resource + plate = self.loading_tray.resource return { - "device": resource_reference(incubator), + "device": resource_reference(self), "resources": [] if plate is None else [resource_reference(plate)], - "source": resource_reference(incubator.loading_tray), + "source": resource_reference(self.loading_tray), "destination": resource_reference(site) if isinstance(site, PlateHolder) else site, } diff --git a/pylabrobot/legacy/temperature_controlling/temperature_controller.py b/pylabrobot/legacy/temperature_controlling/temperature_controller.py index 36f915be0e8..3840396702d 100644 --- a/pylabrobot/legacy/temperature_controlling/temperature_controller.py +++ b/pylabrobot/legacy/temperature_controlling/temperature_controller.py @@ -2,40 +2,56 @@ import time from typing import Any, Optional -from pylabrobot.events import evented_operation, resource_reference +from pylabrobot.events import event_operation, evented_operation, resource_reference from pylabrobot.legacy.machines.machine import Machine from pylabrobot.resources import Coordinate, ResourceHolder from .backend import TemperatureControllerBackend -def _temperature_event_context( - temperature_controller: "TemperatureController", - temperature: Optional[float] = None, - passive: Optional[bool] = None, - timeout: Optional[float] = None, - tolerance: Optional[float] = None, - duration: Optional[float] = None, - **_: Any, +def _temperature_controller_event_context( + self: "TemperatureController", ) -> dict[str, Any]: - """Describe a thermal-device command and its directly loaded resource, when present.""" - - context: dict[str, Any] = {"device": resource_reference(temperature_controller)} - if temperature_controller.resource is not None: - context["resources"] = [resource_reference(temperature_controller.resource)] - target_temperature = ( - temperature_controller.target_temperature if temperature is None else temperature - ) - if target_temperature is not None: - context["target_temperature"] = float(target_temperature) + """Describe a thermal device and its directly loaded resource, when present.""" + context: dict[str, Any] = {"device": resource_reference(self)} + if self.resource is not None: + context["resources"] = [resource_reference(self.resource)] + if self.target_temperature is not None: + context["target_temperature"] = float(self.target_temperature) + return context + + +def _set_temperature_event_context( + self: "TemperatureController", + temperature: float, + passive: bool = False, +) -> dict[str, Any]: + context = _temperature_controller_event_context(self) + context["target_temperature"] = float(temperature) if passive is not None: context["passive"] = passive + return context + + +def _wait_for_temperature_event_context( + self: "TemperatureController", + timeout: float = 300.0, + tolerance: float = 0.5, +) -> dict[str, Any]: + context = _temperature_controller_event_context(self) if timeout is not None: context["timeout"] = float(timeout) if tolerance is not None: context["tolerance"] = float(tolerance) - if duration is not None: - context["duration"] = float(duration) + return context + + +def _hold_temperature_event_context( + self: "TemperatureController", + duration: float, +) -> dict[str, Any]: + context = _temperature_controller_event_context(self) + context["duration"] = float(duration) return context @@ -67,7 +83,7 @@ def __init__( self.backend: TemperatureControllerBackend = backend # fix type self.target_temperature: Optional[float] = None - @evented_operation("temperature_controller.set_temperature", _temperature_event_context) + @evented_operation("temperature_controller.set_temperature", _set_temperature_event_context) async def set_temperature(self, temperature: float, passive: bool = False): """Set the temperature of the temperature controller. @@ -100,7 +116,6 @@ async def get_temperature(self) -> float: """Get the current temperature of the temperature controller in Celsius.""" return await self.backend.get_current_temperature() - @evented_operation("temperature_controller.wait_for_temperature", _temperature_event_context) async def wait_for_temperature(self, timeout: float = 300.0, tolerance: float = 0.5) -> None: """Wait for the temperature to reach the target temperature. The target temperature must be set by `set_temperature()`. @@ -109,17 +124,25 @@ async def wait_for_temperature(self, timeout: float = 300.0, tolerance: float = timeout: Timeout in seconds. tolerance: Tolerance in Celsius. """ - if self.target_temperature is None: - raise RuntimeError("Target temperature is not set.") - start = time.time() - while time.time() - start < timeout: - temperature = await self.get_temperature() - if abs(temperature - self.target_temperature) < tolerance: - return - await asyncio.sleep(1.0) - raise TimeoutError(f"Temperature did not reach target temperature within {timeout} seconds.") - - @evented_operation("temperature_controller.hold_temperature", _temperature_event_context) + operation_data = _wait_for_temperature_event_context(self, timeout, tolerance) + completion_data: dict[str, Any] = {} + with event_operation( + "temperature_controller.wait_for_temperature", + **operation_data, + completed_data_factory=lambda: {**operation_data, **completion_data}, + ): + if self.target_temperature is None: + raise RuntimeError("Target temperature is not set.") + start = time.time() + while time.time() - start < timeout: + temperature = await self.get_temperature() + if abs(temperature - self.target_temperature) < tolerance: + completion_data["current_temperature"] = float(temperature) + return + await asyncio.sleep(1.0) + raise TimeoutError(f"Temperature did not reach target temperature within {timeout} seconds.") + + @evented_operation("temperature_controller.hold_temperature", _hold_temperature_event_context) async def hold_temperature(self, duration: float) -> None: """Hold the currently configured thermal condition for a requested dwell. @@ -135,7 +158,7 @@ async def hold_temperature(self, duration: float) -> None: raise ValueError("Temperature hold duration must not be negative.") await asyncio.sleep(duration) - @evented_operation("temperature_controller.deactivate", _temperature_event_context) + @evented_operation("temperature_controller.deactivate", _temperature_controller_event_context) async def deactivate(self): """Deactivate the temperature controller. This will stop the heating or cooling, and return the temperature to ambient temperature. The target temperature will be reset to `None`. diff --git a/pylabrobot/legacy/temperature_controlling/temperature_controller_tests.py b/pylabrobot/legacy/temperature_controlling/temperature_controller_tests.py index 79263e8416a..9f98465bbb2 100644 --- a/pylabrobot/legacy/temperature_controlling/temperature_controller_tests.py +++ b/pylabrobot/legacy/temperature_controlling/temperature_controller_tests.py @@ -89,6 +89,8 @@ async def test_set_and_wait_for_temperature_emit_device_scoped_events(self): self.assertEqual(events[0].context["target_temperature"], 37.0) self.assertEqual(events[2].context["timeout"], 1.0) self.assertEqual(events[2].context["tolerance"], 0.5) + self.assertNotIn("current_temperature", events[2].data) + self.assertEqual(events[3].data["current_temperature"], 37.0) self.assertNotIn("target_temperature_c", events[0].context) self.assertNotIn("timeout_seconds", events[2].context) self.assertNotIn("tolerance_c", events[2].context) @@ -113,6 +115,52 @@ async def test_temperature_events_include_currently_loaded_resource(self): self.assertEqual(events[0].context["resources"][0]["name"], "plate") + async def test_wait_for_temperature_positional_arguments_preserve_context(self): + temperature_controller = TemperatureController( + name="test_temperature_module", + size_x=1, + size_y=1, + size_z=1, + backend=TemperatureControllerChatterboxBackend(dummy_temperature=37.0), + child_location=Coordinate.zero(), + ) + temperature_controller.target_temperature = 37.0 + events: list[PLREvent] = [] + event_bus = EventBus() + event_bus.subscribe(events.append) + + with use_event_bus(event_bus): + await temperature_controller.wait_for_temperature(1.0, 0.25) + + self.assertEqual(events[0].context["target_temperature"], 37.0) + self.assertEqual(events[0].context["timeout"], 1.0) + self.assertEqual(events[0].context["tolerance"], 0.25) + self.assertNotIn("current_temperature", events[0].data) + self.assertEqual(events[1].data["current_temperature"], 37.0) + self.assertNotIn("passive", events[0].context) + + async def test_wait_for_temperature_completion_reports_final_sensor_reading(self): + temperature_controller = TemperatureController( + name="test_temperature_module", + size_x=1, + size_y=1, + size_z=1, + backend=TemperatureControllerChatterboxBackend(dummy_temperature=36.8), + child_location=Coordinate.zero(), + ) + temperature_controller.target_temperature = 37.0 + events: list[PLREvent] = [] + event_bus = EventBus() + event_bus.subscribe(events.append) + + with use_event_bus(event_bus): + await temperature_controller.wait_for_temperature(timeout=1.0, tolerance=0.5) + + started, completed = events + self.assertNotIn("current_temperature", started.data) + self.assertEqual(completed.data["target_temperature"], 37.0) + self.assertEqual(completed.data["current_temperature"], 36.8) + async def test_hold_temperature_emits_loaded_resource_without_reissuing_target(self): backend = TemperatureControllerChatterboxBackend(dummy_temperature=20.0) temperature_controller = TemperatureController( @@ -183,6 +231,31 @@ async def test_hold_temperature_failure_emits_invocation_context(self): self.assertNotIn("target_temperature", events[0].context) self.assertEqual(events[1].data["error_type"], "ValueError") + async def test_hold_temperature_positional_argument_preserves_context(self): + temperature_controller = TemperatureController( + name="test_temperature_module", + size_x=1, + size_y=1, + size_z=1, + backend=TemperatureControllerChatterboxBackend(dummy_temperature=20.0), + child_location=Coordinate.zero(), + ) + temperature_controller.target_temperature = 37.0 + events: list[PLREvent] = [] + event_bus = EventBus() + event_bus.subscribe(events.append) + + with patch( + "pylabrobot.legacy.temperature_controlling.temperature_controller.asyncio.sleep", + new_callable=AsyncMock, + ) as sleep: + with use_event_bus(event_bus): + await temperature_controller.hold_temperature(120.0) + + self.assertEqual(events[0].context["target_temperature"], 37.0) + self.assertEqual(events[0].context["duration"], 120.0) + sleep.assert_awaited_once_with(120.0) + class _FakeBackend(TemperatureControllerBackend): def __init__(self, temperature: float = 25.0):