diff --git a/api/src/opentrons/protocol_engine/protocol_engine.py b/api/src/opentrons/protocol_engine/protocol_engine.py index 3c408828337..9155a6da678 100644 --- a/api/src/opentrons/protocol_engine/protocol_engine.py +++ b/api/src/opentrons/protocol_engine/protocol_engine.py @@ -241,7 +241,7 @@ def estop(self, maintenance_run: bool) -> None: if self._state_store.commands.get_is_stopped(): return current_id = ( - self._state_store.commands.state.running_command_id + self._state_store.commands.get_running_command_id() or self._state_store.commands.state.queued_command_ids.head(None) ) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index f143e8ccd08..6a93197ee4d 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -271,45 +271,30 @@ def handle_action(self, action: Action) -> None: # noqa: C901 error=action.error, ) prev_entry = self._state.commands_by_id[action.command_id] - self._state.commands_by_id[action.command_id] = CommandEntry( - index=prev_entry.index, - # TODO(mc, 2022-06-06): add new "cancelled" status or similar - # and don't set `completedAt` in commands other than the - # specific one that failed - command=prev_entry.command.copy( - update={ - "error": error_occurrence, - "completedAt": action.failed_at, - "status": CommandStatus.FAILED, - } - ), + # TODO(mc, 2022-06-06): add new "cancelled" status or similar + self._update_to_failed( + command_id=action.command_id, + failed_at=action.failed_at, + error_occurrence=error_occurrence, ) self._state.failed_command = self._state.commands_by_id[action.command_id] + if prev_entry.command.intent == CommandIntent.SETUP: - other_command_ids_to_fail = [ - *[i for i in self._state.queued_setup_command_ids], - ] + 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 + ) self._state.queued_setup_command_ids.clear() else: - other_command_ids_to_fail = [ - *[i for i in self._state.queued_command_ids], - ] + other_command_ids_to_fail = self._state.queued_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 + ) self._state.queued_command_ids.clear() - for command_id in other_command_ids_to_fail: - prev_entry = self._state.commands_by_id[command_id] - - self._state.commands_by_id[command_id] = CommandEntry( - index=prev_entry.index, - command=prev_entry.command.copy( - update={ - "completedAt": action.failed_at, - "status": CommandStatus.FAILED, - } - ), - ) - if self._state.running_command_id == action.command_id: self._state.running_command_id = None @@ -378,6 +363,24 @@ def handle_action(self, action: Action) -> None: # noqa: C901 elif action.door_state == DoorState.CLOSED: self._state.is_door_blocking = False + def _update_to_failed( + self, + command_id: str, + failed_at: datetime, + error_occurrence: Optional[ErrorOccurrence], + ) -> 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 {}), + } + ) + self._state.commands_by_id[command_id] = CommandEntry( + index=prev_entry.index, command=updated_command + ) + @staticmethod def _map_run_exception_to_error_occurrence( error_id: str, created_at: datetime, exception: Exception @@ -516,6 +519,10 @@ def get_error(self) -> Optional[ErrorOccurrence]: else: return run_error or finish_error + def get_running_command_id(self) -> Optional[str]: + """Return the ID of the command that's currently running, if there is one.""" + return self._state.running_command_id + def get_current(self) -> Optional[CurrentCommand]: """Return the "current" command, if any. @@ -632,6 +639,9 @@ def get_all_commands_final(self) -> bool: ) if no_command_running and no_command_to_execute: + # TODO(mm, 2024-03-14): This is a slow O(n) scan. When a long run ends and + # we reach this loop, it can disrupt the robot server. + # https://opentrons.atlassian.net/browse/EXEC-55 for command_id in self._state.all_command_ids: command = self._state.commands_by_id[command_id].command if command.error and command.intent != CommandIntent.SETUP: diff --git a/api/tests/opentrons/protocol_engine/state/test_command_view.py b/api/tests/opentrons/protocol_engine/state/test_command_view.py index 82fb21dc1f1..034e1276063 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_view.py @@ -683,6 +683,15 @@ def test_get_okay_to_clear(subject: CommandView, expected_is_okay: bool) -> None assert subject.get_is_okay_to_clear() is expected_is_okay +def test_get_running_command_id() -> None: + """It should return the running command ID.""" + subject_with_running = get_command_view(running_command_id="command-id") + assert subject_with_running.get_running_command_id() == "command-id" + + subject_without_running = get_command_view(running_command_id=None) + assert subject_without_running.get_running_command_id() is None + + def test_get_current() -> None: """It should return the "current" command.""" subject = get_command_view( @@ -851,7 +860,7 @@ def test_get_slice_default_cursor_running() -> None: def test_get_slice_default_cursor_queued() -> None: - """It should select a cursor based on the next queued command, if present.""" + """It should select a cursor automatically.""" command_1 = create_succeeded_command(command_id="command-id-1") command_2 = create_succeeded_command(command_id="command-id-2") command_3 = create_succeeded_command(command_id="command-id-3") @@ -861,7 +870,7 @@ def test_get_slice_default_cursor_queued() -> None: subject = get_command_view( commands=[command_1, command_2, command_3, command_4, command_5], running_command_id=None, - queued_command_ids=["command-id-4", "command-id-4", "command-id-5"], + queued_command_ids=[command_4.id, command_5.id], ) result = subject.get_slice(cursor=None, length=2) diff --git a/api/tests/opentrons/protocol_engine/test_protocol_engine.py b/api/tests/opentrons/protocol_engine/test_protocol_engine.py index 59772c868ed..1508373152d 100644 --- a/api/tests/opentrons/protocol_engine/test_protocol_engine.py +++ b/api/tests/opentrons/protocol_engine/test_protocol_engine.py @@ -749,7 +749,7 @@ async def test_estop_during_command( decoy.when(model_utils.get_timestamp()).then_return(timestamp) decoy.when(model_utils.generate_id()).then_return(error_id) decoy.when(state_store.commands.get_is_stopped()).then_return(False) - decoy.when(state_store.commands.state.running_command_id).then_return(command_id) + decoy.when(state_store.commands.get_running_command_id()).then_return(command_id) decoy.when(state_store.commands.state.queued_command_ids).then_return( fake_command_set ) @@ -793,7 +793,7 @@ async def test_estop_without_command( decoy.when(model_utils.get_timestamp()).then_return(timestamp) decoy.when(model_utils.generate_id()).then_return(error_id) decoy.when(state_store.commands.get_is_stopped()).then_return(False) - decoy.when(state_store.commands.state.running_command_id).then_return(None) + decoy.when(state_store.commands.get_running_command_id()).then_return(None) expected_stop = StopAction(from_estop=True) expected_hardware_stop = HardwareStoppedAction( diff --git a/robot-server/robot_server/commands/router.py b/robot-server/robot_server/commands/router.py index 9a06f9a7171..0d617e38a5a 100644 --- a/robot-server/robot_server/commands/router.py +++ b/robot-server/robot_server/commands/router.py @@ -140,7 +140,7 @@ async def get_commands_list( description=( "The starting index of the desired first command in the list." " If unspecified, a cursor will be selected automatically" - " based on the next queued or more recently executed command." + " based on the currently running or most recently executed command." ), ), pageLength: int = Query( 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 e468c8de84a..65929b5c9be 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 @@ -329,7 +329,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 14 + key: !anystr + createdAt: !anystr meta: cursor: 0 totalLength: 15 @@ -564,7 +571,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 14 + key: !anystr + createdAt: !anystr meta: cursor: 5 totalLength: 15 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 db35113b5ca..d9266dff9b0 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 @@ -81,7 +81,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 4 + key: !anystr + createdAt: !anystr meta: cursor: 3 totalLength: 5 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 bd11483d511..580feda6597 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 @@ -329,7 +329,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 14 + key: !anystr + createdAt: !anystr meta: cursor: 0 totalLength: 15 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 443767c27fc..f7f54b8ac3e 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 @@ -82,7 +82,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 3 + key: !anystr + createdAt: !anystr meta: cursor: 3 totalLength: 4 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 e0521f3e655..ddac99be771 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 @@ -151,7 +151,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 1 + key: !anystr + createdAt: !anystr meta: cursor: 0 totalLength: 2 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 3434f210bd0..31de3799870 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 @@ -139,7 +139,14 @@ stages: status_code: 200 json: links: - current: !anydict + current: + href: !anystr + meta: + runId: !anystr + commandId: !anystr + index: 3 + key: !anystr + createdAt: !anystr meta: cursor: 0 totalLength: 4