feat(api,robot-server): check pipette offset consistency (#13583)

Add a basic instrument offset consistency check to drive UI indicating
that an insturment may be miscalibrated and the user should run
calibration.

Closes RAUT-738

api-client/src/instruments/types.ts

@@ -3,6 +3,14 @@ export type InstrumentData = PipetteData | GripperData | BadPipette | BadGripper
 // pipettes module already exports type `Mount`
 type Mount = 'left' | 'right' | 'extension'
 
+export interface InconsistentCalibrationFailure {
+  kind: 'inconsistent-pipette-offset'
+  offsets: Map<'left' | 'right', { x: number; y: number; z: number }>
+  limit: number
+}
+
+export type CalibrationReasonabilityCheckFailure = InconsistentCalibrationFailure
+
 export interface SharedInstrumentData {
   mount: Mount
 }
@@ -13,6 +21,7 @@ export interface GripperData {
       offset: { x: number; y: number; z: number }
       source: string
       last_modified?: string
+      reasonability_check_failures?: null[]
     }
   }
   firmwareVersion?: string
@@ -32,6 +41,7 @@ export interface PipetteData {
       offset: { x: number; y: number; z: number }
       source: string
       last_modified?: string
+      reasonability_check_failures?: CalibrationReasonabilityCheckFailure[]
     }
   }
   firmwareVersion?: string

api/src/opentrons/hardware_control/instruments/ot3/instrument_calibration.py

@@ -1,5 +1,6 @@
 import typing
-from dataclasses import dataclass
+from typing_extensions import Literal, Final
+from dataclasses import dataclass, field
 from datetime import datetime
 
 from opentrons.config import feature_flags as ff
@@ -20,6 +21,18 @@
 )
 from opentrons.hardware_control.types import OT3Mount
 
+PIPETTE_OFFSET_CONSISTENCY_LIMIT: Final = 1.5
+
+
+@dataclass
+class InconsistentPipetteOffsets:
+    kind: Literal["inconsistent-pipette-offset"]
+    offsets: typing.Dict[Mount, Point]
+    limit: float
+
+
+ReasonabilityCheckFailure = typing.Union[InconsistentPipetteOffsets]
+
 
 @dataclass
 class PipetteOffsetByPipetteMount:
@@ -33,6 +46,13 @@ class PipetteOffsetByPipetteMount:
     last_modified: typing.Optional[datetime] = None
 
 
+@dataclass
+class PipetteOffsetSummary(PipetteOffsetByPipetteMount):
+    reasonability_check_failures: typing.List[ReasonabilityCheckFailure] = field(
+        default_factory=list
+    )
+
+
 @dataclass
 class GripperCalibrationOffset:
     """
@@ -122,3 +142,25 @@ def save_gripper_calibration_offset(
 ) -> None:
     if gripper_id and ff.enable_ot3_hardware_controller():
         ot3_gripper_offset.save_gripper_calibration(delta, gripper_id)
+
+
+def check_instrument_offset_reasonability(
+    left_offset: Point, right_offset: Point
+) -> typing.List[ReasonabilityCheckFailure]:
+    if (
+        not left_offset
+        or left_offset == Point(0, 0, 0)
+        or not right_offset
+        or right_offset == Point(0, 0, 0)
+    ):
+        return []
+    diff = left_offset - right_offset
+    if any(abs(d) > PIPETTE_OFFSET_CONSISTENCY_LIMIT for d in diff):
+        return [
+            InconsistentPipetteOffsets(
+                "inconsistent-pipette-offset",
+                {Mount.LEFT: left_offset, Mount.RIGHT: right_offset},
+                PIPETTE_OFFSET_CONSISTENCY_LIMIT,
+            )
+        ]
+    return []

api/src/opentrons/hardware_control/instruments/ot3/pipette_handler.py

@@ -45,7 +45,11 @@
 
 from opentrons.hardware_control.dev_types import PipetteDict
 from .pipette import Pipette
-from .instrument_calibration import PipetteOffsetByPipetteMount
+from .instrument_calibration import (
+    PipetteOffsetSummary,
+    PipetteOffsetByPipetteMount,
+    check_instrument_offset_reasonability,
+)
 
 
 MOD_LOG = logging.getLogger(__name__)
@@ -159,16 +163,16 @@ def _reset(m: OT3Mount) -> None:
         else:
             _reset(mount)
 
-    def get_instrument_offset(
-        self, mount: OT3Mount
-    ) -> Optional[PipetteOffsetByPipetteMount]:
+    def get_instrument_offset(self, mount: OT3Mount) -> Optional[PipetteOffsetSummary]:
         """Get the specified pipette's offset."""
         assert mount != OT3Mount.GRIPPER, "Wrong mount type to fetch pipette offset"
         try:
             pipette = self.get_pipette(mount)
         except top_types.PipetteNotAttachedError:
             return None
-        return pipette.pipette_offset
+        return self._return_augmented_offset_data(
+            pipette, mount, pipette.pipette_offset
+        )
 
     def reset_instrument_offset(self, mount: OT3Mount, to_default: bool) -> None:
         """
@@ -180,14 +184,47 @@ def reset_instrument_offset(self, mount: OT3Mount, to_default: bool) -> None:
 
     def save_instrument_offset(
         self, mount: OT3Mount, delta: top_types.Point
-    ) -> PipetteOffsetByPipetteMount:
+    ) -> PipetteOffsetSummary:
         """
         Save a new instrument offset the pipette offset to a particular value.
         :param mount: Modify the given mount.
         :param delta: The offset to set for the pipette.
         """
         pipette = self.get_pipette(mount)
-        return pipette.save_pipette_offset(mount, delta)
+        offset_data = pipette.save_pipette_offset(mount, delta)
+        return self._return_augmented_offset_data(pipette, mount, offset_data)
+
+    def _return_augmented_offset_data(
+        self,
+        pipette: Pipette,
+        mount: OT3Mount,
+        offset_data: PipetteOffsetByPipetteMount,
+    ) -> PipetteOffsetSummary:
+        if mount == OT3Mount.LEFT:
+            other_pipette = self._attached_instruments.get(OT3Mount.RIGHT, None)
+            if other_pipette:
+                other_offset = other_pipette.pipette_offset.offset
+            else:
+                other_offset = top_types.Point(0, 0, 0)
+            reasonability = check_instrument_offset_reasonability(
+                offset_data.offset, other_offset
+            )
+        else:
+            other_pipette = self._attached_instruments.get(OT3Mount.LEFT, None)
+            if other_pipette:
+                other_offset = other_pipette.pipette_offset.offset
+            else:
+                other_offset = top_types.Point(0, 0, 0)
+            reasonability = check_instrument_offset_reasonability(
+                other_offset, offset_data.offset
+            )
+        return PipetteOffsetSummary(
+            offset=offset_data.offset,
+            source=offset_data.source,
+            status=offset_data.status,
+            last_modified=offset_data.last_modified,
+            reasonability_check_failures=reasonability,
+        )
 
     # TODO(mc, 2022-01-11): change returned map value type to `Optional[PipetteDict]`
     # instead of potentially returning an empty dict

api/src/opentrons/hardware_control/ot3api.py

@@ -62,7 +62,7 @@
 from .instruments.ot3.gripper import compare_gripper_config_and_check_skip, Gripper
 from .instruments.ot3.instrument_calibration import (
     GripperCalibrationOffset,
-    PipetteOffsetByPipetteMount,
+    PipetteOffsetSummary,
 )
 from .backends.ot3controller import OT3Controller
 from .backends.ot3simulator import OT3Simulator
@@ -2040,7 +2040,7 @@ def reset_instrument(
 
     def get_instrument_offset(
         self, mount: OT3Mount
-    ) -> Union[GripperCalibrationOffset, PipetteOffsetByPipetteMount, None]:
+    ) -> Union[GripperCalibrationOffset, PipetteOffsetSummary, None]:
         """Get instrument calibration data."""
         # TODO (spp, 2023-04-19): We haven't introduced a 'calibration_offset' key in
         #  PipetteDict because the dict is shared with OT2 pipettes which have
@@ -2066,7 +2066,7 @@ async def reset_instrument_offset(
 
     async def save_instrument_offset(
         self, mount: Union[top_types.Mount, OT3Mount], delta: top_types.Point
-    ) -> Union[GripperCalibrationOffset, PipetteOffsetByPipetteMount]:
+    ) -> Union[GripperCalibrationOffset, PipetteOffsetSummary]:
         """Save a new offset for a given instrument."""
         checked_mount = OT3Mount.from_mount(mount)
         if checked_mount == OT3Mount.GRIPPER:

api/tests/opentrons/hardware_control/instruments/test_instrument_calibration.py

@@ -16,10 +16,13 @@
 )
 
 
-from opentrons import calibration_storage
+from opentrons import calibration_storage, types as top_types
 from opentrons.calibration_storage import helpers as calibration_storage_helpers
 from opentrons.calibration_storage.ot2.models import v1 as v1_models
 from opentrons.hardware_control.instruments.ot2 import instrument_calibration as subject
+from opentrons.hardware_control.instruments.ot3 import (
+    instrument_calibration as subject_ot3,
+)
 
 
 @pytest.fixture(autouse=True)
@@ -110,3 +113,35 @@ def test_load_tip_length(
             markedAt=datetime(year=2023, month=2, day=2),
         ),
     )
+
+
+@pytest.mark.parametrize(
+    "left,right,ok",
+    [
+        # If either point is all 0 (uncalibrated) then the check should pass
+        (top_types.Point(0, 0, 0), top_types.Point(0, 0, 2), True),
+        (top_types.Point(0, 0, 2), top_types.Point(0, 0, 0), True),
+        (top_types.Point(0, 0, 0), top_types.Point(0, 0, 0), True),
+        # If both points are non-zero but all values are within the range the
+        # check should pass
+        (top_types.Point(0, 1.0, 1.5), top_types.Point(-1, 0, 0.2), True),
+        # If both points are non-zero but at least one element is more than
+        # the range different the test should fail
+        (top_types.Point(0.1, -1, 1.5), top_types.Point(1.7, 0, 0.2), False),
+        (top_types.Point(0.1, -1, 1.5), top_types.Point(0.6, 0.6, 1.3), False),
+        (top_types.Point(0.1, -1, 1.5), top_types.Point(-0.2, -0.1, 5), False),
+    ],
+)
+def test_instrument_consistency_check_ot3(
+    left: top_types.Point, right: top_types.Point, ok: bool
+) -> None:
+    result = subject_ot3.check_instrument_offset_reasonability(left, right)
+    if ok:
+        assert result == []
+    else:
+        assert result[0].kind == "inconsistent-pipette-offset"
+        assert result[0].offsets == {
+            top_types.Mount.LEFT: left,
+            top_types.Mount.RIGHT: right,
+        }
+        assert result[0].limit == 1.5

api/tests/opentrons/hardware_control/test_pipette_handler.py

@@ -2,10 +2,10 @@
 import pytest
 
 from decoy import Decoy
-from typing import Optional
+from typing import Optional, Tuple, Dict
 
 from opentrons import types
-from opentrons.hardware_control.types import Axis
+from opentrons.hardware_control.types import Axis, OT3Mount
 from opentrons.hardware_control.instruments.ot2.pipette import Pipette
 from opentrons.hardware_control.instruments.ot2.pipette_handler import (
     PipetteHandlerProvider,
@@ -27,6 +27,11 @@ def mock_pipette_ot3(decoy: Decoy) -> OT3Pipette:
     return decoy.mock(cls=OT3Pipette)
 
 
+@pytest.fixture
+def mock_pipettes_ot3(decoy: Decoy) -> Tuple[OT3Pipette, OT3Pipette]:
+    return (decoy.mock(cls=OT3Pipette), decoy.mock(cls=OT3Pipette))
+
+
 @pytest.fixture
 def subject(decoy: Decoy, mock_pipette: Pipette) -> PipetteHandlerProvider:
     inst_by_mount = {types.Mount.LEFT: mock_pipette, types.Mount.RIGHT: None}
@@ -134,3 +139,44 @@ def test_plan_check_pick_up_tip_with_presses_argument_ot3(
 def test_get_pipette_fails(decoy: Decoy, subject: PipetteHandlerProvider):
     with pytest.raises(types.PipetteNotAttachedError):
         subject.get_pipette(types.Mount.RIGHT)
+
+
+@pytest.mark.parametrize(
+    "left_offset,right_offset,ok",
+    [
+        (types.Point(100, 200, 300), None, True),
+        (None, types.Point(-100, 200, -500), True),
+        (types.Point(100, 200, 300), types.Point(200, 400, 500), False),
+    ],
+)
+def test_ot3_pipette_handler_gives_checks_with_different_pipettes(
+    left_offset: Optional[types.Point],
+    right_offset: Optional[types.Point],
+    ok: bool,
+    mock_pipettes_ot3: Tuple[OT3Pipette],
+    decoy: Decoy,
+) -> None:
+    """Should give you reasonable results with one or two pipettes attached."""
+    # with a left and not right pipette, we should be able to pass our checks
+    inst_by_mount: Dict[OT3Mount, OT3Pipette] = {}
+    if left_offset is not None:
+        inst_by_mount[OT3Mount.LEFT] = mock_pipettes_ot3[0]
+        decoy.when(mock_pipettes_ot3[0].pipette_offset.offset).then_return(left_offset)
+    if right_offset is not None:
+        inst_by_mount[OT3Mount.RIGHT] = mock_pipettes_ot3[1]
+        decoy.when(mock_pipettes_ot3[1].pipette_offset.offset).then_return(right_offset)
+    subject = OT3PipetteHandler(attached_instruments=inst_by_mount)
+    if left_offset is not None:
+        left_result = subject.get_instrument_offset(OT3Mount.LEFT)
+        assert left_result.offset == left_offset
+        if ok:
+            assert left_result.reasonability_check_failures == []
+        else:
+            assert len(left_result.reasonability_check_failures) == 1
+    if right_offset is not None:
+        right_result = subject.get_instrument_offset(OT3Mount.RIGHT)
+        assert right_result.offset == right_offset
+        if ok:
+            assert right_result.reasonability_check_failures == []
+        else:
+            assert len(right_result.reasonability_check_failures) == 1

robot-server/robot_server/instruments/instrument_models.py

@@ -2,7 +2,7 @@
 from __future__ import annotations
 
 from typing_extensions import Literal
-from typing import Optional, TypeVar, Union, Generic
+from typing import Optional, TypeVar, Union, Generic, Dict, List
 from datetime import datetime
 from pydantic import BaseModel, Field
 from pydantic.generics import GenericModel
@@ -26,6 +26,21 @@
 InstrumentType = Literal["pipette", "gripper"]
 
 
+class InconsistentCalibrationFailure(BaseModel):
+    """Pipette offsets are very different from each other.
+
+    This indicates that one of the pipettes (though we can't tell which one)
+    was likely calibrated with its calibration probe not fully attached, and
+    should be redone. However, it's possible that the pipettes are in fact
+    calibrated correctly and their offsets look strange, so this should be
+    taken as an advisory.
+    """
+
+    kind: Literal["inconsistent-pipette-offset"] = "inconsistent-pipette-offset"
+    offsets: Dict[str, Vec3f]
+    limit: float
+
+
 class _GenericInstrument(GenericModel, Generic[InstrumentModelT, InstrumentDataT]):
     """Base instrument response."""
 
@@ -54,6 +69,7 @@ class InstrumentCalibrationData(BaseModel):
     offset: Vec3f
     source: SourceType
     last_modified: Optional[datetime] = None
+    reasonability_check_failures: List[Union[InconsistentCalibrationFailure]]
 
 
 class GripperData(BaseModel):