From e5c756995e640ceb1bb1e70ff3bb60eb2356f10e Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Thu, 13 Aug 2026 00:47:02 -0400 Subject: [PATCH] Bridge: only push completed trace replies, with real per-hop SNRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut pushed TRACE_DATA for any received trace packet, so every trace read as an instant success: the dispatcher is promiscuous and fires the packet-received callback for strangers' traces and for our own outbound echoed back by the first repeater (same tag, still in flight). Two corrections: - Correlate against traces we originated. Wrap send_trace_path_raw (the one send seam the frame server calls) to remember each tag and its route, and push only a received trace whose tag we sent. Entries expire after 90s so a never-answered trace can't match a recycled tag. - Read the SNRs from the right place. A TRACE keeps its route (hop hashes) in the payload unchanged; the per-hop SNRs ride in the packet *header* path field, one signed byte (SNR*4) per hop. The earlier code waited for SNRs to appear in the payload, so it never fired. Now completeness is "the header has accumulated an SNR for every route hop", and that's the only thing we push — the finished reply, with genuine per-hop SNRs. Pending on-air verification (RF conditions poor at commit time); leaves verbose per-trace INFO logging in place for that test, to be trimmed once a clean multi-hop trace is confirmed. Co-Authored-By: Claude Opus 4.8 --- scripts/meshterm-spi-bridge | 101 ++++++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 28 deletions(-) diff --git a/scripts/meshterm-spi-bridge b/scripts/meshterm-spi-bridge index 9f6e901..0d4d8f7 100755 --- a/scripts/meshterm-spi-bridge +++ b/scripts/meshterm-spi-bridge @@ -680,13 +680,32 @@ def _wire_trace_push(companion, server, log) -> None: the hash/SNR blocks ``push_trace_data`` wants and push the standard 0x89 frame — exactly what the ``meshcore`` client on MeshTerm's side decodes. + Reading the SNRs is the subtle part. A TRACE packet keeps the *route* (the + hop hashes) in its payload, unchanged as it travels; the per-hop SNRs ride in + the packet **header's path field** instead — one signed byte (SNR × 4) appended + by each hop, which is where ``push_trace_data``'s SNR block and MeshTerm's + decoder both source them. So the payload never grows, and an earlier attempt + that waited for SNRs to appear *in the payload* never fired. + + A received TRACE packet is also not a reply just because it exists. The + dispatcher is promiscuous — it runs the trace handler and this callback for + every non-duplicate TRACE frame it overhears, including third parties' traces + and our own outbound echoed back by the first repeater (same tag, header path + still filling). Pushing on those makes MeshTerm's tag-filtered wait resolve + instantly against a half-walked trace, so *every* trace reads as success. So + we gate hard: remember the tag and route of every trace we originate + (``send_trace_path_raw`` is the one send seam the frame server calls), and push + only a received TRACE whose tag we sent *and* whose header has accumulated an + SNR for every hop of that route — the completed reply. + Upstream, the clean fix is the mirror of every other push: have - ``_on_packet_received`` fire ``trace_received`` for a TRACE packet and have + ``_on_packet_received`` fire ``trace_received`` for a completed trace and have ``frame_server._setup_push_callbacks`` register ``on_trace_received`` → ``push_trace_data``. Until a pymc_core release carries that, this shim stands (same spirit as :func:`_patch_pymc_ack_compat`). """ import struct + import time try: from pymc_core.protocol.constants import PAYLOAD_TYPE_TRACE @@ -699,40 +718,66 @@ def _wire_trace_push(companion, server, log) -> None: log.warning("trace push not wired (no dispatcher)") return + orig_send = getattr(companion, "send_trace_path_raw", None) + if orig_send is None: + log.warning("trace push not wired (no send_trace_path_raw)") + return + + # tag -> (flags, path_bytes, deadline). Only a trace WE originated, walked in + # full, becomes a push. Entries expire so a never-answered trace can't later + # match a recycled tag. + SENT_TTL_S = 90.0 + sent: dict = {} + + def _prune(now: float) -> None: + for tag in [t for t, (_f, _p, dl) in sent.items() if dl < now]: + sent.pop(tag, None) + + async def send_trace_path_raw(tag, auth_code, flags, path_bytes, _orig=orig_send): + now = time.monotonic() + _prune(now) + key = tag & 0xFFFFFFFF + sent[key] = (flags, bytes(path_bytes), now + SENT_TTL_S) + log.info( + "trace sent tag=0x%08X flags=0x%02x path=%s", + key, flags, bytes(path_bytes).hex().upper(), + ) + return await _orig(tag, auth_code, flags, path_bytes) + + companion.send_trace_path_raw = send_trace_path_raw + prev = getattr(companion, "_on_packet_received", None) def push_reply(pkt) -> None: payload = bytes(pkt.payload or b"") if len(payload) < 9: - log.debug("trace reply too short (%d bytes)", len(payload)) return tag, auth_code, flags = struct.unpack_from(" None: if prev is not None: @@ -748,7 +793,7 @@ def _wire_trace_push(companion, server, log) -> None: log.warning("trace push failed: %s", exc) dispatcher.set_packet_received_callback(on_packet) - log.info("compat: TRACE replies now push PUSH_CODE_TRACE_DATA to the client") + log.info("compat: completed TRACE replies push PUSH_CODE_TRACE_DATA to the client") def run_bridge(host: str, port: int, node_name: str) -> int: