From ae996453e82bfabc20075453a061d16b692f12e9 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 21 Dec 2020 13:05:47 -0600 Subject: [PATCH 1/2] drop polyfill for chromimum < v85 This removes the polyfill for using older chrome browsers before the writeWithResponse and writeWithoutResponse APIs were added. --- src/actions/lwp3-bootloader.ts | 10 ++--- src/components/notification-i18n.en.json | 1 - src/components/notification-i18n.ts | 1 - src/reducers/notification.ts | 11 ----- src/sagas/ble-uart.ts | 20 ++++----- src/sagas/flash-firmware.ts | 53 ++++++++---------------- src/sagas/lwp3-bootloader-ble.ts | 30 ++++---------- src/utils/web-bluetooth.test.ts | 29 ------------- src/utils/web-bluetooth.ts | 35 ---------------- 9 files changed, 37 insertions(+), 153 deletions(-) delete mode 100644 src/utils/web-bluetooth.test.ts delete mode 100644 src/utils/web-bluetooth.ts diff --git a/src/actions/lwp3-bootloader.ts b/src/actions/lwp3-bootloader.ts index 4a14a3b3..ffe0b890 100644 --- a/src/actions/lwp3-bootloader.ts +++ b/src/actions/lwp3-bootloader.ts @@ -54,14 +54,10 @@ export function connect(): BootloaderConnectionConnectAction { return { type: BootloaderConnectionActionType.Connect }; } -export type BootloaderConnectionDidConnectAction = Action & { - canWriteWithoutResponse: boolean; -}; +export type BootloaderConnectionDidConnectAction = Action; -export function didConnect( - canWriteWithoutResponse: boolean, -): BootloaderConnectionDidConnectAction { - return { type: BootloaderConnectionActionType.DidConnect, canWriteWithoutResponse }; +export function didConnect(): BootloaderConnectionDidConnectAction { + return { type: BootloaderConnectionActionType.DidConnect }; } /** diff --git a/src/components/notification-i18n.en.json b/src/components/notification-i18n.en.json index fe5be52d..197d5a7d 100644 --- a/src/components/notification-i18n.en.json +++ b/src/components/notification-i18n.en.json @@ -1,6 +1,5 @@ { "ble": { - "cannotWriteWithoutResponse": "This web browser does not support Web Bluetooth Write Characteristic Without Response. Flashing firmware will take a long time.", "gattPermission": "The web browser did not give permission to use Bluetooth Low Energy", "gattServiceNotFound": "Connected to hub but failed to get {serviceName} service. Try removing the \"{hubName}\" device in your OS Bluetooth settings, then try again.", "noWebBluetooth": "This web browser does not support Web Bluetooth or it is not enabled.", diff --git a/src/components/notification-i18n.ts b/src/components/notification-i18n.ts index d036f618..9d4fe68c 100644 --- a/src/components/notification-i18n.ts +++ b/src/components/notification-i18n.ts @@ -4,7 +4,6 @@ // Notification translation keys. export enum MessageId { - BleCannotWriteWithoutResponse = 'ble.cannotWriteWithoutResponse', BleConnectFailed = 'ble.connectFailed', BleGattPermission = 'ble.gattPermission', BleGattServiceNotFound = 'ble.gattServiceNotFound', diff --git a/src/reducers/notification.ts b/src/reducers/notification.ts index 1e95df69..d956a4e5 100644 --- a/src/reducers/notification.ts +++ b/src/reducers/notification.ts @@ -93,17 +93,6 @@ const list: Reducer = (state = [], action) => { return append(state, Level.Error, MessageId.BleConnectFailed); } return state; - case BootloaderConnectionActionType.DidConnect: - if (!action.canWriteWithoutResponse) { - return append( - state, - Level.Warning, - MessageId.BleCannotWriteWithoutResponse, - undefined, - 'https://github.com/WebBluetoothCG/web-bluetooth/blob/master/implementation-status.md', - ); - } - return state; case BootloaderConnectionActionType.DidFailToConnect: switch (action.reason) { case BootloaderConnectionFailureReason.GattServiceNotFound: diff --git a/src/sagas/ble-uart.ts b/src/sagas/ble-uart.ts index ca5919a9..75159607 100644 --- a/src/sagas/ble-uart.ts +++ b/src/sagas/ble-uart.ts @@ -33,10 +33,6 @@ import { import { ServiceUUID as pybricksServiceUUID } from '../protocols/pybricks'; import { RootState } from '../reducers'; import { BleConnectionState } from '../reducers/ble'; -import { - PolyfillBluetoothRemoteGATTCharacteristic, - polyfillBluetoothRemoteGATTCharacteristic, -} from '../utils/web-bluetooth'; function disconnect( server: BluetoothRemoteGATTServer, @@ -50,11 +46,11 @@ function* handleValueChanged(data: DataView): Generator { } function* write( - rxChar: PolyfillBluetoothRemoteGATTCharacteristic, + rxChar: BluetoothRemoteGATTCharacteristic, action: BleUartWriteAction, ): Generator { try { - yield call(() => rxChar.xWriteValueWithoutResponse(action.value.buffer)); + yield call(() => rxChar.writeValueWithoutResponse(action.value.buffer)); yield put(didWrite(action.id)); } catch (err) { yield put(didFailToWrite(action.id, err)); @@ -129,14 +125,12 @@ function* connect(_action: BleDeviceConnectAction): Generator { return; } - let rxChar: PolyfillBluetoothRemoteGATTCharacteristic; + let rxChar: BluetoothRemoteGATTCharacteristic; try { - rxChar = polyfillBluetoothRemoteGATTCharacteristic( - (yield call( - [service, 'getCharacteristic'], - urtRxCharUUID, - )) as BluetoothRemoteGATTCharacteristic, - ); + rxChar = (yield call( + [service, 'getCharacteristic'], + urtRxCharUUID, + )) as BluetoothRemoteGATTCharacteristic; } catch (err) { server.disconnect(); yield takeMaybe(disconnectChannel); diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts index 3e101a79..eb7a862b 100644 --- a/src/sagas/flash-firmware.ts +++ b/src/sagas/flash-firmware.ts @@ -236,21 +236,6 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { } } - // City hub bootloader is buggy. See note in encodeRequest(). - if (info[0].hubType === HubType.CityHub && !connectResult.canWriteWithoutResponse) { - yield put( - notification.add( - 'error', - 'City Hub bootloader is not compatible with this web browser.', - ), - ); - const disconnectAction = (yield put( - disconnectRequest(), - )) as BootloaderDisconnectRequestAction; - yield waitForDidSend(disconnectAction.id); - return; - } - const eraseAction = (yield put(eraseRequest())) as BootloaderEraseRequestAction; const [, erase] = (yield all([ waitForDidSend(eraseAction.id), @@ -296,26 +281,24 @@ function* flashFirmware(action: FlashFirmwareFlashAction): Generator { break; } - if (connectResult.canWriteWithoutResponse) { - // 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(), - )) as BootloaderChecksumRequestAction; - const [, checksum] = (yield all([ - waitForDidSend(checksumAction.id), - waitForResponse(BootloaderResponseActionType.Checksum, 5000), - ])) as [ - BootloaderDidRequestAction, - WaitResponse, - ]; - if (!checksum[0]) { - // TODO: proper error handling - throw Error(`Failed to get checksum: ${checksum}`); - } + // 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(), + )) as BootloaderChecksumRequestAction; + const [, checksum] = (yield all([ + waitForDidSend(checksumAction.id), + waitForResponse(BootloaderResponseActionType.Checksum, 5000), + ])) as [ + BootloaderDidRequestAction, + WaitResponse, + ]; + if (!checksum[0]) { + // TODO: proper error handling + throw Error(`Failed to get checksum: ${checksum}`); } } } diff --git a/src/sagas/lwp3-bootloader-ble.ts b/src/sagas/lwp3-bootloader-ble.ts index 7f2a28ea..946b45d3 100644 --- a/src/sagas/lwp3-bootloader-ble.ts +++ b/src/sagas/lwp3-bootloader-ble.ts @@ -17,24 +17,20 @@ import { 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, + characteristic: BluetoothRemoteGATTCharacteristic, action: BootloaderConnectionSendAction, ): Generator { try { if (action.withResponse) { - yield call(() => characteristic.xWriteValueWithResponse(action.data)); + yield call(() => characteristic.writeValueWithResponse(action.data)); } else { - yield call(() => characteristic.xWriteValueWithoutResponse(action.data)); + yield call(() => characteristic.writeValueWithoutResponse(action.data)); } yield put(didSend()); } catch (err) { @@ -110,14 +106,12 @@ function* connect(_action: BootloaderConnectionAction): Generator { return; } - let characteristic: PolyfillBluetoothRemoteGATTCharacteristic; + let characteristic: BluetoothRemoteGATTCharacteristic; try { - characteristic = polyfillBluetoothRemoteGATTCharacteristic( - (yield call( - [service, 'getCharacteristic'], - CharacteristicUUID, - )) as BluetoothRemoteGATTCharacteristic, - ); + characteristic = (yield call( + [service, 'getCharacteristic'], + CharacteristicUUID, + )) as BluetoothRemoteGATTCharacteristic; } catch (err) { server.disconnect(); yield takeMaybe(disconnectChannel); @@ -150,13 +144,7 @@ function* connect(_action: BootloaderConnectionAction): Generator { 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 put(didConnect()); yield takeMaybe(disconnectChannel); notificationChannel.close(); diff --git a/src/utils/web-bluetooth.test.ts b/src/utils/web-bluetooth.test.ts deleted file mode 100644 index 88fa8740..00000000 --- a/src/utils/web-bluetooth.test.ts +++ /dev/null @@ -1,29 +0,0 @@ -// SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors - -import { polyfillBluetoothRemoteGATTCharacteristic } from './web-bluetooth'; - -describe('polyfillBluetoothRemoteGATTCharacteristic', () => { - test('old browser falls back to writeValue', () => { - const char = {} as BluetoothRemoteGATTCharacteristic; - Object.defineProperty(char, 'writeValue', { value: 'writeValue' }); - expect(polyfillBluetoothRemoteGATTCharacteristic(char)).toEqual({ - xWriteValueWithResponse: 'writeValue', - xWriteValueWithoutResponse: 'writeValue', - }); - }); - test('new browser uses writeValueWith(out)Response', () => { - const char = {} as BluetoothRemoteGATTCharacteristic; - Object.defineProperty(char, 'writeValue', { value: 'writeValue' }); - Object.defineProperty(char, 'writeValueWithResponse', { - value: 'writeValueWithResponse', - }); - Object.defineProperty(char, 'writeValueWithoutResponse', { - value: 'writeValueWithoutResponse', - }); - expect(polyfillBluetoothRemoteGATTCharacteristic(char)).toEqual({ - xWriteValueWithResponse: 'writeValueWithResponse', - xWriteValueWithoutResponse: 'writeValueWithoutResponse', - }); - }); -}); diff --git a/src/utils/web-bluetooth.ts b/src/utils/web-bluetooth.ts deleted file mode 100644 index 3196b65f..00000000 --- a/src/utils/web-bluetooth.ts +++ /dev/null @@ -1,35 +0,0 @@ -// SPDX-License-Identifier: MIT -// Copyright (c) 2020 The Pybricks Authors - -/** - * Current definition of BluetoothRemoteGATTCharacteristic doesn't include - * new Web Bluetooth APIs. - */ -export interface PolyfillBluetoothRemoteGATTCharacteristic - extends BluetoothRemoteGATTCharacteristic { - /** - * Calls writeValueWithResponse() if available otherwise falls back to writeValue() - * @param value data to send - */ - xWriteValueWithResponse(value: BufferSource): Promise; - /** - * Calls writeValueWithoutResponse() if available otherwise falls back to writeValue() - * @param value data to send - */ - xWriteValueWithoutResponse(value: BufferSource): Promise; -} - -/** - * Fills in writeValueWithResponse and writeValueWithoutResponse for backward - * compatibility. - * @param char a remote GATT characteristic object - */ -export function polyfillBluetoothRemoteGATTCharacteristic( - char: BluetoothRemoteGATTCharacteristic, -): PolyfillBluetoothRemoteGATTCharacteristic { - const polyfill = (char as unknown) as PolyfillBluetoothRemoteGATTCharacteristic; - polyfill.xWriteValueWithResponse = char.writeValueWithResponse || char.writeValue; - polyfill.xWriteValueWithoutResponse = - char.writeValueWithoutResponse || char.writeValue; - return polyfill; -} From a3d568d4b6a17cba84ef6dbf2cbd1dc4958e8026 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 8 Jan 2021 11:12:31 -0600 Subject: [PATCH 2/2] Pybricks firmware v3.0.0a13 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 75504593..cc9f04d2 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "private": true, "dependencies": { "@blueprintjs/core": "^3.30.1", - "@pybricks/firmware": "4.2.0", + "@pybricks/firmware": "4.3.0", "@pybricks/mpy-cross-v5": "^1.1.0", "@shopify/react-i18n": "^5.0.0", "@testing-library/jest-dom": "^5.8.0", diff --git a/yarn.lock b/yarn.lock index 4234c8fe..c63cf92d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1458,10 +1458,10 @@ schema-utils "^2.6.5" source-map "^0.7.3" -"@pybricks/firmware@4.2.0": - version "4.2.0" - resolved "https://npm.pkg.github.com/download/@pybricks/firmware/4.2.0/7b91542f6fc6461f8654e67a437d6164bf63bea34bd34e7af22c648c670dad0c#666bedf9deb98a23852ea60c4c8410b9fc6c1009" - integrity sha512-71YaYrqoLz2lxOCwnXyX1d2bbs3CXU1eNgRWlpd7+/Lymv5oLK3cTQnW/QhnGcv1pSLhyGg5kKVxX2rOVx+R/A== +"@pybricks/firmware@4.3.0": + version "4.3.0" + resolved "https://npm.pkg.github.com/download/@pybricks/firmware/4.3.0/89f260a050abcc837c803b7851edcb234944d5aff0a2f45706b556796fbc3a45#817b3bd9aa3daf77c80b67a5c5d5776779773491" + integrity sha512-5cTtBpTtRa0P98DS+M2WswYsF4FCQdeL65tMZd98S47sjZn/Yzw6AJzhSFfZHPJbRQzqDcHJm065awQHb7SXMg== dependencies: jszip "^3.5.0"