Skip to content

Commit

Permalink
Better handling for surprise disconnects
Browse files Browse the repository at this point in the history
- Fix a state issue where we didn't clear sessions; we now clear them on
retry or cancel
- Handle surprise unmounts that leave /media populated by also watching
/dev
- Fix a bug where the initial usb scan used full paths instead of device
names; now everything uses full paths
  • Loading branch information
sfoster1 committed Nov 7, 2023
1 parent 3cb4ecb commit c3a6a0f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 16 deletions.
7 changes: 7 additions & 0 deletions app-shell-odd/src/system-update/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,15 @@ export function registerRobotSystemUpdate(dispatch: Dispatch): Dispatch {
massStorageUpdateSet !== null &&
massStorageUpdateSet.system.startsWith(action.payload.rootPath)
) {
console.log(

Check warning on line 144 in app-shell-odd/src/system-update/index.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/system-update/index.ts#L144

Added line #L144 was not covered by tests
`Mass storage device ${action.payload.rootPath} removed, reverting to non-usb updates`
)
massStorageUpdateSet = null
getCachedSystemUpdateFiles(dispatch)

Check warning on line 148 in app-shell-odd/src/system-update/index.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/system-update/index.ts#L147-L148

Added lines #L147 - L148 were not covered by tests
} else {
console.log(

Check warning on line 150 in app-shell-odd/src/system-update/index.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/system-update/index.ts#L150

Added line #L150 was not covered by tests
`Mass storage device ${action.payload.rootPath} removed but this was not an update source`
)
}
break

Check warning on line 154 in app-shell-odd/src/system-update/index.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/system-update/index.ts#L154

Added line #L154 was not covered by tests
}
Expand Down
39 changes: 32 additions & 7 deletions app-shell-odd/src/usb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
robotMassStorageDeviceRemoved,
} from '@opentrons/app/src/redux/shell/actions'
const FLEX_USB_MOUNT_DIR = '/media/'
const FLEX_USB_MOUNT_FILTER = /sd[a-z][0-9]$/
const FLEX_USB_DEVICE_DIR = '/dev/'
const FLEX_USB_MOUNT_FILTER = /sd[a-z]+[0-9]+$/
const MOUNT_ENUMERATION_DELAY_MS = 1000

Check warning on line 14 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L11-L14

Added lines #L11 - L14 were not covered by tests

// These directories are generated by OSX and contain entries for all
Expand Down Expand Up @@ -89,7 +90,7 @@ export function watchForMassStorage(dispatch: Dispatch): () => void {
prevDirs = present.filter((entry): entry is string => entry !== null)

Check warning on line 90 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L90

Added line #L90 was not covered by tests
})

const watcher = fs.watch(
const mediaWatcher = fs.watch(

Check warning on line 93 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L93

Added line #L93 was not covered by tests
FLEX_USB_MOUNT_DIR,
{ persistent: true },
(event, fileName) => {
Expand All @@ -107,25 +108,49 @@ export function watchForMassStorage(dispatch: Dispatch): () => void {
if (!info.isDirectory) {
return

Check warning on line 109 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L109

Added line #L109 was not covered by tests
}
if (prevDirs.includes(fileName)) {
if (prevDirs.includes(fullPath)) {
return

Check warning on line 112 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L112

Added line #L112 was not covered by tests
}
console.log(`New mass storage device ${fileName} detected`)
prevDirs.push(fileName)
prevDirs.push(fullPath)
return handleNewlyPresent(fullPath)

Check warning on line 116 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L114-L116

Added lines #L114 - L116 were not covered by tests
})
.catch(() => {
if (prevDirs.includes(fileName)) {
if (prevDirs.includes(fullPath)) {
console.log(`Mass storage device at ${fileName} removed`)
prevDirs = prevDirs.filter(entry => entry !== fileName)
prevDirs = prevDirs.filter(entry => entry !== fullPath)
dispatch(robotMassStorageDeviceRemoved(fullPath))

Check warning on line 122 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L120-L122

Added lines #L120 - L122 were not covered by tests
}
})
}
)

const devWatcher = fs.watch(

Check warning on line 128 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L128

Added line #L128 was not covered by tests
FLEX_USB_DEVICE_DIR,
{ persistent: true },
(event, fileName) => {
if (!!!fileName) return
if (!fileName.match(FLEX_USB_MOUNT_FILTER)) return
const fullPath = join(FLEX_USB_DEVICE_DIR, fileName)
const mountPath = join(FLEX_USB_MOUNT_DIR, fileName)
fsPromises.stat(fullPath).catch(() => {

Check warning on line 136 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L134-L136

Added lines #L134 - L136 were not covered by tests
if (prevDirs.includes(mountPath)) {
console.log(`Mass storage device at ${fileName} removed`)
prevDirs = prevDirs.filter(entry => entry !== mountPath)
dispatch(

Check warning on line 140 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L138-L140

Added lines #L138 - L140 were not covered by tests
robotMassStorageDeviceRemoved(join(FLEX_USB_MOUNT_DIR, fileName))
)
// we don't care if this fails because it's racing the system removing
// the mount dir in the common case
fsPromises.unlink(mountPath).catch(() => {})

Check warning on line 145 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L145

Added line #L145 was not covered by tests
}
})
}
)

rescan(dispatch)
return () => {
watcher.close()
mediaWatcher.close()
devWatcher.close()

Check warning on line 154 in app-shell-odd/src/usb.ts

View check run for this annotation

Codecov / codecov/patch

app-shell-odd/src/usb.ts#L151-L154

Added lines #L151 - L154 were not covered by tests
}
}
17 changes: 12 additions & 5 deletions app/src/pages/OnDeviceDisplay/UpdateRobot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import { Flex, SPACING, DIRECTION_ROW } from '@opentrons/components'
import { getLocalRobot } from '../../redux/discovery'
import {
getRobotUpdateAvailable,
startRobotUpdate,
clearRobotUpdateSession,
} from '../../redux/robot-update'
import { useDispatchStartRobotUpdate } from '../../redux/robot-update/hooks'
import { UNREACHABLE } from '../../redux/discovery/constants'

import { MediumButton } from '../../atoms/buttons'
Expand All @@ -24,15 +25,15 @@ import type { State, Dispatch } from '../../redux/types'
export function UpdateRobot(): JSX.Element {
const history = useHistory()
const { i18n, t } = useTranslation(['device_settings', 'shared'])
const dispatch = useDispatch<Dispatch>()

const localRobot = useSelector(getLocalRobot)
const robotUpdateType = useSelector((state: State) => {
return localRobot != null && localRobot.status !== UNREACHABLE
? getRobotUpdateAvailable(state, localRobot)
: null
})
const robotName = localRobot?.name != null ? localRobot.name : 'no name'
const dispatchStartRobotUpdate = useDispatchStartRobotUpdate()
const dispatch = useDispatch<Dispatch>()

const [errorString, setErrorString] = React.useState<string | null>(null)

Expand All @@ -45,11 +46,17 @@ export function UpdateRobot(): JSX.Element {
flex="1"
buttonType="secondary"
buttonText={t('cancel_software_update')}
onClick={() => history.goBack()}
onClick={() => {
dispatch(clearRobotUpdateSession())
history.goBack()
}}
/>
<MediumButton
flex="1"
onClick={() => dispatch(startRobotUpdate(robotName))}
onClick={() => {
setErrorString(null)
dispatchStartRobotUpdate(robotName)
}}
buttonText={i18n.format(t('shared:try_again'), 'capitalize')}
/>
</Flex>
Expand Down
13 changes: 9 additions & 4 deletions app/src/pages/OnDeviceDisplay/UpdateRobotDuringOnboarding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import { useTranslation } from 'react-i18next'

import { Flex, SPACING, DIRECTION_ROW } from '@opentrons/components'

import { useDispatchStartRobotUpdate } from '../../redux/robot-update/hooks'

import { getLocalRobot } from '../../redux/discovery'
import {
getRobotUpdateAvailable,
startRobotUpdate,
clearRobotUpdateSession,
} from '../../redux/robot-update'
import { UNREACHABLE } from '../../redux/discovery/constants'
import {
Expand All @@ -34,7 +36,7 @@ export function UpdateRobotDuringOnboarding(): JSX.Element {
] = React.useState<boolean>(true)
const history = useHistory()
const { i18n, t } = useTranslation(['device_settings', 'shared'])

const dispatchStartRobotUpdate = useDispatchStartRobotUpdate()
const dispatch = useDispatch<Dispatch>()
const localRobot = useSelector(getLocalRobot)
const robotUpdateType = useSelector((state: State) => {
Expand Down Expand Up @@ -82,11 +84,14 @@ export function UpdateRobotDuringOnboarding(): JSX.Element {
flex="1"
buttonType="secondary"
buttonText={t('proceed_without_updating')}
onClick={() => history.push('/emergency-stop')}
onClick={() => {
dispatch(clearRobotUpdateSession())
history.push('/emergency-stop')
}}
/>
<MediumButton
flex="1"
onClick={() => dispatch(startRobotUpdate(robotName))}
onClick={() => dispatchStartRobotUpdate(robotName)}
buttonText={i18n.format(t('shared:try_again'), 'capitalize')}
/>
</Flex>
Expand Down

0 comments on commit c3a6a0f

Please sign in to comment.