From 3d572631faec8235af2cf90cd5e7f74004f0a853 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 20 Mar 2024 09:58:33 -0400 Subject: [PATCH 01/24] Split UpdateCommandAction into RunCommandAction and SucceedCommandAction. --- .../protocol_engine/actions/__init__.py | 6 ++-- .../protocol_engine/actions/actions.py | 28 ++++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/protocol_engine/actions/__init__.py b/api/src/opentrons/protocol_engine/actions/__init__.py index b7875021cf3..b1181e6a50e 100644 --- a/api/src/opentrons/protocol_engine/actions/__init__.py +++ b/api/src/opentrons/protocol_engine/actions/__init__.py @@ -14,7 +14,8 @@ FinishAction, HardwareStoppedAction, QueueCommandAction, - UpdateCommandAction, + RunCommandAction, + SucceedCommandAction, FailCommandAction, AddLabwareOffsetAction, AddLabwareDefinitionAction, @@ -40,7 +41,8 @@ "FinishAction", "HardwareStoppedAction", "QueueCommandAction", - "UpdateCommandAction", + "RunCommandAction", + "SucceedCommandAction", "FailCommandAction", "AddLabwareOffsetAction", "AddLabwareDefinitionAction", diff --git a/api/src/opentrons/protocol_engine/actions/actions.py b/api/src/opentrons/protocol_engine/actions/actions.py index 04e5f1999ce..287cbabe885 100644 --- a/api/src/opentrons/protocol_engine/actions/actions.py +++ b/api/src/opentrons/protocol_engine/actions/actions.py @@ -121,16 +121,35 @@ class QueueCommandAction: @dataclass(frozen=True) -class UpdateCommandAction: - """Update a given command.""" +class RunCommandAction: + """Mark a given command as running. + + The command must be queued immediately prior to this action. + """ + + command_id: str + started_at: datetime + + +@dataclass(frozen=True) +class SucceedCommandAction: + """Mark a given command as succeeded. + + The command must be running immediately prior to this action. + """ command: Command + """The command in its new succeeded state.""" + private_result: CommandPrivateResult @dataclass(frozen=True) class FailCommandAction: - """Mark a given command as failed.""" + """Mark a given command as failed. + + The command must be running immediately prior to this action. + """ command_id: str error_id: str @@ -211,7 +230,8 @@ class SetPipetteMovementSpeedAction: HardwareStoppedAction, DoorChangeAction, QueueCommandAction, - UpdateCommandAction, + RunCommandAction, + SucceedCommandAction, FailCommandAction, AddLabwareOffsetAction, AddLabwareDefinitionAction, From 43f50d38e355869d350335600151f162ec5ef0bb Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 20 Mar 2024 17:15:53 -0400 Subject: [PATCH 02/24] Update CommandStore. --- .../protocol_engine/state/commands.py | 73 +++-- .../state/test_command_store.py | 257 ++++++++++-------- 2 files changed, 186 insertions(+), 144 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 90b3ea210f6..656dbb90a2d 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -13,13 +13,16 @@ from opentrons.ordered_set import OrderedSet from opentrons.hardware_control.types import DoorState -from opentrons.protocol_engine.actions.actions import ResumeFromRecoveryAction +from opentrons.protocol_engine.actions.actions import ( + ResumeFromRecoveryAction, + RunCommandAction, +) from opentrons.protocol_engine.error_recovery_policy import ErrorRecoveryType from ..actions import ( Action, QueueCommandAction, - UpdateCommandAction, + SucceedCommandAction, FailCommandAction, PlayAction, PauseAction, @@ -261,41 +264,55 @@ def handle_action(self, action: Action) -> None: # noqa: C901 if action.request_hash is not None: self._state.latest_command_hash = action.request_hash - # TODO(mc, 2021-12-28): replace "UpdateCommandAction" with explicit - # state change actions (e.g. RunCommandAction, SucceedCommandAction) - # to make a command's queue transition logic easier to follow - elif isinstance(action, UpdateCommandAction): - command = action.command - prev_entry = self._state.commands_by_id.get(command.id) - - if prev_entry is None: - index = len(self._state.all_command_ids) - self._state.all_command_ids.append(command.id) - self._state.commands_by_id[command.id] = CommandEntry( - index=index, - command=command, - ) - else: - self._state.commands_by_id[command.id] = CommandEntry( - index=prev_entry.index, - command=command, - ) + elif isinstance(action, RunCommandAction): + prev_entry = self._state.commands_by_id[action.command_id] + assert prev_entry.command.status == CommandStatus.QUEUED - self._state.queued_command_ids.discard(command.id) - self._state.queued_setup_command_ids.discard(command.id) + running_command = prev_entry.command.copy( + update={ + "status": CommandStatus.RUNNING, + "startedAt": action.started_at, + } + ) - if command.status == CommandStatus.RUNNING: - self._state.running_command_id = command.id - elif self._state.running_command_id == command.id: - self._state.running_command_id = None + self._state.commands_by_id[action.command_id] = CommandEntry( + index=prev_entry.index, command=running_command + ) + + assert self._state.running_command_id is None + self._state.running_command_id = action.command_id + + self._state.queued_command_ids.discard(action.command_id) + self._state.queued_setup_command_ids.discard(action.command_id) + + elif isinstance(action, SucceedCommandAction): + prev_entry = self._state.commands_by_id[action.command.id] + assert prev_entry.command.status == CommandStatus.RUNNING + + succeeded_command = action.command + assert succeeded_command.status == CommandStatus.SUCCEEDED + + self._state.commands_by_id[action.command.id] = CommandEntry( + index=prev_entry.index, + command=succeeded_command, + ) + + assert self._state.running_command_id == action.command.id + self._state.running_command_id = None + + self._state.queued_command_ids.discard(succeeded_command.id) + self._state.queued_setup_command_ids.discard(succeeded_command.id) elif isinstance(action, FailCommandAction): + prev_entry = self._state.commands_by_id[action.command_id] + assert prev_entry.command.status == CommandStatus.RUNNING + error_occurrence = ErrorOccurrence.from_failed( id=action.error_id, createdAt=action.failed_at, error=action.error, ) - prev_entry = self._state.commands_by_id[action.command_id] + # TODO(mc, 2022-06-06): add new "cancelled" status or similar self._update_to_failed( command_id=action.command_id, diff --git a/api/tests/opentrons/protocol_engine/state/test_command_store.py b/api/tests/opentrons/protocol_engine/state/test_command_store.py index d9b6305a414..e2aedc9ebe4 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_store.py @@ -8,6 +8,7 @@ from opentrons_shared_data.pipette.dev_types import PipetteNameType from opentrons.ordered_set import OrderedSet +from opentrons.protocol_engine.actions.actions import RunCommandAction from opentrons.types import MountType, DeckSlotName from opentrons.hardware_control.types import DoorState @@ -25,7 +26,7 @@ from opentrons.protocol_engine.actions import ( QueueCommandAction, - UpdateCommandAction, + SucceedCommandAction, FailCommandAction, PlayAction, PauseAction, @@ -37,12 +38,7 @@ DoorChangeAction, ) -from .command_fixtures import ( - create_queued_command, - create_running_command, - create_succeeded_command, - create_failed_command, -) +from .command_fixtures import create_succeeded_command def _make_config(block_on_door_open: bool = False) -> Config: @@ -271,7 +267,7 @@ def test_command_queue_with_hash() -> None: def test_command_queue_and_unqueue() -> None: - """It should queue on QueueCommandAction and dequeue on UpdateCommandAction.""" + """It should queue on QueueCommandAction and dequeue on RunCommandAction.""" queue_1 = QueueCommandAction( request=commands.WaitForResumeCreate(params=commands.WaitForResumeParams()), request_hash=None, @@ -284,13 +280,17 @@ def test_command_queue_and_unqueue() -> None: created_at=datetime(year=2022, month=2, day=2), command_id="command-id-2", ) - update_1 = UpdateCommandAction( - private_result=None, - command=create_running_command(command_id="command-id-1"), + run_1 = RunCommandAction( + command_id="command-id-1", + started_at=datetime(year=2021, month=1, day=1), ) - update_2 = UpdateCommandAction( + run_2 = RunCommandAction( + command_id="command-id-2", + started_at=datetime(year=2022, month=2, day=2), + ) + succeed_2 = SucceedCommandAction( private_result=None, - command=create_running_command(command_id="command-id-2"), + command=create_succeeded_command(command_id="command-id-2"), ) subject = CommandStore(is_door_open=False, config=_make_config()) @@ -303,10 +303,11 @@ def test_command_queue_and_unqueue() -> None: ["command-id-1", "command-id-2"] ) - subject.handle_action(update_2) + subject.handle_action(run_2) assert subject.state.queued_command_ids == OrderedSet(["command-id-1"]) - subject.handle_action(update_1) + subject.handle_action(succeed_2) + subject.handle_action(run_1) assert subject.state.queued_command_ids == OrderedSet() @@ -330,13 +331,15 @@ def test_setup_command_queue_and_unqueue() -> None: created_at=datetime(year=2022, month=2, day=2), command_id="command-id-2", ) - update_1 = UpdateCommandAction( - private_result=None, - command=create_running_command(command_id="command-id-1"), + run_1 = RunCommandAction( + command_id="command-id-1", started_at=datetime(year=2021, month=1, day=1) ) - update_2 = UpdateCommandAction( + run_2 = RunCommandAction( + command_id="command-id-2", started_at=datetime(year=2022, month=2, day=2) + ) + succeed_2 = SucceedCommandAction( private_result=None, - command=create_running_command(command_id="command-id-2"), + command=create_succeeded_command(command_id="command-id-2"), ) subject = CommandStore(is_door_open=False, config=_make_config()) @@ -349,10 +352,11 @@ def test_setup_command_queue_and_unqueue() -> None: ["command-id-1", "command-id-2"] ) - subject.handle_action(update_2) + subject.handle_action(run_2) assert subject.state.queued_setup_command_ids == OrderedSet(["command-id-1"]) - subject.handle_action(update_1) + subject.handle_action(succeed_2) + subject.handle_action(run_1) assert subject.state.queued_setup_command_ids == OrderedSet() @@ -394,11 +398,11 @@ def test_running_command_id() -> None: created_at=datetime(year=2021, month=1, day=1), command_id="command-id-1", ) - running_update = UpdateCommandAction( - private_result=None, - command=create_running_command(command_id="command-id-1"), + run = RunCommandAction( + command_id="command-id-1", + started_at=datetime(year=2021, month=1, day=1), ) - completed_update = UpdateCommandAction( + succeed = SucceedCommandAction( private_result=None, command=create_succeeded_command(command_id="command-id-1"), ) @@ -408,32 +412,10 @@ def test_running_command_id() -> None: subject.handle_action(queue) assert subject.state.running_command_id is None - subject.handle_action(running_update) - assert subject.state.running_command_id == "command-id-1" - - subject.handle_action(completed_update) - assert subject.state.running_command_id is None - - -def test_running_command_no_queue() -> None: - """It should add a running command to state, even if there was no queue action.""" - running_update = UpdateCommandAction( - private_result=None, - command=create_running_command(command_id="command-id-1"), - ) - completed_update = UpdateCommandAction( - private_result=None, - command=create_succeeded_command(command_id="command-id-1"), - ) - - subject = CommandStore(is_door_open=False, config=_make_config()) - - subject.handle_action(running_update) - assert subject.state.all_command_ids == ["command-id-1"] + subject.handle_action(run) assert subject.state.running_command_id == "command-id-1" - subject.handle_action(completed_update) - assert subject.state.all_command_ids == ["command-id-1"] + subject.handle_action(succeed) assert subject.state.running_command_id is None @@ -455,16 +437,9 @@ def test_command_failure_clears_queues() -> None: created_at=datetime(year=2021, month=1, day=1), command_id="command-id-2", ) - running_1 = UpdateCommandAction( - private_result=None, - command=commands.WaitForResume( - id="command-id-1", - key="command-key-1", - createdAt=datetime(year=2021, month=1, day=1), - startedAt=datetime(year=2022, month=2, day=2), - params=commands.WaitForResumeParams(), - status=commands.CommandStatus.RUNNING, - ), + run_1 = RunCommandAction( + command_id="command-id-1", + started_at=datetime(year=2022, month=2, day=2), ) fail_1 = FailCommandAction( command_id="command-id-1", @@ -504,7 +479,7 @@ def test_command_failure_clears_queues() -> None: subject.handle_action(queue_1) subject.handle_action(queue_2) - subject.handle_action(running_1) + subject.handle_action(run_1) subject.handle_action(fail_1) assert subject.state.running_command_id is None @@ -558,17 +533,9 @@ def test_setup_command_failure_only_clears_setup_command_queue() -> None: command_id="command-id-3", ) - running_cmd_2 = UpdateCommandAction( - private_result=None, - command=commands.WaitForResume( - id="command-id-2", - key="command-key-2", - createdAt=datetime(year=2021, month=1, day=1), - startedAt=datetime(year=2022, month=2, day=2), - params=commands.WaitForResumeParams(), - status=commands.CommandStatus.RUNNING, - intent=commands.CommandIntent.SETUP, - ), + run_action_cmd_2 = RunCommandAction( + command_id="command-id-2", + started_at=datetime(year=2022, month=2, day=2), ) failed_action_cmd_2 = FailCommandAction( command_id="command-id-2", @@ -610,7 +577,7 @@ def test_setup_command_failure_only_clears_setup_command_queue() -> None: subject.handle_action(queue_action_1_non_setup) subject.handle_action(queue_action_2_setup) subject.handle_action(queue_action_3_setup) - subject.handle_action(running_cmd_2) + subject.handle_action(run_action_cmd_2) subject.handle_action(failed_action_cmd_2) assert subject.state.running_command_id is None @@ -650,16 +617,9 @@ def test_nonfatal_command_failure() -> None: created_at=datetime(year=2021, month=1, day=1), command_id="command-id-2", ) - run_1 = UpdateCommandAction( - private_result=None, - command=commands.WaitForResume( - id="command-id-1", - key="command-key-1", - createdAt=datetime(year=2021, month=1, day=1), - startedAt=datetime(year=2022, month=2, day=2), - params=commands.WaitForResumeParams(), - status=commands.CommandStatus.RUNNING, - ), + run_1 = RunCommandAction( + command_id="command-id-1", + started_at=datetime(year=2022, month=2, day=2), ) fail_1 = FailCommandAction( command_id="command-id-1", @@ -712,34 +672,74 @@ def test_nonfatal_command_failure() -> None: } -def test_command_store_preserves_handle_order() -> None: - """It should store commands in the order they are handled.""" - # Any arbitrary 3 commands that compare non-equal (!=) to each other. - command_a = create_queued_command(command_id="command-id-1") - command_b = create_running_command(command_id="command-id-2") - command_c = create_succeeded_command(command_id="command-id-1") +def test_command_store_keeps_commands_in_queue_order() -> None: + """It should keep commands in the order they were originally enqueued.""" + command_create_1_non_setup = commands.CommentCreate( + params=commands.CommentParams(message="hello world"), + ) + command_create_2_setup = commands.CommentCreate( + params=commands.CommentParams(message="hello world"), + intent=commands.CommandIntent.SETUP, + ) + command_create_3_non_setup = commands.CommentCreate( + params=commands.CommentParams(message="hello world"), + ) subject = CommandStore(is_door_open=False, config=_make_config()) - subject.handle_action(UpdateCommandAction(private_result=None, command=command_a)) + subject.handle_action( + QueueCommandAction( + "command-id-1", + created_at=datetime(year=2021, month=1, day=1), + request=command_create_1_non_setup, + request_hash=None, + ) + ) assert subject.state.all_command_ids == ["command-id-1"] - assert subject.state.commands_by_id == { - "command-id-1": CommandEntry(index=0, command=command_a), - } - subject.handle_action(UpdateCommandAction(private_result=None, command=command_b)) + subject.handle_action( + QueueCommandAction( + "command-id-2", + created_at=datetime(year=2021, month=1, day=1), + request=command_create_2_setup, + request_hash=None, + ) + ) assert subject.state.all_command_ids == ["command-id-1", "command-id-2"] - assert subject.state.commands_by_id == { - "command-id-1": CommandEntry(index=0, command=command_a), - "command-id-2": CommandEntry(index=1, command=command_b), - } - subject.handle_action(UpdateCommandAction(private_result=None, command=command_c)) - assert subject.state.all_command_ids == ["command-id-1", "command-id-2"] - assert subject.state.commands_by_id == { - "command-id-1": CommandEntry(index=0, command=command_c), - "command-id-2": CommandEntry(index=1, command=command_b), - } + subject.handle_action( + QueueCommandAction( + "command-id-3", + created_at=datetime(year=2021, month=1, day=1), + request=command_create_3_non_setup, + request_hash=None, + ) + ) + assert subject.state.all_command_ids == [ + "command-id-1", + "command-id-2", + "command-id-3", + ] + + # Running and completing commands shouldn't affect the command order. + subject.handle_action( + RunCommandAction( + command_id="command-id-2", started_at=datetime(year=2021, month=1, day=1) + ) + ) + subject.handle_action( + SucceedCommandAction( + command=create_succeeded_command( + command_id="command-id-2", + ), + private_result=None, + ) + ) + assert subject.state.all_command_ids == [ + "command-id-1", + "command-id-2", + "command-id-3", + ] @pytest.mark.parametrize("pause_source", PauseSource) @@ -1155,29 +1155,52 @@ def test_command_store_ignores_finish_after_non_graceful_stop() -> None: def test_command_store_handles_command_failed() -> None: """It should store an error and mark the command if it fails.""" - command = create_running_command(command_id="command-id") - expected_error_occurrence = errors.ErrorOccurrence( id="error-id", errorType="ProtocolEngineError", - createdAt=datetime(year=2022, month=2, day=2), + createdAt=datetime(year=2023, month=3, day=3), detail="oh no", errorCode=ErrorCodes.GENERAL_ERROR.value.code, ) - expected_failed_command = create_failed_command( - command_id="command-id", + expected_failed_command = commands.Comment( + id="command-id", + commandType="comment", + key="command-key", + createdAt=datetime(year=2021, month=1, day=1), + startedAt=datetime(year=2022, month=2, day=2), + completedAt=expected_error_occurrence.createdAt, + status=commands.CommandStatus.FAILED, + params=commands.CommentParams(message="hello, world"), + result=None, error=expected_error_occurrence, - completed_at=datetime(year=2022, month=2, day=2), + notes=None, ) subject = CommandStore(is_door_open=False, config=_make_config()) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + + subject.handle_action( + QueueCommandAction( + command_id=expected_failed_command.id, + created_at=expected_failed_command.createdAt, + request=commands.CommentCreate( + params=expected_failed_command.params, key=expected_failed_command.key + ), + request_hash=None, + ) + ) + subject.handle_action( + RunCommandAction( + command_id=expected_failed_command.id, + # Ignore arg-type errors because we know this isn't None. + started_at=expected_failed_command.startedAt, # type: ignore[arg-type] + ) + ) subject.handle_action( FailCommandAction( - command_id="command-id", - error_id="error-id", - failed_at=datetime(year=2022, month=2, day=2), + command_id=expected_failed_command.id, + error_id=expected_error_occurrence.id, + failed_at=expected_error_occurrence.createdAt, error=errors.ProtocolEngineError(message="oh no"), type=ErrorRecoveryType.FAIL_RUN, ) @@ -1189,11 +1212,13 @@ def test_command_store_handles_command_failed() -> None: run_completed_at=None, is_door_blocking=False, running_command_id=None, - all_command_ids=["command-id"], + all_command_ids=[expected_failed_command.id], queued_command_ids=OrderedSet(), queued_setup_command_ids=OrderedSet(), commands_by_id={ - "command-id": CommandEntry(index=0, command=expected_failed_command), + expected_failed_command.id: CommandEntry( + index=0, command=expected_failed_command + ), }, run_error=None, finish_error=None, From fad24160ad821d5933ff9ad97aff91a9928de8a9 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 20 Mar 2024 11:19:26 -0400 Subject: [PATCH 03/24] Trivially update remaining state stores. These are the easy ones, since they only care about command completions. --- .../state/addressable_areas.py | 4 +- .../protocol_engine/state/labware.py | 4 +- .../protocol_engine/state/modules.py | 4 +- .../protocol_engine/state/pipettes.py | 4 +- .../opentrons/protocol_engine/state/tips.py | 4 +- .../state/test_addressable_area_store.py | 6 +- .../state/test_labware_store.py | 12 +-- .../state/test_module_store.py | 44 +++++----- .../state/test_pipette_store.py | 80 ++++++++--------- .../protocol_engine/state/test_tip_state.py | 85 +++++++++---------- 10 files changed, 123 insertions(+), 124 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/addressable_areas.py b/api/src/opentrons/protocol_engine/state/addressable_areas.py index add8fa35dc1..04894fe3338 100644 --- a/api/src/opentrons/protocol_engine/state/addressable_areas.py +++ b/api/src/opentrons/protocol_engine/state/addressable_areas.py @@ -35,7 +35,7 @@ DeckConfigurationType, Dimensions, ) -from ..actions import Action, UpdateCommandAction, PlayAction, AddAddressableAreaAction +from ..actions import Action, SucceedCommandAction, PlayAction, AddAddressableAreaAction from .config import Config from .abstract_store import HasState, HandlesActions @@ -182,7 +182,7 @@ def __init__( def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" - if isinstance(action, UpdateCommandAction): + if isinstance(action, SucceedCommandAction): self._handle_command(action.command) elif isinstance(action, AddAddressableAreaAction): self._check_location_is_addressable_area(action.addressable_area) diff --git a/api/src/opentrons/protocol_engine/state/labware.py b/api/src/opentrons/protocol_engine/state/labware.py index c8792f516b0..7709410fd0f 100644 --- a/api/src/opentrons/protocol_engine/state/labware.py +++ b/api/src/opentrons/protocol_engine/state/labware.py @@ -52,7 +52,7 @@ ) from ..actions import ( Action, - UpdateCommandAction, + SucceedCommandAction, AddLabwareOffsetAction, AddLabwareDefinitionAction, ) @@ -152,7 +152,7 @@ def __init__( def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" - if isinstance(action, UpdateCommandAction): + if isinstance(action, SucceedCommandAction): self._handle_command(action.command) elif isinstance(action, AddLabwareOffsetAction): diff --git a/api/src/opentrons/protocol_engine/state/modules.py b/api/src/opentrons/protocol_engine/state/modules.py index 7a01b824315..84093de0d4a 100644 --- a/api/src/opentrons/protocol_engine/state/modules.py +++ b/api/src/opentrons/protocol_engine/state/modules.py @@ -54,7 +54,7 @@ temperature_module, thermocycler, ) -from ..actions import Action, UpdateCommandAction, AddModuleAction +from ..actions import Action, SucceedCommandAction, AddModuleAction from .abstract_store import HasState, HandlesActions from .module_substates import ( MagneticModuleSubState, @@ -195,7 +195,7 @@ def __init__( def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" - if isinstance(action, UpdateCommandAction): + if isinstance(action, SucceedCommandAction): self._handle_command(action.command) elif isinstance(action, AddModuleAction): diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 26c71a6884f..6803d19272b 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -56,7 +56,7 @@ from ..actions import ( Action, SetPipetteMovementSpeedAction, - UpdateCommandAction, + SucceedCommandAction, ) from .abstract_store import HasState, HandlesActions @@ -150,7 +150,7 @@ def __init__(self) -> None: def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" - if isinstance(action, UpdateCommandAction): + if isinstance(action, SucceedCommandAction): self._handle_command(action.command, action.private_result) elif isinstance(action, SetPipetteMovementSpeedAction): self._state.movement_speed_by_id[action.pipette_id] = action.speed diff --git a/api/src/opentrons/protocol_engine/state/tips.py b/api/src/opentrons/protocol_engine/state/tips.py index 67598c32bba..a2539ff45e7 100644 --- a/api/src/opentrons/protocol_engine/state/tips.py +++ b/api/src/opentrons/protocol_engine/state/tips.py @@ -6,7 +6,7 @@ from .abstract_store import HasState, HandlesActions from ..actions import ( Action, - UpdateCommandAction, + SucceedCommandAction, ResetTipsAction, ) from ..commands import ( @@ -64,7 +64,7 @@ def __init__(self) -> None: def handle_action(self, action: Action) -> None: """Modify state in reaction to an action.""" - if isinstance(action, UpdateCommandAction): + if isinstance(action, SucceedCommandAction): if isinstance(action.private_result, PipetteConfigUpdateResultMixin): pipette_id = action.private_result.pipette_id config = action.private_result.config diff --git a/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py b/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py index f49cd164c1f..63e9cea2925 100644 --- a/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_addressable_area_store.py @@ -8,7 +8,7 @@ from opentrons.protocol_engine.commands import Command from opentrons.protocol_engine.actions import ( - UpdateCommandAction, + SucceedCommandAction, AddAddressableAreaAction, ) from opentrons.protocol_engine.state import Config @@ -178,7 +178,7 @@ def test_addressable_area_referencing_commands_load_on_simulated_deck( ) -> None: """It should check and store the addressable area when referenced in a command.""" simulated_subject.handle_action( - UpdateCommandAction(private_result=None, command=command) + SucceedCommandAction(private_result=None, command=command) ) assert expected_area in simulated_subject.state.loaded_addressable_areas_by_name @@ -244,7 +244,7 @@ def test_addressable_area_referencing_commands_load( subject: AddressableAreaStore, ) -> None: """It should check that the addressable area is in the deck config.""" - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert expected_area in subject.state.loaded_addressable_areas_by_name diff --git a/api/tests/opentrons/protocol_engine/state/test_labware_store.py b/api/tests/opentrons/protocol_engine/state/test_labware_store.py index efe67422da0..2c0c8cdefd9 100644 --- a/api/tests/opentrons/protocol_engine/state/test_labware_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_labware_store.py @@ -21,7 +21,7 @@ from opentrons.protocol_engine.actions import ( AddLabwareOffsetAction, AddLabwareDefinitionAction, - UpdateCommandAction, + SucceedCommandAction, ) from opentrons.protocol_engine.state.labware import LabwareStore, LabwareState @@ -125,7 +125,7 @@ def test_handles_load_labware( created_at=datetime(year=2021, month=1, day=2), ) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert subject.state.labware_by_id["test-labware-id"] == expected_labware_data @@ -173,7 +173,7 @@ def test_handles_move_labware( ) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_labware_command) + SucceedCommandAction(private_result=None, command=load_labware_command) ) move_command = create_move_labware_command( @@ -183,7 +183,7 @@ def test_handles_move_labware( strategy=LabwareMovementStrategy.MANUAL_MOVE_WITH_PAUSE, ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_command) + SucceedCommandAction(private_result=None, command=move_command) ) assert subject.state.labware_by_id["my-labware-id"].location == DeckSlotLocation( @@ -217,7 +217,7 @@ def test_handles_move_labware_off_deck( ) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_labware_command) + SucceedCommandAction(private_result=None, command=load_labware_command) ) move_labware_off_deck_cmd = create_move_labware_command( @@ -226,7 +226,7 @@ def test_handles_move_labware_off_deck( strategy=LabwareMovementStrategy.MANUAL_MOVE_WITH_PAUSE, ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_labware_off_deck_cmd) + SucceedCommandAction(private_result=None, command=move_labware_off_deck_cmd) ) assert subject.state.labware_by_id["my-labware-id"].location == OFF_DECK_LOCATION assert subject.state.labware_by_id["my-labware-id"].offsetId is None diff --git a/api/tests/opentrons/protocol_engine/state/test_module_store.py b/api/tests/opentrons/protocol_engine/state/test_module_store.py index 3608e720b83..1d0d7003496 100644 --- a/api/tests/opentrons/protocol_engine/state/test_module_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_module_store.py @@ -142,7 +142,7 @@ def test_load_module( expected_substate: ModuleSubStateType, ) -> None: """It should handle a successful LoadModule command.""" - action = actions.UpdateCommandAction( + action = actions.SucceedCommandAction( private_result=None, command=commands.LoadModule.construct( # type: ignore[call-arg] params=commands.LoadModuleParams( @@ -202,7 +202,7 @@ def test_load_thermocycler_in_thermocycler_slot( thermocycler_v2_def: ModuleDefinition, ) -> None: """It should update additional slots for thermocycler module.""" - action = actions.UpdateCommandAction( + action = actions.SucceedCommandAction( private_result=None, command=commands.LoadModule.construct( # type: ignore[call-arg] params=commands.LoadModuleParams( @@ -346,10 +346,10 @@ def test_handle_hs_temperature_commands(heater_shaker_v1_def: ModuleDefinition) subject = ModuleStore(_OT2_STANDARD_CONFIG) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_module_cmd) + actions.SucceedCommandAction(private_result=None, command=load_module_cmd) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=set_temp_cmd) + actions.SucceedCommandAction(private_result=None, command=set_temp_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -360,7 +360,7 @@ def test_handle_hs_temperature_commands(heater_shaker_v1_def: ModuleDefinition) ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=deactivate_cmd) + actions.SucceedCommandAction(private_result=None, command=deactivate_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -397,10 +397,10 @@ def test_handle_hs_shake_commands(heater_shaker_v1_def: ModuleDefinition) -> Non subject = ModuleStore(_OT2_STANDARD_CONFIG) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_module_cmd) + actions.SucceedCommandAction(private_result=None, command=load_module_cmd) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=set_shake_cmd) + actions.SucceedCommandAction(private_result=None, command=set_shake_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -411,7 +411,7 @@ def test_handle_hs_shake_commands(heater_shaker_v1_def: ModuleDefinition) -> Non ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=deactivate_cmd) + actions.SucceedCommandAction(private_result=None, command=deactivate_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -450,7 +450,7 @@ def test_handle_hs_labware_latch_commands( subject = ModuleStore(_OT2_STANDARD_CONFIG) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_module_cmd) + actions.SucceedCommandAction(private_result=None, command=load_module_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -462,7 +462,7 @@ def test_handle_hs_labware_latch_commands( } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=close_latch_cmd) + actions.SucceedCommandAction(private_result=None, command=close_latch_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -473,7 +473,7 @@ def test_handle_hs_labware_latch_commands( ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=open_latch_cmd) + actions.SucceedCommandAction(private_result=None, command=open_latch_cmd) ) assert subject.state.substate_by_module_id == { "module-id": HeaterShakerModuleSubState( @@ -514,10 +514,10 @@ def test_handle_tempdeck_temperature_commands( subject = ModuleStore(_OT2_STANDARD_CONFIG) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_module_cmd) + actions.SucceedCommandAction(private_result=None, command=load_module_cmd) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=set_temp_cmd) + actions.SucceedCommandAction(private_result=None, command=set_temp_cmd) ) assert subject.state.substate_by_module_id == { "module-id": TemperatureModuleSubState( @@ -525,7 +525,7 @@ def test_handle_tempdeck_temperature_commands( ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=deactivate_cmd) + actions.SucceedCommandAction(private_result=None, command=deactivate_cmd) ) assert subject.state.substate_by_module_id == { "module-id": TemperatureModuleSubState( @@ -573,10 +573,10 @@ def test_handle_thermocycler_temperature_commands( subject = ModuleStore(_OT2_STANDARD_CONFIG) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_module_cmd) + actions.SucceedCommandAction(private_result=None, command=load_module_cmd) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=set_block_temp_cmd) + actions.SucceedCommandAction(private_result=None, command=set_block_temp_cmd) ) assert subject.state.substate_by_module_id == { "module-id": ThermocyclerModuleSubState( @@ -587,7 +587,7 @@ def test_handle_thermocycler_temperature_commands( ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=set_lid_temp_cmd) + actions.SucceedCommandAction(private_result=None, command=set_lid_temp_cmd) ) assert subject.state.substate_by_module_id == { "module-id": ThermocyclerModuleSubState( @@ -598,7 +598,7 @@ def test_handle_thermocycler_temperature_commands( ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=deactivate_lid_cmd) + actions.SucceedCommandAction(private_result=None, command=deactivate_lid_cmd) ) assert subject.state.substate_by_module_id == { "module-id": ThermocyclerModuleSubState( @@ -609,7 +609,7 @@ def test_handle_thermocycler_temperature_commands( ) } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=deactivate_block_cmd) + actions.SucceedCommandAction(private_result=None, command=deactivate_block_cmd) ) assert subject.state.substate_by_module_id == { "module-id": ThermocyclerModuleSubState( @@ -656,10 +656,10 @@ def test_handle_thermocycler_lid_commands( ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_module_cmd) + actions.SucceedCommandAction(private_result=None, command=load_module_cmd) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=open_lid_cmd) + actions.SucceedCommandAction(private_result=None, command=open_lid_cmd) ) assert subject.state.substate_by_module_id == { "module-id": ThermocyclerModuleSubState( @@ -671,7 +671,7 @@ def test_handle_thermocycler_lid_commands( } subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=close_lid_cmd) + actions.SucceedCommandAction(private_result=None, command=close_lid_cmd) ) assert subject.state.substate_by_module_id == { "module-id": ThermocyclerModuleSubState( diff --git a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py index df6a97d1315..d2479a55bc8 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -20,7 +20,7 @@ ) from opentrons.protocol_engine.actions import ( SetPipetteMovementSpeedAction, - UpdateCommandAction, + SucceedCommandAction, ) from opentrons.protocol_engine.state.pipettes import ( PipetteStore, @@ -86,7 +86,7 @@ def test_handles_load_pipette(subject: PipetteStore) -> None: mount=MountType.LEFT, ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) result = subject.state @@ -117,10 +117,10 @@ def test_handles_pick_up_and_drop_tip(subject: PipetteStore) -> None: ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=pick_up_tip_command) + SucceedCommandAction(private_result=None, command=pick_up_tip_command) ) assert subject.state.attached_tip_by_id["abc"] == TipGeometry( volume=42, length=101, diameter=8.0 @@ -128,7 +128,7 @@ def test_handles_pick_up_and_drop_tip(subject: PipetteStore) -> None: assert subject.state.aspirated_volume_by_id["abc"] == 0 subject.handle_action( - UpdateCommandAction(private_result=None, command=drop_tip_command) + SucceedCommandAction(private_result=None, command=drop_tip_command) ) assert subject.state.attached_tip_by_id["abc"] is None assert subject.state.aspirated_volume_by_id["abc"] is None @@ -151,10 +151,10 @@ def test_handles_drop_tip_in_place(subject: PipetteStore) -> None: ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=pick_up_tip_command) + SucceedCommandAction(private_result=None, command=pick_up_tip_command) ) assert subject.state.attached_tip_by_id["xyz"] == TipGeometry( volume=42, length=101, diameter=8.0 @@ -162,7 +162,7 @@ def test_handles_drop_tip_in_place(subject: PipetteStore) -> None: assert subject.state.aspirated_volume_by_id["xyz"] == 0 subject.handle_action( - UpdateCommandAction(private_result=None, command=drop_tip_in_place_command) + SucceedCommandAction(private_result=None, command=drop_tip_in_place_command) ) assert subject.state.attached_tip_by_id["xyz"] is None assert subject.state.aspirated_volume_by_id["xyz"] is None @@ -188,16 +188,16 @@ def test_aspirate_adds_volume( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_command) + SucceedCommandAction(private_result=None, command=load_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=aspirate_command) + SucceedCommandAction(private_result=None, command=aspirate_command) ) assert subject.state.aspirated_volume_by_id["pipette-id"] == 42 subject.handle_action( - UpdateCommandAction(private_result=None, command=aspirate_command) + SucceedCommandAction(private_result=None, command=aspirate_command) ) assert subject.state.aspirated_volume_by_id["pipette-id"] == 84 @@ -230,19 +230,19 @@ def test_dispense_subtracts_volume( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_command) + SucceedCommandAction(private_result=None, command=load_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=aspirate_command) + SucceedCommandAction(private_result=None, command=aspirate_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=dispense_command) + SucceedCommandAction(private_result=None, command=dispense_command) ) assert subject.state.aspirated_volume_by_id["pipette-id"] == 21 subject.handle_action( - UpdateCommandAction(private_result=None, command=dispense_command) + SucceedCommandAction(private_result=None, command=dispense_command) ) assert subject.state.aspirated_volume_by_id["pipette-id"] == 0 @@ -271,13 +271,13 @@ def test_blow_out_clears_volume( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_command) + SucceedCommandAction(private_result=None, command=load_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=aspirate_command) + SucceedCommandAction(private_result=None, command=aspirate_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=blow_out_command) + SucceedCommandAction(private_result=None, command=blow_out_command) ) assert subject.state.aspirated_volume_by_id["pipette-id"] is None @@ -378,9 +378,9 @@ def test_movement_commands_update_current_well( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert subject.state.current_location == expected_location @@ -462,12 +462,12 @@ def test_movement_commands_without_well_clear_current_well( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_command) + SucceedCommandAction(private_result=None, command=move_command) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert subject.state.current_location is None @@ -515,12 +515,12 @@ def test_heater_shaker_command_without_movement( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_command) + SucceedCommandAction(private_result=None, command=move_command) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert subject.state.current_location == CurrentWell( pipette_id="pipette-id", @@ -626,14 +626,14 @@ def test_move_labware_clears_current_well( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_to_well_command) + SucceedCommandAction(private_result=None, command=move_to_well_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_labware_command) + SucceedCommandAction(private_result=None, command=move_labware_command) ) assert subject.state.current_location == expected_current_well @@ -647,7 +647,7 @@ def test_set_movement_speed(subject: PipetteStore) -> None: mount=MountType.LEFT, ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( SetPipetteMovementSpeedAction(pipette_id=pipette_id, speed=123.456) @@ -690,7 +690,7 @@ def test_add_pipette_config( ), ) subject.handle_action( - UpdateCommandAction(command=command, private_result=private_result) + SucceedCommandAction(command=command, private_result=private_result) ) assert subject.state.static_config_by_id["pipette-id"] == StaticPipetteConfig( @@ -791,9 +791,9 @@ def test_movement_commands_update_deck_point( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert subject.state.current_deck_point == CurrentDeckPoint( mount=MountType.LEFT, deck_point=DeckPoint(x=11, y=22, z=33) @@ -872,17 +872,17 @@ def test_homing_commands_clear_deck_point( ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=move_command) + SucceedCommandAction(private_result=None, command=move_command) ) assert subject.state.current_deck_point == CurrentDeckPoint( mount=MountType.LEFT, deck_point=DeckPoint(x=1, y=2, z=3) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=command)) + subject.handle_action(SucceedCommandAction(private_result=None, command=command)) assert subject.state.current_deck_point == CurrentDeckPoint( mount=None, deck_point=None @@ -909,18 +909,18 @@ def test_prepare_to_aspirate_marks_pipette_ready( pipette_id="pipette-id", tip_volume=42, tip_length=101, tip_diameter=8.0 ) subject.handle_action( - UpdateCommandAction(private_result=None, command=load_pipette_command) + SucceedCommandAction(private_result=None, command=load_pipette_command) ) subject.handle_action( - UpdateCommandAction(private_result=None, command=pick_up_tip_command) + SucceedCommandAction(private_result=None, command=pick_up_tip_command) ) - subject.handle_action(UpdateCommandAction(private_result=None, command=previous)) + subject.handle_action(SucceedCommandAction(private_result=None, command=previous)) prepare_to_aspirate_command = create_prepare_to_aspirate_command( pipette_id="pipette-id" ) subject.handle_action( - UpdateCommandAction(private_result=None, command=prepare_to_aspirate_command) + SucceedCommandAction(private_result=None, command=prepare_to_aspirate_command) ) assert subject.state.aspirated_volume_by_id["pipette-id"] == 0.0 diff --git a/api/tests/opentrons/protocol_engine/state/test_tip_state.py b/api/tests/opentrons/protocol_engine/state/test_tip_state.py index 3f4ff0cf860..25894554027 100644 --- a/api/tests/opentrons/protocol_engine/state/test_tip_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_tip_state.py @@ -121,7 +121,7 @@ def test_get_next_tip_returns_none( ) -> None: """It should start at the first tip in the labware.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -150,7 +150,7 @@ def test_get_next_tip_returns_none( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -174,7 +174,7 @@ def test_get_next_tip_returns_first_tip( ) -> None: """It should start at the first tip in the labware.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -210,7 +210,7 @@ def test_get_next_tip_returns_first_tip( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -235,7 +235,7 @@ def test_get_next_tip_used_starting_tip( ) -> None: """It should start searching at the given starting tip.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -264,7 +264,7 @@ def test_get_next_tip_used_starting_tip( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -305,7 +305,7 @@ def test_get_next_tip_skips_picked_up_tip( ) -> None: """It should get the next tip in the column if one has been picked up.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -352,12 +352,12 @@ def test_get_next_tip_skips_picked_up_tip( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) subject.handle_action( - actions.UpdateCommandAction(command=pick_up_tip_command, private_result=None) + actions.SucceedCommandAction(command=pick_up_tip_command, private_result=None) ) result = TipView(subject.state).get_next_tip( @@ -377,7 +377,7 @@ def test_get_next_tip_with_starting_tip( ) -> None: """It should return the starting tip, and then the following tip after that.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -406,7 +406,7 @@ def test_get_next_tip_with_starting_tip( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -431,7 +431,7 @@ def test_get_next_tip_with_starting_tip( ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip) ) result = TipView(subject.state).get_next_tip( @@ -451,7 +451,7 @@ def test_get_next_tip_with_starting_tip_8_channel( ) -> None: """It should return the starting tip, and then the following tip after that.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -480,7 +480,7 @@ def test_get_next_tip_with_starting_tip_8_channel( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -506,7 +506,7 @@ def test_get_next_tip_with_starting_tip_8_channel( ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip) ) result = TipView(subject.state).get_next_tip( @@ -526,7 +526,7 @@ def test_get_next_tip_with_starting_tip_out_of_tips( ) -> None: """It should return the starting tip of H12 and then None after that.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -555,7 +555,7 @@ def test_get_next_tip_with_starting_tip_out_of_tips( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -581,7 +581,7 @@ def test_get_next_tip_with_starting_tip_out_of_tips( ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip) ) result = TipView(subject.state).get_next_tip( @@ -601,7 +601,7 @@ def test_get_next_tip_with_column_and_starting_tip( ) -> None: """It should return the first tip in a column, taking starting tip into account.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -630,7 +630,7 @@ def test_get_next_tip_with_column_and_starting_tip( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -653,7 +653,7 @@ def test_reset_tips( ) -> None: """It should be able to reset tip tracking state.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -683,13 +683,13 @@ def test_reset_tips( ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip_command) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip_command) ) subject.handle_action(actions.ResetTipsAction(labware_id="cool-labware")) @@ -734,7 +734,7 @@ def test_handle_pipette_config_action( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -757,7 +757,7 @@ def test_has_tip_not_tip_rack( ) -> None: """It should return False if labware isn't a tip rack.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) result = TipView(state=subject.state).has_clean_tip("cool-labware", "A1") @@ -770,7 +770,7 @@ def test_has_tip_tip_rack( ) -> None: """It should return False if labware isn't a tip rack.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) result = TipView(state=subject.state).has_clean_tip("cool-labware", "A1") @@ -788,7 +788,7 @@ def test_drop_tip( ) -> None: """It should be clear tip length when a tip is dropped.""" subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) load_pipette_command = commands.LoadPipette.construct( # type: ignore[call-arg] result=commands.LoadPipetteResult(pipetteId="pipette-id") @@ -817,31 +817,31 @@ def test_drop_tip( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip_command) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip_command) ) result = TipView(subject.state).get_tip_length("pipette-id") assert result == 1.23 subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=drop_tip_command) + actions.SucceedCommandAction(private_result=None, command=drop_tip_command) ) result = TipView(subject.state).get_tip_length("pipette-id") assert result == 0 subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip_command) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip_command) ) result = TipView(subject.state).get_tip_length("pipette-id") assert result == 1.23 subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=None, command=drop_tip_in_place_command ) ) @@ -922,7 +922,7 @@ def test_active_channels( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -936,7 +936,7 @@ def test_active_channels( nozzle_map=nozzle_map, ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=configure_nozzle_private_result, command=configure_nozzle_layout_cmd, ) @@ -956,7 +956,7 @@ def test_next_tip_uses_active_channels( """Test that tip tracking logic uses pipette's active channels.""" # Load labware subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) # Load pipette @@ -987,7 +987,7 @@ def test_next_tip_uses_active_channels( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -1008,14 +1008,14 @@ def test_next_tip_uses_active_channels( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=configure_nozzle_private_result, command=configure_nozzle_layout_cmd, ) ) # Pick up partial tips subject.handle_action( - actions.UpdateCommandAction(command=pick_up_tip_command, private_result=None) + actions.SucceedCommandAction(command=pick_up_tip_command, private_result=None) ) result = TipView(subject.state).get_next_tip( @@ -1036,7 +1036,7 @@ def test_next_tip_automatic_tip_tracking_with_partial_configurations( """Test tip tracking logic using multiple pipette configurations.""" # Load labware subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=load_labware_command) + actions.SucceedCommandAction(private_result=None, command=load_labware_command) ) # Load pipette @@ -1067,7 +1067,7 @@ def test_next_tip_automatic_tip_tracking_with_partial_configurations( ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=load_pipette_private_result, command=load_pipette_command ) ) @@ -1093,7 +1093,7 @@ def _assert_and_pickup(well: str, nozzle_map: NozzleMap) -> None: ) subject.handle_action( - actions.UpdateCommandAction(private_result=None, command=pick_up_tip) + actions.SucceedCommandAction(private_result=None, command=pick_up_tip) ) # Configure nozzle for partial configurations @@ -1102,7 +1102,6 @@ def _assert_and_pickup(well: str, nozzle_map: NozzleMap) -> None: ) def _reconfigure_nozzle_layout(start: str, back_l: str, front_r: str) -> NozzleMap: - configure_nozzle_private_result = commands.ConfigureNozzleLayoutPrivateResult( pipette_id="pipette-id", nozzle_map=NozzleMap.build( @@ -1115,7 +1114,7 @@ def _reconfigure_nozzle_layout(start: str, back_l: str, front_r: str) -> NozzleM ), ) subject.handle_action( - actions.UpdateCommandAction( + actions.SucceedCommandAction( private_result=configure_nozzle_private_result, command=configure_nozzle_layout_cmd, ) From d186f1f1f7a3eaa5df170ffbc6788cadf991c42a Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 12:24:37 -0400 Subject: [PATCH 04/24] Update legacy equipment loads. --- .../protocol_runner/legacy_command_mapper.py | 80 +++++++++++++++---- .../protocol_runner/legacy_context_plugin.py | 17 ++-- 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index b47ba1813fa..76805a2b9d7 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -259,16 +259,14 @@ def map_command( # noqa: C901 return results - def map_equipment_load( - self, load_info: LegacyLoadInfo - ) -> Tuple[pe_commands.Command, pe_commands.CommandPrivateResult]: + def map_equipment_load(self, load_info: LegacyLoadInfo) -> List[pe_actions.Action]: """Map a labware, instrument (pipette), or module load to a PE command.""" if isinstance(load_info, LegacyLabwareLoadInfo): - return (self._map_labware_load(load_info), None) + return self._map_labware_load(load_info) elif isinstance(load_info, LegacyInstrumentLoadInfo): return self._map_instrument_load(load_info) elif isinstance(load_info, LegacyModuleLoadInfo): - return (self._map_module_load(load_info), None) + return self._map_module_load(load_info) def _build_initial_command( self, @@ -525,7 +523,7 @@ def _build_blow_out_command( def _map_labware_load( self, labware_load_info: LegacyLabwareLoadInfo - ) -> pe_commands.Command: + ) -> List[pe_actions.Action]: """Map a legacy labware load to a ProtocolEngine command.""" now = ModelUtils.get_timestamp() count = self._command_count["LOAD_LABWARE"] @@ -541,7 +539,7 @@ def _map_labware_load( command_id = f"commands.LOAD_LABWARE-{count}" labware_id = f"labware-{count}" - load_labware_command = pe_commands.LoadLabware.construct( + succeeded_command = pe_commands.LoadLabware.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.SUCCEEDED, @@ -563,18 +561,36 @@ def _map_labware_load( offsetId=labware_load_info.offset_id, ), ) + queue_action = pe_actions.QueueCommandAction( + command_id=succeeded_command.id, + created_at=succeeded_command.createdAt, + request=pe_commands.LoadLabwareCreate.construct( + params=succeeded_command.params + ), + request_hash=None, + ) + run_action = pe_actions.RunCommandAction( + command_id=succeeded_command.id, + # We just set this above, so we know it's not None. + started_at=succeeded_command.startedAt, # type: ignore[arg-type] + ) + succeed_action = pe_actions.SucceedCommandAction( + command=succeeded_command, + private_result=None, + ) self._command_count["LOAD_LABWARE"] = count + 1 if isinstance(location, pe_types.DeckSlotLocation): self._labware_id_by_slot[location.slotName] = labware_id elif isinstance(location, pe_types.ModuleLocation): self._labware_id_by_module_id[location.moduleId] = labware_id - return load_labware_command + + return [queue_action, run_action, succeed_action] def _map_instrument_load( self, instrument_load_info: LegacyInstrumentLoadInfo, - ) -> Tuple[pe_commands.Command, pe_commands.CommandPrivateResult]: + ) -> List[pe_actions.Action]: """Map a legacy instrument (pipette) load to a ProtocolEngine command. Also creates a `AddPipetteConfigAction`, which is not necessary for the run, @@ -586,7 +602,7 @@ def _map_instrument_load( pipette_id = f"pipette-{count}" mount = MountType(str(instrument_load_info.mount).lower()) - load_pipette_command = pe_commands.LoadPipette.construct( + succeeded_command = pe_commands.LoadPipette.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.SUCCEEDED, @@ -607,15 +623,32 @@ def _map_instrument_load( instrument_load_info.pipette_dict ), ) + queue_action = pe_actions.QueueCommandAction( + command_id=succeeded_command.id, + created_at=succeeded_command.createdAt, + request=pe_commands.LoadPipetteCreate.construct( + params=succeeded_command.params + ), + request_hash=None, + ) + run_action = pe_actions.RunCommandAction( + command_id=succeeded_command.id, + # We just set this above, so we know it's not None. + started_at=succeeded_command.startedAt, # type: ignore[arg-type] + ) + succeed_action = pe_actions.SucceedCommandAction( + command=succeeded_command, + private_result=pipette_config_result, + ) self._command_count["LOAD_PIPETTE"] = count + 1 self._pipette_id_by_mount[mount] = pipette_id - return (load_pipette_command, pipette_config_result) + return [queue_action, run_action, succeed_action] def _map_module_load( self, module_load_info: LegacyModuleLoadInfo - ) -> pe_commands.Command: + ) -> List[pe_actions.Action]: """Map a legacy module load to a Protocol Engine command.""" now = ModelUtils.get_timestamp() @@ -634,7 +667,7 @@ def _map_module_load( loaded_model ) or self._module_data_provider.get_definition(loaded_model) - load_module_command = pe_commands.LoadModule.construct( + succeeded_command = pe_commands.LoadModule.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.SUCCEEDED, @@ -655,7 +688,26 @@ def _map_module_load( model=loaded_model, ), ) + queue_action = pe_actions.QueueCommandAction( + command_id=succeeded_command.id, + created_at=succeeded_command.createdAt, + request=pe_commands.LoadModuleCreate.construct( + params=succeeded_command.params + ), + request_hash=None, + ) + run_action = pe_actions.RunCommandAction( + command_id=succeeded_command.id, + # We just set this above, so we know it's not None. + started_at=succeeded_command.startedAt, # type: ignore[arg-type] + ) + succeed_action = pe_actions.SucceedCommandAction( + command=succeeded_command, + private_result=None, + ) + self._command_count["LOAD_MODULE"] = count + 1 self._module_id_by_slot[module_load_info.deck_slot] = module_id self._module_definition_by_model[loaded_model] = loaded_definition - return load_module_command + + return [queue_action, run_action, succeed_action] diff --git a/api/src/opentrons/protocol_runner/legacy_context_plugin.py b/api/src/opentrons/protocol_runner/legacy_context_plugin.py index 41ba0c62268..41cc5cd489a 100644 --- a/api/src/opentrons/protocol_runner/legacy_context_plugin.py +++ b/api/src/opentrons/protocol_runner/legacy_context_plugin.py @@ -123,16 +123,13 @@ def _handle_legacy_command(self, command: LegacyCommand) -> None: self._actions_to_dispatch.put(pe_action) def _handle_equipment_loaded(self, load_info: LegacyLoadInfo) -> None: - ( - pe_command, - pe_private_result, - ) = self._legacy_command_mapper.map_equipment_load(load_info=load_info) - - self._actions_to_dispatch.put( - pe_actions.UpdateCommandAction( - command=pe_command, private_result=pe_private_result - ) - ) + """Handle an equipment load reported by the APIv2 protocol. + + Used as a broker callback, so this will run in the APIv2 protocol's thread. + """ + pe_actions = self._legacy_command_mapper.map_equipment_load(load_info=load_info) + for pe_action in pe_actions: + self._actions_to_dispatch.put(pe_action) async def _dispatch_all_actions(self) -> None: """Dispatch all actions to the `ProtocolEngine`. From c7554858b864053f1618283a6cbb506e81c1f68b Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 19 Mar 2024 20:01:39 -0400 Subject: [PATCH 05/24] WIP: CommandExecutor. executor --- .../execution/command_executor.py | 26 ++++++++++++------- .../execution/test_command_executor.py | 18 +++++++------ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/api/src/opentrons/protocol_engine/execution/command_executor.py b/api/src/opentrons/protocol_engine/execution/command_executor.py index 337d3671be2..2fe4fe921ab 100644 --- a/api/src/opentrons/protocol_engine/execution/command_executor.py +++ b/api/src/opentrons/protocol_engine/execution/command_executor.py @@ -22,7 +22,12 @@ CommandPrivateResult, Command, ) -from ..actions import ActionDispatcher, UpdateCommandAction, FailCommandAction +from ..actions import ( + ActionDispatcher, + RunCommandAction, + SucceedCommandAction, + FailCommandAction, +) from ..errors import RunStoppedError from ..errors.exceptions import EStopActivatedError as PE_EStopActivatedError from ..notes import CommandNote, CommandNoteTracker @@ -140,7 +145,7 @@ async def execute(self, command_id: str) -> None: ) self._action_dispatcher.dispatch( - UpdateCommandAction(command=running_command, private_result=None) + RunCommandAction(command_id=command.id, started_at=started_at) ) try: @@ -168,12 +173,15 @@ async def execute(self, command_id: str) -> None: ) if notes_update: - command_with_new_notes = running_command.copy(update=notes_update) - self._action_dispatcher.dispatch( - UpdateCommandAction( - command=command_with_new_notes, private_result=None - ) - ) + # FIX BEFORE MERGE + pass + # command_with_new_notes = running_command.copy(update=notes_update) + # self._action_dispatcher.dispatch( + # # FIX BEFORE MERGE + # SucceedCommandAction( + # command=command_with_new_notes, private_result=None + # ) + # ) self._action_dispatcher.dispatch( FailCommandAction( @@ -203,7 +211,7 @@ async def execute(self, command_id: str) -> None: } completed_command = running_command.copy(update=update) self._action_dispatcher.dispatch( - UpdateCommandAction( + SucceedCommandAction( command=completed_command, private_result=private_result ), ) diff --git a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py index 34b459a9661..b642768e4c6 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py +++ b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py @@ -21,7 +21,7 @@ from opentrons.protocol_engine.state import StateStore from opentrons.protocol_engine.actions import ( ActionDispatcher, - UpdateCommandAction, + SucceedCommandAction, FailCommandAction, ) @@ -319,10 +319,10 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: decoy.verify( action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=running_command) + SucceedCommandAction(private_result=None, command=running_command) ), action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=completed_command) + SucceedCommandAction(private_result=None, command=completed_command) ), ) @@ -451,7 +451,7 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: decoy.verify( action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=running_command) + SucceedCommandAction(private_result=None, command=running_command) ), action_dispatcher.dispatch( FailCommandAction( @@ -581,10 +581,10 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: decoy.verify( action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=running_command) + SucceedCommandAction(private_result=None, command=running_command) ), action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=completed_command) + SucceedCommandAction(private_result=None, command=completed_command) ), ) @@ -696,10 +696,12 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: decoy.verify( action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=running_command) + SucceedCommandAction(private_result=None, command=running_command) ), action_dispatcher.dispatch( - UpdateCommandAction(private_result=None, command=running_command_with_notes) + SucceedCommandAction( + private_result=None, command=running_command_with_notes + ) ), action_dispatcher.dispatch( FailCommandAction( From 4127d7e281b331bc1027008bd2abe993a1877aba Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 12:24:34 -0400 Subject: [PATCH 06/24] WIP: legacy legacy --- .../protocol_runner/legacy_command_mapper.py | 43 ++++++++++--------- .../test_legacy_command_mapper.py | 14 +++--- .../test_legacy_context_plugin.py | 8 ++-- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index 76805a2b9d7..88205d9a4bd 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -48,7 +48,6 @@ class LegacyContextCommandError(ProtocolEngineError): """An error returned when a PAPIv2 ProtocolContext command fails.""" def __init__(self, wrapping_exc: BaseException) -> None: - if isinstance(wrapping_exc, EnumeratedError): super().__init__( wrapping_exc.code, @@ -145,13 +144,19 @@ def map_command( # noqa: C901 if stage == "before": count = self._command_count[command_type] command_id = f"{command_type}-{count}" - engine_command = self._build_initial_command(command, command_id, now) + running_command = self._build_initial_command(command, command_id, now) self._command_count[command_type] = count + 1 - self._commands_by_broker_id[broker_id] = engine_command - + self._commands_by_broker_id[broker_id] = running_command + + # TODO + # results.append(pe_actions.QueueCommandAction( + # command_id=command_id, + # created_at=running_command.createdAt, + # request= + # ) results.append( - pe_actions.UpdateCommandAction(engine_command, private_result=None) + pe_actions.RunCommandAction(running_command, private_result=None) ) elif stage == "after": @@ -231,8 +236,9 @@ def map_command( # noqa: C901 "completedAt": now, } ) + # TODO results.append( - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( completed_command, private_result=None ) ) @@ -273,14 +279,13 @@ def _build_initial_command( command: legacy_command_types.CommandMessage, command_id: str, now: datetime, - ) -> pe_commands.Command: - engine_command: pe_commands.Command + ) -> pe_commands.CommandCreate: if command["name"] == legacy_command_types.PICK_UP_TIP: - engine_command = self._build_pick_up_tip_command( + return self._build_pick_up_tip_create( command=command, command_id=command_id, now=now ) elif command["name"] == legacy_command_types.DROP_TIP: - engine_command = self._build_drop_tip_command( + return self._build_drop_tip_create( command=command, command_id=command_id, now=now ) @@ -288,15 +293,15 @@ def _build_initial_command( command["name"] == legacy_command_types.ASPIRATE or command["name"] == legacy_command_types.DISPENSE ): - engine_command = self._build_liquid_handling_command( + return self._build_liquid_handling_create( command=command, command_id=command_id, now=now ) elif command["name"] == legacy_command_types.BLOW_OUT: - engine_command = self._build_blow_out_command( + return self._build_blow_out_create( command=command, command_id=command_id, now=now ) elif command["name"] == legacy_command_types.PAUSE: - engine_command = pe_commands.WaitForResume.construct( + return pe_commands.WaitForResumeCreate.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -307,7 +312,7 @@ def _build_initial_command( ), ) else: - engine_command = pe_commands.Custom.construct( + return pe_commands.CustomCreate.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -319,9 +324,7 @@ def _build_initial_command( ), ) - return engine_command - - def _build_drop_tip_command( + def _build_drop_tip_create( self, command: legacy_command_types.DropTipMessage, command_id: str, @@ -348,7 +351,7 @@ def _build_drop_tip_command( ), ) - def _build_pick_up_tip_command( + def _build_pick_up_tip_create( self, command: legacy_command_types.PickUpTipMessage, command_id: str, @@ -377,7 +380,7 @@ def _build_pick_up_tip_command( ), ) - def _build_liquid_handling_command( + def _build_liquid_handling_create( self, command: Union[ legacy_command_types.AspirateMessage, legacy_command_types.DispenseMessage @@ -469,7 +472,7 @@ def _build_liquid_handling_command( ), ) - def _build_blow_out_command( + def _build_blow_out_create( self, command: legacy_command_types.BlowOutMessage, command_id: str, diff --git a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py index 54a5a435c6b..0c4a3e2e562 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py @@ -70,7 +70,7 @@ def test_map_before_command() -> None: result = subject.map_command(legacy_command) assert result == [ - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.Custom.construct( id="command.COMMENT-0", @@ -110,7 +110,7 @@ def test_map_after_command() -> None: result = subject.map_command(legacy_command_end) assert result == [ - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.Custom.construct( id="command.COMMENT-0", @@ -204,7 +204,7 @@ def test_command_stack() -> None: ] assert result == [ - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.Custom.construct( id="command.COMMENT-0", @@ -218,7 +218,7 @@ def test_command_stack() -> None: ), ), ), - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.Custom.construct( id="command.COMMENT-1", @@ -232,7 +232,7 @@ def test_command_stack() -> None: ), ), ), - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.Custom.construct( id="command.COMMENT-0", @@ -444,7 +444,7 @@ def test_map_pause() -> None: ] assert result == [ - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.WaitForResume.construct( id="command.PAUSE-0", @@ -455,7 +455,7 @@ def test_map_pause() -> None: params=pe_commands.WaitForResumeParams(message="hello world"), ), ), - pe_actions.UpdateCommandAction( + pe_actions.SucceedCommandAction( private_result=None, command=pe_commands.WaitForResume.construct( id="command.PAUSE-0", diff --git a/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py b/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py index e86f959ff2e..56eb58c84c0 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py @@ -160,7 +160,9 @@ async def test_command_broker_messages( decoy.when( mock_legacy_command_mapper.map_command(command=legacy_command) - ).then_return([pe_actions.UpdateCommandAction(engine_command, private_result=None)]) + ).then_return( + [pe_actions.SucceedCommandAction(engine_command, private_result=None)] + ) await to_thread.run_sync(handler, legacy_command) @@ -168,7 +170,7 @@ async def test_command_broker_messages( decoy.verify( mock_action_dispatcher.dispatch( - pe_actions.UpdateCommandAction(engine_command, private_result=None) + pe_actions.SucceedCommandAction(engine_command, private_result=None) ) ) @@ -225,6 +227,6 @@ async def test_equipment_broker_messages( decoy.verify( mock_action_dispatcher.dispatch( - pe_actions.UpdateCommandAction(command=engine_command, private_result=None) + pe_actions.SucceedCommandAction(command=engine_command, private_result=None) ), ) From 6df0bcc5adec7900b9b18dc2b828b414c4925bb2 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 14:23:04 -0400 Subject: [PATCH 07/24] WIP: Legacy --- .../protocol_runner/legacy_command_mapper.py | 127 +++++++++++++----- 1 file changed, 91 insertions(+), 36 deletions(-) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index 88205d9a4bd..e3978cf2bed 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -144,19 +144,26 @@ def map_command( # noqa: C901 if stage == "before": count = self._command_count[command_type] command_id = f"{command_type}-{count}" - running_command = self._build_initial_command(command, command_id, now) + command_create, running_command = self._build_initial_command( + command, command_id, now + ) self._command_count[command_type] = count + 1 self._commands_by_broker_id[broker_id] = running_command - # TODO - # results.append(pe_actions.QueueCommandAction( - # command_id=command_id, - # created_at=running_command.createdAt, - # request= - # ) results.append( - pe_actions.RunCommandAction(running_command, private_result=None) + pe_actions.QueueCommandAction( + command_id=command_id, + created_at=running_command.createdAt, + request=command_create, + request_hash=None, + ) + ) + assert running_command.startedAt is not None + results.append( + pe_actions.RunCommandAction( + running_command.id, started_at=running_command.startedAt + ) ) elif stage == "after": @@ -236,7 +243,6 @@ def map_command( # noqa: C901 "completedAt": now, } ) - # TODO results.append( pe_actions.SucceedCommandAction( completed_command, private_result=None @@ -279,29 +285,25 @@ def _build_initial_command( command: legacy_command_types.CommandMessage, command_id: str, now: datetime, - ) -> pe_commands.CommandCreate: + ) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]: if command["name"] == legacy_command_types.PICK_UP_TIP: - return self._build_pick_up_tip_create( + return self._build_pick_up_tip( command=command, command_id=command_id, now=now ) elif command["name"] == legacy_command_types.DROP_TIP: - return self._build_drop_tip_create( - command=command, command_id=command_id, now=now - ) + return self._build_drop_tip(command=command, command_id=command_id, now=now) elif ( command["name"] == legacy_command_types.ASPIRATE or command["name"] == legacy_command_types.DISPENSE ): - return self._build_liquid_handling_create( + return self._build_liquid_handling( command=command, command_id=command_id, now=now ) elif command["name"] == legacy_command_types.BLOW_OUT: - return self._build_blow_out_create( - command=command, command_id=command_id, now=now - ) + return self._build_blow_out(command=command, command_id=command_id, now=now) elif command["name"] == legacy_command_types.PAUSE: - return pe_commands.WaitForResumeCreate.construct( + wait_for_resume_running = pe_commands.WaitForResume.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -311,8 +313,15 @@ def _build_initial_command( message=command["payload"]["userMessage"], ), ) + wait_for_resume_create: pe_commands.CommandCreate = ( + pe_commands.WaitForResumeCreate.construct( + key=wait_for_resume_running.key, + params=wait_for_resume_running.params, + ) + ) + return wait_for_resume_create, wait_for_resume_running else: - return pe_commands.CustomCreate.construct( + custom_running = pe_commands.Custom.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -323,13 +332,18 @@ def _build_initial_command( legacyCommandText=command["payload"]["text"], ), ) + custom_create = pe_commands.CustomCreate.construct( + key=custom_running.key, + params=custom_running.params, + ) + return custom_create, custom_running - def _build_drop_tip_create( + def _build_drop_tip( self, command: legacy_command_types.DropTipMessage, command_id: str, now: datetime, - ) -> pe_commands.Command: + ) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]: pipette: LegacyPipetteContext = command["payload"]["instrument"] well = command["payload"]["location"] mount = MountType(pipette.mount) @@ -338,7 +352,8 @@ def _build_drop_tip_create( well_name = well.well_name labware_id = self._labware_id_by_slot[slot] pipette_id = self._pipette_id_by_mount[mount] - return pe_commands.DropTip.construct( + + running = pe_commands.DropTip.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -350,13 +365,18 @@ def _build_drop_tip_create( wellName=well_name, ), ) + create = pe_commands.DropTipCreate.construct( + key=running.key, + params=running.params, + ) + return create, running - def _build_pick_up_tip_create( + def _build_pick_up_tip( self, command: legacy_command_types.PickUpTipMessage, command_id: str, now: datetime, - ) -> pe_commands.Command: + ) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]: pipette: LegacyPipetteContext = command["payload"]["instrument"] location = command["payload"]["location"] well = location @@ -367,7 +387,7 @@ def _build_pick_up_tip_create( labware_id = self._labware_id_by_slot[slot] pipette_id = self._pipette_id_by_mount[mount] - return pe_commands.PickUpTip.construct( + running = pe_commands.PickUpTip.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -379,15 +399,19 @@ def _build_pick_up_tip_create( wellName=well_name, ), ) + create = pe_commands.PickUpTipCreate.construct( + key=running.key, params=running.params + ) + return create, running - def _build_liquid_handling_create( + def _build_liquid_handling( self, command: Union[ legacy_command_types.AspirateMessage, legacy_command_types.DispenseMessage ], command_id: str, now: datetime, - ) -> pe_commands.Command: + ) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]: pipette: LegacyPipetteContext = command["payload"]["instrument"] location = command["payload"]["location"] volume = command["payload"]["volume"] @@ -411,7 +435,11 @@ def _build_liquid_handling_create( # or aspirate() with a volume of 0, which behaves roughly like # move_to(). Protocol Engine aspirate and dispense commands must have # volume > 0, so we can't map into those. - return pe_commands.MoveToWell.construct( + # + # TODO(mm, 2024-03-22): I don't think this has been true since + # https://github.com/Opentrons/opentrons/pull/14211. Can we just use + # aspirate and dispense commands now? + move_to_well_running = pe_commands.MoveToWell.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -423,9 +451,13 @@ def _build_liquid_handling_create( wellName=well_name, ), ) + move_to_well_create = pe_commands.MoveToWellCreate.construct( + key=move_to_well_running.key, params=move_to_well_running.params + ) + return move_to_well_create, move_to_well_running elif command["name"] == legacy_command_types.ASPIRATE: flow_rate = command["payload"]["rate"] * pipette.flow_rate.aspirate - return pe_commands.Aspirate.construct( + aspirate_running = pe_commands.Aspirate.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -441,9 +473,13 @@ def _build_liquid_handling_create( flowRate=flow_rate, ), ) + aspirate_create = pe_commands.AspirateCreate.construct( + key=aspirate_running.key, params=aspirate_running.params + ) + return aspirate_create, aspirate_running else: flow_rate = command["payload"]["rate"] * pipette.flow_rate.dispense - return pe_commands.Dispense.construct( + dispense_running = pe_commands.Dispense.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -459,8 +495,13 @@ def _build_liquid_handling_create( flowRate=flow_rate, ), ) + dispense_create = pe_commands.DispenseCreate.construct( + key=dispense_running.key, params=dispense_running.params + ) + return dispense_create, dispense_running + else: - return pe_commands.Custom.construct( + running = pe_commands.Custom.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -471,13 +512,17 @@ def _build_liquid_handling_create( legacyCommandText=command["payload"]["text"], ), ) + create = pe_commands.CustomCreate.construct( + key=running.key, params=running.params + ) + return create, running - def _build_blow_out_create( + def _build_blow_out( self, command: legacy_command_types.BlowOutMessage, command_id: str, now: datetime, - ) -> pe_commands.Command: + ) -> Tuple[pe_commands.CommandCreate, pe_commands.Command]: pipette: LegacyPipetteContext = command["payload"]["instrument"] location = command["payload"]["location"] flow_rate = pipette.flow_rate.blow_out @@ -495,7 +540,8 @@ def _build_blow_out_create( mount = MountType(pipette.mount) well_name = well.well_name pipette_id = self._pipette_id_by_mount[mount] - return pe_commands.BlowOut.construct( + + blow_out_running = pe_commands.BlowOut.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -509,10 +555,15 @@ def _build_blow_out_create( flowRate=flow_rate, ), ) + blow_out_create = pe_commands.BlowOutCreate.construct( + key=blow_out_running.key, params=blow_out_running.params + ) + return blow_out_create, blow_out_running + # TODO:(jr, 15.08.2022): blow_out commands with no specified labware get filtered # into custom. Refactor this in followup legacy command mapping else: - return pe_commands.Custom.construct( + custom_running = pe_commands.Custom.construct( id=command_id, key=command_id, status=pe_commands.CommandStatus.RUNNING, @@ -523,6 +574,10 @@ def _build_blow_out_create( legacyCommandText=command["payload"]["text"], ), ) + custom_create = pe_commands.CustomCreate.construct( + key=custom_running.key, params=custom_running.params + ) + return custom_create, custom_running def _map_labware_load( self, labware_load_info: LegacyLabwareLoadInfo From f02ec73df8ca710925a5b4c57f39f97738dc104b Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 15:10:01 -0400 Subject: [PATCH 08/24] Dispatch actions atomically from LegacyContextPlugin. --- .../protocol_runner/legacy_context_plugin.py | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/api/src/opentrons/protocol_runner/legacy_context_plugin.py b/api/src/opentrons/protocol_runner/legacy_context_plugin.py index 41cc5cd489a..428d67e09a2 100644 --- a/api/src/opentrons/protocol_runner/legacy_context_plugin.py +++ b/api/src/opentrons/protocol_runner/legacy_context_plugin.py @@ -3,7 +3,7 @@ from asyncio import create_task, Task from contextlib import ExitStack -from typing import Optional +from typing import List, Optional from opentrons.commands.types import CommandMessage as LegacyCommand from opentrons.legacy_broker import LegacyBroker @@ -55,7 +55,7 @@ def __init__( # So if the protocol had to wait for the event loop to be free # every time it reported some activity, # it could visibly stall for a moment, making its motion jittery. - self._actions_to_dispatch = ThreadAsyncQueue[pe_actions.Action]() + self._actions_to_dispatch = ThreadAsyncQueue[List[pe_actions.Action]]() self._action_dispatching_task: Optional[Task[None]] = None self._subscription_exit_stack: Optional[ExitStack] = None @@ -119,8 +119,7 @@ def _handle_legacy_command(self, command: LegacyCommand) -> None: Used as a broker callback, so this will run in the APIv2 protocol's thread. """ pe_actions = self._legacy_command_mapper.map_command(command=command) - for pe_action in pe_actions: - self._actions_to_dispatch.put(pe_action) + self._actions_to_dispatch.put(pe_actions) def _handle_equipment_loaded(self, load_info: LegacyLoadInfo) -> None: """Handle an equipment load reported by the APIv2 protocol. @@ -128,8 +127,7 @@ def _handle_equipment_loaded(self, load_info: LegacyLoadInfo) -> None: Used as a broker callback, so this will run in the APIv2 protocol's thread. """ pe_actions = self._legacy_command_mapper.map_equipment_load(load_info=load_info) - for pe_action in pe_actions: - self._actions_to_dispatch.put(pe_action) + self._actions_to_dispatch.put(pe_actions) async def _dispatch_all_actions(self) -> None: """Dispatch all actions to the `ProtocolEngine`. @@ -137,5 +135,18 @@ async def _dispatch_all_actions(self) -> None: Exits only when `self._actions_to_dispatch` is closed (or an unexpected exception is raised). """ - async for action in self._actions_to_dispatch.get_async_until_closed(): - self.dispatch(action) + async for action_batch in self._actions_to_dispatch.get_async_until_closed(): + # It's critical that we dispatch this batch of actions as one atomic + # sequence, without yielding to the event loop. + # Although this plugin only means to use the ProtocolEngine as a way of + # passively exposing the protocol's progress, the ProtocolEngine is still + # theoretically active, which means it's constantly watching in the + # background to execute any commands that it finds `queued`. + # + # For example, one of these action batches will often want to + # instantaneously create a running command by having a queue action + # immediately followed by a run action. We cannot let the + # ProtocolEngine's background task see the command in the `queued` state, + # or it will try to execute it, which the legacy protocol is already doing. + for action in action_batch: + self.dispatch(action) From 815297ec1d0ef8b7675a023c76ce9a1dcacd1aab Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 14:23:09 -0400 Subject: [PATCH 09/24] WIP: legacy tests --- .../protocol_runner/test_legacy_command_mapper.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py index 0c4a3e2e562..56af939bb16 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py @@ -312,10 +312,17 @@ def test_map_instrument_load(decoy: Decoy) -> None: pipette_data_provider.get_pipette_static_config(pipette_dict) ).then_return(pipette_config) - result = LegacyCommandMapper().map_equipment_load(input) + [ + result_queue, + result_run, + result_complete, + ] = LegacyCommandMapper().map_equipment_load(input) + pipette_id_captor = matchers.Captor() - assert result[0] == pe_commands.LoadPipette.construct( + assert isinstance(result_complete, pe_actions.SucceedCommandAction) + + assert result_complete.command == pe_commands.LoadPipette.construct( id=matchers.IsA(str), key=matchers.IsA(str), status=pe_commands.CommandStatus.SUCCEEDED, @@ -327,7 +334,7 @@ def test_map_instrument_load(decoy: Decoy) -> None: ), result=pe_commands.LoadPipetteResult.construct(pipetteId=pipette_id_captor), ) - assert result[1] == pe_commands.LoadPipettePrivateResult( + assert result_complete.private_result == pe_commands.LoadPipettePrivateResult( pipette_id="pipette-0", serial_number="fizzbuzz", config=pipette_config, From 10b33723ca8335bd4c989d2a1fc6d664f77485ed Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 16:36:23 -0400 Subject: [PATCH 10/24] Fix race condition in legacy Python protocols' initial home command. The exact sequence is unclear to me, but it's something like the PAPI thread trying to synthesize a running command while the engine is actually running the initial home. I don't know why this apparent concurrent robot control hasn't caused physical problems with robot motion. --- .../protocol_runner/protocol_runner.py | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/api/src/opentrons/protocol_runner/protocol_runner.py b/api/src/opentrons/protocol_runner/protocol_runner.py index d2c67b9cfb3..9a9d1ce02eb 100644 --- a/api/src/opentrons/protocol_runner/protocol_runner.py +++ b/api/src/opentrons/protocol_runner/protocol_runner.py @@ -190,17 +190,21 @@ async def load( equipment_broker=equipment_broker, ) initial_home_command = pe_commands.HomeCreate( + # this command homes all axes, including pipette plugner and gripper jaw params=pe_commands.HomeParams(axes=None) ) - # this command homes all axes, including pipette plunger and gripper jaw - self._protocol_engine.add_command(request=initial_home_command) - self._task_queue.set_run_func( - func=self._legacy_executor.execute, - protocol=protocol, - context=context, - run_time_param_values=run_time_param_values, - ) + async def run_func() -> None: + await self._protocol_engine.add_and_execute_command( + request=initial_home_command + ) + await self._legacy_executor.execute( + protocol=protocol, + context=context, + run_time_param_values=run_time_param_values, + ) + + self._task_queue.set_run_func(run_func) async def run( # noqa: D102 self, From b5c54afef5b5634d53d77c5f8207d4dc02ff3c0e Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 22 Mar 2024 17:14:58 -0400 Subject: [PATCH 11/24] Leave a todo for removing the old ThreadAsyncQueue. --- .../opentrons/protocol_runner/legacy_context_plugin.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/api/src/opentrons/protocol_runner/legacy_context_plugin.py b/api/src/opentrons/protocol_runner/legacy_context_plugin.py index 428d67e09a2..3e32877f232 100644 --- a/api/src/opentrons/protocol_runner/legacy_context_plugin.py +++ b/api/src/opentrons/protocol_runner/legacy_context_plugin.py @@ -55,6 +55,14 @@ def __init__( # So if the protocol had to wait for the event loop to be free # every time it reported some activity, # it could visibly stall for a moment, making its motion jittery. + # + # TODO(mm, 2024-03-22): See if we can remove this non-blockingness now. + # It was one of several band-aids introduced in ~v5.0.0 to mitigate performance + # problems. v6.3.0 started running some Python protocols directly through + # Protocol Engine, without this plugin, and without any non-blocking queue. + # If performance is sufficient for those, that probably means the + # performance problems have been resolved in better ways elsewhere + # and we don't need this anymore. self._actions_to_dispatch = ThreadAsyncQueue[List[pe_actions.Action]]() self._action_dispatching_task: Optional[Task[None]] = None From cfa9053cbf2e5774db8f748e49221ff0bc0552d3 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 11:03:11 -0400 Subject: [PATCH 12/24] Less ambiguous docstrings. --- api/src/opentrons/protocol_engine/actions/actions.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/protocol_engine/actions/actions.py b/api/src/opentrons/protocol_engine/actions/actions.py index 287cbabe885..256442f9f1b 100644 --- a/api/src/opentrons/protocol_engine/actions/actions.py +++ b/api/src/opentrons/protocol_engine/actions/actions.py @@ -124,7 +124,8 @@ class QueueCommandAction: class RunCommandAction: """Mark a given command as running. - The command must be queued immediately prior to this action. + At the time of dispatching this action, the command must be queued, + and no other command may be running. """ command_id: str @@ -135,7 +136,7 @@ class RunCommandAction: class SucceedCommandAction: """Mark a given command as succeeded. - The command must be running immediately prior to this action. + At the time of dispatching this action, the command must be running. """ command: Command @@ -148,7 +149,7 @@ class SucceedCommandAction: class FailCommandAction: """Mark a given command as failed. - The command must be running immediately prior to this action. + At the time of dispatching this action, the command must be running. """ command_id: str From e0de9dd129df268933f3c81425b0b62a860eff8c Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 12:41:55 -0400 Subject: [PATCH 13/24] Let FailCommandAction set command notes. Simplify note update logic so it only overwrites, never appends. Now that UpdateCommandAction has been replaced by terminal SucceedCommandAction/FailCommandAction, appending can't happen in practice. Refactor tests to test command notes as part of general success/failure, to save a bunch of test boilerplate. --- .../protocol_engine/actions/actions.py | 6 +- .../execution/command_executor.py | 48 +-- .../protocol_engine/state/commands.py | 15 +- .../execution/test_command_executor.py | 293 +++--------------- 4 files changed, 71 insertions(+), 291 deletions(-) diff --git a/api/src/opentrons/protocol_engine/actions/actions.py b/api/src/opentrons/protocol_engine/actions/actions.py index 256442f9f1b..d5c6bb49abc 100644 --- a/api/src/opentrons/protocol_engine/actions/actions.py +++ b/api/src/opentrons/protocol_engine/actions/actions.py @@ -6,8 +6,7 @@ from dataclasses import dataclass from datetime import datetime from enum import Enum -from typing import Optional, Union -from opentrons.protocol_engine.error_recovery_policy import ErrorRecoveryType +from typing import List, Optional, Union from opentrons.protocols.models import LabwareDefinition from opentrons.hardware_control.types import DoorState @@ -16,6 +15,8 @@ from opentrons_shared_data.errors import EnumeratedError from ..commands import Command, CommandCreate, CommandPrivateResult +from ..error_recovery_policy import ErrorRecoveryType +from ..notes.notes import CommandNote from ..types import ( LabwareOffsetCreate, ModuleDefinition, @@ -156,6 +157,7 @@ class FailCommandAction: error_id: str failed_at: datetime error: EnumeratedError + notes: List[CommandNote] type: ErrorRecoveryType diff --git a/api/src/opentrons/protocol_engine/execution/command_executor.py b/api/src/opentrons/protocol_engine/execution/command_executor.py index 2fe4fe921ab..b34f7028337 100644 --- a/api/src/opentrons/protocol_engine/execution/command_executor.py +++ b/api/src/opentrons/protocol_engine/execution/command_executor.py @@ -119,9 +119,9 @@ async def execute(self, command_id: str) -> None: command_id: The identifier of the command to execute. The command itself will be looked up from state. """ - command = self._state_store.commands.get(command_id=command_id) + queued_command = self._state_store.commands.get(command_id=command_id) note_tracker = self._command_note_tracker_provider() - command_impl = command._ImplementationCls( + command_impl = queued_command._ImplementationCls( state_view=self._state_store, hardware_api=self._hardware_api, equipment=self._equipment, @@ -137,29 +137,24 @@ async def execute(self, command_id: str) -> None: ) started_at = self._model_utils.get_timestamp() - running_command = command.copy( - update={ - "status": CommandStatus.RUNNING, - "startedAt": started_at, - } - ) self._action_dispatcher.dispatch( - RunCommandAction(command_id=command.id, started_at=started_at) + RunCommandAction(command_id=queued_command.id, started_at=started_at) ) + running_command = self._state_store.commands.get(queued_command.id) try: log.debug( - f"Executing {command.id}, {command.commandType}, {command.params}" + f"Executing {queued_command.id}, {queued_command.commandType}, {queued_command.params}" ) if isinstance(command_impl, AbstractCommandImpl): - result: CommandResult = await command_impl.execute(command.params) # type: ignore[arg-type] + result: CommandResult = await command_impl.execute(queued_command.params) # type: ignore[arg-type] private_result: Optional[CommandPrivateResult] = None else: - result, private_result = await command_impl.execute(command.params) # type: ignore[arg-type] + result, private_result = await command_impl.execute(queued_command.params) # type: ignore[arg-type] except (Exception, asyncio.CancelledError) as error: - log.warning(f"Execution of {command.id} failed", exc_info=error) + log.warning(f"Execution of {queued_command.id} failed", exc_info=error) # TODO(mc, 2022-11-14): mark command as stopped rather than failed # https://opentrons.atlassian.net/browse/RCORE-390 if isinstance(error, asyncio.CancelledError): @@ -168,20 +163,6 @@ async def execute(self, command_id: str) -> None: error = PE_EStopActivatedError(message=str(error), wrapping=[error]) elif not isinstance(error, EnumeratedError): error = PythonException(error) - notes_update = _append_notes_if_notes( - running_command, note_tracker.get_notes() - ) - - if notes_update: - # FIX BEFORE MERGE - pass - # command_with_new_notes = running_command.copy(update=notes_update) - # self._action_dispatcher.dispatch( - # # FIX BEFORE MERGE - # SucceedCommandAction( - # command=command_with_new_notes, private_result=None - # ) - # ) self._action_dispatcher.dispatch( FailCommandAction( @@ -189,6 +170,7 @@ async def execute(self, command_id: str) -> None: command_id=command_id, error_id=self._model_utils.generate_id(), failed_at=self._model_utils.get_timestamp(), + notes=note_tracker.get_notes(), # todo(mm, 2024-03-13): # When a command fails recoverably, and we handle it with # WAIT_FOR_RECOVERY or CONTINUE, we want to update our logical @@ -207,7 +189,7 @@ async def execute(self, command_id: str) -> None: "result": result, "status": CommandStatus.SUCCEEDED, "completedAt": self._model_utils.get_timestamp(), - **_append_notes_if_notes(running_command, note_tracker.get_notes()), + "notes": note_tracker.get_notes(), } completed_command = running_command.copy(update=update) self._action_dispatcher.dispatch( @@ -215,13 +197,3 @@ async def execute(self, command_id: str) -> None: command=completed_command, private_result=private_result ), ) - - -def _append_notes_if_notes( - running_command: Command, notes: List[CommandNote] -) -> Dict[str, Any]: - if not notes: - return {} - if running_command.notes is None: - return {"notes": notes} - return {"notes": running_command.notes + notes} diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 656dbb90a2d..7a4b88c7cd2 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -18,6 +18,7 @@ RunCommandAction, ) from opentrons.protocol_engine.error_recovery_policy import ErrorRecoveryType +from opentrons.protocol_engine.notes.notes import CommandNote from ..actions import ( Action, @@ -318,6 +319,7 @@ def handle_action(self, action: Action) -> None: # noqa: C901 command_id=action.command_id, failed_at=action.failed_at, error_occurrence=error_occurrence, + notes=action.notes, ) self._state.failed_command = self._state.commands_by_id[action.command_id] @@ -326,7 +328,10 @@ def handle_action(self, action: Action) -> None: # noqa: C901 other_command_ids_to_fail = self._state.queued_setup_command_ids for id in other_command_ids_to_fail: self._update_to_failed( - command_id=id, failed_at=action.failed_at, error_occurrence=None + command_id=id, + failed_at=action.failed_at, + error_occurrence=None, + notes=None, ) self._state.queued_setup_command_ids.clear() elif ( @@ -342,6 +347,7 @@ def handle_action(self, action: Action) -> None: # noqa: C901 command_id=id, failed_at=action.failed_at, error_occurrence=None, + notes=None, ) self._state.queued_command_ids.clear() else: @@ -427,13 +433,18 @@ def _update_to_failed( command_id: str, failed_at: datetime, error_occurrence: Optional[ErrorOccurrence], + notes: Optional[List[CommandNote]], ) -> None: prev_entry = self._state.commands_by_id[command_id] updated_command = prev_entry.command.copy( update={ "completedAt": failed_at, "status": CommandStatus.FAILED, - **({"error": error_occurrence} if error_occurrence else {}), + **({"error": error_occurrence} if error_occurrence is not None else {}), + # Assume we're not overwriting any existing notes because they can + # only be added when a command completes, and if we're failing this + # command, it wouldn't have completed before now. + **({"notes": notes} if notes is not None else {}), } ) self._state.commands_by_id[command_id] = CommandEntry( diff --git a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py index b642768e4c6..5ef61540168 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py +++ b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py @@ -10,6 +10,7 @@ from opentrons.hardware_control import HardwareControlAPI, OT2HardwareControlAPI from opentrons.protocol_engine import errors +from opentrons.protocol_engine.actions.actions import RunCommandAction from opentrons.protocol_engine.error_recovery_policy import ( ErrorRecoveryPolicy, ErrorRecoveryType, @@ -271,7 +272,16 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: ), ) - completed_command = cast( + command_notes = [ + CommandNote( + noteKind="warning", + shortMessage="hello", + longMessage="test command note", + source="test", + ) + ] + + expected_completed_command = cast( Command, _TestCommand( id="command-id", @@ -282,6 +292,7 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: status=CommandStatus.SUCCEEDED, params=command_params, result=command_result, + notes=command_notes, ), ) @@ -289,6 +300,18 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: queued_command ) + decoy.when( + action_dispatcher.dispatch( + RunCommandAction( + command_id="command-id", started_at=datetime(year=2022, month=2, day=2) + ) + ) + ).then_do( + lambda _: decoy.when( + state_store.commands.get(command_id="command-id") + ).then_return(running_command) + ) + decoy.when( queued_command._ImplementationCls( state_view=state_store, @@ -308,6 +331,8 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: command_impl # type: ignore[arg-type] ) + decoy.when(command_note_tracker.get_notes()).then_return(command_notes) + decoy.when(await command_impl.execute(command_params)).then_return(command_result) decoy.when(model_utils.get_timestamp()).then_return( @@ -319,10 +344,9 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: decoy.verify( action_dispatcher.dispatch( - SucceedCommandAction(private_result=None, command=running_command) - ), - action_dispatcher.dispatch( - SucceedCommandAction(private_result=None, command=completed_command) + SucceedCommandAction( + private_result=None, command=expected_completed_command + ) ), ) @@ -412,104 +436,6 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: ), ) - decoy.when(state_store.commands.get(command_id="command-id")).then_return( - queued_command - ) - - decoy.when( - queued_command._ImplementationCls( - state_view=state_store, - hardware_api=hardware_api, - equipment=equipment, - movement=movement, - gantry_mover=mock_gantry_mover, - labware_movement=labware_movement, - pipetting=pipetting, - tip_handler=mock_tip_handler, - run_control=run_control, - rail_lights=rail_lights, - status_bar=status_bar, - command_note_adder=command_note_tracker, - ) - ).then_return( - command_impl # type: ignore[arg-type] - ) - - decoy.when(await command_impl.execute(command_params)).then_raise(command_error) - - decoy.when(model_utils.generate_id()).then_return("error-id") - decoy.when(model_utils.get_timestamp()).then_return( - datetime(year=2022, month=2, day=2), - datetime(year=2023, month=3, day=3), - ) - - decoy.when(error_recovery_policy(matchers.Anything(), expected_error)).then_return( - ErrorRecoveryType.WAIT_FOR_RECOVERY - ) - - await subject.execute("command-id") - - decoy.verify( - action_dispatcher.dispatch( - SucceedCommandAction(private_result=None, command=running_command) - ), - action_dispatcher.dispatch( - FailCommandAction( - command_id="command-id", - error_id="error-id", - failed_at=datetime(year=2023, month=3, day=3), - error=expected_error, - type=ErrorRecoveryType.WAIT_FOR_RECOVERY, - ) - ), - ) - - -async def test_executor_forwards_notes_on_command_success( - decoy: Decoy, - hardware_api: HardwareControlAPI, - state_store: StateStore, - action_dispatcher: ActionDispatcher, - equipment: EquipmentHandler, - movement: MovementHandler, - mock_gantry_mover: GantryMover, - labware_movement: LabwareMovementHandler, - pipetting: PipettingHandler, - mock_tip_handler: TipHandler, - run_control: RunControlHandler, - rail_lights: RailLightsHandler, - status_bar: StatusBarHandler, - model_utils: ModelUtils, - command_note_tracker: CommandNoteTracker, - subject: CommandExecutor, -) -> None: - """It should be able to add notes during OK execution to command updates.""" - TestCommandImplCls = decoy.mock(func=_TestCommandImpl) - command_impl = decoy.mock(cls=_TestCommandImpl) - - class _TestCommand(BaseCommand[_TestCommandParams, _TestCommandResult]): - commandType: str = "testCommand" - params: _TestCommandParams - result: Optional[_TestCommandResult] - - @property - def _ImplementationCls(self) -> Type[_TestCommandImpl]: - return TestCommandImplCls - - command_params = _TestCommandParams() - command_result = _TestCommandResult() - - queued_command = cast( - Command, - _TestCommand( - id="command-id", - key="command-key", - createdAt=datetime(year=2021, month=1, day=1), - status=CommandStatus.QUEUED, - params=command_params, - ), - ) - command_notes = [ CommandNote( noteKind="warning", @@ -519,144 +445,20 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: ) ] - running_command = cast( - Command, - _TestCommand( - id="command-id", - key="command-key", - createdAt=datetime(year=2021, month=1, day=1), - startedAt=datetime(year=2022, month=2, day=2), - status=CommandStatus.RUNNING, - params=command_params, - ), - ) - - completed_command = cast( - Command, - _TestCommand( - id="command-id", - key="command-key", - createdAt=datetime(year=2021, month=1, day=1), - startedAt=datetime(year=2022, month=2, day=2), - completedAt=datetime(year=2023, month=3, day=3), - status=CommandStatus.SUCCEEDED, - params=command_params, - result=command_result, - notes=command_notes, - ), - ) - decoy.when(state_store.commands.get(command_id="command-id")).then_return( queued_command ) decoy.when( - queued_command._ImplementationCls( - state_view=state_store, - hardware_api=hardware_api, - equipment=equipment, - movement=movement, - gantry_mover=mock_gantry_mover, - labware_movement=labware_movement, - pipetting=pipetting, - tip_handler=mock_tip_handler, - run_control=run_control, - rail_lights=rail_lights, - status_bar=status_bar, - command_note_adder=command_note_tracker, - ) - ).then_return( - command_impl # type: ignore[arg-type] - ) - - decoy.when(await command_impl.execute(command_params)).then_return(command_result) - - decoy.when(model_utils.get_timestamp()).then_return( - datetime(year=2022, month=2, day=2), - datetime(year=2023, month=3, day=3), - ) - decoy.when(command_note_tracker.get_notes()).then_return(command_notes) - - await subject.execute("command-id") - - decoy.verify( - action_dispatcher.dispatch( - SucceedCommandAction(private_result=None, command=running_command) - ), action_dispatcher.dispatch( - SucceedCommandAction(private_result=None, command=completed_command) - ), - ) - - -async def test_executor_forwards_notes_on_command_failure( - decoy: Decoy, - hardware_api: HardwareControlAPI, - state_store: StateStore, - action_dispatcher: ActionDispatcher, - equipment: EquipmentHandler, - movement: MovementHandler, - mock_gantry_mover: GantryMover, - labware_movement: LabwareMovementHandler, - pipetting: PipettingHandler, - mock_tip_handler: TipHandler, - run_control: RunControlHandler, - rail_lights: RailLightsHandler, - status_bar: StatusBarHandler, - model_utils: ModelUtils, - subject: CommandExecutor, - command_note_tracker: CommandNoteTracker, - error_recovery_policy: ErrorRecoveryPolicy, -) -> None: - """It should handle an error occuring during execution.""" - TestCommandImplCls = decoy.mock(func=_TestCommandImpl) - command_impl = decoy.mock(cls=_TestCommandImpl) - - class _TestCommand(BaseCommand[_TestCommandParams, _TestCommandResult]): - commandType: str = "testCommand" - params: _TestCommandParams - result: Optional[_TestCommandResult] - - @property - def _ImplementationCls(self) -> Type[_TestCommandImpl]: - return TestCommandImplCls - - command_params = _TestCommandParams() - command_notes = [ - CommandNote( - noteKind="warning", - shortMessage="hello", - longMessage="test command note", - source="test", + RunCommandAction( + command_id="command-id", started_at=datetime(year=2022, month=2, day=2) + ) ) - ] - - queued_command = cast( - Command, - _TestCommand( - id="command-id", - key="command-key", - createdAt=datetime(year=2021, month=1, day=1), - status=CommandStatus.QUEUED, - params=command_params, - ), - ) - - running_command = cast( - Command, - _TestCommand( - id="command-id", - key="command-key", - createdAt=datetime(year=2021, month=1, day=1), - startedAt=datetime(year=2022, month=2, day=2), - status=CommandStatus.RUNNING, - params=command_params, - ), - ) - running_command_with_notes = running_command.copy(update={"notes": command_notes}) - - decoy.when(state_store.commands.get(command_id="command-id")).then_return( - queued_command + ).then_do( + lambda _: decoy.when( + state_store.commands.get(command_id="command-id") + ).then_return(running_command) ) decoy.when( @@ -678,38 +480,31 @@ def _ImplementationCls(self) -> Type[_TestCommandImpl]: command_impl # type: ignore[arg-type] ) - decoy.when(await command_impl.execute(command_params)).then_raise( - RuntimeError("oh no") - ) + decoy.when(await command_impl.execute(command_params)).then_raise(command_error) decoy.when(model_utils.generate_id()).then_return("error-id") decoy.when(model_utils.get_timestamp()).then_return( datetime(year=2022, month=2, day=2), datetime(year=2023, month=3, day=3), ) - decoy.when( - error_recovery_policy(matchers.Anything(), matchers.Anything()) - ).then_return(ErrorRecoveryType.WAIT_FOR_RECOVERY) + + decoy.when(error_recovery_policy(matchers.Anything(), expected_error)).then_return( + ErrorRecoveryType.WAIT_FOR_RECOVERY + ) + decoy.when(command_note_tracker.get_notes()).then_return(command_notes) await subject.execute("command-id") decoy.verify( - action_dispatcher.dispatch( - SucceedCommandAction(private_result=None, command=running_command) - ), - action_dispatcher.dispatch( - SucceedCommandAction( - private_result=None, command=running_command_with_notes - ) - ), action_dispatcher.dispatch( FailCommandAction( command_id="command-id", error_id="error-id", failed_at=datetime(year=2023, month=3, day=3), - error=matchers.ErrorMatching(PythonException, match="oh no"), + error=expected_error, type=ErrorRecoveryType.WAIT_FOR_RECOVERY, + notes=command_notes, ) ), ) From dd05fd89ef5284a22adb97f349b5b40a1f7d9314 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 12:48:05 -0400 Subject: [PATCH 14/24] Import fixup. --- .../protocol_engine/execution/test_command_executor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py index 5ef61540168..94b7ad25509 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_command_executor.py +++ b/api/tests/opentrons/protocol_engine/execution/test_command_executor.py @@ -10,7 +10,6 @@ from opentrons.hardware_control import HardwareControlAPI, OT2HardwareControlAPI from opentrons.protocol_engine import errors -from opentrons.protocol_engine.actions.actions import RunCommandAction from opentrons.protocol_engine.error_recovery_policy import ( ErrorRecoveryPolicy, ErrorRecoveryType, @@ -22,6 +21,7 @@ from opentrons.protocol_engine.state import StateStore from opentrons.protocol_engine.actions import ( ActionDispatcher, + RunCommandAction, SucceedCommandAction, FailCommandAction, ) From 098fc2e0a0373baca1f07ace2f1271e9c59ad467 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 12:56:24 -0400 Subject: [PATCH 15/24] Update various things to supply notes. --- .../protocol_engine/protocol_engine.py | 2 + .../protocol_runner/legacy_command_mapper.py | 1 + .../state/test_command_store.py | 66 ++++++++++++++++++- .../protocol_engine/test_protocol_engine.py | 2 + .../test_legacy_command_mapper.py | 2 + 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index 7d4464174b4..3fedb61c000 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -266,6 +266,7 @@ def estop(self, maintenance_run: bool) -> None: error_id=self._model_utils.generate_id(), failed_at=self._model_utils.get_timestamp(), error=EStopActivatedError(message="Estop Activated"), + notes=[], type=ErrorRecoveryType.FAIL_RUN, ) self._action_dispatcher.dispatch(fail_action) @@ -279,6 +280,7 @@ def estop(self, maintenance_run: bool) -> None: error_id=self._model_utils.generate_id(), failed_at=self._model_utils.get_timestamp(), error=EStopActivatedError(message="Estop Activated"), + notes=[], type=ErrorRecoveryType.FAIL_RUN, ) self._action_dispatcher.dispatch(fail_action) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index e3978cf2bed..c5dd6b80113 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -261,6 +261,7 @@ def map_command( # noqa: C901 error_id=ModelUtils.generate_id(), failed_at=now, error=LegacyContextCommandError(command_error), + notes=[], # For legacy protocols, we don't attempt to support any kind # of error recovery at the Protocol Engine level. # These protocols only run on the OT-2, which doesn't have diff --git a/api/tests/opentrons/protocol_engine/state/test_command_store.py b/api/tests/opentrons/protocol_engine/state/test_command_store.py index e2aedc9ebe4..c5db00a96ae 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_store.py @@ -9,6 +9,7 @@ from opentrons.ordered_set import OrderedSet from opentrons.protocol_engine.actions.actions import RunCommandAction +from opentrons.protocol_engine.notes.notes import CommandNote from opentrons.types import MountType, DeckSlotName from opentrons.hardware_control.types import DoorState @@ -446,6 +447,14 @@ def test_command_failure_clears_queues() -> None: error_id="error-id", failed_at=datetime(year=2023, month=3, day=3), error=errors.ProtocolEngineError(message="oh no"), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], type=ErrorRecoveryType.FAIL_RUN, ) @@ -459,6 +468,14 @@ def test_command_failure_clears_queues() -> None: errorType="ProtocolEngineError", detail="oh no", ), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], createdAt=datetime(year=2021, month=1, day=1), startedAt=datetime(year=2022, month=2, day=2), completedAt=datetime(year=2023, month=3, day=3), @@ -542,6 +559,14 @@ def test_setup_command_failure_only_clears_setup_command_queue() -> None: error_id="error-id", failed_at=datetime(year=2023, month=3, day=3), error=errors.ProtocolEngineError(message="oh no"), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], type=ErrorRecoveryType.FAIL_RUN, ) expected_failed_cmd_2 = commands.WaitForResume( @@ -554,6 +579,14 @@ def test_setup_command_failure_only_clears_setup_command_queue() -> None: detail="oh no", errorCode=ErrorCodes.GENERAL_ERROR.value.code, ), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], createdAt=datetime(year=2021, month=1, day=1), startedAt=datetime(year=2022, month=2, day=2), completedAt=datetime(year=2023, month=3, day=3), @@ -626,6 +659,14 @@ def test_nonfatal_command_failure() -> None: error_id="error-id", failed_at=datetime(year=2023, month=3, day=3), error=errors.ProtocolEngineError(message="oh no"), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], type=ErrorRecoveryType.WAIT_FOR_RECOVERY, ) @@ -639,6 +680,14 @@ def test_nonfatal_command_failure() -> None: errorType="ProtocolEngineError", detail="oh no", ), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], createdAt=datetime(year=2021, month=1, day=1), startedAt=datetime(year=2022, month=2, day=2), completedAt=datetime(year=2023, month=3, day=3), @@ -1174,7 +1223,14 @@ def test_command_store_handles_command_failed() -> None: params=commands.CommentParams(message="hello, world"), result=None, error=expected_error_occurrence, - notes=None, + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], ) subject = CommandStore(is_door_open=False, config=_make_config()) @@ -1202,6 +1258,14 @@ def test_command_store_handles_command_failed() -> None: error_id=expected_error_occurrence.id, failed_at=expected_error_occurrence.createdAt, error=errors.ProtocolEngineError(message="oh no"), + notes=[ + CommandNote( + noteKind="noteKind", + shortMessage="shortMessage", + longMessage="longMessage", + source="source", + ) + ], type=ErrorRecoveryType.FAIL_RUN, ) ) diff --git a/api/tests/opentrons/protocol_engine/test_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_protocol_engine.py index 8af30d521df..1d9839af871 100644 --- a/api/tests/opentrons/protocol_engine/test_protocol_engine.py +++ b/api/tests/opentrons/protocol_engine/test_protocol_engine.py @@ -780,6 +780,7 @@ async def test_estop_during_command( error_id=error_id, failed_at=timestamp, error=EStopActivatedError(message="Estop Activated"), + notes=[], type=ErrorRecoveryType.FAIL_RUN, ) expected_action_2 = FailCommandAction( @@ -787,6 +788,7 @@ async def test_estop_during_command( error_id=error_id, failed_at=timestamp, error=EStopActivatedError(message="Estop Activated"), + notes=[], type=ErrorRecoveryType.FAIL_RUN, ) diff --git a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py index 56af939bb16..4ae20ce87dd 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py @@ -159,6 +159,7 @@ def test_map_after_with_error_command() -> None: LegacyContextCommandError, match="oh no", ), + notes=[], type=ErrorRecoveryType.FAIL_RUN, ) ] @@ -253,6 +254,7 @@ def test_command_stack() -> None: error_id=matchers.IsA(str), failed_at=matchers.IsA(datetime), error=matchers.ErrorMatching(LegacyContextCommandError, "oh no"), + notes=[], type=ErrorRecoveryType.FAIL_RUN, ), ] From e012b78b41523a09dfd6ec00435e6346e3e3f9b4 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 13:02:52 -0400 Subject: [PATCH 16/24] Leave todo comment for maintenance_run arg. --- api/src/opentrons/protocol_engine/protocol_engine.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index 3fedb61c000..5056385dea5 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -242,7 +242,13 @@ async def add_and_execute_command( await self.wait_for_command(command.id) return self._state_store.commands.get(command.id) - def estop(self, maintenance_run: bool) -> None: + def estop( + self, + # TODO(mm, 2024-03-26): Maintenance runs are a robot-server concept that + # ProtocolEngine should not have to know about. Can this be simplified or + # defined in other terms? + maintenance_run: bool, + ) -> None: """Signal to the engine that an estop event occurred. If there are any queued commands for the engine, they will be marked From 308c5bc8f91358099933333a43f3bc4d5359c319 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 14:01:36 -0400 Subject: [PATCH 17/24] Forgot my keys. --- api/src/opentrons/protocol_runner/legacy_command_mapper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index c5dd6b80113..ae853b94d6d 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -624,7 +624,7 @@ def _map_labware_load( command_id=succeeded_command.id, created_at=succeeded_command.createdAt, request=pe_commands.LoadLabwareCreate.construct( - params=succeeded_command.params + key=succeeded_command.key, params=succeeded_command.params ), request_hash=None, ) @@ -686,7 +686,7 @@ def _map_instrument_load( command_id=succeeded_command.id, created_at=succeeded_command.createdAt, request=pe_commands.LoadPipetteCreate.construct( - params=succeeded_command.params + key=succeeded_command.key, params=succeeded_command.params ), request_hash=None, ) @@ -751,7 +751,7 @@ def _map_module_load( command_id=succeeded_command.id, created_at=succeeded_command.createdAt, request=pe_commands.LoadModuleCreate.construct( - params=succeeded_command.params + key=succeeded_command.key, params=succeeded_command.params ), request_hash=None, ) From f173f7bcefe738b21e704c46933ba203c49b0665 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 14:38:20 -0400 Subject: [PATCH 18/24] Set notes=[] for succeeded commands in LegacyCommandMapper. This keeps succeeded commands consistent with failed commands. --- .../protocol_runner/legacy_command_mapper.py | 10 ++++++ .../smoke_tests/test_legacy_command_mapper.py | 32 +++++++++++++++++++ .../test_legacy_module_commands.py | 6 ++++ .../smoke_tests/test_protocol_runner.py | 4 +++ 4 files changed, 52 insertions(+) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index ae853b94d6d..53846baf653 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -180,6 +180,7 @@ def map_command( # noqa: C901 ), "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) elif isinstance(running_command, pe_commands.DropTip): @@ -190,6 +191,7 @@ def map_command( # noqa: C901 ), "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) elif isinstance(running_command, pe_commands.Aspirate): @@ -203,6 +205,7 @@ def map_command( # noqa: C901 ), "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) elif isinstance(running_command, pe_commands.Dispense): @@ -216,6 +219,7 @@ def map_command( # noqa: C901 ), "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) elif isinstance(running_command, pe_commands.BlowOut): @@ -226,6 +230,7 @@ def map_command( # noqa: C901 ), "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) elif isinstance(running_command, pe_commands.Custom): @@ -234,6 +239,7 @@ def map_command( # noqa: C901 "result": pe_commands.CustomResult.construct(), "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) else: @@ -241,6 +247,7 @@ def map_command( # noqa: C901 update={ "status": pe_commands.CommandStatus.SUCCEEDED, "completedAt": now, + "notes": [], } ) results.append( @@ -612,6 +619,7 @@ def _map_labware_load( version=labware_load_info.labware_version, displayName=labware_load_info.labware_display_name, ), + notes=[], result=pe_commands.LoadLabwareResult.construct( labwareId=labware_id, definition=LabwareDefinition.parse_obj( @@ -672,6 +680,7 @@ def _map_instrument_load( pipetteName=PipetteNameType(instrument_load_info.instrument_load_name), mount=mount, ), + notes=[], result=pe_commands.LoadPipetteResult.construct(pipetteId=pipette_id), ) serial = instrument_load_info.pipette_dict.get("pipette_id", None) or "" @@ -740,6 +749,7 @@ def _map_module_load( ), moduleId=module_id, ), + notes=[], result=pe_commands.LoadModuleResult.construct( moduleId=module_id, serialNumber=module_load_info.module_serial, diff --git a/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py index 5baf821ca91..5d6595227b9 100644 --- a/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py @@ -165,6 +165,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=commands.HomeParams(axes=None), + notes=[], result=commands.HomeResult(), ) assert commands_result[1] == commands.LoadLabware.construct( @@ -180,6 +181,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: namespace="opentrons", version=1, ), + notes=[], result=tiprack_1_result_captor, ) assert commands_result[2] == commands.LoadLabware.construct( @@ -195,6 +197,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: namespace="opentrons", version=1, ), + notes=[], result=tiprack_2_result_captor, ) assert commands_result[3] == commands.LoadModule.construct( @@ -209,6 +212,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4), moduleId="module-0", ), + notes=[], result=module_1_result_captor, ) assert commands_result[4] == commands.LoadLabware.construct( @@ -224,6 +228,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: namespace="opentrons", version=1, ), + notes=[], result=well_plate_1_result_captor, ) assert commands_result[5] == commands.LoadLabware.construct( @@ -239,6 +244,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: namespace="opentrons", version=1, ), + notes=[], result=module_plate_1_result_captor, ) @@ -252,6 +258,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: params=commands.LoadPipetteParams( pipetteName=PipetteNameType.P300_SINGLE, mount=MountType.LEFT ), + notes=[], result=pipette_left_result_captor, ) @@ -265,6 +272,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: params=commands.LoadPipetteParams( pipetteName=PipetteNameType.P300_MULTI, mount=MountType.RIGHT ), + notes=[], result=pipette_right_result_captor, ) @@ -289,6 +297,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: labwareId=tiprack_1_id, wellName="A1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, position=DeckPoint(x=0, y=0, z=0) ), @@ -305,6 +314,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: labwareId=tiprack_2_id, wellName="A1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, position=DeckPoint(x=0, y=0, z=0) ), @@ -322,6 +332,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: labwareId="fixedTrash", wellName="A1", ), + notes=[], result=commands.DropTipResult(position=DeckPoint(x=0, y=0, z=0)), ) @@ -337,6 +348,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: labwareId=tiprack_1_id, wellName="B1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, position=DeckPoint(x=0, y=0, z=0) ), @@ -355,6 +367,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=40, flowRate=150, ), + notes=[], result=commands.AspirateResult(volume=40, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[13] == commands.Dispense.construct( @@ -371,6 +384,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=35, flowRate=360, ), + notes=[], result=commands.DispenseResult(volume=35, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[14] == commands.Aspirate.construct( @@ -387,6 +401,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=40, flowRate=150.0, ), + notes=[], result=commands.AspirateResult(volume=40, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[15] == commands.Dispense.construct( @@ -403,6 +418,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=35, flowRate=300, ), + notes=[], result=commands.DispenseResult(volume=35, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[16] == commands.BlowOut.construct( @@ -418,6 +434,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: wellName="B1", flowRate=1000.0, ), + notes=[], result=commands.BlowOutResult(position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[17] == commands.Aspirate.construct( @@ -434,6 +451,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=50, flowRate=150, ), + notes=[], result=commands.AspirateResult(volume=50, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[18] == commands.Dispense.construct( @@ -450,6 +468,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=50, flowRate=300, ), + notes=[], result=commands.DispenseResult(volume=50, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[19] == commands.BlowOut.construct( @@ -465,6 +484,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: wellName="B1", flowRate=1000.0, ), + notes=[], result=commands.BlowOutResult(position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[20] == commands.Aspirate.construct( @@ -481,6 +501,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=300, flowRate=150, ), + notes=[], result=commands.AspirateResult(volume=300, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[21] == commands.Dispense.construct( @@ -497,6 +518,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=300, flowRate=300, ), + notes=[], result=commands.DispenseResult(volume=300, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[22] == commands.BlowOut.construct( @@ -512,6 +534,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: wellName="B1", flowRate=1000.0, ), + notes=[], result=commands.BlowOutResult(position=DeckPoint(x=0, y=0, z=0)), ) # TODO:(jr, 15.08.2022): this should map to move_to when move_to is mapped in a followup ticket RSS-62 @@ -526,6 +549,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: legacyCommandText="Moving to (100, 100, 10)", legacyCommandType="command.MOVE_TO", ), + notes=[], result=commands.CustomResult(), ) # TODO:(jr, 15.08.2022): aspirate commands with no labware get filtered @@ -541,6 +565,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: legacyCommandText="Aspirating 300.0 uL from (100, 100, 10) at 150.0 uL/sec", legacyCommandType="command.ASPIRATE", ), + notes=[], result=commands.CustomResult(), ) # TODO:(jr, 15.08.2022): dispense commands with no labware get filtered @@ -556,6 +581,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: legacyCommandText="Dispensing 300.0 uL into (100, 100, 10) at 300.0 uL/sec", legacyCommandType="command.DISPENSE", ), + notes=[], result=commands.CustomResult(), ) # TODO:(jr, 15.08.2022): blow_out commands with no labware get filtered @@ -571,6 +597,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: legacyCommandText="Blowing out at (100, 100, 10)", legacyCommandType="command.BLOW_OUT", ), + notes=[], result=commands.CustomResult(), ) assert commands_result[27] == commands.Aspirate.construct( @@ -587,6 +614,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=50, flowRate=150, ), + notes=[], result=commands.AspirateResult(volume=50, position=DeckPoint(x=0, y=0, z=0)), ) assert commands_result[28] == commands.Dispense.construct( @@ -603,6 +631,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: volume=50, flowRate=300, ), + notes=[], result=commands.DispenseResult(volume=50, position=DeckPoint(x=0, y=0, z=0)), ) # TODO:(jr, 15.08.2022): aspirate commands with no labware get filtered @@ -618,6 +647,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: legacyCommandText="Aspirating 50.0 uL from Opentrons 96 Well Aluminum Block with NEST Well Plate 100 µL on 3 at 150.0 uL/sec", legacyCommandType="command.ASPIRATE", ), + notes=[], result=commands.CustomResult(), ) # TODO:(jr, 15.08.2022): dispense commands with no labware get filtered @@ -633,6 +663,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: legacyCommandText="Dispensing 50.0 uL into Opentrons 96 Well Aluminum Block with NEST Well Plate 100 µL on 3 at 300.0 uL/sec", legacyCommandType="command.DISPENSE", ), + notes=[], result=commands.CustomResult(), ) assert commands_result[31] == commands.DropTip.construct( @@ -647,6 +678,7 @@ async def test_big_protocol_commands(big_protocol_file: Path) -> None: labwareId=tiprack_1_id, wellName="A1", ), + notes=[], result=commands.DropTipResult(position=DeckPoint(x=0, y=0, z=0)), ) diff --git a/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_module_commands.py b/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_module_commands.py index b7f80506593..afc9c500c29 100644 --- a/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_module_commands.py +++ b/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_module_commands.py @@ -82,6 +82,7 @@ async def test_runner_with_modules_in_legacy_python( startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=commands.HomeParams(axes=None), + notes=[], result=commands.HomeResult(), ) assert commands_result[1] == commands.LoadLabware.construct( @@ -92,6 +93,7 @@ async def test_runner_with_modules_in_legacy_python( startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=matchers.Anything(), + notes=[], result=matchers.Anything(), ) @@ -103,6 +105,7 @@ async def test_runner_with_modules_in_legacy_python( startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=matchers.Anything(), + notes=[], result=temp_module_result_captor, ) @@ -114,6 +117,7 @@ async def test_runner_with_modules_in_legacy_python( startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=matchers.Anything(), + notes=[], result=mag_module_result_captor, ) @@ -125,6 +129,7 @@ async def test_runner_with_modules_in_legacy_python( startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=matchers.Anything(), + notes=[], result=thermocycler_result_captor, ) @@ -136,6 +141,7 @@ async def test_runner_with_modules_in_legacy_python( startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=matchers.Anything(), + notes=[], result=heater_shaker_result_captor, ) diff --git a/api/tests/opentrons/protocol_runner/smoke_tests/test_protocol_runner.py b/api/tests/opentrons/protocol_runner/smoke_tests/test_protocol_runner.py index 21aecc7a546..7253a6e2f91 100644 --- a/api/tests/opentrons/protocol_runner/smoke_tests/test_protocol_runner.py +++ b/api/tests/opentrons/protocol_runner/smoke_tests/test_protocol_runner.py @@ -96,6 +96,7 @@ async def test_runner_with_python( labwareId=labware_id_captor.value, wellName="A1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, @@ -157,6 +158,7 @@ async def test_runner_with_json(json_protocol_file: Path) -> None: labwareId="labware-id", wellName="A1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, @@ -224,6 +226,7 @@ async def test_runner_with_legacy_python(legacy_python_protocol_file: Path) -> N labwareId=labware_id_captor.value, wellName="A1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, position=DeckPoint(x=0, y=0, z=0) ), @@ -288,6 +291,7 @@ async def test_runner_with_legacy_json(legacy_json_protocol_file: Path) -> None: labwareId=labware_id_captor.value, wellName="A1", ), + notes=[], result=commands.PickUpTipResult( tipVolume=300.0, tipLength=51.83, position=DeckPoint(x=0, y=0, z=0) ), From cb3d655aeb85d5304c51856b6237625b0625a0cf Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 15:06:55 -0400 Subject: [PATCH 19/24] Update integration tests to expect `notes: []` on completed commands. --- .../protocols/test_v6_json_upload.tavern.yaml | 31 ++++++++++-- .../test_v8_json_upload_flex.tavern.yaml | 39 ++++++++++++--- .../test_v8_json_upload_ot2.tavern.yaml | 48 +++++++++++++++---- .../test_json_v6_protocol_run.tavern.yaml | 22 ++++++++- .../runs/test_json_v6_run_failure.tavern.yaml | 3 +- .../test_json_v7_protocol_run.tavern.yaml | 22 +++++++-- .../runs/test_papi_v2_run_failure.tavern.yaml | 1 + .../runs/test_protocol_run.tavern.yaml | 5 +- ...t_run_queued_protocol_commands.tavern.yaml | 4 ++ 9 files changed, 149 insertions(+), 26 deletions(-) diff --git a/robot-server/tests/integration/http_api/protocols/test_v6_json_upload.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_v6_json_upload.tavern.yaml index f2d17aff265..710a693e595 100644 --- a/robot-server/tests/integration/http_api/protocols/test_v6_json_upload.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_v6_json_upload.tavern.yaml @@ -68,7 +68,7 @@ stages: analysisSummaries: - id: '{analysis_id}' status: completed - links: + links: referencingRuns: [] - name: Get protocol analysis by ID request: @@ -132,8 +132,9 @@ stages: commandType: home key: !anystr status: succeeded - params: { } - result: { } + params: {} + result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -147,6 +148,7 @@ stages: pipetteId: pipetteId result: pipetteId: pipetteId + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -164,6 +166,7 @@ stages: definition: !anydict model: magneticModuleV2 serialNumber: !anystr + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -181,6 +184,7 @@ stages: definition: !anydict model: temperatureModuleV2 serialNumber: !anystr + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -199,6 +203,7 @@ stages: result: labwareId: sourcePlateId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -217,6 +222,7 @@ stages: result: labwareId: destPlateId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -235,6 +241,7 @@ stages: result: labwareId: tipRackId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -249,6 +256,7 @@ stages: A1: 100 B1: 100 result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -258,6 +266,7 @@ stages: status: succeeded params: {} result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -280,6 +289,7 @@ stages: tipVolume: 10.0 tipLength: 35.910000000000004 tipDiameter: 3.27 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -302,6 +312,7 @@ stages: result: position: { 'x': 16.76, 'y': 75.28, 'z': 157.09 } volume: 5 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -312,6 +323,7 @@ stages: params: seconds: 42 result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -334,6 +346,7 @@ stages: result: position: { 'x': 284.635, 'y': 56.025, 'z': 158.25 } volume: 4.5 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -355,6 +368,7 @@ stages: speed: 42.0 result: position: { 'x': 284.635, 'y': 56.025, 'z': 168.25 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -375,6 +389,7 @@ stages: flowRate: 2 result: position: { 'x': 284.635, 'y': 56.025, 'z': 169.25 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -396,6 +411,7 @@ stages: result: position: { 'x': 304.52500000000003, 'y': 56.025, 'z': 182.25 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -419,6 +435,7 @@ stages: result: position: { 'x': 306.52500000000003, 'y': 59.025, 'z': 167.25 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -439,6 +456,7 @@ stages: alternateDropLocation: false result: position: { 'x': 347.84000000000003, 'y': 325.06, 'z': 82.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -449,6 +467,7 @@ stages: params: message: pause command result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -467,6 +486,7 @@ stages: speed: 12.3 result: position: { 'x': 0.0, 'y': 0.0, 'z': 0.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -483,6 +503,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -499,6 +520,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -515,6 +537,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -531,6 +554,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -548,6 +572,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr errors: [] diff --git a/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_flex.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_flex.tavern.yaml index a592d757baf..991d88df87f 100644 --- a/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_flex.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_flex.tavern.yaml @@ -69,7 +69,7 @@ stages: analysisSummaries: - id: '{analysis_id}' status: completed - links: + links: referencingRuns: [] - name: Get protocol analysis by ID request: @@ -134,8 +134,9 @@ stages: commandType: home key: !anystr status: succeeded - params: { } - result: { } + params: {} + result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -149,6 +150,7 @@ stages: pipetteId: pipetteId result: pipetteId: pipetteId + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -166,6 +168,7 @@ stages: definition: !anydict model: magneticModuleV2 serialNumber: !anystr + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -183,6 +186,7 @@ stages: definition: !anydict model: temperatureModuleV2 serialNumber: !anystr + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -201,6 +205,7 @@ stages: result: labwareId: sourcePlateId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -219,6 +224,7 @@ stages: result: labwareId: destPlateId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -237,6 +243,7 @@ stages: result: labwareId: tipRackId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -255,6 +262,7 @@ stages: result: labwareId: fixedTrash definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -269,6 +277,7 @@ stages: A1: 100.0 B1: 100.0 result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -278,6 +287,7 @@ stages: status: succeeded params: {} result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -300,6 +310,7 @@ stages: tipVolume: 1000.0 tipLength: 77.5 tipDiameter: 7.23 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -322,6 +333,7 @@ stages: result: position: { 'x': 14.38, 'y': 74.24, 'z': 12.05 } volume: 5.0 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -332,6 +344,7 @@ stages: params: seconds: 42.0 result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -354,6 +367,7 @@ stages: result: position: { 'x': 341.205, 'y': 65.115, 'z': 84.3 } volume: 4.5 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -375,6 +389,7 @@ stages: speed: 42.0 result: position: { 'x': 341.205, 'y': 65.115, 'z': 94.3 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -395,6 +410,7 @@ stages: flowRate: 2.0 result: position: { 'x': 341.205, 'y': 65.115, 'z': 95.3 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -411,6 +427,7 @@ stages: forceDirect: false result: position: { 'x': 100.0, 'y': 100.0, 'z': 100.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -431,8 +448,8 @@ stages: forceDirect: false speed: 12.3 result: - position: - { 'x': 350.205, 'y': 65.115, 'z': 98.25 } + position: { 'x': 350.205, 'y': 65.115, 'z': 98.25 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -453,8 +470,8 @@ stages: minimumZHeight: 35.0 forceDirect: true result: - position: - { 'x': 352.205, 'y': 68.115, 'z': 93.3 } + position: { 'x': 352.205, 'y': 68.115, 'z': 93.3 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -475,6 +492,7 @@ stages: alternateDropLocation: false result: position: { 'x': 410.84000000000003, 'y': 374.56, 'z': 82.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -485,6 +503,7 @@ stages: params: message: pause command result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -502,6 +521,7 @@ stages: forceDirect: true result: position: { 'x': 0.0, 'y': 0.0, 'z': 0.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -518,6 +538,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -534,6 +555,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -550,6 +572,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -566,6 +589,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -583,6 +607,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr errors: [] diff --git a/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_ot2.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_ot2.tavern.yaml index afc1644afbd..66a896ec3dd 100644 --- a/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_ot2.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_v8_json_upload_ot2.tavern.yaml @@ -68,7 +68,7 @@ stages: analysisSummaries: - id: '{analysis_id}' status: completed - links: + links: referencingRuns: [] - name: Get protocol analysis by ID request: @@ -133,8 +133,9 @@ stages: commandType: home key: !anystr status: succeeded - params: { } - result: { } + params: {} + result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -148,6 +149,7 @@ stages: pipetteId: pipetteId result: pipetteId: pipetteId + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -165,6 +167,7 @@ stages: definition: !anydict model: magneticModuleV2 serialNumber: !anystr + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -182,6 +185,7 @@ stages: definition: !anydict model: temperatureModuleV2 serialNumber: !anystr + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -200,6 +204,7 @@ stages: result: labwareId: sourcePlateId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -218,6 +223,7 @@ stages: result: labwareId: destPlateId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -236,6 +242,7 @@ stages: result: labwareId: tipRackId definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -254,6 +261,7 @@ stages: result: labwareId: fixedTrash definition: !anydict + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -268,6 +276,7 @@ stages: A1: 100.0 B1: 100.0 result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -277,6 +286,7 @@ stages: status: succeeded params: {} result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -299,6 +309,7 @@ stages: tipVolume: 10.0 tipLength: 35.910000000000004 tipDiameter: 3.27 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -319,8 +330,14 @@ stages: volume: 5.0 flowRate: 3.0 result: - position: { 'x': 12.930000000000001, 'y': 74.08999999999999, 'z': 83.14 } + position: + { + 'x': 12.930000000000001, + 'y': 74.08999999999999, + 'z': 83.14, + } volume: 5.0 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -331,6 +348,7 @@ stages: params: seconds: 42.0 result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -353,6 +371,7 @@ stages: result: position: { 'x': 280.805, 'y': 65.115, 'z': 84.3 } volume: 4.5 + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -373,6 +392,7 @@ stages: radius: 1.0 result: position: { 'x': 280.805, 'y': 65.115, 'z': 94.3 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -393,6 +413,7 @@ stages: flowRate: 2.0 result: position: { 'x': 280.805, 'y': 65.115, 'z': 95.3 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -408,7 +429,8 @@ stages: z: 100.0 forceDirect: false result: - position: {'x': 100.0, 'y': 100.0, 'z': 100.0} + position: { 'x': 100.0, 'y': 100.0, 'z': 100.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -428,8 +450,8 @@ stages: z: 0 forceDirect: false result: - position: - { 'x': 289.805, 'y': 65.115, 'z': 98.25 } + position: { 'x': 289.805, 'y': 65.115, 'z': 98.25 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -450,8 +472,8 @@ stages: minimumZHeight: 35.0 forceDirect: true result: - position: - { 'x': 291.805, 'y': 68.115, 'z': 93.3 } + position: { 'x': 291.805, 'y': 68.115, 'z': 93.3 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -472,6 +494,7 @@ stages: alternateDropLocation: false result: position: { 'x': 347.84000000000003, 'y': 325.06, 'z': 82.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -482,6 +505,7 @@ stages: params: message: pause command result: {} + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -499,6 +523,7 @@ stages: forceDirect: true result: position: { 'x': 0.0, 'y': 0.0, 'z': 0.0 } + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -515,6 +540,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -531,6 +557,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -547,6 +574,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -563,6 +591,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr - id: !anystr @@ -580,6 +609,7 @@ stages: x: !anyfloat y: !anyfloat z: !anyfloat + notes: [] startedAt: !anystr completedAt: !anystr errors: [] diff --git a/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml index 1118d9a8870..1e7d7e20be4 100644 --- a/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_json_v6_protocol_run.tavern.yaml @@ -104,7 +104,7 @@ stages: commandType: home createdAt: !anystr status: queued - params: { } + params: {} - id: !anystr key: !anystr commandType: loadPipette @@ -292,6 +292,7 @@ stages: completedAt: '{setup_command_completed_at}' status: succeeded params: {} + notes: [] - name: Play the run request: @@ -347,7 +348,8 @@ stages: commandType: home key: !anystr status: succeeded - params: { } + notes: [] + params: {} startedAt: !anystr completedAt: !anystr - id: !anystr @@ -357,6 +359,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteName: p10_single mount: left @@ -368,6 +371,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: model: magneticModuleV1 location: @@ -380,6 +384,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: model: temperatureModuleV2 location: @@ -392,6 +397,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: moduleId: temperatureModuleId @@ -407,6 +413,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: moduleId: magneticModuleId @@ -422,6 +429,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: slotName: '8' @@ -437,6 +445,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: labwareId: sourcePlateId liquidId: waterId @@ -450,6 +459,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: tipRackId @@ -467,6 +477,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: sourcePlateId @@ -486,6 +497,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: destPlateId @@ -505,6 +517,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: destPlateId @@ -523,6 +536,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: destPlateId @@ -543,6 +557,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: fixedTrash @@ -562,6 +577,7 @@ stages: startedAt: '{setup_command_started_at}' completedAt: '{setup_command_completed_at}' status: succeeded + notes: [] params: {} - name: Verify commands succeeded with pageLength and cursor @@ -591,6 +607,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: moduleId: magneticModuleId @@ -606,6 +623,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: slotName: '8' diff --git a/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml index d9266dff9b0..46eccbae280 100644 --- a/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_json_v6_run_failure.tavern.yaml @@ -69,7 +69,7 @@ stages: - id: !anystr createdAt: !anystr errorCode: '3005' - errorType: TipNotAttachedError + errorType: TipNotAttachedError detail: Pipette should have a tip attached, but does not. errorInfo: !anydict wrappedErrors: !anylist @@ -100,6 +100,7 @@ stages: startedAt: !anystr completedAt: !anystr status: failed + notes: [] error: id: !anystr errorType: TipNotAttachedError diff --git a/robot-server/tests/integration/http_api/runs/test_json_v7_protocol_run.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_json_v7_protocol_run.tavern.yaml index ace0c47cf64..089b5f30c03 100644 --- a/robot-server/tests/integration/http_api/runs/test_json_v7_protocol_run.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_json_v7_protocol_run.tavern.yaml @@ -104,7 +104,7 @@ stages: commandType: home createdAt: !anystr status: queued - params: { } + params: {} - id: !anystr key: !anystr commandType: loadPipette @@ -291,6 +291,7 @@ stages: startedAt: '{setup_command_started_at}' completedAt: '{setup_command_completed_at}' status: succeeded + notes: [] params: {} - name: Play the run @@ -349,7 +350,8 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded - params: { } + notes: [] + params: {} - id: !anystr key: !anystr commandType: loadPipette @@ -357,6 +359,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteName: p10_single mount: left @@ -368,6 +371,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: model: magneticModuleV1 location: @@ -380,6 +384,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: model: temperatureModuleV2 location: @@ -392,6 +397,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: moduleId: temperatureModuleId @@ -407,6 +413,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: moduleId: magneticModuleId @@ -422,6 +429,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: location: slotName: '8' @@ -437,6 +445,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: labwareId: sourcePlateId liquidId: waterId @@ -450,6 +459,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: tipRackId @@ -467,6 +477,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: sourcePlateId @@ -486,6 +497,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: destPlateId @@ -505,6 +517,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: destPlateId @@ -523,6 +536,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: destPlateId @@ -543,6 +557,7 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded + notes: [] params: pipetteId: pipetteId labwareId: fixedTrash @@ -562,4 +577,5 @@ stages: startedAt: '{setup_command_started_at}' completedAt: '{setup_command_completed_at}' status: succeeded - params: { } + notes: [] + params: {} diff --git a/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml index f7f54b8ac3e..bf2af00ac10 100644 --- a/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_papi_v2_run_failure.tavern.yaml @@ -101,6 +101,7 @@ stages: startedAt: !anystr completedAt: !anystr status: failed + notes: [] error: id: !anystr errorType: LegacyContextCommandError diff --git a/robot-server/tests/integration/http_api/runs/test_protocol_run.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_protocol_run.tavern.yaml index 5064dd28202..48dc570d6c9 100644 --- a/robot-server/tests/integration/http_api/runs/test_protocol_run.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_protocol_run.tavern.yaml @@ -171,11 +171,13 @@ stages: startedAt: !anystr completedAt: !anystr status: succeeded - params: { } + notes: [] + params: {} - id: !anystr key: !anystr commandType: loadLabware status: succeeded + notes: [] params: location: slotName: '1' @@ -201,6 +203,7 @@ stages: key: !anystr commandType: loadLabware status: succeeded + notes: [] params: location: slotName: '1' diff --git a/robot-server/tests/integration/http_api/runs/test_run_queued_protocol_commands.tavern.yaml b/robot-server/tests/integration/http_api/runs/test_run_queued_protocol_commands.tavern.yaml index 3d252ac5244..cc8cea69356 100644 --- a/robot-server/tests/integration/http_api/runs/test_run_queued_protocol_commands.tavern.yaml +++ b/robot-server/tests/integration/http_api/runs/test_run_queued_protocol_commands.tavern.yaml @@ -158,6 +158,7 @@ stages: commandType: comment status: succeeded intent: protocol + notes: [] params: message: 'test 1' createdAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" @@ -168,6 +169,7 @@ stages: commandType: comment status: succeeded intent: protocol + notes: [] params: message: 'test 2' createdAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" @@ -178,6 +180,7 @@ stages: commandType: comment status: succeeded intent: protocol + notes: [] params: message: 'test 3' createdAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" @@ -188,6 +191,7 @@ stages: commandType: comment status: succeeded intent: protocol + notes: [] params: message: 'test 4' createdAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" From 7c48aca93ad672b3262b727ab7480cc146cecaf8 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 18:16:39 -0400 Subject: [PATCH 20/24] I'm sorry, Mike. --- .../protocol_runner/test_protocol_runner.py | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/api/tests/opentrons/protocol_runner/test_protocol_runner.py b/api/tests/opentrons/protocol_runner/test_protocol_runner.py index 64034e663bd..de58162bc33 100644 --- a/api/tests/opentrons/protocol_runner/test_protocol_runner.py +++ b/api/tests/opentrons/protocol_runner/test_protocol_runner.py @@ -478,20 +478,27 @@ async def test_load_legacy_python( run_time_param_values=None, ) + run_func_captor = matchers.Captor() + decoy.verify( protocol_engine.add_labware_definition(labware_definition), protocol_engine.add_plugin(matchers.IsA(LegacyContextPlugin)), - protocol_engine.add_command( + task_queue.set_run_func(run_func_captor), + ) + + assert broker_captor.value is legacy_python_runner_subject.broker + + # Verify that the run func calls the right things: + run_func = run_func_captor.value + await run_func() + decoy.verify( + await protocol_engine.add_and_execute_command( request=pe_commands.HomeCreate(params=pe_commands.HomeParams(axes=None)) ), - task_queue.set_run_func( - func=legacy_executor.execute, - protocol=legacy_protocol, - context=legacy_context, - run_time_param_values=None, + await legacy_executor.execute( + protocol=legacy_protocol, context=legacy_context, run_time_param_values=None ), ) - assert broker_captor.value is legacy_python_runner_subject.broker async def test_load_python_with_pe_papi_core( @@ -612,17 +619,23 @@ async def test_load_legacy_json( run_time_param_values=None, ) + run_func_captor = matchers.Captor() + decoy.verify( protocol_engine.add_labware_definition(labware_definition), protocol_engine.add_plugin(matchers.IsA(LegacyContextPlugin)), - protocol_engine.add_command( + task_queue.set_run_func(run_func_captor), + ) + + # Verify that the run func calls the right things: + run_func = run_func_captor.value + await run_func() + decoy.verify( + await protocol_engine.add_and_execute_command( request=pe_commands.HomeCreate(params=pe_commands.HomeParams(axes=None)) ), - task_queue.set_run_func( - func=legacy_executor.execute, - protocol=legacy_protocol, - context=legacy_context, - run_time_param_values=None, + await legacy_executor.execute( + protocol=legacy_protocol, context=legacy_context, run_time_param_values=None ), ) From 32543f84b7d8a14ec3860419c4172466c9888926 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 18:32:42 -0400 Subject: [PATCH 21/24] Update test_legacy_context_plugin.py. --- .../opentrons/protocol_runner/test_legacy_context_plugin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py b/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py index 56eb58c84c0..1f7de8388ca 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py @@ -219,7 +219,9 @@ async def test_equipment_broker_messages( decoy.when( mock_legacy_command_mapper.map_equipment_load(load_info=load_info) - ).then_return((engine_command, None)) + ).then_return( + [pe_actions.SucceedCommandAction(command=engine_command, private_result=None)] + ) await to_thread.run_sync(handler, load_info) From ec58e46eda9b0e24411527cdf5a228c9aeabbca9 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 26 Mar 2024 13:59:12 -0400 Subject: [PATCH 22/24] Update legacy command mapper unit tests. --- .../test_legacy_command_mapper.py | 317 +++++++++++------- 1 file changed, 203 insertions(+), 114 deletions(-) diff --git a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py index 4ae20ce87dd..8a8ec50b779 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py @@ -70,20 +70,22 @@ def test_map_before_command() -> None: result = subject.map_command(legacy_command) assert result == [ - pe_actions.SucceedCommandAction( - private_result=None, - command=pe_commands.Custom.construct( - id="command.COMMENT-0", + pe_actions.QueueCommandAction( + command_id="command.COMMENT-0", + created_at=matchers.IsA(datetime), + request=pe_commands.CustomCreate( key="command.COMMENT-0", - status=pe_commands.CommandStatus.RUNNING, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), params=LegacyCommandParams( legacyCommandType="command.COMMENT", legacyCommandText="hello world", ), ), - ) + request_hash=None, + ), + pe_actions.RunCommandAction( + command_id="command.COMMENT-0", + started_at=matchers.IsA(datetime), + ), ] @@ -124,6 +126,7 @@ def test_map_after_command() -> None: legacyCommandText="hello world", ), result=pe_commands.CustomResult(), + notes=[], ), ) ] @@ -205,33 +208,35 @@ def test_command_stack() -> None: ] assert result == [ - pe_actions.SucceedCommandAction( - private_result=None, - command=pe_commands.Custom.construct( - id="command.COMMENT-0", + pe_actions.QueueCommandAction( + command_id="command.COMMENT-0", + created_at=matchers.IsA(datetime), + request=pe_commands.CustomCreate( key="command.COMMENT-0", - status=pe_commands.CommandStatus.RUNNING, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), params=LegacyCommandParams( legacyCommandType="command.COMMENT", legacyCommandText="hello", ), ), + request_hash=None, ), - pe_actions.SucceedCommandAction( - private_result=None, - command=pe_commands.Custom.construct( - id="command.COMMENT-1", + pe_actions.RunCommandAction( + command_id="command.COMMENT-0", started_at=matchers.IsA(datetime) + ), + pe_actions.QueueCommandAction( + command_id="command.COMMENT-1", + created_at=matchers.IsA(datetime), + request=pe_commands.CustomCreate( key="command.COMMENT-1", - status=pe_commands.CommandStatus.RUNNING, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), params=LegacyCommandParams( legacyCommandType="command.COMMENT", legacyCommandText="goodbye", ), ), + request_hash=None, + ), + pe_actions.RunCommandAction( + command_id="command.COMMENT-1", started_at=matchers.IsA(datetime) ), pe_actions.SucceedCommandAction( private_result=None, @@ -247,6 +252,7 @@ def test_command_stack() -> None: legacyCommandText="hello", ), result=pe_commands.CustomResult(), + notes=[], ), ), pe_actions.FailCommandAction( @@ -272,32 +278,55 @@ def test_map_labware_load(minimal_labware_def: LabwareDefinition) -> None: offset_id="labware-offset-id-123", labware_display_name="My special labware", ) - expected_output = pe_commands.LoadLabware.construct( - id=matchers.IsA(str), - key=matchers.IsA(str), - status=pe_commands.CommandStatus.SUCCEEDED, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), - completedAt=matchers.IsA(datetime), - params=pe_commands.LoadLabwareParams.construct( - location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1), - namespace="some_namespace", - loadName="some_load_name", - version=123, - displayName="My special labware", - labwareId=None, + + expected_id_and_key = "commands.LOAD_LABWARE-0" + expected_params = pe_commands.LoadLabwareParams( + location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1), + namespace="some_namespace", + loadName="some_load_name", + version=123, + displayName="My special labware", + labwareId=None, + ) + expected_queue = pe_actions.QueueCommandAction( + command_id=expected_id_and_key, + created_at=matchers.IsA(datetime), + request=pe_commands.LoadLabwareCreate( + key=expected_id_and_key, + params=expected_params, ), - result=pe_commands.LoadLabwareResult.construct( - labwareId=matchers.IsA(str), - # Trusting that the exact fields within in the labware definition - # get passed through correctly. - definition=matchers.Anything(), - offsetId="labware-offset-id-123", + request_hash=None, + ) + expected_run = pe_actions.RunCommandAction( + command_id=expected_id_and_key, + started_at=matchers.IsA(datetime), + ) + expected_succeed = pe_actions.SucceedCommandAction( + command=pe_commands.LoadLabware.construct( + id=expected_id_and_key, + key=expected_id_and_key, + params=expected_params, + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=matchers.IsA(datetime), + startedAt=matchers.IsA(datetime), + completedAt=matchers.IsA(datetime), + result=pe_commands.LoadLabwareResult.construct( + labwareId=matchers.IsA(str), + # Trusting that the exact fields within in the labware definition + # get passed through correctly. + definition=matchers.Anything(), + offsetId="labware-offset-id-123", + ), + notes=[], ), + private_result=None, ) - output = LegacyCommandMapper().map_equipment_load(input) - assert output[0] == expected_output - assert output[1] is None + result_queue, result_run, result_succeed = LegacyCommandMapper().map_equipment_load( + input + ) + assert result_queue == expected_queue + assert result_run == expected_run + assert result_succeed == expected_succeed def test_map_instrument_load(decoy: Decoy) -> None: @@ -314,33 +343,47 @@ def test_map_instrument_load(decoy: Decoy) -> None: pipette_data_provider.get_pipette_static_config(pipette_dict) ).then_return(pipette_config) + expected_id_and_key = "commands.LOAD_PIPETTE-0" + expected_params = pe_commands.LoadPipetteParams.construct( + pipetteName=PipetteNameType.P1000_SINGLE_GEN2, mount=MountType.LEFT + ) + expected_queue = pe_actions.QueueCommandAction( + command_id=expected_id_and_key, + created_at=matchers.IsA(datetime), + request=pe_commands.LoadPipetteCreate( + key=expected_id_and_key, params=expected_params + ), + request_hash=None, + ) + expected_run = pe_actions.RunCommandAction( + command_id=expected_id_and_key, started_at=matchers.IsA(datetime) + ) + expected_succeed = pe_actions.SucceedCommandAction( + command=pe_commands.LoadPipette.construct( + id=expected_id_and_key, + key=expected_id_and_key, + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=matchers.IsA(datetime), + startedAt=matchers.IsA(datetime), + completedAt=matchers.IsA(datetime), + params=expected_params, + result=pe_commands.LoadPipetteResult(pipetteId="pipette-0"), + notes=[], + ), + private_result=pe_commands.LoadPipettePrivateResult( + pipette_id="pipette-0", serial_number="fizzbuzz", config=pipette_config + ), + ) + [ result_queue, result_run, - result_complete, + result_succeed, ] = LegacyCommandMapper().map_equipment_load(input) - pipette_id_captor = matchers.Captor() - - assert isinstance(result_complete, pe_actions.SucceedCommandAction) - - assert result_complete.command == pe_commands.LoadPipette.construct( - id=matchers.IsA(str), - key=matchers.IsA(str), - status=pe_commands.CommandStatus.SUCCEEDED, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), - completedAt=matchers.IsA(datetime), - params=pe_commands.LoadPipetteParams.construct( - pipetteName=PipetteNameType.P1000_SINGLE_GEN2, mount=MountType.LEFT - ), - result=pe_commands.LoadPipetteResult.construct(pipetteId=pipette_id_captor), - ) - assert result_complete.private_result == pe_commands.LoadPipettePrivateResult( - pipette_id="pipette-0", - serial_number="fizzbuzz", - config=pipette_config, - ) + assert result_queue == expected_queue + assert result_run == expected_run + assert result_succeed == expected_succeed def test_map_module_load( @@ -361,30 +404,50 @@ def test_map_module_load( module_data_provider.get_definition(ModuleModel.TEMPERATURE_MODULE_V2) ).then_return(test_definition) - expected_output = pe_commands.LoadModule.construct( - id=matchers.IsA(str), - key=matchers.IsA(str), - status=pe_commands.CommandStatus.SUCCEEDED, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), - completedAt=matchers.IsA(datetime), - params=pe_commands.LoadModuleParams.construct( - model=ModuleModel.TEMPERATURE_MODULE_V1, - location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1), - moduleId=matchers.IsA(str), + expected_id_and_key = "commands.LOAD_MODULE-0" + expected_params = pe_commands.LoadModuleParams.construct( + model=ModuleModel.TEMPERATURE_MODULE_V1, + location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1), + moduleId=matchers.IsA(str), + ) + expected_queue = pe_actions.QueueCommandAction( + command_id=expected_id_and_key, + created_at=matchers.IsA(datetime), + request=pe_commands.LoadModuleCreate( + key=expected_id_and_key, params=expected_params ), - result=pe_commands.LoadModuleResult.construct( - moduleId=matchers.IsA(str), - serialNumber="module-serial", - definition=test_definition, - model=ModuleModel.TEMPERATURE_MODULE_V2, + request_hash=None, + ) + expected_run = pe_actions.RunCommandAction( + command_id=expected_id_and_key, started_at=matchers.IsA(datetime) + ) + expected_succeed = pe_actions.SucceedCommandAction( + command=pe_commands.LoadModule.construct( + id=expected_id_and_key, + key=expected_id_and_key, + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=matchers.IsA(datetime), + startedAt=matchers.IsA(datetime), + completedAt=matchers.IsA(datetime), + params=expected_params, + result=pe_commands.LoadModuleResult.construct( + moduleId=matchers.IsA(str), + serialNumber="module-serial", + definition=test_definition, + model=ModuleModel.TEMPERATURE_MODULE_V2, + ), + notes=[], ), + private_result=None, ) - output = LegacyCommandMapper( + + [result_queue, result_run, result_succeed] = LegacyCommandMapper( module_data_provider=module_data_provider ).map_equipment_load(input) - assert output[0] == expected_output - assert output[1] is None + + assert result_queue == expected_queue + assert result_run == expected_run + assert result_succeed == expected_succeed def test_map_module_labware_load(minimal_labware_def: LabwareDefinition) -> None: @@ -400,33 +463,56 @@ def test_map_module_labware_load(minimal_labware_def: LabwareDefinition) -> None offset_id="labware-offset-id-123", ) - expected_output = pe_commands.LoadLabware.construct( - id=matchers.IsA(str), - key=matchers.IsA(str), - status=pe_commands.CommandStatus.SUCCEEDED, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), - completedAt=matchers.IsA(datetime), - params=pe_commands.LoadLabwareParams.construct( - location=ModuleLocation(moduleId="module-123"), - namespace="some_namespace", - loadName="some_load_name", - version=123, - displayName="My very special module labware", - labwareId=None, + expected_id_and_key = "commands.LOAD_LABWARE-0" + expected_params = pe_commands.LoadLabwareParams.construct( + location=ModuleLocation(moduleId="module-123"), + namespace="some_namespace", + loadName="some_load_name", + version=123, + displayName="My very special module labware", + labwareId=None, + ) + expected_queue = pe_actions.QueueCommandAction( + command_id=expected_id_and_key, + created_at=matchers.IsA(datetime), + request=pe_commands.LoadLabwareCreate( + key=expected_id_and_key, + params=expected_params, ), - result=pe_commands.LoadLabwareResult.construct( - labwareId=matchers.IsA(str), - definition=matchers.Anything(), - offsetId="labware-offset-id-123", + request_hash=None, + ) + expected_run = pe_actions.RunCommandAction( + command_id="commands.LOAD_LABWARE-0", + started_at=matchers.IsA(datetime), + ) + expected_succeed = pe_actions.SucceedCommandAction( + command=pe_commands.LoadLabware.construct( + id=expected_id_and_key, + key=expected_id_and_key, + params=expected_params, + status=pe_commands.CommandStatus.SUCCEEDED, + createdAt=matchers.IsA(datetime), + startedAt=matchers.IsA(datetime), + completedAt=matchers.IsA(datetime), + result=pe_commands.LoadLabwareResult.construct( + labwareId=matchers.IsA(str), + # Trusting that the exact fields within in the labware definition + # get passed through correctly. + definition=matchers.Anything(), + offsetId="labware-offset-id-123", + ), + notes=[], ), + private_result=None, ) + subject = LegacyCommandMapper() subject._module_id_by_slot = {DeckSlotName.SLOT_1: "module-123"} - output = subject.map_equipment_load(load_input) + result_queue, result_run, result_succeed = subject.map_equipment_load(load_input) - assert output[0] == expected_output - assert output[1] is None + assert result_queue == expected_queue + assert result_run == expected_run + assert result_succeed == expected_succeed def test_map_pause() -> None: @@ -453,16 +539,18 @@ def test_map_pause() -> None: ] assert result == [ - pe_actions.SucceedCommandAction( - private_result=None, - command=pe_commands.WaitForResume.construct( - id="command.PAUSE-0", + pe_actions.QueueCommandAction( + command_id="command.PAUSE-0", + created_at=matchers.IsA(datetime), + request=pe_commands.WaitForResumeCreate( key="command.PAUSE-0", - status=pe_commands.CommandStatus.RUNNING, - createdAt=matchers.IsA(datetime), - startedAt=matchers.IsA(datetime), params=pe_commands.WaitForResumeParams(message="hello world"), ), + request_hash=None, + ), + pe_actions.RunCommandAction( + command_id="command.PAUSE-0", + started_at=matchers.IsA(datetime), ), pe_actions.SucceedCommandAction( private_result=None, @@ -474,6 +562,7 @@ def test_map_pause() -> None: startedAt=matchers.IsA(datetime), completedAt=matchers.IsA(datetime), params=pe_commands.WaitForResumeParams(message="hello world"), + notes=[], ), ), pe_actions.PauseAction(source=pe_actions.PauseSource.PROTOCOL), From 8c6f65d75bf9c888e3e015684b2f9e886a89f873 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 27 Mar 2024 02:30:59 -0400 Subject: [PATCH 23/24] Lint. --- .../opentrons/protocol_engine/execution/command_executor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/opentrons/protocol_engine/execution/command_executor.py b/api/src/opentrons/protocol_engine/execution/command_executor.py index b34f7028337..96ef93fd962 100644 --- a/api/src/opentrons/protocol_engine/execution/command_executor.py +++ b/api/src/opentrons/protocol_engine/execution/command_executor.py @@ -1,7 +1,7 @@ """Command side-effect execution logic container.""" import asyncio from logging import getLogger -from typing import Optional, List, Dict, Any, Protocol +from typing import Optional, List, Protocol from opentrons.hardware_control import HardwareControlAPI @@ -20,7 +20,6 @@ AbstractCommandImpl, CommandResult, CommandPrivateResult, - Command, ) from ..actions import ( ActionDispatcher, From 33e39e0b03c0cf61b5c0ff235d606e7c36487545 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Wed, 27 Mar 2024 12:35:20 -0400 Subject: [PATCH 24/24] Flow variables from queued->running->completed. --- .../protocol_engine/execution/command_executor.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/api/src/opentrons/protocol_engine/execution/command_executor.py b/api/src/opentrons/protocol_engine/execution/command_executor.py index 96ef93fd962..d44d37f5641 100644 --- a/api/src/opentrons/protocol_engine/execution/command_executor.py +++ b/api/src/opentrons/protocol_engine/execution/command_executor.py @@ -144,16 +144,16 @@ async def execute(self, command_id: str) -> None: try: log.debug( - f"Executing {queued_command.id}, {queued_command.commandType}, {queued_command.params}" + f"Executing {running_command.id}, {running_command.commandType}, {running_command.params}" ) if isinstance(command_impl, AbstractCommandImpl): - result: CommandResult = await command_impl.execute(queued_command.params) # type: ignore[arg-type] + result: CommandResult = await command_impl.execute(running_command.params) # type: ignore[arg-type] private_result: Optional[CommandPrivateResult] = None else: - result, private_result = await command_impl.execute(queued_command.params) # type: ignore[arg-type] + result, private_result = await command_impl.execute(running_command.params) # type: ignore[arg-type] except (Exception, asyncio.CancelledError) as error: - log.warning(f"Execution of {queued_command.id} failed", exc_info=error) + log.warning(f"Execution of {running_command.id} failed", exc_info=error) # TODO(mc, 2022-11-14): mark command as stopped rather than failed # https://opentrons.atlassian.net/browse/RCORE-390 if isinstance(error, asyncio.CancelledError): @@ -166,7 +166,7 @@ async def execute(self, command_id: str) -> None: self._action_dispatcher.dispatch( FailCommandAction( error=error, - command_id=command_id, + command_id=running_command.id, error_id=self._model_utils.generate_id(), failed_at=self._model_utils.get_timestamp(), notes=note_tracker.get_notes(), @@ -190,9 +190,9 @@ async def execute(self, command_id: str) -> None: "completedAt": self._model_utils.get_timestamp(), "notes": note_tracker.get_notes(), } - completed_command = running_command.copy(update=update) + succeeded_command = running_command.copy(update=update) self._action_dispatcher.dispatch( SucceedCommandAction( - command=completed_command, private_result=private_result + command=succeeded_command, private_result=private_result ), )