Skip to content

Commit

Permalink
refactor(app): Remove robot admin restart logic (#13778)
Browse files Browse the repository at this point in the history
* refactor(app): remove unused robot restart timeout logic

Removes the redux layer robot update restart timeout logic that is replaced by more generalized
robot update flow error handling.

* refactor(app): refactor robot update failure message
  • Loading branch information
mjhuff authored Oct 12, 2023
1 parent d66d878 commit 20ca4a5
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 78 deletions.
2 changes: 1 addition & 1 deletion app/src/assets/localization/en/device_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@
"resets_cannot_be_undone": "Resets cannot be undone",
"restart_now": "Restart now?",
"restart_robot_confirmation_description": "<span>It will take a few minutes for <bold>{{robotName}}</bold> to restart.</span>",
"restart_taking_too_long": "{{robotName}} restart is taking longer than expected. If the robot appears unresponsive, try restarting the robot manually.",
"restart_taking_too_long": "{{robotName}} is taking longer than expected to restart. Check the Advanced tab of its settings page to see whether it updated successfully. If the robot is unresponsive, restart it manually.",
"restarting_robot": "Install complete, robot restarting...",
"resume_robot_operations": "Resume robot operations",
"returns_your_device_to_new_state": "This returns your device to a new state.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,6 @@ describe('DownloadUpdateModal', () => {
})

findByText('Try restarting the update.')
findByText('testRobot restart is taking longer than expected.')
findByText('testRobot restart is taking longer than expected to restart.')
})
})
39 changes: 0 additions & 39 deletions app/src/redux/robot-admin/__tests__/selectors.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { add } from 'date-fns'
import { CONNECTABLE, REACHABLE } from '../../discovery'
import {
getRobotRestarting,
getNextRestartStatus,
getResetConfigOptions,
} from '../selectors'
import { ConnectivityStatus } from '../../discovery/types'
import { RESTART_TIMEOUT_SEC } from '../constants'
import type { State } from '../../types'

const START_TIME = new Date('2000-01-01')
Expand Down Expand Up @@ -197,42 +195,5 @@ describe('robot admin selectors', () => {

expect(result).toBe('restart-in-progress')
})

it('should return restart timed out if it takes too long', () => {
const startTime = new Date('2000-01-01')
const notLongEnough = add(startTime, { seconds: RESTART_TIMEOUT_SEC - 1 })
const tooLong = add(startTime, { seconds: RESTART_TIMEOUT_SEC + 1 })

const state: State = {
robotAdmin: {
robotName: {
restart: {
...RESTART_BASE,
startTime,
status: 'restart-in-progress',
},
},
},
} as any

const before = getNextRestartStatus(
state,
'robotName',
REACHABLE,
null,
notLongEnough
)

const after = getNextRestartStatus(
state,
'robotName',
REACHABLE,
null,
tooLong
)

expect(before).toBe(null)
expect(after).toBe('restart-timed-out')
})
})
})
4 changes: 0 additions & 4 deletions app/src/redux/robot-admin/constants.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// general constants

// 15 minute restart timeout
export const RESTART_TIMEOUT_SEC = 900

// restart statuses
export const RESTART_PENDING_STATUS: 'restart-pending' = 'restart-pending'
export const RESTART_IN_PROGRESS_STATUS: 'restart-in-progress' =
'restart-in-progress'
export const RESTART_FAILED_STATUS: 'restart-failed' = 'restart-failed'
export const RESTART_TIMED_OUT_STATUS: 'restart-timed-out' = 'restart-timed-out'
export const RESTART_SUCCEEDED_STATUS: 'restart-succeeded' = 'restart-succeeded'

// action type strings
Expand Down
24 changes: 0 additions & 24 deletions app/src/redux/robot-admin/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { add, isWithinInterval } from 'date-fns'
import { CONNECTABLE } from '../discovery'
import {
RESTART_TIMEOUT_SEC,
RESTART_PENDING_STATUS,
RESTART_IN_PROGRESS_STATUS,
RESTART_SUCCEEDED_STATUS,
RESTART_TIMED_OUT_STATUS,
} from './constants'

import type { ConnectivityStatus } from '../discovery/types'
Expand Down Expand Up @@ -43,10 +40,6 @@ export function getNextRestartStatus(
return RESTART_IN_PROGRESS_STATUS
}

if (getRestartHasTimedOut(state, robotName, now)) {
return RESTART_TIMED_OUT_STATUS
}

return null
}

Expand All @@ -59,23 +52,6 @@ function getRestartHasBegun(
return status === RESTART_PENDING_STATUS && connectivityStatus !== CONNECTABLE
}

function getRestartHasTimedOut(
state: State,
robotName: string,
now: Date
): boolean {
const restartState = robotState(state, robotName)?.restart

if (!restartState?.startTime || !getRobotRestarting(state, robotName)) {
return false
}

const { startTime } = restartState
const endTime = add(startTime, { seconds: RESTART_TIMEOUT_SEC })

return !isWithinInterval(now, { start: startTime, end: endTime })
}

function getRestartIsComplete(
state: State,
robotName: string,
Expand Down
10 changes: 1 addition & 9 deletions app/src/redux/robot-update/epic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
RESTART_PATH,
RESTART_STATUS_CHANGED,
RESTART_SUCCEEDED_STATUS,
RESTART_TIMED_OUT_STATUS,
restartRobotSuccess,
} from '../robot-admin'

Expand Down Expand Up @@ -93,7 +92,6 @@ const UNABLE_TO_CANCEL_UPDATE_SESSION =
const UNABLE_TO_COMMIT_UPDATE = 'Unable to commit update'
const UNABLE_TO_RESTART_ROBOT = 'Unable to restart robot'
const ROBOT_RECONNECTED_WITH_VERSION = 'Robot reconnected with version'
const ROBOT_DID_NOT_RECONNECT = 'Robot did not successfully reconnect'
const BUT_WE_EXPECTED = 'but we expected'
const UNKNOWN = 'unknown'
const CHECK_TO_VERIFY_UPDATE =
Expand Down Expand Up @@ -375,8 +373,7 @@ export const finishAfterRestartEpic: Epic = (action$, state$) => {
const session = getRobotUpdateSession(state)
const robot = getRobotUpdateRobot(state)
const restartDone =
action.payload.restartStatus === RESTART_SUCCEEDED_STATUS ||
action.payload.restartStatus === RESTART_TIMED_OUT_STATUS
action.payload.restartStatus === RESTART_SUCCEEDED_STATUS

return (
restartDone &&
Expand All @@ -393,7 +390,6 @@ export const finishAfterRestartEpic: Epic = (action$, state$) => {
)

const robotVersion = getRobotApiVersion(robot)
const timedOut = action.payload.restartStatus === RESTART_TIMED_OUT_STATUS
const actual = robotVersion ?? UNKNOWN
const expected = targetVersion ?? UNKNOWN
let finishAction
Expand All @@ -404,10 +400,6 @@ export const finishAfterRestartEpic: Epic = (action$, state$) => {
robotVersion === targetVersion
) {
finishAction = setRobotUpdateSessionStep(FINISHED)
} else if (timedOut) {
finishAction = unexpectedRobotUpdateError(
`${ROBOT_DID_NOT_RECONNECT}. ${CHECK_TO_VERIFY_UPDATE}.`
)
} else {
finishAction = unexpectedRobotUpdateError(
`${ROBOT_RECONNECTED_WITH_VERSION} ${actual}, ${BUT_WE_EXPECTED} ${expected}. ${CHECK_TO_VERIFY_UPDATE}.`
Expand Down

0 comments on commit 20ca4a5

Please sign in to comment.