Bluetooth/Usb automatic reconnection #516

EricLin-BBpos · 2023-08-03T06:22:34Z

Summary

Support Bluetooth/Usb automatic reconnection while a reader loses connection unexpected .

Motivation

Add automatic reconnection support for Bluetooth in iOS and Bluetooth and USB in Android SDK . So that while a reader loses connection unexpected , the sdk will automatically attempts to reconnect to the reader.
This works for M2, WisePad 3 and Chipper.

Testing

I tested this manually
I added automated tests

Documentation

Select one:

I have added relevant documentation for my changes.
This PR does not result in any developer-facing changes.

Update reader status while reconnecting fail.

….com/EricLin-BBpos/stripe-terminal-react-native into Bluetooth-USB-automatic-reconnection

nazli-stripe · 2023-08-11T22:40:50Z

src/types/index.ts

@@ -35,11 +35,13 @@ export type GetLocationsParams = {
 export type ConnectBluetoothReaderParams = {
  reader: Reader.Type;
  locationId?: string;
+  autoReconnect: boolean;


looks like this param is named autoReconnectOnUnexpectedDisconnect on the native SDKs, can we stick to that here as well?

nazli-stripe · 2023-08-11T22:40:56Z

src/types/index.ts

 };

 export type ConnectUsbReaderParams = {
  reader: Reader.Type;
  locationId?: string;
+  autoReconnect: boolean;


same as above

nazli-stripe · 2023-08-11T22:41:13Z

src/hooks/useStripeTerminal.tsx

+  }, [onDidStartReaderReconnect]);
+
+  const terminalDidSucceedReaderReconnect = useCallback(() => {
+    console.log('Eric userStripeTerminal terminalDidSucceedReaderReconnect');


nazli-stripe · 2023-08-11T22:41:17Z

src/hooks/useStripeTerminal.tsx

+  }, [onTerminalDidSucceedReaderReconnect]);
+
+  const terminalDidFailReaderReconnect = useCallback(() => {
+    console.log('Eric userStripeTerminal terminalDidFailReaderReconnect');


nazli-stripe · 2023-08-11T22:42:45Z

src/components/StripeTerminalProvider.tsx

+  const didStartReaderReconnect = useCallback(() => {
+    log('didStartReaderReconnect');
+    emitter?.emit(START_READER_RECONNECT);
+  }, [log]);
+
+  const terminalDidSucceedReaderReconnect = useCallback(() => {
+    log('terminalDidSucceedReaderReconnect');
+    emitter?.emit(TERMINAL_SUCCEED_READER_RECONNECT);
+  }, [log]);
+
+  const terminalDidFailReaderReconnect = useCallback(() => {
+    log('terminalDidFailReaderReconnect');
+    emitter?.emit(TERMINAL_FAIL_READER_RECONNECT);
+  }, [log]);


should there be results emitted here?

nazli-stripe · 2023-08-11T23:57:42Z

dev-app/src/util/merchantStorage.ts

+export const setEnableAutoReconnect = async (autoReconnection: boolean) =>
+  await AsyncStorage.setItem(
+    AUTOMATIC_RECONNECTION_KEY,
+    JSON.stringify(autoReconnection)
+  );


asyncStorage seems like a bit of an overkill to persist this. maybe we can leverage AppContext instead https://github.com/stripe/stripe-terminal-react-native/blob/4bfa8622/dev-app/src/AppContext.ts#L8-L14

bric-stripe · 2023-08-14T15:32:31Z

ios/StripeTerminalReactNative.swift

+        var connectionConfig: BluetoothConnectionConfiguration
+        if autoReconnectOnUnexpectedDisconnect {
+            connectionConfig = BluetoothConnectionConfiguration(
+                        locationId: locationId ?? selectedReader.locationId ?? "",
+                        autoReconnectOnUnexpectedDisconnect: true,
+                        autoReconnectionDelegate: self
+                    )
+        } else {
+            connectionConfig = BluetoothConnectionConfiguration(
+						locationId: locationId ?? selectedReader.locationId ?? "")
+        }


I think this could be simplified to a single BluetoothConnectionConfiguration if you use the autoReconnetOnUnexpectedDisconnect variable as the value in the init, so:

let connectionConfig = BluetoothConnectionConfiguration( locationId: locationId ?? selectedReader.locationId ?? "", autoReconnectOnUnexpectedDisconnect: autoReconnectOnUnexpectedDisconnect, autoReconnectionDelegate: autoReconnectOnUnexpectedDisconnect ? self : nil

bric-stripe · 2023-08-14T15:35:03Z

android/src/main/java/com/stripeterminalreactnative/ReactNativeConstants.kt

+    TERMINAL_SUCCEED_READER_RECONNECT("terminalDidSucceedReaderReconnect"),
+    TERMINAL_FAIL_READER_RECONNECT("terminalDidFailReaderReconnect"),


I think we should drop TERMINAL from these. "Terminal" is implied (for the native iOS callbacks it was added for consistency with other Terminal delegates but in a future version of the SDK its switching to be a reader delegate and will include the instance of the reader that succeeded or failed).

SUCCEED_READER_RECONNECT("didSucceedReaderReconnect"), FAIL_READER_RECONNECT("didFailReaderReconnect"),

nazli-stripe · 2023-08-14T23:07:09Z

android/src/main/java/com/stripeterminalreactnative/listener/RNReaderReconnectionListener.kt

+    override fun onReaderReconnectFailed(reader: Reader) {
+        context.sendEvent(TERMINAL_FAIL_READER_RECONNECT.listenerName) {
+            putMap("reader", mapFromReader(reader))
+        }
+    }


do we need to put a failure message here similar to

stripe-terminal-react-native/android/src/main/java/com/stripeterminalreactnative/listener/RNTerminalListener.kt

Lines 20 to 24 in 1870117

putMap("error", nativeMapOf {

putString("code", TerminalErrorCode.UNEXPECTED_SDK_ERROR.toString())

putString("message", "Reader has been disconnected unexpectedly")

})

}

cc @chr-stripe

nazli-stripe · 2023-08-14T23:09:02Z

ios/StripeTerminalReactNative.swift

+    func terminalDidFailReaderReconnect(_ terminal: Terminal) {
+        sendEvent(withName: ReactNativeConstants.TERMINAL_FAIL_READER_RECONNECT.rawValue, body: nil)
+    }


same as Android comment above, do we want to send an error here like https://github.com/stripe/stripe-terminal-react-native/blob/1870117c/ios/StripeTerminalReactNative.swift#L331-L334

cc @bric-stripe

nazli-stripe · 2023-08-14T23:14:40Z

ios/StripeTerminalReactNative.swift

+    func terminal(_ terminal: Terminal, didStartReaderReconnect cancelable: Cancelable) {
+        sendEvent(withName: ReactNativeConstants.START_READER_RECONNECT.rawValue, body: nil)
+    }
+
+    func terminalDidSucceedReaderReconnect(_ terminal: Terminal) {
+        sendEvent(withName: ReactNativeConstants.TERMINAL_SUCCEED_READER_RECONNECT.rawValue, body: nil)
+    }


in the Android listener we use mapFromReader to send the reader in body, should that here as well instead of sending nil

nazli-stripe · 2023-08-15T00:05:48Z

ios/StripeTerminalReactNative.swift

@@ -513,6 +525,18 @@ class StripeTerminalReactNative: RCTEventEmitter, DiscoveryDelegate, BluetoothRe
        sendEvent(withName: ReactNativeConstants.CHANGE_CONNECTION_STATUS.rawValue, body: ["result": result])
    }

+    func terminal(_ terminal: Terminal, didStartReaderReconnect cancelable: Cancelable) {


looks like we are not doing much with the cancelable here. We should save that to var like cancelReaderConnectionCancellable and then expose a cancelReaderReconnection method for users to call
cc @bric-stripe

ah, good catch. Yeah that should be made available to the integrations so it can be used 👍

nazli-stripe · 2023-08-15T00:13:34Z

android/src/main/java/com/stripeterminalreactnative/listener/RNReaderReconnectionListener.kt

+        }
+    }
+
+    override fun onReaderReconnectStarted(reader: Reader, cancelReconnect: Cancelable) {


need to deal with the Cancelable here and expose a cancelReaderReconnect method similar to Android. Existing example: https://github.com/stripe/stripe-terminal-react-native/blob/1870117c/android/src/main/java/com/stripeterminalreactnative/listener/RNBluetoothReaderListener.kt#L35-L37

cc @chr-stripe

nazli-stripe · 2023-08-15T00:14:13Z

android/src/main/java/com/stripeterminalreactnative/listener/RNReaderReconnectionListener.kt

@@ -0,0 +1,32 @@
+package com.stripeterminalreactnative.listener


can we add tests for this? example: https://github.com/stripe/stripe-terminal-react-native/blob/1870117c/android/src/test/java/com/stripeterminalreactnative/listener/RNBluetoothReaderListenerTest.kt#L1

Add test for RNBluetoothReaderListener;

nazli-stripe · 2023-08-16T05:42:15Z

src/components/StripeTerminalProvider.tsx

+  const didFailReaderReconnect = useCallback(() => {
+    log('didFailReaderReconnect');
+  }, [log]);
+


should emit the error message here

example https://github.com/stripe/stripe-terminal-react-native/blob/1870117c/src/components/StripeTerminalProvider.tsx#L106

nazli-stripe · 2023-08-16T05:42:43Z

src/components/StripeTerminalProvider.tsx

+  const didStartReaderReconnect = useCallback(() => {
+    log('didStartReaderReconnect');
+  }, [log]);
+
+  const didSucceedReaderReconnect = useCallback(() => {
+    log('didSucceedReaderReconnect');
+  }, [log]);


should emit the connection reader here

example https://github.com/stripe/stripe-terminal-react-native/blob/1870117c/src/components/StripeTerminalProvider.tsx#L90

EricLin-BBpos added 3 commits August 1, 2023 15:48

Add ReaderReconnectionListener in sdk

7f8ed62

Support iOS Bluetooth automatic reconnection.

3b010b5

Fix type

05ff404

EricLin-BBpos requested review from billfinn-stripe, bric-stripe, nazli-stripe, rv-stripe and chr-stripe as code owners August 3, 2023 06:22

EricLin-BBpos and others added 7 commits August 3, 2023 16:07

Add missing import.

c94b21f

Add auto-reconnect toggle for QA testers.

9e82916

Fix syntax errors on iOS.

5684d82

Remove unnecessary parameter in reconnecting callbacks.

d72fba5

Update reader status while reconnecting fail.

Merge branch 'main' into Bluetooth-USB-automatic-reconnection

5aa434b

Fix e2e failing in auto-reconnect testing.

e6e9738

Merge branch 'Bluetooth-USB-automatic-reconnection' of https://github…

d734481

….com/EricLin-BBpos/stripe-terminal-react-native into Bluetooth-USB-automatic-reconnection

nazli-stripe reviewed Aug 11, 2023

View reviewed changes

Rename autoReconnect to autoReconnectOnUnexpectedDisconnect.

351e56f

EricLin-BBpos requested a review from nazli-stripe August 14, 2023 10:17

bric-stripe reviewed Aug 14, 2023

View reviewed changes

nazli-stripe reviewed Aug 14, 2023

View reviewed changes

nazli-stripe reviewed Aug 15, 2023

View reviewed changes

EricLin-BBpos added 2 commits August 15, 2023 19:16

Code optimization , expose cancelReaderReconnection method ;

3455132

Add test for RNBluetoothReaderListener;

Simplified BluetoothConnectionConfiguration/UsbConnectionConfiguration

dd941e0

nazli-stripe reviewed Aug 16, 2023

View reviewed changes

Add emit event on reconnection callbacks.

faa72e3

nazli-stripe approved these changes Aug 16, 2023

View reviewed changes

nazli-stripe merged commit 22fe81a into stripe:main Aug 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bluetooth/Usb automatic reconnection #516

Bluetooth/Usb automatic reconnection #516

EricLin-BBpos commented Aug 3, 2023

nazli-stripe Aug 11, 2023

nazli-stripe Aug 11, 2023

nazli-stripe Aug 11, 2023

nazli-stripe Aug 11, 2023

nazli-stripe Aug 11, 2023

nazli-stripe Aug 11, 2023

bric-stripe Aug 14, 2023

bric-stripe Aug 14, 2023

nazli-stripe Aug 14, 2023

nazli-stripe Aug 14, 2023

nazli-stripe Aug 14, 2023

nazli-stripe Aug 15, 2023

bric-stripe Aug 15, 2023

nazli-stripe Aug 15, 2023

nazli-stripe Aug 15, 2023

nazli-stripe Aug 16, 2023

nazli-stripe Aug 16, 2023

nazli-stripe Aug 16, 2023

nazli-stripe Aug 16, 2023

		TERMINAL_SUCCEED_READER_RECONNECT("terminalDidSucceedReaderReconnect"),
		TERMINAL_FAIL_READER_RECONNECT("terminalDidFailReaderReconnect"),

	putMap("error", nativeMapOf {
	putString("code", TerminalErrorCode.UNEXPECTED_SDK_ERROR.toString())
	putString("message", "Reader has been disconnected unexpectedly")
	})
	}

		@@ -0,0 +1,32 @@
		package com.stripeterminalreactnative.listener

Bluetooth/Usb automatic reconnection #516

Bluetooth/Usb automatic reconnection #516

Conversation

EricLin-BBpos commented Aug 3, 2023

Summary

Motivation

Testing

Documentation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment