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): Fix entering error recovery while door is open #16245

Merged
merged 3 commits into from
Sep 12, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Sep 12, 2024

Closes RABR-608

Overview

Currently, if you open the robot door while on the error recovery splash screen, the "launch recovery" button is not disabled, and the door CTA appears as soon as the user clicks "launch recovery". The pipette does not home (which isn't great), but for the most part, a user can complete recovery. However, the robot position is lost, which has downstream affects when the user either retries a failed command or resumes the protocol run, typically resulting in a MUST_HOME_ERROR.

To solve for the pipettes not homing and this downstream error, we should just block the users from entering ER until the door is closed. After testing a few different approaches, showing a warning toast seems best.

1edd67d - The fix
9261cc9 - In the low-fi days of ER designs, we had a "run paused" screen instead of the fancy splash screen. Calling that view RunPausedSplash doesn't make much sense anymore.

Desktop

Screenshot 2024-09-12 at 9 55 43 AM

ODD

Screenshot 2024-09-12 at 10 05 23 AM

Test Plan and Hands on Testing

  • Verified the new functionality works as intended.
  • Verified that the old in-flow functionality works as intended...it actually doesn't fully for different reasons, but @TamarZanzouri is putting up a PR soon!

Changelog

  • You can no longer click an error recovery splash page button and proceed if the door is open.

Risk assessment

low

@mjhuff mjhuff requested a review from a team as a code owner September 12, 2024 16:15
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.

Okay, sounds good to me. Wish we could make the buttons visually disabled but I think this is fine.

@mjhuff
Copy link
Contributor Author

mjhuff commented Sep 12, 2024

Okay, sounds good to me. Wish we could make the buttons visually disabled but I think this is fine.

Here you go. You can still click the buttons and get the toast.

Screenshot 2024-09-12 at 1 10 00 PM Screenshot 2024-09-12 at 1 10 07 PM

@mjhuff mjhuff force-pushed the app_recovery-door-open-splash-fix branch from 5ff6f52 to c20c5f4 Compare September 12, 2024 17:17
Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

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

Nice job!

@mjhuff mjhuff merged commit 79cfedf into chore_release-8.0.0 Sep 12, 2024
20 checks passed
@mjhuff mjhuff deleted the app_recovery-door-open-splash-fix branch September 12, 2024 17:37
mjhuff added a commit that referenced this pull request Sep 16, 2024
…recovery (#16259)

Closes EXEC-702

In #16245, we made sure to block an open door while on the recovery splash view, but the logic for automatically transition a run from paused -> awaiting recovery is not quite right, which results in the app seemingly automatically issuing a POST play to transition a run from a recovery paused state to awaiting recovery whenever a user opens the door during error recovery flows (ie, anywhere but the splash screen). The tricky part is that when checking the network tab, there are no POST requests to initiate the transition.

This problem occurs because another app issues the POST request. The logic for dispatching this POST in the useEffect condition within RecoverySplash is to check if the error recovery wizard is active, but this doesn't account for any other app that isn't doing the recovery. The solution: if the current app displays the takeover modal (which is true for any app except the sole app controlling the robot), then don't fire the POST request.
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.

3 participants