From b1efa049476f283b824c40e908279ec2715f11d1 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 11:38:26 -0600 Subject: [PATCH 01/32] proper error handling for mpy-cross ABI version --- src/sagas/flash-firmware.test.ts | 45 ++++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 10 +++++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index cf63d4d6..d03bb64b 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -10,6 +10,7 @@ import JSZip from 'jszip'; import { AsyncSaga } from '../../test'; import { FailToStartReasonType, + MetadataProblem, didFailToStart, didFinish, didProgress, @@ -350,6 +351,50 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('unsupported mpy-cross version', 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': 4, // unsupported version + '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 } }); + + // saga is triggered by this action + + saga.put( + flashFirmwareAction(await zip.generateAsync({ type: 'arraybuffer' })), + ); + + // should get failure due to unsupported mpy-cross version + + const action = await saga.take(); + expect(action).toStrictEqual( + didFailToStart( + FailToStartReasonType.BadMetadata, + 'mpy-abi-version', + MetadataProblem.NotSupported, + ), + ); + + await saga.end(); + }); }); test('user supplied main.py', async () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index c6bc1ff1..6e368b8b 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -23,6 +23,7 @@ import { FailToStartReasonType, FlashFirmwareActionType, FlashFirmwareFlashAction, + MetadataProblem, didFailToStart, didFinish, didProgress, @@ -139,9 +140,14 @@ function* loadFirmware( } if (metadata['mpy-abi-version'] !== 5) { - throw Error( - `Firmware requires mpy-cross ABI version ${metadata['mpy-abi-version']} we have v5`, + yield* put( + didFailToStart( + FailToStartReasonType.BadMetadata, + 'mpy-abi-version', + MetadataProblem.NotSupported, + ), ); + yield* cancel(); } yield* put(compile(program, metadata['mpy-cross-options'])); From e229f08ffdb3cf215a2a9ca920ca4615ffdbc43f Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 11:41:46 -0600 Subject: [PATCH 02/32] switch to inline snapshots in tests --- .../__snapshots__/flash-firmware.test.ts.snap | 31 ------------------- src/sagas/__snapshots__/mpy.test.ts.snap | 9 ------ src/sagas/flash-firmware.test.ts | 30 ++++++++++++++++-- src/sagas/mpy.test.ts | 8 ++++- 4 files changed, 34 insertions(+), 44 deletions(-) delete mode 100644 src/sagas/__snapshots__/flash-firmware.test.ts.snap delete mode 100644 src/sagas/__snapshots__/mpy.test.ts.snap diff --git a/src/sagas/__snapshots__/flash-firmware.test.ts.snap b/src/sagas/__snapshots__/flash-firmware.test.ts.snap deleted file mode 100644 index 77e153cd..00000000 --- a/src/sagas/__snapshots__/flash-firmware.test.ts.snap +++ /dev/null @@ -1,31 +0,0 @@ -// 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 success 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/__snapshots__/mpy.test.ts.snap b/src/sagas/__snapshots__/mpy.test.ts.snap deleted file mode 100644 index 1e4167cd..00000000 --- a/src/sagas/__snapshots__/mpy.test.ts.snap +++ /dev/null @@ -1,9 +0,0 @@ -// 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.test.ts b/src/sagas/flash-firmware.test.ts index d03bb64b..5fd20359 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -92,7 +92,15 @@ describe('flashFirmware', () => { // then compile main.py to .mpy action = await saga.take(); - expect(action).toMatchSnapshot(); + expect(action).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); @@ -209,7 +217,15 @@ describe('flashFirmware', () => { // the first step is to compile main.py to .mpy let action = await saga.take(); - expect(action).toMatchSnapshot(); + expect(action).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); @@ -452,7 +468,15 @@ describe('flashFirmware', () => { // then compile main.py to .mpy action = await saga.take(); - expect(action).toMatchSnapshot(); + expect(action).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); diff --git a/src/sagas/mpy.test.ts b/src/sagas/mpy.test.ts index 141a0f62..1ca2232d 100644 --- a/src/sagas/mpy.test.ts +++ b/src/sagas/mpy.test.ts @@ -37,7 +37,13 @@ test('compiler error works', async () => { const action = await saga.take(); expect(action.type).toBe(MpyActionType.DidFailToCompile); const { err } = action as MpyDidFailToCompileAction; - expect(err).toMatchSnapshot(); + expect(err).toMatchInlineSnapshot(` + Array [ + "Traceback (most recent call last):", + " File \\"main.py\\", line 1", + "SyntaxError: invalid syntax", + ] + `); await saga.end(); }); From 4707b77135698d7e7c549db3148e7943a4a9b1f4 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 11:49:51 -0600 Subject: [PATCH 03/32] proper error handling for compiler error --- src/sagas/flash-firmware.test.ts | 57 +++++++++++++++++++++++++++++++- src/sagas/flash-firmware.ts | 8 +++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 5fd20359..a18de881 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -34,7 +34,7 @@ import { programResponse, rebootRequest, } from '../actions/lwp3-bootloader'; -import { didCompile } from '../actions/mpy'; +import { didCompile, didFailToCompile } from '../actions/mpy'; import { HubType, Result } from '../protocols/lwp3-bootloader'; import { createCountFunc } from '../utils/iter'; import flashFirmware from './flash-firmware'; @@ -411,6 +411,61 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('compile 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, + }; + + 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 } }); + + // 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + saga.put(didFailToCompile(['test'])); + + // compiler error should trigger firmware flash failure + + action = await saga.take(); + expect(action).toEqual( + didFailToStart(FailToStartReasonType.FailedToCompile), + ); + + await saga.end(); + }); }); test('user supplied main.py', async () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 6e368b8b..2369e6d6 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -109,7 +109,10 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator { } /** - * Loads Pybricks firmware from a .zip file + * 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 */ @@ -157,7 +160,8 @@ function* loadFirmware( }); if (mpyFail) { - throw Error(mpyFail.err.join('\n')); + yield* put(didFailToStart(FailToStartReasonType.FailedToCompile)); + yield* cancel(); } defined(mpy); From 9261b8743c27cd7bb5d2394d5e6afe8d81467621 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 11:54:06 -0600 Subject: [PATCH 04/32] proper error handling for firmware too big --- src/sagas/flash-firmware.test.ts | 55 ++++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 3 +- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index a18de881..dfc5eace 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -466,6 +466,61 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('firmware too big', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + 'checksum-type': 'sum', + 'firmware-version': '1.2.3', + 'max-firmware-size': 1, // low limit to trigger error + '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 } }); + + // 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); + + // should fail due to firmware being too big + + action = await saga.take(); + expect(action).toEqual(didFailToStart(FailToStartReasonType.FirmwareSize)); + + await saga.end(); + }); }); test('user supplied main.py', async () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 2369e6d6..7b25a65e 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -174,7 +174,8 @@ function* loadFirmware( const firmwareView = new DataView(firmware.buffer); if (firmware.length > metadata['max-firmware-size']) { - throw Error('firmware + main.mpy is too large'); + yield* put(didFailToStart(FailToStartReasonType.FirmwareSize)); + yield* cancel(); } firmware.set(firmwareBase); From d38ae70ab21bd3cbea807c433237add5a08f6a9a Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 11:58:28 -0600 Subject: [PATCH 05/32] proper error handling for bad checksum algorithm --- src/sagas/flash-firmware.test.ts | 62 ++++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 9 ++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index dfc5eace..89959dfe 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -521,6 +521,68 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('bad checksum algorithm', async () => { + const metadata: FirmwareMetadata = { + 'metadata-version': '1.0.0', + 'device-id': HubType.MoveHub, + // @ts-expect-error: testing bad value + 'checksum-type': 'bad', + '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 } }); + + // 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); + + // should fail due to bad checksum algorithm + + action = await saga.take(); + expect(action).toEqual( + didFailToStart( + FailToStartReasonType.BadMetadata, + 'checksum-type', + MetadataProblem.NotSupported, + ), + ); + + await saga.end(); + }); }); test('user supplied main.py', async () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 7b25a65e..a81c6f0b 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -183,7 +183,14 @@ function* loadFirmware( firmware.set(mpy.data, metadata['user-mpy-offset'] + 4); if (metadata['checksum-type'] !== 'sum') { - throw Error(`Unknown checksum type "${metadata['checksum-type']}"`); + yield* put( + didFailToStart( + FailToStartReasonType.BadMetadata, + 'checksum-type', + MetadataProblem.NotSupported, + ), + ); + yield* cancel(); } firmwareView.setUint32( From 2d4470511275e8b762009a2582c612eaa3305402 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 12:15:28 -0600 Subject: [PATCH 06/32] proper error handling for failure to connect --- src/sagas/flash-firmware.test.ts | 273 +++++++++++++++++++------------ src/sagas/flash-firmware.ts | 9 +- 2 files changed, 168 insertions(+), 114 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 89959dfe..6e39d2c8 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -18,11 +18,13 @@ import { flashFirmware as flashFirmwareAction, } from '../actions/flash-firmware'; import { + BootloaderConnectionFailureReason, BootloaderProgramRequestAction, checksumRequest, checksumResponse, connect, didConnect, + didFailToConnect, didRequest, eraseRequest, eraseResponse, @@ -44,143 +46,198 @@ afterEach(() => { }); 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, - }; + describe('normal flow', () => { + 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'); - jest.spyOn(window, 'fetch').mockResolvedValueOnce( - new Response(await zip.generateAsync({ type: 'blob' })), - ); + jest.spyOn(window, 'fetch').mockResolvedValueOnce( + new Response(await zip.generateAsync({ type: 'blob' })), + ); - const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(flashFirmware, { + nextMessageId: createCountFunc(), + }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.setState({ settings: { flashCurrentProgram: false } }); - // saga is triggered by this action + // saga is triggered by this action - saga.put(flashFirmwareAction()); + saga.put(flashFirmwareAction()); - // first step is to connect to the hub bootloader + // first step is to connect to the hub bootloader - let action = await saga.take(); - expect(action).toEqual(connect()); + let action = await saga.take(); + expect(action).toEqual(connect()); - saga.put(didConnect()); + saga.put(didConnect()); - // then find out what kind of hub it is + // 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).toMatchInlineSnapshot(` - Object { - "options": Array [ - "-mno-unicode", - ], - "script": "print(\\"test\\")", - "type": "mpy.action.compile", - } - `); - - 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), - ); + 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + 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(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('fail to connect', 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(); + 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'); - saga.put(didRequest(id)); - saga.put(checksumResponse(0)); - } - } + jest.spyOn(window, 'fetch').mockResolvedValueOnce( + new Response(await zip.generateAsync({ type: 'blob' })), + ); - // hub indicates success + const saga = new AsyncSaga(flashFirmware, { + nextMessageId: createCountFunc(), + }); - saga.put(programResponse(0, totalFirmwareSize)); + saga.setState({ settings: { flashCurrentProgram: false } }); - action = await saga.take(); - expect(action).toEqual(didProgress(1)); + // saga is triggered by this action - // and finally reboot the hub + saga.put(flashFirmwareAction()); - action = await saga.take(); - expect(action).toEqual(rebootRequest(++id)); + // first step is to connect to the hub bootloader - saga.put(didRequest(id)); + let action = await saga.take(); + expect(action).toEqual(connect()); - // then we are done + saga.put( + didFailToConnect(BootloaderConnectionFailureReason.GattServiceNotFound), + ); - action = await saga.take(); - expect(action).toEqual(didFinish()); + // it should fail here because of failure to connect - await saga.end(); + action = await saga.take(); + expect(action).toEqual( + didFailToStart(FailToStartReasonType.FailedToConnect), + ); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index a81c6f0b..231cf814 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -31,9 +31,8 @@ import { } from '../actions/flash-firmware'; import { BootloaderChecksumResponseAction, + BootloaderConnectionAction, BootloaderConnectionActionType, - BootloaderConnectionDidConnectAction, - BootloaderConnectionDidFailToConnectAction, BootloaderDidRequestAction, BootloaderDidRequestType, BootloaderEraseResponseAction, @@ -233,15 +232,13 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } yield* put(connect()); - const connectResult = yield* take< - | BootloaderConnectionDidConnectAction - | BootloaderConnectionDidFailToConnectAction - >([ + const connectResult = yield* take([ BootloaderConnectionActionType.DidConnect, BootloaderConnectionActionType.DidFailToConnect, ]); if (connectResult.type === BootloaderConnectionActionType.DidFailToConnect) { + yield* put(didFailToStart(FailToStartReasonType.FailedToConnect)); return; } From 18edc7870e855ce18580b528182cc032d7e59e65 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 12:57:37 -0600 Subject: [PATCH 07/32] add disconnect monitor to firmware flash sagas --- src/actions/flash-firmware.ts | 5 +++ src/sagas/flash-firmware.test.ts | 59 ++++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 51 +++++++++++++++++++++++++++ 3 files changed, 115 insertions(+) diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index 0187bd6c..d4837573 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -50,6 +50,8 @@ type Reason = { export enum FailToStartReasonType { /** Connecting to the hub failed. */ FailedToConnect = 'flashFirmware.failToStart.reason.failedToConnect', + /** The hub was disconnected. */ + Disconnected = 'flashFirmware.failToStart.reason.disconnected', /** 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. */ @@ -68,6 +70,8 @@ export enum FailToStartReasonType { export type FailToStartReasonFailedToConnect = Reason; +export type FailToStartReasonDisconnected = Reason; + export type FailToStartReasonNoFirmware = Reason; export type FailToStartReasonDeviceMismatch = Reason; @@ -91,6 +95,7 @@ export type FailToStartReasonUnknown = Reason & { export type FailToStartReason = | FailToStartReasonFailedToConnect + | FailToStartReasonDisconnected | FailToStartReasonNoFirmware | FailToStartReasonDeviceMismatch | FailToStartReasonZipError diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 6e39d2c8..f7d6a23f 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -24,6 +24,7 @@ import { checksumResponse, connect, didConnect, + didDisconnect, didFailToConnect, didRequest, eraseRequest, @@ -238,6 +239,64 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('untimely disconnect before start cancels saga', 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 } }); + + // 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)); + + // hub disconnects before replying + + saga.put(didDisconnect()); + + // should get a failure to start + + action = await saga.take(); + expect(action).toEqual(didFailToStart(FailToStartReasonType.Disconnected)); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 231cf814..e3016800 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -11,6 +11,7 @@ import { call, cancel, delay, + fork, getContext, put, race, @@ -19,11 +20,14 @@ import { takeEvery, } from 'typed-redux-saga/macro'; import { Action } from '../actions'; +import { disconnect } from '../actions/ble'; import { + FailToFinishReasonType, FailToStartReasonType, FlashFirmwareActionType, FlashFirmwareFlashAction, MetadataProblem, + didFailToFinish, didFailToStart, didFinish, didProgress, @@ -201,6 +205,50 @@ function* loadFirmware( return { firmware, deviceId: metadata['device-id'] }; } +/** + * The purpose of this function is two-fold. If the BLE device is disconnected, + * then it will raise a failure action and cancel the task (including the + * parent task). Or, if the parent task fails, it will disconnect the BLE device + * and return. + */ +function* disconnectMonitor(): SagaGenerator { + const { disconnectedBeforeStart, failedToStart } = yield* race({ + disconnectedBeforeStart: take(BootloaderConnectionActionType.DidDisconnect), + started: take(FlashFirmwareActionType.DidStart), + failedToStart: take(FlashFirmwareActionType.DidFailToStart), + }); + + if (disconnectedBeforeStart) { + yield* put(didFailToStart(FailToStartReasonType.Disconnected)); + yield* cancel(); + } + + if (failedToStart) { + yield* put(disconnect()); + return; + } + + // if we get here, `started` won the race + + const { disconnectedAfterStart, failedToFinish } = yield* race({ + disconnectedAfterStart: take(BootloaderConnectionActionType.DidDisconnect), + finished: take(FlashFirmwareActionType.DidFinish), + failedToFinish: take(FlashFirmwareActionType.DidFailToFinish), + }); + + if (disconnectedAfterStart) { + yield* put(didFailToFinish(FailToFinishReasonType.Disconnected)); + yield* cancel(); + } + + if (failedToFinish) { + yield* put(disconnect()); + return; + } + + // if we get here, `finished` won the race. +} + /** * Flashes firmware to a Powered Up device. * @param action The action that triggered this saga. @@ -242,6 +290,8 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { return; } + const disconnectMonitorTask = yield* fork(disconnectMonitor); + const nextMessageId = yield* getContext<() => number>('nextMessageId'); const infoAction = yield* put(infoRequest(nextMessageId())); @@ -378,6 +428,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // this will cause the remote device to disconnect and reboot const rebootAction = yield* put(rebootRequest(nextMessageId())); + disconnectMonitorTask.cancel(); yield* waitForDidRequest(rebootAction.id); yield* put(didFinish()); From f3eef681318c2e0b5717373b06754ba6e7d9d0bc Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 14:10:55 -0600 Subject: [PATCH 08/32] add proper error handling of BLE send error --- src/actions/flash-firmware.ts | 79 +++++++++++++++++++++++- src/actions/lwp3-bootloader.ts | 10 +++ src/sagas/flash-firmware.test.ts | 63 +++++++++++++++++++ src/sagas/flash-firmware.ts | 102 ++++++++++++++----------------- src/sagas/lwp3-bootloader-ble.ts | 1 + 5 files changed, 196 insertions(+), 59 deletions(-) diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index d4837573..cb0dbe87 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -50,8 +50,14 @@ type Reason = { 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. */ @@ -70,8 +76,18 @@ export enum FailToStartReasonType { 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; @@ -95,7 +111,10 @@ export type FailToStartReasonUnknown = Reason & { export type FailToStartReason = | FailToStartReasonFailedToConnect + | FailToStartReasonTimedOut + | FailToStartReasonBleError | FailToStartReasonDisconnected + | FailToStartReasonHubError | FailToStartReasonNoFirmware | FailToStartReasonDeviceMismatch | FailToStartReasonZipError @@ -119,7 +138,9 @@ export enum FailToFinishReasonType { export type FailToFinishReasonTimedOut = Reason; -export type FailToFinishReasonBleError = Reason; +export type FailToFinishReasonBleError = Reason & { + err: Error; +}; export type FailToFinishReasonDisconnected = Reason; @@ -170,6 +191,16 @@ export type FlashFirmwareDidFailToStartAction = Action, ): FlashFirmwareDidFailToFinishAction; @@ -303,6 +365,17 @@ export function didFailToFinish( reason: FailToFinishReasonType, arg1?: HubError | Error, ): FlashFirmwareDidFailToFinishAction { + if (reason === FailToFinishReasonType.BleError) { + // 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 }, + }; + } + if (reason === FailToFinishReasonType.HubError) { // istanbul ignore if: programmer error give wrong arg if (!isHubError(arg1)) { diff --git a/src/actions/lwp3-bootloader.ts b/src/actions/lwp3-bootloader.ts index 6bcfbe6a..c0066c1b 100644 --- a/src/actions/lwp3-bootloader.ts +++ b/src/actions/lwp3-bootloader.ts @@ -41,6 +41,10 @@ export enum BootloaderConnectionActionType { * The connection received a message. */ DidReceive = 'bootloader.action.connection.did.receive', + /** + * Initiate disconnection/ + */ + Disconnect = 'bootloader.action.connection.disconnect', /** * The connection has been closed. */ @@ -59,6 +63,12 @@ export function didConnect(): BootloaderConnectionDidConnectAction { return { type: BootloaderConnectionActionType.DidConnect }; } +export type BootloaderConnectionDisconnectAction = Action; + +export function disconnect(): BootloaderConnectionDisconnectAction { + return { type: BootloaderConnectionActionType.Disconnect }; +} + /** * Possible reasons a device could fail to connect. */ diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index f7d6a23f..6fcbfd01 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -27,6 +27,7 @@ import { didDisconnect, didFailToConnect, didRequest, + disconnect, eraseRequest, eraseResponse, infoRequest, @@ -297,6 +298,68 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('fail to send info request', 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 } }); + + // 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)); + + const testError = new Error('test'); + saga.put(didRequest(0, testError)); + + // should get a failure to start + + action = await saga.take(); + expect(action).toEqual( + didFailToStart(FailToStartReasonType.BleError, testError), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index e3016800..b0c1f95d 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -20,12 +20,12 @@ import { takeEvery, } from 'typed-redux-saga/macro'; import { Action } from '../actions'; -import { disconnect } from '../actions/ble'; import { FailToFinishReasonType, FailToStartReasonType, FlashFirmwareActionType, FlashFirmwareFlashAction, + HubError, MetadataProblem, didFailToFinish, didFailToStart, @@ -48,6 +48,7 @@ import { BootloaderResponseActionType, checksumRequest, connect, + disconnect, disconnectRequest, eraseRequest, infoRequest, @@ -74,9 +75,16 @@ const firmwareZipMap = new Map([ ]); function* waitForDidRequest(id: number): SagaGenerator { - return yield* take( + const request = yield* take( (a: Action) => a.type === BootloaderDidRequestType && a.id === id, ); + if (request.err) { + yield* put(didFailToStart(FailToStartReasonType.BleError, request.err)); + yield* put(disconnect()); + yield* cancel(); + } + + return request; } /** @@ -88,16 +96,30 @@ function* waitForDidRequest(id: number): SagaGenerator( type: BootloaderResponseActionType, timeout = 500, -): SagaGenerator<{ - response?: T; - error?: BootloaderErrorResponseAction; - timeout?: boolean; -}> { - return yield* race({ +): SagaGenerator { + const { response, error, timedOut } = yield* race({ response: take(type), error: take(BootloaderResponseActionType.Error), - timeout: delay(timeout), + timedOut: delay(timeout), }); + + if (timedOut) { + yield* put(didFailToStart(FailToStartReasonType.TimedOut)); + yield* put(disconnect()); + yield* cancel(); + } + + if (error) { + yield* put( + didFailToStart(FailToStartReasonType.HubError, HubError.UnknownCommand), + ); + yield* put(disconnect()); + cancel(); + } + + defined(response); + + return response; } function* firmwareIterator(data: DataView, maxSize: number): Generator { @@ -206,16 +228,13 @@ function* loadFirmware( } /** - * The purpose of this function is two-fold. If the BLE device is disconnected, - * then it will raise a failure action and cancel the task (including the - * parent task). Or, if the parent task fails, it will disconnect the BLE device - * and return. + * Monitors for BLE disconnection event. If disconnection occurs, then a failure + * action is raised and the task (including the parent task) is canceled. */ function* disconnectMonitor(): SagaGenerator { - const { disconnectedBeforeStart, failedToStart } = yield* race({ + const { disconnectedBeforeStart } = yield* race({ disconnectedBeforeStart: take(BootloaderConnectionActionType.DidDisconnect), started: take(FlashFirmwareActionType.DidStart), - failedToStart: take(FlashFirmwareActionType.DidFailToStart), }); if (disconnectedBeforeStart) { @@ -223,17 +242,11 @@ function* disconnectMonitor(): SagaGenerator { yield* cancel(); } - if (failedToStart) { - yield* put(disconnect()); - return; - } - // if we get here, `started` won the race - const { disconnectedAfterStart, failedToFinish } = yield* race({ + const { disconnectedAfterStart } = yield* race({ disconnectedAfterStart: take(BootloaderConnectionActionType.DidDisconnect), finished: take(FlashFirmwareActionType.DidFinish), - failedToFinish: take(FlashFirmwareActionType.DidFailToFinish), }); if (disconnectedAfterStart) { @@ -241,11 +254,6 @@ function* disconnectMonitor(): SagaGenerator { yield* cancel(); } - if (failedToFinish) { - yield* put(disconnect()); - return; - } - // if we get here, `finished` won the race. } @@ -301,18 +309,13 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { BootloaderResponseActionType.Info, ), }); - if (!info.response) { - throw Error(`failed to get info: ${info}`); - } - if (deviceId !== undefined && info.response.hubType !== deviceId) { - throw Error( - `Connected to ${info.response.hubType} but firmware is for ${deviceId}`, - ); + if (deviceId !== undefined && info.hubType !== deviceId) { + throw Error(`Connected to ${info.hubType} but firmware is for ${deviceId}`); } if (firmware === undefined) { - const firmwarePath = firmwareZipMap.get(info.response.hubType); + const firmwarePath = firmwareZipMap.get(info.hubType); if (firmwarePath === undefined) { yield* put( notification.add( @@ -335,10 +338,8 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { const data = yield* call(() => response.arrayBuffer()); ({ firmware, deviceId } = yield* loadFirmware(data, program)); - if (deviceId !== undefined && info.response.hubType !== deviceId) { - throw Error( - `Connected to ${info.response.hubType} but firmware is for ${deviceId}`, - ); + if (deviceId !== undefined && info.hubType !== deviceId) { + throw Error(`Connected to ${info.hubType} but firmware is for ${deviceId}`); } } @@ -352,7 +353,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { 5000, ), }); - if (!erase.response || erase.response.result) { + if (erase.result) { // TODO: proper error handling throw Error(`Failed to erase: ${erase}`); } @@ -364,22 +365,18 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { BootloaderResponseActionType.Init, ), }); - if (!init.response || init.response.result) { + if (init.result) { // TODO: proper error handling throw Error(`Failed to init: ${init}`); } // 14 is "safe" size for all hubs - const maxDataSize = MaxProgramFlashSize.get(info.response.hubType) || 14; + const maxDataSize = MaxProgramFlashSize.get(info.hubType) || 14; for (let count = 1, offset = 0; ; count++) { const payload = firmware.slice(offset, offset + maxDataSize); const programAction = yield* put( - programRequest( - nextMessageId(), - info.response.startAddress + offset, - payload.buffer, - ), + programRequest(nextMessageId(), info.startAddress + offset, payload.buffer), ); yield* waitForDidRequest(programAction.id); @@ -398,17 +395,13 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // the hub is not known and could vary by device. if (count % 10 === 0) { const checksumAction = yield* put(checksumRequest(nextMessageId())); - const { checksum } = yield* all({ + 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}`); - } } } @@ -416,10 +409,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { BootloaderResponseActionType.Program, 5000, ); - if (!flash.response) { - throw Error(`failed to get final response: ${flash}`); - } - if (flash.response.count !== firmware.length) { + if (flash.count !== firmware.length) { // TODO: proper error handling throw Error("Didn't flash all bytes"); } diff --git a/src/sagas/lwp3-bootloader-ble.ts b/src/sagas/lwp3-bootloader-ble.ts index 946b45d3..c5235355 100644 --- a/src/sagas/lwp3-bootloader-ble.ts +++ b/src/sagas/lwp3-bootloader-ble.ts @@ -143,6 +143,7 @@ function* connect(_action: BootloaderConnectionAction): Generator { yield takeEvery(notificationChannel, handleNotify); yield takeEvery(BootloaderConnectionActionType.Send, write, characteristic); + yield takeEvery(BootloaderConnectionActionType.Disconnect, server.disconnect); yield put(didConnect()); From a88fa3d425ac07a0f6c8c9de8051351af1bbe3f2 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 14:30:56 -0600 Subject: [PATCH 09/32] 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; } From 7a5e344b664f13452d1734f7c95a7514d110d3cf Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 15:00:27 -0600 Subject: [PATCH 10/32] add helper function to disconnect and cancel --- src/sagas/flash-firmware.test.ts | 43 ++++++++++++++++++++++++++++++-- src/sagas/flash-firmware.ts | 42 ++++++++++++++++++++++--------- 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index f0e95a85..a92911f0 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -40,6 +40,7 @@ import { } from '../actions/lwp3-bootloader'; import { didCompile, didFailToCompile } from '../actions/mpy'; import { HubType, Result } from '../protocols/lwp3-bootloader'; +import { BootloaderConnectionState } from '../reducers/bootloader'; import { createCountFunc } from '../utils/iter'; import flashFirmware from './flash-firmware'; @@ -86,6 +87,9 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); saga.put(didConnect()); // then find out what kind of hub it is @@ -278,6 +282,9 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); saga.put(didConnect()); // then find out what kind of hub it is @@ -289,6 +296,9 @@ describe('flashFirmware', () => { // hub disconnects before replying + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Disconnected }, + }); saga.put(didDisconnect()); // should get a failure to start @@ -338,6 +348,9 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); saga.put(didConnect()); // then find out what kind of hub it is @@ -417,6 +430,9 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(connect()); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); saga.put(didConnect()); // then find out what kind of hub it is @@ -526,7 +542,10 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }); // saga is triggered by this action @@ -571,7 +590,10 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }); // saga is triggered by this action @@ -636,6 +658,12 @@ describe('flashFirmware', () => { } `); + // this triggers a failure + + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Disconnected }, + }); + saga.put(didFailToCompile(['test'])); // compiler error should trigger firmware flash failure @@ -691,6 +719,10 @@ describe('flashFirmware', () => { } `); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Disconnected }, + }); + const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); @@ -749,6 +781,10 @@ describe('flashFirmware', () => { } `); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Disconnected }, + }); + const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); @@ -810,6 +846,9 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); + saga.setState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); saga.put(didConnect()); // then find out what kind of hub it is diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 29e90597..5c2cb0f3 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -63,6 +63,7 @@ import { import * as notification from '../actions/notification'; import { MaxProgramFlashSize } from '../protocols/lwp3-bootloader'; import { RootState } from '../reducers'; +import { BootloaderConnectionState } from '../reducers/bootloader'; import { defined, maybe } from '../utils'; import { fmod, sumComplement32 } from '../utils/math'; @@ -72,14 +73,33 @@ const firmwareZipMap = new Map([ [HubType.MoveHub, moveHubZip], ]); +/** + * Disconnects the BLE if we are connected and cancels the task (including the + * parent task). + */ +function* disconnectAndCancel(): SagaGenerator { + const connection = yield* select((s: RootState) => s.bootloader.connection); + + if (connection === BootloaderConnectionState.Connected) { + yield* put(disconnect()); + } + + yield* cancel(); + + // HACK: cancel effect doesn't return, so we need this to make typescript + // happy about the never return type. + + // istanbul ignore next: not reachable + throw undefined; +} + function* waitForDidRequest(id: number): SagaGenerator { const request = yield* take( (a: Action) => a.type === BootloaderDidRequestType && a.id === id, ); if (request.err) { yield* put(didFailToFinish(FailToFinishReasonType.BleError, request.err)); - yield* put(disconnect()); - yield* cancel(); + yield* disconnectAndCancel(); } return request; @@ -103,16 +123,14 @@ function* waitForResponse( if (timedOut) { yield* put(didFailToFinish(FailToFinishReasonType.TimedOut)); - yield* put(disconnect()); - yield* cancel(); + yield* disconnectAndCancel(); } if (error) { yield* put( didFailToFinish(FailToFinishReasonType.HubError, HubError.UnknownCommand), ); - yield* put(disconnect()); - cancel(); + yield* disconnectAndCancel(); } defined(response); @@ -150,7 +168,7 @@ function* loadFirmware( } else { yield* put(didFailToFinish(FailToFinishReasonType.Unknown, readerErr)); } - yield* cancel(); + yield* disconnectAndCancel(); } defined(reader); @@ -171,7 +189,7 @@ function* loadFirmware( MetadataProblem.NotSupported, ), ); - yield* cancel(); + yield* disconnectAndCancel(); } yield* put(compile(program, metadata['mpy-cross-options'])); @@ -182,7 +200,7 @@ function* loadFirmware( if (mpyFail) { yield* put(didFailToFinish(FailToFinishReasonType.FailedToCompile)); - yield* cancel(); + yield* disconnectAndCancel(); } defined(mpy); @@ -196,7 +214,7 @@ function* loadFirmware( if (firmware.length > metadata['max-firmware-size']) { yield* put(didFailToFinish(FailToFinishReasonType.FirmwareSize)); - yield* cancel(); + yield* disconnectAndCancel(); } firmware.set(firmwareBase); @@ -211,7 +229,7 @@ function* loadFirmware( MetadataProblem.NotSupported, ), ); - yield* cancel(); + yield* disconnectAndCancel(); } firmwareView.setUint32( @@ -236,7 +254,7 @@ function* disconnectMonitor(): SagaGenerator { if (disconnected) { yield* put(didFailToFinish(FailToFinishReasonType.Disconnected)); - yield* cancel(); + yield* disconnectAndCancel(); } } From 79e06bcd5147c98a115c0a6db51fa01b9d54eac6 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 15:03:14 -0600 Subject: [PATCH 11/32] add test for response timeout --- src/sagas/flash-firmware.test.ts | 62 ++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index a92911f0..3d8a8785 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -375,6 +375,68 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('timeout waiting for info response', 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 } }); + + // 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.setState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + 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)); + + // should get a timed-out failure + + action = await saga.take(); + expect(action).toEqual(didFailToFinish(FailToFinishReasonType.TimedOut)); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { From 08fea237c90bc47008b7e3b44a5c684e05006ccf Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 15:22:44 -0600 Subject: [PATCH 12/32] change setState() to updateState() this way we don't wipe out state if we change a different value --- src/sagas/editor.test.ts | 6 ++--- src/sagas/flash-firmware.test.ts | 44 ++++++++++++++++---------------- src/sagas/hub.test.ts | 2 +- src/sagas/license.test.ts | 6 ++--- src/sagas/settings.test.ts | 8 +++--- src/sagas/terminal.test.ts | 16 ++++++------ test/index.ts | 4 +-- 7 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/sagas/editor.test.ts b/src/sagas/editor.test.ts index e76570c6..725953dc 100644 --- a/src/sagas/editor.test.ts +++ b/src/sagas/editor.test.ts @@ -15,7 +15,7 @@ test('open', async () => { const mockEditor = mock(); const data = new Uint8Array().buffer; - saga.setState({ editor: { current: mockEditor } }); + saga.updateState({ editor: { current: mockEditor } }); saga.put(open(data)); expect(mockEditor.setValue).toBeCalled(); @@ -27,7 +27,7 @@ test('saveAs', async () => { const saga = new AsyncSaga(editor); const mockEditor = mock(); - saga.setState({ editor: { current: mockEditor } }); + saga.updateState({ editor: { current: mockEditor } }); saga.put(saveAs()); expect(mockEditor.getValue).toBeCalled(); @@ -39,7 +39,7 @@ test('reloadProgram', async () => { const saga = new AsyncSaga(editor); const mockEditor = mock(); - saga.setState({ editor: { current: mockEditor } }); + saga.updateState({ editor: { current: mockEditor } }); saga.put(reloadProgram()); expect(mockEditor.setValue).toHaveBeenCalled(); diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 3d8a8785..48d416ea 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -76,7 +76,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -87,7 +87,7 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Connected }, }); saga.put(didConnect()); @@ -220,7 +220,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -271,7 +271,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -282,7 +282,7 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Connected }, }); saga.put(didConnect()); @@ -296,7 +296,7 @@ describe('flashFirmware', () => { // hub disconnects before replying - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Disconnected }, }); saga.put(didDisconnect()); @@ -337,7 +337,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -348,7 +348,7 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Connected }, }); saga.put(didConnect()); @@ -402,7 +402,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -413,7 +413,7 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Connected }, }); saga.put(didConnect()); @@ -462,7 +462,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -492,7 +492,7 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(connect()); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Connected }, }); saga.put(didConnect()); @@ -604,7 +604,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Disconnected }, settings: { flashCurrentProgram: false }, }); @@ -652,7 +652,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Disconnected }, settings: { flashCurrentProgram: false }, }); @@ -699,7 +699,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -722,7 +722,7 @@ describe('flashFirmware', () => { // this triggers a failure - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Disconnected }, }); @@ -760,7 +760,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -781,7 +781,7 @@ describe('flashFirmware', () => { } `); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Disconnected }, }); @@ -822,7 +822,7 @@ describe('flashFirmware', () => { nextMessageId: createCountFunc(), }); - saga.setState({ settings: { flashCurrentProgram: false } }); + saga.updateState({ settings: { flashCurrentProgram: false } }); // saga is triggered by this action @@ -843,7 +843,7 @@ describe('flashFirmware', () => { } `); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Disconnected }, }); @@ -894,7 +894,7 @@ describe('flashFirmware', () => { const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); - saga.setState({ + saga.updateState({ editor: { current: editor }, settings: { flashCurrentProgram: true }, }); @@ -908,7 +908,7 @@ describe('flashFirmware', () => { let action = await saga.take(); expect(action).toEqual(connect()); - saga.setState({ + saga.updateState({ bootloader: { connection: BootloaderConnectionState.Connected }, }); saga.put(didConnect()); diff --git a/src/sagas/hub.test.ts b/src/sagas/hub.test.ts index 9af02713..64968085 100644 --- a/src/sagas/hub.test.ts +++ b/src/sagas/hub.test.ts @@ -25,7 +25,7 @@ describe('downloadAndRun', () => { const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); const mockEditor = mock(); - saga.setState({ editor: { current: mockEditor } }); + saga.updateState({ editor: { current: mockEditor } }); saga.put(downloadAndRun()); diff --git a/src/sagas/license.test.ts b/src/sagas/license.test.ts index 57d993cc..494bfb37 100644 --- a/src/sagas/license.test.ts +++ b/src/sagas/license.test.ts @@ -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 } }); + saga.updateState({ 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 } }); + saga.updateState({ 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 } }); + saga.updateState({ 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 8e14d15b..2c5322d8 100644 --- a/src/sagas/settings.test.ts +++ b/src/sagas/settings.test.ts @@ -220,7 +220,7 @@ describe('store settings to local storage', () => { throw testError; }); - saga.setState({ settings: { showDocs: false } }); + saga.updateState({ settings: { showDocs: false } }); saga.put(setBoolean(SettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); @@ -245,7 +245,7 @@ describe('store settings to local storage', () => { expect(value).toBe('true'); }); - saga.setState({ settings: { showDocs: false } }); + saga.updateState({ settings: { showDocs: false } }); saga.put(setBoolean(SettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); @@ -265,7 +265,7 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.setState({ settings: { darkMode: true } }); + saga.updateState({ settings: { darkMode: true } }); saga.put(setBoolean(SettingId.DarkMode, false)); expect(mockSetItem).toHaveBeenCalled(); @@ -285,7 +285,7 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.setState({ settings: { flashCurrentProgram: true } }); + saga.updateState({ settings: { flashCurrentProgram: true } }); saga.put(setBoolean(SettingId.FlashCurrentProgram, false)); expect(mockSetItem).toHaveBeenCalled(); diff --git a/src/sagas/terminal.test.ts b/src/sagas/terminal.test.ts index abefe1d2..769f930f 100644 --- a/src/sagas/terminal.test.ts +++ b/src/sagas/terminal.test.ts @@ -33,7 +33,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // sending ASCII space character - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put(notify(new DataView(new Uint8Array([0x20]).buffer))); const action = await saga.take(); @@ -46,7 +46,7 @@ describe('Data receiver filters out hub status', () => { test('checksum message', async () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); - saga.setState({ hub: { runtime: HubRuntimeState.Loading } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Loading } }); saga.put(notify(new DataView(new Uint8Array([0xaa]).buffer))); const action = await saga.take(); @@ -60,7 +60,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> IDLE' - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -92,7 +92,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> IDLE1' - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -136,7 +136,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> ERROR' - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -169,7 +169,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> ERROR1' - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -214,7 +214,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> ERROR' - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -249,7 +249,7 @@ describe('Data receiver filters out hub status', () => { const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> RUNNING1' - saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); + saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( diff --git a/test/index.ts b/test/index.ts index b1833718..1adaa03d 100644 --- a/test/index.ts +++ b/test/index.ts @@ -67,8 +67,8 @@ export class AsyncSaga { return Promise.resolve(next); } - public setState(state: RecursivePartial): void { - this.state = state; + public updateState(state: RecursivePartial): void { + this.state = { ...this.state, ...state }; } public async end(): Promise { From c1c08e5ced9a376054461e0a71624b56a31ba600 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 15:40:30 -0600 Subject: [PATCH 13/32] add state initializer in async saga constructor --- src/sagas/editor.test.ts | 11 +- src/sagas/flash-firmware.test.ts | 199 +++++++++++++++++++------------ src/sagas/hub.test.ts | 12 +- src/sagas/license.test.ts | 9 +- src/sagas/notification.test.ts | 6 +- src/sagas/settings.test.ts | 14 +-- src/sagas/terminal.test.ts | 68 +++++++---- test/index.ts | 8 +- 8 files changed, 195 insertions(+), 132 deletions(-) diff --git a/src/sagas/editor.test.ts b/src/sagas/editor.test.ts index 725953dc..c09aed41 100644 --- a/src/sagas/editor.test.ts +++ b/src/sagas/editor.test.ts @@ -11,11 +11,10 @@ jest.mock('ace-builds'); jest.mock('file-saver'); test('open', async () => { - const saga = new AsyncSaga(editor); const mockEditor = mock(); - const data = new Uint8Array().buffer; + const saga = new AsyncSaga(editor, { editor: { current: mockEditor } }); - saga.updateState({ editor: { current: mockEditor } }); + const data = new Uint8Array().buffer; saga.put(open(data)); expect(mockEditor.setValue).toBeCalled(); @@ -24,10 +23,9 @@ test('open', async () => { }); test('saveAs', async () => { - const saga = new AsyncSaga(editor); const mockEditor = mock(); + const saga = new AsyncSaga(editor, { editor: { current: mockEditor } }); - saga.updateState({ editor: { current: mockEditor } }); saga.put(saveAs()); expect(mockEditor.getValue).toBeCalled(); @@ -36,10 +34,9 @@ test('saveAs', async () => { }); test('reloadProgram', async () => { - const saga = new AsyncSaga(editor); const mockEditor = mock(); + const saga = new AsyncSaga(editor, { editor: { current: mockEditor } }); - saga.updateState({ editor: { current: mockEditor } }); saga.put(reloadProgram()); expect(mockEditor.setValue).toHaveBeenCalled(); diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 48d416ea..94f9a49d 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -10,6 +10,7 @@ import JSZip from 'jszip'; import { AsyncSaga } from '../../test'; import { FailToFinishReasonType, + HubError, MetadataProblem, didFailToFinish, didFinish, @@ -72,11 +73,16 @@ describe('flashFirmware', () => { new Response(await zip.generateAsync({ type: 'blob' })), ); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -216,11 +222,16 @@ describe('flashFirmware', () => { new Response(await zip.generateAsync({ type: 'blob' })), ); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -267,11 +278,16 @@ describe('flashFirmware', () => { new Response(await zip.generateAsync({ type: 'blob' })), ); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -333,11 +349,16 @@ describe('flashFirmware', () => { new Response(await zip.generateAsync({ type: 'blob' })), ); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -398,11 +419,16 @@ describe('flashFirmware', () => { new Response(await zip.generateAsync({ type: 'blob' })), ); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -458,11 +484,16 @@ describe('flashFirmware', () => { zip.file('main.py', 'print("test")'); zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -600,14 +631,16 @@ describe('flashFirmware', () => { zip.file('main.py', 'print("test")'); zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ - bootloader: { connection: BootloaderConnectionState.Disconnected }, - settings: { flashCurrentProgram: false }, - }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -648,14 +681,16 @@ describe('flashFirmware', () => { zip.file('main.py', 'print("test")'); zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ - bootloader: { connection: BootloaderConnectionState.Disconnected }, - settings: { flashCurrentProgram: false }, - }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -695,11 +730,16 @@ describe('flashFirmware', () => { zip.file('main.py', 'print("test")'); zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -722,10 +762,6 @@ describe('flashFirmware', () => { // this triggers a failure - saga.updateState({ - bootloader: { connection: BootloaderConnectionState.Disconnected }, - }); - saga.put(didFailToCompile(['test'])); // compiler error should trigger firmware flash failure @@ -756,11 +792,16 @@ describe('flashFirmware', () => { zip.file('main.py', 'print("test")'); zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -781,10 +822,6 @@ describe('flashFirmware', () => { } `); - saga.updateState({ - bootloader: { connection: BootloaderConnectionState.Disconnected }, - }); - const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); @@ -818,11 +855,16 @@ describe('flashFirmware', () => { zip.file('main.py', 'print("test")'); zip.file('ReadMe_OSS.txt', 'test'); - const saga = new AsyncSaga(flashFirmware, { - nextMessageId: createCountFunc(), - }); - - saga.updateState({ settings: { flashCurrentProgram: false } }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); // saga is triggered by this action @@ -843,10 +885,6 @@ describe('flashFirmware', () => { } `); - saga.updateState({ - bootloader: { connection: BootloaderConnectionState.Disconnected }, - }); - const mpySize = 20; const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); @@ -892,12 +930,15 @@ describe('flashFirmware', () => { getValue: () => 'print("test")', }; - const saga = new AsyncSaga(flashFirmware, { nextMessageId: createCountFunc() }); - - saga.updateState({ - editor: { current: editor }, - settings: { flashCurrentProgram: true }, - }); + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + editor: { current: editor }, + settings: { flashCurrentProgram: true }, + }, + { nextMessageId: createCountFunc() }, + ); // saga is triggered by this action diff --git a/src/sagas/hub.test.ts b/src/sagas/hub.test.ts index 64968085..150b2054 100644 --- a/src/sagas/hub.test.ts +++ b/src/sagas/hub.test.ts @@ -22,10 +22,12 @@ jest.mock('ace-builds'); describe('downloadAndRun', () => { test('no errors', async () => { - const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); - const mockEditor = mock(); - saga.updateState({ editor: { current: mockEditor } }); + const saga = new AsyncSaga( + hub, + { editor: { current: mockEditor } }, + { nextMessageId: createCountFunc() }, + ); saga.put(downloadAndRun()); @@ -76,7 +78,7 @@ describe('downloadAndRun', () => { }); test('repl', async () => { - const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(hub, {}, { nextMessageId: createCountFunc() }); saga.put(repl()); @@ -87,7 +89,7 @@ test('repl', async () => { }); test('stop', async () => { - const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(hub, {}, { nextMessageId: createCountFunc() }); saga.put(stop()); diff --git a/src/sagas/license.test.ts b/src/sagas/license.test.ts index 494bfb37..95daafb1 100644 --- a/src/sagas/license.test.ts +++ b/src/sagas/license.test.ts @@ -16,7 +16,7 @@ afterAll(() => { describe('fetchLicenses', () => { test('first call', async () => { const testLicenseList: LicenseList = []; - const saga = new AsyncSaga(license); + const saga = new AsyncSaga(license, { license: { list: null } }); jest.spyOn(globalThis, 'fetch').mockResolvedValue( new Response(JSON.stringify(testLicenseList)), @@ -24,7 +24,6 @@ describe('fetchLicenses', () => { // initially, license list starts as null, so fetch is called to get // the list - saga.updateState({ license: { list: null } }); saga.put(openLicenseDialog()); const action = await saga.take(); @@ -34,7 +33,7 @@ describe('fetchLicenses', () => { }); test('second call', async () => { const testLicenseList: LicenseList = []; - const saga = new AsyncSaga(license); + const saga = new AsyncSaga(license, { license: { list: testLicenseList } }); jest.spyOn(globalThis, 'fetch').mockRejectedValue( 'fetch () should not have been called', @@ -42,7 +41,6 @@ describe('fetchLicenses', () => { // after we have the list, we don't fetch it again since it will // always be the same list - saga.updateState({ license: { list: testLicenseList } }); saga.put(openLicenseDialog()); // have to yield to be sure fetch call would have taken place on error @@ -52,11 +50,10 @@ describe('fetchLicenses', () => { }); test('failed fetch', async () => { const failResponse = new Response(undefined, { status: 404 }); - const saga = new AsyncSaga(license); + const saga = new AsyncSaga(license, { license: { list: null } }); jest.spyOn(globalThis, 'fetch').mockResolvedValue(failResponse); - saga.updateState({ license: { list: null } }); saga.put(openLicenseDialog()); const action = await saga.take(); diff --git a/src/sagas/notification.test.ts b/src/sagas/notification.test.ts index 9b529eb9..28887502 100644 --- a/src/sagas/notification.test.ts +++ b/src/sagas/notification.test.ts @@ -50,7 +50,7 @@ test.each([ clear, }; - const saga = new AsyncSaga(notification, { notification: { toaster } }); + const saga = new AsyncSaga(notification, {}, { notification: { toaster } }); saga.put(action); @@ -78,7 +78,7 @@ test.each([ clear, }; - const saga = new AsyncSaga(notification, { notification: { toaster } }); + const saga = new AsyncSaga(notification, {}, { notification: { toaster } }); saga.put(action); @@ -105,7 +105,7 @@ test.each([[didCompile(new Uint8Array()), MessageId.MpyError]])( clear, }; - const saga = new AsyncSaga(notification, { notification: { toaster } }); + const saga = new AsyncSaga(notification, {}, { notification: { toaster } }); saga.put(action); diff --git a/src/sagas/settings.test.ts b/src/sagas/settings.test.ts index 2c5322d8..da9dea7c 100644 --- a/src/sagas/settings.test.ts +++ b/src/sagas/settings.test.ts @@ -210,7 +210,7 @@ describe('startup', () => { describe('store settings to local storage', () => { test('failed storage', async () => { - const saga = new AsyncSaga(settings); + const saga = new AsyncSaga(settings, { settings: { showDocs: false } }); const testError = new Error('local storage is disabled'); @@ -220,7 +220,6 @@ describe('store settings to local storage', () => { throw testError; }); - saga.updateState({ settings: { showDocs: false } }); saga.put(setBoolean(SettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); @@ -236,7 +235,7 @@ describe('store settings to local storage', () => { }); test('showDocs', async () => { - const saga = new AsyncSaga(settings); + const saga = new AsyncSaga(settings, { settings: { showDocs: false } }); const mockSetItem = jest .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem') @@ -245,7 +244,6 @@ describe('store settings to local storage', () => { expect(value).toBe('true'); }); - saga.updateState({ settings: { showDocs: false } }); saga.put(setBoolean(SettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); @@ -256,7 +254,7 @@ describe('store settings to local storage', () => { }); test('darkMode', async () => { - const saga = new AsyncSaga(settings); + const saga = new AsyncSaga(settings, { settings: { darkMode: true } }); const mockSetItem = jest .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem') @@ -265,7 +263,6 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.updateState({ settings: { darkMode: true } }); saga.put(setBoolean(SettingId.DarkMode, false)); expect(mockSetItem).toHaveBeenCalled(); @@ -276,7 +273,9 @@ describe('store settings to local storage', () => { }); test('flashCurrentProgram', async () => { - const saga = new AsyncSaga(settings); + const saga = new AsyncSaga(settings, { + settings: { flashCurrentProgram: true }, + }); const mockSetItem = jest .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem') @@ -285,7 +284,6 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.updateState({ settings: { flashCurrentProgram: true } }); saga.put(setBoolean(SettingId.FlashCurrentProgram, false)); expect(mockSetItem).toHaveBeenCalled(); diff --git a/src/sagas/terminal.test.ts b/src/sagas/terminal.test.ts index 769f930f..b1fbbaa2 100644 --- a/src/sagas/terminal.test.ts +++ b/src/sagas/terminal.test.ts @@ -30,10 +30,13 @@ import terminal from './terminal'; describe('Data receiver filters out hub status', () => { test('normal message - no status', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // sending ASCII space character - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put(notify(new DataView(new Uint8Array([0x20]).buffer))); const action = await saga.take(); @@ -44,9 +47,12 @@ describe('Data receiver filters out hub status', () => { }); test('checksum message', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Loading } }, + { nextMessageId: createCountFunc() }, + ); - saga.updateState({ hub: { runtime: HubRuntimeState.Loading } }); saga.put(notify(new DataView(new Uint8Array([0xaa]).buffer))); const action = await saga.take(); @@ -57,10 +63,13 @@ describe('Data receiver filters out hub status', () => { }); test('idle message', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // '>>>> IDLE' - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -89,10 +98,13 @@ describe('Data receiver filters out hub status', () => { }); test('idle message with extra text', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // '0>>>> IDLE1' - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -133,10 +145,13 @@ describe('Data receiver filters out hub status', () => { }); test('error message', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // '>>>> ERROR' - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -166,10 +181,13 @@ describe('Data receiver filters out hub status', () => { }); test('error message with extra text', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // '0>>>> ERROR1' - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -211,10 +229,13 @@ describe('Data receiver filters out hub status', () => { }); test('running message', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // '>>>> ERROR' - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -246,10 +267,13 @@ describe('Data receiver filters out hub status', () => { }); test('running message with extra text', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga( + terminal, + { hub: { runtime: HubRuntimeState.Unknown } }, + { nextMessageId: createCountFunc() }, + ); // '0>>>> RUNNING1' - saga.updateState({ hub: { runtime: HubRuntimeState.Unknown } }); saga.put( notify( new DataView( @@ -294,7 +318,7 @@ describe('Data receiver filters out hub status', () => { }); test('Terminal data source responds to send data actions', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(terminal, {}, { nextMessageId: createCountFunc() }); saga.put(didStart()); const dataSourceAction = await saga.take(); @@ -321,7 +345,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, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(terminal, {}, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); @@ -333,7 +357,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('messages are queued until previous has completed', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(terminal, {}, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); await delay(50); // without delay, messages are combined @@ -361,7 +385,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('messages are queued until previous has failed', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(terminal, {}, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); await delay(50); // without delay, messages are combined @@ -391,7 +415,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('small messages are combined', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(terminal, {}, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); saga.put(receiveData('test1234')); @@ -406,7 +430,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('long messages are split', async () => { - const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); + const saga = new AsyncSaga(terminal, {}, { nextMessageId: createCountFunc() }); saga.put(receiveData('012345678901234567890123456789')); diff --git a/test/index.ts b/test/index.ts index 1adaa03d..0fad318b 100644 --- a/test/index.ts +++ b/test/index.ts @@ -16,11 +16,15 @@ export class AsyncSaga { private state: RecursivePartial; private task: Task; - public constructor(saga: Saga, context?: Record) { + public constructor( + saga: Saga, + state: RecursivePartial = {}, + context?: Record, + ) { this.channel = stdChannel(); this.dispatches = []; this.takers = []; - this.state = {}; + this.state = state; this.task = runSaga( { channel: this.channel, From b58ec9a7548c014739c5af84708ad867d042e599 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 15:55:14 -0600 Subject: [PATCH 14/32] fix HubError type check --- src/actions/flash-firmware.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index 3435bb91..f28f8519 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -35,10 +35,7 @@ export enum HubError { } function isHubError(arg: unknown): arg is HubError { - if (typeof arg !== 'string') { - return false; - } - return Object.keys(HubError).includes(arg); + return Object.values(HubError).includes(arg as HubError); } export enum FailToFinishReasonType { From 8ad2b78d4ceba9ae0cb04fb511126def69cca01e Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 15:57:51 -0600 Subject: [PATCH 15/32] add proper error handling for hub erase fail --- src/sagas/flash-firmware.test.ts | 101 +++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 10 +-- 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 94f9a49d..4644fac2 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -463,6 +463,107 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('erase response is failed', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + 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.Error)); + + // should get a hub error + + action = await saga.take(); + expect(action).toEqual( + didFailToFinish(FailToFinishReasonType.HubError, HubError.EraseFailed), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 5c2cb0f3..2fd6dfde 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -61,7 +61,7 @@ import { compile, } from '../actions/mpy'; import * as notification from '../actions/notification'; -import { MaxProgramFlashSize } from '../protocols/lwp3-bootloader'; +import { MaxProgramFlashSize, Result } from '../protocols/lwp3-bootloader'; import { RootState } from '../reducers'; import { BootloaderConnectionState } from '../reducers/bootloader'; import { defined, maybe } from '../utils'; @@ -354,9 +354,11 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { 5000, ), }); - if (erase.result) { - // TODO: proper error handling - throw Error(`Failed to erase: ${erase}`); + if (erase.result !== Result.OK) { + yield* put( + didFailToFinish(FailToFinishReasonType.HubError, HubError.EraseFailed), + ); + yield* disconnectAndCancel(); } const initAction = yield* put(initRequest(nextMessageId(), firmware.length)); From 3ec067e930f5c346833dc08280fee260f31cec8e Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 16:08:13 -0600 Subject: [PATCH 16/32] add test for unknown bootloader command --- src/sagas/flash-firmware.test.ts | 76 +++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 4644fac2..d38ce423 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -31,6 +31,7 @@ import { disconnect, eraseRequest, eraseResponse, + errorResponse, infoRequest, infoResponse, initRequest, @@ -40,7 +41,7 @@ import { rebootRequest, } from '../actions/lwp3-bootloader'; import { didCompile, didFailToCompile } from '../actions/mpy'; -import { HubType, Result } from '../protocols/lwp3-bootloader'; +import { Command, HubType, Result } from '../protocols/lwp3-bootloader'; import { BootloaderConnectionState } from '../reducers/bootloader'; import { createCountFunc } from '../utils/iter'; import flashFirmware from './flash-firmware'; @@ -397,6 +398,79 @@ describe('flashFirmware', () => { await saga.end(); }); + test('info request is unknown command', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + 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(errorResponse(Command.GetInfo)); + + // should get an unknown command failure + + action = await saga.take(); + expect(action).toEqual( + didFailToFinish( + FailToFinishReasonType.HubError, + HubError.UnknownCommand, + ), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); + test('timeout waiting for info response', async () => { const metadata: FirmwareMetadata = { 'metadata-version': '1.0.0', From e5d98cdd84cf6cc07f78dce65bdb6dda633bef36 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 16:22:54 -0600 Subject: [PATCH 17/32] add proper error handling for bad device id --- src/sagas/flash-firmware.test.ts | 158 +++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 23 ++--- 2 files changed, 165 insertions(+), 16 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index d38ce423..c0afb14d 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -538,6 +538,78 @@ describe('flashFirmware', () => { await saga.end(); }); + test('unsupported device', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + // received an unknown hub type ID + + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, 0 as HubType)); + + // should raise an error that we don't have any firmware for this hub + + action = await saga.take(); + expect(action).toStrictEqual( + didFailToFinish(FailToFinishReasonType.NoFirmware), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); + test('erase response is failed', async () => { const metadata: FirmwareMetadata = { 'metadata-version': '1.0.0', @@ -1077,6 +1149,92 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('connected device type does not match firmware device type', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + // connected hub type does not match firmware hub type + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.CityHub)); + + // should raise an error that we don't have any firmware for this hub + + action = await saga.take(); + expect(action).toStrictEqual( + didFailToFinish(FailToFinishReasonType.DeviceMismatch), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); test('user supplied main.py', async () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 2fd6dfde..9f47b47d 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -77,7 +77,7 @@ const firmwareZipMap = new Map([ * Disconnects the BLE if we are connected and cancels the task (including the * parent task). */ -function* disconnectAndCancel(): SagaGenerator { +function* disconnectAndCancel(): SagaGenerator { const connection = yield* select((s: RootState) => s.bootloader.connection); if (connection === BootloaderConnectionState.Connected) { @@ -85,12 +85,6 @@ function* disconnectAndCancel(): SagaGenerator { } yield* cancel(); - - // HACK: cancel effect doesn't return, so we need this to make typescript - // happy about the never return type. - - // istanbul ignore next: not reachable - throw undefined; } function* waitForDidRequest(id: number): SagaGenerator { @@ -312,22 +306,19 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { }); if (deviceId !== undefined && info.hubType !== deviceId) { - throw Error(`Connected to ${info.hubType} but firmware is for ${deviceId}`); + yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch)); + yield* disconnectAndCancel(); } if (firmware === undefined) { const firmwarePath = firmwareZipMap.get(info.hubType); if (firmwarePath === undefined) { - yield* put( - notification.add( - 'error', - "Sorry, we don't have firmware for this hub yet.", - ), - ); - yield* put(disconnectRequest(nextMessageId())); - return; + yield* put(didFailToFinish(FailToFinishReasonType.NoFirmware)); + yield* disconnectAndCancel(); } + defined(firmwarePath); + const response = yield* call(() => fetch(firmwarePath)); if (!response.ok) { yield* put(notification.add('error', 'Failed to fetch firmware.')); From e984cffbb4f8097af3a4cb2d6c9606037548b263 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 16:30:52 -0600 Subject: [PATCH 18/32] add try/catch for unexpected errors --- src/sagas/flash-firmware.ts | 281 +++++++++++++++++++----------------- 1 file changed, 146 insertions(+), 135 deletions(-) diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 9f47b47d..a26be803 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -257,165 +257,176 @@ function* disconnectMonitor(): SagaGenerator { * @param action The action that triggered this saga. */ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { - let firmware: Uint8Array | undefined = undefined; - let deviceId: HubType | undefined = undefined; + try { + let firmware: Uint8Array | undefined = undefined; + let deviceId: HubType | undefined = undefined; - let program: string | undefined = undefined; + let program: string | undefined = undefined; - const flashCurrentProgram = yield* select( - (s: RootState) => s.settings.flashCurrentProgram, - ); + const flashCurrentProgram = yield* select( + (s: RootState) => s.settings.flashCurrentProgram, + ); - if (flashCurrentProgram) { - const editor = yield* select((s: RootState) => s.editor.current); + if (flashCurrentProgram) { + 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) { - console.error('flashFirmware: No current editor'); + // istanbul ignore if: it is a bug to dispatch this action with no current editor + if (editor === null) { + console.error('flashFirmware: No current editor'); + return; + } + + program = editor.getValue(); + } + + if (action.data !== undefined) { + ({ firmware, deviceId } = yield* loadFirmware(action.data, program)); + } + + yield* put(connect()); + const connectResult = yield* take([ + BootloaderConnectionActionType.DidConnect, + BootloaderConnectionActionType.DidFailToConnect, + ]); + + if (connectResult.type === BootloaderConnectionActionType.DidFailToConnect) { + yield* put(didFailToFinish(FailToFinishReasonType.FailedToConnect)); return; } - program = editor.getValue(); - } + const disconnectMonitorTask = yield* fork(disconnectMonitor); - if (action.data !== undefined) { - ({ firmware, deviceId } = yield* loadFirmware(action.data, program)); - } + const nextMessageId = yield* getContext<() => number>('nextMessageId'); - yield* put(connect()); - const connectResult = yield* take([ - BootloaderConnectionActionType.DidConnect, - BootloaderConnectionActionType.DidFailToConnect, - ]); + const infoAction = yield* put(infoRequest(nextMessageId())); + const { info } = yield* all({ + sent: waitForDidRequest(infoAction.id), + info: waitForResponse( + BootloaderResponseActionType.Info, + ), + }); - if (connectResult.type === BootloaderConnectionActionType.DidFailToConnect) { - yield* put(didFailToFinish(FailToFinishReasonType.FailedToConnect)); - return; - } - - const disconnectMonitorTask = yield* fork(disconnectMonitor); - - const nextMessageId = yield* getContext<() => number>('nextMessageId'); - - const infoAction = yield* put(infoRequest(nextMessageId())); - const { info } = yield* all({ - sent: waitForDidRequest(infoAction.id), - info: waitForResponse( - BootloaderResponseActionType.Info, - ), - }); - - if (deviceId !== undefined && info.hubType !== deviceId) { - yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch)); - yield* disconnectAndCancel(); - } - - if (firmware === undefined) { - const firmwarePath = firmwareZipMap.get(info.hubType); - if (firmwarePath === undefined) { - yield* put(didFailToFinish(FailToFinishReasonType.NoFirmware)); + if (deviceId !== undefined && info.hubType !== deviceId) { + yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch)); yield* disconnectAndCancel(); } - defined(firmwarePath); + if (firmware === undefined) { + const firmwarePath = firmwareZipMap.get(info.hubType); + if (firmwarePath === undefined) { + yield* put(didFailToFinish(FailToFinishReasonType.NoFirmware)); + yield* disconnectAndCancel(); + } - const response = yield* call(() => fetch(firmwarePath)); - if (!response.ok) { - yield* put(notification.add('error', 'Failed to fetch firmware.')); - const disconnectAction = yield* put(disconnectRequest(nextMessageId())); - yield* waitForDidRequest(disconnectAction.id); - return; + defined(firmwarePath); + + const response = yield* call(() => fetch(firmwarePath)); + if (!response.ok) { + 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()); + ({ firmware, deviceId } = yield* loadFirmware(data, program)); + + if (deviceId !== undefined && info.hubType !== deviceId) { + throw Error( + `Connected to ${info.hubType} but firmware is for ${deviceId}`, + ); + } } - const data = yield* call(() => response.arrayBuffer()); - ({ firmware, deviceId } = yield* loadFirmware(data, program)); + yield* put(didStart()); - if (deviceId !== undefined && info.hubType !== deviceId) { - throw Error(`Connected to ${info.hubType} but firmware is for ${deviceId}`); + const eraseAction = yield* put(eraseRequest(nextMessageId())); + const { erase } = yield* all({ + sent: waitForDidRequest(eraseAction.id), + erase: waitForResponse( + BootloaderResponseActionType.Erase, + 5000, + ), + }); + if (erase.result !== Result.OK) { + yield* put( + didFailToFinish(FailToFinishReasonType.HubError, HubError.EraseFailed), + ); + yield* disconnectAndCancel(); } - } - yield* put(didStart()); + const initAction = yield* put(initRequest(nextMessageId(), firmware.length)); + const { init } = yield* all({ + sent: waitForDidRequest(initAction.id), + init: waitForResponse( + BootloaderResponseActionType.Init, + ), + }); + if (init.result) { + // TODO: proper error handling + throw Error(`Failed to init: ${init}`); + } - const eraseAction = yield* put(eraseRequest(nextMessageId())); - const { erase } = yield* all({ - sent: waitForDidRequest(eraseAction.id), - erase: waitForResponse( - BootloaderResponseActionType.Erase, + // 14 is "safe" size for all hubs + const maxDataSize = MaxProgramFlashSize.get(info.hubType) || 14; + + for (let count = 1, offset = 0; ; count++) { + const payload = firmware.slice(offset, offset + maxDataSize); + const programAction = yield* put( + programRequest( + nextMessageId(), + info.startAddress + offset, + payload.buffer, + ), + ); + yield* waitForDidRequest(programAction.id); + + 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. + offset += maxDataSize; + if (offset >= firmware.length) { + break; + } + + // Request checksum every 10 packets to prevent buffer overrun on + // 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) { + const checksumAction = yield* put(checksumRequest(nextMessageId())); + yield* all({ + sent: waitForDidRequest(checksumAction.id), + checksum: waitForResponse( + BootloaderResponseActionType.Checksum, + 5000, + ), + }); + } + } + + const flash = yield* waitForResponse( + BootloaderResponseActionType.Program, 5000, - ), - }); - if (erase.result !== Result.OK) { - yield* put( - didFailToFinish(FailToFinishReasonType.HubError, HubError.EraseFailed), ); + if (flash.count !== firmware.length) { + // TODO: proper error handling + throw Error("Didn't flash all bytes"); + } + + yield* put(didProgress(1)); + + // this will cause the remote device to disconnect and reboot + const rebootAction = yield* put(rebootRequest(nextMessageId())); + disconnectMonitorTask.cancel(); + yield* waitForDidRequest(rebootAction.id); + + yield* put(didFinish()); + } catch (err) { + yield* put(didFailToFinish(FailToFinishReasonType.Unknown, err)); yield* disconnectAndCancel(); } - - const initAction = yield* put(initRequest(nextMessageId(), firmware.length)); - const { init } = yield* all({ - sent: waitForDidRequest(initAction.id), - init: waitForResponse( - BootloaderResponseActionType.Init, - ), - }); - if (init.result) { - // TODO: proper error handling - throw Error(`Failed to init: ${init}`); - } - - // 14 is "safe" size for all hubs - const maxDataSize = MaxProgramFlashSize.get(info.hubType) || 14; - - for (let count = 1, offset = 0; ; count++) { - const payload = firmware.slice(offset, offset + maxDataSize); - const programAction = yield* put( - programRequest(nextMessageId(), info.startAddress + offset, payload.buffer), - ); - yield* waitForDidRequest(programAction.id); - - 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. - offset += maxDataSize; - if (offset >= firmware.length) { - break; - } - - // Request checksum every 10 packets to prevent buffer overrun on - // 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) { - const checksumAction = yield* put(checksumRequest(nextMessageId())); - yield* all({ - sent: waitForDidRequest(checksumAction.id), - checksum: waitForResponse( - BootloaderResponseActionType.Checksum, - 5000, - ), - }); - } - } - - const flash = yield* waitForResponse( - BootloaderResponseActionType.Program, - 5000, - ); - if (flash.count !== firmware.length) { - // TODO: proper error handling - throw Error("Didn't flash all bytes"); - } - - yield* put(didProgress(1)); - - // this will cause the remote device to disconnect and reboot - const rebootAction = yield* put(rebootRequest(nextMessageId())); - disconnectMonitorTask.cancel(); - yield* waitForDidRequest(rebootAction.id); - - yield* put(didFinish()); } export default function* (): Generator { From 2f5d1fb976a231d166b7c8f8f61f55dc857c572b Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 16:39:41 -0600 Subject: [PATCH 19/32] add another test for mismatch device type --- src/sagas/flash-firmware.test.ts | 90 +++++++++++++++++++++++++++++++- src/sagas/flash-firmware.ts | 5 +- 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index c0afb14d..fd8dce38 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -51,7 +51,7 @@ afterEach(() => { }); describe('flashFirmware', () => { - describe('normal flow', () => { + describe('normal flow using app supplied firmware', () => { test('success', async () => { const metadata: FirmwareMetadata = { 'metadata-version': '1.0.0', @@ -538,6 +538,94 @@ describe('flashFirmware', () => { await saga.end(); }); + test('connected device does not match firmware device', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + // connected hub type does not match firmware hub type + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.CityHub)); + + // then compile main.py to .mpy + + action = await saga.take(); + expect(action).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + const mpySize = 20; + const mpyBinaryData = new Uint8Array(mpySize); + saga.put(didCompile(mpyBinaryData)); + + // should raise an error that we don't have any firmware for this hub + + action = await saga.take(); + expect(action).toStrictEqual( + didFailToFinish(FailToFinishReasonType.DeviceMismatch), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); + test('unsupported device', async () => { const metadata: FirmwareMetadata = { 'metadata-version': '1.0.0', diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index a26be803..06ee3575 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -332,9 +332,8 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { ({ firmware, deviceId } = yield* loadFirmware(data, program)); if (deviceId !== undefined && info.hubType !== deviceId) { - throw Error( - `Connected to ${info.hubType} but firmware is for ${deviceId}`, - ); + yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch)); + yield* disconnectAndCancel(); } } From b1883a9d1c8b8db3748c6ac3406792894daa6571 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 16:51:42 -0600 Subject: [PATCH 20/32] make sure we are getting coverage of checksum messages --- src/sagas/flash-firmware.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index fd8dce38..5d0e3c37 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -120,7 +120,9 @@ describe('flashFirmware', () => { } `); - const mpySize = 20; + // this makes total firmware size 140 bytes to check for + // https://github.com/pybricks/support/issues/178 + const mpySize = 40 - 8; const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); @@ -167,6 +169,7 @@ describe('flashFirmware', () => { // last payload is sent, otherwise the hub gets confused. if (offset + 14 >= totalFirmwareSize) { + expect(count).toBe(10); break; } @@ -849,7 +852,9 @@ describe('flashFirmware', () => { } `); - const mpySize = 20; + // make sure that total firmware size is big enough that checksum + // is called at least once + const mpySize = 100; const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); @@ -914,6 +919,7 @@ describe('flashFirmware', () => { // last payload is sent, otherwise the hub gets confused. if (offset + 14 >= totalFirmwareSize) { + expect(count).toBeGreaterThan(10); break; } From 3208faa7e6db5025854e025b4585ec9fe8f5d62c Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 17:44:10 -0600 Subject: [PATCH 21/32] add proper error handling for fail to fetch --- src/actions/flash-firmware.ts | 26 +++++++++++- src/sagas/flash-firmware.test.ts | 71 ++++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 10 ++--- 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/src/actions/flash-firmware.ts b/src/actions/flash-firmware.ts index f28f8519..cda81bac 100644 --- a/src/actions/flash-firmware.ts +++ b/src/actions/flash-firmware.ts @@ -53,6 +53,8 @@ export enum FailToFinishReasonType { NoFirmware = 'flashFirmware.failToFinish.reason.noFirmware', /** The provided firmware.zip does not match the connected hub. */ DeviceMismatch = 'flashFirmware.failToFinish.reason.deviceMismatch', + /** Failed to fetch firmware from the server. */ + FailedToFetch = 'flashFirmware.failToFinish.reason.failedToFetch', /** There was a problem with the zip file. */ ZipError = 'flashFirmware.failToFinish.reason.zipError', /** Metadata property is missing or invalid. */ @@ -87,6 +89,10 @@ export type FailToFinishReasonNoFirmware = Reason; +export type FailToFinishReasonFailedToFetch = Reason & { + response: Response; +}; + export type FailToFinishReasonZipError = Reason & { err: FirmwareReaderError; }; @@ -112,6 +118,7 @@ export type FailToFinishReason = | FailToFinishReasonHubError | FailToFinishReasonNoFirmware | FailToFinishReasonDeviceMismatch + | FailToFinishReasonFailedToFetch | FailToFinishReasonZipError | FailToFinishReasonBadMetadata | FailToFinishReasonFirmwareSize @@ -183,6 +190,11 @@ export function didFailToFinish( hubError: HubError, ): FlashFirmwareDidFailToFinishAction; +export function didFailToFinish( + reason: FailToFinishReasonType.FailedToFetch, + response: Response, +): FlashFirmwareDidFailToFinishAction; + export function didFailToFinish( reason: FailToFinishReasonType.ZipError, err: FirmwareReaderError, @@ -204,6 +216,7 @@ export function didFailToFinish( FailToFinishReasonType, | FailToFinishReasonType.BleError | FailToFinishReasonType.HubError + | FailToFinishReasonType.FailedToFetch | FailToFinishReasonType.ZipError | FailToFinishReasonType.BadMetadata | FailToFinishReasonType.Unknown @@ -216,7 +229,7 @@ export function didFailToFinish( */ export function didFailToFinish( reason: FailToFinishReasonType, - arg1?: string | HubError | Error, + arg1?: string | HubError | Error | Response, arg2?: MetadataProblem, ): FlashFirmwareDidFailToFinishAction { if (reason === FailToFinishReasonType.BleError) { @@ -241,6 +254,17 @@ export function didFailToFinish( }; } + if (reason === FailToFinishReasonType.FailedToFetch) { + // istanbul ignore if: programmer error give wrong arg + if (!(arg1 instanceof Response)) { + throw new Error('missing or invalid response'); + } + return { + type: FlashFirmwareActionType.DidFailToFinish, + reason: { reason, response: arg1 }, + }; + } + if (reason === FailToFinishReasonType.ZipError) { // istanbul ignore if: programmer error give wrong arg if (!(arg1 instanceof FirmwareReaderError)) { diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 5d0e3c37..240ed60f 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -541,6 +541,77 @@ describe('flashFirmware', () => { await saga.end(); }); + test('failed to fetch firmware', 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 response = new Response(undefined, { status: 404 }); + jest.spyOn(window, 'fetch').mockResolvedValueOnce(response); + + const saga = new AsyncSaga( + flashFirmware, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + saga.put(didConnect()); + + // then find out what kind of hub it is + + action = await saga.take(); + expect(action).toEqual(infoRequest(0)); + + // received an unknown hub type ID + + saga.put(didRequest(0)); + saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); + + // should raise an error that we don't have any firmware for this hub + + action = await saga.take(); + expect(action).toStrictEqual( + didFailToFinish(FailToFinishReasonType.FailedToFetch, response), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); + test('connected device does not match firmware device', async () => { const metadata: FirmwareMetadata = { 'metadata-version': '1.0.0', diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 06ee3575..218c5cea 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -47,7 +47,6 @@ import { checksumRequest, connect, disconnect, - disconnectRequest, eraseRequest, infoRequest, initRequest, @@ -60,7 +59,6 @@ import { MpyDidFailToCompileAction, compile, } from '../actions/mpy'; -import * as notification from '../actions/notification'; import { MaxProgramFlashSize, Result } from '../protocols/lwp3-bootloader'; import { RootState } from '../reducers'; import { BootloaderConnectionState } from '../reducers/bootloader'; @@ -322,10 +320,10 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { const response = yield* call(() => fetch(firmwarePath)); if (!response.ok) { - yield* put(notification.add('error', 'Failed to fetch firmware.')); - const disconnectAction = yield* put(disconnectRequest(nextMessageId())); - yield* waitForDidRequest(disconnectAction.id); - return; + yield* put( + didFailToFinish(FailToFinishReasonType.FailedToFetch, response), + ); + yield* disconnectAndCancel(); } const data = yield* call(() => response.arrayBuffer()); From 4bbd1efad54688e2f30814ae88278acd10a0ff64 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 18:23:59 -0600 Subject: [PATCH 22/32] add proper error handling for init command error --- src/sagas/flash-firmware.test.ts | 110 +++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 6 +- 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 240ed60f..a439064f 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -872,6 +872,116 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('init response is failed', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + 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.Error)); + + // should get a hub error + + action = await saga.take(); + expect(action).toEqual( + didFailToFinish(FailToFinishReasonType.HubError, HubError.InitFailed), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 218c5cea..b3b348f9 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -360,8 +360,10 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { ), }); if (init.result) { - // TODO: proper error handling - throw Error(`Failed to init: ${init}`); + yield* put( + didFailToFinish(FailToFinishReasonType.HubError, HubError.InitFailed), + ); + yield* disconnectAndCancel(); } // 14 is "safe" size for all hubs From 768762347a44d269ae6366523ca152250d9daf8a Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 18:29:47 -0600 Subject: [PATCH 23/32] add proper error handling for firmware flash size --- src/sagas/flash-firmware.test.ts | 150 +++++++++++++++++++++++++++++++ src/sagas/flash-firmware.ts | 9 +- 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index a439064f..6a2bab88 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -982,6 +982,156 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('size mismatch after flashing', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + 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) { + expect(count).toBe(10); + break; + } + + if (count % 10 === 0) { + action = await saga.take(); + expect(action).toEqual(checksumRequest(++id)); + + saga.put(didRequest(id)); + saga.put(checksumResponse(0)); + } + } + + // hub indicates incorrect size + + saga.put(programResponse(0, totalFirmwareSize - 1)); + + // should get a hub error + + action = await saga.take(); + expect(action).toEqual( + didFailToFinish( + FailToFinishReasonType.HubError, + HubError.CountMismatch, + ), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index b3b348f9..814e1e39 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -410,8 +410,13 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { 5000, ); if (flash.count !== firmware.length) { - // TODO: proper error handling - throw Error("Didn't flash all bytes"); + yield* put( + didFailToFinish( + FailToFinishReasonType.HubError, + HubError.CountMismatch, + ), + ); + yield* disconnectAndCancel(); } yield* put(didProgress(1)); From b5d3fe17da13484a7e5d2224c28946397b0a36bd Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 18:54:43 -0600 Subject: [PATCH 24/32] add proper error handling for checksum mismatch --- src/sagas/flash-firmware.test.ts | 158 ++++++++++++++++++++++++++++++- src/sagas/flash-firmware.ts | 31 ++++-- 2 files changed, 177 insertions(+), 12 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 6a2bab88..eea5b67c 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -184,7 +184,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0, totalFirmwareSize)); + saga.put(programResponse(0xffffff42, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); @@ -1113,7 +1113,7 @@ describe('flashFirmware', () => { // hub indicates incorrect size - saga.put(programResponse(0, totalFirmwareSize - 1)); + saga.put(programResponse(0xffffff33, totalFirmwareSize - 1)); // should get a hub error @@ -1132,6 +1132,156 @@ describe('flashFirmware', () => { await saga.end(); }); + + test('checksum mismatch after flashing', 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, + { + bootloader: { connection: BootloaderConnectionState.Disconnected }, + settings: { flashCurrentProgram: false }, + }, + { + nextMessageId: createCountFunc(), + }, + ); + + // 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.updateState({ + bootloader: { connection: BootloaderConnectionState.Connected }, + }); + 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).toMatchInlineSnapshot(` + Object { + "options": Array [ + "-mno-unicode", + ], + "script": "print(\\"test\\")", + "type": "mpy.action.compile", + } + `); + + 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) { + expect(count).toBe(10); + break; + } + + if (count % 10 === 0) { + action = await saga.take(); + expect(action).toEqual(checksumRequest(++id)); + + saga.put(didRequest(id)); + saga.put(checksumResponse(0)); + } + } + + // hub indicates incorrect checksum + + saga.put(programResponse(0xffffffff, totalFirmwareSize)); + + // should get a hub error + + action = await saga.take(); + expect(action).toEqual( + didFailToFinish( + FailToFinishReasonType.HubError, + HubError.ChecksumMismatch, + ), + ); + + // should request to disconnect after failure + + action = await saga.take(); + expect(action).toEqual(disconnect()); + + await saga.end(); + }); }); describe('user supplied firmware.zip', () => { @@ -1265,7 +1415,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0, totalFirmwareSize)); + saga.put(programResponse(0xffffff97, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); @@ -1794,7 +1944,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0, totalFirmwareSize)); + saga.put(programResponse(0xffffff33, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 814e1e39..a2a57674 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -150,7 +150,7 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator { function* loadFirmware( data: ArrayBuffer, program: string | undefined, -): SagaGenerator<{ firmware: Uint8Array; deviceId: HubType }> { +): SagaGenerator<{ firmware: Uint8Array; deviceId: HubType; checksum: number }> { const [reader, readerErr] = yield* call(() => maybe(FirmwareReader.load(data))); if (readerErr) { @@ -224,13 +224,13 @@ function* loadFirmware( yield* disconnectAndCancel(); } - firmwareView.setUint32( - checksumOffset, - sumComplement32(firmwareIterator(firmwareView, metadata['max-firmware-size'])), - true, + const checksum = sumComplement32( + firmwareIterator(firmwareView, metadata['max-firmware-size']), ); - return { firmware, deviceId: metadata['device-id'] }; + firmwareView.setUint32(checksumOffset, checksum, true); + + return { firmware, deviceId: metadata['device-id'], checksum }; } /** @@ -258,6 +258,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { try { let firmware: Uint8Array | undefined = undefined; let deviceId: HubType | undefined = undefined; + let checksum: number | undefined = undefined; let program: string | undefined = undefined; @@ -278,7 +279,10 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } if (action.data !== undefined) { - ({ firmware, deviceId } = yield* loadFirmware(action.data, program)); + ({ firmware, deviceId, checksum } = yield* loadFirmware( + action.data, + program, + )); } yield* put(connect()); @@ -327,7 +331,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } const data = yield* call(() => response.arrayBuffer()); - ({ firmware, deviceId } = yield* loadFirmware(data, program)); + ({ firmware, deviceId, checksum } = yield* loadFirmware(data, program)); if (deviceId !== undefined && info.hubType !== deviceId) { yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch)); @@ -409,6 +413,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { BootloaderResponseActionType.Program, 5000, ); + if (flash.count !== firmware.length) { yield* put( didFailToFinish( @@ -419,6 +424,16 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { yield* disconnectAndCancel(); } + if (~flash.checksum !== checksum) { + yield* put( + didFailToFinish( + FailToFinishReasonType.HubError, + HubError.ChecksumMismatch, + ), + ); + yield* disconnectAndCancel(); + } + yield* put(didProgress(1)); // this will cause the remote device to disconnect and reboot From bf6d64c211d6d37432ba4c96d509bf8b282d885a Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 19:03:37 -0600 Subject: [PATCH 25/32] fix unbound method call --- src/sagas/lwp3-bootloader-ble.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sagas/lwp3-bootloader-ble.ts b/src/sagas/lwp3-bootloader-ble.ts index c5235355..f22c155e 100644 --- a/src/sagas/lwp3-bootloader-ble.ts +++ b/src/sagas/lwp3-bootloader-ble.ts @@ -143,7 +143,10 @@ function* connect(_action: BootloaderConnectionAction): Generator { yield takeEvery(notificationChannel, handleNotify); yield takeEvery(BootloaderConnectionActionType.Send, write, characteristic); - yield takeEvery(BootloaderConnectionActionType.Disconnect, server.disconnect); + yield takeEvery( + BootloaderConnectionActionType.Disconnect, + server.disconnect.bind(server), + ); yield put(didConnect()); From 783f7e233701ee0632f8c4e59f8cc109fb1f67f3 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 22 Jan 2021 20:25:01 -0600 Subject: [PATCH 26/32] fix write function being cancelled on disconnect --- src/sagas/lwp3-bootloader-ble.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/sagas/lwp3-bootloader-ble.ts b/src/sagas/lwp3-bootloader-ble.ts index f22c155e..0fbcbe60 100644 --- a/src/sagas/lwp3-bootloader-ble.ts +++ b/src/sagas/lwp3-bootloader-ble.ts @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors // File: sagas/lwp3-bootloader-ble.ts // Handles Bluetooth Low Energy connection to LEGO Wireless Protocol v3 Bootloader service. import { END, eventChannel } from 'redux-saga'; -import { call, cancel, put, takeEvery, takeMaybe } from 'redux-saga/effects'; +import { call, cancel, put, spawn, takeEvery, takeMaybe } from 'redux-saga/effects'; import { BootloaderConnectionAction, BootloaderConnectionActionType, @@ -141,8 +141,15 @@ function* connect(_action: BootloaderConnectionAction): Generator { return; } + // Spawning write so that it can't be canceled. This is important because + // other sagas always expect it to complete with success action or error + // action. + function* spawnWrite(action: BootloaderConnectionSendAction): Generator { + yield spawn(write, characteristic, action); + } + yield takeEvery(notificationChannel, handleNotify); - yield takeEvery(BootloaderConnectionActionType.Send, write, characteristic); + yield takeEvery(BootloaderConnectionActionType.Send, spawnWrite); yield takeEvery( BootloaderConnectionActionType.Disconnect, server.disconnect.bind(server), From 819810e52abc33737ddd401f798787cd0058e915 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sat, 23 Jan 2021 13:36:01 -0600 Subject: [PATCH 27/32] add workaround for bluez notification issue https://crbug.com/1170085 --- src/sagas/lwp3-bootloader-ble.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/sagas/lwp3-bootloader-ble.ts b/src/sagas/lwp3-bootloader-ble.ts index 0fbcbe60..8fe9fbc7 100644 --- a/src/sagas/lwp3-bootloader-ble.ts +++ b/src/sagas/lwp3-bootloader-ble.ts @@ -132,6 +132,14 @@ function* connect(_action: BootloaderConnectionAction): Generator { }); try { + try { + yield call([characteristic, 'stopNotifications']); + } catch { + // HACK: Chromium on Linux (BlueZ) will not receive notifications + // if a device disconnects while notifications are enabled and then + // reconnects. So we have to call stopNotifications() first to get + // back to a known state. https://crbug.com/1170085 + } yield call([characteristic, 'startNotifications']); } catch (err) { notificationChannel.close(); From 1a6f77ece359bd89bdf4324aa88aa6de9ae4eace Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sat, 23 Jan 2021 13:46:54 -0600 Subject: [PATCH 28/32] remove disconnectMonitor() This fixes multiple didFailToFinish() actions for a single flash firmware request in certain cases. Since 783f7e2 is fixed, waitForDidRequest() will always return with success or error. So when used alone, waitForDidRequest() will process the error. When using race(waitForDidRequest(), waitForResponse()), waitForResponse() will handle the error if it wins the race and cancel waitForResponse(). --- src/sagas/flash-firmware.test.ts | 7 +++++-- src/sagas/flash-firmware.ts | 29 +++++++---------------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index eea5b67c..580105d3 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -312,8 +312,6 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(infoRequest(0)); - saga.put(didRequest(0)); - // hub disconnects before replying saga.updateState({ @@ -328,6 +326,11 @@ describe('flashFirmware', () => { didFailToFinish(FailToFinishReasonType.Disconnected), ); + // On city hub, we can end up in this situation. BLE writeValueWithResponse() + // doesn't return until erasing is done, so there is a long window for + // this to happen. + saga.put(didRequest(0, new Error('failed due to disconnect'))); + await saga.end(); }); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index a2a57674..2d2484da 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -11,7 +11,6 @@ import { call, cancel, delay, - fork, getContext, put, race, @@ -107,9 +106,10 @@ function* waitForResponse( type: BootloaderResponseActionType, timeout = 500, ): SagaGenerator { - const { response, error, timedOut } = yield* race({ + const { response, error, disconnected, timedOut } = yield* race({ response: take(type), error: take(BootloaderResponseActionType.Error), + disconnected: take(BootloaderConnectionActionType.DidDisconnect), timedOut: delay(timeout), }); @@ -125,6 +125,11 @@ function* waitForResponse( yield* disconnectAndCancel(); } + if (disconnected) { + yield* put(didFailToFinish(FailToFinishReasonType.Disconnected)); + yield* disconnectAndCancel(); + } + defined(response); return response; @@ -233,23 +238,6 @@ function* loadFirmware( return { firmware, deviceId: metadata['device-id'], checksum }; } -/** - * Monitors for BLE disconnection event. If disconnection occurs, then a failure - * action is raised and the task (including the parent task) is canceled. - */ -function* disconnectMonitor(): SagaGenerator { - const { disconnected } = yield* race({ - disconnected: take(BootloaderConnectionActionType.DidDisconnect), - finished: take(FlashFirmwareActionType.DidFinish), - failedToFinish: take(FlashFirmwareActionType.DidFailToFinish), - }); - - if (disconnected) { - yield* put(didFailToFinish(FailToFinishReasonType.Disconnected)); - yield* disconnectAndCancel(); - } -} - /** * Flashes firmware to a Powered Up device. * @param action The action that triggered this saga. @@ -296,8 +284,6 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { return; } - const disconnectMonitorTask = yield* fork(disconnectMonitor); - const nextMessageId = yield* getContext<() => number>('nextMessageId'); const infoAction = yield* put(infoRequest(nextMessageId())); @@ -438,7 +424,6 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // this will cause the remote device to disconnect and reboot const rebootAction = yield* put(rebootRequest(nextMessageId())); - disconnectMonitorTask.cancel(); yield* waitForDidRequest(rebootAction.id); yield* put(didFinish()); From 5d1311216c083c4225391464dfc33da7e0a204b6 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sat, 23 Jan 2021 14:46:46 -0600 Subject: [PATCH 29/32] add notifications for firmware flash errors --- src/components/notification-i18n.en.json | 15 ++++++ src/components/notification-i18n.ts | 13 +++++ src/sagas/notification.test.ts | 33 ++++++++++++ src/sagas/notification.ts | 69 ++++++++++++++++++++++++ 4 files changed, 130 insertions(+) diff --git a/src/components/notification-i18n.en.json b/src/components/notification-i18n.en.json index d0ae6e09..277d643a 100644 --- a/src/components/notification-i18n.en.json +++ b/src/components/notification-i18n.en.json @@ -13,6 +13,21 @@ "action": "Reload" } }, + "flashFirmware": { + "connectionFailed": "Could not connect to the hub. Restart the hub and try again.", + "timedOut": "The hub took too long to respond. Restart the hub and try again.", + "bleError": "There was a problem with Bluetooth.", + "disconnected": "The hub was disconnected before flashing was completed. Restart the hub and try again.", + "hubError": "The hub said something went wrong.", + "unsupportedDevice": "The connected hub is not supported.", + "deviceMismatch": "The firmware is for a different kind of hub from the connected hub.", + "failToFetch": "Failed to fetch firmware from the server: {status}", + "badZipFile": "The firmware.zip file is missing required files or is corrupt.", + "badMetadata": "The firmware.metadata.py file contains missing or invalid entries. Fix it then try again.", + "compileError": "The included main.py file could not be compiled. Fix it then try again.", + "sizeTooBig": "The combined firmware and main.py are too big to fit in the flash memory.", + "unexpectedError": "Unexpected error while trying to flash firmware: {errorMessage}" + }, "mpy": { "error": "{errorMessage}" }, diff --git a/src/components/notification-i18n.ts b/src/components/notification-i18n.ts index 48414268..c27294a6 100644 --- a/src/components/notification-i18n.ts +++ b/src/components/notification-i18n.ts @@ -10,6 +10,19 @@ export enum MessageId { BleGattPermission = 'ble.gattPermission', BleGattServiceNotFound = 'ble.gattServiceNotFound', BleNoWebBluetooth = 'ble.noWebBluetooth', + FlashFirmwareConnectionFailed = 'flashFirmware.connectionFailed', + FlashFirmwareTimedOut = 'flashFirmware.timedOut', + FlashFirmwareBleError = 'flashFirmware.bleError', + FlashFirmwareDisconnected = 'flashFirmware.disconnected', + FlashFirmwareHubError = 'flashFirmware.hubError', + FlashFirmwareUnsupportedDevice = 'flashFirmware.unsupportedDevice', + FlashFirmwareDeviceMismatch = 'flashFirmware.deviceMismatch', + FlashFirmwareFailToFetch = 'flashFirmware.failToFetch', + FlashFirmwareBadZipFile = 'flashFirmware.badZipFile', + FlashFirmwareBadMetadata = 'flashFirmware.badMetadata', + FlashFirmwareCompileError = 'flashFirmware.compileError', + FlashFirmwareSizeTooBig = 'flashFirmware.sizeTooBig', + FlashFirmwareUnexpectedError = 'flashFirmware.unexpectedError', ProgramChangedMessage = 'editor.programChanged.message', ProgramChangedAction = 'editor.programChanged.action', ServiceWorkerUpdateMessage = 'serviceWorker.update.message', diff --git a/src/sagas/notification.test.ts b/src/sagas/notification.test.ts index 28887502..0fa962a8 100644 --- a/src/sagas/notification.test.ts +++ b/src/sagas/notification.test.ts @@ -2,6 +2,7 @@ // Copyright (c) 2021 The Pybricks Authors import { IToaster } from '@blueprintjs/core'; +import { FirmwareReaderError, FirmwareReaderErrorCode } from '@pybricks/firmware'; import { AsyncSaga } from '../../test'; import { Action } from '../actions'; import { @@ -9,6 +10,12 @@ import { didFailToConnect as bleDidFailToConnect, } from '../actions/ble'; import { storageChanged } from '../actions/editor'; +import { + FailToFinishReasonType, + HubError, + MetadataProblem, + didFailToFinish, +} from '../actions/flash-firmware'; import { BootloaderConnectionFailureReason, didFailToConnect as bootloaderDidFailToConnect, @@ -37,6 +44,32 @@ test.each([ add('warning', 'message'), add('error', 'message', 'url'), didUpdate({} as ServiceWorkerRegistration), + didFailToFinish(FailToFinishReasonType.FailedToConnect), + didFailToFinish(FailToFinishReasonType.TimedOut), + didFailToFinish( + FailToFinishReasonType.BleError, + new DOMException('test error', 'NetworkError'), + ), + didFailToFinish(FailToFinishReasonType.Disconnected), + didFailToFinish(FailToFinishReasonType.HubError, HubError.UnknownCommand), + didFailToFinish(FailToFinishReasonType.NoFirmware), + didFailToFinish(FailToFinishReasonType.DeviceMismatch), + didFailToFinish( + FailToFinishReasonType.FailedToFetch, + new Response(undefined, { status: 404 }), + ), + didFailToFinish( + FailToFinishReasonType.ZipError, + new FirmwareReaderError(FirmwareReaderErrorCode.ZipError), + ), + didFailToFinish( + FailToFinishReasonType.BadMetadata, + 'device-id', + MetadataProblem.NotSupported, + ), + didFailToFinish(FailToFinishReasonType.FailedToCompile), + didFailToFinish(FailToFinishReasonType.FirmwareSize), + didFailToFinish(FailToFinishReasonType.Unknown, new Error('test error')), ])('actions that should show notification: %o', async (action: Action) => { const getToasts = jest.fn().mockReturnValue([]); const show = jest.fn(); diff --git a/src/sagas/notification.ts b/src/sagas/notification.ts index 687e0e59..1312f0a9 100644 --- a/src/sagas/notification.ts +++ b/src/sagas/notification.ts @@ -21,6 +21,11 @@ import { BleDeviceFailToConnectReasonType, } from '../actions/ble'; import { EditorActionType, reloadProgram } from '../actions/editor'; +import { + FailToFinishReasonType, + FlashFirmwareActionType, + FlashFirmwareDidFailToFinishAction, +} from '../actions/flash-firmware'; import { BootloaderConnectionActionType, BootloaderConnectionDidFailToConnectAction, @@ -226,6 +231,69 @@ function* showEditorStorageChanged(): Generator { yield put(reloadProgram()); } +function* showFlashFirmwareError( + action: FlashFirmwareDidFailToFinishAction, +): Generator { + switch (action.reason.reason) { + case FailToFinishReasonType.FailedToConnect: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareConnectionFailed); + break; + case FailToFinishReasonType.TimedOut: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareTimedOut); + break; + case FailToFinishReasonType.BleError: + yield* showUnexpectedError( + MessageId.FlashFirmwareBleError, + action.reason.err, + ); + break; + case FailToFinishReasonType.Disconnected: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareDisconnected); + break; + case FailToFinishReasonType.HubError: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareHubError); + if (process.env.NODE_ENV !== 'test') { + console.error(action.reason.hubError); + } + break; + case FailToFinishReasonType.NoFirmware: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareUnsupportedDevice); + break; + case FailToFinishReasonType.DeviceMismatch: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareDeviceMismatch); + break; + case FailToFinishReasonType.FailedToFetch: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareFailToFetch, { + status: action.reason.response.statusText, + }); + break; + case FailToFinishReasonType.ZipError: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareBadZipFile); + if (process.env.NODE_ENV !== 'test') { + console.error(action.reason.err); + } + break; + case FailToFinishReasonType.BadMetadata: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareBadMetadata); + if (process.env.NODE_ENV !== 'test') { + console.error(action.reason.property, action.reason.problem); + } + break; + case FailToFinishReasonType.FailedToCompile: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareCompileError); + break; + case FailToFinishReasonType.FirmwareSize: + yield* showSingleton(Level.Error, MessageId.FlashFirmwareSizeTooBig); + break; + case FailToFinishReasonType.Unknown: + yield* showUnexpectedError( + MessageId.FlashFirmwareUnexpectedError, + action.reason.err, + ); + break; + } +} + function* dismissCompilerError(): Generator { const { toaster } = (yield getContext('notification')) as NotificationContext; toaster.dismiss(MessageId.MpyError); @@ -282,6 +350,7 @@ export default function* (): Generator { showBootloaderDidFailToConnectError, ); yield takeEvery(EditorActionType.StorageChanged, showEditorStorageChanged); + yield takeEvery(FlashFirmwareActionType.DidFailToFinish, showFlashFirmwareError); yield takeEvery(MpyActionType.DidCompile, dismissCompilerError); yield takeEvery(MpyActionType.DidFailToCompile, showCompilerError); yield takeEvery(NotificationActionType.Add, addNotification); From dcaf9f6ff44825ba584fb499a40f33e54fe67f03 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sat, 23 Jan 2021 15:28:38 -0600 Subject: [PATCH 30/32] fix checksum verification the checksum returned by the hub is only one byte, so the 4-byte checksum used in the firmware can't be used --- src/sagas/flash-firmware.test.ts | 10 +++++----- src/sagas/flash-firmware.ts | 22 +++++++++++++--------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts index 580105d3..2f356a60 100644 --- a/src/sagas/flash-firmware.test.ts +++ b/src/sagas/flash-firmware.test.ts @@ -184,7 +184,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0xffffff42, totalFirmwareSize)); + saga.put(programResponse(0x62, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); @@ -1116,7 +1116,7 @@ describe('flashFirmware', () => { // hub indicates incorrect size - saga.put(programResponse(0xffffff33, totalFirmwareSize - 1)); + saga.put(programResponse(0x62, totalFirmwareSize - 1)); // should get a hub error @@ -1266,7 +1266,7 @@ describe('flashFirmware', () => { // hub indicates incorrect checksum - saga.put(programResponse(0xffffffff, totalFirmwareSize)); + saga.put(programResponse(0x100, totalFirmwareSize)); // should get a hub error @@ -1418,7 +1418,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0xffffff97, totalFirmwareSize)); + saga.put(programResponse(0xf3, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); @@ -1947,7 +1947,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0xffffff33, totalFirmwareSize)); + saga.put(programResponse(0x27, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 2d2484da..84961607 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -155,7 +155,7 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator { function* loadFirmware( data: ArrayBuffer, program: string | undefined, -): SagaGenerator<{ firmware: Uint8Array; deviceId: HubType; checksum: number }> { +): SagaGenerator<{ firmware: Uint8Array; deviceId: HubType }> { const [reader, readerErr] = yield* call(() => maybe(FirmwareReader.load(data))); if (readerErr) { @@ -235,7 +235,7 @@ function* loadFirmware( firmwareView.setUint32(checksumOffset, checksum, true); - return { firmware, deviceId: metadata['device-id'], checksum }; + return { firmware, deviceId: metadata['device-id'] }; } /** @@ -246,7 +246,6 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { try { let firmware: Uint8Array | undefined = undefined; let deviceId: HubType | undefined = undefined; - let checksum: number | undefined = undefined; let program: string | undefined = undefined; @@ -267,10 +266,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } if (action.data !== undefined) { - ({ firmware, deviceId, checksum } = yield* loadFirmware( - action.data, - program, - )); + ({ firmware, deviceId } = yield* loadFirmware(action.data, program)); } yield* put(connect()); @@ -317,7 +313,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } const data = yield* call(() => response.arrayBuffer()); - ({ firmware, deviceId, checksum } = yield* loadFirmware(data, program)); + ({ firmware, deviceId } = yield* loadFirmware(data, program)); if (deviceId !== undefined && info.hubType !== deviceId) { yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch)); @@ -410,7 +406,15 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { yield* disconnectAndCancel(); } - if (~flash.checksum !== checksum) { + const checksum = firmware.reduce((prev, curr) => prev ^ curr, 0xff); + if (flash.checksum !== checksum) { + if (process.env.NODE_ENV !== 'test') { + console.log( + 'checksum:', + flash.checksum.toString(16).padStart(2, '0').padStart(4, '0x'), + checksum.toString(16).padStart(2, '0').padStart(4, '0x'), + ); + } yield* put( didFailToFinish( FailToFinishReasonType.HubError, From 5d9040647b501743f884dcd1dc166191212f40aa Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sat, 23 Jan 2021 15:46:20 -0600 Subject: [PATCH 31/32] fix two-level merging of state in tests --- test/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/index.ts b/test/index.ts index 0fad318b..6451e979 100644 --- a/test/index.ts +++ b/test/index.ts @@ -72,7 +72,10 @@ export class AsyncSaga { } public updateState(state: RecursivePartial): void { - this.state = { ...this.state, ...state }; + for (const key of Object.keys(state) as Array) { + // @ts-expect-error: writing to readonly for testing + this.state[key] = { ...this.state[key], ...state[key] }; + } } public async end(): Promise { From 467627dc7586a7ea5c031ffd62a971cf1cfa5024 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sat, 23 Jan 2021 17:29:43 -0600 Subject: [PATCH 32/32] flash-firmware: drop error on fail to connect This error is already handled elsewhere. --- src/components/notification-i18n.en.json | 1 - src/components/notification-i18n.ts | 1 - src/sagas/notification.test.ts | 2 +- src/sagas/notification.ts | 3 --- 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/components/notification-i18n.en.json b/src/components/notification-i18n.en.json index 277d643a..7555b749 100644 --- a/src/components/notification-i18n.en.json +++ b/src/components/notification-i18n.en.json @@ -14,7 +14,6 @@ } }, "flashFirmware": { - "connectionFailed": "Could not connect to the hub. Restart the hub and try again.", "timedOut": "The hub took too long to respond. Restart the hub and try again.", "bleError": "There was a problem with Bluetooth.", "disconnected": "The hub was disconnected before flashing was completed. Restart the hub and try again.", diff --git a/src/components/notification-i18n.ts b/src/components/notification-i18n.ts index c27294a6..c519a1f6 100644 --- a/src/components/notification-i18n.ts +++ b/src/components/notification-i18n.ts @@ -10,7 +10,6 @@ export enum MessageId { BleGattPermission = 'ble.gattPermission', BleGattServiceNotFound = 'ble.gattServiceNotFound', BleNoWebBluetooth = 'ble.noWebBluetooth', - FlashFirmwareConnectionFailed = 'flashFirmware.connectionFailed', FlashFirmwareTimedOut = 'flashFirmware.timedOut', FlashFirmwareBleError = 'flashFirmware.bleError', FlashFirmwareDisconnected = 'flashFirmware.disconnected', diff --git a/src/sagas/notification.test.ts b/src/sagas/notification.test.ts index 0fa962a8..b7b114bc 100644 --- a/src/sagas/notification.test.ts +++ b/src/sagas/notification.test.ts @@ -44,7 +44,6 @@ test.each([ add('warning', 'message'), add('error', 'message', 'url'), didUpdate({} as ServiceWorkerRegistration), - didFailToFinish(FailToFinishReasonType.FailedToConnect), didFailToFinish(FailToFinishReasonType.TimedOut), didFailToFinish( FailToFinishReasonType.BleError, @@ -97,6 +96,7 @@ test.each([ test.each([ bleDidFailToConnect({ reason: BleDeviceFailToConnectReasonType.Canceled }), bootloaderDidFailToConnect(BootloaderConnectionFailureReason.Canceled), + didFailToFinish(FailToFinishReasonType.FailedToConnect), didSucceed({} as ServiceWorkerRegistration), ])('actions that should not show a notification: %o', async (action: Action) => { const getToasts = jest.fn().mockReturnValue([]); diff --git a/src/sagas/notification.ts b/src/sagas/notification.ts index 1312f0a9..7f2aa445 100644 --- a/src/sagas/notification.ts +++ b/src/sagas/notification.ts @@ -235,9 +235,6 @@ function* showFlashFirmwareError( action: FlashFirmwareDidFailToFinishAction, ): Generator { switch (action.reason.reason) { - case FailToFinishReasonType.FailedToConnect: - yield* showSingleton(Level.Error, MessageId.FlashFirmwareConnectionFailed); - break; case FailToFinishReasonType.TimedOut: yield* showSingleton(Level.Error, MessageId.FlashFirmwareTimedOut); break;