Add significant change filtering to Google (#45566)

homeassistant/components/google_assistant/report_state.py

@@ -4,7 +4,9 @@ import logging
 from homeassistant.const import MATCH_ALL
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers.event import async_call_later
+from homeassistant.helpers.significant_change import create_checker
 
+from .const import DOMAIN
 from .error import SmartHomeError
 from .helpers import AbstractConfig, GoogleEntity, async_get_entities
 
@@ -19,6 +21,7 @@ _LOGGER = logging.getLogger(__name__)
 @callback
 def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig):
     """Enable state reporting."""
+    checker = None
 
     async def async_entity_state_listener(changed_entity, old_state, new_state):
         if not hass.is_running:
@@ -35,25 +38,15 @@ def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig
         if not entity.is_supported():
             return
 
+        if not checker.async_is_significant_change(new_state):
+            return
+
         try:
             entity_data = entity.query_serialize()
         except SmartHomeError as err:
             _LOGGER.debug("Not reporting state for %s: %s", changed_entity, err.code)
             return
 
-        if old_state:
-            old_entity = GoogleEntity(hass, google_config, old_state)
-
-            # Only report to Google if data that Google cares about has changed
-            try:
-                if entity_data == old_entity.query_serialize():
-                    return
-            except SmartHomeError:
-                # Happens if old state could not be serialized.
-                # In that case the data is different and should be
-                # reported.
-                pass
-
         _LOGGER.debug("Reporting state for %s: %s", changed_entity, entity_data)
 
         await google_config.async_report_state_all(
@@ -62,12 +55,20 @@ def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig
 
     async def inital_report(_now):
         """Report initially all states."""
+        nonlocal unsub, checker
         entities = {}
 
+        checker = await create_checker(hass, DOMAIN)
+
         for entity in async_get_entities(hass, google_config):
             if not entity.should_expose():
                 continue
 
+            # Tell our significant change checker that we're reporting
+            # So it knows with subsequent changes what was already reported.
+            if not checker.async_is_significant_change(entity.state):
+                continue
+
             try:
                 entities[entity.entity_id] = entity.query_serialize()
             except SmartHomeError:
@@ -78,8 +79,11 @@ def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig
 
         await google_config.async_report_state_all({"devices": {"states": entities}})
 
-    async_call_later(hass, INITIAL_REPORT_DELAY, inital_report)
+        unsub = hass.helpers.event.async_track_state_change(
+            MATCH_ALL, async_entity_state_listener
+        )
 
-    return hass.helpers.event.async_track_state_change(
-        MATCH_ALL, async_entity_state_listener
-    )
+    unsub = async_call_later(hass, INITIAL_REPORT_DELAY, inital_report)
+
+    # pylint: disable=unnecessary-lambda
+    return lambda: unsub()

homeassistant/components/sensor/significant_change.py

@@ -6,12 +6,13 @@ from homeassistant.const import (
     ATTR_UNIT_OF_MEASUREMENT,
     TEMP_FAHRENHEIT,
 )
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 
 from . import DEVICE_CLASS_BATTERY, DEVICE_CLASS_HUMIDITY, DEVICE_CLASS_TEMPERATURE
 
 
-async def async_check_significant_change(
+@callback
+def async_check_significant_change(
     hass: HomeAssistant,
     old_state: str,
     old_attrs: dict,

homeassistant/components/switch/significant_change.py

@@ -0,0 +1,17 @@
+"""Helper to test significant Switch state changes."""
+from typing import Any, Optional
+
+from homeassistant.core import HomeAssistant, callback
+
+
+@callback
+def async_check_significant_change(
+    hass: HomeAssistant,
+    old_state: str,
+    old_attrs: dict,
+    new_state: str,
+    new_attrs: dict,
+    **kwargs: Any,
+) -> Optional[bool]:
+    """Test if state significantly changed."""
+    return old_state != new_state

script/scaffold/templates/significant_change/integration/significant_change.py

@@ -2,10 +2,11 @@
 from typing import Any, Optional
 
 from homeassistant.const import ATTR_DEVICE_CLASS
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 
 
-async def async_check_significant_change(
+@callback
+def async_check_significant_change(
     hass: HomeAssistant,
     old_state: str,
     old_attrs: dict,

tests/components/google_assistant/test_report_state.py

@@ -2,6 +2,7 @@
 from unittest.mock import AsyncMock, patch
 
 from homeassistant.components.google_assistant import error, report_state
+from homeassistant.setup import async_setup_component
 from homeassistant.util.dt import utcnow
 
 from . import BASIC_CONFIG
@@ -11,6 +12,7 @@ from tests.common import async_fire_time_changed
 
 async def test_report_state(hass, caplog, legacy_patchable_time):
     """Test report state works."""
+    assert await async_setup_component(hass, "switch", {})
     hass.states.async_set("light.ceiling", "off")
     hass.states.async_set("switch.ac", "on")
 
@@ -44,14 +46,11 @@ async def test_report_state(hass, caplog, legacy_patchable_time):
         "devices": {"states": {"light.kitchen": {"on": True, "online": True}}}
     }
 
-    # Test that state changes that change something that Google doesn't care about
-    # do not trigger a state report.
+    # Test that only significant state changes are reported
     with patch.object(
         BASIC_CONFIG, "async_report_state_all", AsyncMock()
     ) as mock_report:
-        hass.states.async_set(
-            "light.kitchen", "on", {"irrelevant": "should_be_ignored"}
-        )
+        hass.states.async_set("switch.ac", "on", {"something": "else"})
         await hass.async_block_till_done()
 
     assert len(mock_report.mock_calls) == 0

tests/components/sensor/test_significant_change.py

@@ -19,13 +19,13 @@ async def test_significant_change_temperature():
         ATTR_DEVICE_CLASS: DEVICE_CLASS_TEMPERATURE,
         ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS,
     }
-    assert not await async_check_significant_change(
+    assert not async_check_significant_change(
         None, "12", celsius_attrs, "12", celsius_attrs
     )
-    assert await async_check_significant_change(
+    assert async_check_significant_change(
         None, "12", celsius_attrs, "13", celsius_attrs
     )
-    assert not await async_check_significant_change(
+    assert not async_check_significant_change(
         None, "12.1", celsius_attrs, "12.2", celsius_attrs
     )
 
@@ -33,10 +33,10 @@ async def test_significant_change_temperature():
         ATTR_DEVICE_CLASS: DEVICE_CLASS_TEMPERATURE,
         ATTR_UNIT_OF_MEASUREMENT: TEMP_FAHRENHEIT,
     }
-    assert await async_check_significant_change(
+    assert async_check_significant_change(
         None, "70", freedom_attrs, "74", freedom_attrs
     )
-    assert not await async_check_significant_change(
+    assert not async_check_significant_change(
         None, "70", freedom_attrs, "71", freedom_attrs
     )
 
@@ -46,8 +46,8 @@ async def test_significant_change_battery():
     attrs = {
         ATTR_DEVICE_CLASS: DEVICE_CLASS_BATTERY,
     }
-    assert not await async_check_significant_change(None, "100", attrs, "100", attrs)
-    assert await async_check_significant_change(None, "100", attrs, "97", attrs)
+    assert not async_check_significant_change(None, "100", attrs, "100", attrs)
+    assert async_check_significant_change(None, "100", attrs, "97", attrs)
 
 
 async def test_significant_change_humidity():
@@ -55,5 +55,5 @@ async def test_significant_change_humidity():
     attrs = {
         ATTR_DEVICE_CLASS: DEVICE_CLASS_HUMIDITY,
     }
-    assert not await async_check_significant_change(None, "100", attrs, "100", attrs)
-    assert await async_check_significant_change(None, "100", attrs, "97", attrs)
+    assert not async_check_significant_change(None, "100", attrs, "100", attrs)
+    assert async_check_significant_change(None, "100", attrs, "97", attrs)

tests/components/switch/test_significant_change.py

@@ -0,0 +1,12 @@
+"""Test the sensor significant change platform."""
+from homeassistant.components.switch.significant_change import (
+    async_check_significant_change,
+)
+
+
+async def test_significant_change():
+    """Detect Switch significant change."""
+    attrs = {}
+    assert not async_check_significant_change(None, "on", attrs, "on", attrs)
+    assert not async_check_significant_change(None, "off", attrs, "off", attrs)
+    assert async_check_significant_change(None, "on", attrs, "off", attrs)