v0.7.7

src/engine.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashMap, time::{Duration, Instant}};
+use std::{collections::HashMap, sync::atomic::Ordering, time::{Duration, Instant}};
 use anyhow::Result;
 use chrono::{DateTime, Datelike, Local, NaiveTime, Timelike, Utc, Weekday};
 use serde_json::{json, Value};
@@ -16,8 +16,13 @@ pub fn start(state: AppState) {
     tokio::spawn(async move {
         sleep(Duration::from_millis(500)).await;
         loop {
-            if let Err(err) = poll_all(&poll_state).await {
-                tracing::error!(error=?err, "device poll cycle failed");
+            match poll_all(&poll_state).await {
+                Ok(()) => {
+                    if !poll_state.initial_device_sync_complete.swap(true, Ordering::AcqRel) {
+                        tracing::info!("initial device state synchronized; thermostat control enabled");
+                    }
+                }
+                Err(err) => tracing::error!(error=?err, "device poll cycle failed"),
             }
             let seconds = poll_state.settings.read().await.poll_interval_seconds.max(2);
             sleep(Duration::from_secs(seconds)).await;
@@ -28,6 +33,13 @@ pub fn start(state: AppState) {
     tokio::spawn(async move {
         sleep(Duration::from_secs(2)).await;
         loop {
+            // A restart must never make decisions from the persisted, potentially stale
+            // device snapshot. Wait for one full live poll before thermostat/schedule/automation
+            // ownership can emit commands. Manual API/remote control remains available.
+            if !control_state.initial_device_sync_complete.load(Ordering::Acquire) {
+                sleep(Duration::from_millis(250)).await;
+                continue;
+            }
             if let Err(err) = control_zones(&control_state).await {
                 tracing::error!(error=?err, "zone cycle failed");
             }
@@ -198,27 +210,40 @@ async fn send_command_locked_inner(
         if command.quiet.is_some() && applied_command.quiet.is_none() { device.supports_quiet = Some(false); }
         if command.sleep.is_some() && applied_command.sleep.is_none() { device.supports_sleep = Some(false); }
 
-        // A command ACK confirms transport/acceptance, not the resulting climate state. Read
-        // status before publishing device_setpoint/power/mode as factual. If verification is
-        // unavailable, keep the previous confirmed values and mark communication uncertainty.
+        // A command ACK confirms transport/acceptance, but several GREE firmwares keep
+        // returning the pre-command status for a short settling window. Publishing that first
+        // stale read makes Home Assistant visibly bounce ON -> OFF -> ON. Verify a few times
+        // with bounded backoff and only publish a differing state after the settling window.
         if !confirmed_state {
-            let mut observed = device.clone();
-            match state.gree.poll(&mut observed).await {
-                Ok(()) => {
-                    if !applied_command.changed_from(&observed).is_empty() {
-                        confirmed_requested_state = false;
-                        tracing::debug!(device=%device.id, command=?applied_command, "GREE command acknowledged but verified status differs");
+            let verification_delays_ms = [0_u64, 150, 350, 650];
+            let mut last_verification_error: Option<String> = None;
+            for delay_ms in verification_delays_ms {
+                if delay_ms > 0 { sleep(Duration::from_millis(delay_ms)).await; }
+                let mut observed = device.clone();
+                match state.gree.poll(&mut observed).await {
+                    Ok(()) => {
+                        let requested_matches = applied_command.changed_from(&observed).is_empty();
+                        device = observed;
+                        confirmed_state = true;
+                        confirmed_requested_state = requested_matches;
+                        last_verification_error = None;
+                        if requested_matches { break; }
+                    }
+                    Err(err) => {
+                        last_verification_error = Some(err.to_string());
                     }
-                    device = observed;
-                    confirmed_state = true;
-                }
-                Err(err) => {
-                    record_poll_failure(&mut device, &format!("command accepted but status verification failed: {err}"));
-                    state.log("warn", "device.command_unconfirmed", &format!("Command accepted by {}, but resulting state could not be verified", device.name), json!({
-                        "device_id": device.id, "error": err.to_string()
-                    }));
                 }
             }
+
+            if confirmed_state && !confirmed_requested_state {
+                tracing::debug!(device=%device.id, command=?applied_command, "GREE command acknowledged but status still differs after settling window");
+            } else if !confirmed_state {
+                let error = last_verification_error.unwrap_or_else(|| "status verification failed".into());
+                record_poll_failure(&mut device, &format!("command accepted but status verification failed: {error}"));
+                state.log("warn", "device.command_unconfirmed", &format!("Command accepted by {}, but resulting state could not be verified", device.name), json!({
+                    "device_id": device.id, "error": error
+                }));
+            }
         }
         if confirmed_state {
             device.response_time_ms = Some(response_started.elapsed().as_millis().min(u64::MAX as u128) as u64);
@@ -446,19 +471,56 @@ pub fn reset_device_manual_override(zone: &mut Zone) -> bool {
     let changed = zone.device_manual_override
         || zone.device_manual_override_since.is_some()
         || zone.device_manual_override_until.is_some()
-        || !zone.device_manual_override_fields.is_empty();
+        || !zone.device_manual_override_fields.is_empty()
+        || zone.device_manual_override_baseline.is_some();
     zone.device_manual_override = false;
     zone.device_manual_override_since = None;
     zone.device_manual_override_until = None;
     zone.device_manual_override_fields.clear();
+    zone.device_manual_override_baseline = None;
     changed
 }
 
-fn set_device_manual_override(state: &AppState, zone: &mut Zone, fields: Vec<String>, source: &str) -> Result<(), AppError> {
+fn manual_override_matches_baseline(zone: &Zone, device: &Device) -> bool {
+    let Some(baseline) = zone.device_manual_override_baseline.as_ref() else { return false; };
+    if zone.device_manual_override_fields.is_empty() { return false; }
+    // If the unit was OFF before takeover, returning it to OFF is operationally the same
+    // controller state even if the remote retained a different mode/target internally.
+    // Those dormant values will be set explicitly if automation later powers the unit.
+    if !baseline.power { return !device.power; }
+    zone.device_manual_override_fields.iter().all(|field| match field.as_str() {
+        "power" => device.power == baseline.power,
+        "mode" => device.mode == baseline.mode,
+        "target_temperature" => device.target_temperature.round() == baseline.target_temperature.round(),
+        "fan_speed" => device.fan_speed == baseline.fan_speed,
+        "quiet" => device.quiet == baseline.quiet,
+        "sleep" => device.sleep == baseline.sleep,
+        _ => false,
+    })
+}
+
+fn persist_manual_override_clear(state: &AppState, zone: &mut Zone, source: &str, restored: bool) -> Result<bool, AppError> {
+    if !reset_device_manual_override(zone) { return Ok(false); }
+    zone.updated_at = Utc::now();
+    state.db.save_zone(zone)?;
+    state.broadcast("zone.updated", serde_json::to_value(&*zone)?);
+    let (kind, message) = if restored {
+        ("zone.device_manual_override_restored", format!("Manual device control returned {} to its previous state", zone.name))
+    } else {
+        ("zone.device_manual_override_cleared", format!("Manual device control ended for {}", zone.name))
+    };
+    state.log("info", kind, &message, json!({
+        "zone_id": zone.id, "device_id": zone.device_id, "source": source
+    }));
+    Ok(true)
+}
+
+fn set_device_manual_override(state: &AppState, zone: &mut Zone, fields: Vec<String>, source: &str, baseline: &Device) -> Result<(), AppError> {
     if fields.is_empty() { return Ok(()); }
     let now = Utc::now();
     if !zone.device_manual_override {
         zone.device_manual_override_since = Some(now);
+        zone.device_manual_override_baseline = Some(baseline.into());
     }
     zone.device_manual_override = true;
     zone.device_manual_override_until = if zone.enabled {
@@ -466,7 +528,11 @@ fn set_device_manual_override(state: &AppState, zone: &mut Zone, fields: Vec<Str
     } else {
         None
     };
-    zone.device_manual_override_fields = fields.clone();
+    for field in fields {
+        if !zone.device_manual_override_fields.iter().any(|existing| existing == &field) {
+            zone.device_manual_override_fields.push(field);
+        }
+    }
     zone.demand = false;
     zone.demand_since = None;
     zone.updated_at = now;
@@ -475,7 +541,7 @@ fn set_device_manual_override(state: &AppState, zone: &mut Zone, fields: Vec<Str
     state.log("info", "zone.device_manual_override", &format!("Manual device control detected for {}", zone.name), json!({
         "zone_id": zone.id,
         "device_id": zone.device_id,
-        "fields": fields,
+        "fields": zone.device_manual_override_fields,
         "source": source,
         "override_until": zone.device_manual_override_until,
     }));
@@ -486,21 +552,18 @@ fn detect_external_device_control(state: &AppState, before: &Device, after: &Dev
     if before.id != after.id { return Ok(()); }
     for mut zone in state.db.list_zones()?.into_iter().filter(|zone| zone.device_id == after.id) {
         let fields = externally_changed_control_fields(before, after, &zone);
+        if zone.device_manual_override && manual_override_matches_baseline(&zone, after) {
+            persist_manual_override_clear(state, &mut zone, "gree_poll", true)?;
+            continue;
+        }
         if fields.is_empty() { continue; }
         // A disabled zone is outside controller ownership. When its manually operated unit is
         // switched off there is no takeover left to display or remember.
         if !zone.enabled && !after.power {
-            if reset_device_manual_override(&mut zone) {
-                zone.updated_at = Utc::now();
-                state.db.save_zone(&zone)?;
-                state.broadcast("zone.updated", serde_json::to_value(&zone)?);
-                state.log("info", "zone.device_manual_override_cleared", &format!("Manual device control ended for {}", zone.name), json!({
-                    "zone_id": zone.id, "device_id": zone.device_id, "source": "gree_poll"
-                }));
-            }
+            persist_manual_override_clear(state, &mut zone, "gree_poll", false)?;
             continue;
         }
-        set_device_manual_override(state, &mut zone, fields, "gree_poll")?;
+        set_device_manual_override(state, &mut zone, fields, "gree_poll", before)?;
     }
     Ok(())
 }
@@ -516,15 +579,15 @@ pub async fn send_manual_command(state: &AppState, device_id: &str, command: Dev
     let updated = send_command_locked(state, device_id, command).await?;
     if !fields.is_empty() {
         for mut zone in state.db.list_zones()?.into_iter().filter(|zone| zone.device_id == device_id) {
-            if !zone.enabled && !updated.power {
-                if reset_device_manual_override(&mut zone) {
-                    zone.updated_at = Utc::now();
-                    state.db.save_zone(&zone)?;
-                    state.broadcast("zone.updated", serde_json::to_value(&zone)?);
-                }
+            if zone.device_manual_override && manual_override_matches_baseline(&zone, &updated) {
+                persist_manual_override_clear(state, &mut zone, source, true)?;
                 continue;
             }
-            set_device_manual_override(state, &mut zone, fields.clone(), source)?;
+            if !zone.enabled && !updated.power {
+                persist_manual_override_clear(state, &mut zone, source, false)?;
+                continue;
+            }
+            set_device_manual_override(state, &mut zone, fields.clone(), source, &before)?;
         }
     }
     Ok(updated)
@@ -1068,10 +1131,17 @@ async fn control_zones(state: &AppState) -> Result<()> {
 
         // Setpoint modulation: keep the indoor unit powered and let its own inverter/compressor
         // stop naturally when we move the target to the satisfied side of room temperature.
-        let assist = outdoor_assist_offset(effective_mode, outdoor_assist_temperature, temp, target);
+        let outdoor_assist = outdoor_assist_offset(effective_mode, outdoor_assist_temperature, temp, target);
+        // When an independent room sensor is actually driving cooling, the indoor unit's
+        // own sensor can satisfy too early. Apply a half-degree pre-rounding bias: because
+        // GREE setpoints are sent as whole degrees, this selects the next lower whole-degree
+        // target (0.5-1.0 C below the room target). Do not stack it with outdoor assist and
+        // do not use it during device/fallback control.
+        let room_sensor_assist = external_room_sensor_cooling_assist(effective_mode, &zone.control_temperature_source);
+        let demand_assist = outdoor_assist.max(room_sensor_assist);
         let active_target = match effective_mode {
-            "heat" => target + assist,
-            _ => target - assist,
+            "heat" => target + outdoor_assist,
+            _ => target - demand_assist,
         };
         let standby_target = match effective_mode {
             "heat" => target - zone.standby_offset_c.max(0.5),
@@ -1326,6 +1396,10 @@ fn adjustment_allowed(zone: &Zone) -> bool {
     (Utc::now() - last).num_seconds().max(0) as u64 >= zone.min_adjust_seconds.max(15)
 }
 
+fn external_room_sensor_cooling_assist(mode: &str, control_source: &str) -> f64 {
+    if mode == "cool" && matches!(control_source, "external" | "combined") { 0.5 } else { 0.0 }
+}
+
 fn round_device_setpoint(mode: &str, demand: bool, value: f64) -> f64 {
     let value = value.clamp(16.0, 30.0);
     match (mode, demand) {
@@ -2015,7 +2089,7 @@ mod tests {
             external_sensor_weight: 0.4, max_sensor_difference: 3.0, device_temperature: None, external_temperature: None,
             current_temperature: None, control_temperature_source: "device".into(), active_preset: "comfort".into(),
             manual_preset: None, manual_setpoint: None, manual_override_until: None,
-            device_manual_override: false, device_manual_override_since: None, device_manual_override_until: None, device_manual_override_fields: Vec::new(),
+            device_manual_override: false, device_manual_override_since: None, device_manual_override_until: None, device_manual_override_fields: Vec::new(), device_manual_override_baseline: None,
             effective_mode: "heat".into(), effective_setpoint: Some(21.0), device_setpoint: None,
             demand: false, demand_since: None, target_alerted_at: None, last_action_at: None, created_at: Utc::now(), updated_at: Utc::now(),
         }
@@ -2060,15 +2134,37 @@ mod tests {
     #[test]
     fn reset_device_manual_override_clears_takeover_state() {
         let mut zone = test_zone("device");
+        let device = Device::simulated_default();
         zone.device_manual_override = true;
         zone.device_manual_override_since = Some(Utc::now());
         zone.device_manual_override_until = Some(Utc::now());
         zone.device_manual_override_fields = vec!["target_temperature".into()];
+        zone.device_manual_override_baseline = Some((&device).into());
         assert!(reset_device_manual_override(&mut zone));
         assert!(!zone.device_manual_override);
         assert!(zone.device_manual_override_since.is_none());
         assert!(zone.device_manual_override_until.is_none());
         assert!(zone.device_manual_override_fields.is_empty());
+        assert!(zone.device_manual_override_baseline.is_none());
+    }
+
+    #[test]
+    fn restored_manual_device_state_matches_original_takeover_baseline() {
+        let mut zone = test_zone("device");
+        let baseline = Device::simulated_default();
+        zone.device_manual_override = true;
+        zone.device_manual_override_fields = vec!["power".into(), "target_temperature".into()];
+        zone.device_manual_override_baseline = Some((&baseline).into());
+
+        let mut changed = baseline.clone();
+        changed.power = !baseline.power;
+        changed.target_temperature = baseline.target_temperature + 2.0;
+        assert!(!manual_override_matches_baseline(&zone, &changed));
+
+        let mut returned_off = baseline.clone();
+        returned_off.target_temperature = baseline.target_temperature + 3.0;
+        assert!(manual_override_matches_baseline(&zone, &returned_off));
+        assert!(manual_override_matches_baseline(&zone, &baseline));
     }
 
     #[test]
@@ -2152,6 +2248,15 @@ mod tests {
         assert_eq!(round_device_setpoint("heat", false, 19.5), 19.0);
     }
 
+    #[test]
+    fn external_room_sensor_selects_lower_cooling_setpoint_only_when_used() {
+        assert_eq!(external_room_sensor_cooling_assist("cool", "external"), 0.5);
+        assert_eq!(external_room_sensor_cooling_assist("cool", "combined"), 0.5);
+        assert_eq!(external_room_sensor_cooling_assist("cool", "device_fallback"), 0.0);
+        assert_eq!(external_room_sensor_cooling_assist("cool", "device_discrepancy_fallback"), 0.0);
+        assert_eq!(external_room_sensor_cooling_assist("heat", "external"), 0.0);
+    }
+
     #[test]
     fn smart_fan_uses_low_speed_when_zone_is_satisfied() {
         assert_eq!(smart_fan_speed("heat", 21.0, 21.0, None, false), 1);