From f186e6414dfb2e1c2f073d4bbcfac0587f930cfd Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 20 Jan 2021 17:04:51 -0600 Subject: [PATCH] decouple message id counter from actions Actions should be pure functions for ease of testing. --- src/actions/ble-uart.ts | 9 ++---- src/actions/lwp3-bootloader.ts | 37 +++++++++++----------- src/index.tsx | 9 +++++- src/sagas/flash-firmware.ts | 29 ++++++++++++----- src/sagas/hub.test.ts | 7 ++-- src/sagas/hub.ts | 17 +++++++--- src/sagas/lwp3-bootloader-protocol.test.ts | 25 ++++++++------- src/sagas/terminal.test.ts | 31 +++++++++--------- src/sagas/terminal.ts | 7 ++-- 9 files changed, 101 insertions(+), 70 deletions(-) diff --git a/src/actions/ble-uart.ts b/src/actions/ble-uart.ts index 56e92107..32eb07db 100644 --- a/src/actions/ble-uart.ts +++ b/src/actions/ble-uart.ts @@ -1,9 +1,8 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors // actions/ble-uart.ts: Actions for Bluetooth Low Energy nRF UART service import { Action } from 'redux'; -import { createCountFunc } from '../utils/iter'; /** * BLE nRF UART service actions types. @@ -27,15 +26,13 @@ export enum BleUartActionType { Notify = 'ble.data.action.receive', } -const nextId = createCountFunc(); - export type BleUartWriteAction = Action & { id: number; value: Uint8Array; }; -export function write(value: Uint8Array): BleUartWriteAction { - return { type: BleUartActionType.Write, id: nextId(), value }; +export function write(id: number, value: Uint8Array): BleUartWriteAction { + return { type: BleUartActionType.Write, id, value }; } export type BleUartDidWriteAction = Action & { diff --git a/src/actions/lwp3-bootloader.ts b/src/actions/lwp3-bootloader.ts index 15e43a3c..6bcfbe6a 100644 --- a/src/actions/lwp3-bootloader.ts +++ b/src/actions/lwp3-bootloader.ts @@ -8,7 +8,6 @@ import { ProtectionLevel, Result, } from '../protocols/lwp3-bootloader'; -import { createCountFunc } from '../utils/iter'; /** * Bootloader BLE connection actions. @@ -193,8 +192,6 @@ export enum BootloaderRequestActionType { Disconnect = 'bootloader.action.request.disconnect', } -const nextRequestId = createCountFunc(); - type BaseBootloaderRequestAction = Action & { /** * Unique identifier for this action. @@ -210,8 +207,8 @@ export type BootloaderEraseRequestAction = BaseBootloaderRequestAction false }); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index f80d6c8d..85191478 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -11,6 +11,7 @@ import { all, call, delay, + getContext, put, race, select, @@ -221,7 +222,11 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { return; } - const infoAction = (yield put(infoRequest())) as BootloaderInfoRequestAction; + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + + const infoAction = (yield put( + infoRequest(nextMessageId()), + )) as BootloaderInfoRequestAction; const [, info] = (yield all([ waitForDidSend(infoAction.id), waitForResponse(BootloaderResponseActionType.Info), @@ -243,7 +248,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { "Sorry, we don't have firmware for this hub yet.", ), ); - yield put(disconnectRequest()); + yield put(disconnectRequest(nextMessageId())); return; } @@ -251,7 +256,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { if (!response.ok) { yield put(notification.add('error', 'Failed to fetch firmware.')); const disconnectAction = (yield put( - disconnectRequest(), + disconnectRequest(nextMessageId()), )) as BootloaderDisconnectRequestAction; yield waitForDidSend(disconnectAction.id); return; @@ -269,7 +274,9 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { yield put(didStart()); - const eraseAction = (yield put(eraseRequest())) as BootloaderEraseRequestAction; + const eraseAction = (yield put( + eraseRequest(nextMessageId()), + )) as BootloaderEraseRequestAction; const [, erase] = (yield all([ waitForDidSend(eraseAction.id), waitForResponse(BootloaderResponseActionType.Erase, 5000), @@ -280,7 +287,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } const initAction = (yield put( - initRequest(firmware.length), + initRequest(nextMessageId(), firmware.length), )) as BootloaderInitRequestAction; const [, init] = (yield all([ waitForDidSend(initAction.id), @@ -301,7 +308,11 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { for (let offset = 0; ; ) { const payload = firmware.slice(offset, offset + maxDataSize); const programAction = (yield put( - programRequest(info[0].startAddress + offset, payload.buffer), + programRequest( + nextMessageId(), + info[0].startAddress + offset, + payload.buffer, + ), )) as BootloaderProgramRequestAction; yield waitForDidSend(programAction.id); @@ -320,7 +331,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { // the hub is not known and could vary by device. if (++count % 10 === 0) { const checksumAction = (yield put( - checksumRequest(), + checksumRequest(nextMessageId()), )) as BootloaderChecksumRequestAction; const [, checksum] = (yield all([ waitForDidSend(checksumAction.id), @@ -351,7 +362,9 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { yield put(didProgress(1)); // this will cause the remote device to disconnect and reboot - const rebootAction = (yield put(rebootRequest())) as BootloaderRebootRequestAction; + const rebootAction = (yield put( + rebootRequest(nextMessageId()), + )) as BootloaderRebootRequestAction; yield waitForDidSend(rebootAction.id); yield put(didFinish()); diff --git a/src/sagas/hub.test.ts b/src/sagas/hub.test.ts index 380ed6a8..9af02713 100644 --- a/src/sagas/hub.test.ts +++ b/src/sagas/hub.test.ts @@ -15,13 +15,14 @@ import { stop, } from '../actions/hub'; import { MpyActionType, didCompile } from '../actions/mpy'; +import { createCountFunc } from '../utils/iter'; import hub from './hub'; jest.mock('ace-builds'); describe('downloadAndRun', () => { test('no errors', async () => { - const saga = new AsyncSaga(hub); + const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); const mockEditor = mock(); saga.setState({ editor: { current: mockEditor } }); @@ -75,7 +76,7 @@ describe('downloadAndRun', () => { }); test('repl', async () => { - const saga = new AsyncSaga(hub); + const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); saga.put(repl()); @@ -86,7 +87,7 @@ test('repl', async () => { }); test('stop', async () => { - const saga = new AsyncSaga(hub); + const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() }); saga.put(stop()); diff --git a/src/sagas/hub.ts b/src/sagas/hub.ts index 91e17530..ba1ba949 100644 --- a/src/sagas/hub.ts +++ b/src/sagas/hub.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { Ace } from 'ace-builds'; import { Channel } from 'redux-saga'; @@ -7,6 +7,7 @@ import { RaceEffect, TakeEffect, actionChannel, + getContext, put, race, select, @@ -80,11 +81,15 @@ function* downloadAndRun(_action: HubDownloadAndRunAction): Generator { HubMessageActionType.Checksum, )) as Channel; + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + // first send payload size as big-endian 32-bit integer const sizeBuf = new Uint8Array(4); const sizeView = new DataView(sizeBuf.buffer); sizeView.setUint32(0, mpy.data.byteLength, true); - const writeAction = (yield put(write(sizeBuf))) as BleUartWriteAction; + const writeAction = (yield put( + write(nextMessageId(), sizeBuf), + )) as BleUartWriteAction; const [, didFailToWrite] = (yield waitForWrite(writeAction.id)) as [ BleUartDidWriteAction, BleUartDidFailToWriteAction, @@ -113,7 +118,7 @@ function* downloadAndRun(_action: HubDownloadAndRunAction): Generator { // we can actually only write 20 bytes at a time for (let j = 0; j < chunk.length; j += SafeTxCharLength) { const writeAction = (yield put( - write(chunk.slice(j, j + SafeTxCharLength)), + write(nextMessageId(), chunk.slice(j, j + SafeTxCharLength)), )) as BleUartWriteAction; const [, didFailToWrite] = (yield waitForWrite(writeAction.id)) as [ BleUartDidWriteAction, @@ -146,14 +151,16 @@ function* downloadAndRun(_action: HubDownloadAndRunAction): Generator { const startReplCommand = new Uint8Array([0x20, 0x20, 0x20, 0x20]); function* startRepl(_action: HubReplAction): Generator { - yield put(write(startReplCommand)); + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + yield put(write(nextMessageId(), startReplCommand)); } // CTRL+C, CTRL+C, CTRL+D const stopCommand = new Uint8Array([0x03, 0x03, 0x04]); function* stop(_action: HubStopAction): Generator { - yield put(write(stopCommand)); + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + yield put(write(nextMessageId(), stopCommand)); } export default function* (): Generator { diff --git a/src/sagas/lwp3-bootloader-protocol.test.ts b/src/sagas/lwp3-bootloader-protocol.test.ts index 8e59a032..5805928f 100644 --- a/src/sagas/lwp3-bootloader-protocol.test.ts +++ b/src/sagas/lwp3-bootloader-protocol.test.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors // File: sagas/lwp3-bootloader-protocol.test.ts import { AsyncSaga } from '../../test'; @@ -40,7 +40,7 @@ describe('message encoder', () => { test.each([ [ 'erase', - eraseRequest(), + eraseRequest(0), [ 0x11, // erase command ], @@ -48,6 +48,7 @@ describe('message encoder', () => { [ 'program', programRequest( + 1, 0x08005000, new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]).buffer, ), @@ -76,14 +77,14 @@ describe('message encoder', () => { ], [ 'reboot', - rebootRequest(), + rebootRequest(2), [ 0x33, // reboot command ], ], [ 'init', - initRequest(100000), + initRequest(3, 100000), [ 0x44, // init command 0xa0, // size LSB @@ -94,28 +95,28 @@ describe('message encoder', () => { ], [ 'info', - infoRequest(), + infoRequest(4), [ 0x55, // info command ], ], [ 'checksum', - checksumRequest(), + checksumRequest(5), [ 0x66, // checksum command ], ], [ 'state', - stateRequest(), + stateRequest(6), [ 0x77, // state command ], ], [ 'disconnect', - disconnectRequest(), + disconnectRequest(7), [ 0x88, // disconnect command ], @@ -143,10 +144,10 @@ describe('message encoder', () => { const saga = new AsyncSaga(bootloader); // we send 4 requests - saga.put({ ...eraseRequest(), id: 0 }); - saga.put({ ...eraseRequest(), id: 1 }); - saga.put({ ...eraseRequest(), id: 2 }); - saga.put({ ...eraseRequest(), id: 3 }); + saga.put(eraseRequest(0)); + saga.put(eraseRequest(1)); + saga.put(eraseRequest(2)); + saga.put(eraseRequest(3)); // but only two didSend action meaning only the first two completed saga.put(didSend()); diff --git a/src/sagas/terminal.test.ts b/src/sagas/terminal.test.ts index 6673b7df..abefe1d2 100644 --- a/src/sagas/terminal.test.ts +++ b/src/sagas/terminal.test.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { AsyncSaga, delay } from '../../test'; @@ -25,11 +25,12 @@ import { sendData, } from '../actions/terminal'; import { HubRuntimeState } from '../reducers/hub'; +import { createCountFunc } from '../utils/iter'; import terminal from './terminal'; describe('Data receiver filters out hub status', () => { test('normal message - no status', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // sending ASCII space character saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -43,7 +44,7 @@ describe('Data receiver filters out hub status', () => { }); test('checksum message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.setState({ hub: { runtime: HubRuntimeState.Loading } }); saga.put(notify(new DataView(new Uint8Array([0xaa]).buffer))); @@ -56,7 +57,7 @@ describe('Data receiver filters out hub status', () => { }); test('idle message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> IDLE' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -88,7 +89,7 @@ describe('Data receiver filters out hub status', () => { }); test('idle message with extra text', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> IDLE1' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -132,7 +133,7 @@ describe('Data receiver filters out hub status', () => { }); test('error message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> ERROR' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -165,7 +166,7 @@ describe('Data receiver filters out hub status', () => { }); test('error message with extra text', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> ERROR1' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -210,7 +211,7 @@ describe('Data receiver filters out hub status', () => { }); test('running message', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '>>>> ERROR' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -245,7 +246,7 @@ describe('Data receiver filters out hub status', () => { }); test('running message with extra text', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); // '0>>>> RUNNING1' saga.setState({ hub: { runtime: HubRuntimeState.Unknown } }); @@ -293,7 +294,7 @@ describe('Data receiver filters out hub status', () => { }); test('Terminal data source responds to send data actions', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(didStart()); const dataSourceAction = await saga.take(); @@ -320,7 +321,7 @@ describe('Terminal data source responds to receive data actions', () => { const expected = new Uint8Array([0x74, 0x65, 0x73, 0x74, 0x31, 0x32, 0x33, 0x34]); test('basic function works', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); @@ -332,7 +333,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('messages are queued until previous has completed', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); await delay(50); // without delay, messages are combined @@ -360,7 +361,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('messages are queued until previous has failed', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); await delay(50); // without delay, messages are combined @@ -390,7 +391,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('small messages are combined', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('test1234')); saga.put(receiveData('test1234')); @@ -405,7 +406,7 @@ describe('Terminal data source responds to receive data actions', () => { }); test('long messages are split', async () => { - const saga = new AsyncSaga(terminal); + const saga = new AsyncSaga(terminal, { nextMessageId: createCountFunc() }); saga.put(receiveData('012345678901234567890123456789')); diff --git a/src/sagas/terminal.ts b/src/sagas/terminal.ts index 141ed9ec..7a828b93 100644 --- a/src/sagas/terminal.ts +++ b/src/sagas/terminal.ts @@ -1,11 +1,12 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors +// Copyright (c) 2020-2021 The Pybricks Authors import { Channel } from 'redux-saga'; import { actionChannel, delay, fork, + getContext, put, race, select, @@ -120,11 +121,13 @@ function* receiveTerminalData(): Generator { value += action.value; } + const nextMessageId = (yield getContext('nextMessageId')) as () => number; + // stdin gets piped to BLE connection const data = encoder.encode(value); for (let i = 0; i < data.length; i += SafeTxCharLength) { const { id } = (yield put( - write(data.slice(i, i + SafeTxCharLength)), + write(nextMessageId(), data.slice(i, i + SafeTxCharLength)), )) as BleUartWriteAction; yield take(