feat(robot-server,api): Add the skeleton for a new `complete-recovery…

…` run action (#14674)

api-client/src/runs/types.ts

@@ -82,11 +82,14 @@ export interface Runs {
 export const RUN_ACTION_TYPE_PLAY: 'play' = 'play'
 export const RUN_ACTION_TYPE_PAUSE: 'pause' = 'pause'
 export const RUN_ACTION_TYPE_STOP: 'stop' = 'stop'
+export const RUN_ACTION_TYPE_RESUME_FROM_RECOVERY: 'resume-from-recovery' =
+  'resume-from-recovery'
 
 export type RunActionType =
   | typeof RUN_ACTION_TYPE_PLAY
   | typeof RUN_ACTION_TYPE_PAUSE
   | typeof RUN_ACTION_TYPE_STOP
+  | typeof RUN_ACTION_TYPE_RESUME_FROM_RECOVERY
 
 export interface RunAction {
   id: string

api/.flake8

@@ -14,6 +14,9 @@ extend-ignore =
     ANN102
     # do not require docstring for __init__, put them on the class
     D107,
+    # Don't forbid the function signature from being mentioned in the first line of the
+    # docstring. It tends to raise false positives when referring to other functions.
+    D402,
 
 # configure flake8-docstrings
 # https://pypi.org/project/flake8-docstrings/

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

@@ -61,6 +61,13 @@ class StopAction:
     from_estop: bool = False
 
 
+@dataclass(frozen=True)
+class ResumeFromRecoveryAction:
+    """See `ProtocolEngine.resume_from_recovery()`."""
+
+    pass
+
+
 @dataclass(frozen=True)
 class FinishErrorDetails:
     """Error details for the payload of a FinishAction or HardwareStoppedAction."""
@@ -203,6 +210,7 @@ class SetPipetteMovementSpeedAction:
     PlayAction,
     PauseAction,
     StopAction,
+    ResumeFromRecoveryAction,
     FinishAction,
     HardwareStoppedAction,
     DoorChangeAction,

api/src/opentrons/protocol_engine/protocol_engine.py

@@ -2,6 +2,7 @@
 from contextlib import AsyncExitStack
 from logging import getLogger
 from typing import Dict, Optional, Union
+from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction
 
 from opentrons.protocols.models import LabwareDefinition
 from opentrons.hardware_control import HardwareControlAPI
@@ -159,6 +160,13 @@ def pause(self) -> None:
         self._action_dispatcher.dispatch(action)
         self._hardware_api.pause(HardwarePauseType.PAUSE)
 
+    def resume_from_recovery(self) -> None:
+        """Resume normal protocol execution after the engine was `AWAITING_RECOVERY`."""
+        action = self._state_store.commands.validate_action_allowed(
+            ResumeFromRecoveryAction()
+        )
+        self._action_dispatcher.dispatch(action)
+
     def add_command(self, request: commands.CommandCreate) -> commands.Command:
         """Add a command to the `ProtocolEngine`'s queue.
 

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

@@ -11,6 +11,7 @@
 from opentrons.ordered_set import OrderedSet
 
 from opentrons.hardware_control.types import DoorState
+from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction
 
 from ..actions import (
     Action,
@@ -665,10 +666,22 @@ def get_is_terminal(self) -> bool:
         """Get whether engine is in a terminal state."""
         return self._state.run_result is not None
 
-    def validate_action_allowed(
+    def validate_action_allowed(  # noqa: C901
         self,
-        action: Union[PlayAction, PauseAction, StopAction, QueueCommandAction],
-    ) -> Union[PlayAction, PauseAction, StopAction, QueueCommandAction]:
+        action: Union[
+            PlayAction,
+            PauseAction,
+            StopAction,
+            ResumeFromRecoveryAction,
+            QueueCommandAction,
+        ],
+    ) -> Union[
+        PlayAction,
+        PauseAction,
+        StopAction,
+        ResumeFromRecoveryAction,
+        QueueCommandAction,
+    ]:
         """Validate whether a given control action is allowed.
 
         Returns:
@@ -681,6 +694,17 @@ def validate_action_allowed(
             SetupCommandNotAllowedError: The engine is running, so a setup command
                 may not be added.
         """
+        if self.get_status() == EngineStatus.AWAITING_RECOVERY:
+            # While we're developing error recovery, we'll conservatively disallow
+            # all actions, to avoid putting the engine in weird undefined states.
+            # We'll allow specific actions here as we flesh things out and add support
+            # for them.
+            raise NotImplementedError()
+
+        if isinstance(action, ResumeFromRecoveryAction):
+            # https://opentrons.atlassian.net/browse/EXEC-301
+            raise NotImplementedError()
+
         if self._state.run_result is not None:
             raise RunStoppedError("The run has already stopped.")
 
@@ -701,13 +725,6 @@ def validate_action_allowed(
                     "Setup commands are not allowed after run has started."
                 )
 
-        elif self.get_status() == EngineStatus.AWAITING_RECOVERY:
-            # While we're developing error recovery, we'll conservatively disallow
-            # all actions, to avoid putting the engine in weird undefined states.
-            # We'll allow specific actions here as we flesh things out and add support
-            # for them.
-            raise NotImplementedError()
-
         return action
 
     def get_status(self) -> EngineStatus:

api/src/opentrons/protocol_runner/protocol_runner.py

@@ -101,6 +101,10 @@ async def stop(self) -> None:
                 post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE,
             )
 
+    def resume_from_recovery(self) -> None:
+        """See `ProtocolEngine.resume_from_recovery()`."""
+        self._protocol_engine.resume_from_recovery()
+
     @abstractmethod
     async def run(
         self,

api/tests/opentrons/protocol_engine/state/test_command_view.py

@@ -14,6 +14,7 @@
     StopAction,
     QueueCommandAction,
 )
+from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction
 
 from opentrons.protocol_engine.state.commands import (
     CommandState,
@@ -322,8 +323,14 @@ class ActionAllowedSpec(NamedTuple):
     """Spec data to test CommandView.validate_action_allowed."""
 
     subject: CommandView
-    action: Union[PlayAction, PauseAction, StopAction, QueueCommandAction]
-    expected_error: Optional[Type[errors.ProtocolEngineError]]
+    action: Union[
+        PlayAction,
+        PauseAction,
+        StopAction,
+        QueueCommandAction,
+        ResumeFromRecoveryAction,
+    ]
+    expected_error: Optional[Type[Exception]]
 
 
 action_allowed_specs: List[ActionAllowedSpec] = [
@@ -455,14 +462,21 @@ class ActionAllowedSpec(NamedTuple):
         ),
         expected_error=errors.SetupCommandNotAllowedError,
     ),
+    # Resuming from error recovery is not implemented yet.
+    # https://opentrons.atlassian.net/browse/EXEC-301
+    ActionAllowedSpec(
+        subject=get_command_view(),
+        action=ResumeFromRecoveryAction(),
+        expected_error=NotImplementedError,
+    ),
 ]
 
 
 @pytest.mark.parametrize(ActionAllowedSpec._fields, action_allowed_specs)
 def test_validate_action_allowed(
     subject: CommandView,
     action: Union[PlayAction, PauseAction, StopAction],
-    expected_error: Optional[Type[errors.ProtocolEngineError]],
+    expected_error: Optional[Type[Exception]],
 ) -> None:
     """It should validate allowed play/pause/stop actions."""
     expectation = pytest.raises(expected_error) if expected_error else does_not_raise()

api/tests/opentrons/protocol_engine/test_protocol_engine.py

@@ -8,6 +8,7 @@
 
 from opentrons_shared_data.robot.dev_types import RobotType
 from opentrons.ordered_set import OrderedSet
+from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction
 
 from opentrons.types import DeckSlotName
 from opentrons.hardware_control import HardwareControlAPI, OT2HardwareControlAPI
@@ -427,6 +428,24 @@ def test_pause(
     )
 
 
+def test_resume_from_recovery(
+    decoy: Decoy,
+    state_store: StateStore,
+    action_dispatcher: ActionDispatcher,
+    subject: ProtocolEngine,
+) -> None:
+    """It should dispatch a ResumeFromRecoveryAction."""
+    expected_action = ResumeFromRecoveryAction()
+
+    decoy.when(
+        state_store.commands.validate_action_allowed(expected_action)
+    ).then_return(expected_action)
+
+    subject.resume_from_recovery()
+
+    decoy.verify(action_dispatcher.dispatch(expected_action))
+
+
 @pytest.mark.parametrize("drop_tips_after_run", [True, False])
 @pytest.mark.parametrize("set_run_status", [True, False])
 @pytest.mark.parametrize(

api/tests/opentrons/protocol_runner/test_protocol_runner.py

@@ -169,7 +169,7 @@ def live_runner_subject(
         (None, LiveRunner),
     ],
 )
-async def test_create_protocol_runner(
+def test_create_protocol_runner(
     protocol_engine: ProtocolEngine,
     hardware_api: HardwareAPI,
     task_queue: TaskQueue,
@@ -203,7 +203,7 @@ async def test_create_protocol_runner(
         (lazy_fixture("live_runner_subject")),
     ],
 )
-async def test_play_starts_run(
+def test_play_starts_run(
     decoy: Decoy,
     protocol_engine: ProtocolEngine,
     task_queue: TaskQueue,
@@ -223,7 +223,7 @@ async def test_play_starts_run(
         (lazy_fixture("live_runner_subject")),
     ],
 )
-async def test_pause(
+def test_pause(
     decoy: Decoy,
     protocol_engine: ProtocolEngine,
     subject: AnyRunner,
@@ -286,6 +286,25 @@ async def test_stop_when_run_never_started(
     )
 
 
+@pytest.mark.parametrize(
+    "subject",
+    [
+        (lazy_fixture("json_runner_subject")),
+        (lazy_fixture("legacy_python_runner_subject")),
+        (lazy_fixture("live_runner_subject")),
+    ],
+)
+def test_resume_from_recovery(
+    decoy: Decoy,
+    protocol_engine: ProtocolEngine,
+    subject: AnyRunner,
+) -> None:
+    """It should call `resume_from_recovery()` on the underlying engine."""
+    subject.resume_from_recovery()
+
+    decoy.verify(protocol_engine.resume_from_recovery(), times=1)
+
+
 async def test_run_json_runner(
     decoy: Decoy,
     hardware_api: HardwareAPI,

robot-server/robot_server/runs/action_models.py

@@ -7,17 +7,20 @@
 
 
 class RunActionType(str, Enum):
-    """Types of run control actions.
-
-    Args:
-        PLAY: Start or resume a protocol run.
-        PAUSE: Pause a run.
-        STOP: Stop (cancel) a run.
+    """The type of the run control action.
+
+    * `"play"`: Start or resume a run.
+    * `"pause"`: Pause a run.
+    * `"stop"`: Stop (cancel) a run.
+    * `"resume-from-recovery"`: Resume normal protocol execution after a command failed,
+      the run was placed in `awaiting-recovery` mode, and manual recovery steps
+      were taken.
     """
 
     PLAY = "play"
     PAUSE = "pause"
     STOP = "stop"
+    RESUME_FROM_RECOVERY = "resume-from-recovery"
 
 
 class RunActionCreate(BaseModel):

robot-server/robot_server/runs/router/actions_router.py

@@ -87,7 +87,6 @@ async def get_run_controller(
 async def create_run_action(
     runId: str,
     request_body: RequestModel[RunActionCreate],
-    engine_store: EngineStore = Depends(get_engine_store),
     run_controller: RunController = Depends(get_run_controller),
     action_id: str = Depends(get_unique_id),
     created_at: datetime = Depends(get_current_time),

robot-server/robot_server/runs/run_controller.py

@@ -85,6 +85,9 @@ def create_action(
                 log.info(f'Stopping run "{self._run_id}".')
                 self._task_runner.run(self._engine_store.runner.stop)
 
+            elif action_type == RunActionType.RESUME_FROM_RECOVERY:
+                self._engine_store.runner.resume_from_recovery()
+
         except ProtocolEngineError as e:
             raise RunActionNotAllowedError(message=e.message, wrapping=[e]) from e
 

robot-server/tests/runs/test_run_controller.py

@@ -168,7 +168,7 @@ async def test_create_play_action_to_start(
     )
 
 
-async def test_create_pause_action(
+def test_create_pause_action(
     decoy: Decoy,
     mock_engine_store: EngineStore,
     mock_run_store: RunStore,
@@ -193,7 +193,7 @@ async def test_create_pause_action(
     decoy.verify(mock_engine_store.runner.pause(), times=1)
 
 
-async def test_create_stop_action(
+def test_create_stop_action(
     decoy: Decoy,
     mock_engine_store: EngineStore,
     mock_run_store: RunStore,
@@ -219,6 +219,32 @@ async def test_create_stop_action(
     decoy.verify(mock_task_runner.run(mock_engine_store.runner.stop), times=1)
 
 
+def test_create_resume_from_recovery_action(
+    decoy: Decoy,
+    mock_engine_store: EngineStore,
+    mock_run_store: RunStore,
+    mock_task_runner: TaskRunner,
+    run_id: str,
+    subject: RunController,
+) -> None:
+    """It should call `resume_from_recovery()` on the underlying engine store."""
+    result = subject.create_action(
+        action_id="some-action-id",
+        action_type=RunActionType.RESUME_FROM_RECOVERY,
+        created_at=datetime(year=2021, month=1, day=1),
+        action_payload=[],
+    )
+
+    assert result == RunAction(
+        id="some-action-id",
+        actionType=RunActionType.RESUME_FROM_RECOVERY,
+        createdAt=datetime(year=2021, month=1, day=1),
+    )
+
+    decoy.verify(mock_run_store.insert_action(run_id, result), times=1)
+    decoy.verify(mock_engine_store.runner.resume_from_recovery())
+
+
 @pytest.mark.parametrize(
     ("action_type", "exception"),
     [