OMG - we were accidentally leaving two GATTs alive - which is super bad

This commit is contained in:
geeksville
2020-06-11 17:34:22 -07:00
parent d282c7911e
commit eb5a492ade
2 changed files with 44 additions and 33 deletions

View File

@@ -5,13 +5,12 @@ import android.bluetooth.BluetoothGattCharacteristic
import android.bluetooth.BluetoothGattService
import android.bluetooth.BluetoothManager
import android.content.Context
import com.geeksville.analytics.DataPair
import com.geeksville.android.GeeksvilleApplication
import com.geeksville.android.Logging
import com.geeksville.concurrent.handledLaunch
import com.geeksville.util.anonymize
import com.geeksville.util.exceptionReporter
import com.geeksville.util.ignoreException
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import java.lang.reflect.Method
import java.util.*
@@ -230,17 +229,29 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String
doReadFromRadio(false)
}
} catch (ex: Exception) {
warn("error during asyncWriteCharacteristic - disconnecting, ${ex.message}")
service.serviceScope.handledLaunch { retryDueToException() }
scheduleReconnect("error during asyncWriteCharacteristic - disconnecting, ${ex.message}")
}
}
}
} catch (ex: BLEException) {
warn("error during handleSendToRadio - disconnecting, ${ex.message}")
service.serviceScope.handledLaunch { retryDueToException() }
scheduleReconnect("error during handleSendToRadio ${ex.message}")
}
}
@Volatile
private var reconnectJob: Job? = null
/**
* We had some problem, schedule a reconnection attempt (if one isn't already queued)
*/
fun scheduleReconnect(reason: String) {
if (reconnectJob == null) {
warn("Scheduling reconnect because $reason")
reconnectJob = service.serviceScope.handledLaunch { retryDueToException() }
} else {
warn("Skipping reconnect for $reason")
}
}
/// Attempt to read from the fromRadio mailbox, if data is found broadcast it to android apps
private fun doReadFromRadio(firstRead: Boolean) {
@@ -263,8 +274,7 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String
startWatchingFromNum()
}
} catch (ex: BLEException) {
warn("error during doReadFromRadio - disconnecting, ${ex.message}")
service.serviceScope.handledLaunch { retryDueToException() }
scheduleReconnect("error during doReadFromRadio - disconnecting, ${ex.message}")
}
}
}
@@ -309,12 +319,6 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String
* disconnect and try again when the device reenumerates.
*/
private suspend fun retryDueToException() {
// Track how often in the field we need this hack
GeeksvilleApplication.analytics.track(
"ble_reconnect_hack",
DataPair(1)
)
/// We gracefully handle safe being null because this can occur if someone has unpaired from our device - just abandon the reconnect attempt
val s = safe
if (s != null) {
@@ -324,6 +328,7 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String
s.closeConnection()
}
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
warn("Attempting reconnect")
startConnect()
} else {
@@ -365,11 +370,9 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String
// Immediately broadcast any queued packets sitting on the device
doReadFromRadio(true)
} catch (ex: BLEException) {
errormsg(
"Unexpected error in initial device enumeration, forcing disconnect",
ex
scheduleReconnect(
"Unexpected error in initial device enumeration, forcing disconnect $ex"
)
retryDueToException()
}
}
}
@@ -398,12 +401,10 @@ class BluetoothInterface(val service: RadioInterfaceService, val address: String
doDiscoverServicesAndInit()
} catch (ex: BLEException) {
errormsg(
"Giving up on setting MTUs, forcing disconnect",
ex
)
shouldSetMtu = false
service.serviceScope.handledLaunch { retryDueToException() }
scheduleReconnect(
"Giving up on setting MTUs, forcing disconnect $ex"
)
}
}
else

View File

@@ -196,7 +196,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
val oldstate = state
state = newState
if (oldstate == BluetoothProfile.STATE_CONNECTED) {
info("Lost connection - aborting current work")
info("Lost connection - aborting current work: $currentWork")
/*
Supposedly this reconnect attempt happens automatically
@@ -224,8 +224,10 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
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
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 }
@@ -243,6 +245,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
// to initiate a background connection.
if (autoConnect) {
warn("Failed on non-auto connect, falling back to auto connect attempt")
closeGatt() // Close the old non-auto connection
lowLevelConnect(true)
}
}
@@ -484,6 +487,7 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
private fun lowLevelConnect(autoNow: Boolean): BluetoothGatt? {
currentConnectIsAuto = autoNow
logAssert(gatt == null)
val g = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
device.connectGatt(
@@ -663,6 +667,18 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
) = queueWriteDescriptor(c, CallbackContinuation(cb))
/** Close just the GATT device but keep our pending callbacks active */
fun closeGatt() {
gatt?.let { g ->
info("Closing our GATT connection")
gatt =
null // Clear this first so the onConnectionChange callback can ignore while we are shutting down
g.disconnect()
g.close()
}
}
/**
* Close down any existing connection, any existing calls (including async connects will be
* cancelled and you'll need to recall connect to use this againt
@@ -675,18 +691,12 @@ class SafeBluetooth(private val context: Context, private val device: BluetoothD
// Cancel any notifications - because when the device comes back it might have forgotten about us
notifyHandlers.clear()
closeGatt()
ignoreException {
// Hmm - sometimes the "Connection closing" exception comes back to us - ignore it
failAllWork(BLEException("Connection closing"))
}
gatt?.let { g ->
info("Closing our GATT connection")
gatt =
null // Clear this first so the onConnectionChange callback can ignore while we are shutting down
g.disconnect()
g.close()
}
}
/**