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

feature(api, robot-server): Allow fixit commands to recover from an error #14908

Merged
merged 43 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
59710a2
added schema definition for fixtures
TamarZanzouri Sep 14, 2023
755abd3
initial commit WIP
TamarZanzouri Apr 4, 2024
8fcbcee
WIP added fixit commands to action validation
TamarZanzouri Apr 5, 2024
eb2caed
WIP commands_history now containing fixit queue and executed commands
TamarZanzouri Apr 9, 2024
3d1f734
added logic for checking failed command and adding to fixit queue
TamarZanzouri Apr 9, 2024
b1bde23
fixed circular dependency and added test
TamarZanzouri Apr 9, 2024
942f8f0
added tests
TamarZanzouri Apr 9, 2024
04bb500
wip next command to execute
TamarZanzouri Apr 9, 2024
d28fe8d
get next queued prioritized fixit
TamarZanzouri Apr 10, 2024
e8d2fc0
WIP e2e test for fixit recovery
TamarZanzouri Apr 10, 2024
e059aef
WIP resume from recovery
TamarZanzouri Apr 12, 2024
0840846
merge
TamarZanzouri Apr 15, 2024
9517147
WIP pr feedback - move failed_command_id to base command ad route and…
TamarZanzouri Apr 15, 2024
9dfa58a
removed tests that are not in prod
TamarZanzouri Apr 15, 2024
a20fdda
WIP failed_command_id from router to BaseCommand
TamarZanzouri Apr 16, 2024
5e631ec
fixed fixit in hash, renaming to protocol hash WIP
TamarZanzouri Apr 16, 2024
f7511c0
has protocol and command queue action set to None
TamarZanzouri Apr 17, 2024
e8436d6
moved setup and fixit hashing logic to add_command
TamarZanzouri Apr 17, 2024
a66a8b9
removed raise and fix failing test
TamarZanzouri Apr 17, 2024
239fad5
fixed rehearsal
TamarZanzouri Apr 18, 2024
0090dd6
added assert for failed command id and tests
TamarZanzouri Apr 18, 2024
ca25777
bad request when giving failed command and non fixit
TamarZanzouri Apr 18, 2024
746b3e0
dont access private methods from tests WIP
TamarZanzouri Apr 19, 2024
86e9651
do not access private methods from test
TamarZanzouri Apr 19, 2024
2527555
changed all tests to not access private methods
TamarZanzouri Apr 19, 2024
c736f81
raise from add_command and catch in router
TamarZanzouri Apr 19, 2024
764bf9d
raise 400 with bad data
TamarZanzouri Apr 19, 2024
9d464a6
Update robot-server/robot_server/runs/router/commands_router.py
TamarZanzouri Apr 19, 2024
b0249a7
Update api/tests/opentrons/protocol_engine/test_protocol_engine.py
TamarZanzouri Apr 19, 2024
2a494ec
Merge branch 'edge' into EXEC-285-add-fixit-commands
TamarZanzouri Apr 19, 2024
4234564
linting
TamarZanzouri Apr 19, 2024
2120bee
linting
TamarZanzouri Apr 19, 2024
0b3229b
looks like the editor from the 80s wins again
sfoster1 Apr 19, 2024
e6ff173
lint-js
TamarZanzouri Apr 22, 2024
5877e38
router name typo
TamarZanzouri Apr 22, 2024
5da6b3a
changed return error type
TamarZanzouri Apr 22, 2024
bd9d061
reverted raise for testing
TamarZanzouri Apr 22, 2024
364c42d
rename to failedCommandId
TamarZanzouri Apr 22, 2024
635d00a
Delete shared-data/fixture/schemas/1.json
TamarZanzouri Apr 22, 2024
7bbc78b
fixed failing tests
TamarZanzouri Apr 22, 2024
b2ed3cf
lint
TamarZanzouri Apr 22, 2024
057f974
format-js
TamarZanzouri Apr 22, 2024
775bb8a
removed tavern test for fixit
TamarZanzouri Apr 22, 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
1 change: 1 addition & 0 deletions api/src/opentrons/protocol_engine/actions/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class QueueCommandAction:
created_at: datetime
request: CommandCreate
request_hash: Optional[str]
failed_command_id: Optional[str] = None


@dataclass(frozen=True)
Expand Down
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_engine/commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from . import thermocycler
from . import calibration

from .hash_command_params import hash_command_params
from .hash_command_params import hash_protocol_command_params
from .generate_command_schema import generate_command_schema

from .command import (
Expand Down Expand Up @@ -333,7 +333,7 @@
"CommandStatus",
"CommandIntent",
# command parameter hashing
"hash_command_params",
"hash_protocol_command_params",
# command schema generation
"generate_command_schema",
# aspirate command models
Expand Down
7 changes: 7 additions & 0 deletions api/src/opentrons/protocol_engine/commands/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class CommandIntent(str, Enum):

PROTOCOL = "protocol"
SETUP = "setup"
FIXIT = "fixit"


class BaseCommandCreate(GenericModel, Generic[CommandParamsT]):
Expand Down Expand Up @@ -159,6 +160,12 @@ class BaseCommand(GenericModel, Generic[CommandParamsT, CommandResultT]):
" the command's execution or the command's generation."
),
)
failedCommandId: Optional[str] = Field(
None,
description=(
"FIXIT command use only. Reference of the failed command id we are trying to fix."
),
)


class AbstractCommandImpl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# TODO(mm, 2023-04-28):
# This implementation will not notice that commands are different if they have different params
# but share the same commandType. We should also hash command params. (Jira RCORE-326.)
def hash_command_params(
def hash_protocol_command_params(
create: CommandCreate, last_hash: Optional[str]
) -> Optional[str]:
"""Given a command create object, return a hash.
Expand All @@ -28,12 +28,11 @@ def hash_command_params(
The command hash, if the command is a protocol command.
`None` if the command is a setup command.
"""
if create.intent == CommandIntent.SETUP:
if create.intent != CommandIntent.PROTOCOL:
return None
else:
# We avoid Python's built-in hash() function because it's not stable across
# runs of the Python interpreter. (Jira RSS-215.)
last_contribution = b"" if last_hash is None else last_hash.encode("ascii")
this_contribution = md5(create.commandType.encode("ascii")).digest()
to_hash = last_contribution + this_contribution
return md5(to_hash).hexdigest()
# We avoid Python's built-in hash() function because it's not stable across
TamarZanzouri marked this conversation as resolved.
Show resolved Hide resolved
# runs of the Python interpreter. (Jira RSS-215.)
last_contribution = b"" if last_hash is None else last_hash.encode("ascii")
this_contribution = md5(create.commandType.encode("ascii")).digest()
to_hash = last_contribution + this_contribution
return md5(to_hash).hexdigest()
7 changes: 6 additions & 1 deletion api/src/opentrons/protocol_engine/errors/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
MustHomeError,
RunStoppedError,
SetupCommandNotAllowedError,
FixitCommandNotAllowedError,
ModuleNotAttachedError,
ModuleAlreadyPresentError,
WrongModuleTypeError,
Expand All @@ -55,6 +56,7 @@
InvalidHoldTimeError,
CannotPerformModuleAction,
PauseNotAllowedError,
ResumeFromRecoveryNotAllowedError,
GripperNotAttachedError,
CannotPerformGripperAction,
HardwareNotSupportedError,
Expand All @@ -65,6 +67,7 @@
LocationIsStagingSlotError,
InvalidAxisForRobotType,
NotSupportedOnRobotType,
CommandNotAllowedError,
)

from .error_occurrence import ErrorOccurrence, ProtocolCommandFailedError
Expand Down Expand Up @@ -109,6 +112,7 @@
"MustHomeError",
"RunStoppedError",
"SetupCommandNotAllowedError",
"FixitCommandNotAllowedError",
"ModuleNotAttachedError",
"ModuleAlreadyPresentError",
"WrongModuleTypeError",
Expand All @@ -124,6 +128,7 @@
"InvalidBlockVolumeError",
"InvalidHoldTimeError",
"CannotPerformModuleAction",
"ResumeFromRecoveryNotAllowedError",
"PauseNotAllowedError",
"ProtocolCommandFailedError",
"GripperNotAttachedError",
Expand All @@ -138,5 +143,5 @@
"NotSupportedOnRobotType",
# error occurrence models
"ErrorOccurrence",
"FailedGripperPickupError",
"CommandNotAllowedError",
]
39 changes: 39 additions & 0 deletions api/src/opentrons/protocol_engine/errors/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,32 @@ def __init__(
super().__init__(ErrorCodes.POSITION_UNKNOWN, message, details, wrapping)


class CommandNotAllowedError(ProtocolEngineError):
"""Raised when adding a command with bad data."""

def __init__(
self,
message: Optional[str] = None,
details: Optional[Dict[str, Any]] = None,
wrapping: Optional[Sequence[EnumeratedError]] = None,
) -> None:
"""Build a CommandNotAllowedError."""
super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping)


class FixitCommandNotAllowedError(ProtocolEngineError):
"""Raised when adding a fixit command to a non-recoverable engine."""

def __init__(
self,
message: Optional[str] = None,
details: Optional[Dict[str, Any]] = None,
wrapping: Optional[Sequence[EnumeratedError]] = None,
) -> None:
"""Build a SetupCommandNotAllowedError."""
super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping)


class SetupCommandNotAllowedError(ProtocolEngineError):
"""Raised when adding a setup command to a non-idle/non-paused engine."""

Expand All @@ -518,6 +544,19 @@ def __init__(
super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping)


class ResumeFromRecoveryNotAllowedError(ProtocolEngineError):
"""Raised when attempting to resume a run from recovery that has a fixit command in the queue."""

def __init__(
self,
message: Optional[str] = None,
details: Optional[Dict[str, Any]] = None,
wrapping: Optional[Sequence[EnumeratedError]] = None,
) -> None:
"""Build a ResumeFromRecoveryNotAllowedError."""
super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping)


class PauseNotAllowedError(ProtocolEngineError):
"""Raised when attempting to pause a run that is not running."""

Expand Down
28 changes: 22 additions & 6 deletions api/src/opentrons/protocol_engine/protocol_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
EnumeratedError,
)

from .errors import ProtocolCommandFailedError, ErrorOccurrence
from .errors import ProtocolCommandFailedError, ErrorOccurrence, CommandNotAllowedError
from .errors.exceptions import EStopActivatedError
from . import commands, slot_standardization
from .resources import ModelUtils, ModuleDataProvider
Expand Down Expand Up @@ -176,7 +176,9 @@ def resume_from_recovery(self) -> None:
)
self._action_dispatcher.dispatch(action)

def add_command(self, request: commands.CommandCreate) -> commands.Command:
def add_command(
self, request: commands.CommandCreate, failed_command_id: Optional[str] = None
TamarZanzouri marked this conversation as resolved.
Show resolved Hide resolved
) -> commands.Command:
"""Add a command to the `ProtocolEngine`'s queue.

Arguments:
Expand All @@ -191,23 +193,37 @@ def add_command(self, request: commands.CommandCreate) -> commands.Command:
but the engine was not idle or paused.
RunStoppedError: the run has been stopped, so no new commands
may be added.
CommandNotAllowedError: the request specified a failed command id
with a non fixit command.
"""
request = slot_standardization.standardize_command(
request, self.state_view.config.robot_type
)

if failed_command_id and request.intent != commands.CommandIntent.FIXIT:
raise CommandNotAllowedError(
"failed command id should be supplied with a FIXIT command."
)

command_id = self._model_utils.generate_id()
request_hash = commands.hash_command_params(
create=request,
last_hash=self._state_store.commands.get_latest_command_hash(),
)
if request.intent in (
commands.CommandIntent.SETUP,
commands.CommandIntent.FIXIT,
):
request_hash = None
else:
request_hash = commands.hash_protocol_command_params(
create=request,
last_hash=self._state_store.commands.get_latest_protocol_command_hash(),
)

action = self.state_view.commands.validate_action_allowed(
QueueCommandAction(
request=request,
request_hash=request_hash,
command_id=command_id,
created_at=self._model_utils.get_timestamp(),
failed_command_id=failed_command_id,
)
)
self._action_dispatcher.dispatch(action)
Expand Down
23 changes: 23 additions & 0 deletions api/src/opentrons/protocol_engine/state/command_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ class CommandHistory:
_queued_setup_command_ids: OrderedSet[str]
"""The IDs of queued setup commands, in FIFO order"""

_queued_fixit_command_ids: OrderedSet[str]
"""The IDs of queued fixit commands, in FIFO order"""

_running_command_id: Optional[str]
"""The ID of the currently running command, if any"""

Expand All @@ -43,6 +46,7 @@ def __init__(self) -> None:
self._all_command_ids = []
self._queued_command_ids = OrderedSet()
self._queued_setup_command_ids = OrderedSet()
self._queued_fixit_command_ids = OrderedSet()
self._commands_by_id = OrderedDict()
self._running_command_id = None
self._terminal_command_id = None
Expand Down Expand Up @@ -135,6 +139,10 @@ def get_setup_queue_ids(self) -> OrderedSet[str]:
"""Get the IDs of all queued setup commands, in FIFO order."""
return self._queued_setup_command_ids

def get_fixit_queue_ids(self) -> OrderedSet[str]:
"""Get the IDs of all queued fixit commands, in FIFO order."""
return self._queued_fixit_command_ids

def clear_queue(self) -> None:
"""Clears all commands within the queued command ids structure."""
self._queued_command_ids.clear()
Expand All @@ -143,6 +151,10 @@ def clear_setup_queue(self) -> None:
"""Clears all commands within the queued setup command ids structure."""
self._queued_setup_command_ids.clear()

def clear_fixit_queue(self) -> None:
"""Clears all commands within the queued setup command ids structure."""
self._queued_fixit_command_ids.clear()

def set_command_queued(self, command: Command) -> None:
"""Validate and mark a command as queued in the command history."""
assert command.status == CommandStatus.QUEUED
Expand All @@ -157,6 +169,8 @@ def set_command_queued(self, command: Command) -> None:

if command.intent == CommandIntent.SETUP:
self._add_to_setup_queue(command.id)
elif command.intent == CommandIntent.FIXIT:
self._add_to_fixit_queue(command.id)
else:
self._add_to_queue(command.id)

Expand All @@ -177,6 +191,7 @@ def set_command_running(self, command: Command) -> None:

self._remove_queue_id(command.id)
self._remove_setup_queue_id(command.id)
self._remove_fixit_queue_id(command.id)

def set_command_succeeded(self, command: Command) -> None:
"""Validate and mark a command as succeeded in the command history."""
Expand Down Expand Up @@ -239,6 +254,10 @@ def _add_to_setup_queue(self, command_id: str) -> None:
"""Add a new ID to the queued setup."""
self._queued_setup_command_ids.add(command_id)

def _add_to_fixit_queue(self, command_id: str) -> None:
"""Add a new ID to the queued fixit."""
self._queued_fixit_command_ids.add(command_id)

def _remove_queue_id(self, command_id: str) -> None:
"""Remove a specific command from the queued command ids structure."""
self._queued_command_ids.discard(command_id)
Expand All @@ -247,6 +266,10 @@ def _remove_setup_queue_id(self, command_id: str) -> None:
"""Remove a specific command from the queued setup command ids structure."""
self._queued_setup_command_ids.discard(command_id)

def _remove_fixit_queue_id(self, command_id: str) -> None:
"""Remove a specific command from the queued fixit command ids structure."""
self._queued_fixit_command_ids.discard(command_id)

def _set_terminal_command_id(self, command_id: str) -> None:
"""Set the ID of the most recently dequeued command."""
self._terminal_command_id = command_id
Expand Down
Loading
Loading