From a88fa3d425ac07a0f6c8c9de8051351af1bbe3f2 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 14:30:56 -0600 Subject: [PATCH] consolidate firmware fail to start and finish they had too much overlap --- src/actions/flash-firmware.ts | 313 ++++++++++--------------------- src/sagas/flash-firmware.test.ts | 30 +-- src/sagas/flash-firmware.ts | 49 ++--- 3 files changed, 135 insertions(+), 257 deletions(-) diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index cb0dbe87..3435bb91 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -11,10 +11,8 @@ import { assert } from '../utils'; export enum FlashFirmwareActionType { /** Request to flash new firmware to the device. */ FlashFirmware = 'flashFirmware.action.flashFirmware', - /** Flashing started. */ + /** Actual modification of the flash memory on the device 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. */ @@ -43,99 +41,39 @@ function isHubError(arg: unknown): arg is HubError { return Object.keys(HubError).includes(arg); } -type Reason = { - reason: T; -}; - -export enum FailToStartReasonType { - /** Connecting to the hub failed. */ - FailedToConnect = 'flashFirmware.failToStart.reason.failedToConnect', - /** The hub connection timed out. */ - TimedOut = 'flashFirmware.failToStart.reason.timedOut', - /** Something went wrong with the BLE connection. */ - BleError = 'flashFirmware.failToStart.reason.bleError', - /** The hub was disconnected. */ - Disconnected = 'flashFirmware.failToStart.reason.disconnected', - /** The hub sent a response indicating a problem. */ - HubError = 'flashFirmware.failToStart.reason.hubError', - /** 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 FailToStartReasonTimedOut = Reason; - -export type FailToStartReasonBleError = Reason & { - err: Error; -}; - -export type FailToStartReasonDisconnected = Reason; - -export type FailToStartReasonHubError = Reason & { - hubError: HubError; -}; - -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 - | FailToStartReasonTimedOut - | FailToStartReasonBleError - | FailToStartReasonDisconnected - | FailToStartReasonHubError - | FailToStartReasonNoFirmware - | FailToStartReasonDeviceMismatch - | FailToStartReasonZipError - | FailToStartReasonBadMetadata - | FailToStartReasonFirmwareSize - | FailToStartReasonFailedToCompile - | FailToStartReasonUnknown; - export enum FailToFinishReasonType { - /** Waiting for a response from the hub took too long. */ + /** Connecting to the hub failed. */ + FailedToConnect = 'flashFirmware.failToFinish.reason.failedToConnect', + /** The hub connection timed out. */ 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. */ + /** The hub was disconnected. */ Disconnected = 'flashFirmware.failToFinish.reason.disconnected', /** The hub sent a response indicating a problem. */ HubError = 'flashFirmware.failToFinish.reason.hubError', + /** The is no firmware available that matches the connected hub. */ + NoFirmware = 'flashFirmware.failToFinish.reason.noFirmware', + /** The provided firmware.zip does not match the connected hub. */ + DeviceMismatch = 'flashFirmware.failToFinish.reason.deviceMismatch', + /** There was a problem with the zip file. */ + ZipError = 'flashFirmware.failToFinish.reason.zipError', + /** Metadata property is missing or invalid. */ + BadMetadata = 'flashFirmware.failToFinish.reason.badMetadata', + /** The main.py file failed to compile. */ + FailedToCompile = 'flashFirmware.failToFinish.reason.failedToCompile', + /** The combined firmware-base.bin and main.mpy are too big. */ + FirmwareSize = 'flashFirmware.failToFinish.reason.firmwareSize', /** An unexpected error occurred. */ Unknown = 'flashFirmware.failToFinish.reason.unknown', } +type Reason = { + reason: T; +}; + +export type FailToFinishReasonFailedToConnect = Reason; + export type FailToFinishReasonTimedOut = Reason; export type FailToFinishReasonBleError = Reason & { @@ -148,15 +86,39 @@ export type FailToFinishReasonHubError = Reason hubError: HubError; }; +export type FailToFinishReasonNoFirmware = Reason; + +export type FailToFinishReasonDeviceMismatch = Reason; + +export type FailToFinishReasonZipError = Reason & { + err: FirmwareReaderError; +}; + +export type FailToFinishReasonBadMetadata = Reason & { + property: keyof FirmwareMetadata; + problem: MetadataProblem; +}; + +export type FailToFinishReasonFirmwareSize = Reason; + +export type FailToFinishReasonFailedToCompile = Reason; + export type FailToFinishReasonUnknown = Reason & { err: Error; }; export type FailToFinishReason = + | FailToFinishReasonFailedToConnect | FailToFinishReasonTimedOut | FailToFinishReasonBleError | FailToFinishReasonDisconnected | FailToFinishReasonHubError + | FailToFinishReasonNoFirmware + | FailToFinishReasonDeviceMismatch + | FailToFinishReasonZipError + | FailToFinishReasonBadMetadata + | FailToFinishReasonFirmwareSize + | FailToFinishReasonFailedToCompile | FailToFinishReasonUnknown; /** @@ -186,128 +148,6 @@ 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.BleError, - err: Error, -): FlashFirmwareDidFailToStartAction; - -export function didFailToStart( - reason: FailToStartReasonType.HubError, - hubError: HubError, -): FlashFirmwareDidFailToStartAction; - -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.BleError - | FailToStartReasonType.HubError - | 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 | HubError | Error, - arg2?: MetadataProblem, -): FlashFirmwareDidFailToStartAction { - if (reason === FailToStartReasonType.BleError) { - // 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 }, - }; - } - - if (reason === FailToStartReasonType.HubError) { - // istanbul ignore if: programmer error give wrong arg - if (!isHubError(arg1)) { - throw new Error('missing or invalid hubError'); - } - return { - type: FlashFirmwareActionType.DidFailToStart, - reason: { reason, hubError: arg1 }, - }; - } - - 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). */ @@ -346,6 +186,17 @@ export function didFailToFinish( hubError: HubError, ): FlashFirmwareDidFailToFinishAction; +export function didFailToFinish( + reason: FailToFinishReasonType.ZipError, + err: FirmwareReaderError, +): FlashFirmwareDidFailToFinishAction; + +export function didFailToFinish( + reason: FailToFinishReasonType.BadMetadata, + property: keyof FirmwareMetadata, + problem: MetadataProblem, +): FlashFirmwareDidFailToFinishAction; + export function didFailToFinish( reason: FailToFinishReasonType.Unknown, err: Error, @@ -356,14 +207,20 @@ export function didFailToFinish( FailToFinishReasonType, | FailToFinishReasonType.BleError | FailToFinishReasonType.HubError + | FailToFinishReasonType.ZipError + | FailToFinishReasonType.BadMetadata | FailToFinishReasonType.Unknown >, ): FlashFirmwareDidFailToFinishAction; -/** Action that indicates that flashing failed. */ +/** + * Action that indicates flashing did not start because of an error. + * @param total The total number of bytes to be flashed. + */ export function didFailToFinish( reason: FailToFinishReasonType, - arg1?: HubError | Error, + arg1?: string | HubError | Error, + arg2?: MetadataProblem, ): FlashFirmwareDidFailToFinishAction { if (reason === FailToFinishReasonType.BleError) { // istanbul ignore if: programmer error give wrong arg @@ -379,7 +236,7 @@ export function didFailToFinish( if (reason === FailToFinishReasonType.HubError) { // istanbul ignore if: programmer error give wrong arg if (!isHubError(arg1)) { - throw new Error('missing or invalid err'); + throw new Error('missing or invalid hubError'); } return { type: FlashFirmwareActionType.DidFailToFinish, @@ -387,6 +244,41 @@ export function didFailToFinish( }; } + if (reason === FailToFinishReasonType.ZipError) { + // istanbul ignore if: programmer error give wrong arg + if (!(arg1 instanceof FirmwareReaderError)) { + throw new Error('missing or invalid err'); + } + return { + type: FlashFirmwareActionType.DidFailToFinish, + reason: { reason, err: arg1 }, + }; + } + + if (reason === FailToFinishReasonType.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.DidFailToFinish, + reason: { reason, property: arg1, problem: arg2 }, + }; + } + if (reason === FailToFinishReasonType.Unknown) { // istanbul ignore if: programmer error give wrong arg if (!(arg1 instanceof Error)) { @@ -407,7 +299,6 @@ export function didFailToFinish( export type FlashFirmwareAction = | FlashFirmwareFlashAction | FlashFirmwareDidStartAction - | FlashFirmwareDidFailToStartAction | FlashFirmwareDidProgressAction | FlashFirmwareDidFinishAction | FlashFirmwareDidFailToFinishAction; diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 6fcbfd01..f0e95a85 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -9,9 +9,9 @@ import { import JSZip from 'jszip'; import { AsyncSaga } from '../../test'; import { - FailToStartReasonType, + FailToFinishReasonType, MetadataProblem, - didFailToStart, + didFailToFinish, didFinish, didProgress, didStart, @@ -235,7 +235,7 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual( - didFailToStart(FailToStartReasonType.FailedToConnect), + didFailToFinish(FailToFinishReasonType.FailedToConnect), ); await saga.end(); @@ -294,7 +294,9 @@ describe('flashFirmware', () => { // should get a failure to start action = await saga.take(); - expect(action).toEqual(didFailToStart(FailToStartReasonType.Disconnected)); + expect(action).toEqual( + didFailToFinish(FailToFinishReasonType.Disconnected), + ); await saga.end(); }); @@ -350,7 +352,7 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual( - didFailToStart(FailToStartReasonType.BleError, testError), + didFailToFinish(FailToFinishReasonType.BleError, testError), ); // should request to disconnect after failure @@ -536,8 +538,8 @@ describe('flashFirmware', () => { const action = await saga.take(); expect(action).toStrictEqual( - didFailToStart( - FailToStartReasonType.ZipError, + didFailToFinish( + FailToFinishReasonType.ZipError, new FirmwareReaderError( FirmwareReaderErrorCode.MissingFirmwareBaseBin, ), @@ -581,8 +583,8 @@ describe('flashFirmware', () => { const action = await saga.take(); expect(action).toStrictEqual( - didFailToStart( - FailToStartReasonType.BadMetadata, + didFailToFinish( + FailToFinishReasonType.BadMetadata, 'mpy-abi-version', MetadataProblem.NotSupported, ), @@ -640,7 +642,7 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual( - didFailToStart(FailToStartReasonType.FailedToCompile), + didFailToFinish(FailToFinishReasonType.FailedToCompile), ); await saga.end(); @@ -696,7 +698,9 @@ describe('flashFirmware', () => { // should fail due to firmware being too big action = await saga.take(); - expect(action).toEqual(didFailToStart(FailToStartReasonType.FirmwareSize)); + expect(action).toEqual( + didFailToFinish(FailToFinishReasonType.FirmwareSize), + ); await saga.end(); }); @@ -753,8 +757,8 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual( - didFailToStart( - FailToStartReasonType.BadMetadata, + didFailToFinish( + FailToFinishReasonType.BadMetadata, 'checksum-type', MetadataProblem.NotSupported, ), diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index b0c1f95d..29e90597 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -22,13 +22,11 @@ import { import { Action } from '../actions'; import { FailToFinishReasonType, - FailToStartReasonType, FlashFirmwareActionType, FlashFirmwareFlashAction, HubError, MetadataProblem, didFailToFinish, - didFailToStart, didFinish, didProgress, didStart, @@ -79,7 +77,7 @@ function* waitForDidRequest(id: number): SagaGenerator a.type === BootloaderDidRequestType && a.id === id, ); if (request.err) { - yield* put(didFailToStart(FailToStartReasonType.BleError, request.err)); + yield* put(didFailToFinish(FailToFinishReasonType.BleError, request.err)); yield* put(disconnect()); yield* cancel(); } @@ -104,14 +102,14 @@ function* waitForResponse( }); if (timedOut) { - yield* put(didFailToStart(FailToStartReasonType.TimedOut)); + yield* put(didFailToFinish(FailToFinishReasonType.TimedOut)); yield* put(disconnect()); yield* cancel(); } if (error) { yield* put( - didFailToStart(FailToStartReasonType.HubError, HubError.UnknownCommand), + didFailToFinish(FailToFinishReasonType.HubError, HubError.UnknownCommand), ); yield* put(disconnect()); cancel(); @@ -136,8 +134,6 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator { /** * Loads Pybricks firmware from a .zip file. * - * This can raise didFailToStart() actions, so don't call this after didStart(). - * * @param data The zip file raw data * @param program User program or `undefined` to use main.py from firmware.zip */ @@ -150,9 +146,9 @@ function* loadFirmware( if (readerErr) { // istanbul ignore else: unexpected error if (readerErr instanceof FirmwareReaderError) { - yield* put(didFailToStart(FailToStartReasonType.ZipError, readerErr)); + yield* put(didFailToFinish(FailToFinishReasonType.ZipError, readerErr)); } else { - yield* put(didFailToStart(FailToStartReasonType.Unknown, readerErr)); + yield* put(didFailToFinish(FailToFinishReasonType.Unknown, readerErr)); } yield* cancel(); } @@ -169,8 +165,8 @@ function* loadFirmware( if (metadata['mpy-abi-version'] !== 5) { yield* put( - didFailToStart( - FailToStartReasonType.BadMetadata, + didFailToFinish( + FailToFinishReasonType.BadMetadata, 'mpy-abi-version', MetadataProblem.NotSupported, ), @@ -185,7 +181,7 @@ function* loadFirmware( }); if (mpyFail) { - yield* put(didFailToStart(FailToStartReasonType.FailedToCompile)); + yield* put(didFailToFinish(FailToFinishReasonType.FailedToCompile)); yield* cancel(); } @@ -199,7 +195,7 @@ function* loadFirmware( const firmwareView = new DataView(firmware.buffer); if (firmware.length > metadata['max-firmware-size']) { - yield* put(didFailToStart(FailToStartReasonType.FirmwareSize)); + yield* put(didFailToFinish(FailToFinishReasonType.FirmwareSize)); yield* cancel(); } @@ -209,8 +205,8 @@ function* loadFirmware( if (metadata['checksum-type'] !== 'sum') { yield* put( - didFailToStart( - FailToStartReasonType.BadMetadata, + didFailToFinish( + FailToFinishReasonType.BadMetadata, 'checksum-type', MetadataProblem.NotSupported, ), @@ -232,29 +228,16 @@ function* loadFirmware( * action is raised and the task (including the parent task) is canceled. */ function* disconnectMonitor(): SagaGenerator { - const { disconnectedBeforeStart } = yield* race({ - disconnectedBeforeStart: take(BootloaderConnectionActionType.DidDisconnect), - started: take(FlashFirmwareActionType.DidStart), - }); - - if (disconnectedBeforeStart) { - yield* put(didFailToStart(FailToStartReasonType.Disconnected)); - yield* cancel(); - } - - // if we get here, `started` won the race - - const { disconnectedAfterStart } = yield* race({ - disconnectedAfterStart: take(BootloaderConnectionActionType.DidDisconnect), + const { disconnected } = yield* race({ + disconnected: take(BootloaderConnectionActionType.DidDisconnect), finished: take(FlashFirmwareActionType.DidFinish), + failedToFinish: take(FlashFirmwareActionType.DidFailToFinish), }); - if (disconnectedAfterStart) { + if (disconnected) { yield* put(didFailToFinish(FailToFinishReasonType.Disconnected)); yield* cancel(); } - - // if we get here, `finished` won the race. } /** @@ -294,7 +277,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { ]); if (connectResult.type === BootloaderConnectionActionType.DidFailToConnect) { - yield* put(didFailToStart(FailToStartReasonType.FailedToConnect)); + yield* put(didFailToFinish(FailToFinishReasonType.FailedToConnect)); return; }