Skip to content

Commit

Permalink
fix(app,robot-server): Account for failed commands not having a pipet…
Browse files Browse the repository at this point in the history
…teId (#16859)

Co-authored-by: Jamey Huffnagle <jamey.huffnagle@opentrons.com>
  • Loading branch information
SyntaxColoring and mjhuff authored Nov 18, 2024
1 parent 6d5b3a2 commit 29e03ae
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 53 deletions.
5 changes: 5 additions & 0 deletions api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export interface Runs {
export interface RunCurrentStateData {
estopEngaged: boolean
activeNozzleLayouts: Record<string, NozzleLayoutValues> // keyed by pipetteId
tipStates: Record<string, TipStates> // keyed by pipetteId
placeLabwareState?: PlaceLabwareState
}

Expand Down Expand Up @@ -218,3 +219,7 @@ export interface PlaceLabwareState {
location: OnDeckLabwareLocation
shouldPlaceDown: boolean
}

export interface TipStates {
hasTip: boolean
}
15 changes: 15 additions & 0 deletions api/src/opentrons/protocol_runner/run_orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,21 @@ def get_nozzle_maps(self) -> Dict[str, NozzleMap]:
"""Get current nozzle maps keyed by pipette id."""
return self._protocol_engine.state_view.tips.get_pipette_nozzle_maps()

def get_tip_attached(self) -> Dict[str, bool]:
"""Get current tip state keyed by pipette id."""

def has_tip_attached(pipette_id: str) -> bool:
return (
self._protocol_engine.state_view.pipettes.get_attached_tip(pipette_id)
is not None
)

pipette_ids = (
pipette.id
for pipette in self._protocol_engine.state_view.pipettes.get_all()
)
return {pipette_id: has_tip_attached(pipette_id) for pipette_id in pipette_ids}

def set_error_recovery_policy(self, policy: ErrorRecoveryPolicy) -> None:
"""Create error recovery policy for the run."""
self._protocol_engine.set_error_recovery_policy(policy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ def get_next_to_execute() -> Generator[str, None, None]:
index = index + 1


async def test_create_error_recovery_policy(
def test_create_error_recovery_policy(
decoy: Decoy,
mock_protocol_engine: ProtocolEngine,
live_protocol_subject: RunOrchestrator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ export function useDropTipFlowUtils({
subMapUtils,
routeUpdateActions,
recoveryMap,
errorKind,
}: RecoveryContentProps): FixitCommandTypeUtils {
const { t } = useTranslation('error_recovery')
const {
Expand All @@ -210,7 +211,7 @@ export function useDropTipFlowUtils({
ERROR_WHILE_RECOVERING,
DROP_TIP_FLOWS,
} = RECOVERY_MAP
const { runId } = tipStatusUtils
const { runId, gripperErrorFirstPipetteWithTip } = tipStatusUtils
const { step } = recoveryMap
const { selectedRecoveryOption } = currentRecoveryOptionUtils
const { proceedToRouteAndStep } = routeUpdateActions
Expand Down Expand Up @@ -304,11 +305,12 @@ export function useDropTipFlowUtils({
}

const pipetteId =
failedCommand != null &&
gripperErrorFirstPipetteWithTip ??
(failedCommand != null &&
'params' in failedCommand.byRunRecord &&
'pipetteId' in failedCommand.byRunRecord.params
? failedCommand.byRunRecord.params.pipetteId
: null
: null)

return {
runId,
Expand Down
1 change: 1 addition & 0 deletions app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export function useERUtils({
const tipStatusUtils = useRecoveryTipStatus({
runId,
runRecord,
failedCommand,
attachedInstruments,
failedPipetteInfo,
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
import { useState } from 'react'
import head from 'lodash/head'

import { useHost } from '@opentrons/react-api-client'
import { useHost, useRunCurrentState } from '@opentrons/react-api-client'
import { getPipetteModelSpecs } from '@opentrons/shared-data'
import { useTipAttachmentStatus } from '/app/organisms/DropTipWizardFlows'
import { ERROR_KINDS } from '/app/organisms/ErrorRecoveryFlows/constants'
import { getErrorKind } from '/app/organisms/ErrorRecoveryFlows/utils'

import type { Run, Instruments, PipetteData } from '@opentrons/api-client'
import type {
PipetteWithTip,
TipAttachmentStatusResult,
} from '/app/organisms/DropTipWizardFlows'
import type { ERUtilsProps } from '/app/organisms/ErrorRecoveryFlows/hooks/useERUtils'

interface UseRecoveryTipStatusProps {
runId: string
failedCommand: ERUtilsProps['failedCommand']
failedPipetteInfo: PipetteData | null
attachedInstruments?: Instruments
runRecord?: Run
Expand All @@ -22,6 +26,7 @@ export type RecoveryTipStatusUtils = TipAttachmentStatusResult & {
/* Whether the robot is currently determineTipStatus() */
isLoadingTipStatus: boolean
runId: string
gripperErrorFirstPipetteWithTip: string | null
}

// Wraps the tip attachment status utils with Error Recovery specific states and values.
Expand Down Expand Up @@ -77,11 +82,26 @@ export function useRecoveryTipStatus(
})
}

// TODO(jh, 11-15-24): This is temporary. Collaborate with design a better way to do drop tip wizard for multiple
// pipettes during error recovery. The tip detection logic will shortly be simplified, too!
const errorKind = getErrorKind(props.failedCommand)
const currentTipStates =
useRunCurrentState(props.runId, {
enabled: errorKind === ERROR_KINDS.GRIPPER_ERROR,
}).data?.data.tipStates ?? null

const gripperErrorFirstPipetteWithTip =
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
Object.entries(currentTipStates ?? {}).find(
([_, state]) => state.hasTip
)?.[0] ?? null

return {
...tipAttachmentStatusUtils,
aPipetteWithTip: failedCommandPipette,
determineTipStatus: determineTipStatusWithLoading,
isLoadingTipStatus,
runId: props.runId,
gripperErrorFirstPipetteWithTip,
}
}
52 changes: 29 additions & 23 deletions robot-server/robot_server/runs/router/base_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
RunCurrentState,
CommandLinkNoMeta,
NozzleLayoutConfig,
TipState,
)
from ..run_auto_deleter import RunAutoDeleter
from ..run_models import Run, BadRun, RunCreate, RunUpdate
Expand Down Expand Up @@ -591,33 +592,27 @@ async def get_current_state( # noqa: C901
"""
try:
run = run_data_manager.get(run_id=runId)
active_nozzle_maps = run_data_manager.get_nozzle_maps(run_id=runId)

nozzle_layouts = {
pipetteId: ActiveNozzleLayout.construct(
startingNozzle=nozzle_map.starting_nozzle,
activeNozzles=list(nozzle_map.map_store.keys()),
config=NozzleLayoutConfig(nozzle_map.configuration.value.lower()),
)
for pipetteId, nozzle_map in active_nozzle_maps.items()
}

run = run_data_manager.get(run_id=runId)
current_command = run_data_manager.get_current_command(run_id=runId)
last_completed_command = run_data_manager.get_last_completed_command(
run_id=runId
)
except RunNotCurrentError as e:
raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT)

links = CurrentStateLinks.construct(
lastCompleted=CommandLinkNoMeta.construct(
id=last_completed_command.command_id,
href=f"/runs/{runId}/commands/{last_completed_command.command_id}",
active_nozzle_maps = run_data_manager.get_nozzle_maps(run_id=runId)
nozzle_layouts = {
pipetteId: ActiveNozzleLayout.construct(
startingNozzle=nozzle_map.starting_nozzle,
activeNozzles=list(nozzle_map.map_store.keys()),
config=NozzleLayoutConfig(nozzle_map.configuration.value.lower()),
)
if last_completed_command is not None
else None
)
for pipetteId, nozzle_map in active_nozzle_maps.items()
}

tip_states = {
pipette_id: TipState.construct(hasTip=has_tip)
for pipette_id, has_tip in run_data_manager.get_tip_attached(
run_id=runId
).items()
}

current_command = run_data_manager.get_current_command(run_id=runId)

estop_engaged = False
place_labware = None
Expand Down Expand Up @@ -672,11 +667,22 @@ async def get_current_state( # noqa: C901
if place_labware:
break

last_completed_command = run_data_manager.get_last_completed_command(run_id=runId)
links = CurrentStateLinks.construct(
lastCompleted=CommandLinkNoMeta.construct(
id=last_completed_command.command_id,
href=f"/runs/{runId}/commands/{last_completed_command.command_id}",
)
if last_completed_command is not None
else None
)

return await PydanticResponse.create(
content=Body.construct(
data=RunCurrentState.construct(
estopEngaged=estop_engaged,
activeNozzleLayouts=nozzle_layouts,
tipStates=tip_states,
placeLabwareState=place_labware,
),
links=links,
Expand Down
7 changes: 7 additions & 0 deletions robot-server/robot_server/runs/run_data_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,13 @@ def get_nozzle_maps(self, run_id: str) -> Dict[str, NozzleMap]:

raise RunNotCurrentError()

def get_tip_attached(self, run_id: str) -> Dict[str, bool]:
"""Get current tip attached states, keyed by pipette id."""
if run_id == self._run_orchestrator_store.current_run_id:
return self._run_orchestrator_store.get_tip_attached()

raise RunNotCurrentError()

def get_all_commands_as_preserialized_list(
self, run_id: str, include_fixit_commands: bool
) -> List[str]:
Expand Down
28 changes: 25 additions & 3 deletions robot-server/robot_server/runs/run_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,16 @@ class ActiveNozzleLayout(BaseModel):
)


class TipState(BaseModel):
"""Information about the tip, if any, currently attached to a pipette."""

hasTip: bool

# todo(mm, 2024-11-15): I think the frontend is currently scraping the commands
# list to figure out where the current tip came from. Extend this class with that
# information so the frontend doesn't have to do that.


class PlaceLabwareState(BaseModel):
"""Details the labware being placed by the gripper."""

Expand All @@ -331,9 +341,21 @@ class PlaceLabwareState(BaseModel):
class RunCurrentState(BaseModel):
"""Current details about a run."""

estopEngaged: bool = Field(..., description="")
activeNozzleLayouts: Dict[str, ActiveNozzleLayout] = Field(...)
placeLabwareState: Optional[PlaceLabwareState] = Field(None)
# todo(mm, 2024-11-15): Having estopEngaged here is a bit of an odd man out because
# it's sensor state that can change on its own at any time, whereas the rest of
# these fields are logical state that changes only when commands are run.
#
# Our current mechanism for anchoring these fields to a specific point in time
# (important for avoiding torn-read problems when a client combines this info with
# info from other endpoints) is `links.currentCommand`, which is based on the idea
# that these fields only change when the current command changes.
#
# We should see if clients can replace this with `GET /robot/control/estopStatus`.
estopEngaged: bool

activeNozzleLayouts: Dict[str, ActiveNozzleLayout]
tipStates: Dict[str, TipState]
placeLabwareState: Optional[PlaceLabwareState]


class CommandLinkNoMeta(BaseModel):
Expand Down
7 changes: 7 additions & 0 deletions robot-server/robot_server/runs/run_orchestrator_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ async def clear(self) -> RunResult:
state_summary=run_data, commands=commands, parameters=run_time_parameters
)

# todo(mm, 2024-11-15): Are all of these pass-through methods helpful?
# Can we delete them and make callers just call .run_orchestrator.play(), etc.?

def play(self, deck_configuration: Optional[DeckConfigurationType] = None) -> None:
"""Start or resume the run."""
self.run_orchestrator.play(deck_configuration=deck_configuration)
Expand Down Expand Up @@ -331,6 +334,10 @@ def get_nozzle_maps(self) -> Dict[str, NozzleMap]:
"""Get the current nozzle map keyed by pipette id."""
return self.run_orchestrator.get_nozzle_maps()

def get_tip_attached(self) -> Dict[str, bool]:
"""Get current tip state keyed by pipette id."""
return self.run_orchestrator.get_tip_attached()

def get_run_time_parameters(self) -> List[RunTimeParameter]:
"""Parameter definitions defined by protocol, if any. Will always be empty before execution."""
return self.run_orchestrator.get_run_time_parameters()
Expand Down
41 changes: 19 additions & 22 deletions robot-server/tests/runs/router/test_base_router.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
"""Tests for base /runs routes."""
from typing import Dict

from opentrons.hardware_control import HardwareControlAPI
from opentrons_shared_data.robot.types import RobotTypeEnum
import pytest
Expand Down Expand Up @@ -53,6 +51,7 @@
ActiveNozzleLayout,
CommandLinkNoMeta,
NozzleLayoutConfig,
TipState,
)
from robot_server.runs.run_orchestrator_store import RunConflictError
from robot_server.runs.run_data_manager import (
Expand Down Expand Up @@ -112,23 +111,6 @@ def labware_offset_create() -> LabwareOffsetCreate:
)


@pytest.fixture
def mock_nozzle_maps() -> Dict[str, NozzleMap]:
"""Get mock NozzleMaps."""
return {
"mock-pipette-id": NozzleMap(
configuration=NozzleConfigurationType.FULL,
columns={"1": ["A1"]},
rows={"A": ["A1"]},
map_store={"A1": Point(0, 0, 0)},
starting_nozzle="A1",
valid_map_key="mock-key",
full_instrument_map_store={},
full_instrument_rows={},
)
}


async def test_create_run(
decoy: Decoy,
mock_run_data_manager: RunDataManager,
Expand Down Expand Up @@ -876,7 +858,6 @@ async def test_get_current_state_success(
decoy: Decoy,
mock_run_data_manager: RunDataManager,
mock_hardware_api: HardwareControlAPI,
mock_nozzle_maps: Dict[str, NozzleMap],
) -> None:
"""It should return different state from the current run.
Expand All @@ -885,8 +866,23 @@ async def test_get_current_state_success(
"""
run_id = "test-run-id"

decoy.when(mock_run_data_manager.get_tip_attached(run_id=run_id)).then_return(
{"mock-pipette-id": True}
)

decoy.when(mock_run_data_manager.get_nozzle_maps(run_id=run_id)).then_return(
mock_nozzle_maps
{
"mock-pipette-id": NozzleMap(
configuration=NozzleConfigurationType.FULL,
columns={"1": ["A1"]},
rows={"A": ["A1"]},
map_store={"A1": Point(0, 0, 0)},
starting_nozzle="A1",
valid_map_key="mock-key",
full_instrument_map_store={},
full_instrument_rows={},
)
}
)
command_pointer = CommandPointer(
command_id="command-id",
Expand Down Expand Up @@ -918,6 +914,7 @@ async def test_get_current_state_success(
config=NozzleLayoutConfig.FULL,
)
},
tipStates={"mock-pipette-id": TipState(hasTip=True)},
)
assert result.content.links == CurrentStateLinks(
lastCompleted=CommandLinkNoMeta(
Expand All @@ -935,7 +932,7 @@ async def test_get_current_state_run_not_current(
"""It should raise RunStopped when the run is not current."""
run_id = "non-current-run-id"

decoy.when(mock_run_data_manager.get_nozzle_maps(run_id=run_id)).then_raise(
decoy.when(mock_run_data_manager.get(run_id=run_id)).then_raise(
RunNotCurrentError("Run is not current")
)

Expand Down

0 comments on commit 29e03ae

Please sign in to comment.