From c819fdfd2f4d1c43ac0a53b65e39c774bbcb0f09 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Sat, 11 Jul 2026 00:24:16 -0400 Subject: [PATCH] Auto traces walk proven routes: the device almost never has one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An auto (device-routed) Trace target went on the air as a bare destination hash — an observer confirmed no repeater hops — so any target past a direct neighbour could never answer. Dumping the radio's contact table showed why: every contact reports out_path_len -1 (flood). MeshCore firmware only learns a contact's out_path from two-way addressed traffic, and tracing repeaters teaches it nothing, so "device-routed" has nothing to route with. Auto now resolves a route the session can prove, in order: the device's learned route when one genuinely exists, else the outbound leg of the last successful stored walk (_previous_outbound extracts it from the recorded boomerang), else a bare direct attempt. One resolver feeds both the wire and the screen, so the planned route line renders exactly what Trace will transmit, with its provenance under it — (auto · device route), (auto · last trace · stamp), or (auto · direct — no known route) — and the summary's path row says the same instead of claiming a device route that never existed. The connection layer's path-less fallback keeps the mirrored return leg for contacts that do carry a learned route (the trace protocol reflects nothing home), and its docstring now says the firmware never routes a trace itself. Resolution verified against the real database and radio: a multi-hop target resolves to its full stored boomerang, the direct gateway stays a single-hop spec. Co-Authored-By: Claude Fable 5 --- meshterm/core/connection.py | 34 ++++++--- meshterm/ui/trace_screen.py | 142 +++++++++++++++++++++++++++++++----- tests/test_live_screens.py | 83 ++++++++++++++++++++- tests/test_skeleton.py | 13 ++-- 4 files changed, 235 insertions(+), 37 deletions(-) diff --git a/meshterm/core/connection.py b/meshterm/core/connection.py index e4d704b..3664997 100644 --- a/meshterm/core/connection.py +++ b/meshterm/core/connection.py @@ -409,7 +409,9 @@ class Device(ABC): target: Name or key prefix of the destination node. path: Optional explicit path to force, as a comma-separated string of single-byte hex key prefixes (e.g. ``"3d,f2,3d"``). When ``None`` - the device chooses the path. + the connection resolves one from the contact's learned route (the + firmware itself never routes a trace — an explicit path is all it + walks), falling back to path-less only for unknown targets. timeout: Seconds to wait for the trace reply. Returns: @@ -1498,11 +1500,17 @@ class MeshCoreDevice(Device): A trace reply only comes back when the *destination's own hash* is the final hop in the path — an empty/destination-less path is silently dropped (verified on hardware: a direct neighbor answers a single-hop trace to its own hash but - not a path-less one). So we always end the path at the contact's key prefix, - prepending any learned repeater hops (``out_path``) ahead of it: + not a path-less one). So we always end the outbound leg at the contact's key + prefix, prepending any learned repeater hops (``out_path``) ahead of it — and, + since the trace protocol has no separate return-path field, mirror those same + repeaters back afterwards (see :func:`~meshterm.services.topology.render_forced_spec`, + which does the same for a composed/adopted path): without an explicit return + leg the repeaters have nothing to relay the reply through, so it never comes + home. - * direct neighbor / no learned route → just ``[destination]``; - * learned multi-hop route → ``[repeater…, destination]``. + * direct neighbor / no learned route → just ``[destination]`` (no repeaters + to mirror, so the outbound leg is the whole path); + * learned multi-hop route → ``[repeater…, destination, repeater… (reversed)]``. Each hash is re-encoded at the trace's own width (``1 << flags``, only 1/2/4/8 bytes), collapsing a region's routing width (e.g. 3) to the widest representable @@ -1541,17 +1549,21 @@ class MeshCoreDevice(Device): size = max(mode + 1, 1) trace_size = max(s for s in (1, 2, 4, 8) if s <= size) - hops: list[bytes] = [] + repeaters: list[bytes] = [] out_path = (info.get("out_path") or "").strip().lower().removeprefix("0x") out_path_len = int(info.get("out_path_len", -1)) if 1 <= out_path_len <= 254 and out_path: # Learned multi-hop route: walk each repeater, collapsed to trace width. route = bytes.fromhex(out_path)[: out_path_len * size] - hops += [route[i * size : i * size + trace_size] for i in range(out_path_len)] - # Always finish at the destination's own hash so the target recognizes the - # trace and replies; for a direct neighbor this single hop is the whole path. - hops.append(bytes.fromhex(pub)[:trace_size]) - return b"".join(hops), path_hash_flags(trace_size) or 0 + repeaters = [ + route[i * size : i * size + trace_size] for i in range(out_path_len) + ] + dest = bytes.fromhex(pub)[:trace_size] + # The outbound leg always finishes at the destination's own hash so it + # recognizes the trace and replies; the return leg mirrors the same + # repeaters back to us, since nothing reflects the packet automatically. + path_bytes = b"".join(repeaters) + dest + b"".join(reversed(repeaters)) + return path_bytes, path_hash_flags(trace_size) or 0 return None async def subscribe_events( # noqa: D102 - inherited docstring diff --git a/meshterm/ui/trace_screen.py b/meshterm/ui/trace_screen.py index 3aca37a..81a93a5 100644 --- a/meshterm/ui/trace_screen.py +++ b/meshterm/ui/trace_screen.py @@ -43,8 +43,10 @@ whatever you make it, and nothing transmits until you say so. An action list dri screen keeps aggregating every trace of the session into its medians. * **Back** leaves the screen, exactly like Esc. -Layout, top to bottom: the walked route (live when a reply has landed, else the planned -composed path, else the most recent stored trace), the run's robust aggregates, the +Layout, top to bottom: the walked route (live when a reply has landed, else the route +the next Trace will walk — composed by hand, or auto-resolved from the device's learned +route or the stored history and labelled with that provenance — else the most recent +stored trace), the run's robust aggregates, the action list, per-hop median SNR with quality bars, and the individual traces newest-first. The body scrolls with PgUp/PgDn/Home/End (↑/↓ belong to the action cursor, which only pins the view while it is actually being moved). @@ -68,6 +70,7 @@ from rich.text import Text from ..core.models import PATH_TRACE_TARGET, TraceResult, TraceStats from ..services import trace_runner +from ..services.topology import render_forced_spec from .theme import snr_style from .tui.render import render_lines, render_to_ansi from .tui.screen import Screen @@ -235,6 +238,8 @@ class TraceScreen(Screen): explore: Optional[PathFlow] = None, pace_s: float = 1.0, previous: Optional[TraceResult] = None, + auto_spec: Callable[[], str] = lambda: "", + auto_source: str = "", ) -> None: """Create the screen (nothing transmits until the user commits Trace). @@ -265,6 +270,13 @@ class TraceScreen(Screen): sampling session never reads as a burst to the repeaters. previous: The most recent stored trace, if any — its route seeds the route line so the screen opens knowing the path history last saw. + auto_spec: Renders the spec an *auto* trace (no composed path) actually + walks right now — the session resolves it from the device's learned + route or the stored history at the current width. ``""`` means a + path-less trace (unaddressable target, or a path walk). + auto_source: Short provenance of the auto route (e.g. ``device route``, + ``last trace · Jul 09 14:32``) for the route line and summary, so + the screen never claims a route the radio wasn't given. """ super().__init__() self.title = f"trace · {target}" if mode == "target" else "trace path" @@ -284,6 +296,8 @@ class TraceScreen(Screen): self._sample_count = sample_count self._pace_s = pace_s self._previous = previous + self._auto_spec = auto_spec + self._auto_source = auto_source self._path_spec = "" self._traces: list[TraceResult] = [] self._running = False @@ -579,17 +593,32 @@ class TraceScreen(Screen): line.append("unknown — press Enter to trace", style="muted") return line - def _planned_route(self) -> Optional[Text]: - """The composed path as a route preview, or ``None`` without one. + def _effective_spec(self) -> tuple[str, bool]: + """The wire spec the next Trace walks, and whether auto resolution supplied it. - ``_path_spec`` is the literal wire spec — the whole walk, since the trace - protocol has no separate return-path field. In target mode the spec is the - symmetric boomerang (outbound hops, the target, those hops mirrored): its - second half is dimmed, reading as "this part isn't yours to compose". A path - walk was hand-composed hop by hop, so every hop renders in full colour and - only the automatic landing back on us stays faint. + A composed/adopted spec wins verbatim; with none, target mode asks the + session's auto resolver what it would force right now (the device's learned + route, or the stored history) — the same call :func:`_open_session`'s + ``trace_once`` makes, so the route on screen is the route on the air. """ - tokens = [h.strip() for h in self._path_spec.split(",") if h.strip()] + if self._path_spec: + return self._path_spec, False + spec = self._auto_spec() if self._mode == "target" else "" + return spec, bool(spec) + + def _planned_route(self) -> Optional[Text]: + """The route the next Trace walks as a preview, or ``None`` without one. + + Renders the literal wire spec — the whole walk, since the trace protocol + has no separate return-path field. In target mode the spec is the symmetric + boomerang (outbound hops, the target, those hops mirrored): its second half + is dimmed, reading as "this part isn't yours to compose". A path walk was + hand-composed hop by hop, so every hop renders in full colour and only the + automatic landing back on us stays faint. An auto-resolved spec carries a + faint provenance line naming where the route came from. + """ + spec, auto = self._effective_spec() + tokens = [h.strip() for h in spec.split(",") if h.strip()] if not tokens: return None if self._mode == "target" and len(tokens) % 2 and tokens == tokens[::-1]: @@ -606,6 +635,10 @@ class TraceScreen(Screen): text.append_text(self._planned_hop_text(hop, dim=True)) text.append(" → ", style="faint") text.append(self._device_label, style="faint") + if auto and self._auto_source: + # On its own line (like the previous-trace stamp) so a long route + # never squeezes the provenance off the right edge. + text.append(f"\n(auto · {self._auto_source})", style="faint") return text def _planned_hop_text(self, hop: str, *, dim: bool) -> Text: @@ -636,7 +669,8 @@ class TraceScreen(Screen): """ if current is not None: return sum(1 for h in current.hops if h.node) - tokens = [h for h in self._path_spec.split(",") if h.strip()] + spec, _ = self._effective_spec() + tokens = [h for h in spec.split(",") if h.strip()] if tokens: return len(tokens) if self._previous is not None and self._previous.hops: @@ -661,8 +695,10 @@ class TraceScreen(Screen): summary.append(self._path_spec, style="brand") elif self._mode == "path": summary.append("none — compose a path first", style="muted") + elif self._effective_spec()[1] and self._auto_source: + summary.append(f"auto · {self._auto_source}", style="muted") else: - summary.append("auto (device-routed)", style="muted") + summary.append("auto — path-less (unknown target)", style="muted") hops = self._displayed_hop_count(current) if hops is not None: summary.append(f" · {hops} hop{'s' if hops != 1 else ''}", style="muted") @@ -738,6 +774,38 @@ def _collapse_trace_width(mode: int) -> int: return max(s for s in (1, 2, 4, 8) if s <= size) +def _previous_outbound( + previous: Optional[TraceResult], target_hash: str +) -> Optional[tuple[str, ...]]: + """Extract the outbound repeaters from the last successful walk to a target. + + A target-mode trace walks the symmetric boomerang, so its stored hop hashes + (the final hash-less hop is us) read ``[out…, target, out reversed…]`` — an + odd-length palindrome whose middle entry is the target. When the stored walk + has that shape, its first half is a route the mesh has already proven, ready + to force again. + + Args: + previous: The most recent successful stored trace, if any. + target_hash: The target's full hex hash, to confirm the walk really + turned at this target (hop hashes are prefixes of it). + + Returns: + The outbound repeater hashes in order from us outward (empty = the + target answered directly), or ``None`` when there is no stored walk or + it isn't a recognizable boomerang. + """ + if previous is None or not previous.success: + return None + tokens = [h.node.lower() for h in previous.hops if h.node] + if not tokens or len(tokens) % 2 == 0 or tokens != tokens[::-1]: + return None + mid = len(tokens) // 2 + if not target_hash.lower().startswith(tokens[mid]): + return None + return tuple(tokens[:mid]) + + async def open_trace(ctx: "AppContext", target: str) -> int: """Open the live *Trace target* screen for ``target`` and run it until dismissed. @@ -879,6 +947,42 @@ async def _open_session(ctx: "AppContext", target: Optional[str]) -> int: neighbour_links=ctx.repo.neighbour_links(), ) + previous = ctx.repo.latest_trace(record_target) + + # What an *auto* trace (no composed path) forces on the air, and where that + # route came from. A trace only replies when its destination is the path's + # final outbound hop and nothing reflects it home, so auto must always spell + # out a full boomerang — but the device rarely has one to offer: firmware + # only learns a contact's ``out_path`` from two-way addressed traffic + # (verified on hardware: every repeater contact reported ``out_path_len`` + # -1, flood), so a device-routed trace to anything further than a direct + # neighbour would go out with no repeaters and die. Precedence: the device's + # learned route when it genuinely has one, else the outbound leg of the last + # successful stored walk (the route the screen shows), else the bare + # destination — a direct attempt, honest about being one. + device_route: Optional[tuple[str, ...]] = None + if target_contact is not None and target_contact.route_hops is not None: + topo0 = fresh_topology() + device_route = tuple(topo0.canonical(h) or h for h in target_contact.route_hops) + auto_hops: Optional[tuple[str, ...]] = None + auto_source = "" + if target_hash is not None: + if device_route is not None: + auto_hops, auto_source = device_route, "device route" + else: + auto_hops = _previous_outbound(previous, target_hash) + if auto_hops is not None and previous is not None: + stamp = previous.timestamp.astimezone().strftime("%b %d %H:%M") + auto_source = f"last trace · {stamp}" + else: + auto_hops, auto_source = (), "direct — no known route" + + def auto_spec() -> str: + """The spec auto forces at the session's current width (``""`` = path-less).""" + if target_hash is None or auto_hops is None: + return "" + return render_forced_spec(auto_hops, target_hash, width_bytes) + async def unaddressable() -> None: """Explain why target-mode path features need a resolvable target.""" await session.message_dialog( @@ -1294,11 +1398,6 @@ async def _open_session(ctx: "AppContext", target: Optional[str]) -> int: return None topo = fresh_topology() target_id = topo.canonical(target_hash) or target_hash[:12] - device_route: Optional[tuple[str, ...]] = None - if target_contact is not None and target_contact.route_hops is not None: - device_route = tuple( - topo.canonical(h) or h for h in target_contact.route_hops - ) scenarios = topo.scenarios(target_id, device_route=device_route) if not scenarios: await session.message_dialog( @@ -1370,7 +1469,8 @@ async def _open_session(ctx: "AppContext", target: Optional[str]) -> int: The screen calls this once per sample; multi-trace runs are paced there, so the repeaters never see a burst regardless of the chosen count. """ - path = trace_runner.parse_trace_path(path_spec, contacts) if path_spec.strip() else None + spec = path_spec.strip() or auto_spec() + path = trace_runner.parse_trace_path(spec, contacts) if spec else None params: dict[str, Any] = {"target": record_target} if path: params["path"] = path @@ -1410,7 +1510,9 @@ async def _open_session(ctx: "AppContext", target: Optional[str]) -> int: width_bytes=lambda: width_bytes, sample_count=lambda: sample_count, pace_s=ctx.settings.trace_cooldown_s, - previous=ctx.repo.latest_trace(record_target), + previous=previous, + auto_spec=auto_spec, + auto_source=auto_source, ) try: await session.run_screen(screen) diff --git a/tests/test_live_screens.py b/tests/test_live_screens.py index 9eea111..59cf682 100644 --- a/tests/test_live_screens.py +++ b/tests/test_live_screens.py @@ -68,12 +68,57 @@ def test_snr_bar_clamps_out_of_range_readings() -> None: assert snr_bar(-99.0).plain.count("▆") == 1 # a heard hop always shows something +# --- _previous_outbound ----------------------------------------------------------- + + +def _walk(*nodes: str, success: bool = True) -> TraceResult: + """A stored walk whose hop hashes are ``nodes`` (plus the final hash-less us).""" + hops = [Hop(index=i, node=n, snr=1.0) for i, n in enumerate(nodes)] + hops.append(Hop(index=len(nodes), node=None, snr=1.0)) + return TraceResult( + target="Alice", success=success, hops=hops, round_trip_ms=200.0, path_hash_bytes=2 + ) + + +def test_previous_outbound_extracts_the_proven_route() -> None: + """The last successful boomerang's first half is the reusable outbound leg. + + Verified on hardware that the device itself almost never has a learned route + (contacts report flood), so this stored evidence is what auto mode actually + walks for a multi-hop target. + """ + from meshterm.ui.trace_screen import _previous_outbound + + walk = _walk("3d63", "f2c2", "aabb", "f2c2", "3d63") + assert _previous_outbound(walk, "aabb" + "00" * 30) == ("3d63", "f2c2") + + +def test_previous_outbound_direct_walk_yields_no_repeaters() -> None: + """A direct answer (target only) extracts an empty outbound leg — dest-only again.""" + from meshterm.ui.trace_screen import _previous_outbound + + assert _previous_outbound(_walk("aabb"), "aabb" + "00" * 30) == () + + +def test_previous_outbound_rejects_unusable_history() -> None: + """Failures, asymmetric walks, and walks that turned elsewhere are never reused.""" + from meshterm.ui.trace_screen import _previous_outbound + + target = "aabb" + "00" * 30 + assert _previous_outbound(None, target) is None + assert _previous_outbound(_walk("3d63", "aabb", "3d63", success=False), target) is None + # Asymmetric: came home a different way — not a boomerang to this target. + assert _previous_outbound(_walk("3d63", "aabb", "f2c2"), target) is None + # Palindromic, but it turned at some other node, not our target. + assert _previous_outbound(_walk("3d63", "9999", "3d63"), target) is None + + # --- TraceScreen ---------------------------------------------------------------- def _trace_screen( trace=None, compose_path=None, explore=None, pick_width=None, pick_samples=None, - previous=None, mode="target", samples=1, + previous=None, mode="target", samples=1, auto_spec=None, auto_source="", ) -> tuple[TraceScreen, _FakeSession]: session = _FakeSession() @@ -99,6 +144,8 @@ def _trace_screen( sample_count=lambda: samples, pace_s=0.0, # tests never sleep; pacing is asserted through the statuses previous=previous, + auto_spec=auto_spec or (lambda: ""), + auto_source=auto_source, ) return screen, session @@ -359,6 +406,40 @@ def test_summary_appends_the_displayed_hop_count() -> None: assert "· 2 hops" in _plain(screen.render_body(100)) +def test_auto_resolved_route_renders_with_its_provenance() -> None: + """With no composed path, the auto route shows as the plan, labelled with its source. + + What the screen draws is exactly what Trace will put on the air (both read the + same resolver), so the user can see the forced boomerang — and where it came + from — before committing a transmission. + """ + screen, _ = _trace_screen( + auto_spec=lambda: "3d63,f2c2,aabb,f2c2,3d63", auto_source="last trace · Jul 09 14:32" + ) + plan = screen._planned_route().plain + assert "Us → 3d63 → f2c2 → aabb → f2c2 → 3d63 → Us" in plan + assert "(auto · last trace · Jul 09 14:32)" in plan + body = _plain(screen.render_body(100)) + assert "auto · last trace · Jul 09 14:32" in body # the summary's path row + assert "· 5 hops" in body + + +def test_composed_path_outranks_the_auto_route() -> None: + """A hand-composed spec replaces the auto plan everywhere — display and wire.""" + screen, _ = _trace_screen(auto_spec=lambda: "aabb", auto_source="device route") + screen._path_spec = "3d63,aabb,3d63" + plan = screen._planned_route().plain + assert "Us → 3d63 → aabb → 3d63 → Us" in plan + assert "(auto ·" not in plan + assert screen._effective_spec() == ("3d63,aabb,3d63", False) + + +def test_unaddressable_target_reads_as_path_less_auto() -> None: + """With nothing to force (no target hash), the summary says so instead of lying.""" + screen, _ = _trace_screen() # auto_spec resolves "" + assert "auto — path-less (unknown target)" in _plain(screen.render_body(100)) + + def test_trace_log_section_hidden_until_there_is_something_to_log() -> None: """Idle with no traces, the Traces heading (and its old hint) don't render.""" screen, _ = _trace_screen() diff --git a/tests/test_skeleton.py b/tests/test_skeleton.py index 9c81eba..752f0fa 100644 --- a/tests/test_skeleton.py +++ b/tests/test_skeleton.py @@ -492,12 +492,14 @@ def _mc_with(contacts: dict, path_hash_mode: int = 2): async def test_three_byte_route_appends_destination_hash() -> None: - """A learned multi-hop route walks the repeaters then ends at the target (#1). + """A learned multi-hop route walks the repeaters, the target, then back (#1). Two things matter for a non-power-of-two region (3-byte hashes): each 3-byte routing hash collapses to its leading 2 bytes (the widest representable trace - width), and the destination's own hash is appended as the final hop — a trace - only replies when its destination is the last hop. + width), and the destination's own hash is appended as the final outbound hop — + a trace only replies when its destination is the last outbound hop. The trace + protocol has no separate return-path field, so the same repeaters are mirrored + back afterwards — otherwise nothing relays the reply home. """ from meshterm.core.connection import MeshCoreDevice @@ -518,8 +520,9 @@ async def test_three_byte_route_appends_destination_hash() -> None: resolved = await device._trace_path_to_contact(mc, "Repeater") assert resolved is not None path_bytes, flags = resolved - # Two repeater hops collapsed to 2 bytes, then the target's own 2-byte hash. - assert path_bytes == bytes.fromhex("1122" "4455" "aabb") + # Two repeater hops collapsed to 2 bytes, the target's own 2-byte hash, then + # the same two repeaters mirrored back to us. + assert path_bytes == bytes.fromhex("1122" "4455" "aabb" "4455" "1122") assert flags == trace_runner.path_hash_flags(2) # 2-byte width