Skip to content

Commit

Permalink
feat(api): remove use of LPC offsets during labware movement (#13144)
Browse files Browse the repository at this point in the history
  • Loading branch information
sanni-t authored Jul 21, 2023
1 parent 98f191d commit e1da55c
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 151 deletions.
4 changes: 0 additions & 4 deletions api/src/opentrons/protocol_api/core/engine/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,6 @@ def move_labware(
DeckSlotName, LabwareCore, ModuleCore, NonConnectedModuleCore, OffDeckType
],
use_gripper: bool,
use_pick_up_location_lpc_offset: bool,
use_drop_location_lpc_offset: bool,
pick_up_offset: Optional[Tuple[float, float, float]],
drop_offset: Optional[Tuple[float, float, float]],
) -> None:
Expand Down Expand Up @@ -286,8 +284,6 @@ def move_labware(
labware_id=labware_core.labware_id,
new_location=to_location,
strategy=strategy,
use_pick_up_location_lpc_offset=use_pick_up_location_lpc_offset,
use_drop_location_lpc_offset=use_drop_location_lpc_offset,
pick_up_offset=_pick_up_offset,
drop_offset=_drop_offset,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ def move_labware(
OffDeckType,
],
use_gripper: bool,
use_pick_up_location_lpc_offset: bool,
use_drop_location_lpc_offset: bool,
pick_up_offset: Optional[Tuple[float, float, float]],
drop_offset: Optional[Tuple[float, float, float]],
) -> None:
Expand Down
2 changes: 0 additions & 2 deletions api/src/opentrons/protocol_api/core/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ def move_labware(
labware_core: LabwareCoreType,
new_location: Union[DeckSlotName, LabwareCoreType, ModuleCoreType, OffDeckType],
use_gripper: bool,
use_pick_up_location_lpc_offset: bool,
use_drop_location_lpc_offset: bool,
pick_up_offset: Optional[Tuple[float, float, float]],
drop_offset: Optional[Tuple[float, float, float]],
) -> None:
Expand Down
4 changes: 0 additions & 4 deletions api/src/opentrons/protocol_api/protocol_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,6 @@ def move_labware(
labware: Labware,
new_location: Union[DeckLocation, Labware, ModuleTypes, OffDeckType],
use_gripper: bool = False,
use_pick_up_location_lpc_offset: bool = False,
use_drop_location_lpc_offset: bool = False,
pick_up_offset: Optional[Mapping[str, float]] = None,
drop_offset: Optional[Mapping[str, float]] = None,
) -> None:
Expand Down Expand Up @@ -600,8 +598,6 @@ def move_labware(
labware_core=labware._core,
new_location=location,
use_gripper=use_gripper,
use_pick_up_location_lpc_offset=use_pick_up_location_lpc_offset,
use_drop_location_lpc_offset=use_drop_location_lpc_offset,
pick_up_offset=_pick_up_offset,
drop_offset=_drop_offset,
)
Expand Down
4 changes: 0 additions & 4 deletions api/src/opentrons/protocol_engine/clients/sync_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,6 @@ def move_labware(
labware_id: str,
new_location: LabwareLocation,
strategy: LabwareMovementStrategy,
use_pick_up_location_lpc_offset: bool,
use_drop_location_lpc_offset: bool,
pick_up_offset: Optional[LabwareOffsetVector],
drop_offset: Optional[LabwareOffsetVector],
) -> commands.MoveLabwareResult:
Expand All @@ -156,8 +154,6 @@ def move_labware(
labwareId=labware_id,
newLocation=new_location,
strategy=strategy,
usePickUpLocationLpcOffset=use_pick_up_location_lpc_offset,
useDropLocationLpcOffset=use_drop_location_lpc_offset,
pickUpOffset=pick_up_offset,
dropOffset=drop_offset,
)
Expand Down
19 changes: 3 additions & 16 deletions api/src/opentrons/protocol_engine/commands/move_labware.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
OnLabwareLocation,
LabwareMovementStrategy,
LabwareOffsetVector,
ExperimentalOffsetData,
LabwareMovementOffsetData,
)
from ..errors import LabwareMovementNotAllowedError
from ..resources import labware_validation
Expand All @@ -35,16 +35,6 @@ class MoveLabwareParams(BaseModel):
description="Whether to use the gripper to perform the labware movement"
" or to perform a manual movement with an option to pause.",
)
usePickUpLocationLpcOffset: bool = Field(
False,
description="Whether to use LPC offset of the labware associated with its "
"pick up location. Experimental param, subject to change.",
)
useDropLocationLpcOffset: bool = Field(
False,
description="Whether to use LPC offset of the labware associated with its "
"drop off location. Experimental param, subject to change.",
)
pickUpOffset: Optional[LabwareOffsetVector] = Field(
None,
description="Offset to use when picking up labware. "
Expand Down Expand Up @@ -145,9 +135,7 @@ async def execute(self, params: MoveLabwareParams) -> MoveLabwareResult:
validated_new_loc = self._labware_movement.ensure_valid_gripper_location(
available_new_location,
)
experimental_offset_data = ExperimentalOffsetData(
usePickUpLocationLpcOffset=params.usePickUpLocationLpcOffset,
useDropLocationLpcOffset=params.useDropLocationLpcOffset,
user_offset_data = LabwareMovementOffsetData(
pickUpOffset=params.pickUpOffset,
dropOffset=params.dropOffset,
)
Expand All @@ -156,8 +144,7 @@ async def execute(self, params: MoveLabwareParams) -> MoveLabwareResult:
labware_id=params.labwareId,
current_location=validated_current_loc,
new_location=validated_new_loc,
experimental_offset_data=experimental_offset_data,
new_offset_id=new_offset_id,
user_offset_data=user_offset_data,
)
elif params.strategy == LabwareMovementStrategy.MANUAL_MOVE_WITH_PAUSE:
# Pause to allow for manual labware movement
Expand Down
48 changes: 11 additions & 37 deletions api/src/opentrons/protocol_engine/execution/labware_movement.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
OnLabwareLocation,
LabwareLocation,
LabwareOffsetVector,
ExperimentalOffsetData,
LabwareMovementOffsetData,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -89,8 +89,7 @@ async def move_labware_with_gripper(
labware_id: str,
current_location: Union[DeckSlotLocation, ModuleLocation, OnLabwareLocation],
new_location: Union[DeckSlotLocation, ModuleLocation, OnLabwareLocation],
experimental_offset_data: ExperimentalOffsetData,
new_offset_id: Optional[str],
user_offset_data: LabwareMovementOffsetData,
) -> None:
"""Move a loaded labware from one location to another."""
use_virtual_gripper = self._state_store.config.use_virtual_gripper
Expand Down Expand Up @@ -125,12 +124,8 @@ async def move_labware_with_gripper(
async with self._thermocycler_plate_lifter.lift_plate_for_labware_movement(
labware_location=current_location
):
labware_pickup_offset = self.get_experimental_labware_movement_offset_vector(
use_current_offset=experimental_offset_data.usePickUpLocationLpcOffset,
current_offset_vector=self._state_store.labware.get_labware_offset_vector(
labware_id
),
additional_offset_vector=experimental_offset_data.pickUpOffset,
labware_pickup_offset = self.get_final_labware_movement_offset_vector(
additional_offset_vector=user_offset_data.pickUpOffset,
is_pickup_from_tc2=is_tc2_pickup,
)

Expand All @@ -153,18 +148,9 @@ async def move_labware_with_gripper(
await ot3api.move_to(
mount=gripper_mount, abs_position=waypoints_to_labware[-1]
)

await ot3api.grip(force_newtons=LABWARE_GRIP_FORCE)

new_labware_offset = (
self._state_store.labware.get_labware_offset(new_offset_id).vector
if new_offset_id
else None
)
labware_drop_offset = self.get_experimental_labware_movement_offset_vector(
use_current_offset=experimental_offset_data.useDropLocationLpcOffset,
current_offset_vector=new_labware_offset,
additional_offset_vector=experimental_offset_data.dropOffset,
labware_drop_offset = self.get_final_labware_movement_offset_vector(
additional_offset_vector=user_offset_data.dropOffset
)

# TODO: see https://opentrons.atlassian.net/browse/RLAB-215
Expand Down Expand Up @@ -220,31 +206,19 @@ def _get_gripper_movement_waypoints(

# TODO (spp, 2022-12-14): https://opentrons.atlassian.net/browse/RLAB-237
@staticmethod
def get_experimental_labware_movement_offset_vector(
use_current_offset: bool,
current_offset_vector: Optional[LabwareOffsetVector],
def get_final_labware_movement_offset_vector(
additional_offset_vector: Optional[LabwareOffsetVector],
is_pickup_from_tc2: bool = False,
) -> LabwareOffsetVector:
"""Calculate the final labware offset vector to use in labware movement."""
_current_offset_vector = current_offset_vector or LabwareOffsetVector(
x=0, y=0, z=0
)
_additional_offset_vector = additional_offset_vector or LabwareOffsetVector(
user_offset_vector = additional_offset_vector or LabwareOffsetVector(
x=0, y=0, z=0
)
if is_pickup_from_tc2:
# TODO (fps, 2022-05-30): Remove this once RLAB-295 is merged
_additional_offset_vector.z += _ADDITIONAL_TC2_PICKUP_OFFSET

if not use_current_offset:
return _additional_offset_vector
else:
return LabwareOffsetVector(
x=_current_offset_vector.x + _additional_offset_vector.x,
y=_current_offset_vector.y + _additional_offset_vector.y,
z=_current_offset_vector.z + _additional_offset_vector.z,
)
user_offset_vector.z += _ADDITIONAL_TC2_PICKUP_OFFSET

return user_offset_vector

# TODO (spp, 2022-10-20): move to labware view
@staticmethod
Expand Down
9 changes: 3 additions & 6 deletions api/src/opentrons/protocol_engine/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,10 @@ class LabwareMovementStrategy(str, Enum):
MANUAL_MOVE_WITHOUT_PAUSE = "manualMoveWithoutPause"


# TODO (spp, 2022-12-14): https://opentrons.atlassian.net/browse/RLAB-237
@dataclass(frozen=True)
class ExperimentalOffsetData(BaseModel):
"""The result of a load module procedure."""
@dataclass
class LabwareMovementOffsetData:
"""Offsets to be used during labware movement."""

usePickUpLocationLpcOffset: bool
useDropLocationLpcOffset: bool
pickUpOffset: Optional[LabwareOffsetVector]
dropOffset: Optional[LabwareOffsetVector]

Expand Down
28 changes: 4 additions & 24 deletions api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,17 +517,11 @@ def test_load_adapter(
],
)
@pytest.mark.parametrize(
argnames=[
"use_pick_up_location_lpc_offset",
"use_drop_location_lpc_offset",
"pick_up_offset",
"drop_offset",
],
argnames=["pick_up_offset", "drop_offset"],
argvalues=[
(False, False, None, None),
(True, False, None, None),
(False, True, None, (4, 5, 6)),
(True, True, (4, 5, 6), (4, 5, 6)),
(None, None),
(None, (4, 5, 6)),
((4, 5, 6), (4, 5, 6)),
],
)
def test_move_labware(
Expand All @@ -536,8 +530,6 @@ def test_move_labware(
mock_engine_client: EngineClient,
expected_strategy: LabwareMovementStrategy,
use_gripper: bool,
use_pick_up_location_lpc_offset: bool,
use_drop_location_lpc_offset: bool,
pick_up_offset: Optional[Tuple[float, float, float]],
drop_offset: Optional[Tuple[float, float, float]],
) -> None:
Expand All @@ -552,8 +544,6 @@ def test_move_labware(
labware_core=labware,
new_location=DeckSlotName.SLOT_5,
use_gripper=use_gripper,
use_pick_up_location_lpc_offset=use_pick_up_location_lpc_offset,
use_drop_location_lpc_offset=use_drop_location_lpc_offset,
pick_up_offset=pick_up_offset,
drop_offset=drop_offset,
)
Expand All @@ -562,8 +552,6 @@ def test_move_labware(
labware_id="labware-id",
new_location=DeckSlotLocation(slotName=DeckSlotName.SLOT_5),
strategy=expected_strategy,
use_pick_up_location_lpc_offset=use_pick_up_location_lpc_offset,
use_drop_location_lpc_offset=use_drop_location_lpc_offset,
pick_up_offset=LabwareOffsetVector(x=4, y=5, z=6)
if pick_up_offset
else None,
Expand Down Expand Up @@ -594,8 +582,6 @@ def test_move_labware_on_non_connected_module(
labware_core=labware,
new_location=non_connected_module_core,
use_gripper=False,
use_pick_up_location_lpc_offset=False,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=None,
)
Expand All @@ -604,8 +590,6 @@ def test_move_labware_on_non_connected_module(
labware_id="labware-id",
new_location=ModuleLocation(moduleId="module-id"),
strategy=LabwareMovementStrategy.MANUAL_MOVE_WITH_PAUSE,
use_pick_up_location_lpc_offset=False,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=None,
)
Expand All @@ -630,8 +614,6 @@ def test_move_labware_off_deck(
labware_core=labware,
new_location=OFF_DECK,
use_gripper=False,
use_pick_up_location_lpc_offset=False,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=None,
)
Expand All @@ -640,8 +622,6 @@ def test_move_labware_off_deck(
labware_id="labware-id",
new_location=OFF_DECK_LOCATION,
strategy=LabwareMovementStrategy.MANUAL_MOVE_WITH_PAUSE,
use_pick_up_location_lpc_offset=False,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=None,
)
Expand Down
7 changes: 0 additions & 7 deletions api/tests/opentrons/protocol_api/test_protocol_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,6 @@ def test_move_labware_to_slot(
subject.move_labware(
labware=movable_labware,
new_location=42,
use_pick_up_location_lpc_offset=True,
drop_offset=drop_offset,
)

Expand All @@ -513,8 +512,6 @@ def test_move_labware_to_slot(
labware_core=mock_labware_core,
new_location=DeckSlotName.SLOT_1,
use_gripper=False,
use_pick_up_location_lpc_offset=True,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=(1, 2, 3),
)
Expand Down Expand Up @@ -554,8 +551,6 @@ def test_move_labware_to_module(
labware_core=mock_labware_core,
new_location=mock_module_core,
use_gripper=False,
use_pick_up_location_lpc_offset=False,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=None,
)
Expand Down Expand Up @@ -586,8 +581,6 @@ def test_move_labware_off_deck(
labware_core=mock_labware_core,
new_location=OFF_DECK,
use_gripper=False,
use_pick_up_location_lpc_offset=False,
use_drop_location_lpc_offset=False,
pick_up_offset=None,
drop_offset=None,
)
Expand Down
31 changes: 31 additions & 0 deletions api/tests/opentrons/protocol_engine/clients/test_sync_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
DropTipWellLocation,
MotorAxis,
Liquid,
LabwareMovementStrategy,
LabwareOffsetVector,
)


Expand Down Expand Up @@ -235,6 +237,35 @@ def test_load_pipette(
assert result == expected_result


def test_move_labware(
decoy: Decoy,
transport: ChildThreadTransport,
subject: SyncClient,
) -> None:
"""It should execute a move labware command."""
request = commands.MoveLabwareCreate(
params=commands.MoveLabwareParams(
labwareId="movable-labware-id",
newLocation=DeckSlotLocation(slotName=DeckSlotName.SLOT_4),
strategy=LabwareMovementStrategy.USING_GRIPPER,
pickUpOffset=LabwareOffsetVector(x=1, y=2, z=3),
dropOffset=LabwareOffsetVector(x=10, y=20, z=30),
)
)

response = commands.MoveLabwareResult(offsetId="offset-id")
decoy.when(transport.execute_command(request=request)).then_return(response)

result = subject.move_labware(
labware_id="movable-labware-id",
new_location=DeckSlotLocation(slotName=DeckSlotName.SLOT_4),
strategy=LabwareMovementStrategy.USING_GRIPPER,
pick_up_offset=LabwareOffsetVector(x=1, y=2, z=3),
drop_offset=LabwareOffsetVector(x=10, y=20, z=30),
)
assert result == response


def test_move_to_well(
decoy: Decoy,
transport: ChildThreadTransport,
Expand Down
Loading

0 comments on commit e1da55c

Please sign in to comment.