Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(robot-server): Return a currentlyRecoveringFrom command pointer #15198

Merged
merged 11 commits into from
May 21, 2024
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_engine/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
CommandType,
CommandIntent,
)
from .state import State, StateView, StateSummary, CommandSlice, CurrentCommand, Config
from .state import State, StateView, StateSummary, CommandSlice, CommandPointer, Config
from .plugins import AbstractPlugin

from .types import (
Expand Down Expand Up @@ -85,7 +85,7 @@
"State",
"StateView",
"CommandSlice",
"CurrentCommand",
"CommandPointer",
# public value interfaces and models
"LabwareOffset",
"LabwareOffsetCreate",
Expand Down
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_engine/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
CommandState,
CommandView,
CommandSlice,
CurrentCommand,
CommandPointer,
)
from .command_history import CommandEntry
from .labware import LabwareState, LabwareView
Expand Down Expand Up @@ -39,7 +39,7 @@
"CommandState",
"CommandView",
"CommandSlice",
"CurrentCommand",
"CommandPointer",
"CommandEntry",
# labware state and values
"LabwareState",
Expand Down
40 changes: 22 additions & 18 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ class CommandSlice:


@dataclass(frozen=True)
class CurrentCommand:
"""The "current" command's ID and index in the overall commands list."""
class CommandPointer:
"""Brief info about a command and where to find it."""

command_id: str
command_key: str
Expand Down Expand Up @@ -593,28 +593,28 @@ def get_queue_ids(self) -> OrderedSet[str]:
"""Get the IDs of all queued protocol commands, in FIFO order."""
return self._state.command_history.get_queue_ids()

def get_current(self) -> Optional[CurrentCommand]:
def get_current(self) -> Optional[CommandPointer]:
"""Return the "current" command, if any.

The "current" command is the command that is currently executing,
or the most recent command to have completed.
"""
running_command = self._state.command_history.get_running_command()
if running_command:
return CurrentCommand(
return CommandPointer(
command_id=running_command.command.id,
command_key=running_command.command.key,
created_at=running_command.command.createdAt,
index=running_command.index,
)

final_command = self.get_final_command()
if final_command:
return CurrentCommand(
command_id=final_command.command.id,
command_key=final_command.command.key,
created_at=final_command.command.createdAt,
index=final_command.index,
most_recently_finalized_command = self.get_most_recently_finalized_command()
if most_recently_finalized_command:
return CommandPointer(
command_id=most_recently_finalized_command.command.id,
command_key=most_recently_finalized_command.command.key,
created_at=most_recently_finalized_command.command.createdAt,
index=most_recently_finalized_command.index,
)

return None
Expand Down Expand Up @@ -678,7 +678,7 @@ def get_is_running(self) -> bool:
"""Get whether the protocol is running & queued commands should be executed."""
return self._state.queue_status == QueueStatus.RUNNING

def get_final_command(self) -> Optional[CommandEntry]:
def get_most_recently_finalized_command(self) -> Optional[CommandEntry]:
"""Get the most recent command that has reached its final `status`. See get_command_is_final."""
run_requested_to_stop = self._state.run_result is not None

Expand All @@ -691,22 +691,22 @@ def get_final_command(self) -> Optional[CommandEntry]:
else:
return self._state.command_history.get_prev(tail_command.command.id)
else:
final_command = self._state.command_history.get_terminal_command()
most_recently_finalized = self._state.command_history.get_terminal_command()
# This iteration is effectively O(1) as we'll only ever have to iterate one or two times at most.
while final_command is not None:
while most_recently_finalized is not None:
next_command = self._state.command_history.get_next(
final_command.command.id
most_recently_finalized.command.id
)
if (
next_command is not None
and next_command.command.status != CommandStatus.QUEUED
and next_command.command.status != CommandStatus.RUNNING
):
final_command = next_command
most_recently_finalized = next_command
else:
break

return final_command
return most_recently_finalized

def get_command_is_final(self, command_id: str) -> bool:
"""Get whether a given command has reached its final `status`.
Expand Down Expand Up @@ -751,9 +751,13 @@ def get_all_commands_final(self) -> bool:

return no_command_running and no_command_to_execute

def get_recovery_target_id(self) -> Optional[str]:
"""Return the ID of the command currently undergoing error recovery, if any."""
return self._state.recovery_target_command_id

def get_recovery_in_progress_for_command(self, command_id: str) -> bool:
"""Return whether the given command failed and its error recovery is in progress."""
return self._state.recovery_target_command_id == command_id
return self.get_recovery_target_id() == command_id

def raise_fatal_command_error(self) -> None:
"""Raise the run's fatal command error, if there was one, as an exception.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,8 @@ def test_error_recovery_type_tracking() -> None:
assert view.get_error_recovery_type("c2") == ErrorRecoveryType.FAIL_RUN


def test_get_recovery_in_progress_for_command() -> None:
"""It should return whether error recovery is in progress for the given command."""
def test_recovery_target_tracking() -> None:
"""It should keep track of the command currently undergoing error recovery."""
subject = CommandStore(config=_make_config(), is_door_open=False)
subject_view = CommandView(subject.state)

Expand All @@ -382,12 +382,14 @@ def test_get_recovery_in_progress_for_command() -> None:
subject.handle_action(fail_1)

# c1 failed recoverably and we're currently recovering from it.
assert subject_view.get_recovery_target_id() == "c1"
assert subject_view.get_recovery_in_progress_for_command("c1")

resume_from_1_recovery = actions.ResumeFromRecoveryAction()
subject.handle_action(resume_from_1_recovery)

# c1 failed recoverably, but we've already completed its recovery.
assert subject_view.get_recovery_target_id() is None
assert not subject_view.get_recovery_in_progress_for_command("c1")

queue_2 = actions.QueueCommandAction(
Expand All @@ -411,6 +413,7 @@ def test_get_recovery_in_progress_for_command() -> None:
subject.handle_action(fail_2)

# c2 failed recoverably and we're currently recovering from it.
assert subject_view.get_recovery_target_id() == "c2"
assert subject_view.get_recovery_in_progress_for_command("c2")
# ...and that means we're *not* currently recovering from c1,
# even though it failed recoverably before.
Expand Down Expand Up @@ -439,7 +442,8 @@ def test_get_recovery_in_progress_for_command() -> None:
subject.handle_action(fail_3)

# c3 failed, but not recoverably.
assert not subject_view.get_recovery_in_progress_for_command("c2")
assert subject_view.get_recovery_target_id() is None
assert not subject_view.get_recovery_in_progress_for_command("c3")
DerekMaggio marked this conversation as resolved.
Show resolved Hide resolved


def test_final_state_after_estop() -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
CommandState,
CommandView,
CommandSlice,
CurrentCommand,
CommandPointer,
RunResult,
QueueStatus,
)
Expand Down Expand Up @@ -846,7 +846,7 @@ def test_get_current() -> None:
queued_command_ids=[],
commands=[command],
)
assert subject.get_current() == CurrentCommand(
assert subject.get_current() == CommandPointer(
index=0,
command_id="command-id",
command_key="command-key",
Expand All @@ -866,7 +866,7 @@ def test_get_current() -> None:
subject = get_command_view(commands=[command_1, command_2])
subject.state.command_history._set_terminal_command_id(command_1.id)

assert subject.get_current() == CurrentCommand(
assert subject.get_current() == CommandPointer(
index=1,
command_id="command-id-2",
command_key="key-2",
Expand All @@ -886,7 +886,7 @@ def test_get_current() -> None:
subject = get_command_view(commands=[command_1, command_2])
subject.state.command_history._set_terminal_command_id(command_1.id)

assert subject.get_current() == CurrentCommand(
assert subject.get_current() == CommandPointer(
index=1,
command_id="command-id-2",
command_key="key-2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
LabwareOffsetCreate,
StateSummary,
CommandSlice,
CurrentCommand,
CommandPointer,
Command,
)

Expand Down Expand Up @@ -183,7 +183,7 @@ def get_commands_slice(
)
return the_slice

def get_current_command(self, run_id: str) -> Optional[CurrentCommand]:
def get_current_command(self, run_id: str) -> Optional[CommandPointer]:
"""Get the "current" command, if any.

See `ProtocolEngine.state_view.commands.get_current()` for the definition
Expand Down
22 changes: 16 additions & 6 deletions robot-server/robot_server/runs/command_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,17 @@ class CommandLinkMeta(BaseModel):

runId: str = Field(..., description="The ID of the command's run.")
commandId: str = Field(..., description="The ID of the command.")
index: int = Field(..., description="Index of the command in the overall list.")
key: str = Field(..., description="Value of the current command's `key` field.")
createdAt: datetime = Field(
...,
description="When the current command was created.",
index: int = Field(
..., description="The index of the command in the run's overall command list."
)
key: str = Field(..., description="The value of the command's `key` field.")
createdAt: datetime = Field(..., description="When the command was created.")


class CommandLink(BaseModel):
"""A link to a command resource."""

href: str = Field(..., description="The path to a command")
href: str = Field(..., description="The HTTP API path to the command")
meta: CommandLinkMeta = Field(..., description="Information about the command.")


Expand All @@ -55,3 +54,14 @@ class CommandCollectionLinks(BaseModel):
" or, if there is none, the one that was running most recently."
),
)

recoveryTarget: Optional[CommandLink] = Field(
SyntaxColoring marked this conversation as resolved.
Show resolved Hide resolved
None,
description=(
"Information about the command currently undergoing error recovery."
" This is basically the most recent protocol command to have failed,"
" except that once you complete error recovery"
" (see `GET /runs/{id}/actions`), this goes back to being"
" `null` or omitted."
),
)
4 changes: 2 additions & 2 deletions robot-server/robot_server/runs/run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
LabwareOffsetCreate,
StateSummary,
CommandSlice,
CurrentCommand,
CommandPointer,
Command,
)
from opentrons.protocol_engine.types import RunTimeParamValuesType
Expand Down Expand Up @@ -373,7 +373,7 @@ def get_commands_slice(
run_id=run_id, cursor=cursor, length=length
)

def get_current_command(self, run_id: str) -> Optional[CurrentCommand]:
def get_current_command(self, run_id: str) -> Optional[CommandPointer]:
"""Get the "current" command, if any.

See `ProtocolEngine.state_view.commands.get_current()` for the definition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from dataclasses import dataclass
from typing import Callable, Optional

from opentrons.protocol_engine import CurrentCommand, StateSummary, EngineStatus
from opentrons.protocol_engine import CommandPointer, StateSummary, EngineStatus

from server_utils.fastapi_utils.app_state import (
AppState,
Expand All @@ -20,15 +20,15 @@ class RunHooks:
"""Generated during a protocol run. Utilized by RunsPublisher."""

run_id: str
get_current_command: Callable[[str], Optional[CurrentCommand]]
get_current_command: Callable[[str], Optional[CommandPointer]]
get_state_summary: Callable[[str], Optional[StateSummary]]


@dataclass
class EngineStateSlice:
"""Protocol Engine state relevant to RunsPublisher."""

current_command: Optional[CurrentCommand] = None
current_command: Optional[CommandPointer] = None
state_summary_status: Optional[EngineStatus] = None


Expand All @@ -54,7 +54,7 @@ def __init__(
async def initialize(
self,
run_id: str,
get_current_command: Callable[[str], Optional[CurrentCommand]],
get_current_command: Callable[[str], Optional[CommandPointer]],
get_state_summary: Callable[[str], Optional[StateSummary]],
) -> None:
"""Initialize RunsPublisher with necessary information derived from the current run.
Expand Down
4 changes: 2 additions & 2 deletions robot-server/tests/commands/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from opentrons.protocol_engine import (
ProtocolEngine,
CommandSlice,
CurrentCommand,
CommandPointer,
commands as pe_commands,
)
from opentrons.protocol_engine.errors import CommandDoesNotExistError
Expand Down Expand Up @@ -147,7 +147,7 @@ async def test_get_commands_list(
)

decoy.when(protocol_engine.state_view.commands.get_current()).then_return(
CurrentCommand(
CommandPointer(
command_id="abc123",
command_key="command-key-1",
created_at=datetime(year=2021, month=1, day=1),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from opentrons.protocol_engine import (
CommandSlice,
CurrentCommand,
CommandPointer,
ProtocolEngine,
commands as pe_commands,
errors as pe_errors,
Expand Down Expand Up @@ -199,7 +199,7 @@ async def test_get_run_commands(
decoy.when(
mock_maintenance_run_data_manager.get_current_command("run-id")
).then_return(
CurrentCommand(
CommandPointer(
command_id="current-command-id",
command_key="current-command-key",
created_at=datetime(year=2024, month=4, day=4),
Expand Down
4 changes: 2 additions & 2 deletions robot-server/tests/runs/router/test_commands_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from opentrons.protocol_engine import (
CommandSlice,
CurrentCommand,
CommandPointer,
ProtocolEngine,
CommandNote,
commands as pe_commands,
Expand Down Expand Up @@ -321,7 +321,7 @@ async def test_get_run_commands(
)

decoy.when(mock_run_data_manager.get_current_command("run-id")).then_return(
CurrentCommand(
CommandPointer(
command_id="current-command-id",
command_key="current-command-key",
created_at=datetime(year=2024, month=4, day=4),
Expand Down
4 changes: 2 additions & 2 deletions robot-server/tests/runs/test_run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
commands,
types as pe_types,
CommandSlice,
CurrentCommand,
CommandPointer,
ErrorOccurrence,
LoadedLabware,
LoadedPipette,
Expand Down Expand Up @@ -847,7 +847,7 @@ def test_get_current_command(
run_command: commands.Command,
) -> None:
"""Should get current command from engine store."""
expected_current = CurrentCommand(
expected_current = CommandPointer(
command_id=run_command.id,
command_key=run_command.key,
created_at=run_command.createdAt,
Expand Down
Loading
Loading