Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

refactor(api): Save full nozzle map configuration and update state store accordingly #14529

Merged
merged 3 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""Common configuration command base models."""

from pydantic import BaseModel, Field
from typing import Optional
from dataclasses import dataclass
from opentrons.hardware_control.nozzle_manager import (
NozzleMap,
Expand All @@ -22,7 +21,7 @@ class PipetteNozzleLayoutResultMixin(BaseModel):
"""A nozzle layout result for updating the pipette state."""

pipette_id: str
nozzle_map: Optional[NozzleMap] = Field(
default=None,
nozzle_map: NozzleMap = Field(
...,
description="A dataclass object holding information about the current nozzle configuration.",
)
4 changes: 1 addition & 3 deletions api/src/opentrons/protocol_engine/execution/equipment.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ async def load_pipette(
)

pipette_id = pipette_id or self._model_utils.generate_id()

if not use_virtual_pipettes:
cache_request = {mount.to_hw_mount(): pipette_name_value}

Expand Down Expand Up @@ -244,7 +243,6 @@ async def load_pipette(
)
)
serial = serial_number or ""

return LoadedPipetteData(
pipette_id=pipette_id,
serial_number=serial,
Expand Down Expand Up @@ -390,7 +388,7 @@ async def configure_nozzle_layout(
primary_nozzle: Optional[str] = None,
front_right_nozzle: Optional[str] = None,
back_left_nozzle: Optional[str] = None,
) -> Optional[NozzleMap]:
) -> NozzleMap:
"""Ensure the requested nozzle layout is compatible with the current pipette.

Args:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
)

from ..types import FlowRates
from ...types import Point


@dataclass(frozen=True)
Expand All @@ -37,8 +36,7 @@ class LoadedStaticPipetteData:
float, pipette_definition.SupportedTipsDefinition
]
nominal_tip_overlap: Dict[str, float]
back_left_nozzle_offset: Point
front_right_nozzle_offset: Point
nozzle_map: NozzleMap


class VirtualPipetteDataProvider:
Expand Down Expand Up @@ -173,8 +171,7 @@ def _get_virtual_pipette_static_config_by_model(
nominal_tip_overlap=config.liquid_properties[
liquid_class
].tip_overlap_dictionary,
back_left_nozzle_offset=nozzle_manager.current_configuration.back_left_nozzle_offset,
front_right_nozzle_offset=nozzle_manager.current_configuration.front_right_nozzle_offset,
nozzle_map=nozzle_manager.current_configuration,
)

def get_virtual_pipette_static_config(
Expand Down Expand Up @@ -208,11 +205,5 @@ def get_pipette_static_config(pipette_dict: PipetteDict) -> LoadedStaticPipetteD
# https://opentrons.atlassian.net/browse/RCORE-655
home_position=0,
nozzle_offset_z=0,
# TODO (spp): Confirm that the nozzle map is populated by the hardware api by default
back_left_nozzle_offset=pipette_dict[
"current_nozzle_map"
].back_left_nozzle_offset,
front_right_nozzle_offset=pipette_dict[
"current_nozzle_map"
].front_right_nozzle_offset,
nozzle_map=pipette_dict["current_nozzle_map"],
)
15 changes: 12 additions & 3 deletions api/src/opentrons/protocol_engine/state/pipettes.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class StaticPipetteConfig:
home_position: float
nozzle_offset_z: float
bounding_nozzle_offsets: BoundingNozzlesOffsets
default_nozzle_map: NozzleMap


@dataclass
Expand Down Expand Up @@ -167,11 +168,15 @@ def _handle_command( # noqa: C901
home_position=config.home_position,
nozzle_offset_z=config.nozzle_offset_z,
bounding_nozzle_offsets=BoundingNozzlesOffsets(
back_left_offset=config.back_left_nozzle_offset,
front_right_offset=config.front_right_nozzle_offset,
back_left_offset=config.nozzle_map.back_left_nozzle_offset,
front_right_offset=config.nozzle_map.front_right_nozzle_offset,
),
default_nozzle_map=config.nozzle_map,
)
self._state.flow_rates_by_id[private_result.pipette_id] = config.flow_rates
self._state.nozzle_configuration_by_id[
private_result.pipette_id
] = config.nozzle_map
Comment on lines +177 to +179
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make nozzle_configuration_by_id values non-Optional now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed with @Laura-Danielle. We'll defer this change. Might want to leave a # TODO explaining what needs to happen, though.

elif isinstance(private_result, PipetteNozzleLayoutResultMixin):
self._state.nozzle_configuration_by_id[
private_result.pipette_id
Expand All @@ -188,7 +193,11 @@ def _handle_command( # noqa: C901
self._state.aspirated_volume_by_id[pipette_id] = None
self._state.movement_speed_by_id[pipette_id] = None
self._state.attached_tip_by_id[pipette_id] = None
self._state.nozzle_configuration_by_id[pipette_id] = None
static_config = self._state.static_config_by_id.get(pipette_id)
if static_config:
self._state.nozzle_configuration_by_id[
pipette_id
] = static_config.default_nozzle_map

elif isinstance(command.result, (AspirateResult, AspirateInPlaceResult)):
pipette_id = command.params.pipetteId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
ConfigureForVolumePrivateResult,
ConfigureForVolumeImplementation,
)
from opentrons.types import Point
from opentrons_shared_data.pipette.dev_types import PipetteNameType
from ..pipette_fixtures import get_default_nozzle_map


async def test_configure_for_volume_implementation(
Expand Down Expand Up @@ -44,8 +45,7 @@ async def test_configure_for_volume_implementation(
),
tip_configuration_lookup_table={},
nominal_tip_overlap={},
back_left_nozzle_offset=Point(x=1, y=2, z=3),
front_right_nozzle_offset=Point(x=4, y=5, z=6),
nozzle_map=get_default_nozzle_map(PipetteNameType.P300_MULTI),
)

decoy.when(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Test configure nozzle layout commands."""
import pytest
from decoy import Decoy
from typing import Union, Optional, Dict
from typing import Union, Dict
from collections import OrderedDict

from opentrons.protocol_engine.execution import (
Expand Down Expand Up @@ -73,11 +73,6 @@
),
{"primary_nozzle": "A1", "front_right_nozzle": "E1"},
],
[
AllNozzleLayoutConfiguration(),
None,
{},
],
],
)
async def test_configure_nozzle_layout_implementation(
Expand All @@ -90,7 +85,7 @@ async def test_configure_nozzle_layout_implementation(
QuadrantNozzleLayoutConfiguration,
SingleNozzleLayoutConfiguration,
],
expected_nozzlemap: Optional[NozzleMap],
expected_nozzlemap: NozzleMap,
nozzle_params: Dict[str, str],
) -> None:
"""A ConfigureForVolume command should have an execution implementation."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from opentrons_shared_data.pipette.dev_types import PipetteNameType
from opentrons_shared_data.robot.dev_types import RobotType
from opentrons.types import MountType, Point
from opentrons.types import MountType

from opentrons.protocol_engine.errors import InvalidSpecificationForRobotTypeError
from opentrons.protocol_engine.types import FlowRates
Expand All @@ -19,6 +19,7 @@
LoadPipettePrivateResult,
LoadPipetteImplementation,
)
from ..pipette_fixtures import get_default_nozzle_map


async def test_load_pipette_implementation(
Expand All @@ -41,8 +42,7 @@ async def test_load_pipette_implementation(
),
tip_configuration_lookup_table={},
nominal_tip_overlap={},
back_left_nozzle_offset=Point(x=1, y=2, z=3),
front_right_nozzle_offset=Point(x=4, y=5, z=6),
nozzle_map=get_default_nozzle_map(PipetteNameType.P300_MULTI),
)
data = LoadPipetteParams(
pipetteName=PipetteNameType.P300_SINGLE,
Expand Down Expand Up @@ -98,8 +98,7 @@ async def test_load_pipette_implementation_96_channel(
),
tip_configuration_lookup_table={},
nominal_tip_overlap={},
back_left_nozzle_offset=Point(x=1, y=2, z=3),
front_right_nozzle_offset=Point(x=4, y=5, z=6),
nozzle_map=get_default_nozzle_map(PipetteNameType.P1000_96),
)

decoy.when(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from opentrons_shared_data.labware.dev_types import LabwareUri

from opentrons.calibration_storage.helpers import uri_from_details
from opentrons.types import Mount as HwMount, MountType, DeckSlotName, Point
from opentrons.types import Mount as HwMount, MountType, DeckSlotName
from opentrons.hardware_control import HardwareControlAPI
from opentrons.hardware_control.modules import (
TempDeck,
Expand Down Expand Up @@ -56,6 +56,7 @@
LoadedPipetteData,
LoadedModuleData,
)
from ..pipette_fixtures import get_default_nozzle_map


def _make_config(use_virtual_modules: bool) -> Config:
Expand Down Expand Up @@ -147,8 +148,7 @@ def loaded_static_pipette_data(
nominal_tip_overlap={"default": 9.87},
home_position=10.11,
nozzle_offset_z=12.13,
back_left_nozzle_offset=Point(x=1, y=2, z=3),
front_right_nozzle_offset=Point(x=4, y=5, z=6),
nozzle_map=get_default_nozzle_map(PipetteNameType.P300_SINGLE),
)


Expand Down
34 changes: 34 additions & 0 deletions api/tests/opentrons/protocol_engine/pipette_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
from collections import OrderedDict

from opentrons.types import Point
from opentrons.hardware_control.nozzle_manager import NozzleMap
from opentrons_shared_data.pipette.dev_types import PipetteNameType


NINETY_SIX_ROWS = OrderedDict(
(
Expand Down Expand Up @@ -317,3 +320,34 @@
("H1", Point(0.0, -31.5, 35.52)),
)
)


def get_default_nozzle_map(pipette_type: PipetteNameType) -> NozzleMap:
"""Get default nozzle map for a given pipette type."""
if "multi" in pipette_type.value:
return NozzleMap.build(
physical_nozzles=EIGHT_CHANNEL_MAP,
physical_rows=EIGHT_CHANNEL_ROWS,
physical_columns=EIGHT_CHANNEL_COLS,
starting_nozzle="A1",
back_left_nozzle="A1",
front_right_nozzle="A1",
)
elif "96" in pipette_type.value:
return NozzleMap.build(
physical_nozzles=NINETY_SIX_MAP,
physical_rows=NINETY_SIX_ROWS,
physical_columns=NINETY_SIX_COLS,
starting_nozzle="A1",
back_left_nozzle="A1",
front_right_nozzle="A1",
)
else:
return NozzleMap.build(
physical_nozzles=OrderedDict({"A1": Point(0, 0, 0)}),
physical_rows=OrderedDict({"A": ["A1"]}),
physical_columns=OrderedDict({"1": ["A1"]}),
starting_nozzle="A1",
back_left_nozzle="A1",
front_right_nozzle="A1",
)
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
"""Test pipette data provider."""
from collections import OrderedDict

import pytest
from opentrons_shared_data.pipette.dev_types import PipetteNameType, PipetteModel
from opentrons_shared_data.pipette import pipette_definition, types as pip_types

from opentrons.hardware_control.dev_types import PipetteDict
from opentrons.hardware_control.nozzle_manager import NozzleMap
from opentrons.protocol_engine.types import FlowRates
from opentrons.protocol_engine.resources.pipette_data_provider import (
LoadedStaticPipetteData,
VirtualPipetteDataProvider,
)

from opentrons.protocol_engine.resources import pipette_data_provider as subject
from opentrons.types import Point
from ..pipette_fixtures import get_default_nozzle_map


@pytest.fixture
Expand Down Expand Up @@ -54,8 +51,7 @@ def test_get_virtual_pipette_static_config(
"opentrons/opentrons_96_tiprack_10ul/1": 8.25,
"opentrons/opentrons_96_tiprack_20ul/1": 8.25,
},
back_left_nozzle_offset=Point(x=0.0, y=0.0, z=10.45),
front_right_nozzle_offset=Point(x=0.0, y=0.0, z=10.45),
nozzle_map=result.nozzle_map,
)


Expand All @@ -81,8 +77,7 @@ def test_configure_virtual_pipette_for_volume(
),
tip_configuration_lookup_table=result1.tip_configuration_lookup_table,
nominal_tip_overlap=result1.nominal_tip_overlap,
back_left_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15),
front_right_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15),
nozzle_map=result1.nozzle_map,
Comment on lines 78 to +80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, just realizing these aren't great asserts. They're just comparing an item with itself; like result1.nozzle_map == result1.nozzle_map. We should check it against the actual expected nozzle map.

)
subject_instance.configure_virtual_pipette_for_volume(
"my-pipette", 1, result1.model
Expand All @@ -105,8 +100,7 @@ def test_configure_virtual_pipette_for_volume(
),
tip_configuration_lookup_table=result2.tip_configuration_lookup_table,
nominal_tip_overlap=result2.nominal_tip_overlap,
back_left_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15),
front_right_nozzle_offset=Point(x=-8.0, y=-22.0, z=-259.15),
nozzle_map=result2.nozzle_map,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

)


Expand All @@ -132,8 +126,7 @@ def test_load_virtual_pipette_by_model_string(
),
tip_configuration_lookup_table=result.tip_configuration_lookup_table,
nominal_tip_overlap=result.nominal_tip_overlap,
back_left_nozzle_offset=Point(x=0.0, y=31.5, z=35.52),
front_right_nozzle_offset=Point(x=0.0, y=-31.5, z=35.52),
nozzle_map=result.nozzle_map,
)


Expand Down Expand Up @@ -179,6 +172,7 @@ def test_get_pipette_static_config(
supported_tip_fixture: pipette_definition.SupportedTipsDefinition,
) -> None:
"""It should return config data given a PipetteDict."""
dummy_nozzle_map = get_default_nozzle_map(PipetteNameType.P300_SINGLE_GEN2)
pipette_dict: PipetteDict = {
"name": "p300_single_gen2",
"min_volume": 20,
Expand Down Expand Up @@ -214,14 +208,7 @@ def test_get_pipette_static_config(
"default_aspirate_speeds": {"2.0": 5.021202, "2.6": 10.042404},
"default_push_out_volume": 3,
"supported_tips": {pip_types.PipetteTipType.t300: supported_tip_fixture},
"current_nozzle_map": NozzleMap.build(
physical_nozzles=OrderedDict({"A1": Point(1, 2, 3), "B1": Point(4, 5, 6)}),
physical_rows=OrderedDict({"A": ["A1"], "B": ["B1"]}),
physical_columns=OrderedDict({"1": ["A1", "B1"]}),
starting_nozzle="A1",
back_left_nozzle="A1",
front_right_nozzle="B1",
),
"current_nozzle_map": dummy_nozzle_map,
}

result = subject.get_pipette_static_config(pipette_dict)
Expand All @@ -247,6 +234,5 @@ def test_get_pipette_static_config(
# https://opentrons.atlassian.net/browse/RCORE-655
nozzle_offset_z=0,
home_position=0,
back_left_nozzle_offset=Point(x=1, y=2, z=3),
front_right_nozzle_offset=Point(x=4, y=5, z=6),
nozzle_map=dummy_nozzle_map,
)
Loading
Loading