Merge pull request #91 from geeksville/dev

Dev

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