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

feat(engine): pause protocol execution if door is opened #11021

Merged
merged 15 commits into from
Jul 11, 2022

Conversation

jbleon95
Copy link
Contributor

@jbleon95 jbleon95 commented Jul 6, 2022

Overview

Closes #9125

Adds pausing protocol execution if a door state hardware notification is detected to protocol engine. Protocol Engine hardware action and handlers renamed to reflect dealing with door events only, along with simplification of the door change action.

Changelog

  • Pauses protocol execution upon DoorStateNotification
  • Refactor and rename PE HardwareEvent names
  • Remove legacy action handling

Review requests

Risk assessment

Low, mostly refactoring and renaming with only small and contained functionality added

@jbleon95 jbleon95 requested a review from a team as a code owner July 6, 2022 18:51
@jbleon95 jbleon95 added the robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). label Jul 6, 2022
@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #11021 (912d60f) into edge (53d3212) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #11021      +/-   ##
==========================================
- Coverage   73.80%   73.79%   -0.01%     
==========================================
  Files        2076     2076              
  Lines       57318    57351      +33     
  Branches     5729     5734       +5     
==========================================
+ Hits        42302    42322      +20     
- Misses      13778    13791      +13     
  Partials     1238     1238              
Flag Coverage Δ
notify-server 89.17% <ø> (ø)

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

Impacted Files Coverage Δ
.../src/opentrons/protocol_engine/actions/__init__.py 100.00% <ø> (ø)
...i/src/opentrons/protocol_engine/actions/actions.py 100.00% <100.00%> (ø)
...rc/opentrons/protocol_engine/execution/__init__.py 100.00% <100.00%> (ø)
...i/src/opentrons/protocol_engine/protocol_engine.py 100.00% <100.00%> (ø)
...pi/src/opentrons/protocol_engine/state/commands.py 99.50% <100.00%> (ø)
...opentrons/protocol_runner/legacy_context_plugin.py 100.00% <100.00%> (ø)
...colRun/SetupLiquids/LiquidsLabwareDetailsModal.tsx 86.36% <0.00%> (-8.09%) ⬇️
...e/hardware_control/motion_planning/move_manager.py 89.36% <0.00%> (-4.26%) ⬇️
...are/hardware_control/motion_planning/move_utils.py 89.83% <0.00%> (-1.61%) ⬇️
hardware/opentrons_hardware/scripts/move.py 0.00% <0.00%> (ø)
... and 14 more

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.

LGTM, pending robot testing. Will approve once someone can work through these smoke tests

JSONv6, door pause setting off:

  • Door open does nothing during LPC
  • Door close does nothing during LPC
  • Door open does nothing during protocol run
  • Door close does nothing during protocol run

JSONv6, door pause setting on:

  • Door open does nothing during LPC
  • Door close does nothing during LPC
  • Door open during protocol run pauses
  • {"actionType": "play"} errors while door is still open
  • Door close followed by {"actionType": "play"} resumes the run

PAPIv2, door pause setting off:

  • Door open does nothing during LPC
  • Door close does nothing during LPC
  • Door open does nothing during protocol run
  • Door close does nothing during protocol run

PAPIv2, door pause setting on:

  • Door open does nothing during LPC
  • Door close does nothing during LPC
  • Door open during protocol run pauses
  • {"actionType": "play"} errors while door is still open
  • Door close followed by {"actionType": "play"} resumes the run

Edit: All above tested by @SyntaxColoring.

@mcous mcous dismissed their stale review July 7, 2022 19:20

Changes addressed

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

I haven't looked at the code, but I've run through @mcous's tests, and this appears to be working as specified.

@SyntaxColoring SyntaxColoring requested a review from mcous July 8, 2022 18:18
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.

Approving based on @SyntaxColoring's testing

@jbleon95 jbleon95 merged commit c3dc40e into edge Jul 11, 2022
@jbleon95 jbleon95 deleted the pe-pause-protocol-door-open branch July 11, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

feat(engine): pause protocol execution if front door is opened
3 participants