From 79ff739e616c9ec8c37dade18b955e005497c8f0 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 17 Dec 2020 16:01:40 +0000 Subject: [PATCH 1/3] Related to #111. Advertiser API divergence. Related to #111. Advertiser API divergence. - May fix issues on S20 (Exynos based ones, SnapDragon already work) - For External testing Signed-off-by: Adam Fowler --- .idea/codeStyles/Project.xml | 15 +++--------- .../sensor/ble/ConcreteBLETransmitter.java | 23 +++++++++++-------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index 0d156937..663459aa 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -3,18 +3,9 @@ - diff --git a/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java b/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java index 7950d9d3..bd4c881c 100644 --- a/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java +++ b/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java @@ -104,19 +104,24 @@ private enum AdvertLoopState { private BluetoothLeAdvertiser bluetoothLeAdvertiser() { final BluetoothAdapter bluetoothAdapter = BluetoothAdapter.getDefaultAdapter(); if (bluetoothAdapter == null) { - //logger.fault("Bluetooth adapter unavailable"); + logger.debug("bluetoothLeAdvertiser, no Bluetooth Adapter available"); return null; } - if (!bluetoothAdapter.isMultipleAdvertisementSupported()) { - //logger.fault("Bluetooth advertisement unsupported"); - return null; - } - final BluetoothLeAdvertiser bluetoothLeAdvertiser = bluetoothAdapter.getBluetoothLeAdvertiser(); - if (bluetoothLeAdvertiser == null) { - //logger.fault("Bluetooth advertisement unavailable"); + boolean supported = bluetoothAdapter.isMultipleAdvertisementSupported(); + try { + final BluetoothLeAdvertiser bluetoothLeAdvertiser = bluetoothAdapter.getBluetoothLeAdvertiser(); + if (bluetoothLeAdvertiser == null) { + logger.debug("bluetoothLeAdvertiser, no LE advertiser present (multiSupported={}, exception=no)", supported); + return null; + } + // log this, as this will allow us to identify handsets with a different API implementation + logger.debug("bluetoothLeAdvertiser, LE advertiser present (multiSupported={})", supported); + return bluetoothLeAdvertiser; + } catch (Exception e) { + // log it, as this will allow us to identify handsets with the expected API implementation (from Android API source code) + logger.debug("bluetoothLeAdvertiser, no LE advertiser present (multiSupported={}, exception={})", supported, e.getMessage()); return null; } - return bluetoothLeAdvertiser; } private class AdvertLoopTask implements BLETimerDelegate { From a63669b0fe07fe0be2542190df7ccede8cdb8449 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 17 Dec 2020 16:18:44 +0000 Subject: [PATCH 2/3] Extra debug logging to try and replicate #107 Extra debug logging to try and replicate #107 - Now issue potentially traced to Exynos based Samsung devices we can try and replicate - This change only applies extra logging to confirm the issue cause locally - ISV partner has an Exynos S20 for testing Signed-off-by: Adam Fowler --- .../sensor/ble/ConcreteBLETransmitter.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java b/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java index bd4c881c..cab49da6 100644 --- a/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java +++ b/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java @@ -540,6 +540,13 @@ private static void setGattService(final SensorLogger logger, final Context cont bluetoothGattServer.cancelConnection(device); } bluetoothGattServer.clearServices(); + + // Logic check - ensure there are now no Gatt Services + List services = bluetoothGattServer.getServices(); + for (BluetoothGattService svc : services) { + logger.fault("setGattService device clearServices() call did not correctly clear service (service={})",svc.getUuid()); + } + final BluetoothGattService service = new BluetoothGattService(BLESensorConfiguration.serviceUUID, BluetoothGattService.SERVICE_TYPE_PRIMARY); final BluetoothGattCharacteristic signalCharacteristic = new BluetoothGattCharacteristic( BLESensorConfiguration.androidSignalCharacteristicUUID, @@ -558,6 +565,19 @@ private static void setGattService(final SensorLogger logger, final Context cont } service.addCharacteristic(payloadCharacteristic); bluetoothGattServer.addService(service); + + // Logic check - ensure there can be only one Herald service + services = bluetoothGattServer.getServices(); + int count = 0; + for (BluetoothGattService svc : services) { + if (svc.getUuid().equals(BLESensorConfiguration.serviceUUID)) { + count++; + } + } + if (count > 1) { + logger.fault("setGattService device incorrectly sharing multiple Herald services (count={})", count); + } + logger.debug("setGattService successful (service={},signalCharacteristic={},payloadCharacteristic={})", service.getUuid(), signalCharacteristic.getUuid(), payloadCharacteristic.getUuid()); } From f7cd8b6d3d8d9bbca4d8e87d8f3fbe9f3c78ecb7 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 17 Dec 2020 18:03:01 +0000 Subject: [PATCH 3/3] Potential workaround for Exynos gatt server implementation Related to #107. Potential workaround for Exynos gatt server implementation - Existing gattServer instances on devices with the Exynos base chipset do not remove adverts from the same source app - Affects Exynos version of some Samsung handsets, but not the more prevalent SnapDragon versions - Only causes an issue in testing if Bluetooth is turned on/off 23 times - a rare occurrence in normal usage - Without the fix Herald would still allow an affected device to interact with other devices (iOS and Android) thanks to the 'calling card' / write characteristic - Does not cause an issue for detection unless write characteristic/calling card is disabled (Not done in the Herald code base or library) - Issue #110 also occurs on Exynos based handsets only - Committing to develop to allow third party ISV testing today Signed-off-by: Adam Fowler --- .../sensor/ble/ConcreteBLETransmitter.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java b/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java index cab49da6..7192f345 100644 --- a/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java +++ b/herald/src/main/java/com/vmware/herald/sensor/ble/ConcreteBLETransmitter.java @@ -64,6 +64,9 @@ public class ConcreteBLETransmitter implements BLETransmitter, BluetoothStateMan private final BLEDatabase database; private final ExecutorService operationQueue = Executors.newSingleThreadExecutor(); + // Referenced by startAdvert and stopExistingGattServer ONLY + private BluetoothGattServer bluetoothGattServer = null; + /** * Transmitter starts automatically when Bluetooth is enabled. */ @@ -203,13 +206,29 @@ public void accept(Boolean value) { // MARK:- Start and stop advert + private void stopExistingGattServer() { + if (null != bluetoothGattServer) { + // Stop old version, if there's already a proxy reference + try { + bluetoothGattServer.clearServices(); + bluetoothGattServer.close(); + bluetoothGattServer = null; + } catch (Throwable e2) { + logger.fault("stopGattServer failed to stop EXISTING GATT server", e2); + bluetoothGattServer = null; + } + } + } + private void startAdvert(final BluetoothLeAdvertiser bluetoothLeAdvertiser, final Callback> callback) { logger.debug("startAdvert"); operationQueue.execute(new Runnable() { @Override public void run() { boolean result = true; - BluetoothGattServer bluetoothGattServer = null; + + stopExistingGattServer(); + try { bluetoothGattServer = startGattServer(logger, context, payloadDataSupplier, database); } catch (Throwable e) {