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

perf(api): P1000 pick-up-tip shake and drop-tip plunger #2446

Merged
merged 14 commits into from
Oct 11, 2018
Merged

perf(api): P1000 pick-up-tip shake and drop-tip plunger #2446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
00929fc
perf(api): Increase distance travelled down when picking up tip to 15mm
andySigler Jul 13, 2018
664e6da
Merge branch 'edge' into api-increase-pick-up-plunger-for-p1000
andySigler Jul 16, 2018
4a96ecf
Merge branch 'edge' into api-increase-pick-up-plunger-for-p1000
andySigler Jul 16, 2018
06c5c92
moves pick-up-distance to pipette configs
andySigler Jul 20, 2018
1980a9a
Merge branch 'api-increase-pick-up-plunger-for-p1000' of https://gith…
timo8396 Jul 23, 2018
10cc7e9
changes to shake off speed= 50, distance = 2.25 and double shake off …
timo8396 Sep 10, 2018
44efe1c
fixes merge conflicts
Sep 10, 2018
d906753
include pick-up-distance in pipette-config constructor
andySigler Sep 10, 2018
5259e0c
increases drop tip current for p1000s and adjust plunger position dro…
Sep 10, 2018
e66148c
fixed merge conflicts
andySigler Oct 8, 2018
3308e45
only shake for p1000 pick-up-tips
andySigler Oct 9, 2018
4893eea
legacy_api/api inits pipette with pick_up_distance
andySigler Oct 9, 2018
af8b820
removes call to set_pick_up_current in _create_pipette_from_config an…
andySigler Oct 11, 2018
e6931b6
adds 'quirks' field to pipette-config to enable the extra shake for p…
andySigler Oct 11, 2018
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: 3 additions & 1 deletion api/opentrons/legacy_api/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,11 @@ def _create_pipette_from_config(
plunger_current=config.plunger_current,
drop_tip_current=config.drop_tip_current,
plunger_positions=config.plunger_positions.copy(),
pick_up_current=config.pick_up_current,
pick_up_distance=config.pick_up_distance,
quirks=config.quirks,
fallback_tip_length=config.tip_length) # TODO move to labware

p.set_pick_up_current(config.pick_up_current)
return p

def _retrieve_version_number(self, mount, expected_model_substring):
Expand Down
30 changes: 19 additions & 11 deletions api/opentrons/legacy_api/instruments/pipette.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@
DEFAULT_ASPIRATE_SPEED = 5
DEFAULT_DISPENSE_SPEED = 10

DEFAULT_TIP_PRESS_MM = -10

DEFAULT_TIP_PRESS_MM = 10
DEFAULT_PLUNGE_CURRENT = 0.1

SHAKE_OFF_TIPS_SPEED = 25
SHAKE_OFF_TIPS_DISTANCE = 2
SHAKE_OFF_TIPS_SPEED = 50
SHAKE_OFF_TIPS_DISTANCE = 2.25


def _sleep(seconds):
Expand Down Expand Up @@ -124,6 +123,9 @@ def __init__(
plunger_current=0.5,
drop_tip_current=0.5,
plunger_positions=PLUNGER_POSITIONS,
pick_up_current=DEFAULT_PLUNGE_CURRENT,
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, the pick up distance (and current) is added here, and added to the configs, but not actually passed from the configs to Pipette.__init__(). We need to add it to the call in InstrumentsWrapper._create_pipette_from_config().

Copy link
Member

Choose a reason for hiding this comment

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

Also if we're going to add pick_up_current as an argument here, we should pass it as an argument in _create_pipette_from_config rather than calling set_pick_up_current() in _create_pipette_from_config()` after the pipette object is initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

pick_up_distance=DEFAULT_TIP_PRESS_MM,
quirks=[],
fallback_tip_length=51.7): # TODO (andy): move to tip-rack

self.robot = robot
Expand Down Expand Up @@ -177,8 +179,8 @@ def __init__(
self.max_volume = max_volume
self.min_volume = min_volume

self._pick_up_current = None
self.set_pick_up_current(DEFAULT_PLUNGE_CURRENT)
self._pick_up_distance = pick_up_distance
self._pick_up_current = pick_up_current

# TODO (andy) these values maybe should persist between sessions,
# by saving within `robot_config`
Expand All @@ -195,6 +197,8 @@ def __init__(
self.robot.config.tip_length[self.name] = \
self.robot.config.tip_length.get(self.name, fallback_tip_length)

self.quirks = quirks if isinstance(quirks, list) else []

def reset(self):
"""
Resets the state of this pipette, removing associated placeables,
Expand Down Expand Up @@ -910,7 +914,7 @@ def pick_up_tip(self, location=None, presses=3, increment=1):

@commands.publish.both(command=commands.pick_up_tip)
def _pick_up_tip(
self, location, presses, plunge_depth, increment):
self, location, presses, increment):
self.instrument_actuator.set_active_current(self._plunger_current)
self.robot.poses = self.instrument_actuator.move(
self.robot.poses,
Expand All @@ -926,7 +930,7 @@ def _pick_up_tip(
self.instrument_mover.push_active_current()
self.instrument_mover.set_active_current(self._pick_up_current)
self.instrument_mover.set_speed(30)
dist = plunge_depth + (-1 * increment * i)
dist = (-1 * self._pick_up_distance) + (-1 * increment * i)
self.move_to(
self.current_tip().top(dist),
strategy='direct')
Expand All @@ -939,17 +943,22 @@ def _pick_up_tip(
self._add_tip(
length=self._tip_length
)
# 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
if 'needs-pickup-shake' in self.quirks:
self._shake_off_tips(location)
self._shake_off_tips(location)
self.previous_placeable = None # no longer inside a placeable
self.robot.poses = self.instrument_mover.fast_home(
self.robot.poses, abs(plunge_depth))
self.robot.poses, self._pick_up_distance)

return self

return _pick_up_tip(
self,
location=location,
presses=presses,
plunge_depth=DEFAULT_TIP_PRESS_MM,
increment=increment)

def drop_tip(self, location=None, home_after=True):
Expand Down Expand Up @@ -1027,7 +1036,6 @@ def _drop_tip(location, instrument=self):
x=pos_drop_tip
)
self._shake_off_tips(location)

if home_after:
self._home_after_drop_tip()

Expand Down
5 changes: 4 additions & 1 deletion api/opentrons/legacy_api/instruments/pipette_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
[
'plunger_positions',
'pick_up_current',
'pick_up_distance',
'aspirate_flow_rate',
'dispense_flow_rate',
'channels',
Expand All @@ -23,11 +24,11 @@
'drop_tip_current',
'min_volume',
'max_volume',
'quirks',
'tip_length' # TODO (andy): remove from pipette, move to tip-rack
]
)


# Notes:
# - multi-channel pipettes share the same dimensional offsets
# - single-channel pipettes have different lengths
Expand Down Expand Up @@ -78,6 +79,7 @@ def load(pipette_model: str) -> pipette_config:
'drop_tip': plunger_pos.get('dropTip'),
},
pick_up_current=cfg.get('pickUpCurrent'),
pick_up_distance=cfg.get('pickUpDistance'),
aspirate_flow_rate=cfg.get('aspirateFlowRate'),
dispense_flow_rate=cfg.get('dispenseFlowRate'),
channels=cfg.get('channels'),
Expand All @@ -87,6 +89,7 @@ def load(pipette_model: str) -> pipette_config:
drop_tip_current=cfg.get('dropTipCurrent'),
min_volume=cfg.get('minVolume'),
max_volume=cfg.get('maxVolume'),
quirks=cfg.get('quirks'),
tip_length=cfg.get('tipLength')
)

Expand Down
26 changes: 15 additions & 11 deletions api/tests/opentrons/labware/test_pipette_unittest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,8 +1004,8 @@ def test_tip_tracking_simple(self):
self.p200.pick_up_tip()

assert self.p200.move_to.mock_calls == \
self.build_pick_up_tip(self.tiprack1[0]) + \
self.build_pick_up_tip(self.tiprack1[1])
self.build_pick_up_tip(self.p200, self.tiprack1[0]) + \
Copy link
Member

Choose a reason for hiding this comment

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

Comment: Why did these have to change? Were they failing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, the movements generated are different depending on the pipette now, because they have different distances that they move down to make the seal (10 vs 15 mm)

self.build_pick_up_tip(self.p200, self.tiprack1[1])

def test_simulate_plunger_while_enqueing(self):

Expand Down Expand Up @@ -1060,9 +1060,9 @@ def test_tip_tracking_chain(self):

expected = []
for i in range(0, total_tips_per_plate):
expected.extend(self.build_pick_up_tip(self.tiprack1[i]))
expected.extend(self.build_pick_up_tip(self.p200, self.tiprack1[i]))
for i in range(0, total_tips_per_plate):
expected.extend(self.build_pick_up_tip(self.tiprack2[i]))
expected.extend(self.build_pick_up_tip(self.p200, self.tiprack2[i]))

self.assertEqual(
self.p200.move_to.mock_calls,
Expand Down Expand Up @@ -1103,9 +1103,11 @@ def test_tip_tracking_chain_multi_channel(self):

expected = []
for i in range(0, 12):
expected.extend(self.build_pick_up_tip(self.tiprack1.cols[i]))
expected.extend(
self.build_pick_up_tip(p200_multi, self.tiprack1.cols[i]))
for i in range(0, 12):
expected.extend(self.build_pick_up_tip(self.tiprack2.cols[i]))
expected.extend(
self.build_pick_up_tip(p200_multi, self.tiprack2.cols[i]))

self.assertEqual(
p200_multi.move_to.mock_calls,
Expand Down Expand Up @@ -1163,14 +1165,16 @@ def test_direct_movement_within_well(self):
]
self.assertEqual(self.robot.move_to.mock_calls, expected)

def build_pick_up_tip(self, well):
plunge = -10
def build_pick_up_tip(self, pipette, well):
return [
mock.call(well.top()),
mock.call(well.top(plunge), strategy='direct'),
mock.call(
well.top(-pipette._pick_up_distance), strategy='direct'),
mock.call(well.top(), strategy='direct'),
mock.call(well.top(plunge - 1), strategy='direct'),
mock.call(
well.top(-pipette._pick_up_distance - 1), strategy='direct'),
mock.call(well.top(), strategy='direct'),
mock.call(well.top(plunge - 2), strategy='direct'),
mock.call(
well.top(-pipette._pick_up_distance - 2), strategy='direct'),
mock.call(well.top(), strategy='direct')
]
Loading