diff --git a/CHANGELOG.md b/CHANGELOG.md index c67a3e36..04c90597 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,9 @@ ### Fixed - Fixed run button enabled when no file open ([support#691]). - Fixed flash firmware dialog not showing when settings not open ([support#694]). +- Fixed errors not handled while flashing firmware via USB ([pybricks-code#1011]). +[pybricks-code#1011]: https://github.com/pybricks/pybricks-code/issues/1011 [support#691]: https://github.com/pybricks/support/issues/691 [support#694]: https://github.com/pybricks/support/issues/694 diff --git a/src/firmware/alerts/DfuError.tsx b/src/firmware/alerts/DfuError.tsx new file mode 100644 index 00000000..25149052 --- /dev/null +++ b/src/firmware/alerts/DfuError.tsx @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Button, Intent } from '@blueprintjs/core'; +import React from 'react'; +import { CreateToast } from '../../i18nToaster'; +import { useI18n } from './i18n'; + +type DfuErrorProps = { + onTryAgain: () => void; +}; + +const DfuError: React.VoidFunctionComponent = ({ onTryAgain }) => { + const i18n = useI18n(); + return ( + <> +

{i18n.translate('dfuError.message')}

+

{i18n.translate('dfuError.suggestion')}

+ + + ); +}; + +export const dfuError: CreateToast = (onAction) => { + return { + message: onAction('tryAgain')} />, + icon: 'error', + intent: Intent.DANGER, + onDismiss: () => onAction('dismiss'), + }; +}; diff --git a/src/firmware/alerts/index.ts b/src/firmware/alerts/index.ts index eba5d418..7b99c303 100644 --- a/src/firmware/alerts/index.ts +++ b/src/firmware/alerts/index.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors +import { dfuError } from './DfuError'; import { firmwareMismatch } from './FirmwareMismatch'; import { noDfuHub } from './NoDfuHub'; import { noDfuInterface } from './NoDfuInterface'; @@ -8,6 +9,7 @@ import { noWebUsb } from './NoWebUsb'; import { releaseButton } from './ReleaseButton'; export default { + dfuError, firmwareMismatch, noDfuHub, noDfuInterface, diff --git a/src/firmware/alerts/translations/en.json b/src/firmware/alerts/translations/en.json index 4777c5b2..d893bd5f 100644 --- a/src/firmware/alerts/translations/en.json +++ b/src/firmware/alerts/translations/en.json @@ -1,4 +1,9 @@ { + "dfuError": { + "message": "A USB error ocurred while flashing the firmware.", + "suggestion": "Ensure the USB cable is not damaged and is firmly attached to the hub and to the computer.", + "tryAgainButton": "Try again" + }, "noWebUsb": { "message": "This browser does not support Web USB or Web USB is not enabled.", "suggestion": "Use a supported browser such as Google Chrome or Microsoft Edge." diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 8cfeed59..f0e6f7e9 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -13,6 +13,7 @@ import moveHubZip from '@pybricks/firmware/build/movehub.zip'; import technicHubZip from '@pybricks/firmware/build/technichub.zip'; import { WebDFU } from 'dfu'; import { AnyAction } from 'redux'; +import { eventChannel } from 'redux-saga'; import { ActionPattern } from 'redux-saga/effects'; import { SagaGenerator, @@ -27,7 +28,7 @@ import { take, takeEvery, } from 'typed-redux-saga/macro'; -import { alertsShowAlert } from '../alerts/actions'; +import { alertsDidShowAlert, alertsShowAlert } from '../alerts/actions'; import { fileStorageDidFailToReadFile, fileStorageDidReadFile, @@ -595,6 +596,8 @@ const productIdMap: ReadonlyMap = new Map([ // currently all hubs use the same start address const dfuFirmwareStartAddress = 0x08008000; +const firmwareDfuProgressToastId = 'firmware.dfu.progress'; + function* handleFlashUsbDfu(action: ReturnType): Generator { const defer = new Array<() => void>(); @@ -671,7 +674,20 @@ function* handleFlashUsbDfu(action: ReturnType): Gen yield* call(() => dfu.connect(ifaceIndex)); - defer.push(() => dfu.close()); + defer.push(() => + dfu.close().catch((err) => { + if ( + err instanceof DOMException && + err.code === DOMException.NETWORK_ERR + ) { + // device was disconnected + return; + } + + // not expected + console.log(err); + }), + ); const { firmware, deviceId } = yield* loadFirmware( action.data, @@ -690,35 +706,85 @@ function* handleFlashUsbDfu(action: ReturnType): Gen const toaster = yield* getContext('toaster'); - writeProc.events.on('erase/process', (sent, total) => { - toaster.show( - flashProgress(() => undefined, { - action: 'erase', - progress: sent / total, - }), - 'firmware.dfu.progress', - ); + defer.push( + writeProc.events.on('erase/process', (sent, total) => { + toaster.show( + flashProgress(() => undefined, { + action: 'erase', + progress: sent / total, + }), + firmwareDfuProgressToastId, + ); + }), + ); + + defer.push( + writeProc.events.on('write/process', (sent, total) => { + toaster.show( + flashProgress(() => undefined, { + action: 'flash', + progress: sent / total, + }), + firmwareDfuProgressToastId, + ); + }), + ); + + const endChan = eventChannel((emit) => { + // can't emit null or undefined, so have to emit something + return writeProc.events.on('end', () => emit(true)); }); - writeProc.events.on('write/process', (sent, total) => { - toaster.show( - flashProgress(() => undefined, { - action: 'flash', - progress: sent / total, - }), - 'firmware.dfu.progress', - ); + defer.push(() => endChan.close()); + + const errorChan = eventChannel((emit) => { + return writeProc.events.on('error', emit); }); - writeProc.events.on('error', console.error); + defer.push(() => errorChan.close()); - // REVISIT: we could possibly race the 'write/end' and 'error' events - // here instead of waiting for disconnect + const { error } = yield* (function* () { + // HACK: Somehow an error during the write phase can cause the + // race generator to throw instead of returning the error. + // So we catch the error and return it as if errorChan won the + // race. + try { + return yield* race({ + end: take(endChan), + error: take(errorChan), + }); + } catch (err) { + return { error: err }; + } + })(); - // this is a bit of a hack, but the hub resets when flashing is done - // so we get a disconnect event unless there was an error, so the user - // will probably see the timeout error instead of the underlying error - yield* call(() => dfu.waitDisconnected(30000)); + // errors can happen, e.g. if the USB cable is disconnected while + // flashing the firmware + if (error) { + if (process.env.NODE_ENV !== 'test') { + console.error(error); + } + + toaster.dismiss(firmwareDfuProgressToastId); + yield* put(firmwareDidFailToFlashUsbDfu()); + + yield* put(alertsShowAlert('firmware', 'dfuError')); + + const { action: alertAction } = yield* take< + ReturnType> + >( + alertsDidShowAlert.when( + (a) => a.domain === 'firmware' && a.specific === 'dfuError', + ), + ); + + if (alertAction === 'tryAgain') { + // queue the action that triggered this saga to retry + yield* put(action); + } + + return; + } yield* put(firmwareDidFlashUsbDfu()); } catch (err) {