diff --git a/api/src/opentrons/protocol_engine/commands/command_unions.py b/api/src/opentrons/protocol_engine/commands/command_unions.py index 604fca50e14..f01e10aa63e 100644 --- a/api/src/opentrons/protocol_engine/commands/command_unions.py +++ b/api/src/opentrons/protocol_engine/commands/command_unions.py @@ -392,6 +392,7 @@ unsafe.UnsafeDropTipInPlace, unsafe.UpdatePositionEstimators, unsafe.UnsafeEngageAxes, + unsafe.UnsafeUngripLabware, ], Field(discriminator="commandType"), ] @@ -467,6 +468,7 @@ unsafe.UnsafeDropTipInPlaceParams, unsafe.UpdatePositionEstimatorsParams, unsafe.UnsafeEngageAxesParams, + unsafe.UnsafeUngripLabwareParams, ] CommandType = Union[ @@ -540,6 +542,7 @@ unsafe.UnsafeDropTipInPlaceCommandType, unsafe.UpdatePositionEstimatorsCommandType, unsafe.UnsafeEngageAxesCommandType, + unsafe.UnsafeUngripLabwareCommandType, ] CommandCreate = Annotated[ @@ -614,6 +617,7 @@ unsafe.UnsafeDropTipInPlaceCreate, unsafe.UpdatePositionEstimatorsCreate, unsafe.UnsafeEngageAxesCreate, + unsafe.UnsafeUngripLabwareCreate, ], Field(discriminator="commandType"), ] @@ -689,6 +693,7 @@ unsafe.UnsafeDropTipInPlaceResult, unsafe.UpdatePositionEstimatorsResult, unsafe.UnsafeEngageAxesResult, + unsafe.UnsafeUngripLabwareResult, ] # todo(mm, 2024-06-12): Ideally, command return types would have specific diff --git a/api/src/opentrons/protocol_engine/commands/unsafe/__init__.py b/api/src/opentrons/protocol_engine/commands/unsafe/__init__.py index 6b92cc2e18e..72698a3b0f2 100644 --- a/api/src/opentrons/protocol_engine/commands/unsafe/__init__.py +++ b/api/src/opentrons/protocol_engine/commands/unsafe/__init__.py @@ -31,6 +31,15 @@ UnsafeEngageAxesCreate, ) +from .unsafe_ungrip_labware import ( + UnsafeUngripLabwareCommandType, + UnsafeUngripLabwareParams, + UnsafeUngripLabwareResult, + UnsafeUngripLabware, + UnsafeUngripLabwareCreate, +) + + __all__ = [ # Unsafe blow-out-in-place command models "UnsafeBlowOutInPlaceCommandType", @@ -56,4 +65,10 @@ "UnsafeEngageAxesResult", "UnsafeEngageAxes", "UnsafeEngageAxesCreate", + # Unsafe ungrip labware + "UnsafeUngripLabwareCommandType", + "UnsafeUngripLabwareParams", + "UnsafeUngripLabwareResult", + "UnsafeUngripLabware", + "UnsafeUngripLabwareCreate", ] diff --git a/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py b/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py new file mode 100644 index 00000000000..e64beaa7ea7 --- /dev/null +++ b/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py @@ -0,0 +1,73 @@ +"""Ungrip labware payload, result, and implementaiton.""" + +from __future__ import annotations +from opentrons.protocol_engine.errors.exceptions import GripperNotAttachedError +from pydantic import BaseModel +from typing import Optional, Type +from typing_extensions import Literal + +from ..command import AbstractCommandImpl, BaseCommand, BaseCommandCreate, SuccessData +from ...errors.error_occurrence import ErrorOccurrence +from ...resources import ensure_ot3_hardware + +from opentrons.hardware_control import HardwareControlAPI + + +UnsafeUngripLabwareCommandType = Literal["unsafe/ungripLabware"] + + +class UnsafeUngripLabwareParams(BaseModel): + """Payload required for an UngripLabware command.""" + + +class UnsafeUngripLabwareResult(BaseModel): + """Result data from the execution of an UngripLabware command.""" + + +class UnsafeUngripLabwareImplementation( + AbstractCommandImpl[ + UnsafeUngripLabwareParams, + SuccessData[UnsafeUngripLabwareResult, None], + ] +): + """Ungrip labware command implementation.""" + + def __init__( + self, + hardware_api: HardwareControlAPI, + **kwargs: object, + ) -> None: + self._hardware_api = hardware_api + + async def execute( + self, params: UnsafeUngripLabwareParams + ) -> SuccessData[UnsafeUngripLabwareResult, None]: + """Ungrip Labware.""" + ot3_hardware_api = ensure_ot3_hardware(self._hardware_api) + if not ot3_hardware_api.has_gripper(): + raise GripperNotAttachedError("No gripper found to perform ungrip.") + await ot3_hardware_api.ungrip() + return SuccessData(public=UnsafeUngripLabwareResult(), private=None) + + +class UnsafeUngripLabware( + BaseCommand[UnsafeUngripLabwareParams, UnsafeUngripLabwareResult, ErrorOccurrence] +): + """UnsafeUngripLabware command model.""" + + commandType: UnsafeUngripLabwareCommandType = "unsafe/ungripLabware" + params: UnsafeUngripLabwareParams + result: Optional[UnsafeUngripLabwareResult] + + _ImplementationCls: Type[ + UnsafeUngripLabwareImplementation + ] = UnsafeUngripLabwareImplementation + + +class UnsafeUngripLabwareCreate(BaseCommandCreate[UnsafeUngripLabwareParams]): + """UnsafeEngageAxes command request model.""" + + commandType: UnsafeUngripLabwareCommandType = "unsafe/ungripLabware" + params: UnsafeUngripLabwareParams + + _CommandCls: Type[UnsafeUngripLabware] = UnsafeUngripLabware diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index d01926862de..6723c521892 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -17,6 +17,9 @@ RunCommandAction, SetErrorRecoveryPolicyAction, ) +from opentrons.protocol_engine.commands.unsafe.unsafe_ungrip_labware import ( + UnsafeUngripLabwareCommandType, +) from opentrons.protocol_engine.error_recovery_policy import ( ErrorRecoveryPolicy, ErrorRecoveryType, @@ -36,7 +39,7 @@ DoorChangeAction, ) -from ..commands import Command, CommandStatus, CommandIntent +from ..commands import Command, CommandStatus, CommandIntent, CommandCreate from ..errors import ( RunStoppedError, ErrorOccurrence, @@ -95,7 +98,9 @@ class QueueStatus(enum.Enum): AWAITING_RECOVERY_PAUSED = enum.auto() """Execution of fixit commands has been paused. - New protocol and fixit commands may be enqueued, but will wait to execute. + New protocol and fixit commands may be enqueued, but will usually wait to execute. + There are certain exceptions where fixit commands will still run. + New setup commands may not be enqueued. """ @@ -740,6 +745,12 @@ def get_next_to_execute(self) -> Optional[str]: next_fixit_cmd = self._state.command_history.get_fixit_queue_ids().head(None) if next_fixit_cmd and self._state.queue_status == QueueStatus.AWAITING_RECOVERY: return next_fixit_cmd + if ( + next_fixit_cmd + and self._state.queue_status == QueueStatus.AWAITING_RECOVERY_PAUSED + and self._may_run_with_door_open(fixit_command=self.get(next_fixit_cmd)) + ): + return next_fixit_cmd # if there is a setup command queued, prioritize it next_setup_cmd = self._state.command_history.get_setup_queue_ids().head(None) @@ -970,12 +981,23 @@ def validate_action_allowed( # noqa: C901 "Setup commands are not allowed after run has started." ) elif action.request.intent == CommandIntent.FIXIT: - if self._state.queue_status != QueueStatus.AWAITING_RECOVERY: + if self.get_status() == EngineStatus.AWAITING_RECOVERY: + return action + elif self.get_status() in ( + EngineStatus.AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR, + EngineStatus.AWAITING_RECOVERY_PAUSED, + ): + if self._may_run_with_door_open(fixit_command=action.request): + return action + else: + raise FixitCommandNotAllowedError( + f"{action.request.commandType} fixit command may not run" + " until the door is closed and the run is played again." + ) + else: raise FixitCommandNotAllowedError( "Fixit commands are not allowed when the run is not in a recoverable state." ) - else: - return action else: return action @@ -1060,3 +1082,22 @@ def get_error_recovery_policy(self) -> ErrorRecoveryPolicy: higher-level code. """ return self._state.error_recovery_policy + + def _may_run_with_door_open( + self, *, fixit_command: Command | CommandCreate + ) -> bool: + """Return whether the given fixit command is exempt from the usual open-door auto pause. + + This is required for certain error recovery flows, where we want the robot to + do stuff while the door is open. + """ + # CommandIntent.PROTOCOL and CommandIntent.SETUP have their own rules for whether + # they run while the door is open. Passing one of those commands to this function + # is probably a mistake in the caller's logic. + assert fixit_command.intent == CommandIntent.FIXIT + + # This type annotation is to make sure the string constant stays in sync and isn't typo'd. + required_command_type: UnsafeUngripLabwareCommandType = "unsafe/ungripLabware" + # todo(mm, 2024-10-04): Instead of allowlisting command types, maybe we should + # add a `mayRunWithDoorOpen: bool` field to command requests. + return fixit_command.commandType == required_command_type diff --git a/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py b/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py new file mode 100644 index 00000000000..1a41244d556 --- /dev/null +++ b/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py @@ -0,0 +1,40 @@ +"""Test update-position-estimator commands.""" +from decoy import Decoy + +from opentrons.protocol_engine.commands.unsafe.unsafe_ungrip_labware import ( + UnsafeUngripLabwareParams, + UnsafeUngripLabwareResult, + UnsafeUngripLabwareImplementation, +) +from opentrons.protocol_engine.commands.command import SuccessData +from opentrons.protocol_engine.errors.exceptions import GripperNotAttachedError +from opentrons.hardware_control import OT3HardwareControlAPI +import pytest + + +async def test_ungrip_labware_implementation( + decoy: Decoy, ot3_hardware_api: OT3HardwareControlAPI +) -> None: + """Test UngripLabware command execution.""" + subject = UnsafeUngripLabwareImplementation(hardware_api=ot3_hardware_api) + + decoy.when(ot3_hardware_api.has_gripper()).then_return(True) + + result = await subject.execute(params=UnsafeUngripLabwareParams()) + + assert result == SuccessData(public=UnsafeUngripLabwareResult(), private=None) + + decoy.verify( + await ot3_hardware_api.ungrip(), + ) + + +async def test_ungrip_labware_implementation_raises_no_gripper_attached( + decoy: Decoy, ot3_hardware_api: OT3HardwareControlAPI +) -> None: + """Test UngripLabware command execution.""" + subject = UnsafeUngripLabwareImplementation(hardware_api=ot3_hardware_api) + + decoy.when(ot3_hardware_api.has_gripper()).then_return(False) + with pytest.raises(GripperNotAttachedError): + await subject.execute(params=UnsafeUngripLabwareParams()) diff --git a/api/tests/opentrons/protocol_engine/state/test_command_state.py b/api/tests/opentrons/protocol_engine/state/test_command_state.py index afafcc3cabe..6f090612a74 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_state.py @@ -557,6 +557,79 @@ def test_door_during_error_recovery() -> None: assert subject.state.failed_command_errors == [expected_error_occurance] +@pytest.mark.parametrize("close_door_before_queueing", [False, True]) +def test_door_ungrip_labware(close_door_before_queueing: bool) -> None: + """Ungrip commands should be able to run even when the door is open.""" + subject = CommandStore( + is_door_open=False, + error_recovery_policy=_placeholder_error_recovery_policy, + config=Config( + block_on_door_open=True, + # Choice of robot and deck type are arbitrary. + robot_type="OT-2 Standard", + deck_type=DeckType.OT2_STANDARD, + ), + ) + subject_view = CommandView(subject.state) + + # Fail a command to put the subject in recovery mode. + queue_failing = actions.QueueCommandAction( + request=commands.CommentCreate( + params=commands.CommentParams(message=""), key="command-key-1" + ), + request_hash=None, + created_at=datetime(year=2021, month=1, day=1), + command_id="failing-command-id", + ) + subject.handle_action(queue_failing) + run_failing = actions.RunCommandAction( + command_id="failing-command-id", + started_at=datetime(year=2022, month=2, day=2), + ) + subject.handle_action(run_failing) + expected_error = errors.ProtocolEngineError(message="oh no") + fail_failing = actions.FailCommandAction( + command_id="failing-command-id", + running_command=subject_view.get("failing-command-id"), + error_id="error-id", + failed_at=datetime(year=2023, month=3, day=3), + error=expected_error, + notes=[], + type=ErrorRecoveryType.WAIT_FOR_RECOVERY, + ) + subject.handle_action(fail_failing) + + # Open the door: + subject.handle_action(actions.DoorChangeAction(DoorState.OPEN)) + assert ( + subject_view.get_status() == EngineStatus.AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR + ) + assert subject_view.get_next_to_execute() is None + + if close_door_before_queueing: + subject.handle_action(actions.DoorChangeAction(DoorState.CLOSED)) + + assert subject_view.get_status() in ( + EngineStatus.AWAITING_RECOVERY_PAUSED, # If we closed the door. + EngineStatus.AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR, # If we didn't. + ) + + # Make sure the special ungrip command can be queued and that it will be returned + # as next to execute: + queue_fixit = actions.QueueCommandAction( + request=commands.unsafe.UnsafeUngripLabwareCreate( + params=commands.unsafe.UnsafeUngripLabwareParams(), + intent=CommandIntent.FIXIT, + ), + request_hash=None, + created_at=datetime(year=2021, month=1, day=1), + command_id="fixit-command-id", + ) + subject_view.validate_action_allowed(queue_fixit) + subject.handle_action(queue_fixit) + assert subject_view.get_next_to_execute() == "fixit-command-id" + + @pytest.mark.parametrize( ("door_initially_open", "expected_engine_status_after_play"), [ diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py index 48918eec6eb..06318cb8d36 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view_old.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view_old.py @@ -595,6 +595,21 @@ class ActionAllowedSpec(NamedTuple): action=ResumeFromRecoveryAction(), expected_error=errors.ResumeFromRecoveryNotAllowedError, ), + ActionAllowedSpec( + subject=get_command_view( + queue_status=QueueStatus.AWAITING_RECOVERY_PAUSED, is_door_blocking=True + ), + action=QueueCommandAction( + request=cmd.unsafe.UnsafeUngripLabwareCreate( + params=cmd.unsafe.UnsafeUngripLabwareParams(), + intent=cmd.CommandIntent.FIXIT, + ), + request_hash=None, + command_id="command-id", + created_at=datetime(year=2021, month=1, day=1), + ), + expected_error=None, + ), ] diff --git a/robot-server/simulators/test-flex.json b/robot-server/simulators/test-flex.json index ed694a8cb92..34544cfa099 100644 --- a/robot-server/simulators/test-flex.json +++ b/robot-server/simulators/test-flex.json @@ -9,6 +9,10 @@ "left": { "model": "p50_single_3.4", "id": "123" + }, + "gripper":{ + "model": "gripper_1.3", + "id": "1234" } }, "attached_modules": { diff --git a/shared-data/command/schemas/10.json b/shared-data/command/schemas/10.json index d83ec30e388..07afff241a1 100644 --- a/shared-data/command/schemas/10.json +++ b/shared-data/command/schemas/10.json @@ -74,7 +74,8 @@ "unsafe/blowOutInPlace": "#/definitions/UnsafeBlowOutInPlaceCreate", "unsafe/dropTipInPlace": "#/definitions/UnsafeDropTipInPlaceCreate", "unsafe/updatePositionEstimators": "#/definitions/UpdatePositionEstimatorsCreate", - "unsafe/engageAxes": "#/definitions/UnsafeEngageAxesCreate" + "unsafe/engageAxes": "#/definitions/UnsafeEngageAxesCreate", + "unsafe/ungripLabware": "#/definitions/UnsafeUngripLabwareCreate" } }, "oneOf": [ @@ -287,6 +288,9 @@ }, { "$ref": "#/definitions/UnsafeEngageAxesCreate" + }, + { + "$ref": "#/definitions/UnsafeUngripLabwareCreate" } ], "definitions": { @@ -4526,6 +4530,42 @@ } }, "required": ["params"] + }, + "UnsafeUngripLabwareParams": { + "title": "UnsafeUngripLabwareParams", + "description": "Payload required for an UngripLabware command.", + "type": "object", + "properties": {} + }, + "UnsafeUngripLabwareCreate": { + "title": "UnsafeUngripLabwareCreate", + "description": "UnsafeEngageAxes command request model.", + "type": "object", + "properties": { + "commandType": { + "title": "Commandtype", + "default": "unsafe/ungripLabware", + "enum": ["unsafe/ungripLabware"], + "type": "string" + }, + "params": { + "$ref": "#/definitions/UnsafeUngripLabwareParams" + }, + "intent": { + "description": "The reason the command was added. If not specified or `protocol`, the command will be treated as part of the protocol run itself, and added to the end of the existing command queue.\n\nIf `setup`, the command will be treated as part of run setup. A setup command may only be enqueued if the run has not started.\n\nUse setup commands for activities like pre-run calibration checks and module setup, like pre-heating.", + "allOf": [ + { + "$ref": "#/definitions/CommandIntent" + } + ] + }, + "key": { + "title": "Key", + "description": "A key value, unique in this run, that can be used to track the same logical command across multiple runs of the same protocol. If a value is not provided, one will be generated.", + "type": "string" + } + }, + "required": ["params"] } }, "$id": "opentronsCommandSchemaV10",