diff --git a/TODO.md b/TODO.md index a9be6fb06..5fb6cb475 100644 --- a/TODO.md +++ b/TODO.md @@ -1,8 +1,7 @@ # High priority -* if radio disconnects, we need to requeue a new connect attempt in RadioService * when notified phone should download messages -* fix startup race conditions in services, allow reads to block as needed +* at connect we might receive messages before finished downloading the nodeinfo. In that case, process those messages later * investigate the Signal SMS message flow path, see if I could just make Mesh a third peer to signal & sms? * make signal work when there is no internet up * make Signal rx path work @@ -20,6 +19,7 @@ * connect to bluetooth device automatically using minimum power, start looking at phone boot * fix BT device scanning * call crashlytics from exceptionReporter!!! currently not logging failures caught there +* if nessary restart entire BT adapter with this tip from Michael https://stackoverflow.com/questions/35103701/ble-android-onconnectionstatechange-not-being-called # Medium priority @@ -68,4 +68,5 @@ Don't leave device discoverable. Don't let unpaired users do things with device * investigate a 16 bit node number. If possible it would make collisions super rare. Much easier to just pick a nodenum and go. * remove example code boilerplate from the service * switch from protobuf-java to protobuf-javalite - much faster and smaller, just no JSON debug printing -* have phone use our local node number as its node number (instead of hardwired) \ No newline at end of file +* have phone use our local node number as its node number (instead of hardwired) +* if radio disconnects, we need to requeue a new connect attempt in RadioService \ No newline at end of file diff --git a/app/src/main/java/com/geeksville/mesh/RadioInterfaceService.kt b/app/src/main/java/com/geeksville/mesh/RadioInterfaceService.kt index 3c2660aae..4e6897b49 100644 --- a/app/src/main/java/com/geeksville/mesh/RadioInterfaceService.kt +++ b/app/src/main/java/com/geeksville/mesh/RadioInterfaceService.kt @@ -193,6 +193,40 @@ class RadioInterfaceService : Service(), Logging { } + private fun onDisconnect() { + broadcastConnectionChanged(false) + isConnected = false + } + + private fun onConnect(connRes: Result) { + // This callback is invoked after we are connected + + connRes.getOrThrow() // FIXME, instead just try to reconnect? + info("Connected to radio!") + + // FIXME - no need to discover services more than once - instead use lazy() to use them in future attempts + safe.asyncDiscoverServices { discRes -> + discRes.getOrThrow() // FIXME, instead just try to reconnect? + debug("Discovered services!") + + // we begin by setting our MTU size as high as it can go + safe.asyncRequestMtu(512) { mtuRes -> + debug("requested MTU result=$mtuRes") + mtuRes.getOrThrow() // FIXME - why sometimes is the result Unit!?! + + fromRadio = service.getCharacteristic(BTM_FROMRADIO_CHARACTER) + fromNum = service.getCharacteristic(BTM_FROMNUM_CHARACTER) + + // Now tell clients they can (finally use the api) + broadcastConnectionChanged(true) + isConnected = true + + // Immediately broadcast any queued packets sitting on the device + doReadFromRadio() + } + } + } + override fun onCreate() { super.onCreate() @@ -212,35 +246,7 @@ class RadioInterfaceService : Service(), Logging { // 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 - // FIXME, broadcast connection lost/gained via broadcastConnecionChanged - safe.asyncConnect(true) { connRes -> - // This callback is invoked after we are connected - - connRes.getOrThrow() // FIXME, instead just try to reconnect? - info("Connected to radio!") - - // FIXME - no need to discover services, instead just hardwire the characteristics (like we do for toRadio) - safe.asyncDiscoverServices { discRes -> - discRes.getOrThrow() // FIXME, instead just try to reconnect? - debug("Discovered services!") - - // we begin by setting our MTU size as high as it can go - safe.asyncRequestMtu(512) { mtuRes -> - debug("requested MTU result=$mtuRes") - mtuRes.getOrThrow() - - fromRadio = service.getCharacteristic(BTM_FROMRADIO_CHARACTER) - fromNum = service.getCharacteristic(BTM_FROMNUM_CHARACTER) - - // Now tell clients they can (finally use the api) - broadcastConnectionChanged(true) - isConnected = true - - // Immediately broadcast any queued packets sitting on the device - doReadFromRadio() - } - } - } + safe.asyncConnect(true, ::onConnect, ::onDisconnect) if (logSends) sentPacketsLog = BinaryLogFile(this, "sent_log.pb") diff --git a/app/src/main/java/com/geeksville/mesh/SafeBluetooth.kt b/app/src/main/java/com/geeksville/mesh/SafeBluetooth.kt index a4c0cdacd..58c8adcb9 100644 --- a/app/src/main/java/com/geeksville/mesh/SafeBluetooth.kt +++ b/app/src/main/java/com/geeksville/mesh/SafeBluetooth.kt @@ -35,6 +35,10 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD private var currentWork: BluetoothContinuation? = null private val workQueue = mutableListOf() + // Called for reconnection attemps + private var connectionCallback: ((Result) -> Unit)? = null + private var lostConnectCallback: (() -> Unit)? = null + /// When we see the BT stack getting disabled/renabled we handle that as a connect/disconnect event private val btStateReceiver = object : BroadcastReceiver() { override fun onReceive(context: Context, intent: Intent) = exceptionReporter { @@ -42,13 +46,18 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD val newstate = intent.getIntExtra(BluetoothAdapter.EXTRA_STATE, -1) when (newstate) { // Simulate a disconnection if the user disables bluetooth entirely - BluetoothAdapter.STATE_OFF -> if (gatt != null) gattCallback.onConnectionStateChange( - gatt!!, - 0, - BluetoothProfile.STATE_DISCONNECTED - ) + BluetoothAdapter.STATE_OFF -> { + if (state == BluetoothProfile.STATE_CONNECTED) + gattCallback.onConnectionStateChange( + gatt!!, + 0, + BluetoothProfile.STATE_DISCONNECTED + ) + else + debug("We were not connected, so ignoring bluetooth shutdown") + } BluetoothAdapter.STATE_ON -> { - warn("FIXME - requeue a connect anytime bluetooth is reenabled") + warn("FIXME - requeue a connect anytime bluetooth is reenabled?") } } } @@ -86,20 +95,51 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD g: BluetoothGatt, status: Int, newState: Int - ) { - info("new bluetooth connection state $newState") - state = newState + ) = exceptionReporter { + info("new bluetooth connection state $newState, status $status") when (newState) { BluetoothProfile.STATE_CONNECTED -> { + state = + newState // we only care about connected/disconnected - not the transitional states //logAssert(workQueue.isNotEmpty()) //val work = workQueue.removeAt(0) completeWork(status, Unit) } BluetoothProfile.STATE_DISCONNECTED -> { - // cancel any queued ops? for now I think it is best to keep them around - // failAllWork(IOException("Lost connection")) + // 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") - gatt = null; + + /* + 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(IOException("Lost connection")) + + debug("calling lostConnect handler") + lostConnectCallback?.invoke() + + // Queue a new connection attempt + val cb = connectionCallback + if (cb != null) { + debug("queuing a reconnection callback") + assert(currentWork == null) + + // 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") + } + } } } } @@ -169,7 +209,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD w } - debug("work ${work.tag} is completed, resuming with status $status") + debug("work ${work.tag} is completed, resuming status=$status, res=$res") if (status != 0) work.completion.resumeWithException(IOException("Bluetooth status=$status")) else @@ -185,6 +225,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD it.completion.resumeWithException(ex) } workQueue.clear() + currentWork = null } } @@ -210,8 +251,27 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD } } - fun asyncConnect(autoConnect: Boolean = false, cb: (Result) -> Unit) { + + /** + * start a connection attempt. + * + * Note: if autoConnect is true, the callback you provide will be kept around _even after the connection is complete. + * If we ever lose the connection, this class will immediately requque the attempt (after canceling + * any outstanding queued operations). + * + * So you should expect your callback might be called multiple times, each time to reestablish a new connection. + */ + fun asyncConnect( + autoConnect: Boolean = false, + cb: (Result) -> Unit, + lostConnectCb: () -> Unit + ) { logAssert(workQueue.isEmpty() && currentWork == null) // I don't think anything should be able to sneak in front + lostConnectCallback = lostConnectCb + connectionCallback = if (autoConnect) + cb + else + null queueConnect(autoConnect, CallbackContinuation(cb)) } @@ -272,12 +332,21 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD fun writeCharacteristic(c: BluetoothGattCharacteristic): BluetoothGattCharacteristic = makeSync { queueWriteCharacteristic(c, it) } - fun disconnect() { - if (gatt != null) + private fun closeConnection() { + failAllWork(IOException("Connection closing")) + + if (gatt != null) { + info("Closing our GATT connection") gatt!!.disconnect() + gatt!!.close() + gatt = null + } + } + + fun disconnect() { + closeConnection() context.unregisterReceiver(btStateReceiver) - failAllWork(Exception("SafeBluetooth disconnected")) } }