From 3927333864737e31b736f410cabe6cf678c3ce26 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Wed, 24 Apr 2024 15:16:57 -0400 Subject: [PATCH 1/7] keep pipette mounts disengaged when attaching and detaching 96-channel pipette and bracket --- .../move_to_maintenance_position.py | 46 ++++++------------- 1 file changed, 13 insertions(+), 33 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/calibration/move_to_maintenance_position.py b/api/src/opentrons/protocol_engine/commands/calibration/move_to_maintenance_position.py index 67d60eead86..8ce067963ab 100644 --- a/api/src/opentrons/protocol_engine/commands/calibration/move_to_maintenance_position.py +++ b/api/src/opentrons/protocol_engine/commands/calibration/move_to_maintenance_position.py @@ -80,46 +80,25 @@ async def execute( ot3_api = ensure_ot3_hardware( self._hardware_api, ) - # the 96-channel mount is disengaged during gripper calibration and - # must be homed before the gantry position can be called + max_height_z_tip = ot3_api.get_instrument_max_height(Mount.LEFT) + # disengage the gripper z mount if present and enabled await ot3_api.prepare_for_mount_movement(Mount.LEFT) - current_position_mount = await ot3_api.gantry_position( + + await ot3_api.retract(Mount.LEFT) + current_pos = await ot3_api.gantry_position( Mount.LEFT, critical_point=CriticalPoint.MOUNT ) - max_height_z_mount = ot3_api.get_instrument_max_height( - Mount.LEFT, critical_point=CriticalPoint.MOUNT + await ot3_api.move_to( + mount=Mount.LEFT, + abs_position=Point(x=_ATTACH_POINT.x, y=_ATTACH_POINT.y, z=current_pos.z), + critical_point=CriticalPoint.MOUNT, ) - max_height_z_tip = ot3_api.get_instrument_max_height(Mount.LEFT) - # avoid using motion planning waypoints because we do not need to move the z at this moment - movement_points = [ - # move the z to the highest position - Point( - x=current_position_mount.x, - y=current_position_mount.y, - z=max_height_z_mount, - ), - # move in x,y without going down the z - Point(x=_ATTACH_POINT.x, y=_ATTACH_POINT.y, z=max_height_z_mount), - ] - - await ot3_api.prepare_for_mount_movement(Mount.LEFT) - for movement in movement_points: - await ot3_api.move_to( - mount=Mount.LEFT, - abs_position=movement, - critical_point=CriticalPoint.MOUNT, - ) if params.mount != MountType.EXTENSION: - - # disengage the gripper z to enable the e-brake, this prevents the gripper - # z from dropping when the right mount carriage gets released from the - # mount during 96-channel detach flow - if ot3_api.has_gripper(): - await ot3_api.disengage_axes([Axis.Z_G]) - if params.maintenancePosition == MaintenancePosition.ATTACH_INSTRUMENT: - mount_to_axis = Axis.by_mount(params.mount.to_hw_mount()) + mount = params.mount.to_hw_mount() + mount_to_axis = Axis.by_mount(mount) + await ot3_api.prepare_for_mount_movement(mount) await ot3_api.move_axes( { mount_to_axis: _INSTRUMENT_ATTACH_Z_POINT, @@ -134,6 +113,7 @@ async def execute( Axis.Z_R: max_motion_range + _RIGHT_MOUNT_Z_MARGIN, } ) + await ot3_api.disengage_axes([Axis.Z_L, Axis.Z_R]) return MoveToMaintenancePositionResult() From 70c2348fb49e0fb452247b0cb8e7076d05145a34 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:34:10 -0400 Subject: [PATCH 2/7] update tests --- .../test_move_to_maintenance_position.py | 61 +++++++------------ 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py index 8cc9017a021..66211979cb8 100644 --- a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py +++ b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py @@ -45,6 +45,7 @@ def subject( ), ], ) +@pytest.mark.parametrize("calibrate_mount", [MountType.LEFT, MountType.RIGHT]) async def test_calibration_move_to_location_implementation( decoy: Decoy, subject: MoveToMaintenancePositionImplementation, @@ -52,23 +53,18 @@ async def test_calibration_move_to_location_implementation( ot3_hardware_api: OT3API, maintenance_position: MaintenancePosition, verify_axes: Mapping[Axis, float], + calibrate_mount: MountType, ) -> None: """Command should get a move to target location and critical point and should verify move_to call.""" params = MoveToMaintenancePositionParams( - mount=MountType.LEFT, maintenancePosition=maintenance_position + mount=calibrate_mount, maintenancePosition=maintenance_position ) decoy.when( await ot3_hardware_api.gantry_position( Mount.LEFT, critical_point=CriticalPoint.MOUNT ) - ).then_return(Point(x=1, y=2, z=3)) - - decoy.when( - ot3_hardware_api.get_instrument_max_height( - Mount.LEFT, critical_point=CriticalPoint.MOUNT - ) - ).then_return(250) + ).then_return(Point(x=1, y=2, z=250)) decoy.when(ot3_hardware_api.get_instrument_max_height(Mount.LEFT)).then_return(300) @@ -76,14 +72,13 @@ async def test_calibration_move_to_location_implementation( assert result == MoveToMaintenancePositionResult() decoy.verify( - await ot3_hardware_api.move_to( - mount=Mount.LEFT, - abs_position=Point(x=1, y=2, z=250), - critical_point=CriticalPoint.MOUNT, - ), + await ot3_hardware_api.prepare_for_mount_movement(Mount.LEFT), + times=1, + ) + decoy.verify( + await ot3_hardware_api.retract(Mount.LEFT), times=1, ) - decoy.verify( await ot3_hardware_api.move_to( mount=Mount.LEFT, @@ -92,21 +87,18 @@ async def test_calibration_move_to_location_implementation( ), times=1, ) - decoy.verify( await ot3_hardware_api.move_axes( position=verify_axes, ), times=1, ) - - if params.maintenancePosition == MaintenancePosition.ATTACH_INSTRUMENT: - decoy.verify( - await ot3_hardware_api.disengage_axes( - list(verify_axes.keys()), - ), - times=1, - ) + decoy.verify( + await ot3_hardware_api.disengage_axes( + list(verify_axes.keys()), + ), + times=1, + ) @pytest.mark.ot3_only @@ -118,35 +110,26 @@ async def test_calibration_move_to_location_implementation_for_gripper( ) -> None: """Command should get a move to target location and critical point and should verify move_to call.""" params = MoveToMaintenancePositionParams( - mount=MountType.LEFT, maintenancePosition=MaintenancePosition.ATTACH_INSTRUMENT + mount=MountType.EXTENSION, maintenancePosition=MaintenancePosition.ATTACH_INSTRUMENT ) decoy.when( await ot3_hardware_api.gantry_position( Mount.LEFT, critical_point=CriticalPoint.MOUNT ) - ).then_return(Point(x=1, y=2, z=3)) - - decoy.when( - ot3_hardware_api.get_instrument_max_height( - Mount.LEFT, critical_point=CriticalPoint.MOUNT - ) - ).then_return(250) - + ).then_return(Point(x=1, y=2, z=250)) decoy.when(ot3_hardware_api.get_instrument_max_height(Mount.LEFT)).then_return(300) result = await subject.execute(params=params) assert result == MoveToMaintenancePositionResult() - decoy.verify( - await ot3_hardware_api.move_to( - mount=Mount.LEFT, - abs_position=Point(x=1, y=2, z=250), - critical_point=CriticalPoint.MOUNT, - ), + await ot3_hardware_api.prepare_for_mount_movement(Mount.LEFT), + times=1, + ) + decoy.verify( + await ot3_hardware_api.retract(Mount.LEFT), times=1, ) - decoy.verify( await ot3_hardware_api.move_to( mount=Mount.LEFT, From b81fa09e5ada94697ba4be069058e8ce74a87464 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:39:27 -0400 Subject: [PATCH 3/7] make sure we tests right mount as well --- .../test_move_to_maintenance_position.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py index 66211979cb8..b664bb80929 100644 --- a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py +++ b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py @@ -33,27 +33,38 @@ def subject( @pytest.mark.ot3_only @pytest.mark.parametrize( - "maintenance_position, verify_axes", + "calibrate_moumt, maintenance_position, verify_axes", [ ( + MountType.LEFT, MaintenancePosition.ATTACH_INSTRUMENT, {Axis.Z_L: 400}, ), ( + MountType.RIGHT, + MaintenancePosition.ATTACH_INSTRUMENT, + {Axis.Z_R: 400}, + ), + ( + MountType.LEFT, + MaintenancePosition.ATTACH_PLATE, + {Axis.Z_L: 90, Axis.Z_R: 105}, + ), + ( + MountType.RIGHT, MaintenancePosition.ATTACH_PLATE, {Axis.Z_L: 90, Axis.Z_R: 105}, ), ], ) -@pytest.mark.parametrize("calibrate_mount", [MountType.LEFT, MountType.RIGHT]) async def test_calibration_move_to_location_implementation( decoy: Decoy, subject: MoveToMaintenancePositionImplementation, state_view: StateView, ot3_hardware_api: OT3API, + calibrate_mount: MountType, maintenance_position: MaintenancePosition, verify_axes: Mapping[Axis, float], - calibrate_mount: MountType, ) -> None: """Command should get a move to target location and critical point and should verify move_to call.""" params = MoveToMaintenancePositionParams( From 6f205c006eba835f0c95a4a16ca01bfcd288dc54 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:40:19 -0400 Subject: [PATCH 4/7] typo --- .../commands/calibration/test_move_to_maintenance_position.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py index b664bb80929..dab3046abe6 100644 --- a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py +++ b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py @@ -33,7 +33,7 @@ def subject( @pytest.mark.ot3_only @pytest.mark.parametrize( - "calibrate_moumt, maintenance_position, verify_axes", + "calibrate_mount, maintenance_position, verify_axes", [ ( MountType.LEFT, From fd01f7638f7e6839880e5384ae25da6341a3293b Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:44:35 -0400 Subject: [PATCH 5/7] test order --- .../test_move_to_maintenance_position.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py index dab3046abe6..bc72bc42f75 100644 --- a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py +++ b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py @@ -82,33 +82,22 @@ async def test_calibration_move_to_location_implementation( result = await subject.execute(params=params) assert result == MoveToMaintenancePositionResult() + decoy.verify( await ot3_hardware_api.prepare_for_mount_movement(Mount.LEFT), - times=1, - ) - decoy.verify( await ot3_hardware_api.retract(Mount.LEFT), - times=1, - ) - decoy.verify( await ot3_hardware_api.move_to( mount=Mount.LEFT, abs_position=Point(x=0, y=110, z=250), critical_point=CriticalPoint.MOUNT, ), - times=1, - ) - decoy.verify( + await ot3_hardware_api.prepare_for_mount_movement(calibrate_mount.to_hw_mount()), await ot3_hardware_api.move_axes( position=verify_axes, ), - times=1, - ) - decoy.verify( await ot3_hardware_api.disengage_axes( list(verify_axes.keys()), ), - times=1, ) From 19973dc9dc024a37b1503c197ad711904b57bf89 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:00:17 -0400 Subject: [PATCH 6/7] make sure were testing the call order --- .../test_move_to_maintenance_position.py | 93 +++++++++++-------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py index bc72bc42f75..ea84ed7da2d 100644 --- a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py +++ b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py @@ -32,43 +32,61 @@ def subject( @pytest.mark.ot3_only -@pytest.mark.parametrize( - "calibrate_mount, maintenance_position, verify_axes", - [ - ( - MountType.LEFT, - MaintenancePosition.ATTACH_INSTRUMENT, - {Axis.Z_L: 400}, - ), - ( - MountType.RIGHT, - MaintenancePosition.ATTACH_INSTRUMENT, - {Axis.Z_R: 400}, +@pytest.mark.parametrize("mount_type", [MountType.LEFT, MountType.RIGHT]) +async def test_calibration_move_to_location_implementatio_for_attach_instrument( + decoy: Decoy, + subject: MoveToMaintenancePositionImplementation, + state_view: StateView, + ot3_hardware_api: OT3API, + mount_type: MountType, +) -> None: + """Command should get a move to target location and critical point and should verify move_to call.""" + params = MoveToMaintenancePositionParams( + mount=mount_type, maintenancePosition=MaintenancePosition.ATTACH_INSTRUMENT + ) + + decoy.when( + await ot3_hardware_api.gantry_position( + Mount.LEFT, critical_point=CriticalPoint.MOUNT + ) + ).then_return(Point(x=1, y=2, z=250)) + + decoy.when(ot3_hardware_api.get_instrument_max_height(Mount.LEFT)).then_return(300) + + result = await subject.execute(params=params) + assert result == MoveToMaintenancePositionResult() + + hw_mount = mount_type.to_hw_mount() + decoy.verify( + await ot3_hardware_api.prepare_for_mount_movement(Mount.LEFT), + await ot3_hardware_api.retract(Mount.LEFT), + await ot3_hardware_api.move_to( + mount=Mount.LEFT, + abs_position=Point(x=0, y=110, z=250), + critical_point=CriticalPoint.MOUNT, ), - ( - MountType.LEFT, - MaintenancePosition.ATTACH_PLATE, - {Axis.Z_L: 90, Axis.Z_R: 105}, + await ot3_hardware_api.prepare_for_mount_movement(hw_mount), + await ot3_hardware_api.move_axes( + position={Axis.by_mount(hw_mount): 400}, ), - ( - MountType.RIGHT, - MaintenancePosition.ATTACH_PLATE, - {Axis.Z_L: 90, Axis.Z_R: 105}, + await ot3_hardware_api.disengage_axes( + [Axis.by_mount(hw_mount)], ), - ], -) -async def test_calibration_move_to_location_implementation( + ) + + +@pytest.mark.ot3_only +@pytest.mark.parametrize("mount_type", [MountType.LEFT, MountType.RIGHT]) +async def test_calibration_move_to_location_implementatio_for_attach_plate( decoy: Decoy, subject: MoveToMaintenancePositionImplementation, state_view: StateView, ot3_hardware_api: OT3API, - calibrate_mount: MountType, - maintenance_position: MaintenancePosition, - verify_axes: Mapping[Axis, float], + mount_type: MountType, ) -> None: """Command should get a move to target location and critical point and should verify move_to call.""" params = MoveToMaintenancePositionParams( - mount=calibrate_mount, maintenancePosition=maintenance_position + mount=mount_type, maintenancePosition=MaintenancePosition.ATTACH_PLATE ) decoy.when( @@ -82,7 +100,6 @@ async def test_calibration_move_to_location_implementation( result = await subject.execute(params=params) assert result == MoveToMaintenancePositionResult() - decoy.verify( await ot3_hardware_api.prepare_for_mount_movement(Mount.LEFT), await ot3_hardware_api.retract(Mount.LEFT), @@ -91,12 +108,14 @@ async def test_calibration_move_to_location_implementation( abs_position=Point(x=0, y=110, z=250), critical_point=CriticalPoint.MOUNT, ), - await ot3_hardware_api.prepare_for_mount_movement(calibrate_mount.to_hw_mount()), await ot3_hardware_api.move_axes( - position=verify_axes, + position={ + Axis.Z_L: 90, + Axis.Z_R: 105, + } ), await ot3_hardware_api.disengage_axes( - list(verify_axes.keys()), + [Axis.Z_L, Axis.Z_R], ), ) @@ -110,7 +129,8 @@ async def test_calibration_move_to_location_implementation_for_gripper( ) -> None: """Command should get a move to target location and critical point and should verify move_to call.""" params = MoveToMaintenancePositionParams( - mount=MountType.EXTENSION, maintenancePosition=MaintenancePosition.ATTACH_INSTRUMENT + mount=MountType.EXTENSION, + maintenancePosition=MaintenancePosition.ATTACH_INSTRUMENT, ) decoy.when( @@ -122,21 +142,15 @@ async def test_calibration_move_to_location_implementation_for_gripper( result = await subject.execute(params=params) assert result == MoveToMaintenancePositionResult() + decoy.verify( await ot3_hardware_api.prepare_for_mount_movement(Mount.LEFT), - times=1, - ) - decoy.verify( await ot3_hardware_api.retract(Mount.LEFT), - times=1, - ) - decoy.verify( await ot3_hardware_api.move_to( mount=Mount.LEFT, abs_position=Point(x=0, y=110, z=250), critical_point=CriticalPoint.MOUNT, ), - times=1, ) decoy.verify( @@ -145,7 +159,6 @@ async def test_calibration_move_to_location_implementation_for_gripper( ), times=0, ) - decoy.verify( await ot3_hardware_api.disengage_axes( [Axis.Z_G], From 8a6e5c2f4cd3e6f5aae6e5319bd3e5cc904918b1 Mon Sep 17 00:00:00 2001 From: ahiuchingau <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:01:31 -0400 Subject: [PATCH 7/7] fix lint --- .../commands/calibration/test_move_to_maintenance_position.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py index ea84ed7da2d..df58ab7dbc0 100644 --- a/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py +++ b/api/tests/opentrons/protocol_engine/commands/calibration/test_move_to_maintenance_position.py @@ -1,6 +1,6 @@ """Test for Calibration Set Up Position Implementation.""" from __future__ import annotations -from typing import TYPE_CHECKING, Mapping +from typing import TYPE_CHECKING import pytest from decoy import Decoy