From 23566a7d4ae891d6921462a2f1531ac4b75f5429 Mon Sep 17 00:00:00 2001
From: David Lechner <david@pybricks.com>
Date: Wed, 20 May 2020 21:26:13 -0500
Subject: [PATCH] Check Web Bluetooth capabilities

This adds/improves checks for the various Web Bluetooth capabilities
that we are using and notifies the user when capabilities are missing.
---
 src/actions/ble.ts                   | 10 +++++----
 src/actions/bootloader.ts            | 13 ++++++-----
 src/actions/notification.ts          | 22 +++++++++++++++++--
 src/components/Notification.tsx      | 16 +++++++++++++-
 src/components/NotificationStack.tsx |  1 +
 src/reducers/notification.ts         |  2 ++
 src/sagas/bootloader.ts              |  6 ++++-
 src/services/bootloader.ts           | 33 ++++++++++++++++++++++++----
 src/utils/web-bluetooth.ts           | 24 +++++++++++++-------
 9 files changed, 102 insertions(+), 25 deletions(-)

diff --git a/src/actions/ble.ts b/src/actions/ble.ts
index 40151489..194b536e 100644
--- a/src/actions/ble.ts
+++ b/src/actions/ble.ts
@@ -81,11 +81,13 @@ export function connect(): BLEThunkAction {
             dispatch(
                 notification.add(
                     'error',
-                    'Browser does not support WebBluetooth or it is not enabled',
+                    'This web browser does not support Web Bluetooth or it is not enabled.',
+                    'https://github.com/WebBluetoothCG/web-bluetooth/blob/master/implementation-status.md',
                 ),
             );
             return;
         }
+        // TODO: check navigator.bluetooth.getAvailability()
         dispatch(beginConnect());
         try {
             device = await navigator.bluetooth.requestDevice({
@@ -112,7 +114,7 @@ export function connect(): BLEThunkAction {
             return;
         }
         if (device.gatt === undefined) {
-            dispatch(notification.add('error', 'Device does not support GATT'));
+            dispatch(notification.add('error', 'Device does not support GATT.'));
             dispatch(endDisconnect());
             return;
         }
@@ -137,7 +139,7 @@ export function connect(): BLEThunkAction {
             await txChar.startNotifications();
         } catch (err) {
             console.error(err);
-            dispatch(notification.add('error', 'Getting nRF UART service failed'));
+            dispatch(notification.add('error', 'Getting nRF UART service failed.'));
             device.gatt.disconnect();
             return;
         }
@@ -156,7 +158,7 @@ export function write(value: ArrayBuffer): BLEThunkAction {
     return async function (): Promise<void> {
         // TODO: do we need to dispatch any Action<>s here?
         for (let i = 0; i < value.byteLength; i += bleNusMaxSize) {
-            await rxChar?.writeValueWithoutResponse(value.slice(i, i + bleNusMaxSize));
+            await rxChar?.xWriteValueWithoutResponse(value.slice(i, i + bleNusMaxSize));
         }
     };
 }
diff --git a/src/actions/bootloader.ts b/src/actions/bootloader.ts
index 8a061065..463136fb 100644
--- a/src/actions/bootloader.ts
+++ b/src/actions/bootloader.ts
@@ -47,12 +47,15 @@ export function connect(): BootloaderConnectionConnectAction {
     return { type: BootloaderConnectionActionType.Connect };
 }
 
-export type BootloaderConnectionDidConnectAction = Action<
-    BootloaderConnectionActionType.DidConnect
->;
+export interface BootloaderConnectionDidConnectAction
+    extends Action<BootloaderConnectionActionType.DidConnect> {
+    canWriteWithoutResponse: boolean;
+}
 
-export function didConnect(): BootloaderConnectionDidConnectAction {
-    return { type: BootloaderConnectionActionType.DidConnect };
+export function didConnect(
+    canWriteWithoutResponse: boolean,
+): BootloaderConnectionDidConnectAction {
+    return { type: BootloaderConnectionActionType.DidConnect, canWriteWithoutResponse };
 }
 
 export type BootloaderConnectionDidCancelAction = Action<
diff --git a/src/actions/notification.ts b/src/actions/notification.ts
index ec113bf9..e581acc5 100644
--- a/src/actions/notification.ts
+++ b/src/actions/notification.ts
@@ -26,6 +26,10 @@ export interface NotificationAddAction extends Action<NotificationActionType.Add
      * The message to be displayed to the user.
      */
     readonly message: string;
+    /**
+     * URL for help or more information.
+     */
+    readonly helpUrl?: string;
 }
 
 export interface NotificationRemoveAction
@@ -40,10 +44,24 @@ export type NotificationAction = NotificationAddAction | NotificationRemoveActio
 
 let nextId = 0;
 
-export function add(level: NotificationLevel, message: string): NotificationAddAction {
-    return { type: NotificationActionType.Add, id: nextId++, level, message };
+/**
+ * Action to add a notification to the list.
+ * @param level The severity level
+ * @param message The message to display to the user
+ * @param helpUrl An optional URL for more info
+ */
+export function add(
+    level: NotificationLevel,
+    message: string,
+    helpUrl?: string,
+): NotificationAddAction {
+    return { type: NotificationActionType.Add, id: nextId++, level, message, helpUrl };
 }
 
+/**
+ * Action to removes a notification from the list.
+ * @param id The id of the notification to remove
+ */
 export function remove(id: number): NotificationRemoveAction {
     return { type: NotificationActionType.Remove, id };
 }
diff --git a/src/components/Notification.tsx b/src/components/Notification.tsx
index 40b7707d..512911aa 100644
--- a/src/components/Notification.tsx
+++ b/src/components/Notification.tsx
@@ -15,6 +15,7 @@ interface OwnProps {
     id: number;
     style: string;
     message: string;
+    helpUrl?: string;
 }
 
 type NotificationProps = DispatchProps & OwnProps;
@@ -45,7 +46,20 @@ class Notification extends React.Component<NotificationProps> {
                         {title}
                     </strong>
                 </Toast.Header>
-                <Toast.Body>{this.props.message}</Toast.Body>
+                <Toast.Body>
+                    <p>{this.props.message}</p>
+                    <p>
+                        {this.props.helpUrl && (
+                            <a
+                                href={this.props.helpUrl}
+                                target="_blank"
+                                rel="noopener noreferrer"
+                            >
+                                More info
+                            </a>
+                        )}
+                    </p>
+                </Toast.Body>
             </Toast>
         );
     }
diff --git a/src/components/NotificationStack.tsx b/src/components/NotificationStack.tsx
index 6c19d9fe..ae3d5326 100644
--- a/src/components/NotificationStack.tsx
+++ b/src/components/NotificationStack.tsx
@@ -32,6 +32,7 @@ class NotificationStack extends React.Component<NotificationStackProps> {
                                     id={n.id}
                                     style={n.style}
                                     message={n.message}
+                                    helpUrl={n.helpUrl}
                                 />
                             </Collapse>
                         ))}
diff --git a/src/reducers/notification.ts b/src/reducers/notification.ts
index 614d17ae..e88095d3 100644
--- a/src/reducers/notification.ts
+++ b/src/reducers/notification.ts
@@ -6,6 +6,7 @@ export type NotificationList = Array<{
     readonly id: number;
     readonly style: string;
     readonly message: string;
+    readonly helpUrl?: string;
 }>;
 
 const levelMap = {
@@ -23,6 +24,7 @@ const list: Reducer<NotificationList, NotificationAction> = (state = [], action)
                     id: action.id,
                     style: levelMap[action.level],
                     message: action.message,
+                    helpUrl: action.helpUrl,
                 },
             ];
         case NotificationActionType.Remove:
diff --git a/src/sagas/bootloader.ts b/src/sagas/bootloader.ts
index e11d76ce..840aa8d2 100644
--- a/src/sagas/bootloader.ts
+++ b/src/sagas/bootloader.ts
@@ -306,15 +306,19 @@ function* flashFirmware(action: BootloaderFlashFirmwareAction): Generator {
         const payload = firmware.slice(offset, offset + MaxProgramFlashSize);
         yield put(programRequest(info[0].startAddress + offset, payload.buffer));
 
+        // TODO: wait for request to actually be sent before reporting progress
         yield put(progress(offset, firmware.length));
 
+        // TODO: we can skip getting the checksum when canWriteWithoutResponse === false
+        // when the todo above is done.
+
         // 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(
                 BootloaderResponseActionType.Checksum,
-                5000,
+                didConnect.canWriteWithoutResponse ? 5000 : 60000,
             )) as WaitResponse<BootloaderChecksumResponseAction>;
             if (!checksum[0]) {
                 // TODO: proper error handling
diff --git a/src/services/bootloader.ts b/src/services/bootloader.ts
index 4d6c760b..6585a38e 100644
--- a/src/services/bootloader.ts
+++ b/src/services/bootloader.ts
@@ -9,6 +9,7 @@ import {
     didReceive,
     didSend,
 } from '../actions/bootloader';
+import * as notification from '../actions/notification';
 import { CharacteristicUUID, ServiceUUID } from '../protocols/bootloader';
 import {
     PolyfillBluetoothRemoteGATTCharacteristic,
@@ -29,8 +30,16 @@ async function connect(action: Action, dispatch: Dispatch): Promise<void> {
             throw Error('already connected');
         }
         if (navigator.bluetooth === undefined) {
-            throw Error('No web bluetooth');
+            dispatch(
+                notification.add(
+                    'error',
+                    'This web browser does not support Web Bluetooth or it is not enabled.',
+                    'https://github.com/WebBluetoothCG/web-bluetooth/blob/master/implementation-status.md',
+                ),
+            );
+            return;
         }
+        // TODO: check navigator.bluetooth.getAvailability()
         try {
             device = await navigator.bluetooth.requestDevice({
                 filters: [{ services: [ServiceUUID] }],
@@ -69,11 +78,27 @@ async function connect(action: Action, dispatch: Dispatch): Promise<void> {
                 dispatch(didReceive(char.value));
             });
             await char.startNotifications();
+
+            // char.writeValueWithoutResponse() was introduced in Chrome 85
+            // Older versions of Chrome for Android will write without response
+            // by default, so don't warn on Android.
+            if (
+                !char.writeValueWithoutResponse &&
+                !/Android/i.test(navigator.userAgent)
+            ) {
+                dispatch(
+                    notification.add(
+                        'warning',
+                        'This web browser does not support Web Bluetooth Write Characteristic Without Response. Flashing firmware will take a long time.',
+                        'https://github.com/WebBluetoothCG/web-bluetooth/blob/master/implementation-status.md',
+                    ),
+                );
+            }
         } catch (err) {
             device.gatt.disconnect();
             throw err;
         }
-        dispatch(didConnect());
+        dispatch(didConnect(char.writeValueWithoutResponse !== undefined));
     } catch (err) {
         dispatch(didError(err));
     }
@@ -89,9 +114,9 @@ async function send(action: Action, dispatch: Dispatch): Promise<void> {
         }
         const sendAction = action as BootloaderConnectionSendAction;
         if (sendAction.withResponse) {
-            await char.writeValueWithResponse(sendAction.data);
+            await char.xWriteValueWithResponse(sendAction.data);
         } else {
-            await char.writeValueWithoutResponse(sendAction.data);
+            await char.xWriteValueWithoutResponse(sendAction.data);
         }
         dispatch(didSend());
     } catch (err) {
diff --git a/src/utils/web-bluetooth.ts b/src/utils/web-bluetooth.ts
index 66f5191c..d03ecaa5 100644
--- a/src/utils/web-bluetooth.ts
+++ b/src/utils/web-bluetooth.ts
@@ -4,8 +4,18 @@
  */
 export interface PolyfillBluetoothRemoteGATTCharacteristic
     extends BluetoothRemoteGATTCharacteristic {
-    writeValueWithResponse(value: BufferSource): Promise<void>;
-    writeValueWithoutResponse(value: BufferSource): Promise<void>;
+    writeValueWithResponse?(value: BufferSource): Promise<void>;
+    writeValueWithoutResponse?(value: BufferSource): Promise<void>;
+    /**
+     * Calls writeValueWithResponse() if available otherwise falls back to writeValue()
+     * @param value data to send
+     */
+    xWriteValueWithResponse(value: BufferSource): Promise<void>;
+    /**
+     * Calls writeValueWithoutResponse() if available otherwise falls back to writeValue()
+     * @param value data to send
+     */
+    xWriteValueWithoutResponse(value: BufferSource): Promise<void>;
 }
 
 /**
@@ -17,11 +27,9 @@ export function polyfillBluetoothRemoteGATTCharacteristic(
     char: BluetoothRemoteGATTCharacteristic,
 ): PolyfillBluetoothRemoteGATTCharacteristic {
     const polyfill = (char as unknown) as PolyfillBluetoothRemoteGATTCharacteristic;
-    if (!polyfill.writeValueWithResponse) {
-        polyfill.writeValueWithResponse = char.writeValue;
-    }
-    if (!polyfill.writeValueWithoutResponse) {
-        polyfill.writeValueWithoutResponse = char.writeValue;
-    }
+    polyfill.xWriteValueWithResponse =
+        polyfill.writeValueWithResponse || char.writeValue;
+    polyfill.xWriteValueWithoutResponse =
+        polyfill.writeValueWithoutResponse || char.writeValue;
     return polyfill;
 }