From 712117c8d89094794e0a719c38297f562af5b662 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 12 Jun 2020 21:29:13 -0500 Subject: [PATCH] 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 | 3 +- src/sagas/ble-uart.ts | 2 + src/sagas/flash-firmware.ts | 84 ++++++--- src/sagas/index.ts | 6 +- src/sagas/lwp3-bootloader-ble.ts | 172 ++++++++++++++++++ ...st.ts => lwp3-bootloader-protocol.test.ts} | 3 +- ...tloader.ts => lwp3-bootloader-protocol.ts} | 2 + src/services/index.ts | 45 ----- src/services/lwp3-bootloader.ts | 122 ------------- 9 files changed, 239 insertions(+), 200 deletions(-) create mode 100644 src/sagas/lwp3-bootloader-ble.ts rename src/sagas/{lwp3-bootloader.test.ts => lwp3-bootloader-protocol.test.ts} (98%) rename src/sagas/{lwp3-bootloader.ts => lwp3-bootloader-protocol.ts} (98%) delete mode 100644 src/services/index.ts delete mode 100644 src/services/lwp3-bootloader.ts diff --git a/src/index.tsx b/src/index.tsx index e8a7bf3e..7e05631b 100644 --- a/src/index.tsx +++ b/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); diff --git a/src/sagas/ble-uart.ts b/src/sagas/ble-uart.ts index cecf8473..ca5919a9 100644 --- a/src/sagas/ble-uart.ts +++ b/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 })); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index eb164168..bbabb78b 100644 --- a/src/sagas/flash-firmware.ts +++ b/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 = [ 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; 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; @@ -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; 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; @@ -282,7 +309,7 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } } - const flash = (yield wait( + const flash = (yield waitForResponse( BootloaderResponseActionType.Program, 5000, )) as WaitResponse; @@ -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 { diff --git a/src/sagas/index.ts b/src/sagas/index.ts index 5295e630..d7594ae1 100644 --- a/src/sagas/index.ts +++ b/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(), diff --git a/src/sagas/lwp3-bootloader-ble.ts b/src/sagas/lwp3-bootloader-ble.ts new file mode 100644 index 00000000..7f2a28ea --- /dev/null +++ b/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((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); +} diff --git a/src/sagas/lwp3-bootloader.test.ts b/src/sagas/lwp3-bootloader-protocol.test.ts similarity index 98% rename from src/sagas/lwp3-bootloader.test.ts rename to src/sagas/lwp3-bootloader-protocol.test.ts index 6e7b7d59..b79d4839 100644 --- a/src/sagas/lwp3-bootloader.test.ts +++ b/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([ diff --git a/src/sagas/lwp3-bootloader.ts b/src/sagas/lwp3-bootloader-protocol.ts similarity index 98% rename from src/sagas/lwp3-bootloader.ts rename to src/sagas/lwp3-bootloader-protocol.ts index 35c4657b..8cc4cc33 100644 --- a/src/sagas/lwp3-bootloader.ts +++ b/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'; diff --git a/src/services/index.ts b/src/services/index.ts deleted file mode 100644 index fd227409..00000000 --- a/src/services/index.ts +++ /dev/null @@ -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; - -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; diff --git a/src/services/lwp3-bootloader.ts b/src/services/lwp3-bootloader.ts deleted file mode 100644 index 32ad65d7..00000000 --- a/src/services/lwp3-bootloader.ts +++ /dev/null @@ -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 { - 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 { - 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);