fix traceroute/ improve dest parser

Meshtastic.Cli/CommandHandlers/TraceRouteCommandHandler.cs

@@ -1,15 +1,17 @@
 using Meshtastic.Data;
 using Meshtastic.Data.MessageFactories;
 using Meshtastic.Display;
-using Meshtastic.Extensions;
 using Meshtastic.Protobufs;
 using Microsoft.Extensions.Logging;
 
 namespace Meshtastic.Cli.CommandHandlers;
 
 public class TraceRouteCommandHandler : DeviceCommandHandler
 {
-    public TraceRouteCommandHandler(DeviceConnectionContext context, CommandContext commandContext) : base(context, commandContext) { }
+    public TraceRouteCommandHandler(DeviceConnectionContext context, CommandContext commandContext) : base(context,
+        commandContext)
+    {
+    }
 
     public async Task<DeviceStateContainer> Handle()
     {
@@ -24,22 +26,18 @@ public override async Task OnCompleted(FromRadio packet, DeviceStateContainer co
         Logger.LogInformation($"Tracing route to {container.GetNodeDisplayName(Destination!.Value)}...");
         var messageFactory = new TraceRouteMessageFactory(container, Destination);
         var message = messageFactory.CreateRouteDiscoveryPacket();
+
         await Connection.WriteToRadio(ToRadioMessageFactory.CreateMeshPacketMessage(message),
-            (fromRadio, container) =>
+            (fromRadio, _) =>
             {
-                var routeDiscovery = fromRadio.GetPayload<RouteDiscovery>();
-                if (routeDiscovery != null)
-                {
-                    if (routeDiscovery.Route.Count > 0)
-                    {
-                        var printer = new ProtobufPrinter(container, OutputFormat);
-                        printer.PrintRoute(routeDiscovery.Route);
-                    }
-                    else
-                        Logger.LogWarning("No routes discovered");
-                    return Task.FromResult(true);
-                }
-                return Task.FromResult(false);
+                if (fromRadio.Packet?.Decoded == null ||
+                    fromRadio.Packet.Decoded.RequestId != message.Id ||
+                    fromRadio.Packet.Decoded.Portnum != message.Decoded.Portnum) 
+                    return Task.FromResult(false);
+
+                var printer = new ProtobufPrinter(container, OutputFormat);
+                printer.PrintRoute(fromRadio);
+                return Task.FromResult(true);
             });
     }
 }

Meshtastic.Cli/Display/ProtobufPrinter.cs

@@ -247,28 +247,79 @@ public void PrintMetadata(DeviceMetadata metadata)
         }
     }
 
-    public void PrintRoute(RepeatedField<uint> route)
+    public void PrintRoute(FromRadio fromRadio)
     {
-        if (outputFormat == OutputFormat.PrettyConsole)
+        const int unknownSnr = -128;
+
+        if (outputFormat != OutputFormat.PrettyConsole) 
+            return;
+
+        var routeDiscovery = fromRadio.GetPayload<RouteDiscovery>()!;
+
+        // invert roles when receiving data
+        var srcNodeId = fromRadio.Packet!.To;
+        var dstNodeId = fromRadio.Packet!.From;
+
+        // toward
+        var towardRoute = Enumerable.Empty<uint>()
+            .Append(srcNodeId)
+            .Concat(routeDiscovery.Route)
+            .Append(dstNodeId)
+            .ToList();
+        var towardSnr = Enumerable.Empty<float?>()
+            .Append(null)
+            .Concat(routeDiscovery.SnrTowards.Select(x => x == unknownSnr ? (float?)null : x / 4f))
+            .ToList();
+        var toward = towardRoute
+            .Select((nodeId, i) => (nodeId, snr: towardSnr.Count == towardRoute.Count ? towardSnr[i] : null))
+            .ToList();
+        PrintRouteDirection("toward", toward);
+
+        // backward
+        var backwardRoute = Enumerable.Empty<uint>()
+            .Append(dstNodeId)
+            .Concat(routeDiscovery.RouteBack)
+            .Append(srcNodeId)
+            .ToList();
+        var backwardSnr = Enumerable.Empty<float?>()
+            .Append(null)
+            .Concat(routeDiscovery.SnrBack.Select(x => x == unknownSnr ? (float?)null : x / 4f))
+            .ToList();
+        var backward = backwardRoute
+            .Select((nodeId, i) => (nodeId, snr: backwardSnr.Count == backwardRoute.Count ? backwardSnr[i] : null))
+            .ToList();
+        PrintRouteDirection("backward", backward);
+
+    }
+
+    private void PrintRouteDirection(string name, IReadOnlyList<(uint nodeId, float? snr)> route)
+    {
+        var root = new Tree($"[bold]Route {name}[/]")
         {
-            var root = new Tree("[bold]Route[/]")
+            Style = new Style(StyleResources.MESHTASTIC_GREEN)
+        };
+        IHasTreeNodes currentTreeNode = root;
+        for (var i = 0; i < route.Count; i++)
+        {
+            var hop = route[i];
+            var node = container.Nodes.Find(n => n.Num == hop.nodeId);
+            var positionStr = node?.Position.ToDisplayString() ?? "???";
+            var snrStr = hop.snr == null
+                ? i == 0
+                    ? "Not applicable"
+                    : "???"
+                : hop.snr.Value.ToString("F");
+            var content =
+                $"Position: {positionStr}{Environment.NewLine}" +
+                $"SNR: {snrStr}{Environment.NewLine}";
+            var panel = new Panel(content)
             {
-                Style = new Style(StyleResources.MESHTASTIC_GREEN)
+                Header = new PanelHeader(container.GetNodeDisplayName(hop.nodeId)),
             };
-
-            IHasTreeNodes currentTreeNode = root;
-            foreach (var nodeNum in route)
-            {
-                var node = container.Nodes.Find(n => n.Num == nodeNum);
-                var content = $"Position: {node?.Position.ToDisplayString()}{Environment.NewLine}";
-                var panel = new Panel(content)
-                {
-                    Header = new PanelHeader(container.GetNodeDisplayName(nodeNum))
-                };
-                currentTreeNode = currentTreeNode.AddNode(panel);
-            }
-            AnsiConsole.Write(root);
+            currentTreeNode = currentTreeNode.AddNode(panel);
         }
+
+        AnsiConsole.Write(root);
     }
 
     public Panel PrintTrafficCharts()

Meshtastic.Cli/Program.cs

@@ -6,6 +6,7 @@
 using Microsoft.Extensions.Logging;
 using System.CommandLine.Builder;
 using System.CommandLine.Parsing;
+using Meshtastic.Cli.Utilities;
 
 var port = new Option<string>("--port", description: "Target serial port for meshtastic device");
 var host = new Option<string>("--host", description: "Target host ip or name for meshtastic device");
@@ -18,7 +19,9 @@
 log.SetDefaultValue(LogLevel.Information);
 log.AddCompletions(Enum.GetNames(typeof(LogLevel)));
 
-var dest = new Option<uint?>("--dest", description: "Destination node address for command");
+var dest = new Option<uint?>("--dest", description: "Destination node address for command", parseArgument: ArgumentParsers.NodeIdParser);
+
+
 var selectDest = new Option<bool>("--select-dest", description: "Interactively select a destination from device's node list");
 selectDest.AddAlias("-sd");
 selectDest.SetDefaultValue(false);

Meshtastic.Cli/Utilities/ArgumentParsers.cs

@@ -0,0 +1,56 @@
+using System.CommandLine.Parsing;
+using System.Text.RegularExpressions;
+
+namespace Meshtastic.Cli.Utilities;
+
+public static class ArgumentParsers
+{
+    public static uint? NodeIdParser(ArgumentResult result)
+    {
+        if (result.Tokens.Count == 0)
+        {
+            return null;
+        }
+        else if (result.Tokens.Count > 1)
+        {
+            result.ErrorMessage = $"Argument --{result.Argument.Name} expects one argument but got {result.Tokens.Count}";
+            return null;
+        }
+
+        var nodeStr = result.Tokens[0].Value.ToLowerInvariant();
+
+        var hexMatch = Regex.Match(nodeStr, "^(!|0x)(?<num>([a-f|0-9][a-f|0-9]){1,4})$");
+        if (hexMatch.Success)
+        {
+            try
+            {
+                return Convert.ToUInt32("0x" + hexMatch.Groups["num"].Value, 16);
+            }
+            catch (Exception e)
+            {
+                result.ErrorMessage = $"Argument --{result.Argument.Name} can not be parsed. " +
+                                      $"{e.Message}";
+                return null;
+            }
+        }
+
+        var decMatch = Regex.Match(nodeStr, "^(?<num>[0-9]+)$");
+        if (decMatch.Success)
+        {
+            try
+            {
+                return Convert.ToUInt32(decMatch.Groups["num"].Value, 10);
+            }
+            catch (Exception e)
+            {
+                result.ErrorMessage = $"Argument --{result.Argument.Name} can not be parsed. " +
+                                      $"{e.Message}";
+                return null;
+            }
+        }
+
+        result.ErrorMessage = $"Argument --{result.Argument.Name} can not be parsed. " +
+                              "One of formats expected: 0xDEADBEEF, !DEADBEEF, 3735928559";
+        return null;
+    }
+}

Meshtastic.Test/Utilities/ArgumentParsersTests.cs

@@ -0,0 +1,37 @@
+using Meshtastic.Cli.Utilities;
+using System.CommandLine;
+
+namespace Meshtastic.Test.Utilities;
+
+public class ArgumentParsersTests
+{
+    [TestCase("!DEADBEEF", 3735928559u, false)]
+    [TestCase("!deadbeef", 3735928559u, false)]
+    [TestCase("!deadbeef0", null, true)]
+    [TestCase("!d", null, true)]
+    [TestCase("0xDEADBEEF", 3735928559u, false)]
+    [TestCase("0xdeadbeef", 3735928559u, false)]
+    [TestCase("0xd", null, true)]
+    [TestCase("0x00", 0u, false)]
+    [TestCase("0xFFF", null, true)]
+    [TestCase("4294967295", uint.MaxValue, false)]
+    [TestCase("42949672950", null, true)]
+    [TestCase("429496729a", null, true)]
+    public void NodeIdParser_ParsesHexAndDecimal(string valueForParse, uint? expected, bool withError)
+    {
+        var opt = new Option<uint?>("--dest", 
+            description: "Destination node address for command",
+            parseArgument: ArgumentParsers.NodeIdParser);
+
+        var result = opt.Parse(["--dest", valueForParse]);
+        if (!withError)
+        {
+            result.Errors.Should().HaveCount(0);
+            result.GetValueForOption(opt).Should().Be(expected);
+        }
+        else
+        {
+            result.Errors.Should().HaveCountGreaterThanOrEqualTo(1);
+        }
+    }
+}

Meshtastic/Data/MessageFactories/TraceRouteMessageFactory.cs

@@ -1,5 +1,6 @@
 using Google.Protobuf;
 using Meshtastic.Protobufs;
+using Meshtastic.Utilities;
 
 namespace Meshtastic.Data.MessageFactories;
 
@@ -20,8 +21,9 @@ public MeshPacket CreateRouteDiscoveryPacket(uint channel = 0)
         {
             Channel = channel,
             To = dest!.Value,
-            Id = (uint)Math.Floor(Random.Shared.Next() * 1e9),
-            HopLimit = container?.GetHopLimitOrDefault() ?? 3,
+            Id = PacketUtils.GenerateRandomPacketId(),
+            HopLimit = container.GetHopLimitOrDefault(),
+            Priority = MeshPacket.Types.Priority.Reliable,
             Decoded = new Protobufs.Data()
             {
                 WantResponse = true,

Meshtastic/Utilities/PacketUtils.cs

@@ -0,0 +1,12 @@
+namespace Meshtastic.Utilities;
+
+public static class PacketUtils
+{
+    /// <summary>
+    /// Generate next random packet id
+    /// </summary>
+    public static uint GenerateRandomPacketId()
+    {
+        return (uint)Random.Shared.NextInt64(0, (long)uint.MaxValue + 1);
+    }
+}