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

@@ -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 {

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") })
     }
 
 

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")