fix failed full weather fetch and implement cache for full

Cargo.lock

@@ -1169,6 +1169,7 @@ dependencies = [
  "reqwest",
  "rust-embed",
  "tokio",
+ "zbus 5.13.2",
 ]
 
 [[package]]

Cargo.toml

@@ -19,6 +19,9 @@ rust-embed = "8.8.0"
 # Web requests for weather
 reqwest = { version = "0.13.1", features = [] }
 
+# D-Bus for NetworkManager connectivity monitoring
+zbus = { version = "5", default-features = false, features = ["tokio"] }
+
 [dependencies.libcosmic]
 git = "https://github.com/pop-os/libcosmic.git"
 default-features = false

README.md

@@ -39,6 +39,43 @@ killall cosmic-panel
 
 The panel will relaunch automatically and load the updated applet.
 
+## Configuration
+
+### Temperature and wind units
+
+By default, the applet lets [wttr.in](https://wttr.in) pick units based on
+your IP-based location (so users in the US get Fahrenheit, everyone else
+gets Celsius).
+
+At startup the applet additionally inspects your measurement locale
+(`LC_MEASUREMENT`, then `LC_ALL`, then `LANG`). Locales in the US,
+Liberia, the Bahamas, Belize, the Cayman Islands, and Palau are requested
+as Fahrenheit / USCS; all other locales fall through to the wttr.in
+default.
+
+To override the choice explicitly, set `COSMIC_WEATHER_UNITS`:
+
+| Value  | Meaning                                   |
+| ------ | ----------------------------------------- |
+| `u`    | Fahrenheit, mph (USCS)                    |
+| `m`    | Celsius, km/h (metric)                    |
+| `M`    | Celsius, m/s                              |
+| `auto` | (or unset) Use locale / geolocation       |
+
+Because the applet is launched by `cosmic-panel`, a persistent override
+is easiest to set on that service, e.g.:
+
+```sh
+systemctl --user edit cosmic-panel
+# In the override file:
+# [Service]
+# Environment=COSMIC_WEATHER_UNITS=u
+
+systemctl --user restart cosmic-panel
+```
+
+Changes take effect after the applet is restarted.
+
 ## Localization
 
 [Fluent](https://projectfluent.org/) is used for localization. Translation files are in the [i18n directory](./i18n). To add a new language, copy the [English (en) localization](./i18n/en), rename the directory to the target [ISO 639-1 language code](https://en.wikipedia.org/wiki/List_of_ISO_639-1_codes), and translate the message values.

src/app.rs

@@ -9,11 +9,35 @@ use cosmic::widget::{self, autosize, container, rectangle_tracker::{
 use cosmic::{iced_futures, prelude::*, surface};
 use cosmic::applet::cosmic_panel_config::PanelAnchor;
 use cosmic::iced_core::Shadow;
-use futures_util::SinkExt;
+use futures_util::{SinkExt, StreamExt};
 use std::time::Duration;
 
+use crate::config::Units;
+
 const WTTR_URL: &str = "https://wttr.lerch.org";
 const WEATHER_UPDATE_INTERVAL_MINUTES: u64 = 15;
+const REQUEST_TIMEOUT_SECS: u64 = 15;
+const MAX_RETRIES: u32 = 10;
+const INITIAL_RETRY_DELAY_SECS: u64 = 5;
+const MAX_RETRY_DELAY_SECS: u64 = 120;
+/// Minimum time between acting on `NetworkManager` signals (debounce).
+const NM_DEBOUNCE_SECS: u64 = 5;
+/// Delay after NM reports connected before we fetch, to let DNS/routing settle.
+const NM_SETTLE_DELAY_SECS: u64 = 2;
+
+/// D-Bus proxy for monitoring `NetworkManager` connectivity changes.
+#[zbus::proxy(
+    interface = "org.freedesktop.NetworkManager",
+    default_service = "org.freedesktop.NetworkManager",
+    default_path = "/org/freedesktop/NetworkManager"
+)]
+trait NetworkManager {
+    /// Signal emitted when the overall NM connection state changes.
+    /// `state` values: 70 = Connected (global), 60 = Connected (site),
+    /// 50 = Connected (local), 40 = Connecting, etc.
+    #[zbus(signal)]
+    fn state_changed(&self, state: u32);
+}
 
 /// The applet model stores app-specific state.
 pub struct AppModel {
@@ -27,6 +51,10 @@ pub struct AppModel {
     last_updated: String,
     /// Full weather report for popup
     full_weather: String,
+    /// Last error from a full-weather fetch, if any. Kept separate from
+    /// `full_weather` so that a failed fetch does not poison the cache
+    /// and block the next popup-open from retrying.
+    full_weather_error: Option<String>,
     /// Loading state
     is_loading: bool,
     /// Popup window ID
@@ -35,6 +63,10 @@ pub struct AppModel {
     rectangle: Rectangle,
     /// Rectangle tracker
     rectangle_tracker: Option<RectangleTracker<u32>>,
+    /// Number of consecutive fetch retries (for captive portal / network issues)
+    retry_count: u32,
+    /// Unit system to request from wttr.in (None = let the service geolocate).
+    units: Option<Units>,
 }
 
 /// Messages emitted by the applet.
@@ -43,6 +75,7 @@ pub enum Message {
     WeatherUpdate(Result<(String, String), String>),
     FullWeatherUpdate(Result<String, String>),
     RefreshWeather,
+    NetworkChanged,
     TogglePopup,
     CloseRequested(window::Id),
     Rectangle(RectangleUpdate<u32>),
@@ -51,7 +84,7 @@ pub enum Message {
 
 impl cosmic::Application for AppModel {
     type Executor = cosmic::executor::Default;
-    type Flags = ();
+    type Flags = Option<Units>;
     type Message = Message;
     const APP_ID: &'static str = "org.lerch.weather";
 
@@ -65,7 +98,7 @@ impl cosmic::Application for AppModel {
 
     fn init(
         core: cosmic::Core,
-        _flags: Self::Flags,
+        flags: Self::Flags,
     ) -> (Self, Task<cosmic::Action<Self::Message>>) {
         let app = AppModel {
             core,
@@ -73,13 +106,17 @@ impl cosmic::Application for AppModel {
             location: String::new(),
             last_updated: String::new(),
             full_weather: String::new(),
+            full_weather_error: None,
             is_loading: true,
             popup: None,
             rectangle: Rectangle::default(),
             rectangle_tracker: None,
+            retry_count: 0,
+            units: flags,
         };
 
-        let command = Task::perform(fetch_weather(), |result| {
+        let units = app.units;
+        let command = Task::perform(fetch_weather(units), |result| {
             cosmic::Action::App(Message::WeatherUpdate(result))
         });
 
@@ -143,11 +180,24 @@ impl cosmic::Application for AppModel {
     }
 
     fn view_window(&self, _id: window::Id) -> Element<'_, Self::Message> {
+        // Pick what to render: the cached forecast, a transient error
+        // (not stored in `full_weather` so the next open retries), or a
+        // loading placeholder while the in-flight request completes.
+        let display_text: String = if !self.full_weather.is_empty() {
+            self.full_weather.clone()
+        } else if let Some(err) = &self.full_weather_error {
+            format!(
+                "Failed to load weather:\n{err}\n\nClose and reopen to retry."
+            )
+        } else {
+            "Loading…".to_string()
+        };
+
         // Calculate the width needed for the monospace content.
         // The default monospace font is ~8.4px per character at 14px font size,
         // plus padding for container (12*2) and popup chrome (~32).
         #[allow(clippy::cast_precision_loss)]
-        let max_line_len = self.full_weather.lines()
+        let max_line_len = display_text.lines()
             .map(|line| line.chars().count())
             .max()
             .unwrap_or(40) as f32;
@@ -156,7 +206,7 @@ impl cosmic::Application for AppModel {
         let popup_width = content_width.max(400.0);
 
         let content = container(
-            widget::text(&self.full_weather)
+            widget::text(display_text)
                 .font(Font::MONOSPACE)
                 .wrapping(cosmic::iced::widget::text::Wrapping::None)
                 .width(cosmic::iced::Length::Fixed(content_width))
@@ -214,6 +264,7 @@ impl cosmic::Application for AppModel {
     fn subscription(&self) -> Subscription<Self::Message> {
         Subscription::batch([
             rectangle_tracker_subscription(0).map(|e| Message::Rectangle(e.1)),
+            // Periodic weather refresh timer
             Subscription::run(|| {
                 iced_futures::stream::channel(1, |mut emitter| async move {
                     let mut interval = tokio::time::interval(Duration::from_secs(WEATHER_UPDATE_INTERVAL_MINUTES * 60));
@@ -225,6 +276,51 @@ impl cosmic::Application for AppModel {
                     }
                 })
             }),
+            // NetworkManager connectivity change monitor
+            Subscription::run(|| {
+                iced_futures::stream::channel(1, |mut emitter| async move {
+                    let connection = match zbus::Connection::system().await {
+                        Ok(c) => c,
+                        Err(e) => {
+                            eprintln!("Failed to connect to system D-Bus for network monitoring: {e}");
+                            futures_util::future::pending::<()>().await;
+                            unreachable!();
+                        }
+                    };
+
+                    let proxy = match NetworkManagerProxy::new(&connection).await {
+                        Ok(p) => p,
+                        Err(e) => {
+                            eprintln!("Failed to create NetworkManager proxy: {e}");
+                            futures_util::future::pending::<()>().await;
+                            unreachable!();
+                        }
+                    };
+
+                    let mut stream = match proxy.receive_state_changed().await {
+                        Ok(s) => s,
+                        Err(e) => {
+                            eprintln!("Failed to subscribe to NM StateChanged: {e}");
+                            futures_util::future::pending::<()>().await;
+                            unreachable!();
+                        }
+                    };
+
+                    let mut last_emit = tokio::time::Instant::now() - Duration::from_secs(NM_DEBOUNCE_SECS + 1);
+                    while let Some(signal) = stream.next().await {
+                        if let Ok(args) = signal.args() {
+                            // NM state >= 60 means some form of network connectivity
+                            if args.state >= 60 && last_emit.elapsed() >= Duration::from_secs(NM_DEBOUNCE_SECS) {
+                                // Brief delay to let DNS/routing stabilize
+                                tokio::time::sleep(Duration::from_secs(NM_SETTLE_DELAY_SECS)).await;
+                                _ = emitter.send(Message::NetworkChanged).await;
+                                last_emit = tokio::time::Instant::now();
+                            }
+                        }
+                    }
+                    eprintln!("NetworkManager signal stream ended unexpectedly");
+                })
+            }),
         ])
     }
 
@@ -237,14 +333,34 @@ impl cosmic::Application for AppModel {
                         self.location = location;
                         self.weather_text = weather;
                         self.last_updated = format_current_time();
+                        self.retry_count = 0;
                     }
                     Err(e) => {
                         eprintln!("Weather fetch error: {e}");
-                        self.weather_text = if e.contains("network") || e.contains("timeout") {
-                            "🌐❌".to_string() // Network issue
+                        self.weather_text = if e.contains("network") || e.contains("timeout") || e.contains("captive") || e.contains("Captive") {
+                            "\u{1f310}\u{274c}".to_string() // Network issue
                         } else {
-                            "❌".to_string() // Generic error
+                            "\u{274c}".to_string() // Generic error
                         };
+
+                        // Retry with exponential backoff (handles captive portals,
+                        // transient failures after network changes, etc.)
+                        if self.retry_count < MAX_RETRIES {
+                            self.retry_count += 1;
+                            let delay = retry_delay(self.retry_count);
+                            eprintln!(
+                                "Scheduling retry {}/{MAX_RETRIES} in {delay}s",
+                                self.retry_count
+                            );
+                            let units = self.units;
+                            return Task::perform(
+                                fetch_weather_delayed(delay, units),
+                                |result| cosmic::Action::App(Message::WeatherUpdate(result)),
+                            );
+                        }
+                        eprintln!(
+                            "Max retries ({MAX_RETRIES}) reached, waiting for next scheduled refresh"
+                        );
                     }
                 }
                 Task::none()
@@ -253,16 +369,38 @@ impl cosmic::Application for AppModel {
                 match result {
                     Ok(weather) => {
                         self.full_weather = weather;
+                        self.full_weather_error = None;
                     }
                     Err(e) => {
                         eprintln!("Full weather fetch error: {e}");
-                        self.full_weather = "Failed to load weather".to_string();
+                        // Leave `full_weather` empty so that the next
+                        // popup open triggers a refetch (see toggle_popup).
+                        self.full_weather_error = Some(e);
                     }
                 }
                 Task::none()
             }
             Message::RefreshWeather => {
-                Task::perform(fetch_weather(), |result| {
+                self.retry_count = 0;
+                // Invalidate the popup cache on each scheduled refresh so
+                // that the next popup open fetches a fresh forecast. This
+                // piggybacks the popup's freshness on the panel's 15-minute
+                // refresh cadence (WEATHER_UPDATE_INTERVAL_MINUTES).
+                self.full_weather.clear();
+                self.full_weather_error = None;
+                let units = self.units;
+                Task::perform(fetch_weather(units), |result| {
+                    cosmic::Action::App(Message::WeatherUpdate(result))
+                })
+            }
+            Message::NetworkChanged => {
+                eprintln!("Network connectivity changed, refreshing weather");
+                self.retry_count = 0;
+                // Clear cached full weather so the popup re-fetches on next open
+                self.full_weather.clear();
+                self.full_weather_error = None;
+                let units = self.units;
+                Task::perform(fetch_weather(units), |result| {
                     cosmic::Action::App(Message::WeatherUpdate(result))
                 })
             }
@@ -344,7 +482,8 @@ impl AppModel {
             .max_height(1080.0);
 
         if self.full_weather.is_empty() {
-            let fetch_task = Task::perform(fetch_full_weather(), |result| {
+            let units = self.units;
+            let fetch_task = Task::perform(fetch_full_weather(units), |result| {
                 cosmic::Action::App(Message::FullWeatherUpdate(result))
             });
             Task::batch([get_popup(popup_settings), fetch_task])
@@ -354,13 +493,29 @@ impl AppModel {
     }
 }
 
-async fn fetch_weather() -> Result<(String, String), String> {
-    let response = reqwest::get(&format!("{WTTR_URL}/?format=%l|%c|%t"))
+async fn fetch_weather(units: Option<Units>) -> Result<(String, String), String> {
+    let client = reqwest::Client::builder()
+        .timeout(Duration::from_secs(REQUEST_TIMEOUT_SECS))
+        .build()
+        .map_err(|e| e.to_string())?;
+
+    let response = client
+        .get(format!(
+            "{WTTR_URL}/?format=%l|%c|%t{suffix}",
+            suffix = units_suffix(units)
+        ))
+        .send()
         .await
         .map_err(|e| e.to_string())?;
 
     let text = response.text().await.map_err(|e| e.to_string())?;
     let text = text.trim();
+
+    // Detect captive portal: the weather API returns plain text, not HTML.
+    if looks_like_captive_portal(text) {
+        return Err("Captive portal detected: received HTML instead of weather data".to_string());
+    }
+
     // Format: "location|icon|temp"
     let parts: Vec<&str> = text.splitn(3, '|').collect();
     if parts.len() == 3 {
@@ -387,9 +542,56 @@ fn format_current_time() -> String {
         .unwrap_or_default()
 }
 
-async fn fetch_full_weather() -> Result<String, String> {
-    let response = reqwest::get(&format!("{WTTR_URL}?A&T")).await.map_err(|e| e.to_string())?;
+async fn fetch_full_weather(units: Option<Units>) -> Result<String, String> {
+    let client = reqwest::Client::builder()
+        .timeout(Duration::from_secs(REQUEST_TIMEOUT_SECS))
+        .build()
+        .map_err(|e| e.to_string())?;
+
+    let response = client
+        .get(format!("{WTTR_URL}?A&T{suffix}", suffix = units_suffix(units)))
+        .send()
+        .await
+        .map_err(|e| e.to_string())?;
 
     let text = response.text().await.map_err(|e| e.to_string())?;
-    Ok(text.trim().to_string())
+    let text = text.trim();
+
+    if looks_like_captive_portal(text) {
+        return Err("Captive portal detected: received HTML instead of weather data".to_string());
+    }
+
+    Ok(text.to_string())
+}
+
+/// Returns `true` if the response body looks like an HTML captive portal page
+/// rather than the expected plain-text weather data.
+fn looks_like_captive_portal(body: &str) -> bool {
+    let lower = body.to_ascii_lowercase();
+    lower.contains("<html") || lower.contains("<!doctype")
+}
+
+/// Compute the retry delay using exponential backoff:
+/// 5s, 10s, 20s, 40s, 80s, 120s, 120s, ...
+fn retry_delay(retry_count: u32) -> u64 {
+    let delay = INITIAL_RETRY_DELAY_SECS.saturating_mul(2u64.saturating_pow(retry_count - 1));
+    delay.min(MAX_RETRY_DELAY_SECS)
+}
+
+/// Fetch weather after waiting `delay_secs`, used for retry backoff.
+async fn fetch_weather_delayed(
+    delay_secs: u64,
+    units: Option<Units>,
+) -> Result<(String, String), String> {
+    tokio::time::sleep(Duration::from_secs(delay_secs)).await;
+    fetch_weather(units).await
+}
+
+/// Build the URL suffix that selects a unit system on wttr.in.
+/// Returns an empty string when no explicit choice has been made.
+fn units_suffix(units: Option<Units>) -> String {
+    match units {
+        Some(u) => format!("&{}", u.query_param()),
+        None => String::new(),
+    }
 }

src/config.rs

@@ -1,3 +1,147 @@
 // SPDX-License-Identifier: MIT
 
-// Config module placeholder - currently unused
+//! Runtime configuration for the applet.
+//!
+//! Currently only handles temperature/wind unit selection for wttr.in.
+//! Units are resolved once at startup from (in order):
+//!   1. The `COSMIC_WEATHER_UNITS` environment variable
+//!      (values: `u`, `m`, `M`, or `auto`/empty to fall through).
+//!   2. The user's measurement locale (`LC_MEASUREMENT`, then `LC_ALL`,
+//!      then `LANG`). Fahrenheit-using locales map to `Uscs`; everything
+//!      else falls through.
+//!   3. `None`, which leaves the choice to wttr.in's IP-based geolocation
+//!      (the previous behavior).
+
+/// Unit system to request from wttr.in.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Units {
+    /// Celsius, km/h. Corresponds to wttr.in's `m` flag.
+    Metric,
+    /// Celsius, m/s. Corresponds to wttr.in's `M` flag.
+    MetricMs,
+    /// Fahrenheit, mph. Corresponds to wttr.in's `u` flag.
+    Uscs,
+}
+
+impl Units {
+    /// Single-letter flag understood by wttr.in (e.g. appended as `&u`).
+    pub const fn query_param(self) -> &'static str {
+        match self {
+            Self::Metric => "m",
+            Self::MetricMs => "M",
+            Self::Uscs => "u",
+        }
+    }
+}
+
+/// Locales whose measurement convention is Fahrenheit / USCS.
+/// Source: CLDR measurement-system data (US, Liberia, Bahamas, Belize,
+/// Cayman Islands, Palau).
+const USCS_LOCALES: &[&str] = &["en_US", "en_LR", "en_BS", "en_BZ", "en_KY", "en_PW"];
+
+/// Resolve the units to request at applet startup.
+///
+/// Returns `None` when neither the env var nor the locale gives a
+/// definitive answer, in which case wttr.in decides based on the
+/// client's geolocation.
+#[must_use]
+pub fn resolve_units() -> Option<Units> {
+    if let Some(u) = units_from_env() {
+        return u;
+    }
+    units_from_locale()
+}
+
+/// Parse `COSMIC_WEATHER_UNITS`.
+///
+/// Returns:
+/// - `Some(Some(units))` — explicit override.
+/// - `Some(None)` — explicit `auto` or empty; caller should still return
+///   `None` overall (skip locale detection? No — we want `auto` to mean
+///   "fall through to locale", so this returns `None` to signal fall-through).
+/// - `None` — env var unset; fall through.
+///
+/// To keep the control flow simple we collapse all fall-through cases to
+/// a single `None` return and only return `Some(Some(_))` for an explicit
+/// unit choice.
+fn units_from_env() -> Option<Option<Units>> {
+    let raw = std::env::var("COSMIC_WEATHER_UNITS").ok()?;
+    let trimmed = raw.trim();
+    if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("auto") {
+        return None;
+    }
+    let parsed = match trimmed {
+        "u" | "U" => Some(Units::Uscs),
+        "m" => Some(Units::Metric),
+        "M" => Some(Units::MetricMs),
+        other => {
+            eprintln!(
+                "COSMIC_WEATHER_UNITS={other:?} is not recognized; \
+                 expected one of: u, m, M, auto"
+            );
+            return None;
+        }
+    };
+    Some(parsed)
+}
+
+/// Inspect measurement-related locale env vars and map to `Units`.
+fn units_from_locale() -> Option<Units> {
+    for var in ["LC_MEASUREMENT", "LC_ALL", "LANG"] {
+        if let Ok(val) = std::env::var(var) {
+            if let Some(units) = classify_locale(&val) {
+                return Some(units);
+            }
+        }
+    }
+    None
+}
+
+/// Strip codeset (`.UTF-8`) and modifier (`@euro`) and check the
+/// language_TERRITORY prefix against the USCS list.
+fn classify_locale(value: &str) -> Option<Units> {
+    let trimmed = value.trim();
+    if trimmed.is_empty() || trimmed == "C" || trimmed == "POSIX" {
+        return None;
+    }
+    let without_codeset = trimmed.split('.').next().unwrap_or(trimmed);
+    let without_modifier = without_codeset.split('@').next().unwrap_or(without_codeset);
+    if USCS_LOCALES
+        .iter()
+        .any(|l| l.eq_ignore_ascii_case(without_modifier))
+    {
+        Some(Units::Uscs)
+    } else {
+        None
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn query_params() {
+        assert_eq!(Units::Metric.query_param(), "m");
+        assert_eq!(Units::MetricMs.query_param(), "M");
+        assert_eq!(Units::Uscs.query_param(), "u");
+    }
+
+    #[test]
+    fn classify_known_uscs_locales() {
+        assert_eq!(classify_locale("en_US.UTF-8"), Some(Units::Uscs));
+        assert_eq!(classify_locale("en_US"), Some(Units::Uscs));
+        assert_eq!(classify_locale("en_LR.UTF-8"), Some(Units::Uscs));
+        assert_eq!(classify_locale("en_BZ@something"), Some(Units::Uscs));
+    }
+
+    #[test]
+    fn classify_non_uscs_locales() {
+        assert_eq!(classify_locale("fi_FI.UTF-8"), None);
+        assert_eq!(classify_locale("en_GB.UTF-8"), None);
+        assert_eq!(classify_locale("de_DE"), None);
+        assert_eq!(classify_locale("C"), None);
+        assert_eq!(classify_locale("POSIX"), None);
+        assert_eq!(classify_locale(""), None);
+    }
+}

src/main.rs

@@ -11,6 +11,15 @@ fn main() -> cosmic::iced::Result {
     // Enable localizations to be applied.
     i18n::init(&requested_languages);
 
-    // Starts the applet's event loop with `()` as the application's flags.
-    cosmic::applet::run::<app::AppModel>(())
+    // Resolve unit preference once at startup. This checks the
+    // COSMIC_WEATHER_UNITS env var first, then measurement locale vars,
+    // falling back to `None` (let wttr.in decide by IP).
+    let units = config::resolve_units();
+    match units {
+        Some(u) => eprintln!("Weather units resolved to {u:?} (wttr.in flag: {})", u.query_param()),
+        None => eprintln!("Weather units not explicitly set; deferring to wttr.in geolocation"),
+    }
+
+    // Starts the applet's event loop, passing the resolved units as flags.
+    cosmic::applet::run::<app::AppModel>(units)
 }