From 4490c9bb8c4657eadd84e8d8e69cd3c2be467438 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sun, 8 Mar 2026 17:23:20 -0700 Subject: [PATCH 1/4] Add packet recording for injection-only types in RepeaterHandler - Introduced `record_packet_only` method in `RepeaterHandler` to log packets for UI/storage without forwarding or duplicate checks, specifically for injection-only types like ANON_REQ and ACK. - Updated `PacketRouter` to call `_record_for_ui` method, ensuring that relevant metadata is recorded for packets processed by various handlers. - Enhanced handling of packet metadata, including RSSI and SNR values, to improve the visibility of packet information in the web UI. --- repeater/engine.py | 82 +++++++++++++++++++++++++++++++++++++++ repeater/packet_router.py | 27 +++++++++++++ 2 files changed, 109 insertions(+) diff --git a/repeater/engine.py b/repeater/engine.py index 0cb6083..90ead0b 100644 --- a/repeater/engine.py +++ b/repeater/engine.py @@ -11,6 +11,7 @@ from pymc_core.protocol import Packet from pymc_core.protocol.constants import ( MAX_PATH_SIZE, PAYLOAD_TYPE_ADVERT, + PAYLOAD_TYPE_ANON_REQ, PH_ROUTE_MASK, PH_TYPE_MASK, PH_TYPE_SHIFT, @@ -409,6 +410,87 @@ class RepeaterHandler(BaseHandler): if len(self.recent_packets) > self.max_recent_packets: self.recent_packets.pop(0) + def record_packet_only(self, packet: Packet, metadata: dict) -> None: + """Record a packet for UI/storage without running forwarding or duplicate logic. + + Used by the packet router for injection-only types (ANON_REQ, ACK, PATH, etc.) + so they still appear in the web UI. + """ + if not self.storage: + return + rssi = metadata.get("rssi", 0) + snr = metadata.get("snr", 0.0) + if not hasattr(packet, "header") or packet.header is None: + logger.debug("record_packet_only: packet missing header, skipping") + return + header_info = PacketHeaderUtils.parse_header(packet.header) + payload_type = header_info["payload_type"] + route_type = header_info["route_type"] + pkt_hash = packet.calculate_packet_hash().hex().upper() + original_path_hashes = packet.get_path_hashes_hex() + path_hash_size = packet.get_path_hash_size() + display_hashes = original_path_hashes + path_hash = None + if display_hashes: + display = display_hashes[:8] + if len(display_hashes) > 8: + display = list(display) + ["..."] + path_hash = "[" + ", ".join(display) + "]" + src_hash = None + dst_hash = None + if payload_type in [0x00, 0x01, 0x02, 0x08]: + if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 2: + dst_hash = f"{packet.payload[0]:02X}" + src_hash = f"{packet.payload[1]:02X}" + elif payload_type == PAYLOAD_TYPE_ADVERT: + if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 1: + src_hash = f"{packet.payload[0]:02X}" + elif payload_type == PAYLOAD_TYPE_ANON_REQ: + if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 1: + dst_hash = f"{packet.payload[0]:02X}" + packet_record = { + "timestamp": time.time(), + "header": f"0x{packet.header:02X}", + "payload": ( + packet.payload.hex() if hasattr(packet, "payload") and packet.payload else None + ), + "payload_length": ( + len(packet.payload) if hasattr(packet, "payload") and packet.payload else 0 + ), + "type": payload_type, + "route": route_type, + "length": len(packet.payload or b""), + "rssi": rssi, + "snr": snr, + "score": self.calculate_packet_score( + snr, len(packet.payload or b""), self.radio_config["spreading_factor"] + ), + "tx_delay_ms": 0.0, + "transmitted": False, + "is_duplicate": False, + "packet_hash": pkt_hash[:16], + "drop_reason": None, + "path_hash": path_hash, + "src_hash": src_hash, + "dst_hash": dst_hash, + "original_path": original_path_hashes or None, + "forwarded_path": None, + "path_hash_size": path_hash_size, + "raw_packet": packet.write_to().hex() if hasattr(packet, "write_to") else None, + "lbt_attempts": 0, + "lbt_backoff_delays_ms": None, + "lbt_channel_busy": False, + } + try: + self.storage.record_packet(packet_record, skip_letsmesh_if_invalid=False) + except Exception as e: + logger.error(f"Failed to store packet record (record_packet_only): {e}") + return + self.recent_packets.append(packet_record) + if len(self.recent_packets) > self.max_recent_packets: + self.recent_packets.pop(0) + self.rx_count += 1 + def cleanup_cache(self): now = time.time() diff --git a/repeater/packet_router.py b/repeater/packet_router.py index 843fc44..3a94466 100644 --- a/repeater/packet_router.py +++ b/repeater/packet_router.py @@ -69,6 +69,15 @@ class PacketRouter: self._companion_delivered[key] = now + _COMPANION_DEDUPE_TTL_SEC return True + def _record_for_ui(self, packet, metadata: dict) -> None: + """Record an injection-only packet for the web UI (storage + recent_packets).""" + handler = getattr(self.daemon, "repeater_handler", None) + if handler and getattr(handler, "storage", None): + try: + handler.record_packet_only(packet, metadata) + except Exception as e: + logger.debug("Record for UI failed: %s", e) + async def enqueue(self, packet): """Add packet to router queue.""" await self.queue.put(packet) @@ -124,6 +133,11 @@ class PacketRouter: payload_type = packet.get_payload_type() processed_by_injection = False + metadata = { + "rssi": getattr(packet, "rssi", 0), + "snr": getattr(packet, "snr", 0.0), + "timestamp": getattr(packet, "timestamp", 0), + } # Route to specific handlers for parsing only if payload_type == TraceHandler.payload_type(): @@ -132,6 +146,7 @@ class PacketRouter: await self.daemon.trace_helper.process_trace_packet(packet) # Skip engine processing for trace packets - they're handled by trace helper processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == ControlHandler.payload_type(): # Process control/discovery packet @@ -180,6 +195,8 @@ class PacketRouter: else: # Login request for remote repeater (we already TXed it via inject); don't treat as RX. processed_by_injection = True + if processed_by_injection: + self._record_for_ui(packet, metadata) elif payload_type == AckHandler.payload_type(): # ACK has no dest in payload (4-byte CRC only); deliver to all bridges so sender sees send_confirmed @@ -190,6 +207,7 @@ class PacketRouter: except Exception as e: logger.debug(f"Companion bridge ACK error: {e}") processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == TextMessageHandler.payload_type(): dest_hash = packet.payload[0] if packet.payload else None @@ -197,6 +215,7 @@ class PacketRouter: if dest_hash is not None and dest_hash in companion_bridges: await companion_bridges[dest_hash].process_received_packet(packet) processed_by_injection = True + self._record_for_ui(packet, metadata) elif self.daemon.text_helper: handled = await self.daemon.text_helper.process_text_packet(packet) if handled: @@ -209,6 +228,7 @@ class PacketRouter: if self._should_deliver_path_to_companions(packet): await companion_bridges[dest_hash].process_received_packet(packet) processed_by_injection = True + self._record_for_ui(packet, metadata) elif companion_bridges and self._should_deliver_path_to_companions(packet): # Dest not in bridges: path-return with ephemeral dest (e.g. multi-hop login). # Deliver to all bridges; each will try to decrypt and ignore if not relevant. @@ -223,6 +243,7 @@ class PacketRouter: len(companion_bridges), ) processed_by_injection = True + self._record_for_ui(packet, metadata) elif self.daemon.path_helper: await self.daemon.path_helper.process_path_packet(packet) @@ -244,6 +265,7 @@ class PacketRouter: except Exception as e: logger.debug(f"Companion bridge RESPONSE error: {e}") processed_by_injection = True + self._record_for_ui(packet, metadata) elif dest_hash == local_hash and companion_bridges: # Response addressed to this repeater (e.g. path-based reply to first hop) for bridge in companion_bridges.values(): @@ -257,6 +279,7 @@ class PacketRouter: len(companion_bridges), ) processed_by_injection = True + self._record_for_ui(packet, metadata) elif companion_bridges: # Dest not in bridges and not local: likely ANON_REQ response (dest = ephemeral # sender hash). Deliver to all bridges; each will try to decrypt and ignore if @@ -272,6 +295,7 @@ class PacketRouter: len(companion_bridges), ) processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == ProtocolResponseHandler.payload_type(): # PAYLOAD_TYPE_PATH (0x08): protocol responses (telemetry, binary, etc.). @@ -285,6 +309,7 @@ class PacketRouter: logger.debug(f"Companion bridge RESPONSE error: {e}") if companion_bridges: processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == ProtocolRequestHandler.payload_type(): dest_hash = packet.payload[0] if packet.payload else None @@ -292,10 +317,12 @@ class PacketRouter: if dest_hash is not None and dest_hash in companion_bridges: await companion_bridges[dest_hash].process_received_packet(packet) processed_by_injection = True + self._record_for_ui(packet, metadata) elif self.daemon.protocol_request_helper: handled = await self.daemon.protocol_request_helper.process_request_packet(packet) if handled: processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == GroupTextHandler.payload_type(): # GRP_TXT: pass to all companions (they filter by channel); still forward From 1002ba31945b41b9690d1c1382a7114191a2e86c Mon Sep 17 00:00:00 2001 From: agessaman Date: Sun, 8 Mar 2026 18:23:16 -0700 Subject: [PATCH 2/4] Refactor packet handling in RepeaterHandler and PacketRouter - Introduced helper methods `_path_hash_display` and `_packet_record_src_dst` in `RepeaterHandler` to streamline path hash and source/destination hash extraction. - Updated `record_packet` method to utilize a new `_build_packet_record` method for improved readability and maintainability. - Enhanced `PacketRouter` comments for clarity on handling remote destinations and packet processing, ensuring better understanding of the routing logic. --- docs/DIRECT_packets_not_forwarded.md | 66 ++++++++ repeater/engine.py | 239 ++++++++++++++------------- repeater/packet_router.py | 7 +- 3 files changed, 196 insertions(+), 116 deletions(-) create mode 100644 docs/DIRECT_packets_not_forwarded.md diff --git a/docs/DIRECT_packets_not_forwarded.md b/docs/DIRECT_packets_not_forwarded.md new file mode 100644 index 0000000..92f663b --- /dev/null +++ b/docs/DIRECT_packets_not_forwarded.md @@ -0,0 +1,66 @@ +# DIRECT packets not forwarded (router consumes them) + +## Summary + +Besides ANON_REQ (fixed), these payload types are **always** marked `processed_by_injection` in the router and **never** passed to the engine, so they are **never forwarded** even when this repeater is a middle hop on a DIRECT path: + +| Payload type | Router behavior | Can be DIRECT? | Should forward when middle hop? | +|---------------------|-----------------|-----------------|----------------------------------| +| **ACK** | Deliver to all companion bridges, set processed | Yes (return path) | Yes | +| **PATH** | Deliver to companion(s) or all bridges (anon), set processed | Yes (path response) | Yes | +| **LoginResponse** | Deliver to bridge(s), set processed | Yes (login response) | Yes | +| **ProtocolResponse**| Deliver to companions, set processed | Yes (telemetry etc.) | Yes | +| **Trace** | trace_helper only, set processed | Yes/No | No (diagnostic, not forwarded by design) | + +So **ACK, PATH, LoginResponse (RESPONSE), and ProtocolResponse** are DIRECT packet types that we should be forwarding when we're in the path but currently are not, because the router consumes them every time. + +## Detail by type + +### ACK (AckHandler) + +- **Router:** Delivers to all companion bridges (so sender sees send_confirmed), then sets `processed_by_injection = True`. Never passes to engine. +- **Use case:** ACKs travel back along the path. When we're a middle hop we should forward the ACK toward the sender. +- **Conclusion:** Should be passed to engine after companion delivery so DIRECT ACKs can be forwarded. + +### PATH (PathHandler) + +- **Router:** If dest in companion_bridges → deliver to that bridge, set processed. If dest not in bridges but we have companions → deliver to all bridges (anon), set processed. Only when path_helper runs (and no companion anon delivery) do we *not* set processed, so the packet can reach the engine. +- **Use case:** PATH responses come back along the path. When we're a middle hop we should forward. When we're final hop (dest in companion or we're path_helper) we should not forward. +- **Conclusion:** When we deliver to companions we still need to pass to engine so we can forward when we're a middle hop. Today we set processed in both companion branches so we never forward PATH. + +### LoginResponse (RESPONSE, LoginResponseHandler) + +- **Router:** All three branches (dest in companion, dest == local_hash, or anon to all bridges) set `processed_by_injection = True`. Never passes to engine. +- **Use case:** Login responses come back along the path. When we're a middle hop we should forward. +- **Conclusion:** Should be passed to engine after companion delivery so DIRECT RESPONSE can be forwarded. + +### ProtocolResponse (ProtocolResponseHandler) + +- **Router:** When we have companion_bridges we deliver and set `processed_by_injection = True`. Never passes to engine. +- **Use case:** Protocol responses (telemetry, etc.) come back along the path. When we're a middle hop we should forward. +- **Conclusion:** Should be passed to engine after companion delivery so DIRECT ProtocolResponse can be forwarded. + +## Recommended fix (two parts) + +### 1. Router: pass to engine after companion delivery + +For **ACK, PATH, LoginResponse, ProtocolResponse**, do **not** set `processed_by_injection` so the packet is always passed to the engine after any companion delivery. That implies: + +- **ACK:** Remove the unconditional `processed_by_injection = True` (or only set it when we have no companions and are definitely final). Then always pass to engine; engine will forward when we're next hop, drop when "Direct: not for us" or duplicate. +- **PATH:** In both branches where we set processed (dest in companion, anon to all), stop setting `processed_by_injection` so the packet also goes to the engine. Keep companion delivery and `_record_for_ui` as today. +- **LoginResponse:** In all three branches, stop setting `processed_by_injection` so the packet also goes to the engine. +- **ProtocolResponse:** When we have companions, stop setting `processed_by_injection` so the packet also goes to the engine. + +We still deliver to companions and call `_record_for_ui` where we do today; we just also pass the packet to the engine so it can forward when we're a middle hop. + +### 2. Engine: do not forward when we're final hop (optional but recommended) + +In `direct_forward`, after stripping our hash from the path, if the path is empty (hop_count was 1), we're the final destination and should not forward. Today we return the packet and would schedule a transmit with an empty path. Add: + +- After `packet.path = bytearray(packet.path[hash_size:])` and updating `path_len`, if `hop_count - 1 == 0` (or `len(packet.path) == 0`), set e.g. `packet.drop_reason = "Direct: final hop (deliver only)"` and return `None` so we don't transmit. + +That avoids transmitting when we're the final hop and only delivering to companions. + +## Recording for UI + +When we pass these packets to the engine, the engine will record them (forwarded or dropped with reason). We can keep calling `_record_for_ui` before passing to the engine for consistency, or rely on the engine's recording; if we do both we might double-record. Prefer: only the engine records when we pass to it (no `_record_for_ui` for these when we're also passing to engine), or keep a single record in the router and don't pass to engine for recording (pass only for forwarding). Simplest is: pass to engine, let engine do the only record (it already builds packet_record for every packet it sees). So we may remove `_record_for_ui` for these four types when we add the "pass to engine" path, to avoid duplicate entries. Alternatively we could keep _record_for_ui and have the engine skip recording when it's a type that the router already recorded—more complex. Easiest: pass to engine, remove the router's _record_for_ui for these four so the engine is the single place that records them. diff --git a/repeater/engine.py b/repeater/engine.py index 90ead0b..d06f9c8 100644 --- a/repeater/engine.py +++ b/repeater/engine.py @@ -151,6 +151,9 @@ class RepeaterHandler(BaseHandler): transmitted = False tx_delay_ms = 0.0 drop_reason = None + lbt_attempts = 0 + lbt_backoff_delays_ms = None + lbt_channel_busy = False original_path_hashes = packet.get_path_hashes_hex() path_hash_size = packet.get_path_hash_size() @@ -292,70 +295,33 @@ class RepeaterHandler(BaseHandler): if is_dupe and drop_reason is None: drop_reason = "Duplicate" - path_hash = None display_hashes = ( original_path_hashes if original_path_hashes else packet.get_path_hashes_hex() ) - if display_hashes: - display = display_hashes[:8] - if len(display_hashes) > 8: - display = list(display) + ["..."] - path_hash = "[" + ", ".join(display) + "]" - - src_hash = None - dst_hash = None - - # Payload types with dest_hash and src_hash as first 2 bytes - if payload_type in [0x00, 0x01, 0x02, 0x08]: - if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 2: - dst_hash = f"{packet.payload[0]:02X}" - src_hash = f"{packet.payload[1]:02X}" - - # ADVERT packets have source identifier as first byte - elif payload_type == PAYLOAD_TYPE_ADVERT: - if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 1: - src_hash = f"{packet.payload[0]:02X}" + path_hash = self._path_hash_display(display_hashes) + src_hash, dst_hash = self._packet_record_src_dst(packet, payload_type) # Record packet for charts - packet_record = { - "timestamp": time.time(), - "header": ( - f"0x{packet.header:02X}" - if hasattr(packet, "header") and packet.header is not None - else None - ), - "payload": ( - packet.payload.hex() if hasattr(packet, "payload") and packet.payload else None - ), - "payload_length": ( - len(packet.payload) if hasattr(packet, "payload") and packet.payload else 0 - ), - "type": payload_type, - "route": route_type, - "length": len(packet.payload or b""), - "rssi": rssi, - "snr": snr, - "score": self.calculate_packet_score( - snr, len(packet.payload or b""), self.radio_config["spreading_factor"] - ), - "tx_delay_ms": tx_delay_ms, - "transmitted": transmitted, - "is_duplicate": is_dupe, - "packet_hash": pkt_hash[:16], - "drop_reason": drop_reason, - "path_hash": path_hash, - "src_hash": src_hash, - "dst_hash": dst_hash, - "original_path": original_path_hashes or None, - "forwarded_path": forwarded_path_hashes, - "path_hash_size": path_hash_size, - "raw_packet": packet.write_to().hex() if hasattr(packet, "write_to") else None, - "lbt_attempts": lbt_attempts if transmitted else 0, - "lbt_backoff_delays_ms": ( - lbt_backoff_delays_ms if transmitted and lbt_backoff_delays_ms else None - ), - "lbt_channel_busy": lbt_channel_busy if transmitted else False, - } + packet_record = self._build_packet_record( + packet, + payload_type, + route_type, + rssi, + snr, + original_path_hashes, + path_hash_size, + path_hash, + src_hash, + dst_hash, + transmitted=transmitted, + drop_reason=drop_reason, + is_duplicate=is_dupe, + forwarded_path=forwarded_path_hashes, + tx_delay_ms=tx_delay_ms, + lbt_attempts=lbt_attempts, + lbt_backoff_delays_ms=lbt_backoff_delays_ms, + lbt_channel_busy=lbt_channel_busy, + ) # Store packet record to persistent storage # Skip LetsMesh only for invalid packets (not duplicates or operational drops) @@ -426,61 +392,22 @@ class RepeaterHandler(BaseHandler): header_info = PacketHeaderUtils.parse_header(packet.header) payload_type = header_info["payload_type"] route_type = header_info["route_type"] - pkt_hash = packet.calculate_packet_hash().hex().upper() original_path_hashes = packet.get_path_hashes_hex() path_hash_size = packet.get_path_hash_size() - display_hashes = original_path_hashes - path_hash = None - if display_hashes: - display = display_hashes[:8] - if len(display_hashes) > 8: - display = list(display) + ["..."] - path_hash = "[" + ", ".join(display) + "]" - src_hash = None - dst_hash = None - if payload_type in [0x00, 0x01, 0x02, 0x08]: - if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 2: - dst_hash = f"{packet.payload[0]:02X}" - src_hash = f"{packet.payload[1]:02X}" - elif payload_type == PAYLOAD_TYPE_ADVERT: - if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 1: - src_hash = f"{packet.payload[0]:02X}" - elif payload_type == PAYLOAD_TYPE_ANON_REQ: - if hasattr(packet, "payload") and packet.payload and len(packet.payload) >= 1: - dst_hash = f"{packet.payload[0]:02X}" - packet_record = { - "timestamp": time.time(), - "header": f"0x{packet.header:02X}", - "payload": ( - packet.payload.hex() if hasattr(packet, "payload") and packet.payload else None - ), - "payload_length": ( - len(packet.payload) if hasattr(packet, "payload") and packet.payload else 0 - ), - "type": payload_type, - "route": route_type, - "length": len(packet.payload or b""), - "rssi": rssi, - "snr": snr, - "score": self.calculate_packet_score( - snr, len(packet.payload or b""), self.radio_config["spreading_factor"] - ), - "tx_delay_ms": 0.0, - "transmitted": False, - "is_duplicate": False, - "packet_hash": pkt_hash[:16], - "drop_reason": None, - "path_hash": path_hash, - "src_hash": src_hash, - "dst_hash": dst_hash, - "original_path": original_path_hashes or None, - "forwarded_path": None, - "path_hash_size": path_hash_size, - "raw_packet": packet.write_to().hex() if hasattr(packet, "write_to") else None, - "lbt_attempts": 0, - "lbt_backoff_delays_ms": None, - "lbt_channel_busy": False, - } + path_hash = self._path_hash_display(original_path_hashes) + src_hash, dst_hash = self._packet_record_src_dst(packet, payload_type) + packet_record = self._build_packet_record( + packet, + payload_type, + route_type, + rssi, + snr, + original_path_hashes, + path_hash_size, + path_hash, + src_hash, + dst_hash, + ) try: self.storage.record_packet(packet_record, skip_letsmesh_if_invalid=False) except Exception as e: @@ -498,6 +425,94 @@ class RepeaterHandler(BaseHandler): for k in expired: del self.seen_packets[k] + def _path_hash_display(self, display_hashes) -> Optional[str]: + """Build path hash string for packet record from path hashes list.""" + if not display_hashes: + return None + display = display_hashes[:8] + if len(display_hashes) > 8: + display = list(display) + ["..."] + return "[" + ", ".join(display) + "]" + + def _packet_record_src_dst( + self, packet: Packet, payload_type: int + ) -> Tuple[Optional[str], Optional[str]]: + """Return (src_hash, dst_hash) for packet_record from packet and payload_type.""" + src_hash = None + dst_hash = None + payload = getattr(packet, "payload", None) + if payload_type in [0x00, 0x01, 0x02, 0x08]: + if payload and len(payload) >= 2: + dst_hash = f"{payload[0]:02X}" + src_hash = f"{payload[1]:02X}" + elif payload_type == PAYLOAD_TYPE_ADVERT: + if payload and len(payload) >= 1: + src_hash = f"{payload[0]:02X}" + elif payload_type == PAYLOAD_TYPE_ANON_REQ: + if payload and len(payload) >= 1: + dst_hash = f"{payload[0]:02X}" + return (src_hash, dst_hash) + + def _build_packet_record( + self, + packet: Packet, + payload_type: int, + route_type: int, + rssi: int, + snr: float, + original_path_hashes, + path_hash_size: int, + path_hash: Optional[str], + src_hash: Optional[str], + dst_hash: Optional[str], + *, + transmitted: bool = False, + drop_reason: Optional[str] = None, + is_duplicate: bool = False, + forwarded_path=None, + tx_delay_ms: float = 0.0, + lbt_attempts: int = 0, + lbt_backoff_delays_ms=None, + lbt_channel_busy: bool = False, + ) -> dict: + """Build a single packet_record dict for storage and recent_packets.""" + pkt_hash = packet.calculate_packet_hash().hex().upper() + payload = getattr(packet, "payload", None) + payload_len = len(payload or b"") + return { + "timestamp": time.time(), + "header": ( + f"0x{packet.header:02X}" + if hasattr(packet, "header") and packet.header is not None + else None + ), + "payload": payload.hex() if payload else None, + "payload_length": len(payload) if payload else 0, + "type": payload_type, + "route": route_type, + "length": payload_len, + "rssi": rssi, + "snr": snr, + "score": self.calculate_packet_score( + snr, payload_len, self.radio_config["spreading_factor"] + ), + "tx_delay_ms": tx_delay_ms, + "transmitted": transmitted, + "is_duplicate": is_duplicate, + "packet_hash": pkt_hash[:16], + "drop_reason": drop_reason, + "path_hash": path_hash, + "src_hash": src_hash, + "dst_hash": dst_hash, + "original_path": original_path_hashes or None, + "forwarded_path": forwarded_path, + "path_hash_size": path_hash_size, + "raw_packet": packet.write_to().hex() if hasattr(packet, "write_to") else None, + "lbt_attempts": lbt_attempts, + "lbt_backoff_delays_ms": lbt_backoff_delays_ms, + "lbt_channel_busy": lbt_channel_busy, + } + def _get_drop_reason(self, packet: Packet) -> str: if self.is_duplicate(packet): diff --git a/repeater/packet_router.py b/repeater/packet_router.py index 3a94466..53c5ff1 100644 --- a/repeater/packet_router.py +++ b/repeater/packet_router.py @@ -182,7 +182,8 @@ class PacketRouter: elif payload_type == LoginServerHandler.payload_type(): # Route to companion if dest is a companion; else to login_helper (for logging into this repeater). - # If dest is remote (no local handler), mark processed so we don't pass our own outbound login TX to the repeater as RX. + # When dest is remote (not handled), pass to engine so DIRECT/FLOOD ANON_REQ can be forwarded. + # Our own injected ANON_REQ is suppressed by the engine's duplicate (mark_seen) check. dest_hash = packet.payload[0] if packet.payload else None companion_bridges = getattr(self.daemon, "companion_bridges", {}) if dest_hash is not None and dest_hash in companion_bridges: @@ -192,9 +193,6 @@ class PacketRouter: handled = await self.daemon.login_helper.process_login_packet(packet) if handled: processed_by_injection = True - else: - # Login request for remote repeater (we already TXed it via inject); don't treat as RX. - processed_by_injection = True if processed_by_injection: self._record_for_ui(packet, metadata) @@ -220,6 +218,7 @@ class PacketRouter: handled = await self.daemon.text_helper.process_text_packet(packet) if handled: processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == PathHandler.payload_type(): dest_hash = packet.payload[0] if packet.payload else None From f2a5eab726a1cf4aba4368914aa7a3eebd5ac9b6 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sun, 8 Mar 2026 19:39:49 -0700 Subject: [PATCH 3/4] Refactor packet handling in PacketRouter and RepeaterHandler - Removed redundant original_path assignment in `RepeaterHandler` to streamline packet processing. - Introduced `_is_direct_final_hop` helper method in `PacketRouter` to determine if a packet is the final destination for direct routes with an empty path. - Updated comments in `PacketRouter` to clarify the handling of packets during routing, especially for direct forwarding scenarios. - Adjusted logic to ensure packets are correctly processed or delivered based on their routing status, enhancing overall packet management. --- repeater/engine.py | 1 - repeater/packet_router.py | 51 ++++++++++++++++++++++++++++++--------- tests/test_engine.py | 5 ++-- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/repeater/engine.py b/repeater/engine.py index d06f9c8..e192df6 100644 --- a/repeater/engine.py +++ b/repeater/engine.py @@ -789,7 +789,6 @@ class RepeaterHandler(BaseHandler): self.mark_seen(packet) - original_path = list(packet.path) # Remove first hash entry (hash_size bytes) packet.path = bytearray(packet.path[hash_size:]) packet.path_len = PathUtils.encode_path_len(hash_size, hop_count - 1) diff --git a/repeater/packet_router.py b/repeater/packet_router.py index 53c5ff1..d9c02e9 100644 --- a/repeater/packet_router.py +++ b/repeater/packet_router.py @@ -13,6 +13,11 @@ from pymc_core.node.handlers.protocol_request import ProtocolRequestHandler from pymc_core.node.handlers.protocol_response import ProtocolResponseHandler from pymc_core.node.handlers.text import TextMessageHandler from pymc_core.node.handlers.trace import TraceHandler +from pymc_core.protocol.constants import ( + PH_ROUTE_MASK, + ROUTE_TYPE_DIRECT, + ROUTE_TYPE_TRANSPORT_DIRECT, +) logger = logging.getLogger("PacketRouter") @@ -29,6 +34,15 @@ def _companion_dedup_key(packet) -> str | None: return None +def _is_direct_final_hop(packet) -> bool: + """True if packet is DIRECT (or TRANSPORT_DIRECT) with empty path — we're the final destination.""" + route = getattr(packet, "header", 0) & PH_ROUTE_MASK + if route != ROUTE_TYPE_DIRECT and route != ROUTE_TYPE_TRANSPORT_DIRECT: + return False + path = getattr(packet, "path", None) + return not path or len(path) == 0 + + class PacketRouter: def __init__(self, daemon_instance): @@ -197,15 +211,14 @@ class PacketRouter: self._record_for_ui(packet, metadata) elif payload_type == AckHandler.payload_type(): - # ACK has no dest in payload (4-byte CRC only); deliver to all bridges so sender sees send_confirmed + # ACK has no dest in payload (4-byte CRC only); deliver to all bridges so sender sees send_confirmed. + # Do not set processed_by_injection so packet also reaches engine for DIRECT forwarding when we're a middle hop. companion_bridges = getattr(self.daemon, "companion_bridges", {}) for bridge in companion_bridges.values(): try: await bridge.process_received_packet(packet) except Exception as e: logger.debug(f"Companion bridge ACK error: {e}") - processed_by_injection = True - self._record_for_ui(packet, metadata) elif payload_type == TextMessageHandler.payload_type(): dest_hash = packet.payload[0] if packet.payload else None @@ -226,8 +239,7 @@ class PacketRouter: if dest_hash is not None and dest_hash in companion_bridges: if self._should_deliver_path_to_companions(packet): await companion_bridges[dest_hash].process_received_packet(packet) - processed_by_injection = True - self._record_for_ui(packet, metadata) + # Do not set processed_by_injection so packet also reaches engine for DIRECT forwarding when we're a middle hop. elif companion_bridges and self._should_deliver_path_to_companions(packet): # Dest not in bridges: path-return with ephemeral dest (e.g. multi-hop login). # Deliver to all bridges; each will try to decrypt and ignore if not relevant. @@ -241,8 +253,7 @@ class PacketRouter: dest_hash or 0, len(companion_bridges), ) - processed_by_injection = True - self._record_for_ui(packet, metadata) + # Do not set processed_by_injection so packet also reaches engine for DIRECT forwarding when we're a middle hop. elif self.daemon.path_helper: await self.daemon.path_helper.process_path_packet(packet) @@ -251,6 +262,7 @@ class PacketRouter: # Deliver to the bridge that is the destination, or to all bridges when the # response is addressed to this repeater (path-based reply: firmware sends # to first hop instead of original requester). + # Do not set processed_by_injection so packet also reaches engine for DIRECT forwarding when we're a middle hop. dest_hash = packet.payload[0] if packet.payload and len(packet.payload) >= 1 else None companion_bridges = getattr(self.daemon, "companion_bridges", {}) local_hash = getattr(self.daemon, "local_hash", None) @@ -263,8 +275,6 @@ class PacketRouter: ) except Exception as e: logger.debug(f"Companion bridge RESPONSE error: {e}") - processed_by_injection = True - self._record_for_ui(packet, metadata) elif dest_hash == local_hash and companion_bridges: # Response addressed to this repeater (e.g. path-based reply to first hop) for bridge in companion_bridges.values(): @@ -277,8 +287,6 @@ class PacketRouter: dest_hash, len(companion_bridges), ) - processed_by_injection = True - self._record_for_ui(packet, metadata) elif companion_bridges: # Dest not in bridges and not local: likely ANON_REQ response (dest = ephemeral # sender hash). Deliver to all bridges; each will try to decrypt and ignore if @@ -293,12 +301,15 @@ class PacketRouter: dest_hash or 0, len(companion_bridges), ) + if companion_bridges and _is_direct_final_hop(packet): + # DIRECT with empty path: we're the final hop; don't pass to engine (it would drop with "Direct: no path") processed_by_injection = True self._record_for_ui(packet, metadata) elif payload_type == ProtocolResponseHandler.payload_type(): # PAYLOAD_TYPE_PATH (0x08): protocol responses (telemetry, binary, etc.). # Deliver at most once per logical packet so the client is not spammed with duplicates. + # Do not set processed_by_injection so packet also reaches engine for DIRECT forwarding when we're a middle hop. companion_bridges = getattr(self.daemon, "companion_bridges", {}) if companion_bridges and self._should_deliver_path_to_companions(packet): for bridge in companion_bridges.values(): @@ -306,7 +317,14 @@ class PacketRouter: await bridge.process_received_packet(packet) except Exception as e: logger.debug(f"Companion bridge RESPONSE error: {e}") - if companion_bridges: + if companion_bridges and _is_direct_final_hop(packet): + # DIRECT with empty path: we're the final hop; ensure delivery to all bridges (anon) + if not self._should_deliver_path_to_companions(packet): + for bridge in companion_bridges.values(): + try: + await bridge.process_received_packet(packet) + except Exception as e: + logger.debug(f"Companion bridge RESPONSE (final hop) error: {e}") processed_by_injection = True self._record_for_ui(packet, metadata) @@ -322,6 +340,15 @@ class PacketRouter: if handled: processed_by_injection = True self._record_for_ui(packet, metadata) + elif companion_bridges and _is_direct_final_hop(packet): + # DIRECT with empty path: we're the final hop; deliver to all bridges for anon matching + for bridge in companion_bridges.values(): + try: + await bridge.process_received_packet(packet) + except Exception as e: + logger.debug(f"Companion bridge REQ (final hop) error: {e}") + processed_by_injection = True + self._record_for_ui(packet, metadata) elif payload_type == GroupTextHandler.payload_type(): # GRP_TXT: pass to all companions (they filter by channel); still forward diff --git a/tests/test_engine.py b/tests/test_engine.py index 7c4adf9..5669614 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -287,9 +287,10 @@ class TestDirectForward: assert result.path_len == 2 def test_single_hop_path_consumed(self, handler): - """After consuming our hash the path becomes empty — packet delivered.""" + """Single hop to us: we strip and return packet with empty path (forward so it can reach destination).""" pkt = _make_direct_packet(path=bytes([LOCAL_HASH])) result = handler.direct_forward(pkt) + assert result is not None assert list(result.path) == [] assert result.path_len == 0 @@ -1044,7 +1045,7 @@ GOOD_PACKETS = [ lambda: _make_flood_packet(payload=b"\xAB\x01\x02\x03", payload_type=4)), ("good_direct_minimal", - "Direct, 1-byte payload, single hop to us", + "Direct, 1-byte payload, single hop to us (forward with empty path)", lambda: _make_direct_packet(payload=b"\x01", path=bytes([LOCAL_HASH]))), ("good_direct_multihop", From ee97ff736a9eca4da0967191c793067d268efba6 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sun, 8 Mar 2026 19:47:09 -0700 Subject: [PATCH 4/4] Remove obsolete documentation on DIRECT packet handling - Deleted the `DIRECT_packets_not_forwarded.md` file, which outlined the behavior of certain packet types in the router and their forwarding status. This change reflects the removal of outdated information that is no longer relevant to the current implementation. --- docs/DIRECT_packets_not_forwarded.md | 66 ---------------------------- 1 file changed, 66 deletions(-) delete mode 100644 docs/DIRECT_packets_not_forwarded.md diff --git a/docs/DIRECT_packets_not_forwarded.md b/docs/DIRECT_packets_not_forwarded.md deleted file mode 100644 index 92f663b..0000000 --- a/docs/DIRECT_packets_not_forwarded.md +++ /dev/null @@ -1,66 +0,0 @@ -# DIRECT packets not forwarded (router consumes them) - -## Summary - -Besides ANON_REQ (fixed), these payload types are **always** marked `processed_by_injection` in the router and **never** passed to the engine, so they are **never forwarded** even when this repeater is a middle hop on a DIRECT path: - -| Payload type | Router behavior | Can be DIRECT? | Should forward when middle hop? | -|---------------------|-----------------|-----------------|----------------------------------| -| **ACK** | Deliver to all companion bridges, set processed | Yes (return path) | Yes | -| **PATH** | Deliver to companion(s) or all bridges (anon), set processed | Yes (path response) | Yes | -| **LoginResponse** | Deliver to bridge(s), set processed | Yes (login response) | Yes | -| **ProtocolResponse**| Deliver to companions, set processed | Yes (telemetry etc.) | Yes | -| **Trace** | trace_helper only, set processed | Yes/No | No (diagnostic, not forwarded by design) | - -So **ACK, PATH, LoginResponse (RESPONSE), and ProtocolResponse** are DIRECT packet types that we should be forwarding when we're in the path but currently are not, because the router consumes them every time. - -## Detail by type - -### ACK (AckHandler) - -- **Router:** Delivers to all companion bridges (so sender sees send_confirmed), then sets `processed_by_injection = True`. Never passes to engine. -- **Use case:** ACKs travel back along the path. When we're a middle hop we should forward the ACK toward the sender. -- **Conclusion:** Should be passed to engine after companion delivery so DIRECT ACKs can be forwarded. - -### PATH (PathHandler) - -- **Router:** If dest in companion_bridges → deliver to that bridge, set processed. If dest not in bridges but we have companions → deliver to all bridges (anon), set processed. Only when path_helper runs (and no companion anon delivery) do we *not* set processed, so the packet can reach the engine. -- **Use case:** PATH responses come back along the path. When we're a middle hop we should forward. When we're final hop (dest in companion or we're path_helper) we should not forward. -- **Conclusion:** When we deliver to companions we still need to pass to engine so we can forward when we're a middle hop. Today we set processed in both companion branches so we never forward PATH. - -### LoginResponse (RESPONSE, LoginResponseHandler) - -- **Router:** All three branches (dest in companion, dest == local_hash, or anon to all bridges) set `processed_by_injection = True`. Never passes to engine. -- **Use case:** Login responses come back along the path. When we're a middle hop we should forward. -- **Conclusion:** Should be passed to engine after companion delivery so DIRECT RESPONSE can be forwarded. - -### ProtocolResponse (ProtocolResponseHandler) - -- **Router:** When we have companion_bridges we deliver and set `processed_by_injection = True`. Never passes to engine. -- **Use case:** Protocol responses (telemetry, etc.) come back along the path. When we're a middle hop we should forward. -- **Conclusion:** Should be passed to engine after companion delivery so DIRECT ProtocolResponse can be forwarded. - -## Recommended fix (two parts) - -### 1. Router: pass to engine after companion delivery - -For **ACK, PATH, LoginResponse, ProtocolResponse**, do **not** set `processed_by_injection` so the packet is always passed to the engine after any companion delivery. That implies: - -- **ACK:** Remove the unconditional `processed_by_injection = True` (or only set it when we have no companions and are definitely final). Then always pass to engine; engine will forward when we're next hop, drop when "Direct: not for us" or duplicate. -- **PATH:** In both branches where we set processed (dest in companion, anon to all), stop setting `processed_by_injection` so the packet also goes to the engine. Keep companion delivery and `_record_for_ui` as today. -- **LoginResponse:** In all three branches, stop setting `processed_by_injection` so the packet also goes to the engine. -- **ProtocolResponse:** When we have companions, stop setting `processed_by_injection` so the packet also goes to the engine. - -We still deliver to companions and call `_record_for_ui` where we do today; we just also pass the packet to the engine so it can forward when we're a middle hop. - -### 2. Engine: do not forward when we're final hop (optional but recommended) - -In `direct_forward`, after stripping our hash from the path, if the path is empty (hop_count was 1), we're the final destination and should not forward. Today we return the packet and would schedule a transmit with an empty path. Add: - -- After `packet.path = bytearray(packet.path[hash_size:])` and updating `path_len`, if `hop_count - 1 == 0` (or `len(packet.path) == 0`), set e.g. `packet.drop_reason = "Direct: final hop (deliver only)"` and return `None` so we don't transmit. - -That avoids transmitting when we're the final hop and only delivering to companions. - -## Recording for UI - -When we pass these packets to the engine, the engine will record them (forwarded or dropped with reason). We can keep calling `_record_for_ui` before passing to the engine for consistency, or rely on the engine's recording; if we do both we might double-record. Prefer: only the engine records when we pass to it (no `_record_for_ui` for these when we're also passing to engine), or keep a single record in the router and don't pass to engine for recording (pass only for forwarding). Simplest is: pass to engine, let engine do the only record (it already builds packet_record for every packet it sees). So we may remove `_record_for_ui` for these four types when we add the "pass to engine" path, to avoid duplicate entries. Alternatively we could keep _record_for_ui and have the engine skip recording when it's a type that the router already recorded—more complex. Easiest: pass to engine, remove the router's _record_for_ui for these four so the engine is the single place that records them.