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): Handle Unsafe Move to Plunger during Drop-Tip #14910

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Apr 15, 2024

Closes EXEC-186

Overview

If the gantry is not homed and a powercycle occurs, drop-tip wizard cannot proceed with flows. An error is raised during the flow, and ultimately a home command is dispatched that has the side effect of potentially aspirating liquid into the pipette, damaging it. We need to special case home errors to prevent this. See ticket for designs.

The primary functional difference is now that any time an error occurs, exiting the wizard via the header should not home the gantry. Homing as a result of an error should only occur when the "Confirm removal and home" button is presented and clicked.

There's some minor refactor work done in this PR as well. More refactoring is definitely needed, but that should be handled in a separate PR (and take into account the wizard's use in the new error recovery flows).

Note that I'm doing something a bit different for testing purposes, as a lot of our large wizard flows lack testing and are hard to appropriately approach with one big DOM driven integration test. Let's create some local utility functions and unit test those. Let me know what ya'll think.

ODD

Screenshot 2024-04-15 at 1 42 25 PM

Desktop

Screenshot 2024-04-15 at 1 00 33 PM

Test Plan

  • Do something that causes the gantry not to be homed. Power cycle the robot.
  • Launch drop tip wizard, and proceed through the flow until the MustHomeError occurs.
  • Confirm error recovery functionality works as intended based on designs.

Changelog

  • Fixed unsafe move plunger commands that may occur drop tip wizard.

Risk assessment

low

@mjhuff mjhuff requested review from ecormany and a team April 15, 2024 21:11
@mjhuff mjhuff requested a review from a team as a code owner April 15, 2024 21:11
@mjhuff mjhuff requested review from sanni-t, jbleon95, shlokamin, koji, jerader and ncdiehl11 and removed request for a team, sanni-t, jbleon95 and shlokamin April 15, 2024 21:11
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.

I think this is great and obviously I'm very into the testing approach. I wonder if we could take it a bit further and drop these hooks into their own file though. Plus some nits

app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
app/src/organisms/DropTipWizard/index.tsx Outdated Show resolved Hide resolved
@mjhuff mjhuff added DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available and removed DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available labels Apr 16, 2024
@mjhuff
Copy link
Contributor Author

mjhuff commented Apr 16, 2024

@ecormany is OOO until Thursday. I'll merge this and just open a copy PR later if necessary.

@mjhuff mjhuff merged commit 9152f22 into edge Apr 16, 2024
24 checks passed
@mjhuff mjhuff deleted the app_fix-drop-tip-must-home-error branch April 16, 2024 17:29
DerekMaggio pushed a commit that referenced this pull request Apr 16, 2024
Closes EXEC-186

If the gantry is not homed and a powercycle occurs, drop-tip wizard cannot proceed with flows. An error is raised during the flow, and ultimately a home command is dispatched that has the side effect of potentially aspirating liquid into the pipette, damaging it. We special case home errors to prevent this. 

The primary functional difference is now that any time an error occurs, exiting the wizard via the header should not home the gantry. Homing as a result of an error should only occur when the "Confirm removal and home" button is presented and clicked.
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
Closes EXEC-186

If the gantry is not homed and a powercycle occurs, drop-tip wizard cannot proceed with flows. An error is raised during the flow, and ultimately a home command is dispatched that has the side effect of potentially aspirating liquid into the pipette, damaging it. We special case home errors to prevent this. 

The primary functional difference is now that any time an error occurs, exiting the wizard via the header should not home the gantry. Homing as a result of an error should only occur when the "Confirm removal and home" button is presented and clicked.
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