fix window setback logic for multiple windows, fix 2

apps/rules/impl/window_setback.py

@@ -18,18 +18,6 @@ class WindowSetbackObjects(BaseModel):
     thermostats: list[str] = Field(..., min_length=1, description="Thermostats to control")
 
 
-def __get_redis_key_current_target(rule_id: str, thermo_id: str) -> str:
-    """Get Redis key for current target temperature of a thermostat"""
-    return f"rule:{rule_id}:thermo:{thermo_id}:current_target"
-def __get_redis_key_previous_target(rule_id: str, thermo_id: str) -> str:
-    """Get Redis key for previous target temperature of a thermostat"""
-    return f"rule:{rule_id}:thermo:{thermo_id}:previous"
-def __get_redis_key_contact_state(rule_id: str, contact_id: str) -> str:
-    """Get Redis key for contact sensor state"""
-    return f"rule:{rule_id}:contact:{contact_id}:is_open"
-def __get_redis_key_rule_state(rule_id: str) -> str:
-    """Get Redis key for overall rule state"""
-    return f"rule:{rule_id}:state"
 
 class WindowSetbackRule(Rule):
     """
@@ -58,6 +46,41 @@ class WindowSetbackRule(Rule):
         3. Window closes → restore from previous, then delete previous key
     """
 
+    @staticmethod
+    def __get_redis_key_current_target(rule_id: str, thermo_id: str) -> str:
+        """Get Redis key for current target temperature of a thermostat"""
+        return f"rule:{rule_id}:thermo:{thermo_id}:current_target"
+    @staticmethod
+    def __get_redis_key_previous_target(rule_id: str, thermo_id: str) -> str:
+        """Get Redis key for previous target temperature of a thermostat"""
+        return f"rule:{rule_id}:thermo:{thermo_id}:previous"
+    @staticmethod
+    def __get_redis_key_contact_state(rule_id: str, contact_id: str) -> str:
+        """Get Redis key for contact sensor state"""
+        return f"rule:{rule_id}:contact:{contact_id}:is_open"
+    @staticmethod
+    def __get_redis_key_rule_state(rule_id: str) -> str:
+        """Get Redis key for overall rule state"""
+        return f"rule:{rule_id}:state"
+    @staticmethod
+    async def __redis_get(ctx: RuleContext, key: str) -> Any:
+        """Helper to get value from Redis"""
+        v = await ctx.redis.get(key)
+        ctx.logger.debug(f"Redis GET {key} -> {v}")
+        return v
+    @staticmethod
+    async def __redis_set(ctx: RuleContext, key: str, value: Any) -> None:
+        """Helper to set value in Redis"""
+        ctx.logger.debug(f"Redis SET {key} = {value}")       
+        await ctx.redis.set(key, value)
+    @staticmethod
+    async def __redis_delete(ctx: RuleContext, key: str) -> None:
+        """Helper to delete key from Redis"""
+        ctx.logger.debug(f"Redis DEL {key}")
+        await ctx.redis.delete(key)
+
+
+
     def __init__(self):
         super().__init__()
         self._validated_objects: dict[str, WindowSetbackObjects] = {}
@@ -135,61 +158,54 @@ class WindowSetbackRule(Rule):
         """Handle contact sensor state change."""
         device_id = evt['device_id']
         contact_state = evt['payload'].get('contact')  # "open" or "closed"
-        event_ts = evt.get('ts', ctx.now().isoformat())
         
         if not contact_state:
             ctx.logger.warning(f"Contact event missing 'contact' field: {evt}")
             return
                 
-        contact_state_key = __get_redis_key_contact_state(desc.id, device_id)
+        contact_state_key = WindowSetbackRule.__get_redis_key_contact_state(desc.id, device_id)
-        await ctx.redis.set(contact_state_key, '1' if contact_state == 'open' else '0')
+        await WindowSetbackRule.__redis_set(ctx, contact_state_key, '1' if contact_state == 'open' else '0')
 
         # Check if any contact is open
         is_open = False
         for contact_id in desc.objects.get('contacts', []):
-            state_key = __get_redis_key_contact_state(desc.id, contact_id)
+            state_key = WindowSetbackRule.__get_redis_key_contact_state(desc.id, contact_id)
-            state_val = await ctx.redis.get(state_key)
+            state_val = await WindowSetbackRule.__redis_get(ctx, state_key)
             if state_val == '1':
                 is_open = True
                 break
 
-        rule_state_key = __get_redis_key_rule_state(desc.id)
+        rule_state_key = WindowSetbackRule.__get_redis_key_rule_state(desc.id)
-        current_rule_state = await ctx.redis.get(rule_state_key)
+        current_rule_state = await WindowSetbackRule.__redis_get(ctx, rule_state_key)
         if is_open and current_rule_state != '1':
             # At least one contact is open, and we are not already in eco mode
             await self._set_eco_mode(desc, ctx)
-            await ctx.redis.set(rule_state_key, '1')
+            await WindowSetbackRule.__redis_set(ctx, rule_state_key, '1')
         elif not is_open and current_rule_state != '0':
             # All contacts are closed, and we are currently in eco mode
             await self._unset_eco_mode(desc, ctx)
-            await ctx.redis.set(rule_state_key, '0')
+            await WindowSetbackRule.__redis_set(ctx, rule_state_key, '0')
 
 
-    async def _on_window_opened(self, desc: RuleDescriptor, ctx: RuleContext) -> None:
+    async def _set_eco_mode(self, desc: RuleDescriptor, ctx: RuleContext) -> None:
-        """
+        """Set thermostats to eco temperature when window opens."""
-        Window opened - save current temperatures, then set thermostats to eco.
+        eco_target = desc.params.get('eco_target', 7.0)
-        
-        Important: We must save the current target BEFORE setting to eco,
-        otherwise we'll save the eco temperature instead of the original.
-        """
-        eco_target = desc.params.get('eco_target', 16.0)
         target_thermostats = desc.objects.get('thermostats', [])
-        ttl_secs = desc.params.get('previous_target_ttl_secs', 86400)
         
         ctx.logger.info(
-            f"Rule {desc.id}: Window opened, setting {len(target_thermostats)} "
+            f"Rule {desc.id}: At least one window is opened, setting {len(target_thermostats)} "
             f"thermostats to eco temperature {eco_target}°C"
         )
         
         # FIRST: Save current target temperatures as "previous" (before we change them!)
         for thermo_id in target_thermostats:
-            current_key = __get_redis_key_current_target(desc.id, thermo_id)
+            current_key = WindowSetbackRule.__get_redis_key_current_target(desc.id, thermo_id)
-            current_temp_str = await ctx.redis.get(current_key)
+            current_temp_str = await WindowSetbackRule.__redis_get(ctx, current_key)
             
             if current_temp_str:
                 # Save current as previous (with TTL)
-                prev_key = __get_redis_key_previous_target(desc.id, thermo_id)
+                prev_key = WindowSetbackRule.__get_redis_key_previous_target(desc.id, thermo_id)
-                await ctx.redis.set(prev_key, current_temp_str, ttl_secs=ttl_secs)
+                await WindowSetbackRule.__redis_set(ctx, prev_key, current_temp_str)
                 ctx.logger.debug(
                     f"Saved previous target for {thermo_id}: {current_temp_str}°C"
                 )
@@ -206,24 +222,20 @@ class WindowSetbackRule(Rule):
             except Exception as e:
                 ctx.logger.error(f"Failed to set {thermo_id}: {e}")
 
-    async def _on_window_closed(self, desc: RuleDescriptor, ctx: RuleContext) -> None:
-        """
-        Window closed - restore previous temperatures.
 
-        Note: This is simplified. A production implementation would check
+    async def _unset_eco_mode(self, desc: RuleDescriptor, ctx: RuleContext) -> None:
-        close_min_secs and use a timer/scheduler.
+        """Restore thermostats to previous temperature when window closes."""
-        """
         target_thermostats = desc.objects.get('thermostats', [])
         
         ctx.logger.info(
-            f"Rule {desc.id}: Window closed, restoring {len(target_thermostats)} "
+            f"Rule {desc.id}: All windows closed, restoring {len(target_thermostats)} "
             f"thermostats to previous temperatures"
         )
         
         # Restore previous temperatures
         for thermo_id in target_thermostats:
-            prev_key = __get_redis_key_previous_target(desc.id, thermo_id)
+            prev_key = WindowSetbackRule.__get_redis_key_previous_target(desc.id, thermo_id)
-            prev_temp_str = await ctx.redis.get(prev_key)
+            prev_temp_str = await WindowSetbackRule.__redis_get(ctx, prev_key)
             
             if prev_temp_str:
                 try:
@@ -232,7 +244,7 @@ class WindowSetbackRule(Rule):
                     ctx.logger.debug(f"Restored {thermo_id} to {prev_temp}°C")
                     
                     # Delete the previous key after restoring
-                    await ctx.redis.delete(prev_key)
+                    await WindowSetbackRule.__redis_delete(ctx, prev_key)
                 except Exception as e:
                     ctx.logger.error(f"Failed to restore {thermo_id}: {e}")
             else:
@@ -263,10 +275,8 @@ class WindowSetbackRule(Rule):
             return  # No target in this state update
         
         # Store current target (always update, even if it's the eco temperature)
-        current_key = __get_redis_key_current_target(desc.id, device_id)
+        current_key = WindowSetbackRule.__get_redis_key_current_target(desc.id, device_id)
-        ttl_secs = desc.params.get('previous_target_ttl_secs', 86400)
+        await WindowSetbackRule.__redis_set(ctx, current_key, str(current_target))
-        
-        await ctx.redis.set(current_key, str(current_target), ttl_secs=ttl_secs)
         
         ctx.logger.debug(
             f"Rule {desc.id}: Updated current target for {device_id}: {current_target}°C"