From d05922f716bd282c91ed14ccf4fde614bde6f5df Mon Sep 17 00:00:00 2001 From: geeksville Date: Thu, 23 Jan 2020 21:58:23 -0800 Subject: [PATCH] alas coroutines sounds great, but the implementation is buggy --- app/build.gradle | 2 + .../java/com/geeksville/mesh/MainActivity.kt | 3 +- .../geeksville/mesh/SoftwareUpdateService.kt | 338 ++++-------------- .../geeksville/mesh/SyncBluetoothDevice.kt | 127 +++++++ 4 files changed, 203 insertions(+), 267 deletions(-) create mode 100644 app/src/main/java/com/geeksville/mesh/SyncBluetoothDevice.kt diff --git a/app/build.gradle b/app/build.gradle index 3211b0a5d..d1084b97f 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -88,6 +88,8 @@ dependencies { // Add the Firebase SDK for Crashlytics. implementation 'com.google.firebase:firebase-crashlytics:17.0.0-beta01' + implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.3" + // add SDKs for any other desired Firebase products // https://firebase.google.com/docs/android/setup#available-libraries diff --git a/app/src/main/java/com/geeksville/mesh/MainActivity.kt b/app/src/main/java/com/geeksville/mesh/MainActivity.kt index 79f92359e..f34649183 100644 --- a/app/src/main/java/com/geeksville/mesh/MainActivity.kt +++ b/app/src/main/java/com/geeksville/mesh/MainActivity.kt @@ -8,6 +8,7 @@ import android.content.Intent import android.content.ServiceConnection import android.content.pm.PackageManager import android.os.Bundle +import android.os.Debug import android.os.IBinder import android.os.RemoteException import androidx.appcompat.app.AppCompatActivity @@ -108,7 +109,7 @@ class MainActivity : AppCompatActivity(), Logging { // Note: We don't want this service to die just because our activity goes away (because it is doing a software update) // So we use the application context instead of the activity SoftwareUpdateService.enqueueWork(applicationContext, - SoftwareUpdateService.scanDevicesIntent + SoftwareUpdateService.startUpdateIntent ) } }) diff --git a/app/src/main/java/com/geeksville/mesh/SoftwareUpdateService.kt b/app/src/main/java/com/geeksville/mesh/SoftwareUpdateService.kt index 560cc0091..d1c1e6a6a 100644 --- a/app/src/main/java/com/geeksville/mesh/SoftwareUpdateService.kt +++ b/app/src/main/java/com/geeksville/mesh/SoftwareUpdateService.kt @@ -10,112 +10,18 @@ import android.os.SystemClock import android.widget.Toast import androidx.core.app.JobIntentService import com.geeksville.android.Logging +import kotlinx.coroutines.* import java.io.IOException import java.io.InputStream import java.util.* import java.util.zip.CRC32 -import kotlin.coroutines.Continuation -import kotlin.coroutines.resume -import kotlin.coroutines.resumeWithException -import kotlin.coroutines.suspendCoroutine - -/** - * Uses coroutines to safely access a bluetooth GATT device with a synchronous API - * - * The BTLE API on android is dumb. You can only have one outstanding operation in flight to - * the device. If you try to do something when something is pending, the operation just returns - * false. You are expected to chain your operations from the results callbacks. - * - * This class fixes the API by using coroutines to let you safely do a series of BTLE operations. - */ -class SyncBluetoothDevice(context: Context, device: BluetoothDevice) : Logging { - - private val gattCallback = object : BluetoothGattCallback() { - override fun onServicesDiscovered(gatt: BluetoothGatt, status: Int) { - logAssert(pendingServiceDesc != null) - if (status != 0) - pendingServiceDesc!!.resumeWithException(IOException("Bluetooth status=$status")) - else - pendingServiceDesc!!.resume(Unit) - } - - override fun onCharacteristicRead( - gatt: BluetoothGatt, - characteristic: BluetoothGattCharacteristic, - status: Int - ) { - logAssert(pendingReadC != null) - if (status != 0) - pendingReadC!!.resumeWithException(IOException("Bluetooth status=$status")) - else - pendingReadC!!.resume(characteristic) - } - - override fun onCharacteristicWrite( - gatt: BluetoothGatt, - characteristic: BluetoothGattCharacteristic, - status: Int - ) { - logAssert(pendingWriteC != null) - if (status != 0) - pendingWriteC!!.resumeWithException(IOException("Bluetooth status=$status")) - else - pendingWriteC!!.resume(Unit) - } - - override fun onMtuChanged(gatt: BluetoothGatt, mtu: Int, status: Int) { - logAssert(pendingMtu != null) - if (status != 0) - pendingMtu!!.resumeWithException(IOException("Bluetooth status=$status")) - else - pendingMtu!!.resume(mtu) - } - } - - /// Users can access the GATT directly as needed - val gatt = device.connectGatt(context, true, gattCallback)!! - - private var pendingServiceDesc: Continuation? = null - private var pendingMtu: Continuation? = null - private var pendingWriteC: Continuation? = null - private var pendingReadC: Continuation? = null - - suspend fun discoverServices(c: BluetoothGattCharacteristic) = - suspendCoroutine { cont -> - pendingServiceDesc = cont - logAssert(gatt.discoverServices()) - } - - /// Returns the actual MTU size used - suspend fun requestMtu(len: Int) = suspendCoroutine { cont -> - pendingMtu = cont - logAssert(gatt.requestMtu(len)) - } - - suspend fun writeCharacteristic(c: BluetoothGattCharacteristic) = - suspendCoroutine { cont -> - pendingWriteC = cont - logAssert(gatt.writeCharacteristic(c)) - } - - suspend fun readCharacteristic(c: BluetoothGattCharacteristic) = - suspendCoroutine { cont -> - pendingReadC = cont - logAssert(gatt.readCharacteristic(c)) - } - - fun disconnect() { - gatt.disconnect() - } -} +import kotlin.coroutines.* /** * typical flow * * startScan * startUpdate - * sendNextBlock - * finishUpdate * * stopScan * @@ -123,177 +29,90 @@ class SyncBluetoothDevice(context: Context, device: BluetoothDevice) : Logging { * FIXME - broadcast when we found devices, made progress sending blocks or when the update is complete * FIXME - make the user decide to start an update on a particular device */ -class SoftwareUpdateService : JobIntentService(), Logging { +class SoftwareUpdateService : JobIntentService(), Logging, CoroutineScope by MainScope() { private val bluetoothAdapter: BluetoothAdapter by lazy(LazyThreadSafetyMode.NONE) { val bluetoothManager = getSystemService(Context.BLUETOOTH_SERVICE) as BluetoothManager bluetoothManager.adapter!! } + lateinit var device: BluetoothDevice fun startUpdate() { info("starting update") - firmwareStream = assets.open("firmware.bin") - firmwareCrc.reset() - firmwareNumSent = 0 - firmwareSize = firmwareStream.available() - // we begin by setting our MTU size as high as it can go - logAssert(updateGatt.requestMtu(512)) - } + // FIXME, is this the right scope to use? I want it to block in the job queue + launch { + val sync = SyncBluetoothDevice(this@SoftwareUpdateService, device) - // Send the next block of our file to the device - fun sendNextBlock() { + val firmwareStream = assets.open("firmware.bin") + val firmwareCrc = CRC32() + var firmwareNumSent = 0 + val firmwareSize = firmwareStream.available() - if (firmwareNumSent < firmwareSize) { - info("sending block ${firmwareNumSent * 100 / firmwareSize}%") - var blockSize = 512 - 3 // Max size MTU excluding framing + sync.connect() + sync.discoverServices() // Get our services - if (blockSize > firmwareStream.available()) - blockSize = firmwareStream.available() - val buffer = ByteArray(blockSize) + val service = sync.gatt.services.find { it.uuid == SW_UPDATE_UUID }!! - // slightly expensive to keep reallocing this buffer, but whatever - logAssert(firmwareStream.read(buffer) == blockSize) - firmwareCrc.update(buffer) + val totalSizeDesc = service.getCharacteristic(SW_UPDATE_TOTALSIZE_CHARACTER) + val dataDesc = service.getCharacteristic(SW_UPDATE_DATA_CHARACTER) + val crc32Desc = service.getCharacteristic(SW_UPDATE_CRC32_CHARACTER) + val updateResultDesc = service.getCharacteristic(SW_UPDATE_RESULT_CHARACTER) + + // we begin by setting our MTU size as high as it can go + sync.requestMtu(512) + + // Start the update by writing the # of bytes in the image + logAssert( + totalSizeDesc.setValue( + firmwareSize, + BluetoothGattCharacteristic.FORMAT_UINT32, + 0 + ) + ) + sync.writeCharacteristic(totalSizeDesc) + +// Our write completed, queue up a readback + val totalSizeReadback = sync.readCharacteristic(totalSizeDesc) + .getIntValue(BluetoothGattCharacteristic.FORMAT_UINT32, 0) + logAssert(totalSizeReadback != 0) // FIXME - handle this case + + // Send all the blocks + while (firmwareNumSent < firmwareSize) { + info("sending block ${firmwareNumSent * 100 / firmwareSize}%") + var blockSize = 512 - 3 // Max size MTU excluding framing + + if (blockSize > firmwareStream.available()) + blockSize = firmwareStream.available() + val buffer = ByteArray(blockSize) + + // slightly expensive to keep reallocing this buffer, but whatever + logAssert(firmwareStream.read(buffer) == blockSize) + firmwareCrc.update(buffer) + + // updateGatt.beginReliableWrite() + dataDesc.value = buffer + sync.writeCharacteristic(dataDesc) + firmwareNumSent += blockSize + } - // updateGatt.beginReliableWrite() - dataDesc.value = buffer - logAssert(updateGatt.writeCharacteristic(dataDesc)) - firmwareNumSent += blockSize - } else { // We have finished sending all our blocks, so post the CRC so our state machine can advance val c = firmwareCrc.value info("Sent all blocks, crc is $c") logAssert(crc32Desc.setValue(c.toInt(), BluetoothGattCharacteristic.FORMAT_UINT32, 0)) - logAssert(updateGatt.writeCharacteristic(crc32Desc)) + sync.writeCharacteristic(crc32Desc) + + // we just read the update result if !0 we have an error + val updateResult = + sync.readCharacteristic(updateResultDesc) + .getIntValue(BluetoothGattCharacteristic.FORMAT_UINT8, 0) + logAssert(updateResult == 0) // FIXME - handle this case + + // FIXME perhaps ask device to reboot } } - fun connectToDevice(device: BluetoothDevice) { - debug("Connect to $device") - - lateinit var bluetoothGatt: BluetoothGatt // late init so we can declare our callback and use this there - - //var connectionState = STATE_DISCONNECTED - - // Various callback methods defined by the BLE API. - val gattCallback = object : BluetoothGattCallback() { - - override fun onConnectionStateChange( - gatt: BluetoothGatt, - status: Int, - newState: Int - ) { - info("new bluetooth connection state $newState") - //val intentAction: String - when (newState) { - BluetoothProfile.STATE_CONNECTED -> { - //intentAction = ACTION_GATT_CONNECTED - //connectionState = STATE_CONNECTED - // broadcastUpdate(intentAction) - logAssert(bluetoothGatt.discoverServices()) - } - BluetoothProfile.STATE_DISCONNECTED -> { - //intentAction = ACTION_GATT_DISCONNECTED - //connectionState = STATE_DISCONNECTED - // broadcastUpdate(intentAction) - } - } - } - - // New services discovered - override fun onServicesDiscovered(gatt: BluetoothGatt, status: Int) { - info("onServicesDiscovered") - logAssert(status == BluetoothGatt.GATT_SUCCESS) - - // broadcastUpdate(ACTION_GATT_SERVICES_DISCOVERED) - - val service = gatt.services.find { it.uuid == SW_UPDATE_UUID } - logAssert(service != null) - - // FIXME instead of slamming in the target device here, instead make it a param for startUpdate - updateService = service!! - totalSizeDesc = service.getCharacteristic(SW_UPDATE_TOTALSIZE_CHARACTER) - dataDesc = service.getCharacteristic(SW_UPDATE_DATA_CHARACTER) - crc32Desc = service.getCharacteristic(SW_UPDATE_CRC32_CHARACTER) - updateResultDesc = service.getCharacteristic(SW_UPDATE_RESULT_CHARACTER) - - // FIXME instead of keeping the connection open, make start update just reconnect (needed once user can choose devices) - updateGatt = bluetoothGatt - enqueueWork(this@SoftwareUpdateService, startUpdateIntent) - } - - override fun onMtuChanged(gatt: BluetoothGatt, mtu: Int, status: Int) { - debug("onMtuChanged $mtu") - logAssert(status == BluetoothGatt.GATT_SUCCESS) - - // Start the update by writing the # of bytes in the image - logAssert( - totalSizeDesc.setValue( - firmwareSize, - BluetoothGattCharacteristic.FORMAT_UINT32, - 0 - ) - ) - logAssert(updateGatt.writeCharacteristic(totalSizeDesc)) - } - - // Result of a characteristic read operation - override fun onCharacteristicRead( - gatt: BluetoothGatt, - characteristic: BluetoothGattCharacteristic, - status: Int - ) { - debug("onCharacteristicRead $characteristic") - logAssert(status == BluetoothGatt.GATT_SUCCESS) - - if (characteristic == totalSizeDesc) { - // Our read of this has completed, either fail or continue updating - val readvalue = - characteristic.getIntValue(BluetoothGattCharacteristic.FORMAT_UINT32, 0) - logAssert(readvalue != 0) // FIXME - handle this case - enqueueWork(this@SoftwareUpdateService, sendNextBlockIntent) - } else if (characteristic == updateResultDesc) { - // we just read the update result if !0 we have an error - val readvalue = - characteristic.getIntValue(BluetoothGattCharacteristic.FORMAT_UINT8, 0) - logAssert(readvalue == 0) // FIXME - handle this case - } else { - warn("Unexpected read: $characteristic") - } - - // broadcastUpdate(ACTION_DATA_AVAILABLE, characteristic) - } - - override fun onCharacteristicWrite( - gatt: BluetoothGatt?, - characteristic: BluetoothGattCharacteristic?, - status: Int - ) { - debug("onCharacteristicWrite $characteristic") - logAssert(status == BluetoothGatt.GATT_SUCCESS) - - if (characteristic == totalSizeDesc) { - // Our write completed, queue up a readback - logAssert(updateGatt.readCharacteristic(totalSizeDesc)) - } else if (characteristic == dataDesc) { - enqueueWork(this@SoftwareUpdateService, sendNextBlockIntent) - } else if (characteristic == crc32Desc) { - // Now that we wrote the CRC, we should read the result code - logAssert(updateGatt.readCharacteristic(updateResultDesc)) - } else { - warn("Unexpected write: $characteristic") - } - } - } - bluetoothGatt = - device.connectGatt(this@SoftwareUpdateService.applicationContext, true, gattCallback)!! - toast("Connected to $device") - - // too early to do this here - // logAssert(bluetoothGatt.discoverServices()) - } private val scanCallback = object : ScanCallback() { override fun onScanFailed(errorCode: Int) { @@ -314,13 +133,13 @@ class SoftwareUpdateService : JobIntentService(), Logging { // We don't need any more results now bluetoothAdapter.bluetoothLeScanner.stopScan(this) - connectToDevice(result.device) + device = result.device } } // Until my race condition with scanning is fixed fun connectToTestDevice() { - connectToDevice(bluetoothAdapter.getRemoteDevice("B4:E6:2D:EA:32:B7")) + device = bluetoothAdapter.getRemoteDevice("B4:E6:2D:EA:32:B7") } private fun scanLeDevice(enable: Boolean) { @@ -359,9 +178,11 @@ class SoftwareUpdateService : JobIntentService(), Logging { // holding a wake lock for us at this point, so we can just go. debug("Executing work: $intent") when (intent.action) { - scanDevicesIntent.action -> connectToTestDevice() // FIXME scanLeDevice(true) - startUpdateIntent.action -> startUpdate() - sendNextBlockIntent.action -> sendNextBlock() + scanDevicesIntent.action -> scanLeDevice(true) + startUpdateIntent.action -> { + connectToTestDevice() // FIXME, pass in as an intent arg instead + startUpdate() + } else -> logAssert(false) } @@ -371,6 +192,7 @@ class SoftwareUpdateService : JobIntentService(), Logging { } override fun onDestroy() { + cancel() // Stop the CoroutineScope super.onDestroy() // toast("All work complete") } @@ -391,8 +213,6 @@ class SoftwareUpdateService : JobIntentService(), Logging { val scanDevicesIntent = Intent("com.geeksville.com.geeeksville.mesh.SCAN_DEVICES") val startUpdateIntent = Intent("com.geeksville.com.geeeksville.mesh.START_UPDATE") - private val sendNextBlockIntent = - Intent("com.geeksville.com.geeeksville.mesh.SEND_NEXT_BLOCK") private const val SCAN_PERIOD: Long = 10000 @@ -412,20 +232,6 @@ class SoftwareUpdateService : JobIntentService(), Logging { private val SW_UPDATE_RESULT_CHARACTER = UUID.fromString("5e134862-7411-4424-ac4a-210937432c77") // read|notify result code, readable but will notify when the OTA operation completes - // FIXME - this is state that really more properly goes with the serice instance, but - // it can go away if our work queue gets empty. So we keep it here instead. Not sure - // if there is a better approach? - lateinit var updateGatt: BluetoothGatt // the gatt api used to talk to our device - lateinit var updateService: BluetoothGattService // The service we are currently talking to to do the update - lateinit var totalSizeDesc: BluetoothGattCharacteristic - lateinit var dataDesc: BluetoothGattCharacteristic - lateinit var crc32Desc: BluetoothGattCharacteristic - lateinit var updateResultDesc: BluetoothGattCharacteristic - lateinit var firmwareStream: InputStream - val firmwareCrc = CRC32() - var firmwareNumSent = 0 - var firmwareSize = 0 - /** * Convenience method for enqueuing work in to this service. */ diff --git a/app/src/main/java/com/geeksville/mesh/SyncBluetoothDevice.kt b/app/src/main/java/com/geeksville/mesh/SyncBluetoothDevice.kt new file mode 100644 index 000000000..d3d7edc61 --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/SyncBluetoothDevice.kt @@ -0,0 +1,127 @@ +package com.geeksville.mesh + +import android.bluetooth.* +import android.content.Context +import com.geeksville.android.Logging +import java.io.IOException +import kotlin.coroutines.Continuation +import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException +import kotlin.coroutines.suspendCoroutine + + +/** + * Uses coroutines to safely access a bluetooth GATT device with a synchronous API + * + * The BTLE API on android is dumb. You can only have one outstanding operation in flight to + * the device. If you try to do something when something is pending, the operation just returns + * false. You are expected to chain your operations from the results callbacks. + * + * This class fixes the API by using coroutines to let you safely do a series of BTLE operations. + */ +class SyncBluetoothDevice(private val context: Context, private val device: BluetoothDevice) : Logging { + + private var pendingServiceDesc: Continuation? = null + private var pendingMtu: Continuation? = null + private var pendingWriteC: Continuation? = null + private var pendingReadC: Continuation? = null + private var pendingConnect: Continuation? = null + + private val gattCallback = object : BluetoothGattCallback() { + + override fun onConnectionStateChange( + gatt: BluetoothGatt, + status: Int, + newState: Int + ) { + info("new bluetooth connection state $newState") + when (newState) { + BluetoothProfile.STATE_CONNECTED -> { + if(pendingConnect != null) { // If someone was waiting to connect unblock them + pendingConnect!!.resume(Unit) + pendingConnect = null + } + } + BluetoothProfile.STATE_DISCONNECTED -> { + TODO("handle loss of connection") + } + } + } + + override fun onServicesDiscovered(gatt: BluetoothGatt, status: Int) { + if (status != 0) + pendingServiceDesc!!.resumeWithException(IOException("Bluetooth status=$status")) + else + pendingServiceDesc!!.resume(Unit) + pendingServiceDesc = null + } + + override fun onCharacteristicRead( + gatt: BluetoothGatt, + characteristic: BluetoothGattCharacteristic, + status: Int + ) { + if (status != 0) + pendingReadC!!.resumeWithException(IOException("Bluetooth status=$status")) + else + pendingReadC!!.resume(characteristic) + pendingReadC = null + } + + override fun onCharacteristicWrite( + gatt: BluetoothGatt, + characteristic: BluetoothGattCharacteristic, + status: Int + ) { + if (status != 0) + pendingWriteC!!.resumeWithException(IOException("Bluetooth status=$status")) + else + pendingWriteC!!.resume(Unit) + } + + override fun onMtuChanged(gatt: BluetoothGatt, mtu: Int, status: Int) { + if (status != 0) + pendingMtu!!.resumeWithException(IOException("Bluetooth status=$status")) + else + pendingMtu!!.resume(mtu) + pendingMtu = null + } + } + + /// Users can access the GATT directly as needed + lateinit var gatt: BluetoothGatt + + suspend fun connect() = + suspendCoroutine { cont -> + pendingConnect = cont + gatt = device.connectGatt(context, false, gattCallback)!! + } + + suspend fun discoverServices() = + suspendCoroutine { cont -> + pendingServiceDesc = cont + logAssert(gatt.discoverServices()) + } + + /// Returns the actual MTU size used + suspend fun requestMtu(len: Int): kotlin.Int = suspendCoroutine { cont -> + pendingMtu = cont + logAssert(gatt.requestMtu(len)) + } + + suspend fun writeCharacteristic(c: BluetoothGattCharacteristic) = + suspendCoroutine { cont -> + pendingWriteC = cont + logAssert(gatt.writeCharacteristic(c)) + } + + suspend fun readCharacteristic(c: BluetoothGattCharacteristic) = + suspendCoroutine { cont -> + pendingReadC = cont + logAssert(gatt.readCharacteristic(c)) + } + + fun disconnect() { + gatt.disconnect() + } +}