refactor: update user lookups and localize traceroute responses (#5294)

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<Int, Long>())
@@ -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) {

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 {

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(

core/resources/src/commonMain/composeResources/values/strings.xml

@@ -444,8 +444,8 @@
     <string name="traceroute_map_no_data">This traceroute does not have any mappable nodes yet.</string>
     <string name="traceroute_showing_nodes">Showing %1$d/%2$d nodes</string>
     <string name="traceroute_duration">Duration: %1$s s</string>
-    <string name="traceroute_route_towards_dest">Route traced toward destination:\n\n</string>
-    <string name="traceroute_route_back_to_us">Route traced back to us:\n\n</string>
+    <string name="traceroute_route_towards_dest">Route traced toward destination:\n</string>
+    <string name="traceroute_route_back_to_us">Route traced back to us:\n</string>
     <string name="traceroute_forward_hops">Forward Hops</string>
     <string name="traceroute_return_hops">Return Hops</string>
     <string name="traceroute_round_trip">Round Trip</string>