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): add a buffer to tip action home moves to make sure the limit switch is triggered #12743

Merged
merged 1 commit into from
May 22, 2023

Conversation

Laura-Danielle
Copy link
Contributor

Overview

During pick up and drop tip movements on the 96 channel, we move the motor exactly the same distance as we do back to the home position. Strangely, these are also the only moves that seem to not be able to reset their position on a commanded home message.

There was another issue (resolved by Opentrons/ot3-firmware#669) which I think was probably a red herring for the 'resetting from home' position issue.

Test Plan

  • Run a protocol that executes pickup/drop tip a bunch of times and visually check to make sure it's going to the right distance every time. Also check in the can monitor that on homed moves of the tip motor the position gets reset every time.

Changelog

  • Add a home buffer to the pick up and drop tip moves rather than using the exact same distance to move the motors back to the home position.

Review requests

Does this solution seem OK for now? Also I know we need to do something with the floating hardcoded values in the pipette_handler, but decided to not address that here.

Risk assessment

Low. Just adding a buffer for homing moves on the tip motors for the 96 channel.

…t switch is triggered

During pick up and drop tip movements on the 96 channel, we move the motor exactly the same distance
as we do back to the home position. Strangely, these are also the only moves that seem to not be
able to reset their position on a commanded home message.
@Laura-Danielle Laura-Danielle requested a review from a team as a code owner May 19, 2023 18:48
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Merging #12743 (a784e4c) into edge (f7a29a3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #12743   +/-   ##
=======================================
  Coverage   73.11%   73.11%           
=======================================
  Files        1510     1510           
  Lines       49521    49521           
  Branches     3022     3022           
=======================================
  Hits        36209    36209           
  Misses      12848    12848           
  Partials      464      464           
Flag Coverage Δ
g-code-testing 96.44% <ø> (ø)
notify-server 89.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/src/opentrons/hardware_control/ot3api.py 79.66% <ø> (ø)

Copy link
Contributor

@fsinapi fsinapi left a comment

Choose a reason for hiding this comment

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

This makes sense to me as long as it fixes the issue - I'm assuming the unit is mm?

I would even think it could make sense to just set the home distance to an enormous distance like we do on the other axes.

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Hm. Okay, let's do it.

@Laura-Danielle Laura-Danielle merged commit 8e800ef into edge May 22, 2023
@Laura-Danielle Laura-Danielle deleted the add_buffer_to_home_moves branch May 22, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants