Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): Fix liquid probes always causing opentrons_simulate to raise MustHomeError #15900

Merged
merged 4 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ async def create_simulating_orchestrator(
robot_type=robot_type
)

# TODO(mc, 2021-08-25): move initial home to protocol engine
# TODO(mm, 2024-08-06): This home has theoretically been replaced by Protocol Engine
# `home` commands within the `RunOrchestrator` or `ProtocolRunner`. However, it turns
# out that this `HardwareControlAPI`-level home is accidentally load-bearing,
# working around Protocol Engine bugs where *both* layers need to be homed for
# certain commands to work. https://opentrons.atlassian.net/browse/EXEC-646
await simulating_hardware_api.home()

protocol_engine = await create_protocol_engine(
Expand Down
13 changes: 13 additions & 0 deletions api/src/opentrons/simulate.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,12 @@ def get_protocol_api(
# Intentional difference from execute.get_protocol_api():
# For the caller's convenience, we home the virtual hardware so they don't get MustHomeErrors.
# Since this hardware is virtual, there's no harm in commanding this "movement" implicitly.
#
# Calling `checked_hardware_sync.home()` is a hack. It ought to be redundant with
# `context.home()`. We need it here to work around a Protocol Engine simulation bug
# where both the `HardwareControlAPI` level and the `ProtocolEngine` level need to
# be homed for certain commands to work. https://opentrons.atlassian.net/browse/EXEC-646
checked_hardware.sync.home()
context.home()

return context
Expand Down Expand Up @@ -936,6 +942,13 @@ async def run(protocol_source: ProtocolSource) -> _SimulateResult:
),
)

# TODO(mm, 2024-08-06): This home is theoretically redundant with Protocol
# Engine `home` commands within the `RunOrchestrator`. However, we need this to
# work around Protocol Engine bugs where both the `HardwareControlAPI` level
# and the `ProtocolEngine` level need to be homed for certain commands to work.
# https://opentrons.atlassian.net/browse/EXEC-646
await hardware_api_wrapped.home()

scraper = _CommandScraper(stack_logger, log_level, protocol_runner.broker)
with scraper.scrape():
result = await orchestrator.run(
Expand Down
39 changes: 39 additions & 0 deletions api/tests/opentrons/test_simulate.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,45 @@ def test_get_protocol_api_usable_without_homing(api_version: APIVersion) -> None
pipette.pick_up_tip(tip_rack["A1"]) # Should not raise.


def test_liquid_probe_get_protocol_api() -> None:
"""Covers `simulate.get_protocol_api()`-specific issues with liquid probes.

See https://opentrons.atlassian.net/browse/EXEC-646.
"""
protocol = simulate.get_protocol_api(version="2.20", robot_type="Flex")
pipette = protocol.load_instrument("flex_1channel_1000", mount="left")
tip_rack = protocol.load_labware("opentrons_flex_96_tiprack_1000ul", "A1")
well_plate = protocol.load_labware(
"opentrons_96_wellplate_200ul_pcr_full_skirt", "A2"
)
pipette.pick_up_tip(tip_rack["A1"])
pipette.require_liquid_presence(well_plate["A1"]) # Should not raise MustHomeError.


def test_liquid_probe_simulate_file() -> None:
"""Covers `opentrons_simulate`-specific issues with liquid probes.

See https://opentrons.atlassian.net/browse/EXEC-646.
"""
protocol_contents = textwrap.dedent(
"""\
requirements = {"robotType": "Flex", "apiLevel": "2.20"}
def run(protocol):
pipette = protocol.load_instrument("flex_1channel_1000", mount="left")
tip_rack = protocol.load_labware("opentrons_flex_96_tiprack_1000ul", "A1")
well_plate = protocol.load_labware(
"opentrons_96_wellplate_200ul_pcr_full_skirt", "A2"
)
pipette.pick_up_tip(tip_rack["A1"])
pipette.require_liquid_presence(well_plate["A1"])
"""
)
protocol_contents_stream = io.StringIO(protocol_contents)
simulate.simulate(
protocol_file=protocol_contents_stream
) # Should not raise MustHomeError.


class TestGetProtocolAPILabware:
"""Tests for making sure get_protocol_api() handles extra labware correctly."""

Expand Down
5 changes: 5 additions & 0 deletions hardware-testing/hardware_testing/gravimetric/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ async def _thread_manager_build_hw_api(
extra_labware=extra_labware,
hardware_simulator=ThreadManager(_thread_manager_build_hw_api),
robot_type="Flex",
# use_virtual_hardware=False makes this simulation work unlike
# opentrons_simulate, app-side analysis, and server-side analysis.
# We need to do this because some of our hardware testing scripts still
# interact directly with the OT3API and there is no way to tell Protocol
# Engine's hardware virtualization about those updates.
use_virtual_hardware=False,
)
else:
Expand Down
Loading