From 4d8624494c3dad4cbd9a68633a283a193ecc1fa4 Mon Sep 17 00:00:00 2001 From: agessaman Date: Fri, 21 Aug 2026 21:10:03 -0700 Subject: [PATCH] feat(path): expose {path_distance} in the path command reply prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Total path distance was already a solved problem here: the test command computes it and exposes {path_distance} through the piped-template engine, with pipe filters and docs. The path command just had no way to reach it, since reply_prefix went through plain str.format. Rather than add a second distance implementation, this reuses what exists. BaseCommand.get_standard_placeholder_fields now returns the shared placeholder set so subclasses can extend it, the path command renders reply_prefix through format_piped_template with an added {path_distance}, and the sum itself uses utils.calculate_distance over the nodes the path command has already resolved. Distance covers sender -> each hop -> bot and is deliberately blank when the chain cannot be measured end to end (unresolved hop, prefix collision, missing or 0,0 coordinates, unknown sender, no configured bot position), so a partial sum is never reported as the real distance. Because the prefix now supports pipe filters, an empty value takes its label with it: {path_distance|prefix_if_nonempty:πŸ“ } Supersedes #198, which added a parallel haversine, a show_path_distance flag, and a distance_traveled key across 10 locales. --- CHANGELOG.md | 5 ++ config.ini.example | 11 +++- docs/path-command-config.md | 8 ++- modules/commands/base_command.py | 53 +++++++++++----- modules/commands/path_command.py | 61 +++++++++++++++++- modules/commands/test_command.py | 4 +- tests/unit/test_path_command_distance.py | 80 ++++++++++++++++++++++++ 7 files changed, 200 insertions(+), 22 deletions(-) create mode 100644 tests/unit/test_path_command_distance.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c31e8e7..c1c2839 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ semantic versioning. - Migration 23: nullable `snr` / `rssi` columns on `observed_paths` for zero-hop advert rows. +- `{path_distance}` is now available in the path command's `[Path_Command] reply_prefix`, + reporting total distance travelled (sender β†’ hops β†’ bot, e.g. `12.4km`) and rendering + empty when any node in the chain has no usable coordinates. The prefix now supports the + same pipe filters as the test command's `response_format`, so + `{path_distance|prefix_if_nonempty:πŸ“ }` drops the label along with the value. - `install-service.sh --install-extras` installs the optional profanity-filter and geocoding packages without prompting, for unattended installs and upgrades. It takes precedence over the in-place `--update-venv` path, so the two can be diff --git a/config.ini.example b/config.ini.example index 430af1d..e0ba523 100644 --- a/config.ini.example +++ b/config.ini.example @@ -985,10 +985,17 @@ require_path_bytes_failure_response = # false: Only respond to "path", "decode", or "route" keywords enable_p_shortcut = true -# Optional first line prepended to path command RF replies (Python str.format on the triggering message). +# Optional first line prepended to path command RF replies. # Placeholders match keyword-style responses: {sender}, {connection_info}, {path}, -# {hops}, {hops_label}, {timestamp}, {snr}, {rssi}. Only the first RF chunk includes the prefix when the reply is split. +# {hops}, {hops_label}, {timestamp}, {snr}, {rssi}, plus {path_distance}. +# {path_distance} is the total distance travelled (sender -> hops -> bot), e.g. "12.4km". +# It renders empty unless every node in the chain has known coordinates and the bot has +# bot_latitude/bot_longitude set, so a partial sum is never mistaken for the full distance. +# Feed-style pipe filters are supported (same engine as [Test_Command] response_format), +# so a label can disappear along with an empty value. +# Only the first RF chunk includes the prefix when the reply is split. # Example: @[{sender}] +# Example: {path_distance|prefix_if_nonempty:\U0001F4CF } reply_prefix = # Bytes per hop required before resolving repeater names from the database. diff --git a/docs/path-command-config.md b/docs/path-command-config.md index a8bafe9..aea1ef2 100644 --- a/docs/path-command-config.md +++ b/docs/path-command-config.md @@ -18,7 +18,13 @@ These options only affect the **path** command’s reply text and whether repeat **`reply_prefix`** (string, default empty) - Prepended as the first line of path command RF replies (only the **first** chunk when the reply is split for length). -- Uses Python `str.format` on the **triggering** message. Placeholders: `{sender}`, `{connection_info}`, `{path}`, `{hops}`, `{hops_label}`, `{timestamp}`, `{snr}`, `{rssi}`. +- Placeholders: `{sender}`, `{connection_info}`, `{path}`, `{hops}`, `{hops_label}`, `{timestamp}`, `{snr}`, `{rssi}`, `{path_distance}`. +- `{path_distance}` is the total distance travelled, summed sender β†’ each resolved hop β†’ bot (e.g. `12.4km`). It is **empty** whenever the chain cannot be measured end to end: an unresolved hop, a prefix collision, a node with no stored coordinates, an unknown sender position, or no `[Bot] bot_latitude`/`bot_longitude`. A partial sum is never reported, since it would understate the real distance. +- Supports the same **feed-style pipe filters** as the test command's `response_format` (see `modules/response_template.py`). Use `prefix_if_nonempty` so a label disappears along with an empty distance: + +```ini +reply_prefix = "{path_distance|prefix_if_nonempty:πŸ“ }\n" +``` **`minimum_path_bytes`** (integer `0`–`3`, default `0`) diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py index 5dd10fe..cb2f801 100644 --- a/modules/commands/base_command.py +++ b/modules/commands/base_command.py @@ -1193,24 +1193,43 @@ class BaseCommand(ABC): hops_label = "1 hop" if hops_val == 1 else f"{hops_val} hops" return hops_str, hops_label - def format_response(self, message: MeshMessage, response_format: str) -> str: - """Format a response string with message data""" - try: - connection_info = self.build_enhanced_connection_info(message) - path_display = self.get_path_display_string(message) - hops, hops_label = self.get_hops_display_values(message) - timestamp = self.format_timestamp(message) + def get_standard_placeholder_fields(self, message: MeshMessage) -> dict[str, Any]: + """Standard response placeholders shared by every command template. - return response_format.format( - sender=message.sender_id or "Unknown", - connection_info=connection_info, - path=path_display, - hops=hops, - hops_label=hops_label, - timestamp=timestamp, - snr=message.snr or "Unknown", - rssi=message.rssi or "Unknown" - ) + Subclasses that render templates through + :func:`~modules.response_template.format_piped_template` start from this + mapping and add their own fields, so the common names stay identical + across commands. + """ + hops, hops_label = self.get_hops_display_values(message) + return { + 'sender': message.sender_id or "Unknown", + 'connection_info': self.build_enhanced_connection_info(message), + 'path': self.get_path_display_string(message), + 'hops': hops, + 'hops_label': hops_label, + 'timestamp': self.format_timestamp(message), + 'snr': message.snr or "Unknown", + 'rssi': message.rssi or "Unknown", + } + + def format_response(self, message: MeshMessage, response_format: str, + extra: Optional[dict[str, Any]] = None) -> str: + """Format a response string with message data. + + Args: + message: The message the placeholders describe. + response_format: Template string using ``{placeholder}`` names. + extra: Additional command-specific placeholders. Values here are + merged over the standard set, so a command can expose fields + only it can compute (e.g. the path command's ``{distance}``). + """ + try: + fields = self.get_standard_placeholder_fields(message) + if extra: + fields.update(extra) + + return response_format.format(**fields) except (KeyError, ValueError) as e: self.logger.warning(f"Error formatting response: {e}") return response_format diff --git a/modules/commands/path_command.py b/modules/commands/path_command.py index 16c7716..92c2518 100644 --- a/modules/commands/path_command.py +++ b/modules/commands/path_command.py @@ -15,8 +15,10 @@ from ..path_inference import ( select_node_repeater, select_repeater_by_graph, ) +from ..response_template import format_piped_template from ..utils import ( bytes_per_hop_from_routing_and_nodes, + calculate_distance, parse_path_string, public_key_has_prefix, ) @@ -380,10 +382,29 @@ class PathCommand(BaseCommand): bph = self._bytes_per_hop_from_nodes_and_routing(node_ids, routing_info) return bph >= self.minimum_path_bytes + def _format_path_distance(self) -> str: + """Render the {path_distance} placeholder; empty when the path cannot be measured. + + Matches the ``{path_distance}`` name and ``12.4km`` shape already used by the + test command, so one prefix template reads the same across both commands. + """ + distance = getattr(self, '_last_path_distance_km', None) + if distance is None: + return '' + return f"{distance:.1f}km" + def _format_path_reply_prefix(self, message: MeshMessage) -> str: if not self.path_reply_prefix: return '' - formatted = self.format_response(message, self.path_reply_prefix).rstrip() + fields = self.get_standard_placeholder_fields(message) + fields['path_distance'] = self._format_path_distance() + formatted = format_piped_template( + self.path_reply_prefix, + {k: str(v) for k, v in fields.items()}, + message=message, + logger=self.logger, + prefix_hex_chars=getattr(self.bot, 'prefix_hex_chars', 2), + ).rstrip() if not formatted: return '' return formatted + '\n' @@ -401,8 +422,10 @@ class PathCommand(BaseCommand): ) -> str: self.logger.info(f"Decoding path with {len(node_ids)} nodes: {','.join(node_ids)}") if not self._should_resolve_repeater_names(node_ids, routing_info): + self._last_path_distance_km = None return self._format_repeater_resolution_deferred(node_ids) repeater_info = await self._lookup_repeater_names(node_ids) + self._last_path_distance_km = self._calculate_path_distance_km(node_ids, repeater_info) return self._format_path_response(node_ids, repeater_info) def can_execute(self, message: MeshMessage, skip_channel_check: bool = False) -> bool: @@ -905,6 +928,42 @@ class PathCommand(BaseCommand): path_prefix_hex_chars=path_prefix_hex_chars, ) + def _calculate_path_distance_km( + self, node_ids: list[str], repeater_info: dict[str, dict[str, Any]] + ) -> Optional[float]: + """Total distance along sender -> each hop -> bot, in kilometres. + + Returns None when any node in the chain has no usable coordinates, since a + partial sum would understate the real distance travelled. + """ + if self.bot_latitude is None or self.bot_longitude is None: + return None + + chain: list[tuple[float, float]] = [] + + sender = self._get_sender_location() + if sender is None: + return None + chain.append(sender) + + for node_id in node_ids: + info = repeater_info.get(node_id, {}) + # A prefix collision has no single node to measure from. + if not info.get('found', False) or info.get('collision', False): + return None + lat = info.get('latitude') + lon = info.get('longitude') + if lat is None or lon is None or (lat == 0 and lon == 0): + return None + chain.append((lat, lon)) + + chain.append((self.bot_latitude, self.bot_longitude)) + + total = 0.0 + for (lat1, lon1), (lat2, lon2) in zip(chain, chain[1:], strict=False): + total += calculate_distance(lat1, lon1, lat2, lon2) + return total + def _format_path_response(self, node_ids: list[str], repeater_info: dict[str, dict[str, Any]]) -> str: """Format the path decode response diff --git a/modules/commands/test_command.py b/modules/commands/test_command.py index c1f31a1..7cc725e 100644 --- a/modules/commands/test_command.py +++ b/modules/commands/test_command.py @@ -644,12 +644,14 @@ class TestCommand(BaseCommand): return f"{distance:.1f}km" - def format_response(self, message: MeshMessage, response_format: str) -> str: + def format_response(self, message: MeshMessage, response_format: str, + extra: Optional[dict[str, Any]] = None) -> str: """Override to handle phrase extraction. Args: message: The original message. response_format: The format string. + extra: Additional placeholders merged over this command's own fields. Returns: str: Formatted response string. diff --git a/tests/unit/test_path_command_distance.py b/tests/unit/test_path_command_distance.py new file mode 100644 index 0000000..0ccc4c6 --- /dev/null +++ b/tests/unit/test_path_command_distance.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +""" +Unit tests for the PathCommand {path_distance} placeholder. + +Distance is summed sender -> each resolved hop -> bot, and is deliberately blank +whenever any node in that chain lacks usable coordinates, so a partial sum never +gets reported as the real distance travelled. +""" + + +import pytest + +from modules.commands.path_command import PathCommand + + +@pytest.mark.unit +class TestPathCommandDistance: + """_calculate_path_distance_km and its {path_distance} rendering.""" + + @pytest.fixture + def path_command(self, mock_bot): + cmd = PathCommand(mock_bot) + # Bot sits at the origin; sender and hops are placed east of it. + cmd.bot_latitude = 47.0 + cmd.bot_longitude = -122.0 + cmd._get_sender_location = lambda: (47.0, -122.5) + return cmd + + @staticmethod + def _info(lat, lon, **over): + base = {'found': True, 'collision': False, 'latitude': lat, 'longitude': lon} + base.update(over) + return base + + def test_sums_sender_through_hops_to_bot(self, path_command): + info = {'AA': self._info(47.0, -122.4), 'BB': self._info(47.0, -122.2)} + km = path_command._calculate_path_distance_km(['AA', 'BB'], info) + assert km is not None + # Three legs spanning 0.5 deg of longitude at 47N (~75.9 km/deg) => ~38 km. + assert 37.0 < km < 39.0 + + def test_renders_with_km_suffix(self, path_command): + info = {'AA': self._info(47.0, -122.4), 'BB': self._info(47.0, -122.2)} + path_command._last_path_distance_km = path_command._calculate_path_distance_km( + ['AA', 'BB'], info + ) + rendered = path_command._format_path_distance() + assert rendered.endswith("km") + assert rendered[0].isdigit() + + def test_blank_when_a_hop_has_no_coordinates(self, path_command): + info = {'AA': self._info(47.0, -122.4), 'BB': self._info(None, None)} + assert path_command._calculate_path_distance_km(['AA', 'BB'], info) is None + + def test_blank_when_a_hop_is_a_prefix_collision(self, path_command): + info = {'AA': self._info(47.0, -122.4), 'BB': self._info(47.0, -122.2, collision=True)} + assert path_command._calculate_path_distance_km(['AA', 'BB'], info) is None + + def test_blank_when_a_hop_is_unresolved(self, path_command): + info = {'AA': self._info(47.0, -122.4), 'BB': {'found': False}} + assert path_command._calculate_path_distance_km(['AA', 'BB'], info) is None + + def test_blank_when_hop_coordinates_are_null_island(self, path_command): + """0,0 in the DB means 'unset', not a real position in the Gulf of Guinea.""" + info = {'AA': self._info(0, 0)} + assert path_command._calculate_path_distance_km(['AA'], info) is None + + def test_blank_when_sender_location_unknown(self, path_command): + path_command._get_sender_location = lambda: None + info = {'AA': self._info(47.0, -122.4)} + assert path_command._calculate_path_distance_km(['AA'], info) is None + + def test_blank_when_bot_has_no_configured_position(self, path_command): + path_command.bot_latitude = None + info = {'AA': self._info(47.0, -122.4)} + assert path_command._calculate_path_distance_km(['AA'], info) is None + + def test_placeholder_is_empty_string_when_unmeasurable(self, path_command): + path_command._last_path_distance_km = None + assert path_command._format_path_distance() == ""