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): use containers and instruments to set RPC pipette tip rack list #5147

Merged
merged 1 commit into from
Mar 3, 2020

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Mar 2, 2020

overview

This works PR works around a "bug" from the RPC API in PAPIv1 (Existing expected bahavior? Who really knows?) that was discovered by an internal user.

A pipette may be reported to the app to use no tip racks even if a tip rack object used in the protocol reports usage by that same pipette. E.g. the session can come back with:

  • RPC Session
    • "Instruments" list
      • Some pipette
        • Tip racks list: []
    • "Containers" list
      • Some tip rack
        • Pipettes list: [ "Some pipette" ]

This appears to happen if one constructs the pipettes without using the tip_racks argument of the constructor in PAPIv1. To work around this issue, the app will now build the tip rack usage list from both the RPC instruments and the RPC containers, instead of just the instruments.

changelog

  • fix(app): use containers and instruments to set RPC pipette tip rack list

review requests

To test, upload a Protocol API v1 protocol that does not use the tip_racks argument of the pipette constructors, but makes various pick_up_tip and return_tip calls

  • Pipette is available for tip probe
    • i.e. it is not greyed out saying "This pipette is not used in this protocol"

The added functionality is covered by unit tests, but is a very old, un-typed part of the app's source code.

…struments

This works around a bug from the RPC API in PAPIv1 where a pipette may be reported to use no tip
racks even if a tip rack itself reports usage by that same pipette
@mcous mcous added app Affects the `app` project ready for review fix PR fixes a bug labels Mar 2, 2020
@mcous mcous requested a review from a team as a code owner March 2, 2020 20:56
@mcous mcous requested review from a team, amitlissack and sanni-t and removed request for a team March 2, 2020 20:57
@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #5147 into edge will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #5147      +/-   ##
==========================================
+ Coverage   68.91%   68.92%   +0.01%     
==========================================
  Files        1082     1084       +2     
  Lines       36045    36086      +41     
==========================================
+ Hits        24839    24873      +34     
- Misses      11206    11213       +7
Impacted Files Coverage Δ
app/src/robot/api-client/client.js 90.67% <100%> (+0.28%) ⬆️
protocol-designer/src/components/ProtocolEditor.js 0% <0%> (ø) ⬆️
...mponents/modals/AnnouncementModal/announcements.js 100% <0%> (ø)
...r/src/components/modals/AnnouncementModal/index.js 100% <0%> (ø)
protocol-designer/src/persist.js 33.96% <0%> (+33.96%) ⬆️

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 3f7aecc...3bf6590. Read the comment docs.

Copy link
Contributor

@amitlissack amitlissack left a comment

Choose a reason for hiding this comment

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

lgtm

@mcous mcous added the bug label Mar 3, 2020
@mcous mcous requested review from a team, Kadee80 and shlokamin and removed request for a team March 3, 2020 20:25
Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

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

🎙 💧

@mcous mcous merged commit 2c5fc9f into edge Mar 3, 2020
@mcous mcous deleted the app_reconcile-rpc-tiprac-usage branch March 3, 2020 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project bug fix PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants