Merge pull request #61 from geeksville/dev

Dev

TODO.md

@@ -6,6 +6,7 @@
 Things for the betaish period.
 
 * let user change more channel parameters
+* really great notes about importance of clean BLE disconnects: https://blog.classycode.com/a-short-story-about-android-ble-connection-timeouts-and-gatt-internal-errors-fa89e3f6a456
 
 # Documentation tasks
 

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 10778 // format is Mmmss (where M is 1+the numeric major number
-        versionName "0.7.78"
+        versionCode 10780 // format is Mmmss (where M is 1+the numeric major number
+        versionName "0.7.80"
         testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
     }
     buildTypes {
@@ -137,7 +137,7 @@ dependencies {
     implementation 'com.google.android.gms:play-services-auth:18.0.0'
 
     // Add the Firebase SDK for Crashlytics.
-    implementation 'com.google.firebase:firebase-crashlytics:17.0.1'
+    implementation 'com.google.firebase:firebase-crashlytics:17.1.0'
 
     // alas implementation bug deep in the bowels when I tried it for my SyncBluetoothDevice class
     // implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.3"

app/src/main/java/com/geeksville/mesh/NodeInfo.kt

@@ -89,7 +89,11 @@ data class NodeInfo(
     /// return the position if it is valid, else null
     val validPosition: Position?
         get() {
-            return position?.takeIf { it.latitude != 0.0 || it.longitude != 0.0 }
+            return position?.takeIf {
+                (it.latitude <= 90.0 && it.latitude >= -90) && // If GPS gives a crap position don't crash our app
+                        it.latitude != 0.0 &&
+                        it.longitude != 0.0
+            }
         }
 
     /// @return distance in meters to some other node (or null if unknown)

app/src/main/java/com/geeksville/mesh/service/BluetoothStateReceiver.kt

@@ -0,0 +1,32 @@
+package com.geeksville.mesh.service
+
+import android.bluetooth.BluetoothAdapter
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import android.content.IntentFilter
+import com.geeksville.util.exceptionReporter
+
+/**
+ * A helper class to call onChanged when bluetooth is enabled or disabled
+ */
+class BluetoothStateReceiver(val onChanged: (Boolean) -> Unit) : BroadcastReceiver() {
+    val intent =
+        IntentFilter(BluetoothAdapter.ACTION_STATE_CHANGED) // Can be used for registering
+
+    override fun onReceive(context: Context, intent: Intent) =
+        exceptionReporter {
+            if (intent.action == BluetoothAdapter.ACTION_STATE_CHANGED) {
+                when (intent.getIntExtra(
+                    BluetoothAdapter.EXTRA_STATE,
+                    -1
+                )) {
+                    // Simulate a disconnection if the user disables bluetooth entirely
+                    BluetoothAdapter.STATE_OFF -> onChanged(
+                        false
+                    )
+                    BluetoothAdapter.STATE_ON -> onChanged(true)
+                }
+            }
+        }
+}

app/src/main/java/com/geeksville/mesh/service/RadioInterfaceService.kt

@@ -132,7 +132,9 @@ class RadioInterfaceService : Service(), Logging {
      */
     private val bluetoothStateReceiver = BluetoothStateReceiver { enabled ->
         if (enabled)
-            startInterface() // If bluetooth just got turned on, try to restart our ble link
+            startInterface() // If bluetooth just got turned on, try to restart our ble link (which might be bluetooth)
+        else if (radioIf is BluetoothInterface)
+            stopInterface() // Was using bluetooth, need to shutdown
     }
 
     private fun broadcastConnectionChanged(isConnected: Boolean, isPermanent: Boolean) {

app/src/main/java/com/geeksville/mesh/service/SafeBluetooth.kt

@@ -1,10 +1,7 @@
 package com.geeksville.mesh.service
 
 import android.bluetooth.*
-import android.content.BroadcastReceiver
 import android.content.Context
-import android.content.Intent
-import android.content.IntentFilter
 import android.os.Build
 import android.os.DeadObjectException
 import android.os.Handler
@@ -22,26 +19,9 @@ import java.util.*
 
 
 /// Return a standard BLE 128 bit UUID from the short 16 bit versions
-fun longBLEUUID(hexFour: String) = UUID.fromString("0000$hexFour-0000-1000-8000-00805f9b34fb")
+fun longBLEUUID(hexFour: String): UUID = UUID.fromString("0000$hexFour-0000-1000-8000-00805f9b34fb")
 
 
-/**
- * A helper class to call onChanged when bluetooth is enabled or disabled
- */
-class BluetoothStateReceiver(val onChanged: (Boolean) -> Unit) : BroadcastReceiver() {
-    val intent = IntentFilter(BluetoothAdapter.ACTION_STATE_CHANGED) // Can be used for registering
-
-    override fun onReceive(context: Context, intent: Intent) = exceptionReporter {
-        if (intent.action == BluetoothAdapter.ACTION_STATE_CHANGED) {
-            when (intent.getIntExtra(BluetoothAdapter.EXTRA_STATE, -1)) {
-                // Simulate a disconnection if the user disables bluetooth entirely
-                BluetoothAdapter.STATE_OFF -> onChanged(false)
-                BluetoothAdapter.STATE_ON -> onChanged(true)
-            }
-        }
-    }
-}
-
 /**
  * Uses coroutines to safely access a bluetooth GATT device with a synchronous API
  *
@@ -79,7 +59,8 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
 
     private val serviceScope = CoroutineScope(Dispatchers.IO)
 
-    /// When we see the BT stack getting disabled/renabled we handle that as a connect/disconnect event
+    /// When we see the BT stack getting disabled we handle that as a disconnect event
+    /*
     private val btStateReceiver = BluetoothStateReceiver { enabled ->
         // Sometimes we might not have a gatt object, while that is true, we don't care about BLE state changes
         gatt?.let { g ->
@@ -90,14 +71,14 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
                         0,
                         BluetoothProfile.STATE_DISCONNECTED
                     )
-                else
-                    debug("We were not connected, so ignoring bluetooth shutdown")
-            } else {
-                warn("requeue a connect anytime bluetooth is reenabled")
-                reconnect()
+                else {
+                    debug("we are not connected, but BLE was disabled so shutdown everything")
+                    closeConnection()
+                }
             }
         }
     }
+     */
 
     /**
      * A BLE status code based error
@@ -109,10 +90,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
         longBLEUUID("2902")
 
     init {
-        context.registerReceiver(
-            btStateReceiver,
-            btStateReceiver.intent
-        )
+        //context.registerReceiver( btStateReceiver, btStateReceiver.intent )
     }
 
     /**
@@ -191,51 +169,17 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
                         completeWork(status, Unit)
                 }
                 BluetoothProfile.STATE_DISCONNECTED -> {
-                    if (gatt == null)
+                    if (gatt == null) {
                         info("No gatt: ignoring connection state $newState, status $status") // Probably just shutting down
-                    else {
+                        g.close() // Finish closing our gatt here
+                    } else {
                         // cancel any queued ops if we were already connected
                         val oldstate = state
                         state = newState
                         if (oldstate == BluetoothProfile.STATE_CONNECTED) {
                             info("Lost connection - aborting current work: $currentWork")
 
-                            /*
-                            Supposedly this reconnect attempt happens automatically
-                            "If the connection was established through an auto connect, Android will
-                            automatically try to reconnect to the remote device when it gets disconnected
-                            until you manually call disconnect() or close(). Once a connection established
-                            through direct connect disconnects, no attempt is made to reconnect to the remote device."
-                            https://stackoverflow.com/questions/37965337/what-exactly-does-androids-bluetooth-autoconnect-parameter-do?rq=1
-
-                            closeConnection()
-                            */
-                            failAllWork(BLEException("Lost connection"))
-
-                            // Cancel any notifications - because when the device comes back it might have forgotten about us
-                            notifyHandlers.clear()
-
-                            lostConnectCallback?.let {
-                                debug("calling lostConnect handler")
-                                it.invoke()
-                            }
-
-                            // Queue a new connection attempt
-                            val cb = connectionCallback
-                            if (cb != null) {
-                                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
-                                    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 }
-                            } else {
-                                debug("No connectionCallback registered")
-                            }
+                            dropAndReconnect()
                         } else if (status == 133) {
                             // We were not previously connected and we just failed with our non-auto connection attempt.  Therefore we now need
                             // to do an autoconnection attempt.  When that attempt succeeds/fails the normal callbacks will be called
@@ -562,11 +506,53 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
 
     /// Restart any previous connect attempts
     private fun reconnect() {
+        // closeGatt() // Get rid of any old gatt
+
         connectionCallback?.let { cb ->
             queueConnect(true, CallbackContinuation(cb))
         }
     }
 
+    /// Drop our current connection and then requeue a connect as needed
+    private fun dropAndReconnect() {
+        /*
+        Supposedly this reconnect attempt happens automatically
+        "If the connection was established through an auto connect, Android will
+        automatically try to reconnect to the remote device when it gets disconnected
+        until you manually call disconnect() or close(). Once a connection established
+        through direct connect disconnects, no attempt is made to reconnect to the remote device."
+        https://stackoverflow.com/questions/37965337/what-exactly-does-androids-bluetooth-autoconnect-parameter-do?rq=1
+
+        closeConnection()
+        */
+        failAllWork(BLEException("Lost connection"))
+
+        // Cancel any notifications - because when the device comes back it might have forgotten about us
+        notifyHandlers.clear()
+
+        lostConnectCallback?.let {
+            debug("calling lostConnect handler")
+            it.invoke()
+        }
+
+        // Queue a new connection attempt
+        val cb = connectionCallback
+        if (cb != null) {
+            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
+                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 }
+        } else {
+            debug("No connectionCallback registered")
+        }
+    }
+
     fun connect(autoConnect: Boolean = false) =
         makeSync<Unit> { queueConnect(autoConnect, it) }
 
@@ -667,7 +653,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
     private fun queueWriteDescriptor(
         c: BluetoothGattDescriptor,
         cont: Continuation<BluetoothGattDescriptor>
-    ) = queueWork("writeD", cont) { gatt!!.writeDescriptor(c) }
+    ) = queueWork("writeD", cont) { gatt?.writeDescriptor(c) ?: false }
 
     fun asyncWriteDescriptor(
         c: BluetoothGattDescriptor,
@@ -684,7 +670,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
                 null // Clear this first so the onConnectionChange callback can ignore while we are shutting down
             try {
                 g.disconnect()
-                g.close()
+                g.close() // movedinto the onDisconnect callback?
             } catch (ex: DeadObjectException) {
                 Exceptions.report(ex, "Dead object while closing GATT")
             }
@@ -717,7 +703,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
     override fun close() {
         closeConnection()
 
-        context.unregisterReceiver(btStateReceiver)
+        // context.unregisterReceiver(btStateReceiver)
     }
 
 

app/src/test/java/com/geeksville/mesh/PositionTest.kt

@@ -0,0 +1,18 @@
+package com.geeksville.mesh
+
+
+import org.junit.Assert
+import org.junit.Test
+
+class PositionTest {
+    @Test
+    fun degGood() {
+        Assert.assertEquals(Position.degI(89.0), 890000000)
+        Assert.assertEquals(Position.degI(-89.0), -890000000)
+
+        Assert.assertEquals(Position.degD(Position.degI(89.0)), 89.0, 0.01)
+        Assert.assertEquals(Position.degD(Position.degI(-89.0)), -89.0, 0.01)
+    }
+
+
+}

build.gradle

@@ -21,7 +21,7 @@ buildscript {
         // Check that you have the Google Services Gradle plugin v4.3.2 or later
         // (if not, add it).
         classpath 'com.google.gms:google-services:4.3.3'
-        classpath 'com.google.firebase:firebase-crashlytics-gradle:2.0.0-beta04'
+        classpath 'com.google.firebase:firebase-crashlytics-gradle:2.2.0'
 
         // protobuf plugin - docs here https://github.com/google/protobuf-gradle-plugin
         classpath 'com.google.protobuf:protobuf-gradle-plugin:0.8.12'

geeksville-androidlib

@@ -1 +1 @@
-Subproject commit 62172dbfa2d85a389f6edaea6b416663e8bf4d2c
+Subproject commit b2871e409cc3b24b00a26231f4efeb7e2dc170bb