From 2ff5ce9852be77e0e0f4586b10eebf821922be5f Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 10:32:21 -0600 Subject: [PATCH 01/18] dismiss compile error notification on successful compile If we got a successful compile, then the error is no longer applicable. --- src/sagas/notification.test.ts | 30 +++++++++++++++++++++++++++++- src/sagas/notification.ts | 6 ++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/sagas/notification.test.ts b/src/sagas/notification.test.ts index 2838c9b2..86212272 100644 --- a/src/sagas/notification.test.ts +++ b/src/sagas/notification.test.ts @@ -13,9 +13,10 @@ import { BootloaderConnectionFailureReason, didFailToConnect as bootloaderDidFailToConnect, } from '../actions/lwp3-bootloader'; -import { didFailToCompile } from '../actions/mpy'; +import { didCompile, didFailToCompile } from '../actions/mpy'; import { add } from '../actions/notification'; import { didSucceed, didUpdate } from '../actions/service-worker'; +import { MessageId } from '../components/notification-i18n'; import notification from './notification'; test.each([ @@ -86,3 +87,30 @@ test.each([ await saga.end(); }); + +test.each([[didCompile(new Uint8Array()), MessageId.MpyError]])( + 'actions that should close a notification: %o', + async (action: Action, key: string) => { + const getToasts = jest.fn().mockReturnValue([]); + const show = jest.fn(); + const dismiss = jest.fn(); + const clear = jest.fn(); + + const toaster: IToaster = { + getToasts, + show, + dismiss, + clear, + }; + + const saga = new AsyncSaga(notification, { notification: { toaster } }); + + saga.put(action); + + expect(show).not.toBeCalled(); + expect(dismiss).toBeCalledWith(key); + expect(clear).not.toBeCalled(); + + await saga.end(); + }, +); diff --git a/src/sagas/notification.ts b/src/sagas/notification.ts index 23b60cbd..47ee3e5c 100644 --- a/src/sagas/notification.ts +++ b/src/sagas/notification.ts @@ -214,6 +214,11 @@ function* showEditorStorageChanged(): Generator { yield put(reloadProgram()); } +function* dismissCompilerError(): Generator { + const { toaster } = (yield getContext('notification')) as NotificationContext; + toaster.dismiss(MessageId.MpyError); +} + function* showCompilerError(action: MpyDidFailToCompileAction): Generator { yield* showSingleton(Level.Error, MessageId.MpyError, { errorMessage: action.err }); } @@ -263,6 +268,7 @@ export default function* (): Generator { showBootloaderDidFailToConnectError, ); yield takeEvery(EditorActionType.StorageChanged, showEditorStorageChanged); + yield takeEvery(MpyActionType.DidCompile, dismissCompilerError); yield takeEvery(MpyActionType.DidFailToCompile, showCompilerError); yield takeEvery(NotificationActionType.Add, addNotification); yield takeEvery(ServiceWorkerActionType.DidUpdate, showServiceWorkerUpdate); From 816053ba7c4ae766fa9d44f88f48f726ca372307 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 11:35:25 -0600 Subject: [PATCH 02/18] don't expect response on disconnect commands The LWP3 bootloader disconnect and reboot commands cause the device to disconnect before sending a response, so we must always use write without response for these, otherwise the program can hang waiting for a response. --- src/sagas/lwp3-bootloader-protocol.test.ts | 7 ++++++- src/sagas/lwp3-bootloader-protocol.ts | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/sagas/lwp3-bootloader-protocol.test.ts b/src/sagas/lwp3-bootloader-protocol.test.ts index b79d4839..8e59a032 100644 --- a/src/sagas/lwp3-bootloader-protocol.test.ts +++ b/src/sagas/lwp3-bootloader-protocol.test.ts @@ -121,6 +121,11 @@ describe('message encoder', () => { ], ], ])('encode %s request', async (_n, request, expected) => { + const messageTypesThatShouldBeCalledWithoutResponse = [ + BootloaderRequestActionType.Program, + BootloaderRequestActionType.Reboot, + BootloaderRequestActionType.Disconnect, + ]; const saga = new AsyncSaga(bootloader); saga.put(request); const message = new Uint8Array(expected); @@ -128,7 +133,7 @@ describe('message encoder', () => { expect(action).toEqual( send( message, - /* withResponse */ request.type !== BootloaderRequestActionType.Program, + !messageTypesThatShouldBeCalledWithoutResponse.includes(request.type), ), ); await saga.end(); diff --git a/src/sagas/lwp3-bootloader-protocol.ts b/src/sagas/lwp3-bootloader-protocol.ts index 8cc4cc33..e60f5422 100644 --- a/src/sagas/lwp3-bootloader-protocol.ts +++ b/src/sagas/lwp3-bootloader-protocol.ts @@ -81,7 +81,7 @@ function* encodeRequest(): Generator { ); break; case BootloaderRequestActionType.Reboot: - yield put(send(createStartAppRequest())); + yield put(send(createStartAppRequest(), /* withResponse */ false)); break; case BootloaderRequestActionType.Init: yield put(send(createInitLoaderRequest(action.firmwareSize))); @@ -96,7 +96,7 @@ function* encodeRequest(): Generator { yield put(send(createGetFlashStateRequest())); break; case BootloaderRequestActionType.Disconnect: - yield put(send(createDisconnectRequest())); + yield put(send(createDisconnectRequest(), /* withResponse */ false)); break; /* istanbul ignore next: should not be possible to reach */ default: From bab35e8c454a6e0490314fdd91fbbd07b6378694 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 12:09:22 -0600 Subject: [PATCH 03/18] move progress from status bar to firmware button --- src/actions/flash-firmware.ts | 73 ++++++++++++++++++++++-------- src/components/FlashButton.tsx | 11 +++-- src/components/OpenFileButton.tsx | 31 +++++++++++-- src/components/StatusBar.tsx | 27 +++-------- src/components/button-i18n.en.json | 5 +- src/components/button-i18n.ts | 5 +- src/components/status-bar.scss | 10 ---- src/reducers/firmware.ts | 38 ++++++++++++++++ src/reducers/index.ts | 8 ++-- src/reducers/status.ts | 22 --------- src/sagas/flash-firmware.ts | 12 +++-- src/variables.scss | 2 +- 12 files changed, 154 insertions(+), 90 deletions(-) create mode 100644 src/reducers/firmware.ts delete mode 100644 src/reducers/status.ts diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index c31b86cb..a55454f6 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -1,20 +1,23 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { Action } from 'redux'; +import { assert } from '../utils'; /** * High-level bootloader actions. */ export enum FlashFirmwareActionType { - /** - * Flash new firmware to the device. - */ + /** Request to flash new firmware to the device. */ FlashFirmware = 'flashFirmware.action.flashFirmware', - /** - * Firmware flash progress. - */ - Progress = 'flashFirmware.action.progress', + /** Flashing started. */ + DidStart = 'flashFirmware.action.didStart', + /** Firmware flash progress. */ + DidProgress = 'flashFirmware.action.didProgress', + /** Flashing finished successfully. */ + DidFinish = 'flashFirmware.action.didFinish', + /** Flashing firmware failed. */ + DidFailToFinish = 'flashFirmware.action.didFailToFinish', } /** @@ -33,19 +36,46 @@ export function flashFirmware(data?: ArrayBuffer): FlashFirmwareFlashAction { return { type: FlashFirmwareActionType.FlashFirmware, data }; } -export type FlashFirmwareProgressAction = Action & { - /** - * The number of bytes that have been flashed so far. - */ - complete: number; - /** - * The total number of bytes to be flashed. - */ - total: number; +/** Action that indicates flashing firmware started. */ +export type FlashFirmwareDidStartAction = Action; + +/** + * Action that indicates flashing firmware started. + * @param total The total number of bytes to be flashed. + */ +export function didStart(): FlashFirmwareDidStartAction { + return { type: FlashFirmwareActionType.DidStart }; +} + +/** Action that indicates current firmware flashing progress. */ +export type FlashFirmwareDidProgressAction = Action & { + /** The current progress (0 to 1). */ + value: number; }; -export function progress(complete: number, total: number): FlashFirmwareProgressAction { - return { type: FlashFirmwareActionType.Progress, complete, total }; +/** + * Action that indicates current firmware flashing progress. + * @param value The current progress (0 to 1). + */ +export function didProgress(value: number): FlashFirmwareDidProgressAction { + assert(value >= 0 && value <= 1, 'value out of range'); + return { type: FlashFirmwareActionType.DidProgress, value }; +} + +/** Action that indicates that flashing firmware completed successfully. */ +export type FlashFirmwareDidFinishAction = Action; + +/** Action that indicates that flashing firmware completed successfully. */ +export function didFinish(): FlashFirmwareDidFinishAction { + return { type: FlashFirmwareActionType.DidFinish }; +} + +/** Action that indicates that flashing failed. */ +export type FlashFirmwareDidFailToFinishAction = Action; + +/** Action that indicates that flashing failed. */ +export function didFailToFinish(): FlashFirmwareDidFailToFinishAction { + return { type: FlashFirmwareActionType.DidFailToFinish }; } /** @@ -53,4 +83,7 @@ export function progress(complete: number, total: number): FlashFirmwareProgress */ export type FlashFirmwareAction = | FlashFirmwareFlashAction - | FlashFirmwareProgressAction; + | FlashFirmwareDidStartAction + | FlashFirmwareDidProgressAction + | FlashFirmwareDidFinishAction + | FlashFirmwareDidFailToFinishAction; diff --git a/src/components/FlashButton.tsx b/src/components/FlashButton.tsx index fdfafc96..86d60e28 100644 --- a/src/components/FlashButton.tsx +++ b/src/components/FlashButton.tsx @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { connect } from 'react-redux'; import { Dispatch } from '../actions'; @@ -11,12 +11,18 @@ import OpenFileButton, { OpenFileButtonProps } from './OpenFileButton'; import { TooltipId } from './button-i18n'; import firmwareIcon from './images/firmware.svg'; -type StateProps = Pick; +type StateProps = Pick< + OpenFileButtonProps, + 'tooltip' | 'enabled' | 'showProgress' | 'progress' +>; type DispatchProps = Pick; type OwnProps = Pick; const mapStateToProps = (state: RootState): StateProps => ({ + tooltip: state.firmware.flashing ? TooltipId.FlashProgress : TooltipId.Flash, enabled: state.bootloader.connection === BootloaderConnectionState.Disconnected, + showProgress: state.firmware.flashing, + progress: state.firmware.progress === null ? undefined : state.firmware.progress, }); const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => ({ @@ -39,7 +45,6 @@ const mergeProps = ( ownProps: OwnProps, ): OpenFileButtonProps => ({ fileExtension: '.zip', - tooltip: TooltipId.Flash, icon: firmwareIcon, ...ownProps, ...stateProps, diff --git a/src/components/OpenFileButton.tsx b/src/components/OpenFileButton.tsx index 5b15d74f..e63c07e7 100644 --- a/src/components/OpenFileButton.tsx +++ b/src/components/OpenFileButton.tsx @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors -import { Button, Intent, Position, Tooltip } from '@blueprintjs/core'; +import { Button, Intent, Position, Spinner, Tooltip } from '@blueprintjs/core'; import { WithI18nProps, withI18n } from '@shopify/react-i18n'; import React from 'react'; import Dropzone, { FileRejection } from 'react-dropzone'; @@ -20,6 +20,10 @@ export interface OpenFileButtonProps { readonly icon: string; /** When true or undefined, the button is enabled. */ readonly enabled?: boolean; + /** Show progress spinner instead of icon. */ + readonly showProgress?: boolean; + /** The progress value (0 to 1) for the progress spinner. */ + readonly progress?: number; /** Callback that is called when a file has been selected and opened for reading. */ readonly onFile: (data: ArrayBuffer) => void; /** Callback that is called when a file has been rejected (e.g. bad file extension). */ @@ -80,7 +84,19 @@ class OpenFileButton extends React.Component { > {({ getRootProps, getInputProps }): JSX.Element => ( @@ -102,7 +118,14 @@ class OpenFileButton extends React.Component { : {})} > - {this.props.id} + {this.props.showProgress ? ( + + ) : ( + {this.props.id} + )} )} diff --git a/src/components/StatusBar.tsx b/src/components/StatusBar.tsx index aa330b5a..e49b305e 100644 --- a/src/components/StatusBar.tsx +++ b/src/components/StatusBar.tsx @@ -1,33 +1,20 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors -import { ProgressBar } from '@blueprintjs/core'; import React from 'react'; import { connect } from 'react-redux'; -import { RootState } from '../reducers'; import './status-bar.scss'; -type StateProps = { progress: number }; - -type StatusProps = StateProps; - -class StatusBar extends React.Component { +class StatusBar extends React.Component { render(): JSX.Element { return ( -
e.preventDefault()}> - -
+
e.preventDefault()} + >
); } } -const mapStateToProps = (state: RootState): StateProps => ({ - progress: state.status.progress, -}); - -export default connect(mapStateToProps)(StatusBar); +export default connect()(StatusBar); diff --git a/src/components/button-i18n.en.json b/src/components/button-i18n.en.json index fbeee51d..ec1f2a8d 100644 --- a/src/components/button-i18n.en.json +++ b/src/components/button-i18n.en.json @@ -8,6 +8,9 @@ "connect": { "tooltip": "Connect using Bluetooth" }, "disconnect": { "tooltip": "Disconnect Bluetooth" } }, - "flash": { "tooltip": "Install Pybricks firmware" }, + "flash": { + "action": { "tooltip": "Install Pybricks firmware" }, + "progress": { "tooltip": "Flashing… {percent}" } + }, "settings": { "tooltip": "Settings" } } diff --git a/src/components/button-i18n.ts b/src/components/button-i18n.ts index b34b8aac..2fafaadf 100644 --- a/src/components/button-i18n.ts +++ b/src/components/button-i18n.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors // File: components/button-i18n.ts // Button translation keys. @@ -9,7 +9,8 @@ export enum TooltipId { Run = 'run.tooltip', Stop = 'stop.tooltip', Repl = 'repl.tooltip', - Flash = 'flash.tooltip', + Flash = 'flash.action.tooltip', + FlashProgress = 'flash.progress.tooltip', BluetoothConnect = 'bluetooth.connect.tooltip', BluetoothDisconnect = 'bluetooth.disconnect.tooltip', Settings = 'settings.tooltip', diff --git a/src/components/status-bar.scss b/src/components/status-bar.scss index ea0b6469..3ac18ad6 100644 --- a/src/components/status-bar.scss +++ b/src/components/status-bar.scss @@ -14,13 +14,3 @@ display: flex; align-items: center; } - -.status-bar-item { - width: 25%; - margin-left: 10px; -} - -.#{$ns}-progress-bar.status-bar-item { - // override progress bar default gray1 backgound - background-color: $pt-app-background-color; -} diff --git a/src/reducers/firmware.ts b/src/reducers/firmware.ts new file mode 100644 index 00000000..fee94835 --- /dev/null +++ b/src/reducers/firmware.ts @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2021 The Pybricks Authors + +import { Reducer, combineReducers } from 'redux'; +import { Action } from '../actions'; +import { FlashFirmwareActionType } from '../actions/flash-firmware'; + +export interface FirmwareState { + /** The firmware is being erased/flashed right now. */ + flashing: boolean; + /** The current progress (0 to 1) or null for unknown (e.g erasing) */ + progress: number | null; +} + +const flashing: Reducer = (state = false, action) => { + switch (action.type) { + case FlashFirmwareActionType.DidStart: + return true; + case FlashFirmwareActionType.DidFinish: + case FlashFirmwareActionType.DidFailToFinish: + return false; + default: + return state; + } +}; + +const progress: Reducer = (state = null, action) => { + switch (action.type) { + case FlashFirmwareActionType.DidStart: + return null; + case FlashFirmwareActionType.DidProgress: + return action.value; + default: + return state; + } +}; + +export default combineReducers({ flashing, progress }); diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 25c65740..46aa9798 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -1,15 +1,15 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { combineReducers } from 'redux'; import app, { AppState } from './app'; import ble, { BleState } from './ble'; import bootloader, { BootloaderState } from './bootloader'; import editor, { EditorState } from './editor'; +import firmware, { FirmwareState } from './firmware'; import hub, { HubState } from './hub'; import license, { LicenseState } from './license'; import settings, { SettingsState } from './settings'; -import status, { StatusState } from './status'; import terminal, { TerminalState } from './terminal'; /** @@ -20,10 +20,10 @@ export interface RootState { readonly bootloader: BootloaderState; readonly ble: BleState; readonly editor: EditorState; + readonly firmware: FirmwareState; readonly hub: HubState; readonly license: LicenseState; readonly settings: SettingsState; - readonly status: StatusState; readonly terminal: TerminalState; } @@ -32,9 +32,9 @@ export default combineReducers({ bootloader, ble, editor, + firmware, hub, license, settings, - status, terminal, }); diff --git a/src/reducers/status.ts b/src/reducers/status.ts deleted file mode 100644 index d882116c..00000000 --- a/src/reducers/status.ts +++ /dev/null @@ -1,22 +0,0 @@ -// SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors - -import { Reducer } from 'react'; -import { combineReducers } from 'redux'; -import { Action } from '../actions'; -import { FlashFirmwareActionType } from '../actions/flash-firmware'; - -const progress: Reducer = (state = -1, action) => { - switch (action.type) { - case FlashFirmwareActionType.Progress: - return action.complete / action.total; - default: - return state; - } -}; - -export interface StatusState { - readonly progress: number; -} - -export default combineReducers({ progress }); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 7ef58344..bb095420 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -21,7 +21,9 @@ import { Action } from '../actions'; import { FlashFirmwareActionType, FlashFirmwareFlashAction, - progress, + didFinish, + didProgress, + didStart, } from '../actions/flash-firmware'; import { BootloaderChecksumRequestAction, @@ -265,6 +267,8 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } } + yield put(didStart()); + const eraseAction = (yield put(eraseRequest())) as BootloaderEraseRequestAction; const [, erase] = (yield all([ waitForDidSend(eraseAction.id), @@ -301,7 +305,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { )) as BootloaderProgramRequestAction; yield waitForDidSend(programAction.id); - yield put(progress(offset, firmware.length)); + yield put(didProgress(offset / firmware.length)); // we don't want to request checksum if this is the last packet since // the bootloader will send a response to the program request already. @@ -344,11 +348,13 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { throw Error("Didn't flash all bytes"); } - yield put(progress(firmware.length, firmware.length)); + yield put(didProgress(1)); // this will cause the remote device to disconnect and reboot const rebootAction = (yield put(rebootRequest())) as BootloaderRebootRequestAction; yield waitForDidSend(rebootAction.id); + + yield put(didFinish()); } export default function* (): Generator { diff --git a/src/variables.scss b/src/variables.scss index 17189444..80abb973 100644 --- a/src/variables.scss +++ b/src/variables.scss @@ -11,7 +11,7 @@ $pt-font-size-large: $pt-grid-size * 1.8; $pt-font-size-small: $pt-grid-size * 1.4; $pt-navbar-height: 72px; -$pb-status-bar-height: 3vh; +$pb-status-bar-height: 24px; $pb-pybricks-blue: #0088ce; $pt-app-background-color: #e8e8e8; From 647985906db854da4fb4cb1a46063dc288a0a43d Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 14:17:27 -0600 Subject: [PATCH 04/18] add test for app sagas --- src/sagas/app.test.ts | 37 +++++++++++++++++++++++++++++++++++++ src/sagas/app.ts | 2 -- 2 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 src/sagas/app.test.ts diff --git a/src/sagas/app.test.ts b/src/sagas/app.test.ts new file mode 100644 index 00000000..44fa538e --- /dev/null +++ b/src/sagas/app.test.ts @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2021 The Pybricks Authors + +import { AsyncSaga, delay } from '../../test'; +import { reload } from '../actions/app'; +import app from './app'; + +test('reload', async () => { + const saga = new AsyncSaga(app); + + // mock registration as if service worker was register on app startup + const registration: Partial = { + unregister: jest.fn(), + }; + + // @ts-expect-error: navigator.serviceWorker is not implemented in JSDOM + navigator.serviceWorker = { + getRegistrations: jest.fn().mockResolvedValue([registration]), + }; + + // @ts-expect-error: JSDOM implementation of location.reload() causes error + delete window.location; + // @ts-expect-error: JSDOM implementation of location.reload() causes error + window.location = { + reload: jest.fn(), + }; + + saga.put(reload()); + + // yield to allow generators to complete + await delay(0); + + expect(registration.unregister).toHaveBeenCalled(); + expect(location.reload).toHaveBeenCalled(); + + await saga.end(); +}); diff --git a/src/sagas/app.ts b/src/sagas/app.ts index ceae5066..1a4b2041 100644 --- a/src/sagas/app.ts +++ b/src/sagas/app.ts @@ -5,8 +5,6 @@ import { call, takeEvery } from 'redux-saga/effects'; import { AppActionType } from '../actions/app'; function* reload(): Generator { - console.log('reload'); - // unregister the service worker so that when the page reloads, it uses // the new version const registrations = (yield call(() => From 904de515c0452636e27916043e2411b8ef72a5b8 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 17:20:20 -0600 Subject: [PATCH 05/18] fix missing newlines in compiler error --- craco.config.js | 1 - package.json | 2 +- src/actions/mpy.ts | 4 ++-- src/sagas/__snapshots__/mpy.test.ts.snap | 9 +++++++++ src/sagas/flash-firmware.ts | 2 +- src/sagas/mpy.test.ts | 2 +- src/sagas/notification.test.ts | 2 +- src/sagas/notification.ts | 4 +++- yarn.lock | 8 ++++---- 9 files changed, 22 insertions(+), 12 deletions(-) create mode 100644 src/sagas/__snapshots__/mpy.test.ts.snap diff --git a/craco.config.js b/craco.config.js index 4da4d1dc..05312466 100644 --- a/craco.config.js +++ b/craco.config.js @@ -111,7 +111,6 @@ SOFTWARE.`; const licenseTextOverrides = { '@pybricks/firmware': pybricksLicense, - '@pybricks/mpy-cross-v5': pybricksLicense, '@shopify/dates': shopifyLicense, '@shopify/decorators': shopifyLicense, '@shopify/function-enhancers': shopifyLicense, diff --git a/package.json b/package.json index aba96565..892bd861 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "@blueprintjs/core": "^3.36.0", "@craco/craco": "^6.0.0", "@pybricks/firmware": "4.4.0", - "@pybricks/mpy-cross-v5": "^1.2.0", + "@pybricks/mpy-cross-v5": "^2.0.0", "@shopify/react-i18n": "^5.2.0", "@testing-library/dom": "^7.29.2", "@testing-library/jest-dom": "^5.11.8", diff --git a/src/actions/mpy.ts b/src/actions/mpy.ts index 7f34be36..7663f295 100644 --- a/src/actions/mpy.ts +++ b/src/actions/mpy.ts @@ -32,10 +32,10 @@ export function didCompile(data: Uint8Array): MpyDidCompileAction { export type MpyDidFailToCompileAction = Action & { /** Error output. */ - readonly err: string; + readonly err: string[]; }; -export function didFailToCompile(err: string): MpyDidFailToCompileAction { +export function didFailToCompile(err: string[]): MpyDidFailToCompileAction { return { type: MpyActionType.DidFailToCompile, err }; } diff --git a/src/sagas/__snapshots__/mpy.test.ts.snap b/src/sagas/__snapshots__/mpy.test.ts.snap new file mode 100644 index 00000000..1e4167cd --- /dev/null +++ b/src/sagas/__snapshots__/mpy.test.ts.snap @@ -0,0 +1,9 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`compiler error works 1`] = ` +Array [ + "Traceback (most recent call last):", + " File \\"main.py\\", line 1", + "SyntaxError: invalid syntax", +] +`; diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index bb095420..f80d6c8d 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -146,7 +146,7 @@ function* loadFirmware( ])) as [MpyDidCompileAction, MpyDidFailToCompileAction]; if (mpyFail) { - throw Error(mpyFail.err); + throw Error(mpyFail.err.join('\n')); } // compute offset for checksum - must be aligned to 4-byte boundary diff --git a/src/sagas/mpy.test.ts b/src/sagas/mpy.test.ts index b4d81493..141a0f62 100644 --- a/src/sagas/mpy.test.ts +++ b/src/sagas/mpy.test.ts @@ -37,7 +37,7 @@ test('compiler error works', async () => { const action = await saga.take(); expect(action.type).toBe(MpyActionType.DidFailToCompile); const { err } = action as MpyDidFailToCompileAction; - expect(err).toContain('SyntaxError'); + expect(err).toMatchSnapshot(); await saga.end(); }); diff --git a/src/sagas/notification.test.ts b/src/sagas/notification.test.ts index 86212272..5ffd07f1 100644 --- a/src/sagas/notification.test.ts +++ b/src/sagas/notification.test.ts @@ -31,7 +31,7 @@ test.each([ bootloaderDidFailToConnect(BootloaderConnectionFailureReason.NoWebBluetooth), bootloaderDidFailToConnect(BootloaderConnectionFailureReason.GattServiceNotFound), storageChanged('test'), - didFailToCompile('reason'), + didFailToCompile(['reason']), add('warning', 'message'), add('error', 'message', 'url'), didUpdate({} as ServiceWorkerRegistration), diff --git a/src/sagas/notification.ts b/src/sagas/notification.ts index 47ee3e5c..d386583b 100644 --- a/src/sagas/notification.ts +++ b/src/sagas/notification.ts @@ -220,7 +220,9 @@ function* dismissCompilerError(): Generator { } function* showCompilerError(action: MpyDidFailToCompileAction): Generator { - yield* showSingleton(Level.Error, MessageId.MpyError, { errorMessage: action.err }); + yield* showSingleton(Level.Error, MessageId.MpyError, { + errorMessage: React.createElement('pre', undefined, action.err.join('\n')), + }); } function* addNotification(action: NotificationAddAction): Generator { diff --git a/yarn.lock b/yarn.lock index 4018380f..c6a9ebcc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1462,10 +1462,10 @@ dependencies: jszip "^3.5.0" -"@pybricks/mpy-cross-v5@^1.2.0": - version "1.2.0" - resolved "https://registry.yarnpkg.com/@pybricks/mpy-cross-v5/-/mpy-cross-v5-1.2.0.tgz#29cbd949c579c0551792d2fd1cd777d193aa925c" - integrity sha512-A1FXGP0teuZa3tPBTz9niCdEol4Ld6jOABl9FkglmutGWO99eLwuyYibh8wQqHNKwXRHrzCJmo/W+/42HmSjog== +"@pybricks/mpy-cross-v5@^2.0.0": + version "2.0.0" + resolved "https://registry.yarnpkg.com/@pybricks/mpy-cross-v5/-/mpy-cross-v5-2.0.0.tgz#9d64e1dedda0a7a028117f510ce5365f504df400" + integrity sha512-s3B+0tsXRHF3Y+FfOeDkNLoM+dwwT8P05UowVYPQvTV/UjhBGwruUdEoG/cYz2cFrqx+VFF8gpgmpw+5aA7/hQ== "@redux-saga/core@^1.1.3": version "1.1.3" From 2659c064fa5a9ff0c8f7f859e410ad6596ed667f Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 18:09:22 -0600 Subject: [PATCH 06/18] make bootloader failure reasons more strongly typed This way we don't have to do extra null checks elsewhere. --- src/actions/lwp3-bootloader.ts | 55 ++++++++++++++++++++++++++++------ src/sagas/error-log.test.ts | 6 +--- src/sagas/error-log.ts | 2 -- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/actions/lwp3-bootloader.ts b/src/actions/lwp3-bootloader.ts index ffe0b890..15e43a3c 100644 --- a/src/actions/lwp3-bootloader.ts +++ b/src/actions/lwp3-bootloader.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { Action } from 'redux'; import { @@ -64,26 +64,63 @@ export function didConnect(): BootloaderConnectionDidConnectAction { * Possible reasons a device could fail to connect. */ export enum BootloaderConnectionFailureReason { - /** The reason is not known */ - Unknown = 'unknown', - /** The connection was canceled */ - Canceled = 'canceled', /** Web Bluetooth is not available */ NoWebBluetooth = 'no-web-bluetooth', /** Connected but failed to find the bootloader GATT service */ GattServiceNotFound = 'gatt-service-not-found', + /** The connection was canceled */ + Canceled = 'canceled', + /** The reason is not known */ + Unknown = 'unknown', } -export type BootloaderConnectionDidFailToConnectAction = Action & { - reason: BootloaderConnectionFailureReason; - err?: Error; +type Reason = { + reason: T; }; +export type BootloaderConnectionFailToConnectNoWebBluetoothReason = Reason; + +export type BootloaderConnectionFailToConnectGattServiceNotFoundReason = Reason; + +export type BootloaderConnectionFailToConnectCanceledReason = Reason; + +export type BootloaderConnectionFailToConnectUnknownReason = Reason & { + err: Error; +}; + +export type BootloaderConnectionDidFailToConnectReason = + | BootloaderConnectionFailToConnectNoWebBluetoothReason + | BootloaderConnectionFailToConnectGattServiceNotFoundReason + | BootloaderConnectionFailToConnectCanceledReason + | BootloaderConnectionFailToConnectUnknownReason; + +export type BootloaderConnectionDidFailToConnectAction = Action & + BootloaderConnectionDidFailToConnectReason; + +export function didFailToConnect( + reason: Exclude< + BootloaderConnectionFailureReason, + BootloaderConnectionFailureReason.Unknown + >, +): BootloaderConnectionDidFailToConnectAction; + +export function didFailToConnect( + reason: BootloaderConnectionFailureReason.Unknown, + err: Error, +): BootloaderConnectionDidFailToConnectAction; + export function didFailToConnect( reason: BootloaderConnectionFailureReason, err?: Error, ): BootloaderConnectionDidFailToConnectAction { - return { type: BootloaderConnectionActionType.DidFailToConnect, reason, err }; + if (reason === BootloaderConnectionFailureReason.Unknown) { + return { + type: BootloaderConnectionActionType.DidFailToConnect, + reason, + err, + }; + } + return { type: BootloaderConnectionActionType.DidFailToConnect, reason }; } export type BootloaderConnectionDidErrorAction = Action & { diff --git a/src/sagas/error-log.test.ts b/src/sagas/error-log.test.ts index b0bd8f25..5bc961ea 100644 --- a/src/sagas/error-log.test.ts +++ b/src/sagas/error-log.test.ts @@ -49,12 +49,8 @@ test('bleDataDidFailToWrite', async () => { test('bootloaderDidFailToConnect', async () => { const saga = new AsyncSaga(errorLog); - console.debug = jest.fn(); - saga.put(didFailToConnect(BootloaderConnectionFailureReason.Canceled)); - expect(console.debug).toHaveBeenCalledTimes(1); - console.error = jest.fn(); - saga.put(didFailToConnect(BootloaderConnectionFailureReason.Unknown)); + saga.put(didFailToConnect(BootloaderConnectionFailureReason.Unknown, {})); expect(console.error).toHaveBeenCalledTimes(1); await saga.end(); diff --git a/src/sagas/error-log.ts b/src/sagas/error-log.ts index d7e96dbe..5353c020 100644 --- a/src/sagas/error-log.ts +++ b/src/sagas/error-log.ts @@ -31,8 +31,6 @@ function bootloaderDidFailToConnect( ): void { if (action.reason === BootloaderConnectionFailureReason.Unknown) { console.error(action.err); - } else { - console.debug(action.err); } } From 91938e34d96452d9de46590dd913fb659d25c855 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 19 Jan 2021 18:49:32 -0600 Subject: [PATCH 07/18] add special notification for unexpected errors Since these errors are not expected, we want to make it really easy to report them. --- src/components/Notification.tsx | 6 +-- .../UnexpectedErrorNotification.tsx | 54 +++++++++++++++++++ src/components/notification-i18n.en.json | 4 +- src/components/notification-i18n.ts | 4 +- src/sagas/notification.test.ts | 4 +- src/sagas/notification.ts | 16 +++++- 6 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 src/components/UnexpectedErrorNotification.tsx diff --git a/src/components/Notification.tsx b/src/components/Notification.tsx index 7a2edbee..3976044f 100644 --- a/src/components/Notification.tsx +++ b/src/components/Notification.tsx @@ -1,13 +1,13 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2021 The Pybricks Authors + +// provides translation for notification text import { Replacements, useI18n } from '@shopify/react-i18n'; import React from 'react'; import { MessageId } from './notification-i18n'; import en from './notification-i18n.en.json'; -// provides translation for notification text - type OwnProps = { messageId: MessageId; replacements?: Replacements; diff --git a/src/components/UnexpectedErrorNotification.tsx b/src/components/UnexpectedErrorNotification.tsx new file mode 100644 index 00000000..8f9957ea --- /dev/null +++ b/src/components/UnexpectedErrorNotification.tsx @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2021 The Pybricks Authors + +// Provides special notification contents for unexpected errors. + +import { AnchorButton, Button, ButtonGroup, Intent } from '@blueprintjs/core'; +import { useI18n } from '@shopify/react-i18n'; +import React from 'react'; +import { MessageId } from './notification-i18n'; +import en from './notification-i18n.en.json'; + +type OwnProps = { + messageId: MessageId; + err: Error; +}; + +export default function UnexpectedErrorNotification(props: OwnProps): JSX.Element { + const [i18n] = useI18n({ + id: 'notification', + translations: { en }, + fallback: en, + }); + const { messageId, err } = props; + return ( + <> +

{i18n.translate(messageId, { errorMessage: err.message })}

+
+ + + + {i18n.translate(MessageId.ReportBug)} + + +
+ + ); +} diff --git a/src/components/notification-i18n.en.json b/src/components/notification-i18n.en.json index 90dc0233..d0ae6e09 100644 --- a/src/components/notification-i18n.en.json +++ b/src/components/notification-i18n.en.json @@ -1,9 +1,11 @@ { + "copyErrorMessage": "Copy Error Message", + "reportBug": "Report Bug", "ble": { "gattPermission": "The web browser did not give permission to use Bluetooth Low Energy", "gattServiceNotFound": "Connected to hub but failed to get {serviceName} service. Try removing the \"{hubName}\" device in your OS Bluetooth settings, then try again.", "noWebBluetooth": "This web browser does not support Web Bluetooth or it is not enabled.", - "connectFailed": "Unexpected error while trying to connect. Check console log and report the error." + "unexpectedError": "Unexpected error while trying to connect: {errorMessage}" }, "editor": { "programChanged": { diff --git a/src/components/notification-i18n.ts b/src/components/notification-i18n.ts index e5948e18..48414268 100644 --- a/src/components/notification-i18n.ts +++ b/src/components/notification-i18n.ts @@ -4,7 +4,9 @@ // Notification translation keys. export enum MessageId { - BleConnectFailed = 'ble.connectFailed', + CopyErrorMessage = 'copyErrorMessage', + ReportBug = 'reportBug', + BleUnexpectedError = 'ble.unexpectedError', BleGattPermission = 'ble.gattPermission', BleGattServiceNotFound = 'ble.gattServiceNotFound', BleNoWebBluetooth = 'ble.noWebBluetooth', diff --git a/src/sagas/notification.test.ts b/src/sagas/notification.test.ts index 5ffd07f1..9b529eb9 100644 --- a/src/sagas/notification.test.ts +++ b/src/sagas/notification.test.ts @@ -27,7 +27,9 @@ test.each([ reason: BleDeviceFailToConnectReasonType.Unknown, err: { name: 'test', message: 'unknown' }, }), - bootloaderDidFailToConnect(BootloaderConnectionFailureReason.Unknown), + bootloaderDidFailToConnect(BootloaderConnectionFailureReason.Unknown, { + message: 'test', + }), bootloaderDidFailToConnect(BootloaderConnectionFailureReason.NoWebBluetooth), bootloaderDidFailToConnect(BootloaderConnectionFailureReason.GattServiceNotFound), storageChanged('test'), diff --git a/src/sagas/notification.ts b/src/sagas/notification.ts index d386583b..687e0e59 100644 --- a/src/sagas/notification.ts +++ b/src/sagas/notification.ts @@ -30,6 +30,7 @@ import { MpyActionType, MpyDidFailToCompileAction } from '../actions/mpy'; import { NotificationActionType, NotificationAddAction } from '../actions/notification'; import { ServiceWorkerActionType } from '../actions/service-worker'; import Notification from '../components/Notification'; +import UnexpectedErrorNotification from '../components/UnexpectedErrorNotification'; import { MessageId } from '../components/notification-i18n'; import { appName } from '../settings/ui'; @@ -140,6 +141,17 @@ function* showSingleton( ); } +/** Shows a special notification for unexpected errors. */ +function* showUnexpectedError(messageId: MessageId, err: Error): Generator { + const { toaster } = (yield getContext('notification')) as NotificationContext; + toaster.show({ + intent: mapIntent(Level.Error), + icon: mapIcon(Level.Error), + message: React.createElement(UnexpectedErrorNotification, { messageId, err }), + timeout: 0, + }); +} + function* showBleDeviceDidFailToConnectError( action: BleDeviceDidFailToConnectAction, ): Generator { @@ -165,7 +177,7 @@ function* showBleDeviceDidFailToConnectError( ); break; case BleDeviceFailToConnectReasonType.Unknown: - yield* showSingleton(Level.Error, MessageId.BleConnectFailed); + yield* showUnexpectedError(MessageId.BleUnexpectedError, action.err); break; } } @@ -191,7 +203,7 @@ function* showBootloaderDidFailToConnectError( ); break; case BootloaderConnectionFailureReason.Unknown: - yield* showSingleton(Level.Error, MessageId.BleConnectFailed); + yield* showUnexpectedError(MessageId.BleUnexpectedError, action.err); break; } } From 0333fa51c0d6a9c8dcf579817e9c494c55dd8e5a Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 20 Jan 2021 12:38:56 -0600 Subject: [PATCH 08/18] add developer settings with pseudolocalization This is currently broken due to https://github.com/Shopify/quilt/pull/1725 --- src/components/SettingsDrawer.tsx | 10 ++++++++++ src/index.tsx | 12 ++++-------- src/settings/i18n.ts | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 src/settings/i18n.ts diff --git a/src/components/SettingsDrawer.tsx b/src/components/SettingsDrawer.tsx index 30e482ed..b24da192 100644 --- a/src/components/SettingsDrawer.tsx +++ b/src/components/SettingsDrawer.tsx @@ -19,6 +19,7 @@ import { Action, Dispatch } from '../actions'; import { closeSettings, openAboutDialog } from '../actions/app'; import { setBoolean } from '../actions/settings'; import { RootState } from '../reducers'; +import { pseudolocalize } from '../settings/i18n'; import { pybricksBugReportsUrl, pybricksGitterUrl, @@ -206,6 +207,15 @@ class SettingsDrawer extends React.PureComponent { + {process.env.NODE_ENV === 'development' && ( + + pseudolocalize(!i18n.pseudolocalize)} + label="Pseudolocalize" + /> + + )} diff --git a/src/index.tsx b/src/index.tsx index ba9e496e..7dc552bc 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -2,7 +2,7 @@ // Copyright (c) 2020-2021 The Pybricks Authors import { Classes, ResizeSensor } from '@blueprintjs/core'; -import { I18nContext, I18nManager } from '@shopify/react-i18n'; +import { I18nContext } from '@shopify/react-i18n'; import React from 'react'; import ReactDOM from 'react-dom'; import { Provider } from 'react-redux'; @@ -17,13 +17,9 @@ import rootReducer from './reducers'; import reportWebVitals from './reportWebVitals'; import rootSaga from './sagas'; import * as serviceWorkerRegistration from './serviceWorkerRegistration'; +import { i18nManager } from './settings/i18n'; -const i18n = new I18nManager({ - locale: 'en', - onError: (err): void => console.error(err), -}); - -const toaster = I18nToaster.create(i18n); +const toaster = I18nToaster.create(i18nManager); const sagaMiddleware = createSagaMiddleware({ context: { notification: { toaster } } }); // TODO: add runtime option or filter - logger affects firmware flash performance @@ -58,7 +54,7 @@ sagaMiddleware.run(rootSaga); ReactDOM.render( - + {/* This is a hack for correctly sizing to view height on mobile when not running in fullscreen mode. */} {/* https://css-tricks.com/the-trick-to-viewport-units-on-mobile/ */} console.error(err), +}); + +/** Enables or disables pseudolocalization for development. */ +export function pseudolocalize(pseudolocalize: boolean): void { + i18nManager.update({ ...i18nManager.details, pseudolocalize }); +} From f186e6414dfb2e1c2f073d4bbcfac0587f930cfd Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 20 Jan 2021 17:04:51 -0600 Subject: [PATCH 09/18] decouple message id counter from actions Actions should be pure functions for ease of testing. --- src/actions/ble-uart.ts | 9 ++---- src/actions/lwp3-bootloader.ts | 37 +++++++++++----------- src/index.tsx | 9 +++++- src/sagas/flash-firmware.ts | 29 ++++++++++++----- src/sagas/hub.test.ts | 7 ++-- src/sagas/hub.ts | 17 +++++++--- src/sagas/lwp3-bootloader-protocol.test.ts | 25 ++++++++------- src/sagas/terminal.test.ts | 31 +++++++++--------- src/sagas/terminal.ts | 7 ++-- 9 files changed, 101 insertions(+), 70 deletions(-) diff --git a/src/actions/ble-uart.ts b/src/actions/ble-uart.ts index 56e92107..32eb07db 100644 --- a/src/actions/ble-uart.ts +++ b/src/actions/ble-uart.ts @@ -1,9 +1,8 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors // actions/ble-uart.ts: Actions for Bluetooth Low Energy nRF UART service import { Action } from 'redux'; -import { createCountFunc } from '../utils/iter'; /** * BLE nRF UART service actions types. @@ -27,15 +26,13 @@ export enum BleUartActionType { Notify = 'ble.data.action.receive', } -const nextId = createCountFunc(); - export type BleUartWriteAction = Action & { id: number; value: Uint8Array; }; -export function write(value: Uint8Array): BleUartWriteAction { - return { type: BleUartActionType.Write, id: nextId(), value }; +export function write(id: number, value: Uint8Array): BleUartWriteAction { + return { type: BleUartActionType.Write, id, value }; } export type BleUartDidWriteAction = Action & { diff --git a/src/actions/lwp3-bootloader.ts b/src/actions/lwp3-bootloader.ts index 15e43a3c..6bcfbe6a 100644 --- a/src/actions/lwp3-bootloader.ts +++ b/src/actions/lwp3-bootloader.ts @@ -8,7 +8,6 @@ import { ProtectionLevel, Result, } from '../protocols/lwp3-bootloader'; -import { createCountFunc } from '../utils/iter'; /** * Bootloader BLE connection actions. @@ -193,8 +192,6 @@ export enum BootloaderRequestActionType { Disconnect = 'bootloader.action.request.disconnect', } -const nextRequestId = createCountFunc(); - type BaseBootloaderRequestAction = Action & { /** * Unique identifier for this action. @@ -210,8 +207,8 @@ export type BootloaderEraseRequestAction = BaseBootloaderRequestAction false }); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index f80d6c8d..85191478 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -11,6 +11,7 @@ import { all, call, delay, + getContext, put, race, select, @@ -221,7 +222,11 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { return; } - const infoAction = (yield put(infoRequest())) as BootloaderInfoRequestAction; + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + + const infoAction = (yield put( + infoRequest(nextMessageId()), + )) as BootloaderInfoRequestAction; const [, info] = (yield all([ waitForDidSend(infoAction.id), waitForResponse(BootloaderResponseActionType.Info), @@ -243,7 +248,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { "Sorry, we don't have firmware for this hub yet.", ), ); - yield put(disconnectRequest()); + yield put(disconnectRequest(nextMessageId())); return; } @@ -251,7 +256,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { if (!response.ok) { yield put(notification.add('error', 'Failed to fetch firmware.')); const disconnectAction = (yield put( - disconnectRequest(), + disconnectRequest(nextMessageId()), )) as BootloaderDisconnectRequestAction; yield waitForDidSend(disconnectAction.id); return; @@ -269,7 +274,9 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { yield put(didStart()); - const eraseAction = (yield put(eraseRequest())) as BootloaderEraseRequestAction; + const eraseAction = (yield put( + eraseRequest(nextMessageId()), + )) as BootloaderEraseRequestAction; const [, erase] = (yield all([ waitForDidSend(eraseAction.id), waitForResponse(BootloaderResponseActionType.Erase, 5000), @@ -280,7 +287,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } const initAction = (yield put( - initRequest(firmware.length), + initRequest(nextMessageId(), firmware.length), )) as BootloaderInitRequestAction; const [, init] = (yield all([ waitForDidSend(initAction.id), @@ -301,7 +308,11 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { for (let offset = 0; ; ) { const payload = firmware.slice(offset, offset + maxDataSize); const programAction = (yield put( - programRequest(info[0].startAddress + offset, payload.buffer), + programRequest( + nextMessageId(), + info[0].startAddress + offset, + payload.buffer, + ), )) as BootloaderProgramRequestAction; yield waitForDidSend(programAction.id); @@ -320,7 +331,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // the hub is not known and could vary by device. if (++count % 10 === 0) { const checksumAction = (yield put( - checksumRequest(), + checksumRequest(nextMessageId()), )) as BootloaderChecksumRequestAction; const [, checksum] = (yield all([ waitForDidSend(checksumAction.id), @@ -351,7 +362,9 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { yield put(didProgress(1)); // this will cause the remote device to disconnect and reboot - const rebootAction = (yield put(rebootRequest())) as BootloaderRebootRequestAction; + const rebootAction = (yield put( + rebootRequest(nextMessageId()), + )) as BootloaderRebootRequestAction; yield waitForDidSend(rebootAction.id); yield put(didFinish()); diff --git a/src/sagas/hub.test.ts b/src/sagas/hub.test.ts index 380ed6a8..9af02713 100644 --- a/src/sagas/hub.test.ts +++ b/src/sagas/hub.test.ts @@ -15,13 +15,14 @@ import { stop, } from '../actions/hub'; import { MpyActionType, didCompile } from '../actions/mpy'; +import { createCountFunc } from '../utils/iter'; import hub from './hub'; jest.mock('ace-builds'); describe('downloadAndRun', () => { test('no errors', async () => { - const saga = new AsyncSaga(hub); + const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); const mockEditor = mock(); saga.setState({ editor: { current: mockEditor } }); @@ -75,7 +76,7 @@ describe('downloadAndRun', () => { }); test('repl', async () => { - const saga = new AsyncSaga(hub); + const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); saga.put(repl()); @@ -86,7 +87,7 @@ test('repl', async () => { }); test('stop', async () => { - const saga = new AsyncSaga(hub); + const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); saga.put(stop()); diff --git a/src/sagas/hub.ts b/src/sagas/hub.ts index 91e17530..ba1ba949 100644 --- a/src/sagas/hub.ts +++ b/src/sagas/hub.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { Ace } from 'ace-builds'; import { Channel } from 'redux-saga'; @@ -7,6 +7,7 @@ import { RaceEffect, TakeEffect, actionChannel, + getContext, put, race, select, @@ -80,11 +81,15 @@ function* downloadAndRun(_action: HubDownloadAndRunAction): Generator { HubMessageActionType.Checksum, )) as Channel; + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + // first send payload size as big-endian 32-bit integer const sizeBuf = new Uint8Array(4); const sizeView = new DataView(sizeBuf.buffer); sizeView.setUint32(0, mpy.data.byteLength, true); - const writeAction = (yield put(write(sizeBuf))) as BleUartWriteAction; + const writeAction = (yield put( + write(nextMessageId(), sizeBuf), + )) as BleUartWriteAction; const [, didFailToWrite] = (yield waitForWrite(writeAction.id)) as [ BleUartDidWriteAction, BleUartDidFailToWriteAction, @@ -113,7 +118,7 @@ function* downloadAndRun(_action: HubDownloadAndRunAction): Generator { // we can actually only write 20 bytes at a time for (let j = 0; j < chunk.length; j += SafeTxCharLength) { const writeAction = (yield put( - write(chunk.slice(j, j + SafeTxCharLength)), + write(nextMessageId(), chunk.slice(j, j + SafeTxCharLength)), )) as BleUartWriteAction; const [, didFailToWrite] = (yield waitForWrite(writeAction.id)) as [ BleUartDidWriteAction, @@ -146,14 +151,16 @@ function* downloadAndRun(_action: HubDownloadAndRunAction): Generator { const startReplCommand = new Uint8Array([0x20, 0x20, 0x20, 0x20]); function* startRepl(_action: HubReplAction): Generator { - yield put(write(startReplCommand)); + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + yield put(write(nextMessageId(), startReplCommand)); } // CTRL+C, CTRL+C, CTRL+D const stopCommand = new Uint8Array([0x03, 0x03, 0x04]); function* stop(_action: HubStopAction): Generator { - yield put(write(stopCommand)); + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + yield put(write(nextMessageId(), stopCommand)); } export default function* (): Generator { diff --git a/src/sagas/lwp3-bootloader-protocol.test.ts b/src/sagas/lwp3-bootloader-protocol.test.ts index 8e59a032..5805928f 100644 --- a/src/sagas/lwp3-bootloader-protocol.test.ts +++ b/src/sagas/lwp3-bootloader-protocol.test.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors // File: sagas/lwp3-bootloader-protocol.test.ts import { AsyncSaga } from '../../test'; @@ -40,7 +40,7 @@ describe('message encoder', () => { test.each([ [ 'erase', - eraseRequest(), + eraseRequest(0), [ 0x11, // erase command ], @@ -48,6 +48,7 @@ describe('message encoder', () => { [ 'program', programRequest( + 1, 0x08005000, new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]).buffer, ), @@ -76,14 +77,14 @@ describe('message encoder', () => { ], [ 'reboot', - rebootRequest(), + rebootRequest(2), [ 0x33, // reboot command ], ], [ 'init', - initRequest(100000), + initRequest(3, 100000), [ 0x44, // init command 0xa0, // size LSB @@ -94,28 +95,28 @@ describe('message encoder', () => { ], [ 'info', - infoRequest(), + infoRequest(4), [ 0x55, // info command ], ], [ 'checksum', - checksumRequest(), + checksumRequest(5), [ 0x66, // checksum command ], ], [ 'state', - stateRequest(), + stateRequest(6), [ 0x77, // state command ], ], [ 'disconnect', - disconnectRequest(), + disconnectRequest(7), [ 0x88, // disconnect command ], @@ -143,10 +144,10 @@ describe('message encoder', () => { const saga = new AsyncSaga(bootloader); // we send 4 requests - saga.put({ ...eraseRequest(), id: 0 }); - saga.put({ ...eraseRequest(), id: 1 }); - saga.put({ ...eraseRequest(), id: 2 }); - saga.put({ ...eraseRequest(), id: 3 }); + saga.put(eraseRequest(0)); + saga.put(eraseRequest(1)); + saga.put(eraseRequest(2)); + saga.put(eraseRequest(3)); // but only two didSend action meaning only the first two completed saga.put(didSend()); diff --git a/src/sagas/terminal.test.ts b/src/sagas/terminal.test.ts index 6673b7df..abefe1d2 100644 --- a/src/sagas/terminal.test.ts +++ b/src/sagas/terminal.test.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { AsyncSaga, delay } from '../../test'; @@ -25,11 +25,12 @@ import { sendData, } from '../actions/terminal'; import { HubRuntimeState } from '../reducers/hub'; +import { createCountFunc } from '../utils/iter'; import terminal from './terminal'; describe('Data receiver filters out hub status', () => { test('normal message - no status', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // sending ASCII space character saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -43,7 +44,7 @@ describe('Data receiver filters out hub status', () => { }); test('checksum message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.setState({ hub: { runtime: HubRuntimeState.Loading } }); saga.put(notify(new DataView(new Uint8Array([0xaa]).buffer))); @@ -56,7 +57,7 @@ describe('Data receiver filters out hub status', () => { }); test('idle message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> IDLE' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -88,7 +89,7 @@ describe('Data receiver filters out hub status', () => { }); test('idle message with extra text', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> IDLE1' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -132,7 +133,7 @@ describe('Data receiver filters out hub status', () => { }); test('error message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> ERROR' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -165,7 +166,7 @@ describe('Data receiver filters out hub status', () => { }); test('error message with extra text', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> ERROR1' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -210,7 +211,7 @@ describe('Data receiver filters out hub status', () => { }); test('running message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> ERROR' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -245,7 +246,7 @@ describe('Data receiver filters out hub status', () => { }); test('running message with extra text', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> RUNNING1' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -293,7 +294,7 @@ describe('Data receiver filters out hub status', () => { }); test('Terminal data source responds to send data actions', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(didStart()); const dataSourceAction = await saga.take(); @@ -320,7 +321,7 @@ describe('Terminal data source responds to receive data actions', () => { const expected = new Uint8Array([0x74, 0x65, 0x73, 0x74, 0x31, 0x32, 0x33, 0x34]); test('basic function works', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); @@ -332,7 +333,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('messages are queued until previous has completed', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); await delay(50); // without delay, messages are combined @@ -360,7 +361,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('messages are queued until previous has failed', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); await delay(50); // without delay, messages are combined @@ -390,7 +391,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('small messages are combined', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); saga.put(receiveData('test1234')); @@ -405,7 +406,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('long messages are split', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('012345678901234567890123456789')); diff --git a/src/sagas/terminal.ts b/src/sagas/terminal.ts index 141ed9ec..7a828b93 100644 --- a/src/sagas/terminal.ts +++ b/src/sagas/terminal.ts @@ -1,11 +1,12 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { Channel } from 'redux-saga'; import { actionChannel, delay, fork, + getContext, put, race, select, @@ -120,11 +121,13 @@ function* receiveTerminalData(): Generator { value += action.value; } + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + // stdin gets piped to BLE connection const data = encoder.encode(value); for (let i = 0; i < data.length; i += SafeTxCharLength) { const { id } = (yield put( - write(data.slice(i, i + SafeTxCharLength)), + write(nextMessageId(), data.slice(i, i + SafeTxCharLength)), )) as BleUartWriteAction; yield take( From 9d7845237c809ca31382583221ae5b6b77c266d9 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 20 Jan 2021 17:11:32 -0600 Subject: [PATCH 10/18] rename waitForDidSend to waitForDidRequest This better reflects what the function actually does. --- src/sagas/flash-firmware.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 85191478..ac90b02e 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -82,7 +82,7 @@ type WaitResponse = [ boolean, ]; -function* waitForDidSend(id: number): Generator { +function* waitForDidRequest(id: number): Generator { const didRequest = (yield take( (a: Action) => a.type === BootloaderDidRequestType && @@ -228,7 +228,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { infoRequest(nextMessageId()), )) as BootloaderInfoRequestAction; const [, info] = (yield all([ - waitForDidSend(infoAction.id), + waitForDidRequest(infoAction.id), waitForResponse(BootloaderResponseActionType.Info), ])) as [BootloaderDidRequestAction, WaitResponse]; if (!info[0]) { @@ -258,7 +258,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { const disconnectAction = (yield put( disconnectRequest(nextMessageId()), )) as BootloaderDisconnectRequestAction; - yield waitForDidSend(disconnectAction.id); + yield waitForDidRequest(disconnectAction.id); return; } @@ -278,7 +278,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { eraseRequest(nextMessageId()), )) as BootloaderEraseRequestAction; const [, erase] = (yield all([ - waitForDidSend(eraseAction.id), + waitForDidRequest(eraseAction.id), waitForResponse(BootloaderResponseActionType.Erase, 5000), ])) as [BootloaderDidRequestAction, WaitResponse]; if (!erase[0] || erase[0].result) { @@ -290,7 +290,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { initRequest(nextMessageId(), firmware.length), )) as BootloaderInitRequestAction; const [, init] = (yield all([ - waitForDidSend(initAction.id), + waitForDidRequest(initAction.id), waitForResponse(BootloaderResponseActionType.Init), ])) as [BootloaderDidRequestAction, WaitResponse]; if (!init[0] || init[0].result) { @@ -314,7 +314,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { payload.buffer, ), )) as BootloaderProgramRequestAction; - yield waitForDidSend(programAction.id); + yield waitForDidRequest(programAction.id); yield put(didProgress(offset / firmware.length)); @@ -334,7 +334,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { checksumRequest(nextMessageId()), )) as BootloaderChecksumRequestAction; const [, checksum] = (yield all([ - waitForDidSend(checksumAction.id), + waitForDidRequest(checksumAction.id), waitForResponse(BootloaderResponseActionType.Checksum, 5000), ])) as [ BootloaderDidRequestAction, @@ -365,7 +365,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { const rebootAction = (yield put( rebootRequest(nextMessageId()), )) as BootloaderRebootRequestAction; - yield waitForDidSend(rebootAction.id); + yield waitForDidRequest(rebootAction.id); yield put(didFinish()); } From 6f6bf268548886bea0c98d2d684ef1b4c25173fa Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 11:55:21 -0600 Subject: [PATCH 11/18] add basic flash-firmware sagas tests So far just testing the success paths, no errors. --- package.json | 2 + .../__snapshots__/flash-firmware.test.ts.snap | 31 ++ src/sagas/flash-firmware.test.ts | 437 ++++++++++++++++++ yarn.lock | 9 +- 4 files changed, 478 insertions(+), 1 deletion(-) create mode 100644 src/sagas/__snapshots__/flash-firmware.test.ts.snap create mode 100644 src/sagas/flash-firmware.test.ts diff --git a/package.json b/package.json index 892bd861..bedd77a5 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "@testing-library/user-event": "^12.6.0", "@types/file-saver": "^2.0.1", "@types/jest": "^25.2.3", + "@types/jszip": "^3.4.1", "@types/node": "^12.0.0", "@types/react": "^16.9.35", "@types/react-dom": "^16.9.8", @@ -29,6 +30,7 @@ "@types/zen-push": "^0.1.1", "ace-builds": "^1.4.12", "file-saver": "^2.0.5", + "jszip": "^3.5.0", "license-webpack-plugin": "^2.3.11", "node-sass": "^4.14.1", "prop-types": "^15.7.2", diff --git a/src/sagas/__snapshots__/flash-firmware.test.ts.snap b/src/sagas/__snapshots__/flash-firmware.test.ts.snap new file mode 100644 index 00000000..f3b2d664 --- /dev/null +++ b/src/sagas/__snapshots__/flash-firmware.test.ts.snap @@ -0,0 +1,31 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`flashFirmware normal flow 1`] = ` +Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", +} +`; + +exports[`flashFirmware user supplied firmware.zip 1`] = ` +Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", +} +`; + +exports[`flashFirmware user supplied main.py 1`] = ` +Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", +} +`; diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts new file mode 100644 index 00000000..91952bcf --- /dev/null +++ b/src/sagas/flash-firmware.test.ts @@ -0,0 +1,437 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2021 The Pybricks Authors + +import { FirmwareMetadata } from '@pybricks/firmware'; +import JSZip from 'jszip'; +import { AsyncSaga } from '../../test'; +import { + didFinish, + didProgress, + didStart, + flashFirmware as flashFirmwareAction, +} from '../actions/flash-firmware'; +import { + BootloaderProgramRequestAction, + checksumRequest, + checksumResponse, + connect, + didConnect, + didRequest, + eraseRequest, + eraseResponse, + infoRequest, + infoResponse, + initRequest, + initResponse, + programRequest, + programResponse, + rebootRequest, +} from '../actions/lwp3-bootloader'; +import { didCompile } from '../actions/mpy'; +import { HubType, Result } from '../protocols/lwp3-bootloader'; +import { EditorState } from '../reducers/editor'; +import { SettingsState } from '../reducers/settings'; +import { createCountFunc } from '../utils/iter'; +import flashFirmware from './flash-firmware'; + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe('flashFirmware', () => { + test('normal flow', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + 'checksum-type': 'sum', + 'firmware-version': '1.2.3', + 'max-firmware-size': 1024, + 'mpy-abi-version': 5, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 100, + }; + + const zip = new JSZip(); + zip.file('firmware-base.bin', new Uint8Array(64)); + zip.file('firmware.metadata.json', JSON.stringify(metadata)); + zip.file('main.py', 'print("test")'); + zip.file('ReadMe_OSS.txt', 'test'); + + jest.spyOn(window, 'fetch').mockResolvedValueOnce( + new Response(await zip.generateAsync({ type: 'blob' })), + ); + + const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); + + saga.setState({ settings: { flashCurrentProgram: false } as SettingsState }); + + // saga is triggered by this action + + saga.put(flashFirmwareAction()); + + // first step is to connect to the hub bootloader + + let action = await saga.take(); + expect(action).toEqual(connect()); + + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); + + // then compile main.py to .mpy + + action = await saga.take(); + expect(action).toMatchSnapshot(); + + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); + + // then start flashing the firmware + + // should get didStart action just before starting to erase + action = await saga.take(); + expect(action).toEqual(didStart()); + + // erase first + + action = await saga.take(); + expect(action).toEqual(eraseRequest(1)); + + saga.put(didRequest(1)); + saga.put(eraseResponse(Result.OK)); + + // then write the new firmware + + const totalFirmwareSize = metadata['user-mpy-offset'] + mpySize + 8; + action = await saga.take(); + expect(action).toEqual(initRequest(2, totalFirmwareSize)); + + saga.put(didRequest(2)); + saga.put(initResponse(Result.OK)); + + const dummyPayload = new ArrayBuffer(0); + let id = 2; + for (let count = 1, offset = 0; ; count++, offset += 14) { + action = await saga.take(); + expect(action).toEqual( + programRequest(++id, 0x08005000 + offset, dummyPayload), + ); + expect((action as BootloaderProgramRequestAction).payload.byteLength).toBe( + Math.min(14, totalFirmwareSize - offset), + ); + + saga.put(didRequest(id)); + + action = await saga.take(); + expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + + // Have to be careful that a checksum request is not sent after + // last payload is sent, otherwise the hub gets confused. + + if (offset + 14 >= totalFirmwareSize) { + break; + } + + if (count % 10 === 0) { + action = await saga.take(); + expect(action).toEqual(checksumRequest(++id)); + + saga.put(didRequest(id)); + saga.put(checksumResponse(0)); + } + } + + // hub indicates success + + saga.put(programResponse(0, totalFirmwareSize)); + + action = await saga.take(); + expect(action).toEqual(didProgress(1)); + + // and finally reboot the hub + + action = await saga.take(); + expect(action).toEqual(rebootRequest(++id)); + + saga.put(didRequest(id)); + + // then we are done + + action = await saga.take(); + expect(action).toEqual(didFinish()); + + await saga.end(); + }); + + test('user supplied firmware.zip', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + 'checksum-type': 'sum', + 'firmware-version': '1.2.3', + 'max-firmware-size': 1024, + 'mpy-abi-version': 5, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 100, + }; + + const zip = new JSZip(); + zip.file('firmware-base.bin', new Uint8Array(64)); + zip.file('firmware.metadata.json', JSON.stringify(metadata)); + zip.file('main.py', 'print("test")'); + zip.file('ReadMe_OSS.txt', 'test'); + + const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); + + saga.setState({ settings: { flashCurrentProgram: false } as SettingsState }); + + // saga is triggered by this action + + saga.put(flashFirmwareAction(await zip.generateAsync({ type: 'arraybuffer' }))); + + // the first step is to compile main.py to .mpy + + let action = await saga.take(); + expect(action).toMatchSnapshot(); + + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); + + // then connect to the hub bootloader + + action = await saga.take(); + expect(action).toEqual(connect()); + + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); + + // then start flashing the firmware + + // should get didStart action just before starting to erase + action = await saga.take(); + expect(action).toEqual(didStart()); + + // erase first + + action = await saga.take(); + expect(action).toEqual(eraseRequest(1)); + + saga.put(didRequest(1)); + saga.put(eraseResponse(Result.OK)); + + // then write the new firmware + + const totalFirmwareSize = metadata['user-mpy-offset'] + mpySize + 8; + action = await saga.take(); + expect(action).toEqual(initRequest(2, totalFirmwareSize)); + + saga.put(didRequest(2)); + saga.put(initResponse(Result.OK)); + + const dummyPayload = new ArrayBuffer(0); + let id = 2; + for (let count = 1, offset = 0; ; count++, offset += 14) { + action = await saga.take(); + expect(action).toEqual( + programRequest(++id, 0x08005000 + offset, dummyPayload), + ); + expect((action as BootloaderProgramRequestAction).payload.byteLength).toBe( + Math.min(14, totalFirmwareSize - offset), + ); + + saga.put(didRequest(id)); + + action = await saga.take(); + expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + + // Have to be careful that a checksum request is not sent after + // last payload is sent, otherwise the hub gets confused. + + if (offset + 14 >= totalFirmwareSize) { + break; + } + + if (count % 10 === 0) { + action = await saga.take(); + expect(action).toEqual(checksumRequest(++id)); + + saga.put(didRequest(id)); + saga.put(checksumResponse(0)); + } + } + + // hub indicates success + + saga.put(programResponse(0, totalFirmwareSize)); + + action = await saga.take(); + expect(action).toEqual(didProgress(1)); + + // and finally reboot the hub + + action = await saga.take(); + expect(action).toEqual(rebootRequest(++id)); + + saga.put(didRequest(id)); + + // then we are done + + action = await saga.take(); + expect(action).toEqual(didFinish()); + + await saga.end(); + }); + + test('user supplied main.py', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + 'checksum-type': 'sum', + 'firmware-version': '1.2.3', + 'max-firmware-size': 1024, + 'mpy-abi-version': 5, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 100, + }; + + const zip = new JSZip(); + zip.file('firmware-base.bin', new Uint8Array(64)); + zip.file('firmware.metadata.json', JSON.stringify(metadata)); + zip.file('main.py', 'print("test")'); + zip.file('ReadMe_OSS.txt', 'test'); + + jest.spyOn(window, 'fetch').mockResolvedValueOnce( + new Response(await zip.generateAsync({ type: 'blob' })), + ); + + const editor = { + getValue: () => 'print("test")', + }; + + const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); + + saga.setState({ + editor: { current: editor } as EditorState, + settings: { flashCurrentProgram: true } as SettingsState, + }); + + // saga is triggered by this action + + saga.put(flashFirmwareAction()); + + // first step is to connect to the hub bootloader + + let action = await saga.take(); + expect(action).toEqual(connect()); + + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); + + // then compile main.py to .mpy + + action = await saga.take(); + expect(action).toMatchSnapshot(); + + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); + + // then start flashing the firmware + + // should get didStart action just before starting to erase + action = await saga.take(); + expect(action).toEqual(didStart()); + + // erase first + + action = await saga.take(); + expect(action).toEqual(eraseRequest(1)); + + saga.put(didRequest(1)); + saga.put(eraseResponse(Result.OK)); + + // then write the new firmware + + const totalFirmwareSize = metadata['user-mpy-offset'] + mpySize + 8; + action = await saga.take(); + expect(action).toEqual(initRequest(2, totalFirmwareSize)); + + saga.put(didRequest(2)); + saga.put(initResponse(Result.OK)); + + const dummyPayload = new ArrayBuffer(0); + let id = 2; + for (let count = 1, offset = 0; ; count++, offset += 14) { + action = await saga.take(); + expect(action).toEqual( + programRequest(++id, 0x08005000 + offset, dummyPayload), + ); + expect((action as BootloaderProgramRequestAction).payload.byteLength).toBe( + Math.min(14, totalFirmwareSize - offset), + ); + + saga.put(didRequest(id)); + + action = await saga.take(); + expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + + // Have to be careful that a checksum request is not sent after + // last payload is sent, otherwise the hub gets confused. + + if (offset + 14 >= totalFirmwareSize) { + break; + } + + if (count % 10 === 0) { + action = await saga.take(); + expect(action).toEqual(checksumRequest(++id)); + + saga.put(didRequest(id)); + saga.put(checksumResponse(0)); + } + } + + // hub indicates success + + saga.put(programResponse(0, totalFirmwareSize)); + + action = await saga.take(); + expect(action).toEqual(didProgress(1)); + + // and finally reboot the hub + + action = await saga.take(); + expect(action).toEqual(rebootRequest(++id)); + + saga.put(didRequest(id)); + + // then we are done + + action = await saga.take(); + expect(action).toEqual(didFinish()); + + await saga.end(); + }); +}); diff --git a/yarn.lock b/yarn.lock index c6a9ebcc..8b8652dd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1935,6 +1935,13 @@ resolved "https://registry.yarnpkg.com/@types/json5/-/json5-0.0.29.tgz#ee28707ae94e11d2b827bcbe5270bcea7f3e71ee" integrity sha1-7ihweulOEdK4J7y+UnC86n8+ce4= +"@types/jszip@^3.4.1": + version "3.4.1" + resolved "https://registry.yarnpkg.com/@types/jszip/-/jszip-3.4.1.tgz#e7a4059486e494c949ef750933d009684227846f" + integrity sha512-TezXjmf3lj+zQ651r6hPqvSScqBLvyPI9FxdXBqpEwBijNGQ2NXpaFW/7joGzveYkKQUil7iiDHLo6LV71Pc0A== + dependencies: + jszip "*" + "@types/minimatch@*": version "3.0.3" resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d" @@ -7389,7 +7396,7 @@ jsprim@^1.2.2: array-includes "^3.1.2" object.assign "^4.1.2" -jszip@^3.5.0: +jszip@*, jszip@^3.5.0: version "3.5.0" resolved "https://registry.yarnpkg.com/jszip/-/jszip-3.5.0.tgz#b4fd1f368245346658e781fec9675802489e15f6" integrity sha512-WRtu7TPCmYePR1nazfrtuF216cIVon/3GWOvHS9QR5bIwSbnxtdpma6un3jyGGNhHsKCSzn5Ypk+EkDRvTGiFA== From 3b3141218622405845e416455bf32f9c13b14f3b Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 12:30:11 -0600 Subject: [PATCH 12/18] minor code improvements in flash-firmware --- src/sagas/flash-firmware.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index ac90b02e..787337e5 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -298,14 +298,10 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { throw Error(`Failed to init: ${init}`); } - let count = 0; - const maxDataSize = MaxProgramFlashSize.get(info[0].hubType); - if (maxDataSize === undefined) { - // istanbul ignore next: indicates programmer error if reached - throw Error('Missing hub type in MaxProgramFlashSize'); - } + // 14 is "safe" size for all hubs + const maxDataSize = MaxProgramFlashSize.get(info[0].hubType) || 14; - for (let offset = 0; ; ) { + for (let count = 1, offset = 0; ; count++) { const payload = firmware.slice(offset, offset + maxDataSize); const programAction = (yield put( programRequest( @@ -329,7 +325,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // the hub because of sending too much data at once. The actual // number of packets that can be queued in the Bluetooth chip on // the hub is not known and could vary by device. - if (++count % 10 === 0) { + if (count % 10 === 0) { const checksumAction = (yield put( checksumRequest(nextMessageId()), )) as BootloaderChecksumRequestAction; From a7ae66021be18c39fa3939068e83500067e327bc Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 13:41:27 -0600 Subject: [PATCH 13/18] add firmware flash failure reasons --- src/actions/flash-firmware.ts | 252 +++++++++++++++++++++++++++++++++- 1 file changed, 249 insertions(+), 3 deletions(-) diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index a55454f6..0187bd6c 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2020-2021 The Pybricks Authors +import { FirmwareMetadata, FirmwareReaderError } from '@pybricks/firmware'; import { Action } from 'redux'; import { assert } from '../utils'; @@ -12,6 +13,8 @@ export enum FlashFirmwareActionType { FlashFirmware = 'flashFirmware.action.flashFirmware', /** Flashing started. */ DidStart = 'flashFirmware.action.didStart', + /** Flashing was not able to start. */ + DidFailToStart = 'flashFirmware.action.didFailStart', /** Firmware flash progress. */ DidProgress = 'flashFirmware.action.didProgress', /** Flashing finished successfully. */ @@ -20,6 +23,116 @@ export enum FlashFirmwareActionType { DidFailToFinish = 'flashFirmware.action.didFailToFinish', } +export enum MetadataProblem { + Missing = 'metadata.missing', + NotSupported = 'metadata.notSupported', +} + +export enum HubError { + UnknownCommand = 'hubError.unknownCommand', + EraseFailed = 'hubError.eraseFailed', + InitFailed = 'hubError.initFailed', + CountMismatch = 'hubError.countMismatch', + ChecksumMismatch = 'hubError.checksumMismatch', +} + +function isHubError(arg: unknown): arg is HubError { + if (typeof arg !== 'string') { + return false; + } + return Object.keys(HubError).includes(arg); +} + +type Reason = { + reason: T; +}; + +export enum FailToStartReasonType { + /** Connecting to the hub failed. */ + FailedToConnect = 'flashFirmware.failToStart.reason.failedToConnect', + /** The is no firmware available that matches the connected hub. */ + NoFirmware = 'flashFirmware.failToStart.reason.noFirmware', + /** The provided firmware.zip does not match the connected hub. */ + DeviceMismatch = 'flashFirmware.failToStart.reason.deviceMismatch', + /** There was a problem with the zip file. */ + ZipError = 'flashFirmware.failToStart.reason.zipError', + /** Metadata property is missing or invalid. */ + BadMetadata = 'flashFirmware.failToStart.reason.badMetadata', + /** The main.py file failed to compile. */ + FailedToCompile = 'flashFirmware.failToStart.reason.failedToCompile', + /** The combined firmware-base.bin and main.mpy are too big. */ + FirmwareSize = 'flashFirmware.failToStart.reason.firmwareSize', + /** An unexpected error occurred. */ + Unknown = 'flashFirmware.failToStart.reason.unknown', +} + +export type FailToStartReasonFailedToConnect = Reason; + +export type FailToStartReasonNoFirmware = Reason; + +export type FailToStartReasonDeviceMismatch = Reason; + +export type FailToStartReasonZipError = Reason & { + err: FirmwareReaderError; +}; + +export type FailToStartReasonBadMetadata = Reason & { + property: keyof FirmwareMetadata; + problem: MetadataProblem; +}; + +export type FailToStartReasonFirmwareSize = Reason; + +export type FailToStartReasonFailedToCompile = Reason; + +export type FailToStartReasonUnknown = Reason & { + err: Error; +}; + +export type FailToStartReason = + | FailToStartReasonFailedToConnect + | FailToStartReasonNoFirmware + | FailToStartReasonDeviceMismatch + | FailToStartReasonZipError + | FailToStartReasonBadMetadata + | FailToStartReasonFirmwareSize + | FailToStartReasonFailedToCompile + | FailToStartReasonUnknown; + +export enum FailToFinishReasonType { + /** Waiting for a response from the hub took too long. */ + TimedOut = 'flashFirmware.failToFinish.reason.timedOut', + /** Something went wrong with the BLE connection. */ + BleError = 'flashFirmware.failToFinish.reason.bleError', + /** The BLE connection was lost before flashing completed. */ + Disconnected = 'flashFirmware.failToFinish.reason.disconnected', + /** The hub sent a response indicating a problem. */ + HubError = 'flashFirmware.failToFinish.reason.hubError', + /** An unexpected error occurred. */ + Unknown = 'flashFirmware.failToFinish.reason.unknown', +} + +export type FailToFinishReasonTimedOut = Reason; + +export type FailToFinishReasonBleError = Reason; + +export type FailToFinishReasonDisconnected = Reason; + +export type FailToFinishReasonHubError = Reason & { + hubError: HubError; +}; + +export type FailToFinishReasonUnknown = Reason & { + err: Error; +}; + +export type FailToFinishReason = + | FailToFinishReasonTimedOut + | FailToFinishReasonBleError + | FailToFinishReasonDisconnected + | FailToFinishReasonHubError + | FailToFinishReasonUnknown; + /** * Action that flashes firmware to a hub. */ @@ -47,6 +160,94 @@ export function didStart(): FlashFirmwareDidStartAction { return { type: FlashFirmwareActionType.DidStart }; } +/** Action that indicates flashing did not start because of an error. */ +export type FlashFirmwareDidFailToStartAction = Action & { + reason: FailToStartReason; +}; + +export function didFailToStart( + reason: FailToStartReasonType.ZipError, + err: FirmwareReaderError, +): FlashFirmwareDidFailToStartAction; + +export function didFailToStart( + reason: FailToStartReasonType.BadMetadata, + property: keyof FirmwareMetadata, + problem: MetadataProblem, +): FlashFirmwareDidFailToStartAction; + +export function didFailToStart( + reason: FailToStartReasonType.Unknown, + err: Error, +): FlashFirmwareDidFailToStartAction; + +export function didFailToStart( + reason: Exclude< + FailToStartReasonType, + | FailToStartReasonType.ZipError + | FailToStartReasonType.BadMetadata + | FailToStartReasonType.Unknown + >, +): FlashFirmwareDidFailToStartAction; + +/** + * Action that indicates flashing did not start because of an error. + * @param total The total number of bytes to be flashed. + */ +export function didFailToStart( + reason: FailToStartReasonType, + arg1?: string | Error, + arg2?: MetadataProblem, +): FlashFirmwareDidFailToStartAction { + if (reason === FailToStartReasonType.ZipError) { + // istanbul ignore if: programmer error give wrong arg + if (!(arg1 instanceof FirmwareReaderError)) { + throw new Error('missing or invalid err'); + } + return { + type: FlashFirmwareActionType.DidFailToStart, + reason: { reason, err: arg1 }, + }; + } + + if (reason === FailToStartReasonType.BadMetadata) { + // istanbul ignore if: programmer error give wrong arg + if ( + arg1 !== 'metadata-version' && + arg1 !== 'firmware-version' && + arg1 !== 'device-id' && + arg1 !== 'checksum-type' && + arg1 !== 'mpy-abi-version' && + arg1 !== 'mpy-cross-options' && + arg1 !== 'user-mpy-offset' && + arg1 !== 'max-firmware-size' + ) { + throw new Error('missing or invalid property'); + } + // istanbul ignore if: programmer error give wrong arg + if (arg2 === undefined) { + throw new Error('missing or invalid problem'); + } + return { + type: FlashFirmwareActionType.DidFailToStart, + reason: { reason, property: arg1, problem: arg2 }, + }; + } + + if (reason === FailToStartReasonType.Unknown) { + // istanbul ignore if: programmer error give wrong arg + if (!(arg1 instanceof Error)) { + throw new Error('missing or invalid err'); + } + return { + type: FlashFirmwareActionType.DidFailToStart, + reason: { reason, err: arg1 }, + }; + } + + return { type: FlashFirmwareActionType.DidFailToStart, reason: { reason } }; +} + /** Action that indicates current firmware flashing progress. */ export type FlashFirmwareDidProgressAction = Action & { /** The current progress (0 to 1). */ @@ -71,11 +272,55 @@ export function didFinish(): FlashFirmwareDidFinishAction { } /** Action that indicates that flashing failed. */ -export type FlashFirmwareDidFailToFinishAction = Action; +export type FlashFirmwareDidFailToFinishAction = Action & { + reason: FailToFinishReason; +}; + +export function didFailToFinish( + reason: FailToFinishReasonType.HubError, + hubError: HubError, +): FlashFirmwareDidFailToFinishAction; + +export function didFailToFinish( + reason: FailToFinishReasonType.Unknown, + err: Error, +): FlashFirmwareDidFailToFinishAction; + +export function didFailToFinish( + reason: Exclude< + FailToFinishReasonType, + FailToFinishReasonType.HubError | FailToFinishReasonType.Unknown + >, +): FlashFirmwareDidFailToFinishAction; /** Action that indicates that flashing failed. */ -export function didFailToFinish(): FlashFirmwareDidFailToFinishAction { - return { type: FlashFirmwareActionType.DidFailToFinish }; +export function didFailToFinish( + reason: FailToFinishReasonType, + arg1?: HubError | Error, +): FlashFirmwareDidFailToFinishAction { + if (reason === FailToFinishReasonType.HubError) { + // istanbul ignore if: programmer error give wrong arg + if (!isHubError(arg1)) { + throw new Error('missing or invalid err'); + } + return { + type: FlashFirmwareActionType.DidFailToFinish, + reason: { reason, hubError: arg1 }, + }; + } + + if (reason === FailToFinishReasonType.Unknown) { + // istanbul ignore if: programmer error give wrong arg + if (!(arg1 instanceof Error)) { + throw new Error('missing or invalid err'); + } + return { + type: FlashFirmwareActionType.DidFailToFinish, + reason: { reason, err: arg1 }, + }; + } + + return { type: FlashFirmwareActionType.DidFailToFinish, reason: { reason } }; } /** @@ -84,6 +329,7 @@ export function didFailToFinish(): FlashFirmwareDidFailToFinishAction { export type FlashFirmwareAction = | FlashFirmwareFlashAction | FlashFirmwareDidStartAction + | FlashFirmwareDidFailToStartAction | FlashFirmwareDidProgressAction | FlashFirmwareDidFinishAction | FlashFirmwareDidFailToFinishAction; From ab0c850b8095394c56f33aec08956706650b46b8 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 19:47:54 -0600 Subject: [PATCH 14/18] add proper error handling for firmware zip error --- .../__snapshots__/flash-firmware.test.ts.snap | 2 +- src/sagas/flash-firmware.test.ts | 263 +++++++++++------- src/sagas/flash-firmware.ts | 30 +- src/utils/index.test.ts | 13 +- src/utils/index.ts | 11 + 5 files changed, 212 insertions(+), 107 deletions(-) diff --git a/src/sagas/__snapshots__/flash-firmware.test.ts.snap b/src/sagas/__snapshots__/flash-firmware.test.ts.snap index f3b2d664..77e153cd 100644 --- a/src/sagas/__snapshots__/flash-firmware.test.ts.snap +++ b/src/sagas/__snapshots__/flash-firmware.test.ts.snap @@ -10,7 +10,7 @@ Object { } `; -exports[`flashFirmware user supplied firmware.zip 1`] = ` +exports[`flashFirmware user supplied firmware.zip success 1`] = ` Object { "options": Array [ "-mno-unicode", diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 91952bcf..25ed6d58 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -1,10 +1,16 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2021 The Pybricks Authors -import { FirmwareMetadata } from '@pybricks/firmware'; +import { + FirmwareMetadata, + FirmwareReaderError, + FirmwareReaderErrorCode, +} from '@pybricks/firmware'; import JSZip from 'jszip'; import { AsyncSaga } from '../../test'; import { + FailToStartReasonType, + didFailToStart, didFinish, didProgress, didStart, @@ -170,131 +176,186 @@ describe('flashFirmware', () => { await saga.end(); }); - test('user supplied firmware.zip', async () => { - const metadata: FirmwareMetadata = { - 'metadata-version': '1.0.0', - 'device-id': HubType.MoveHub, - 'checksum-type': 'sum', - 'firmware-version': '1.2.3', - 'max-firmware-size': 1024, - 'mpy-abi-version': 5, - 'mpy-cross-options': ['-mno-unicode'], - 'user-mpy-offset': 100, - }; + describe('user supplied firmware.zip', () => { + test('success', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + 'checksum-type': 'sum', + 'firmware-version': '1.2.3', + 'max-firmware-size': 1024, + 'mpy-abi-version': 5, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 100, + }; - const zip = new JSZip(); - zip.file('firmware-base.bin', new Uint8Array(64)); - zip.file('firmware.metadata.json', JSON.stringify(metadata)); - zip.file('main.py', 'print("test")'); - zip.file('ReadMe_OSS.txt', 'test'); + const zip = new JSZip(); + zip.file('firmware-base.bin', new Uint8Array(64)); + zip.file('firmware.metadata.json', JSON.stringify(metadata)); + zip.file('main.py', 'print("test")'); + zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(flashFirmware, { + nextMessageId: createCountFunc(), + }); - saga.setState({ settings: { flashCurrentProgram: false } as SettingsState }); + saga.setState({ + settings: { flashCurrentProgram: false } as SettingsState, + }); - // saga is triggered by this action + // saga is triggered by this action - saga.put(flashFirmwareAction(await zip.generateAsync({ type: 'arraybuffer' }))); + saga.put( + flashFirmwareAction(await zip.generateAsync({ type: 'arraybuffer' })), + ); - // the first step is to compile main.py to .mpy + // the first step is to compile main.py to .mpy - let action = await saga.take(); - expect(action).toMatchSnapshot(); + let action = await saga.take(); + expect(action).toMatchSnapshot(); - const mpySize = 20; - const mpyBinaryData = new Uint8Array(mpySize); - saga.put(didCompile(mpyBinaryData)); + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); - // then connect to the hub bootloader + // then connect to the hub bootloader - action = await saga.take(); - expect(action).toEqual(connect()); - - saga.put(didConnect()); - - // then find out what kind of hub it is - - action = await saga.take(); - expect(action).toEqual(infoRequest(0)); - - saga.put(didRequest(0)); - saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); - - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - - // erase first - - action = await saga.take(); - expect(action).toEqual(eraseRequest(1)); - - saga.put(didRequest(1)); - saga.put(eraseResponse(Result.OK)); - - // then write the new firmware - - const totalFirmwareSize = metadata['user-mpy-offset'] + mpySize + 8; - action = await saga.take(); - expect(action).toEqual(initRequest(2, totalFirmwareSize)); - - saga.put(didRequest(2)); - saga.put(initResponse(Result.OK)); - - const dummyPayload = new ArrayBuffer(0); - let id = 2; - for (let count = 1, offset = 0; ; count++, offset += 14) { action = await saga.take(); - expect(action).toEqual( - programRequest(++id, 0x08005000 + offset, dummyPayload), - ); - expect((action as BootloaderProgramRequestAction).payload.byteLength).toBe( - Math.min(14, totalFirmwareSize - offset), - ); + expect(action).toEqual(connect()); + + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); + + // then start flashing the firmware + + // should get didStart action just before starting to erase + action = await saga.take(); + expect(action).toEqual(didStart()); + + // erase first + + action = await saga.take(); + expect(action).toEqual(eraseRequest(1)); + + saga.put(didRequest(1)); + saga.put(eraseResponse(Result.OK)); + + // then write the new firmware + + const totalFirmwareSize = metadata['user-mpy-offset'] + mpySize + 8; + action = await saga.take(); + expect(action).toEqual(initRequest(2, totalFirmwareSize)); + + saga.put(didRequest(2)); + saga.put(initResponse(Result.OK)); + + const dummyPayload = new ArrayBuffer(0); + let id = 2; + for (let count = 1, offset = 0; ; count++, offset += 14) { + action = await saga.take(); + expect(action).toEqual( + programRequest(++id, 0x08005000 + offset, dummyPayload), + ); + expect( + (action as BootloaderProgramRequestAction).payload.byteLength, + ).toBe(Math.min(14, totalFirmwareSize - offset)); + + saga.put(didRequest(id)); + + action = await saga.take(); + expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + + // Have to be careful that a checksum request is not sent after + // last payload is sent, otherwise the hub gets confused. + + if (offset + 14 >= totalFirmwareSize) { + break; + } + + if (count % 10 === 0) { + action = await saga.take(); + expect(action).toEqual(checksumRequest(++id)); + + saga.put(didRequest(id)); + saga.put(checksumResponse(0)); + } + } + + // hub indicates success + + saga.put(programResponse(0, totalFirmwareSize)); + + action = await saga.take(); + expect(action).toEqual(didProgress(1)); + + // and finally reboot the hub + + action = await saga.take(); + expect(action).toEqual(rebootRequest(++id)); saga.put(didRequest(id)); + // then we are done + action = await saga.take(); - expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + expect(action).toEqual(didFinish()); - // Have to be careful that a checksum request is not sent after - // last payload is sent, otherwise the hub gets confused. + await saga.end(); + }); - if (offset + 14 >= totalFirmwareSize) { - break; - } + test('zip error', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + 'checksum-type': 'sum', + 'firmware-version': '1.2.3', + 'max-firmware-size': 1024, + 'mpy-abi-version': 5, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 100, + }; - if (count % 10 === 0) { - action = await saga.take(); - expect(action).toEqual(checksumRequest(++id)); + const zip = new JSZip(); + // no firmware-base.bin - triggers zip error + zip.file('firmware.metadata.json', JSON.stringify(metadata)); + zip.file('main.py', 'print("test")'); + zip.file('ReadMe_OSS.txt', 'test'); - saga.put(didRequest(id)); - saga.put(checksumResponse(0)); - } - } + const saga = new AsyncSaga(flashFirmware, { + nextMessageId: createCountFunc(), + }); - // hub indicates success + saga.setState({ + settings: { flashCurrentProgram: false } as SettingsState, + }); - saga.put(programResponse(0, totalFirmwareSize)); + // saga is triggered by this action - action = await saga.take(); - expect(action).toEqual(didProgress(1)); + saga.put( + flashFirmwareAction(await zip.generateAsync({ type: 'arraybuffer' })), + ); - // and finally reboot the hub + // should get failure due to missing file - action = await saga.take(); - expect(action).toEqual(rebootRequest(++id)); + const action = await saga.take(); + expect(action).toStrictEqual( + didFailToStart( + FailToStartReasonType.ZipError, + new FirmwareReaderError( + FirmwareReaderErrorCode.MissingFirmwareBaseBin, + ), + ), + ); - saga.put(didRequest(id)); - - // then we are done - - action = await saga.take(); - expect(action).toEqual(didFinish()); - - await saga.end(); + await saga.end(); + }); }); test('user supplied main.py', async () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 787337e5..0ba93ab6 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -1,15 +1,22 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors -import { FirmwareMetadata, FirmwareReader, HubType } from '@pybricks/firmware'; +import { + FirmwareMetadata, + FirmwareReader, + FirmwareReaderError, + HubType, +} from '@pybricks/firmware'; import cityHubZip from '@pybricks/firmware/build/cityhub.zip'; import moveHubZip from '@pybricks/firmware/build/movehub.zip'; import technicHubZip from '@pybricks/firmware/build/technichub.zip'; import { Ace } from 'ace-builds'; import { Effect, + StrictEffect, all, call, + cancel, delay, getContext, put, @@ -20,8 +27,10 @@ import { } from 'redux-saga/effects'; import { Action } from '../actions'; import { + FailToStartReasonType, FlashFirmwareActionType, FlashFirmwareFlashAction, + didFailToStart, didFinish, didProgress, didStart, @@ -65,6 +74,7 @@ import { import * as notification from '../actions/notification'; import { MaxProgramFlashSize } from '../protocols/lwp3-bootloader'; import { RootState } from '../reducers'; +import { Maybe, maybe } from '../utils'; import { fmod, sumComplement32 } from '../utils/math'; const firmwareZipMap = new Map([ @@ -123,8 +133,20 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator { function* loadFirmware( data: ArrayBuffer, program: string | undefined, -): Generator { - const reader = (yield call(() => FirmwareReader.load(data))) as FirmwareReader; +): Generator { + const reader = (yield call(() => + maybe(FirmwareReader.load(data)), + )) as Maybe; + + if (reader instanceof Error) { + if (reader instanceof FirmwareReaderError) { + yield put(didFailToStart(FailToStartReasonType.ZipError, reader)); + } else { + yield put(didFailToStart(FailToStartReasonType.Unknown, reader)); + } + yield cancel(); + throw 'not reached'; + } const firmwareBase = (yield call(() => reader.readFirmwareBase())) as Uint8Array; const metadata = (yield call(() => reader.readMetadata())) as FirmwareMetadata; diff --git a/src/utils/index.test.ts b/src/utils/index.test.ts index 203ceede..2d3fd7b4 100644 --- a/src/utils/index.test.ts +++ b/src/utils/index.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2020 The Pybricks Authors -import { assert, hex } from '.'; +import { assert, hex, maybe } from '.'; test('assert', () => { const assertTrue = jest.fn(() => assert(true, 'should not throw')); @@ -11,6 +11,17 @@ test('assert', () => { expect(() => assert(false, 'should throw')).toThrow(); }); +describe('maybe', () => { + test('resolved', async () => { + const result = await maybe(Promise.resolve('test')); + expect(result).toBe('test'); + }); + test('rejected', async () => { + const result = await maybe(Promise.reject(new Error('test'))); + expect(result).toBeInstanceOf(Error); + }); +}); + test('hex', () => { expect(hex(0, 2)).toBe('0x00'); expect(hex(1, 4)).toBe('0x0001'); diff --git a/src/utils/index.ts b/src/utils/index.ts index ea7af95f..ed26210f 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -13,6 +13,17 @@ export function assert(condition: boolean, message: string): void { } } +export type Maybe = T | Error; + +/** Wraps a promise in try/catch and returns the promise result or error. */ +export async function maybe(promise: Promise): Promise> { + try { + return await promise; + } catch (err) { + return err; + } +} + /** * Formats a number as hex (0x00...) * @param n The number to format From 218dda4d0b6b6843df0cb11b0e05886427465102 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 22:22:46 -0600 Subject: [PATCH 15/18] use typed-redux-saga in flash-firmware sagas This make things a bit more type safe and a bit easier to read. --- package.json | 5 + src/sagas/flash-firmware.ts | 250 +++++++++++++++++------------------- src/utils/index.test.ts | 15 ++- src/utils/index.ts | 17 ++- yarn.lock | 127 +++++++++++++++++- 5 files changed, 267 insertions(+), 147 deletions(-) diff --git a/package.json b/package.json index bedd77a5..589073ec 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "@types/web-bluetooth": "^0.0.9", "@types/zen-push": "^0.1.1", "ace-builds": "^1.4.12", + "babel-plugin-macros": "^3.0.1", "file-saver": "^2.0.5", "jszip": "^3.5.0", "license-webpack-plugin": "^2.3.11", @@ -45,6 +46,7 @@ "redux-logger": "^3.0.6", "redux-saga": "^1.1.3", "spdx-satisfies": "^5.0.0", + "typed-redux-saga": "^1.3.1", "typescript": "~4.1.3", "web-vitals": "^1.0.1", "xterm": "^4.9.0", @@ -80,9 +82,12 @@ "@typescript-eslint/parser": "^4.13.0", "eslint": "^7.17.0", "eslint-config-prettier": "^7.1.0", + "eslint-config-typed-fp": "^1.3.0", + "eslint-plugin-functional": "^3.2.1", "eslint-plugin-import": "^2.22.1", "eslint-plugin-prettier": "^3.3.1", "eslint-plugin-react": "^7.22.0", + "eslint-plugin-total-functions": "^4.7.2", "jest-mock-extended": "^1.0.9", "prettier": "^2.2.1" } diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 0ba93ab6..c6bc1ff1 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -1,19 +1,12 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2020-2021 The Pybricks Authors -import { - FirmwareMetadata, - FirmwareReader, - FirmwareReaderError, - HubType, -} from '@pybricks/firmware'; +import { FirmwareReader, FirmwareReaderError, HubType } from '@pybricks/firmware'; import cityHubZip from '@pybricks/firmware/build/cityhub.zip'; import moveHubZip from '@pybricks/firmware/build/movehub.zip'; import technicHubZip from '@pybricks/firmware/build/technichub.zip'; -import { Ace } from 'ace-builds'; import { - Effect, - StrictEffect, + SagaGenerator, all, call, cancel, @@ -24,7 +17,7 @@ import { select, take, takeEvery, -} from 'redux-saga/effects'; +} from 'typed-redux-saga/macro'; import { Action } from '../actions'; import { FailToStartReasonType, @@ -36,24 +29,17 @@ import { didStart, } from '../actions/flash-firmware'; import { - BootloaderChecksumRequestAction, BootloaderChecksumResponseAction, BootloaderConnectionActionType, BootloaderConnectionDidConnectAction, BootloaderConnectionDidFailToConnectAction, BootloaderDidRequestAction, BootloaderDidRequestType, - BootloaderDisconnectRequestAction, - BootloaderEraseRequestAction, BootloaderEraseResponseAction, BootloaderErrorResponseAction, - BootloaderInfoRequestAction, BootloaderInfoResponseAction, - BootloaderInitRequestAction, BootloaderInitResponseAction, - BootloaderProgramRequestAction, BootloaderProgramResponseAction, - BootloaderRebootRequestAction, BootloaderResponseAction, BootloaderResponseActionType, checksumRequest, @@ -74,7 +60,7 @@ import { import * as notification from '../actions/notification'; import { MaxProgramFlashSize } from '../protocols/lwp3-bootloader'; import { RootState } from '../reducers'; -import { Maybe, maybe } from '../utils'; +import { defined, maybe } from '../utils'; import { fmod, sumComplement32 } from '../utils/math'; const firmwareZipMap = new Map([ @@ -83,25 +69,10 @@ const firmwareZipMap = new Map([ [HubType.MoveHub, moveHubZip], ]); -/** - * Helper type for return value of wait() function. - */ -type WaitResponse = [ - T, - BootloaderErrorResponseAction, - boolean, -]; - -function* waitForDidRequest(id: number): Generator { - const didRequest = (yield take( - (a: Action) => - a.type === BootloaderDidRequestType && - (a as BootloaderDidRequestAction).id === id, - )) as BootloaderDidRequestAction; - if (didRequest.err) { - console.error(didRequest.err); - } - return didRequest; +function* waitForDidRequest(id: number): SagaGenerator { + return yield* take( + (a: Action) => a.type === BootloaderDidRequestType && a.id === id, + ); } /** @@ -110,8 +81,19 @@ function* waitForDidRequest(id: number): Generator { * @param type The action type to wait for. * @param timeout The timeout in milliseconds. */ -function waitForResponse(type: BootloaderResponseActionType, timeout = 500): Effect { - return race([take(type), take(BootloaderResponseActionType.Error), delay(timeout)]); +function* waitForResponse( + type: BootloaderResponseActionType, + timeout = 500, +): SagaGenerator<{ + response?: T; + error?: BootloaderErrorResponseAction; + timeout?: boolean; +}> { + return yield* race({ + response: take(type), + error: take(BootloaderResponseActionType.Error), + timeout: delay(timeout), + }); } function* firmwareIterator(data: DataView, maxSize: number): Generator { @@ -133,27 +115,27 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator { function* loadFirmware( data: ArrayBuffer, program: string | undefined, -): Generator { - const reader = (yield call(() => - maybe(FirmwareReader.load(data)), - )) as Maybe; +): SagaGenerator<{ firmware: Uint8Array; deviceId: HubType }> { + const [reader, readerErr] = yield* call(() => maybe(FirmwareReader.load(data))); - if (reader instanceof Error) { - if (reader instanceof FirmwareReaderError) { - yield put(didFailToStart(FailToStartReasonType.ZipError, reader)); + if (readerErr) { + // istanbul ignore else: unexpected error + if (readerErr instanceof FirmwareReaderError) { + yield* put(didFailToStart(FailToStartReasonType.ZipError, readerErr)); } else { - yield put(didFailToStart(FailToStartReasonType.Unknown, reader)); + yield* put(didFailToStart(FailToStartReasonType.Unknown, readerErr)); } - yield cancel(); - throw 'not reached'; + yield* cancel(); } - const firmwareBase = (yield call(() => reader.readFirmwareBase())) as Uint8Array; - const metadata = (yield call(() => reader.readMetadata())) as FirmwareMetadata; + defined(reader); + + const firmwareBase = yield* call(() => reader.readFirmwareBase()); + const metadata = yield* call(() => reader.readMetadata()); // if a user program was not given, then use main.py from the frimware.zip if (program === undefined) { - program = (yield call(() => reader.readMainPy())) as string; + program = yield* call(() => reader.readMainPy()); } if (metadata['mpy-abi-version'] !== 5) { @@ -162,16 +144,18 @@ function* loadFirmware( ); } - yield put(compile(program, metadata['mpy-cross-options'])); - const [mpy, mpyFail] = (yield race([ - take(MpyActionType.DidCompile), - take(MpyActionType.DidFailToCompile), - ])) as [MpyDidCompileAction, MpyDidFailToCompileAction]; + yield* put(compile(program, metadata['mpy-cross-options'])); + const { mpy, mpyFail } = yield* race({ + mpy: take(MpyActionType.DidCompile), + mpyFail: take(MpyActionType.DidFailToCompile), + }); if (mpyFail) { throw Error(mpyFail.err.join('\n')); } + defined(mpy); + // compute offset for checksum - must be aligned to 4-byte boundary const checksumOffset = metadata['user-mpy-offset'] + 4 + mpy.data.length + fmod(-mpy.data.length, 4); @@ -210,14 +194,12 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { let program: string | undefined = undefined; - const flashCurrentProgram = (yield select( + const flashCurrentProgram = yield* select( (s: RootState) => s.settings.flashCurrentProgram, - )) as boolean; + ); if (flashCurrentProgram) { - const editor = (yield select( - (s: RootState) => s.editor.current, - )) as Ace.EditSession | null; + const editor = yield* select((s: RootState) => s.editor.current); // istanbul ignore if: it is a bug to dispatch this action with no current editor if (editor === null) { @@ -232,109 +214,111 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { ({ firmware, deviceId } = yield* loadFirmware(action.data, program)); } - yield put(connect()); - const connectResult = (yield take([ + yield* put(connect()); + const connectResult = yield* take< + | BootloaderConnectionDidConnectAction + | BootloaderConnectionDidFailToConnectAction + >([ BootloaderConnectionActionType.DidConnect, BootloaderConnectionActionType.DidFailToConnect, - ])) as - | BootloaderConnectionDidConnectAction - | BootloaderConnectionDidFailToConnectAction; + ]); if (connectResult.type === BootloaderConnectionActionType.DidFailToConnect) { return; } - const nextMessageId = (yield getContext('nextMessageId')) as () => number; + const nextMessageId = yield* getContext<() => number>('nextMessageId'); - const infoAction = (yield put( - infoRequest(nextMessageId()), - )) as BootloaderInfoRequestAction; - const [, info] = (yield all([ - waitForDidRequest(infoAction.id), - waitForResponse(BootloaderResponseActionType.Info), - ])) as [BootloaderDidRequestAction, WaitResponse]; - if (!info[0]) { + const infoAction = yield* put(infoRequest(nextMessageId())); + const { info } = yield* all({ + sent: waitForDidRequest(infoAction.id), + info: waitForResponse( + BootloaderResponseActionType.Info, + ), + }); + if (!info.response) { throw Error(`failed to get info: ${info}`); } - if (deviceId !== undefined && info[0].hubType !== deviceId) { - throw Error(`Connected to ${info[0].hubType} but firmware is for ${deviceId}`); + if (deviceId !== undefined && info.response.hubType !== deviceId) { + throw Error( + `Connected to ${info.response.hubType} but firmware is for ${deviceId}`, + ); } if (firmware === undefined) { - const firmwarePath = firmwareZipMap.get(info[0].hubType); + const firmwarePath = firmwareZipMap.get(info.response.hubType); if (firmwarePath === undefined) { - yield put( + yield* put( notification.add( 'error', "Sorry, we don't have firmware for this hub yet.", ), ); - yield put(disconnectRequest(nextMessageId())); + yield* put(disconnectRequest(nextMessageId())); return; } - const response = (yield call(() => fetch(firmwarePath))) as Response; + const response = yield* call(() => fetch(firmwarePath)); if (!response.ok) { - yield put(notification.add('error', 'Failed to fetch firmware.')); - const disconnectAction = (yield put( - disconnectRequest(nextMessageId()), - )) as BootloaderDisconnectRequestAction; - yield waitForDidRequest(disconnectAction.id); + yield* put(notification.add('error', 'Failed to fetch firmware.')); + const disconnectAction = yield* put(disconnectRequest(nextMessageId())); + yield* waitForDidRequest(disconnectAction.id); return; } - const data = (yield call(() => response.arrayBuffer())) as ArrayBuffer; + const data = yield* call(() => response.arrayBuffer()); ({ firmware, deviceId } = yield* loadFirmware(data, program)); - if (deviceId !== undefined && info[0].hubType !== deviceId) { + if (deviceId !== undefined && info.response.hubType !== deviceId) { throw Error( - `Connected to ${info[0].hubType} but firmware is for ${deviceId}`, + `Connected to ${info.response.hubType} but firmware is for ${deviceId}`, ); } } - yield put(didStart()); + yield* put(didStart()); - const eraseAction = (yield put( - eraseRequest(nextMessageId()), - )) as BootloaderEraseRequestAction; - const [, erase] = (yield all([ - waitForDidRequest(eraseAction.id), - waitForResponse(BootloaderResponseActionType.Erase, 5000), - ])) as [BootloaderDidRequestAction, WaitResponse]; - if (!erase[0] || erase[0].result) { + const eraseAction = yield* put(eraseRequest(nextMessageId())); + const { erase } = yield* all({ + sent: waitForDidRequest(eraseAction.id), + erase: waitForResponse( + BootloaderResponseActionType.Erase, + 5000, + ), + }); + if (!erase.response || erase.response.result) { // TODO: proper error handling throw Error(`Failed to erase: ${erase}`); } - const initAction = (yield put( - initRequest(nextMessageId(), firmware.length), - )) as BootloaderInitRequestAction; - const [, init] = (yield all([ - waitForDidRequest(initAction.id), - waitForResponse(BootloaderResponseActionType.Init), - ])) as [BootloaderDidRequestAction, WaitResponse]; - if (!init[0] || init[0].result) { + const initAction = yield* put(initRequest(nextMessageId(), firmware.length)); + const { init } = yield* all({ + sent: waitForDidRequest(initAction.id), + init: waitForResponse( + BootloaderResponseActionType.Init, + ), + }); + if (!init.response || init.response.result) { // TODO: proper error handling throw Error(`Failed to init: ${init}`); } // 14 is "safe" size for all hubs - const maxDataSize = MaxProgramFlashSize.get(info[0].hubType) || 14; + const maxDataSize = MaxProgramFlashSize.get(info.response.hubType) || 14; for (let count = 1, offset = 0; ; count++) { const payload = firmware.slice(offset, offset + maxDataSize); - const programAction = (yield put( + const programAction = yield* put( programRequest( nextMessageId(), - info[0].startAddress + offset, + info.response.startAddress + offset, payload.buffer, ), - )) as BootloaderProgramRequestAction; - yield waitForDidRequest(programAction.id); + ); + yield* waitForDidRequest(programAction.id); - yield put(didProgress(offset / firmware.length)); + yield* put(didProgress(offset / firmware.length)); // we don't want to request checksum if this is the last packet since // the bootloader will send a response to the program request already. @@ -348,46 +332,42 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // number of packets that can be queued in the Bluetooth chip on // the hub is not known and could vary by device. if (count % 10 === 0) { - const checksumAction = (yield put( - checksumRequest(nextMessageId()), - )) as BootloaderChecksumRequestAction; - const [, checksum] = (yield all([ - waitForDidRequest(checksumAction.id), - waitForResponse(BootloaderResponseActionType.Checksum, 5000), - ])) as [ - BootloaderDidRequestAction, - WaitResponse, - ]; - if (!checksum[0]) { + const checksumAction = yield* put(checksumRequest(nextMessageId())); + const { checksum } = yield* all({ + sent: waitForDidRequest(checksumAction.id), + checksum: waitForResponse( + BootloaderResponseActionType.Checksum, + 5000, + ), + }); + if (!checksum.response) { // TODO: proper error handling throw Error(`Failed to get checksum: ${checksum}`); } } } - const flash = (yield waitForResponse( + const flash = yield* waitForResponse( BootloaderResponseActionType.Program, 5000, - )) as WaitResponse; - if (!flash[0]) { + ); + if (!flash.response) { throw Error(`failed to get final response: ${flash}`); } - if (flash[0].count !== firmware.length) { + if (flash.response.count !== firmware.length) { // TODO: proper error handling throw Error("Didn't flash all bytes"); } - yield put(didProgress(1)); + yield* put(didProgress(1)); // this will cause the remote device to disconnect and reboot - const rebootAction = (yield put( - rebootRequest(nextMessageId()), - )) as BootloaderRebootRequestAction; - yield waitForDidRequest(rebootAction.id); + const rebootAction = yield* put(rebootRequest(nextMessageId())); + yield* waitForDidRequest(rebootAction.id); - yield put(didFinish()); + yield* put(didFinish()); } export default function* (): Generator { - yield takeEvery(FlashFirmwareActionType.FlashFirmware, flashFirmware); + yield* takeEvery(FlashFirmwareActionType.FlashFirmware, flashFirmware); } diff --git a/src/utils/index.test.ts b/src/utils/index.test.ts index 2d3fd7b4..39dd7be5 100644 --- a/src/utils/index.test.ts +++ b/src/utils/index.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2020 The Pybricks Authors -import { assert, hex, maybe } from '.'; +import { assert, defined, hex, maybe } from '.'; test('assert', () => { const assertTrue = jest.fn(() => assert(true, 'should not throw')); @@ -11,14 +11,21 @@ test('assert', () => { expect(() => assert(false, 'should throw')).toThrow(); }); +describe('defined', () => { + expect(() => defined('test')).not.toThrow(); + expect(() => defined(undefined)).toThrowError(); +}); + describe('maybe', () => { test('resolved', async () => { - const result = await maybe(Promise.resolve('test')); + const [result, error] = await maybe(Promise.resolve('test')); expect(result).toBe('test'); + expect(error).toBeUndefined(); }); test('rejected', async () => { - const result = await maybe(Promise.reject(new Error('test'))); - expect(result).toBeInstanceOf(Error); + const [result, error] = await maybe(Promise.reject(new Error('test'))); + expect(result).toBeUndefined(); + expect(error).toBeInstanceOf(Error); }); }); diff --git a/src/utils/index.ts b/src/utils/index.ts index ed26210f..60fee754 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -7,20 +7,29 @@ * @param condition A condition that is assumed to be true * @param message Informational message for debugging */ -export function assert(condition: boolean, message: string): void { +export function assert(condition: boolean, message: string): asserts condition { if (!condition) { throw Error(message); } } -export type Maybe = T | Error; +/** + * Asserts that an object is not undefined. This is used to make the type + * checker happy with `maybe()` and saga `race()` and `all()` effects where + * we have the condition "if A is undefined, then B is not undefined". + */ +export function defined(obj: T): asserts obj is NonNullable { + assert(obj !== undefined, 'undefined object'); +} + +export type Maybe = [T?, Error?]; /** Wraps a promise in try/catch and returns the promise result or error. */ export async function maybe(promise: Promise): Promise> { try { - return await promise; + return [await promise]; } catch (err) { - return err; + return [undefined, err]; } } diff --git a/yarn.lock b/yarn.lock index 8b8652dd..8dbc8900 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2138,6 +2138,20 @@ semver "^7.3.2" tsutils "^3.17.1" +"@typescript-eslint/eslint-plugin@^4.8.2": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-4.14.0.tgz#92db8e7c357ed7d69632d6843ca70b71be3a721d" + integrity sha512-IJ5e2W7uFNfg4qh9eHkHRUCbgZ8VKtGwD07kannJvM5t/GU8P8+24NX8gi3Hf5jST5oWPY8kyV1s/WtfiZ4+Ww== + dependencies: + "@typescript-eslint/experimental-utils" "4.14.0" + "@typescript-eslint/scope-manager" "4.14.0" + debug "^4.1.1" + functional-red-black-tree "^1.0.1" + lodash "^4.17.15" + regexpp "^3.0.0" + semver "^7.3.2" + tsutils "^3.17.1" + "@typescript-eslint/experimental-utils@4.13.0", "@typescript-eslint/experimental-utils@^4.0.1": version "4.13.0" resolved "https://registry.yarnpkg.com/@typescript-eslint/experimental-utils/-/experimental-utils-4.13.0.tgz#9dc9ab375d65603b43d938a0786190a0c72be44e" @@ -2150,6 +2164,18 @@ eslint-scope "^5.0.0" eslint-utils "^2.0.0" +"@typescript-eslint/experimental-utils@4.14.0", "@typescript-eslint/experimental-utils@^4.8.2", "@typescript-eslint/experimental-utils@^4.9.1": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/experimental-utils/-/experimental-utils-4.14.0.tgz#5aa7b006736634f588a69ee343ca959cd09988df" + integrity sha512-6i6eAoiPlXMKRbXzvoQD5Yn9L7k9ezzGRvzC/x1V3650rUk3c3AOjQyGYyF9BDxQQDK2ElmKOZRD0CbtdkMzQQ== + dependencies: + "@types/json-schema" "^7.0.3" + "@typescript-eslint/scope-manager" "4.14.0" + "@typescript-eslint/types" "4.14.0" + "@typescript-eslint/typescript-estree" "4.14.0" + eslint-scope "^5.0.0" + eslint-utils "^2.0.0" + "@typescript-eslint/experimental-utils@^3.10.1": version "3.10.1" resolved "https://registry.yarnpkg.com/@typescript-eslint/experimental-utils/-/experimental-utils-3.10.1.tgz#e179ffc81a80ebcae2ea04e0332f8b251345a686" @@ -2171,6 +2197,16 @@ "@typescript-eslint/typescript-estree" "4.13.0" debug "^4.1.1" +"@typescript-eslint/parser@^4.8.2": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/parser/-/parser-4.14.0.tgz#62d4cd2079d5c06683e9bfb200c758f292c4dee7" + integrity sha512-sUDeuCjBU+ZF3Lzw0hphTyScmDDJ5QVkyE21pRoBo8iDl7WBtVFS+WDN3blY1CH3SBt7EmYCw6wfmJjF0l/uYg== + dependencies: + "@typescript-eslint/scope-manager" "4.14.0" + "@typescript-eslint/types" "4.14.0" + "@typescript-eslint/typescript-estree" "4.14.0" + debug "^4.1.1" + "@typescript-eslint/scope-manager@4.13.0": version "4.13.0" resolved "https://registry.yarnpkg.com/@typescript-eslint/scope-manager/-/scope-manager-4.13.0.tgz#5b45912a9aa26b29603d8fa28f5e09088b947141" @@ -2179,6 +2215,14 @@ "@typescript-eslint/types" "4.13.0" "@typescript-eslint/visitor-keys" "4.13.0" +"@typescript-eslint/scope-manager@4.14.0": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/scope-manager/-/scope-manager-4.14.0.tgz#55a4743095d684e1f7b7180c4bac2a0a3727f517" + integrity sha512-/J+LlRMdbPh4RdL4hfP1eCwHN5bAhFAGOTsvE6SxsrM/47XQiPSgF5MDgLyp/i9kbZV9Lx80DW0OpPkzL+uf8Q== + dependencies: + "@typescript-eslint/types" "4.14.0" + "@typescript-eslint/visitor-keys" "4.14.0" + "@typescript-eslint/types@3.10.1": version "3.10.1" resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-3.10.1.tgz#1d7463fa7c32d8a23ab508a803ca2fe26e758727" @@ -2189,6 +2233,11 @@ resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-4.13.0.tgz#6a7c6015a59a08fbd70daa8c83dfff86250502f8" integrity sha512-/+aPaq163oX+ObOG00M0t9tKkOgdv9lq0IQv/y4SqGkAXmhFmCfgsELV7kOCTb2vVU5VOmVwXBXJTDr353C1rQ== +"@typescript-eslint/types@4.14.0": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/types/-/types-4.14.0.tgz#d8a8202d9b58831d6fd9cee2ba12f8a5a5dd44b6" + integrity sha512-VsQE4VvpldHrTFuVPY1ZnHn/Txw6cZGjL48e+iBxTi2ksa9DmebKjAeFmTVAYoSkTk7gjA7UqJ7pIsyifTsI4A== + "@typescript-eslint/typescript-estree@3.10.1": version "3.10.1" resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-3.10.1.tgz#fd0061cc38add4fad45136d654408569f365b853" @@ -2217,6 +2266,20 @@ semver "^7.3.2" tsutils "^3.17.1" +"@typescript-eslint/typescript-estree@4.14.0": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-4.14.0.tgz#4bcd67486e9acafc3d0c982b23a9ab8ac8911ed7" + integrity sha512-wRjZ5qLao+bvS2F7pX4qi2oLcOONIB+ru8RGBieDptq/SudYwshveORwCVU4/yMAd4GK7Fsf8Uq1tjV838erag== + dependencies: + "@typescript-eslint/types" "4.14.0" + "@typescript-eslint/visitor-keys" "4.14.0" + debug "^4.1.1" + globby "^11.0.1" + is-glob "^4.0.1" + lodash "^4.17.15" + semver "^7.3.2" + tsutils "^3.17.1" + "@typescript-eslint/visitor-keys@3.10.1": version "3.10.1" resolved "https://registry.yarnpkg.com/@typescript-eslint/visitor-keys/-/visitor-keys-3.10.1.tgz#cd4274773e3eb63b2e870ac602274487ecd1e931" @@ -2232,6 +2295,14 @@ "@typescript-eslint/types" "4.13.0" eslint-visitor-keys "^2.0.0" +"@typescript-eslint/visitor-keys@4.14.0": + version "4.14.0" + resolved "https://registry.yarnpkg.com/@typescript-eslint/visitor-keys/-/visitor-keys-4.14.0.tgz#b1090d9d2955b044b2ea2904a22496849acbdf54" + integrity sha512-MeHHzUyRI50DuiPgV9+LxcM52FCJFYjJiWHtXlbyC27b80mfOwKeiKI+MHOTEpcpfmoPFm/vvQS88bYIx6PZTA== + dependencies: + "@typescript-eslint/types" "4.14.0" + eslint-visitor-keys "^2.0.0" + "@webassemblyjs/ast@1.9.0": version "1.9.0" resolved "https://registry.yarnpkg.com/@webassemblyjs/ast/-/ast-1.9.0.tgz#bd850604b4042459a5a41cd7d338cbed695ed964" @@ -2681,7 +2752,7 @@ array.prototype.flat@^1.2.3: define-properties "^1.1.3" es-abstract "^1.18.0-next.1" -array.prototype.flatmap@^1.2.3: +array.prototype.flatmap@^1.2.3, array.prototype.flatmap@^1.2.4: version "1.2.4" resolved "https://registry.yarnpkg.com/array.prototype.flatmap/-/array.prototype.flatmap-1.2.4.tgz#94cfd47cc1556ec0747d97f7c7738c58122004c9" integrity sha512-r9Z0zYoxqHz60vvQbWEdXIEtCwHF0yxaWfno9qzXeNHvfyl3BZqygmGzb84dsubyaXLH4husF+NFgMSdpZhk2Q== @@ -2893,7 +2964,7 @@ babel-plugin-jest-hoist@^26.6.2: "@types/babel__core" "^7.0.0" "@types/babel__traverse" "^7.0.6" -babel-plugin-macros@2.8.0: +babel-plugin-macros@2.8.0, babel-plugin-macros@^2.8.0: version "2.8.0" resolved "https://registry.yarnpkg.com/babel-plugin-macros/-/babel-plugin-macros-2.8.0.tgz#0f958a7cc6556b1e65344465d99111a1e5e10138" integrity sha512-SEP5kJpfGYqYKpBrj5XU3ahw5p5GOHJ0U5ssOSQ/WBVdwkD2Dzlce95exQTs3jOVWPPKLBN2rlEWkCK7dSmLvg== @@ -2902,6 +2973,15 @@ babel-plugin-macros@2.8.0: cosmiconfig "^6.0.0" resolve "^1.12.0" +babel-plugin-macros@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/babel-plugin-macros/-/babel-plugin-macros-3.0.1.tgz#0d412d68f5b3d1b64358f24ab099bd148724e2a9" + integrity sha512-CKt4+Oy9k2wiN+hT1uZzOw7d8zb1anbQpf7KLwaaXRCi/4pzKdFKHf7v5mvoPmjkmxshh7eKZQuRop06r5WP4w== + dependencies: + "@babel/runtime" "^7.12.5" + cosmiconfig "^7.0.0" + resolve "^1.19.0" + babel-plugin-named-asset-import@^0.3.7: version "0.3.7" resolved "https://registry.yarnpkg.com/babel-plugin-named-asset-import/-/babel-plugin-named-asset-import-0.3.7.tgz#156cd55d3f1228a5765774340937afc8398067dd" @@ -4861,6 +4941,11 @@ escape-string-regexp@^1.0.2, escape-string-regexp@^1.0.5: resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4" integrity sha1-G2HAViGQqN/2rjuyzwIAyhMLhtQ= +escape-string-regexp@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz#14ba83a5d373e3d311e5afca29cf5bfad965bf34" + integrity sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA== + escodegen@^1.14.1: version "1.14.3" resolved "https://registry.yarnpkg.com/escodegen/-/escodegen-1.14.3.tgz#4e7b81fba61581dc97582ed78cab7f0e8d63f503" @@ -4885,6 +4970,11 @@ eslint-config-react-app@^6.0.0: dependencies: confusing-browser-globals "^1.0.10" +eslint-config-typed-fp@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/eslint-config-typed-fp/-/eslint-config-typed-fp-1.3.0.tgz#ca62050793a80c0b9af6f370e925797fa4c243f9" + integrity sha512-I6+/szKXAbZQ23pCjVAoqaM0AtYXIfo40QrLKHFfZ/Fh+ROnd2vOawCJzgYfIuPZrjbVbnlsLuM8LTK19YihYA== + eslint-import-resolver-node@^0.3.4: version "0.3.4" resolved "https://registry.yarnpkg.com/eslint-import-resolver-node/-/eslint-import-resolver-node-0.3.4.tgz#85ffa81942c25012d8231096ddf679c03042c717" @@ -4909,6 +4999,17 @@ eslint-plugin-flowtype@^5.2.0: lodash "^4.17.15" string-natural-compare "^3.0.1" +eslint-plugin-functional@^3.2.1: + version "3.2.1" + resolved "https://registry.yarnpkg.com/eslint-plugin-functional/-/eslint-plugin-functional-3.2.1.tgz#d5ad668b57646ad24f4ef0476328408681d59061" + integrity sha512-uJ8W0FznWsKp4exxO79b0xSc1WNROzDiVNGgSFOwdZCBeUHQf89BqwqlshNW9aSz/kg2gVGs+Ue6AeTpNSFM/g== + dependencies: + "@typescript-eslint/experimental-utils" "^4.9.1" + array.prototype.flatmap "^1.2.4" + deepmerge "^4.2.2" + escape-string-regexp "^4.0.0" + object.fromentries "^2.0.3" + eslint-plugin-import@^2.22.1: version "2.22.1" resolved "https://registry.yarnpkg.com/eslint-plugin-import/-/eslint-plugin-import-2.22.1.tgz#0896c7e6a0cf44109a2d97b95903c2bb689d7702" @@ -4988,6 +5089,16 @@ eslint-plugin-testing-library@^3.9.2: dependencies: "@typescript-eslint/experimental-utils" "^3.10.1" +eslint-plugin-total-functions@^4.7.2: + version "4.7.2" + resolved "https://registry.yarnpkg.com/eslint-plugin-total-functions/-/eslint-plugin-total-functions-4.7.2.tgz#e60801da31e1f0e30a2d28b42921ab1f72cb66de" + integrity sha512-NG0Is/W+l9vGMbo6wABGGw00Wl6VR0JdN99u28bTKfRzisxnRFJGR/i12jFgQqJHxPxGxC+lLxZ/E5NtwOjo+A== + dependencies: + "@typescript-eslint/eslint-plugin" "^4.8.2" + "@typescript-eslint/experimental-utils" "^4.8.2" + "@typescript-eslint/parser" "^4.8.2" + tsutils "^3.17.1" + eslint-scope@^4.0.3: version "4.0.3" resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-4.0.3.tgz#ca03833310f6889a3264781aa82e63eb9cfe7848" @@ -8386,7 +8497,7 @@ object.entries@^1.1.0, object.entries@^1.1.2: es-abstract "^1.18.0-next.1" has "^1.0.3" -object.fromentries@^2.0.2: +object.fromentries@^2.0.2, object.fromentries@^2.0.3: version "2.0.3" resolved "https://registry.yarnpkg.com/object.fromentries/-/object.fromentries-2.0.3.tgz#13cefcffa702dc67750314a3305e8cb3fad1d072" integrity sha512-IDUSMXs6LOSJBWE++L0lzIbSqHl9KDCfff2x/JSEIDtEUavUnyMYC2ZGay/04Zq4UT8lvd4xNhU4/YHKibAOlw== @@ -10417,7 +10528,7 @@ resolve@1.18.1: is-core-module "^2.0.0" path-parse "^1.0.6" -resolve@^1.10.0, resolve@^1.12.0, resolve@^1.13.1, resolve@^1.14.2, resolve@^1.17.0, resolve@^1.18.1, resolve@^1.3.2, resolve@^1.8.1: +resolve@^1.10.0, resolve@^1.12.0, resolve@^1.13.1, resolve@^1.14.2, resolve@^1.17.0, resolve@^1.18.1, resolve@^1.19.0, resolve@^1.3.2, resolve@^1.8.1: version "1.19.0" resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.19.0.tgz#1af5bf630409734a067cae29318aac7fa29a267c" integrity sha512-rArEXAgsBG4UgRGcynxWIWKFvh/XZCcS8UJdHhwy91zwAvCZIbcs+vAbflgBnNjYMs/i/i+/Ux6IZhML1yPvxg== @@ -11855,6 +11966,14 @@ type@^2.0.0: resolved "https://registry.yarnpkg.com/type/-/type-2.1.0.tgz#9bdc22c648cf8cf86dd23d32336a41cfb6475e3f" integrity sha512-G9absDWvhAWCV2gmF1zKud3OyC61nZDwWvBL2DApaVFogI07CprggiQAOOjvp2NRjYWFzPyu7vwtDrQFq8jeSA== +typed-redux-saga@^1.3.1: + version "1.3.1" + resolved "https://registry.yarnpkg.com/typed-redux-saga/-/typed-redux-saga-1.3.1.tgz#92b01db41e3510102f87eb9ff261ec73d38a2e44" + integrity sha512-nUj1/1/SAesEsZrr7o24ID+++CqZ6QfPVDcwhY2rVmm4vEBr/vbDHJ6j/w6SomOcooLwnh3sdaWVhNEIy7VgNA== + optionalDependencies: + "@babel/helper-module-imports" "^7.12.1" + babel-plugin-macros "^2.8.0" + typed-styles@^0.0.7: version "0.0.7" resolved "https://registry.yarnpkg.com/typed-styles/-/typed-styles-0.0.7.tgz#93392a008794c4595119ff62dde6809dbc40a3d9" From e1932febb2a1454bfa4fccb7e7fdd0cbffcd14eb Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 22:25:13 -0600 Subject: [PATCH 16/18] drop CurrentEditSession type It is better to not hide the nullability. --- src/reducers/editor.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/reducers/editor.ts b/src/reducers/editor.ts index 111869de..593d9ebb 100644 --- a/src/reducers/editor.ts +++ b/src/reducers/editor.ts @@ -6,9 +6,7 @@ import { Reducer, combineReducers } from 'redux'; import { Action } from '../actions'; import { EditorActionType } from '../actions/editor'; -type CurrentEditSession = Ace.EditSession | null; - -const current: Reducer = (state = null, action) => { +const current: Reducer = (state = null, action) => { switch (action.type) { case EditorActionType.Current: return action.editSession || null; @@ -18,6 +16,6 @@ const current: Reducer = (state = null, action) => { }; export interface EditorState { - current: CurrentEditSession; + current: Ace.EditSession | null; } export default combineReducers({ current }); From caacaad341bde0366b49b9f8045813a47bd373f5 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 21 Jan 2021 22:35:39 -0600 Subject: [PATCH 17/18] use recursive partial so we don't have to use "as" --- src/sagas/flash-firmware.test.ts | 16 +++++----------- src/sagas/license.test.ts | 8 ++++---- src/sagas/settings.test.ts | 9 ++++----- test/index.ts | 8 ++++++-- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 25ed6d58..cf63d4d6 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -35,8 +35,6 @@ import { } from '../actions/lwp3-bootloader'; import { didCompile } from '../actions/mpy'; import { HubType, Result } from '../protocols/lwp3-bootloader'; -import { EditorState } from '../reducers/editor'; -import { SettingsState } from '../reducers/settings'; import { createCountFunc } from '../utils/iter'; import flashFirmware from './flash-firmware'; @@ -69,7 +67,7 @@ describe('flashFirmware', () => { const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); - saga.setState({ settings: { flashCurrentProgram: false } as SettingsState }); + saga.setState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -199,9 +197,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ - settings: { flashCurrentProgram: false } as SettingsState, - }); + saga.setState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -332,9 +328,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ - settings: { flashCurrentProgram: false } as SettingsState, - }); + saga.setState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -387,8 +381,8 @@ describe('flashFirmware', () => { const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); saga.setState({ - editor: { current: editor } as EditorState, - settings: { flashCurrentProgram: true } as SettingsState, + editor: { current: editor }, + settings: { flashCurrentProgram: true }, }); // saga is triggered by this action diff --git a/src/sagas/license.test.ts b/src/sagas/license.test.ts index 609072f3..57d993cc 100644 --- a/src/sagas/license.test.ts +++ b/src/sagas/license.test.ts @@ -6,7 +6,7 @@ import { AsyncSaga, delay } from '../../test'; import { openLicenseDialog } from '../actions/app'; import { didFailToFetchList, didFetchList } from '../actions/license'; -import { LicenseList, LicenseState } from '../reducers/license'; +import { LicenseList } from '../reducers/license'; import license from './license'; afterAll(() => { @@ -24,7 +24,7 @@ describe('fetchLicenses', () => { // initially, license list starts as null, so fetch is called to get // the list - saga.setState({ license: { list: null } as LicenseState }); + saga.setState({ license: { list: null } }); saga.put(openLicenseDialog()); const action = await saga.take(); @@ -42,7 +42,7 @@ describe('fetchLicenses', () => { // after we have the list, we don't fetch it again since it will // always be the same list - saga.setState({ license: { list: testLicenseList } as LicenseState }); + saga.setState({ license: { list: testLicenseList } }); saga.put(openLicenseDialog()); // have to yield to be sure fetch call would have taken place on error @@ -56,7 +56,7 @@ describe('fetchLicenses', () => { jest.spyOn(globalThis, 'fetch').mockResolvedValue(failResponse); - saga.setState({ license: { list: null } as LicenseState }); + saga.setState({ license: { list: null } }); saga.put(openLicenseDialog()); const action = await saga.take(); diff --git a/src/sagas/settings.test.ts b/src/sagas/settings.test.ts index 6e0edb58..8e14d15b 100644 --- a/src/sagas/settings.test.ts +++ b/src/sagas/settings.test.ts @@ -6,7 +6,6 @@ import { AsyncSaga } from '../../test'; import { didStart } from '../actions/app'; import { didBooleanChange, didFailToSetBoolean, setBoolean } from '../actions/settings'; -import { SettingsState } from '../reducers/settings'; import { SettingId } from '../settings/user'; import settings from './settings'; @@ -221,7 +220,7 @@ describe('store settings to local storage', () => { throw testError; }); - saga.setState({ settings: { showDocs: false } as SettingsState }); + saga.setState({ settings: { showDocs: false } }); saga.put(setBoolean(SettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); @@ -246,7 +245,7 @@ describe('store settings to local storage', () => { expect(value).toBe('true'); }); - saga.setState({ settings: { showDocs: false } as SettingsState }); + saga.setState({ settings: { showDocs: false } }); saga.put(setBoolean(SettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); @@ -266,7 +265,7 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.setState({ settings: { darkMode: true } as SettingsState }); + saga.setState({ settings: { darkMode: true } }); saga.put(setBoolean(SettingId.DarkMode, false)); expect(mockSetItem).toHaveBeenCalled(); @@ -286,7 +285,7 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.setState({ settings: { flashCurrentProgram: true } as SettingsState }); + saga.setState({ settings: { flashCurrentProgram: true } }); saga.put(setBoolean(SettingId.FlashCurrentProgram, false)); expect(mockSetItem).toHaveBeenCalled(); diff --git a/test/index.ts b/test/index.ts index e5a7c588..b1833718 100644 --- a/test/index.ts +++ b/test/index.ts @@ -5,11 +5,15 @@ import { END, MulticastChannel, Saga, Task, runSaga, stdChannel } from 'redux-sa import { Action } from '../src/actions'; import { RootState } from '../src/reducers'; +type RecursivePartial = { + [P in keyof T]?: RecursivePartial; +}; + export class AsyncSaga { private channel: MulticastChannel; private dispatches: (Action | END)[]; private takers: { put: (action: Action | END) => void }[]; - private state: Partial; + private state: RecursivePartial; private task: Task; public constructor(saga: Saga, context?: Record) { @@ -63,7 +67,7 @@ export class AsyncSaga { return Promise.resolve(next); } - public setState(state: Partial): void { + public setState(state: RecursivePartial): void { this.state = state; } From 9551bdc7f091bb3fd92f75d2e6589015327e2f12 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 10:28:04 -0600 Subject: [PATCH 18/18] Pybricks firmware v3.0.0b1 --- craco.config.js | 24 ------------------------ package.json | 2 +- yarn.lock | 8 ++++---- 3 files changed, 5 insertions(+), 29 deletions(-) diff --git a/craco.config.js b/craco.config.js index 05312466..636eb660 100644 --- a/craco.config.js +++ b/craco.config.js @@ -64,29 +64,6 @@ OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.`; } -const pybricksLicense = `MIT License - -Copyright (c) 2018-2021 The Pybricks Authors - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. -`; - const shopifyLicense = `MIT License Copyright (c) 2021 Shopify @@ -110,7 +87,6 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.`; const licenseTextOverrides = { - '@pybricks/firmware': pybricksLicense, '@shopify/dates': shopifyLicense, '@shopify/decorators': shopifyLicense, '@shopify/function-enhancers': shopifyLicense, diff --git a/package.json b/package.json index 589073ec..ed5ec5f5 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "dependencies": { "@blueprintjs/core": "^3.36.0", "@craco/craco": "^6.0.0", - "@pybricks/firmware": "4.4.0", + "@pybricks/firmware": "4.5.0", "@pybricks/mpy-cross-v5": "^2.0.0", "@shopify/react-i18n": "^5.2.0", "@testing-library/dom": "^7.29.2", diff --git a/yarn.lock b/yarn.lock index 8dbc8900..65b23dbd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1455,10 +1455,10 @@ schema-utils "^2.6.5" source-map "^0.7.3" -"@pybricks/firmware@4.4.0": - version "4.4.0" - resolved "https://registry.yarnpkg.com/@pybricks/firmware/-/firmware-4.4.0.tgz#bdb7fd5476b914533b09d796fb2d98ec1e911ae1" - integrity sha512-le6EgkipT74D5lmi47FJa2B/Jms4XuRbh6ReoUH9bgaW5TolmE4wwN67uwA2c3fVY16ApCMe9dZoj/5y6ai50w== +"@pybricks/firmware@4.5.0": + version "4.5.0" + resolved "https://registry.yarnpkg.com/@pybricks/firmware/-/firmware-4.5.0.tgz#e1e46da2000e2d4319d19ac0a4d21e3b8040ad0a" + integrity sha512-GsV+mTeUkR3RAK+HlBwJe+gifG5X4UFgjXxsB85ykd83g701b8ORP9M44fNy29fj/ZsOwKTz+MpyibbE6NgZXA== dependencies: jszip "^3.5.0"