refactor(weather): share wx and gwx helpers through WeatherCommandMixin

Five methods were the same in WxCommand and GlobalWxCommand: _format_high_low, _get_custom_mqtt_weather_topic, _mqtt_weather_line (only the catalog namespace differed), _count_display_width and _send_multiday_forecast. They now live once in modules/weather_common.WeatherCommandMixin, which both commands list before BaseCommand; each sets translation_ns ('commands.wx' / 'commands.gwx'). The mixin is outside modules/commands, so the plugin loader does not treat it as a command.

A new characterization test pins how a multi-day forecast is packed into messages (packing, rate-limit skips, pauses); it passes on the code before and after.
This commit is contained in:
agessaman committed 2026-10-01 06:25:01 -07:00
1 parent 2c06a942b1
commit 53361328a2
4 files changed
+173 -231

No files matched your search

@@ -11,20 +11,15 @@ from typing import Any, Optional, Union
import requests
from ...clients.mqtt_weather import (
get_mqtt_weather_topic,
load_mqtt_weather_format_config,
mqtt_weather_display_for_topic,
)
from ...clients.wxsim_parser import WXSIMParser
from ...location import get_bot_lat_lon, get_companion_lat_lon
from ...models import MeshMessage
from ...utils import (
format_temperature_high_low,
geocode_city_sync,
geocode_zipcode_sync,
rate_limited_nominatim_reverse_sync,
)
from ...weather_common import WeatherCommandMixin
from ..base_command import BaseCommand
# Multiday: plain digits, 7day/7-day, or suffix form 7d/10d (min 2, max below). Open-Meteo allows up to 16 forecast days.
@@ -37,11 +32,12 @@ VISIBILITY_CAP_MI = 20
VISIBILITY_CAP_KM = 32
class GlobalWxCommand(BaseCommand):
class GlobalWxCommand(WeatherCommandMixin, BaseCommand):
"""Handles global weather commands with city/location support"""
# Plugin metadata
name = "gwx"
translation_ns = "commands.gwx"
keywords = ['gwx', 'globalweather', 'gwxa']
description = "Get weather information for any global location (usage: gwx Tokyo)"
category = "weather"
@@ -110,11 +106,6 @@ class GlobalWxCommand(BaseCommand):
"""
return self.temperature_unit == 'celsius'
def _format_high_low(self, high: Optional[Union[int, float]], low: Optional[Union[int, float]], temp_symbol: str) -> str:
"""Format high/low using [Weather] temperature_*_format templates."""
return format_temperature_high_low(self.bot.config, high, low, temp_symbol, self.logger,
translator=self.response_translator)
def _load_weather_model(self) -> Optional[str]:
"""Load and normalize Open-Meteo model selection from config.
@@ -169,34 +160,6 @@ class GlobalWxCommand(BaseCommand):
"""Get bot location from config ([Bot] bot_latitude, bot_longitude)."""
return get_bot_lat_lon(self.bot, self.logger)
def _get_custom_mqtt_weather_topic(self, location: Optional[str] = None) -> Optional[str]:
return get_mqtt_weather_topic(self.bot.config, location)
def _mqtt_weather_line(
self,
topic: str,
forecast_type: str,
location_name: Optional[str],
) -> str:
if forecast_type != "default":
return self.translate("commands.gwx.mqtt_forecast_not_supported")
fmt = load_mqtt_weather_format_config(self.bot.config)
cache = getattr(self.bot, "mqtt_weather_cache", None)
text, err = mqtt_weather_display_for_topic(topic, cache, fmt)
if text is not None:
if location_name:
return f"{location_name}: {text}"
return text
if err == "no_cache":
return self.translate("commands.gwx.mqtt_weather_no_subscriber")
if err in ("no_data", "empty_payload", "empty_after_sanitize"):
return self.translate("commands.gwx.mqtt_weather_no_data")
if err == "stale":
return self.translate("commands.gwx.mqtt_weather_stale")
detail = (err or "unknown").replace("_", " ")
return self.translate("commands.gwx.mqtt_weather_payload_error", detail=detail)
def _get_custom_wxsim_source(self, location: Optional[str] = None) -> Optional[str]:
"""Get custom WXSIM source URL from config.
@@ -1241,83 +1204,6 @@ class GlobalWxCommand(BaseCommand):
self.logger.error(f"Error formatting {num_days}-day forecast: {e}")
return self.translate('commands.gwx.multiday_error', num_days=num_days)
def _count_display_width(self, text: str) -> int:
"""Count UTF-8 byte length of text. Matches RF packet byte limit from get_max_message_length()."""
return len(text.encode('utf-8'))
async def _send_multiday_forecast(self, message: MeshMessage, forecast_text: str) -> None:
"""Send multi-day forecast response, splitting into multiple messages if needed.
Args:
message: The original message (for reply context).
forecast_text: The full forecast text (lines separated by \n).
"""
import asyncio
# Get max message length dynamically
max_length = self.get_max_message_length(message)
lines = forecast_text.split('\n')
# Remove empty lines
lines = [line.strip() for line in lines if line.strip()]
if not lines:
return
# If single line and under max_length chars, send as-is
if self._count_display_width(forecast_text) <= max_length:
await self.send_response(message, forecast_text)
return
# Multi-line message - try to fit as many days as possible in one message
# Only split when necessary (message would exceed max_length chars)
current_message = ""
message_count = 0
for i, line in enumerate(lines):
if not line:
continue
# Check if adding this line would exceed max_length characters (using display width)
test_message = current_message + "\n" + line if current_message else line
# Only split if message would exceed max_length chars (using display width)
if self._count_display_width(test_message) > max_length:
# Send current message and start new one
if current_message:
# Per-user rate limit applies only to first message (trigger); skip for continuations
await self.send_response(
message, current_message,
skip_user_rate_limit=(message_count > 0)
)
message_count += 1
# Wait between messages (same as other commands)
if i < len(lines):
await asyncio.sleep(2.0)
current_message = line
else:
# Single line is too long, send it anyway (will be truncated by bot)
await self.send_response(
message, line,
skip_user_rate_limit=(message_count > 0)
)
message_count += 1
if i < len(lines) - 1:
await asyncio.sleep(2.0)
current_message = ""
else:
# Add line to current message (fits within max_length)
if current_message:
current_message += "\n" + line
else:
current_message = line
# Send the last message if there's content (continuation; skip per-user rate limit)
if current_message:
await self.send_response(message, current_message, skip_user_rate_limit=True)
def _degrees_to_direction(self, degrees: float) -> str:
"""Convert wind direction in degrees to compass direction with emoji.
+3 -114
View File
@@ -14,11 +14,6 @@ from typing import Callable, Optional, ParamSpec, TypeVar
import requests
from .. import alert_format
from ..clients.mqtt_weather import (
get_mqtt_weather_topic,
load_mqtt_weather_format_config,
mqtt_weather_display_for_topic,
)
# First-party modules with only required dependencies; they always import.
from ..clients.wxsim_parser import WXSIMParser
@@ -31,6 +26,7 @@ from ..utils import (
geocode_zipcode_sync,
normalize_us_state,
)
from ..weather_common import WeatherCommandMixin
from .alternatives.wx_international import GlobalWxCommand
from .base_command import BaseCommand
from .rain_command import nws_http_means_no_coverage
@@ -42,13 +38,14 @@ _P = ParamSpec("_P")
_T = TypeVar("_T")
class WxCommand(BaseCommand):
class WxCommand(WeatherCommandMixin, BaseCommand):
"""Handles weather commands with zipcode support"""
# Plugin metadata
# Read-only informational output; safe for scheduled {cmd:...} rendering.
render_safe = True
name = "wx"
translation_ns = "commands.wx"
keywords = ['wx', 'weather', 'wxa', 'wxalert']
description = "Get weather information for a zip code (usage: wx 12345)"
category = "weather"
@@ -156,11 +153,6 @@ class WxCommand(BaseCommand):
# Lazy: None = unknown, False = NOAA alerts unavailable (non-US / no coverage)
self._nws_alerts_available = None
def _format_high_low(self, high: Optional[float], low: Optional[float], temp_symbol: str) -> str:
"""Format high/low using [Weather] temperature_*_format templates."""
return format_temperature_high_low(self.bot.config, high, low, temp_symbol, self.logger,
translator=self.response_translator)
@staticmethod
def _noaa_period_temp_symbol(period: dict) -> str:
u = (period.get("temperatureUnit") or "F").upper()
@@ -286,39 +278,6 @@ class WxCommand(BaseCommand):
return None
def _get_custom_mqtt_weather_topic(self, location: Optional[str] = None) -> Optional[str]:
"""MQTT topic for custom.mqtt_weather.<name> (see get_mqtt_weather_topic)."""
return get_mqtt_weather_topic(self.bot.config, location)
def _mqtt_weather_line(
self,
topic: str,
forecast_type: str,
location_name: Optional[str],
) -> str:
"""Format cached MQTT payload for wx output."""
if forecast_type != "default":
return self.translate("commands.wx.mqtt_forecast_not_supported")
fmt = load_mqtt_weather_format_config(self.bot.config)
cache = getattr(self.bot, "mqtt_weather_cache", None)
text, err = mqtt_weather_display_for_topic(topic, cache, fmt)
if text is not None:
if location_name:
return f"{location_name}: {text}"
return text
return self._mqtt_weather_error_key(err)
def _mqtt_weather_error_key(self, err: Optional[str]) -> str:
if err == "no_cache":
return self.translate("commands.wx.mqtt_weather_no_subscriber")
if err in ("no_data", "empty_payload", "empty_after_sanitize"):
return self.translate("commands.wx.mqtt_weather_no_data")
if err == "stale":
return self.translate("commands.wx.mqtt_weather_stale")
detail = (err or "unknown").replace("_", " ")
return self.translate("commands.wx.mqtt_weather_payload_error", detail=detail)
def _get_wxsim_weather(self, source_url: str, forecast_type: str = "default",
num_days: int = 7, message: MeshMessage = None,
location_name: Optional[str] = None) -> str:
@@ -1895,76 +1854,6 @@ class WxCommand(BaseCommand):
return result
def _count_display_width(self, text: str) -> int:
"""Count UTF-8 byte length of text. Matches RF packet byte limit from get_max_message_length()."""
return len(text.encode('utf-8'))
async def _send_multiday_forecast(self, message: MeshMessage, forecast_text: str):
"""Send multi-day forecast response, splitting into multiple messages if needed"""
# Get max message length dynamically
max_length = self.get_max_message_length(message)
lines = forecast_text.split('\n')
# Remove empty lines
lines = [line.strip() for line in lines if line.strip()]
if not lines:
return
# If single line and under max_length chars, send as-is
if self._count_display_width(forecast_text) <= max_length:
await self.send_response(message, forecast_text)
return
# Multi-line message - try to fit as many days as possible in one message
# Only split when necessary (message would exceed max_length chars)
current_message = ""
message_count = 0
for i, line in enumerate(lines):
if not line:
continue
# Check if adding this line would exceed max_length characters (using display width)
test_message = current_message + "\n" + line if current_message else line
# Only split if message would exceed max_length chars (using display width)
if self._count_display_width(test_message) > max_length:
# Send current message and start new one
if current_message:
# Per-user rate limit applies only to first message (trigger); skip for continuations
await self.send_response(
message, current_message,
skip_user_rate_limit=(message_count > 0)
)
message_count += 1
# Wait between messages (same as other commands)
if i < len(lines):
await asyncio.sleep(2.0)
current_message = line
else:
# Single line is too long, send it anyway (will be truncated by bot)
await self.send_response(
message, line,
skip_user_rate_limit=(message_count > 0)
)
message_count += 1
if i < len(lines) - 1:
await asyncio.sleep(2.0)
current_message = ""
else:
# Add line to current message (fits within max_length chars)
if current_message:
current_message += "\n" + line
else:
current_message = line
# Send the last message if there's content (continuation; skip per-user rate limit)
if current_message:
await self.send_response(message, current_message, skip_user_rate_limit=True)
def get_weather_alerts_noaa(self, lat: float, lon: float, return_full_data: bool = False) -> tuple:
"""Get weather alerts from NOAA with full metadata extraction and prioritization
+115
View File
@@ -0,0 +1,115 @@
"""Behavior the wx (NOAA) and gwx (Open-Meteo) commands share.
``WeatherCommandMixin`` goes before ``BaseCommand`` in a weather command's
bases and expects the BaseCommand API (``bot``, ``logger``, ``translate``,
``send_response``, ``get_max_message_length``, ``response_translator``). Each
command sets ``translation_ns`` to its own catalog namespace.
"""
from __future__ import annotations
import asyncio
from typing import Any, Optional, Union
from .clients.mqtt_weather import (
get_mqtt_weather_topic,
load_mqtt_weather_format_config,
mqtt_weather_display_for_topic,
)
from .utils import format_temperature_high_low
Number = Union[int, float]
class WeatherCommandMixin:
# Catalog namespace for this command's strings, e.g. "commands.wx".
translation_ns: str = ""
# Provided by BaseCommand.
bot: Any
logger: Any
response_translator: Any
translate: Any
send_response: Any
get_max_message_length: Any
def _format_high_low(self, high: Optional[Number], low: Optional[Number], temp_symbol: str) -> str:
"""Format high/low using [Weather] temperature_*_format templates."""
return format_temperature_high_low(self.bot.config, high, low, temp_symbol, self.logger,
translator=self.response_translator)
def _get_custom_mqtt_weather_topic(self, location: Optional[str] = None) -> Optional[str]:
"""MQTT topic for custom.mqtt_weather.<name> (see get_mqtt_weather_topic)."""
return get_mqtt_weather_topic(self.bot.config, location)
def _mqtt_weather_line(self, topic: str, forecast_type: str, location_name: Optional[str]) -> str:
"""Format the cached MQTT weather payload for ``topic``, or the matching error text."""
ns = self.translation_ns
if forecast_type != "default":
return self.translate(f"{ns}.mqtt_forecast_not_supported")
fmt = load_mqtt_weather_format_config(self.bot.config)
cache = getattr(self.bot, "mqtt_weather_cache", None)
text, err = mqtt_weather_display_for_topic(topic, cache, fmt)
if text is not None:
return f"{location_name}: {text}" if location_name else text
if err == "no_cache":
return self.translate(f"{ns}.mqtt_weather_no_subscriber")
if err in ("no_data", "empty_payload", "empty_after_sanitize"):
return self.translate(f"{ns}.mqtt_weather_no_data")
if err == "stale":
return self.translate(f"{ns}.mqtt_weather_stale")
detail = (err or "unknown").replace("_", " ")
return self.translate(f"{ns}.mqtt_weather_payload_error", detail=detail)
def _count_display_width(self, text: str) -> int:
"""Count UTF-8 byte length of text. Matches RF packet byte limit from get_max_message_length()."""
return len(text.encode('utf-8'))
async def _send_multiday_forecast(self, message: Any, forecast_text: str) -> None:
"""Send a multi-day forecast, packing whole lines into as few messages as fit.
A line too long for one message goes out on its own. Messages after the
first skip the per-user rate limit and are spaced 2 s apart.
"""
max_length = self.get_max_message_length(message)
lines = [line.strip() for line in forecast_text.split('\n') if line.strip()]
if not lines:
return
if self._count_display_width(forecast_text) <= max_length:
await self.send_response(message, forecast_text)
return
current_message = ""
message_count = 0
for i, line in enumerate(lines):
if not line:
continue
test_message = current_message + "\n" + line if current_message else line
if self._count_display_width(test_message) > max_length:
if current_message:
await self.send_response(
message, current_message,
skip_user_rate_limit=(message_count > 0)
)
message_count += 1
if i < len(lines):
await asyncio.sleep(2.0)
current_message = line
else:
# Single line is too long, send it anyway (will be truncated by bot)
await self.send_response(
message, line,
skip_user_rate_limit=(message_count > 0)
)
message_count += 1
if i < len(lines) - 1:
await asyncio.sleep(2.0)
current_message = ""
elif current_message:
current_message += "\n" + line
else:
current_message = line
# Last message is a continuation, so it skips the per-user rate limit
if current_message:
await self.send_response(message, current_message, skip_user_rate_limit=True)
@@ -0,0 +1,52 @@
"""Characterization: how wx and gwx pack a multi-day forecast into messages."""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from modules.models import MeshMessage
def _cmd(cls):
cmd = object.__new__(cls)
cmd.bot = MagicMock()
cmd.logger = MagicMock()
cmd.get_max_message_length = lambda message: 40
cmd.send_response = AsyncMock(return_value=True)
return cmd
def _classes():
from modules.commands.alternatives.wx_international import GlobalWxCommand
from modules.commands.wx_command import WxCommand
return [WxCommand, GlobalWxCommand]
@pytest.mark.parametrize("cls_index", [0, 1])
@pytest.mark.parametrize(
("text", "expected"),
[
("Mon 50/40 rain\nTue 52/41", [("Mon 50/40 rain\nTue 52/41", {})]),
(
"Mon 50/40 rain all day\n\nTue 52/41 sunny\nWed 49/38 showers late\nThu 51/40",
[
("Mon 50/40 rain all day\nTue 52/41 sunny", {"skip_user_rate_limit": False}),
("Wed 49/38 showers late\nThu 51/40", {"skip_user_rate_limit": True}),
],
),
(
"x" * 50 + "\nshort line",
[("x" * 50, {"skip_user_rate_limit": False}), ("short line", {"skip_user_rate_limit": True})],
),
(" \n ", []),
],
)
async def test_multiday_packing(cls_index, text, expected):
cmd = _cmd(_classes()[cls_index])
msg = MeshMessage(content="wx", sender_id="Ann", channel="general")
with patch("asyncio.sleep", AsyncMock()) as sleep:
await cmd._send_multiday_forecast(msg, text)
sent = [(c.args[1], c.kwargs) for c in cmd.send_response.await_args_list]
assert sent == expected
assert sleep.await_count == max(0, len(expected) - 1)