Don't attempt to set invalid fan percentage and preset_mode (try 3) (h…

…ome-assistant#70294)

Co-authored-by: J. Nick Koston <[email protected]>

homeassistant/components/fan/reproduce_state.py

@@ -2,7 +2,7 @@
 from __future__ import annotations
 
 import asyncio
-from collections.abc import Iterable, Mapping
+from collections.abc import Iterable
 import logging
 from typing import Any
 
@@ -30,13 +30,18 @@
 _LOGGER = logging.getLogger(__name__)
 
 VALID_STATES = {STATE_ON, STATE_OFF}
-ATTRIBUTES = {  # attribute: service
-    ATTR_DIRECTION: SERVICE_SET_DIRECTION,
-    ATTR_OSCILLATING: SERVICE_OSCILLATE,
+
+# These are used as parameters to fan.turn_on service.
+SPEED_AND_MODE_ATTRIBUTES = {
     ATTR_PERCENTAGE: SERVICE_SET_PERCENTAGE,
     ATTR_PRESET_MODE: SERVICE_SET_PRESET_MODE,
 }
 
+SIMPLE_ATTRIBUTES = {  # attribute: service
+    ATTR_DIRECTION: SERVICE_SET_DIRECTION,
+    ATTR_OSCILLATING: SERVICE_OSCILLATE,
+}
+
 
 async def _async_reproduce_state(
     hass: HomeAssistant,
@@ -56,37 +61,49 @@ async def _async_reproduce_state(
         )
         return
 
-    # Return if we are already at the right state.
-    if cur_state.state == state.state and all(
-        check_attr_equal(cur_state.attributes, state.attributes, attr)
-        for attr in ATTRIBUTES
-    ):
-        return
-
-    service_data = {ATTR_ENTITY_ID: state.entity_id}
-    service_calls = {}  # service: service_data
+    service_calls: dict[str, dict[str, Any]] = {}
 
     if state.state == STATE_ON:
         # The fan should be on
         if cur_state.state != STATE_ON:
-            # Turn on the fan at first
-            service_calls[SERVICE_TURN_ON] = service_data
-
-        for attr, service in ATTRIBUTES.items():
-            # Call services to adjust the attributes
-            if attr in state.attributes and not check_attr_equal(
-                state.attributes, cur_state.attributes, attr
-            ):
-                data = service_data.copy()
-                data[attr] = state.attributes[attr]
-                service_calls[service] = data
-
-    elif state.state == STATE_OFF:
-        service_calls[SERVICE_TURN_OFF] = service_data
+            # Turn on the fan with all the speed and modes attributes.
+            # The `turn_on` method will figure out in which mode to
+            # turn the fan on.
+            service_calls[SERVICE_TURN_ON] = {
+                attr: state.attributes.get(attr)
+                for attr in SPEED_AND_MODE_ATTRIBUTES
+                if state.attributes.get(attr) is not None
+            }
+        else:
+            # If the fan is already on, we need to set speed or mode
+            # based on the state.
+            #
+            # Speed and preset mode are mutually exclusive, so one of
+            # them is always going to be stored as None. If we were to
+            # try to set it, it will raise an error. So instead we
+            # only update the one that is non-None.
+            for attr, service in SPEED_AND_MODE_ATTRIBUTES.items():
+                value = state.attributes.get(attr)
+                if value is not None and value != cur_state.attributes.get(attr):
+                    service_calls[service] = {attr: value}
+
+        # The simple attributes are copied directly. They can only be
+        # None if the fan does not support the feature in the first
+        # place, so the equality check ensures we don't call the
+        # services with invalid parameters.
+        for attr, service in SIMPLE_ATTRIBUTES.items():
+            if (value := state.attributes.get(attr)) != cur_state.attributes.get(attr):
+                service_calls[service] = {attr: value}
+    elif state.state == STATE_OFF and cur_state.state != state.state:
+        service_calls[SERVICE_TURN_OFF] = {}
 
     for service, data in service_calls.items():
         await hass.services.async_call(
-            DOMAIN, service, data, context=context, blocking=True
+            DOMAIN,
+            service,
+            {ATTR_ENTITY_ID: state.entity_id, **data},
+            context=context,
+            blocking=True,
         )
 
 
@@ -106,8 +123,3 @@ async def async_reproduce_states(
             for state in states
         )
     )
-
-
-def check_attr_equal(attr1: Mapping, attr2: Mapping, attr_str: str) -> bool:
-    """Return true if the given attributes are equal."""
-    return attr1.get(attr_str) == attr2.get(attr_str)