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