Skip to content

Commit c855f36

Browse files
philips77philips77
authored
Merge pull request #38 from NordicSemiconductor/bugfix/cancellation
Bugfix: Disconnection and cancellaton issues fixed
2 parents 2673c29 + bde8d0e commit c855f36

File tree

14 files changed

+78
-61
lines changed

14 files changed

+78
-61
lines changed

‎app/src/main/java/no/nordicsemi/memfault/di/MemfaultModule.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import dagger.Provides
3838
import dagger.hilt.InstallIn
3939
import dagger.hilt.android.components.ViewModelComponent
4040
import dagger.hilt.android.qualifiers.ApplicationContext
41-
import dagger.hilt.components.SingletonComponent
4241

4342
@Module
4443
@InstallIn(ViewModelComponent::class)

‎lib/observability/src/main/java/no/nordicsemi/memfault/observability/MemfaultDiagnosticsManager.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import kotlinx.coroutines.flow.StateFlow
3939
import no.nordicsemi.kotlin.ble.client.android.CentralManager
4040
import no.nordicsemi.kotlin.ble.client.android.Peripheral
4141
import no.nordicsemi.kotlin.ble.client.android.native
42-
import no.nordicsemi.memfault.observability.internal.MemfaultScope
42+
import no.nordicsemi.memfault.observability.internal.Scope
4343

4444
/**
4545
* Class responsible for managing connection with the remote IoT device which supports
@@ -67,7 +67,7 @@ interface MemfaultDiagnosticsManager {
6767
/**
6868
* Function used to connect to the selected Bluetooth LE peripheral.
6969
*
70-
* The peripheral must support Memfault Diagnostic Service.
70+
* The peripheral must support Monitoring & Diagnostic Service.
7171
*
7272
* Chunks upload will start immediately after establishing the connection.
7373
*
@@ -84,7 +84,7 @@ interface MemfaultDiagnosticsManager {
8484
/**
8585
* Function used to connect to the selected Bluetooth LE peripheral.
8686
*
87-
* The peripheral must support Memfault Diagnostic Service.
87+
* The peripheral must support Monitoring & Diagnostic Service.
8888
*
8989
* Chunks upload will start immediately after establishing the connection.
9090
*
@@ -93,7 +93,7 @@ interface MemfaultDiagnosticsManager {
9393
* @throws IllegalStateException if the manager is already connected to a peripheral.
9494
*/
9595
fun connect(context: Context, device: BluetoothDevice) {
96-
val centralManager = CentralManager.Factory.native(context, MemfaultScope)
96+
val centralManager = CentralManager.Factory.native(context, Scope)
9797
val peripheral = centralManager.getPeripheralById(device.address)!!
9898
connect(peripheral, centralManager)
9999
}

‎lib/observability/src/main/java/no/nordicsemi/memfault/observability/MemfaultDiagnosticsManagerImpl.kt‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -31,26 +31,28 @@
3131

3232
package no.nordicsemi.memfault.observability
3333

34+
import android.Manifest
3435
import android.content.Context
3536
import androidx.annotation.RequiresPermission
3637
import kotlinx.coroutines.Job
3738
import kotlinx.coroutines.NonCancellable
3839
import kotlinx.coroutines.awaitCancellation
40+
import kotlinx.coroutines.cancel
3941
import kotlinx.coroutines.flow.MutableStateFlow
4042
import kotlinx.coroutines.flow.StateFlow
4143
import kotlinx.coroutines.flow.asStateFlow
4244
import kotlinx.coroutines.flow.debounce
45+
import kotlinx.coroutines.flow.drop
4346
import kotlinx.coroutines.flow.launchIn
44-
import kotlinx.coroutines.flow.onCompletion
4547
import kotlinx.coroutines.flow.onEach
4648
import kotlinx.coroutines.launch
4749
import kotlinx.coroutines.withContext
4850
import no.nordicsemi.kotlin.ble.client.android.CentralManager
4951
import no.nordicsemi.kotlin.ble.client.android.Peripheral
50-
import no.nordicsemi.memfault.observability.bluetooth.MemfaultDiagnosticsService
5152
import no.nordicsemi.memfault.observability.bluetooth.DeviceState
53+
import no.nordicsemi.memfault.observability.bluetooth.MemfaultDiagnosticsService
5254
import no.nordicsemi.memfault.observability.data.PersistentChunkQueue
53-
import no.nordicsemi.memfault.observability.internal.MemfaultScope
55+
import no.nordicsemi.memfault.observability.internal.Scope
5456
import no.nordicsemi.memfault.observability.internet.MemfaultCloudManager
5557
import kotlin.time.Duration.Companion.milliseconds
5658

@@ -63,25 +65,22 @@ internal class MemfaultDiagnosticsManagerImpl(
6365
private val _state = MutableStateFlow(MemfaultState())
6466
override val state: StateFlow<MemfaultState> = _state.asStateFlow()
6567

66-
private var bleManager: MemfaultDiagnosticsService? = null
68+
private var service: MemfaultDiagnosticsService? = null
6769
private var chunkQueue: PersistentChunkQueue? = null
6870
private var uploadManager: MemfaultCloudManager? = null
69-
private var job: Job? = null
7071

71-
@RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE)
72+
@RequiresPermission(Manifest.permission.ACCESS_NETWORK_STATE)
7273
override fun connect(peripheral: Peripheral, centralManager: CentralManager) {
73-
check(job == null) { "Already connected to a peripheral" }
74-
75-
// Start the manager in a new scope.
76-
job = MemfaultScope.launch {
77-
val scope = this
74+
check(service == null) { "Already connected to a peripheral" }
7875

79-
// Set up the manager that will collect diagnostic chunks from the device.
80-
bleManager = MemfaultDiagnosticsService(centralManager, peripheral, scope)
76+
// Set up the collector for observable chunks from the device.
77+
Scope.launch {
78+
service = MemfaultDiagnosticsService(centralManager, peripheral, this)
8179
.apply {
82-
// Collect the state of the BLE manager and update the state flow.
8380
var connection: Job? = null
81+
// Collect the state of the device and update the state flow.
8482
state
83+
.drop(1)
8584
.onEach { state ->
8685
_state.value = _state.value.copy(deviceStatus = state)
8786

@@ -90,7 +89,7 @@ internal class MemfaultDiagnosticsManagerImpl(
9089
"Connection scope should be null or cancelled when the config is received"
9190
}
9291

93-
connection = scope.launch {
92+
connection = launch {
9493
chunkQueue = PersistentChunkQueue(
9594
context = context,
9695
deviceId = state.config.deviceId
@@ -114,33 +113,34 @@ internal class MemfaultDiagnosticsManagerImpl(
114113
manager.uploadChunks()
115114
}
116115

117-
try { awaitCancellation()}
118-
finally {
119-
uploadManager?.uploadChunks()
120-
uploadManager?.close()
121-
uploadManager = null
116+
try {
117+
awaitCancellation()
118+
} finally {
119+
// Manager is closing. Flush any remaining chunks.
120+
withContext(NonCancellable) {
121+
uploadManager?.uploadChunks()
122+
uploadManager?.close()
123+
uploadManager = null
124+
}
125+
126+
// Remove all uploaded chunks from the queue.
127+
chunkQueue?.deleteUploaded()
122128
chunkQueue = null
123129
}
124130
}
125-
} else {
131+
}
132+
if (state is DeviceState.Connecting) {
126133
// Otherwise, the device must have been disconnected.
127134
connection?.cancel()
128135
connection = null
129136
}
130-
}
131-
.onCompletion {
132-
// Manager is closing. Clean up and remove all uploaded chunks from the queue.
133-
MemfaultScope.launch {
134-
chunkQueue?.deleteUploaded()
137+
if (state is DeviceState.Disconnected) {
138+
cancel()
135139
}
136-
137-
// Cancel the connection job if it is still active.
138-
connection?.cancel()
139-
connection = null
140140
}
141-
.launchIn(scope)
141+
.launchIn(this@launch)
142142

143-
// Collect the chunks received from the BLE manager and upload them.
143+
// Collect the chunks received from the device and upload them to the cloud.
144144
chunks
145145
.onEach {
146146
// Mind, that that has to be called from a non-main Dispatcher
@@ -154,24 +154,24 @@ internal class MemfaultDiagnosticsManagerImpl(
154154
.onEach {
155155
uploadManager?.uploadChunks()
156156
}
157-
.launchIn(scope)
157+
.launchIn(this@launch)
158158
}
159159

160-
// Start the Bluetooth LE manager to connect to the device and start receiving data.
161-
bleManager?.start()
160+
// Start the MDS service handler to connect to the device and start receiving data.
161+
service?.start()
162162

163163
// Wait for disconnection.
164164
try { awaitCancellation() }
165165
finally {
166166
// Clean up the BLE manager when the scope is cancelled.
167-
bleManager?.close()
168-
bleManager = null
167+
service?.close()
168+
service = null
169169
}
170170
}
171171
}
172172

173173
override fun disconnect() {
174-
job?.cancel()
175-
job = null
174+
service?.close()
175+
service = null
176176
}
177177
}

‎lib/observability/src/main/java/no/nordicsemi/memfault/observability/bluetooth/MemfaultDiagnosticsService.kt‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import android.content.Context
3939
import kotlinx.coroutines.CoroutineExceptionHandler
4040
import kotlinx.coroutines.CoroutineScope
4141
import kotlinx.coroutines.Job
42+
import kotlinx.coroutines.NonCancellable
4243
import kotlinx.coroutines.awaitCancellation
4344
import kotlinx.coroutines.cancel
4445
import kotlinx.coroutines.channels.BufferOverflow
@@ -54,6 +55,7 @@ import kotlinx.coroutines.flow.launchIn
5455
import kotlinx.coroutines.flow.onEach
5556
import kotlinx.coroutines.flow.update
5657
import kotlinx.coroutines.launch
58+
import kotlinx.coroutines.withContext
5759
import kotlinx.coroutines.withTimeout
5860
import no.nordicsemi.kotlin.ble.client.RemoteService
5961
import no.nordicsemi.kotlin.ble.client.android.CentralManager
@@ -67,6 +69,7 @@ import no.nordicsemi.kotlin.ble.core.Manager
6769
import no.nordicsemi.kotlin.ble.core.OperationStatus
6870
import no.nordicsemi.kotlin.ble.core.WriteType
6971
import no.nordicsemi.memfault.observability.data.MemfaultConfig
72+
import no.nordicsemi.memfault.observability.internal.AuthorisationHeader
7073
import org.slf4j.LoggerFactory
7174
import kotlin.time.Duration
7275
import kotlin.time.Duration.Companion.seconds
@@ -217,10 +220,15 @@ class MemfaultDiagnosticsService {
217220
job = null
218221
}
219222

223+
/**
224+
* This method connects the the peripheral and starts observing its state.
225+
*
226+
* It suspends until the scope is cancelled.
227+
*/
220228
private suspend fun CoroutineScope.connect(
221229
centralManager: CentralManager,
222230
peripheral: Peripheral,
223-
) {
231+
): Nothing {
224232
// Observe the peripheral bond state to catch bonding failures.
225233
var wasBonding = false
226234
peripheral.bondState
@@ -253,6 +261,12 @@ class MemfaultDiagnosticsService {
253261
// is not supported (disconnect() method called), or the connection was cancelled
254262
// by the user.
255263
is ConnectionState.Disconnected -> {
264+
if (state.reason is ConnectionState.Disconnected.Reason.UnsupportedAddress) {
265+
// This error is thrown in AutoConnect connection when there is no
266+
// bonding. The library will transition to Direct connection automatically.
267+
// Don't report this state.
268+
return@onEach
269+
}
256270
_state.emit(state.toDeviceState(notSupported, bondingFailed))
257271
if (state.isUserInitiated /* (includes not supported) */ ||
258272
state.reason is ConnectionState.Disconnected.Reason.UnsupportedConfiguration) {
@@ -346,9 +360,12 @@ class MemfaultDiagnosticsService {
346360
finally {
347361
// Make sure the device is disconnected when the scope is cancelled.
348362
// When it was already disconnected, this is a no-op.
349-
peripheral.disconnect()
350-
// The state collection was cancelled together with the scope. Emit the state manually.
351-
_state.emit(DeviceState.Disconnected())
363+
withContext(NonCancellable) {
364+
peripheral.disconnect()
365+
366+
// The state collection was cancelled together with the scope. Emit the state manually.
367+
_state.emit(peripheral.state.value.toDeviceState(notSupported, bondingFailed))
368+
}
352369
}
353370
}
354371

‎lib/observability/src/main/java/no/nordicsemi/memfault/observability/data/Chunk.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ package no.nordicsemi.memfault.observability.data
22

33
/**
44
* Represents a chunk of Memfault Observability data received from the device
5-
* using Memfault Diagnostics Service.
5+
* using Monitoring & Diagnostics Service.
66
*
77
* @property chunkNumber The number of the chunk, in range from 0-31
88
* @property data The bytes received.
99
* @property deviceId The device ID of the device from which the chunk was received.
10-
* @property isUploaded A flag indicating whether the chunk has been uploaded to the Memfault Cloud.
10+
* @property isUploaded A flag indicating whether the chunk has been uploaded to the cloud.
1111
*/
1212
data class Chunk(
1313
val chunkNumber: Int,

‎lib/observability/src/main/java/no/nordicsemi/memfault/observability/data/PersistentChunkQueue.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import androidx.room.Room
55
import com.memfault.cloud.sdk.ChunkQueue
66
import kotlinx.coroutines.flow.Flow
77
import kotlinx.coroutines.flow.map
8-
import no.nordicsemi.memfault.observability.db.ChunksDatabase
9-
import no.nordicsemi.memfault.observability.db.toChunk
10-
import no.nordicsemi.memfault.observability.db.toEntity
8+
import no.nordicsemi.memfault.observability.internal.db.ChunksDatabase
9+
import no.nordicsemi.memfault.observability.internal.db.toChunk
10+
import no.nordicsemi.memfault.observability.internal.db.toEntity
1111

1212
/**
1313
* An implementation of [ChunkQueue] that stores chunks in a persistent database.

lib/observability/src/main/java/no/nordicsemi/memfault/observability/bluetooth/AuthorisationHeader.kt renamed to lib/observability/src/main/java/no/nordicsemi/memfault/observability/internal/AuthorisationHeader.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package no.nordicsemi.memfault.observability.bluetooth
1+
package no.nordicsemi.memfault.observability.internal
22

33
/**
44
* This class parses the Authorization Header read from a characteristic.

lib/observability/src/main/java/no/nordicsemi/memfault/observability/internet/ChunkSenderExt.kt renamed to lib/observability/src/main/java/no/nordicsemi/memfault/observability/internal/ChunkSenderExt.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,10 @@
2929
* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
3030
*/
3131

32-
package no.nordicsemi.memfault.observability.internet
32+
package no.nordicsemi.memfault.observability.internal
3333

3434
import com.memfault.cloud.sdk.ChunkSender
3535
import com.memfault.cloud.sdk.SendChunksCallback
36-
import kotlinx.coroutines.delay
3736
import kotlin.coroutines.resume
3837
import kotlin.coroutines.suspendCoroutine
3938

lib/observability/src/main/java/no/nordicsemi/memfault/observability/internal/MemfaultScope.kt renamed to lib/observability/src/main/java/no/nordicsemi/memfault/observability/internal/Scope.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,4 @@ import kotlinx.coroutines.CoroutineScope
3535
import kotlinx.coroutines.Dispatchers
3636
import kotlinx.coroutines.SupervisorJob
3737

38-
internal val MemfaultScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
38+
internal val Scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)

lib/observability/src/main/java/no/nordicsemi/memfault/observability/db/ChunkEntity.kt renamed to lib/observability/src/main/java/no/nordicsemi/memfault/observability/internal/db/ChunkEntity.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
3030
*/
3131

32-
package no.nordicsemi.memfault.observability.db
32+
package no.nordicsemi.memfault.observability.internal.db
3333

3434
import androidx.room.ColumnInfo
3535
import androidx.room.Entity

0 commit comments

Comments
 (0)