decouple message id counter from actions

Actions should be pure functions for ease of testing.

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<BleUartActionType.Write> & {
     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<BleUartActionType.DidWrite> & {

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<T extends BootloaderRequestActionType> = Action<T> & {
     /**
      * Unique identifier for this action.
@@ -210,8 +207,8 @@ export type BootloaderEraseRequestAction = BaseBootloaderRequestAction<Bootloade
 /**
  * Creates a request to erase the flash memory.
  */
-export function eraseRequest(): BootloaderEraseRequestAction {
-    return { type: BootloaderRequestActionType.Erase, id: nextRequestId() };
+export function eraseRequest(id: number): BootloaderEraseRequestAction {
+    return { type: BootloaderRequestActionType.Erase, id };
 }
 
 /**
@@ -228,12 +225,13 @@ export type BootloaderProgramRequestAction = BaseBootloaderRequestAction<Bootloa
  * @param payload The bytes to write (max 14 bytes!)
  */
 export function programRequest(
+    id: number,
     address: number,
     payload: ArrayBuffer,
 ): BootloaderProgramRequestAction {
     return {
         type: BootloaderRequestActionType.Program,
-        id: nextRequestId(),
+        id,
         address,
         payload,
     };
@@ -247,8 +245,8 @@ export type BootloaderRebootRequestAction = BaseBootloaderRequestAction<Bootload
 /**
  * Creates a request to reboot the hub.
  */
-export function rebootRequest(): BootloaderRebootRequestAction {
-    return { type: BootloaderRequestActionType.Reboot, id: nextRequestId() };
+export function rebootRequest(id: number): BootloaderRebootRequestAction {
+    return { type: BootloaderRequestActionType.Reboot, id };
 }
 
 /**
@@ -262,10 +260,13 @@ export type BootloaderInitRequestAction = BaseBootloaderRequestAction<Bootloader
  * Creates a request to initialize the firmware flashing process.
  * @param firmwareSize The size of the firmware to written to flash memory.
  */
-export function initRequest(firmwareSize: number): BootloaderInitRequestAction {
+export function initRequest(
+    id: number,
+    firmwareSize: number,
+): BootloaderInitRequestAction {
     return {
         type: BootloaderRequestActionType.Init,
-        id: nextRequestId(),
+        id,
         firmwareSize,
     };
 }
@@ -278,8 +279,8 @@ export type BootloaderInfoRequestAction = BaseBootloaderRequestAction<Bootloader
 /**
  * Creates a request to get information about the hub.
  */
-export function infoRequest(): BootloaderInfoRequestAction {
-    return { type: BootloaderRequestActionType.Info, id: nextRequestId() };
+export function infoRequest(id: number): BootloaderInfoRequestAction {
+    return { type: BootloaderRequestActionType.Info, id };
 }
 
 /**
@@ -292,8 +293,8 @@ export type BootloaderChecksumRequestAction = BaseBootloaderRequestAction<Bootlo
  * Creates a request to get the checksum of the bytes that have been written
  * to flash so far.
  */
-export function checksumRequest(): BootloaderChecksumRequestAction {
-    return { type: BootloaderRequestActionType.Checksum, id: nextRequestId() };
+export function checksumRequest(id: number): BootloaderChecksumRequestAction {
+    return { type: BootloaderRequestActionType.Checksum, id };
 }
 
 /**
@@ -304,8 +305,8 @@ export type BootloaderStateRequestAction = BaseBootloaderRequestAction<Bootloade
 /**
  * Creates a request to get the bootloader flash memory protection state.
  */
-export function stateRequest(): BootloaderStateRequestAction {
-    return { type: BootloaderRequestActionType.State, id: nextRequestId() };
+export function stateRequest(id: number): BootloaderStateRequestAction {
+    return { type: BootloaderRequestActionType.State, id };
 }
 
 /**
@@ -316,8 +317,8 @@ export type BootloaderDisconnectRequestAction = BaseBootloaderRequestAction<Boot
 /**
  * Creates a request to disconnect the hub.
  */
-export function disconnectRequest(): BootloaderDisconnectRequestAction {
-    return { type: BootloaderRequestActionType.Disconnect, id: nextRequestId() };
+export function disconnectRequest(id: number): BootloaderDisconnectRequestAction {
+    return { type: BootloaderRequestActionType.Disconnect, id };
 }
 
 /**

src/index.tsx

@@ -18,10 +18,17 @@ import reportWebVitals from './reportWebVitals';
 import rootSaga from './sagas';
 import * as serviceWorkerRegistration from './serviceWorkerRegistration';
 import { i18nManager } from './settings/i18n';
+import { createCountFunc } from './utils/iter';
 
 const toaster = I18nToaster.create(i18nManager);
 
-const sagaMiddleware = createSagaMiddleware({ context: { notification: { toaster } } });
+const sagaMiddleware = createSagaMiddleware({
+    context: {
+        nextMessageId: createCountFunc(),
+        notification: { toaster },
+    },
+});
+
 // TODO: add runtime option or filter - logger affects firmware flash performance
 const loggerMiddleware = createLogger({ predicate: () => false });
 

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());

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<Ace.EditSession>();
         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());
 

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<HubChecksumMessageAction>;
 
+    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 {

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());

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'));
 

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(