From 1966889c2dff336b01ad789398484e3c35a05249 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:18:57 -0500 Subject: [PATCH] feat(connections): connection sorting & conversation empty channel ranking (#5295) --- .../org/meshtastic/core/testing/FakeBle.kt | 1 + .../feature/connections/ScannerViewModel.kt | 54 +++++++------- .../connections/ScannerViewModelTest.kt | 71 +++++++++++++++++++ .../feature/messaging/ui/contact/Contacts.kt | 8 +-- 4 files changed, 102 insertions(+), 32 deletions(-) diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeBle.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeBle.kt index bed4b1146..de925f937 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeBle.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeBle.kt @@ -42,6 +42,7 @@ class FakeBleDevice( override val address: String, override val name: String? = "Fake Device", initialState: BleConnectionState = BleConnectionState.Disconnected(), + override val rssi: Int? = null, ) : BaseFake(), BleDevice { private val _state = mutableStateFlow(initialState) diff --git a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt index 8e930e977..b942beac1 100644 --- a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt +++ b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt @@ -97,6 +97,7 @@ open class ScannerViewModel( val bleAutoScan: StateFlow = uiPrefs.bleAutoScan private val scannedBleDevices = MutableStateFlow>(emptyMap()) + private val discoveryOrder = MutableStateFlow>(emptyList()) private var scanJob: Job? = null // ── Network scanning (NSD gating) ───────────────────────────────────────────────────────── @@ -156,37 +157,36 @@ open class ScannerViewModel( /** * Combined bonded + scanned BLE devices for the UI. * - * Sorted by signal strength — scanned devices with a known RSSI appear first in descending order (strongest signal - * at the top), followed by bonded-only devices without a scan RSSI, sorted by name. + * Sorted for stability to prevent "shifting" as advertisements arrive: bonded devices always appear first (sorted + * by name), followed by unbonded scanned devices in the order they were first discovered. RSSI updates are + * reflected on the cards but do not trigger a re-sort. */ val bleDevicesForUi: StateFlow> = - combine(discoveredDevicesFlow, scannedBleDevices) { discovered, scannedMap -> + combine(discoveredDevicesFlow, scannedBleDevices, discoveryOrder) { discovered, scannedMap, order -> val bonded = discovered.bleDevices.filterIsInstance() val bondedAddresses = bonded.mapTo(mutableSetOf()) { it.address } // Scanned-but-not-bonded devices are explicitly flagged unbonded so the UI routes through // requestBonding() — which on Android triggers createBond() for the pairing dialog before connecting. + // Preserves discovery order to prevent items jumping around during the scan burst. val unbondedScanned = - scannedMap.values - .asSequence() - .filter { it.address !in bondedAddresses } - .map { DeviceListEntry.Ble(device = it, bonded = false) } + order + .filter { it !in bondedAddresses } + .mapNotNull { address -> + scannedMap[address]?.let { DeviceListEntry.Ble(device = it, bonded = false) } + } - // For bonded devices, attach the latest scan RSSI (if we've seen an advertisement this session) so they - // sort alongside unbonded entries by signal strength. - val bondedWithRssi = - bonded.asSequence().map { entry -> - val scanned = scannedMap[entry.address] - if (scanned != null && scanned.rssi != null) entry.copy(device = scanned) else entry - } + // For bonded devices, attach the latest scan RSSI (if we've seen an advertisement this session) so the + // UI can show the signal indicator, but keep them sorted by name for stability. + val bondedForUi = + bonded + .map { entry -> + val scanned = scannedMap[entry.address] + if (scanned != null && scanned.rssi != null) entry.copy(device = scanned) else entry + } + .sortedBy { it.name } - (bondedWithRssi + unbondedScanned) - .sortedWith( - compareByDescending { it.device.rssi != null } - .thenByDescending { it.device.rssi ?: Int.MIN_VALUE } - .thenBy { it.name }, - ) - .toList() + bondedForUi + unbondedScanned } .flowOn(dispatchers.default) .distinctUntilChanged() @@ -220,7 +220,6 @@ open class ScannerViewModel( if (_isBleScanning.value || bleScanner == null) return _isBleScanning.value = true - scannedBleDevices.value = emptyMap() scanJob = safeLaunch(tag = "startBleScan") { @@ -239,6 +238,9 @@ open class ScannerViewModel( current + (device.address to device) } } + if (device.address !in discoveryOrder.value) { + discoveryOrder.update { it + device.address } + } } } finally { _isBleScanning.value = false @@ -250,8 +252,6 @@ open class ScannerViewModel( scanJob?.cancel() scanJob = null _isBleScanning.value = false - // Drop cached advertisements so stale RSSI values don't linger in the UI after the scan ends. - scannedBleDevices.value = emptyMap() } /** Convenience command: start scanning if idle, stop otherwise. Persists the resulting state to prefs. */ @@ -307,6 +307,7 @@ open class ScannerViewModel( */ fun onSelected(entry: DeviceListEntry): Boolean { radioPrefs.setDevName(entry.name) + addRecentAddress(entry.fullAddress, entry.name) return when (entry) { is DeviceListEntry.Ble -> { if (entry.bonded) { @@ -327,10 +328,7 @@ open class ScannerViewModel( } } is DeviceListEntry.Tcp -> { - safeLaunch(tag = "onSelectedTcp") { - addRecentAddress(entry.fullAddress, entry.name) - changeDeviceAddress(entry.fullAddress) - } + safeLaunch(tag = "onSelectedTcp") { changeDeviceAddress(entry.fullAddress) } true } is DeviceListEntry.Mock -> { diff --git a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt index 73ad03435..f825553df 100644 --- a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt +++ b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt @@ -25,6 +25,7 @@ import dev.mokkery.mock import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest @@ -33,6 +34,7 @@ import org.meshtastic.core.network.repository.DiscoveredService import org.meshtastic.core.network.repository.NetworkRepository import org.meshtastic.core.repository.RadioInterfaceService import org.meshtastic.core.repository.RadioPrefs +import org.meshtastic.core.testing.FakeBleDevice import org.meshtastic.core.testing.FakeRadioController import org.meshtastic.core.testing.FakeServiceRepository import org.meshtastic.feature.connections.model.DeviceListEntry @@ -208,4 +210,73 @@ class ScannerViewModelTest { cancelAndIgnoreRemainingEvents() } } + + @Test + fun `bleDevicesForUi sorts by bonded then discovery order`() = runTest { + val device1 = FakeBleDevice(address = "01:02:03:04:05:06", name = "Node B", rssi = -50) + val device2 = FakeBleDevice(address = "07:08:09:0A:0B:0C", name = "Node A", rssi = -30) + val bondedDevice = + DeviceListEntry.Ble( + device = FakeBleDevice(address = "0D:0E:0F:10:11:12", name = "Bonded C", rssi = null), + bonded = true, + ) + + val scanFlow = MutableStateFlow(null) + every { bleScanner.scan(any(), any()) } returns scanFlow.filterNotNull() + + viewModel.bleDevicesForUi.test { + assertEquals(emptyList(), awaitItem()) + + // 1. Bonded device appears (via use case) + baseDevicesFlow.value = DiscoveredDevices(bleDevices = listOf(bondedDevice)) + assertEquals(listOf(bondedDevice), awaitItem()) + + // 2. Scan finds Device 1 (Node B, -50dBm) + viewModel.startBleScan() + scanFlow.value = device1 + val itemsAfterDevice1 = awaitItem() + assertEquals(2, itemsAfterDevice1.size) + assertEquals(bondedDevice.address, (itemsAfterDevice1[0] as DeviceListEntry.Ble).address) + assertEquals(device1.address, (itemsAfterDevice1[1] as DeviceListEntry.Ble).address) + + // 3. Scan finds Device 2 (Node A, -30dBm) - stronger signal but should be AFTER Device 1 per discovery + // order + scanFlow.value = device2 + val itemsAfterDevice2 = awaitItem() + assertEquals(3, itemsAfterDevice2.size) + assertEquals(bondedDevice.address, (itemsAfterDevice2[0] as DeviceListEntry.Ble).address) + assertEquals(device1.address, (itemsAfterDevice1[1] as DeviceListEntry.Ble).address) + assertEquals(device2.address, (itemsAfterDevice2[2] as DeviceListEntry.Ble).address) + + // 4. Device 1 RSSI updates to -20dBm (strongest) - should NOT re-sort + scanFlow.value = FakeBleDevice(address = device1.address, name = device1.name, rssi = -20) + val itemsAfterRssiUpdate = awaitItem() + assertEquals(3, itemsAfterRssiUpdate.size) + assertEquals(device1.address, (itemsAfterRssiUpdate[1] as DeviceListEntry.Ble).address) + assertEquals(-20, (itemsAfterRssiUpdate[1] as DeviceListEntry.Ble).device.rssi) + + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `stopBleScan does not clear scanned devices`() = runTest { + val device = FakeBleDevice(address = "01:02:03:04:05:06", name = "Node", rssi = -50) + val scanFlow = MutableStateFlow(null) + every { bleScanner.scan(any(), any()) } returns scanFlow.filterNotNull() + + viewModel.bleDevicesForUi.test { + assertEquals(emptyList(), awaitItem()) + + viewModel.startBleScan() + scanFlow.value = device + assertEquals(1, awaitItem().size) + + viewModel.stopBleScan() + // Should not emit a new empty list + expectNoEvents() + + cancelAndIgnoreRemainingEvents() + } + } } diff --git a/feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt b/feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt index 7abaf6db6..fb006e8e9 100644 --- a/feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt +++ b/feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt @@ -530,8 +530,8 @@ private fun ContactListContentInternal( val visiblePlaceholders = rememberVisiblePlaceholders(contacts, channelPlaceholders) LazyColumn(state = listState, modifier = modifier.fillMaxSize()) { - contactListPlaceholdersItems( - placeholders = visiblePlaceholders, + contactListPagedItems( + contacts = contacts, selectedList = selectedList, activeContactKey = activeContactKey, onClick = onClick, @@ -541,8 +541,8 @@ private fun ContactListContentInternal( haptic = haptic, ) - contactListPagedItems( - contacts = contacts, + contactListPlaceholdersItems( + placeholders = visiblePlaceholders, selectedList = selectedList, activeContactKey = activeContactKey, onClick = onClick,