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 dtwiz after estop #16168

Merged
merged 9 commits into from
Sep 3, 2024

Conversation

TamarZanzouri
Copy link
Contributor

@TamarZanzouri TamarZanzouri commented Aug 29, 2024

Overview

Fixes some issues around estop and drop tip. Specifically,

  • When you hit the estop, we surface an error and lose positioning - normal for errors like this - but we also turn off the motors, and they need to be turned back on, so add a command to do that and use it in dtwiz
  • on desktop, the estop takeover wouldn't work if you were anywhere but the top level devices page because of either the react-router update or because of an oversight that was made during the flex launch
  • the estop takeover is polling, and slowly. we should add notifications at some point but for now we can do some strategic query invalidations and fast polling when the estop is triggered to make it more responsive

Closes RQA-3103, RQA-3133

Test Plan and Hands on Testing

  • run a protocol, stop it with estop, and check that the modal fires; is responsive; and leaves you to a dtwiz, which opertaes correctly
    • on desktop, viewing the run, while network connected
    • on desktop, viewing the run, while usb connected
    • on odd

TamarZanzouri and others added 7 commits August 28, 2024 14:23
The estop system is now checked a little better:
- When the estop is detected as engaged, we poll it a lot more
frequently so the buttons are more responsive
- When we hit the software interlock to disengage the estop, we use the
result immediately instead of waiting for a poll, which makes the result
nice and snappy
- When we see a run get an error, or a maintenance run fail, we
invalidate the estop query, so we don't have a weird lag between the run
failing with an estop error and the estop modal later popping up
I don't know whether this was the react-router update, or whether this
has always just been broken, but this useRoute for sure was not handling
any _subroutes_ below /devices/:robotName, just that route itself, so
you would never see the estop modal if you were, shall we say, looking
at a run.
If there was an estop, we need to update the position estimators of
everything, and to do that we need to go reactivate them.

Closes RQA-3103
@sfoster1 sfoster1 marked this pull request as ready for review August 30, 2024 21:04
@sfoster1 sfoster1 requested review from a team as code owners August 30, 2024 21:04
@sfoster1 sfoster1 requested review from brenthagen and removed request for a team August 30, 2024 21:04
@sfoster1 sfoster1 changed the title Rqa 3103 home before drop tip wizard fix(app): handle dtwiz after estop Sep 3, 2024
@sfoster1 sfoster1 requested a review from mjhuff September 3, 2024 14:44
Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

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

This was surprisingly involved. Thanks to you both for fixing this!

const deviceRouteMatch = useMatch('/devices/:robotName')
const params = deviceRouteMatch?.params as DesktopRouteParams
const robotName = params?.robotName
const deviceRouteMatch = useMatch('/devices/:robotName/*')
Copy link
Contributor

Choose a reason for hiding this comment

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

Super funny/weird change between RR versions.

It looks like this is the only place in the app we use useMatch.

const { data: estopStatus } = useEstopQuery({
refetchInterval: ESTOP_REFETCH_INTERVAL_MS,
refetchInterval: estopEngaged
? ESTOP_CURRENTLY_ENGAGED_REFETCH_INTERVAL_MS
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, great thinking.

})
console.log(`estop status: ${estopStatus?.data.status}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

stray log?

Copy link
Member

Choose a reason for hiding this comment

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

yup, good catch, thanks!

},
onError: (error: any) => {
setIsResuming(false)
console.log(error)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe console.error?

@sfoster1 sfoster1 merged commit 5999238 into chore_release-8.0.0 Sep 3, 2024
49 of 50 checks passed
@sfoster1 sfoster1 deleted the RQA-3103-home-before-drop-tip-wizard branch September 3, 2024 15:48
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