From f994171818cf92917f427346bccc1ea72f017bb1 Mon Sep 17 00:00:00 2001 From: agessaman Date: Fri, 2 Oct 2026 07:55:18 -0700 Subject: [PATCH] feat(weather): [Weather] always_show_location names the place in every wx/gwx reply wx and gwx name the place only when it adds information: a city outside default_state or default_country, or a position. always_show_location (default false) names every place a lookup finds instead, including cities in the bot's own state and ZIP codes. For a ZIP code, gwx now does a reverse lookup when the option is on (it skipped that since the name was never shown), and wx reuses the reverse lookup it already did for a companion position. A failed lookup leaves the reply unnamed rather than repeating the ZIP code. The prefix comes out of the message budget as before. The option is in the wx settings schema for the web viewer, the example config, the command reference and the changelog. --- CHANGELOG.md | 1 + config.ini.example | 7 +++ docs/command-reference.md | 4 +- .../commands/alternatives/wx_international.py | 28 +++++++++-- modules/commands/wx_command.py | 10 +++- tests/unit/test_gwx_location_label.py | 48 +++++++++++++++++- tests/unit/test_wx_always_show_location.py | 49 +++++++++++++++++++ 7 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_wx_always_show_location.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ab933f7..b8f8962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ semantic versioning. ### Added +- `[Weather] always_show_location` (default `false`): when `true`, `wx` and `gwx` name the place in every reply a place is found for (`Lake Stevens, WA: …`), including ZIP codes and cities in the bot's own state or country, instead of only when the name adds information. The name takes message length from the forecast, and a ZIP code costs one extra reverse lookup. It's also in the web viewer's `wx` settings. - `gwx hourly`: the next hours from Open-Meteo, packed into one reply like `wx`'s NOAA hourly forecast (time, condition, chance of precipitation, temperature, wind), in the configured units and translated. `gwx` listed `hourly` in its help but read it as part of the place name. `wx` with `weather_provider = openmeteo` gets it too. - Web viewer admin role when `web_viewer_password` is set: Dashboard, Contacts, and Mesh Graph are readable anonymously; Radio, Config, Logs, and all diff --git a/config.ini.example b/config.ini.example index 62a09c0..04b8cb6 100644 --- a/config.ini.example +++ b/config.ini.example @@ -915,6 +915,13 @@ default_country = US # Default: false (show usage instead) use_bot_location_when_no_location = false +# When true, wx and gwx name the place in every reply a place is found for ("Lake Stevens, WA: ..."). +# When false, they name it only when it adds information: a city outside default_state or +# default_country, or a position (the sender's, the bot's or typed coordinates). +# The name takes message length from the forecast, and a ZIP code costs one extra reverse lookup. +# Default: false +always_show_location = false + # Temperature unit for weather display (wx, gwx, the weather service) # Options: fahrenheit, celsius # Celsius also switches distances (visibility) to kilometers. diff --git a/docs/command-reference.md b/docs/command-reference.md index ba15152..cde0410 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -193,7 +193,7 @@ wx **Units** follow `[Weather] temperature_unit` and `wind_speed_unit` (`[Wx_Command]` overrides them for `wx`). The default is °F and mph. -**The place is named** in the reply only when it adds information: for a position (the sender's, the bot's or typed coordinates) when a place is found for it, and for a city in another state than `[Weather] default_state` (or any city, when `default_state` is not set). ZIP codes are not named. +**The place is named** in the reply only when it adds information: for a position (the sender's, the bot's or typed coordinates) when a place is found for it, and for a city in another state than `[Weather] default_state` (or any city, when `default_state` is not set). ZIP codes are not named. With `[Weather] always_show_location = true`, every city, ZIP code and position a place is found for is named; the name takes message length from the forecast, and a ZIP code costs one extra reverse lookup. --- @@ -224,7 +224,7 @@ gwx Reykjavik 10d **Units** follow `[Weather] temperature_unit`, `wind_speed_unit` and `precipitation_unit`. -**The place is named** in the reply when it adds information: a city in another country than `[Weather] default_country` (in the US, another state than `default_state`, or any US city when `default_state` is not set), or a position a place was found for. A city in the bot's own region and ZIP codes are not named. +**The place is named** in the reply when it adds information: a city in another country than `[Weather] default_country` (in the US, another state than `default_state`, or any US city when `default_state` is not set), or a position a place was found for. A city in the bot's own region and ZIP codes are not named. `[Weather] always_show_location = true` names every place a lookup finds, as for `wx`. **Custom sources:** a `custom.wxsim.` or `custom.mqtt_weather.` key in `[Weather]` answers `wx ` and `gwx ` from that station. WXSIM sources answer current conditions, `tomorrow` and `Nd`; MQTT sources answer current conditions only. Options a source cannot answer (`hourly` and `alerts` for both) reply that they are not available instead of sending the current conditions. diff --git a/modules/commands/alternatives/wx_international.py b/modules/commands/alternatives/wx_international.py index 3c243c9..a6a93d9 100644 --- a/modules/commands/alternatives/wx_international.py +++ b/modules/commands/alternatives/wx_international.py @@ -98,6 +98,7 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand): self.default_city = self.bot.config.get('Weather', 'default_city', fallback='').strip() self.default_state = self.bot.config.get('Weather', 'default_state', fallback='') self.default_country = self.bot.config.get('Weather', 'default_country', fallback='US') + self.always_show_location = self.bot.config.getboolean('Weather', 'always_show_location', fallback=False) # Get unit preferences from config self.temperature_unit = self.bot.config.get('Weather', 'temperature_unit', fallback='fahrenheit').lower() @@ -662,7 +663,19 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand): timeout=10 ) if lat is not None and lon is not None: - # A ZIP code is not named in the reply (as in wx), so no reverse lookup. + # A ZIP code is not named in the reply (as in wx), so no reverse lookup, + # unless [Weather] always_show_location asks for every place to be named. + if not self.always_show_location: + return lat, lon, {}, None + try: + reverse_location = rate_limited_nominatim_reverse_sync( + self.bot, f"{lat}, {lon}", timeout=10 + ) + except Exception as e: + self.logger.debug(f"Reverse geocoding failed for ZIP code {location}: {e}") + reverse_location = None + if reverse_location: + return lat, lon, reverse_location.raw.get('address', {}) or {}, reverse_location return lat, lon, {}, None # Invalid or unknown US ZIP; do not fall through to city (avoids foreign matches) return None, None, None, None @@ -707,14 +720,19 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand): place was found for them. A ZIP code is not named. A city is named when it resolved to another country than [Weather] default_country or, in the US, to another state than default_state (or no default_state is set). + + With [Weather] always_show_location, any place a lookup found is named. """ - if self._ZIP_RE.match(location.strip()): - return False if not address_info: return False - if self._COORDINATES_RE.match(location): - # Without a place name the label would only repeat the coordinates. + is_zip = bool(self._ZIP_RE.match(location.strip())) + if is_zip and not self.always_show_location: + return False + if is_zip or self._COORDINATES_RE.match(location): + # Without a place name the label would only repeat the ZIP code or coordinates. return any(address_info.get(field) for field in ('city', 'town', 'village', 'municipality', 'city_district')) + if self.always_show_location: + return True country = (address_info.get('country_code') or '').upper() default_country = (self.default_country or '').strip().upper() if country and default_country and country != default_country: diff --git a/modules/commands/wx_command.py b/modules/commands/wx_command.py index 8e95f5e..09efe02 100644 --- a/modules/commands/wx_command.py +++ b/modules/commands/wx_command.py @@ -222,6 +222,11 @@ class WxCommand(WeatherCommandMixin, BaseCommand): "default": "", "help": "2-letter state for city disambiguation (e.g. WA). Shared weather setting."}, {"key": "default_country", "label": "Default country", "type": "str", "section": "Weather", "default": "US", "help": "2-letter country code (e.g. US). Shared weather setting."}, + {"key": "always_show_location", "label": "Always name the location", "type": "bool", "section": "Weather", + "default": False, + "help": "Name the place in every wx/gwx reply a place is found for, not only when it is outside the " + "default state or country. Costs message length, and a reverse lookup for ZIP codes. " + "Shared weather setting."}, ] # Error constants @@ -265,6 +270,7 @@ class WxCommand(WeatherCommandMixin, BaseCommand): self.default_city = self.bot.config.get('Weather', 'default_city', fallback='').strip() self.default_state = self.bot.config.get('Weather', 'default_state', fallback='') self.default_country = self.bot.config.get('Weather', 'default_country', fallback='US') + self.always_show_location = self.bot.config.getboolean('Weather', 'always_show_location', fallback=False) # Initialize geocoder (will use rate-limited helpers for actual calls) # Keep geolocator for backwards compatibility, but prefer rate-limited helpers @@ -984,9 +990,9 @@ class WxCommand(WeatherCommandMixin, BaseCommand): states_different = (actual_state != self.default_state and actual_state != default_state_full) # Always show location if using companion location, or if state is different - if using_companion_location or states_different: + if using_companion_location or states_different or self.always_show_location: location_prefix = f"{actual_city}, {actual_state}: " if actual_state else f"{actual_city}: " - elif location_type == "zipcode" and using_companion_location: + elif location_type == "zipcode" and (using_companion_location or self.always_show_location): # For zipcode with companion location, try to get city name from reverse geocoding location_str = self._coordinates_to_location_string(lat, lon) if location_str: diff --git a/tests/unit/test_gwx_location_label.py b/tests/unit/test_gwx_location_label.py index fb0ea2c..1a49568 100644 --- a/tests/unit/test_gwx_location_label.py +++ b/tests/unit/test_gwx_location_label.py @@ -10,10 +10,13 @@ from modules.commands.alternatives.wx_international import GlobalWxCommand MODULE = "modules.commands.alternatives.wx_international" -def _gwx(default_country="US", default_state=""): +def _gwx(default_country="US", default_state="", always=None): + weather = {"default_country": default_country, "default_state": default_state} + if always is not None: + weather["always_show_location"] = str(always).lower() config = configparser.ConfigParser() config.read_dict({ - "Weather": {"default_country": default_country, "default_state": default_state}, + "Weather": weather, "Gwx_Command": {}, "Bot": {}, }) @@ -74,3 +77,44 @@ def test_an_unnamed_reply_keeps_the_whole_budget(): cmd = _gwx("GB") _reply(cmd, "London", LONDON) assert cmd._get_open_meteo_weather_with_conditions.call_args.kwargs["location_prefix_len"] == 0 + + +@pytest.mark.parametrize(("location", "address", "expected"), [ + ("Seattle", SEATTLE, "Seattle, WA: Clear 20°C"), # inside the default state + ("London", LONDON, "London, GB: Clear 20°C"), + ("98104", SEATTLE, "Seattle, WA: Clear 20°C"), + ("98104", {}, "Clear 20°C"), # the reverse lookup failed + ("98104", {"country_code": "us", "state": "Washington"}, "Clear 20°C"), # no place name + ("47.60620,-122.33210", {"country_code": "us", "state": "Washington"}, "Clear 20°C"), +]) +def test_always_show_location_names_every_place_found(location, address, expected): + assert _reply(_gwx("US", "WA", always=True), location, address) == expected + + +@pytest.mark.parametrize("always", [None, False]) +def test_always_show_location_is_off_by_default(always): + assert _reply(_gwx("US", "WA", always=always), "Seattle", SEATTLE) == "Clear 20°C" + + +def test_always_show_location_reverse_geocodes_a_zip_code(): + cmd = _gwx("US", "WA", always=True) + place = Mock(raw={"address": SEATTLE}) + with patch(f"{MODULE}.geocode_zipcode_sync", return_value=(47.6, -122.3)), patch( + f"{MODULE}.rate_limited_nominatim_reverse_sync", return_value=place + ) as reverse: + assert cmd.geocode_location("98104") == (47.6, -122.3, SEATTLE, place) + reverse.assert_called_once() + + +def test_always_show_location_survives_a_failed_zip_reverse_lookup(): + cmd = _gwx("US", "WA", always=True) + with patch(f"{MODULE}.geocode_zipcode_sync", return_value=(47.6, -122.3)), patch( + f"{MODULE}.rate_limited_nominatim_reverse_sync", side_effect=TimeoutError + ): + assert cmd.geocode_location("98104") == (47.6, -122.3, {}, None) + + +def test_a_named_place_comes_out_of_the_budget(): + cmd = _gwx("US", "WA", always=True) + _reply(cmd, "Seattle", SEATTLE) + assert cmd._get_open_meteo_weather_with_conditions.call_args.kwargs["location_prefix_len"] == len("Seattle, WA: ") diff --git a/tests/unit/test_wx_always_show_location.py b/tests/unit/test_wx_always_show_location.py new file mode 100644 index 0000000..60ccdd5 --- /dev/null +++ b/tests/unit/test_wx_always_show_location.py @@ -0,0 +1,49 @@ +"""[Weather] always_show_location makes wx (NOAA) name the place for cities in the default state and ZIP codes.""" + +import configparser +from unittest.mock import Mock, patch + +import pytest + +from modules.commands.wx_command import WxCommand + + +def _wx(always=None): + weather = {"weather_provider": "noaa", "default_state": "WA"} + if always is not None: + weather["always_show_location"] = str(always).lower() + config = configparser.ConfigParser() + config.read_dict({"Weather": weather, "Wx_Command": {}, "Bot": {}}) + bot = Mock() + bot.config = config + bot.db_manager.get_cached_geocoding = Mock(return_value=(None, None)) + return WxCommand(bot) + + +def _reply(cmd, location, location_type): + with patch.object(WxCommand, "get_noaa_weather", Mock(return_value=("Today: Clear 60°F", {}))), patch.object( + WxCommand, "get_weather_alerts_noaa", Mock(return_value=WxCommand.NO_ALERTS) + ), patch.object(WxCommand, "zipcode_to_lat_lon", Mock(return_value=(48.0, -122.1))), patch.object( + WxCommand, "city_to_lat_lon", + Mock(return_value=(47.6, -122.3, {"city": "Seattle", "state": "Washington", "country_code": "us"})), + ), patch.object(WxCommand, "_coordinates_to_location_string", Mock(return_value="Lake Stevens, WA")): + return cmd._get_weather_for_location_sync(location, location_type, message=None) + + +@pytest.mark.parametrize(("location", "location_type", "expected"), [ + ("seattle", "city", "Seattle, WA: Today: Clear 60°F"), + ("98258", "zipcode", "Lake Stevens, WA: Today: Clear 60°F"), +]) +def test_always_show_location_names_the_place(location, location_type, expected): + assert _reply(_wx(always=True), location, location_type) == expected + + +@pytest.mark.parametrize("always", [None, False]) +@pytest.mark.parametrize(("location", "location_type"), [("seattle", "city"), ("98258", "zipcode")]) +def test_off_by_default_a_city_in_the_default_state_and_a_zip_code_are_not_named(always, location, location_type): + assert _reply(_wx(always=always), location, location_type) == "Today: Clear 60°F" + + +def test_the_option_is_in_the_settings_schema(): + entry = next(e for e in WxCommand.settings_schema if e["key"] == "always_show_location") + assert entry["section"] == "Weather" and entry["type"] == "bool" and entry["default"] is False