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

fix(api): bump y max bounds to match robot geometry #7140

Merged
merged 2 commits into from
Jan 4, 2021

Conversation

sfoster1
Copy link
Member

We now helpfully check that jogs don't overlap the maximum motion range
of each axis, at least in the positive direction, using the home
positions. This works pretty well for Z and X since it straightforwardly
prevents hard limit errors from unintentional endstop triggers, which
happen at the home position as one might expect.

Unfortunately, the robot geometry makes this approach completely invalid
for Y. When homing, the Y endstop (which is located on the back of the
head) actually interacts with a post projecting from the back of the
robot, which is located on the right side of the robot. This is the
reason we must always home X before homing Y: to make sure the head is
in the correct position for the Y endstop to interact with the post.

If the head is not all the way to the right, it can actually go much
farther back - past the home location, past where the switch would
interact with the post. And the deck layout is designed to take
advantage of this; the position that A1 of most labware lives in is
farther back than the post, and thus customers would get spurious jog
failures when calibrating labwares in slots 10 and 11 using single
channel pipettes (because multi channels are wider in Y, the gantry
doesn't go far enough back to trigger the issue).

The fix is to create a different bound in the Y.

Closes #6886

Risk

Low, only happens during jog during labware calibration and widens the bound

Testing

Put a labware in slot 10 or 11 and try to calibrate it using a single channel. You should be able to jog freely.

@sfoster1 sfoster1 added api Affects the `api` project fix PR fixes a bug labels Dec 17, 2020
@sfoster1 sfoster1 requested a review from a team December 17, 2020 16:02
@sfoster1 sfoster1 requested a review from a team as a code owner December 17, 2020 16:02
@codecov
Copy link

codecov bot commented Dec 17, 2020

Codecov Report

Merging #7140 (b4721ea) into edge (61d9b4f) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #7140      +/-   ##
==========================================
+ Coverage   78.96%   78.97%   +0.01%     
==========================================
  Files         265      265              
  Lines       19015    19027      +12     
==========================================
+ Hits        15015    15027      +12     
  Misses       4000     4000              
Impacted Files Coverage Δ
opentrons/hardware_control/simulator.py 84.78% <0.00%> (-0.22%) ⬇️
opentrons/drivers/smoothie_drivers/driver_3_0.py 67.22% <0.00%> (+0.21%) ⬆️
opentrons/drivers/smoothie_drivers/__init__.py 70.37% <0.00%> (+5.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61d9b4f...b4721ea. Read the comment docs.

Comment on lines 37 to 41

_HOME_POSITION = {'X': 418.0, 'Y': 353.0, 'Z': 218.0,
_HOME_POSITION: Final = {'X': 418.0, 'Y': 353.0, 'Z': 218.0,
'A': 218.0, 'B': 19.0, 'C': 19.0}

_BOUNDS: Final = {'X': 418.0, 'Y': 370.0, 'Z': 218.0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't have to happen in this PR, but can we deduplicate these numbers with the ones in driver_3_0.py?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

}
Y_BOUND_OVERRIDE: Final = 370
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we leave a comment briefly explaining what this is and why we need it (as you've done in the PR description)?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

from typing_extensions import Final


HOMED_POSITION: Final = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI, we might want to check these numbers. Most of these match nominal home of my OT-2, but not all of them...

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a bunch of weirdness going on with these, and in the actual running smoothie driver they get updated from the results of doing the actual home(). I hate all of this.

We now helpfully check that jogs don't overlap the maximum motion range
of each axis, at least in the positive direction, using the home
positions. This works pretty well for Z and X since it straightforwardly
prevents hard limit errors from unintentional endstop triggers, which
happen at the home position as one might expect.

Unfortunately, the robot geometry makes this approach completely invalid
for Y. When homing, the Y endstop (which is located on the back of the
head) actually interacts with a post projecting from the back of the
robot, which is located on the right side of the robot. This is the
reason we must always home X before homing Y: to make sure the head is
in the correct position for the Y endstop to interact with the post.

If the head is _not_ all the way to the right, it can actually go much
farther back - past the home location, past where the switch would
interact with the post. And the deck layout is designed to take
advantage of this; the position that A1 of most labware lives in is
farther back than the post, and thus customers would get spurious jog
failures when calibrating labwares in slots 10 and 11 using single
channel pipettes (because multi channels are wider in Y, the gantry
doesn't go far enough back to trigger the issue).

The fix is to create a different bound in the Y.

Closes #6886
@sfoster1 sfoster1 force-pushed the api_fix-y-max-bounds branch from 19b2b1b to b4721ea Compare January 4, 2021 14:31
Copy link
Contributor

@Laura-Danielle Laura-Danielle left a comment

Choose a reason for hiding this comment

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

Tested on my robot and looks good to me. I agree with the comments made by Mike, and that we should revisit this later.

@sfoster1 sfoster1 merged commit 85b3d6b into edge Jan 4, 2021
@sfoster1 sfoster1 deleted the api_fix-y-max-bounds branch January 4, 2021 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the `api` project fix PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Single-Channel pipettes can't calibrate labware at the back of the deck
4 participants