From 91a75a38becc20c09520ca288bbcccc0c737bcd8 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 22 Apr 2024 15:51:33 -0400 Subject: [PATCH 1/5] Some debug logs. --- .../hardware_control/instruments/ot2/pipette_handler.py | 3 +++ .../protocol_engine/actions/action_dispatcher.py | 9 +++++++++ .../opentrons/protocol_engine/state/command_history.py | 2 ++ 3 files changed, 14 insertions(+) diff --git a/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py b/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py index 35b38a1732a..ff8dbf2b8ef 100644 --- a/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py +++ b/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py @@ -413,6 +413,8 @@ async def reset_nozzle_configuration(self, mount: MountType) -> None: instr.reset_nozzle_configuration() async def add_tip(self, mount: MountType, tip_length: float) -> None: + # breakpoint() + self._ihp_log.warning("attach tip") instr = self._attached_instruments[mount] attached = self.attached_instruments instr_dict = attached[mount] @@ -428,6 +430,7 @@ async def add_tip(self, mount: MountType, tip_length: float) -> None: ) async def remove_tip(self, mount: MountType) -> None: + self._ihp_log.warning("detach tip") instr = self._attached_instruments[mount] attached = self.attached_instruments instr_dict = attached[mount] diff --git a/api/src/opentrons/protocol_engine/actions/action_dispatcher.py b/api/src/opentrons/protocol_engine/actions/action_dispatcher.py index df045ec4250..1addd0ea725 100644 --- a/api/src/opentrons/protocol_engine/actions/action_dispatcher.py +++ b/api/src/opentrons/protocol_engine/actions/action_dispatcher.py @@ -5,6 +5,12 @@ from .actions import Action +import logging + + +_log = logging.getLogger(__name__) + + class ActionDispatcher: """A pipeline, with an endpoint, that actions can be dispatched into.""" @@ -24,6 +30,9 @@ def add_handler(self, handler: ActionHandler) -> None: def dispatch(self, action: Action) -> None: """Dispatch an action into the pipeline.""" + + _log.warning(f"Dispatching action {action}") + for handler in self._handlers: handler.handle_action(action) diff --git a/api/src/opentrons/protocol_engine/state/command_history.py b/api/src/opentrons/protocol_engine/state/command_history.py index 6a66a2b8209..ededa5a225a 100644 --- a/api/src/opentrons/protocol_engine/state/command_history.py +++ b/api/src/opentrons/protocol_engine/state/command_history.py @@ -172,6 +172,8 @@ def set_command_running(self, command: Command) -> None: CommandEntry(index=prev_entry.index, command=command), ) + if not self.get_running_command() is None: + breakpoint() assert self.get_running_command() is None self._set_running_command_id(command.id) From 157d2026824e60855d8f33cd391bbabdfecd4e0c Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 23 Apr 2024 11:10:32 -0400 Subject: [PATCH 2/5] Revert "Some debug logs." This reverts commit 91a75a38becc20c09520ca288bbcccc0c737bcd8. --- .../hardware_control/instruments/ot2/pipette_handler.py | 3 --- .../protocol_engine/actions/action_dispatcher.py | 9 --------- .../opentrons/protocol_engine/state/command_history.py | 2 -- 3 files changed, 14 deletions(-) diff --git a/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py b/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py index ff8dbf2b8ef..35b38a1732a 100644 --- a/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py +++ b/api/src/opentrons/hardware_control/instruments/ot2/pipette_handler.py @@ -413,8 +413,6 @@ async def reset_nozzle_configuration(self, mount: MountType) -> None: instr.reset_nozzle_configuration() async def add_tip(self, mount: MountType, tip_length: float) -> None: - # breakpoint() - self._ihp_log.warning("attach tip") instr = self._attached_instruments[mount] attached = self.attached_instruments instr_dict = attached[mount] @@ -430,7 +428,6 @@ async def add_tip(self, mount: MountType, tip_length: float) -> None: ) async def remove_tip(self, mount: MountType) -> None: - self._ihp_log.warning("detach tip") instr = self._attached_instruments[mount] attached = self.attached_instruments instr_dict = attached[mount] diff --git a/api/src/opentrons/protocol_engine/actions/action_dispatcher.py b/api/src/opentrons/protocol_engine/actions/action_dispatcher.py index 1addd0ea725..df045ec4250 100644 --- a/api/src/opentrons/protocol_engine/actions/action_dispatcher.py +++ b/api/src/opentrons/protocol_engine/actions/action_dispatcher.py @@ -5,12 +5,6 @@ from .actions import Action -import logging - - -_log = logging.getLogger(__name__) - - class ActionDispatcher: """A pipeline, with an endpoint, that actions can be dispatched into.""" @@ -30,9 +24,6 @@ def add_handler(self, handler: ActionHandler) -> None: def dispatch(self, action: Action) -> None: """Dispatch an action into the pipeline.""" - - _log.warning(f"Dispatching action {action}") - for handler in self._handlers: handler.handle_action(action) diff --git a/api/src/opentrons/protocol_engine/state/command_history.py b/api/src/opentrons/protocol_engine/state/command_history.py index ededa5a225a..6a66a2b8209 100644 --- a/api/src/opentrons/protocol_engine/state/command_history.py +++ b/api/src/opentrons/protocol_engine/state/command_history.py @@ -172,8 +172,6 @@ def set_command_running(self, command: Command) -> None: CommandEntry(index=prev_entry.index, command=command), ) - if not self.get_running_command() is None: - breakpoint() assert self.get_running_command() is None self._set_running_command_id(command.id) From bdf1afa89ccfd4b2cb9dcd804bb1fcef59201dbc Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 23 Apr 2024 11:10:13 -0400 Subject: [PATCH 3/5] Add a unit test for AIR_GAP. --- .../opentrons/protocol_runner/test_legacy_command_mapper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py index f0412878856..a0581001a82 100644 --- a/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py @@ -579,6 +579,7 @@ def test_map_pause() -> None: "command.DISTRIBUTE", "command.TRANSFER", "command.RETURN_TIP", + "command.AIR_GAP", ], ) def test_filter_higher_order_commands(command_type: str) -> None: From 65808bcff6ab55b0e8f7681d8c6c09caf40c15d2 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 23 Apr 2024 12:18:38 -0400 Subject: [PATCH 4/5] Add smoke test. --- .../smoke_tests/test_legacy_command_mapper.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py b/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py index 5d6595227b9..c8950cbe090 100644 --- a/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py +++ b/api/tests/opentrons/protocol_runner/smoke_tests/test_legacy_command_mapper.py @@ -5,6 +5,7 @@ """ from datetime import datetime from pathlib import Path +from textwrap import dedent from typing import List import pytest @@ -753,3 +754,46 @@ async def test_zero_volume_dispense_commands( labwareId=load_well_plate.result.labwareId, wellName="D7", ) + + +async def test_air_gap(tmp_path: Path) -> None: + """An `air_gap()` should be mapped to an `aspirate`. + + This covers RQA-2621. + """ + path = tmp_path / "protocol.py" + path.write_text( + dedent( + """\ + metadata = {"apiLevel": "2.13"} + def run(protocol): + # Prep: + tip_rack = protocol.load_labware("opentrons_96_tiprack_300ul", 1) + well_plate = protocol.load_labware("biorad_96_wellplate_200ul_pcr", 2) + pipette = protocol.load_instrument("p300_single_gen2", mount="left", tip_racks=[tip_rack]) + pipette.pick_up_tip() + + # Test: + pipette.move_to(well_plate["A1"].top()) + pipette.air_gap(100) + """ + ) + ) + result_commands = await simulate_and_get_commands(path) + [ + initial_home, + load_tip_rack, + load_well_plate, + load_pipette, + pick_up_tip, + move_to_well, + air_gap_aspirate, + ] = result_commands + assert isinstance(initial_home, commands.Home) + assert isinstance(load_tip_rack, commands.LoadLabware) + assert isinstance(load_well_plate, commands.LoadLabware) + assert isinstance(load_pipette, commands.LoadPipette) + assert isinstance(pick_up_tip, commands.PickUpTip) + # TODO(mm, 2024-04-23): This commands.Custom looks wrong. This should be a commands.MoveToWell. + assert isinstance(move_to_well, commands.Custom) + assert isinstance(air_gap_aspirate, commands.Aspirate) From ad6c3f01db761db5159b5dc63e9cf0c08aa735bc Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Tue, 23 Apr 2024 11:10:17 -0400 Subject: [PATCH 5/5] Add AIR_GAP to the higher-order exclusion list. --- api/src/opentrons/protocol_runner/legacy_command_mapper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/opentrons/protocol_runner/legacy_command_mapper.py b/api/src/opentrons/protocol_runner/legacy_command_mapper.py index e835a6af8e6..9243f50f70d 100644 --- a/api/src/opentrons/protocol_runner/legacy_command_mapper.py +++ b/api/src/opentrons/protocol_runner/legacy_command_mapper.py @@ -79,6 +79,7 @@ def __init__(self, wrapping_exc: BaseException) -> None: legacy_command_types.DISTRIBUTE, legacy_command_types.TRANSFER, legacy_command_types.RETURN_TIP, + legacy_command_types.AIR_GAP, }