Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ sealed class RadioResponseResult {
data class Error(val message: UiText, val routingError: Routing.Error? = null) : RadioResponseResult()

data object Success : RadioResponseResult()

/**
* A routing ACK for one of our requests arrived from [from], not the node the request was addressed to. For a
* request addressed to the connected node this means the radio renumbered itself between the request and its ACK —
* firmware 2.8 moves `my_node_num` to `crc32(public_key)` when it mints the key on the first region set.
*/
data class UnexpectedAckSender(val from: Int) : RadioResponseResult()
}

/** Use case for processing incoming [MeshPacket]s that are responses to admin requests. */
Expand Down Expand Up @@ -90,7 +97,7 @@ open class ProcessRadioResponseUseCase {

packet.from == destNum -> RadioResponseResult.Success

else -> null
else -> RadioResponseResult.UnexpectedAckSender(from = packet.from)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,24 @@ class ProcessRadioResponseUseCaseTest {
assertEquals(RadioResponseResult.Success, result)
}

@Test
fun `routing ack from a node other than the addressed one is reported instead of dropped`() {
val packet =
MeshPacket(
from = 456,
decoded =
Data(
portnum = PortNum.ROUTING_APP,
request_id = 42,
payload = Routing(error_reason = Routing.Error.NONE).encode().toByteString(),
),
)

val result = useCase(packet, 123, setOf(42))

assertEquals(RadioResponseResult.UnexpectedAckSender(from = 456), result)
}

@Test
fun `invoke with metadata response returns metadata result`() {
// Arrange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import org.meshtastic.core.repository.LocationService
import org.meshtastic.core.repository.LockdownCoordinator
import org.meshtastic.core.repository.LockdownPassphraseStore
import org.meshtastic.core.repository.MapConsentPrefs
import org.meshtastic.core.repository.MeshConnectionManager
import org.meshtastic.core.repository.MqttManager
import org.meshtastic.core.repository.NodeRepository
import org.meshtastic.core.repository.NodeRestartTracker
Expand Down Expand Up @@ -152,7 +153,7 @@ data class RadioConfigState(
@KoinViewModel
@Suppress("LongParameterList", "LargeClass")
open class RadioConfigViewModel(
@InjectedParam private val destNum: Int?,
@InjectedParam initialDestNum: Int?,
private val radioConfigRepository: RadioConfigRepository,
private val packetRepository: PacketRepository,
private val serviceRepository: ServiceRepository,
Expand All @@ -175,9 +176,20 @@ open class RadioConfigViewModel(
private val securityKeyBackupStore: SecurityKeyBackupStore,
private val snackbarManager: SnackbarManager,
private val nodeRestartTracker: NodeRestartTracker,
private val connectionManager: MeshConnectionManager,
private val analytics: PlatformAnalytics,
) : ViewModel() {

/**
* The node this session addresses. Starts as the injected destination (null = the connected node, whatever its
* number). A session opened on the connected node *by number* — Node detail → Administration — drops to null when
* that number moves under it (firmware 2.8 renumbers on the first region set), so later writes follow the connected
* node through [destNode] instead of a number the radio no longer answers to.
*/
private val activeDestNum = MutableStateFlow(initialDestNum)
private val destNum: Int?
get() = activeDestNum.value

val lockdownTokenInfo = serviceRepository.lockdownTokenInfo
val sessionAuthorized = serviceRepository.sessionAuthorized
val lockdownState = serviceRepository.lockdownState
Expand Down Expand Up @@ -309,8 +321,9 @@ open class RadioConfigViewModel(
locationService.getCurrentLocation()

init {
nodeRepository.nodeDBbyNum
.map { nodes -> if (destNum != null) nodes[destNum] else nodes.values.firstOrNull() }
combine(nodeRepository.nodeDBbyNum, activeDestNum) { nodes, dest ->
if (dest != null) nodes[dest] else nodes.values.firstOrNull()
}
.distinctUntilChanged()
.onEach {
_destNode.value = it
Expand All @@ -325,11 +338,10 @@ open class RadioConfigViewModel(
// Derive isLocal from the immutable destNum and the (possibly changing) myNodeInfo.
// flatMapLatest cancels the previous inner flow on every change, so there is
// no window where stale local config can leak through.
nodeRepository.myNodeInfo
.map { ni ->
val isLocal = (destNum == null) || (destNum == ni?.myNodeNum)
isLocal to if (isLocal) ni?.pioEnv else null
}
combine(nodeRepository.myNodeInfo, activeDestNum) { ni, dest ->
val isLocal = (dest == null) || (dest == ni?.myNodeNum)
isLocal to if (isLocal) ni?.pioEnv else null
}
.distinctUntilChanged()
.flatMapLatest { (isLocal, pioEnv) ->
if (isLocal) {
Expand Down Expand Up @@ -1235,6 +1247,27 @@ open class RadioConfigViewModel(
}
}

is RadioResponseResult.UnexpectedAckSender -> {
// The connected radio ACKed our local write from a node number other than the one we addressed:
// firmware 2.8 renumbers itself (num = crc32(public_key)) when it mints the PKI key on the first
// region set, live and without a reboot, so our cached number is now stale and every further admin
// write would NAK PKI_SEND_FAIL_PUBLIC_KEY until we re-learn it. Re-run the config handshake to pick
// up the new my_node_num, and treat this ACK as the save's confirmation. Scoped to a local save
// (route empty, isLocal): a remote admin target legitimately answers from a different node.
if (requestId != null && !isLateRemoteRead && route.isEmpty() && radioConfigState.value.isLocal) {
clearRequestIds()
// A session opened on the connected node by number would otherwise keep addressing the old
// number (and stop counting as local) once the handshake reports the new one.
activeDestNum.value = null
connectionManager.startConfigOnly()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
setResponseStateSuccess()
}
// Otherwise an unaddressed node's ack says nothing about our request: leave it pending, exactly as
// when the use case returned null for it. Falling through would let the generic completion below
// retire the request and resolve a remote save on a foreign ack.
return
}

is RadioResponseResult.Metadata -> {
_radioConfigState.update { it.copy(metadata = result.metadata) }
if (!isLateRemoteRead) incrementCompleted()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class ProfileRoundTripTest {

viewModel =
RadioConfigViewModel(
destNum = null,
initialDestNum = null,
radioConfigRepository = radioConfigRepository,
packetRepository = packetRepository,
serviceRepository = serviceRepository,
Expand All @@ -146,6 +146,7 @@ class ProfileRoundTripTest {
securityKeyBackupStore = securityKeyBackupStore,
snackbarManager = snackbarManager,
nodeRestartTracker = nodeRestartTracker,
connectionManager = mock(MockMode.autofill),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.setMain
import okio.ByteString
import okio.ByteString.Companion.encodeUtf8
import org.meshtastic.core.domain.usecase.settings.AdminActionsUseCase
import org.meshtastic.core.domain.usecase.settings.ExportProfileUseCase
Expand All @@ -64,6 +65,7 @@ import org.meshtastic.core.repository.HomoglyphPrefs
import org.meshtastic.core.repository.LocationRepository
import org.meshtastic.core.repository.LocationService
import org.meshtastic.core.repository.MapConsentPrefs
import org.meshtastic.core.repository.MeshConnectionManager
import org.meshtastic.core.repository.MqttManager
import org.meshtastic.core.repository.NodeRestartTracker
import org.meshtastic.core.repository.PacketRepository
Expand Down Expand Up @@ -92,6 +94,7 @@ import org.meshtastic.proto.LoRaRegionPresetMap
import org.meshtastic.proto.LocalConfig
import org.meshtastic.proto.LocalModuleConfig
import org.meshtastic.proto.MeshPacket
import org.meshtastic.proto.PortNum
import org.meshtastic.proto.Routing
import org.meshtastic.proto.User
import kotlin.test.AfterTest
Expand Down Expand Up @@ -170,6 +173,7 @@ class RadioConfigViewModelTest {
private val uiPrefs: UiPrefs = mock(MockMode.autofill)
private val securityKeyBackupStore: SecurityKeyBackupStore = mock(MockMode.autofill)
private val snackbarManager: SnackbarManager = mock(MockMode.autofill)
private val connectionManager: MeshConnectionManager = mock(MockMode.autofill)
private val trackerScope = CoroutineScope(SupervisorJob())
private val nodeRestartTracker = NodeRestartTracker(trackerScope)

Expand Down Expand Up @@ -222,34 +226,131 @@ class RadioConfigViewModelTest {
private fun runTest(block: suspend TestScope.() -> Unit) =
kotlinx.coroutines.test.runTest(testDispatcher, testBody = block)

private fun createViewModel(destNum: Int? = null, snackbarManager: SnackbarManager = this.snackbarManager) =
RadioConfigViewModel(
destNum = destNum,
radioConfigRepository = radioConfigRepository,
packetRepository = packetRepository,
serviceRepository = serviceRepository,
nodeRepository = nodeRepository,
locationRepository = locationRepository,
mapConsentPrefs = mapConsentPrefs,
analyticsPrefs = analyticsPrefs,
homoglyphEncodingPrefs = homoglyphEncodingPrefs,
importProfileUseCase = importProfileUseCase,
exportProfileUseCase = exportProfileUseCase,
importSecurityConfigUseCase = importSecurityConfigUseCase,
securityKeyBackupStore = securityKeyBackupStore,
snackbarManager = snackbarManager,
nodeRestartTracker = nodeRestartTracker,
installProfileUseCase = installProfileUseCase,
radioConfigUseCase = radioConfigUseCase,
adminActionsUseCase = adminActionsUseCase,
processRadioResponseUseCase = processRadioResponseUseCase,
locationService = locationService,
fileService = fileService,
mqttManager = mqttManager,
lockdownCoordinator = FakeLockdownCoordinator(),
analytics = mock(MockMode.autofill),
private fun createViewModel(
destNum: Int? = null,
snackbarManager: SnackbarManager = this.snackbarManager,
processRadioResponseUseCase: ProcessRadioResponseUseCase = this.processRadioResponseUseCase,
) = RadioConfigViewModel(
initialDestNum = destNum,
radioConfigRepository = radioConfigRepository,
packetRepository = packetRepository,
serviceRepository = serviceRepository,
nodeRepository = nodeRepository,
locationRepository = locationRepository,
mapConsentPrefs = mapConsentPrefs,
analyticsPrefs = analyticsPrefs,
homoglyphEncodingPrefs = homoglyphEncodingPrefs,
importProfileUseCase = importProfileUseCase,
exportProfileUseCase = exportProfileUseCase,
importSecurityConfigUseCase = importSecurityConfigUseCase,
securityKeyBackupStore = securityKeyBackupStore,
snackbarManager = snackbarManager,
nodeRestartTracker = nodeRestartTracker,
connectionManager = connectionManager,
installProfileUseCase = installProfileUseCase,
radioConfigUseCase = radioConfigUseCase,
adminActionsUseCase = adminActionsUseCase,
processRadioResponseUseCase = processRadioResponseUseCase,
locationService = locationService,
fileService = fileService,
mqttManager = mqttManager,
lockdownCoordinator = FakeLockdownCoordinator(),
analytics = mock(MockMode.autofill),
)
.also { createdViewModels += it }

@Test
fun `local save acked by a renumbered radio re-runs the handshake and completes the save`() = runTest {
val node = Node(num = 123, user = User(id = "!123"))
nodeRepository.setNodes(listOf(node))
nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 123))
val packetFlow = MutableSharedFlow<MeshPacket>()
every { serviceRepository.meshPacketFlow } returns packetFlow
everySuspend { radioConfigUseCase.setConfig(any(), any(), any()) } calls
{
it.args.onRequestIdArg()(77)
77
}
viewModel = createViewModel(processRadioResponseUseCase = ProcessRadioResponseUseCase())

viewModel.setConfig(Config(lora = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.US)))
runCurrent()
assertTrue(viewModel.radioConfigState.value.responseState is ResponseState.Loading)

// Firmware 2.8 renumbered itself on the first region set: the ACK comes from the new number.
packetFlow.emit(routingAck(requestId = 77, from = 456))
runCurrent()

verify { connectionManager.startConfigOnly() }
assertTrue(viewModel.radioConfigState.value.responseState is ResponseState.Success)
}

@Test
fun `a session opened on the connected node by number follows it after the renumber`() = runTest {
nodeRepository.setNodes(listOf(Node(num = 123, user = User(id = "!123"))))
nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 123))
val packetFlow = MutableSharedFlow<MeshPacket>()
every { serviceRepository.meshPacketFlow } returns packetFlow
val writeTargets = mutableListOf<Int>()
var nextPacketId = 90
everySuspend { radioConfigUseCase.setConfig(any(), any(), any()) } calls
{
writeTargets += it.args[0] as Int
val id = ++nextPacketId
it.args.onRequestIdArg()(id)
id
}
// Node detail → Administration opens the connected node's settings by its number, not as a null local session.
viewModel = createViewModel(destNum = 123, processRadioResponseUseCase = ProcessRadioResponseUseCase())
assertTrue(viewModel.radioConfigState.value.isLocal)

viewModel.setConfig(Config(lora = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.US)))
runCurrent()
packetFlow.emit(routingAck(requestId = 91, from = 456))
runCurrent()
verify { connectionManager.startConfigOnly() }
assertTrue(viewModel.radioConfigState.value.responseState is ResponseState.Success)

// The re-handshake lands: the radio reports its new number and is installed under it.
nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 456))
nodeRepository.setNodes(listOf(Node(num = 456, user = User(id = "!456"))))
runCurrent()
assertTrue(viewModel.radioConfigState.value.isLocal)

viewModel.setConfig(Config(lora = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.US)))
runCurrent()
assertEquals(listOf(123, 456), writeTargets)
}

@Test
fun `remote save acked by an unexpected node neither re-handshakes nor completes`() = runTest {
nodeRepository.setNodes(
listOf(Node(num = 123, user = User(id = "!123")), Node(num = 200, user = User(id = "!200"))),
)
.also { createdViewModels += it }
nodeRepository.setMyNodeInfo(myNodeInfo(myNodeNum = 123))
val packetFlow = MutableSharedFlow<MeshPacket>()
every { serviceRepository.meshPacketFlow } returns packetFlow
everySuspend { radioConfigUseCase.setConfig(any(), any(), any()) } calls
{
it.args.onRequestIdArg()(78)
78
}
viewModel = createViewModel(destNum = 200, processRadioResponseUseCase = ProcessRadioResponseUseCase())

viewModel.setConfig(Config(lora = Config.LoRaConfig(region = Config.LoRaConfig.RegionCode.US)))
runCurrent()
assertTrue(viewModel.radioConfigState.value.responseState is ResponseState.Loading)

packetFlow.emit(routingAck(requestId = 78, from = 456))
runCurrent()

verify(exactly(0)) { connectionManager.startConfigOnly() }
val state = viewModel.radioConfigState.value
assertTrue(
state.responseState is ResponseState.Loading,
"expected Loading, got ${state.responseState} (isLocal=${state.isLocal}, route='${state.route}')",
)
}

@Test
fun `setConfig calls useCase`() = runTest {
Expand Down Expand Up @@ -2092,3 +2193,14 @@ class RadioConfigViewModelTest {
/** Extracts the trailing `onRequestId` callback from a mocked request method's args. */
@Suppress("UNCHECKED_CAST")
private fun List<Any?>.onRequestIdArg(): (Int) -> Unit = last() as (Int) -> Unit

/** A real ROUTING_APP ack for [requestId] as the radio would deliver it, sent by node [from]. */
private fun routingAck(requestId: Int, from: Int) = MeshPacket(
from = from,
decoded =
Data(
portnum = PortNum.ROUTING_APP,
request_id = requestId,
payload = ByteString.of(*Routing(error_reason = Routing.Error.NONE).encode()),
),
)