From 58340e4ee2f0567fbd008c7aaf8771fb0e929379 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Thu, 27 Apr 2023 15:07:51 -0400 Subject: [PATCH 01/14] return proper deck slot name based on robot type --- .../protocol_api/core/engine/module_core.py | 7 +++++++ .../protocol_api/core/engine/protocol.py | 11 +++++++++-- .../core/legacy/legacy_module_core.py | 3 +++ .../core/legacy/legacy_protocol_core.py | 7 ++++++- api/src/opentrons/protocol_api/core/module.py | 4 ++++ api/src/opentrons/protocol_api/core/protocol.py | 8 +++++++- api/src/opentrons/protocol_api/deck.py | 15 +++++++++++++-- api/src/opentrons/protocol_api/module_contexts.py | 2 +- .../opentrons/protocol_api/test_module_context.py | 6 +----- .../opentrons/protocol_api/test_validation.py | 2 +- 10 files changed, 52 insertions(+), 13 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/module_core.py b/api/src/opentrons/protocol_api/core/engine/module_core.py index f7af76460de..e8e8d77e61b 100644 --- a/api/src/opentrons/protocol_api/core/engine/module_core.py +++ b/api/src/opentrons/protocol_api/core/engine/module_core.py @@ -78,6 +78,13 @@ def get_deck_slot(self) -> DeckSlotName: """Get the module's deck slot.""" return self._engine_client.state.modules.get_location(self.module_id).slotName + def get_deck_slot_display_name(self) -> str: + slot_name = self.get_deck_slot() + if self._engine_client.state.config.robot_type == "OT-2 Standard": + return str(slot_name) + else: + return slot_name.as_coordinate() + def get_display_name(self) -> str: """Get the module's display name.""" return self._engine_client.state.modules.get_definition( diff --git a/api/src/opentrons/protocol_api/core/engine/protocol.py b/api/src/opentrons/protocol_api/core/engine/protocol.py index f1a3c753796..3636b5d91e7 100644 --- a/api/src/opentrons/protocol_api/core/engine/protocol.py +++ b/api/src/opentrons/protocol_api/core/engine/protocol.py @@ -73,6 +73,10 @@ def api_version(self) -> APIVersion: """Get the api version protocol target.""" return self._api_version + @property + def robot_type(self) -> RobotType: + return self._engine_client.state.config.robot_type + @property def fixed_trash(self) -> LabwareCore: """Get the fixed trash labware.""" @@ -436,13 +440,16 @@ def define_liquid( def get_labware_location( self, labware_core: LabwareCore - ) -> Union[DeckSlotName, ModuleCore, None]: + ) -> Union[str, ModuleCore, None]: """Get labware parent location.""" labware_location = self._engine_client.state.labware.get_location( labware_core.labware_id ) if isinstance(labware_location, DeckSlotLocation): - return labware_location.slotName + if self._engine_client.state.config.robot_type == "OT-2 Standard": + return str(labware_location.slotName) + else: + return labware_location.slotName.as_coordinate() elif isinstance(labware_location, ModuleLocation): return self._module_cores_by_id.get(labware_location.moduleId) return None diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py index 67b9b1b4c06..1b90d98e926 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py @@ -87,6 +87,9 @@ def get_deck_slot(self) -> DeckSlotName: """Get the module's deck slot.""" return DeckSlotName.from_primitive(self._geometry.parent) # type: ignore[arg-type] + def get_deck_slot_display_name(self) -> str: + return str(self.get_deck_slot()) + def get_display_name(self) -> str: """Get the module's display name.""" return self._geometry.display_name diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py index 389516933aa..081fc76bb02 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py @@ -4,6 +4,7 @@ from opentrons_shared_data.deck.dev_types import DeckDefinitionV3 from opentrons_shared_data.labware.dev_types import LabwareDefinition from opentrons_shared_data.pipette.dev_types import PipetteNameType +from opentrons_shared_data.robot.dev_types import RobotType from opentrons.types import DeckSlotName, Location, Mount, Point from opentrons.equipment_broker import EquipmentBroker @@ -90,6 +91,10 @@ def api_version(self) -> APIVersion: """Get the API version the protocol is adhering to.""" return self._api_version + @property + def robot_type(self) -> RobotType: + return "OT-2 Standard" + @property def equipment_broker(self) -> EquipmentBroker[LoadInfo]: """A message broker to to publish equipment load events. @@ -437,6 +442,6 @@ def define_liquid( def get_labware_location( self, labware_core: LegacyLabwareCore - ) -> Union[DeckSlotName, legacy_module_core.LegacyModuleCore, None]: + ) -> Union[str, legacy_module_core.LegacyModuleCore, None]: """Get labware parent location.""" assert False, "get_labware_location only supported on engine core" diff --git a/api/src/opentrons/protocol_api/core/module.py b/api/src/opentrons/protocol_api/core/module.py index 7e4a4807ba8..9802e5f2148 100644 --- a/api/src/opentrons/protocol_api/core/module.py +++ b/api/src/opentrons/protocol_api/core/module.py @@ -36,6 +36,10 @@ def get_serial_number(self) -> str: def get_deck_slot(self) -> DeckSlotName: """Get the module's deck slot.""" + @abstractmethod + def get_deck_slot_display_name(self) -> str: + """Get the module's deck slot in a robot accurate format.""" + def get_display_name(self) -> str: """Get the module's display name.""" diff --git a/api/src/opentrons/protocol_api/core/protocol.py b/api/src/opentrons/protocol_api/core/protocol.py index 335c76fafc6..824730b2d79 100644 --- a/api/src/opentrons/protocol_api/core/protocol.py +++ b/api/src/opentrons/protocol_api/core/protocol.py @@ -8,6 +8,7 @@ from opentrons_shared_data.deck.dev_types import DeckDefinitionV3 from opentrons_shared_data.pipette.dev_types import PipetteNameType from opentrons_shared_data.labware.dev_types import LabwareDefinition +from opentrons_shared_data.robot.dev_types import RobotType from opentrons.types import DeckSlotName, Location, Mount, Point from opentrons.hardware_control import SyncHardwareAPI @@ -29,6 +30,11 @@ def fixed_trash(self) -> LabwareCoreType: """Get the fixed trash labware core.""" ... + @property + @abstractmethod + def robot_type(self) -> RobotType: + ... + @abstractmethod def get_max_speeds(self) -> AxisMaxSpeeds: ... @@ -174,5 +180,5 @@ def define_liquid( @abstractmethod def get_labware_location( self, labware_core: LabwareCoreType - ) -> Union[DeckSlotName, ModuleCoreType, None]: + ) -> Union[str, ModuleCoreType, None]: """Get labware parent location.""" diff --git a/api/src/opentrons/protocol_api/deck.py b/api/src/opentrons/protocol_api/deck.py index 39c9a9ea89d..a76d0fa050c 100644 --- a/api/src/opentrons/protocol_api/deck.py +++ b/api/src/opentrons/protocol_api/deck.py @@ -3,6 +3,7 @@ from typing import Iterator, List, Mapping, Optional, Tuple, Union from opentrons_shared_data.deck.dev_types import SlotDefV3 +from opentrons_shared_data.robot.dev_types import RobotType from opentrons.motion_planning import adjacent_slots_getters from opentrons.types import DeckLocation, DeckSlotName, Location, Point @@ -31,6 +32,16 @@ class CalibrationPosition: displayName: str +def _get_robot_canonical_slot_name( + slot_key: DeckLocation, robot_type: RobotType +) -> str: + deck_slot_name = _get_slot_name(slot_key) + if robot_type == "OT-2 Standard": + return str(deck_slot_name) + else: + return deck_slot_name.as_coordinate() + + def _get_slot_name(slot_key: DeckLocation) -> DeckSlotName: try: return validation.ensure_deck_slot(slot_key) @@ -105,8 +116,8 @@ def position_for(self, slot: DeckLocation) -> Location: def get_slot_definition(self, slot: DeckLocation) -> SlotDefV3: """Get the geometric definition data of a slot.""" - slot_name = _get_slot_name(slot) - return self._slot_definitions_by_name[slot_name.value] + slot_name = _get_robot_canonical_slot_name(slot, self._protocol_core.robot_type) + return self._slot_definitions_by_name[slot_name] def get_slot_center(self, slot: DeckLocation) -> Point: """Get the absolute coordinates of a slot's center.""" diff --git a/api/src/opentrons/protocol_api/module_contexts.py b/api/src/opentrons/protocol_api/module_contexts.py index 0d6d80abfa4..a4c98514e13 100644 --- a/api/src/opentrons/protocol_api/module_contexts.py +++ b/api/src/opentrons/protocol_api/module_contexts.py @@ -214,7 +214,7 @@ def labware(self) -> Optional[Labware]: @requires_version(2, 14) def parent(self) -> str: """The name of the slot the module is on.""" - return self._core.get_deck_slot().value + return self._core.get_deck_slot_display_name() @property # type: ignore[misc] @requires_version(2, 0) diff --git a/api/tests/opentrons/protocol_api/test_module_context.py b/api/tests/opentrons/protocol_api/test_module_context.py index d2bd9b29fdf..44b089161c4 100644 --- a/api/tests/opentrons/protocol_api/test_module_context.py +++ b/api/tests/opentrons/protocol_api/test_module_context.py @@ -8,7 +8,6 @@ from opentrons.hardware_control.modules.types import ModuleType, HeaterShakerModuleModel from opentrons.broker import Broker -from opentrons.types import DeckSlotName from opentrons.protocols.api_support.types import APIVersion from opentrons.protocol_api import MAX_SUPPORTED_VERSION, ModuleContext, Labware from opentrons.protocol_api.core.common import LabwareCore, ModuleCore, ProtocolCore @@ -166,10 +165,7 @@ def test_load_labware_from_definition( def test_parent(decoy: Decoy, mock_core: ModuleCore, subject: ModuleContext) -> None: """Should get the parent slot name.""" - decoy.when(mock_core.get_deck_slot()).then_return(DeckSlotName.SLOT_1) - - # TODO (tz, 1-17-23): make sure that the subject is not returning an Enum that subclasses str - assert type(subject.parent) == str + decoy.when(mock_core.get_deck_slot_display_name()).then_return("1") assert subject.parent == "1" diff --git a/api/tests/opentrons/protocol_api/test_validation.py b/api/tests/opentrons/protocol_api/test_validation.py index b54dbebaae7..ac5effe70bb 100644 --- a/api/tests/opentrons/protocol_api/test_validation.py +++ b/api/tests/opentrons/protocol_api/test_validation.py @@ -82,7 +82,7 @@ def test_ensure_deck_slot(input_value: Union[str, int], expected: DeckSlotName) def test_ensure_deck_slot_invalid() -> None: """It should raise a ValueError if given an invalid name.""" - input_values: List[Union[str, int]] = ["0", 0, "13", 13] + input_values: List[Union[str, int]] = ["0", 0, "13", 13, "b7", "B7"] for input_value in input_values: with pytest.raises(ValueError, match="not a valid deck slot"): From 45088eb7b6973a12a826e452baf0cb5e41faf78a Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 28 Apr 2023 10:03:17 -0400 Subject: [PATCH 02/14] convert to deck string with validation module --- .../protocol_api/core/engine/module_core.py | 8 ++++---- .../protocol_api/core/engine/protocol.py | 9 +++++---- api/src/opentrons/protocol_api/deck.py | 15 +++------------ api/src/opentrons/protocol_api/validation.py | 8 ++++++++ 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/module_core.py b/api/src/opentrons/protocol_api/core/engine/module_core.py index e8e8d77e61b..75bd6514d24 100644 --- a/api/src/opentrons/protocol_api/core/engine/module_core.py +++ b/api/src/opentrons/protocol_api/core/engine/module_core.py @@ -25,6 +25,7 @@ from opentrons.protocols.api_support.types import APIVersion +from ... import validation from ..module import ( AbstractModuleCore, AbstractTemperatureModuleCore, @@ -80,10 +81,9 @@ def get_deck_slot(self) -> DeckSlotName: def get_deck_slot_display_name(self) -> str: slot_name = self.get_deck_slot() - if self._engine_client.state.config.robot_type == "OT-2 Standard": - return str(slot_name) - else: - return slot_name.as_coordinate() + return validation.ensure_deck_slot_string( + slot_name, robot_type=self._engine_client.state.config.robot_type + ) def get_display_name(self) -> str: """Get the module's display name.""" diff --git a/api/src/opentrons/protocol_api/core/engine/protocol.py b/api/src/opentrons/protocol_api/core/engine/protocol.py index 3636b5d91e7..2c9f7e16025 100644 --- a/api/src/opentrons/protocol_api/core/engine/protocol.py +++ b/api/src/opentrons/protocol_api/core/engine/protocol.py @@ -6,6 +6,7 @@ from opentrons_shared_data.labware.labware_definition import LabwareDefinition from opentrons_shared_data.labware.dev_types import LabwareDefinition as LabwareDefDict from opentrons_shared_data.pipette.dev_types import PipetteNameType +from opentrons_shared_data.robot.dev_types import RobotType from opentrons.types import DeckSlotName, Location, Mount, MountType, Point from opentrons.hardware_control import SyncHardwareAPI, SynchronousAdapter @@ -27,6 +28,7 @@ from opentrons.protocol_engine.clients import SyncClient as ProtocolEngineClient from opentrons.protocol_engine.errors import LabwareNotLoadedOnModuleError +from ... import validation from ..._liquid import Liquid from ..protocol import AbstractProtocol from ..labware import LabwareLoadParams @@ -446,10 +448,9 @@ def get_labware_location( labware_core.labware_id ) if isinstance(labware_location, DeckSlotLocation): - if self._engine_client.state.config.robot_type == "OT-2 Standard": - return str(labware_location.slotName) - else: - return labware_location.slotName.as_coordinate() + return validation.ensure_deck_slot_string( + labware_location.slotName, self._engine_client.state.config.robot_type + ) elif isinstance(labware_location, ModuleLocation): return self._module_cores_by_id.get(labware_location.moduleId) return None diff --git a/api/src/opentrons/protocol_api/deck.py b/api/src/opentrons/protocol_api/deck.py index a76d0fa050c..b547ad85dc7 100644 --- a/api/src/opentrons/protocol_api/deck.py +++ b/api/src/opentrons/protocol_api/deck.py @@ -3,7 +3,6 @@ from typing import Iterator, List, Mapping, Optional, Tuple, Union from opentrons_shared_data.deck.dev_types import SlotDefV3 -from opentrons_shared_data.robot.dev_types import RobotType from opentrons.motion_planning import adjacent_slots_getters from opentrons.types import DeckLocation, DeckSlotName, Location, Point @@ -32,16 +31,6 @@ class CalibrationPosition: displayName: str -def _get_robot_canonical_slot_name( - slot_key: DeckLocation, robot_type: RobotType -) -> str: - deck_slot_name = _get_slot_name(slot_key) - if robot_type == "OT-2 Standard": - return str(deck_slot_name) - else: - return deck_slot_name.as_coordinate() - - def _get_slot_name(slot_key: DeckLocation) -> DeckSlotName: try: return validation.ensure_deck_slot(slot_key) @@ -116,7 +105,9 @@ def position_for(self, slot: DeckLocation) -> Location: def get_slot_definition(self, slot: DeckLocation) -> SlotDefV3: """Get the geometric definition data of a slot.""" - slot_name = _get_robot_canonical_slot_name(slot, self._protocol_core.robot_type) + slot_name = validation.ensure_deck_slot_string( + _get_slot_name(slot), self._protocol_core.robot_type + ) return self._slot_definitions_by_name[slot_name] def get_slot_center(self, slot: DeckLocation) -> Point: diff --git a/api/src/opentrons/protocol_api/validation.py b/api/src/opentrons/protocol_api/validation.py index aa1225df60d..175cb678839 100644 --- a/api/src/opentrons/protocol_api/validation.py +++ b/api/src/opentrons/protocol_api/validation.py @@ -15,6 +15,7 @@ from typing_extensions import TypeGuard from opentrons_shared_data.pipette.dev_types import PipetteNameType +from opentrons_shared_data.robot.dev_types import RobotType from opentrons.types import Mount, DeckSlotName, Location from opentrons.hardware_control.modules.types import ( @@ -76,6 +77,13 @@ def ensure_deck_slot(deck_slot: Union[int, str]) -> DeckSlotName: raise ValueError(f"'{deck_slot}' is not a valid deck slot") from e +def ensure_deck_slot_string(slot_name: DeckSlotName, robot_type: RobotType) -> str: + if robot_type == "OT-2 Standard": + return str(slot_name) + else: + return slot_name.as_coordinate() + + def ensure_lowercase_name(name: str) -> str: """Ensure that a given name string is all lowercase.""" if not isinstance(name, str): From 002bf807102d2d4b0eee6d012433cabe4ca18c45 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 28 Apr 2023 10:04:04 -0400 Subject: [PATCH 03/14] test fixes --- .../core/engine/test_module_core.py | 27 ++++++++++++++- api/tests/opentrons/protocol_api/test_deck.py | 34 +++++++++++++------ 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/api/tests/opentrons/protocol_api/core/engine/test_module_core.py b/api/tests/opentrons/protocol_api/core/engine/test_module_core.py index 9b35179fa7b..b22f2e11196 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_module_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_module_core.py @@ -1,5 +1,6 @@ """Tests for opentrons.protocol_api.core.engine.ModuleCore.""" import pytest +import inspect from decoy import Decoy from opentrons.hardware_control import SynchronousAdapter @@ -7,7 +8,7 @@ from opentrons.protocol_engine import DeckSlotLocation from opentrons.protocol_engine.clients import SyncClient as EngineClient from opentrons.protocol_engine.types import ModuleModel, ModuleDefinition -from opentrons.protocol_api import MAX_SUPPORTED_VERSION +from opentrons.protocol_api import MAX_SUPPORTED_VERSION, validation as mock_validation from opentrons.protocols.api_support.types import APIVersion from opentrons.protocol_api.core.engine.module_core import ModuleCore from opentrons.types import DeckSlotName @@ -31,6 +32,13 @@ def mock_sync_module_hardware(decoy: Decoy) -> SynchronousAdapter[AbstractModule return decoy.mock(name="SynchronousAdapter[AbstractModule]") # type: ignore[no-any-return] +@pytest.fixture(autouse=True) +def _mock_validation_module(decoy: Decoy, monkeypatch: pytest.MonkeyPatch) -> None: + """Mock out opentrons.legacy.validation functions.""" + for name, func in inspect.getmembers(mock_validation, inspect.isfunction): + monkeypatch.setattr(mock_validation, name, decoy.mock(func=func)) + + @pytest.fixture def subject( mock_engine_client: EngineClient, @@ -63,6 +71,23 @@ def test_get_deck_slot( assert subject.get_deck_slot() == DeckSlotName.SLOT_1 +def test_get_deck_slot_display_name( + decoy: Decoy, subject: ModuleCore, mock_engine_client: EngineClient +) -> None: + """It should return the correct deck slot string associated with the module and robot type""" + decoy.when(mock_engine_client.state.modules.get_location("1234")).then_return( + DeckSlotLocation(slotName=DeckSlotName.SLOT_1) + ) + + decoy.when(mock_engine_client.state.config.robot_type).then_return("OT-3 Standard") + + decoy.when( + mock_validation.ensure_deck_slot_string(DeckSlotName.SLOT_1, "OT-3 Standard") + ).then_return("foo") + + assert subject.get_deck_slot_display_name() == "foo" + + def test_get_model( decoy: Decoy, subject: ModuleCore, mock_engine_client: EngineClient ) -> None: diff --git a/api/tests/opentrons/protocol_api/test_deck.py b/api/tests/opentrons/protocol_api/test_deck.py index ae1a7f1b9e2..6ccc52f9b90 100644 --- a/api/tests/opentrons/protocol_api/test_deck.py +++ b/api/tests/opentrons/protocol_api/test_deck.py @@ -137,26 +137,32 @@ def test_slot_keys_iter(subject: Deck) -> None: { "locations": { "orderedSlots": [ - {"id": "1"}, - {"id": "2"}, - {"id": "3"}, + {"id": "fee"}, + {"id": "foe"}, + {"id": "fum"}, ], "calibrationPoints": [], } }, ], ) -def test_get_slots(decoy: Decoy, subject: Deck) -> None: +def test_get_slots( + decoy: Decoy, mock_protocol_core: ProtocolCore, subject: Deck +) -> None: """It should provide slot definitions.""" decoy.when(mock_validation.ensure_deck_slot(222)).then_return(DeckSlotName.SLOT_2) + decoy.when(mock_protocol_core.robot_type).then_return("OT-2 Standard") + decoy.when( + mock_validation.ensure_deck_slot_string(DeckSlotName.SLOT_2, "OT-2 Standard") + ).then_return("fee") assert subject.slots == [ - {"id": "1"}, - {"id": "2"}, - {"id": "3"}, + {"id": "fee"}, + {"id": "foe"}, + {"id": "fum"}, ] - assert subject.get_slot_definition(222) == {"id": "2"} + assert subject.get_slot_definition(222) == {"id": "fee"} @pytest.mark.parametrize( @@ -165,21 +171,27 @@ def test_get_slots(decoy: Decoy, subject: Deck) -> None: { "locations": { "orderedSlots": [ - {"id": "3", "position": [1.0, 2.0, 3.0]}, + {"id": "foo", "position": [1.0, 2.0, 3.0]}, ], "calibrationPoints": [], } }, ], ) -def test_get_position_for(decoy: Decoy, subject: Deck) -> None: +def test_get_position_for( + decoy: Decoy, mock_protocol_core: ProtocolCore, subject: Deck +) -> None: """It should return a `Location` for a deck slot.""" decoy.when(mock_validation.ensure_deck_slot(333)).then_return(DeckSlotName.SLOT_3) + decoy.when(mock_protocol_core.robot_type).then_return("OT-3 Standard") + decoy.when( + mock_validation.ensure_deck_slot_string(DeckSlotName.SLOT_3, "OT-3 Standard") + ).then_return("foo") result = subject.position_for(333) assert result.point == Point(x=1.0, y=2.0, z=3.0) assert result.labware.is_slot is True - assert str(result.labware) == "3" + assert str(result.labware) == "foo" def test_highest_z( From b1fc0554aed3922a8b29ca4b515f2cd6cf917abf Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 28 Apr 2023 13:54:08 -0400 Subject: [PATCH 04/14] more tests and fixes --- .../core/engine/test_module_core.py | 2 +- .../core/engine/test_protocol_core.py | 88 ++++++++++++++++++- .../opentrons/protocol_api/test_labware.py | 12 +-- .../protocol_api/test_module_context.py | 4 +- robot-server/tests/integration/conftest.py | 16 +++- .../test_deck_coordinate_load.tavern.yaml | 3 +- .../protocols/deck_coordinate_load.py | 6 +- 7 files changed, 114 insertions(+), 17 deletions(-) diff --git a/api/tests/opentrons/protocol_api/core/engine/test_module_core.py b/api/tests/opentrons/protocol_api/core/engine/test_module_core.py index b22f2e11196..5605ea46c25 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_module_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_module_core.py @@ -74,7 +74,7 @@ def test_get_deck_slot( def test_get_deck_slot_display_name( decoy: Decoy, subject: ModuleCore, mock_engine_client: EngineClient ) -> None: - """It should return the correct deck slot string associated with the module and robot type""" + """It should return the correct deck slot string associated with the module and robot type.""" decoy.when(mock_engine_client.state.modules.get_location("1234")).then_return( DeckSlotLocation(slotName=DeckSlotName.SLOT_1) ) diff --git a/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py b/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py index 89ad4fcf4b9..470e6ac47d5 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py @@ -58,7 +58,7 @@ ThermocyclerModuleCore, HeaterShakerModuleCore, ) -from opentrons.protocol_api import MAX_SUPPORTED_VERSION +from opentrons.protocol_api import validation, MAX_SUPPORTED_VERSION from opentrons.protocols.api_support.types import APIVersion @@ -67,11 +67,18 @@ def patch_mock_load_labware_params( decoy: Decoy, monkeypatch: pytest.MonkeyPatch ) -> None: - """Mock out point_calculations.py functions.""" + """Mock out load_labware_params.py functions.""" for name, func in inspect.getmembers(load_labware_params, inspect.isfunction): monkeypatch.setattr(load_labware_params, name, decoy.mock(func=func)) +@pytest.fixture(autouse=True) +def patch_mock_validation(decoy: Decoy, monkeypatch: pytest.MonkeyPatch) -> None: + """Mock out validation.py functions.""" + for name, func in inspect.getmembers(validation, inspect.isfunction): + monkeypatch.setattr(validation, name, decoy.mock(func=func)) + + @pytest.fixture(autouse=True) def patch_mock_deck_conflict_check( decoy: Decoy, monkeypatch: pytest.MonkeyPatch @@ -765,3 +772,80 @@ def test_add_liquid( ) assert result == expected_result + + +def test_get_labware_location_deck_slot( + decoy: Decoy, + mock_engine_client: EngineClient, + subject: ProtocolCore, +) -> None: + """It should return the labware location as a deck slot string.""" + decoy.when(mock_engine_client.state.labware.get_definition("abc")).then_return( + LabwareDefinition.construct(ordering=[]) # type: ignore[call-arg] + ) + decoy.when(mock_engine_client.state.labware.get_location("abc")).then_return( + DeckSlotLocation(slotName=DeckSlotName.SLOT_1) + ) + decoy.when(mock_engine_client.state.config.robot_type).then_return("OT-2 Standard") + decoy.when( + validation.ensure_deck_slot_string(DeckSlotName.SLOT_1, "OT-2 Standard") + ).then_return("777") + + labware = LabwareCore( + labware_id="abc", + engine_client=mock_engine_client, + ) + + assert subject.get_labware_location(labware) == "777" + + +def test_get_labware_location_module( + decoy: Decoy, + mock_engine_client: EngineClient, + api_version: APIVersion, + mock_sync_module_hardware: SynchronousAdapter[AbstractModule], + subject: ProtocolCore, +) -> None: + """It should return the labware location as a module.""" + decoy.when(mock_engine_client.state.labware.get_definition("abc")).then_return( + LabwareDefinition.construct(ordering=[]) # type: ignore[call-arg] + ) + decoy.when(mock_engine_client.state.labware.get_location("abc")).then_return( + ModuleLocation(moduleId="123") + ) + + module = ModuleCore( + module_id="module-id", + engine_client=mock_engine_client, + api_version=api_version, + sync_module_hardware=mock_sync_module_hardware, + ) + subject._module_cores_by_id["123"] = module + + labware = LabwareCore( + labware_id="abc", + engine_client=mock_engine_client, + ) + + assert subject.get_labware_location(labware) == module + + +def test_get_labware_location_off_deck( + decoy: Decoy, + mock_engine_client: EngineClient, + subject: ProtocolCore, +) -> None: + """It should return the labware location as None to represent an off deck labware.""" + decoy.when(mock_engine_client.state.labware.get_definition("abc")).then_return( + LabwareDefinition.construct(ordering=[]) # type: ignore[call-arg] + ) + decoy.when(mock_engine_client.state.labware.get_location("abc")).then_return( + "offDeck" + ) + + labware = LabwareCore( + labware_id="abc", + engine_client=mock_engine_client, + ) + + assert subject.get_labware_location(labware) is None diff --git a/api/tests/opentrons/protocol_api/test_labware.py b/api/tests/opentrons/protocol_api/test_labware.py index b8dc5840c63..d0104e3d231 100644 --- a/api/tests/opentrons/protocol_api/test_labware.py +++ b/api/tests/opentrons/protocol_api/test_labware.py @@ -155,24 +155,18 @@ def test_reset_tips( decoy.verify(mock_labware_core.reset_tips(), times=1) -@pytest.mark.parametrize( - "labware_location, expected_result", - [(DeckSlotName.SLOT_1, "1"), (None, None)], -) def test_parent_slot( decoy: Decoy, subject: Labware, mock_labware_core: LabwareCore, mock_protocol_core: ProtocolCore, - labware_location: Union[None, DeckSlotName], - expected_result: Optional[str], ) -> None: """Should get the labware's parent slot name or None.""" decoy.when(mock_protocol_core.get_labware_location(mock_labware_core)).then_return( - labware_location + "bloop" ) - subject.parent == expected_result + assert subject.parent == "bloop" def test_parent_module_context( @@ -194,7 +188,7 @@ def test_parent_module_context( mock_temp_module_context ) - subject.parent == mock_temp_module_context + assert subject.parent == mock_temp_module_context @pytest.mark.parametrize("api_version", [APIVersion(2, 13)]) diff --git a/api/tests/opentrons/protocol_api/test_module_context.py b/api/tests/opentrons/protocol_api/test_module_context.py index 44b089161c4..6f9c2d5f9a7 100644 --- a/api/tests/opentrons/protocol_api/test_module_context.py +++ b/api/tests/opentrons/protocol_api/test_module_context.py @@ -165,8 +165,8 @@ def test_load_labware_from_definition( def test_parent(decoy: Decoy, mock_core: ModuleCore, subject: ModuleContext) -> None: """Should get the parent slot name.""" - decoy.when(mock_core.get_deck_slot_display_name()).then_return("1") - assert subject.parent == "1" + decoy.when(mock_core.get_deck_slot_display_name()).then_return("bar") + assert subject.parent == "bar" def test_module_model( diff --git a/robot-server/tests/integration/conftest.py b/robot-server/tests/integration/conftest.py index a833d64a1a5..67832813b77 100644 --- a/robot-server/tests/integration/conftest.py +++ b/robot-server/tests/integration/conftest.py @@ -91,10 +91,24 @@ def set_disable_fast_analysis( request_session: requests.Session, ) -> Iterator[None]: """For integration tests that need to set then clear the - enableHttpProtocolSessions feature flag""" + disableFastProtocolUpload feature flag""" url = "http://localhost:31950/settings" data = {"id": "disableFastProtocolUpload", "value": True} request_session.post(url, json=data) yield None data["value"] = None request_session.post(url, json=data) + + +@pytest.fixture +def set_enable_ot3_hardware_controller( + request_session: requests.Session, +) -> Iterator[None]: + """For integration tests that need to set then clear the + enableOT3HardwareController feature flag""" + url = "http://localhost:31950/settings" + data = {"id": "enableOT3HardwareController", "value": True} + request_session.post(url, json=data) + yield None + data["value"] = None + request_session.post(url, json=data) diff --git a/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml index 79d84b317a1..24154ef89ff 100644 --- a/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml @@ -3,6 +3,7 @@ test_name: Upload, analyze, and confirm correctness of loading via deck coordina marks: - usefixtures: - run_server + - set_enable_ot3_hardware_controller - clean_server_state stages: - name: Upload deck_coordinate_load protocol @@ -25,7 +26,7 @@ stages: - name: deck_coordinate_load.py role: main createdAt: !re_search "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d+\\+\\d{2}:\\d{2}$" - robotType: OT-2 Standard + robotType: OT-3 Standard metadata: protocolName: Deck Coordinate PAPIv2 Test analyses: [] diff --git a/robot-server/tests/integration/protocols/deck_coordinate_load.py b/robot-server/tests/integration/protocols/deck_coordinate_load.py index 6ae930fc805..40b14a7ac7b 100644 --- a/robot-server/tests/integration/protocols/deck_coordinate_load.py +++ b/robot-server/tests/integration/protocols/deck_coordinate_load.py @@ -2,13 +2,17 @@ "protocolName": "Deck Coordinate PAPIv2 Test", } -requirements = {"robotType": "OT-2", "apiLevel": "2.14"} +requirements = {"robotType": "OT-3", "apiLevel": "2.14"} def run(context): pipette = context.load_instrument("p1000_single_gen2", mount="left") + labware = context.load_labware("armadillo_96_wellplate_200ul_pcr_full_skirt", "d3") + assert labware.parent == "D3" + module = context.load_module("temperatureModuleV2", "A1") + assert module.parent == "A1" pipette.move_to(labware.wells()[0].top()) module_labware = module.load_labware("armadillo_96_wellplate_200ul_pcr_full_skirt") From e91b689881ec9bf697e1758eb0b0509e55358ef6 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 28 Apr 2023 15:19:34 -0400 Subject: [PATCH 05/14] lint fixes --- api/tests/opentrons/protocol_api/test_labware.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/tests/opentrons/protocol_api/test_labware.py b/api/tests/opentrons/protocol_api/test_labware.py index d0104e3d231..cc7d2e0b1cd 100644 --- a/api/tests/opentrons/protocol_api/test_labware.py +++ b/api/tests/opentrons/protocol_api/test_labware.py @@ -1,8 +1,6 @@ """Tests for the InstrumentContext public interface.""" import inspect -from typing import Optional, Union - import pytest from decoy import Decoy @@ -19,7 +17,7 @@ from opentrons.protocol_api.core.core_map import LoadedCoreMap from opentrons.protocol_api import TemperatureModuleContext -from opentrons.types import DeckSlotName, Point +from opentrons.types import Point @pytest.fixture(autouse=True) From fc8e07f404bcb1c2a2b90b35a50d589a2296bc6f Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Mon, 1 May 2023 11:07:40 -0400 Subject: [PATCH 06/14] fix labware return --- api/src/opentrons/protocol_api/labware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/protocol_api/labware.py b/api/src/opentrons/protocol_api/labware.py index 54774700727..78ab1836206 100644 --- a/api/src/opentrons/protocol_api/labware.py +++ b/api/src/opentrons/protocol_api/labware.py @@ -363,7 +363,7 @@ def parent(self) -> Union[str, ModuleTypes, None]: if isinstance(labware_location, AbstractModuleCore): return self._core_map.get(labware_location) - return None if labware_location is None else labware_location.id + return labware_location @property # type: ignore[misc] @requires_version(2, 0) From 32ec9050fb94accc9bac30b3767019a47063189a Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Mon, 1 May 2023 11:09:44 -0400 Subject: [PATCH 07/14] rename get_deck_slot_display-name to get_deck_slot_id --- api/src/opentrons/protocol_api/core/engine/module_core.py | 2 +- .../opentrons/protocol_api/core/legacy/legacy_module_core.py | 2 +- api/src/opentrons/protocol_api/core/module.py | 2 +- api/src/opentrons/protocol_api/module_contexts.py | 2 +- .../opentrons/protocol_api/core/engine/test_module_core.py | 4 ++-- api/tests/opentrons/protocol_api/test_module_context.py | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/module_core.py b/api/src/opentrons/protocol_api/core/engine/module_core.py index 75bd6514d24..f9fa4b9276c 100644 --- a/api/src/opentrons/protocol_api/core/engine/module_core.py +++ b/api/src/opentrons/protocol_api/core/engine/module_core.py @@ -79,7 +79,7 @@ def get_deck_slot(self) -> DeckSlotName: """Get the module's deck slot.""" return self._engine_client.state.modules.get_location(self.module_id).slotName - def get_deck_slot_display_name(self) -> str: + def get_deck_slot_id(self) -> str: slot_name = self.get_deck_slot() return validation.ensure_deck_slot_string( slot_name, robot_type=self._engine_client.state.config.robot_type diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py index 1b90d98e926..3c470d5a54c 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py @@ -87,7 +87,7 @@ def get_deck_slot(self) -> DeckSlotName: """Get the module's deck slot.""" return DeckSlotName.from_primitive(self._geometry.parent) # type: ignore[arg-type] - def get_deck_slot_display_name(self) -> str: + def get_deck_slot_id(self) -> str: return str(self.get_deck_slot()) def get_display_name(self) -> str: diff --git a/api/src/opentrons/protocol_api/core/module.py b/api/src/opentrons/protocol_api/core/module.py index 9802e5f2148..62b3bb396eb 100644 --- a/api/src/opentrons/protocol_api/core/module.py +++ b/api/src/opentrons/protocol_api/core/module.py @@ -37,7 +37,7 @@ def get_deck_slot(self) -> DeckSlotName: """Get the module's deck slot.""" @abstractmethod - def get_deck_slot_display_name(self) -> str: + def get_deck_slot_id(self) -> str: """Get the module's deck slot in a robot accurate format.""" def get_display_name(self) -> str: diff --git a/api/src/opentrons/protocol_api/module_contexts.py b/api/src/opentrons/protocol_api/module_contexts.py index ee4df0a970f..095f497e883 100644 --- a/api/src/opentrons/protocol_api/module_contexts.py +++ b/api/src/opentrons/protocol_api/module_contexts.py @@ -214,7 +214,7 @@ def labware(self) -> Optional[Labware]: @requires_version(2, 14) def parent(self) -> str: """The name of the slot the module is on.""" - return self._core.get_deck_slot_display_name() + return self._core.get_deck_slot_id() @property # type: ignore[misc] @requires_version(2, 0) diff --git a/api/tests/opentrons/protocol_api/core/engine/test_module_core.py b/api/tests/opentrons/protocol_api/core/engine/test_module_core.py index 5605ea46c25..b9dfb1539a1 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_module_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_module_core.py @@ -71,7 +71,7 @@ def test_get_deck_slot( assert subject.get_deck_slot() == DeckSlotName.SLOT_1 -def test_get_deck_slot_display_name( +def test_get_deck_slot_id( decoy: Decoy, subject: ModuleCore, mock_engine_client: EngineClient ) -> None: """It should return the correct deck slot string associated with the module and robot type.""" @@ -85,7 +85,7 @@ def test_get_deck_slot_display_name( mock_validation.ensure_deck_slot_string(DeckSlotName.SLOT_1, "OT-3 Standard") ).then_return("foo") - assert subject.get_deck_slot_display_name() == "foo" + assert subject.get_deck_slot_id() == "foo" def test_get_model( diff --git a/api/tests/opentrons/protocol_api/test_module_context.py b/api/tests/opentrons/protocol_api/test_module_context.py index 6f9c2d5f9a7..8c7cc8c2c69 100644 --- a/api/tests/opentrons/protocol_api/test_module_context.py +++ b/api/tests/opentrons/protocol_api/test_module_context.py @@ -165,7 +165,7 @@ def test_load_labware_from_definition( def test_parent(decoy: Decoy, mock_core: ModuleCore, subject: ModuleContext) -> None: """Should get the parent slot name.""" - decoy.when(mock_core.get_deck_slot_display_name()).then_return("bar") + decoy.when(mock_core.get_deck_slot_id()).then_return("bar") assert subject.parent == "bar" From 7324c747de9315650446324d9a644b0ceba05116 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Mon, 1 May 2023 14:05:58 -0400 Subject: [PATCH 08/14] another (better) try at ot-3 tavern tests --- robot-server/dev_ot3.env | 8 +++ robot-server/tests/conftest.py | 18 +++++++ robot-server/tests/integration/common.yaml | 1 + robot-server/tests/integration/conftest.py | 49 +++++++++++++------ robot-server/tests/integration/dev_server.py | 5 +- .../test_deck_coordinate_load.tavern.yaml | 9 ++-- 6 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 robot-server/dev_ot3.env diff --git a/robot-server/dev_ot3.env b/robot-server/dev_ot3.env new file mode 100644 index 00000000000..e02cbd06975 --- /dev/null +++ b/robot-server/dev_ot3.env @@ -0,0 +1,8 @@ +# Environment for development + +ENABLE_VIRTUAL_SMOOTHIE=true +OT_ROBOT_SERVER_persistence_directory=automatically_make_temporary +OT_ROBOT_SERVER_simulator_configuration_file_path=simulators/test.json +DEV_ROBOT_NAME=opentrons-dev + +OT_API_FF_enableOT3HardwareController=true \ No newline at end of file diff --git a/robot-server/tests/conftest.py b/robot-server/tests/conftest.py index c464c9368a8..22793ab3a95 100644 --- a/robot-server/tests/conftest.py +++ b/robot-server/tests/conftest.py @@ -184,6 +184,24 @@ async def _clean_server_state() -> None: asyncio.run(_clean_server_state()) +# TODO(jbl 2023-05-01) merge this with ot3_run_server, along with clean_server_state and run_server +@pytest.fixture() +def ot3_clean_server_state() -> Iterator[None]: + # async fn that does the things below + # make a robot client + # delete protocols + async def _clean_server_state() -> None: + port = "31960" + async with RobotClient.make( + host="http://localhost", port=port, version="*" + ) as robot_client: + await _delete_all_runs(robot_client) + await _delete_all_protocols(robot_client) + + yield + asyncio.run(_clean_server_state()) + + @pytest.fixture(scope="session") def run_server( request_session: requests.Session, server_temp_directory: str diff --git a/robot-server/tests/integration/common.yaml b/robot-server/tests/integration/common.yaml index 67e6a769203..14f2d4f7747 100644 --- a/robot-server/tests/integration/common.yaml +++ b/robot-server/tests/integration/common.yaml @@ -6,3 +6,4 @@ variables: # These must match the session_server_host and session_server_port fixtures in conftest.py. host: http://localhost port: 31950 + port_ot3: 31960 \ No newline at end of file diff --git a/robot-server/tests/integration/conftest.py b/robot-server/tests/integration/conftest.py index 67832813b77..8ab677415b4 100644 --- a/robot-server/tests/integration/conftest.py +++ b/robot-server/tests/integration/conftest.py @@ -14,6 +14,7 @@ # Must match our Tavern config in common.yaml. _SESSION_SERVER_HOST = "http://localhost" _SESSION_SERVER_PORT = "31950" +_OT3_SESSION_SERVER_PORT = "31960" def pytest_tavern_beta_before_every_test_run( @@ -74,6 +75,40 @@ def run_server( yield +@pytest.fixture(scope="session") +def ot3_run_server( + request_session: requests.Session, + server_temp_directory: str, +) -> Iterator[None]: + """Run the robot server in a background process.""" + with DevServer( + port=_OT3_SESSION_SERVER_PORT, + is_ot3=True, + ot_api_config_dir=Path(server_temp_directory), + ) as dev_server: + dev_server.start() + + # Wait for a bit to get started by polling /hcpealth + from requests.exceptions import ConnectionError + + while True: + try: + request_session.get( + f"{_SESSION_SERVER_HOST}:{_OT3_SESSION_SERVER_PORT}/health" + ) + except ConnectionError: + pass + else: + break + time.sleep(0.5) + request_session.post( + f"{_SESSION_SERVER_HOST}:{_OT3_SESSION_SERVER_PORT}/home", + json={"target": "robot"}, + ) + + yield + + @pytest.fixture(scope="session") def session_server_host(run_server: object) -> str: """Return the host of the running session-scoped dev server.""" @@ -98,17 +133,3 @@ def set_disable_fast_analysis( yield None data["value"] = None request_session.post(url, json=data) - - -@pytest.fixture -def set_enable_ot3_hardware_controller( - request_session: requests.Session, -) -> Iterator[None]: - """For integration tests that need to set then clear the - enableOT3HardwareController feature flag""" - url = "http://localhost:31950/settings" - data = {"id": "enableOT3HardwareController", "value": True} - request_session.post(url, json=data) - yield None - data["value"] = None - request_session.post(url, json=data) diff --git a/robot-server/tests/integration/dev_server.py b/robot-server/tests/integration/dev_server.py index c70f22838b2..f14f8ca0f2e 100644 --- a/robot-server/tests/integration/dev_server.py +++ b/robot-server/tests/integration/dev_server.py @@ -12,6 +12,7 @@ class DevServer: def __init__( self, port: str = "31950", + is_ot3: bool = False, ot_api_config_dir: Optional[Path] = None, persistence_directory: Optional[Path] = None, maximum_runs: Optional[int] = None, @@ -19,6 +20,7 @@ def __init__( ) -> None: """Initialize a dev server.""" self.port: str = port + self.is_ot3 = is_ot3 self.ot_api_config_dir: Path = ( ot_api_config_dir @@ -49,8 +51,9 @@ def start(self) -> None: """Run the robot server in a background process.""" # This environment is only for the subprocess so it should # not have any side effects. + env_file: str = "dev.env" if not self.is_ot3 else "dev_ot3.env" env = { - "OT_ROBOT_SERVER_DOT_ENV_PATH": "dev.env", + "OT_ROBOT_SERVER_DOT_ENV_PATH": env_file, "OT_API_CONFIG_DIR": str(self.ot_api_config_dir), "OT_ROBOT_SERVER_persistence_directory": str(self.persistence_directory), } diff --git a/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml index 24154ef89ff..d8dbc343ad7 100644 --- a/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml @@ -2,13 +2,12 @@ test_name: Upload, analyze, and confirm correctness of loading via deck coordina marks: - usefixtures: - - run_server - - set_enable_ot3_hardware_controller - - clean_server_state + - ot3_run_server + - ot3_clean_server_state stages: - name: Upload deck_coordinate_load protocol request: - url: '{host:s}:{port:d}/protocols' + url: '{host:s}:{port_ot3:d}/protocols' method: POST files: files: 'tests/integration/protocols/deck_coordinate_load.py' @@ -37,7 +36,7 @@ stages: max_retries: 10 delay_after: 0.1 request: - url: '{host:s}:{port:d}/protocols/{protocol_id}/analyses' + url: '{host:s}:{port_ot3:d}/protocols/{protocol_id}/analyses' method: GET response: strict: From 5137839fea2dec6aa4f6d4b94b302ecd78b19693 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Mon, 1 May 2023 15:03:12 -0400 Subject: [PATCH 09/14] will this pass CI? --- robot-server/dev_ot3.env | 8 -------- robot-server/tests/integration/dev_server.py | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) delete mode 100644 robot-server/dev_ot3.env diff --git a/robot-server/dev_ot3.env b/robot-server/dev_ot3.env deleted file mode 100644 index e02cbd06975..00000000000 --- a/robot-server/dev_ot3.env +++ /dev/null @@ -1,8 +0,0 @@ -# Environment for development - -ENABLE_VIRTUAL_SMOOTHIE=true -OT_ROBOT_SERVER_persistence_directory=automatically_make_temporary -OT_ROBOT_SERVER_simulator_configuration_file_path=simulators/test.json -DEV_ROBOT_NAME=opentrons-dev - -OT_API_FF_enableOT3HardwareController=true \ No newline at end of file diff --git a/robot-server/tests/integration/dev_server.py b/robot-server/tests/integration/dev_server.py index f14f8ca0f2e..023e37d66a7 100644 --- a/robot-server/tests/integration/dev_server.py +++ b/robot-server/tests/integration/dev_server.py @@ -51,9 +51,9 @@ def start(self) -> None: """Run the robot server in a background process.""" # This environment is only for the subprocess so it should # not have any side effects. - env_file: str = "dev.env" if not self.is_ot3 else "dev_ot3.env" env = { - "OT_ROBOT_SERVER_DOT_ENV_PATH": env_file, + "OT_ROBOT_SERVER_DOT_ENV_PATH": "dev.env", + "OT_API_FF_enableOT3HardwareController": "true" if self.is_ot3 else "false", "OT_API_CONFIG_DIR": str(self.ot_api_config_dir), "OT_ROBOT_SERVER_persistence_directory": str(self.persistence_directory), } From 601bd47b9ba2d671824d26283876c15b3018ddc7 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Mon, 1 May 2023 16:00:17 -0400 Subject: [PATCH 10/14] mark test as ot3_only and actually run those --- .github/workflows/robot-server-lint-test.yaml | 6 +++++- .../protocols/test_deck_coordinate_load.tavern.yaml | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/robot-server-lint-test.yaml b/.github/workflows/robot-server-lint-test.yaml index b4d2a03feff..fcc71c299f4 100644 --- a/.github/workflows/robot-server-lint-test.yaml +++ b/.github/workflows/robot-server-lint-test.yaml @@ -76,8 +76,12 @@ jobs: run: make -C robot-server setup-ot2 - name: Lint run: make -C robot-server lint - - name: Test + - if: ${{ matrix.with-ot-hardware == 'false' }} + name: Test without opentrons_hardware run: make -C robot-server test-cov test_opts="-m 'not ot3_only'" + - if: ${{ matrix.with-ot-hardware == 'true' }} + name: Test with opentrons_hardware + run: make -C robot-server test-cov test_opts="-m 'not ot2_only'" - name: Ensure assets build run: make -C robot-server sdist wheel - name: Upload coverage report diff --git a/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml b/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml index d8dbc343ad7..d04274ef60e 100644 --- a/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml +++ b/robot-server/tests/integration/http_api/protocols/test_deck_coordinate_load.tavern.yaml @@ -1,6 +1,7 @@ test_name: Upload, analyze, and confirm correctness of loading via deck coordinate names marks: + - ot3_only - usefixtures: - ot3_run_server - ot3_clean_server_state From dd2c5d270b9a1502889d206795501fe51d192177 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 5 May 2023 10:26:23 -0400 Subject: [PATCH 11/14] reverting deck slot changes for this PR --- api/src/opentrons/protocol_api/deck.py | 6 ++-- api/tests/opentrons/protocol_api/test_deck.py | 34 ++++++------------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/api/src/opentrons/protocol_api/deck.py b/api/src/opentrons/protocol_api/deck.py index b547ad85dc7..6a2525eae42 100644 --- a/api/src/opentrons/protocol_api/deck.py +++ b/api/src/opentrons/protocol_api/deck.py @@ -105,10 +105,8 @@ def position_for(self, slot: DeckLocation) -> Location: def get_slot_definition(self, slot: DeckLocation) -> SlotDefV3: """Get the geometric definition data of a slot.""" - slot_name = validation.ensure_deck_slot_string( - _get_slot_name(slot), self._protocol_core.robot_type - ) - return self._slot_definitions_by_name[slot_name] + slot_name = _get_slot_name(slot) + return self._slot_definitions_by_name[slot_name.id] def get_slot_center(self, slot: DeckLocation) -> Point: """Get the absolute coordinates of a slot's center.""" diff --git a/api/tests/opentrons/protocol_api/test_deck.py b/api/tests/opentrons/protocol_api/test_deck.py index 6ccc52f9b90..ae1a7f1b9e2 100644 --- a/api/tests/opentrons/protocol_api/test_deck.py +++ b/api/tests/opentrons/protocol_api/test_deck.py @@ -137,32 +137,26 @@ def test_slot_keys_iter(subject: Deck) -> None: { "locations": { "orderedSlots": [ - {"id": "fee"}, - {"id": "foe"}, - {"id": "fum"}, + {"id": "1"}, + {"id": "2"}, + {"id": "3"}, ], "calibrationPoints": [], } }, ], ) -def test_get_slots( - decoy: Decoy, mock_protocol_core: ProtocolCore, subject: Deck -) -> None: +def test_get_slots(decoy: Decoy, subject: Deck) -> None: """It should provide slot definitions.""" decoy.when(mock_validation.ensure_deck_slot(222)).then_return(DeckSlotName.SLOT_2) - decoy.when(mock_protocol_core.robot_type).then_return("OT-2 Standard") - decoy.when( - mock_validation.ensure_deck_slot_string(DeckSlotName.SLOT_2, "OT-2 Standard") - ).then_return("fee") assert subject.slots == [ - {"id": "fee"}, - {"id": "foe"}, - {"id": "fum"}, + {"id": "1"}, + {"id": "2"}, + {"id": "3"}, ] - assert subject.get_slot_definition(222) == {"id": "fee"} + assert subject.get_slot_definition(222) == {"id": "2"} @pytest.mark.parametrize( @@ -171,27 +165,21 @@ def test_get_slots( { "locations": { "orderedSlots": [ - {"id": "foo", "position": [1.0, 2.0, 3.0]}, + {"id": "3", "position": [1.0, 2.0, 3.0]}, ], "calibrationPoints": [], } }, ], ) -def test_get_position_for( - decoy: Decoy, mock_protocol_core: ProtocolCore, subject: Deck -) -> None: +def test_get_position_for(decoy: Decoy, subject: Deck) -> None: """It should return a `Location` for a deck slot.""" decoy.when(mock_validation.ensure_deck_slot(333)).then_return(DeckSlotName.SLOT_3) - decoy.when(mock_protocol_core.robot_type).then_return("OT-3 Standard") - decoy.when( - mock_validation.ensure_deck_slot_string(DeckSlotName.SLOT_3, "OT-3 Standard") - ).then_return("foo") result = subject.position_for(333) assert result.point == Point(x=1.0, y=2.0, z=3.0) assert result.labware.is_slot is True - assert str(result.labware) == "foo" + assert str(result.labware) == "3" def test_highest_z( From 420703bf277bbd18c3334c25ad78da73608c7109 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 5 May 2023 14:55:07 -0400 Subject: [PATCH 12/14] changes per review comments --- .github/workflows/robot-server-lint-test.yaml | 2 +- .../core/legacy/legacy_module_core.py | 2 +- robot-server/tests/integration/conftest.py | 17 ++++++++++------- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.github/workflows/robot-server-lint-test.yaml b/.github/workflows/robot-server-lint-test.yaml index fcc71c299f4..a1dc938be63 100644 --- a/.github/workflows/robot-server-lint-test.yaml +++ b/.github/workflows/robot-server-lint-test.yaml @@ -81,7 +81,7 @@ jobs: run: make -C robot-server test-cov test_opts="-m 'not ot3_only'" - if: ${{ matrix.with-ot-hardware == 'true' }} name: Test with opentrons_hardware - run: make -C robot-server test-cov test_opts="-m 'not ot2_only'" + run: make -C robot-server test-cov - name: Ensure assets build run: make -C robot-server sdist wheel - name: Upload coverage report diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py index 3c470d5a54c..d14699da6c6 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_module_core.py @@ -88,7 +88,7 @@ def get_deck_slot(self) -> DeckSlotName: return DeckSlotName.from_primitive(self._geometry.parent) # type: ignore[arg-type] def get_deck_slot_id(self) -> str: - return str(self.get_deck_slot()) + return self.get_deck_slot().id def get_display_name(self) -> str: """Get the module's display name.""" diff --git a/robot-server/tests/integration/conftest.py b/robot-server/tests/integration/conftest.py index 79f083bc2c2..bfca2bebb2e 100644 --- a/robot-server/tests/integration/conftest.py +++ b/robot-server/tests/integration/conftest.py @@ -96,18 +96,21 @@ def ot3_run_server( while True: try: - request_session.get( + health_response = request_session.get( f"{_SESSION_SERVER_HOST}:{_OT3_SESSION_SERVER_PORT}/health" ) except ConnectionError: + # The server isn't up yet to accept requests. Keep polling. pass else: - break - time.sleep(0.5) - request_session.post( - f"{_SESSION_SERVER_HOST}:{_OT3_SESSION_SERVER_PORT}/home", - json={"target": "robot"}, - ) + if health_response.status_code == 503: + # The server is accepting requests but reporting not ready. Keep polling. + pass + else: + # The server's replied with something other than a busy indicator. Stop polling. + break + + time.sleep(0.1) yield From 89726090c3a448650d656f6a1d0f6eb0b400a8cf Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 5 May 2023 15:09:53 -0400 Subject: [PATCH 13/14] internal release notes update --- api/release-notes-internal.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/release-notes-internal.md b/api/release-notes-internal.md index 9df6c752803..5e3ea4d05e5 100644 --- a/api/release-notes-internal.md +++ b/api/release-notes-internal.md @@ -54,6 +54,8 @@ Some things are known not to work, and are listed below. Specific compatibility - Fixed an issue where pinging `GET /instruments` during automated calibration would cause calibration to fail - Increased reliability of automated calibration - Increased reliability of gripper pickups from modules +- Python API `load_labware`, `load_module` and `move_labware` can accept location as deck coordinate in addition to slot numbers +- The `.parent` property for Module and Labware objects (loaded on the deck) will return a coordinate style deck location for protocols with robotType "OT-3 Standard" ## Big Things That Do Work Please Do Report Bugs About Them ### Robot Control From abdc60852e0990027812ad7c65aaf57bc9e388d5 Mon Sep 17 00:00:00 2001 From: jbleon95 Date: Fri, 5 May 2023 15:18:51 -0400 Subject: [PATCH 14/14] exempli gratia --- api/release-notes-internal.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/release-notes-internal.md b/api/release-notes-internal.md index 5e3ea4d05e5..0271af4bc59 100644 --- a/api/release-notes-internal.md +++ b/api/release-notes-internal.md @@ -54,8 +54,8 @@ Some things are known not to work, and are listed below. Specific compatibility - Fixed an issue where pinging `GET /instruments` during automated calibration would cause calibration to fail - Increased reliability of automated calibration - Increased reliability of gripper pickups from modules -- Python API `load_labware`, `load_module` and `move_labware` can accept location as deck coordinate in addition to slot numbers -- The `.parent` property for Module and Labware objects (loaded on the deck) will return a coordinate style deck location for protocols with robotType "OT-3 Standard" +- Python API `load_labware`, `load_module` and `move_labware` can accept location as deck coordinates (e.g. "A1", "D3") in addition to slot numbers +- The `.parent` property for Module and Labware objects (loaded on the deck) will return a coordinate style deck location (e.g. "B2", "C3") for protocols with robotType "OT-3 Standard" instead of the slot number ## Big Things That Do Work Please Do Report Bugs About Them ### Robot Control