Skip to content

Commit 43f8c90

Browse files
jeremiah-kjeremiah-k
committed
test(ble): cover OTA cache refresh retry and flag lifecycle
Add focused tests for the three CodeRabbit-identified coverage gaps: BleOtaTransport: cache refresh returns true, first profile finds stale services, transport reconnects once, second profile succeeds. BleRadioTransport: one-shot flag is consumed during connect and triggers the cache invalidation + reconnect path. SharedRadioInterfaceService: flag is consumed exactly once, cleared on disconnect, cleared on genuine device switch (A to B), preserved on same-address rebind, and preserved on post-OTA null-to-address reselect. Extend FakeBleConnection with invalidateServiceCacheResult and an onDisconnect hook to support the retry test scenario.
1 parent 1435610 commit 43f8c90

4 files changed

Lines changed: 230 additions & 0 deletions

File tree

‎core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportTest.kt‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,15 @@ import org.meshtastic.core.testing.FakeBleConnectionFactory
3838
import org.meshtastic.core.testing.FakeBleDevice
3939
import org.meshtastic.core.testing.FakeBleScanner
4040
import org.meshtastic.core.testing.FakeBluetoothRepository
41+
import org.meshtastic.core.testing.FakeRadioInterfaceService
4142
import org.meshtastic.core.testing.failBondAfterRecording
4243
import org.meshtastic.core.testing.failBondWith
4344
import kotlin.test.BeforeTest
4445
import kotlin.test.Test
4546
import kotlin.test.assertEquals
47+
import kotlin.test.assertFalse
4648
import kotlin.test.assertNotNull
49+
import kotlin.test.assertTrue
4750

4851
@OptIn(ExperimentalCoroutinesApi::class)
4952
class BleRadioTransportTest {
@@ -389,4 +392,58 @@ class BleRadioTransportTest {
389392
bleTransport.close()
390393
}
391394
}
395+
396+
/**
397+
* Post-OTA GATT cache invalidation: when the service-layer one-shot flag is armed
398+
* ([RadioInterfaceService.requestGattCacheInvalidationOnNextConnect]), [BleRadioTransport.attemptConnection] must
399+
* consume it and call [BleConnection.invalidateServiceCache]. When the cache refresh succeeds it disconnects and
400+
* reconnects once to force fresh service discovery.
401+
*
402+
* Uses [FakeRadioInterfaceService] as the callback so the flag's real atomic getAndSet semantics drive the
403+
* consume-once behavior under test.
404+
*/
405+
@Test
406+
fun `post-OTA cache invalidation flag is consumed during connect and triggers reconnect`() = runTest {
407+
val device = FakeBleDevice(address = address, name = "Test Device")
408+
bluetoothRepository.bond(device)
409+
scanner.emitDevice(device)
410+
connection.service.addCharacteristic(FROMNUM_CHARACTERISTIC)
411+
connection.service.addCharacteristic(FROMRADIO_CHARACTERISTIC)
412+
connection.invalidateServiceCacheResult = true
413+
414+
val radioService = FakeRadioInterfaceService()
415+
radioService.requestGattCacheInvalidationOnNextConnect()
416+
417+
val bleTransport =
418+
BleRadioTransport(
419+
scope = this,
420+
scanner = scanner,
421+
bluetoothRepository = bluetoothRepository,
422+
connectionFactory = connectionFactory,
423+
callback = radioService,
424+
address = address,
425+
)
426+
bleTransport.start()
427+
try {
428+
// 3s settle + connectAndAwait + 500ms reconnect delay + reconnect connectAndAwait, then profile setup
429+
// bounded by CONNECTED_GATE_TIMEOUT / SUBSCRIPTION_READY_TIMEOUT (5s each).
430+
advanceTimeBy(20_000)
431+
432+
// The transport consumed the flag during its first connect cycle: a second consume must return false.
433+
assertFalse(
434+
radioService.consumeGattCacheInvalidationRequest(),
435+
"GATT cache invalidation flag must have been consumed by the transport",
436+
)
437+
assertTrue(
438+
connection.invalidateServiceCacheCalls >= 1,
439+
"invalidateServiceCache must be called after consuming the flag",
440+
)
441+
assertTrue(
442+
connection.connectAndAwaitCalls >= 2,
443+
"transport must reconnect after cache invalidation (got ${connection.connectAndAwaitCalls} calls)",
444+
)
445+
} finally {
446+
bleTransport.close()
447+
}
448+
}
392449
}

‎core/service/src/commonTest/kotlin/org/meshtastic/core/service/SharedRadioInterfaceServiceLivenessTest.kt‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,4 +1683,127 @@ class SharedRadioInterfaceServiceLivenessTest {
16831683
advanceTimeBy(1_000L)
16841684
}
16851685
}
1686+
1687+
// ─── Post-OTA GATT cache invalidation flag lifecycle ───────────────────────────────────────
1688+
1689+
/**
1690+
* The one-shot cache-invalidation flag is armed by
1691+
* [SharedRadioInterfaceService.requestGattCacheInvalidationOnNextConnect] and consumed exactly once by
1692+
* [SharedRadioInterfaceService.consumeGattCacheInvalidationRequest] (atomic getAndSet).
1693+
* [SharedRadioInterfaceService.disconnect] must clear any still-pending flag so a later reconnect does not silently
1694+
* trigger a stale invalidation.
1695+
*/
1696+
@Test
1697+
fun `gatt cache invalidation flag is consumed once and cleared on disconnect`() = runTest(testDispatcher) {
1698+
val service = createConnectedService("xAA:BB:CC:DD:EE:FF")
1699+
try {
1700+
assertFalse(
1701+
service.consumeGattCacheInvalidationRequest(),
1702+
"flag must default to unset before any request",
1703+
)
1704+
1705+
service.requestGattCacheInvalidationOnNextConnect()
1706+
assertTrue(
1707+
service.consumeGattCacheInvalidationRequest(),
1708+
"first consume after request must return true",
1709+
)
1710+
assertFalse(
1711+
service.consumeGattCacheInvalidationRequest(),
1712+
"second consume must return false (one-shot getAndSet)",
1713+
)
1714+
1715+
// Re-arm; disconnect below must clear it.
1716+
service.requestGattCacheInvalidationOnNextConnect()
1717+
} finally {
1718+
service.disconnect()
1719+
advanceTimeBy(1_000L)
1720+
}
1721+
1722+
assertFalse(
1723+
service.consumeGattCacheInvalidationRequest(),
1724+
"disconnect must clear the pending GATT cache invalidation flag",
1725+
)
1726+
}
1727+
1728+
/**
1729+
* Rebinding to a DIFFERENT device address must drop the pending flag — the cache invalidation was requested for the
1730+
* previous device's post-OTA reboot and must not bleed into the new device's connection.
1731+
* ([SharedRadioInterfaceService.setDeviceAddress] clears `gattCacheInvalidationRequested` when the address
1732+
* changes.)
1733+
*/
1734+
@Test
1735+
fun `setDeviceAddress with a different address clears the pending gatt cache invalidation flag`() =
1736+
runTest(testDispatcher) {
1737+
val service = createConnectedService("xAA:BB:CC:DD:EE:FF")
1738+
try {
1739+
service.requestGattCacheInvalidationOnNextConnect()
1740+
1741+
assertTrue(service.setDeviceAddress("xBB:11:22:33:44:55"), "setDeviceAddress must accept a new address")
1742+
// The clear happens inside the launched transportMutex.withLock; flush it.
1743+
testDispatcher.scheduler.runCurrent()
1744+
advanceTimeBy(1_000L)
1745+
1746+
assertFalse(
1747+
service.consumeGattCacheInvalidationRequest(),
1748+
"switching to a different device address must clear the pending flag",
1749+
)
1750+
} finally {
1751+
service.disconnect()
1752+
advanceTimeBy(1_000L)
1753+
}
1754+
}
1755+
1756+
/**
1757+
* Counterpart: rebinding to the SAME address is a documented no-op ([SharedRadioInterfaceService.setDeviceAddress]
1758+
* early-returns when already Connected to that address), so it must NOT touch the pending flag.
1759+
*/
1760+
@Test
1761+
fun `setDeviceAddress with the same address preserves the pending gatt cache invalidation flag`() =
1762+
runTest(testDispatcher) {
1763+
val address = "xAA:BB:CC:DD:EE:FF"
1764+
val service = createConnectedService(address)
1765+
try {
1766+
service.requestGattCacheInvalidationOnNextConnect()
1767+
1768+
assertFalse(
1769+
service.setDeviceAddress(address),
1770+
"setDeviceAddress with the same connected address is a documented no-op (returns false)",
1771+
)
1772+
testDispatcher.scheduler.runCurrent()
1773+
1774+
assertTrue(
1775+
service.consumeGattCacheInvalidationRequest(),
1776+
"same-address rebind must preserve the pending GATT cache invalidation flag",
1777+
)
1778+
} finally {
1779+
service.disconnect()
1780+
advanceTimeBy(1_000L)
1781+
}
1782+
}
1783+
1784+
@Test
1785+
fun `setDeviceAddress after deselect preserves pending gatt cache invalidation flag`() = runTest(testDispatcher) {
1786+
val address = "xAA:BB:CC:DD:EE:FF"
1787+
val service = createConnectedService(address)
1788+
try {
1789+
// OTA handler deselects to free the GATT
1790+
assertTrue(service.setDeviceAddress("n"), "deselect must succeed")
1791+
testDispatcher.scheduler.runCurrent()
1792+
advanceTimeBy(1_000L)
1793+
1794+
// Post-OTA: arm flag, then re-select the SAME device
1795+
service.requestGattCacheInvalidationOnNextConnect()
1796+
assertTrue(service.setDeviceAddress(address), "re-select must start transport")
1797+
testDispatcher.scheduler.runCurrent()
1798+
advanceTimeBy(1_000L)
1799+
1800+
assertTrue(
1801+
service.consumeGattCacheInvalidationRequest(),
1802+
"post-OTA re-select (null to address) must preserve the pending flag",
1803+
)
1804+
} finally {
1805+
service.disconnect()
1806+
advanceTimeBy(1_000L)
1807+
}
1808+
}
16861809
}

‎core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeBle.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,15 @@ class FakeBleConnection :
114114
/** Negotiated write length exposed to callers; `null` means unknown / not negotiated. */
115115
var maxWriteValueLength: Int? = null
116116

117+
/** Result returned by [invalidateServiceCache]; defaults to `false` to match the [BleConnection] default. */
118+
var invalidateServiceCacheResult: Boolean = false
119+
120+
/** Number of times [invalidateServiceCache] has been invoked. */
121+
var invalidateServiceCacheCalls: Int = 0
122+
123+
/** Optional callback invoked at the end of [disconnect] (e.g. seed freshly-discovered services on reconnect). */
124+
var onDisconnect: (() -> Unit)? = null
125+
117126
/** Number of times [disconnect] has been invoked. */
118127
var disconnectCalls: Int = 0
119128

@@ -164,6 +173,7 @@ class FakeBleConnection :
164173
currentDevice.setState(BleConnectionState.Disconnected())
165174
}
166175
_device.value = null
176+
onDisconnect?.invoke()
167177
}
168178

169179
override suspend fun <T> profile(
@@ -182,6 +192,11 @@ class FakeBleConnection :
182192
}
183193

184194
override fun maximumWriteValueLength(writeType: BleWriteType): Int? = maxWriteValueLength
195+
196+
override fun invalidateServiceCache(): Boolean {
197+
invalidateServiceCacheCalls++
198+
return invalidateServiceCacheResult
199+
}
185200
}
186201

187202
class FakeBleWrite(val characteristic: BleCharacteristic, val data: ByteArray, val writeType: BleWriteType) {

‎feature/firmware/src/commonTest/kotlin/org/meshtastic/feature/firmware/ota/BleOtaTransportTest.kt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,41 @@ class BleOtaTransportTest {
163163
assertEquals(failure.message, cause.message)
164164
}
165165

166+
/**
167+
* Regression: after a cache refresh leaves stale services, the first [bleConnection.profile] throws
168+
* [OtaProtocolException.ConnectionFailed] from `requireOtaCharacteristics`. When the cache was invalidated
169+
* (`invalidateServiceCache()` returned true), the transport must disconnect, reconnect once, and retry the profile
170+
* — which succeeds because the reconnect surfaces the freshly-advertised OTA characteristics.
171+
*/
172+
@Test
173+
fun `connect reconnects once when cache refresh leaves stale OTA services`() = runTest {
174+
val scanner = FakeBleScanner()
175+
val connection = FakeBleConnection()
176+
// Seed no characteristics → first profile() rejects the service as missing OTA chars.
177+
val (transport) = createTransport(scanner, connection, seedOtaCharacteristics = false)
178+
179+
// Cache invalidation succeeds, arming the single reconnect retry in discoverAndPrepareOtaService().
180+
connection.invalidateServiceCacheResult = true
181+
// The forced disconnect surfaces the OTA loader's real service table; seed it for the retry.
182+
connection.onDisconnect = {
183+
connection.service.addCharacteristic(OTA_NOTIFY_CHARACTERISTIC)
184+
connection.service.addCharacteristic(OTA_WRITE_CHARACTERISTIC)
185+
}
186+
187+
scanner.emitDevice(FakeBleDevice(address))
188+
189+
val result = transport.connect()
190+
191+
assertTrue(
192+
result.isSuccess,
193+
"connect() must succeed after one cache-refresh retry: ${result.exceptionOrNull()}",
194+
)
195+
assertTrue(
196+
connection.connectAndAwaitCalls >= 2,
197+
"expected initial connect + one reconnect, got ${connection.connectAndAwaitCalls}",
198+
)
199+
}
200+
166201
// -----------------------------------------------------------------------
167202
// startOta()
168203
// -----------------------------------------------------------------------

0 commit comments

Comments
 (0)