diff --git a/CHANGELOG.md b/CHANGELOG.md index e205a8b..20a4354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ semantic versioning. ### Fixed +- `{hops}` and `{hops_label}` in a `[Keywords]` response now report the same hop count + a command response would for the same packet. The keyword formatter carried its own + implementation that consulted only `message.hops` and the path display string, so it + answered `?` whenever the count was known only from `routing_info`, and preferred a + stale path string over the packet's own `path_length` when both were present. All + three formatters now share `utils.message_hop_count`. + - `pathbytes_min` no longer treats a direct message as a multi-byte path, so `{path_distance|pathbytes_min:2|prefix_if_nonempty: | Path Dist: }` stops printing `| Path Dist: N/A` on a hopless packet. `bytes_per_hop` describes how a path is diff --git a/modules/utils.py b/modules/utils.py index 0deb4cb..1c813ab 100644 --- a/modules/utils.py +++ b/modules/utils.py @@ -2455,26 +2455,16 @@ def format_keyword_response_with_placeholders( replacements['timestamp'] = time_str replacements['packet_hash'] = get_packet_hash_placeholder(message) - # Total hops: use message.hops when set, else parse from path string (e.g. "01,5f (2 hops)") - hops_val = getattr(message, 'hops', None) - if hops_val is not None and isinstance(hops_val, int): - replacements['hops'] = str(hops_val) - else: - path_str = message.path or "" - hop_match = re.search(r'\((\d+)\s*hops?', path_str, re.IGNORECASE) - if hop_match: - replacements['hops'] = hop_match.group(1) - elif re.search(r'\bdirect\b|\b0\s*hops?\b', path_str, re.IGNORECASE): - replacements['hops'] = "0" - else: - replacements['hops'] = "?" - # Pluralized label: "1 hop", "2 hops", or "?" when unknown - h = replacements['hops'] - if h == "?": + # Shared with BaseCommand.get_hops_display_values and the hops_min filter, so + # a keyword response and a command response report the same hop count for the + # same packet. "?" only when the count cannot be determined at all. + hops_val = message_hop_count(message) + if hops_val is None: + replacements['hops'] = "?" replacements['hops_label'] = "?" else: - n = int(h) - replacements['hops_label'] = "1 hop" if n == 1 else f"{n} hops" + replacements['hops'] = str(hops_val) + replacements['hops_label'] = "1 hop" if hops_val == 1 else f"{hops_val} hops" else: # No message - use defaults for message-based placeholders replacements['sender'] = "Unknown" diff --git a/tests/test_utils.py b/tests/test_utils.py index fc0f168..70f0ef6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -712,6 +712,54 @@ class TestFormatKeywordResponseWithPlaceholders: result = format_keyword_response_with_placeholders("{hops_label}", msg, bot) assert result == "3 hops" + def test_hops_prefers_routing_info_over_the_path_string(self): + """The keyword formatter used to parse only the path string, so it reported a + different hop count than the command formatter for the same packet.""" + bot = self._bot() + msg = self._msg(hops=None, path="01 (1 hop)", routing_info={"path_length": 2}) + with patch("modules.utils.calculate_path_distances", return_value=("", "")): + result = format_keyword_response_with_placeholders("{hops}|{hops_label}", msg, bot) + assert result == "2|2 hops" + + def test_hops_falls_back_to_counting_routing_path_nodes(self): + bot = self._bot() + msg = self._msg(hops=None, routing_info={"path_nodes": ["01", "02", "03"]}) + with patch("modules.utils.calculate_path_distances", return_value=("", "")): + result = format_keyword_response_with_placeholders("{hops}", msg, bot) + assert result == "3" + + def test_hops_still_parses_the_path_string_without_routing_info(self): + bot = self._bot() + msg = self._msg(hops=None, path="01,5f (2 hops)") + with patch("modules.utils.calculate_path_distances", return_value=("", "")): + result = format_keyword_response_with_placeholders("{hops}|{hops_label}", msg, bot) + assert result == "2|2 hops" + + def test_hops_is_unknown_only_when_nothing_can_be_determined(self): + bot = self._bot() + msg = self._msg(hops=None, path=None, routing_info=None) + with patch("modules.utils.calculate_path_distances", return_value=("", "")): + result = format_keyword_response_with_placeholders("{hops}|{hops_label}", msg, bot) + assert result == "?|?" + + def test_hops_matches_the_command_formatter_for_the_same_message(self): + """One implementation, so a keyword reply and a command reply cannot disagree.""" + from modules.commands.base_command import BaseCommand + + bot = self._bot() + for kwargs in ( + {"hops": 3}, + {"hops": None, "routing_info": {"path_length": 2}}, + {"hops": None, "path": "Direct"}, + {"hops": None, "path": "01,5f (2 hops)"}, + {"hops": None}, + ): + msg = self._msg(**kwargs) + with patch("modules.utils.calculate_path_distances", return_value=("", "")): + keyword_hops = format_keyword_response_with_placeholders("{hops}", msg, bot) + command_hops, _ = BaseCommand.get_hops_display_values(Mock(), msg) + assert keyword_hops == command_hops, kwargs + def test_connection_info_contains_snr_rssi(self): bot = self._bot() msg = self._msg(snr=12, rssi=-75)