From eb5a492adecdaab9a5d8bd3aeb37e13a2e08eb54 Mon Sep 17 00:00:00 2001 From: geeksville Date: Thu, 11 Jun 2020 17:34:22 -0700 Subject: [PATCH] OMG - we were accidentally leaving two GATTs alive - which is super bad --- .../mesh/service/BluetoothInterface.kt | 47 ++++++++++--------- .../geeksville/mesh/service/SafeBluetooth.kt | 30 ++++++++---- 2 files changed, 44 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/com/geeksville/mesh/service/BluetoothInterface.kt b/app/src/main/java/com/geeksville/mesh/service/BluetoothInterface.kt index 73d2c70c8..2f34ae733 100644 --- a/app/src/main/java/com/geeksville/mesh/service/BluetoothInterface.kt +++ b/app/src/main/java/com/geeksville/mesh/service/BluetoothInterface.kt @@ -5,13 +5,12 @@ import android.bluetooth.BluetoothGattCharacteristic import android.bluetooth.BluetoothGattService import android.bluetooth.BluetoothManager import android.content.Context -import com.geeksville.analytics.DataPair -import com.geeksville.android.GeeksvilleApplication import com.geeksville.android.Logging import com.geeksville.concurrent.handledLaunch import com.geeksville.util.anonymize import com.geeksville.util.exceptionReporter import com.geeksville.util.ignoreException +import kotlinx.coroutines.Job import kotlinx.coroutines.delay import java.lang.reflect.Method import java.util.* @@ -230,17 +229,29 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String doReadFromRadio(false) } } catch (ex: Exception) { - warn("error during asyncWriteCharacteristic - disconnecting, ${ex.message}") - service.serviceScope.handledLaunch { retryDueToException() } + scheduleReconnect("error during asyncWriteCharacteristic - disconnecting, ${ex.message}") } } } } catch (ex: BLEException) { - warn("error during handleSendToRadio - disconnecting, ${ex.message}") - service.serviceScope.handledLaunch { retryDueToException() } + scheduleReconnect("error during handleSendToRadio ${ex.message}") } } + @Volatile + private var reconnectJob: Job? = null + + /** + * We had some problem, schedule a reconnection attempt (if one isn't already queued) + */ + fun scheduleReconnect(reason: String) { + if (reconnectJob == null) { + warn("Scheduling reconnect because $reason") + reconnectJob = service.serviceScope.handledLaunch { retryDueToException() } + } else { + warn("Skipping reconnect for $reason") + } + } /// Attempt to read from the fromRadio mailbox, if data is found broadcast it to android apps private fun doReadFromRadio(firstRead: Boolean) { @@ -263,8 +274,7 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String startWatchingFromNum() } } catch (ex: BLEException) { - warn("error during doReadFromRadio - disconnecting, ${ex.message}") - service.serviceScope.handledLaunch { retryDueToException() } + scheduleReconnect("error during doReadFromRadio - disconnecting, ${ex.message}") } } } @@ -309,12 +319,6 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String * disconnect and try again when the device reenumerates. */ private suspend fun retryDueToException() { - // Track how often in the field we need this hack - GeeksvilleApplication.analytics.track( - "ble_reconnect_hack", - DataPair(1) - ) - /// We gracefully handle safe being null because this can occur if someone has unpaired from our device - just abandon the reconnect attempt val s = safe if (s != null) { @@ -324,6 +328,7 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String s.closeConnection() } delay(1000) // Give some nasty time for buggy BLE stacks to shutdown (500ms was not enough) + reconnectJob = null // Any new reconnect requests after this will be allowed to run warn("Attempting reconnect") startConnect() } else { @@ -365,11 +370,9 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String // Immediately broadcast any queued packets sitting on the device doReadFromRadio(true) } catch (ex: BLEException) { - errormsg( - "Unexpected error in initial device enumeration, forcing disconnect", - ex + scheduleReconnect( + "Unexpected error in initial device enumeration, forcing disconnect $ex" ) - retryDueToException() } } } @@ -398,12 +401,10 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String doDiscoverServicesAndInit() } catch (ex: BLEException) { - errormsg( - "Giving up on setting MTUs, forcing disconnect", - ex - ) shouldSetMtu = false - service.serviceScope.handledLaunch { retryDueToException() } + scheduleReconnect( + "Giving up on setting MTUs, forcing disconnect $ex" + ) } } else diff --git a/app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt b/app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt index 8be3e8933..1ff591646 100644 --- a/app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt +++ b/app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt @@ -196,7 +196,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD val oldstate = state state = newState if (oldstate == BluetoothProfile.STATE_CONNECTED) { - info("Lost connection - aborting current work") + info("Lost connection - aborting current work: $currentWork") /* Supposedly this reconnect attempt happens automatically @@ -224,8 +224,10 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD debug("queuing a reconnection callback") assert(currentWork == null) - if (!currentConnectIsAuto) // we must have been running during that 1-time manual connect, switch to auto-mode from now on + if (!currentConnectIsAuto) { // we must have been running during that 1-time manual connect, switch to auto-mode from now on + closeGatt() // Close the old non-auto connection lowLevelConnect(true) + } // note - we don't need an init fn (because that would normally redo the connectGatt call - which we don't need) queueWork("reconnect", CallbackContinuation(cb)) { -> true } @@ -243,6 +245,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD // to initiate a background connection. if (autoConnect) { warn("Failed on non-auto connect, falling back to auto connect attempt") + closeGatt() // Close the old non-auto connection lowLevelConnect(true) } } @@ -484,6 +487,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD private fun lowLevelConnect(autoNow: Boolean): BluetoothGatt? { currentConnectIsAuto = autoNow + logAssert(gatt == null) val g = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { device.connectGatt( @@ -663,6 +667,18 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD ) = queueWriteDescriptor(c, CallbackContinuation(cb)) + /** Close just the GATT device but keep our pending callbacks active */ + fun closeGatt() { + + gatt?.let { g -> + info("Closing our GATT connection") + gatt = + null // Clear this first so the onConnectionChange callback can ignore while we are shutting down + g.disconnect() + g.close() + } + } + /** * Close down any existing connection, any existing calls (including async connects will be * cancelled and you'll need to recall connect to use this againt @@ -675,18 +691,12 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD // Cancel any notifications - because when the device comes back it might have forgotten about us notifyHandlers.clear() + closeGatt() + ignoreException { // Hmm - sometimes the "Connection closing" exception comes back to us - ignore it failAllWork(BLEException("Connection closing")) } - - gatt?.let { g -> - info("Closing our GATT connection") - gatt = - null // Clear this first so the onConnectionChange callback can ignore while we are shutting down - g.disconnect() - g.close() - } } /**