From 712a5a098d3cc787976f67628103adb32cb2d0ad Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 26 Jan 2021 21:45:09 +0100 Subject: [PATCH] Add significant change filtering to Google (#45566) --- .../google_assistant/report_state.py | 38 ++++++++++--------- .../components/sensor/significant_change.py | 5 ++- .../components/switch/significant_change.py | 17 +++++++++ .../integration/significant_change.py | 5 ++- .../google_assistant/test_report_state.py | 9 ++--- .../sensor/test_significant_change.py | 18 ++++----- .../switch/test_significant_change.py | 12 ++++++ 7 files changed, 69 insertions(+), 35 deletions(-) create mode 100644 homeassistant/components/switch/significant_change.py create mode 100644 tests/components/switch/test_significant_change.py diff --git a/homeassistant/components/google_assistant/report_state.py b/homeassistant/components/google_assistant/report_state.py index 8ffdad14140..8943d4d211e 100644 --- a/homeassistant/components/google_assistant/report_state.py +++ b/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() diff --git a/homeassistant/components/sensor/significant_change.py b/homeassistant/components/sensor/significant_change.py index 08d0da68d8b..74e2779c8d7 100644 --- a/homeassistant/components/sensor/significant_change.py +++ b/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, diff --git a/homeassistant/components/switch/significant_change.py b/homeassistant/components/switch/significant_change.py new file mode 100644 index 00000000000..f4dcddc3f34 --- /dev/null +++ b/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 diff --git a/script/scaffold/templates/significant_change/integration/significant_change.py b/script/scaffold/templates/significant_change/integration/significant_change.py index 26f5b84d99e..23a00c603ac 100644 --- a/script/scaffold/templates/significant_change/integration/significant_change.py +++ b/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, diff --git a/tests/components/google_assistant/test_report_state.py b/tests/components/google_assistant/test_report_state.py index 4ab206edb90..72130dbfdb9 100644 --- a/tests/components/google_assistant/test_report_state.py +++ b/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 diff --git a/tests/components/sensor/test_significant_change.py b/tests/components/sensor/test_significant_change.py index 0386322625f..1013d96f524 100644 --- a/tests/components/sensor/test_significant_change.py +++ b/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) diff --git a/tests/components/switch/test_significant_change.py b/tests/components/switch/test_significant_change.py new file mode 100644 index 00000000000..194efb68583 --- /dev/null +++ b/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)