From f2b746a4d424f459b968e9895b6795f95a1f777e Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 2 Nov 2020 09:44:00 -0500 Subject: [PATCH] feat(api): Mark calibrations as bad when determined they exceed threshold closes #6730 --- api/src/opentrons/calibration_storage/get.py | 4 +- .../opentrons/calibration_storage/modify.py | 88 ++++++++++++++---- .../opentrons/calibration_storage/types.py | 6 +- api/src/opentrons/types.py | 7 ++ .../robot/calibration/check/user_flow.py | 92 +++++++++++++++---- .../robot/calibration/helper_classes.py | 8 ++ .../robot/calibration/check/test_user_flow.py | 62 +++++++++++++ 7 files changed, 222 insertions(+), 45 deletions(-) diff --git a/api/src/opentrons/calibration_storage/get.py b/api/src/opentrons/calibration_storage/get.py index ae035866ffac..7a38e17a8bc4 100644 --- a/api/src/opentrons/calibration_storage/get.py +++ b/api/src/opentrons/calibration_storage/get.py @@ -18,7 +18,7 @@ def _format_calibration_type( - data: 'CalibrationDict') -> local_types.CalibrationTypes: + data: 'CalibrationDict') -> local_types.LabwareCalibrationTypes: offset = local_types.OffsetData( value=data['default']['offset'], last_modified=data['default']['lastModified'] @@ -27,7 +27,7 @@ def _format_calibration_type( # the labware calibraiton file. We should # have a follow-up PR to grab tip lengths # based on the loaded pips + labware - return local_types.CalibrationTypes( + return local_types.LabwareCalibrationTypes( offset=offset, tip_length=local_types.TipLengthData()) diff --git a/api/src/opentrons/calibration_storage/modify.py b/api/src/opentrons/calibration_storage/modify.py index 9f59c391fe04..83b65d4deab7 100644 --- a/api/src/opentrons/calibration_storage/modify.py +++ b/api/src/opentrons/calibration_storage/modify.py @@ -8,7 +8,8 @@ from pathlib import Path from opentrons import config -from opentrons.types import Mount +from opentrons.types import Mount, Point + from opentrons.util.helpers import utc_now from . import ( @@ -22,7 +23,7 @@ TipLengthCalibration, PipTipLengthCalibration, DeckCalibrationData, PipetteCalibrationData, CalibrationStatusDict) from opentrons_shared_data.labware.dev_types import LabwareDefinition - from opentrons.types import Point + from opentrons.protocol_api.labware import Labware def _add_to_index_offset_file(parent: str, slot: str, uri: str, lw_hash: str): @@ -76,7 +77,7 @@ def add_existing_labware_to_index_file( def save_labware_calibration( labware_path: local_types.StrPath, definition: 'LabwareDefinition', - delta: 'Point', + delta: Point, slot: str = '', parent: str = ''): """ @@ -106,7 +107,9 @@ def save_labware_calibration( def create_tip_length_data( definition: 'LabwareDefinition', parent: str, - length: float) -> 'PipTipLengthCalibration': + length: float, + cal_status: typing.Optional[local_types.CalibrationStatus] = None + ) -> 'PipTipLengthCalibration': """ Function to correctly format tip length data. @@ -121,22 +124,25 @@ def create_tip_length_data( # assert labware._is_tiprack, \ # 'cannot save tip length for non-tiprack labware' labware_hash = helpers.hash_labware_def(definition) - status: 'CalibrationStatusDict' =\ - helpers.convert_to_dict( # type: ignore - local_types.CalibrationStatus()) + if cal_status: + status = cal_status + else: + status = local_types.CalibrationStatus() + status_dict: 'CalibrationStatusDict' =\ + helpers.convert_to_dict(status) # type: ignore tip_length_data: 'TipLengthCalibration' = { 'tipLength': length, 'lastModified': utc_now(), 'source': local_types.SourceType.user, - 'status': status + 'status': status_dict } data = {labware_hash + parent: tip_length_data} return data -def _helper_offset_data_format(filepath: str, delta: 'Point') -> dict: +def _helper_offset_data_format(filepath: str, delta: Point) -> dict: if not Path(filepath).is_file(): calibration_data = { "default": { @@ -197,20 +203,25 @@ def save_robot_deck_attitude( transform: local_types.AttitudeMatrix, pip_id: typing.Optional[str], lw_hash: typing.Optional[str], - source: local_types.SourceType = None): + source: local_types.SourceType = None, + cal_status: typing.Optional[local_types.CalibrationStatus] = None): robot_dir = config.get_opentrons_path('robot_calibration_dir') robot_dir.mkdir(parents=True, exist_ok=True) gantry_path = robot_dir/'deck_calibration.json' - status: 'CalibrationStatusDict' = \ - helpers.convert_to_dict( # type: ignore - local_types.CalibrationStatus()) + if cal_status: + status = cal_status + else: + status = local_types.CalibrationStatus() + status_dict: 'CalibrationStatusDict' =\ + helpers.convert_to_dict(status) # type: ignore + gantry_dict: 'DeckCalibrationData' = { 'attitude': transform, 'pipette_calibrated_with': pip_id, 'last_modified': utc_now(), 'tiprack': lw_hash, 'source': source or local_types.SourceType.user, - 'status': status + 'status': status_dict } io.save_to_file(gantry_path, gantry_dict) @@ -233,15 +244,19 @@ def _add_to_pipette_offset_index_file(pip_id: str, mount: Mount): def save_pipette_calibration( - offset: 'Point', + offset: Point, pip_id: str, mount: Mount, - tiprack_hash: str, tiprack_uri: str): + tiprack_hash: str, tiprack_uri: str, + cal_status: typing.Optional[local_types.CalibrationStatus] = None): pip_dir = config.get_opentrons_path( 'pipette_calibration_dir') / mount.name.lower() pip_dir.mkdir(parents=True, exist_ok=True) - status: 'CalibrationStatusDict' =\ - helpers.convert_to_dict( # type: ignore - local_types.CalibrationStatus()) + if cal_status: + status = cal_status + else: + status = local_types.CalibrationStatus() + status_dict: 'CalibrationStatusDict' =\ + helpers.convert_to_dict(status) # type: ignore offset_path = pip_dir/f'{pip_id}.json' offset_dict: 'PipetteCalibrationData' = { 'offset': [offset.x, offset.y, offset.z], @@ -249,7 +264,40 @@ def save_pipette_calibration( 'uri': tiprack_uri, 'last_modified': utc_now(), 'source': local_types.SourceType.user, - 'status': status + 'status': status_dict } io.save_to_file(offset_path, offset_dict) _add_to_pipette_offset_index_file(pip_id, mount) + + +def mark_bad( + calibration: typing.Union[ + local_types.DeckCalibration, + local_types.PipetteOffsetCalibration, + typing.Tuple[float, str, 'Labware']], + source_marked_bad: local_types.SourceType): + status = local_types.CalibrationStatus( + markedBad=True, source=source_marked_bad, markedAt=utc_now()) + if isinstance(calibration, local_types.DeckCalibration): + save_robot_deck_attitude( + transform=calibration.attitude, + pip_id=calibration.pipette_calibrated_with, + lw_hash=calibration.tiprack, + source=calibration.source, + cal_status=status) + elif isinstance(calibration, local_types.PipetteOffsetCalibration): + save_pipette_calibration( + offset=Point(*calibration.offset), + pip_id=calibration.pipette, + mount=Mount.string_to_mount(calibration.mount), + tiprack_hash=calibration.tiprack, + tiprack_uri=calibration.uri, + cal_status=status) + else: + tip_length, pipette_id, tiprack = calibration + tip_length_dict = create_tip_length_data( + definition=tiprack._implementation.get_definition(), + parent=tiprack.parent, + length=tip_length, + cal_status=status) + save_tip_length_calibration(pipette_id, tip_length_dict) diff --git a/api/src/opentrons/calibration_storage/types.py b/api/src/opentrons/calibration_storage/types.py index 54d9c7ca558a..8e8d7bc71f97 100644 --- a/api/src/opentrons/calibration_storage/types.py +++ b/api/src/opentrons/calibration_storage/types.py @@ -75,7 +75,7 @@ class ParentOptions: @dataclass -class CalibrationTypes: +class LabwareCalibrationTypes: """ Class to categorize what calibration data might be stored for a labware. @@ -90,7 +90,7 @@ class CalibrationInformation: Class to store important calibration info for labware. """ - calibration: CalibrationTypes + calibration: LabwareCalibrationTypes parent: ParentOptions labware_id: str uri: str @@ -119,7 +119,7 @@ class DeckCalibration: @dataclass class PipetteOffsetByPipetteMount: """ - Class to store pipette offset without pipette and monut info + Class to store pipette offset without pipette and mount info """ offset: PipetteOffset source: SourceType diff --git a/api/src/opentrons/types.py b/api/src/opentrons/types.py index 17afbabb96d4..3cb37fe79de9 100644 --- a/api/src/opentrons/types.py +++ b/api/src/opentrons/types.py @@ -119,6 +119,13 @@ class Mount(enum.Enum): def __str__(self): return self.name + @classmethod + def string_to_mount(cls, mount: str) -> 'Mount': + if mount == 'right': + return cls.RIGHT + else: + return cls.LEFT + class TransferTipPolicy(enum.Enum): ONCE = enum.auto() diff --git a/robot-server/robot_server/robot/calibration/check/user_flow.py b/robot-server/robot_server/robot/calibration/check/user_flow.py index 3dea378c5e9c..3b4ee9a8cc66 100644 --- a/robot-server/robot_server/robot/calibration/check/user_flow.py +++ b/robot-server/robot_server/robot/calibration/check/user_flow.py @@ -4,9 +4,9 @@ Callable, Dict, Any, TYPE_CHECKING) from typing_extensions import Literal -from opentrons.calibration_storage import get, helpers +from opentrons.calibration_storage import get, helpers, modify from opentrons.calibration_storage.types import ( - TipLengthCalNotFound, PipetteOffsetByPipetteMount) + TipLengthCalNotFound, PipetteOffsetByPipetteMount, SourceType) from opentrons.types import Mount, Point, Location from opentrons.hardware_control import ( ThreadManager, CriticalPoint, Pipette, robot_calibration, util) @@ -80,6 +80,11 @@ def __init__( deck_load_name = SHORT_TRASH_DECK if ff.short_fixed_trash() \ else STANDARD_DECK self._deck = Deck(load_name=deck_load_name) + self._filtered_hw_pips = self._filter_hw_pips() + self._deck_calibration, self._pipette_calibrations,\ + self._tip_lengths = self._get_current_calibrations() + self._check_valid_calibrations() + self._tip_racks: Optional[List['LabwareDefinition']] = tip_rack_defs self._active_pipette, self._pip_info = self._select_starting_pipette() @@ -212,6 +217,10 @@ def get_required_labware(self) -> List[RequiredLabware]: def get_active_tiprack(self) -> RequiredLabware: return RequiredLabware.from_lw(self.active_tiprack) + def _filter_hw_pips(self): + hw_instr = self._hardware._attached_instruments + return {m: p for m, p in hw_instr.items() if p} + def _select_starting_pipette( self) -> Tuple[PipetteInfo, List[PipetteInfo]]: """ @@ -224,15 +233,12 @@ def _select_starting_pipette( raise RobotServerError( definition=CalibrationError.NO_PIPETTE_ATTACHED, flow='Calibration Health Check') - pips = {m: p for m, p in self._hardware._attached_instruments.items() - if p} + pips = self._filtered_hw_pips # TODO(lc - 10/30): Clean up repeated logic here by fetching/storing # calibrations at the beginning of the session - self._check_valid_calibrations(pips) if len(pips) == 1: for mount, pip in pips.items(): - pip_calibration = \ - self._get_stored_pipette_offset_cal(pip, mount) + pip_calibration = self._pipette_calibrations[mount] info = PipetteInfo( channels=pip.config.channels, rank=PipetteRank.first, @@ -270,8 +276,22 @@ def _select_starting_pipette( l_info.rank = PipetteRank.second return r_info, [r_info, l_info] + def _get_current_calibrations(self): + deck = get.get_robot_deck_attitude() + pipette_offsets = { + m: get.get_pipette_offset(p.pipette_id, m) + for m, p in self._filtered_hw_pips.items() + } + tip_lengths = { + m: self._get_tip_length_from_pipette(m, p) + for m, p in self._filtered_hw_pips.items()} + return deck, pipette_offsets, tip_lengths + def _get_tip_length_from_pipette( - self, mount, pipette) -> Optional[float]: + self, mount: Mount, pipette: Pipette + ) -> Optional[Tuple[float, str, labware.Labware]]: + if not pipette.pipette_id: + return None pip_offset = get.get_pipette_offset( pipette.pipette_id, mount) if not pip_offset or not pip_offset.uri: @@ -282,20 +302,17 @@ def _get_tip_length_from_pipette( namespace=details.namespace, version=details.version, parent=position) - return get.load_tip_length_calibration( + tip_length = get.load_tip_length_calibration( pipette.pipette_id, tiprack._implementation.get_definition(), '')['tipLength'] + return (tip_length, pipette.pipette_id, tiprack) - def _check_valid_calibrations(self, pipettes: Dict): - deck = get.get_robot_deck_attitude() + def _check_valid_calibrations(self): + deck = self._deck_calibration tip_length = all( - self._get_tip_length_from_pipette(mount, pip) - for mount, pip in pipettes.items()) - pipette = all( - get.get_pipette_offset( - pip.pipette_id, mount) - for mount, pip in pipettes.items()) + tl for tl in self._tip_lengths.values()) + pipette = all(po for po in self._pipette_calibrations.values()) if not deck or not pipette or not tip_length: raise RobotServerError( definition=CalibrationError.UNCALIBRATED_ROBOT, @@ -535,28 +552,62 @@ def _update_compare_status_by_state( status=status.value, comparingHeight=info) intermediate_map.set_value('pipetteOffset', pip) elif self.current_state == State.comparingPointOne: + old_status = RobotHealthCheck.status_from_string( + intermediate_map.pipetteOffset.status) updated_status =\ self._check_and_update_status( - status, intermediate_map.pipetteOffset.status) + status, old_status) intermediate_map.pipetteOffset.comparingPointOne = info intermediate_map.pipetteOffset.status = updated_status deck = DeckComparisonMap( status=status.value, comparingPointOne=info) intermediate_map.set_value('deck', deck) elif self.current_state == State.comparingPointTwo: + old_status = RobotHealthCheck.status_from_string( + intermediate_map.deck.status) updated_status =\ self._check_and_update_status( - status, intermediate_map.deck.status) + status, old_status) intermediate_map.deck.status = updated_status intermediate_map.deck.comparingPointTwo = info elif self.current_state == State.comparingPointThree: + old_status = RobotHealthCheck.status_from_string( + intermediate_map.deck.status) updated_status =\ self._check_and_update_status( - status, intermediate_map.deck.status) + status, old_status) intermediate_map.deck.status = updated_status intermediate_map.deck.comparingPointThree = info return intermediate_map + def _mark_bad(self): + pipette_offset_states = [ + State.comparingHeight, + State.comparingPointOne] + deck_calibration_states = [ + State.comparingPointOne, + State.comparingPointTwo, + State.comparingPointThree] + active_mount = self.active_pipette.mount + is_second_pipette =\ + self.active_pipette.rank == PipetteRank.second + only_one_pipette =\ + not self._is_checking_both_mounts() + pipette_state = is_second_pipette or only_one_pipette + if self.current_state == State.comparingTip: + modify.mark_bad( + self._tip_lengths[active_mount], + SourceType.calibration_check) + elif self.current_state in pipette_offset_states: + modify.mark_bad( + self._pipette_calibrations[active_mount], + SourceType.calibration_check) + elif self.current_state in deck_calibration_states\ + and pipette_state: + modify.mark_bad( + self._deck_calibration, + SourceType.calibration_check) + async def update_comparison_map(self): ref_pt, jogged_pt = self._get_reference_points_by_state() rank = self.active_pipette.rank @@ -581,6 +632,7 @@ async def update_comparison_map(self): status = RobotHealthCheck.IN_THRESHOLD if exceeds: + self._mark_bad() status = RobotHealthCheck.OUTSIDE_THRESHOLD info = ComparisonStatus(differenceVector=(jogged_pt - ref_pt), diff --git a/robot-server/robot_server/robot/calibration/helper_classes.py b/robot-server/robot_server/robot/calibration/helper_classes.py index 9e5b18ebfc88..3531ef2f7a27 100644 --- a/robot-server/robot_server/robot/calibration/helper_classes.py +++ b/robot-server/robot_server/robot/calibration/helper_classes.py @@ -19,6 +19,14 @@ class RobotHealthCheck(Enum): def __str__(self): return self.name + @classmethod + def status_from_string( + cls, status: str) -> 'RobotHealthCheck': + if status == 'IN_THRESHOLD': + return cls.IN_THRESHOLD + else: + return cls.OUTSIDE_THRESHOLD + class PipetteRank(str, Enum): """The rank in the order of pipettes to use within flow""" diff --git a/robot-server/tests/robot/calibration/check/test_user_flow.py b/robot-server/tests/robot/calibration/check/test_user_flow.py index 6b3cf918917e..b05af2cfdc1d 100644 --- a/robot-server/tests/robot/calibration/check/test_user_flow.py +++ b/robot-server/tests/robot/calibration/check/test_user_flow.py @@ -240,6 +240,27 @@ def mock_user_flow(mock_hw): yield m +@pytest.fixture +def mock_user_flow_bad_vectors(mock_hw): + with patch.object( + get, + 'get_robot_deck_attitude', + new=build_mock_deck_calibration()),\ + patch.object( + get, + 'load_tip_length_calibration', + new=build_mock_stored_tip_length()),\ + patch.object( + get, 'get_pipette_offset', + new=build_mock_stored_pipette_offset()): + m = CheckCalibrationUserFlow(hardware=mock_hw) + initial_pt = Point(1, 6, 5) + final_pt = Point(1, 1, 0) + m._get_reference_points_by_state =\ + MagicMock(return_value=(initial_pt, final_pt)) + yield m + + async def test_move_to_tip_rack(mock_user_flow): uf = mock_user_flow await uf.move_to_tip_rack() @@ -403,3 +424,44 @@ async def test_compare_points(mock_user_flow): assert uf.comparison_map.first.deck == expected_deck assert uf.comparison_map.second.pipetteOffset == new_pip assert uf.comparison_map.second.deck == new_deck + + +async def test_mark_bad_calibration(mock_user_flow_bad_vectors): + uf = mock_user_flow_bad_vectors + + with patch('opentrons.calibration_storage.modify.mark_bad') as m: + uf._current_state = CalibrationCheckState.comparingTip + await uf.update_comparison_map() + expected_tip_length_call = [ + uf._tip_lengths[uf.mount], + CSTypes.SourceType.calibration_check] + + m.assert_called_once_with(*expected_tip_length_call) + m.reset_mock() + + uf._current_state = CalibrationCheckState.comparingHeight + + await uf.update_comparison_map() + expected_pip_offset_call = ( + uf._pipette_calibrations[uf.mount], + CSTypes.SourceType.calibration_check) + m.assert_called_once_with(*expected_pip_offset_call) + m.reset_mock() + + await uf.change_active_pipette() + + uf._current_state = CalibrationCheckState.comparingHeight + await uf.update_comparison_map() + + uf._current_state = CalibrationCheckState.comparingPointOne + await uf.update_comparison_map() + m.reset_mock() + + uf._current_state = CalibrationCheckState.comparingPointTwo + await uf.update_comparison_map() + + expected_deck_cal_call = [ + uf._deck_calibration, + CSTypes.SourceType.calibration_check] + + m.assert_called_once_with(*expected_deck_cal_call)