Skip to content

Commit 1e5b289

Browse files
authored
fix(api): Fix the absorbance reader lid pickup (#19699)
# Overview The updated gripper failure check did break the absorbance reader movement check. This PR adds a labware quirk that makes the check revert back to the legacy check.
1 parent 4a75419 commit 1e5b289

File tree

11 files changed

+97
-27
lines changed

11 files changed

+97
-27
lines changed

api/src/opentrons/hardware_control/backends/flex_protocol.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ def check_gripper_position_within_bounds(
451451
max_allowed_grip_error: float,
452452
hard_limit_lower: float,
453453
hard_limit_upper: float,
454+
disable_geometry_grip_check: bool = False,
454455
) -> None:
455456
...
456457

api/src/opentrons/hardware_control/backends/ot3controller.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,6 +1763,7 @@ def check_gripper_position_within_bounds(
17631763
max_allowed_grip_error: float,
17641764
hard_limit_lower: float,
17651765
hard_limit_upper: float,
1766+
disable_geometry_grip_check: bool = False,
17661767
) -> None:
17671768
"""
17681769
Check if the gripper is at the expected location.
@@ -1805,6 +1806,7 @@ def check_gripper_position_within_bounds(
18051806
if (
18061807
current_gripper_position - expected_gripper_position_min
18071808
< -max_allowed_grip_error
1809+
and not disable_geometry_grip_check
18081810
):
18091811
raise FailedGripperPickupError(
18101812
message="Failed to grip: jaws closed too far",
@@ -1818,6 +1820,7 @@ def check_gripper_position_within_bounds(
18181820
if (
18191821
current_gripper_position - expected_gripper_position_max
18201822
> max_allowed_grip_error
1823+
and not disable_geometry_grip_check
18211824
):
18221825
raise FailedGripperPickupError(
18231826
message="Failed to grip: jaws could not close far enough",

api/src/opentrons/hardware_control/backends/ot3simulator.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,7 @@ def check_gripper_position_within_bounds(
848848
max_allowed_grip_error: float,
849849
hard_limit_lower: float,
850850
hard_limit_upper: float,
851+
disable_geometry_grip_check: bool = False,
851852
) -> None:
852853
# This is a (pretty bad) simulation of the gripper actually gripping something,
853854
# but it should work.

api/src/opentrons/hardware_control/ot3api.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1462,6 +1462,7 @@ def raise_error_if_gripper_pickup_failed(
14621462
expected_grip_width: float,
14631463
grip_width_uncertainty_wider: float,
14641464
grip_width_uncertainty_narrower: float,
1465+
disable_geometry_grip_check: bool = False,
14651466
) -> None:
14661467
"""Ensure that a gripper pickup succeeded.
14671468
@@ -1482,6 +1483,7 @@ def raise_error_if_gripper_pickup_failed(
14821483
gripper.max_allowed_grip_error,
14831484
gripper.min_jaw_width,
14841485
gripper.max_jaw_width,
1486+
disable_geometry_grip_check,
14851487
)
14861488

14871489
def gripper_jaw_can_home(self) -> bool:

api/src/opentrons/hardware_control/protocols/gripper_controller.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ def raise_error_if_gripper_pickup_failed(
4141
expected_grip_width: float,
4242
grip_width_uncertainty_wider: float,
4343
grip_width_uncertainty_narrower: float,
44+
disable_geometry_grip_check: bool = False,
4445
) -> None:
4546
"""Ensure that a gripper pickup succeeded."""
4647

api/src/opentrons/protocol_engine/execution/labware_movement.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from typing import Optional, TYPE_CHECKING, overload
66

7-
from opentrons_shared_data.labware.labware_definition import LabwareDefinition
7+
from opentrons_shared_data.labware.labware_definition import LabwareDefinition, Quirks
88

99
from opentrons.types import Point
1010

@@ -238,13 +238,21 @@ async def move_labware_with_gripper( # noqa: C901
238238
labware_definition=labware_definition
239239
)
240240

241+
disable_geometry_grip_check = False
242+
if labware_definition.parameters.quirks is not None:
243+
disable_geometry_grip_check = (
244+
Quirks.disableGeometryBasedGripCheck # type: ignore[comparison-overlap]
245+
in labware_definition.parameters.quirks
246+
)
247+
241248
# todo(mm, 2024-09-26): This currently raises a lower-level 2015 FailedGripperPickupError.
242249
# Convert this to a higher-level 3001 LabwareDroppedError or 3002 LabwareNotPickedUpError,
243250
# depending on what waypoint we're at, to propagate a more specific error code to users.
244251
ot3api.raise_error_if_gripper_pickup_failed(
245252
expected_grip_width=grip_specs.targetY,
246253
grip_width_uncertainty_wider=grip_specs.uncertaintyWider,
247254
grip_width_uncertainty_narrower=grip_specs.uncertaintyNarrower,
255+
disable_geometry_grip_check=disable_geometry_grip_check,
248256
)
249257
await ot3api.move_to(
250258
mount=gripper_mount, abs_position=waypoint_data.position

api/tests/opentrons/hardware_control/backends/test_ot3_controller.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,7 @@ async def test_engage_motors(
12921292
(80, 79, 0, 0, 1, 92, 60, False),
12931293
(80, 45, 40, 0, 1, 92, 60, True),
12941294
(80, 100, 0, 40, 0, 92, 60, True),
1295+
(95.5, 84, 0, 5, 5, 94, 60, True),
12951296
],
12961297
)
12971298
def test_grip_error_detection(
@@ -1321,6 +1322,42 @@ def test_grip_error_detection(
13211322
)
13221323

13231324

1325+
@pytest.mark.parametrize(
1326+
"expected_grip_width,actual_grip_width,wider,narrower,allowed_error,hard_max,hard_min,raise_error",
1327+
[
1328+
(95.5, 84, 0, 5, 5, 94, 60, False),
1329+
(95.5, 60, 0, 5, 5, 94, 60, True),
1330+
(95.5, 94, 0, 5, 5, 94, 60, True),
1331+
],
1332+
)
1333+
def test_grip_error_detection_disable_geometry(
1334+
controller: OT3Controller,
1335+
expected_grip_width: float,
1336+
actual_grip_width: float,
1337+
wider: float,
1338+
narrower: float,
1339+
allowed_error: float,
1340+
hard_max: float,
1341+
hard_min: float,
1342+
raise_error: bool,
1343+
) -> None:
1344+
context = cast(
1345+
AbstractContextManager[None],
1346+
pytest.raises(FailedGripperPickupError) if raise_error else does_not_raise(),
1347+
)
1348+
with context:
1349+
controller.check_gripper_position_within_bounds(
1350+
expected_grip_width,
1351+
wider,
1352+
narrower,
1353+
actual_grip_width,
1354+
allowed_error,
1355+
hard_min,
1356+
hard_max,
1357+
True,
1358+
)
1359+
1360+
13241361
def move_group_run_side_effect(
13251362
controller: OT3Controller, target_pos: Dict[Axis, float]
13261363
) -> Iterator[Dict[NodeId, MotorPositionStatus]]:

api/tests/opentrons/protocol_engine/execution/test_labware_movement_handler.py

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@
44

55
from datetime import datetime
66
from typing import TYPE_CHECKING, Union, Optional
7-
from unittest.mock import sentinel
87

98
from decoy import Decoy, matchers
109
import pytest
1110

11+
from opentrons_shared_data.labware.labware_definition import LabwareDefinition2
12+
1213
from opentrons.protocol_engine.execution import EquipmentHandler, MovementHandler
1314
from opentrons.hardware_control import HardwareControlAPI
1415
from opentrons.types import DeckSlotName, Point
@@ -89,8 +90,17 @@ def heater_shaker_movement_flagger(decoy: Decoy) -> HeaterShakerMovementFlagger:
8990
return decoy.mock(cls=HeaterShakerMovementFlagger)
9091

9192

93+
@pytest.fixture
94+
def labware_def(decoy: Decoy) -> LabwareDefinition2:
95+
"""Get a mocked out LabwareDefinition2 instance."""
96+
return decoy.mock(cls=LabwareDefinition2)
97+
98+
9299
async def set_up_decoy_hardware_gripper(
93-
decoy: Decoy, ot3_hardware_api: OT3API, state_store: StateStore
100+
decoy: Decoy,
101+
ot3_hardware_api: OT3API,
102+
state_store: StateStore,
103+
labware_def: LabwareDefinition2,
94104
) -> None:
95105
"""Shared hardware gripper decoy setup."""
96106
decoy.when(state_store.config.use_virtual_gripper).then_return(False)
@@ -111,9 +121,7 @@ async def set_up_decoy_hardware_gripper(
111121

112122
decoy.when(ot3_hardware_api.hardware_gripper.jaw_width).then_return(89)
113123

114-
decoy.when(
115-
state_store.labware.get_grip_force(sentinel.my_teleporting_labware_def)
116-
).then_return(100)
124+
decoy.when(state_store.labware.get_grip_force(labware_def)).then_return(100)
117125

118126
decoy.when(state_store.labware.get_labware_offset("new-offset-id")).then_return(
119127
LabwareOffset(
@@ -156,13 +164,16 @@ async def test_raise_error_if_gripper_pickup_failed(
156164
state_store: StateStore,
157165
thermocycler_plate_lifter: ThermocyclerPlateLifter,
158166
ot3_hardware_api: OT3API,
167+
labware_def: LabwareDefinition2,
159168
subject: LabwareMovementHandler,
160169
) -> None:
161170
"""Test that the gripper position check is called at the right time."""
162171
# This function should only be called when after the gripper opens,
163172
# and then closes again. This is when we expect the labware to be
164173
# in the gripper jaws.
165-
await set_up_decoy_hardware_gripper(decoy, ot3_hardware_api, state_store)
174+
await set_up_decoy_hardware_gripper(
175+
decoy, ot3_hardware_api, state_store, labware_def
176+
)
166177
assert ot3_hardware_api.hardware_gripper
167178

168179
user_pick_up_offset = Point(x=123, y=234, z=345)
@@ -175,9 +186,11 @@ async def test_raise_error_if_gripper_pickup_failed(
175186
starting_location = DeckSlotLocation(slotName=DeckSlotName.SLOT_1)
176187
to_location = DeckSlotLocation(slotName=DeckSlotName.SLOT_2)
177188

189+
labware_def.parameters.quirks = []
190+
178191
decoy.when(
179192
state_store.labware.get_definition("my-teleporting-labware")
180-
).then_return(sentinel.my_teleporting_labware_def)
193+
).then_return(labware_def)
181194

182195
mock_tc_context_manager = decoy.mock(name="mock_tc_context_manager")
183196
decoy.when(
@@ -202,21 +215,19 @@ async def test_raise_error_if_gripper_pickup_failed(
202215

203216
decoy.when(
204217
state_store.geometry.get_labware_grip_point(
205-
labware_definition=sentinel.my_teleporting_labware_def,
218+
labware_definition=labware_def,
206219
location=starting_location,
207220
)
208221
).then_return(Point(101, 102, 119.5))
209222

210223
decoy.when(
211224
state_store.geometry.get_labware_grip_point(
212-
labware_definition=sentinel.my_teleporting_labware_def, location=to_location
225+
labware_definition=labware_def, location=to_location
213226
)
214227
).then_return(Point(201, 202, 219.5))
215228

216229
decoy.when(
217-
state_store.labware.get_gripper_width_specs(
218-
labware_definition=sentinel.my_teleporting_labware_def
219-
)
230+
state_store.labware.get_gripper_width_specs(labware_definition=labware_def)
220231
).then_return(GripSpecs(targetY=100, uncertaintyNarrower=5, uncertaintyWider=10))
221232

222233
await subject.move_labware_with_gripper(
@@ -238,18 +249,21 @@ async def test_raise_error_if_gripper_pickup_failed(
238249
expected_grip_width=100,
239250
grip_width_uncertainty_wider=10,
240251
grip_width_uncertainty_narrower=5,
252+
disable_geometry_grip_check=False,
241253
),
242254
await ot3_hardware_api.grip(force_newtons=100),
243255
ot3_hardware_api.raise_error_if_gripper_pickup_failed(
244256
expected_grip_width=100,
245257
grip_width_uncertainty_wider=10,
246258
grip_width_uncertainty_narrower=5,
259+
disable_geometry_grip_check=False,
247260
),
248261
await ot3_hardware_api.grip(force_newtons=100),
249262
ot3_hardware_api.raise_error_if_gripper_pickup_failed(
250263
expected_grip_width=100,
251264
grip_width_uncertainty_wider=10,
252265
grip_width_uncertainty_narrower=5,
266+
disable_geometry_grip_check=False,
253267
),
254268
await ot3_hardware_api.disengage_axes([Axis.Z_G]),
255269
await ot3_hardware_api.ungrip(),
@@ -292,6 +306,7 @@ async def test_move_labware_with_gripper(
292306
thermocycler_plate_lifter: ThermocyclerPlateLifter,
293307
ot3_hardware_api: OT3API,
294308
subject: LabwareMovementHandler,
309+
labware_def: LabwareDefinition2,
295310
from_location: Union[DeckSlotLocation, ModuleLocation, OnLabwareLocation],
296311
to_location: Union[DeckSlotLocation, ModuleLocation, OnLabwareLocation],
297312
slide_offset: Optional[Point],
@@ -300,11 +315,14 @@ async def test_move_labware_with_gripper(
300315
# TODO (spp, 2023-07-26): this test does NOT stub out movement waypoints in order to
301316
# keep this as the semi-smoke test that it previously was. We should add a proper
302317
# smoke test for gripper labware movement with actual labware and make this a unit test.
303-
await set_up_decoy_hardware_gripper(decoy, ot3_hardware_api, state_store)
318+
labware_def.parameters.quirks = []
319+
await set_up_decoy_hardware_gripper(
320+
decoy, ot3_hardware_api, state_store, labware_def
321+
)
304322

305323
decoy.when(
306324
state_store.labware.get_definition("my-teleporting-labware")
307-
).then_return(sentinel.my_teleporting_labware_def)
325+
).then_return(labware_def)
308326

309327
user_pick_up_offset = Point(x=123, y=234, z=345)
310328
user_drop_offset = Point(x=111, y=222, z=333)
@@ -329,26 +347,22 @@ async def test_move_labware_with_gripper(
329347
) # TODO: Is this used for anything? Could this have been a sentinel? Are sentinels appropriate here?
330348

331349
decoy.when(
332-
state_store.labware.get_dimensions(
333-
labware_definition=sentinel.my_teleporting_labware_def
334-
)
350+
state_store.labware.get_dimensions(labware_definition=labware_def)
335351
).then_return(Dimensions(x=100, y=85, z=0))
336352

337353
decoy.when(
338-
state_store.labware.get_well_bbox(
339-
labware_definition=sentinel.my_teleporting_labware_def
340-
)
354+
state_store.labware.get_well_bbox(labware_definition=labware_def)
341355
).then_return(Dimensions(x=99, y=80, z=1))
342356

343357
decoy.when(
344358
state_store.geometry.get_labware_grip_point(
345-
labware_definition=sentinel.my_teleporting_labware_def,
359+
labware_definition=labware_def,
346360
location=from_location,
347361
)
348362
).then_return(Point(101, 102, 119.5))
349363
decoy.when(
350364
state_store.geometry.get_labware_grip_point(
351-
labware_definition=sentinel.my_teleporting_labware_def, location=to_location
365+
labware_definition=labware_def, location=to_location
352366
)
353367
).then_return(Point(201, 202, 219.5))
354368
mock_tc_context_manager = decoy.mock(name="mock_tc_context_manager")
@@ -359,9 +373,7 @@ async def test_move_labware_with_gripper(
359373
).then_return(mock_tc_context_manager)
360374

361375
decoy.when(
362-
state_store.labware.get_gripper_width_specs(
363-
labware_definition=sentinel.my_teleporting_labware_def
364-
)
376+
state_store.labware.get_gripper_width_specs(labware_definition=labware_def)
365377
).then_return(GripSpecs(targetY=100, uncertaintyNarrower=5, uncertaintyWider=10))
366378

367379
expected_waypoints = [

shared-data/js/__tests__/labwareDefQuirks.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const EXPECTED_VALID_QUIRKS = [
1515
'tiprackAdapterFor96Channel',
1616
'stackingMaxFive',
1717
'stackingOnly',
18+
'disableGeometryBasedGripCheck',
1819
]
1920

2021
describe('check quirks for all labware defs', () => {

shared-data/labware/definitions/2/opentrons_flex_lid_absorbance_plate_reader_module/1.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
],
2525
"parameters": {
2626
"format": "irregular",
27-
"quirks": [],
27+
"quirks": ["disableGeometryBasedGripCheck"],
2828
"isTiprack": false,
2929
"isMagneticModuleCompatible": false,
3030
"loadName": "opentrons_flex_lid_absorbance_plate_reader_module"

0 commit comments

Comments
 (0)