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

Author: TamarZanzouri

api/src/opentrons/protocol_engine/actions/actions.py

@@ -116,6 +116,7 @@ class QueueCommandAction:
     created_at: datetime
     request: CommandCreate
     request_hash: Optional[str]
+    failed_command_id: Optional[str] = None
 
 
 @dataclass(frozen=True)

api/src/opentrons/protocol_engine/commands/__init__.py

@@ -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 (
@@ -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

api/src/opentrons/protocol_engine/commands/command.py

@@ -55,6 +55,7 @@ class CommandIntent(str, Enum):
 
     PROTOCOL = "protocol"
     SETUP = "setup"
+    FIXIT = "fixit"
 
 
 class BaseCommandCreate(GenericModel, Generic[CommandParamsT]):
@@ -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(

api/src/opentrons/protocol_engine/commands/hash_command_params.py

@@ -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.
@@ -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
+    # 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()

api/src/opentrons/protocol_engine/errors/__init__.py

@@ -39,6 +39,7 @@
     MustHomeError,
     RunStoppedError,
     SetupCommandNotAllowedError,
+    FixitCommandNotAllowedError,
     ModuleNotAttachedError,
     ModuleAlreadyPresentError,
     WrongModuleTypeError,
@@ -55,6 +56,7 @@
     InvalidHoldTimeError,
     CannotPerformModuleAction,
     PauseNotAllowedError,
+    ResumeFromRecoveryNotAllowedError,
     GripperNotAttachedError,
     CannotPerformGripperAction,
     HardwareNotSupportedError,
@@ -65,6 +67,7 @@
     LocationIsStagingSlotError,
     InvalidAxisForRobotType,
     NotSupportedOnRobotType,
+    CommandNotAllowedError,
 )
 
 from .error_occurrence import ErrorOccurrence, ProtocolCommandFailedError
@@ -109,6 +112,7 @@
     "MustHomeError",
     "RunStoppedError",
     "SetupCommandNotAllowedError",
+    "FixitCommandNotAllowedError",
     "ModuleNotAttachedError",
     "ModuleAlreadyPresentError",
     "WrongModuleTypeError",
@@ -124,6 +128,7 @@
     "InvalidBlockVolumeError",
     "InvalidHoldTimeError",
     "CannotPerformModuleAction",
+    "ResumeFromRecoveryNotAllowedError",
     "PauseNotAllowedError",
     "ProtocolCommandFailedError",
     "GripperNotAttachedError",
@@ -138,5 +143,5 @@
     "NotSupportedOnRobotType",
     # error occurrence models
     "ErrorOccurrence",
-    "FailedGripperPickupError",
+    "CommandNotAllowedError",
 ]

api/src/opentrons/protocol_engine/errors/exceptions.py

@@ -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."""
 
@@ -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."""
 

api/src/opentrons/protocol_engine/protocol_engine.py

@@ -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
@@ -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
+    ) -> commands.Command:
         """Add a command to the `ProtocolEngine`'s queue.
 
         Arguments:
@@ -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)

api/src/opentrons/protocol_engine/state/command_history.py

@@ -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"""
 
@@ -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
@@ -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()
@@ -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
@@ -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)
 
@@ -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."""
@@ -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)
@@ -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