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

fix(robot-server): mutable configs for attached #13696

Merged
merged 2 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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
95 changes: 69 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,22 @@ 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_id(
pipette_id,
)
for dct in attached_pipettes.values():
if "pipette_id" not in dct:
continue
res[dct["pipette_id"]] = _pipette_settings_from_pipette_dict(dct)
return res


Expand All @@ -298,16 +312,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_id(pipette_id)
for dct in attached_pipettes.values():
if dct.get("pipette_id") == pipette_id:
return _pipette_settings_from_pipette_dict(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 +359,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_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 +387,35 @@ def _pipette_settings_from_config(pipette_id: str) -> PipetteSettings:
),
fields=fields,
)


def _pipette_settings_from_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_from_pipette_dict(
Copy link
Contributor

Choose a reason for hiding this comment

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

could we call this something more like _pipette_settings_from_pipette_default and/or _pipette_settings_from_default?

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 _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)

Check warning on line 209 in shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py

View check run for this annotation

Codecov / codecov/patch

shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py#L209

Added line #L209 was not covered by tests

return full_mutable_configs


def list_mutable_configs(
pipette_serial_number: str, pipette_override_path: Path
) -> OverrideType:
Expand All @@ -207,36 +229,36 @@
# 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 = ""

Check warning on line 238 in shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py

View check run for this annotation

Codecov / codecov/patch

shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py#L238

Added line #L238 was not covered by tests
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(

Check warning on line 256 in shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py

View check run for this annotation

Codecov / codecov/patch

shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py#L253-L256

Added lines #L253 - L256 were not covered by tests
pipette_serial_number, pipette_override_path
)
except FileNotFoundError:
pass
return _load_full_mutable_configs(pipette_model, mutable_configs)

Check warning on line 261 in shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py

View check run for this annotation

Codecov / codecov/patch

shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py#L259-L261

Added lines #L259 - L261 were not covered by tests


def load_with_mutable_configurations(
Expand Down
Loading