Skip to content

fix(api, app): add manualMove as a store command param so we can recover from a stacker error #19178

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

Open
wants to merge 4 commits into
base: chore_release-8.6.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 42 additions & 33 deletions api/src/opentrons/protocol_engine/commands/flex_stacker/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ class StoreParams(BaseModel):
...,
description="Unique ID of the flex stacker.",
)
manualMove: bool | SkipJsonSchema[None] = Field(
None,
description=(
"If true, indicates that the store action is being performed manually, "
"as done in error recovery, without moving the stacker hardware."
),
)


class StoreResult(BaseModel):
Expand Down Expand Up @@ -201,42 +208,44 @@ async def execute(self, params: StoreParams) -> _ExecuteReturn:
stacker_hw = self._equipment.get_module_hardware_api(stacker_state.module_id)

state_update = update_types.StateUpdate()
try:
if stacker_hw is not None:
stacker_hw.set_stacker_identify(True)
if stacker_hw is not None:
stacker_hw.set_stacker_identify(True)

if not params.manualMove and stacker_hw is not None:
try:
await stacker_hw.store_labware(
labware_height=stacker_state.get_pool_height_minus_overlap()
)
except FlexStackerStallError as e:
return DefinedErrorData(
public=FlexStackerStallOrCollisionError(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
wrappedErrors=[
ErrorOccurrence.from_failed(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
error=e,
)
],
errorInfo={"labwareId": primary_id},
),
)
except FlexStackerShuttleMissingError as e:
return DefinedErrorData(
public=FlexStackerShuttleError(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
wrappedErrors=[
ErrorOccurrence.from_failed(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
error=e,
)
],
errorInfo={"labwareId": primary_id},
),
)
except FlexStackerStallError as e:
return DefinedErrorData(
public=FlexStackerStallOrCollisionError(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
wrappedErrors=[
ErrorOccurrence.from_failed(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
error=e,
)
],
errorInfo={"labwareId": primary_id},
),
)
except FlexStackerShuttleMissingError as e:
return DefinedErrorData(
public=FlexStackerShuttleError(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
wrappedErrors=[
ErrorOccurrence.from_failed(
id=self._model_utils.generate_id(),
createdAt=self._model_utils.get_timestamp(),
error=e,
)
],
errorInfo={"labwareId": primary_id},
),
)

id_list = [
id for id in (primary_id, maybe_adapter_id, maybe_lid_id) if id is not None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ async def test_store_raises_if_labware_does_not_match(
await subject.execute(data)


@pytest.mark.parametrize("manual_move", [False, True])
async def test_store(
decoy: Decoy,
state_view: StateView,
Expand All @@ -452,9 +453,10 @@ async def test_store(
subject: StoreImpl,
stacker_hardware: FlexStacker,
flex_50uL_tiprack: LabwareDefinition,
manual_move: bool,
) -> None:
"""It should store the labware on the stack."""
data = flex_stacker.StoreParams(moduleId=stacker_id)
data = flex_stacker.StoreParams(moduleId=stacker_id, manualMove=manual_move)

fs_module_substate = FlexStackerSubState(
module_id=stacker_id,
Expand Down Expand Up @@ -513,7 +515,11 @@ async def test_store(

result = await subject.execute(data)

decoy.verify(await stacker_hardware.store_labware(labware_height=4), times=1)
decoy.verify(
await stacker_hardware.store_labware(labware_height=4),
times=1 if not manual_move else 0,
)

assert result == SuccessData(
public=flex_stacker.StoreResult(
primaryOriginLocationSequence=[
Expand Down
41 changes: 35 additions & 6 deletions app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import type {
CreateCommand,
DispenseInPlaceRunTimeCommand,
DropTipInPlaceRunTimeCommand,
FlexStackerRetrieveRunTimeCommand,
FlexStackerStoreRunTimeCommand,
LoadedLabware,
MoveLabwareParams,
MoveToCoordinatesCreateCommand,
Expand Down Expand Up @@ -85,6 +87,8 @@ export interface UseRecoveryCommandsResult {
homeShuttle: () => Promise<CommandData[]>
/* A non-terminal recovery-command */
manualRetrieve: () => Promise<CommandData[]>
/* A non-terminal recovery-command */
manualStore: () => Promise<CommandData[]>
}

// TODO(jh, 07-24-24): Create tighter abstractions for terminal vs. non-terminal commands.
Expand Down Expand Up @@ -416,6 +420,17 @@ export function useRecoveryCommands({
}
}, [chainRunRecoveryCommands, unvalidatedFailedCommand])

const manualStore = useCallback((): Promise<CommandData[]> => {
const manualStoreCommand = buildManualStore(unvalidatedFailedCommand)
if (manualStoreCommand == null) {
return reportAndRouteFailedCmd(
new Error('Invalid use of manual store command')
)
} else {
return chainRunRecoveryCommands([manualStoreCommand])
}
}, [chainRunRecoveryCommands, unvalidatedFailedCommand])

const moveLabwareWithoutPause = useCallback((): Promise<CommandData[]> => {
const moveLabwareCmd = buildMoveLabwareWithoutPause(
unvalidatedFailedCommand
Expand Down Expand Up @@ -443,6 +458,7 @@ export function useRecoveryCommands({
homeAll,
homeShuttle,
manualRetrieve,
manualStore,
closeLabwareLatch,
releaseLabwareLatch,
}
Expand Down Expand Up @@ -521,15 +537,28 @@ const buildManualRetrieve = (
if (failedCommand == null) {
return null
}
const storeOrRetriveFailedCommandParams = failedCommand.params
const moduleId =
'moduleId' in storeOrRetriveFailedCommandParams
? storeOrRetriveFailedCommandParams.moduleId
: ''
const retrieveCommand = failedCommand as FlexStackerRetrieveRunTimeCommand
return {
commandType: 'unsafe/flexStacker/manualRetrieve',
params: {
moduleId: moduleId,
moduleId: retrieveCommand.params.moduleId,
},
intent: 'fixit',
}
}

const buildManualStore = (
failedCommand: FailedCommand | null
): CreateCommand | null => {
if (failedCommand == null) {
return null
}
const storeCommand = failedCommand as FlexStackerStoreRunTimeCommand
return {
commandType: 'flexStacker/store',
params: {
moduleId: storeCommand.params.moduleId,
manualMove: true,
},
intent: 'fixit',
}
Expand Down
23 changes: 19 additions & 4 deletions app/src/organisms/ErrorRecoveryFlows/shared/SkipStepInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type { RecoveryContentProps } from '../types'

export function SkipStepInfo(props: RecoveryContentProps): JSX.Element {
const {
failedCommand,
recoveryCommands,
routeUpdateActions,
currentRecoveryOptionUtils,
Expand All @@ -24,7 +25,11 @@ export function SkipStepInfo(props: RecoveryContentProps): JSX.Element {
} = RECOVERY_MAP
const { selectedRecoveryOption } = currentRecoveryOptionUtils
const { skipFailedCommand } = recoveryCommands
const { moveLabwareWithoutPause, manualRetrieve } = recoveryCommands
const {
moveLabwareWithoutPause,
manualRetrieve,
manualStore,
} = recoveryCommands
const { handleMotionRouting } = routeUpdateActions
const { ROBOT_SKIPPING_STEP } = RECOVERY_MAP
const { t } = useTranslation('error_recovery')
Expand All @@ -40,9 +45,19 @@ export function SkipStepInfo(props: RecoveryContentProps): JSX.Element {
case STACKER_HOPPER_EMPTY_SKIP.ROUTE:
case STACKER_SHUTTLE_EMPTY_SKIP.ROUTE:
case STACKER_STALLED_SKIP.ROUTE:
void manualRetrieve().then(() => {
skipFailedCommand()
})
if (
failedCommand?.byRunRecord.commandType === 'flexStacker/retrieve'
) {
void manualRetrieve().then(() => {
skipFailedCommand()
})
} else if (
failedCommand?.byRunRecord.commandType === 'flexStacker/store'
) {
void manualStore().then(() => {
skipFailedCommand()
})
}
break
default:
skipFailedCommand()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ describe('SkipStepInfo', () => {
let mockHandleMotionRouting: Mock
let mockSkipFailedCommand: Mock
let mockManualRetrieve: Mock
let mockManualStore: Mock

beforeEach(() => {
mockHandleMotionRouting = vi.fn(() => Promise.resolve())
mockSkipFailedCommand = vi.fn(() => Promise.resolve())
mockManualRetrieve = vi.fn(() => Promise.resolve())
mockManualStore = vi.fn(() => Promise.resolve())

props = {
routeUpdateActions: {
Expand All @@ -29,6 +31,7 @@ describe('SkipStepInfo', () => {
recoveryCommands: {
skipFailedCommand: mockSkipFailedCommand,
manualRetrieve: mockManualRetrieve,
manualStore: mockManualStore,
} as any,
currentRecoveryOptionUtils: {
selectedRecoveryOption: RECOVERY_MAP.SKIP_STEP_WITH_SAME_TIPS.ROUTE,
Expand Down Expand Up @@ -125,6 +128,9 @@ describe('SkipStepInfo', () => {
RECOVERY_MAP.STACKER_STALLED_SKIP.ROUTE,
])('calls manualRetreive when the route is %s', async route => {
props.currentRecoveryOptionUtils.selectedRecoveryOption = route
props.failedCommand = {
byRunRecord: { commandType: 'flexStacker/retrieve' as any },
} as any
render(props)

clickButtonLabeled('Continue run now')
Expand All @@ -144,4 +150,33 @@ describe('SkipStepInfo', () => {
)
})
})

it.each([
RECOVERY_MAP.STACKER_HOPPER_EMPTY_SKIP.ROUTE,
RECOVERY_MAP.STACKER_SHUTTLE_EMPTY_SKIP.ROUTE,
RECOVERY_MAP.STACKER_STALLED_SKIP.ROUTE,
])('calls manualStore when the route is %s', async route => {
props.currentRecoveryOptionUtils.selectedRecoveryOption = route
props.failedCommand = {
byRunRecord: { commandType: 'flexStacker/store' as any },
} as any
render(props)

clickButtonLabeled('Continue run now')

await waitFor(() => {
expect(mockHandleMotionRouting).toHaveBeenCalledWith(
true,
RECOVERY_MAP.ROBOT_SKIPPING_STEP.ROUTE
)
})
await waitFor(() => {
expect(mockManualStore).toHaveBeenCalled()
})
await waitFor(() => {
expect(mockHandleMotionRouting.mock.invocationCallOrder[0]).toBeLessThan(
mockManualStore.mock.invocationCallOrder[0]
)
})
})
})
Loading
Loading