fix(app, robot-server): support retryLocation when retrying `dropTi…

…pInPlace` during Error Recovery (#16839) Closes RQA-3591 When dropping a tip in a trash bin or waste chute, the pipette moves downwards before doing the drop tip. During Error Recovery, when we retry the failed command with a fixit intent, we need to perform a moveToCoordinates first before dispatching the dropTipInPlace or the robot drops the tip(s) from too high a location. To know how far to move, we need the failedCommand to contain retryLocation metadata. We have a pattern for this already with overpressure in place commands, so we extend the same functionality here. Co-authored-by: Max Marrone <[email protected]>
Opentrons · Nov 15, 2024 · df80263 · df80263
1 parent 972c592
commit df80263
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 26 deletions.
diff --git a/api/src/opentrons/protocol_engine/commands/aspirate_in_place.py b/api/src/opentrons/protocol_engine/commands/aspirate_in_place.py
@@ -83,22 +83,22 @@ async def execute(self, params: AspirateInPlaceParams) -> _ExecuteReturn:
             TipNotAttachedError: if no tip is attached to the pipette.
             PipetteNotReadyToAspirateError: pipette plunger is not ready.
         """
+        state_update = StateUpdate()
+
         ready_to_aspirate = self._pipetting.get_is_ready_to_aspirate(
             pipette_id=params.pipetteId,
         )
-
         if not ready_to_aspirate:
             raise PipetteNotReadyToAspirateError(
                 "Pipette cannot aspirate in place because of a previous blow out."
                 " The first aspirate following a blow-out must be from a specific well"
                 " so the plunger can be reset in a known safe position."
             )
 
-        state_update = StateUpdate()
         current_location = self._state_view.pipettes.get_current_location()
+        current_position = await self._gantry_mover.get_position(params.pipetteId)
 
         try:
-            current_position = await self._gantry_mover.get_position(params.pipetteId)
             volume = await self._pipetting.aspirate_in_place(
                 pipette_id=params.pipetteId,
                 volume=params.volume,

diff --git a/api/src/opentrons/protocol_engine/commands/command.py b/api/src/opentrons/protocol_engine/commands/command.py
@@ -185,7 +185,9 @@ class BaseCommand(
     )
     error: Union[
         _ErrorT,
-        # ErrorOccurrence here is for undefined errors not captured by _ErrorT.
+        # ErrorOccurrence here is a catch-all for undefined errors not captured by
+        # _ErrorT, or defined errors that don't parse into _ErrorT because, for example,
+        # they are from an older software version that was missing some fields.
         ErrorOccurrence,
         None,
     ] = Field(

diff --git a/api/src/opentrons/protocol_engine/commands/dispense_in_place.py b/api/src/opentrons/protocol_engine/commands/dispense_in_place.py
@@ -76,8 +76,8 @@ async def execute(self, params: DispenseInPlaceParams) -> _ExecuteReturn:
         """Dispense without moving the pipette."""
         state_update = StateUpdate()
         current_location = self._state_view.pipettes.get_current_location()
+        current_position = await self._gantry_mover.get_position(params.pipetteId)
         try:
-            current_position = await self._gantry_mover.get_position(params.pipetteId)
             volume = await self._pipetting.dispense_in_place(
                 pipette_id=params.pipetteId,
                 volume=params.volume,

diff --git a/api/src/opentrons/protocol_engine/commands/drop_tip.py b/api/src/opentrons/protocol_engine/commands/drop_tip.py
@@ -145,6 +145,7 @@ async def execute(self, params: DropTipParams) -> _ExecuteReturn:
                         error=exception,
                     )
                 ],
+                errorInfo={"retryLocation": position},
             )
             state_update_if_false_positive = update_types.StateUpdate()
             state_update_if_false_positive.update_pipette_tip_state(
@@ -165,7 +166,7 @@ async def execute(self, params: DropTipParams) -> _ExecuteReturn:
             )
 
 
-class DropTip(BaseCommand[DropTipParams, DropTipResult, ErrorOccurrence]):
+class DropTip(BaseCommand[DropTipParams, DropTipResult, TipPhysicallyAttachedError]):
     """Drop tip command model."""
 
     commandType: DropTipCommandType = "dropTip"

diff --git a/api/src/opentrons/protocol_engine/commands/drop_tip_in_place.py b/api/src/opentrons/protocol_engine/commands/drop_tip_in_place.py
@@ -18,7 +18,7 @@
 from ..state import update_types
 
 if TYPE_CHECKING:
-    from ..execution import TipHandler
+    from ..execution import TipHandler, GantryMover
 
 
 DropTipInPlaceCommandType = Literal["dropTipInPlace"]
@@ -57,15 +57,19 @@ def __init__(
         self,
         tip_handler: TipHandler,
         model_utils: ModelUtils,
+        gantry_mover: GantryMover,
         **kwargs: object,
     ) -> None:
         self._tip_handler = tip_handler
         self._model_utils = model_utils
+        self._gantry_mover = gantry_mover
 
     async def execute(self, params: DropTipInPlaceParams) -> _ExecuteReturn:
         """Drop a tip using the requested pipette."""
         state_update = update_types.StateUpdate()
 
+        retry_location = await self._gantry_mover.get_position(params.pipetteId)
+
         try:
             await self._tip_handler.drop_tip(
                 pipette_id=params.pipetteId, home_after=params.homeAfter
@@ -85,6 +89,7 @@ async def execute(self, params: DropTipInPlaceParams) -> _ExecuteReturn:
                         error=exception,
                     )
                 ],
+                errorInfo={"retryLocation": retry_location},
             )
             return DefinedErrorData(
                 public=error,
@@ -99,7 +104,7 @@ async def execute(self, params: DropTipInPlaceParams) -> _ExecuteReturn:
 
 
 class DropTipInPlace(
-    BaseCommand[DropTipInPlaceParams, DropTipInPlaceResult, ErrorOccurrence]
+    BaseCommand[DropTipInPlaceParams, DropTipInPlaceResult, TipPhysicallyAttachedError]
 ):
     """Drop tip in place command model."""
 

diff --git a/api/src/opentrons/protocol_engine/commands/pipetting_common.py b/api/src/opentrons/protocol_engine/commands/pipetting_common.py
@@ -148,7 +148,12 @@ class DestinationPositionResult(BaseModel):
 
 
 class ErrorLocationInfo(TypedDict):
-    """Holds a retry location for in-place error recovery."""
+    """Holds a retry location for in-place error recovery.
+
+    This is appropriate to pass to a `moveToCoordinates` command,
+    assuming the pipette has not been configured with a different nozzle layout
+    in the meantime.
+    """
 
     retryLocation: Tuple[float, float, float]
 
@@ -201,3 +206,5 @@ class TipPhysicallyAttachedError(ErrorOccurrence):
 
     errorCode: str = ErrorCodes.TIP_DROP_FAILED.value.code
     detail: str = ErrorCodes.TIP_DROP_FAILED.value.detail
+
+    errorInfo: ErrorLocationInfo
diff --git a/api/src/opentrons/protocol_engine/errors/error_occurrence.py b/api/src/opentrons/protocol_engine/errors/error_occurrence.py
@@ -12,8 +12,6 @@
 log = getLogger(__name__)
 
 
-# TODO(mc, 2021-11-12): flesh this model out with structured error data
-# for each error type so client may produce better error messages
 class ErrorOccurrence(BaseModel):
     """An occurrence of a specific error during protocol execution."""
 
@@ -44,8 +42,15 @@ def from_failed(
     id: str = Field(..., description="Unique identifier of this error occurrence.")
     createdAt: datetime = Field(..., description="When the error occurred.")
 
+    # Our Python should probably always set this to False--if we want it to be True,
+    # we should probably be using a more specific subclass of ErrorOccurrence anyway.
+    # However, we can't make this Literal[False], because we want this class to be able
+    # to act as a catch-all for parsing defined errors that might be missing some
+    # `errorInfo` fields because they were serialized by older software.
     isDefined: bool = Field(
-        default=False,  # default=False for database backwards compatibility.
+        # default=False for database backwards compatibility, so we can parse objects
+        # serialized before isDefined existed.
+        default=False,
         description=dedent(
             """\
             Whether this error is *defined.*

diff --git a/api/tests/opentrons/protocol_engine/commands/test_drop_tip.py b/api/tests/opentrons/protocol_engine/commands/test_drop_tip.py
@@ -281,6 +281,7 @@ async def test_tip_attached_error(
             id="error-id",
             createdAt=datetime(year=1, month=2, day=3),
             wrappedErrors=[matchers.Anything()],
+            errorInfo={"retryLocation": (111, 222, 333)},
         ),
         state_update=update_types.StateUpdate(
             pipette_location=update_types.PipetteLocationUpdate(

diff --git a/api/tests/opentrons/protocol_engine/commands/test_drop_tip_in_place.py b/api/tests/opentrons/protocol_engine/commands/test_drop_tip_in_place.py
@@ -14,12 +14,13 @@
     DropTipInPlaceImplementation,
 )
 from opentrons.protocol_engine.errors.exceptions import TipAttachedError
-from opentrons.protocol_engine.execution import TipHandler
+from opentrons.protocol_engine.execution import TipHandler, GantryMover
 from opentrons.protocol_engine.resources.model_utils import ModelUtils
 from opentrons.protocol_engine.state.update_types import (
     PipetteTipStateUpdate,
     StateUpdate,
 )
+from opentrons.types import Point
 
 
 @pytest.fixture
@@ -34,14 +35,23 @@ def mock_model_utils(decoy: Decoy) -> ModelUtils:
     return decoy.mock(cls=ModelUtils)
 
 
+@pytest.fixture
+def mock_gantry_mover(decoy: Decoy) -> GantryMover:
+    """Get a mock GantryMover."""
+    return decoy.mock(cls=GantryMover)
+
+
 async def test_success(
     decoy: Decoy,
     mock_tip_handler: TipHandler,
     mock_model_utils: ModelUtils,
+    mock_gantry_mover: GantryMover,
 ) -> None:
     """A DropTip command should have an execution implementation."""
     subject = DropTipInPlaceImplementation(
-        tip_handler=mock_tip_handler, model_utils=mock_model_utils
+        tip_handler=mock_tip_handler,
+        model_utils=mock_model_utils,
+        gantry_mover=mock_gantry_mover,
     )
     params = DropTipInPlaceParams(pipetteId="abc", homeAfter=False)
 
@@ -64,14 +74,20 @@ async def test_tip_attached_error(
     decoy: Decoy,
     mock_tip_handler: TipHandler,
     mock_model_utils: ModelUtils,
+    mock_gantry_mover: GantryMover,
 ) -> None:
     """A DropTip command should have an execution implementation."""
     subject = DropTipInPlaceImplementation(
-        tip_handler=mock_tip_handler, model_utils=mock_model_utils
+        tip_handler=mock_tip_handler,
+        model_utils=mock_model_utils,
+        gantry_mover=mock_gantry_mover,
     )
 
     params = DropTipInPlaceParams(pipetteId="abc", homeAfter=False)
 
+    decoy.when(await mock_gantry_mover.get_position(pipette_id="abc")).then_return(
+        Point(9, 8, 7)
+    )
     decoy.when(
         await mock_tip_handler.drop_tip(pipette_id="abc", home_after=False)
     ).then_raise(TipAttachedError("Egads!"))
@@ -88,6 +104,7 @@ async def test_tip_attached_error(
             id="error-id",
             createdAt=datetime(year=1, month=2, day=3),
             wrappedErrors=[matchers.Anything()],
+            errorInfo={"retryLocation": (9, 8, 7)},
         ),
         state_update=StateUpdate(),
         state_update_if_false_positive=StateUpdate(

diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts
@@ -139,24 +139,41 @@ describe('useRecoveryCommands', () => {
       false
     )
   })
-  ;([
+
+  const IN_PLACE_COMMANDS = [
     '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 () => {
+  ] as const
+
+  const ERROR_SCENARIOS = [
+    { type: 'overpressure', code: '3006' },
+    { type: 'tipPhysicallyAttached', code: '3007' },
+  ] as const
+
+  it.each(
+    ERROR_SCENARIOS.flatMap(error =>
+      IN_PLACE_COMMANDS.map(commandType => ({
+        errorType: error.type,
+        errorCode: error.code,
+        commandType,
+      }))
+    )
+  )(
+    'Should move to retryLocation if failed command is $commandType and error is $errorType when retrying',
+    async ({ errorType, errorCode, commandType }) => {
       const { result } = renderHook(() => {
         const failedCommand = {
           ...mockFailedCommand,
-          commandType: inPlaceCommandType,
+          commandType,
           params: {
             pipetteId: 'mock-pipette-id',
           },
           error: {
-            errorType: 'overpressure',
-            errorCode: '3006',
+            errorType,
+            errorCode,
             isDefined: true,
             errorInfo: {
               retryLocation: [1, 2, 3],
@@ -180,9 +197,11 @@ describe('useRecoveryCommands', () => {
           selectedRecoveryOption: RECOVERY_MAP.RETRY_NEW_TIPS.ROUTE,
         })
       })
+
       await act(async () => {
         await result.current.retryFailedCommand()
       })
+
       expect(mockChainRunCommands).toHaveBeenLastCalledWith(
         [
           {
@@ -194,14 +213,14 @@ describe('useRecoveryCommands', () => {
             },
           },
           {
-            commandType: inPlaceCommandType,
+            commandType,
             params: { pipetteId: 'mock-pipette-id' },
           },
         ],
         false
       )
-    })
-  })
+    }
+  )
 
   it('should call resumeRun with runId and show success toast on success', async () => {
     const { result } = renderHook(() => useRecoveryCommands(props))

diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts
@@ -9,7 +9,7 @@ import {
 } from '@opentrons/react-api-client'
 
 import { useChainRunCommands } from '/app/resources/runs'
-import { ERROR_KINDS, RECOVERY_MAP } from '../constants'
+import { DEFINED_ERROR_TYPES, ERROR_KINDS, RECOVERY_MAP } from '../constants'
 import { getErrorKind } from '/app/organisms/ErrorRecoveryFlows/utils'
 
 import type {
@@ -127,23 +127,33 @@ export function useRecoveryCommands({
       | DispenseInPlaceRunTimeCommand
       | DropTipInPlaceRunTimeCommand
       | PrepareToAspirateRunTimeCommand
+
     const IN_PLACE_COMMAND_TYPES = [
       'aspirateInPlace',
       'dispenseInPlace',
       'blowOutInPlace',
       'dropTipInPlace',
       'prepareToAspirate',
     ] as const
+
+    const RETRY_ERROR_TYPES = [
+      DEFINED_ERROR_TYPES.OVERPRESSURE,
+      DEFINED_ERROR_TYPES.TIP_PHYSICALLY_ATTACHED,
+    ] as const
+
     const isInPlace = (
       failedCommand: FailedCommand
     ): failedCommand is InPlaceCommand =>
       IN_PLACE_COMMAND_TYPES.includes(
         (failedCommand as InPlaceCommand).commandType
       )
+
     return unvalidatedFailedCommand != null
       ? isInPlace(unvalidatedFailedCommand)
         ? unvalidatedFailedCommand.error?.isDefined &&
-          unvalidatedFailedCommand.error?.errorType === 'overpressure' &&
+          RETRY_ERROR_TYPES.includes(
+            unvalidatedFailedCommand.error?.errorType
+          ) &&
           // Paranoia: this value comes from the wire and may be unevenly implemented
           typeof unvalidatedFailedCommand.error?.errorInfo?.retryLocation?.at(
             0