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, react-api-client): rely on subsystem status before instrument status #13588

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

smb2268
Copy link
Contributor

@smb2268 smb2268 commented Sep 19, 2023

fix RQA-1585

Overview

We can't rely on the /instruments endpoint response while a subsystem update is ongoing. Instead, we first need to poll for the presence of an update on a given subsystem.

Test Plan

  1. Attach an instrument that needs an update during a firmware update flow. Observe that when after the firmware update takes place, you don't see an "instrument not attached" screen before you see a success screen.
  2. Update the firmware of an instrument via the ODD. On desktop, watch the instrument card for the corresponding instrument. The instrument card should display a subsystem update in progress until the update is complete, and then show the instrument card for an attached instrument.

Changelog

  1. In the firmware update modal, once the update is done refetch the instruments. If the instrument is ok, proceed to the results screen. If it is still not ok, wait another 10 seconds before proceeding.
  2. In the gripper and pipette cards, continuously poll for a subsystem update. If an update is ongoing, rely on that information to show the "bad" instrument card and update banner. If it is not, rely on the /instruments endpoint response for what to show.

Review requests

Run through the test plan

Risk assessment

Low

@smb2268 smb2268 requested a review from sfoster1 September 19, 2023 16:25
@smb2268 smb2268 requested a review from a team as a code owner September 19, 2023 16:25
@smb2268 smb2268 self-assigned this Sep 19, 2023
@smb2268 smb2268 requested review from mjhuff and removed request for a team September 19, 2023 16:25
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #13588 (b53306d) into chore_release-7.0.0 (4ec3bb7) will increase coverage by 0.00%.
Report is 1 commits behind head on chore_release-7.0.0.
The diff coverage is 70.58%.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##           chore_release-7.0.0   #13588   +/-   ##
====================================================
  Coverage                71.32%   71.32%           
====================================================
  Files                     2422     2422           
  Lines                    68141    68145    +4     
  Branches                  7933     7931    -2     
====================================================
+ Hits                     48600    48603    +3     
- Misses                   17685    17687    +2     
+ Partials                  1856     1855    -1     
Flag Coverage Δ
app 68.96% <76.92%> (+<0.01%) ⬆️
react-api-client 69.12% <50.00%> (-0.18%) ⬇️

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

Files Changed Coverage Δ
app/src/organisms/FirmwareUpdateModal/index.tsx 82.75% <50.00%> (+4.18%) ⬆️
...c/subsystems/useCurrentAllSubsystemUpdatesQuery.ts 87.50% <50.00%> (-12.50%) ⬇️
...t/src/subsystems/useCurrentSubsystemUpdateQuery.ts 87.50% <50.00%> (-12.50%) ⬇️
app/src/organisms/Devices/PipetteCard/index.tsx 61.29% <100.00%> (ø)
...sms/FirmwareUpdateModal/FirmwareUpdateTakeover.tsx 100.00% <100.00%> (ø)
app/src/organisms/GripperCard/index.tsx 92.59% <100.00%> (-0.27%) ⬇️

@sfoster1 sfoster1 requested a review from a team as a code owner September 19, 2023 17:57
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.

Tested on a robot with a pipette with an erased application; the app follows along and immediately (well, 5s poll duration) shows that the update is complete even when it was started from the ODD.

@smb2268 smb2268 force-pushed the app_subsystem-fixes branch from 870407b to b53306d Compare September 19, 2023 18:30
@smb2268 smb2268 changed the title fix(app): rely on subsystem status before instrument status fix(app, react-api-client): rely on subsystem status before instrument status Sep 19, 2023
@smb2268 smb2268 merged commit 4e8fd3a into chore_release-7.0.0 Sep 19, 2023
@smb2268 smb2268 deleted the app_subsystem-fixes branch September 19, 2023 19:16
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.

2 participants