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.

Sign up for GitHub

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): add new pick up tip function #15275

Merged
merged 4 commits into from
May 29, 2024
Merged

feat(api): add new pick up tip function #15275

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0022c2b
add execute pick_up_tip function
caila-marashaj May 28, 2024
b45c600
add ot2 compatibility
caila-marashaj May 28, 2024
9a05f8b
update tip state for simulator
caila-marashaj May 29, 2024
159eacf
remove simulator fix from old pick up tip function
caila-marashaj May 29, 2024
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
77 changes: 44 additions & 33 deletions api/src/opentrons/hardware_control/ot3api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,49 @@ async def hold_jaw_width(self, jaw_width_mm: int) -> None:
self._gripper_handler.check_ready_for_jaw_move("hold_jaw_width")
await self._hold_jaw_width(jaw_width_mm)

async def execute_pick_up_tip(
self,
mount: OT3Mount,
presses: Optional[int] = None,
increment: Optional[float] = None,
) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Does this also need to exist on the OT-2 API class so the two classes can be substituted for each other?
  2. Could we have docstrings defining what this does and how it's different from pick_up_tip()?
  3. I feel like there could be a more specific name for this, but I don't know what. pick_up_tip_bare() or just_pick_up_tip() or pick_up_tip_motions() or something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah thats a good point, I'll make another commit with some changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pick_up_tip_in_place? I think at some poitn we're going to want to get it out of this file anyway

Copy link
Contributor

@SyntaxColoring SyntaxColoring May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't use _in_place because in Protocol Engine, _in_place is used to roughly mean "do the operation without moving X/Y," whereas here, neither function moves in X/Y, and the difference instead has to do with tip-state and volume-state setting.

realmount = OT3Mount.from_mount(mount)
instrument = self._pipette_handler.get_pipette(realmount)
if (
self.gantry_load == GantryLoad.HIGH_THROUGHPUT
and instrument.nozzle_manager.current_configuration.configuration
== NozzleConfigurationType.FULL
):
spec = self._pipette_handler.plan_ht_pick_up_tip(
instrument.nozzle_manager.current_configuration.tip_count
)
if spec.z_distance_to_tiprack:
await self.move_rel(
realmount, top_types.Point(z=spec.z_distance_to_tiprack)
)
await self._tip_motor_action(realmount, spec.tip_action_moves)
else:
spec = self._pipette_handler.plan_lt_pick_up_tip(
realmount,
instrument.nozzle_manager.current_configuration.tip_count,
presses,
increment,
)
await self._force_pick_up_tip(realmount, spec)

# neighboring tips tend to get stuck in the space between
# the volume chamber and the drop-tip sleeve on p1000.
# This extra shake ensures those tips are removed
for rel_point, speed in spec.shake_off_moves:
await self.move_rel(realmount, rel_point, speed=speed)

# fixme: really only need this during labware position check so user
# can verify if a tip is properly attached
if spec.ending_z_retract_distance:
await self.move_rel(
realmount, top_types.Point(z=spec.ending_z_retract_distance)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to do the simulator state update thing that's on line 2159 too, this:

if isinstance(self._backend, OT3Simulator):
     self._backend._update_tip_state(realmount, True)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want this? My impression was that we want to leave any kind of state updates to the caller

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This specifically is a hack to get the simulated tip presence sensors returning the right thing - that's why it's checking the backend. You're right in general, but this is an exception


async def _move_to_plunger_bottom(
self,
mount: OT3Mount,
Expand Down Expand Up @@ -2160,40 +2203,8 @@ def add_tip_to_instr() -> None:
self._backend._update_tip_state(realmount, True)

await self._move_to_plunger_bottom(realmount, rate=1.0)
if (
self.gantry_load == GantryLoad.HIGH_THROUGHPUT
and instrument.nozzle_manager.current_configuration.configuration
== NozzleConfigurationType.FULL
):
spec = self._pipette_handler.plan_ht_pick_up_tip(
instrument.nozzle_manager.current_configuration.tip_count
)
if spec.z_distance_to_tiprack:
await self.move_rel(
realmount, top_types.Point(z=spec.z_distance_to_tiprack)
)
await self._tip_motor_action(realmount, spec.tip_action_moves)
else:
spec = self._pipette_handler.plan_lt_pick_up_tip(
realmount,
instrument.nozzle_manager.current_configuration.tip_count,
presses,
increment,
)
await self._force_pick_up_tip(realmount, spec)

# neighboring tips tend to get stuck in the space between
# the volume chamber and the drop-tip sleeve on p1000.
# This extra shake ensures those tips are removed
for rel_point, speed in spec.shake_off_moves:
await self.move_rel(realmount, rel_point, speed=speed)

# fixme: really only need this during labware position check so user
# can verify if a tip is properly attached
if spec.ending_z_retract_distance:
await self.move_rel(
realmount, top_types.Point(z=spec.ending_z_retract_distance)
)
await self.execute_pick_up_tip(realmount, presses, increment)

add_tip_to_instr()

Expand Down
13 changes: 13 additions & 0 deletions api/tests/opentrons/hardware_control/test_instruments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,19 @@ async def test_no_pipette(sim_and_instr):
assert not hw_api._current_volume[types.Mount.RIGHT]


async def test_execute_pick_up_tip(dummy_instruments_ot3, ot3_api_obj):
"""Make sure that execute_pick_up_tip does not add a tip to the instrument."""
hw_api = await ot3_api_obj(
attached_instruments=dummy_instruments_ot3[0], loop=asyncio.get_running_loop()
)
mount = types.Mount.LEFT
await hw_api.home()
await hw_api.cache_instruments()

await hw_api.execute_pick_up_tip(mount)
assert not hw_api.hardware_instruments[mount].has_tip


async def test_pick_up_tip(is_robot, sim_and_instr):
sim_builder, (dummy_instruments, _) = sim_and_instr
hw_api = await sim_builder(
Expand Down
19 changes: 19 additions & 0 deletions api/tests/opentrons/hardware_control/test_ot3_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ async def test_pickup_moves(
pipette_handler.get_pipette(
OT3Mount.LEFT
).nozzle_manager.current_configuration.configuration = NozzleConfigurationType.FULL
pipette_handler.get_pipette(OT3Mount.LEFT).current_volume = 0
z_tiprack_distance = 8.0
end_z_retract_dist = 9.0
move_plan_return_val = TipActionSpec(
Expand Down Expand Up @@ -644,6 +645,24 @@ async def test_pickup_moves(
]
else:
assert move_call_list == [(OT3Mount.LEFT, Point(z=end_z_retract_dist))]
# pick up tip should have two calls to move_to_plunger_bottom, one before and one after
# the tip pickup
assert len(mock_move_to_plunger_bottom.call_args_list) == 2
mock_move_to_plunger_bottom.reset_mock()
mock_move_rel.reset_mock()

# make sure that execute_pick_up_tip has the same set of moves,
# except no calls to move_to_plunger_bottom
await ot3_hardware.execute_pick_up_tip(Mount.LEFT, 40.0)
move_call_list = [call.args for call in mock_move_rel.call_args_list]
if gantry_load == GantryLoad.HIGH_THROUGHPUT:
assert move_call_list == [
(OT3Mount.LEFT, Point(z=z_tiprack_distance)),
(OT3Mount.LEFT, Point(z=end_z_retract_dist)),
]
else:
assert move_call_list == [(OT3Mount.LEFT, Point(z=end_z_retract_dist))]
assert len(mock_move_to_plunger_bottom.call_args_list) == 0


@pytest.mark.parametrize("load_configs", load_pipette_configs)
Expand Down
Loading