From 92431298b2c2d7fe45ece3f2e6a8cf9a3725a8c9 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 10 Aug 2022 12:47:18 -0500 Subject: [PATCH 1/6] toolbar/RunButton: enable only when active file Before multi-file support, it was not possible to have no open file. So we need to change the state logic to only enable the button when there is an active file rather than when the editor "is ready". Fixes: https://github.com/pybricks/support/issues/691 --- CHANGELOG.md | 5 +++++ src/toolbar/buttons/run/RunButton.test.tsx | 4 ++-- src/toolbar/buttons/run/RunButton.tsx | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f31e6178..3574d5f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ## [Unreleased] +### Fixed +- Fixed run button enabled when no file open ([support#691]). + +[support#691]: https://github.com/pybricks/support/issues/691 + ## [2.0.0-beta.5] - 2022-07-28 ### Fixed diff --git a/src/toolbar/buttons/run/RunButton.test.tsx b/src/toolbar/buttons/run/RunButton.test.tsx index 9359fb44..653ed1a7 100644 --- a/src/toolbar/buttons/run/RunButton.test.tsx +++ b/src/toolbar/buttons/run/RunButton.test.tsx @@ -3,7 +3,7 @@ import { cleanup } from '@testing-library/react'; import React from 'react'; -import { testRender } from '../../../../test'; +import { testRender, uuid } from '../../../../test'; import { downloadAndRun } from '../../../hub/actions'; import { HubRuntimeState } from '../../../hub/reducers'; import RunButton from './RunButton'; @@ -14,7 +14,7 @@ afterEach(() => { it('should dispatch action when clicked', async () => { const [user, button, dispatch] = testRender(, { - editor: { isReady: true }, + editor: { activeFileUuid: uuid(0) }, hub: { runtime: HubRuntimeState.Idle }, }); diff --git a/src/toolbar/buttons/run/RunButton.tsx b/src/toolbar/buttons/run/RunButton.tsx index 4eb265cd..35cee8dd 100644 --- a/src/toolbar/buttons/run/RunButton.tsx +++ b/src/toolbar/buttons/run/RunButton.tsx @@ -16,7 +16,7 @@ const RunButton: React.VoidFunctionComponent = ({ id }) => { const downloadProgress = useSelector((s) => s.hub.downloadProgress); const mpyAbiVersion = useSelector((s) => s.hub.mpyAbiVersion); const runtime = useSelector((s) => s.hub.runtime); - const isEditorReady = useSelector((s) => s.editor.isReady); + const activeFile = useSelector((s) => s.editor.activeFileUuid); const keyboardShortcut = 'F5'; const i18n = useI18n(); @@ -35,7 +35,7 @@ const RunButton: React.VoidFunctionComponent = ({ id }) => { : i18n.translate('tooltip.action', { key: keyboardShortcut }) } icon={icon} - enabled={isEditorReady && runtime === HubRuntimeState.Idle} + enabled={activeFile !== null && runtime === HubRuntimeState.Idle} showProgress={runtime === HubRuntimeState.Loading} progress={downloadProgress === null ? undefined : downloadProgress} onAction={() => dispatch(downloadAndRun(mpyAbiVersion))} From 5b71c7314af5bcd9d076097be1c9e66ed09bdadd Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 10 Aug 2022 16:45:40 -0500 Subject: [PATCH 2/6] app/App: move firmware flash dialogs from settings The flash firmware dialog show action can be triggered even when the settings panel is not mounted, so we need to move the dialogs up in the tree to the app level. Fixes: https://github.com/pybricks/support/issues/694 --- CHANGELOG.md | 2 ++ src/app/App.tsx | 4 ++++ src/settings/Settings.tsx | 4 ---- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3574d5f0..c67a3e36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,10 @@ ### Fixed - Fixed run button enabled when no file open ([support#691]). +- Fixed flash firmware dialog not showing when settings not open ([support#694]). [support#691]: https://github.com/pybricks/support/issues/691 +[support#694]: https://github.com/pybricks/support/issues/694 ## [2.0.0-beta.5] - 2022-07-28 diff --git a/src/app/App.tsx b/src/app/App.tsx index af08dad1..434af466 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -10,6 +10,8 @@ import SplitterLayout from 'react-splitter-layout'; import { useLocalStorage, useTernaryDarkMode } from 'usehooks-ts'; import Activities from '../activities/Activities'; import Editor from '../editor/Editor'; +import { InstallPybricksDialog } from '../firmware/installPybricksDialog/InstallPybricksDialog'; +import RestoreOfficialDialog from '../firmware/restoreOfficialDialog/RestoreOfficialDialog'; import { useSettingIsShowDocsEnabled } from '../settings/hooks'; import StatusBar from '../status-bar/StatusBar'; import Terminal from '../terminal/Terminal'; @@ -198,6 +200,8 @@ const App: React.VFC = () => { + + ); }; diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index b31f823e..9c805a48 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -22,8 +22,6 @@ import { import { Button } from '../components/Button'; import HelpButton from '../components/HelpButton'; import { firmwareInstallPybricks } from '../firmware/actions'; -import { InstallPybricksDialog } from '../firmware/installPybricksDialog/InstallPybricksDialog'; -import RestoreOfficialDialog from '../firmware/restoreOfficialDialog/RestoreOfficialDialog'; import { firmwareRestoreOfficialDialogShow } from '../firmware/restoreOfficialDialog/actions'; import { pseudolocalize } from '../i18n'; import { useSelector } from '../reducers'; @@ -104,7 +102,6 @@ const Settings: React.VoidFunctionComponent = () => { label={i18n.translate('firmware.flashPybricksButton.label')} onPress={() => dispatch(firmwareInstallPybricks())} /> - + + ); +}; + +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) {