diff --git a/CHANGELOG.md b/CHANGELOG.md index 5463baa1..7f1bd8d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Notable changes for Hessenbot (Meshhessen). Format inspired by [Keep a Changelog ### Fixed +- **`!trace`**: ROUTING_APP-NAKs (z. B. mit `route_request`) nicht mehr als `RouteDiscovery` parsen — verhindert `Trace fehlgeschlagen: Error parsing message with type 'meshtastic.protobuf.RouteDiscovery'`; klarere DE-Hinweise bei Routing-Fehler / unlesbarer Antwort - DM: `!ping` und `ping` beide gültig (`messageTrap` strippt optionales `!`) - Diverse Admin-/Dashboard- und Hop-/MQTT-Korrekturen (siehe Commits seit 1.1.0) diff --git a/modules/test_trace.py b/modules/test_trace.py new file mode 100644 index 00000000..ea2b5e1d --- /dev/null +++ b/modules/test_trace.py @@ -0,0 +1,146 @@ +# Unit tests for !trace formatting (no radio). +from __future__ import annotations + +import os +import sys +import unittest +from unittest.mock import patch + +parent_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) +sys.path.insert(0, parent_path) + + +class TestTraceFormat(unittest.TestCase): + @classmethod + def setUpClass(cls): + # settings.py may create an empty config.ini on first import; seed from template. + cfg = os.path.join(parent_path, "config.ini") + tpl = os.path.join(parent_path, "config.template") + cls._had_cfg = None + if os.path.isfile(cfg): + with open(cfg, "rb") as f: + cls._had_cfg = f.read() + if os.path.isfile(tpl): + with open(tpl, "rb") as src, open(cfg, "wb") as dst: + dst.write(src.read()) + # Import after config exists so modules.system side effects succeed. + import modules.trace as trace # noqa: F401 + + cls.trace = trace + + @classmethod + def tearDownClass(cls): + cfg = os.path.join(parent_path, "config.ini") + if cls._had_cfg is None: + try: + os.remove(cfg) + except OSError: + pass + else: + with open(cfg, "wb") as f: + f.write(cls._had_cfg) + + def setUp(self): + self._label = patch.object( + self.trace, + "_node_label", + side_effect=lambda n, _i: f"N{int(n):08x}" if n is not None else "N?", + ) + self._local = patch.object(self.trace, "_local_node_num", return_value=0x100) + self._hops = patch.object(self.trace, "record_mesh_hops_from_trace") + self._label.start() + self._local.start() + self._hops.start() + self.addCleanup(self._label.stop) + self.addCleanup(self._local.stop) + self.addCleanup(self._hops.stop) + + def test_format_valid_route_discovery_payload(self): + from meshtastic import mesh_pb2 + + rd = mesh_pb2.RouteDiscovery() + rd.route.extend([0x22222222]) + rd.snr_towards.extend([8, 12]) # hop + final + payload = rd.SerializeToString() + packet = { + "from": 0x22222222, + "to": 0x100, + "hopStart": 3, + "decoded": {"portnum": "TRACEROUTE_APP", "payload": payload}, + } + text = self.trace.format_traceroute_packet(packet, node_int=1, dest_id=0x22222222) + self.assertIn("Trace →", text) + self.assertIn("Hin:", text) + self.assertNotIn("fehlgeschlagen", text) + self.assertNotIn("unlesbar", text) + + def test_format_prefers_predecoded_traceroute_dict(self): + packet = { + "from": 0xABCDEF01, + "to": 0x100, + "hopStart": 2, + "decoded": { + "portnum": "TRACEROUTE_APP", + # Corrupt payload must be ignored when traceroute dict is present. + "payload": b"\xff\xff\xff\xff", + "traceroute": { + "route": [], + "snrTowards": [16], + "raw": object(), + }, + }, + } + text = self.trace.format_traceroute_packet(packet, node_int=1, dest_id=0xABCDEF01) + self.assertIn("Direktverbindung", text) + + def test_routing_app_max_retransmit_not_parsed_as_route_discovery(self): + from meshtastic import mesh_pb2 + + routing = mesh_pb2.Routing() + routing.error_reason = mesh_pb2.Routing.Error.MAX_RETRANSMIT + # Nested route_request is what historically DecodeError'd as RouteDiscovery. + routing.route_request.route.extend([0x11111111]) + + packet = { + "from": 0x1, + "to": 0x100, + "decoded": { + "portnum": "ROUTING_APP", + "payload": routing.SerializeToString(), + "routing": {"errorReason": "MAX_RETRANSMIT"}, + }, + } + text = self.trace.format_traceroute_packet(packet, node_int=1, dest_id=0xABCDEF01) + self.assertIn("Routing-Fehler", text) + self.assertIn("MAX_RETRANSMIT", text) + self.assertNotIn("Error parsing message", text) + self.assertNotIn("Wire format", text) + + def test_corrupt_payload_returns_german_unreadable_message(self): + packet = { + "from": 0x1, + "to": 0x100, + "decoded": { + "portnum": "TRACEROUTE_APP", + "payload": b"\xff\xff\xff\xff", + }, + } + text = self.trace.format_traceroute_packet(packet, node_int=1, dest_id=0xABCDEF01) + self.assertIn("Antwort unlesbar", text) + self.assertNotIn("Error parsing message", text) + + def test_routing_route_request_bytes_alone_would_decodeerror(self): + """Document root cause: Routing.route_request ≠ RouteDiscovery wire layout.""" + from meshtastic import mesh_pb2 + from google.protobuf.message import DecodeError + + routing = mesh_pb2.Routing() + routing.route_request.route.extend([0x11111111]) + payload = routing.SerializeToString() + with self.assertRaises(DecodeError) as ctx: + mesh_pb2.RouteDiscovery().ParseFromString(payload) + self.assertIn("RouteDiscovery", str(ctx.exception)) + + +if __name__ == "__main__": + unittest.main() diff --git a/modules/trace.py b/modules/trace.py index 8b3d21d4..49b67616 100644 --- a/modules/trace.py +++ b/modules/trace.py @@ -2,13 +2,15 @@ from __future__ import annotations +import base64 import queue import threading import time from dataclasses import dataclass -from typing import Any, Dict +from typing import Any, Dict, Optional, Tuple import google.protobuf.json_format +from google.protobuf.message import DecodeError from meshtastic import mesh_pb2, portnums_pb2 from modules.log import logger @@ -27,6 +29,23 @@ _TRACE_INTERFACE_COOLDOWN_S = 65.0 _TRACE_RESULT_MIN_GAP_S = 5.0 # min seconds after job start before result DM (after ack) +# ROUTING_APP NAK reasons we map to short DE hints (unknown → raw reason). +_ROUTING_ERROR_HINTS: Dict[str, str] = { + "NO_ROUTE": "keine Route", + "TIMEOUT": "Zeitüberschreitung", + "MAX_RETRANSMIT": "MAX_RETRANSMIT", + "NO_RESPONSE": "keine Antwort", + "NO_CHANNEL": "Kanal unbekannt/ohne Key", + "NO_INTERFACE": "kein Interface", + "GOT_NAK": "NAK", + "TOO_LARGE": "Paket zu groß", + "DUTY_CYCLE_LIMIT": "Duty-Cycle", + "RATE_LIMIT_EXCEEDED": "Rate-Limit", + "PKI_FAILED": "PKI fehlgeschlagen", + "PKI_UNKNOWN_PUBKEY": "unbekannter Public Key", + "NOT_AUTHORIZED": "nicht autorisiert", +} + @dataclass(frozen=True) class _TraceJob: @@ -75,6 +94,77 @@ def _local_node_num(node_int: int) -> int | None: return None +def _coerce_payload_bytes(value: Any) -> Optional[bytes]: + """Normalize decoded.payload to bytes (library may leave base64 str).""" + if value is None: + return None + if isinstance(value, (bytes, bytearray, memoryview)): + return bytes(value) + if isinstance(value, str): + s = value.strip() + if not s: + return b"" + try: + return base64.b64decode(s, validate=True) + except Exception: + return None + return None + + +def _routing_error_reason(decoded: Dict[str, Any]) -> Optional[str]: + """Return non-NONE ROUTING errorReason, or None if not a routing NAK.""" + routing = decoded.get("routing") + if not isinstance(routing, dict): + return None + reason = routing.get("errorReason") or routing.get("error_reason") + if reason is None or str(reason) in ("", "NONE", "0"): + return None + return str(reason) + + +def _is_routing_app(decoded: Dict[str, Any]) -> bool: + return str(decoded.get("portnum") or "") == "ROUTING_APP" + + +def _format_routing_failure(dest_label: str, reason: str) -> str: + hint = _ROUTING_ERROR_HINTS.get(reason, reason) + return f"Trace zu {dest_label}: Routing-Fehler ({hint})." + + +def _route_discovery_dict( + packet: dict, +) -> Tuple[Optional[Dict[str, Any]], Optional[str]]: + """ + Extract RouteDiscovery as a MessageToDict-style dict. + + Prefers the library-decoded ``decoded.traceroute`` map (avoids re-parse). + Falls back to ParseFromString on payload bytes. + + Returns (as_dict, error_code). error_code is set on failure: + empty | decode | missing + """ + decoded = packet.get("decoded") if isinstance(packet.get("decoded"), dict) else {} + pre = decoded.get("traceroute") + if isinstance(pre, dict): + # Drop non-JSON ``raw`` protobuf object from meshtastic handler. + return {k: v for k, v in pre.items() if k != "raw"}, None + + payload = _coerce_payload_bytes(decoded.get("payload")) + if payload is None: + return None, "missing" + if not payload: + return None, "empty" + + route_discovery = mesh_pb2.RouteDiscovery() + try: + route_discovery.ParseFromString(payload) + except DecodeError: + return None, "decode" + + as_dict = google.protobuf.json_format.MessageToDict(route_discovery) + return as_dict, None + + def _trace_reached_dest( packet: dict, dest_id: int, @@ -100,32 +190,55 @@ def _trace_reached_dest( def format_traceroute_packet(packet: dict, node_int: int, dest_id: int) -> str: - """Format a TRACEROUTE_APP response packet for mesh DM.""" + """Format a TRACEROUTE_APP (or ROUTING_APP NAK) response packet for mesh DM.""" if not packet: return "Trace: keine Antwort vom Netz." - payload = (packet.get("decoded") or {}).get("payload") - if not payload: - return "Trace: leere Antwort." - - route_discovery = mesh_pb2.RouteDiscovery() - route_discovery.ParseFromString(payload) - as_dict = google.protobuf.json_format.MessageToDict(route_discovery) - dest_label = _node_label(dest_id, node_int) - dest_s = int(dest_id) - my_num = _local_node_num(node_int) + decoded = packet.get("decoded") if isinstance(packet.get("decoded"), dict) else {} + + # wantResponse NAKs/ACKs arrive as ROUTING_APP; never parse those as RouteDiscovery + # (nested route_request wire bytes → DecodeError on RouteDiscovery). + if _is_routing_app(decoded): + routing_reason = _routing_error_reason(decoded) + if routing_reason is not None: + logger.info( + f"Trace: routing error dest={dest_label} reason={routing_reason} " + f"from={packet.get('from')} to={packet.get('to')}" + ) + return _format_routing_failure(dest_label, routing_reason) + return ( + f"Trace zu {dest_label}: Ziel nicht erreicht " + "(Routing-ACK ohne Route — ggf. zu weit, offline oder MAX_RETRANSMIT)." + ) + + as_dict, err = _route_discovery_dict(packet) + if err == "empty": + return "Trace: leere Antwort." + if err == "missing": + return "Trace: leere Antwort." + if err == "decode" or as_dict is None: + logger.info( + f"Trace: RouteDiscovery decode failed dest={dest_label} " + f"portnum={decoded.get('portnum')} " + f"payload_type={type(decoded.get('payload')).__name__}" + ) + return ( + f"Trace zu {dest_label}: Antwort unlesbar " + "(Protokoll/Firmware — bitte meshtastic-Lib und Node-FW prüfen)." + ) pkt_to = packet.get("to") pkt_from = packet.get("from") + my_num = _local_node_num(node_int) route = as_dict.get("route") or [] - snr_towards = as_dict.get("snrTowards") or [] + snr_towards = as_dict.get("snrTowards") or as_dict.get("snr_towards") or [] len_towards = len(route) snr_t_valid = len(snr_towards) == len_towards + 1 - route_back = as_dict.get("routeBack") or [] - snr_back = as_dict.get("snrBack") or [] + route_back = as_dict.get("routeBack") or as_dict.get("route_back") or [] + snr_back = as_dict.get("snrBack") or as_dict.get("snr_back") or [] len_back = len(route_back) if not _trace_reached_dest(packet, dest_id, route, route_back, node_int): @@ -243,13 +356,26 @@ def _run_trace_job(job: _TraceJob) -> None: import modules.settings as st job_started = time.monotonic() + dest_label = _node_label(job.dest_id, job.node_int) try: result = run_traceroute(job.dest_id, job.node_int, job.channel) + except DecodeError as e: + # Should be handled in format_traceroute_packet; keep a clear DE fallback. + logger.debug(f"Trace: DecodeError dest={job.dest_id} requester={job.requester_id}: {e}") + result = ( + f"Trace zu {dest_label}: Antwort unlesbar " + "(Protokoll/Firmware — bitte meshtastic-Lib und Node-FW prüfen)." + ) except Exception as e: logger.debug(f"Trace: failed dest={job.dest_id} requester={job.requester_id}: {e}") err = str(e).strip() if "Timed out" in err or "timeout" in err.lower(): - result = f"Trace zu {_node_label(job.dest_id, job.node_int)}: Zeitüberschreitung (keine Antwort)." + result = f"Trace zu {dest_label}: Zeitüberschreitung (keine Antwort)." + elif "RouteDiscovery" in err or "Wire format was corrupt" in err: + result = ( + f"Trace zu {dest_label}: Antwort unlesbar " + "(Protokoll/Firmware — bitte meshtastic-Lib und Node-FW prüfen)." + ) else: result = f"Trace fehlgeschlagen: {err[:120]}" min_gap = max(