From 603dc2f30b94825dcf438c632df847aff5ee8c5f Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Wed, 24 Jul 2024 11:57:32 -0400 Subject: [PATCH 1/8] refactor(app): enable recovery errors for failed pickUpTip retry commands (#15779) --- .../hooks/__tests__/useRecoveryCommands.test.ts | 2 +- .../organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts index d75387a99d4..1a54da00103 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts @@ -217,7 +217,7 @@ describe('useRecoveryCommands', () => { expect(mockChainRunCommands).toHaveBeenCalledWith( [buildPickUpTipsCmd], - true + false ) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts index 3ef8f5b3809..58c398b33f6 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts @@ -84,7 +84,6 @@ export function useRecoveryCommands({ }, [chainRunRecoveryCommands]) // Pick up the user-selected tips - // TODO(jh, 06-14-24): Do not ignore pickUpTip errors once Pipettes can support tip pick up. const pickUpTips = React.useCallback((): Promise => { const { selectedTipLocations, failedLabware } = failedLabwareUtils @@ -97,7 +96,7 @@ export function useRecoveryCommands({ if (pickUpTipCmd == null) { return Promise.reject(new Error('Invalid use of pickUpTips command')) } else { - return chainRunRecoveryCommands([pickUpTipCmd], true) + return chainRunRecoveryCommands([pickUpTipCmd]) } }, [chainRunRecoveryCommands, failedCommand, failedLabwareUtils]) From e4ff49aea5755afb3439592085a50c09be3cdffa Mon Sep 17 00:00:00 2001 From: aaron-kulkarni <107003644+aaron-kulkarni@users.noreply.github.com> Date: Wed, 24 Jul 2024 13:12:25 -0400 Subject: [PATCH 2/8] feat(robot-server): start logic for dynamic error recovery policy (#15707) # Overview Create robot-server function that turns a List of ErrorRecoveryRules into a full ErrorRecoveryPolicy. A future PR will implement the HTTP calls that actually create the list of rules. EXEC-589 # Test Plan # Changelog # Review requests # Risk assessment --- .../protocol_engine/error_recovery_policy.py | 6 +- .../protocol_engine/state/commands.py | 5 +- .../runs/error_recovery_mapping.py | 46 +++++++ .../runs/error_recovery_models.py | 69 ++++++++++ robot-server/robot_server/service/errors.py | 1 + .../tests/runs/test_error_recovery_mapping.py | 118 ++++++++++++++++++ 6 files changed, 240 insertions(+), 5 deletions(-) create mode 100644 robot-server/robot_server/runs/error_recovery_mapping.py create mode 100644 robot-server/robot_server/runs/error_recovery_models.py create mode 100644 robot-server/tests/runs/test_error_recovery_mapping.py diff --git a/api/src/opentrons/protocol_engine/error_recovery_policy.py b/api/src/opentrons/protocol_engine/error_recovery_policy.py index f7468961131..2623a696c4d 100644 --- a/api/src/opentrons/protocol_engine/error_recovery_policy.py +++ b/api/src/opentrons/protocol_engine/error_recovery_policy.py @@ -28,10 +28,8 @@ class ErrorRecoveryType(enum.Enum): WAIT_FOR_RECOVERY = enum.auto() """Stop and wait for the error to be recovered from manually.""" - # TODO(mm, 2023-03-18): Add something like this for - # https://opentrons.atlassian.net/browse/EXEC-302. - # CONTINUE = enum.auto() - # """Continue with the run, as if the command never failed.""" + IGNORE_AND_CONTINUE = enum.auto() + """Continue with the run, as if the command never failed.""" class ErrorRecoveryPolicy(Protocol): diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index a558210cbff..53a11a66ba0 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -337,7 +337,10 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None: other_command_ids_to_fail = list( self._state.command_history.get_queue_ids() ) - elif action.type == ErrorRecoveryType.WAIT_FOR_RECOVERY: + elif ( + action.type == ErrorRecoveryType.WAIT_FOR_RECOVERY + or action.type == ErrorRecoveryType.IGNORE_AND_CONTINUE + ): other_command_ids_to_fail = [] else: assert_never(action.type) diff --git a/robot-server/robot_server/runs/error_recovery_mapping.py b/robot-server/robot_server/runs/error_recovery_mapping.py new file mode 100644 index 00000000000..dfef53ad05f --- /dev/null +++ b/robot-server/robot_server/runs/error_recovery_mapping.py @@ -0,0 +1,46 @@ +"""Functions used for managing error recovery policy.""" +from typing import Optional +from opentrons.protocol_engine.state.config import Config +from robot_server.runs.error_recovery_models import ErrorRecoveryRule, ReactionIfMatch +from opentrons.protocol_engine.commands.command_unions import ( + Command, + CommandDefinedErrorData, +) +from opentrons.protocol_engine.error_recovery_policy import ( + ErrorRecoveryPolicy, + ErrorRecoveryType, + standard_run_policy, +) + + +def create_error_recovery_policy_from_rules( + rules: list[ErrorRecoveryRule], +) -> ErrorRecoveryPolicy: + """Given a list of error recovery rules return an error recovery policy.""" + + def _policy( + config: Config, + failed_command: Command, + defined_error_data: Optional[CommandDefinedErrorData], + ) -> ErrorRecoveryType: + for rule in rules: + for i, criteria in enumerate(rule.matchCriteria): + command_type_matches = ( + failed_command.commandType == criteria.command.commandType + ) + error_type_matches = ( + defined_error_data is not None + and defined_error_data.public.errorType + == criteria.command.error.errorType + ) + if command_type_matches and error_type_matches: + if rule.ifMatch[i] == ReactionIfMatch.IGNORE_AND_CONTINUE: + raise NotImplementedError # No protocol engine support for this yet. It's in EXEC-302. + elif rule.ifMatch[i] == ReactionIfMatch.FAIL_RUN: + return ErrorRecoveryType.FAIL_RUN + elif rule.ifMatch[i] == ReactionIfMatch.WAIT_FOR_RECOVERY: + return ErrorRecoveryType.WAIT_FOR_RECOVERY + + return standard_run_policy(config, failed_command, defined_error_data) + + return _policy diff --git a/robot-server/robot_server/runs/error_recovery_models.py b/robot-server/robot_server/runs/error_recovery_models.py new file mode 100644 index 00000000000..95a5a1e5631 --- /dev/null +++ b/robot-server/robot_server/runs/error_recovery_models.py @@ -0,0 +1,69 @@ +"""Request and response models for dealing with error recovery policies.""" +from enum import Enum +from pydantic import BaseModel, Field + + +class ReactionIfMatch(Enum): + """The type of the error recovery setting. + + * `"ignoreAndContinue"`: Ignore this error and future errors of the same type. + * `"failRun"`: Errors of this type should fail the run. + * `"waitForRecovery"`: Instances of this error should initiate a recover operation. + + """ + + IGNORE_AND_CONTINUE = "ignoreAndContinue" + FAIL_RUN = "failRun" + WAIT_FOR_RECOVERY = "waitForRecovery" + + +# There's a lot of nested classes here. This is the JSON schema this code models. +# "ErrorRecoveryRule": { +# "matchCriteria": { +# "command": { +# "commandType": "foo", +# "error": { +# "errorType": "bar" +# } +# } +# }, +# "ifMatch": "baz" +# } + + +class ErrorMatcher(BaseModel): + """The error type that this rule applies to.""" + + errorType: str = Field(..., description="The error type that this rule applies to.") + + +class CommandMatcher(BaseModel): + """Command/error data used for matching rules.""" + + commandType: str = Field( + ..., description="The command type that this rule applies to." + ) + error: ErrorMatcher = Field( + ..., description="The error details that this rule applies to." + ) + + +class MatchCriteria(BaseModel): + """The criteria that this rule will attempt to match.""" + + command: CommandMatcher = Field( + ..., description="The command and error types that this rule applies to." + ) + + +class ErrorRecoveryRule(BaseModel): + """Request/Response model for new error recovery rule creation.""" + + matchCriteria: list[MatchCriteria] = Field( + default_factory=list, + description="The criteria that must be met for this rule to be applied.", + ) + ifMatch: list[ReactionIfMatch] = Field( + default_factory=list, + description="The specific recovery setting that will be in use if the type parameters match.", + ) diff --git a/robot-server/robot_server/service/errors.py b/robot-server/robot_server/service/errors.py index f9bd269b965..94a8d758563 100644 --- a/robot-server/robot_server/service/errors.py +++ b/robot-server/robot_server/service/errors.py @@ -1,5 +1,6 @@ # TODO(mc, 2021-05-10): delete this file; these models have been moved to # robot_server/errors/error_responses.py and robot_server/errors/global_errors.py +# Note: (2024-07-18): this file does not actually seem to be safe to delete from dataclasses import dataclass, asdict from enum import Enum from typing import Any, Dict, Optional, Sequence, Tuple diff --git a/robot-server/tests/runs/test_error_recovery_mapping.py b/robot-server/tests/runs/test_error_recovery_mapping.py new file mode 100644 index 00000000000..4d01ad50085 --- /dev/null +++ b/robot-server/tests/runs/test_error_recovery_mapping.py @@ -0,0 +1,118 @@ +"""Unit tests for `error_recovery_mapping`.""" +import pytest +from decoy import Decoy + + +from opentrons.protocol_engine.commands.pipetting_common import ( + LiquidNotFoundError, + LiquidNotFoundErrorInternalData, +) +from opentrons.protocol_engine.commands.command import ( + DefinedErrorData, +) +from opentrons.protocol_engine.commands.command_unions import CommandDefinedErrorData +from opentrons.protocol_engine.commands.liquid_probe import LiquidProbe +from opentrons.protocol_engine.error_recovery_policy import ErrorRecoveryType +from opentrons.protocol_engine.state.config import Config +from opentrons.protocol_engine.types import DeckType +from robot_server.runs.error_recovery_mapping import ( + create_error_recovery_policy_from_rules, +) +from robot_server.runs.error_recovery_models import ( + ErrorRecoveryRule, + MatchCriteria, + CommandMatcher, + ErrorMatcher, + ReactionIfMatch, +) + + +@pytest.fixture +def mock_command(decoy: Decoy) -> LiquidProbe: + """Get a mock PickUpTip command.""" + mock = decoy.mock(cls=LiquidProbe) + decoy.when(mock.commandType).then_return("liquidProbe") + return mock + + +@pytest.fixture +def mock_error_data(decoy: Decoy) -> CommandDefinedErrorData: + """Get a mock TipPhysicallyMissingError.""" + mock = decoy.mock( + cls=DefinedErrorData[LiquidNotFoundError, LiquidNotFoundErrorInternalData] + ) + mock_lnfe = decoy.mock(cls=LiquidNotFoundError) + decoy.when(mock.public).then_return(mock_lnfe) + decoy.when(mock_lnfe.errorType).then_return("liquidNotFound") + return mock + + +@pytest.fixture +def mock_criteria(decoy: Decoy) -> MatchCriteria: + """Get a mock Match Criteria.""" + mock = decoy.mock(cls=MatchCriteria) + mock_command = decoy.mock(cls=CommandMatcher) + decoy.when(mock_command.commandType).then_return("liquidProbe") + mock_error_matcher = decoy.mock(cls=ErrorMatcher) + decoy.when(mock_error_matcher.errorType).then_return("liquidNotFound") + decoy.when(mock.command).then_return(mock_command) + decoy.when(mock_command.error).then_return(mock_error_matcher) + return mock + + +@pytest.fixture +def mock_rule(decoy: Decoy, mock_criteria: MatchCriteria) -> ErrorRecoveryRule: + """Get a mock ErrorRecoveryRule.""" + mock = decoy.mock(cls=ErrorRecoveryRule) + decoy.when(mock.ifMatch).then_return([ReactionIfMatch.IGNORE_AND_CONTINUE]) + decoy.when(mock.matchCriteria).then_return([mock_criteria]) + return mock + + +def test_create_error_recovery_policy_with_rules( + decoy: Decoy, + mock_command: LiquidProbe, + mock_error_data: CommandDefinedErrorData, + mock_rule: ErrorRecoveryRule, +) -> None: + """Should return IGNORE_AND_CONTINUE if that's what we specify as the rule.""" + policy = create_error_recovery_policy_from_rules([mock_rule]) + exampleConfig = Config( + robot_type="OT-3 Standard", + deck_type=DeckType.OT3_STANDARD, + ) + with pytest.raises(NotImplementedError): + policy(exampleConfig, mock_command, mock_error_data) + + +def test_create_error_recovery_policy_undefined_error( + decoy: Decoy, mock_command: LiquidProbe +) -> None: + """Should return a FAIL_RUN policy when error is not defined.""" + rule1 = ErrorRecoveryRule(matchCriteria=[], ifMatch=[]) + + policy = create_error_recovery_policy_from_rules([rule1]) + exampleConfig = Config( + robot_type="OT-3 Standard", + deck_type=DeckType.OT3_STANDARD, + ) + + assert policy(exampleConfig, mock_command, None) == ErrorRecoveryType.FAIL_RUN + + +def test_create_error_recovery_policy_defined_error( + decoy: Decoy, mock_command: LiquidProbe, mock_error_data: CommandDefinedErrorData +) -> None: + """Should return a WAIT_FOR_RECOVERY policy when error is defined.""" + rule1 = ErrorRecoveryRule(matchCriteria=[], ifMatch=[]) + + policy = create_error_recovery_policy_from_rules([rule1]) + exampleConfig = Config( + robot_type="OT-3 Standard", + deck_type=DeckType.OT3_STANDARD, + ) + + assert ( + policy(exampleConfig, mock_command, mock_error_data) + == ErrorRecoveryType.WAIT_FOR_RECOVERY + ) From 75e2ee68cb5cfab3eea5c07c47fd367e5f03d6f9 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Wed, 24 Jul 2024 13:55:30 -0400 Subject: [PATCH 3/8] feat(app): add error recovery "error while dispensing" flows to desktop (#15774) Closes EXEC-559 --- .../localization/en/error_recovery.json | 2 +- .../DropTipWizardFlows/DropTipWizard.tsx | 4 +++ app/src/organisms/DropTipWizardFlows/types.ts | 1 + .../RecoveryOptions/ManageTips.tsx | 27 ++++++++++++++++- .../__tests__/ManageTips.test.tsx | 25 +++++++++++++++- .../ErrorRecoveryFlows/shared/SelectTips.tsx | 29 +++++++++++++++---- 6 files changed, 80 insertions(+), 8 deletions(-) diff --git a/app/src/assets/localization/en/error_recovery.json b/app/src/assets/localization/en/error_recovery.json index c139f21acd2..8bf00af168d 100644 --- a/app/src/assets/localization/en/error_recovery.json +++ b/app/src/assets/localization/en/error_recovery.json @@ -17,7 +17,7 @@ "continue_to_drop_tip": "Continue to drop tip", "error": "Error", "error_on_robot": "Error on {{robot}}", - "failed_dispense_step_not_completed": "The failed dispense step will not be completed. The run will continue from the next step.Close the robot door before proceeding.", + "failed_dispense_step_not_completed": "The failed dispense step will not be completed. The run will continue from the next step with the attached tips.Close the robot door before proceeding.", "failed_step": "Failed step", "first_take_any_necessary_actions": "First, take any necessary actions to prepare the robot to retry the failed step.Then, close the robot door before proceeding.", "go_back": "Go back", diff --git a/app/src/organisms/DropTipWizardFlows/DropTipWizard.tsx b/app/src/organisms/DropTipWizardFlows/DropTipWizard.tsx index b5cd5730e52..329ec38d199 100644 --- a/app/src/organisms/DropTipWizardFlows/DropTipWizard.tsx +++ b/app/src/organisms/DropTipWizardFlows/DropTipWizard.tsx @@ -318,10 +318,14 @@ export const DropTipWizardContent = ( } function buildSuccess(): JSX.Element { + const { tipDropComplete } = fixitCommandTypeUtils?.buttonOverrides ?? {} + // Route to the drop tip route if user is at the blowout success screen, otherwise proceed conditionally. const handleProceed = (): void => { if (currentStep === BLOWOUT_SUCCESS) { void proceedToRoute(DT_ROUTES.DROP_TIP) + } else if (tipDropComplete != null) { + tipDropComplete() } else { proceedWithConditionalClose() } diff --git a/app/src/organisms/DropTipWizardFlows/types.ts b/app/src/organisms/DropTipWizardFlows/types.ts index d7a8309b60b..f4aa36266ae 100644 --- a/app/src/organisms/DropTipWizardFlows/types.ts +++ b/app/src/organisms/DropTipWizardFlows/types.ts @@ -27,6 +27,7 @@ interface ErrorOverrides { interface ButtonOverrides { goBackBeforeBeginning: () => void + tipDropComplete: (() => void) | null } export interface FixitCommandTypeUtils { diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx index 87a195a12a8..5e6d6685195 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx @@ -30,7 +30,7 @@ import { DT_ROUTES } from '../../DropTipWizardFlows/constants' import { SelectRecoveryOption } from './SelectRecoveryOption' import type { PipetteWithTip } from '../../DropTipWizardFlows' -import type { RecoveryContentProps } from '../types' +import type { RecoveryContentProps, RecoveryRoute, RouteStep } from '../types' import type { FixitCommandTypeUtils } from '../../DropTipWizardFlows/types' // The Drop Tip flow entry point. Includes entry from SelectRecoveryOption and CancelRun. @@ -251,6 +251,7 @@ export function useDropTipFlowUtils({ const { t } = useTranslation('error_recovery') const { RETRY_NEW_TIPS, + SKIP_STEP_WITH_NEW_TIPS, ERROR_WHILE_RECOVERING, DROP_TIP_FLOWS, } = RECOVERY_MAP @@ -263,12 +264,35 @@ export function useDropTipFlowUtils({ const buildTipDropCompleteBtn = (): string => { switch (selectedRecoveryOption) { case RETRY_NEW_TIPS.ROUTE: + case SKIP_STEP_WITH_NEW_TIPS.ROUTE: return t('proceed_to_tip_selection') default: return t('proceed_to_cancel') } } + const buildTipDropCompleteRouting = (): (() => void) | null => { + const routeTo = (selectedRoute: RecoveryRoute, step: RouteStep): void => { + void proceedToRouteAndStep(selectedRoute, step) + } + + switch (selectedRecoveryOption) { + case RETRY_NEW_TIPS.ROUTE: + return () => { + routeTo(selectedRecoveryOption, RETRY_NEW_TIPS.STEPS.REPLACE_TIPS) + } + case SKIP_STEP_WITH_NEW_TIPS.ROUTE: + return () => { + routeTo( + selectedRecoveryOption, + SKIP_STEP_WITH_NEW_TIPS.STEPS.REPLACE_TIPS + ) + } + default: + return null + } + } + const buildCopyOverrides = (): FixitCommandTypeUtils['copyOverrides'] => { return { tipDropCompleteBtnCopy: buildTipDropCompleteBtn(), @@ -303,6 +327,7 @@ export function useDropTipFlowUtils({ goBackBeforeBeginning: () => { return proceedToRouteAndStep(DROP_TIP_FLOWS.ROUTE) }, + tipDropComplete: buildTipDropCompleteRouting(), } } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ManageTips.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ManageTips.test.tsx index ed58d7e597a..aa8a7063463 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ManageTips.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ManageTips.test.tsx @@ -264,11 +264,34 @@ describe('useDropTipFlowUtils', () => { }) it('should return the correct button overrides', () => { - const { result } = renderHook(() => useDropTipFlowUtils(mockProps)) + const { result } = renderHook(() => + useDropTipFlowUtils({ + ...mockProps, + recoveryMap: { + route: RETRY_NEW_TIPS.ROUTE, + step: RETRY_NEW_TIPS.STEPS.DROP_TIPS, + }, + currentRecoveryOptionUtils: { + selectedRecoveryOption: RETRY_NEW_TIPS.ROUTE, + } as any, + }) + ) + const { tipDropComplete } = result.current.buttonOverrides result.current.buttonOverrides.goBackBeforeBeginning() expect(mockProceedToRouteAndStep).toHaveBeenCalledWith(DROP_TIP_FLOWS.ROUTE) + + expect(tipDropComplete).toBeDefined() + + if (tipDropComplete != null) { + tipDropComplete() + } + + expect(mockProceedToRouteAndStep).toHaveBeenCalledWith( + RETRY_NEW_TIPS.ROUTE, + RETRY_NEW_TIPS.STEPS.REPLACE_TIPS + ) }) it(`should return correct route overrides when the route is ${DROP_TIP_FLOWS.STEPS.CHOOSE_TIP_DROP}`, () => { diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx index 0b6f66aa484..d4012670c27 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/SelectTips.tsx @@ -12,7 +12,12 @@ import { TipSelection } from './TipSelection' import type { RecoveryContentProps } from '../types' export function SelectTips(props: RecoveryContentProps): JSX.Element | null { - const { failedPipetteInfo, routeUpdateActions, recoveryCommands } = props + const { + failedPipetteInfo, + routeUpdateActions, + recoveryCommands, + isOnDevice, + } = props const { ROBOT_PICKING_UP_TIPS } = RECOVERY_MAP const { pickUpTips } = recoveryCommands const { @@ -33,6 +38,22 @@ export function SelectTips(props: RecoveryContentProps): JSX.Element | null { setShowTipSelectModal(!showTipSelectModal) } + const buildTertiaryBtnProps = (): { + tertiaryBtnDisabled?: boolean + tertiaryBtnOnClick?: () => void + tertiaryBtnText?: string + } => { + if (isOnDevice) { + return { + tertiaryBtnDisabled: failedPipetteInfo?.data.channels === 96, + tertiaryBtnOnClick: toggleModal, + tertiaryBtnText: t('change_location'), + } + } else { + return {} + } + } + return ( <> {showTipSelectModal && ( @@ -50,15 +71,13 @@ export function SelectTips(props: RecoveryContentProps): JSX.Element | null { type="location" bannerText={t('replace_tips_and_select_location')} /> - + From 6864eabfc92bdccdaa1657d3912cf21d850f73a2 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Wed, 24 Jul 2024 13:55:57 -0400 Subject: [PATCH 4/8] feat(app): Add Error Recovery ErrorDetails desktop support (#15784) Closes EXEC-627 --- .../localization/en/error_recovery.json | 5 +- app/src/molecules/InterventionModal/index.tsx | 2 + .../Devices/ProtocolRun/ProtocolRunHeader.tsx | 1 - .../ErrorRecoveryWizard.tsx | 18 +- .../RecoveryOptions/ManageTips.tsx | 5 +- .../ErrorRecoveryFlows/RunPausedSplash.tsx | 6 +- .../ErrorRecoveryFlows/__fixtures__/index.ts | 1 - .../__tests__/ErrorRecoveryFlows.test.tsx | 1 - .../organisms/ErrorRecoveryFlows/constants.ts | 6 +- .../ErrorRecoveryFlows/hooks/useERUtils.ts | 4 +- .../organisms/ErrorRecoveryFlows/index.tsx | 1 - .../shared/ErrorDetailsModal.tsx | 159 +++++++++++++++--- .../shared/RecoveryInterventionModal.tsx | 3 +- .../ErrorRecoveryFlows/shared/StepInfo.tsx | 23 ++- .../__tests__/ErrorDetailsModal.test.tsx | 59 +++---- .../shared/__tests__/StepInfo.test.tsx | 2 +- app/src/organisms/ErrorRecoveryFlows/types.ts | 2 + app/src/pages/RunningProtocol/index.tsx | 1 - 18 files changed, 212 insertions(+), 87 deletions(-) diff --git a/app/src/assets/localization/en/error_recovery.json b/app/src/assets/localization/en/error_recovery.json index 8bf00af168d..65f1fa50314 100644 --- a/app/src/assets/localization/en/error_recovery.json +++ b/app/src/assets/localization/en/error_recovery.json @@ -16,6 +16,7 @@ "continue_run_now": "Continue run now", "continue_to_drop_tip": "Continue to drop tip", "error": "Error", + "error_details": "Error details", "error_on_robot": "Error on {{robot}}", "failed_dispense_step_not_completed": "The failed dispense step will not be completed. The run will continue from the next step with the attached tips.Close the robot door before proceeding.", "failed_step": "Failed step", @@ -40,6 +41,7 @@ "recovery_action_failed": "{{action}} failed", "recovery_mode": "Recovery Mode", "recovery_mode_explanation": "Recovery Mode provides you with guided and manual controls for handling errors at runtime.
You can make changes to ensure the step in progress when the error occurred can be completed or choose to cancel the protocol. When changes are made and no subsequent errors are detected, the method completes. Depending on the conditions that caused the error, you will only be provided with appropriate options.", + "remove_tips_from_pipette": "Remove tips from {{mount}} pipette before canceling the run?", "replace_tips_and_select_location": "It's best to replace tips and select the last location used for tip pickup.", "replace_used_tips_in_rack_location": "Replace used tips in rack location {{location}} in slot {{slot}}", "replace_with_new_tip_rack": "Replace with new tip rack in slot {{slot}}", @@ -72,6 +74,5 @@ "tip_not_detected": "Tip not detected", "view_error_details": "View error details", "view_recovery_options": "View recovery options", - "you_can_still_drop_tips": "You can still drop the attached tips before proceeding to tip selection.", - "remove_tips_from_pipette": "Remove tips from {{mount}} pipette before canceling the run?" + "you_can_still_drop_tips": "You can still drop the attached tips before proceeding to tip selection." } diff --git a/app/src/molecules/InterventionModal/index.tsx b/app/src/molecules/InterventionModal/index.tsx index 3faa3b34f2c..aec8c9fea22 100644 --- a/app/src/molecules/InterventionModal/index.tsx +++ b/app/src/molecules/InterventionModal/index.tsx @@ -179,6 +179,8 @@ const ICON_STYLE = css` width: ${SPACING.spacing16}; height: ${SPACING.spacing16}; margin: ${SPACING.spacing4}; + cursor: pointer; + @media (${RESPONSIVENESS.touchscreenMediaQuerySpecs}) { width: ${SPACING.spacing32}; height: ${SPACING.spacing32}; diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx index 9b090a67259..f8338f2521b 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx @@ -297,7 +297,6 @@ export function ProtocolRunHeader({ <> {isERActive ? ( ( - + {t('view_error_details')} ) @@ -119,6 +126,7 @@ export function ErrorRecoveryComponent( !isDoorOpen && route === RECOVERY_MAP.DROP_TIP_FLOWS.ROUTE && step !== RECOVERY_MAP.DROP_TIP_FLOWS.STEPS.BEGIN_REMOVAL + const desktopType = isLargeDesktopStyle ? 'desktop-large' : 'desktop-small' return ( {showModal ? ( - + ) : null} {buildInterventionContent()} diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx index 5e6d6685195..9fe5f22c413 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ManageTips.tsx @@ -11,7 +11,6 @@ import { StyledText, RESPONSIVENESS, } from '@opentrons/components' -import { FLEX_ROBOT_TYPE, OT2_ROBOT_TYPE } from '@opentrons/shared-data' import { RadioButton } from '../../../atoms/buttons' import { @@ -187,10 +186,10 @@ function DropTipFlowsContainer( props: RecoveryContentProps ): JSX.Element | null { const { + robotType, tipStatusUtils, routeUpdateActions, recoveryCommands, - isFlex, currentRecoveryOptionUtils, } = props const { DROP_TIP_FLOWS, ROBOT_CANCELING, RETRY_NEW_TIPS } = RECOVERY_MAP @@ -229,7 +228,7 @@ function DropTipFlowsContainer( return ( {title} { runStatus: RUN_STATUS_AWAITING_RECOVERY, failedCommand: mockFailedCommand, runId: 'MOCK_RUN_ID', - isFlex: true, protocolAnalysis: {} as any, } vi.mocked(ErrorRecoveryWizard).mockReturnValue(
MOCK WIZARD
) diff --git a/app/src/organisms/ErrorRecoveryFlows/constants.ts b/app/src/organisms/ErrorRecoveryFlows/constants.ts index 846f7e2efc0..d61805d1777 100644 --- a/app/src/organisms/ErrorRecoveryFlows/constants.ts +++ b/app/src/organisms/ErrorRecoveryFlows/constants.ts @@ -1,6 +1,6 @@ import { css } from 'styled-components' -import { SPACING, TYPOGRAPHY, RESPONSIVENESS } from '@opentrons/components' +import { SPACING, RESPONSIVENESS } from '@opentrons/components' import type { StepOrder } from './types' @@ -204,10 +204,6 @@ export const INVALID = 'INVALID' as const * Styling */ -export const BODY_TEXT_STYLE = css` - ${TYPOGRAPHY.bodyTextRegular}; -` - export const ODD_SECTION_TITLE_STYLE = css` margin-bottom: ${SPACING.spacing16}; ` diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts index ff05642ff18..c0d867ea25a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts @@ -1,4 +1,5 @@ import { useInstrumentsQuery } from '@opentrons/react-api-client' +import { FLEX_ROBOT_TYPE } from '@opentrons/shared-data' import { useRouteUpdateActions } from './useRouteUpdateActions' import { useRecoveryCommands } from './useRecoveryCommands' @@ -56,7 +57,6 @@ export interface ERUtilsResults { const SUBSEQUENT_COMMAND_DEPTH = 2 // Builds various Error Recovery utilities. export function useERUtils({ - isFlex, failedCommand, runId, toggleERWizard, @@ -96,7 +96,7 @@ export function useERUtils({ const tipStatusUtils = useRecoveryTipStatus({ runId, - isFlex, + isFlex: robotType === FLEX_ROBOT_TYPE, runRecord, attachedInstruments, }) diff --git a/app/src/organisms/ErrorRecoveryFlows/index.tsx b/app/src/organisms/ErrorRecoveryFlows/index.tsx index 6e4e2bf1fd3..677cd512986 100644 --- a/app/src/organisms/ErrorRecoveryFlows/index.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/index.tsx @@ -106,7 +106,6 @@ export interface ErrorRecoveryFlowsProps { runId: string runStatus: RunStatus | null failedCommand: FailedCommand | null - isFlex: boolean protocolAnalysis: CompletedProtocolAnalysis | null } diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/ErrorDetailsModal.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/ErrorDetailsModal.tsx index de4829d937f..f1921b83d02 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/ErrorDetailsModal.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/ErrorDetailsModal.tsx @@ -1,9 +1,11 @@ import * as React from 'react' import { useTranslation } from 'react-i18next' import { createPortal } from 'react-dom' +import { css } from 'styled-components' import { Flex, + StyledText, SPACING, COLORS, BORDERS, @@ -12,16 +14,22 @@ import { import { useErrorName } from '../hooks' import { Modal } from '../../../molecules/Modal' -import { getTopPortalEl } from '../../../App/portal' +import { getModalPortalEl, getTopPortalEl } from '../../../App/portal' import { ERROR_KINDS } from '../constants' import { InlineNotification } from '../../../atoms/InlineNotification' import { StepInfo } from './StepInfo' import { getErrorKind } from '../utils' +import { + LegacyModalShell, + LegacyModalHeader, +} from '../../../molecules/LegacyModal' import type { RobotType } from '@opentrons/shared-data' +import type { IconProps } from '@opentrons/components' import type { ModalHeaderBaseProps } from '../../../molecules/Modal/types' import type { ERUtilsResults } from '../hooks' import type { ErrorRecoveryFlowsProps } from '..' +import type { DesktopSizeType } from '../types' export function useErrorDetailsModal(): { showModal: boolean @@ -41,6 +49,7 @@ type ErrorDetailsModalProps = ErrorRecoveryFlowsProps & toggleModal: () => void isOnDevice: boolean robotType: RobotType + desktopType: DesktopSizeType } export function ErrorDetailsModal(props: ErrorDetailsModalProps): JSX.Element { @@ -64,7 +73,101 @@ export function ErrorDetailsModal(props: ErrorDetailsModalProps): JSX.Element { hasExitIcon: true, } - return createPortal( + const buildModal = (): JSX.Element => { + if (isOnDevice) { + return createPortal( + + {getIsOverpressureErrorKind() ? : null} + , + getTopPortalEl() + ) + } else { + return createPortal( + + {getIsOverpressureErrorKind() ? : null} + , + getModalPortalEl() + ) + } + } + + return buildModal() +} + +type ErrorDetailsModalType = ErrorDetailsModalProps & { + children: React.ReactNode + modalHeader: ModalHeaderBaseProps + toggleModal: () => void + desktopType: DesktopSizeType +} + +export function ErrorDetailsModalDesktop( + props: ErrorDetailsModalType +): JSX.Element { + const { children, modalHeader, toggleModal, desktopType } = props + const { t } = useTranslation('error_recovery') + + const buildIcon = (): IconProps => { + return { + name: 'information', + color: COLORS.grey60, + size: SPACING.spacing20, + marginRight: SPACING.spacing8, + } + } + + const buildHeader = (): JSX.Element => { + return ( + + ) + } + + return ( + + + + {modalHeader.title} + + {children} + + + + + + ) +} + +export function ErrorDetailsModalODD( + props: ErrorDetailsModalType +): JSX.Element { + const { children, modalHeader, toggleModal } = props + + return ( - {getIsOverpressureErrorKind() ? ( - - ) : null} + {children} - + - , - getTopPortalEl() + ) } -export function OverpressureBanner(props: { - isOnDevice: boolean -}): JSX.Element | null { +export function OverpressureBanner(): JSX.Element | null { const { t } = useTranslation('error_recovery') - if (props.isOnDevice) { - return ( - - ) - } else { - return null - } + return ( + + ) } + +// TODO(jh, 07-24-24): Using shared height/width constants for intervention modal sizing and the ErrorDetailsModal sizing +// would be ideal. +const DESKTOP_STEP_INFO_STYLE = css` + background-color: ${COLORS.grey30}; + grid-gap: ${SPACING.spacing10}; + border-radius: ${BORDERS.borderRadius4}; + padding: ${SPACING.spacing6} ${SPACING.spacing24} ${SPACING.spacing6} + ${SPACING.spacing12}; +` + +const DESKTOP_MODAL_STYLE_BASE = css` + width: 47rem; +` + +const DESKTOP_MODAL_STYLE_SMALL = css` + ${DESKTOP_MODAL_STYLE_BASE} + height: 26rem; +` +const DESKTOP_MODAL_STYLE_LARGE = css` + ${DESKTOP_MODAL_STYLE_BASE} + height: 31rem; +` diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx index 00a853ee99a..e044d46054f 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx @@ -8,13 +8,14 @@ import { InterventionModal } from '../../../molecules/InterventionModal' import { getModalPortalEl, getTopPortalEl } from '../../../App/portal' import type { ModalType } from '../../../molecules/InterventionModal' +import type { DesktopSizeType } from '../types' export type RecoveryInterventionModalProps = Omit< React.ComponentProps, 'type' > & { /* If on desktop, specifies the hard-coded dimensions height of the modal. */ - desktopType: 'desktop-small' | 'desktop-large' + desktopType: DesktopSizeType isOnDevice: boolean } diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/StepInfo.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/StepInfo.tsx index 54fb2464124..13cb6f3a702 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/StepInfo.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/StepInfo.tsx @@ -2,7 +2,7 @@ import * as React from 'react' import { useTranslation } from 'react-i18next' -import { Flex, DISPLAY_INLINE, LegacyStyledText } from '@opentrons/components' +import { Flex, DISPLAY_INLINE, StyledText } from '@opentrons/components' import { CommandText } from '../../../molecules/Command' @@ -10,15 +10,17 @@ import type { StyleProps } from '@opentrons/components' import type { RecoveryContentProps } from '../types' interface StepInfoProps extends StyleProps { - textStyle: React.ComponentProps['as'] stepCounts: RecoveryContentProps['stepCounts'] failedCommand: RecoveryContentProps['failedCommand'] robotType: RecoveryContentProps['robotType'] protocolAnalysis: RecoveryContentProps['protocolAnalysis'] + desktopStyle?: React.ComponentProps['desktopStyle'] + oddStyle?: React.ComponentProps['oddStyle'] } export function StepInfo({ - textStyle, + desktopStyle, + oddStyle, stepCounts, failedCommand, robotType, @@ -35,18 +37,27 @@ export function StepInfo({ const currentCopy = currentStepNumber ?? '?' const totalCopy = totalStepCount ?? '?' + const desktopStyleDefaulted = desktopStyle ?? 'bodyDefaultRegular' + const oddStyleDefaulted = oddStyle ?? 'bodyTextRegular' + return ( - + {`${t('at_step')} ${currentCopy}/${totalCopy}: `} - + {analysisCommand != null && protocolAnalysis != null ? ( ) : null} diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/ErrorDetailsModal.test.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/ErrorDetailsModal.test.tsx index 3eb590f1a35..b63464b4382 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/ErrorDetailsModal.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/ErrorDetailsModal.test.tsx @@ -44,27 +44,6 @@ describe('useErrorDetailsModal', () => { }) }) -describe('ErrorDetailsModal', () => { - let props: React.ComponentProps - - beforeEach(() => { - props = { - ...mockRecoveryContentProps, - toggleModal: vi.fn(), - robotType: 'OT-3 Standard', - } - - vi.mocked(StepInfo).mockReturnValue(
MOCK_STEP_INFO
) - }) - - it('renders ErrorDetailsModal', () => { - renderWithProviders(, { - i18nInstance: i18n, - }) - expect(screen.getByText('MOCK_STEP_INFO')).toBeInTheDocument() - }) -}) - const render = (props: React.ComponentProps) => { return renderWithProviders(, { i18nInstance: i18n, @@ -79,6 +58,7 @@ describe('ErrorDetailsModal', () => { ...mockRecoveryContentProps, toggleModal: vi.fn(), robotType: 'OT-3 Standard', + desktopType: 'desktop-small', } vi.mocked(StepInfo).mockReturnValue(
MOCK_STEP_INFO
) @@ -87,7 +67,9 @@ describe('ErrorDetailsModal', () => { ) }) - it('renders the modal with the correct content', () => { + const IS_ODD = [true, false] + + it('renders the ODD modal with the correct content', () => { render(props) expect(vi.mocked(Modal)).toHaveBeenCalledWith( expect.objectContaining({ @@ -102,21 +84,30 @@ describe('ErrorDetailsModal', () => { expect(screen.getByText('MOCK_STEP_INFO')).toBeInTheDocument() }) - it('renders the OverpressureBanner when the error kind is an overpressure error', () => { - props.failedCommand = { - ...props.failedCommand, - commandType: 'aspirate', - error: { isDefined: true, errorType: 'overpressure' }, - } as any - render(props) + it('renders the desktop modal with the correct content', () => { + render({ ...props, isOnDevice: false }) - screen.getByText('MOCK_INLINE_NOTIFICATION') + screen.getByText('MOCK_STEP_INFO') + screen.getByText('Error details') }) - it('does not render the OverpressureBanner when the error kind is not an overpressure error', () => { - render(props) + IS_ODD.forEach(isOnDevice => { + it('renders the OverpressureBanner when the error kind is an overpressure error', () => { + props.failedCommand = { + ...props.failedCommand, + commandType: 'aspirate', + error: { isDefined: true, errorType: 'overpressure' }, + } as any + render({ ...props, isOnDevice }) + + screen.getByText('MOCK_INLINE_NOTIFICATION') + }) + + it('does not render the OverpressureBanner when the error kind is not an overpressure error', () => { + render({ ...props, isOnDevice }) - expect(screen.queryByText('MOCK_INLINE_NOTIFICATION')).toBeNull() + expect(screen.queryByText('MOCK_INLINE_NOTIFICATION')).toBeNull() + }) }) }) @@ -128,7 +119,7 @@ describe('OverpressureBanner', () => { }) it('renders the InlineNotification', () => { - renderWithProviders(, { + renderWithProviders(, { i18nInstance: i18n, }) expect(vi.mocked(InlineNotification)).toHaveBeenCalledWith( diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx index 4e7e8b393fa..9396fcf8f7d 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/StepInfo.test.tsx @@ -25,7 +25,7 @@ describe('StepInfo', () => { ...mockRecoveryContentProps, protocolAnalysis: { commands: [mockFailedCommand] } as any, }, - textStyle: 'h4', + desktopStyle: 'bodyDefaultRegular', stepCounts: { currentStepNumber: 5, totalStepCount: 10, diff --git a/app/src/organisms/ErrorRecoveryFlows/types.ts b/app/src/organisms/ErrorRecoveryFlows/types.ts index c1f0ea49329..747000f2dbb 100644 --- a/app/src/organisms/ErrorRecoveryFlows/types.ts +++ b/app/src/organisms/ErrorRecoveryFlows/types.ts @@ -63,3 +63,5 @@ export type RecoveryContentProps = ErrorRecoveryWizardProps & { errorKind: ErrorKind isOnDevice: boolean } + +export type DesktopSizeType = 'desktop-small' | 'desktop-large' diff --git a/app/src/pages/RunningProtocol/index.tsx b/app/src/pages/RunningProtocol/index.tsx index 5240196e9e1..869fc7c61d8 100644 --- a/app/src/pages/RunningProtocol/index.tsx +++ b/app/src/pages/RunningProtocol/index.tsx @@ -164,7 +164,6 @@ export function RunningProtocol(): JSX.Element { {isERActive ? ( Date: Wed, 24 Jul 2024 15:04:46 -0400 Subject: [PATCH 5/8] feat(app,api): Move to right position before retrying in place command (#15776) This PR adds something in between a nice feature and a hack for retrying commands like `AspirateInPlace` in error recovery. The fundamental problem is that these commands' parameters do not contain the location where you're running them, and we know that we've moved the gantry since they failed. The fix is to add an optional `retryLocation` value to the error info of the well-defined overpressure command (and subsequently to any other well-defined error that will be raised from these commands), as in 2ae195619b6e19e3163528b05b3ba3e8300fac49 ; then use that location as the target of a MoveToCoordinates before we actually retry the target command if necessary, as we do in 9183301e27d351ad4a2b560ad29a52c73ed99c36 ## Testing - [x] Pass tests, mostly. I wanted to keep this separate from actually implementing the errors in those commands just to have a nicer commit log, so you can't really test it right now. I figured I'd follow up with a PR that implements it in those commands and apply any needed fixes there. --- .../protocol_engine/commands/aspirate.py | 1 + .../commands/pipetting_common.py | 10 ++- .../errors/error_occurrence.py | 4 +- .../protocol_engine/commands/test_aspirate.py | 5 +- .../state/test_pipette_store.py | 2 + .../__tests__/useRecoveryCommands.test.ts | 53 +++++++++++++++ .../hooks/useRecoveryCommands.ts | 65 +++++++++++++++++-- .../utils/__tests__/getErrorKind.test.ts | 4 +- .../__tests__/RunFailedModal.test.tsx | 20 +++--- shared-data/command/types/index.ts | 25 +++++-- shared-data/errors/index.ts | 4 ++ shared-data/errors/types/index.ts | 15 +++++ shared-data/tsconfig.json | 5 +- 13 files changed, 185 insertions(+), 28 deletions(-) create mode 100644 shared-data/errors/index.ts create mode 100644 shared-data/errors/types/index.ts diff --git a/api/src/opentrons/protocol_engine/commands/aspirate.py b/api/src/opentrons/protocol_engine/commands/aspirate.py index 46e1147a559..29daea563bb 100644 --- a/api/src/opentrons/protocol_engine/commands/aspirate.py +++ b/api/src/opentrons/protocol_engine/commands/aspirate.py @@ -138,6 +138,7 @@ async def execute(self, params: AspirateParams) -> _ExecuteReturn: error=e, ) ], + errorInfo={"retryLocation": (position.x, position.y, position.z)}, ), private=OverpressureErrorInternalData( position=DeckPoint.construct( diff --git a/api/src/opentrons/protocol_engine/commands/pipetting_common.py b/api/src/opentrons/protocol_engine/commands/pipetting_common.py index 408b1c71478..898cb6f1850 100644 --- a/api/src/opentrons/protocol_engine/commands/pipetting_common.py +++ b/api/src/opentrons/protocol_engine/commands/pipetting_common.py @@ -2,7 +2,7 @@ from dataclasses import dataclass from opentrons_shared_data.errors import ErrorCodes from pydantic import BaseModel, Field -from typing import Literal, Optional +from typing import Literal, Optional, Tuple, TypedDict from opentrons.protocol_engine.errors.error_occurrence import ErrorOccurrence @@ -123,6 +123,12 @@ class DestinationPositionResult(BaseModel): ) +class ErrorLocationInfo(TypedDict): + """Holds a retry location for in-place error recovery.""" + + retryLocation: Tuple[float, float, float] + + class OverpressureError(ErrorOccurrence): """Returned when sensors detect an overpressure error while moving liquid. @@ -138,6 +144,8 @@ class OverpressureError(ErrorOccurrence): errorCode: str = ErrorCodes.PIPETTE_OVERPRESSURE.value.code detail: str = ErrorCodes.PIPETTE_OVERPRESSURE.value.detail + errorInfo: ErrorLocationInfo + @dataclass(frozen=True) class OverpressureErrorInternalData: diff --git a/api/src/opentrons/protocol_engine/errors/error_occurrence.py b/api/src/opentrons/protocol_engine/errors/error_occurrence.py index d890b121c0f..02bcfb38b62 100644 --- a/api/src/opentrons/protocol_engine/errors/error_occurrence.py +++ b/api/src/opentrons/protocol_engine/errors/error_occurrence.py @@ -3,7 +3,7 @@ from datetime import datetime from textwrap import dedent -from typing import Any, Dict, List, Type, Union, Optional, Sequence +from typing import Any, Dict, Mapping, List, Type, Union, Optional, Sequence from pydantic import BaseModel, Field from opentrons_shared_data.errors.codes import ErrorCodes from .exceptions import ProtocolEngineError @@ -118,7 +118,7 @@ def from_failed( ), ) - errorInfo: Dict[str, str] = Field( + errorInfo: Mapping[str, object] = Field( default={}, description=dedent( """\ diff --git a/api/tests/opentrons/protocol_engine/commands/test_aspirate.py b/api/tests/opentrons/protocol_engine/commands/test_aspirate.py index 1dba452ff45..b1e3c1e52df 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_aspirate.py +++ b/api/tests/opentrons/protocol_engine/commands/test_aspirate.py @@ -263,7 +263,10 @@ async def test_overpressure_error( assert result == DefinedErrorData( public=OverpressureError.construct( - id=error_id, createdAt=error_timestamp, wrappedErrors=[matchers.Anything()] + id=error_id, + createdAt=error_timestamp, + wrappedErrors=[matchers.Anything()], + errorInfo={"retryLocation": (position.x, position.y, position.z)}, ), private=OverpressureErrorInternalData( position=DeckPoint(x=position.x, y=position.y, z=position.z) 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 8ccfc06fd07..c60adb96fdb 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -330,6 +330,7 @@ def test_blow_out_clears_volume( public=OverpressureError( id="error-id", createdAt=datetime.now(), + errorInfo={"retryLocation": (0, 0, 0)}, ), private=OverpressureErrorInternalData( position=DeckPoint(x=0, y=0, z=0) @@ -818,6 +819,7 @@ def test_add_pipette_config( id="error-id", detail="error-detail", createdAt=datetime.now(), + errorInfo={"retryLocation": (11, 22, 33)}, ), private=OverpressureErrorInternalData( position=DeckPoint(x=11, y=22, z=33) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts index 1a54da00103..fb7b7367842 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts @@ -125,6 +125,59 @@ describe('useRecoveryCommands', () => { false ) }) + ;([ + 'aspirateInPlace', + 'dispenseInPlace', + 'blowOutInPlace', + 'dropTipInPlace', + 'prepareToAspirate', + ] as const).forEach(inPlaceCommandType => { + it(`Should move to retryLocation if failed command is ${inPlaceCommandType} and error is appropriate when retrying`, async () => { + const { result } = renderHook(() => + useRecoveryCommands({ + runId: mockRunId, + failedCommand: { + ...mockFailedCommand, + commandType: inPlaceCommandType, + params: { + pipetteId: 'mock-pipette-id', + }, + error: { + errorType: 'overpressure', + errorCode: '3006', + isDefined: true, + errorInfo: { + retryLocation: [1, 2, 3], + }, + }, + }, + failedLabwareUtils: mockFailedLabwareUtils, + routeUpdateActions: mockRouteUpdateActions, + recoveryToastUtils: {} as any, + }) + ) + await act(async () => { + await result.current.retryFailedCommand() + }) + expect(mockChainRunCommands).toHaveBeenLastCalledWith( + [ + { + commandType: 'moveToCoordinates', + intent: 'fixit', + params: { + pipetteId: 'mock-pipette-id', + coordinates: { x: 1, y: 2, z: 3 }, + }, + }, + { + commandType: inPlaceCommandType, + params: { pipetteId: 'mock-pipette-id' }, + }, + ], + false + ) + }) + }) it('should call resumeRun with runId and show success toast on success', async () => { const { result } = renderHook(() => diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts index 58c398b33f6..d28fcb6396a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts @@ -9,7 +9,16 @@ import { import { useChainRunCommands } from '../../../resources/runs' import { RECOVERY_MAP } from '../constants' -import type { CreateCommand, LoadedLabware } from '@opentrons/shared-data' +import type { + CreateCommand, + LoadedLabware, + MoveToCoordinatesCreateCommand, + AspirateInPlaceRunTimeCommand, + BlowoutInPlaceRunTimeCommand, + DispenseInPlaceRunTimeCommand, + DropTipInPlaceRunTimeCommand, + PrepareToAspirateRunTimeCommand, +} from '@opentrons/shared-data' import type { CommandData } from '@opentrons/api-client' import type { WellGroup } from '@opentrons/components' import type { FailedCommand } from '../types' @@ -56,6 +65,49 @@ export function useRecoveryCommands({ const { stopRun } = useStopRunMutation() const { makeSuccessToast } = recoveryToastUtils + const buildRetryPrepMove = (): MoveToCoordinatesCreateCommand | null => { + type InPlaceCommand = + | AspirateInPlaceRunTimeCommand + | BlowoutInPlaceRunTimeCommand + | DispenseInPlaceRunTimeCommand + | DropTipInPlaceRunTimeCommand + | PrepareToAspirateRunTimeCommand + const IN_PLACE_COMMAND_TYPES = [ + 'aspirateInPlace', + 'dispenseInPlace', + 'blowOutInPlace', + 'dropTipInPlace', + 'prepareToAspirate', + ] as const + const isInPlace = ( + failedCommand: FailedCommand + ): failedCommand is InPlaceCommand => + IN_PLACE_COMMAND_TYPES.includes( + (failedCommand as InPlaceCommand).commandType + ) + return failedCommand != null + ? isInPlace(failedCommand) + ? failedCommand.error?.isDefined && + failedCommand.error?.errorType === 'overpressure' && + // Paranoia: this value comes from the wire and may be unevenly implemented + typeof failedCommand.error?.errorInfo?.retryLocation?.at(0) === + 'number' + ? { + commandType: 'moveToCoordinates', + intent: 'fixit', + params: { + pipetteId: failedCommand.params?.pipetteId, + coordinates: { + x: failedCommand.error.errorInfo.retryLocation[0], + y: failedCommand.error.errorInfo.retryLocation[1], + z: failedCommand.error.errorInfo.retryLocation[2], + }, + }, + } + : null + : null + : null + } const chainRunRecoveryCommands = React.useCallback( ( commands: CreateCommand[], @@ -72,10 +124,13 @@ export function useRecoveryCommands({ const retryFailedCommand = React.useCallback((): Promise => { const { commandType, params } = failedCommand as FailedCommand // Null case is handled before command could be issued. - - return chainRunRecoveryCommands([ - { commandType, params }, - ] as CreateCommand[]) // the created command is the same command that failed + return chainRunRecoveryCommands( + [ + // move back to the location of the command if it is an in-place command + buildRetryPrepMove(), + { commandType, params }, // retry the command that failed + ].filter(c => c != null) as CreateCommand[] + ) // the created command is the same command that failed }, [chainRunRecoveryCommands, failedCommand]) // Homes the Z-axis of all attached pipettes. diff --git a/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts b/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts index c3406ee6391..adad317fd2a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts @@ -55,10 +55,10 @@ describe('getErrorKind', () => { it(`returns ${ERROR_KINDS.GENERAL_ERROR} for defined errors not handled explicitly`, () => { const result = getErrorKind({ commandType: 'aspirate', - error: { + error: ({ isDefined: true, errorType: 'someHithertoUnknownDefinedErrorType', - } as RunCommandError, + } as unknown) as RunCommandError, } as RunTimeCommand) expect(result).toEqual(ERROR_KINDS.GENERAL_ERROR) }) diff --git a/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/RunFailedModal.test.tsx b/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/RunFailedModal.test.tsx index b57a09cc8aa..629c0b1adea 100644 --- a/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/RunFailedModal.test.tsx +++ b/app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/RunFailedModal.test.tsx @@ -20,39 +20,39 @@ const mockErrors = [ { id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba', errorType: 'generalError', - isDefined: false, + isDefined: false as const, createdAt: '2023-04-09T21:41:51.333171+00:00', detail: 'Error with code 4000 (lowest priority)', errorInfo: {}, - errorCode: '4000', + errorCode: '4000' as const, wrappedErrors: [ { id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba', errorType: 'roboticsInteractionError', - isDefined: false, + isDefined: false as const, createdAt: '2023-04-09T21:41:51.333171+00:00', detail: 'Error with code 3000 (second lowest priortiy)', errorInfo: {}, - errorCode: '3000', + errorCode: '3000' as const, wrappedErrors: [], }, { id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba', errorType: 'roboticsControlError', - isDefined: false, + isDefined: false as const, createdAt: '2023-04-09T21:41:51.333171+00:00', detail: 'Error with code 2000 (second highest priority)', errorInfo: {}, - errorCode: '2000', + errorCode: '2000' as const, wrappedErrors: [ { id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba', errorType: 'hardwareCommunicationError', - isDefined: false, + isDefined: false as const, createdAt: '2023-04-09T21:41:51.333171+00:00', detail: 'Error with code 1000 (highest priority)', errorInfo: {}, - errorCode: '1000', + errorCode: '1000' as const, wrappedErrors: [], }, ], @@ -62,11 +62,11 @@ const mockErrors = [ { id: 'd0245210-dfb9-4f1c-8ad0-3416b603a7ba', errorType: 'roboticsInteractionError', - isDefined: false, + isDefined: false as const, createdAt: '2023-04-09T21:41:51.333171+00:00', detail: 'Error with code 2001 (second highest priortiy)', errorInfo: {}, - errorCode: '2001', + errorCode: '2001' as const, wrappedErrors: [], }, ] diff --git a/shared-data/command/types/index.ts b/shared-data/command/types/index.ts index 5d6503761a3..2970a8e5185 100644 --- a/shared-data/command/types/index.ts +++ b/shared-data/command/types/index.ts @@ -1,3 +1,4 @@ +import type { ErrorCodes } from '../../errors' import type { PipettingRunTimeCommand, PipettingCreateCommand, @@ -78,14 +79,28 @@ export type RunTimeCommand = | AnnotationRunTimeCommand // annotating command execution | IncidentalRunTimeCommand // command with only incidental effects (status bar animations) +export type RunCommandError = + | RunCommandErrorUndefined + | RunCommandErrorOverpressure + // TODO(jh, 05-24-24): Update when some of these newer properties become more finalized. -export interface RunCommandError { +export interface RunCommandErrorBase { createdAt: string detail: string - errorCode: string - errorType: string id: string - isDefined: boolean - errorInfo?: Record wrappedErrors?: RunCommandError[] } + +export interface RunCommandErrorUndefined extends RunCommandErrorBase { + errorCode: ErrorCodes + errorType: string + isDefined: false + errorInfo?: Record +} + +export interface RunCommandErrorOverpressure extends RunCommandErrorBase { + errorCode: '3006' + errorType: 'overpressure' + isDefined: true + errorInfo: { retryLocation: [number, number, number] } +} diff --git a/shared-data/errors/index.ts b/shared-data/errors/index.ts new file mode 100644 index 00000000000..8f12e888d7e --- /dev/null +++ b/shared-data/errors/index.ts @@ -0,0 +1,4 @@ +import errorSchemaV1 from './schemas/1.json' +import errorData from './definitions/1/errors.json' +export type * from './types' +export { errorSchemaV1, errorData } diff --git a/shared-data/errors/types/index.ts b/shared-data/errors/types/index.ts new file mode 100644 index 00000000000..d10699c0672 --- /dev/null +++ b/shared-data/errors/types/index.ts @@ -0,0 +1,15 @@ +// eslint-disable-next-line @typescript-eslint/consistent-type-imports +import ERROR_DATA from '../definitions/1/errors.json' + +export type ErrorCategories = keyof typeof ERROR_DATA['categories'] +export interface CategorySpec { + detail: string + codePrefix: string +} +export type ErrorCodes = keyof typeof ERROR_DATA['codes'] +export interface ErrorSpec { + detail: string + category: ErrorCategories +} +export type ErrorSpecs = Record +export type CategorySpecs = Record diff --git a/shared-data/tsconfig.json b/shared-data/tsconfig.json index cb960e927cb..85db0e9c2bc 100644 --- a/shared-data/tsconfig.json +++ b/shared-data/tsconfig.json @@ -5,7 +5,7 @@ "composite": true, "rootDir": ".", "outDir": "lib", - "moduleResolution": "node", + "moduleResolution": "node" }, "module": "ESNext", "include": [ @@ -15,8 +15,9 @@ "labware", "deck", "command", + "errors", "liquid/types", "commandAnnotation/types", - "vite.config.ts", + "vite.config.ts" ] } From 8357916365cdf5e456611a8dc176826e7b8939da Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Wed, 24 Jul 2024 16:23:12 -0400 Subject: [PATCH 6/8] feat(api): Handle overpressures in dispense (#15754) As in aspirate, we handle overpressure errors and turn them into DefinedError return values, allowing us to recover from overpressures via client-driven error recovery. Closes EXEC-498 --- .../protocol_engine/commands/dispense.py | 85 ++++++++++++----- .../protocol_engine/state/pipettes.py | 5 +- .../protocol_engine/commands/test_dispense.py | 91 ++++++++++++++++++- .../state/test_pipette_store.py | 68 ++++++++++++++ 4 files changed, 220 insertions(+), 29 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/dispense.py b/api/src/opentrons/protocol_engine/commands/dispense.py index 7ba9fe2ae52..b346fb5845a 100644 --- a/api/src/opentrons/protocol_engine/commands/dispense.py +++ b/api/src/opentrons/protocol_engine/commands/dispense.py @@ -1,8 +1,10 @@ """Dispense command request, result, and implementation models.""" from __future__ import annotations -from typing import TYPE_CHECKING, Optional, Type +from typing import TYPE_CHECKING, Optional, Type, Union from typing_extensions import Literal +from opentrons_shared_data.errors.exceptions import PipetteOverpressureError + from pydantic import Field from ..types import DeckPoint @@ -13,12 +15,21 @@ WellLocationMixin, BaseLiquidHandlingResult, DestinationPositionResult, + OverpressureError, + OverpressureErrorInternalData, +) +from .command import ( + AbstractCommandImpl, + BaseCommand, + BaseCommandCreate, + DefinedErrorData, + SuccessData, ) -from .command import AbstractCommandImpl, BaseCommand, BaseCommandCreate, SuccessData from ..errors.error_occurrence import ErrorOccurrence if TYPE_CHECKING: from ..execution import MovementHandler, PipettingHandler + from ..resources import ModelUtils DispenseCommandType = Literal["dispense"] @@ -41,20 +52,27 @@ class DispenseResult(BaseLiquidHandlingResult, DestinationPositionResult): pass -class DispenseImplementation( - AbstractCommandImpl[DispenseParams, SuccessData[DispenseResult, None]] -): +_ExecuteReturn = Union[ + SuccessData[DispenseResult, None], + DefinedErrorData[OverpressureError, OverpressureErrorInternalData], +] + + +class DispenseImplementation(AbstractCommandImpl[DispenseParams, _ExecuteReturn]): """Dispense command implementation.""" def __init__( - self, movement: MovementHandler, pipetting: PipettingHandler, **kwargs: object + self, + movement: MovementHandler, + pipetting: PipettingHandler, + model_utils: ModelUtils, + **kwargs: object, ) -> None: self._movement = movement self._pipetting = pipetting + self._model_utils = model_utils - async def execute( - self, params: DispenseParams - ) -> SuccessData[DispenseResult, None]: + async def execute(self, params: DispenseParams) -> _ExecuteReturn: """Move to and dispense to the requested well.""" position = await self._movement.move_to_well( pipette_id=params.pipetteId, @@ -62,20 +80,41 @@ async def execute( well_name=params.wellName, well_location=params.wellLocation, ) - volume = await self._pipetting.dispense_in_place( - pipette_id=params.pipetteId, - volume=params.volume, - flow_rate=params.flowRate, - push_out=params.pushOut, - ) - - return SuccessData( - public=DispenseResult( - volume=volume, - position=DeckPoint(x=position.x, y=position.y, z=position.z), - ), - private=None, - ) + try: + volume = await self._pipetting.dispense_in_place( + pipette_id=params.pipetteId, + volume=params.volume, + flow_rate=params.flowRate, + push_out=params.pushOut, + ) + except PipetteOverpressureError as e: + return DefinedErrorData( + public=OverpressureError( + 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={"retryLocation": (position.x, position.y, position.z)}, + ), + private=OverpressureErrorInternalData( + position=DeckPoint.construct( + x=position.x, y=position.y, z=position.z + ) + ), + ) + else: + return SuccessData( + public=DispenseResult( + volume=volume, + position=DeckPoint(x=position.x, y=position.y, z=position.z), + ), + private=None, + ) class Dispense(BaseCommand[DispenseParams, DispenseResult, ErrorOccurrence]): diff --git a/api/src/opentrons/protocol_engine/state/pipettes.py b/api/src/opentrons/protocol_engine/state/pipettes.py index 92344dd9600..35cfca94f33 100644 --- a/api/src/opentrons/protocol_engine/state/pipettes.py +++ b/api/src/opentrons/protocol_engine/state/pipettes.py @@ -13,6 +13,7 @@ ) from opentrons.protocol_engine.actions.actions import FailCommandAction from opentrons.protocol_engine.commands.aspirate import Aspirate +from opentrons.protocol_engine.commands.dispense import Dispense from opentrons.protocol_engine.commands.command import DefinedErrorData from opentrons.protocol_engine.commands.pipetting_common import ( OverpressureError, @@ -316,7 +317,7 @@ def _update_current_location( # noqa: C901 ) elif ( isinstance(action, FailCommandAction) - and isinstance(action.running_command, Aspirate) + and isinstance(action.running_command, (Aspirate, Dispense)) and isinstance(action.error, DefinedErrorData) and isinstance(action.error.public, OverpressureError) ): @@ -412,7 +413,7 @@ def _update_deck_point( ) elif ( isinstance(action, FailCommandAction) - and isinstance(action.running_command, Aspirate) + and isinstance(action.running_command, (Aspirate, Dispense)) and isinstance(action.error, DefinedErrorData) and isinstance(action.error.public, OverpressureError) ): diff --git a/api/tests/opentrons/protocol_engine/commands/test_dispense.py b/api/tests/opentrons/protocol_engine/commands/test_dispense.py index 4df18a19152..86c4f6ac93b 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_dispense.py +++ b/api/tests/opentrons/protocol_engine/commands/test_dispense.py @@ -1,26 +1,47 @@ """Test dispense commands.""" -from decoy import Decoy +from datetime import datetime + +import pytest +from decoy import Decoy, matchers + +from opentrons_shared_data.errors.exceptions import PipetteOverpressureError from opentrons.protocol_engine import WellLocation, WellOrigin, WellOffset, DeckPoint from opentrons.protocol_engine.execution import MovementHandler, PipettingHandler from opentrons.types import Point -from opentrons.protocol_engine.commands.command import SuccessData +from opentrons.protocol_engine.commands.command import SuccessData, DefinedErrorData from opentrons.protocol_engine.commands.dispense import ( DispenseParams, DispenseResult, DispenseImplementation, ) +from opentrons.protocol_engine.resources.model_utils import ModelUtils +from opentrons.protocol_engine.commands.pipetting_common import ( + OverpressureError, + OverpressureErrorInternalData, +) + + +@pytest.fixture +def subject( + movement: MovementHandler, + pipetting: PipettingHandler, + model_utils: ModelUtils, +) -> DispenseImplementation: + """Get the implementation subject.""" + return DispenseImplementation( + movement=movement, pipetting=pipetting, model_utils=model_utils + ) async def test_dispense_implementation( decoy: Decoy, movement: MovementHandler, pipetting: PipettingHandler, + subject: DispenseImplementation, ) -> None: """It should move to the target location and then dispense.""" - subject = DispenseImplementation(movement=movement, pipetting=pipetting) - well_location = WellLocation( origin=WellOrigin.BOTTOM, offset=WellOffset(x=0, y=0, z=1) ) @@ -55,3 +76,65 @@ async def test_dispense_implementation( public=DispenseResult(volume=42, position=DeckPoint(x=1, y=2, z=3)), private=None, ) + + +async def test_overpressure_error( + decoy: Decoy, + movement: MovementHandler, + pipetting: PipettingHandler, + subject: DispenseImplementation, + model_utils: ModelUtils, +) -> None: + """It should return an overpressure error if the hardware API indicates that.""" + pipette_id = "pipette-id" + labware_id = "labware-id" + well_name = "well-name" + well_location = WellLocation( + origin=WellOrigin.BOTTOM, offset=WellOffset(x=0, y=0, z=1) + ) + + position = Point(x=1, y=2, z=3) + + error_id = "error-id" + error_timestamp = datetime(year=2020, month=1, day=2) + + data = DispenseParams( + pipetteId=pipette_id, + labwareId=labware_id, + wellName=well_name, + wellLocation=well_location, + volume=50, + flowRate=1.23, + ) + + decoy.when( + await movement.move_to_well( + pipette_id=pipette_id, + labware_id=labware_id, + well_name=well_name, + well_location=well_location, + ), + ).then_return(position) + + decoy.when( + await pipetting.dispense_in_place( + pipette_id=pipette_id, volume=50, flow_rate=1.23, push_out=None + ), + ).then_raise(PipetteOverpressureError()) + + decoy.when(model_utils.generate_id()).then_return(error_id) + decoy.when(model_utils.get_timestamp()).then_return(error_timestamp) + + result = await subject.execute(data) + + assert result == DefinedErrorData( + public=OverpressureError.construct( + id=error_id, + createdAt=error_timestamp, + wrappedErrors=[matchers.Anything()], + errorInfo={"retryLocation": (position.x, position.y, position.z)}, + ), + private=OverpressureErrorInternalData( + position=DeckPoint(x=position.x, y=position.y, z=position.z) + ), + ) 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 c60adb96fdb..c132ea56c73 100644 --- a/api/tests/opentrons/protocol_engine/state/test_pipette_store.py +++ b/api/tests/opentrons/protocol_engine/state/test_pipette_store.py @@ -426,6 +426,43 @@ def test_blow_out_clears_volume( well_name="move-to-well-well-name", ), ), + ( + FailCommandAction( + running_command=cmd.Dispense( + params=cmd.DispenseParams( + pipetteId="pipette-id", + labwareId="dispense-labware-id", + wellName="dispense-well-name", + volume=50, + flowRate=1.23, + ), + id="command-id", + key="command-key", + createdAt=datetime.now(), + status=cmd.CommandStatus.RUNNING, + ), + error=DefinedErrorData( + public=OverpressureError( + id="error-id", + createdAt=datetime.now(), + errorInfo={"retryLocation": (0, 0, 0)}, + ), + private=OverpressureErrorInternalData( + position=DeckPoint(x=0, y=0, z=0) + ), + ), + command_id="command-id", + error_id="error-id", + failed_at=datetime.now(), + notes=[], + type=ErrorRecoveryType.WAIT_FOR_RECOVERY, + ), + CurrentWell( + pipette_id="pipette-id", + labware_id="dispense-labware-id", + well_name="dispense-well-name", + ), + ), ), ) def test_movement_commands_update_current_well( @@ -902,6 +939,37 @@ def test_add_pipette_config( ), private_result=None, ), + FailCommandAction( + running_command=cmd.Dispense( + params=cmd.DispenseParams( + pipetteId="pipette-id", + labwareId="labware-id", + wellName="well-name", + volume=125, + flowRate=1.23, + ), + id="command-id", + key="command-key", + createdAt=datetime.now(), + status=cmd.CommandStatus.RUNNING, + ), + error=DefinedErrorData( + public=OverpressureError( + id="error-id", + detail="error-detail", + createdAt=datetime.now(), + errorInfo={"retryLocation": (11, 22, 33)}, + ), + private=OverpressureErrorInternalData( + position=DeckPoint(x=11, y=22, z=33) + ), + ), + command_id="command-id", + error_id="error-id", + failed_at=datetime.now(), + notes=[], + type=ErrorRecoveryType.WAIT_FOR_RECOVERY, + ), ), ) def test_movement_commands_update_deck_point( From e4c829c207a5b1eba3ea195c924468ce72e0b862 Mon Sep 17 00:00:00 2001 From: Ryan Howard Date: Wed, 24 Jul 2024 16:28:31 -0400 Subject: [PATCH 7/8] feat(hardware): add sensor max value getter (#15786) # Overview Only used in can control right now but this provides a way to get the max sensor value reading, which we can use to differentiate pipettes with the old and new sensor boards. # Test Plan # Changelog # Review requests # Risk assessment --- .../firmware_bindings/constants.py | 3 +++ .../messages/message_definitions.py | 22 +++++++++++++++++++ .../firmware_bindings/messages/messages.py | 2 ++ 3 files changed, 27 insertions(+) diff --git a/hardware/opentrons_hardware/firmware_bindings/constants.py b/hardware/opentrons_hardware/firmware_bindings/constants.py index f0202307647..960419d334c 100644 --- a/hardware/opentrons_hardware/firmware_bindings/constants.py +++ b/hardware/opentrons_hardware/firmware_bindings/constants.py @@ -241,6 +241,9 @@ class MessageId(int, Enum): gear_write_motor_driver_request = 0x506 gear_read_motor_driver_request = 0x507 + max_sensor_value_request = 0x70 + max_sensor_value_response = 0x71 + read_sensor_request = 0x82 write_sensor_request = 0x83 baseline_sensor_request = 0x84 diff --git a/hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py b/hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py index 82ba3040928..12f65f80f81 100644 --- a/hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py +++ b/hardware/opentrons_hardware/firmware_bindings/messages/message_definitions.py @@ -522,6 +522,28 @@ class ReadLimitSwitchResponse(BaseMessage): # noqa: D101 message_id: Literal[MessageId.limit_sw_response] = MessageId.limit_sw_response +@dataclass +class MaxSensorValueRequest(BaseMessage): # noqa: D101 + payload: payloads.ReadFromSensorRequestPayload + payload_type: Type[ + payloads.ReadFromSensorRequestPayload + ] = payloads.ReadFromSensorRequestPayload + message_id: Literal[ + MessageId.max_sensor_value_request + ] = MessageId.max_sensor_value_request + + +@dataclass +class MaxSensorValueResponse(BaseMessage): # noqa: D101 + payload: payloads.ReadFromSensorRequestPayload + payload_type: Type[ + payloads.ReadFromSensorRequestPayload + ] = payloads.ReadFromSensorRequestPayload + message_id: Literal[ + MessageId.max_sensor_value_response + ] = MessageId.max_sensor_value_response + + @dataclass class ReadFromSensorRequest(BaseMessage): # noqa: D101 payload: payloads.ReadFromSensorRequestPayload diff --git a/hardware/opentrons_hardware/firmware_bindings/messages/messages.py b/hardware/opentrons_hardware/firmware_bindings/messages/messages.py index 772bb6c7e86..0249ddec69e 100644 --- a/hardware/opentrons_hardware/firmware_bindings/messages/messages.py +++ b/hardware/opentrons_hardware/firmware_bindings/messages/messages.py @@ -66,6 +66,8 @@ defs.FirmwareUpdateStartApp, defs.ReadLimitSwitchRequest, defs.ReadLimitSwitchResponse, + defs.MaxSensorValueRequest, + defs.MaxSensorValueResponse, defs.ReadFromSensorRequest, defs.WriteToSensorRequest, defs.BaselineSensorRequest, From 93446728c5dd52282bbf8fe8401b2d69e4358ed8 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Wed, 24 Jul 2024 17:12:14 -0400 Subject: [PATCH 8/8] fix(app): prevent eternal intervention purgatory (#15788) If you click on a run that has an intervention modal due to a pause, move labware, etc., you shouldn't have to stare at that run until you close or your app, deal with the robot, or contemplate your navigation actions for all eternity. Navigating away should be a legitimate option. --- app/src/organisms/RunProgressMeter/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/organisms/RunProgressMeter/index.tsx b/app/src/organisms/RunProgressMeter/index.tsx index 2914e07cf53..c1c674d03b3 100644 --- a/app/src/organisms/RunProgressMeter/index.tsx +++ b/app/src/organisms/RunProgressMeter/index.tsx @@ -31,7 +31,7 @@ import { } from '@opentrons/api-client' import { useMostRecentCompletedAnalysis } from '../LabwarePositionCheck/useMostRecentCompletedAnalysis' -import { getTopPortalEl } from '../../App/portal' +import { getModalPortalEl } from '../../App/portal' import { Tooltip } from '../../atoms/Tooltip' import { CommandText } from '../../molecules/Command' import { useRunStatus } from '../RunTimeControl/hooks' @@ -175,7 +175,7 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { run={runData} analysis={analysis} />, - getTopPortalEl() + getModalPortalEl() ) : null}