Skip to content

Commit

Permalink
fix(robot-server): mutable configs for attached (#13696)
Browse files Browse the repository at this point in the history
When something hits /settings/pipettes, we load known mutable
configuration overrides. So if you don't have any for the attached
pipettes, we don't return anything.

The thing is, what we actually want is to return all of the mutable
configurations, with overrides applied. So now we do.

Closes RET-1376
  • Loading branch information
sfoster1 authored Oct 3, 2023
1 parent ee77c49 commit c715cc4
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ def get_attached_instrument(self, mount: MountType) -> PipetteDict:
def attached_instruments(self) -> Dict[MountType, PipetteDict]:
return self.get_attached_instruments()

@property
def attached_pipettes(self) -> Dict[MountType, PipetteDict]:
return self.get_attached_instruments()

@property
def get_attached_pipettes(self) -> Dict[MountType, PipetteDict]:
return self.get_attached_instruments()

@property
def hardware_instruments(self) -> InstrumentsByMount[MountType]:
"""Do not write new code that uses this."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ def get_attached_instrument(self, mount: Mount) -> PipetteDict:
def attached_instruments(self) -> Dict[Mount, PipetteDict]:
return self.get_attached_instruments()

def get_attached_pipettes(self) -> Dict[Mount, PipetteDict]:
"""Get the status dicts of cached attached pipettes.
Works like get_attached_instruments but for pipettes only - on the Flex,
there will be no gripper information here.
"""
...

@property
def attached_pipettes(self) -> Dict[Mount, PipetteDict]:
return self.get_attached_pipettes()

def calibrate_plunger(
self,
mount: Mount,
Expand Down
97 changes: 71 additions & 26 deletions robot-server/robot_server/service/legacy/routers/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@
from fastapi import APIRouter, Depends

from opentrons_shared_data.errors import ErrorCodes
from opentrons.hardware_control import HardwareControlAPI
from opentrons.hardware_control import (
HardwareControlAPI,
dev_types as hardware_dev_types,
)
from opentrons.system import log_control
from opentrons_shared_data.pipette import mutable_configurations, types as pip_types
from opentrons_shared_data.pipette import (
mutable_configurations,
types as pip_types,
pipette_load_name_conversions as pip_names,
)
from opentrons.config import (
reset as reset_util,
robot_configs,
Expand Down Expand Up @@ -276,15 +283,24 @@ async def get_robot_settings(
response_model_by_alias=True,
response_model_exclude_unset=True,
)
async def get_pipette_settings() -> MultiPipetteSettings:
async def get_pipette_settings(
hardware: HardwareControlAPI = Depends(get_hardware),
) -> MultiPipetteSettings:
res = {}
attached_pipettes = hardware.attached_pipettes
for pipette_id in mutable_configurations.known_pipettes(
get_opentrons_path("pipette_config_overrides_dir")
):
# Have to convert to dict using by_alias due to bug in fastapi
res[pipette_id] = _pipette_settings_from_config(
res[pipette_id] = _pipette_settings_from_known_id(
pipette_id,
)
for dct in attached_pipettes.values():
if "pipette_id" not in dct:
continue
res[dct["pipette_id"]] = _pipette_settings_with_defaults_from_attached_pipette(
dct
)
return res


Expand All @@ -298,16 +314,22 @@ async def get_pipette_settings() -> MultiPipetteSettings:
status.HTTP_404_NOT_FOUND: {"model": LegacyErrorResponse},
},
)
async def get_pipette_setting(pipette_id: str) -> PipetteSettings:
if pipette_id not in mutable_configurations.known_pipettes(
async def get_pipette_setting(
pipette_id: str, hardware: HardwareControlAPI = Depends(get_hardware)
) -> PipetteSettings:
attached_pipettes = hardware.attached_pipettes
known_ids = mutable_configurations.known_pipettes(
get_opentrons_path("pipette_config_overrides_dir")
):
raise LegacyErrorResponse(
message=f"{pipette_id} is not a valid pipette id",
errorCode=ErrorCodes.PIPETTE_NOT_PRESENT.value.code,
).as_error(status.HTTP_404_NOT_FOUND)
r = _pipette_settings_from_config(pipette_id)
return r
)
if pipette_id in known_ids:
return _pipette_settings_from_known_id(pipette_id)
for dct in attached_pipettes.values():
if dct.get("pipette_id") == pipette_id:
return _pipette_settings_with_defaults_from_attached_pipette(dct)
raise LegacyErrorResponse(
message=f"{pipette_id} is not a valid pipette id",
errorCode=ErrorCodes.PIPETTE_NOT_PRESENT.value.code,
).as_error(status.HTTP_404_NOT_FOUND)


@router.patch(
Expand Down Expand Up @@ -339,22 +361,13 @@ async def patch_pipette_setting(
raise LegacyErrorResponse(
message=str(e), errorCode=ErrorCodes.GENERAL_ERROR.value.code
).as_error(status.HTTP_412_PRECONDITION_FAILED)
r = _pipette_settings_from_config(pipette_id)
r = _pipette_settings_from_known_id(pipette_id)
return r


def _pipette_settings_from_config(pipette_id: str) -> PipetteSettings:
"""
Create a PipetteSettings object from pipette config for single pipette
:param pc: pipette config module
:param pipette_id: pipette id
:return: PipetteSettings object
"""
mutable_configs = mutable_configurations.list_mutable_configs(
pipette_serial_number=pipette_id,
pipette_override_path=get_opentrons_path("pipette_config_overrides_dir"),
)
def _pipette_settings_from_mutable_configs(
mutable_configs: pip_types.OverrideType,
) -> PipetteSettings:
converted_dict: Dict[str, Union[str, Dict[str, Any]]] = {}
# TODO rather than doing this gross thing, we should
# mess around with pydantic dataclasses.
Expand All @@ -376,3 +389,35 @@ def _pipette_settings_from_config(pipette_id: str) -> PipetteSettings:
),
fields=fields,
)


def _pipette_settings_from_known_id(pipette_id: str) -> PipetteSettings:
"""
Create a PipetteSettings object from pipette config for single pipette
:param pc: pipette config module
:param pipette_id: pipette id
:return: PipetteSettings object
"""
mutable_configs = mutable_configurations.list_mutable_configs(
pipette_serial_number=pipette_id,
pipette_override_path=get_opentrons_path("pipette_config_overrides_dir"),
)
return _pipette_settings_from_mutable_configs(mutable_configs)


def _pipette_settings_with_defaults_from_attached_pipette(
pipette_dict: hardware_dev_types.PipetteDict,
) -> PipetteSettings:
"""
Create a PipetteSettings object from a pipette dict from hardware
"""
pipette_id = pipette_dict["pipette_id"]
pipette_model = pipette_dict["model"]
pipette_modelversion = pip_names.convert_pipette_model(pipette_model)
mutable_configs = mutable_configurations.list_mutable_configs_with_defaults(
pipette_model=pipette_modelversion,
pipette_serial_number=pipette_id,
pipette_override_path=get_opentrons_path("pipette_config_overrides_dir"),
)
return _pipette_settings_from_mutable_configs(mutable_configs)
81 changes: 79 additions & 2 deletions robot-server/tests/service/legacy/routers/test_settings.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from mock import patch, call
from mock import patch, call, MagicMock
from dataclasses import make_dataclass
from typing import Generator
from pathlib import Path
Expand All @@ -10,7 +10,11 @@

from opentrons.config.reset import ResetOptionId
from opentrons.config import advanced_settings
from opentrons_shared_data.pipette import types as pip_types
from opentrons_shared_data.pipette import (
types as pip_types,
pipette_definition as pip_def,
)
from opentrons.types import Mount


from robot_server import app
Expand Down Expand Up @@ -124,6 +128,15 @@ def mock_list_mutable_configs(decoy: Decoy) -> Decoy:
yield m


@pytest.fixture
def mock_list_mutable_configs_with_defaults(decoy: Decoy) -> Decoy:
with patch(
"opentrons_shared_data.pipette.mutable_configurations.list_mutable_configs_with_defaults",
new=decoy.mock(),
) as m:
yield m


@pytest.fixture
def mock_save_overrides(decoy: Decoy) -> Decoy:
with patch(
Expand All @@ -142,6 +155,70 @@ def mock_get_opentrons_dir(decoy: Decoy) -> Decoy:
yield m


def test_receive_attached_pipette_settings(
decoy: Decoy,
api_client,
mock_known_pipettes: Decoy,
mock_get_opentrons_dir: Decoy,
mock_list_mutable_configs_with_defaults: Decoy,
hardware: MagicMock,
) -> None:
decoy.when(mock_get_opentrons_dir("pipette_config_overrides_dir")).then_return(
"nope"
)
decoy.when(mock_known_pipettes("nope")).then_return([])
hardware.attached_pipettes = {
Mount.LEFT: {"pipette_id": "P12345", "model": "p20_multi_v3.5"}
}
decoy.when(
mock_list_mutable_configs_with_defaults(
pipette_model=pip_def.PipetteModelVersionType(
pip_types.PipetteModelType.p20,
pip_types.PipetteChannelType.EIGHT_CHANNEL,
pip_types.PipetteVersionType(3, 5),
),
pipette_serial_number="P12345",
pipette_override_path="nope",
)
).then_return(
{
"pickUpCurrent": pip_types.MutableConfig.build(
**{
"units": "mm",
"type": "float",
"min": 1.0,
"max": 3.0,
"default": 1.5,
"value": 1.2,
},
name="pickUpCurrent",
),
"quirks": {
"dropTipShake": pip_types.QuirkConfig(name="dropTipShake", value=True)
},
"model": "p20_multi_v3.5",
}
)
resp = api_client.get("/settings/pipettes")
assert resp.status_code == 200
assert resp.json() == {
"P12345": {
"info": {"model": "p20_multi_v3.5", "name": ""},
"fields": {
"pickUpCurrent": {
"units": "mm",
"type": "float",
"min": 1.0,
"max": 3.0,
"default": 1.5,
"value": 1.2,
},
"quirks": {"dropTipShake": True},
},
},
}


def test_receive_pipette_settings(
decoy: Decoy,
api_client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,28 @@ def known_pipettes(pipette_override_path: Path) -> List[str]:
]


def _load_full_mutable_configs(
pipette_model: PipetteModelVersionType, overrides: OverrideType
) -> OverrideType:
base_configs = load_definition(
pipette_model.pipette_type,
pipette_model.pipette_channels,
pipette_model.pipette_version,
)
base_configs_dict = base_configs.dict(by_alias=True)
full_mutable_configs = _list_all_mutable_configs(overrides, base_configs_dict)

if not full_mutable_configs.get("name"):
full_mutable_configs["name"] = str(
convert_to_pipette_name_type(cast(PipetteName, str(pipette_model)))
)

if not full_mutable_configs.get("model"):
full_mutable_configs["model"] = str(pipette_model)

return full_mutable_configs


def list_mutable_configs(
pipette_serial_number: str, pipette_override_path: Path
) -> OverrideType:
Expand All @@ -207,36 +229,36 @@ def list_mutable_configs(
# This is mimicing behavior from the original file
# which returned an empty dict if no on disk value was found.
return mutable_configs
else:
serial_key_match = SERIAL_STUB_REGEX.match(pipette_serial_number)

if serial_key_match:
serial_key = serial_key_match.group(0)
else:
serial_key = ""
pipette_model = convert_pipette_model(
cast(PipetteModel, PIPETTE_SERIAL_MODEL_LOOKUP[serial_key])
)

base_configs = load_definition(
pipette_model.pipette_type,
pipette_model.pipette_channels,
pipette_model.pipette_version,
)
base_configs_dict = base_configs.dict(by_alias=True)
full_mutable_configs = _list_all_mutable_configs(
mutable_configs, base_configs_dict
)
serial_key_match = SERIAL_STUB_REGEX.match(pipette_serial_number)

if not full_mutable_configs.get("name"):
full_mutable_configs["name"] = str(
convert_to_pipette_name_type(cast(PipetteName, str(pipette_model)))
)
if serial_key_match:
serial_key = serial_key_match.group(0)
else:
serial_key = ""
pipette_model = convert_pipette_model(
cast(PipetteModel, PIPETTE_SERIAL_MODEL_LOOKUP[serial_key])
)
return _load_full_mutable_configs(pipette_model, mutable_configs)

if not full_mutable_configs.get("model"):
full_mutable_configs["model"] = str(pipette_model)

return full_mutable_configs
def list_mutable_configs_with_defaults(
pipette_model: PipetteModelVersionType,
pipette_serial_number: Optional[str],
pipette_override_path: Path,
) -> OverrideType:
"""
Returns dict of mutable configs only, with their defaults.
"""
mutable_configs: OverrideType = {}
if pipette_serial_number:
try:
mutable_configs = _load_available_overrides(
pipette_serial_number, pipette_override_path
)
except FileNotFoundError:
pass
return _load_full_mutable_configs(pipette_model, mutable_configs)


def load_with_mutable_configurations(
Expand Down

0 comments on commit c715cc4

Please sign in to comment.