From 56897a1ab3e4c0746da79a934c2516fd1cdfeefe Mon Sep 17 00:00:00 2001 From: Brayan Almonte Date: Fri, 9 Jun 2023 15:57:42 -0400 Subject: [PATCH] feat(api,robot-server): expose module calibration offsets via /modules endpoint. (#12860) --- .../opentrons/calibration_storage/__init__.py | 2 + .../calibration_storage/ot3/module_offset.py | 12 +++++ .../hardware_control/module_control.py | 2 +- .../modules/module_calibration.py | 6 +-- api/src/opentrons/hardware_control/ot3api.py | 15 +++++++ .../modules/module_data_mapper.py | 5 ++- .../robot_server/modules/module_models.py | 15 +++++++ robot-server/robot_server/modules/router.py | 26 ++++++++++- .../tests/modules/test_module_data_mapper.py | 18 ++++++++ robot-server/tests/modules/test_router.py | 45 ++++++++++++++++++- 10 files changed, 137 insertions(+), 9 deletions(-) diff --git a/api/src/opentrons/calibration_storage/__init__.py b/api/src/opentrons/calibration_storage/__init__.py index daa312ae519..5daf55b660a 100644 --- a/api/src/opentrons/calibration_storage/__init__.py +++ b/api/src/opentrons/calibration_storage/__init__.py @@ -44,6 +44,7 @@ clear_module_offset_calibrations, get_module_offset, delete_module_offset_file, + load_all_module_offsets, ) else: from .ot2.pipette_offset import ( @@ -87,6 +88,7 @@ "clear_module_offset_calibrations", "get_module_offset", "delete_module_offset_file", + "load_all_module_offsets", # functions only used in robot server "_save_custom_tiprack_definition", "get_custom_tiprack_definition_for_tlc", diff --git a/api/src/opentrons/calibration_storage/ot3/module_offset.py b/api/src/opentrons/calibration_storage/ot3/module_offset.py index 69dae1c00c2..2c45e183db2 100644 --- a/api/src/opentrons/calibration_storage/ot3/module_offset.py +++ b/api/src/opentrons/calibration_storage/ot3/module_offset.py @@ -1,6 +1,7 @@ import json import logging import os +from functools import lru_cache from pathlib import Path from opentrons.hardware_control.modules.types import ModuleType from opentrons.hardware_control.types import OT3Mount @@ -30,6 +31,9 @@ def delete_module_offset_file(module_id: str) -> None: offset_dir = config.get_opentrons_path("module_calibration_dir") offset_path = offset_dir / f"{module_id}.json" io.delete_file(offset_path) + # something changed, clear lru cache + get_module_offset.cache_clear() + load_all_module_offsets.cache_clear() def clear_module_offset_calibrations() -> None: @@ -39,6 +43,9 @@ def clear_module_offset_calibrations() -> None: offset_dir = config.get_opentrons_path("module_calibration_dir") io._remove_json_files_in_directories(offset_dir) + # something changed, clear lru cache + get_module_offset.cache_clear() + load_all_module_offsets.cache_clear() # Save Module Offset Calibrations @@ -76,12 +83,16 @@ def save_module_calibration( status=cal_status_model, ) io.save_to_file(module_dir, module_id, module_calibration) + # something changed, clear lru cache + get_module_offset.cache_clear() + load_all_module_offsets.cache_clear() # Get Module Offset Calibrations @no_type_check +@lru_cache(maxsize=10) def get_module_offset( module: ModuleType, module_id: str, slot: Optional[int] = None ) -> Optional[v1.ModuleOffsetModel]: @@ -102,6 +113,7 @@ def get_module_offset( return None +@lru_cache(maxsize=10) def load_all_module_offsets() -> List[v1.ModuleOffsetModel]: """Load all module offsets from the disk.""" diff --git a/api/src/opentrons/hardware_control/module_control.py b/api/src/opentrons/hardware_control/module_control.py index 3b4b62156ed..6bae9c6242e 100644 --- a/api/src/opentrons/hardware_control/module_control.py +++ b/api/src/opentrons/hardware_control/module_control.py @@ -230,7 +230,7 @@ def get_module_by_module_id( return found_module def load_module_offset( - self, module_type: ModuleType, module_id: str, slot: int + self, module_type: ModuleType, module_id: str, slot: Optional[int] = None ) -> ModuleCalibrationOffset: log.info(f"Loading module offset for {module_type} {module_id}") return load_module_calibration_offset(module_type, module_id, slot) diff --git a/api/src/opentrons/hardware_control/modules/module_calibration.py b/api/src/opentrons/hardware_control/modules/module_calibration.py index 6af058902c0..f95dc26f0f4 100644 --- a/api/src/opentrons/hardware_control/modules/module_calibration.py +++ b/api/src/opentrons/hardware_control/modules/module_calibration.py @@ -21,12 +21,12 @@ class ModuleCalibrationOffset: """Class to store module offset calibration data.""" - slot: int offset: Point module_id: str module: ModuleType source: SourceType status: CalibrationStatus + slot: Optional[int] = None mount: Optional[OT3Mount] = None instrument_id: Optional[str] = None last_modified: Optional[datetime] = None @@ -35,7 +35,7 @@ class ModuleCalibrationOffset: def load_module_calibration_offset( module_type: ModuleType, module_id: str, - slot: int, + slot: Optional[int] = None, ) -> ModuleCalibrationOffset: """Loads the calibration offset for a module.""" # load default if module offset data do not exist @@ -51,9 +51,9 @@ def load_module_calibration_offset( module_offset_data = get_module_offset(module_type, module_id) if module_offset_data: return ModuleCalibrationOffset( - slot=slot, module=module_type, module_id=module_id, + slot=module_offset_data.slot, mount=module_offset_data.mount, offset=module_offset_data.offset, last_modified=module_offset_data.lastModified, diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index b61556c6de1..cd7af6b7465 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -1823,6 +1823,21 @@ async def save_module_offset( module_type, module_id, mount, slot, offset, instrument_id ) + def get_module_calibration_offset( + self, serial_number: str + ) -> Optional[ModuleCalibrationOffset]: + """Get the module calibration offset of a module.""" + module = self._backend.module_controls.get_module_by_module_id(serial_number) + if not module: + self._log.warning( + f"Could not load calibration: unknown module {serial_number}" + ) + return None + module_type = module.MODULE_TYPE + return self._backend.module_controls.load_module_offset( + module_type, serial_number + ) + def get_attached_pipette( self, mount: Union[top_types.Mount, OT3Mount] ) -> PipetteDict: diff --git a/robot-server/robot_server/modules/module_data_mapper.py b/robot-server/robot_server/modules/module_data_mapper.py index 85ebd220123..10d8c9f0cbb 100644 --- a/robot-server/robot_server/modules/module_data_mapper.py +++ b/robot-server/robot_server/modules/module_data_mapper.py @@ -1,5 +1,5 @@ """Module identification and response data mapping.""" -from typing import Type, cast +from typing import Type, cast, Optional from opentrons.hardware_control.modules import ( LiveData, @@ -24,6 +24,7 @@ AttachedModuleData, MagneticModule, MagneticModuleData, + ModuleCalibrationData, TemperatureModule, TemperatureModuleData, ThermocyclerModule, @@ -44,6 +45,7 @@ def map_data( has_available_update: bool, live_data: LiveData, usb_port: HardwareUSBPort, + module_offset: Optional[ModuleCalibrationData], ) -> AttachedModule: """Map hardware control data to an attached module response.""" module_model = ModuleModel(model) @@ -138,4 +140,5 @@ def map_data( moduleType=module_type, # type: ignore[arg-type] moduleModel=module_model, # type: ignore[arg-type] data=module_data, # type: ignore[arg-type] + moduleOffset=module_offset, ) diff --git a/robot-server/robot_server/modules/module_models.py b/robot-server/robot_server/modules/module_models.py index edde7c40e2d..3e50bdac9a5 100644 --- a/robot-server/robot_server/modules/module_models.py +++ b/robot-server/robot_server/modules/module_models.py @@ -1,9 +1,11 @@ """Request and response models for /modules endpoints.""" +from datetime import datetime from pydantic import BaseModel, Field from pydantic.generics import GenericModel from typing import Generic, Optional, TypeVar, Union from typing_extensions import Literal +from opentrons.calibration_storage.types import SourceType from opentrons.hardware_control.modules import ( ModuleType, TemperatureStatus, @@ -16,12 +18,22 @@ HeaterShakerLabwareLatchStatus, ) from opentrons.protocol_engine import ModuleModel +from opentrons.protocol_engine.types import Vec3f ModuleT = TypeVar("ModuleT", bound=ModuleType) ModuleModelT = TypeVar("ModuleModelT", bound=ModuleModel) ModuleDataT = TypeVar("ModuleDataT", bound=BaseModel) +class ModuleCalibrationData(BaseModel): + """A module's calibration data.""" + + offset: Vec3f + slot: Optional[int] = None + source: Optional[SourceType] = None + last_modified: Optional[datetime] = None + + class UsbPort(BaseModel): """The USB port the module is connected to.""" @@ -65,6 +77,9 @@ class _GenericModule(GenericModel, Generic[ModuleT, ModuleModelT, ModuleDataT]): ) moduleType: ModuleT = Field(..., description="General type of the module.") moduleModel: ModuleModelT = Field(..., description="Specific model of the module.") + moduleOffset: Optional[ModuleCalibrationData] = Field( + None, description="The calibrated module offset." + ) data: ModuleDataT usbPort: UsbPort diff --git a/robot-server/robot_server/modules/router.py b/robot-server/robot_server/modules/router.py index 1518a89f1f5..7ece61a72d1 100644 --- a/robot-server/robot_server/modules/router.py +++ b/robot-server/robot_server/modules/router.py @@ -1,8 +1,10 @@ """Modules routes.""" from fastapi import APIRouter, Depends, status -from typing import List +from typing import List, Dict from opentrons.hardware_control import HardwareControlAPI +from opentrons.hardware_control.modules import module_calibration +from opentrons.protocol_engine.types import Vec3f from robot_server.hardware import get_hardware from robot_server.versioning import get_requested_version @@ -15,7 +17,7 @@ PydanticResponse, ) -from .module_models import AttachedModule +from .module_models import AttachedModule, ModuleCalibrationData from .module_identifier import ModuleIdentifier from .module_data_mapper import ModuleDataMapper @@ -42,9 +44,17 @@ async def get_attached_modules( hardware=hardware, ) + # Load any the module calibrations + module_calibrations: Dict[str, module_calibration.ModuleCalibrationOffset] = { + mod.module_id: mod for mod in module_calibration.load_all_module_calibrations() + } + response_data: List[AttachedModule] = [] for mod in hardware.attached_modules: + serial_number = mod.device_info["serial"] + calibrated = module_calibrations.get(serial_number) module_identity = module_identifier.identify(mod.device_info) + response_data.append( module_data_mapper.map_data( model=mod.model(), @@ -52,6 +62,18 @@ async def get_attached_modules( module_identity=module_identity, live_data=mod.live_data, usb_port=mod.usb_port, + module_offset=ModuleCalibrationData.construct( + offset=Vec3f( + x=calibrated.offset.x, + y=calibrated.offset.y, + z=calibrated.offset.z, + ), + slot=calibrated.slot, + source=calibrated.status.source, + last_modified=calibrated.last_modified, + ) + if calibrated + else None, ) ) diff --git a/robot-server/tests/modules/test_module_data_mapper.py b/robot-server/tests/modules/test_module_data_mapper.py index 393a946dee4..50e5456375f 100644 --- a/robot-server/tests/modules/test_module_data_mapper.py +++ b/robot-server/tests/modules/test_module_data_mapper.py @@ -2,6 +2,7 @@ import pytest from opentrons.protocol_engine import ModuleModel +from opentrons.protocol_engine.types import Vec3f from opentrons.drivers.rpi_drivers.types import USBPort as HardwareUSBPort from opentrons.hardware_control.modules import ( LiveData, @@ -25,6 +26,7 @@ ThermocyclerModuleData, HeaterShakerModule, HeaterShakerModuleData, + ModuleCalibrationData, ) @@ -96,6 +98,9 @@ def test_maps_magnetic_module_data( has_available_update=True, live_data=input_data, usb_port=hardware_usb_port, + module_offset=ModuleCalibrationData.construct( + offset=Vec3f(x=0, y=0, z=0), + ), ) assert result == MagneticModule( @@ -108,6 +113,7 @@ def test_maps_magnetic_module_data( moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort(port=101, hub=202, path="/dev/null"), data=expected_output_data, + moduleOffset=ModuleCalibrationData(offset=Vec3f(x=0.0, y=0.0, z=0.0)), ) @@ -148,6 +154,9 @@ def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> has_available_update=True, live_data=input_data, usb_port=hardware_usb_port, + module_offset=ModuleCalibrationData.construct( + offset=Vec3f(x=0, y=0, z=0), + ), ) assert result == TemperatureModule( @@ -159,6 +168,7 @@ def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> moduleType=ModuleType.TEMPERATURE, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort(port=101, hub=202, path="/dev/null"), + moduleOffset=ModuleCalibrationData(offset=Vec3f(x=0.0, y=0.0, z=0.0)), data=TemperatureModuleData( status=TemperatureStatus(input_data["status"]), currentTemperature=input_data["data"]["currentTemp"], # type: ignore[arg-type] @@ -233,6 +243,9 @@ def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) - has_available_update=True, live_data=input_data, usb_port=hardware_usb_port, + module_offset=ModuleCalibrationData.construct( + offset=Vec3f(x=0, y=0, z=0), + ), ) assert result == ThermocyclerModule( @@ -244,6 +257,7 @@ def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) - moduleType=ModuleType.THERMOCYCLER, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort(port=101, hub=202, path="/dev/null"), + moduleOffset=ModuleCalibrationData(offset=Vec3f(x=0.0, y=0.0, z=0.0)), data=ThermocyclerModuleData( status=TemperatureStatus(input_data["status"]), currentTemperature=input_data["data"]["currentTemp"], # type: ignore[arg-type] @@ -320,6 +334,9 @@ def test_maps_heater_shaker_module_data(input_model: str, input_data: LiveData) has_available_update=True, live_data=input_data, usb_port=hardware_usb_port, + module_offset=ModuleCalibrationData.construct( + offset=Vec3f(x=0, y=0, z=0), + ), ) assert result == HeaterShakerModule( @@ -331,6 +348,7 @@ def test_maps_heater_shaker_module_data(input_model: str, input_data: LiveData) moduleType=ModuleType.HEATER_SHAKER, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort(port=101, hub=202, path="/dev/null"), + moduleOffset=ModuleCalibrationData(offset=Vec3f(x=0.0, y=0.0, z=0.0)), data=HeaterShakerModuleData( status=HeaterShakerStatus(input_data["status"]), labwareLatchStatus=input_data["data"]["labwareLatchStatus"], # type: ignore[arg-type] diff --git a/robot-server/tests/modules/test_router.py b/robot-server/tests/modules/test_router.py index adce2e32c34..c1b4c49d97e 100644 --- a/robot-server/tests/modules/test_router.py +++ b/robot-server/tests/modules/test_router.py @@ -1,13 +1,19 @@ """Tests for /modules routes.""" +import inspect import pytest from decoy import Decoy from typing_extensions import Final +from opentrons.calibration_storage.ot3.models.v1 import CalibrationStatus +from opentrons.calibration_storage.types import SourceType from opentrons.hardware_control import HardwareControlAPI from opentrons.drivers.rpi_drivers.types import USBPort as HardwareUSBPort from opentrons.hardware_control.modules import MagDeck, ModuleType, MagneticStatus +from opentrons.hardware_control.modules import module_calibration +from opentrons.types import Point from opentrons.protocol_engine import ModuleModel +from opentrons.protocol_engine.types import Vec3f from robot_server.modules.router import get_attached_modules from robot_server.modules.module_identifier import ModuleIdentifier, ModuleIdentity @@ -15,6 +21,7 @@ from robot_server.modules.module_models import ( MagneticModule, MagneticModuleData, + ModuleCalibrationData, UsbPort, ) @@ -22,6 +29,13 @@ _HTTP_API_VERSION: Final = 3 +@pytest.fixture(autouse=True) +def _use_mock_module_calibration(decoy: Decoy, monkeypatch: pytest.MonkeyPatch) -> None: + """Mock out the opentrons.module_calibration module.""" + for name, func in inspect.getmembers(module_calibration, inspect.isfunction): + monkeypatch.setattr(module_calibration, name, decoy.mock(func=func)) + + @pytest.fixture() def magnetic_module(decoy: Decoy) -> MagDeck: """Get a mock magnetic module control interface.""" @@ -46,6 +60,7 @@ async def test_get_modules_empty( ) -> None: """It should get an empty modules list from the hardware API.""" decoy.when(hardware_api.attached_modules).then_return([]) + decoy.when(module_calibration.load_all_module_calibrations()).then_return([]) result = await get_attached_modules( requested_version=_HTTP_API_VERSION, @@ -73,6 +88,9 @@ async def test_get_modules_maps_data_and_id( moduleType=ModuleType.MAGNETIC, moduleModel=ModuleModel.MAGNETIC_MODULE_V1, usbPort=UsbPort(port=42, hub=None, path="/dev/null"), + moduleOffset=ModuleCalibrationData.construct( + offset=Vec3f(x=0, y=0, z=0), + ), data=MagneticModuleData( status=MagneticStatus.ENGAGED, engaged=True, @@ -80,12 +98,25 @@ async def test_get_modules_maps_data_and_id( ), ) + calibration_offset = module_calibration.ModuleCalibrationOffset( + slot=1, + offset=Point(x=0, y=0, z=0), + module=ModuleType.MAGNETIC, + module_id="serial-number", + source=SourceType.default, + status=CalibrationStatus(), + ) decoy.when(hardware_api.attached_modules).then_return([magnetic_module]) + decoy.when(module_calibration.load_all_module_calibrations()).then_return( + [ + calibration_offset, + ] + ) decoy.when(magnetic_module.has_available_update()).then_return(True) decoy.when(magnetic_module.model()).then_return("magneticModuleV1") decoy.when(magnetic_module.device_info).then_return( { - "serial": "abc", + "serial": "serial-number", "version": "1.2.3", } ) @@ -107,7 +138,7 @@ async def test_get_modules_maps_data_and_id( ) decoy.when( - module_identifier.identify({"serial": "abc", "version": "1.2.3"}) + module_identifier.identify({"serial": "serial-number", "version": "1.2.3"}) ).then_return(module_identity) decoy.when( @@ -117,6 +148,16 @@ async def test_get_modules_maps_data_and_id( has_available_update=True, live_data={"status": "engaged", "data": {"engaged": True, "height": 42}}, usb_port=HardwareUSBPort(name="abc", port_number=101), + module_offset=ModuleCalibrationData.construct( + offset=Vec3f( + x=calibration_offset.offset.x, + y=calibration_offset.offset.y, + z=calibration_offset.offset.z, + ), + slot=calibration_offset.slot, + source=None, + last_modified=None, + ), ) ).then_return(expected_response)