From 2a502b1dde3b499e7eb06dbf08ea6106ef5b8f85 Mon Sep 17 00:00:00 2001 From: Sanniti Pimpley Date: Fri, 1 Sep 2023 10:50:34 -0400 Subject: [PATCH] fix(api): readjust hardware controller end state in engine cleanup (#13431) --- api/src/opentrons/execute.py | 4 +- api/src/opentrons/hardware_control/api.py | 8 ++- api/src/opentrons/hardware_control/ot3api.py | 13 ++--- .../protocols/motion_controller.py | 7 ++- .../protocol_engine/create_protocol_engine.py | 16 +++-- .../execution/hardware_stopper.py | 22 +++++-- .../protocol_engine/protocol_engine.py | 31 +++++++--- api/src/opentrons/protocol_engine/types.py | 27 +++++++++ .../protocol_runner/protocol_runner.py | 4 +- api/tests/opentrons/conftest.py | 5 +- .../execution/test_hardware_stopper.py | 43 +++++++++++--- .../protocol_engine/test_protocol_engine.py | 58 +++++++++++++++---- .../protocol_runner/test_protocol_runner.py | 7 ++- .../maintenance_engine_store.py | 7 ++- .../robot_server/runs/engine_store.py | 7 ++- 15 files changed, 202 insertions(+), 57 deletions(-) diff --git a/api/src/opentrons/execute.py b/api/src/opentrons/execute.py index 75add87a4d6..38a8db44734 100644 --- a/api/src/opentrons/execute.py +++ b/api/src/opentrons/execute.py @@ -41,6 +41,7 @@ ThreadManagedHardware, ThreadManager, ) +from opentrons.protocol_engine.types import PostRunHardwareState from opentrons.protocols import parse from opentrons.protocols.api_support.deck_type import ( @@ -572,7 +573,8 @@ def _create_live_context_pe( create_protocol_engine_in_thread( hardware_api=hardware_api.wrapped(), config=_get_protocol_engine_config(), - drop_tips_and_home_after=False, + drop_tips_after_run=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, ) ) diff --git a/api/src/opentrons/hardware_control/api.py b/api/src/opentrons/hardware_control/api.py index 6205758ff3a..d1ee331e486 100644 --- a/api/src/opentrons/hardware_control/api.py +++ b/api/src/opentrons/hardware_control/api.py @@ -490,14 +490,15 @@ async def _chained_calls() -> None: asyncio.run_coroutine_threadsafe(_chained_calls(), self._loop) - async def halt(self) -> None: + async def halt(self, disengage_before_stopping: bool = False) -> None: """Immediately stop motion, cancel execution manager and cancel running tasks. After this call, the smoothie will be in a bad state until a call to :py:meth:`stop`. """ - await self._backend.hard_halt() - await self._execution_manager.cancel() + if disengage_before_stopping: + await self._backend.hard_halt() + await self._backend.halt() async def stop(self, home_after: bool = True) -> None: """ @@ -508,6 +509,7 @@ async def stop(self, home_after: bool = True) -> None: robot. After this call, no further recovery is necessary. """ await self._backend.halt() # calls smoothie_driver.kill() + await self._execution_manager.cancel() self._log.info("Recovering from halt") await self.reset() await self.cache_instruments() diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index 24354ab167c..308b5cb74fd 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -742,19 +742,18 @@ async def _cancel_execution_and_running_tasks(self) -> None: """Cancel execution manager and all running (hardware module) tasks.""" await self._execution_manager.cancel() - async def halt(self) -> None: + async def halt(self, disengage_before_stopping: bool = False) -> None: """Immediately disengage all present motors and clear motor and module tasks.""" - # TODO (spp, 2023-08-22): check if disengaging motors is really required - await self.disengage_axes( - [ax for ax in Axis if self._backend.axis_is_present(ax)] - ) + if disengage_before_stopping: + await self.disengage_axes( + [ax for ax in Axis if self._backend.axis_is_present(ax)] + ) await self._stop_motors() - await self._cancel_execution_and_running_tasks() async def stop(self, home_after: bool = True) -> None: """Stop motion as soon as possible, reset, and optionally home.""" await self._stop_motors() - + await self._cancel_execution_and_running_tasks() self._log.info("Resetting OT3API") await self.reset() if home_after: diff --git a/api/src/opentrons/hardware_control/protocols/motion_controller.py b/api/src/opentrons/hardware_control/protocols/motion_controller.py index 0fffb94d75a..8fbae1b9a1b 100644 --- a/api/src/opentrons/hardware_control/protocols/motion_controller.py +++ b/api/src/opentrons/hardware_control/protocols/motion_controller.py @@ -8,7 +8,7 @@ class MotionController(Protocol): """Protocol specifying fundamental motion controls.""" - async def halt(self) -> None: + async def halt(self, disengage_before_stopping: bool = False) -> None: """Immediately stop motion. Calls to stop through the synch adapter while other calls @@ -17,8 +17,9 @@ async def halt(self) -> None: smoothie. To provide actual immediate halting, call this method which does not require use of the loop. - After this call, the hardware will be in a bad state until a call to - stop + If disengage_before_stopping is True, the motors will disengage first and then + stop in place. Disengaging creates a smoother halt but requires homing after + in order to resume movement. """ ... diff --git a/api/src/opentrons/protocol_engine/create_protocol_engine.py b/api/src/opentrons/protocol_engine/create_protocol_engine.py index f4e70afc4e7..5139b9b5eba 100644 --- a/api/src/opentrons/protocol_engine/create_protocol_engine.py +++ b/api/src/opentrons/protocol_engine/create_protocol_engine.py @@ -10,6 +10,7 @@ from .protocol_engine import ProtocolEngine from .resources import DeckDataProvider, ModuleDataProvider from .state import Config, StateStore +from .types import PostRunHardwareState # TODO(mm, 2023-06-16): Arguably, this not being a context manager makes us prone to forgetting to @@ -44,7 +45,8 @@ async def create_protocol_engine( def create_protocol_engine_in_thread( hardware_api: HardwareControlAPI, config: Config, - drop_tips_and_home_after: bool, + drop_tips_after_run: bool, + post_run_hardware_state: PostRunHardwareState, ) -> typing.Generator[ typing.Tuple[ProtocolEngine, asyncio.AbstractEventLoop], None, None ]: @@ -66,7 +68,9 @@ def create_protocol_engine_in_thread( 3. Joins the thread. """ with async_context_manager_in_thread( - _protocol_engine(hardware_api, config, drop_tips_and_home_after) + _protocol_engine( + hardware_api, config, drop_tips_after_run, post_run_hardware_state + ) ) as ( protocol_engine, loop, @@ -78,7 +82,8 @@ def create_protocol_engine_in_thread( async def _protocol_engine( hardware_api: HardwareControlAPI, config: Config, - drop_tips_and_home_after: bool, + drop_tips_after_run: bool, + post_run_hardware_state: PostRunHardwareState, ) -> typing.AsyncGenerator[ProtocolEngine, None]: protocol_engine = await create_protocol_engine( hardware_api=hardware_api, @@ -88,4 +93,7 @@ async def _protocol_engine( protocol_engine.play() yield protocol_engine finally: - await protocol_engine.finish(drop_tips_and_home=drop_tips_and_home_after) + await protocol_engine.finish( + drop_tips_after_run=drop_tips_after_run, + post_run_hardware_state=post_run_hardware_state, + ) diff --git a/api/src/opentrons/protocol_engine/execution/hardware_stopper.py b/api/src/opentrons/protocol_engine/execution/hardware_stopper.py index 367182657b7..11f753b0ee4 100644 --- a/api/src/opentrons/protocol_engine/execution/hardware_stopper.py +++ b/api/src/opentrons/protocol_engine/execution/hardware_stopper.py @@ -7,7 +7,7 @@ from ..resources.ot3_validation import ensure_ot3_hardware from ..state import StateStore -from ..types import MotorAxis +from ..types import MotorAxis, PostRunHardwareState from ..errors import HardwareNotSupportedError from .movement import MovementHandler @@ -87,17 +87,27 @@ async def _drop_tip(self) -> None: # should not happen during an actual run log.debug(f"Pipette ID {pipette_id} no longer attached.") - async def do_halt(self) -> None: + async def do_halt(self, disengage_before_stopping: bool = False) -> None: """Issue a halt signal to the hardware API. After issuing a halt, you must call do_stop_and_recover after anything using the HardwareAPI has settled. """ - await self._hardware_api.halt() + await self._hardware_api.halt( + disengage_before_stopping=disengage_before_stopping + ) - async def do_stop_and_recover(self, drop_tips_and_home: bool = False) -> None: + async def do_stop_and_recover( + self, + post_run_hardware_state: PostRunHardwareState, + drop_tips_after_run: bool = False, + ) -> None: """Stop and reset the HardwareAPI, optionally dropping tips and homing.""" - if drop_tips_and_home: + if drop_tips_after_run: await self._drop_tip() - await self._hardware_api.stop(home_after=drop_tips_and_home) + home_after_stop = post_run_hardware_state in ( + PostRunHardwareState.HOME_AND_STAY_ENGAGED, + PostRunHardwareState.HOME_THEN_DISENGAGE, + ) + await self._hardware_api.stop(home_after=home_after_stop) diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index c50eb714c82..032325013f9 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -7,7 +7,6 @@ from opentrons.hardware_control import HardwareControlAPI from opentrons.hardware_control.modules import AbstractModule as HardwareModuleAPI from opentrons.hardware_control.types import PauseType as HardwarePauseType - from opentrons_shared_data.errors import ( ErrorCodes, EnumeratedError, @@ -24,6 +23,7 @@ ModuleModel, Liquid, HexColor, + PostRunHardwareState, ) from .execution import ( QueueWorker, @@ -306,8 +306,9 @@ async def wait_until_complete(self) -> None: async def finish( self, error: Optional[Exception] = None, - drop_tips_and_home: bool = True, + drop_tips_after_run: bool = True, set_run_status: bool = True, + post_run_hardware_state: PostRunHardwareState = PostRunHardwareState.HOME_AND_STAY_ENGAGED, ) -> None: """Gracefully finish using the ProtocolEngine, waiting for it to become idle. @@ -321,19 +322,23 @@ async def finish( Arguments: error: An error that caused the stop, if applicable. - drop_tips_and_home: Whether to home and drop tips as part of cleanup. + drop_tips_after_run: Whether to drop tips as part of cleanup. set_run_status: Whether to calculate a `success` or `failure` run status. If `False`, will set status to `stopped`. + post_run_hardware_state: The state in which to leave the gantry and motors in + after the run is over. """ if self._state_store.commands.state.stopped_by_estop: - drop_tips_and_home = False + drop_tips_after_run = False + post_run_hardware_state = PostRunHardwareState.DISENGAGE_IN_PLACE if error is None: error = EStopActivatedError(message="Estop was activated during a run") if error: if isinstance(error, EnumeratedError) and self._code_in_exception_stack( error=error, code=ErrorCodes.E_STOP_ACTIVATED ): - drop_tips_and_home = False + drop_tips_after_run = False + post_run_hardware_state = PostRunHardwareState.DISENGAGE_IN_PLACE error_details: Optional[FinishErrorDetails] = FinishErrorDetails( error_id=self._model_utils.generate_id(), @@ -357,13 +362,21 @@ async def finish( exit_stack.push_async_callback( # Cleanup after hardware halt and reset the hardware controller self._hardware_stopper.do_stop_and_recover, - drop_tips_and_home=drop_tips_and_home, + post_run_hardware_state=post_run_hardware_state, + drop_tips_after_run=drop_tips_after_run, ) exit_stack.callback(self._door_watcher.stop) - # Halt any movements immediately. Requires a hardware stop & reset after, in order to - # recover from halt and resume hardware operations. - exit_stack.push_async_callback(self._hardware_stopper.do_halt) + disengage_before_stopping = ( + False + if post_run_hardware_state == PostRunHardwareState.STAY_ENGAGED_IN_PLACE + else True + ) + # Halt any movements immediately + exit_stack.push_async_callback( + self._hardware_stopper.do_halt, + disengage_before_stopping=disengage_before_stopping, + ) exit_stack.push_async_callback(self._queue_worker.join) # First step. try: diff --git a/api/src/opentrons/protocol_engine/types.py b/api/src/opentrons/protocol_engine/types.py index 9287696049a..288e644fb16 100644 --- a/api/src/opentrons/protocol_engine/types.py +++ b/api/src/opentrons/protocol_engine/types.py @@ -629,3 +629,30 @@ class LabwareMovementStrategy(str, Enum): USING_GRIPPER = "usingGripper" MANUAL_MOVE_WITH_PAUSE = "manualMoveWithPause" MANUAL_MOVE_WITHOUT_PAUSE = "manualMoveWithoutPause" + + +class PostRunHardwareState(Enum): + """State of robot gantry & motors after a stop is performed and the hardware API is reset. + + HOME_AND_STAY_ENGAGED: home the gantry and keep all motors engaged. This allows the + robot to continue performing movement actions without re-homing + HOME_THEN_DISENGAGE: home the gantry and then disengage motors. + Reduces current consumption of the motors and prevents coil heating. + Re-homing is required to re-engage the motors and resume robot movement. + STAY_ENGAGED_IN_PLACE: do not home after the stop and keep the motors engaged. + Keeps gantry in the same position as prior to `stop()` execution + and allows the robot to execute movement commands without requiring to re-home first. + DISENGAGE_IN_PLACE: disengage motors and do not home the robot + Probable states for pipette: + - for 1- or 8-channel: + - HOME_AND_STAY_ENGAGED after protocol runs + - STAY_ENGAGED_IN_PLACE after maintenance runs + - for 96-channel: + - HOME_THEN_DISENGAGE after protocol runs + - DISENGAGE_IN_PLACE after maintenance runs + """ + + HOME_AND_STAY_ENGAGED = "homeAndStayEngaged" + HOME_THEN_DISENGAGE = "homeThenDisengage" + STAY_ENGAGED_IN_PLACE = "stayEngagedInPlace" + DISENGAGE_IN_PLACE = "disengageInPlace" diff --git a/api/src/opentrons/protocol_runner/protocol_runner.py b/api/src/opentrons/protocol_runner/protocol_runner.py index 9af858275d3..71c3b7623ce 100644 --- a/api/src/opentrons/protocol_runner/protocol_runner.py +++ b/api/src/opentrons/protocol_runner/protocol_runner.py @@ -35,6 +35,7 @@ LegacyExecutor, LegacyLoadInfo, ) +from ..protocol_engine.types import PostRunHardwareState class RunResult(NamedTuple): @@ -80,8 +81,9 @@ async def stop(self) -> None: await self._protocol_engine.stop() else: await self._protocol_engine.finish( - drop_tips_and_home=False, + drop_tips_after_run=False, set_run_status=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, ) @abstractmethod diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index c2b520ad727..b25eb9049f7 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -25,6 +25,8 @@ import pytest from decoy import Decoy +from opentrons.protocol_engine.types import PostRunHardwareState + try: import aionotify # type: ignore[import] except (OSError, ModuleNotFoundError): @@ -296,7 +298,8 @@ def _make_ot3_pe_ctx( use_virtual_gripper=True, block_on_door_open=False, ), - drop_tips_and_home_after=False, + drop_tips_after_run=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, ) as ( engine, loop, diff --git a/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py b/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py index 5030763c3cc..96e0cc3ea88 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py +++ b/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py @@ -15,7 +15,7 @@ TipHandler, HardwareStopper, ) -from opentrons.protocol_engine.types import MotorAxis, TipGeometry +from opentrons.protocol_engine.types import MotorAxis, TipGeometry, PostRunHardwareState if TYPE_CHECKING: from opentrons.hardware_control.ot3api import OT3API @@ -69,9 +69,18 @@ async def test_hardware_halt( """It should halt the hardware API.""" await subject.do_halt() - decoy.verify(await hardware_api.halt()) + decoy.verify(await hardware_api.halt(disengage_before_stopping=False)) +@pytest.mark.parametrize( + argnames=["post_run_hardware_state", "expected_home_after"], + argvalues=[ + (PostRunHardwareState.STAY_ENGAGED_IN_PLACE, False), + (PostRunHardwareState.DISENGAGE_IN_PLACE, False), + (PostRunHardwareState.HOME_AND_STAY_ENGAGED, True), + (PostRunHardwareState.HOME_THEN_DISENGAGE, True), + ], +) async def test_hardware_stopping_sequence( decoy: Decoy, state_store: StateStore, @@ -79,6 +88,8 @@ async def test_hardware_stopping_sequence( movement: MovementHandler, mock_tip_handler: TipHandler, subject: HardwareStopper, + post_run_hardware_state: PostRunHardwareState, + expected_home_after: bool, ) -> None: """It should stop the hardware, home the robot and perform drop tip if required.""" decoy.when(state_store.pipettes.get_all_attached_tips()).then_return( @@ -87,7 +98,9 @@ async def test_hardware_stopping_sequence( ] ) - await subject.do_stop_and_recover(drop_tips_and_home=True) + await subject.do_stop_and_recover( + drop_tips_after_run=True, post_run_hardware_state=post_run_hardware_state + ) decoy.verify( await hardware_api.stop(home_after=False), @@ -104,7 +117,7 @@ async def test_hardware_stopping_sequence( well_name="A1", ), await mock_tip_handler.drop_tip(pipette_id="pipette-id", home_after=False), - await hardware_api.stop(home_after=True), + await hardware_api.stop(home_after=expected_home_after), ) @@ -117,14 +130,17 @@ async def test_hardware_stopping_sequence_without_pipette_tips( """Don't drop tip when there aren't any tips attached to pipettes.""" decoy.when(state_store.pipettes.get_all_attached_tips()).then_return([]) - await subject.do_stop_and_recover(drop_tips_and_home=True) + await subject.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ) decoy.verify( await hardware_api.stop(home_after=True), ) -async def test_hardware_stopping_sequence_no_home( +async def test_hardware_stopping_sequence_no_tip_drop( decoy: Decoy, state_store: StateStore, hardware_api: HardwareAPI, @@ -138,7 +154,10 @@ async def test_hardware_stopping_sequence_no_home( ] ) - await subject.do_stop_and_recover(drop_tips_and_home=False) + await subject.do_stop_and_recover( + drop_tips_after_run=False, + post_run_hardware_state=PostRunHardwareState.DISENGAGE_IN_PLACE, + ) decoy.verify(await hardware_api.stop(home_after=False), times=1) @@ -172,7 +191,10 @@ async def test_hardware_stopping_sequence_no_pipette( ), ).then_raise(HwPipetteNotAttachedError("oh no")) - await subject.do_stop_and_recover(drop_tips_and_home=True) + await subject.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ) decoy.verify( await hardware_api.stop(home_after=True), @@ -202,7 +224,10 @@ async def test_hardware_stopping_sequence_with_gripper( ) decoy.when(state_store.config.use_virtual_gripper).then_return(False) decoy.when(ot3_hardware_api.has_gripper()).then_return(True) - await subject.do_stop_and_recover(drop_tips_and_home=True) + await subject.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ) decoy.verify( await ot3_hardware_api.stop(home_after=False), diff --git a/api/tests/opentrons/protocol_engine/test_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_protocol_engine.py index 09709be4046..8ae068e7480 100644 --- a/api/tests/opentrons/protocol_engine/test_protocol_engine.py +++ b/api/tests/opentrons/protocol_engine/test_protocol_engine.py @@ -27,6 +27,7 @@ ModuleDefinition, ModuleModel, Liquid, + PostRunHardwareState, ) from opentrons.protocol_engine.execution import ( QueueWorker, @@ -408,8 +409,17 @@ def test_pause( ) -@pytest.mark.parametrize("drop_tips_and_home", [True, False]) +@pytest.mark.parametrize("drop_tips_after_run", [True, False]) @pytest.mark.parametrize("set_run_status", [True, False]) +@pytest.mark.parametrize( + argnames=["post_run_hardware_state", "expected_halt_disengage"], + argvalues=[ + (PostRunHardwareState.HOME_AND_STAY_ENGAGED, True), + (PostRunHardwareState.HOME_THEN_DISENGAGE, True), + (PostRunHardwareState.STAY_ENGAGED_IN_PLACE, False), + (PostRunHardwareState.DISENGAGE_IN_PLACE, True), + ], +) async def test_finish( decoy: Decoy, action_dispatcher: ActionDispatcher, @@ -417,8 +427,10 @@ async def test_finish( queue_worker: QueueWorker, subject: ProtocolEngine, hardware_stopper: HardwareStopper, - drop_tips_and_home: bool, + drop_tips_after_run: bool, set_run_status: bool, + post_run_hardware_state: PostRunHardwareState, + expected_halt_disengage: bool, model_utils: ModelUtils, state_store: StateStore, door_watcher: DoorWatcher, @@ -430,16 +442,21 @@ async def test_finish( decoy.when(state_store.commands.state.stopped_by_estop).then_return(False) await subject.finish( - drop_tips_and_home=drop_tips_and_home, + drop_tips_after_run=drop_tips_after_run, set_run_status=set_run_status, + post_run_hardware_state=post_run_hardware_state, ) decoy.verify( action_dispatcher.dispatch(FinishAction(set_run_status=set_run_status)), await queue_worker.join(), + await hardware_stopper.do_halt( + disengage_before_stopping=expected_halt_disengage + ), door_watcher.stop(), await hardware_stopper.do_stop_and_recover( - drop_tips_and_home=drop_tips_and_home + drop_tips_after_run=drop_tips_after_run, + post_run_hardware_state=post_run_hardware_state, ), await plugin_starter.stop(), action_dispatcher.dispatch( @@ -461,11 +478,21 @@ async def test_finish_with_defaults( decoy.verify( action_dispatcher.dispatch(FinishAction(set_run_status=True)), - await hardware_stopper.do_stop_and_recover(drop_tips_and_home=True), + await hardware_stopper.do_halt(disengage_before_stopping=True), + await hardware_stopper.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ), ) -@pytest.mark.parametrize("stopped_by_estop", [True, False]) +@pytest.mark.parametrize( + argnames=["stopped_by_estop", "expected_drop_tips", "expected_end_state"], + argvalues=[ + (True, False, PostRunHardwareState.DISENGAGE_IN_PLACE), + (False, True, PostRunHardwareState.HOME_AND_STAY_ENGAGED), + ], +) async def test_finish_with_error( decoy: Decoy, action_dispatcher: ActionDispatcher, @@ -477,6 +504,8 @@ async def test_finish_with_error( door_watcher: DoorWatcher, state_store: StateStore, stopped_by_estop: bool, + expected_drop_tips: bool, + expected_end_state: PostRunHardwareState, ) -> None: """It should be able to tell the engine it's finished because of an error.""" error = RuntimeError("oh no") @@ -501,9 +530,11 @@ async def test_finish_with_error( FinishAction(error_details=expected_error_details, set_run_status=True) ), await queue_worker.join(), + await hardware_stopper.do_halt(disengage_before_stopping=True), door_watcher.stop(), await hardware_stopper.do_stop_and_recover( - drop_tips_and_home=not stopped_by_estop + drop_tips_after_run=expected_drop_tips, + post_run_hardware_state=expected_end_state, ), await plugin_starter.stop(), action_dispatcher.dispatch( @@ -551,8 +582,12 @@ async def test_finish_with_estop_error_will_not_drop_tip_and_home( FinishAction(error_details=expected_error_details, set_run_status=True) ), await queue_worker.join(), + await hardware_stopper.do_halt(disengage_before_stopping=True), door_watcher.stop(), - await hardware_stopper.do_stop_and_recover(drop_tips_and_home=False), + await hardware_stopper.do_stop_and_recover( + drop_tips_after_run=False, + post_run_hardware_state=PostRunHardwareState.DISENGAGE_IN_PLACE, + ), await plugin_starter.stop(), action_dispatcher.dispatch( HardwareStoppedAction( @@ -594,9 +629,12 @@ async def test_finish_stops_hardware_if_queue_worker_join_fails( action_dispatcher.dispatch(FinishAction()), # await queue_worker.join() should be called, and should raise, here. # We can't verify that step in the sequence here because of a Decoy limitation. - await hardware_stopper.do_halt(), + await hardware_stopper.do_halt(disengage_before_stopping=True), door_watcher.stop(), - await hardware_stopper.do_stop_and_recover(drop_tips_and_home=True), + await hardware_stopper.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ), await plugin_starter.stop(), action_dispatcher.dispatch( HardwareStoppedAction( diff --git a/api/tests/opentrons/protocol_runner/test_protocol_runner.py b/api/tests/opentrons/protocol_runner/test_protocol_runner.py index 42f57270d1a..aa9e36d5740 100644 --- a/api/tests/opentrons/protocol_runner/test_protocol_runner.py +++ b/api/tests/opentrons/protocol_runner/test_protocol_runner.py @@ -11,6 +11,7 @@ from opentrons.broker import Broker from opentrons.equipment_broker import EquipmentBroker from opentrons.hardware_control import API as HardwareAPI +from opentrons.protocol_engine.types import PostRunHardwareState from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.parse import PythonParseMode from opentrons_shared_data.protocol.models import ProtocolSchemaV6, ProtocolSchemaV7 @@ -275,7 +276,11 @@ async def test_stop_when_run_never_started( await subject.stop() decoy.verify( - await protocol_engine.finish(drop_tips_and_home=False, set_run_status=False), + await protocol_engine.finish( + drop_tips_after_run=False, + set_run_status=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, + ), times=1, ) diff --git a/robot-server/robot_server/maintenance_runs/maintenance_engine_store.py b/robot-server/robot_server/maintenance_runs/maintenance_engine_store.py index bae17259f73..42081dbf459 100644 --- a/robot-server/robot_server/maintenance_runs/maintenance_engine_store.py +++ b/robot-server/robot_server/maintenance_runs/maintenance_engine_store.py @@ -2,6 +2,7 @@ from datetime import datetime from typing import List, NamedTuple, Optional +from opentrons.protocol_engine.types import PostRunHardwareState from opentrons_shared_data.robot.dev_types import RobotType from opentrons.config import feature_flags @@ -171,7 +172,11 @@ async def clear(self) -> RunResult: state_view = engine.state_view if state_view.commands.get_is_okay_to_clear(): - await engine.finish(drop_tips_and_home=False, set_run_status=False) + await engine.finish( + drop_tips_after_run=False, + set_run_status=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, + ) else: raise EngineConflictError("Current run is not idle or stopped.") diff --git a/robot-server/robot_server/runs/engine_store.py b/robot-server/robot_server/runs/engine_store.py index 497e1e91a2f..40fd5d343ca 100644 --- a/robot-server/robot_server/runs/engine_store.py +++ b/robot-server/robot_server/runs/engine_store.py @@ -1,6 +1,7 @@ """In-memory storage of ProtocolEngine instances.""" from typing import List, NamedTuple, Optional +from opentrons.protocol_engine.types import PostRunHardwareState from opentrons_shared_data.robot.dev_types import RobotType from opentrons.config import feature_flags @@ -219,7 +220,11 @@ async def clear(self) -> RunResult: state_view = engine.state_view if state_view.commands.get_is_okay_to_clear(): - await engine.finish(drop_tips_and_home=False, set_run_status=False) + await engine.finish( + drop_tips_after_run=False, + set_run_status=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, + ) else: raise EngineConflictError("Current run is not idle or stopped.")