Skip to content

Commit c8d6d3b

Browse files
authored
refactor(api): check both TOF sensors before raising error when retrieve labware from stacker (#19123)
This pull request refactors the `dispense_labware` logic in the Flex Stacker module to improve labware detection. ## Changelog 1. temporarily disables `verify_hopper_presence` call until we implement further calibration function for the TOF sensors. 2. directly calls `labware_detected` on the Z axis, hold on to the result and use it only if the X TOF sensor fails to detect a labware after retrieving. 3. updates tests to ensure all error scenarios are properly handled and validated.
1 parent 6ede00e commit c8d6d3b

File tree

2 files changed

+108
-7
lines changed

2 files changed

+108
-7
lines changed

api/src/opentrons/hardware_control/modules/flex_stacker.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,12 @@ async def dispense_labware(
534534
await self._prepare_for_action()
535535

536536
if enforce_hopper_lw_sensing:
537-
await self.verify_hopper_labware_presence(Direction.EXTEND, True)
537+
# TODO: re-enable this function after TOF calibration is implemented.
538+
# Until then, we should also check the TOF X sensor before raising the error
539+
# await self.verify_hopper_labware_presence(Direction.EXTEND, True)
540+
hopper_empty = not await self.labware_detected(
541+
StackerAxis.Z, Direction.EXTEND
542+
)
538543

539544
# Move platform along the X and make sure we DONT detect labware
540545
await self._move_and_home_axis(StackerAxis.X, Direction.RETRACT, HOME_OFFSET_MD)
@@ -556,7 +561,22 @@ async def dispense_labware(
556561
await self.home_axis(StackerAxis.Z, Direction.RETRACT)
557562

558563
if enforce_shuttle_lw_sensing:
559-
await self.verify_shuttle_labware_presence(Direction.RETRACT, True)
564+
try:
565+
await self.verify_shuttle_labware_presence(Direction.RETRACT, True)
566+
except FlexStackerShuttleLabwareError:
567+
# No labware detected on the shuttle, so we need to check what the Z TOF
568+
# sensor says about the hopper
569+
if hopper_empty:
570+
# homing here so we don't have to modify the error recovery flow
571+
await self._move_and_home_axis(
572+
StackerAxis.X, Direction.EXTEND, HOME_OFFSET_MD
573+
)
574+
raise FlexStackerHopperLabwareError(
575+
self.device_info["serial"],
576+
labware_expected=True,
577+
) from None
578+
raise
579+
560580
await self._move_and_home_axis(StackerAxis.X, Direction.EXTEND, HOME_OFFSET_MD)
561581

562582
async def store_labware(

api/tests/opentrons/hardware_control/modules/test_hc_flexstacker.py

Lines changed: 86 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import asyncio
22
import pytest
33
import mock
4-
from typing import AsyncGenerator
4+
from contextlib import nullcontext as does_not_raise
5+
from typing import AsyncGenerator, ContextManager, Any
56
from opentrons.drivers.flex_stacker.simulator import SimulatingDriver
67
from opentrons.drivers.flex_stacker.types import (
78
Direction,
@@ -26,6 +27,11 @@
2627
from opentrons.hardware_control.modules.types import PlatformState
2728
from opentrons.hardware_control.poller import Poller
2829
from opentrons.hardware_control.types import StatusBarState, StatusBarUpdateEvent
30+
from opentrons_shared_data.errors.exceptions import (
31+
FlexStackerShuttleLabwareError,
32+
FlexStackerHopperLabwareError,
33+
FlexStackerShuttleNotEmptyError,
34+
)
2935

3036

3137
@pytest.fixture
@@ -401,31 +407,38 @@ async def test_dispense_labware_motion_sequence(
401407
subject, "_move_and_home_axis", mock.AsyncMock()
402408
) as _move_and_home_axis,
403409
mock.patch.object(
404-
subject, "verify_shuttle_labware_presence", mock.AsyncMock()
410+
subject, "labware_detected", mock.AsyncMock()
411+
) as labware_detected,
412+
mock.patch.object(
413+
subject,
414+
"verify_shuttle_labware_presence",
415+
autospec=True,
416+
side_effect=subject.verify_shuttle_labware_presence,
405417
) as verify_shuttle_labware_presence,
406418
mock.patch.object(
407-
subject, "verify_hopper_labware_presence", mock.AsyncMock()
419+
subject, "verify_hopper_labware_presence", autospec=True
408420
) as verify_hopper_labware_presence,
409421
mock.patch.object(subject, "move_axis", mock.AsyncMock()) as move_axis,
410422
mock.patch.object(subject, "home_axis", mock.AsyncMock()) as home_axis,
411423
mock.patch.object(subject, "open_latch", mock.AsyncMock()) as open_latch,
412424
mock.patch.object(subject, "close_latch", mock.AsyncMock()) as close_latch,
413425
):
426+
labware_detected.side_effect = [True, False, True]
414427
# Test valid labware height
415428
await subject.dispense_labware(
416429
labware_height=labware_height,
417430
)
418431

419432
# We need to verify the move sequence
420-
verify_hopper_labware_presence.assert_called_once_with(Direction.EXTEND, True)
433+
# verify_hopper_labware_presence.assert_called_once_with(Direction.EXTEND, True)
434+
verify_hopper_labware_presence.assert_not_called()
421435
_prepare_for_action.assert_called()
422436

423437
_move_and_home_axis.assert_any_call(
424438
StackerAxis.X, Direction.RETRACT, HOME_OFFSET_MD
425439
)
426440
# Verify labware presence
427441
verify_shuttle_labware_presence.assert_any_call(Direction.RETRACT, False)
428-
429442
_move_and_home_axis.assert_any_call(
430443
StackerAxis.Z, Direction.EXTEND, HOME_OFFSET_SM
431444
)
@@ -452,9 +465,77 @@ async def test_dispense_labware_motion_sequence(
452465
)
453466

454467
# Make sure labware presense check on the X retract was called twice
468+
labware_detected.assert_has_calls(
469+
[
470+
mock.call(StackerAxis.Z, Direction.EXTEND),
471+
mock.call(StackerAxis.X, Direction.RETRACT),
472+
mock.call(StackerAxis.X, Direction.RETRACT),
473+
]
474+
)
455475
assert verify_shuttle_labware_presence.call_count == 2
456476

457477

478+
@pytest.mark.parametrize("hopper_lw_detected", [True, False])
479+
@pytest.mark.parametrize("shuttle_lw_detected_before", [True, False])
480+
@pytest.mark.parametrize("shuttle_lw_detected_after", [True, False])
481+
async def test_dispense_labware_error_handling(
482+
subject: modules.FlexStacker,
483+
hopper_lw_detected: bool,
484+
shuttle_lw_detected_before: bool,
485+
shuttle_lw_detected_after: bool,
486+
) -> None:
487+
"""
488+
Test different error handling scenarios when dispensing labware.
489+
"""
490+
expected_raise: ContextManager[Any]
491+
if shuttle_lw_detected_before is True:
492+
# If the shuttle is occupied, it should always raise an error
493+
expected_raise = pytest.raises(FlexStackerShuttleNotEmptyError)
494+
elif shuttle_lw_detected_after is True:
495+
# if the shuttle has labware after dispensing, no need to raise any error
496+
expected_raise = does_not_raise()
497+
elif hopper_lw_detected is False:
498+
# if the shuttle has no labware after dispensing, and the hopper has no labware,
499+
# it should raise a FlexStackerHopperLabwareError
500+
expected_raise = pytest.raises(FlexStackerHopperLabwareError)
501+
else:
502+
# if the shuttle has no labware after dispensing, but the hopper has labware,
503+
# it should raise a FlexStackerShuttleLabwareError letting user know
504+
# the labware latch is properly jammed
505+
expected_raise = pytest.raises(FlexStackerShuttleLabwareError)
506+
507+
with (
508+
mock.patch.object(
509+
subject,
510+
"labware_detected",
511+
side_effect=[
512+
hopper_lw_detected,
513+
shuttle_lw_detected_before,
514+
shuttle_lw_detected_after,
515+
],
516+
autospec=True,
517+
) as labware_detected,
518+
):
519+
520+
with expected_raise:
521+
# Test valid labware height
522+
await subject.dispense_labware(
523+
labware_height=100.0,
524+
)
525+
526+
expected_calls = [
527+
mock.call(StackerAxis.Z, Direction.EXTEND),
528+
mock.call(StackerAxis.X, Direction.RETRACT),
529+
mock.call(StackerAxis.X, Direction.RETRACT),
530+
]
531+
if shuttle_lw_detected_before:
532+
assert labware_detected.call_count == 2
533+
labware_detected.assert_has_calls(expected_calls[:2])
534+
else:
535+
assert labware_detected.call_count == 3
536+
labware_detected.assert_has_calls(expected_calls)
537+
538+
458539
@pytest.mark.parametrize(
459540
("labware_height"),
460541
[(0), (-10), (200)],

0 commit comments

Comments
 (0)