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(rpc): ensure load name is attached to RPC "containers" #4530

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Dec 3, 2019

overview

Prior to this PR,

  • The app determined that a given RPC labware is a tiprack by running a regex for /tiprack/i against the type field of the RPC Container
  • Container.type was set to the name of the v2 Labware class
  • Labware.name was always set to the load name of the labware

#4452 introduced a change that set Labware.name to the passed-in label parameter of the labware if it was used. This broke tiprack detection in the client anytime label was set to a string that didn't contain the exact case-insensitive string "tiprack".

The Protocol Designer always passes in the display name of the labware as label by default, which means tiprack detection was automatically broken for all PD protocols.

This PR adds a new getter method to Labware for load_name, which is now used to set Container.type. Additionally, it adds a Container.is_tiprack property, which the RPC client will use in favor of the regex if available. Finally, it changes the regex (now only used as a back-compat backup) to /tip ?rack/i to match on our tiprack display names as well as load names.

changelog

  • fix(rpc): ensure load name is attached to RPC "containers"

review requests

Exhaustive QA sheet. This is a pretty small PR codewise, so I'd be ok giving this PR some smoke tests and then using the list below to invalidate the necessary entries in the 3.15 master QA

  • JSON protocol has its tipracks recognized on 3.15 robot
  • JSON protocol has its tipracks recognized on 3.14 robot
  • PAPIv2 protocol has its tipracks recognized on 3.15 robot
  • PAPIv2 protocol has its tipracks recognized on 3.14 robot
  • PAPIv1 protocol using Labware v2 has its tipracks recognized on 3.15 robot
  • PAPIv1 protocol has using Labware v2 has its tipracks recognized on 3.14 robot
  • PAPIv1 protocol using Labware v1 has its tipracks recognized on 3.15 robot
  • PAPIv1 protocol has using Labware v1 has its tipracks recognized on 3.14 robot

@mcous mcous added bug app Affects the `app` project api Affects the `api` project fix PR fixes a bug rpc This involves Opentrons' deprecated RPC system. labels Dec 3, 2019
@mcous mcous requested review from a team December 3, 2019 21:59
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.

  • JSON protocol has its tipracks recognized on 3.15 robot
  • JSON protocol has its tipracks recognized on 3.14 robot
  • PAPIv2 protocol has its tipracks recognized on 3.15 robot
  • PAPIv2 protocol has its tipracks recognized on 3.14 robot
  • PAPIv1 protocol using Labware v2 has its tipracks recognized on 3.15 robot
  • PAPIv1 protocol has using Labware v2 has its tipracks recognized on 3.14 robot
  • PAPIv1 protocol using Labware v1 has its tipracks recognized on 3.15 robot
  • PAPIv1 protocol has using Labware v1 has its tipracks recognized on 3.14 robot

@codecov
Copy link

codecov bot commented Dec 3, 2019

Codecov Report

Merging #4530 into release_3.15.0 will increase coverage by 0.65%.
The diff coverage is 100%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           release_3.15.0    #4530      +/-   ##
==================================================
+ Coverage           57.05%   57.71%   +0.65%     
==================================================
  Files                 905      905              
  Lines               25701    26201     +500     
==================================================
+ Hits                14664    15122     +458     
- Misses              11037    11079      +42
Impacted Files Coverage Δ
app/src/robot/api-client/client.js 89.47% <100%> (ø) ⬆️
app/src/components/MenuPanel/index.js 0% <0%> (ø) ⬆️
app/src/pages/More/index.js 0% <0%> (ø) ⬆️
app-shell/src/main.js 0% <0%> (ø) ⬆️
app/src/pages/More/Resources.js 0% <0%> (ø) ⬆️
opentrons/protocol_api/labware.py 89.98% <0%> (+0.06%) ⬆️
opentrons/legacy_api/instruments/pipette.py 95.27% <0%> (+0.16%) ⬆️
opentrons/api/models.py 85.45% <0%> (+0.26%) ⬆️
app/src/config/index.js 25.71% <0%> (+3.97%) ⬆️
opentrons/deck_calibration/endpoints.py 79.31% <0%> (+6.31%) ⬆️

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 e1724ab...f36f095. Read the comment docs.

@mcous mcous merged commit 4580aa4 into release_3.15.0 Dec 3, 2019
@mcous mcous deleted the rpc_add-is-tiprack branch February 13, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the `api` project app Affects the `app` project bug fix PR fixes a bug rpc This involves Opentrons' deprecated RPC system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants