convert lwp3-bootloader from a service to a saga

For some reason this significantly improved bluetooth performance so we
had to readjust the frequency of getting the checksum. The new value
actually makes much more sense. It is unlikely that the bootloader
actually had an 8k buffer.

src/index.tsx

@@ -16,7 +16,6 @@ import NotificationStack from './components/NotificationStack';
 import rootReducer from './reducers';
 import rootSaga from './sagas';
 import * as serviceWorker from './serviceWorker';
-import serviceMiddleware from './services';
 
 const sagaMiddleware = createSagaMiddleware();
 // TODO: add runtime option or filter - logger affects firmware flash performance
@@ -29,7 +28,7 @@ const i18n = new I18nManager({
 
 const store = createStore(
     rootReducer,
-    applyMiddleware(sagaMiddleware, serviceMiddleware, loggerMiddleware),
+    applyMiddleware(sagaMiddleware, loggerMiddleware),
 );
 
 sagaMiddleware.run(rootSaga);

src/sagas/ble-uart.ts

@@ -120,6 +120,8 @@ function* connect(_action: BleDeviceConnectAction): Generator {
         server.disconnect();
         yield takeMaybe(disconnectChannel);
         if (err instanceof DOMException && err.code === DOMException.NOT_FOUND_ERR) {
+            // Possibly/probably caused by Chrome BlueZ back-end bug
+            // https://chromium-review.googlesource.com/c/chromium/src/+/2214098
             yield put(didFailToConnect({ reason: Reason.NoService }));
         } else {
             yield put(didFailToConnect({ reason: Reason.Unknown, err }));

src/sagas/flash-firmware.ts

@@ -12,18 +12,24 @@ import {
     progress,
 } from '../actions/flash-firmware';
 import {
+    BootloaderChecksumRequestAction,
     BootloaderChecksumResponseAction,
     BootloaderConnectionActionType,
     BootloaderConnectionDidConnectAction,
     BootloaderConnectionDidFailToConnectAction,
     BootloaderDidRequestAction,
     BootloaderDidRequestType,
+    BootloaderDisconnectRequestAction,
+    BootloaderEraseRequestAction,
     BootloaderEraseResponseAction,
     BootloaderErrorResponseAction,
+    BootloaderInfoRequestAction,
     BootloaderInfoResponseAction,
+    BootloaderInitRequestAction,
     BootloaderInitResponseAction,
     BootloaderProgramRequestAction,
     BootloaderProgramResponseAction,
+    BootloaderRebootRequestAction,
     BootloaderResponseAction,
     BootloaderResponseActionType,
     checksumRequest,
@@ -59,13 +65,25 @@ type WaitResponse<T extends BootloaderResponseAction> = [
     boolean,
 ];
 
+function* waitForDidSend(id: number): Generator {
+    const didRequest = (yield take(
+        (a: Action) =>
+            a.type === BootloaderDidRequestType &&
+            (a as BootloaderDidRequestAction).id === id,
+    )) as BootloaderDidRequestAction;
+    if (didRequest.err) {
+        console.error(didRequest.err);
+    }
+    return didRequest;
+}
+
 /**
  * Waits for a response action, an error response or timeout, whichever comes
  * first.
  * @param type The action type to wait for.
  * @param timeout The timeout in milliseconds.
  */
-function wait(type: BootloaderResponseActionType, timeout = 500): Effect {
+function waitForResponse(type: BootloaderResponseActionType, timeout = 500): Effect {
     return race([take(type), take(BootloaderResponseActionType.Error), delay(timeout)]);
 }
 
@@ -175,10 +193,11 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
         return;
     }
 
-    yield put(infoRequest());
-    const info = (yield wait(BootloaderResponseActionType.Info)) as WaitResponse<
-        BootloaderInfoResponseAction
-    >;
+    const infoAction = (yield put(infoRequest())) as BootloaderInfoRequestAction;
+    yield waitForDidSend(infoAction.id);
+    const info = (yield waitForResponse(
+        BootloaderResponseActionType.Info,
+    )) as WaitResponse<BootloaderInfoResponseAction>;
     if (!info[0]) {
         throw Error(`failed to get info: ${info}`);
     }
@@ -203,7 +222,10 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
         const response = (yield call(() => fetch(firmwarePath))) as Response;
         if (!response.ok) {
             yield put(notification.add('error', 'Failed to fetch firmware.'));
-            yield put(disconnectRequest());
+            const disconnectAction = (yield put(
+                disconnectRequest(),
+            )) as BootloaderDisconnectRequestAction;
+            yield waitForDidSend(disconnectAction.id);
             return;
         }
 
@@ -225,12 +247,16 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
                 'City Hub is not compatible with this web browser.',
             ),
         );
-        yield put(disconnectRequest());
+        const disconnectAction = (yield put(
+            disconnectRequest(),
+        )) as BootloaderDisconnectRequestAction;
+        yield waitForDidSend(disconnectAction.id);
         return;
     }
 
-    yield put(eraseRequest());
-    const erase = (yield wait(
+    const eraseAction = (yield put(eraseRequest())) as BootloaderEraseRequestAction;
+    yield waitForDidSend(eraseAction.id);
+    const erase = (yield waitForResponse(
         BootloaderResponseActionType.Erase,
         5000,
     )) as WaitResponse<BootloaderEraseResponseAction>;
@@ -239,10 +265,13 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
         throw Error(`Failed to erase: ${erase}`);
     }
 
-    yield put(initRequest(firmware.length));
-    const init = (yield wait(BootloaderResponseActionType.Init)) as WaitResponse<
-        BootloaderInitResponseAction
-    >;
+    const initAction = (yield put(
+        initRequest(firmware.length),
+    )) as BootloaderInitRequestAction;
+    yield waitForDidSend(initAction.id);
+    const init = (yield waitForResponse(
+        BootloaderResponseActionType.Init,
+    )) as WaitResponse<BootloaderInitResponseAction>;
     if (!init[0] || init[0].result) {
         // TODO: proper error handling
         throw Error(`Failed to init: ${init}`);
@@ -252,25 +281,23 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
 
     for (let offset = 0; offset < firmware.length; offset += MaxProgramFlashSize) {
         const payload = firmware.slice(offset, offset + MaxProgramFlashSize);
-        const req = (yield put(
+        const programAction = (yield put(
             programRequest(info[0].startAddress + offset, payload.buffer),
         )) as BootloaderProgramRequestAction;
-
-        // TODO: check for error
-        yield take(
-            (a: Action) =>
-                a.type === BootloaderDidRequestType &&
-                (a as BootloaderDidRequestAction).id === req.id,
-        );
+        yield waitForDidSend(programAction.id);
 
         yield put(progress(offset, firmware.length));
 
         if (connectResult.canWriteWithoutResponse) {
-            // request checksum every 8K to prevent buffer overrun on the hub
-            // because of sending too much data at once
-            if (++count % 585 === 0) {
-                yield put(checksumRequest());
-                const checksum = (yield wait(
+            // request checksum every 25 packets to prevent buffer overrun on
+            // the hub because of sending too much data at once
+            if (++count % 25 === 0) {
+                const checksumAction = (yield put(
+                    checksumRequest(),
+                )) as BootloaderChecksumRequestAction;
+                yield waitForDidSend(checksumAction.id);
+
+                const checksum = (yield waitForResponse(
                     BootloaderResponseActionType.Checksum,
                     5000,
                 )) as WaitResponse<BootloaderChecksumResponseAction>;
@@ -282,7 +309,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
         }
     }
 
-    const flash = (yield wait(
+    const flash = (yield waitForResponse(
         BootloaderResponseActionType.Program,
         5000,
     )) as WaitResponse<BootloaderProgramResponseAction>;
@@ -297,7 +324,8 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator {
     yield put(progress(firmware.length, firmware.length));
 
     // this will cause the remote device to disconnect and reboot
-    yield put(rebootRequest());
+    const rebootAction = (yield put(rebootRequest())) as BootloaderRebootRequestAction;
+    yield waitForDidSend(rebootAction.id);
 }
 
 export default function* (): Generator {

src/sagas/index.ts

@@ -9,7 +9,8 @@ import editor from './editor';
 import errorLog from './error-log';
 import flashFirmware from './flash-firmware';
 import hub from './hub';
-import bootloader from './lwp3-bootloader';
+import lwp3BootloaderBle from './lwp3-bootloader-ble';
+import lwp3BootloaderProtocol from './lwp3-bootloader-protocol';
 import mpy from './mpy';
 import terminal from './terminal';
 
@@ -18,7 +19,8 @@ export default function* (): Generator {
     yield all([
         app(),
         bleUart(),
-        bootloader(),
+        lwp3BootloaderBle(),
+        lwp3BootloaderProtocol(),
         editor(),
         errorLog(),
         flashFirmware(),

src/sagas/lwp3-bootloader-ble.ts

@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2020 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 {
+    BootloaderConnectionAction,
+    BootloaderConnectionActionType,
+    BootloaderConnectionSendAction,
+    BootloaderConnectionFailureReason as Reason,
+    didConnect,
+    didDisconnect,
+    didFailToConnect,
+    didReceive,
+    didSend,
+} from '../actions/lwp3-bootloader';
+import { CharacteristicUUID, ServiceUUID } from '../protocols/lwp3-bootloader';
+import {
+    PolyfillBluetoothRemoteGATTCharacteristic,
+    polyfillBluetoothRemoteGATTCharacteristic,
+} from '../utils/web-bluetooth';
+
+function* handleNotify(data: DataView): Generator {
+    yield put(didReceive(data));
+}
+
+function* write(
+    characteristic: PolyfillBluetoothRemoteGATTCharacteristic,
+    action: BootloaderConnectionSendAction,
+): Generator {
+    try {
+        if (action.withResponse) {
+            yield call(() => characteristic.xWriteValueWithResponse(action.data));
+        } else {
+            yield call(() => characteristic.xWriteValueWithoutResponse(action.data));
+        }
+        yield put(didSend());
+    } catch (err) {
+        yield put(didSend(err));
+    }
+}
+
+function* connect(_action: BootloaderConnectionAction): Generator {
+    if (navigator.bluetooth === undefined) {
+        yield put(didFailToConnect(Reason.NoWebBluetooth));
+        return;
+    }
+
+    // TODO: check navigator.bluetooth.getAvailability()
+
+    let device: BluetoothDevice;
+    try {
+        device = (yield call(() =>
+            navigator.bluetooth.requestDevice({
+                filters: [{ services: [ServiceUUID] }],
+                optionalServices: [ServiceUUID],
+            }),
+        )) as BluetoothDevice;
+    } catch (err) {
+        if (err instanceof DOMException && err.code === DOMException.NOT_FOUND_ERR) {
+            // this can happen if the use cancels the dialog
+            yield put(didFailToConnect(Reason.Canceled));
+        } else {
+            yield put(didFailToConnect(Reason.Unknown, err));
+        }
+        return;
+    }
+
+    if (device.gatt === undefined) {
+        yield put(
+            didFailToConnect(Reason.Unknown, new Error('Device does not support GATT')),
+        );
+        return;
+    }
+
+    const disconnectChannel = eventChannel((emitter) => {
+        const listener = (): void => emitter(END);
+        device.addEventListener('gattserverdisconnected', listener);
+        return (): void =>
+            device.removeEventListener('gattserverdisconnected', listener);
+    });
+
+    let server: BluetoothRemoteGATTServer;
+    try {
+        server = (yield call([device.gatt, 'connect'])) as BluetoothRemoteGATTServer;
+    } catch (err) {
+        disconnectChannel.close();
+        yield put(didFailToConnect(Reason.Unknown, err));
+        return;
+    }
+
+    let service: BluetoothRemoteGATTService;
+    try {
+        service = (yield call(
+            [server, 'getPrimaryService'],
+            ServiceUUID,
+        )) as BluetoothRemoteGATTService;
+    } catch (err) {
+        server.disconnect();
+        yield takeMaybe(disconnectChannel);
+        if (err instanceof DOMException && err.code === DOMException.NOT_FOUND_ERR) {
+            // Possibly/probably caused by Chrome BlueZ back-end bug
+            // https://chromium-review.googlesource.com/c/chromium/src/+/2214098
+            yield put(didFailToConnect(Reason.GattServiceNotFound));
+        } else {
+            yield put(didFailToConnect(Reason.Unknown, err));
+        }
+        return;
+    }
+
+    let characteristic: PolyfillBluetoothRemoteGATTCharacteristic;
+    try {
+        characteristic = polyfillBluetoothRemoteGATTCharacteristic(
+            (yield call(
+                [service, 'getCharacteristic'],
+                CharacteristicUUID,
+            )) as BluetoothRemoteGATTCharacteristic,
+        );
+    } catch (err) {
+        server.disconnect();
+        yield takeMaybe(disconnectChannel);
+        yield put(didFailToConnect(Reason.Unknown, err));
+        return;
+    }
+
+    const notificationChannel = eventChannel<DataView>((emitter) => {
+        const listener = (): void => {
+            if (!characteristic.value) {
+                return;
+            }
+            emitter(characteristic.value);
+        };
+        characteristic.addEventListener('characteristicvaluechanged', listener);
+        return (): void =>
+            characteristic.removeEventListener('characteristicvaluechanged', listener);
+    });
+
+    try {
+        yield call([characteristic, 'startNotifications']);
+    } catch (err) {
+        notificationChannel.close();
+        server.disconnect();
+        yield takeMaybe(disconnectChannel);
+        yield put(didFailToConnect(Reason.Unknown, err));
+        return;
+    }
+
+    yield takeEvery(notificationChannel, handleNotify);
+    yield takeEvery(BootloaderConnectionActionType.Send, write, characteristic);
+
+    // writeValueWithoutResponse() was introduced in Chrome 85.
+    // Older versions of Chrome for Android will write without response
+    // by default when using the deprecated writeValue().
+    const canWriteWithoutResponse =
+        characteristic.writeValueWithoutResponse !== undefined ||
+        /Android/i.test(navigator.userAgent);
+    yield put(didConnect(canWriteWithoutResponse));
+
+    yield takeMaybe(disconnectChannel);
+    notificationChannel.close();
+    try {
+        yield cancel(); // have to cancel to stop forked effects
+    } finally {
+        yield put(didDisconnect());
+    }
+}
+
+export default function* (): Generator {
+    yield takeEvery(BootloaderConnectionActionType.Connect, connect);
+}

src/sagas/lwp3-bootloader-protocol.test.ts

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) 2020 The Pybricks Authors
+// File: sagas/lwp3-bootloader-protocol.test.ts
 
 import { AsyncSaga } from '../../test';
 import {
@@ -33,7 +34,7 @@ import {
     Result,
 } from '../protocols/lwp3-bootloader';
 import { createCountFunc } from '../utils/iter';
-import bootloader from './lwp3-bootloader';
+import bootloader from './lwp3-bootloader-protocol';
 
 describe('message encoder', () => {
     test.each([

src/sagas/lwp3-bootloader-protocol.ts

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) 2020 The Pybricks Authors
+// File: sagas/lwp3-bootloader-protocol.ts
+// Handles LEGO Wireless Protocol v3 Bootloader protocol.
 
 import { Channel } from 'redux-saga';
 import { actionChannel, fork, put, take, takeEvery } from 'redux-saga/effects';

src/services/index.ts

@@ -1,45 +0,0 @@
-// SPDX-License-Identifier: MIT
-// Copyright (c) 2020 The Pybricks Authors
-
-import { Middleware } from 'redux';
-import { Action, Dispatch } from '../actions';
-import { RootState } from '../reducers';
-import bootloader from './lwp3-bootloader';
-
-type Service = (
-    action: Action,
-    dispatch: Dispatch,
-    state: RootState,
-) => void | Promise<void>;
-
-function runService(
-    service: Service,
-    action: Action,
-    dispatch: Dispatch,
-    state: RootState,
-): void {
-    // Services are deferred so that the current action completes before
-    // dispatching another action by calling dispatch() in the service.
-    setTimeout(async () => {
-        try {
-            await service(action, dispatch, state);
-        } catch (err) {
-            console.log(`Unhandled exception in service: ${err}`);
-        }
-    }, 0);
-}
-
-export function combineServices(...services: Service[]): Service {
-    return (a, d, s): void => {
-        services.forEach((x) => runService(x, a, d, s));
-    };
-}
-
-const rootService = combineServices(bootloader);
-
-const serviceMiddleware: Middleware = (store) => (next) => (action): unknown => {
-    runService(rootService, action, store.dispatch, store.getState());
-    return next(action);
-};
-
-export default serviceMiddleware;

src/services/lwp3-bootloader.ts

@@ -1,122 +0,0 @@
-// SPDX-License-Identifier: MIT
-// Copyright (c) 2020 The Pybricks Authors
-
-import { Action, Dispatch } from '../actions';
-
-import {
-    BootloaderConnectionActionType,
-    BootloaderConnectionFailureReason as Reason,
-    didConnect,
-    didDisconnect,
-    didFailToConnect,
-    didReceive,
-    didSend,
-} from '../actions/lwp3-bootloader';
-import { CharacteristicUUID, ServiceUUID } from '../protocols/lwp3-bootloader';
-import {
-    PolyfillBluetoothRemoteGATTCharacteristic,
-    polyfillBluetoothRemoteGATTCharacteristic,
-} from '../utils/web-bluetooth';
-import { combineServices } from '.';
-
-let device: BluetoothDevice | undefined;
-let char: PolyfillBluetoothRemoteGATTCharacteristic | undefined;
-
-async function connect(action: Action, dispatch: Dispatch): Promise<void> {
-    if (action.type !== BootloaderConnectionActionType.Connect) {
-        return;
-    }
-
-    try {
-        if (device) {
-            throw Error('already connected');
-        }
-        if (navigator.bluetooth === undefined) {
-            dispatch(didFailToConnect(Reason.NoWebBluetooth));
-            return;
-        }
-        // TODO: check navigator.bluetooth.getAvailability()
-        try {
-            device = await navigator.bluetooth.requestDevice({
-                filters: [{ services: [ServiceUUID] }],
-                optionalServices: [ServiceUUID],
-            });
-        } catch (err) {
-            if (
-                err instanceof DOMException &&
-                err.code === DOMException.NOT_FOUND_ERR
-            ) {
-                // this error is received if the user clicks the cancel button in
-                // the bluetooth scan dialog
-                dispatch(didFailToConnect(Reason.Canceled));
-                return;
-            }
-            throw err;
-        }
-        if (device.gatt === undefined) {
-            throw Error('Device does not support GATT');
-        }
-        device.addEventListener('gattserverdisconnected', () => {
-            device = undefined;
-            char = undefined;
-            dispatch(didDisconnect());
-        });
-        const server = await device.gatt.connect();
-        try {
-            const service = await server.getPrimaryService(ServiceUUID);
-            char = polyfillBluetoothRemoteGATTCharacteristic(
-                await service.getCharacteristic(CharacteristicUUID),
-            );
-            char.addEventListener('characteristicvaluechanged', () => {
-                if (!char || !char.value) {
-                    return;
-                }
-                dispatch(didReceive(char.value));
-            });
-            await char.startNotifications();
-        } catch (err) {
-            device.gatt.disconnect();
-            if (
-                err instanceof DOMException &&
-                err.code === DOMException.NOT_FOUND_ERR
-            ) {
-                // Possibly/probably caused by Chrome BlueZ back-end bug
-                // https://chromium-review.googlesource.com/c/chromium/src/+/2214098
-                dispatch(didFailToConnect(Reason.GattServiceNotFound));
-                return;
-            }
-            throw err;
-        }
-
-        // char.writeValueWithoutResponse() was introduced in Chrome 85.
-        // Older versions of Chrome for Android will write without response
-        // by default when using the deprecated writeValue().
-        const canWriteWithoutResponse =
-            char.writeValueWithoutResponse !== undefined ||
-            /Android/i.test(navigator.userAgent);
-        dispatch(didConnect(canWriteWithoutResponse));
-    } catch (err) {
-        dispatch(didFailToConnect(Reason.Unknown, err));
-    }
-}
-
-async function send(action: Action, dispatch: Dispatch): Promise<void> {
-    if (action.type !== BootloaderConnectionActionType.Send) {
-        return;
-    }
-    try {
-        if (!char) {
-            throw Error('Not connected');
-        }
-        if (action.withResponse) {
-            await char.xWriteValueWithResponse(action.data);
-        } else {
-            await char.xWriteValueWithoutResponse(action.data);
-        }
-        dispatch(didSend());
-    } catch (err) {
-        dispatch(didSend(err));
-    }
-}
-
-export default combineServices(connect, send);