fix(ota): Prevent test hangs and improve OTA resource management

jamesarich · jamesarich · commit 019ea5238dfb · 2026-02-20T13:22:01.000-06:00
This commit addresses a race condition in the BLE OTA tests and improves resource handling in the transport layer.

The `BleOtaTransport` now explicitly closes its `responseChannel` upon disconnection. This prevents potential resource leaks and ensures that downstream consumers are properly notified when the transport is closed.

The BLE OTA tests have been made more robust by adding a check to ensure the peripheral is still connected (`otaPeripheral.isConnected`) before attempting to simulate a value update. This prevents tests from hanging or throwing exceptions due to race conditions where a response is simulated after the connection has already been terminated.

Signed-off-by: James Rich &lt;2199651+jamesarich@users.noreply.github.com&gt;
diff --git a/feature/firmware/src/main/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt b/feature/firmware/src/main/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransport.kt
@@ -303,6 +303,7 @@ class BleOtaTransport(
         bleConnection.disconnect()
         isConnected = false
         transportScope.cancel()
+        responseChannel.close()
     }
 
     private suspend fun sendCommand(command: OtaCommand) {
diff --git a/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportErrorTest.kt b/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportErrorTest.kt
@@ -67,7 +67,9 @@ class BleOtaTransportErrorTest {
                     if (command.startsWith("OTA")) {
                         backgroundScope.launch {
                             delay(50.milliseconds)
-                            otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Rejected\n".toByteArray())
+                            if (otaPeripheral.isConnected) {
+                                otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Rejected\n".toByteArray())
+                            }
                         }
                     }
                     return WriteResponse.Success
@@ -130,7 +132,9 @@ class BleOtaTransportErrorTest {
                 ): WriteResponse {
                     backgroundScope.launch {
                         delay(50.milliseconds)
-                        otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                        if (otaPeripheral.isConnected) {
+                            otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                        }
                     }
                     return WriteResponse.Success
                 }
@@ -204,15 +208,19 @@ class BleOtaTransportErrorTest {
                 ): WriteResponse {
                     backgroundScope.launch {
                         delay(50.milliseconds)
-                        otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                        if (otaPeripheral.isConnected) {
+                            otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                        }
                     }
                     return WriteResponse.Success
                 }
 
                 override fun onWriteCommand(characteristic: MockRemoteCharacteristic, value: ByteArray) {
                     backgroundScope.launch {
                         delay(10.milliseconds)
-                        otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())
+                        if (otaPeripheral.isConnected) {
+                            otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())
+                        }
                     }
                 }
             }
@@ -252,7 +260,9 @@ class BleOtaTransportErrorTest {
             // Setup final response to be a Hash Mismatch error after chunks are sent
             backgroundScope.launch {
                 delay(1000.milliseconds)
-                otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Mismatch\n".toByteArray())
+                if (otaPeripheral.isConnected) {
+                    otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Mismatch\n".toByteArray())
+                }
             }
 
             val data = ByteArray(1024) { it.toByte() }
diff --git a/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportNordicMockTest.kt b/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportNordicMockTest.kt
@@ -94,7 +94,9 @@ class BleOtaTransportNordicMockTest {
                         }
                         backgroundScope.launch(testDispatcher) {
                             delay(50.milliseconds)
-                            otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                            if (otaPeripheral.isConnected) {
+                                otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                            }
                         }
                     }
                     return WriteResponse.Success
@@ -106,12 +108,16 @@ class BleOtaTransportNordicMockTest {
                     println("Mock: Received chunk size=${value.size}, total=$currentTotal/$expected")
                     backgroundScope.launch(testDispatcher) {
                         delay(5.milliseconds)
-                        otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())
+                        if (otaPeripheral.isConnected) {
+                            otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())
+                        }
 
                         if (currentTotal >= expected && expected > 0) {
                             delay(10.milliseconds)
                             println("Mock: Sending final OK")
-                            otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                            if (otaPeripheral.isConnected) {
+                                otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                            }
                         }
                     }
                 }
diff --git a/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt b/feature/firmware/src/test/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt
@@ -70,7 +70,9 @@ class BleOtaTransportTest {
                     backgroundScope.launch(testDispatcher) {
                         // Use a very small delay to simulate high speed
                         delay(1.milliseconds)
-                        otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                        if (otaPeripheral.isConnected) {
+                            otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())
+                        }
                     }
                     return WriteResponse.Success
                 }

Original file line number	Diff line number	Diff line change
`@@ -303,6 +303,7 @@ class BleOtaTransport(`
`303`	`303`	`bleConnection.disconnect()`
`304`	`304`	`isConnected = false`
`305`	`305`	`transportScope.cancel()`
	`306`	`+ responseChannel.close()`
`306`	`307`	`}`
`307`	`308`
`308`	`309`	`private suspend fun sendCommand(command: OtaCommand) {`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,9 @@ class BleOtaTransportErrorTest {`
`67`	`67`	`if (command.startsWith("OTA")) {`
`68`	`68`	`backgroundScope.launch {`
`69`	`69`	`delay(50.milliseconds)`
`70`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Rejected\n".toByteArray())`
	`70`	`+ if (otaPeripheral.isConnected) {`
	`71`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Rejected\n".toByteArray())`
	`72`	`+ }`
`71`	`73`	`}`
`72`	`74`	`}`
`73`	`75`	`return WriteResponse.Success`
`@@ -130,7 +132,9 @@ class BleOtaTransportErrorTest {`
`130`	`132`	`): WriteResponse {`
`131`	`133`	`backgroundScope.launch {`
`132`	`134`	`delay(50.milliseconds)`
`133`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`135`	`+ if (otaPeripheral.isConnected) {`
	`136`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`137`	`+ }`
`134`	`138`	`}`
`135`	`139`	`return WriteResponse.Success`
`136`	`140`	`}`
`@@ -204,15 +208,19 @@ class BleOtaTransportErrorTest {`
`204`	`208`	`): WriteResponse {`
`205`	`209`	`backgroundScope.launch {`
`206`	`210`	`delay(50.milliseconds)`
`207`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`211`	`+ if (otaPeripheral.isConnected) {`
	`212`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`213`	`+ }`
`208`	`214`	`}`
`209`	`215`	`return WriteResponse.Success`
`210`	`216`	`}`
`211`	`217`
`212`	`218`	`override fun onWriteCommand(characteristic: MockRemoteCharacteristic, value: ByteArray) {`
`213`	`219`	`backgroundScope.launch {`
`214`	`220`	`delay(10.milliseconds)`
`215`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())`
	`221`	`+ if (otaPeripheral.isConnected) {`
	`222`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())`
	`223`	`+ }`
`216`	`224`	`}`
`217`	`225`	`}`
`218`	`226`	`}`
`@@ -252,7 +260,9 @@ class BleOtaTransportErrorTest {`
`252`	`260`	`// Setup final response to be a Hash Mismatch error after chunks are sent`
`253`	`261`	`backgroundScope.launch {`
`254`	`262`	`delay(1000.milliseconds)`
`255`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Mismatch\n".toByteArray())`
	`263`	`+ if (otaPeripheral.isConnected) {`
	`264`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "ERR Hash Mismatch\n".toByteArray())`
	`265`	`+ }`
`256`	`266`	`}`
`257`	`267`
`258`	`268`	`val data = ByteArray(1024) { it.toByte() }`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,9 @@ class BleOtaTransportNordicMockTest {`
`94`	`94`	`}`
`95`	`95`	`backgroundScope.launch(testDispatcher) {`
`96`	`96`	`delay(50.milliseconds)`
`97`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`97`	`+ if (otaPeripheral.isConnected) {`
	`98`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`99`	`+ }`
`98`	`100`	`}`
`99`	`101`	`}`
`100`	`102`	`return WriteResponse.Success`
`@@ -106,12 +108,16 @@ class BleOtaTransportNordicMockTest {`
`106`	`108`	`println("Mock: Received chunk size=${value.size}, total=$currentTotal/$expected")`
`107`	`109`	`backgroundScope.launch(testDispatcher) {`
`108`	`110`	`delay(5.milliseconds)`
`109`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())`
	`111`	`+ if (otaPeripheral.isConnected) {`
	`112`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "ACK\n".toByteArray())`
	`113`	`+ }`
`110`	`114`
`111`	`115`	`if (currentTotal >= expected && expected > 0) {`
`112`	`116`	`delay(10.milliseconds)`
`113`	`117`	`println("Mock: Sending final OK")`
`114`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`118`	`+ if (otaPeripheral.isConnected) {`
	`119`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`120`	`+ }`
`115`	`121`	`}`
`116`	`122`	`}`
`117`	`123`	`}`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,9 @@ class BleOtaTransportTest {`
`70`	`70`	`backgroundScope.launch(testDispatcher) {`
`71`	`71`	`// Use a very small delay to simulate high speed`
`72`	`72`	`delay(1.milliseconds)`
`73`		`- otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`73`	`+ if (otaPeripheral.isConnected) {`
	`74`	`+ otaPeripheral.simulateValueUpdate(txCharHandle, "OK\n".toByteArray())`
	`75`	`+ }`
`74`	`76`	`}`
`75`	`77`	`return WriteResponse.Success`
`76`	`78`	`}`