fix(protocol-engine): Support door open/close during error recovery (#15478)

Author: SyntaxColoring

api-client/src/runs/types.ts

@@ -21,6 +21,8 @@ export const RUN_STATUS_FINISHING = 'finishing' as const
 export const RUN_STATUS_SUCCEEDED = 'succeeded' as const
 export const RUN_STATUS_BLOCKED_BY_OPEN_DOOR = 'blocked-by-open-door' as const
 export const RUN_STATUS_AWAITING_RECOVERY = 'awaiting-recovery' as const
+export const RUN_STATUS_AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR = 'awaiting-recovery-blocked-by-open-door' as const
+export const RUN_STATUS_AWAITING_RECOVERY_PAUSED = 'awaiting-recovery-paused' as const
 
 export type RunStatus =
   | typeof RUN_STATUS_IDLE
@@ -33,6 +35,8 @@ export type RunStatus =
   | typeof RUN_STATUS_SUCCEEDED
   | typeof RUN_STATUS_BLOCKED_BY_OPEN_DOOR
   | typeof RUN_STATUS_AWAITING_RECOVERY
+  | typeof RUN_STATUS_AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR
+  | typeof RUN_STATUS_AWAITING_RECOVERY_PAUSED
 
 export interface LegacyGoodRunData {
   id: string

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

@@ -88,13 +88,20 @@ class QueueStatus(enum.Enum):
     New fixup commands may be enqueued and will execute immediately.
     """
 
+    AWAITING_RECOVERY_PAUSED = enum.auto()
+    """Execution of fixit commands has been paused.
 
-class RunResult(str, enum.Enum):
+    New protocol and fixit commands may be enqueued, but will wait to execute.
+    New setup commands may not be enqueued.
+    """
+
+
+class RunResult(enum.Enum):
     """Result of the run."""
 
-    SUCCEEDED = "succeeded"
-    FAILED = "failed"
-    STOPPED = "stopped"
+    SUCCEEDED = enum.auto()
+    FAILED = enum.auto()
+    STOPPED = enum.auto()
 
 
 @dataclass(frozen=True)
@@ -348,11 +355,20 @@ def handle_action(self, action: Action) -> None:  # noqa: C901
                 self._state.run_started_at = (
                     self._state.run_started_at or action.requested_at
                 )
-                if self._state.is_door_blocking:
-                    # Always inactivate queue when door is blocking
-                    self._state.queue_status = QueueStatus.PAUSED
-                else:
-                    self._state.queue_status = QueueStatus.RUNNING
+                match self._state.queue_status:
+                    case QueueStatus.SETUP:
+                        self._state.queue_status = (
+                            QueueStatus.PAUSED
+                            if self._state.is_door_blocking
+                            else QueueStatus.RUNNING
+                        )
+                    case QueueStatus.AWAITING_RECOVERY_PAUSED:
+                        self._state.queue_status = QueueStatus.AWAITING_RECOVERY
+                    case QueueStatus.PAUSED:
+                        self._state.queue_status = QueueStatus.RUNNING
+                    case QueueStatus.RUNNING | QueueStatus.AWAITING_RECOVERY:
+                        # Nothing for the play action to do. No-op.
+                        pass
 
         elif isinstance(action, PauseAction):
             self._state.queue_status = QueueStatus.PAUSED
@@ -364,8 +380,7 @@ def handle_action(self, action: Action) -> None:  # noqa: C901
 
         elif isinstance(action, StopAction):
             if not self._state.run_result:
-                if self._state.queue_status == QueueStatus.AWAITING_RECOVERY:
-                    self._state.recovery_target_command_id = None
+                self._state.recovery_target_command_id = None
 
                 self._state.queue_status = QueueStatus.PAUSED
                 if action.from_estop:
@@ -422,10 +437,15 @@ def handle_action(self, action: Action) -> None:  # noqa: C901
             if self._config.block_on_door_open:
                 if action.door_state == DoorState.OPEN:
                     self._state.is_door_blocking = True
-                    # todo(mm, 2024-03-19): It's unclear how the door should interact
-                    # with error recovery (QueueStatus.AWAITING_RECOVERY).
-                    if self._state.queue_status != QueueStatus.SETUP:
-                        self._state.queue_status = QueueStatus.PAUSED
+                    match self._state.queue_status:
+                        case QueueStatus.SETUP:
+                            pass
+                        case QueueStatus.RUNNING | QueueStatus.PAUSED:
+                            self._state.queue_status = QueueStatus.PAUSED
+                        case QueueStatus.AWAITING_RECOVERY | QueueStatus.AWAITING_RECOVERY_PAUSED:
+                            self._state.queue_status = (
+                                QueueStatus.AWAITING_RECOVERY_PAUSED
+                            )
                 elif action.door_state == DoorState.CLOSED:
                     self._state.is_door_blocking = False
 
@@ -847,18 +867,19 @@ def validate_action_allowed(  # noqa: C901
             raise RunStoppedError("The run has already stopped.")
 
         elif isinstance(action, PlayAction):
-            if self.get_status() == EngineStatus.BLOCKED_BY_OPEN_DOOR:
+            if self.get_status() in (
+                EngineStatus.BLOCKED_BY_OPEN_DOOR,
+                EngineStatus.AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR,
+            ):
                 raise RobotDoorOpenError("Front door or top window is currently open.")
-            elif self.get_status() == EngineStatus.AWAITING_RECOVERY:
-                raise NotImplementedError()
             else:
                 return action
 
         elif isinstance(action, PauseAction):
             if not self.get_is_running():
                 raise PauseNotAllowedError("Cannot pause a run that is not running.")
             elif self.get_status() == EngineStatus.AWAITING_RECOVERY:
-                raise NotImplementedError()
+                raise PauseNotAllowedError("Cannot pause a run in recovery mode.")
             else:
                 return action
 
@@ -901,7 +922,7 @@ def validate_action_allowed(  # noqa: C901
         else:
             assert_never(action)
 
-    def get_status(self) -> EngineStatus:
+    def get_status(self) -> EngineStatus:  # noqa: C901
         """Get the current execution status of the engine."""
         if self._state.run_result:
             # The main part of the run is over, or will be over soon.
@@ -936,6 +957,12 @@ def get_status(self) -> EngineStatus:
             else:
                 return EngineStatus.PAUSED
 
+        elif self._state.queue_status == QueueStatus.AWAITING_RECOVERY_PAUSED:
+            if self._state.is_door_blocking:
+                return EngineStatus.AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR
+            else:
+                return EngineStatus.AWAITING_RECOVERY_PAUSED
+
         elif self._state.queue_status == QueueStatus.AWAITING_RECOVERY:
             return EngineStatus.AWAITING_RECOVERY
 

api/src/opentrons/protocol_engine/types.py

@@ -33,27 +33,72 @@
 from opentrons_shared_data.module.dev_types import ModuleType as SharedDataModuleType
 
 
+# todo(mm, 2024-06-24): This monolithic status field is getting to be a bit much.
+# We should consider splitting this up into multiple fields.
 class EngineStatus(str, Enum):
-    """Current execution status of a ProtocolEngine."""
+    """Current execution status of a ProtocolEngine.
+
+    This is a high-level summary of what the robot is doing and what interactions are
+    appropriate.
+    """
+
+    # Statuses for an ongoing run:
 
     IDLE = "idle"
+    """The protocol has not been started yet.
+
+    The robot may truly be idle, or it may be executing commands with `intent: "setup"`.
+    """
+
     RUNNING = "running"
+    """The engine is actively running the protocol."""
+
     PAUSED = "paused"
+    """A pause has been requested. Activity is paused, or will pause soon.
+
+    (There is currently no way to tell which.)
+    """
+
     BLOCKED_BY_OPEN_DOOR = "blocked-by-open-door"
+    """The robot's door is open. Activity is paused, or will pause soon."""
+
     STOP_REQUESTED = "stop-requested"
-    STOPPED = "stopped"
+    """A stop has been requested. Activity will stop soon."""
+
     FINISHING = "finishing"
-    FAILED = "failed"
-    SUCCEEDED = "succeeded"
+    """The robot is doing post-run cleanup, like homing and dropping tips."""
+
+    # Statuses for error recovery mode:
 
     AWAITING_RECOVERY = "awaiting-recovery"
     """The engine is waiting for external input to recover from a nonfatal error.
 
-    New fixup commands may be enqueued, which will run immediately.
+    New commands with `intent: "fixit"` may be enqueued, which will run immediately.
     The run can't be paused in this state, but it can be canceled, or resumed from the
     next protocol command if recovery is complete.
     """
 
+    AWAITING_RECOVERY_PAUSED = "awaiting-recovery-paused"
+    """The engine is paused while in error recovery mode. Activity is paused, or will pause soon.
+
+    This state is not possible to enter manually. It happens when an open door
+    gets closed during error recovery.
+    """
+
+    AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR = "awaiting-recovery-blocked-by-open-door"
+    """The robot's door is open while in recovery mode. Activity is paused, or will pause soon."""
+
+    # Terminal statuses:
+
+    STOPPED = "stopped"
+    """All activity is over; it was stopped by an explicit external request."""
+
+    FAILED = "failed"
+    """All activity is over; there was a fatal error."""
+
+    SUCCEEDED = "succeeded"
+    """All activity is over; things completed without any fatal error."""
+
 
 class DeckSlotLocation(BaseModel):
     """The location of something placed in a single deck slot."""