diff --git a/modules/graph_trace_helper.py b/modules/graph_trace_helper.py index 06917a7..0067634 100644 --- a/modules/graph_trace_helper.py +++ b/modules/graph_trace_helper.py @@ -97,7 +97,6 @@ def update_mesh_graph_from_trace_data( if len(path_hashes) == 0: return - last_node = path_hashes[-1].lower() if is_immediate_neighbor: neighbor_prefix = path_hashes[0].lower() @@ -133,9 +132,8 @@ def update_mesh_graph_from_trace_data( if bot_location: neighbor_result = _get_node_location_from_db(bot, neighbor_prefix, bot_location, recency_days) if neighbor_result: - neighbor_location, selected_neighbor_key = neighbor_result - if not neighbor_key and selected_neighbor_key: - neighbor_key = selected_neighbor_key + # The location match is only for the distance, not the neighbor's identity + neighbor_location, _ = neighbor_result if neighbor_location and bot_location: geographic_distance = calculate_distance( neighbor_location[0], neighbor_location[1], @@ -165,9 +163,57 @@ def update_mesh_graph_from_trace_data( bot.logger.info(f"Mesh graph: Created trusted bidirectional edge with immediate neighbor {neighbor_prefix}") return - # Regular case: trace from elsewhere, bot is destination - geographic_distance = None - last_node_key = None + # Regular case: the bot received the trace at the end of its path. Our own + # trace also left from the bot, so it confirms the bot -> first hop link too. + chain = [h.lower() for h in path_hashes] + if is_our_trace: + chain = [bot_prefix] + chain + chain = chain + [bot_prefix] + + keys: dict[str, Optional[str]] = {bot_prefix: bot_key} + for node in chain: + if node not in keys: + keys[node] = _unique_repeater_key(bot, node, recency_days) + + # Locations only feed distances; a location match among colliding prefixes is a + # guess, so it never becomes the stored public key. + locations: dict[int, Optional[tuple[float, float]]] = {} + reference = bot_location + for i, node in enumerate(chain): + if node == bot_prefix: + locations[i] = bot_location + else: + locations[i] = None + try: + found = _get_node_location_from_db(bot, node, reference, recency_days) + if found and found[0]: + locations[i] = found[0] + except Exception as e: + bot.logger.debug(f"Could not locate trace node {node}: {e}") + if locations[i]: + reference = locations[i] + + for i in range(len(chain) - 1): + from_node, to_node = chain[i], chain[i + 1] + geographic_distance = None + from_location, to_location = locations[i], locations[i + 1] + if from_location and to_location: + geographic_distance = calculate_distance( + from_location[0], from_location[1], to_location[0], to_location[1], + ) + mesh_graph.add_edge( + from_prefix=from_node, + to_prefix=to_node, + from_public_key=keys.get(from_node), + to_public_key=keys.get(to_node), + hop_position=i + 1, + geographic_distance=geographic_distance, + **edge_width, + ) + + +def _unique_repeater_key(bot: Any, prefix: str, recency_days: int) -> Optional[str]: + """The public key of the only recently heard repeater or room server with ``prefix``, else None.""" try: count_query = f""" SELECT COUNT(DISTINCT public_key) as count @@ -176,7 +222,7 @@ def update_mesh_graph_from_trace_data( AND role IN ('repeater', 'roomserver') AND COALESCE(last_advert_timestamp, last_heard) >= datetime('now', '-{recency_days} days') """ - prefix_pattern = f"{last_node}%" + prefix_pattern = f"{prefix}%" count_results = bot.db_manager.execute_query(count_query, (prefix_pattern,)) if count_results and count_results[0].get("count", 0) == 1: query = f""" @@ -190,102 +236,7 @@ def update_mesh_graph_from_trace_data( """ results = bot.db_manager.execute_query(query, (prefix_pattern,)) if results and results[0].get("public_key"): - last_node_key = results[0]["public_key"] + return results[0]["public_key"] except Exception as e: - bot.logger.debug(f"Error checking uniqueness for trace last_node {last_node}: {e}") - - try: - if bot_location: - last_node_result = _get_node_location_from_db(bot, last_node, bot_location, recency_days) - if last_node_result: - last_node_location, selected_key = last_node_result - if not last_node_key and selected_key: - last_node_key = selected_key - if last_node_location and bot_location: - geographic_distance = calculate_distance( - last_node_location[0], last_node_location[1], - bot_location[0], bot_location[1], - ) - except Exception as e: - bot.logger.debug(f"Could not calculate distance for trace edge {last_node}->{bot_prefix}: {e}") - - mesh_graph.add_edge( - from_prefix=last_node, - to_prefix=bot_prefix, - from_public_key=last_node_key, - to_public_key=bot_key, - hop_position=len(path_hashes), - geographic_distance=geographic_distance, - **edge_width, - ) - - # Create edges between nodes in the pathHashes (if more than one) - previous_location = bot_location - for i in range(len(path_hashes) - 1, 0, -1): - from_node = path_hashes[i - 1].lower() - to_node = path_hashes[i].lower() - hop_position = len(path_hashes) - i - - from_node_key = None - to_node_key = None - for node, key_var in [(from_node, "from_node_key"), (to_node, "to_node_key")]: - try: - count_query = f""" - SELECT COUNT(DISTINCT public_key) as count - FROM complete_contact_tracking - WHERE public_key LIKE ? - AND role IN ('repeater', 'roomserver') - AND COALESCE(last_advert_timestamp, last_heard) >= datetime('now', '-{recency_days} days') - """ - prefix_pattern = f"{node}%" - count_results = bot.db_manager.execute_query(count_query, (prefix_pattern,)) - if count_results and count_results[0].get("count", 0) == 1: - query = f""" - SELECT public_key - FROM complete_contact_tracking - WHERE public_key LIKE ? - AND role IN ('repeater', 'roomserver') - AND COALESCE(last_advert_timestamp, last_heard) >= datetime('now', '-{recency_days} days') - ORDER BY is_starred DESC, COALESCE(last_advert_timestamp, last_heard) DESC - LIMIT 1 - """ - results = bot.db_manager.execute_query(query, (prefix_pattern,)) - if results and results[0].get("public_key"): - if key_var == "from_node_key": - from_node_key = results[0]["public_key"] - else: - to_node_key = results[0]["public_key"] - except Exception as e: - bot.logger.debug(f"Error checking uniqueness for trace node {node}: {e}") - - geographic_distance = None - try: - if previous_location: - from_result = _get_node_location_from_db(bot, from_node, previous_location, recency_days) - if from_result: - from_location, selected_from_key = from_result - if not from_node_key and selected_from_key: - from_node_key = selected_from_key - to_result = _get_node_location_from_db(bot, to_node, from_location, recency_days) - if to_result: - to_location, selected_to_key = to_result - if not to_node_key and selected_to_key: - to_node_key = selected_to_key - if from_location and to_location: - geographic_distance = calculate_distance( - from_location[0], from_location[1], - to_location[0], to_location[1], - ) - previous_location = from_location - except Exception as e: - bot.logger.debug(f"Could not calculate distance for trace edge {from_node}->{to_node}: {e}") - - mesh_graph.add_edge( - from_prefix=from_node, - to_prefix=to_node, - from_public_key=from_node_key, - to_public_key=to_node_key, - hop_position=hop_position, - geographic_distance=geographic_distance, - **edge_width, - ) + bot.logger.debug(f"Error checking uniqueness for trace node {prefix}: {e}") + return None diff --git a/modules/transmission_tracker.py b/modules/transmission_tracker.py index c763946..59c03b7 100644 --- a/modules/transmission_tracker.py +++ b/modules/transmission_tracker.py @@ -109,21 +109,26 @@ class TransmissionTracker: @property def bot_prefix(self) -> Optional[str]: - """Our radio's public key prefix (``prefix_hex_chars`` long), or None before it is known.""" - if self._bot_prefix is None: - self._update_bot_prefix() - return self._bot_prefix + """Our radio's public key prefix (``prefix_hex_chars`` long), or None before it is known. + + Read from the radio's current key each time, so a reconnect to another radio + is picked up; a value assigned to it is kept instead. + """ + if self._bot_prefix is not None: + return self._bot_prefix + key = own_public_key(self.bot) + if key and len(key) >= 2: + return key[:self.bot.prefix_hex_chars].lower() + return None @bot_prefix.setter def bot_prefix(self, value: Optional[str]) -> None: self._bot_prefix = value def _update_bot_prefix(self): - """Update bot prefix from the radio's public key""" - key = own_public_key(self.bot) - if key and len(key) >= 2: - self._bot_prefix = key[:self.bot.prefix_hex_chars].lower() - self.logger.debug(f"Bot prefix set to: {self._bot_prefix}") + """Log the bot prefix once the radio's key is known (kept for callers; the property reads it live).""" + if self.bot_prefix: + self.logger.debug(f"Bot prefix set to: {self.bot_prefix}") def record_transmission(self, content: str, target: str, message_type: str, command_id: Optional[str] = None, *, @@ -463,10 +468,9 @@ class TransmissionTracker: last_node = path_nodes[-1] if isinstance(last_node, str) and len(last_node) >= 2: # Take first 2 characters as prefix - prefix = last_node[:self.bot.prefix_hex_chars].lower() - # Filter out our own prefix - if prefix != self.bot_prefix: - return [prefix] + # A companion radio never forwards, so the last hop is never us; a + # repeater sharing our short prefix still gets the repeat. + return [last_node[:self.bot.prefix_hex_chars].lower()] # Fallback to parsing path string elif path: @@ -481,10 +485,7 @@ class TransmissionTracker: if parts: last_part = parts[-1] if len(last_part) >= 2: - prefix = last_part[:self.bot.prefix_hex_chars].lower() - # Filter out our own prefix - if prefix != self.bot_prefix: - return [prefix] + return [last_part[:self.bot.prefix_hex_chars].lower()] return [] # No valid prefix found diff --git a/tests/test_graph_trace_helper.py b/tests/test_graph_trace_helper.py index ba9ad3a..4dd7d12 100644 --- a/tests/test_graph_trace_helper.py +++ b/tests/test_graph_trace_helper.py @@ -336,6 +336,35 @@ class TestBotIdentityFromRadio: bot = _make_bot(bot_prefix=None) bot.meshcore.self_info = {"public_key": "c3d4" + "00" * 30} update_mesh_graph_from_trace_data(bot, ["a1b2", "e5f6"], {}) - last = bot.mesh_graph.add_edge.call_args_list[0].kwargs + last = bot.mesh_graph.add_edge.call_args_list[-1].kwargs assert (last["from_prefix"], last["to_prefix"]) == ("e5f6", "c3d4") assert all(c.kwargs.get("prefix_bytes") == 2 for c in bot.mesh_graph.add_edge.call_args_list) + + + +class TestOwnTraceChain: + def test_our_trace_adds_the_bot_to_first_hop_edge_in_path_order(self): + bot = _make_bot(bot_prefix="dd") + update_mesh_graph_from_trace_data(bot, ["aa", "bb", "cc"], {}, is_our_trace=True) + edges = [(c.kwargs["from_prefix"], c.kwargs["to_prefix"], c.kwargs["hop_position"]) + for c in bot.mesh_graph.add_edge.call_args_list] + assert edges == [("dd", "aa", 1), ("aa", "bb", 2), ("bb", "cc", 3), ("cc", "dd", 4)] + + def test_someone_elses_trace_starts_at_its_first_hop(self): + bot = _make_bot(bot_prefix="dd") + update_mesh_graph_from_trace_data(bot, ["aa", "bb"], {}, is_our_trace=False) + edges = [(c.kwargs["from_prefix"], c.kwargs["to_prefix"], c.kwargs["hop_position"]) + for c in bot.mesh_graph.add_edge.call_args_list] + assert edges == [("aa", "bb", 1), ("bb", "dd", 2)] + + def test_a_location_guess_is_not_stored_as_the_public_key(self, monkeypatch): + import modules.utils as utils + + bot = _make_bot(bot_prefix="dd") + bot.db_manager.execute_query = Mock(return_value=[{"count": 2}]) # two repeaters share "aa" + monkeypatch.setattr(utils, "_get_node_location_from_db", lambda *a, **k: ((47.0, -122.0), "aa11" + "0" * 60)) + update_mesh_graph_from_trace_data(bot, ["aa"], {}, is_our_trace=False) + update_mesh_graph_from_trace_data(bot, ["aa"], {}, is_our_trace=True) + for c in bot.mesh_graph.add_edge.call_args_list: + assert c.kwargs["from_public_key"] != "aa11" + "0" * 60 + assert c.kwargs["to_public_key"] != "aa11" + "0" * 60 diff --git a/tests/test_transmission_tracker.py b/tests/test_transmission_tracker.py index d882f1f..85472e4 100644 --- a/tests/test_transmission_tracker.py +++ b/tests/test_transmission_tracker.py @@ -195,10 +195,11 @@ class TestExtractRepeaterPrefixes: result = tracker.extract_repeater_prefixes_from_path(None, path_nodes=["01", "7e", "86"]) assert result == ["86"] - def test_filters_own_prefix(self, tracker): + def test_a_repeater_sharing_our_prefix_keeps_the_repeat(self, tracker): + """A companion never forwards, so a last hop matching our prefix is a repeater.""" tracker.bot_prefix = "86" - result = tracker.extract_repeater_prefixes_from_path("01,7e,86") - assert result == [] + assert tracker.extract_repeater_prefixes_from_path("01,86") == ["86"] + assert tracker.extract_repeater_prefixes_from_path(None, ["01", "86"]) == ["86"] def test_empty_path_returns_empty(self, tracker): result = tracker.extract_repeater_prefixes_from_path(None) @@ -310,6 +311,18 @@ class TestBotPrefixFromSelfInfo: assert tracker.bot_prefix == "c3" +class TestBotPrefixFollowsReconnect: + def test_a_reconnect_to_another_radio_changes_the_prefix(self, mock_logger): + bot = Mock() + bot.logger = mock_logger + bot.prefix_hex_chars = 2 + bot.meshcore = Mock(self_info={"public_key": "c3" + "00" * 31}) + tracker = TransmissionTracker(bot) + assert tracker.bot_prefix == "c3" + bot.meshcore = Mock(self_info={"public_key": "e5" + "00" * 31}) + assert tracker.bot_prefix == "e5" + + class TestUpdateBotPrefix: """Cover lines 57-67: _update_bot_prefix with str and bytes public_key."""