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): disable module commands when protocol paused #5209

Merged
merged 5 commits into from
Mar 18, 2020

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Mar 11, 2020

The module command cards displayed in the protocol run page have
historically allowed users to send module commands (like setting
temperature) when the protocol was active but paused. That used to work
essentially as a side effect: modules didn't respect pausing, so you
could tell them to do things while the protocol was paused.

Now that modules respect pausing, sending module commands when the
protocol is paused results in the modules not doing anything.

The science and UX intent of controlling modules from the protocol run
page is to implement pre-protocol preparation (like preheat) or
post-protocol behavior (like keeping some incubation going), not to
control the modules when paused - and in fact, when more advanced module
control behaviors land in protocol designer or in the python api,
controlling modules when the protocol is paused could break protocol
execution.

This PR changes the module command cards to only work when the protocol
is not paused or running.

Testing

Note: Testing this pr requires #5194 to be installed on a robot (or a robot on 3.16.1 or previous)

  • The module cards in the protocol run page display properly if the proper modules are configured and present
  • Before the protocol starts or after it finishes, the control interface is displayed and functions
  • While the protocol is running and while it is paused, the control interface is not displayed

Risk Assessment

Limited to out-of-protocol module interaction (and only in the app, at that). However, we should make sure to test both the protocol run page module cards and the pipette info page run cards.

The module command cards displayed in the protocol run page have
historically allowed users to send module commands (like setting
temperature) when the protocol was active but paused. That used to work
essentially as a side effect: modules didn't respect pausing, so you
could tell them to do things while the protocol was paused.

Now that modules respect pausing, sending module commands when the
protocol is paused results in the modules not doing anything.

The science and UX intent of controlling modules from the protocol run
page is to implement pre-protocol preparation (like preheat) or
post-protocol behavior (like keeping some incubation going), not to
control the modules when paused - and in fact, when more advanced module
control behaviors land in protocol designer or in the python api,
controlling modules when the protocol is paused could break protocol
execution.

This PR changes the module command cards to only work when the protocol
is not paused or running.
@sfoster1 sfoster1 added app Affects the `app` project fix PR fixes a bug hmg hardware, motion, and geometry labels Mar 11, 2020
@sfoster1 sfoster1 requested review from a team March 11, 2020 16:43
@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

Merging #5209 into edge will increase coverage by 0.37%.
The diff coverage is 69.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #5209      +/-   ##
==========================================
+ Coverage   60.85%   61.23%   +0.37%     
==========================================
  Files        1027     1033       +6     
  Lines       29316    29817     +501     
==========================================
+ Hits        17841    18259     +418     
- Misses      11475    11558      +83     
Impacted Files Coverage Δ
...mponents/InstrumentSettings/AttachedModulesCard.js 0.00% <0.00%> (ø)
...mponents/InstrumentSettings/ModulesCardContents.js 0.00% <0.00%> (ø)
...rc/components/ModuleControls/TemperatureControl.js 0.00% <ø> (ø)
app/src/components/ModuleControls/index.js 0.00% <0.00%> (ø)
app/src/components/ModuleItem/index.js 0.00% <0.00%> (ø)
...c/components/ModuleLiveStatusCards/TempDeckCard.js 0.00% <ø> (ø)
...mponents/ModuleLiveStatusCards/ThermocyclerCard.js 0.00% <ø> (ø)
app/src/components/ModuleLiveStatusCards/index.js 0.00% <0.00%> (ø)
app/src/robot/selectors.js 80.37% <ø> (+0.50%) ⬆️
app/src/components/ModuleItem/ModuleUpdate.js 92.59% <100.00%> (+0.59%) ⬆️
... and 27 more

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 dbae680...1d749fc. Read the comment docs.

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.

Working as intended

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.

I'm don't really love disappearing the control UI instead of disabling it with an explanatory tooltip

@sfoster1 sfoster1 added this to the HMG Sprint 5 milestone Mar 16, 2020
@nusrat813
Copy link

nusrat813 commented Mar 16, 2020

@mcous

Issues with text spacing on the run screen for module card when the protocol is paused

Note: The issue is only on run tab

Screen Shot 2020-03-16 at 5 18 29 PM

@sfoster1 sfoster1 merged commit b313c95 into edge Mar 18, 2020
@sfoster1 sfoster1 deleted the app_no-mod-commands-in-pause branch March 18, 2020 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project fix PR fixes a bug hmg hardware, motion, and geometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants