From 3a8fa6803df3bcf443c9b889a2c4653acbc58df2 Mon Sep 17 00:00:00 2001 From: Travis Wyatt Date: Tue, 15 Sep 2020 10:34:46 -0700 Subject: [PATCH] Organize exceptions thrown (#84) --- core/api/core.api | 11 ++-- core/src/main/java/device/CoroutinesDevice.kt | 4 +- core/src/main/java/gatt/CoroutinesGatt.kt | 7 ++- core/src/main/java/gatt/GattConnection.kt | 9 ++- .../test/java/device/CoroutinesDeviceTest.kt | 6 +- keep-alive/api/keep-alive.api | 2 +- keep-alive/src/main/java/KeepAliveGatt.kt | 5 +- throw/src/main/java/GattOrThrow.kt | 63 ++++++++++--------- .../java/android/BluetoothDeviceOrThrow.kt | 4 +- throw/src/test/java/GattTest.kt | 30 ++++----- 10 files changed, 79 insertions(+), 62 deletions(-) diff --git a/core/api/core.api b/core/api/core.api index a1beeb5..2bd5151 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -109,10 +109,6 @@ public abstract interface class com/juul/able/gatt/GattConnection { public abstract fun getOnConnectionStateChange ()Lkotlinx/coroutines/flow/Flow; } -public final class com/juul/able/gatt/GattErrorStatusException : java/io/IOException { - public final fun getEvent ()Lcom/juul/able/gatt/OnConnectionStateChange; -} - public abstract interface class com/juul/able/gatt/GattIo { public abstract fun discoverServices (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public abstract fun getOnCharacteristicChanged ()Lkotlinx/coroutines/flow/Flow; @@ -130,6 +126,11 @@ public final class com/juul/able/gatt/GattIoKt { public static final fun writeCharacteristic (Lcom/juul/able/gatt/GattIo;Landroid/bluetooth/BluetoothGattCharacteristic;[BLkotlin/coroutines/Continuation;)Ljava/lang/Object; } +public final class com/juul/able/gatt/GattStatusException : java/io/IOException { + public fun (ILjava/lang/String;)V + public fun (Ljava/lang/String;)V +} + public abstract interface class com/juul/able/gatt/HasGattStatus { public abstract fun getStatus ()I } @@ -242,7 +243,7 @@ public final class com/juul/able/gatt/OnReadRemoteRssi : com/juul/able/gatt/HasG public fun toString ()Ljava/lang/String; } -public final class com/juul/able/gatt/OutOfOrderGattCallbackException : java/io/IOException { +public final class com/juul/able/gatt/OutOfOrderGattCallbackException : java/lang/IllegalStateException { } public final class com/juul/able/logger/AndroidLogger : com/juul/able/logger/Logger { diff --git a/core/src/main/java/device/CoroutinesDevice.kt b/core/src/main/java/device/CoroutinesDevice.kt index c9c6a6e..eed41f3 100644 --- a/core/src/main/java/device/CoroutinesDevice.kt +++ b/core/src/main/java/device/CoroutinesDevice.kt @@ -17,7 +17,7 @@ import com.juul.able.gatt.ConnectionLostException import com.juul.able.gatt.CoroutinesGatt import com.juul.able.gatt.GattCallback import com.juul.able.gatt.GattConnection -import com.juul.able.gatt.GattErrorStatusException +import com.juul.able.gatt.GattStatusException import com.juul.able.gatt.asGattConnectionStateString import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.first @@ -60,7 +60,7 @@ internal class CoroutinesDevice( onConnectionStateChange .onEach { event -> Able.verbose { "← Device $device received $event while waiting for connection" } - if (event.status != GATT_SUCCESS) throw GattErrorStatusException(event) + if (event.status != GATT_SUCCESS) throw GattStatusException(event.toString()) if (event.newState == STATE_DISCONNECTED) throw ConnectionLostException() } .first { (_, newState) -> newState == STATE_CONNECTED } diff --git a/core/src/main/java/gatt/CoroutinesGatt.kt b/core/src/main/java/gatt/CoroutinesGatt.kt index 9e6a321..5a9e7c0 100644 --- a/core/src/main/java/gatt/CoroutinesGatt.kt +++ b/core/src/main/java/gatt/CoroutinesGatt.kt @@ -12,7 +12,6 @@ import android.bluetooth.BluetoothGattService import android.bluetooth.BluetoothProfile.STATE_DISCONNECTED import android.os.RemoteException import com.juul.able.Able -import java.io.IOException import java.util.UUID import kotlinx.coroutines.CancellationException import kotlinx.coroutines.ExecutorCoroutineDispatcher @@ -24,7 +23,9 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext -class OutOfOrderGattCallbackException internal constructor(message: String) : IOException(message) +class OutOfOrderGattCallbackException internal constructor( + message: String +) : IllegalStateException(message) class CoroutinesGatt internal constructor( private val bluetoothGatt: BluetoothGatt, @@ -143,7 +144,7 @@ class CoroutinesGatt internal constructor( } val (status, newState) = event if (status != GATT_SUCCESS && newState != STATE_DISCONNECTED) - throw GattErrorStatusException(event) + throw GattStatusException(event.toString()) } .first { (_, newState) -> newState == STATE_DISCONNECTED } .also { (_, newState) -> diff --git a/core/src/main/java/gatt/GattConnection.kt b/core/src/main/java/gatt/GattConnection.kt index e6c3aa0..faceb53 100644 --- a/core/src/main/java/gatt/GattConnection.kt +++ b/core/src/main/java/gatt/GattConnection.kt @@ -45,9 +45,12 @@ interface GattConnection { suspend fun disconnect(): Unit } -class GattErrorStatusException internal constructor( - val event: OnConnectionStateChange -) : IOException("Received $event") +class GattStatusException(message: String?) : IOException(message) { + constructor( + status: GattStatus, + prefix: String + ) : this("$prefix failed with status ${status.asGattStatusString()}") +} class ConnectionLostException internal constructor( message: String? = null, diff --git a/core/src/test/java/device/CoroutinesDeviceTest.kt b/core/src/test/java/device/CoroutinesDeviceTest.kt index 336777d..3cdf539 100644 --- a/core/src/test/java/device/CoroutinesDeviceTest.kt +++ b/core/src/test/java/device/CoroutinesDeviceTest.kt @@ -19,7 +19,7 @@ import com.juul.able.device.CoroutinesDevice import com.juul.able.gatt.ConnectionLostException import com.juul.able.gatt.GATT_CONN_CANCEL import com.juul.able.gatt.GattCallback -import com.juul.able.gatt.GattErrorStatusException +import com.juul.able.gatt.GattStatusException import com.juul.able.gatt.OnConnectionStateChange import com.juul.able.test.logger.ConsoleLoggerTestRule import io.mockk.every @@ -64,8 +64,8 @@ class CoroutinesDeviceTest { val failure = device.connectGatt(mockk()) as Failure.Connection assertEquals( - expected = OnConnectionStateChange(GATT_CONN_CANCEL, STATE_CONNECTED), - actual = (failure.cause as GattErrorStatusException).event + expected = OnConnectionStateChange(GATT_CONN_CANCEL, STATE_CONNECTED).toString(), + actual = (failure.cause as GattStatusException).message ) verify(exactly = 1) { bluetoothGatt.close() } } diff --git a/keep-alive/api/keep-alive.api b/keep-alive/api/keep-alive.api index 8017c65..e44a3d1 100644 --- a/keep-alive/api/keep-alive.api +++ b/keep-alive/api/keep-alive.api @@ -75,7 +75,7 @@ public final class com/juul/able/keepalive/KeepAliveGattKt { public static final fun keepAliveGatt (Lkotlinx/coroutines/CoroutineScope;Landroid/content/Context;Landroid/bluetooth/BluetoothDevice;J)Lcom/juul/able/keepalive/KeepAliveGatt; } -public final class com/juul/able/keepalive/NotReadyException : java/lang/IllegalStateException { +public final class com/juul/able/keepalive/NotReadyException : java/io/IOException { } public abstract class com/juul/able/keepalive/State { diff --git a/keep-alive/src/main/java/KeepAliveGatt.kt b/keep-alive/src/main/java/KeepAliveGatt.kt index 8b26a01..3aa2618 100644 --- a/keep-alive/src/main/java/KeepAliveGatt.kt +++ b/keep-alive/src/main/java/KeepAliveGatt.kt @@ -25,6 +25,7 @@ import com.juul.able.gatt.OnDescriptorWrite import com.juul.able.gatt.OnMtuChanged import com.juul.able.gatt.OnReadRemoteRssi import com.juul.able.gatt.WriteType +import java.io.IOException import java.util.UUID import java.util.concurrent.atomic.AtomicBoolean import kotlin.coroutines.CoroutineContext @@ -53,7 +54,9 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeoutOrNull -class NotReadyException internal constructor(message: String) : IllegalStateException(message) +class NotReadyException internal constructor( + message: String +) : IOException(message) fun CoroutineScope.keepAliveGatt( androidContext: Context, diff --git a/throw/src/main/java/GattOrThrow.kt b/throw/src/main/java/GattOrThrow.kt index 097e876..12dfd8a 100644 --- a/throw/src/main/java/GattOrThrow.kt +++ b/throw/src/main/java/GattOrThrow.kt @@ -7,62 +7,64 @@ package com.juul.able.throwable import android.bluetooth.BluetoothGatt.GATT_SUCCESS import android.bluetooth.BluetoothGattCharacteristic import android.bluetooth.BluetoothGattDescriptor +import android.os.RemoteException import com.juul.able.gatt.GattIo +import com.juul.able.gatt.GattStatus +import com.juul.able.gatt.GattStatusException import com.juul.able.gatt.WriteType /** - * @throws [IllegalStateException] if [GattIo.discoverServices] call does not return [GATT_SUCCESS]. + * @throws [GattStatusException] if [GattIo.discoverServices] call does not return [GATT_SUCCESS]. */ suspend fun GattIo.discoverServicesOrThrow() { - discoverServices() - .also { status -> - check(status == GATT_SUCCESS) { - "Service discovery failed with gatt status $status." - } - } + discoverServices().also { status -> + check(status) { "Discover services" } + } } /** - * @throws [IllegalStateException] if [GattIo.readRemoteRssi] call does not return [GATT_SUCCESS]. + * @throws [GattStatusException] if [GattIo.readRemoteRssi] call does not return [GATT_SUCCESS]. */ suspend fun GattIo.readRemoteRssiOrThrow(): Int { return readRemoteRssi() .also { (_, status) -> - check(status == GATT_SUCCESS) { - "Service discovery failed with gatt status $status." - } - }.rssi + check(status) { "Read remote RSSI" } + } + .rssi } /** - * @throws [IllegalStateException] if [GattIo.readCharacteristic] call does not return [GATT_SUCCESS]. + * @throws [GattStatusException] if [GattIo.readCharacteristic] call does not return [GATT_SUCCESS]. */ suspend fun GattIo.readCharacteristicOrThrow( characteristic: BluetoothGattCharacteristic ): ByteArray { return readCharacteristic(characteristic) .also { (_, _, status) -> - check(status == GATT_SUCCESS) { - "Reading characteristic ${characteristic.uuid} failed with status $status." - } - }.value + check(status) { "Read characteristic ${characteristic.uuid}" } + } + .value } /** - * @throws [IllegalStateException] if [GattIo.setCharacteristicNotification] call returns `false`. + * @throws [RemoteException] if [GattIo.setCharacteristicNotification] call returns `false`. */ fun GattIo.setCharacteristicNotificationOrThrow( characteristic: BluetoothGattCharacteristic, enable: Boolean ) { setCharacteristicNotification(characteristic, enable) - .also { - check(it) { "Setting characteristic notifications to $enable failed." } + .also { successful -> + if (!successful) { + val uuid = characteristic.uuid + val message = "Setting characteristic $uuid notifications to $enable failed" + throw RemoteException(message) + } } } /** - * @throws [IllegalStateException] if [GattIo.writeCharacteristic] call does not return [GATT_SUCCESS]. + * @throws [GattStatusException] if [GattIo.writeCharacteristic] call does not return [GATT_SUCCESS]. */ suspend fun GattIo.writeCharacteristicOrThrow( characteristic: BluetoothGattCharacteristic, @@ -71,14 +73,12 @@ suspend fun GattIo.writeCharacteristicOrThrow( ) { writeCharacteristic(characteristic, value, writeType) .also { (_, status) -> - check(status == GATT_SUCCESS) { - "Writing characteristic ${characteristic.uuid} failed with status $status." - } + check(status) { "Write characteristic ${characteristic.uuid}" } } } /** - * @throws [IllegalStateException] if [GattIo.writeDescriptor] call does not return [GATT_SUCCESS]. + * @throws [GattStatusException] if [GattIo.writeDescriptor] call does not return [GATT_SUCCESS]. */ suspend fun GattIo.writeDescriptorOrThrow( descriptor: BluetoothGattDescriptor, @@ -86,18 +86,25 @@ suspend fun GattIo.writeDescriptorOrThrow( ) { writeDescriptor(descriptor, value) .also { (_, status) -> - check(status == GATT_SUCCESS) { "Descriptor write failed with status $status." } + check(status) { "Write descriptor ${descriptor.uuid}" } } } /** - * @throws [IllegalStateException] if [GattIo.requestMtu] call does not return [GATT_SUCCESS]. + * @throws [GattStatusException] if [GattIo.requestMtu] call does not return [GATT_SUCCESS]. */ suspend fun GattIo.requestMtuOrThrow( mtu: Int ): Int { return requestMtu(mtu) .also { (_, status) -> - check(status == GATT_SUCCESS) { "Request MTU of $mtu failed with status $status." } + check(status) { "Request MTU of $mtu" } }.mtu } + +private fun check(status: GattStatus, lazyPrefix: () -> Any) { + if (status != GATT_SUCCESS) { + val prefix = lazyPrefix.invoke() + throw GattStatusException(status, prefix.toString()) + } +} diff --git a/throw/src/main/java/android/BluetoothDeviceOrThrow.kt b/throw/src/main/java/android/BluetoothDeviceOrThrow.kt index 4753b36..97445f6 100644 --- a/throw/src/main/java/android/BluetoothDeviceOrThrow.kt +++ b/throw/src/main/java/android/BluetoothDeviceOrThrow.kt @@ -14,11 +14,11 @@ import com.juul.able.device.ConnectGattResult.Failure import com.juul.able.device.ConnectGattResult.Success import com.juul.able.gatt.ConnectionLostException import com.juul.able.gatt.Gatt -import com.juul.able.gatt.GattErrorStatusException +import com.juul.able.gatt.GattStatusException /** * @throws RemoteException if underlying [BluetoothDevice.connectGatt] returns `null`. - * @throws GattErrorStatusException if non-[GATT_SUCCESS] status is received during connection process. + * @throws GattStatusException if non-[GATT_SUCCESS] status is received during connection process. * @throws ConnectionLostException if [STATE_DISCONNECTED] is received during connection process. */ suspend fun BluetoothDevice.connectGattOrThrow( diff --git a/throw/src/test/java/GattTest.kt b/throw/src/test/java/GattTest.kt index 02c2c84..5aad485 100644 --- a/throw/src/test/java/GattTest.kt +++ b/throw/src/test/java/GattTest.kt @@ -10,7 +10,9 @@ import android.bluetooth.BluetoothGatt.GATT_SUCCESS import android.bluetooth.BluetoothGatt.GATT_WRITE_NOT_PERMITTED import android.bluetooth.BluetoothGattCharacteristic import android.bluetooth.BluetoothGattDescriptor +import android.os.RemoteException import com.juul.able.gatt.Gatt +import com.juul.able.gatt.GattStatusException import com.juul.able.gatt.OnCharacteristicRead import com.juul.able.gatt.OnCharacteristicWrite import com.juul.able.gatt.OnDescriptorWrite @@ -37,12 +39,12 @@ private val testUuid = UUID.fromString("01234567-89ab-cdef-0123-456789abcdef") class GattTest { @Test - fun `discoverServicesOrThrow throws IllegalStateException for non-GATT_SUCCESS response`() { + fun `discoverServicesOrThrow throws GattStatusException for non-GATT_SUCCESS response`() { val gatt = mockk { coEvery { discoverServices() } returns GATT_FAILURE } - assertFailsWith { + assertFailsWith { runBlocking { gatt.discoverServicesOrThrow() } @@ -50,12 +52,12 @@ class GattTest { } @Test - fun `readRemoteRssiOrThrow throws IllegalStateException for non-GATT_SUCCESS response`() { + fun `readRemoteRssiOrThrow throws GattStatusException for non-GATT_SUCCESS response`() { val gatt = mockk { coEvery { readRemoteRssi() } returns OnReadRemoteRssi(rssi = 0, status = GATT_FAILURE) } - assertFailsWith { + assertFailsWith { runBlocking { gatt.readRemoteRssiOrThrow() } @@ -79,7 +81,7 @@ class GattTest { } @Test - fun `readCharacteristicOrThrow throws IllegalStateException for non-GATT_SUCCESS response`() { + fun `readCharacteristicOrThrow throws GattStatusException for non-GATT_SUCCESS response`() { val characteristic = mockk { every { uuid } returns testUuid } @@ -91,7 +93,7 @@ class GattTest { ) } - assertFailsWith { + assertFailsWith { runBlocking { gatt.readCharacteristicOrThrow(characteristic) } @@ -123,12 +125,12 @@ class GattTest { } @Test - fun `setCharacteristicNotificationOrThrow throws IllegalStateException for false return`() { + fun `setCharacteristicNotificationOrThrow throws RemoteException for false return`() { val gatt = mockk { coEvery { setCharacteristicNotification(any(), any()) } returns false } - assertFailsWith { + assertFailsWith { val characteristic = mockk { every { uuid } returns testUuid } @@ -137,7 +139,7 @@ class GattTest { } @Test - fun `writeCharacteristicOrThrow throws IllegalStateException for non-GATT_SUCCESS response`() { + fun `writeCharacteristicOrThrow throws GattStatusException for non-GATT_SUCCESS response`() { val characteristic = mockk { every { uuid } returns testUuid } @@ -148,7 +150,7 @@ class GattTest { ) } - assertFailsWith { + assertFailsWith { runBlocking { gatt.writeCharacteristicOrThrow(characteristic, byteArrayOf()) } @@ -156,7 +158,7 @@ class GattTest { } @Test - fun `writeDescriptorOrThrow throws IllegalStateException for non-GATT_SUCCESS response`() { + fun `writeDescriptorOrThrow throws GattStatusException for non-GATT_SUCCESS response`() { val descriptor = mockk { every { uuid } returns testUuid } @@ -167,7 +169,7 @@ class GattTest { ) } - assertFailsWith { + assertFailsWith { runBlocking { gatt.writeDescriptorOrThrow(descriptor, byteArrayOf()) } @@ -175,7 +177,7 @@ class GattTest { } @Test - fun `requestMtuOrThrow throws IllegalStateException for non-GATT_SUCCESS response`() { + fun `requestMtuOrThrow throws GattStatusException for non-GATT_SUCCESS response`() { val gatt = mockk { coEvery { requestMtu(any()) } returns OnMtuChanged( mtu = 23, @@ -183,7 +185,7 @@ class GattTest { ) } - assertFailsWith { + assertFailsWith { runBlocking { gatt.requestMtuOrThrow(1024) }