From 3d2b21843e976f552f8b91be67da0746466e5b8e Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Wed, 29 Apr 2026 11:42:02 -0500 Subject: [PATCH] refactor: update user lookups and localize traceroute responses (#5294) --- .../data/manager/NeighborInfoHandlerImpl.kt | 9 ++- .../data/manager/TracerouteHandlerImpl.kt | 80 ++++++++++--------- .../data/repository/NodeRepositoryImpl.kt | 37 +++++---- .../composeResources/values/strings.xml | 4 +- 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NeighborInfoHandlerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NeighborInfoHandlerImpl.kt index 3f483ba25..e368d5048 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NeighborInfoHandlerImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/NeighborInfoHandlerImpl.kt @@ -25,6 +25,7 @@ import org.meshtastic.core.common.util.NumberFormatter import org.meshtastic.core.common.util.nowMillis import org.meshtastic.core.repository.NeighborInfoHandler import org.meshtastic.core.repository.NodeManager +import org.meshtastic.core.repository.NodeRepository import org.meshtastic.core.repository.ServiceBroadcasts import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.proto.MeshPacket @@ -35,6 +36,7 @@ class NeighborInfoHandlerImpl( private val nodeManager: NodeManager, private val serviceRepository: ServiceRepository, private val serviceBroadcasts: ServiceBroadcasts, + private val nodeRepository: NodeRepository, ) : NeighborInfoHandler { private val startTimes = atomic(persistentMapOf()) @@ -66,12 +68,13 @@ class NeighborInfoHandlerImpl( val neighbors = ni.neighbors.joinToString("\n") { n -> - val node = nodeManager.nodeDBbyNodeNum[n.node_id] - val name = node?.let { "${it.user.long_name} (${it.user.short_name})" } ?: "Unknown" + val user = nodeRepository.getUser(n.node_id) + val name = "${user.long_name} (${user.short_name})" "• $name (SNR: ${n.snr})" } - val formatted = "Neighbors of ${nodeManager.nodeDBbyNodeNum[from]?.user?.long_name ?: "Unknown"}:\n$neighbors" + val fromUser = nodeRepository.getUser(from) + val formatted = "Neighbors of ${fromUser.long_name}:\n$neighbors" val responseText = if (start != null) { diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt index 5d2feb65e..003158313 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/TracerouteHandlerImpl.kt @@ -30,16 +30,18 @@ import org.meshtastic.core.common.util.nowMillis import org.meshtastic.core.model.fullRouteDiscovery import org.meshtastic.core.model.getTracerouteResponse import org.meshtastic.core.model.service.TracerouteResponse -import org.meshtastic.core.repository.NodeManager import org.meshtastic.core.repository.NodeRepository import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.core.repository.TracerouteHandler import org.meshtastic.core.repository.TracerouteSnapshotRepository +import org.meshtastic.core.resources.Res +import org.meshtastic.core.resources.getStringSuspend +import org.meshtastic.core.resources.traceroute_route_back_to_us +import org.meshtastic.core.resources.traceroute_route_towards_dest import org.meshtastic.proto.MeshPacket @Single class TracerouteHandlerImpl( - private val nodeManager: NodeManager, private val serviceRepository: ServiceRepository, private val tracerouteSnapshotRepository: TracerouteSnapshotRepository, private val nodeRepository: NodeRepository, @@ -61,20 +63,20 @@ class TracerouteHandlerImpl( // Require both directions for a "full" traceroute response if (forwardRoute.isEmpty() || returnRoute.isEmpty()) return - val full = - routeDiscovery.getTracerouteResponse( - getUser = { num -> - nodeManager.nodeDBbyNodeNum[num]?.let { "${it.user.long_name} (${it.user.short_name})" } - ?: "Unknown" - }, - headerTowards = "Route towards destination:", - headerBack = "Route back to us:", - ) + scope.handledLaunch { + val full = + routeDiscovery.getTracerouteResponse( + getUser = { num -> + val user = nodeRepository.getUser(num) + "${user.long_name} (${user.short_name})" + }, + headerTowards = getStringSuspend(Res.string.traceroute_route_towards_dest), + headerBack = getStringSuspend(Res.string.traceroute_route_back_to_us), + ) - val requestId = packet.decoded?.request_id ?: 0 + val requestId = packet.decoded?.request_id ?: 0 - if (logUuid != null) { - scope.handledLaunch { + if (logUuid != null) { logInsertJob?.join() val routeNodeNums = (forwardRoute + returnRoute).distinct() val nodeDbByNum = nodeRepository.nodeDBbyNum.value @@ -82,32 +84,32 @@ class TracerouteHandlerImpl( routeNodeNums.mapNotNull { num -> nodeDbByNum[num]?.validPosition?.let { num to it } }.toMap() tracerouteSnapshotRepository.upsertSnapshotPositions(logUuid, requestId, snapshotPositions) } + + val start = startTimes.value[requestId] + startTimes.update { it.remove(requestId) } + val responseText = + if (start != null) { + val elapsedMs = nowMillis - start + val seconds = elapsedMs / MILLIS_PER_SECOND + Logger.i { "Traceroute $requestId complete in $seconds s" } + "$full\n\nDuration: ${NumberFormatter.format(seconds, 1)} s" + } else { + full + } + + val destination = forwardRoute.firstOrNull() ?: returnRoute.lastOrNull() ?: 0 + + serviceRepository.setTracerouteResponse( + TracerouteResponse( + message = responseText, + destinationNodeNum = destination, + requestId = requestId, + forwardRoute = forwardRoute, + returnRoute = returnRoute, + logUuid = logUuid, + ), + ) } - - val start = startTimes.value[requestId] - startTimes.update { it.remove(requestId) } - val responseText = - if (start != null) { - val elapsedMs = nowMillis - start - val seconds = elapsedMs / MILLIS_PER_SECOND - Logger.i { "Traceroute $requestId complete in $seconds s" } - "$full\n\nDuration: ${NumberFormatter.format(seconds, 1)} s" - } else { - full - } - - val destination = forwardRoute.firstOrNull() ?: returnRoute.lastOrNull() ?: 0 - - serviceRepository.setTracerouteResponse( - TracerouteResponse( - message = responseText, - destinationNodeNum = destination, - requestId = requestId, - forwardRoute = forwardRoute, - returnRoute = returnRoute, - logUuid = logUuid, - ), - ) } companion object { diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/NodeRepositoryImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/NodeRepositoryImpl.kt index 852853b9d..4e3a71a4a 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/NodeRepositoryImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/NodeRepositoryImpl.kt @@ -51,7 +51,6 @@ import org.meshtastic.core.model.NodeSortOption import org.meshtastic.core.model.util.onlineTimeThreshold import org.meshtastic.core.repository.NodeRepository import org.meshtastic.proto.DeviceMetadata -import org.meshtastic.proto.HardwareModel import org.meshtastic.proto.LocalStats import org.meshtastic.proto.User @@ -143,24 +142,34 @@ class NodeRepositoryImpl( /** Returns the [User] info for a given [nodeNum]. */ override fun getUser(nodeNum: Int): User = getUser(DataPacket.nodeNumToDefaultId(nodeNum)) + private val last4 = 4 + /** Returns the [User] info for a given [userId]. Falls back to a generic user if not found. */ - override fun getUser(userId: String): User = nodeDBbyNum.value.values.find { it.user.id == userId }?.user - ?: User( - id = userId, - long_name = + override fun getUser(userId: String): User { + val found = nodeDBbyNum.value.values.find { it.user.id == userId }?.user + if (found != null && found.long_name.isNotBlank() && found.short_name.isNotBlank()) { + return found + } + + val fallbackId = userId.takeLast(last4) + val defaultLong = if (userId == DataPacket.ID_LOCAL) { - ourNodeInfo.value?.user?.long_name ?: "Local" + ourNodeInfo.value?.user?.long_name?.takeIf { it.isNotBlank() } ?: "Local" } else { - "Meshtastic ${userId.takeLast(n = 4)}" - }, - short_name = + "Meshtastic $fallbackId" + } + val defaultShort = if (userId == DataPacket.ID_LOCAL) { - ourNodeInfo.value?.user?.short_name ?: "Local" + ourNodeInfo.value?.user?.short_name?.takeIf { it.isNotBlank() } ?: "Local" } else { - userId.takeLast(n = 4) - }, - hw_model = HardwareModel.UNSET, - ) + fallbackId + } + + return found?.copy( + long_name = found.long_name.takeIf { it.isNotBlank() } ?: defaultLong, + short_name = found.short_name.takeIf { it.isNotBlank() } ?: defaultShort, + ) ?: User(id = userId, long_name = defaultLong, short_name = defaultShort) + } /** Returns a flow of nodes filtered and sorted according to the parameters. */ override fun getNodes( diff --git a/core/resources/src/commonMain/composeResources/values/strings.xml b/core/resources/src/commonMain/composeResources/values/strings.xml index fe5f0b808..c9dcee461 100644 --- a/core/resources/src/commonMain/composeResources/values/strings.xml +++ b/core/resources/src/commonMain/composeResources/values/strings.xml @@ -444,8 +444,8 @@ This traceroute does not have any mappable nodes yet. Showing %1$d/%2$d nodes Duration: %1$s s - Route traced toward destination:\n\n - Route traced back to us:\n\n + Route traced toward destination:\n + Route traced back to us:\n Forward Hops Return Hops Round Trip