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): api location cache fix #7609

Merged
merged 2 commits into from
Apr 14, 2021
Merged

fix(api): api location cache fix #7609

merged 2 commits into from
Apr 14, 2021

Conversation

amitlissack
Copy link
Contributor

@amitlissack amitlissack commented Apr 8, 2021

Overview

The location cache is used regardless of which pipette is moving. If left moves immediately after right, the left move's cached location will be used. This can cause jerky motion.

closes #7156

Changelog

  • update protocol api version to 2.10
  • maintain the last mount moved.

Review requests

Test on a robot using example protocols in #7156

Risk assessment

High.

@amitlissack amitlissack added api Affects the `api` project fix PR fixes a bug robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). labels Apr 8, 2021
@amitlissack amitlissack requested review from a team, ahiuchingau and mcous and removed request for a team April 8, 2021 14:39
@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

❗ No coverage uploaded for pull request base (edge@c526b19). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #7609   +/-   ##
=======================================
  Coverage        ?   82.30%           
=======================================
  Files           ?      328           
  Lines           ?    22069           
  Branches        ?        0           
=======================================
  Hits            ?    18164           
  Misses          ?     3905           
  Partials        ?        0           

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 c526b19...06d8835. Read the comment docs.

@amitlissack amitlissack marked this pull request as ready for review April 8, 2021 14:56
@amitlissack amitlissack requested a review from a team as a code owner April 8, 2021 14:56
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.

Looks like a great fix! @nusrat813 would you mind testing?

Copy link
Contributor

@mcous mcous left a comment

Choose a reason for hiding this comment

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

Paired on this so take my review with a grain of salt, but 👍

Copy link

@nusrat813 nusrat813 left a comment

Choose a reason for hiding this comment

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

LGTM, tested on robot

However, still having issue with pipettes
Screen Shot 2021-04-08 at 1 54 00 PM

@mcous
Copy link
Contributor

mcous commented Apr 9, 2021

Unable to reproduce pipettes issue on my machine. I don't really think missing pipettes makes sense for this branch; this code doesn't get anywhere near GET /pipettes

@mcous mcous added the qa QA input / review required label Apr 13, 2021
@amitlissack amitlissack force-pushed the api_location-cache-fix branch from fe24ee1 to 06d8835 Compare April 14, 2021 17:32
@amitlissack amitlissack merged commit df68ea2 into edge Apr 14, 2021
@amitlissack amitlissack deleted the api_location-cache-fix branch April 14, 2021 17:53
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 qa QA input / review required robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(api): Two pipettes in use, dispensing in same well; second pipette dispensing moves diagonally downwards.
4 participants