Merge pull request #1056 from pybricks/dlech

fixes

CHANGELOG.md

@@ -4,6 +4,15 @@
 
 ## [Unreleased]
 
+### 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
+
 ## [2.0.0-beta.5] - 2022-07-28
 
 ### Fixed

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 = () => {
             </div>
             <StatusBar />
             <Tour />
+            <InstallPybricksDialog />
+            <RestoreOfficialDialog />
         </div>
     );
 };

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<DfuErrorProps> = ({ onTryAgain }) => {
+    const i18n = useI18n();
+    return (
+        <>
+            <p>{i18n.translate('dfuError.message')}</p>
+            <p>{i18n.translate('dfuError.suggestion')}</p>
+            <Button onClick={onTryAgain}>
+                {i18n.translate('dfuError.tryAgainButton')}
+            </Button>
+        </>
+    );
+};
+
+export const dfuError: CreateToast<never, 'dismiss' | 'tryAgain'> = (onAction) => {
+    return {
+        message: <DfuError onTryAgain={() => onAction('tryAgain')} />,
+        icon: 'error',
+        intent: Intent.DANGER,
+        onDismiss: () => onAction('dismiss'),
+    };
+};

src/firmware/alerts/FlashProgress.tsx

@@ -35,9 +35,7 @@ const FlashProgress: React.VoidFunctionComponent<FlashProgressProps> = ({
                 </p>
             )}
 
-            <p>
-                <ProgressBar value={progress} />
-            </p>
+            <ProgressBar value={progress} />
         </>
     );
 };

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,

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."

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<LegoUsbProductId, HubType> = new Map([
 // currently all hubs use the same start address
 const dfuFirmwareStartAddress = 0x08008000;
 
+const firmwareDfuProgressToastId = 'firmware.dfu.progress';
+
 function* handleFlashUsbDfu(action: ReturnType<typeof firmwareFlashUsbDfu>): Generator {
     const defer = new Array<() => void>();
 
@@ -671,7 +674,20 @@ function* handleFlashUsbDfu(action: ReturnType<typeof firmwareFlashUsbDfu>): 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<typeof firmwareFlashUsbDfu>): Gen
 
         const toaster = yield* getContext<IToaster>('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<boolean>((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<typeof alertsDidShowAlert<'firmware', 'dfuError'>>
+            >(
+                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) {

src/index.tsx

@@ -52,8 +52,10 @@ const store = configureStore({
                     // contain ArrayBuffer or DataView
                     'data',
                     'firmwareZip',
+                    'payload',
                     // Error is not serializable
                     'error',
+                    'props.error',
                 ],
             },
         })

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())}
                 />
-                <InstallPybricksDialog />
                 <Button
                     id="pb-settings-flash-official-button"
                     minimal={true}
@@ -112,7 +109,6 @@ const Settings: React.VoidFunctionComponent = () => {
                     label={i18n.translate('firmware.flashLegoButton.label')}
                     onPress={() => dispatch(firmwareRestoreOfficialDialogShow())}
                 />
-                <RestoreOfficialDialog />
             </FormGroup>
             <FormGroup label={i18n.translate('help.title')}>
                 <ButtonGroup minimal={true} vertical={true} alignText="left">

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(<RunButton id="test-run-button" />, {
-        editor: { isReady: true },
+        editor: { activeFileUuid: uuid(0) },
         hub: { runtime: HubRuntimeState.Idle },
     });
 

src/toolbar/buttons/run/RunButton.tsx

@@ -16,7 +16,7 @@ const RunButton: React.VoidFunctionComponent<RunButtonProps> = ({ 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<RunButtonProps> = ({ 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))}