Skip to content

Commit

Permalink
feat(api): allow ungrip gripper labware while door is open (#16394)
Browse files Browse the repository at this point in the history
# Overview

closes [EXEC-734](https://opentrons.atlassian.net/browse/EXEC-734).
add ungrip command and allow queuing it and executing it while door is
open.

## Test Plan and Hands on Testing

1. upload a protocol that will enter ER mode.
2. open door.
3. issue an ungrip command:
```
{
    "data": {
        "commandType": "unsafe/ungripLabware",
        "intent": "fixit",
        "params": {
        }
    }
}
```
4. make sure the gripper opens its jaw.

tested with dev server and the command succeeded but still need to test
with an actual gripper (@SyntaxColoring thank you Max)

## Changelog

1. added an ungrip command `unsafe/ungripLabware`
2. added logic for queuing while the door is open.
3. added logic for allowing to execute while door is open.

## Review requests

1. gripper command makes sense?
2. logic changes make sense? 

## Risk assessment

medium. added a new command but need to make sure nothing has changed
with the door saftey.


[EXEC-734]:
https://opentrons.atlassian.net/browse/EXEC-734?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Max Marrone <[email protected]>
  • Loading branch information
TamarZanzouri and SyntaxColoring authored Oct 7, 2024
1 parent 56cd361 commit 7badfee
Show file tree
Hide file tree
Showing 9 changed files with 312 additions and 6 deletions.
5 changes: 5 additions & 0 deletions api/src/opentrons/protocol_engine/commands/command_unions.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@
unsafe.UnsafeDropTipInPlace,
unsafe.UpdatePositionEstimators,
unsafe.UnsafeEngageAxes,
unsafe.UnsafeUngripLabware,
],
Field(discriminator="commandType"),
]
Expand Down Expand Up @@ -467,6 +468,7 @@
unsafe.UnsafeDropTipInPlaceParams,
unsafe.UpdatePositionEstimatorsParams,
unsafe.UnsafeEngageAxesParams,
unsafe.UnsafeUngripLabwareParams,
]

CommandType = Union[
Expand Down Expand Up @@ -540,6 +542,7 @@
unsafe.UnsafeDropTipInPlaceCommandType,
unsafe.UpdatePositionEstimatorsCommandType,
unsafe.UnsafeEngageAxesCommandType,
unsafe.UnsafeUngripLabwareCommandType,
]

CommandCreate = Annotated[
Expand Down Expand Up @@ -614,6 +617,7 @@
unsafe.UnsafeDropTipInPlaceCreate,
unsafe.UpdatePositionEstimatorsCreate,
unsafe.UnsafeEngageAxesCreate,
unsafe.UnsafeUngripLabwareCreate,
],
Field(discriminator="commandType"),
]
Expand Down Expand Up @@ -689,6 +693,7 @@
unsafe.UnsafeDropTipInPlaceResult,
unsafe.UpdatePositionEstimatorsResult,
unsafe.UnsafeEngageAxesResult,
unsafe.UnsafeUngripLabwareResult,
]

# todo(mm, 2024-06-12): Ideally, command return types would have specific
Expand Down
15 changes: 15 additions & 0 deletions api/src/opentrons/protocol_engine/commands/unsafe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@
UnsafeEngageAxesCreate,
)

from .unsafe_ungrip_labware import (
UnsafeUngripLabwareCommandType,
UnsafeUngripLabwareParams,
UnsafeUngripLabwareResult,
UnsafeUngripLabware,
UnsafeUngripLabwareCreate,
)


__all__ = [
# Unsafe blow-out-in-place command models
"UnsafeBlowOutInPlaceCommandType",
Expand All @@ -56,4 +65,10 @@
"UnsafeEngageAxesResult",
"UnsafeEngageAxes",
"UnsafeEngageAxesCreate",
# Unsafe ungrip labware
"UnsafeUngripLabwareCommandType",
"UnsafeUngripLabwareParams",
"UnsafeUngripLabwareResult",
"UnsafeUngripLabware",
"UnsafeUngripLabwareCreate",
]
Original file line number Diff line number Diff line change
@@ -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
51 changes: 46 additions & 5 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -36,7 +39,7 @@
DoorChangeAction,
)

from ..commands import Command, CommandStatus, CommandIntent
from ..commands import Command, CommandStatus, CommandIntent, CommandCreate
from ..errors import (
RunStoppedError,
ErrorOccurrence,
Expand Down Expand Up @@ -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.
"""

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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())
73 changes: 73 additions & 0 deletions api/tests/opentrons/protocol_engine/state/test_command_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
[
Expand Down
15 changes: 15 additions & 0 deletions api/tests/opentrons/protocol_engine/state/test_command_view_old.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
]


Expand Down
4 changes: 4 additions & 0 deletions robot-server/simulators/test-flex.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
"left": {
"model": "p50_single_3.4",
"id": "123"
},
"gripper":{
"model": "gripper_1.3",
"id": "1234"
}
},
"attached_modules": {
Expand Down
Loading

0 comments on commit 7badfee

Please sign in to comment.