-
Notifications
You must be signed in to change notification settings - Fork 180
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(hardware-testing): add testing teams feature requests #15652
Conversation
A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/ |
eb22ce6
to
2ff2cc6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just one change that I think would clean things up a bit
@@ -188,7 +188,7 @@ async def liquid_probe_in_place( | |||
pipette_id=pipette_id | |||
) | |||
z_pos = await self._hardware_api.liquid_probe( | |||
mount=hw_pipette.mount, max_z_dist=well_depth - lld_min_height | |||
mount=hw_pipette.mount, max_z_dist=well_depth - lld_min_height + 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this method take a well_location and use the well_location.offset.z instead of the magic number 2 here?
@@ -2666,6 +2666,8 @@ async def liquid_probe( | |||
except PipetteLiquidNotFoundError as lnfe: | |||
error = lnfe | |||
pos = await self.gantry_position(checked_mount, refresh=True) | |||
await self.move_to(checked_mount, probe_start_pos + top_types.Point(z=2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can just be a move to probe_start_pos, and then line 2671 can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cant actually physically, this 2 is a different magic 2. Will ticket in our tech debt epic
ff2d244
to
9630d8c
Compare
…rror resulting in moving to well.top resulting in the tip being below the liquid
e2f4549
to
f730fea
Compare
Overview
Test Plan
Changelog
Review requests
Risk assessment