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(app): lpc: fix command ordering in pick up tip #12590

Merged
merged 2 commits into from
Apr 28, 2023

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Apr 28, 2023

In LPC, when we picked up a tip that we'd actually use for the rest of LPC, we'd remove the tiprack labware before moving to the trash. This is incorrect - that labware is still physically present, since we haven't told the user to remove it. We used to get away with it, because it used to be that pick_up_tip ended with a z-home that got the pipette up high enough that even if PE thought the deck was empty we wouldn't collide.

Now, though, on the Flex we removed that home because it is slow and we don't need it anymore, and if we tell PE that the tiprack disappeared and then tell it to move the pipette to the trash, it will just zip it straight over to the trash... dragging along that tiprack, which is still physically present.

To fix this, just move to the trash before we remove the labware.

Risk assessment

  • Low: this is a behavior change, but it's definitely more correct

Testing

  • Ran this on a flex that previously would drag the tiprack around every time, and now you get the nice clean move-up-to-clear-labware behavior

In LPC, when we picked up a tip that we'd actually use for the rest of
LPC, we'd remove the tiprack labware before moving to the trash. This is
incorrect - that labware is still physically present, since we haven't
told the user to remove it. We used to get away with it, because it used
to be that pick_up_tip ended with a z-home that got the pipette up high
enough that even if PE thought the deck was empty we wouldn't collide.

Now, though, on the Flex we removed that home because it is slow and we
don't need it anymore, and if we tell PE that the tiprack disappeared
and then tell it to move the pipette to the trash, it will just zip it
straight over to the trash... dragging along that tiprack, which is
still physically present.

To fix this, just move to the trash before we remove the labware.
@sfoster1 sfoster1 requested a review from b-cooper April 28, 2023 16:36
@sfoster1 sfoster1 requested a review from a team as a code owner April 28, 2023 16:36
@sfoster1 sfoster1 requested a review from a team as a code owner April 28, 2023 16:38
Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

🐬

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #12590 (69364bd) into internal-release_0.5.0 (fc09e2f) will decrease coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@                    Coverage Diff                     @@
##           internal-release_0.5.0   #12590      +/-   ##
==========================================================
- Coverage                   73.59%   73.31%   -0.29%     
==========================================================
  Files                        2246     1506     -740     
  Lines                       61830    49349   -12481     
  Branches                     6480     2997    -3483     
==========================================================
- Hits                        45504    36180    -9324     
+ Misses                      14757    12713    -2044     
+ Partials                     1569      456    -1113     
Flag Coverage Δ
app 46.85% <ø> (-25.35%) ⬇️
notify-server 89.13% <ø> (ø)

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

see 785 files with indirect coverage changes

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Nice catch.

@sfoster1
Copy link
Member Author

the api test failure is #12591

@sfoster1 sfoster1 merged commit 5dc6c26 into internal-release_0.5.0 Apr 28, 2023
@sfoster1 sfoster1 deleted the move-up-after-pickup branch April 28, 2023 16:52
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