fixed calculation for labware height on modules

Author: sanni-t

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

@@ -136,7 +136,10 @@ 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.
+        """
         if isinstance(labware_location, DeckSlotLocation):
             return LabwareOffsetVector(x=0, y=0, z=0)
         elif isinstance(labware_location, ModuleLocation):
@@ -434,17 +437,13 @@ 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, (ModuleLocation, OnLabwareLocation)):
+            if isinstance(location, ModuleLocation):
+                location_slot = self._modules.get_location(location.moduleId).slotName
+            else:
+                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)
@@ -455,6 +454,8 @@ def get_labware_grip_point(
             )
         else:
             location_slot = location.slotName
+            offset = LabwareOffsetVector(x=0, y=0, z=0)
+
         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

@@ -1075,13 +1075,6 @@ 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,
@@ -1096,20 +1089,6 @@ def test_get_labware_grip_point(
         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)
     )
@@ -1120,30 +1099,43 @@ def test_get_labware_grip_point(
     assert labware_center == expected_center_point
 
 
+@pytest.mark.parametrize(
+    argnames=["location", "expected_center_point"],
+    argvalues=[
+        (OnLabwareLocation(labwareId="labware-id"), Point(5, 10, 115.0)),
+        (ModuleLocation(moduleId="module-id"), Point(111.0, 122.0, 233)),
+    ],
+)
 def test_get_labware_grip_point_on_labware(
     decoy: Decoy,
     labware_view: LabwareView,
     module_view: ModuleView,
     ot2_standard_deck_def: DeckDefinitionV3,
     subject: GeometryView,
+    location: Union[ModuleLocation, OnLabwareLocation],
+    expected_center_point: Point,
 ) -> None:
     """It should get the grip point of a labware on another labware."""
-    decoy.when(labware_view.get(labware_id="labware-id")).then_return(
-        LoadedLabware(
-            id="labware-id",
-            loadName="above-name",
-            definitionUri="1234",
-            location=OnLabwareLocation(labwareId="below-id"),
+    if isinstance(location, ModuleLocation):
+        decoy.when(module_view.get_location("module-id")).then_return(DeckSlotLocation(slotName=DeckSlotName.SLOT_4))
+    else:
+
+        decoy.when(labware_view.get(labware_id="labware-id")).then_return(
+            LoadedLabware(
+                id="labware-id",
+                loadName="above-name",
+                definitionUri="1234",
+                location=OnLabwareLocation(labwareId="below-id"),
+            )
         )
-    )
-    decoy.when(labware_view.get(labware_id="below-id")).then_return(
-        LoadedLabware(
-            id="below-id",
-            loadName="below-name",
-            definitionUri="1234",
-            location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4),
+        decoy.when(labware_view.get(labware_id="below-id")).then_return(
+            LoadedLabware(
+                id="below-id",
+                loadName="below-name",
+                definitionUri="1234",
+                location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4),
+            )
         )
-    )
 
     decoy.when(labware_view.get_dimensions("below-id")).then_return(
         Dimensions(x=1000, y=1001, z=11)
@@ -1163,7 +1155,7 @@ def test_get_labware_grip_point_on_labware(
         labware_id="labware-id", location=OnLabwareLocation(labwareId="below-id")
     )
 
-    assert grip_point == Point(5, 10, 115.0)
+    assert grip_point == expected_center_point
 
 
 @pytest.mark.parametrize(