From b32aba66863a6cbac90ef99a68b36fe026bc9fdf Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Mon, 11 Sep 2023 17:01:32 -0400 Subject: [PATCH 1/4] chore(api): remove spammy log messages These messages about missing calibrations are completely meaningless on the flex and are incredibly spammy; pull them. They might have meaning on the OT-2 but honestly we have a _lot_ of ui surrounding them so whatever. --- .../opentrons/calibration_storage/ot2/pipette_offset.py | 5 +++-- api/src/opentrons/calibration_storage/ot3/tip_length.py | 7 +++++-- .../protocol_engine/resources/labware_data_provider.py | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/api/src/opentrons/calibration_storage/ot2/pipette_offset.py b/api/src/opentrons/calibration_storage/ot2/pipette_offset.py index 83616a3737a..63ead0fc5bc 100644 --- a/api/src/opentrons/calibration_storage/ot2/pipette_offset.py +++ b/api/src/opentrons/calibration_storage/ot2/pipette_offset.py @@ -88,12 +88,13 @@ def get_pipette_offset( **io.read_cal_file(pipette_calibration_filepath) ) except FileNotFoundError: - log.warning(f"Calibrations for {pipette_id} on {mount} does not exist.") + log.debug(f"Calibrations for {pipette_id} on {mount} does not exist.") return None except (json.JSONDecodeError, ValidationError): - log.warning( + log.debug( f"Malformed calibrations for {pipette_id} on {mount}. Please factory reset your calibrations." ) + # TODO: Delete the bad calibration here maybe? return None diff --git a/api/src/opentrons/calibration_storage/ot3/tip_length.py b/api/src/opentrons/calibration_storage/ot3/tip_length.py index 9525aafd86e..638560851aa 100644 --- a/api/src/opentrons/calibration_storage/ot3/tip_length.py +++ b/api/src/opentrons/calibration_storage/ot3/tip_length.py @@ -37,19 +37,22 @@ def tip_lengths_for_pipette( ) -> typing.Dict[str, v1.TipLengthModel]: tip_lengths = {} try: + # While you technically could drop some data in for tip length calibration on the flex, + # it is not necessary and there is no UI frontend for it, so this code will mostly be + # taking the FileNotFoundError path. tip_length_filepath = config.get_tip_length_cal_path() / f"{pipette_id}.json" all_tip_lengths_for_pipette = io.read_cal_file(tip_length_filepath) for tiprack, data in all_tip_lengths_for_pipette.items(): try: tip_lengths[tiprack] = v1.TipLengthModel(**data) except (json.JSONDecodeError, ValidationError): - log.warning( + log.debug( f"Tip length calibration is malformed for {tiprack} on {pipette_id}" ) pass return tip_lengths except FileNotFoundError: - log.warning(f"Tip length calibrations not found for {pipette_id}") + # this is the overwhelmingly common case return tip_lengths diff --git a/api/src/opentrons/protocol_engine/resources/labware_data_provider.py b/api/src/opentrons/protocol_engine/resources/labware_data_provider.py index 0123c85ad97..0b08720d4e9 100644 --- a/api/src/opentrons/protocol_engine/resources/labware_data_provider.py +++ b/api/src/opentrons/protocol_engine/resources/labware_data_provider.py @@ -82,5 +82,5 @@ def _get_calibrated_tip_length_sync( f"No calibrated tip length found for {pipette_serial}," f" using nominal fallback value of {nominal_fallback}" ) - log.warning(message, exc_info=e) + log.debug(message, exc_info=e) return nominal_fallback From ba2650fc232582b64b8ccfec588036e498080b87 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 12 Sep 2023 11:00:44 -0400 Subject: [PATCH 2/4] more debug logs --- api/src/opentrons/calibration_storage/ot2/pipette_offset.py | 2 +- api/src/opentrons/calibration_storage/ot2/tip_length.py | 2 +- api/src/opentrons/calibration_storage/ot3/pipette_offset.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/calibration_storage/ot2/pipette_offset.py b/api/src/opentrons/calibration_storage/ot2/pipette_offset.py index 63ead0fc5bc..ac09a736b4e 100644 --- a/api/src/opentrons/calibration_storage/ot2/pipette_offset.py +++ b/api/src/opentrons/calibration_storage/ot2/pipette_offset.py @@ -91,7 +91,7 @@ def get_pipette_offset( log.debug(f"Calibrations for {pipette_id} on {mount} does not exist.") return None except (json.JSONDecodeError, ValidationError): - log.debug( + log.warning( f"Malformed calibrations for {pipette_id} on {mount}. Please factory reset your calibrations." ) # TODO: Delete the bad calibration here maybe? diff --git a/api/src/opentrons/calibration_storage/ot2/tip_length.py b/api/src/opentrons/calibration_storage/ot2/tip_length.py index d2be974b12b..eca8f723f09 100644 --- a/api/src/opentrons/calibration_storage/ot2/tip_length.py +++ b/api/src/opentrons/calibration_storage/ot2/tip_length.py @@ -50,7 +50,7 @@ def tip_lengths_for_pipette( pass return tip_lengths except FileNotFoundError: - log.warning(f"Tip length calibrations not found for {pipette_id}") + log.debug(f"Tip length calibrations not found for {pipette_id}") return tip_lengths diff --git a/api/src/opentrons/calibration_storage/ot3/pipette_offset.py b/api/src/opentrons/calibration_storage/ot3/pipette_offset.py index f7f25ccc384..fcd53bbbf3e 100644 --- a/api/src/opentrons/calibration_storage/ot3/pipette_offset.py +++ b/api/src/opentrons/calibration_storage/ot3/pipette_offset.py @@ -85,7 +85,7 @@ def get_pipette_offset( **io.read_cal_file(pipette_calibration_filepath) ) except FileNotFoundError: - log.warning(f"Calibrations for {pipette_id} on {mount} does not exist.") + log.debug(f"Calibrations for {pipette_id} on {mount} does not exist.") return None except (json.JSONDecodeError, ValidationError): log.warning( From e776275751ab804aa34fa55cd1cd70347970663c Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 12 Sep 2023 13:10:12 -0400 Subject: [PATCH 3/4] Refuse to get offset and tl on flex Make the legacy routes for tip_length and pipette_offset 503 on flex and not try and get the offsets using the old calibration accessors, since they are hardcoded to use the ot2 versions of calibration which do not apply to the flex and dumped errors in the logs all the time. the api side change supports testing. --- .../instruments/ot3/instrument_calibration.py | 6 +++-- robot-server/robot_server/hardware.py | 14 ++++++++++- .../service/pipette_offset/router.py | 11 ++++++--- .../robot_server/service/tip_length/router.py | 10 ++++++-- robot-server/tests/conftest.py | 14 ++++++++++- .../test_pipette_offset_access.tavern.yaml | 23 +++++++++++++++++++ .../test_tip_length_access.tavern.yaml | 19 +++++++++++++++ 7 files changed, 88 insertions(+), 9 deletions(-) diff --git a/api/src/opentrons/hardware_control/instruments/ot3/instrument_calibration.py b/api/src/opentrons/hardware_control/instruments/ot3/instrument_calibration.py index d7f6926ea80..c5fe989c2dd 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/instrument_calibration.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/instrument_calibration.py @@ -10,10 +10,12 @@ from opentrons.types import Point, Mount # TODO change this when imports are fixed -from opentrons.calibration_storage.ot3.pipette_offset import save_pipette_calibration +from opentrons.calibration_storage.ot3.pipette_offset import ( + save_pipette_calibration, + get_pipette_offset, +) from opentrons.calibration_storage import ( types as cal_top_types, - get_pipette_offset, ot3_gripper_offset, ) from opentrons.hardware_control.types import OT3Mount diff --git a/robot-server/robot_server/hardware.py b/robot-server/robot_server/hardware.py index faaf63ce959..5c0deb148be 100644 --- a/robot-server/robot_server/hardware.py +++ b/robot-server/robot_server/hardware.py @@ -26,7 +26,7 @@ feature_flags as ff, ) from opentrons.util.helpers import utc_now -from opentrons.hardware_control import ThreadManagedHardware, HardwareControlAPI +from opentrons.hardware_control import ThreadManagedHardware, HardwareControlAPI, API from opentrons.hardware_control.simulator_setup import load_simulator_thread_manager from opentrons.hardware_control.types import ( HardwareEvent, @@ -50,6 +50,7 @@ ) from .errors.robot_errors import ( NotSupportedOnOT2, + NotSupportedOnFlex, HardwareNotYetInitialized, HardwareFailedToInitialize, ) @@ -226,6 +227,17 @@ def get_ot3_hardware( return cast(OT3API, thread_manager.wrapped()) +def get_ot2_hardware( + thread_manager: ThreadManagedHardware = Depends(get_thread_manager), +) -> "API": + """Get an OT2 hardware controller.""" + if not thread_manager.wraps_instance(API): + raise NotSupportedOnFlex( + detail="This route is only available on an OT-2." + ).as_error(status.HTTP_403_FORBIDDEN) + return cast(API, thread_manager.wrapped()) + + async def get_firmware_update_manager( app_state: AppState = Depends(get_app_state), thread_manager: ThreadManagedHardware = Depends(get_thread_manager), diff --git a/robot-server/robot_server/service/pipette_offset/router.py b/robot-server/robot_server/service/pipette_offset/router.py index c6e04dc2a45..e3229a0c3e7 100644 --- a/robot-server/robot_server/service/pipette_offset/router.py +++ b/robot-server/robot_server/service/pipette_offset/router.py @@ -1,15 +1,18 @@ from starlette import status -from fastapi import APIRouter +from fastapi import APIRouter, Depends from typing import Optional from opentrons import types as ot_types from opentrons.calibration_storage.ot2 import pipette_offset, models +from robot_server.hardware import get_ot2_hardware from robot_server.errors import ErrorBody from robot_server.service.pipette_offset import models as pip_models from robot_server.service.errors import RobotServerError, CommonErrorDef from robot_server.service.shared_models import calibration as cal_model +from opentrons.hardware_control import API + router = APIRouter() @@ -45,7 +48,9 @@ def _format_calibration( response_model=pip_models.MultipleCalibrationsResponse, ) async def get_all_pipette_offset_calibrations( - pipette_id: Optional[str] = None, mount: Optional[pip_models.MountType] = None + pipette_id: Optional[str] = None, + mount: Optional[pip_models.MountType] = None, + _: API = Depends(get_ot2_hardware), ) -> pip_models.MultipleCalibrationsResponse: all_calibrations = pipette_offset.get_all_pipette_offset_calibrations() @@ -75,7 +80,7 @@ async def get_all_pipette_offset_calibrations( responses={status.HTTP_404_NOT_FOUND: {"model": ErrorBody}}, ) async def delete_specific_pipette_offset_calibration( - pipette_id: str, mount: pip_models.MountType + pipette_id: str, mount: pip_models.MountType, _: API = Depends(get_ot2_hardware) ): try: pipette_offset.delete_pipette_offset_file( diff --git a/robot-server/robot_server/service/tip_length/router.py b/robot-server/robot_server/service/tip_length/router.py index d79973039d6..2d6461e0b7f 100644 --- a/robot-server/robot_server/service/tip_length/router.py +++ b/robot-server/robot_server/service/tip_length/router.py @@ -1,15 +1,18 @@ from starlette import status -from fastapi import APIRouter +from fastapi import APIRouter, Depends from typing import Optional from opentrons.calibration_storage import types as cal_types from opentrons.calibration_storage.ot2 import tip_length, models +from robot_server.hardware import get_ot2_hardware from robot_server.errors import ErrorBody from robot_server.service.tip_length import models as tl_models from robot_server.service.errors import RobotServerError, CommonErrorDef from robot_server.service.shared_models import calibration as cal_model +from opentrons.hardware_control import API + router = APIRouter() @@ -48,6 +51,7 @@ async def get_all_tip_length_calibrations( tiprack_hash: Optional[str] = None, pipette_id: Optional[str] = None, tiprack_uri: Optional[str] = None, + _: API = Depends(get_ot2_hardware), ) -> tl_models.MultipleCalibrationsResponse: all_calibrations = tip_length.get_all_tip_length_calibrations() if not all_calibrations: @@ -79,7 +83,9 @@ async def get_all_tip_length_calibrations( "serial and tiprack hash", responses={status.HTTP_404_NOT_FOUND: {"model": ErrorBody}}, ) -async def delete_specific_tip_length_calibration(tiprack_hash: str, pipette_id: str): +async def delete_specific_tip_length_calibration( + tiprack_hash: str, pipette_id: str, _: API = Depends(get_ot2_hardware) +): try: tip_length.delete_tip_length_calibration(tiprack_hash, pipette_id) except cal_types.TipLengthCalNotFound: diff --git a/robot-server/tests/conftest.py b/robot-server/tests/conftest.py index 1e30fb7c4a6..a414541f45f 100644 --- a/robot-server/tests/conftest.py +++ b/robot-server/tests/conftest.py @@ -30,7 +30,7 @@ from opentrons.types import Point, Mount from robot_server import app -from robot_server.hardware import get_hardware +from robot_server.hardware import get_hardware, get_ot2_hardware from robot_server.versioning import API_VERSION_HEADER, LATEST_API_VERSION_HEADER_VALUE from robot_server.service.session.manager import SessionManager from robot_server.persistence import get_sql_engine, create_sql_engine @@ -139,11 +139,23 @@ async def get_version_override() -> ComponentVersions: del app.dependency_overrides[get_versions] +@pytest.fixture +def _override_ot2_hardware_with_mock(hardware: MagicMock) -> Iterator[None]: + async def get_ot2_hardware_override() -> API: + """Override for the get_ot2_hardware FastAPI dependency.""" + return MagicMock(spec=API) + + app.dependency_overrides[get_ot2_hardware] = get_ot2_hardware_override + yield + del app.dependency_overrides[get_ot2_hardware] + + @pytest.fixture def api_client( _override_hardware_with_mock: None, _override_sql_engine_with_mock: None, _override_version_with_mock: None, + _override_ot2_hardware_with_mock: None, ) -> TestClient: client = TestClient(app) client.headers.update({API_VERSION_HEADER: LATEST_API_VERSION_HEADER_VALUE}) diff --git a/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml b/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml index 4145c682740..ccf961aa06d 100644 --- a/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml +++ b/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml @@ -124,3 +124,26 @@ stages: mount: 'right' response: status_code: 200 + +--- +test_name: Pipette calibrations inaccessible on flex +marks: + - usefixtures: + - ot3_server_base_url + - set_up_pipette_offset_temp_directory +stages: + - name: GET request 403s + request: + url: "{ot3_server_base_url}/calibration/pipette_offset" + method: GET + response: + status_code: 403 + - name: DELETE request 403s + request: + url: "{ot3_server_base_url}/calibration/pipette_offset" + method: DELETE + params: + pipette_id: '321' + mount: 'right' + response: + status_code: 403 diff --git a/robot-server/tests/integration/test_tip_length_access.tavern.yaml b/robot-server/tests/integration/test_tip_length_access.tavern.yaml index d8d2f009485..9b181e0877a 100644 --- a/robot-server/tests/integration/test_tip_length_access.tavern.yaml +++ b/robot-server/tests/integration/test_tip_length_access.tavern.yaml @@ -159,3 +159,22 @@ stages: method: DELETE response: status_code: 404 +--- +test_name: Tip length inaccessible on flex +marks: + - ot3_only + - usefixtures: + - ot3_server_base_url +stages: + - name: GET request 403s + request: + url: "{ot3_server_base_url}/calibration/tip_length" + method: GET + response: + status_code: 403 + - name: DELETE request 403s + request: + url: "{ot3_server_base_url}/calibration/tip_length" + method: DELETE + response: + status_code: 403 From e29442b9b7c6e4b88d229ad622bcdb04d0ef72f3 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 12 Sep 2023 15:15:55 -0400 Subject: [PATCH 4/4] forgot to save this file --- .../tests/integration/test_pipette_offset_access.tavern.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml b/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml index ccf961aa06d..90495534bfc 100644 --- a/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml +++ b/robot-server/tests/integration/test_pipette_offset_access.tavern.yaml @@ -128,6 +128,7 @@ stages: --- test_name: Pipette calibrations inaccessible on flex marks: + - ot3_only - usefixtures: - ot3_server_base_url - set_up_pipette_offset_temp_directory