diff --git a/analyses-snapshot-testing/Makefile b/analyses-snapshot-testing/Makefile index 30adfbcd5a7..e6041e11f76 100644 --- a/analyses-snapshot-testing/Makefile +++ b/analyses-snapshot-testing/Makefile @@ -90,7 +90,7 @@ build-opentrons-analysis: @echo "The image will be named opentrons-analysis:$(ANALYSIS_REF)" @echo "If you want to build a different version, run 'make build-opentrons-analysis ANALYSIS_REF='" @echo "Cache is always busted to ensure latest version of the code is used" - docker build --build-arg OPENTRONS_VERSION=$(ANALYSIS_REF) --build-arg CACHEBUST=$(CACHEBUST) -t opentrons-analysis:$(ANALYSIS_REF) citools/. + docker build --build-arg ANALYSIS_REF=$(ANALYSIS_REF) --build-arg CACHEBUST=$(CACHEBUST) -t opentrons-analysis:$(ANALYSIS_REF) citools/. .PHONY: generate-protocols generate-protocols: diff --git a/api/src/opentrons/config/advanced_settings.py b/api/src/opentrons/config/advanced_settings.py index 812ca73a661..44cf5c0fcc4 100644 --- a/api/src/opentrons/config/advanced_settings.py +++ b/api/src/opentrons/config/advanced_settings.py @@ -222,6 +222,17 @@ class Setting(NamedTuple): robot_type=[RobotTypeEnum.OT2, RobotTypeEnum.FLEX], internal_only=True, ), + SettingDefinition( + _id="allowLiquidClasses", + title="Allow the use of liquid classes", + description=( + "Do not enable." + " This is an Opentrons internal setting to allow using in-development" + " liquid classes." + ), + robot_type=[RobotTypeEnum.OT2, RobotTypeEnum.FLEX], + internal_only=True, + ), ] @@ -715,6 +726,16 @@ def _migrate34to35(previous: SettingsMap) -> SettingsMap: return newmap +def _migrate35to36(previous: SettingsMap) -> SettingsMap: + """Migrate to version 36 of the feature flags file. + + - Adds the allowLiquidClasses config element. + """ + newmap = {k: v for k, v in previous.items()} + newmap["allowLiquidClasses"] = None + return newmap + + _MIGRATIONS = [ _migrate0to1, _migrate1to2, @@ -751,6 +772,7 @@ def _migrate34to35(previous: SettingsMap) -> SettingsMap: _migrate32to33, _migrate33to34, _migrate34to35, + _migrate35to36, ] """ List of all migrations to apply, indexed by (version - 1). See _migrate below diff --git a/api/src/opentrons/config/feature_flags.py b/api/src/opentrons/config/feature_flags.py index 7eb40721511..2164e66f90a 100644 --- a/api/src/opentrons/config/feature_flags.py +++ b/api/src/opentrons/config/feature_flags.py @@ -78,3 +78,7 @@ def enable_performance_metrics(robot_type: RobotTypeEnum) -> bool: def oem_mode_enabled() -> bool: return advs.get_setting_with_env_overload("enableOEMMode", RobotTypeEnum.FLEX) + + +def allow_liquid_classes(robot_type: RobotTypeEnum) -> bool: + return advs.get_setting_with_env_overload("allowLiquidClasses", robot_type) diff --git a/api/src/opentrons/protocol_api/__init__.py b/api/src/opentrons/protocol_api/__init__.py index 8cc4bd1154e..2f35bb46764 100644 --- a/api/src/opentrons/protocol_api/__init__.py +++ b/api/src/opentrons/protocol_api/__init__.py @@ -29,7 +29,7 @@ AbsorbanceReaderContext, ) from .disposal_locations import TrashBin, WasteChute -from ._liquid import Liquid +from ._liquid import Liquid, LiquidClass from ._types import OFF_DECK from ._nozzle_layout import ( COLUMN, @@ -67,6 +67,7 @@ "WasteChute", "Well", "Liquid", + "LiquidClass", "Parameters", "COLUMN", "PARTIAL_COLUMN", diff --git a/api/src/opentrons/protocol_api/_liquid.py b/api/src/opentrons/protocol_api/_liquid.py index b43a9c08495..75e2c6fb6f2 100644 --- a/api/src/opentrons/protocol_api/_liquid.py +++ b/api/src/opentrons/protocol_api/_liquid.py @@ -1,5 +1,14 @@ from dataclasses import dataclass -from typing import Optional +from typing import Optional, Sequence + +from opentrons_shared_data.liquid_classes.liquid_class_definition import ( + LiquidClassSchemaV1, + AspirateProperties, + SingleDispenseProperties, + MultiDispenseProperties, + ByPipetteSetting, + ByTipTypeSetting, +) @dataclass(frozen=True) @@ -18,3 +27,80 @@ class Liquid: name: str description: Optional[str] display_color: Optional[str] + + +# TODO (spp, 2024-10-17): create PAPI-equivalent types for all the properties +# and have validation on value updates with user-facing error messages +@dataclass +class TransferProperties: + _aspirate: AspirateProperties + _dispense: SingleDispenseProperties + _multi_dispense: Optional[MultiDispenseProperties] + + @property + def aspirate(self) -> AspirateProperties: + """Aspirate properties.""" + return self._aspirate + + @property + def dispense(self) -> SingleDispenseProperties: + """Single dispense properties.""" + return self._dispense + + @property + def multi_dispense(self) -> Optional[MultiDispenseProperties]: + """Multi dispense properties.""" + return self._multi_dispense + + +@dataclass +class LiquidClass: + """A data class that contains properties of a specific class of liquids.""" + + _name: str + _display_name: str + _by_pipette_setting: Sequence[ByPipetteSetting] + + @classmethod + def create(cls, liquid_class_definition: LiquidClassSchemaV1) -> "LiquidClass": + """Liquid class factory method.""" + + return cls( + _name=liquid_class_definition.liquidClassName, + _display_name=liquid_class_definition.displayName, + _by_pipette_setting=liquid_class_definition.byPipette, + ) + + @property + def name(self) -> str: + return self._name + + @property + def display_name(self) -> str: + return self._display_name + + def get_for(self, pipette: str, tiprack: str) -> TransferProperties: + """Get liquid class transfer properties for the specified pipette and tip.""" + settings_for_pipette: Sequence[ByPipetteSetting] = [ + pip_setting + for pip_setting in self._by_pipette_setting + if pip_setting.pipetteModel == pipette + ] + if len(settings_for_pipette) == 0: + raise ValueError( + f"No properties found for {pipette} in {self._name} liquid class" + ) + settings_for_tip: Sequence[ByTipTypeSetting] = [ + tip_setting + for tip_setting in settings_for_pipette[0].byTipType + if tip_setting.tiprack == tiprack + ] + if len(settings_for_tip) == 0: + raise ValueError( + f"No properties found for {tiprack} in {self._name} liquid class" + ) + return TransferProperties( + _aspirate=settings_for_tip[0].aspirate, + _dispense=settings_for_tip[0].singleDispense, + _multi_dispense=settings_for_tip[0].multiDispense, + ) diff --git a/api/src/opentrons/protocol_api/core/engine/protocol.py b/api/src/opentrons/protocol_api/core/engine/protocol.py index 360740cbfcd..0ed5270320a 100644 --- a/api/src/opentrons/protocol_api/core/engine/protocol.py +++ b/api/src/opentrons/protocol_api/core/engine/protocol.py @@ -2,11 +2,17 @@ from __future__ import annotations from typing import Dict, Optional, Type, Union, List, Tuple, TYPE_CHECKING +from opentrons_shared_data.liquid_classes import LiquidClassDefinitionDoesNotExist + from opentrons.protocol_engine import commands as cmd from opentrons.protocol_engine.commands import LoadModuleResult from opentrons_shared_data.deck.types import DeckDefinitionV5, SlotDefV3 from opentrons_shared_data.labware.labware_definition import LabwareDefinition from opentrons_shared_data.labware.types import LabwareDefinition as LabwareDefDict +from opentrons_shared_data import liquid_classes +from opentrons_shared_data.liquid_classes.liquid_class_definition import ( + LiquidClassSchemaV1, +) from opentrons_shared_data.pipette.types import PipetteNameType from opentrons_shared_data.robot.types import RobotType @@ -51,7 +57,7 @@ from ... import validation from ..._types import OffDeckType -from ..._liquid import Liquid +from ..._liquid import Liquid, LiquidClass from ...disposal_locations import TrashBin, WasteChute from ..protocol import AbstractProtocol from ..labware import LabwareLoadParams @@ -103,6 +109,7 @@ def __init__( str, Union[ModuleCore, NonConnectedModuleCore] ] = {} self._disposal_locations: List[Union[Labware, TrashBin, WasteChute]] = [] + self._defined_liquid_class_defs_by_name: Dict[str, LiquidClassSchemaV1] = {} self._load_fixed_trash() @property @@ -747,6 +754,23 @@ def define_liquid( ), ) + def define_liquid_class(self, name: str) -> LiquidClass: + """Define a liquid class for use in transfer functions.""" + try: + # Check if we have already loaded this liquid class' definition + liquid_class_def = self._defined_liquid_class_defs_by_name[name] + except KeyError: + try: + # Fetching the liquid class data from file and parsing it + # is an expensive operation and should be avoided. + # Calling this often will degrade protocol execution performance. + liquid_class_def = liquid_classes.load_definition(name) + self._defined_liquid_class_defs_by_name[name] = liquid_class_def + except LiquidClassDefinitionDoesNotExist: + raise ValueError(f"Liquid class definition not found for '{name}'.") + + return LiquidClass.create(liquid_class_def) + def get_labware_location( self, labware_core: LabwareCore ) -> Union[str, LabwareCore, ModuleCore, NonConnectedModuleCore, OffDeckType]: 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 eb1a7386e38..aeef0e9d7c7 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 @@ -17,7 +17,7 @@ from ...labware import Labware from ...disposal_locations import TrashBin, WasteChute -from ..._liquid import Liquid +from ..._liquid import Liquid, LiquidClass from ..._types import OffDeckType from ..protocol import AbstractProtocol from ..labware import LabwareLoadParams @@ -531,6 +531,10 @@ def define_liquid( """Define a liquid to load into a well.""" assert False, "define_liquid only supported on engine core" + def define_liquid_class(self, name: str) -> LiquidClass: + """Define a liquid class.""" + assert False, "define_liquid_class is only supported on engine core" + def get_labware_location( self, labware_core: LegacyLabwareCore ) -> Union[ diff --git a/api/src/opentrons/protocol_api/core/protocol.py b/api/src/opentrons/protocol_api/core/protocol.py index 687291c390a..9c3692c7e44 100644 --- a/api/src/opentrons/protocol_api/core/protocol.py +++ b/api/src/opentrons/protocol_api/core/protocol.py @@ -18,7 +18,7 @@ from .instrument import InstrumentCoreType from .labware import LabwareCoreType, LabwareLoadParams from .module import ModuleCoreType -from .._liquid import Liquid +from .._liquid import Liquid, LiquidClass from .._types import OffDeckType from ..disposal_locations import TrashBin, WasteChute @@ -247,6 +247,10 @@ def define_liquid( ) -> Liquid: """Define a liquid to load into a well.""" + @abstractmethod + def define_liquid_class(self, name: str) -> LiquidClass: + """Define a liquid class for use in transfer functions.""" + @abstractmethod def get_labware_location( self, labware_core: LabwareCoreType diff --git a/api/src/opentrons/protocol_api/protocol_context.py b/api/src/opentrons/protocol_api/protocol_context.py index 5c21f662a62..43c5956afd9 100644 --- a/api/src/opentrons/protocol_api/protocol_context.py +++ b/api/src/opentrons/protocol_api/protocol_context.py @@ -14,8 +14,10 @@ from opentrons_shared_data.labware.types import LabwareDefinition from opentrons_shared_data.pipette.types import PipetteNameType +from opentrons_shared_data.robot.types import RobotTypeEnum from opentrons.types import Mount, Location, DeckLocation, DeckSlotName, StagingSlotName +from opentrons.config import feature_flags from opentrons.legacy_broker import LegacyBroker from opentrons.hardware_control.modules.types import ( MagneticBlockModel, @@ -61,7 +63,7 @@ from .core.legacy.legacy_protocol_core import LegacyProtocolCore from . import validation -from ._liquid import Liquid +from ._liquid import Liquid, LiquidClass from .disposal_locations import TrashBin, WasteChute from .deck import Deck from .instrument_context import InstrumentContext @@ -1284,6 +1286,18 @@ def define_liquid( display_color=display_color, ) + def define_liquid_class( + self, + name: str, + ) -> LiquidClass: + """Define a liquid class for use in the protocol.""" + if feature_flags.allow_liquid_classes( + robot_type=RobotTypeEnum.robot_literal_to_enum(self._core.robot_type) + ): + return self._core.define_liquid_class(name=name) + else: + raise NotImplementedError("This method is not implemented.") + @property @requires_version(2, 5) def rail_lights_on(self) -> bool: diff --git a/api/src/opentrons/protocol_engine/errors/exceptions.py b/api/src/opentrons/protocol_engine/errors/exceptions.py index 57d420124a7..72698ebf029 100644 --- a/api/src/opentrons/protocol_engine/errors/exceptions.py +++ b/api/src/opentrons/protocol_engine/errors/exceptions.py @@ -952,7 +952,7 @@ def __init__( """Build a InvalidPipettingVolumeError.""" message = ( f"Cannot aspirate {attempted_aspirate_volume} µL when only" - f" {available_volume} is available." + f" {available_volume} is available in the tip." ) details = { "attempted_aspirate_volume": attempted_aspirate_volume, diff --git a/api/tests/opentrons/config/test_advanced_settings_migration.py b/api/tests/opentrons/config/test_advanced_settings_migration.py index 92a45a6d610..a2bcf71a1fb 100644 --- a/api/tests/opentrons/config/test_advanced_settings_migration.py +++ b/api/tests/opentrons/config/test_advanced_settings_migration.py @@ -8,7 +8,7 @@ @pytest.fixture def migrated_file_version() -> int: - return 35 + return 36 # make sure to set a boolean value in default_file_settings only if @@ -30,6 +30,7 @@ def default_file_settings() -> Dict[str, Any]: "enableErrorRecoveryExperiments": None, "enableOEMMode": None, "enablePerformanceMetrics": None, + "allowLiquidClasses": None, } @@ -68,6 +69,7 @@ def v2_config(v1_config: Dict[str, Any]) -> Dict[str, Any]: r.update( { "_version": 2, + "disableLogAggregation": None, } ) return r @@ -410,6 +412,26 @@ def v34_config(v33_config: Dict[str, Any]) -> Dict[str, Any]: return r +@pytest.fixture +def v35_config(v34_config: Dict[str, Any]) -> Dict[str, Any]: + r = v34_config.copy() + r.pop("disableLogAggregation") + r["_version"] = 35 + return r + + +@pytest.fixture +def v36_config(v35_config: Dict[str, Any]) -> Dict[str, Any]: + r = v35_config.copy() + r.update( + { + "_version": 36, + "allowLiquidClasses": None, + } + ) + return r + + @pytest.fixture( scope="session", params=[ @@ -449,6 +471,8 @@ def v34_config(v33_config: Dict[str, Any]) -> Dict[str, Any]: lazy_fixture("v32_config"), lazy_fixture("v33_config"), lazy_fixture("v34_config"), + lazy_fixture("v35_config"), + lazy_fixture("v36_config"), ], ) def old_settings(request: SubRequest) -> Dict[str, Any]: @@ -539,4 +563,5 @@ def test_ensures_config() -> None: "enableErrorRecoveryExperiments": None, "enableOEMMode": None, "enablePerformanceMetrics": None, + "allowLiquidClasses": None, } diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index b1ff1133978..a52e95248c0 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -20,6 +20,7 @@ Union, cast, ) + from typing_extensions import TypedDict import pytest @@ -37,6 +38,23 @@ from opentrons_shared_data.protocol.types import JsonProtocol from opentrons_shared_data.labware.types import LabwareDefinition from opentrons_shared_data.module.types import ModuleDefinitionV3 +from opentrons_shared_data.liquid_classes.liquid_class_definition import ( + LiquidClassSchemaV1, + ByPipetteSetting, + ByTipTypeSetting, + AspirateProperties, + Submerge, + PositionReference, + DelayProperties, + DelayParams, + RetractAspirate, + SingleDispenseProperties, + RetractDispense, + Coordinate, + MixProperties, + TouchTipProperties, + BlowoutProperties, +) from opentrons_shared_data.deck.types import ( RobotModel, DeckDefinitionV3, @@ -763,3 +781,84 @@ def minimal_module_def() -> ModuleDefinitionV3: "cornerOffsetFromSlot": {"x": 0.1, "y": 0.1, "z": 0.0}, "twoDimensionalRendering": {}, } + + +@pytest.fixture +def minimal_liquid_class_def1() -> LiquidClassSchemaV1: + return LiquidClassSchemaV1( + liquidClassName="water1", + displayName="water 1", + schemaVersion=1, + namespace="test-fixture-1", + byPipette=[], + ) + + +@pytest.fixture +def minimal_liquid_class_def2() -> LiquidClassSchemaV1: + return LiquidClassSchemaV1( + liquidClassName="water2", + displayName="water 2", + schemaVersion=1, + namespace="test-fixture-2", + byPipette=[ + ByPipetteSetting( + pipetteModel="p20_single_gen2", + byTipType=[ + ByTipTypeSetting( + tiprack="opentrons_96_tiprack_20ul", + aspirate=AspirateProperties( + submerge=Submerge( + positionReference=PositionReference.LIQUID_MENISCUS, + offset=Coordinate(x=0, y=0, z=-5), + speed=100, + delay=DelayProperties( + enable=True, params=DelayParams(duration=1.5) + ), + ), + retract=RetractAspirate( + positionReference=PositionReference.WELL_TOP, + offset=Coordinate(x=0, y=0, z=5), + speed=100, + airGapByVolume={"default": 2, "5": 3, "10": 4}, + touchTip=TouchTipProperties(enable=False), + delay=DelayProperties(enable=False), + ), + positionReference=PositionReference.WELL_BOTTOM, + offset=Coordinate(x=0, y=0, z=-5), + flowRateByVolume={"default": 50, "10": 40, "20": 30}, + preWet=True, + mix=MixProperties(enable=False), + delay=DelayProperties( + enable=True, params=DelayParams(duration=2) + ), + ), + singleDispense=SingleDispenseProperties( + submerge=Submerge( + positionReference=PositionReference.LIQUID_MENISCUS, + offset=Coordinate(x=0, y=0, z=-5), + speed=100, + delay=DelayProperties(enable=False), + ), + retract=RetractDispense( + positionReference=PositionReference.WELL_TOP, + offset=Coordinate(x=0, y=0, z=5), + speed=100, + airGapByVolume={"default": 2, "5": 3, "10": 4}, + blowout=BlowoutProperties(enable=False), + touchTip=TouchTipProperties(enable=False), + delay=DelayProperties(enable=False), + ), + positionReference=PositionReference.WELL_BOTTOM, + offset=Coordinate(x=0, y=0, z=-5), + flowRateByVolume={"default": 50, "10": 40, "20": 30}, + mix=MixProperties(enable=False), + pushOutByVolume={"default": 5, "10": 7, "20": 10}, + delay=DelayProperties(enable=False), + ), + multiDispense=None, + ) + ], + ) + ], + ) 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 aa575ea1f16..7b549fc035d 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 @@ -3,6 +3,10 @@ from typing import Optional, Type, cast, Tuple import pytest +from opentrons_shared_data import liquid_classes +from opentrons_shared_data.liquid_classes.liquid_class_definition import ( + LiquidClassSchemaV1, +) from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] from decoy import Decoy @@ -69,7 +73,7 @@ ModuleCore, load_labware_params, ) -from opentrons.protocol_api._liquid import Liquid +from opentrons.protocol_api._liquid import Liquid, LiquidClass from opentrons.protocol_api.disposal_locations import TrashBin, WasteChute from opentrons.protocol_api.core.engine.exceptions import InvalidModuleLocationError from opentrons.protocol_api.core.engine.module_core import ( @@ -111,6 +115,15 @@ def patch_mock_load_labware_params( monkeypatch.setattr(load_labware_params, name, decoy.mock(func=func)) +@pytest.fixture(autouse=True) +def patch_mock_load_liquid_class_def( + decoy: Decoy, monkeypatch: pytest.MonkeyPatch +) -> None: + """Mock out liquid_classes.load_definition() function.""" + mock_load = decoy.mock(func=liquid_classes.load_definition) + monkeypatch.setattr(liquid_classes, "load_definition", mock_load) + + @pytest.fixture(autouse=True) def patch_mock_validation(decoy: Decoy, monkeypatch: pytest.MonkeyPatch) -> None: """Mock out validation.py functions.""" @@ -1742,6 +1755,27 @@ def test_add_liquid( assert result == expected_result +def test_define_liquid_class( + decoy: Decoy, + subject: ProtocolCore, + minimal_liquid_class_def1: LiquidClassSchemaV1, + minimal_liquid_class_def2: LiquidClassSchemaV1, +) -> None: + """It should create a LiquidClass and cache the definition.""" + expected_liquid_class = LiquidClass( + _name="water1", _display_name="water 1", _by_pipette_setting=[] + ) + decoy.when(liquid_classes.load_definition("water")).then_return( + minimal_liquid_class_def1 + ) + assert subject.define_liquid_class("water") == expected_liquid_class + + decoy.when(liquid_classes.load_definition("water")).then_return( + minimal_liquid_class_def2 + ) + assert subject.define_liquid_class("water") == expected_liquid_class + + def test_get_labware_location_deck_slot( decoy: Decoy, mock_engine_client: EngineClient, diff --git a/api/tests/opentrons/protocol_api/test_liquid_class.py b/api/tests/opentrons/protocol_api/test_liquid_class.py new file mode 100644 index 00000000000..48f3788f496 --- /dev/null +++ b/api/tests/opentrons/protocol_api/test_liquid_class.py @@ -0,0 +1,38 @@ +"""Tests for LiquidClass methods.""" +import pytest + +from opentrons_shared_data.liquid_classes.liquid_class_definition import ( + LiquidClassSchemaV1, +) +from opentrons.protocol_api import LiquidClass + + +def test_create_liquid_class( + minimal_liquid_class_def1: LiquidClassSchemaV1, +) -> None: + """It should create a LiquidClass from provided definition.""" + assert LiquidClass.create(minimal_liquid_class_def1) == LiquidClass( + _name="water1", _display_name="water 1", _by_pipette_setting=[] + ) + + +def test_get_for_pipette_and_tip( + minimal_liquid_class_def2: LiquidClassSchemaV1, +) -> None: + """It should get the properties for the specified pipette and tip.""" + liq_class = LiquidClass.create(minimal_liquid_class_def2) + result = liq_class.get_for("p20_single_gen2", "opentrons_96_tiprack_20ul") + assert result.aspirate.flowRateByVolume == {"default": 50, "10": 40, "20": 30} + + +def test_get_for_raises_for_incorrect_pipette_or_tip( + minimal_liquid_class_def2: LiquidClassSchemaV1, +) -> None: + """It should raise an error when accessing non-existent properties.""" + liq_class = LiquidClass.create(minimal_liquid_class_def2) + + with pytest.raises(ValueError): + liq_class.get_for("p20_single_gen2", "no_such_tiprack") + + with pytest.raises(ValueError): + liq_class.get_for("p300_single", "opentrons_96_tiprack_20ul") diff --git a/api/tests/opentrons/protocol_api/test_protocol_context.py b/api/tests/opentrons/protocol_api/test_protocol_context.py index 1e1dda706c6..2bedbd5fb6f 100644 --- a/api/tests/opentrons/protocol_api/test_protocol_context.py +++ b/api/tests/opentrons/protocol_api/test_protocol_context.py @@ -7,8 +7,11 @@ from opentrons_shared_data.pipette.types import PipetteNameType from opentrons_shared_data.labware.types import LabwareDefinition as LabwareDefDict +from opentrons_shared_data.robot.types import RobotTypeEnum, RobotType +from opentrons.protocol_api._liquid import LiquidClass from opentrons.types import Mount, DeckSlotName, StagingSlotName +from opentrons.config import feature_flags as ff from opentrons.protocol_api import OFF_DECK from opentrons.legacy_broker import LegacyBroker from opentrons.hardware_control.modules.types import ModuleType, TemperatureModuleModel @@ -1214,6 +1217,28 @@ def test_define_liquid_arg_defaulting( ) +@pytest.mark.parametrize("robot_type", ["OT-2 Standard", "OT-3 Standard"]) +def test_define_liquid_class( + decoy: Decoy, + mock_core: ProtocolCore, + subject: ProtocolContext, + robot_type: RobotType, + mock_feature_flags: None, +) -> None: + """It should create the liquid class definition.""" + expected_liquid_class = LiquidClass( + _name="volatile_100", _display_name="volatile 100%", _by_pipette_setting=[] + ) + decoy.when(mock_core.define_liquid_class("volatile_90")).then_return( + expected_liquid_class + ) + decoy.when(mock_core.robot_type).then_return(robot_type) + decoy.when( + ff.allow_liquid_classes(RobotTypeEnum.robot_literal_to_enum(robot_type)) + ).then_return(True) + assert subject.define_liquid_class("volatile_90") == expected_liquid_class + + def test_bundled_data( decoy: Decoy, mock_core_map: LoadedCoreMap, mock_deck: Deck, mock_core: ProtocolCore ) -> None: diff --git a/api/tests/opentrons/protocol_api_integration/test_liquid_classes.py b/api/tests/opentrons/protocol_api_integration/test_liquid_classes.py new file mode 100644 index 00000000000..049edae5c0f --- /dev/null +++ b/api/tests/opentrons/protocol_api_integration/test_liquid_classes.py @@ -0,0 +1,60 @@ +"""Tests for the APIs around liquid classes.""" +import pytest +from decoy import Decoy +from opentrons_shared_data.robot.types import RobotTypeEnum + +from opentrons import simulate +from opentrons.config import feature_flags as ff + + +@pytest.mark.ot2_only +def test_liquid_class_creation_and_property_fetching( + decoy: Decoy, mock_feature_flags: None +) -> None: + """It should create the liquid class and provide access to its properties.""" + decoy.when(ff.allow_liquid_classes(RobotTypeEnum.OT2)).then_return(True) + protocol_context = simulate.get_protocol_api(version="2.20", robot_type="OT-2") + pipette_left = protocol_context.load_instrument("p20_single_gen2", mount="left") + pipette_right = protocol_context.load_instrument("p300_multi", mount="right") + tiprack = protocol_context.load_labware("opentrons_96_tiprack_20ul", "1") + + glycerol_50 = protocol_context.define_liquid_class("fixture_glycerol50") + + assert glycerol_50.name == "fixture_glycerol50" + assert glycerol_50.display_name == "Glycerol 50%" + + # TODO (spp, 2024-10-17): update this to use pipette's load name instead of pipette.name + assert ( + glycerol_50.get_for( + pipette_left.name, tiprack.load_name + ).dispense.flowRateByVolume["default"] + == 50 + ) + assert ( + glycerol_50.get_for( + pipette_left.name, tiprack.load_name + ).aspirate.submerge.speed + == 100 + ) + + with pytest.raises( + ValueError, + match="No properties found for p300_multi in fixture_glycerol50 liquid class", + ): + glycerol_50.get_for(pipette_right.name, tiprack.load_name) + + with pytest.raises(AttributeError): + glycerol_50.name = "foo" # type: ignore + + with pytest.raises(AttributeError): + glycerol_50.display_name = "bar" # type: ignore + + with pytest.raises(ValueError, match="Liquid class definition not found"): + protocol_context.define_liquid_class("non-existent-liquid") + + +def test_liquid_class_feature_flag() -> None: + """It should raise a not implemented error without the allowLiquidClass flag set.""" + protocol_context = simulate.get_protocol_api(version="2.20", robot_type="OT-2") + with pytest.raises(NotImplementedError): + protocol_context.define_liquid_class("fixture_glycerol50") diff --git a/app/src/App/DesktopApp.tsx b/app/src/App/DesktopApp.tsx index 27d7fd4f238..6b09da2e2d2 100644 --- a/app/src/App/DesktopApp.tsx +++ b/app/src/App/DesktopApp.tsx @@ -14,6 +14,7 @@ import NiceModal from '@ebay/nice-modal-react' import { LocalizationProvider } from '/app/LocalizationProvider' import { Alerts } from '/app/organisms/Desktop/Alerts' import { Breadcrumbs } from '/app/organisms/Desktop/Breadcrumbs' +import { SystemLanguagePreferenceModal } from '/app/organisms/Desktop/SystemLanguagePreferenceModal' import { ToasterOven } from '/app/organisms/ToasterOven' import { CalibrationDashboard } from '/app/pages/Desktop/Devices/CalibrationDashboard' import { DeviceDetails } from '/app/pages/Desktop/Devices/DeviceDetails' @@ -107,6 +108,7 @@ export const DesktopApp = (): JSX.Element => { + { vi.mocked(RobotSettings).mockReturnValue(
Mock RobotSettings
) vi.mocked(GeneralSettings).mockReturnValue(
Mock AppSettings
) vi.mocked(Breadcrumbs).mockReturnValue(
Mock Breadcrumbs
) + vi.mocked(SystemLanguagePreferenceModal).mockReturnValue( +
Mock SystemLanguagePreferenceModal
+ ) vi.mocked(AlertsModal).mockReturnValue(<>) vi.mocked(useIsFlex).mockReturnValue(true) vi.mocked( @@ -85,6 +90,11 @@ describe('DesktopApp', () => { screen.getByText('Mock Breadcrumbs') }) + it('renders a SystemLanguagePreferenceModal component', () => { + render('/protocols') + screen.getByText('Mock SystemLanguagePreferenceModal') + }) + it('renders an AppSettings component', () => { render('/app-settings/general') screen.getByText('Mock AppSettings') diff --git a/app/src/LocalizationProvider.tsx b/app/src/LocalizationProvider.tsx index b8fc0149673..df2bbc8bc40 100644 --- a/app/src/LocalizationProvider.tsx +++ b/app/src/LocalizationProvider.tsx @@ -1,14 +1,14 @@ -import type * as React from 'react' import { I18nextProvider } from 'react-i18next' import { useSelector } from 'react-redux' import reduce from 'lodash/reduce' import { resources } from '/app/assets/localization' import { i18n, i18nCb, i18nConfig } from '/app/i18n' -import { getAppLanguage, getStoredSystemLanguage } from '/app/redux/config' -import { getSystemLanguage } from '/app/redux/shell' +import { getAppLanguage } from '/app/redux/config' import { useIsOEMMode } from '/app/resources/robot-settings/hooks' +import type * as React from 'react' + export interface LocalizationProviderProps { children?: React.ReactNode } @@ -22,16 +22,6 @@ export function LocalizationProvider( const isOEMMode = useIsOEMMode() const language = useSelector(getAppLanguage) - const systemLanguage = useSelector(getSystemLanguage) - const storedSystemLanguage = useSelector(getStoredSystemLanguage) - - // TODO(bh, 2024-10-09): desktop app, check for current system language vs stored config system language value, launch modal - console.log( - 'redux systemLanguage', - systemLanguage, - 'storedSystemLanguage', - storedSystemLanguage - ) // iterate through language resources, nested files, substitute anonymous file for branded file for OEM mode const anonResources = reduce( diff --git a/app/src/assets/localization/en/anonymous.json b/app/src/assets/localization/en/anonymous.json index 5c1db25d10d..280e602088d 100644 --- a/app/src/assets/localization/en/anonymous.json +++ b/app/src/assets/localization/en/anonymous.json @@ -30,6 +30,7 @@ "gripper_successfully_detached": "Gripper successfully detached", "help_us_improve_send_error_report": "Help us improve your experience by sending an error report to support", "ip_description_second": "Work with your network administrator to assign a static IP address to the robot.", + "language_preference_description": "The app matches your system language unless you select another language below. You can change the language later in the app settings.", "learn_uninstalling": "Learn more about uninstalling the app", "loosen_screws_and_detach": "Loosen screws and detach gripper", "modal_instructions": "For step-by-step instructions on setting up your module, consult the Quickstart Guide that came in its box.", @@ -68,6 +69,7 @@ "show_labware_offset_snippets_description": "Only for users who need to apply labware offset data outside of the app. When enabled, code snippets for Jupyter Notebook and SSH are available during protocol setup.", "something_seems_wrong": "There may be a problem with your pipette. Exit setup and contact support for assistance.", "storage_limit_reached_description": "Your robot has reached the limit of quick transfers that it can store. You must delete an existing quick transfer before creating a new one.", + "system_language_preferences_update_description": "Your system’s language was recently updated. Would you like to use the updated language as the default for the app?", "these_are_advanced_settings": "These are advanced settings. Please do not attempt to adjust without assistance from support. Changing these settings may affect the lifespan of your pipette.These settings do not override any pipette settings defined in protocols.", "unexpected_error": "An unexpected error has occurred. If the issue persists, contact customer support for assistance.", "update_requires_restarting_app": "Updating requires restarting the app.", diff --git a/app/src/assets/localization/en/app_settings.json b/app/src/assets/localization/en/app_settings.json index 1b3fcce0a07..5f6354da6c5 100644 --- a/app/src/assets/localization/en/app_settings.json +++ b/app/src/assets/localization/en/app_settings.json @@ -31,6 +31,7 @@ "connect_ip_button": "Done", "connect_ip_link": "Learn more about connecting a robot manually", "discovery_timeout": "Discovery timed out.", + "dont_change": "Don’t change", "download_update": "Downloading update...", "enable_dev_tools": "Developer Tools", "enable_dev_tools_description": "Enabling this setting opens Developer Tools on app launch, enables additional logging and gives access to feature flags.", @@ -46,6 +47,7 @@ "installing_update": "Installing update...", "ip_available": "Available", "ip_description_first": "Enter an IP address or hostname to connect to a robot.", + "language_preference": "Language preference", "manage_versions": "It is very important for the robot and app software to be on the same version. Manage the robot software versions via Robot Settings > Advanced.", "new_features": "New Features", "no_folder": "No additional source folder specified", @@ -71,6 +73,7 @@ "restarting_app": "Download complete, restarting the app...", "restore_previous": "See how to restore a previous software version", "searching": "Searching for 30s", + "select_language": "Select language", "setup_connection": "Set up connection", "share_display_usage": "Share display usage", "share_robot_logs": "Share robot logs", @@ -79,6 +82,7 @@ "software_update_available": "Software Update Available", "software_version": "App Software Version", "successfully_deleted_unavail_robots": "Successfully deleted unavailable robots", + "system_language_preferences_update": "Update to your system language preferences", "tip_length_cal_method": "Tip Length Calibration Method", "trash_bin": "Always use trash bin to calibrate", "try_restarting_the_update": "Try restarting the update.", @@ -100,6 +104,7 @@ "usb_to_ethernet_not_connected": "No USB-to-Ethernet adapter connected", "usb_to_ethernet_unknown_manufacturer": "Unknown Manufacturer", "usb_to_ethernet_unknown_product": "Unknown Adapter", + "use_system_language": "Use system language", "view_software_update": "View software update", "view_update": "View Update" } diff --git a/app/src/assets/localization/en/branded.json b/app/src/assets/localization/en/branded.json index c2fa52cf885..0760c3061b4 100644 --- a/app/src/assets/localization/en/branded.json +++ b/app/src/assets/localization/en/branded.json @@ -30,6 +30,7 @@ "gripper_successfully_detached": "Flex Gripper successfully detached", "help_us_improve_send_error_report": "Help us improve your experience by sending an error report to {{support_email}}", "ip_description_second": "Opentrons recommends working with your network administrator to assign a static IP address to the robot.", + "language_preference_description": "The Opentrons App matches your system language unless you select another language below. You can change the language later in the app settings.", "learn_uninstalling": "Learn more about uninstalling the Opentrons App", "loosen_screws_and_detach": "Loosen screws and detach Flex Gripper", "modal_instructions": "For step-by-step instructions on setting up your module, consult the Quickstart Guide that came in its box. You can also click the link below or scan the QR code to visit the modules section of the Opentrons Help Center.", @@ -68,6 +69,7 @@ "show_labware_offset_snippets_description": "Only for users who need to apply labware offset data outside of the Opentrons App. When enabled, code snippets for Jupyter Notebook and SSH are available during protocol setup.", "something_seems_wrong": "There may be a problem with your pipette. Exit setup and contact Opentrons Support for assistance.", "storage_limit_reached_description": "Your Opentrons Flex has reached the limit of quick transfers that it can store. You must delete an existing quick transfer before creating a new one.", + "system_language_preferences_update_description": "Your system’s language was recently updated. Would you like to use the updated language as the default for the Opentrons App?", "these_are_advanced_settings": "These are advanced settings. Please do not attempt to adjust without assistance from Opentrons Support. Changing these settings may affect the lifespan of your pipette.These settings do not override any pipette settings defined in protocols.", "unexpected_error": "An unexpected error has occurred. If the issue persists, contact Opentrons Support for assistance.", "update_requires_restarting_app": "Updating requires restarting the Opentrons App.", diff --git a/app/src/organisms/Desktop/SystemLanguagePreferenceModal/__tests__/SystemLanguagePreferenceModal.test.tsx b/app/src/organisms/Desktop/SystemLanguagePreferenceModal/__tests__/SystemLanguagePreferenceModal.test.tsx new file mode 100644 index 00000000000..3a378d5f04f --- /dev/null +++ b/app/src/organisms/Desktop/SystemLanguagePreferenceModal/__tests__/SystemLanguagePreferenceModal.test.tsx @@ -0,0 +1,183 @@ +import { useNavigate } from 'react-router-dom' +import { fireEvent, screen } from '@testing-library/react' +import '@testing-library/jest-dom/vitest' +import { describe, it, vi, afterEach, beforeEach, expect } from 'vitest' +import { when } from 'vitest-when' + +import { renderWithProviders } from '/app/__testing-utils__' +import { i18n } from '/app/i18n' +import { + getAppLanguage, + getStoredSystemLanguage, + updateConfigValue, + useFeatureFlag, +} from '/app/redux/config' +import { getSystemLanguage } from '/app/redux/shell' +import { SystemLanguagePreferenceModal } from '..' + +vi.mock('react-router-dom') +vi.mock('/app/redux/config') +vi.mock('/app/redux/shell') + +const render = () => { + return renderWithProviders(, { + i18nInstance: i18n, + })[0] +} + +const mockNavigate = vi.fn() + +const MOCK_DEFAULT_LANGUAGE = 'en-US' + +describe('SystemLanguagePreferenceModal', () => { + beforeEach(() => { + vi.mocked(getAppLanguage).mockReturnValue(MOCK_DEFAULT_LANGUAGE) + vi.mocked(getSystemLanguage).mockReturnValue(MOCK_DEFAULT_LANGUAGE) + vi.mocked(getStoredSystemLanguage).mockReturnValue(MOCK_DEFAULT_LANGUAGE) + when(vi.mocked(useFeatureFlag)) + .calledWith('enableLocalization') + .thenReturn(true) + vi.mocked(useNavigate).mockReturnValue(mockNavigate) + }) + afterEach(() => { + vi.resetAllMocks() + }) + + it('should render null when app language and system language are set', () => { + render() + expect(screen.queryByText('Language preference')).toBeNull() + expect( + screen.queryByText('Update to your system language preferences') + ).toBeNull() + }) + + it('should render the correct header, description, and buttons on first boot', () => { + vi.mocked(getAppLanguage).mockReturnValue(null) + + render() + + screen.getByText('Language preference') + screen.getByText( + 'The Opentrons App matches your system language unless you select another language below. You can change the language later in the app settings.' + ) + const primaryButton = screen.getByRole('button', { + name: 'Continue', + }) + + fireEvent.click(primaryButton) + expect(updateConfigValue).toBeCalledWith( + 'language.appLanguage', + MOCK_DEFAULT_LANGUAGE + ) + expect(updateConfigValue).toBeCalledWith( + 'language.systemLanguage', + MOCK_DEFAULT_LANGUAGE + ) + }) + + it('should default to English (US) if system language is unsupported', () => { + vi.mocked(getAppLanguage).mockReturnValue(null) + vi.mocked(getSystemLanguage).mockReturnValue('es-MX') + + render() + + screen.getByText('Language preference') + screen.getByText( + 'The Opentrons App matches your system language unless you select another language below. You can change the language later in the app settings.' + ) + const primaryButton = screen.getByRole('button', { + name: 'Continue', + }) + + fireEvent.click(primaryButton) + expect(updateConfigValue).toBeCalledWith( + 'language.appLanguage', + MOCK_DEFAULT_LANGUAGE + ) + expect(updateConfigValue).toBeCalledWith('language.systemLanguage', 'es-MX') + }) + + it('should set a supported app language when system language is an unsupported locale of the same language', () => { + vi.mocked(getAppLanguage).mockReturnValue(null) + vi.mocked(getSystemLanguage).mockReturnValue('en-UK') + + render() + + screen.getByText('Language preference') + screen.getByText( + 'The Opentrons App matches your system language unless you select another language below. You can change the language later in the app settings.' + ) + const primaryButton = screen.getByRole('button', { + name: 'Continue', + }) + + fireEvent.click(primaryButton) + expect(updateConfigValue).toBeCalledWith( + 'language.appLanguage', + MOCK_DEFAULT_LANGUAGE + ) + expect(updateConfigValue).toBeCalledWith('language.systemLanguage', 'en-UK') + }) + + it('should render the correct header, description, and buttons when system language changes', () => { + vi.mocked(getSystemLanguage).mockReturnValue('zh-CN') + + render() + + screen.getByText('Update to your system language preferences') + screen.getByText( + 'Your system’s language was recently updated. Would you like to use the updated language as the default for the Opentrons App?' + ) + const secondaryButton = screen.getByRole('button', { name: 'Don’t change' }) + const primaryButton = screen.getByRole('button', { + name: 'Use system language', + }) + + fireEvent.click(primaryButton) + expect(updateConfigValue).toHaveBeenNthCalledWith( + 1, + 'language.appLanguage', + 'zh-CN' + ) + expect(updateConfigValue).toHaveBeenNthCalledWith( + 2, + 'language.systemLanguage', + 'zh-CN' + ) + fireEvent.click(secondaryButton) + expect(updateConfigValue).toHaveBeenNthCalledWith( + 3, + 'language.systemLanguage', + 'zh-CN' + ) + }) + + it('should set a supported app language when system language changes to an unsupported locale of the same language', () => { + vi.mocked(getSystemLanguage).mockReturnValue('zh-Hant') + + render() + + const secondaryButton = screen.getByRole('button', { name: 'Don’t change' }) + const primaryButton = screen.getByRole('button', { + name: 'Use system language', + }) + + fireEvent.click(primaryButton) + expect(updateConfigValue).toHaveBeenNthCalledWith( + 1, + 'language.appLanguage', + 'zh-CN' + ) + expect(updateConfigValue).toHaveBeenNthCalledWith( + 2, + 'language.systemLanguage', + 'zh-Hant' + ) + fireEvent.click(secondaryButton) + expect(updateConfigValue).toHaveBeenNthCalledWith( + 3, + 'language.systemLanguage', + 'zh-Hant' + ) + }) +}) diff --git a/app/src/organisms/Desktop/SystemLanguagePreferenceModal/index.tsx b/app/src/organisms/Desktop/SystemLanguagePreferenceModal/index.tsx new file mode 100644 index 00000000000..42ff74a0eb8 --- /dev/null +++ b/app/src/organisms/Desktop/SystemLanguagePreferenceModal/index.tsx @@ -0,0 +1,143 @@ +import { useEffect, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { useDispatch, useSelector } from 'react-redux' + +import { + ALIGN_CENTER, + DIRECTION_COLUMN, + DropdownMenu, + Flex, + JUSTIFY_FLEX_END, + Modal, + PrimaryButton, + SecondaryButton, + SPACING, + StyledText, +} from '@opentrons/components' + +import { + getAppLanguage, + getStoredSystemLanguage, + updateConfigValue, + useFeatureFlag, +} from '/app/redux/config' +import { getSystemLanguage } from '/app/redux/shell' + +import type { DropdownOption } from '@opentrons/components' +import type { Dispatch } from '/app/redux/types' + +// these strings will not be translated so should not be localized +const languageOptions: DropdownOption[] = [ + { name: 'English (US)', value: 'en-US' }, + { name: '中文', value: 'zh-CN' }, +] + +export function SystemLanguagePreferenceModal(): JSX.Element | null { + const { i18n, t } = useTranslation(['app_settings', 'shared', 'branded']) + const enableLocalization = useFeatureFlag('enableLocalization') + + const [currentOption, setCurrentOption] = useState( + languageOptions[0] + ) + + const dispatch = useDispatch() + + const appLanguage = useSelector(getAppLanguage) + const systemLanguage = useSelector(getSystemLanguage) + const storedSystemLanguage = useSelector(getStoredSystemLanguage) + + const showBootModal = appLanguage == null && systemLanguage != null + const showUpdateModal = + appLanguage != null && + systemLanguage != null && + storedSystemLanguage != null && + systemLanguage !== storedSystemLanguage + + const title = showUpdateModal + ? t('system_language_preferences_update') + : t('language_preference') + + const description = showUpdateModal + ? t('branded:system_language_preferences_update_description') + : t('branded:language_preference_description') + + const primaryButtonText = showUpdateModal + ? t('use_system_language') + : i18n.format(t('shared:continue'), 'capitalize') + + const handleSecondaryClick = (): void => { + // if user chooses "Don't change" on system language update, stored the new system language but don't update the app language + dispatch(updateConfigValue('language.systemLanguage', systemLanguage)) + } + + const handlePrimaryClick = (): void => { + dispatch(updateConfigValue('language.appLanguage', currentOption.value)) + dispatch(updateConfigValue('language.systemLanguage', systemLanguage)) + } + + const handleDropdownClick = (value: string): void => { + const selectedOption = languageOptions.find(lng => lng.value === value) + + if (selectedOption != null) { + setCurrentOption(selectedOption) + void i18n.changeLanguage(selectedOption.value) + } + } + + // set initial language for boot modal; match app language to supported options + useEffect(() => { + if (systemLanguage != null) { + // prefer match entire locale, then match just language e.g. zh-Hant and zh-CN + const matchedSystemLanguageOption = + languageOptions.find(lng => lng.value === systemLanguage) ?? + languageOptions.find( + lng => + new Intl.Locale(lng.value).language === + new Intl.Locale(systemLanguage).language + ) + if (matchedSystemLanguageOption != null) { + // initial current option: set to detected system language + setCurrentOption(matchedSystemLanguageOption) + if (showBootModal) { + // for boot modal temp change app display language based on initial system locale + void i18n.changeLanguage(systemLanguage) + } + } + } + }, [i18n, systemLanguage, showBootModal]) + + return enableLocalization && (showBootModal || showUpdateModal) ? ( + + + + + {description} + + {showBootModal ? ( + + ) : null} + + + {showUpdateModal ? ( + + {t('dont_change')} + + ) : null} + + {primaryButtonText} + + + + + ) : null +} diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/index.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/index.ts index da85e8b770e..baa685c0dcc 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/index.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/index.ts @@ -1,5 +1,4 @@ export { useCurrentlyRecoveringFrom } from './useCurrentlyRecoveringFrom' -export { useErrorMessage } from './useErrorMessage' export { useErrorName } from './useErrorName' export { useRecoveryCommands } from './useRecoveryCommands' export { useRouteUpdateActions } from './useRouteUpdateActions' diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useErrorMessage.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useErrorMessage.ts deleted file mode 100644 index d8efb33d4eb..00000000000 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useErrorMessage.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { useTranslation } from 'react-i18next' - -import type { ErrorKind } from '../types' - -// The generalized error message shown to the user in select locations. -export function useErrorMessage(errorKind: ErrorKind): string { - const { t } = useTranslation('error_recovery') - - switch (errorKind) { - default: - return t('general_error_message') - } -} diff --git a/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts b/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts index 55399fa1d22..1aa7080a52a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/utils/__tests__/getErrorKind.test.ts @@ -6,93 +6,74 @@ import { getErrorKind } from '../getErrorKind' import type { RunCommandError, RunTimeCommand } from '@opentrons/shared-data' describe('getErrorKind', () => { - it(`returns ${ERROR_KINDS.NO_LIQUID_DETECTED} for ${DEFINED_ERROR_TYPES.LIQUID_NOT_FOUND} errorType`, () => { - const result = getErrorKind({ - commandType: 'liquidProbe', - error: { - isDefined: true, - errorType: DEFINED_ERROR_TYPES.LIQUID_NOT_FOUND, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.NO_LIQUID_DETECTED) - }) - - it(`returns ${ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING} for ${DEFINED_ERROR_TYPES.OVERPRESSURE} errorType`, () => { - const result = getErrorKind({ + it.each([ + { commandType: 'aspirate', - error: { - isDefined: true, - errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING) - }) - - it(`returns ${ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING} for ${DEFINED_ERROR_TYPES.OVERPRESSURE} errorType`, () => { - const result = getErrorKind({ + errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, + expectedError: ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING, + }, + { + commandType: 'aspirateInPlace', + errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, + expectedError: ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING, + }, + { commandType: 'dispense', - error: { - isDefined: true, - errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING) - }) - - it(`returns ${ERROR_KINDS.TIP_NOT_DETECTED} for ${DEFINED_ERROR_TYPES.TIP_PHYSICALLY_MISSING} errorType`, () => { - const result = getErrorKind({ - commandType: 'pickUpTip', - error: { - isDefined: true, - errorType: DEFINED_ERROR_TYPES.TIP_PHYSICALLY_MISSING, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.TIP_NOT_DETECTED) - }) - - it(`returns ${ERROR_KINDS.TIP_DROP_FAILED} for ${DEFINED_ERROR_TYPES.TIP_PHYSICALLY_ATTACHED} errorType`, () => { - const result = getErrorKind({ + errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, + expectedError: ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING, + }, + { + commandType: 'dispenseInPlace', + errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, + expectedError: ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING, + }, + { commandType: 'dropTip', - error: { - isDefined: true, - errorType: DEFINED_ERROR_TYPES.TIP_PHYSICALLY_ATTACHED, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.TIP_DROP_FAILED) - }) - - it(`returns ${ERROR_KINDS.GRIPPER_ERROR} for ${DEFINED_ERROR_TYPES.GRIPPER_MOVEMENT} errorType`, () => { - const result = getErrorKind({ + errorType: DEFINED_ERROR_TYPES.TIP_PHYSICALLY_ATTACHED, + expectedError: ERROR_KINDS.TIP_DROP_FAILED, + }, + { + commandType: 'dropTipInPlace', + errorType: DEFINED_ERROR_TYPES.TIP_PHYSICALLY_ATTACHED, + expectedError: ERROR_KINDS.TIP_DROP_FAILED, + }, + { + commandType: 'liquidProbe', + errorType: DEFINED_ERROR_TYPES.LIQUID_NOT_FOUND, + expectedError: ERROR_KINDS.NO_LIQUID_DETECTED, + }, + { + commandType: 'pickUpTip', + errorType: DEFINED_ERROR_TYPES.TIP_PHYSICALLY_MISSING, + expectedError: ERROR_KINDS.TIP_NOT_DETECTED, + }, + { commandType: 'moveLabware', - error: { - isDefined: true, - errorType: DEFINED_ERROR_TYPES.GRIPPER_MOVEMENT, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.GRIPPER_ERROR) - }) - - it(`returns ${ERROR_KINDS.GENERAL_ERROR} for undefined errors`, () => { - const result = getErrorKind({ + errorType: DEFINED_ERROR_TYPES.GRIPPER_MOVEMENT, + expectedError: ERROR_KINDS.GRIPPER_ERROR, + }, + { commandType: 'aspirate', - error: { - isDefined: false, - // It should treat this error as undefined because isDefined===false, - // even though the errorType happens to match a defined error. - errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, - } as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.GENERAL_ERROR) - }) - - it(`returns ${ERROR_KINDS.GENERAL_ERROR} for defined errors not handled explicitly`, () => { - const result = getErrorKind({ + errorType: DEFINED_ERROR_TYPES.OVERPRESSURE, + isDefined: false, + expectedError: ERROR_KINDS.GENERAL_ERROR, + }, + { commandType: 'aspirate', - error: ({ - isDefined: true, - errorType: 'someHithertoUnknownDefinedErrorType', - } as unknown) as RunCommandError, - } as RunTimeCommand) - expect(result).toEqual(ERROR_KINDS.GENERAL_ERROR) - }) + errorType: 'someHithertoUnknownDefinedErrorType', + expectedError: ERROR_KINDS.GENERAL_ERROR, + }, + ])( + 'returns $expectedError for $commandType with errorType $errorType', + ({ commandType, errorType, expectedError, isDefined = true }) => { + const result = getErrorKind({ + commandType, + error: { + isDefined, + errorType, + } as RunCommandError, + } as RunTimeCommand) + expect(result).toEqual(expectedError) + } + ) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/utils/getErrorKind.ts b/app/src/organisms/ErrorRecoveryFlows/utils/getErrorKind.ts index 9c96230836b..a537c3cf295 100644 --- a/app/src/organisms/ErrorRecoveryFlows/utils/getErrorKind.ts +++ b/app/src/organisms/ErrorRecoveryFlows/utils/getErrorKind.ts @@ -16,12 +16,12 @@ export function getErrorKind(failedCommand: RunTimeCommand | null): ErrorKind { // todo(mm, 2024-07-02): Also handle aspirateInPlace and dispenseInPlace. // https://opentrons.atlassian.net/browse/EXEC-593 if ( - commandType === 'aspirate' && + (commandType === 'aspirate' || commandType === 'aspirateInPlace') && errorType === DEFINED_ERROR_TYPES.OVERPRESSURE ) { return ERROR_KINDS.OVERPRESSURE_WHILE_ASPIRATING } else if ( - commandType === 'dispense' && + (commandType === 'dispense' || commandType === 'dispenseInPlace') && errorType === DEFINED_ERROR_TYPES.OVERPRESSURE ) { return ERROR_KINDS.OVERPRESSURE_WHILE_DISPENSING diff --git a/app/src/redux/config/schema-types.ts b/app/src/redux/config/schema-types.ts index 17d4a1ca211..ce4af4ddaeb 100644 --- a/app/src/redux/config/schema-types.ts +++ b/app/src/redux/config/schema-types.ts @@ -31,7 +31,7 @@ export type QuickTransfersOnDeviceSortKey = | 'recentCreated' | 'oldCreated' -export type Language = 'en' | 'zh' +export type Language = 'en-US' | 'zh-CN' export interface OnDeviceDisplaySettings { sleepMs: number diff --git a/app/src/redux/config/selectors.ts b/app/src/redux/config/selectors.ts index 59647b61410..4069aa7320b 100644 --- a/app/src/redux/config/selectors.ts +++ b/app/src/redux/config/selectors.ts @@ -159,7 +159,7 @@ export const getUserId: (state: State) => string = createSelector( export const getAppLanguage: (state: State) => Language | null = createSelector( getConfig, - config => config?.language.appLanguage ?? 'en' + config => config?.language.appLanguage ?? null ) export const getStoredSystemLanguage: ( diff --git a/labware-library/cssModuleSideEffect.ts b/labware-library/cssModuleSideEffect.ts new file mode 100644 index 00000000000..312475debb1 --- /dev/null +++ b/labware-library/cssModuleSideEffect.ts @@ -0,0 +1,22 @@ +import type { Plugin } from 'vite' + +/** + * Plugin to make sure CSS modules do not get tree shaked out of the dist. + * see https://github.com/vitejs/vite/pull/16051 + * + * @returns {Plugin} The Vite plugin object. + */ + +export const cssModuleSideEffect = (): Plugin => { + return { + name: 'css-module-side-effectful', + enforce: 'post', + transform(_: string, id: string) { + if (id.includes('.module.')) { + return { + moduleSideEffects: 'no-treeshake', // or true, which also works with slightly better treeshake + } + } + }, + } +} diff --git a/labware-library/vite.config.mts b/labware-library/vite.config.mts index cd9b5687f86..43d5065c011 100644 --- a/labware-library/vite.config.mts +++ b/labware-library/vite.config.mts @@ -6,6 +6,7 @@ import postCssApply from 'postcss-apply' import postColorModFunction from 'postcss-color-mod-function' import postCssPresetEnv from 'postcss-preset-env' import lostCss from 'lost' +import { cssModuleSideEffect } from './cssModuleSideEffect' const testAliases: {} | { 'file-saver': string } = process.env.CYPRESS === '1' @@ -37,6 +38,7 @@ export default defineConfig({ configFile: true, }, }), + cssModuleSideEffect(), // Note for treeshake ], optimizeDeps: { esbuildOptions: { diff --git a/protocol-designer/src/utils/__tests__/utils.test.ts b/protocol-designer/src/utils/__tests__/utils.test.ts index 460722cdba7..a389f259232 100644 --- a/protocol-designer/src/utils/__tests__/utils.test.ts +++ b/protocol-designer/src/utils/__tests__/utils.test.ts @@ -43,4 +43,10 @@ describe('removeOpentronsPhrases', () => { const expectedOutput = '' expect(removeOpentronsPhrases(input)).toBe(expectedOutput) }) + + it('should remove "Eppendorf" from input ', () => { + const input = 'Eppendorf epT.I.P.S. Tip Rack is long' + const expectedOutput = 'epT.I.P.S. Tip Rack is long' + expect(removeOpentronsPhrases(input)).toBe(expectedOutput) + }) }) diff --git a/protocol-designer/src/utils/index.ts b/protocol-designer/src/utils/index.ts index 73c0a3291fc..8f4c9397066 100644 --- a/protocol-designer/src/utils/index.ts +++ b/protocol-designer/src/utils/index.ts @@ -267,6 +267,7 @@ export const removeOpentronsPhrases = (input: string): string => { 'Opentrons OT-2 96', '\\(Retired\\)', '96', + 'Eppendorf', ] const updatedText = phrasesToRemove diff --git a/shared-data/liquid-class/fixtures/fixture_glycerol50.json b/shared-data/liquid-class/fixtures/fixture_glycerol50.json index 364715b99eb..4b8a3480474 100644 --- a/shared-data/liquid-class/fixtures/fixture_glycerol50.json +++ b/shared-data/liquid-class/fixtures/fixture_glycerol50.json @@ -1,5 +1,6 @@ { - "liquidName": "Glycerol 50%", + "liquidClassName": "fixture_glycerol50", + "displayName": "Glycerol 50%", "schemaVersion": 1, "namespace": "opentrons", "byPipette": [ @@ -7,7 +8,7 @@ "pipetteModel": "p20_single_gen2", "byTipType": [ { - "tipType": "p20_tip", + "tiprack": "opentrons_96_tiprack_20ul", "aspirate": { "submerge": { "positionReference": "liquid-meniscus", diff --git a/shared-data/liquid-class/schemas/1.json b/shared-data/liquid-class/schemas/1.json index bd987442b37..1a5eb18d51a 100644 --- a/shared-data/liquid-class/schemas/1.json +++ b/shared-data/liquid-class/schemas/1.json @@ -418,9 +418,13 @@ } }, "properties": { - "liquidName": { + "liquidClassName": { + "$ref": "#/definitions/safeString", + "description": "The name of the liquid class specified when loading into protocol (e.g., water, ethanol, serum). Should be the same as file name." + }, + "displayName": { "type": "string", - "description": "The name of the liquid (e.g., water, ethanol, serum)." + "description": "User-readable name of the liquid class." }, "schemaVersion": { "description": "Which schema version a liquid class is using", @@ -447,9 +451,9 @@ "items": { "type": "object", "properties": { - "tipType": { + "tiprack": { "type": "string", - "description": "The tip type whose properties will be used when handling this specific liquid class with this pipette" + "description": "The tiprack name whose tip will be used when handling this specific liquid class with this pipette" }, "aspirate": { "$ref": "#/definitions/aspirateParams" @@ -461,7 +465,7 @@ "$ref": "#/definitions/multiDispenseParams" } }, - "required": ["tipType", "aspirate", "singleDispense"], + "required": ["tiprack", "aspirate", "singleDispense"], "additionalProperties": false } } @@ -471,6 +475,12 @@ } } }, - "required": ["liquidName", "schemaVersion", "namespace", "byPipette"], + "required": [ + "liquidClassName", + "displayName", + "schemaVersion", + "namespace", + "byPipette" + ], "additionalProperties": false } diff --git a/shared-data/python/opentrons_shared_data/liquid_classes/__init__.py b/shared-data/python/opentrons_shared_data/liquid_classes/__init__.py index b17bec7bf60..da4f2b1fd43 100644 --- a/shared-data/python/opentrons_shared_data/liquid_classes/__init__.py +++ b/shared-data/python/opentrons_shared_data/liquid_classes/__init__.py @@ -1 +1,25 @@ -"""opentrons_shared_data.liquid_classes: types and functions for accessing liquid class definitions.""" +"""Types and functions for accessing liquid class definitions.""" +import json + +from .. import load_shared_data +from .liquid_class_definition import LiquidClassSchemaV1 + + +class LiquidClassDefinitionDoesNotExist(Exception): + """Specified liquid class definition does not exist.""" + + +# TODO (spp, 2024-10-16): update the path once definitions are added +def load_definition(name: str) -> LiquidClassSchemaV1: + """Load the specified liquid class' definition as a LiquidClassSchemaV1 object. + + Note: this is an expensive operation and should be called sparingly. + """ + try: + return LiquidClassSchemaV1.parse_obj( + json.loads(load_shared_data(f"liquid-class/fixtures/{name}.json")) + ) + except FileNotFoundError: + raise LiquidClassDefinitionDoesNotExist( + f"No definition found for liquid class '{name}'" + ) diff --git a/shared-data/python/opentrons_shared_data/liquid_classes/liquid_class_definition.py b/shared-data/python/opentrons_shared_data/liquid_classes/liquid_class_definition.py index 6b4599227ed..8247bd2b760 100644 --- a/shared-data/python/opentrons_shared_data/liquid_classes/liquid_class_definition.py +++ b/shared-data/python/opentrons_shared_data/liquid_classes/liquid_class_definition.py @@ -312,9 +312,9 @@ class MultiDispenseProperties(BaseModel): class ByTipTypeSetting(BaseModel): """Settings for each kind of tip this pipette can use.""" - tipType: str = Field( + tiprack: str = Field( ..., - description="The tip type whose properties will be used when handling this specific liquid class with this pipette", + description="The name of tiprack whose tip will be used when handling this specific liquid class with this pipette", ) aspirate: AspirateProperties = Field( ..., description="Aspirate parameters for this tip type." @@ -339,9 +339,10 @@ class ByPipetteSetting(BaseModel): class LiquidClassSchemaV1(BaseModel): """Defines a single liquid class's properties for liquid handling functions.""" - liquidName: str = Field( + liquidClassName: str = Field( ..., description="The name of the liquid (e.g., water, ethanol, serum)." ) + displayName: str = Field(..., description="User-readable name of the liquid class.") schemaVersion: Literal[1] = Field( ..., description="Which schema version a liquid class is using" ) diff --git a/shared-data/python/tests/gripper/test_load.py b/shared-data/python/tests/gripper/test_load.py index bc84c635d07..4a8d42a7a24 100644 --- a/shared-data/python/tests/gripper/test_load.py +++ b/shared-data/python/tests/gripper/test_load.py @@ -2,6 +2,7 @@ from opentrons_shared_data.gripper import load_definition, load_schema from opentrons_shared_data.gripper.gripper_definition import ( GripperModel, + GripperDefinition, ) from opentrons_shared_data import load_shared_data @@ -11,6 +12,8 @@ def test_load_schema() -> None: def test_load_definition() -> None: - load_definition(GripperModel.v1, 1) == json.loads( - load_shared_data("gripper/definitions/1/gripperV1.json") - ) + gripper_def = load_definition(GripperModel.v1, 1) + assert type(gripper_def) is GripperDefinition + assert gripper_def.model == GripperModel.v1 + assert gripper_def.grip_force_profile.default_grip_force == 15 + assert gripper_def.geometry.base_offset_from_mount == (19.5, -74.325, -94.825) diff --git a/shared-data/python/tests/liquid_classes/test_load.py b/shared-data/python/tests/liquid_classes/test_load.py new file mode 100644 index 00000000000..be2c9731983 --- /dev/null +++ b/shared-data/python/tests/liquid_classes/test_load.py @@ -0,0 +1,34 @@ +import json + +from opentrons_shared_data import load_shared_data +from opentrons_shared_data.liquid_classes import load_definition +from opentrons_shared_data.liquid_classes.liquid_class_definition import ( + LiquidClassSchemaV1, + PositionReference, + Coordinate, + Submerge, + DelayParams, + DelayProperties, +) + + +def test_load_liquid_class_schema_v1() -> None: + fixture_data = load_shared_data("liquid-class/fixtures/fixture_glycerol50.json") + liquid_class_model = LiquidClassSchemaV1.parse_raw(fixture_data) + liquid_class_def_from_model = json.loads( + liquid_class_model.json(exclude_unset=True) + ) + expected_liquid_class_def = json.loads(fixture_data) + assert liquid_class_def_from_model == expected_liquid_class_def + + +def test_load_definition() -> None: + glycerol_definition = load_definition("fixture_glycerol50") + assert type(glycerol_definition) is LiquidClassSchemaV1 + assert glycerol_definition.byPipette[0].pipetteModel == "p20_single_gen2" + assert glycerol_definition.byPipette[0].byTipType[0].aspirate.submerge == Submerge( + positionReference=PositionReference.LIQUID_MENISCUS, + offset=Coordinate(x=0, y=0, z=-5), + speed=100, + delay=DelayProperties(enable=True, params=DelayParams(duration=1.5)), + ) diff --git a/shared-data/python/tests/liquid_classes/test_load_schema.py b/shared-data/python/tests/liquid_classes/test_load_schema.py deleted file mode 100644 index c29393a8b2b..00000000000 --- a/shared-data/python/tests/liquid_classes/test_load_schema.py +++ /dev/null @@ -1,16 +0,0 @@ -import json - -from opentrons_shared_data import load_shared_data -from opentrons_shared_data.liquid_classes.liquid_class_definition import ( - LiquidClassSchemaV1, -) - - -def test_load_liquid_class_schema_v1() -> None: - fixture_data = load_shared_data("liquid-class/fixtures/fixture_glycerol50.json") - liquid_class_model = LiquidClassSchemaV1.parse_raw(fixture_data) - liquid_class_def_from_model = json.loads( - liquid_class_model.json(exclude_unset=True) - ) - expected_liquid_class_def = json.loads(fixture_data) - assert liquid_class_def_from_model == expected_liquid_class_def diff --git a/shared-data/python/tests/liquid_classes/test_validations.py b/shared-data/python/tests/liquid_classes/test_validations.py new file mode 100644 index 00000000000..4806e4f1a7c --- /dev/null +++ b/shared-data/python/tests/liquid_classes/test_validations.py @@ -0,0 +1,32 @@ +"""Tests that validate the built-in liquid class definitions.""" +import pytest +from typing import List + +from opentrons_shared_data import get_shared_data_root +from opentrons_shared_data.liquid_classes import load_definition + + +def _get_all_liquid_classes() -> List[str]: + # TODO (spp, 2024-10-16): update the path once definitions are added + return [ + deffile.stem + for deffile in (get_shared_data_root() / "liquid-class" / "fixtures").iterdir() + ] + + +@pytest.mark.parametrize("liquid_class_name", list(_get_all_liquid_classes())) +def test_validate_unique_pipette_keys(liquid_class_name: str) -> None: + """A liquid class definition should contain only one set of properties per pipette model.""" + definition_dict = load_definition(liquid_class_name) + pipette_models = [prop.pipetteModel for prop in definition_dict.byPipette] + assert len(pipette_models) == len(set(pipette_models)) + + +@pytest.mark.parametrize("liquid_class_name", list(_get_all_liquid_classes())) +def test_validate_unique_tip_keys(liquid_class_name: str) -> None: + """A liquid class definition should contain only one set of properties per tip type.""" + definition_dict = load_definition(liquid_class_name) + + for by_pip_prop in definition_dict.byPipette: + tipracks = [tip_prop.tiprack for tip_prop in by_pip_prop.byTipType] + assert len(tipracks) == len(set(tipracks))