fix(engine): fix calculation for labware height on modules (#13364)

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

@@ -136,7 +136,19 @@ def get_labware_parent_nominal_position(self, labware_id: str) -> Point:
     def _get_labware_position_offset(
         self, labware_id: str, labware_location: LabwareLocation
     ) -> LabwareOffsetVector:
-        """Gets the offset vector of a labware on the given location."""
+        """Gets the offset vector of a labware on the given location.
+
+        NOTE: Not to be confused with LPC offset.
+        - For labware on Deck Slot: returns an offset of (0, 0, 0)
+        - For labware on a Module: returns the nominal offset for the labware's position
+          when placed on the specified module (using slot-transformed labwareOffset
+          from the module's definition with any stacking overlap).
+          Does not include module calibration offset or LPC offset.
+        - For labware on another labware: returns the nominal offset for the labware
+          as placed on the specified labware, taking into account any offsets for labware
+          on modules as well as stacking overlaps.
+          Does not include module calibration offset or LPC offset.
+        """
         if isinstance(labware_location, DeckSlotLocation):
             return LabwareOffsetVector(x=0, y=0, z=0)
         elif isinstance(labware_location, ModuleLocation):
@@ -434,17 +446,16 @@ def get_labware_grip_point(
         grip_height_from_labware_bottom = (
             self._labware.get_grip_height_from_labware_bottom(labware_id)
         )
-        offset = LabwareOffsetVector(x=0, y=0, z=0)
         location_slot: DeckSlotName
 
-        if isinstance(location, ModuleLocation):
-            deck_type = DeckType(self._labware.get_deck_definition()["otId"])
-            offset = self._modules.get_module_offset(
-                module_id=location.moduleId, deck_type=deck_type
-            )
-            location_slot = self._modules.get_location(location.moduleId).slotName
-        elif isinstance(location, OnLabwareLocation):
-            location_slot = self.get_ancestor_slot_name(location.labwareId)
+        if isinstance(location, DeckSlotLocation):
+            location_slot = location.slotName
+            offset = LabwareOffsetVector(x=0, y=0, z=0)
+        else:
+            if isinstance(location, ModuleLocation):
+                location_slot = self._modules.get_location(location.moduleId).slotName
+            else:  # OnLabwareLocation
+                location_slot = self.get_ancestor_slot_name(location.labwareId)
             labware_offset = self._get_labware_position_offset(labware_id, location)
             # Get the calibrated offset if the on labware location is on top of a module, otherwise return empty one
             cal_offset = self._get_calibrated_module_offset(location)
@@ -453,8 +464,7 @@ def get_labware_grip_point(
                 y=labware_offset.y + cal_offset.y,
                 z=labware_offset.z + cal_offset.z,
             )
-        else:
-            location_slot = location.slotName
+
         slot_center = self._labware.get_slot_center_position(location_slot)
         return Point(
             slot_center.x + offset.x,

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

@@ -3,7 +3,7 @@
 
 import pytest
 from decoy import Decoy
-from typing import cast, List, Tuple, Union, Optional, NamedTuple
+from typing import cast, List, Tuple, Optional, NamedTuple
 
 from opentrons_shared_data.deck.dev_types import DeckDefinitionV3
 from opentrons_shared_data.labware.dev_types import LabwareUri
@@ -1075,49 +1075,26 @@ def test_ensure_location_not_occupied_raises(
     )
 
 
-@pytest.mark.parametrize(
-    argnames=["location", "expected_center_point"],
-    argvalues=[
-        (DeckSlotLocation(slotName=DeckSlotName.SLOT_1), Point(101.0, 102.0, 203)),
-        (ModuleLocation(moduleId="module-id"), Point(111.0, 122.0, 233)),
-    ],
-)
 def test_get_labware_grip_point(
     decoy: Decoy,
     labware_view: LabwareView,
     module_view: ModuleView,
     ot2_standard_deck_def: DeckDefinitionV3,
     subject: GeometryView,
-    location: Union[DeckSlotLocation, ModuleLocation],
-    expected_center_point: Point,
 ) -> None:
     """It should get the grip point of the labware at the specified location."""
     decoy.when(
         labware_view.get_grip_height_from_labware_bottom("labware-id")
     ).then_return(100)
 
-    if isinstance(location, ModuleLocation):
-        decoy.when(labware_view.get_deck_definition()).then_return(
-            ot2_standard_deck_def
-        )
-        decoy.when(
-            module_view.get_module_offset(
-                module_id="module-id", deck_type=DeckType.OT2_STANDARD
-            )
-        ).then_return(LabwareOffsetVector(x=10, y=20, z=30))
-
-        decoy.when(module_view.get_location("module-id")).then_return(
-            DeckSlotLocation(slotName=DeckSlotName.SLOT_1)
-        )
-
     decoy.when(labware_view.get_slot_center_position(DeckSlotName.SLOT_1)).then_return(
         Point(x=101, y=102, z=103)
     )
     labware_center = subject.get_labware_grip_point(
-        labware_id="labware-id", location=location
+        labware_id="labware-id", location=DeckSlotLocation(slotName=DeckSlotName.SLOT_1)
     )
 
-    assert labware_center == expected_center_point
+    assert labware_center == Point(101.0, 102.0, 203)
 
 
 def test_get_labware_grip_point_on_labware(
@@ -1166,6 +1143,47 @@ def test_get_labware_grip_point_on_labware(
     assert grip_point == Point(5, 10, 115.0)
 
 
+def test_get_labware_grip_point_for_labware_on_module(
+    decoy: Decoy,
+    labware_view: LabwareView,
+    module_view: ModuleView,
+    ot2_standard_deck_def: DeckDefinitionV3,
+    subject: GeometryView,
+) -> None:
+    """It should return the grip point for labware directly on a module."""
+    decoy.when(
+        labware_view.get_grip_height_from_labware_bottom("labware-id")
+    ).then_return(500)
+    decoy.when(module_view.get_location("module-id")).then_return(
+        DeckSlotLocation(slotName=DeckSlotName.SLOT_4)
+    )
+    decoy.when(labware_view.get_deck_definition()).then_return(ot2_standard_deck_def)
+    decoy.when(
+        module_view.get_nominal_module_offset(
+            module_id="module-id", deck_type=DeckType.OT2_STANDARD
+        )
+    ).then_return(LabwareOffsetVector(x=1, y=2, z=3))
+    decoy.when(module_view.get_connected_model("module-id")).then_return(
+        ModuleModel.MAGNETIC_MODULE_V2
+    )
+    decoy.when(
+        labware_view.get_module_overlap_offsets(
+            "labware-id", ModuleModel.MAGNETIC_MODULE_V2
+        )
+    ).then_return(OverlapOffset(x=10, y=20, z=30))
+    decoy.when(module_view.get_module_calibration_offset("module-id")).then_return(
+        ModuleOffsetVector(x=100, y=200, z=300)
+    )
+    decoy.when(labware_view.get_slot_center_position(DeckSlotName.SLOT_4)).then_return(
+        Point(100, 200, 300)
+    )
+    result_grip_point = subject.get_labware_grip_point(
+        labware_id="labware-id", location=ModuleLocation(moduleId="module-id")
+    )
+
+    assert result_grip_point == Point(x=191, y=382, z=1073)
+
+
 @pytest.mark.parametrize(
     argnames=["location", "should_dodge", "expected_waypoints"],
     argvalues=[