Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): remove use of LPC offsets during labware movement #13144

Merged
merged 2 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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