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): wire up mmToEdge to liquid class transfer #17293

Merged
merged 3 commits into from
Jan 17, 2025
Merged

feat(api): wire up mmToEdge to liquid class transfer #17293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d0dcf4
add mm_from_edge as argument to core touch tip
jbleon95 Jan 16, 2025
4963bd5
fix tests
jbleon95 Jan 16, 2025
f38a8f1
raise for incompatible values and tests
jbleon95 Jan 17, 2025
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
6 changes: 6 additions & 0 deletions api/src/opentrons/protocol_api/core/engine/instrument.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ def touch_tip(
radius: float,
z_offset: float,
speed: float,
mm_from_edge: Optional[float] = None,
Comment on lines 391 to +394
Copy link
Member

Choose a reason for hiding this comment

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

Hmm.. I think I prefer making both radius and mm_from_edge optional and asserting that only one of them is used.
If not that, then I think we should at least assert that radius is 1 when mm_from_edge is specified. This will make it easier for unit tests to catch wrong usage of these arguments.

If this was a public API then we would definitely want to have both optional and raising an appropriate error if they are used incorrectly.

) -> None:
"""Touch pipette tip to edges of the well

Expand All @@ -400,7 +401,11 @@ def touch_tip(
radius: Percentage modifier for well radius to touch.
z_offset: Vertical offset for pipette tip during touch tip.
speed: Speed for the touch tip movements.
mm_from_edge: Offset from the edge of the well to move to. Requires a radius of 1.
"""
if mm_from_edge is not None and radius != 1.0:
raise ValueError("radius must be set to 1.0 if mm_from_edge is provided.")

well_name = well_core.get_name()
labware_id = well_core.labware_id

Expand All @@ -422,6 +427,7 @@ def touch_tip(
wellName=well_name,
wellLocation=well_location,
radius=radius,
mmFromEdge=mm_from_edge,
speed=speed,
)
)
Expand Down
6 changes: 3 additions & 3 deletions api/src/opentrons/protocol_api/core/engine/transfer_components_executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,13 @@ def retract_after_aspiration(self, volume: float) -> None:
and touch_tip_props.z_offset is not None
and touch_tip_props.mm_to_edge is not None
)
# TODO: update this once touch tip has mmToEdge
self._instrument.touch_tip(
location=retract_location,
well_core=self._target_well,
radius=1,
z_offset=touch_tip_props.z_offset,
speed=touch_tip_props.speed,
mm_from_edge=touch_tip_props.mm_to_edge,
)
self._instrument.move_to(
location=retract_location,
Expand Down Expand Up @@ -460,8 +460,7 @@ def _do_touch_tip_and_air_gap(
and touch_tip_props.z_offset is not None
and touch_tip_props.mm_to_edge is not None
)
# TODO: update this once touch tip has mmToEdge
# Also, check that when blow out is a non-dest-well,
# TODO:, check that when blow out is a non-dest-well,
# whether the touch tip params from transfer props should be used for
# both dest-well touch tip and non-dest-well touch tip.
if well is not None and location is not None:
Expand All @@ -472,6 +471,7 @@ def _do_touch_tip_and_air_gap(
radius=1,
z_offset=touch_tip_props.z_offset,
speed=touch_tip_props.speed,
mm_from_edge=touch_tip_props.mm_to_edge,
)
except TouchTipDisabledError:
# TODO: log a warning
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/protocol_api/core/instrument.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def touch_tip(
radius: float,
z_offset: float,
speed: float,
mm_from_edge: Optional[float] = None,
) -> None:
...

Expand Down
3 changes: 3 additions & 0 deletions api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,14 @@ def touch_tip(
radius: float,
z_offset: float,
speed: float,
mm_from_edge: Optional[float] = None,
) -> None:
"""
Touch the pipette tip to the sides of a well, with the intent of
removing left-over droplets
"""
if mm_from_edge is not None:
raise APIVersionError(api_element="mm_from_edge argument")
# TODO al 20201110 - build_edges relies on where being a Well. This is
# an unpleasant compromise until refactoring build_edges to support
# LegacyWellCore.
Expand Down
3 changes: 3 additions & 0 deletions api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ def touch_tip(
radius: float,
z_offset: float,
speed: float,
mm_from_edge: Optional[float] = None,
) -> None:
if mm_from_edge is not None:
raise APIVersionError(api_element="mm_from_edge argument")
self.move_to(location)

def pick_up_tip(
Expand Down
2 changes: 1 addition & 1 deletion api/tests/opentrons/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,7 @@ def maximal_liquid_class_def() -> LiquidClassSchemaV1:
touchTip=TouchTipProperties(
enable=True,
params=LiquidClassTouchTipParams(
zOffset=-1, mmToEdge=0.5, speed=30
zOffset=-1, mmToEdge=0.75, speed=30
),
),
delay=DelayProperties(
Expand Down
72 changes: 72 additions & 0 deletions api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1303,12 +1303,84 @@ def test_touch_tip(
),
radius=1.23,
speed=7.89,
mmFromEdge=None,
)
),
mock_protocol_core.set_last_location(location=location, mount=Mount.LEFT),
)


def test_touch_tip_with_mm_from_edge(
decoy: Decoy,
subject: InstrumentCore,
mock_engine_client: EngineClient,
mock_protocol_core: ProtocolCore,
) -> None:
"""It should touch the tip to the edges of the well with mm_from_edge."""
location = Location(point=Point(1, 2, 3), labware=None)

well_core = WellCore(
name="my cool well", labware_id="123abc", engine_client=mock_engine_client
)
subject.touch_tip(
location=location,
well_core=well_core,
radius=1.0,
z_offset=4.56,
speed=7.89,
mm_from_edge=9.87,
)

decoy.verify(
pipette_movement_conflict.check_safe_for_pipette_movement(
engine_state=mock_engine_client.state,
pipette_id="abc123",
labware_id="123abc",
well_name="my cool well",
well_location=WellLocation(
origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=4.56)
),
),
mock_engine_client.execute_command(
cmd.TouchTipParams(
pipetteId="abc123",
labwareId="123abc",
wellName="my cool well",
wellLocation=WellLocation(
origin=WellOrigin.TOP, offset=WellOffset(x=0, y=0, z=4.56)
),
radius=1.0,
speed=7.89,
mmFromEdge=9.87,
)
),
mock_protocol_core.set_last_location(location=location, mount=Mount.LEFT),
)


def test_touch_tip_raises_with_radius_and_mm_from_edge(
decoy: Decoy,
subject: InstrumentCore,
mock_engine_client: EngineClient,
mock_protocol_core: ProtocolCore,
) -> None:
"""It should raise if a value of not 1.0 and a mm_from_edge argument is given."""
location = Location(point=Point(1, 2, 3), labware=None)

well_core = WellCore(
name="my cool well", labware_id="123abc", engine_client=mock_engine_client
)
with pytest.raises(ValueError):
subject.touch_tip(
location=location,
well_core=well_core,
radius=1.23,
z_offset=4.56,
speed=7.89,
mm_from_edge=9.87,
)


def test_has_tip(
decoy: Decoy,
subject: InstrumentCore,
Expand Down
9 changes: 8 additions & 1 deletion api/tests/opentrons/protocol_api/core/engine/test_transfer_components_executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,8 @@ def test_retract_after_aspiration(
mock_instrument_core.touch_tip(
location=Location(Point(x=4, y=4, z=4), labware=None),
well_core=source_well,
radius=1, # Update this to use mmToEdge once implemented
radius=1,
mm_from_edge=0.5,
z_offset=-1,
speed=30,
),
Expand Down Expand Up @@ -599,6 +600,7 @@ def test_retract_after_dispense_with_blowout_in_source(
location=Location(Point(12, 24, 36), labware=None),
well_core=dest_well,
radius=1,
mm_from_edge=0.75,
z_offset=-1,
speed=30,
),
Expand All @@ -623,6 +625,7 @@ def test_retract_after_dispense_with_blowout_in_source(
location=Location(Point(10, 20, 30), labware=None),
well_core=source_well,
radius=1,
mm_from_edge=0.75,
z_offset=-1,
speed=30,
),
Expand Down Expand Up @@ -711,6 +714,7 @@ def test_retract_after_dispense_with_blowout_in_destination(
location=Location(Point(12, 24, 36), labware=None),
well_core=dest_well,
radius=1,
mm_from_edge=0.75,
z_offset=-1,
speed=30,
),
Expand Down Expand Up @@ -788,6 +792,7 @@ def test_retract_after_dispense_with_blowout_in_trash_well(
location=Location(Point(12, 24, 36), labware=None),
well_core=dest_well,
radius=1,
mm_from_edge=0.75,
z_offset=-1,
speed=30,
),
Expand All @@ -812,6 +817,7 @@ def test_retract_after_dispense_with_blowout_in_trash_well(
location=trash_location,
well_core=trash_well_core,
radius=1,
mm_from_edge=0.75,
z_offset=-1,
speed=30,
),
Expand Down Expand Up @@ -886,6 +892,7 @@ def test_retract_after_dispense_with_blowout_in_disposal_location(
location=Location(Point(12, 24, 36), labware=None),
well_core=dest_well,
radius=1,
mm_from_edge=0.75,
z_offset=-1,
speed=30,
),
Expand Down
Loading