fix(api): fix incorrect gripper point calculation for schema 3 labware (#18920)

Author: mjhuff

api/src/opentrons/protocol_engine/state/geometry.py

@@ -23,6 +23,7 @@
 from opentrons_shared_data.labware.constants import WELL_NAME_PATTERN
 from opentrons_shared_data.labware.labware_definition import (
     LabwareDefinition,
+    LabwareDefinition2,
     InnerWellGeometry,
 )
 from opentrons_shared_data.deck.types import CutoutFixture
@@ -91,6 +92,7 @@
     WellLocationType,
     WellLocationFunction,
     LabwareParentDefinition,
+    AddressableArea,
 )
 from ..types.liquid_level_detection import SimulatedProbeResult, LiquidTrackingType
 from .config import Config
@@ -1041,26 +1043,44 @@ def get_labware_grip_point(
         grip_height_from_labware_bottom = (
             self._labware.get_grip_height_from_labware_bottom(labware_definition)
         )
-        location_name = self._get_underlying_addressable_area_name(location)
+        aa_name = self._get_underlying_addressable_area_name(location)
         parent_to_lw_offset = self._get_stackup_placement_origin_to_lw_origin(
             location=location,
             definition=labware_definition,
             is_topmost_labware=True,  # We aren't concerned with entities above the gripped labware.
         )
-        mod_cal_offset = self._get_calibrated_module_offset(location)
-        location_center = self._addressable_areas.get_addressable_area_center(
-            location_name
+        addressable_area = self._addressable_areas.get_addressable_area(aa_name)
+        lw_origin_to_parent = self._get_lw_origin_to_parent(
+            labware_definition=labware_definition, addressable_area=addressable_area
         )
+        mod_cal_offset = self._get_calibrated_module_offset(location)
+        location_center = self._addressable_areas.get_addressable_area_center(aa_name)
 
-        return Point(
-            x=location_center.x + parent_to_lw_offset.x + mod_cal_offset.x,
-            y=location_center.y + parent_to_lw_offset.y + mod_cal_offset.y,
-            z=location_center.z
-            + parent_to_lw_offset.z
-            + mod_cal_offset.z
-            + grip_height_from_labware_bottom,
+        return (
+            location_center
+            + parent_to_lw_offset
+            + lw_origin_to_parent
+            + mod_cal_offset
+            + Point(0, 0, grip_height_from_labware_bottom)
         )
 
+    def _get_lw_origin_to_parent(
+        self, labware_definition: LabwareDefinition, addressable_area: AddressableArea
+    ) -> Point:
+        if isinstance(labware_definition, LabwareDefinition2):
+            return Point(0, 0, 0)
+        else:
+            bb_y = addressable_area.bounding_box.y
+            bb_z = addressable_area.bounding_box.z
+            return (
+                Point(
+                    x=0,
+                    y=bb_y,
+                    z=bb_z,
+                )
+                * -1
+            )
+
     def get_extra_waypoints(
         self,
         location: Optional[CurrentPipetteLocation],

api/tests/opentrons/protocol_engine/state/test_geometry_view.py

@@ -192,6 +192,15 @@
     ),
 )
 
+_MOCK_LABWARE_DEFINITION2 = LabwareDefinition2.model_construct(  # type: ignore[call-arg]
+    namespace="test",
+    version=1,
+    schemaVersion=2,
+    dimensions=LabwareDimensions(xDimension=1000, yDimension=1200, zDimension=750),
+    parameters=LabwareDefinition2Parameters.model_construct(loadName="labware-name"),  # type: ignore[call-arg]
+)
+
+
 MOCK_ADDRESSABLE_AREA = AddressableArea(
     area_name="1",
     area_type=AreaType.SLOT,
@@ -2735,31 +2744,76 @@ def test_ensure_location_not_occupied_raises(
     )
 
 
-def test_get_labware_grip_point(
+def test_get_labware_grip_point_v2_definition(
     decoy: Decoy,
     mock_labware_view: LabwareView,
     mock_addressable_area_view: AddressableAreaView,
     subject: GeometryView,
 ) -> None:
-    """It should get the grip point of the labware at the specified location."""
+    """It should get the grip point of a LabwareDefinition2 labware at the specified location."""
     decoy.when(
-        mock_labware_view.get_grip_height_from_labware_bottom(
-            sentinel.labware_definition
-        )
+        mock_labware_view.get_grip_height_from_labware_bottom(_MOCK_LABWARE_DEFINITION2)
     ).then_return(100)
 
     decoy.when(
         mock_addressable_area_view.get_addressable_area_center(DeckSlotName.SLOT_1.id)
     ).then_return(Point(x=101, y=102, z=103))
+
+    decoy.when(
+        mock_addressable_area_view.get_addressable_area(DeckSlotName.SLOT_1.id)
+    ).then_return(MOCK_ADDRESSABLE_AREA)
+
+    expected_lw_origin_to_parent = Point(0, 0, 0)
+
     labware_center = subject.get_labware_grip_point(
-        labware_definition=sentinel.labware_definition,
+        labware_definition=_MOCK_LABWARE_DEFINITION2,
         location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1),
     )
 
     assert labware_center == Point(
-        101.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x,
-        102.0 + +_PARENT_ORIGIN_TO_LABWARE_ORIGIN.y,
-        203 + +_PARENT_ORIGIN_TO_LABWARE_ORIGIN.z,
+        101.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x + expected_lw_origin_to_parent.x,
+        102.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y + expected_lw_origin_to_parent.y,
+        203 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z + expected_lw_origin_to_parent.z,
+    )
+
+
+def test_get_labware_grip_point_v3_definition(
+    decoy: Decoy,
+    mock_labware_view: LabwareView,
+    mock_addressable_area_view: AddressableAreaView,
+    subject: GeometryView,
+) -> None:
+    """It should get the grip point of a LabwareDefinition3 labware at the specified location."""
+    decoy.when(
+        mock_labware_view.get_grip_height_from_labware_bottom(_MOCK_LABWARE_DEFINITION3)
+    ).then_return(100)
+
+    decoy.when(
+        mock_addressable_area_view.get_addressable_area_center(DeckSlotName.SLOT_1.id)
+    ).then_return(Point(x=101, y=102, z=103))
+
+    decoy.when(
+        mock_addressable_area_view.get_addressable_area(DeckSlotName.SLOT_1.id)
+    ).then_return(MOCK_ADDRESSABLE_AREA)
+
+    expected_lw_origin_to_parent = (
+        Point(
+            0,
+            MOCK_ADDRESSABLE_AREA.bounding_box.y,
+            MOCK_ADDRESSABLE_AREA.bounding_box.z,
+        )
+        * -1
+    )
+
+    labware_center = subject.get_labware_grip_point(
+        labware_definition=_MOCK_LABWARE_DEFINITION3,
+        location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1),
+    )
+
+    assert labware_center == Point(
+        101.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x + expected_lw_origin_to_parent.x,
+        102.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y + expected_lw_origin_to_parent.y,
+        203 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z + expected_lw_origin_to_parent.z,
     )
 
 
@@ -2790,15 +2844,31 @@ def test_get_labware_grip_point_on_labware(
         mock_addressable_area_view.get_addressable_area_center(DeckSlotName.SLOT_4.id)
     ).then_return(Point(x=5, y=9, z=10))
 
+    decoy.when(
+        mock_addressable_area_view.get_addressable_area(DeckSlotName.SLOT_4.id)
+    ).then_return(MOCK_ADDRESSABLE_AREA)
+
+    expected_lw_origin_to_parent = (
+        Point(
+            0,
+            MOCK_ADDRESSABLE_AREA.bounding_box.y,
+            MOCK_ADDRESSABLE_AREA.bounding_box.z,
+        )
+        * -1
+    )
+
     grip_point = subject.get_labware_grip_point(
         labware_definition=sentinel.definition,
         location=OnLabwareLocation(labwareId="below-id"),
     )
 
     assert grip_point == Point(
-        5.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x * 2,  # The labware and adapter
-        9.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y * 2,
-        110.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z * 2,
+        5.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x * 2 + expected_lw_origin_to_parent.x,
+        9.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y * 2 + expected_lw_origin_to_parent.y,
+        10.0
+        + 100
+        + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z * 2
+        + expected_lw_origin_to_parent.z,
     )
 
 
@@ -2868,14 +2938,23 @@ def test_get_labware_grip_point_for_labware_on_module(
         )
     ).then_return(Point(x=0, y=0, z=0))
 
+    expected_lw_origin_to_parent = (
+        Point(
+            0,
+            MOCK_ADDRESSABLE_AREA.bounding_box.y,
+            MOCK_ADDRESSABLE_AREA.bounding_box.z,
+        )
+        * -1
+    )
+
     result_grip_point = subject.get_labware_grip_point(
         labware_definition=sentinel.labware_definition,
         location=ModuleLocation(moduleId="module-id"),
     )
     assert result_grip_point == Point(
-        x=492 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x,
-        y=350 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y,
-        z=838 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z,
+        x=492 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x + expected_lw_origin_to_parent.x,
+        y=350 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y + expected_lw_origin_to_parent.y,
+        z=838 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z + expected_lw_origin_to_parent.z,
     )
 
 
@@ -2953,16 +3032,30 @@ def test_get_labware_grip_point_for_labware_stack_on_module(
         "magneticBlockV1C3"
     )
 
+    expected_lw_origin_to_parent = (
+        Point(
+            0,
+            MOCK_ADDRESSABLE_AREA.bounding_box.y,
+            MOCK_ADDRESSABLE_AREA.bounding_box.z,
+        )
+        * -1
+    )
+
     result_grip_point = subject.get_labware_grip_point(
         labware_definition=sentinel.labware_definition,
         location=OnLabwareLocation(labwareId="below-id-9"),
     )
 
     assert result_grip_point == Point(
         x=492.0
-        + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x * 2,  # The labware and module beneath it.
-        y=350.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y * 2,
-        z=838.0 + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z * 2,
+        + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.x * 2  # The labware and module beneath it.
+        + expected_lw_origin_to_parent.x,
+        y=350.0
+        + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.y * 2
+        + expected_lw_origin_to_parent.y,
+        z=838.0
+        + _PARENT_ORIGIN_TO_LABWARE_ORIGIN.z * 2
+        + expected_lw_origin_to_parent.z,
     )