From 38b4bc255cc3a110a9e6e0dbeacc0b6815656629 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 4 Jul 2020 17:37:52 -0700 Subject: [PATCH] 0.7.90 read on if you want to see the stories of bluetooth trauma #76 So I'm trying to get an old version of bluetooth on Sony phones to work In the process I realized I've always been doing something dumb which bastically works fine, but is an ugly hack I also did to support old devices. Due to an accident of history there were two different layers of the app which were trying to manage bluetooth connections, which was dumb. Always only one layer of an app should worry about such things. /** * Should we automatically try to reconnect when we lose our connection? * * Originally this was true, but over time (now that clients are smarter and need to build * up more state) I see this was a mistake. Now if the connection drops we just call * the lostConnection callback and the client of this API is responsible for reconnecting. * This also prevents nasty races when sometimes both the upperlayer and this layer decide to reconnect * simultaneously. */ --- app/build.gradle | 4 ++-- .../geeksville/mesh/service/BluetoothInterface.kt | 14 +++++++++----- .../com/geeksville/mesh/service/SafeBluetooth.kt | 13 ++++++++++++- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index fd19ee04e..be3da5d86 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -17,8 +17,8 @@ android { applicationId "com.geeksville.mesh" minSdkVersion 21 // The oldest emulator image I have tried is 22 (though 21 probably works) targetSdkVersion 29 - versionCode 10789 // format is Mmmss (where M is 1+the numeric major number - versionName "0.7.89" + versionCode 10790 // format is Mmmss (where M is 1+the numeric major number + versionName "0.7.90" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } buildTypes { 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 235a6e58c..06e86969b 100644 --- a/app/src/main/java/com/geeksville/mesh/service/BluetoothInterface.kt +++ b/app/src/main/java/com/geeksville/mesh/service/BluetoothInterface.kt @@ -325,11 +325,15 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String val s = safe if (s != null) { warn("Forcing disconnect and hopefully device will comeback (disabling forced refresh)") - hasForcedRefresh = - true // We've already tossed any old service caches, no need to do it again + + // The following optimization is not currently correct - because the device might be sleeping and come back with different BLE handles + // hasForcedRefresh = true // We've already tossed any old service caches, no need to do it again + + // Make sure the old connection was killed ignoreException { s.closeConnection() } + service.onDisconnect(false) // assume we will fail 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 @@ -416,7 +420,7 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String if (shouldSetMtu) safe?.asyncRequestMtu(512) { mtuRes -> try { - mtuRes.getOrThrow() // FIXME - why sometimes is the result Unit!?! + mtuRes.getOrThrow() debug("MTU change attempted") // throw BLEException("Test MTU set failed") @@ -457,9 +461,9 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String // comes in range (even if we made this connect call long ago when we got powered on) // see https://stackoverflow.com/questions/40156699/which-correct-flag-of-autoconnect-in-connectgatt-of-ble for // more info - safe!!.asyncConnect(true, // FIXME, sometimes this seems to not work or take a very long time! + safe!!.asyncConnect(true, cb = ::onConnect, - lostConnectCb = { service.onDisconnect(isPermanent = false) }) + lostConnectCb = { scheduleReconnect("connection dropped") }) } 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 1e7b6ea07..4306ea3a8 100644 --- a/app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt +++ b/app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt @@ -127,6 +127,17 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD private val STATUS_NOSTART = 4405 private val STATUS_SIMFAILURE = 4406 + /** + * Should we automatically try to reconnect when we lose our connection? + * + * Originally this was true, but over time (now that clients are smarter and need to build + * up more state) I see this was a mistake. Now if the connection drops we just call + * the lostConnection callback and the client of this API is responsible for reconnecting. + * This also prevents nasty races when sometimes both the upperlayer and this layer decide to reconnect + * simultaneously. + */ + private val autoReconnect = false + private val gattCallback = object : BluetoothGattCallback() { override fun onConnectionStateChange( @@ -164,7 +175,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD // If we get a disconnect, just try again otherwise fail all current operations // Note: if no work is pending (likely) we also just totally teardown and restart the connection, because we won't be // throwing a lost connection exception to any worker. - if (currentWork == null || currentWork?.isConnect() == true) + if (autoReconnect && (currentWork == null || currentWork?.isConnect() == true)) dropAndReconnect() else lostConnection("lost connection")