From a6c078a4d309f2e7cb7c55a4050fd18a74f45f5a Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 2 Nov 2020 09:44:00 -0500 Subject: [PATCH 1/9] 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 ae035866ffa..7a38e17a8bc 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 9f59c391fe0..83b65d4deab 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 54d9c7ca558..8e8d7bc71f9 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 17afbabb96d..3cb37fe79de 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 fbdc250523f..021bac34687 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() @@ -213,6 +218,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]]: """ @@ -225,15 +234,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, @@ -271,8 +277,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: @@ -283,20 +303,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, @@ -538,28 +555,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 @@ -584,6 +635,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 9e5b18ebfc8..3531ef2f7a2 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 fb086e743c8..501fe82cdb4 100644 --- a/robot-server/tests/robot/calibration/check/test_user_flow.py +++ b/robot-server/tests/robot/calibration/check/test_user_flow.py @@ -241,6 +241,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() @@ -404,3 +425,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) From d0e672be0ffd5cd902bc95d74511384dc8b7b8fe Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Wed, 4 Nov 2020 15:53:05 -0500 Subject: [PATCH 2/9] Reload robot cal on exit --- robot-server/robot_server/robot/calibration/check/user_flow.py | 2 ++ 1 file changed, 2 insertions(+) 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 021bac34687..051e2ba63fd 100644 --- a/robot-server/robot_server/robot/calibration/check/user_flow.py +++ b/robot-server/robot_server/robot/calibration/check/user_flow.py @@ -831,4 +831,6 @@ async def exit_session(self): if self.hw_pipette.has_tip: await self.move_to_tip_rack() await self.return_tip() + # reload new deck calibration + self._hardware.reset_robot_calibration() await self._hardware.home() From 17ca7d50b5f6d1a4db2f54ef788b20c79309adde Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Thu, 5 Nov 2020 20:21:55 -0500 Subject: [PATCH 3/9] Reduce amount of repetitive code --- api/src/opentrons/calibration_storage/get.py | 18 ++-- .../opentrons/calibration_storage/modify.py | 95 ++++++++++++------- .../legacy_api/containers/__init__.py | 7 +- .../protocol_api/instrument_context.py | 2 +- .../robot/calibration/check/user_flow.py | 54 ++++++++--- .../robot/calibration/deck/user_flow.py | 2 +- .../calibration/pipette_offset/user_flow.py | 2 +- 7 files changed, 118 insertions(+), 62 deletions(-) diff --git a/api/src/opentrons/calibration_storage/get.py b/api/src/opentrons/calibration_storage/get.py index 7a38e17a8bc..11f462a7971 100644 --- a/api/src/opentrons/calibration_storage/get.py +++ b/api/src/opentrons/calibration_storage/get.py @@ -13,8 +13,7 @@ file_operators as io, helpers, migration, modify) if typing.TYPE_CHECKING: from opentrons_shared_data.labware.dev_types import LabwareDefinition - from .dev_types import ( - TipLengthCalibration, CalibrationIndexDict, CalibrationDict) + from .dev_types import CalibrationIndexDict, CalibrationDict def _format_calibration_type( @@ -77,12 +76,19 @@ def get_all_calibrations() -> typing.List[local_types.CalibrationInformation]: def _get_tip_length_data( pip_id: str, labware_hash: str, labware_load_name: str -) -> 'TipLengthCalibration': +) -> local_types.TipLengthCalibration: try: pip_tip_length_path = config.get_tip_length_cal_path()/f'{pip_id}.json' - tip_length_data =\ + tip_rack_data =\ io.read_cal_file(str(pip_tip_length_path)) - return tip_length_data[labware_hash] + tip_length_info = tip_rack_data[labware_hash] + return local_types.TipLengthCalibration( + tip_length=tip_length_info['tipLength'], + pipette=pip_id, + tiprack=labware_hash, + last_modified=tip_length_info['lastModified'], + source=_get_calibration_source(tip_length_info), + status=_get_calibration_status(tip_length_info)) except (FileNotFoundError, KeyError): raise local_types.TipLengthCalNotFound( f'Tip length of {labware_load_name} has not been ' @@ -118,7 +124,7 @@ def get_labware_calibration( def load_tip_length_calibration( pip_id: str, definition: 'LabwareDefinition', - parent: str) -> 'TipLengthCalibration': + parent: str) -> local_types.TipLengthCalibration: """ Function used to grab the current tip length associated with a particular tiprack. diff --git a/api/src/opentrons/calibration_storage/modify.py b/api/src/opentrons/calibration_storage/modify.py index 83b65d4deab..b880573a274 100644 --- a/api/src/opentrons/calibration_storage/modify.py +++ b/api/src/opentrons/calibration_storage/modify.py @@ -7,6 +7,7 @@ import typing from pathlib import Path +from dataclasses import asdict from opentrons import config from opentrons.types import Mount, Point @@ -23,7 +24,6 @@ TipLengthCalibration, PipTipLengthCalibration, DeckCalibrationData, PipetteCalibrationData, CalibrationStatusDict) from opentrons_shared_data.labware.dev_types import LabwareDefinition - from opentrons.protocol_api.labware import Labware def _add_to_index_offset_file(parent: str, slot: str, uri: str, lw_hash: str): @@ -108,7 +108,7 @@ def create_tip_length_data( definition: 'LabwareDefinition', parent: str, length: float, - cal_status: typing.Optional[local_types.CalibrationStatus] = None + cal_status: local_types.CalibrationStatus = None ) -> 'PipTipLengthCalibration': """ Function to correctly format tip length data. @@ -204,7 +204,7 @@ def save_robot_deck_attitude( pip_id: typing.Optional[str], lw_hash: typing.Optional[str], source: local_types.SourceType = None, - cal_status: typing.Optional[local_types.CalibrationStatus] = None): + cal_status: 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' @@ -247,7 +247,7 @@ def save_pipette_calibration( offset: Point, pip_id: str, mount: Mount, tiprack_hash: str, tiprack_uri: str, - cal_status: typing.Optional[local_types.CalibrationStatus] = None): + cal_status: local_types.CalibrationStatus = None): pip_dir = config.get_opentrons_path( 'pipette_calibration_dir') / mount.name.lower() pip_dir.mkdir(parents=True, exist_ok=True) @@ -270,34 +270,61 @@ def save_pipette_calibration( _add_to_pipette_offset_index_file(pip_id, mount) +@typing.overload 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) + calibration: local_types.DeckCalibration, + source_marked_bad: local_types.SourceType + ) -> local_types.DeckCalibration: ... + + +@typing.overload +def mark_bad( + calibration: local_types.PipetteOffsetCalibration, + source_marked_bad: local_types.SourceType + ) -> local_types.PipetteOffsetCalibration: ... + + +@typing.overload +def mark_bad( + calibration: local_types.TipLengthCalibration, + source_marked_bad: local_types.SourceType + ) -> local_types.TipLengthCalibration: ... + + +def mark_bad(calibration, source_marked_bad: local_types.SourceType): + caldict = asdict(calibration) + caldict['status'] = helpers.convert_to_dict(local_types.CalibrationStatus( + markedBad=True, source=source_marked_bad, markedAt=utc_now())) + return type(calibration)(**caldict) + +# 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/legacy_api/containers/__init__.py b/api/src/opentrons/legacy_api/containers/__init__.py index ee7b2592f58..8a7d8b3b12a 100644 --- a/api/src/opentrons/legacy_api/containers/__init__.py +++ b/api/src/opentrons/legacy_api/containers/__init__.py @@ -1,7 +1,6 @@ from collections import OrderedDict import itertools import logging -from typing import TYPE_CHECKING from opentrons.config import CONFIG from opentrons.data_storage import database from opentrons.util.vector import Vector @@ -23,11 +22,9 @@ get, helpers as cal_helpers, file_operators as io, + types as cal_types, modify) -if TYPE_CHECKING: - from opentrons.calibration_storage.dev_types import TipLengthCalibration - __all__ = [ 'Deck', @@ -290,7 +287,7 @@ def load_new_labware_def(definition): def load_tip_length_calibration( - pip_id: str, location) -> 'TipLengthCalibration': + pip_id: str, location) -> cal_types.TipLengthCalibration: placeable, _ = unpack_location(location) lw = placeable.get_parent() return get._get_tip_length_data( diff --git a/api/src/opentrons/protocol_api/instrument_context.py b/api/src/opentrons/protocol_api/instrument_context.py index 863cc058315..86917293c91 100644 --- a/api/src/opentrons/protocol_api/instrument_context.py +++ b/api/src/opentrons/protocol_api/instrument_context.py @@ -1532,6 +1532,6 @@ def _build_length_from_overlap() -> float: return get.load_tip_length_calibration( self.hw_pipette['pipette_id'], tiprack._implementation.get_definition(), - parent)['tipLength'] + parent).tip_length except TipLengthCalNotFound: return _build_length_from_overlap() 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 051e2ba63fd..f4597db9982 100644 --- a/robot-server/robot_server/robot/calibration/check/user_flow.py +++ b/robot-server/robot_server/robot/calibration/check/user_flow.py @@ -4,7 +4,7 @@ Callable, Dict, Any, TYPE_CHECKING) from typing_extensions import Literal -from opentrons.calibration_storage import get, helpers, modify +from opentrons.calibration_storage import get, helpers, modify, types as local_types from opentrons.calibration_storage.types import ( TipLengthCalNotFound, PipetteOffsetByPipetteMount, SourceType) from opentrons.types import Mount, Point, Location @@ -290,7 +290,7 @@ def _get_current_calibrations(self): def _get_tip_length_from_pipette( self, mount: Mount, pipette: Pipette - ) -> Optional[Tuple[float, str, labware.Labware]]: + ) -> Optional[local_types.TipLengthCalibration]: if not pipette.pipette_id: return None pip_offset = get.get_pipette_offset( @@ -303,11 +303,10 @@ def _get_tip_length_from_pipette( namespace=details.namespace, version=details.version, parent=position) - tip_length = get.load_tip_length_calibration( + return 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): deck = self._deck_calibration @@ -546,6 +545,10 @@ def _update_compare_status_by_state( info: ComparisonStatus, status: RobotHealthCheck) -> ComparisonStatePerCalibration: intermediate_map = getattr(self._comparison_map, rank.name) + is_second_pipette =\ + self.active_pipette.rank == PipetteRank.second + only_one_pipette = not self._is_checking_both_mounts() + deck_comparison_state = is_second_pipette or only_one_pipette if self.current_state == State.comparingTip: tip = TipComparisonMap( status=status.value, comparingTip=info) @@ -562,10 +565,11 @@ def _update_compare_status_by_state( 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: + if deck_comparison_state: + deck = DeckComparisonMap( + status=status.value, comparingPointOne=info) + intermediate_map.set_value('deck', deck) + elif self.current_state == State.comparingPointTwo and deck_comparison_state: old_status = RobotHealthCheck.status_from_string( intermediate_map.deck.status) updated_status =\ @@ -573,7 +577,7 @@ def _update_compare_status_by_state( status, old_status) intermediate_map.deck.status = updated_status intermediate_map.deck.comparingPointTwo = info - elif self.current_state == State.comparingPointThree: + elif self.current_state == State.comparingPointThree and deck_comparison_state: old_status = RobotHealthCheck.status_from_string( intermediate_map.deck.status) updated_status =\ @@ -598,18 +602,39 @@ def _mark_bad(self): not self._is_checking_both_mounts() pipette_state = is_second_pipette or only_one_pipette if self.current_state == State.comparingTip: - modify.mark_bad( + calibration = modify.mark_bad( self._tip_lengths[active_mount], SourceType.calibration_check) + tip_length_dict = { + 'tipLength': calibration.tip_length, + 'lastModified': calibration.last_modified, + 'source': calibration.source, + 'status': calibration.status + } + modify.save_tip_length_calibration( + calibration.pipette, tip_length_dict) elif self.current_state in pipette_offset_states: - modify.mark_bad( + calibration = modify.mark_bad( self._pipette_calibrations[active_mount], SourceType.calibration_check) + modify.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=calibration.status) elif self.current_state in deck_calibration_states\ and pipette_state: - modify.mark_bad( + calibration = modify.mark_bad( self._deck_calibration, SourceType.calibration_check) + modify.save_robot_deck_attitude( + transform=calibration.attitude, + pip_id=calibration.pipette_calibrated_with, + lw_hash=calibration.tiprack, + source=calibration.source, + cal_status=calibration.status) async def update_comparison_map(self): ref_pt, jogged_pt = self._get_reference_points_by_state() @@ -730,7 +755,7 @@ def _get_tip_length(self) -> float: return get.load_tip_length_calibration( pip_id, self.active_tiprack._implementation.get_definition(), - '')['tipLength'] + '').tip_length except TipLengthCalNotFound: tip_overlap = self.hw_pipette.config.tip_overlap.get( self.active_tiprack.uri, @@ -746,6 +771,7 @@ async def move_to_tip_rack(self): handler="move_to_tip_rack", condition="active tiprack") await self.register_initial_point() + await self.register_final_point() await self._move(Location(self.tip_origin, None)) async def move_to_deck(self): diff --git a/robot-server/robot_server/robot/calibration/deck/user_flow.py b/robot-server/robot_server/robot/calibration/deck/user_flow.py index 38cea08de8e..59ec28879d9 100644 --- a/robot-server/robot_server/robot/calibration/deck/user_flow.py +++ b/robot-server/robot_server/robot/calibration/deck/user_flow.py @@ -308,7 +308,7 @@ def _get_tip_length(self) -> float: pip_id, self._tip_rack._implementation.get_definition(), '' - )['tipLength'] + ).tip_length except TipLengthCalNotFound: tip_overlap = self._hw_pipette.config.tip_overlap.get( self._tip_rack.uri, diff --git a/robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py b/robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py index 24f98ea2978..f9d752b15ed 100644 --- a/robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py +++ b/robot-server/robot_server/robot/calibration/pipette_offset/user_flow.py @@ -267,7 +267,7 @@ def _get_stored_tip_length_cal(self) -> Optional[float]: return get.load_tip_length_calibration( self._hw_pipette.pipette_id, self._tip_rack._implementation.get_definition(), - '')['tipLength'] + '').tip_length except TipLengthCalNotFound: return None From 25f647b0ed8572eeb91818b9ef6d2bb2eae6f189 Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 9 Nov 2020 10:21:58 -0500 Subject: [PATCH 4/9] Modify return types for tip length --- .../opentrons/calibration_storage/modify.py | 40 +++---------------- .../opentrons/protocol_api/test_context.py | 9 ++++- .../opentrons/protocol_api/test_offsets.py | 11 ++++- 3 files changed, 22 insertions(+), 38 deletions(-) diff --git a/api/src/opentrons/calibration_storage/modify.py b/api/src/opentrons/calibration_storage/modify.py index b880573a274..6ace025adb2 100644 --- a/api/src/opentrons/calibration_storage/modify.py +++ b/api/src/opentrons/calibration_storage/modify.py @@ -293,38 +293,8 @@ def mark_bad( def mark_bad(calibration, source_marked_bad: local_types.SourceType): caldict = asdict(calibration) - caldict['status'] = helpers.convert_to_dict(local_types.CalibrationStatus( - markedBad=True, source=source_marked_bad, markedAt=utc_now())) - return type(calibration)(**caldict) - -# 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) + # remove current status key + del caldict['status'] + status = local_types.CalibrationStatus( + markedBad=True, source=source_marked_bad, markedAt=utc_now()) + return type(calibration)(**caldict, status=status) diff --git a/api/tests/opentrons/protocol_api/test_context.py b/api/tests/opentrons/protocol_api/test_context.py index 54e0150b7c0..dfb37c1107a 100644 --- a/api/tests/opentrons/protocol_api/test_context.py +++ b/api/tests/opentrons/protocol_api/test_context.py @@ -856,7 +856,14 @@ def test_tip_length_for_caldata(loop, monkeypatch, use_new_calibration): instr = ctx.load_instrument('p20_single_gen2', 'left') tiprack = ctx.load_labware('geb_96_tiprack_10ul', '1') mock_tip_length = mock.Mock() - mock_tip_length.return_value = {'tipLength': 2} + mock_tip_length.return_value =\ + cs_types.TipLengthCalibration( + tip_length=2, + pipette='fake id', + tiprack='fake_hash', + last_modified='some time', + source=cs_types.SourceType.user, + status=cs_types.CalibrationStatus(markedBad=False)) monkeypatch.setattr(get, 'load_tip_length_calibration', mock_tip_length) instr._tip_length_for.cache_clear() assert instr._tip_length_for(tiprack) == 2 diff --git a/api/tests/opentrons/protocol_api/test_offsets.py b/api/tests/opentrons/protocol_api/test_offsets.py index 804044f0c39..b32f1ebf97c 100644 --- a/api/tests/opentrons/protocol_api/test_offsets.py +++ b/api/tests/opentrons/protocol_api/test_offsets.py @@ -238,8 +238,15 @@ def test_load_tip_length_calibration_data(monkeypatch, clear_tlc_calibration): modify.save_tip_length_calibration(PIPETTE_ID, test_data) result = get.load_tip_length_calibration( PIPETTE_ID, minimalLabwareDef, parent) - - assert result == test_data[MOCK_HASH] + expected = cs_types.TipLengthCalibration( + tip_length=tip_length, + pipette=PIPETTE_ID, + source=cs_types.SourceType.user, + status=cs_types.CalibrationStatus(markedBad=False), + tiprack=MOCK_HASH, + last_modified=test_data[MOCK_HASH]['lastModified'] + ) + assert result == expected def test_clear_tip_length_calibration_data(monkeypatch): From c4f3b8cb69ee60d747da4325dae3708412f4a456 Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 9 Nov 2020 10:23:12 -0500 Subject: [PATCH 5/9] Fixup tip length comparison and movement to tiprack --- .../robot/calibration/check/user_flow.py | 47 ++++++++++++------- .../robot_server/robot/calibration/util.py | 1 + 2 files changed, 31 insertions(+), 17 deletions(-) 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 f4597db9982..3ed78838429 100644 --- a/robot-server/robot_server/robot/calibration/check/user_flow.py +++ b/robot-server/robot_server/robot/calibration/check/user_flow.py @@ -4,7 +4,8 @@ Callable, Dict, Any, TYPE_CHECKING) from typing_extensions import Literal -from opentrons.calibration_storage import get, helpers, modify, types as local_types +from opentrons.calibration_storage import ( + get, helpers, modify, types as cal_types) from opentrons.calibration_storage.types import ( TipLengthCalNotFound, PipetteOffsetByPipetteMount, SourceType) from opentrons.types import Mount, Point, Location @@ -290,7 +291,7 @@ def _get_current_calibrations(self): def _get_tip_length_from_pipette( self, mount: Mount, pipette: Pipette - ) -> Optional[local_types.TipLengthCalibration]: + ) -> Optional[cal_types.TipLengthCalibration]: if not pipette.pipette_id: return None pip_offset = get.get_pipette_offset( @@ -569,7 +570,8 @@ def _update_compare_status_by_state( deck = DeckComparisonMap( status=status.value, comparingPointOne=info) intermediate_map.set_value('deck', deck) - elif self.current_state == State.comparingPointTwo and deck_comparison_state: + elif self.current_state == State.comparingPointTwo\ + and deck_comparison_state: old_status = RobotHealthCheck.status_from_string( intermediate_map.deck.status) updated_status =\ @@ -577,7 +579,8 @@ def _update_compare_status_by_state( status, old_status) intermediate_map.deck.status = updated_status intermediate_map.deck.comparingPointTwo = info - elif self.current_state == State.comparingPointThree and deck_comparison_state: + elif self.current_state == State.comparingPointThree\ + and deck_comparison_state: old_status = RobotHealthCheck.status_from_string( intermediate_map.deck.status) updated_status =\ @@ -605,22 +608,27 @@ def _mark_bad(self): calibration = modify.mark_bad( self._tip_lengths[active_mount], SourceType.calibration_check) - tip_length_dict = { - 'tipLength': calibration.tip_length, - 'lastModified': calibration.last_modified, - 'source': calibration.source, - 'status': calibration.status - } + tip_definition =\ + self.active_tiprack._implementation.get_definition() + parent = self.active_tiprack.parent + tip_length_dict = modify.create_tip_length_data( + definition=tip_definition, + parent=parent, + length=calibration.tip_length, + cal_status=calibration.status + ) modify.save_tip_length_calibration( calibration.pipette, tip_length_dict) elif self.current_state in pipette_offset_states: calibration = modify.mark_bad( self._pipette_calibrations[active_mount], SourceType.calibration_check) + pipette_id = self.hw_pipette.pipette_id + assert pipette_id, 'Cannot update pipette offset calibraion' modify.save_pipette_calibration( offset=Point(*calibration.offset), - pip_id=calibration.pipette, - mount=Mount.string_to_mount(calibration.mount), + pip_id=pipette_id, + mount=active_mount, tiprack_hash=calibration.tiprack, tiprack_uri=calibration.uri, cal_status=calibration.status) @@ -673,9 +681,7 @@ async def update_comparison_map(self): def _get_reference_points_by_state(self): saved_points = self._reference_points if self.current_state == State.comparingTip: - initial = saved_points.tip.initial_point +\ - Point(0, 0, self._get_tip_length()) - return initial,\ + return saved_points.tip.initial_point,\ saved_points.tip.final_point elif self.current_state == State.comparingHeight: return saved_points.height.initial_point,\ @@ -730,7 +736,14 @@ async def register_final_point(self): critical_point = self.critical_point_override current_point = \ await self.get_current_point(critical_point) - if self.current_state == State.comparingTip: + if self.current_state == State.comparingNozzle: + # The reference point is unique in that + # a user might jog after moving to this + # position so we need to do a final save + # every time a jog command is issued. + self._reference_points.tip.initial_point = \ + current_point + elif self.current_state == State.comparingTip: self._reference_points.tip.final_point = \ current_point elif self.current_state == State.comparingHeight: @@ -771,7 +784,6 @@ async def move_to_tip_rack(self): handler="move_to_tip_rack", condition="active tiprack") await self.register_initial_point() - await self.register_final_point() await self._move(Location(self.tip_origin, None)) async def move_to_deck(self): @@ -804,6 +816,7 @@ async def move_to_reference_point(self): deck=self._deck, cal_block_target_well=cal_block_target_well) await self._move(ref_loc) + await self.register_final_point() async def move_to_point_one(self): await self._move(self._get_move_to_point_loc_by_state()) diff --git a/robot-server/robot_server/robot/calibration/util.py b/robot-server/robot_server/robot/calibration/util.py index 23b7098aa93..13b35aa3dec 100644 --- a/robot-server/robot_server/robot/calibration/util.py +++ b/robot-server/robot_server/robot/calibration/util.py @@ -141,6 +141,7 @@ async def return_tip(user_flow: CalibrationUserFlow, tip_length: float): abs_position=to_pt, critical_point=cp) await user_flow.hardware.drop_tip(user_flow.mount) + user_flow.reset_tip_origin() async def move(user_flow: CalibrationUserFlow, From 126db45413f5943c80f90d2267370c797508b4cf Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 9 Nov 2020 10:25:03 -0500 Subject: [PATCH 6/9] Fixup tests --- .../test_calibration_check.tavern.yaml | 130 ++++++++++-------- .../robot/calibration/check/test_user_flow.py | 26 ++-- 2 files changed, 88 insertions(+), 68 deletions(-) diff --git a/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml b/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml index 29e423bd5c1..c8ffb5e2aa6 100644 --- a/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml +++ b/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml @@ -340,18 +340,20 @@ stages: links: !anydict data: <<: *session_data - details: - <<: *session_data_attribute_details - currentStep: comparingPointOne - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: !anydict - second: - tipLength: null - pipetteOffset: null - deck: null + attributes: + <<: *session_data_attributes + details: + <<: *session_data_attribute_details + currentStep: comparingPointOne + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Go to next check request: @@ -370,18 +372,20 @@ stages: links: !anydict data: <<: *session_data - details: - <<: *session_data_attribute_details - currentStep: comparingPointTwo - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: !anydict - second: - tipLength: null - pipetteOffset: null - deck: null + attributes: + <<: *session_data_attributes + details: + <<: *session_data_attribute_details + currentStep: comparingPointTwo + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Compare first pipette point two request: @@ -400,18 +404,20 @@ stages: links: !anydict data: <<: *session_data - details: - <<: *session_data_attribute_details - currentStep: comparingPointTwo - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: !anydict - second: - tipLength: null - pipetteOffset: null - deck: null + attributes: + <<: *session_data_attributes + details: + <<: *session_data_attribute_details + currentStep: comparingPointTwo + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Go to next check request: @@ -430,18 +436,20 @@ stages: links: !anydict data: <<: *session_data - details: - <<: *session_data_attribute_details - currentStep: comparingPointThree - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: !anydict - second: - tipLength: null - pipetteOffset: null - deck: null + attributes: + <<: *session_data_attributes + details: + <<: *session_data_attribute_details + currentStep: comparingPointThree + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Compare first pipette point three request: @@ -461,18 +469,20 @@ stages: links: !anydict data: <<: *session_data - details: - <<: *session_data_attribute_details - currentStep: comparingPointThree - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: !anydict - second: - tipLength: null - pipetteOffset: null - deck: null + attributes: + <<: *session_data_attributes + details: + <<: *session_data_attribute_details + currentStep: comparingPointThree + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Delete the session request: 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 501fe82cdb4..2d810d53447 100644 --- a/robot-server/tests/robot/calibration/check/test_user_flow.py +++ b/robot-server/tests/robot/calibration/check/test_user_flow.py @@ -154,7 +154,14 @@ def build_mock_stored_pipette_offset(kind='normal'): def build_mock_stored_tip_length(kind='normal'): if kind == 'normal': - return MagicMock(return_value={'tipLength': 30}) + tip_length = CSTypes.TipLengthCalibration( + tip_length=30, + pipette='fake id', + tiprack='fake_hash', + last_modified='some time', + source=CSTypes.SourceType.user, + status=CSTypes.CalibrationStatus(markedBad=False)) + return MagicMock(return_value=tip_length) else: return MagicMock(return_value=None) @@ -375,7 +382,7 @@ async def test_compare_points(mock_user_flow): await uf.update_comparison_map() assert uf.comparison_map.first.pipetteOffset == expected_pip - assert uf.comparison_map.first.deck == expected_deck + assert uf.comparison_map.first.deck is None assert uf.comparison_map.second.pipetteOffset is None assert uf.comparison_map.second.deck is None @@ -388,7 +395,7 @@ async def test_compare_points(mock_user_flow): await uf.update_comparison_map() expected_deck.comparingPointTwo = expected_status assert uf.comparison_map.first.pipetteOffset == expected_pip - assert uf.comparison_map.first.deck == expected_deck + assert uf.comparison_map.first.deck is None assert uf.comparison_map.second.pipetteOffset is None assert uf.comparison_map.second.deck is None @@ -401,7 +408,7 @@ async def test_compare_points(mock_user_flow): await uf.update_comparison_map() expected_deck.comparingPointThree = expected_status assert uf.comparison_map.first.pipetteOffset == expected_pip - assert uf.comparison_map.first.deck == expected_deck + assert uf.comparison_map.first.deck is None assert uf.comparison_map.second.pipetteOffset is None assert uf.comparison_map.second.deck is None @@ -422,21 +429,24 @@ async def test_compare_points(mock_user_flow): comparingPointOne=expected_status) assert uf.comparison_map.first.pipetteOffset == expected_pip - assert uf.comparison_map.first.deck == expected_deck + assert uf.comparison_map.first.deck is None 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: + storage_path = 'opentrons.calibration_storage.modify' + with patch(f'{storage_path}.mark_bad') as m,\ + patch(f'{storage_path}.create_tip_length_data'),\ + patch(f'{storage_path}.save_tip_length_calibration'),\ + patch(f'{storage_path}.save_pipette_calibration'),\ + patch(f'{storage_path}.save_robot_deck_attitude'): 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() From 8368d195cf558fc7fe0ddfe31a0f73b66b3be965 Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 9 Nov 2020 10:30:57 -0500 Subject: [PATCH 7/9] Correctly check for the deck calibration status depending on the amount of pipettes --- .../components/CheckCalibration/ResultsSummary.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/src/components/CheckCalibration/ResultsSummary.js b/app/src/components/CheckCalibration/ResultsSummary.js index c5f5db43f4d..8d2a34f8b45 100644 --- a/app/src/components/CheckCalibration/ResultsSummary.js +++ b/app/src/components/CheckCalibration/ResultsSummary.js @@ -48,7 +48,12 @@ const LOOKING_FOR_DATA = 'Looking for your detailed calibration data?' const DOWNLOAD_SUMMARY = 'Download JSON summary' export function ResultsSummary(props: CalibrationPanelProps): React.Node { - const { comparisonsByPipette, instruments, cleanUpAndExit } = props + const { + comparisonsByPipette, + instruments, + checkBothPipettes, + cleanUpAndExit, + } = props if (!comparisonsByPipette || !instruments) { return null @@ -93,7 +98,10 @@ export function ResultsSummary(props: CalibrationPanelProps): React.Node { }, } - const deckCalibrationResult = comparisonsByPipette.first.deck?.status ?? null + const getDeckCalibration = checkBothPipettes + ? comparisonsByPipette.second.deck?.status + : comparisonsByPipette.first.deck?.status + const deckCalibrationResult = getDeckCalibration ?? null return ( <> From c3d8612ff7df30b18766cf57b110366f5d0f8b8a Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 9 Nov 2020 12:12:11 -0500 Subject: [PATCH 8/9] fixup from rebase --- .../test_calibration_check.tavern.yaml | 130 ++++++++---------- 1 file changed, 60 insertions(+), 70 deletions(-) diff --git a/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml b/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml index c8ffb5e2aa6..326f4e868b0 100644 --- a/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml +++ b/robot-server/tests/integration/sessions/test_calibration_check.tavern.yaml @@ -340,20 +340,18 @@ stages: links: !anydict data: <<: *session_data - attributes: - <<: *session_data_attributes - details: - <<: *session_data_attribute_details - currentStep: comparingPointOne - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: null - second: - tipLength: null - pipetteOffset: null - deck: null + details: + <<: *session_data_attribute_details + currentStep: comparingPointOne + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Go to next check request: @@ -372,20 +370,18 @@ stages: links: !anydict data: <<: *session_data - attributes: - <<: *session_data_attributes - details: - <<: *session_data_attribute_details - currentStep: comparingPointTwo - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: null - second: - tipLength: null - pipetteOffset: null - deck: null + details: + <<: *session_data_attribute_details + currentStep: comparingPointTwo + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Compare first pipette point two request: @@ -404,20 +400,18 @@ stages: links: !anydict data: <<: *session_data - attributes: - <<: *session_data_attributes - details: - <<: *session_data_attribute_details - currentStep: comparingPointTwo - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: null - second: - tipLength: null - pipetteOffset: null - deck: null + details: + <<: *session_data_attribute_details + currentStep: comparingPointTwo + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Go to next check request: @@ -436,20 +430,18 @@ stages: links: !anydict data: <<: *session_data - attributes: - <<: *session_data_attributes - details: - <<: *session_data_attribute_details - currentStep: comparingPointThree - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: null - second: - tipLength: null - pipetteOffset: null - deck: null + details: + <<: *session_data_attribute_details + currentStep: comparingPointThree + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Compare first pipette point three request: @@ -469,20 +461,18 @@ stages: links: !anydict data: <<: *session_data - attributes: - <<: *session_data_attributes - details: - <<: *session_data_attribute_details - currentStep: comparingPointThree - comparisonsByPipette: - first: - tipLength: !anydict - pipetteOffset: !anydict - deck: null - second: - tipLength: null - pipetteOffset: null - deck: null + details: + <<: *session_data_attribute_details + currentStep: comparingPointThree + comparisonsByPipette: + first: + tipLength: !anydict + pipetteOffset: !anydict + deck: null + second: + tipLength: null + pipetteOffset: null + deck: null - name: Delete the session request: From b686d83711f16aef8ff8e48db9e733accd8a399b Mon Sep 17 00:00:00 2001 From: laura_danielle Date: Mon, 9 Nov 2020 15:13:17 -0500 Subject: [PATCH 9/9] Save initial height comparison point correctly --- robot-server/robot_server/robot/calibration/check/user_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3ed78838429..956570b86d8 100644 --- a/robot-server/robot_server/robot/calibration/check/user_flow.py +++ b/robot-server/robot_server/robot/calibration/check/user_flow.py @@ -711,7 +711,7 @@ async def register_initial_point(self): current_point self._reference_points.tip.final_point = \ current_point - elif self.current_state == State.inspectingTip: + elif self.current_state == State.comparingTip: self._reference_points.height.initial_point = \ current_point self._reference_points.height.final_point = \