refactor(weather): wx and gwx execute share their dispatch steps

The custom MQTT and WXSIM replies, the no-location fallback chain (sender's position, default_city, bot's position) and the reply tail (two-part alerts, multiday split, single message) move into WeatherCommandMixin. Each command keeps its own WXSIM fetch (_fetch_wxsim) and wx keeps naming fallback positions in its log lines (_fallback_place_name). The no-location MQTT reply still never splits a multiday answer. The execute dispatch goldens are unchanged.

modules/commands/alternatives/wx_international.py

@@ -212,6 +212,16 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand):
 
         return None
 
+    async def _fetch_wxsim(
+        self, source: str, forecast_type: str, num_days: int, message: MeshMessage, location: Optional[str]
+    ) -> str:
+        # Blocking HTTP fetch of the WXSIM plaintext file.
+        if location is None:
+            return await asyncio.to_thread(self._get_wxsim_weather, source, forecast_type, num_days, message)
+        return await asyncio.to_thread(
+            self._get_wxsim_weather, source, forecast_type, num_days, message, location
+        )
+
     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:
@@ -365,75 +375,21 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand):
         if len(parts) < 2:
             mqtt_topic = self._get_custom_mqtt_weather_topic(None)
             if mqtt_topic:
-                try:
+                return await self._reply_from_mqtt(message, mqtt_topic, option_type, None, split_multiday=False)
-                    self.record_execution(message.sender_id)
-                    weather_data = self._mqtt_weather_line(mqtt_topic, option_type, None)
-                    return bool(await self.send_response(message, weather_data))
-                except Exception as e:
-                    self.logger.error(f"Error reading MQTT weather: {e}")
-                    await self.send_response(message, self.translate("commands.gwx.error", error=str(e)))
-                    return True
 
             wxsim_source = self._get_custom_wxsim_source(None)  # Check for default
             if wxsim_source:
-                # Use custom WXSIM default source
+                return await self._reply_from_wxsim(message, wxsim_source, option_type, option_days)
-                try:
-                    self.record_execution(message.sender_id)
-                    # Blocking HTTP fetch of the WXSIM plaintext file.
-                    weather_data = await asyncio.to_thread(
-                        self._get_wxsim_weather, wxsim_source, option_type, option_days, message
-                    )
-                    if option_type == "multiday":
-                        return await self._send_multiday_forecast(message, weather_data)
-                    return bool(await self.send_response(message, weather_data))
-                except Exception as e:
-                    self.logger.error(f"Error fetching WXSIM weather: {e}")
-                    await self.send_response(message, self.translate('commands.gwx.error', error=str(e)))
-                    return True
 
-            # No custom source, try companion location
+            # No custom source: the sender's position, default city, then the bot's position.
-            companion_location = self._get_companion_location(message)
+            # Positions are forecast as coordinates: re-geocoding a reverse-geocoded
-            if companion_location:
+            # place name could move it to that town's center; the reply's label
-                # Forecast the sender's own point. Re-geocoding a reverse-geocoded
+            # comes from one reverse lookup in geocode_location.
-                # place name could move it to that town's center; the reply's
+            location_str, _ = await self._no_location_fallback(message)
-                # label comes from one reverse lookup in geocode_location.
+            if location_str is None:
-                location_str = self._coordinates_query(*companion_location)
+                await self.send_response(message, self.translate('commands.gwx.usage'))
-                parts = [parts[0], location_str]
+                return True
-                self.logger.info(f"Using companion coordinates: {location_str}")
+            parts = [parts[0], location_str]
-            else:
-                # No companion location: use default city if configured, then bot location fallback
-                if self.default_city:
-                    location_parts = [self.default_city]
-                    if self.default_state:
-                        location_parts.append(self.default_state)
-                    if self.default_country:
-                        location_parts.append(self.default_country)
-                    location_str = ", ".join(location_parts)
-                    parts = [parts[0], location_str]
-                    self.logger.info(f"Using default city (no args): {location_str}")
-                else:
-                    # No default city: optionally use bot's configured coordinates
-                    use_bot = self.get_config_value(
-                        'Wx_Command',
-                        'use_bot_location_when_no_location',
-                        fallback=False,
-                        value_type='bool',
-                    )
-                    bot_loc = self._get_bot_location() if use_bot else None
-                    if bot_loc:
-                        location_str = self._coordinates_query(*bot_loc)
-                        parts = [parts[0], location_str]
-                        self.logger.info(f"Using bot coordinates (no args): {location_str}")
-                    else:
-                        if use_bot:
-                            self.logger.debug(
-                                "use_bot_location_when_no_location enabled but bot_latitude/bot_longitude "
-                                "not set; showing usage"
-                            )
-                        else:
-                            self.logger.debug("No companion/default city location found, showing usage")
-                        await self.send_response(message, self.translate('commands.gwx.usage'))
-                        return True
 
         if option_word:
             parts.append(option_word)
@@ -458,35 +414,12 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand):
         mqtt_topic = self._get_custom_mqtt_weather_topic(location)
         if mqtt_topic:
             self.logger.info(f"Using custom MQTT weather topic for location '{location}': {mqtt_topic}")
-            try:
+            return await self._reply_from_mqtt(message, mqtt_topic, forecast_type, location)
-                self.record_execution(message.sender_id)
-                weather_data = self._mqtt_weather_line(mqtt_topic, forecast_type, location)
-                if forecast_type == "multiday":
-                    return await self._send_multiday_forecast(message, weather_data)
-                return bool(await self.send_response(message, weather_data))
-            except Exception as e:
-                self.logger.error(f"Error reading MQTT weather: {e}")
-                await self.send_response(message, self.translate("commands.gwx.error", error=str(e)))
-                return True
 
         # Check for custom WXSIM source first (before normal geocoding)
         wxsim_source = self._get_custom_wxsim_source(location)
         if wxsim_source:
-            # Use custom WXSIM source
+            return await self._reply_from_wxsim(message, wxsim_source, forecast_type, num_days, location)
-            try:
-                self.record_execution(message.sender_id)
-                # Blocking HTTP fetch of the WXSIM plaintext file.
-                weather_data = await asyncio.to_thread(
-                    self._get_wxsim_weather, wxsim_source, forecast_type, num_days,
-                    message, location,
-                )
-                if forecast_type == "multiday":
-                    return await self._send_multiday_forecast(message, weather_data)
-                return bool(await self.send_response(message, weather_data))
-            except Exception as e:
-                self.logger.error(f"Error fetching WXSIM weather: {e}")
-                await self.send_response(message, self.translate('commands.gwx.error', error=str(e)))
-                return True
 
         if forecast_type == "alerts":
             await self.send_response(message, self.translate('commands.gwx.source_option_not_available'))
@@ -499,25 +432,7 @@ class GlobalWxCommand(WeatherCommandMixin, BaseCommand):
             # Get weather data for the location
             weather_data = await self.get_weather_for_location(location, forecast_type, num_days, message)
 
-            # Check if we need to send multiple messages (for alerts)
+            return await self._send_weather_reply(message, weather_data, forecast_type)
-            if isinstance(weather_data, tuple) and weather_data[0] == "multi_message":
-                # Send weather data first; if it was refused (rate limit, send failure),
-                # do not send its second part on its own.
-                if not await self.send_response(message, weather_data[1]):
-                    return False
-
-                # Wait for bot TX rate limiter
-                rate_limit = self.bot.config.getfloat('Bot', 'bot_tx_rate_limit_seconds', fallback=1.0)
-                sleep_time = max(rate_limit + 1.0, 2.0)
-                await self._pace_reply(message, sleep_time)
-
-                # Send alerts
-                # Second part of the same reply: the reply limiter already let the first through.
-                return bool(await self.send_response(message, weather_data[2], skip_user_rate_limit=True))
-            if forecast_type == "multiday":
-                # Use message splitting for multi-day forecasts
-                return await self._send_multiday_forecast(message, weather_data)
-            return bool(await self.send_response(message, weather_data))
 
         except Exception as e:
             self.logger.error(f"Error in global weather command: {e}")

modules/commands/wx_command.py

@@ -561,6 +561,18 @@ class WxCommand(WeatherCommandMixin, BaseCommand):
                 return f"{location_name}: {current}"
             return current
 
+    async def _fetch_wxsim(
+        self, source: str, forecast_type: str, num_days: int, message: MeshMessage, location: Optional[str]
+    ) -> str:
+        if location is None:
+            return await self._get_wxsim_weather_async(source, forecast_type, num_days, message)
+        return await self._get_wxsim_weather_async(
+            source, forecast_type, num_days, message, location_name=location
+        )
+
+    async def _fallback_place_name(self, lat: float, lon: float) -> Optional[str]:
+        return await self._coordinates_to_location_string_async(lat, lon)
+
     async def _get_wxsim_weather_async(
         self,
         source_url: str,
@@ -646,88 +658,18 @@ class WxCommand(WeatherCommandMixin, BaseCommand):
         if len(parts) < 2:
             mqtt_topic = self._get_custom_mqtt_weather_topic(None)
             if mqtt_topic:
-                try:
+                return await self._reply_from_mqtt(message, mqtt_topic, option_type, None, split_multiday=False)
-                    self.record_execution(message.sender_id)
-                    weather_data = self._mqtt_weather_line(mqtt_topic, option_type, None)
-                    return bool(await self.send_response(message, weather_data))
-                except Exception as e:
-                    self.logger.error(f"Error reading MQTT weather: {e}")
-                    await self.send_response(message, self.translate("commands.wx.error", error=str(e)))
-                    return True
 
             wxsim_source = self._get_custom_wxsim_source(None)  # Check for default
             if wxsim_source:
-                # Use custom WXSIM default source
+                return await self._reply_from_wxsim(message, wxsim_source, option_type, option_days)
-                try:
-                    self.record_execution(message.sender_id)
-                    weather_data = await self._get_wxsim_weather_async(
-                        wxsim_source, option_type, option_days, message
-                    )
-                    if option_type == "multiday":
-                        return await self._send_multiday_forecast(message, weather_data)
-                    return bool(await self.send_response(message, weather_data))
-                except Exception as e:
-                    self.logger.error(f"Error fetching WXSIM weather: {e}")
-                    await self.send_response(message, self.translate('commands.wx.error', error=str(e)))
-                    return True
 
-            # No custom source, try companion location
+            # No custom source: the sender's position, default city, then the bot's position
-            companion_location = self._get_companion_location(message)
+            location_str, using_companion_location = await self._no_location_fallback(message)
-            if companion_location:
+            if location_str is None:
-                # Use coordinates directly to avoid re-geocoding issues
+                await self.send_response(message, self.translate('commands.wx.usage'))
-                location_str = self._coordinates_query(*companion_location)
+                return True
-                parts = [parts[0], location_str]
+            parts = [parts[0], location_str]
-                using_companion_location = True
-                # Get city name for display
-                display_name = await self._coordinates_to_location_string_async(
-                    companion_location[0], companion_location[1]
-                )
-                if display_name:
-                    self.logger.info(f"Using companion location: {display_name} ({companion_location[0]}, {companion_location[1]})")
-                else:
-                    self.logger.info(f"Using companion coordinates: {location_str}")
-            else:
-                # No companion location: use default city if configured, then bot location fallback
-                if self.default_city:
-                    location_parts = [self.default_city]
-                    if self.default_state:
-                        location_parts.append(self.default_state)
-                    if self.default_country:
-                        location_parts.append(self.default_country)
-                    location_str = ", ".join(location_parts)
-                    parts = [parts[0], location_str]
-                    self.logger.info(f"Using default city (no args): {location_str}")
-                else:
-                    # No default city: optionally use bot's configured coordinates
-                    use_bot = self.get_config_value(
-                        'Wx_Command',
-                        'use_bot_location_when_no_location',
-                        fallback=False,
-                        value_type='bool',
-                    )
-                    bot_loc = self._get_bot_location() if use_bot else None
-                    if bot_loc:
-                        location_str = self._coordinates_query(*bot_loc)
-                        parts = [parts[0], location_str]
-                        display_name = await self._coordinates_to_location_string_async(
-                            bot_loc[0], bot_loc[1]
-                        )
-                        if display_name:
-                            self.logger.info(
-                                f"Using bot location (no args): {display_name} ({bot_loc[0]}, {bot_loc[1]})"
-                            )
-                        else:
-                            self.logger.info(f"Using bot coordinates (no args): {location_str}")
-                    else:
-                        if use_bot:
-                            self.logger.debug(
-                                "use_bot_location_when_no_location enabled but bot_latitude/bot_longitude "
-                                "not set; showing usage"
-                            )
-                        else:
-                            self.logger.debug("No companion/default city location found, showing usage")
-                        await self.send_response(message, self.translate('commands.wx.usage'))
-                        return True
 
         if option_word:
             parts.append(option_word)
@@ -758,42 +700,14 @@ class WxCommand(WeatherCommandMixin, BaseCommand):
         mqtt_topic = self._get_custom_mqtt_weather_topic(location)
         if mqtt_topic:
             self.logger.info(f"Using custom MQTT weather topic for location '{location}': {mqtt_topic}")
-            try:
+            return await self._reply_from_mqtt(message, mqtt_topic, forecast_type, location)
-                self.record_execution(message.sender_id)
-                weather_data = self._mqtt_weather_line(
-                    mqtt_topic, forecast_type, location
-                )
-                if forecast_type == "multiday":
-                    return await self._send_multiday_forecast(message, weather_data)
-                return bool(await self.send_response(message, weather_data))
-            except Exception as e:
-                self.logger.error(f"Error reading MQTT weather: {e}")
-                await self.send_response(message, self.translate("commands.wx.error", error=str(e)))
-                return True
 
         # Check for custom WXSIM source first (before checking location type)
         wxsim_source = self._get_custom_wxsim_source(location)
         if wxsim_source:
             self.logger.info(f"Using custom WXSIM source for location '{location}': {wxsim_source}")
-            # Use custom WXSIM source
+            return await self._reply_from_wxsim(message, wxsim_source, forecast_type, num_days, location)
-            try:
+        self.logger.debug(f"No custom WXSIM source found for location '{location}', using normal weather API")
-                self.record_execution(message.sender_id)
-                weather_data = await self._get_wxsim_weather_async(
-                    wxsim_source,
-                    forecast_type,
-                    num_days,
-                    message,
-                    location_name=location,
-                )
-                if forecast_type == "multiday":
-                    return await self._send_multiday_forecast(message, weather_data)
-                return bool(await self.send_response(message, weather_data))
-            except Exception as e:
-                self.logger.error(f"Error fetching WXSIM weather: {e}")
-                await self.send_response(message, self.translate('commands.wx.error', error=str(e)))
-                return True
-        else:
-            self.logger.debug(f"No custom WXSIM source found for location '{location}', using normal weather API")
 
         # Check if it's coordinates, zipcode, or city name
         if re.match(r'^\s*-?\d+\.?\d*\s*,\s*-?\d+\.?\d*\s*$', location):
@@ -847,28 +761,7 @@ class WxCommand(WeatherCommandMixin, BaseCommand):
             # Get weather data for the location
             weather_data = await self.get_weather_for_location(location, location_type, forecast_type, num_days, message, using_companion_location=using_companion_location)
 
-            # Check if we need to send multiple messages
+            return await self._send_weather_reply(message, weather_data, forecast_type)
-            if isinstance(weather_data, tuple) and weather_data[0] == "multi_message":
-                # Send weather data first; if it was refused (rate limit, send failure),
-                # do not send its second part on its own.
-                if not await self.send_response(message, weather_data[1]):
-                    return False
-
-                # Wait for bot TX rate limiter to allow next message
-                rate_limit = self.bot.config.getfloat('Bot', 'bot_tx_rate_limit_seconds', fallback=1.0)
-                # Use a conservative sleep time to avoid rate limiting
-                sleep_time = max(rate_limit + 1.0, 2.0)  # At least 2 seconds, or rate_limit + 1 second
-                await self._pace_reply(message, sleep_time)
-
-                # Send the special weather statement (already formatted with prioritization)
-                alert_text = weather_data[2]
-                # Second part of the same reply: the reply limiter already let the first through.
-                return bool(await self.send_response(message, alert_text, skip_user_rate_limit=True))
-            if forecast_type == "multiday":
-                # Use message splitting for multi-day forecasts
-                return await self._send_multiday_forecast(message, weather_data)
-            # Send single message as usual
-            return bool(await self.send_response(message, weather_data))
 
         except Exception as e:
             self.logger.error(f"Error in weather command: {e}")

modules/weather_common.py

@@ -62,7 +62,14 @@ class WeatherCommandMixin:
     translate: Any
     send_response: Any
     get_max_message_length: Any
+    get_config_value: Any
+    record_execution: Any
     _get_custom_wxsim_source: Any
+    _get_companion_location: Any
+    _get_bot_location: Any
+    default_city: Any
+    default_state: Any
+    default_country: Any
 
     @staticmethod
     def _coordinates_query(lat: float, lon: float) -> str:
@@ -259,3 +266,136 @@ class WeatherCommandMixin:
                 sent_all = False
                 break
         return sent_all
+
+    async def _reply_from_mqtt(
+        self,
+        message: Any,
+        topic: str,
+        forecast_type: str,
+        location: Optional[str],
+        *,
+        split_multiday: bool = True,
+    ) -> bool:
+        """Answer from a custom.mqtt_weather source; errors become the error reply."""
+        try:
+            self.record_execution(message.sender_id)
+            weather_data = self._mqtt_weather_line(topic, forecast_type, location)
+            if split_multiday and forecast_type == "multiday":
+                return await self._send_multiday_forecast(message, weather_data)
+            return bool(await self.send_response(message, weather_data))
+        except Exception as e:
+            self.logger.error(f"Error reading MQTT weather: {e}")
+            await self.send_response(message, self.translate(f"{self.translation_ns}.error", error=str(e)))
+            return True
+
+    async def _reply_from_wxsim(
+        self,
+        message: Any,
+        source: str,
+        forecast_type: str,
+        num_days: int,
+        location: Optional[str] = None,
+    ) -> bool:
+        """Answer from a custom.wxsim source; errors become the error reply."""
+        try:
+            self.record_execution(message.sender_id)
+            weather_data = await self._fetch_wxsim(source, forecast_type, num_days, message, location)
+            if forecast_type == "multiday":
+                return await self._send_multiday_forecast(message, weather_data)
+            return bool(await self.send_response(message, weather_data))
+        except Exception as e:
+            self.logger.error(f"Error fetching WXSIM weather: {e}")
+            await self.send_response(message, self.translate(f"{self.translation_ns}.error", error=str(e)))
+            return True
+
+    async def _fetch_wxsim(
+        self, source: str, forecast_type: str, num_days: int, message: Any, location: Optional[str]
+    ) -> str:
+        """Fetch and format a WXSIM source off the event loop (each command's own way)."""
+        raise NotImplementedError
+
+    async def _fallback_place_name(self, lat: float, lon: float) -> Optional[str]:
+        """A place name for logging a fallback position, or None to log the coordinates."""
+        return None
+
+    def _default_city_query(self) -> Optional[str]:
+        """``default_city[, default_state][, default_country]``, or None when unset."""
+        if not self.default_city:
+            return None
+        location_parts = [self.default_city]
+        if self.default_state:
+            location_parts.append(self.default_state)
+        if self.default_country:
+            location_parts.append(self.default_country)
+        return ", ".join(location_parts)
+
+    async def _no_location_fallback(self, message: Any) -> tuple[Optional[str], bool]:
+        """Where to forecast when the request names no place.
+
+        Tries the sender's position, then default_city, then (when
+        use_bot_location_when_no_location is set) the bot's position. Returns
+        (location query or None, whether it is the sender's position).
+        """
+        companion_location = self._get_companion_location(message)
+        if companion_location:
+            # Use coordinates directly to avoid re-geocoding issues
+            location_str = self._coordinates_query(*companion_location)
+            display_name = await self._fallback_place_name(companion_location[0], companion_location[1])
+            if display_name:
+                self.logger.info(f"Using companion location: {display_name} ({companion_location[0]}, {companion_location[1]})")
+            else:
+                self.logger.info(f"Using companion coordinates: {location_str}")
+            return location_str, True
+
+        default_city = self._default_city_query()
+        if default_city:
+            self.logger.info(f"Using default city (no args): {default_city}")
+            return default_city, False
+
+        use_bot = self.get_config_value(
+            'Wx_Command',
+            'use_bot_location_when_no_location',
+            fallback=False,
+            value_type='bool',
+        )
+        bot_loc = self._get_bot_location() if use_bot else None
+        if bot_loc:
+            location_str = self._coordinates_query(*bot_loc)
+            display_name = await self._fallback_place_name(bot_loc[0], bot_loc[1])
+            if display_name:
+                self.logger.info(
+                    f"Using bot location (no args): {display_name} ({bot_loc[0]}, {bot_loc[1]})"
+                )
+            else:
+                self.logger.info(f"Using bot coordinates (no args): {location_str}")
+            return location_str, False
+
+        if use_bot:
+            self.logger.debug(
+                "use_bot_location_when_no_location enabled but bot_latitude/bot_longitude "
+                "not set; showing usage"
+            )
+        else:
+            self.logger.debug("No companion/default city location found, showing usage")
+        return None, False
+
+    async def _send_weather_reply(self, message: Any, weather_data: Any, forecast_type: str) -> bool:
+        """Send a provider's answer: two parts with alerts, split by day, or one message."""
+        if isinstance(weather_data, tuple) and weather_data[0] == "multi_message":
+            # Send weather data first; if it was refused (rate limit, send failure),
+            # do not send its second part on its own.
+            if not await self.send_response(message, weather_data[1]):
+                return False
+
+            # Wait for bot TX rate limiter to allow next message
+            rate_limit = self.bot.config.getfloat('Bot', 'bot_tx_rate_limit_seconds', fallback=1.0)
+            # Use a conservative sleep time to avoid rate limiting
+            sleep_time = max(rate_limit + 1.0, 2.0)  # At least 2 seconds, or rate_limit + 1 second
+            await self._pace_reply(message, sleep_time)
+
+            # Second part of the same reply: the reply limiter already let the first through.
+            return bool(await self.send_response(message, weather_data[2], skip_user_rate_limit=True))
+        if forecast_type == "multiday":
+            # Use message splitting for multi-day forecasts
+            return await self._send_multiday_forecast(message, weather_data)
+        return bool(await self.send_response(message, weather_data))