Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): allow ungrip gripper labware while door is open #16394

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4aaaeb4
unsafe ungrip and get next command WIP
TamarZanzouri Sep 30, 2024
472540f
commands init and linting
TamarZanzouri Sep 30, 2024
b43e17c
docstring and playing with fixit commands
TamarZanzouri Sep 30, 2024
65d1f1e
removed check from ungrip
TamarZanzouri Oct 1, 2024
10ee57a
commands schema and logic in validate action
TamarZanzouri Oct 1, 2024
117596f
fixed confition to raise fixit commands
TamarZanzouri Oct 1, 2024
09703e8
removed comments
TamarZanzouri Oct 1, 2024
62e1a88
added tests
TamarZanzouri Oct 1, 2024
6f6b27d
linting and tests
TamarZanzouri Oct 1, 2024
5d5cc13
raise if no gripper attached
TamarZanzouri Oct 2, 2024
48e0bd4
remove print
TamarZanzouri Oct 2, 2024
722ebc1
gripper instrument
TamarZanzouri Oct 2, 2024
e1d66e6
Merge branch 'edge' into EXEC-734-allow-the-gripper-to-be-opened-whil…
TamarZanzouri Oct 2, 2024
5ede81a
command schema 10
TamarZanzouri Oct 2, 2024
b55da53
error message and removed not ER door statue
TamarZanzouri Oct 2, 2024
9cc14e9
linting
TamarZanzouri Oct 2, 2024
5b49230
Merge branch 'edge' into EXEC-734-allow-the-gripper-to-be-opened-whil…
SyntaxColoring Oct 4, 2024
4e7f5eb
Revert changes to schema 9.
SyntaxColoring Oct 4, 2024
0f74ded
Typo.
SyntaxColoring Oct 4, 2024
ecc63db
Merge branch 'edge' into EXEC-734-allow-the-gripper-to-be-opened-whil…
SyntaxColoring Oct 4, 2024
94c0589
Various readability refactors, per feedback.
SyntaxColoring Oct 4, 2024
9056123
Fix test?
SyntaxColoring Oct 4, 2024
f4562bc
Update test_door_ungrip_labware().
SyntaxColoring Oct 5, 2024
d792ca6
make format-js
SyntaxColoring Oct 5, 2024
b9a6797
Minor simplification.
SyntaxColoring Oct 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading