From a6d87b7fae4ecebdb56ac4dcb0e9d96cc86a740a Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 23 Apr 2021 10:56:23 -0700 Subject: [PATCH] Batch Google Report State (#49511) * Batch Google Report State * Fix batching --- .../google_assistant/report_state.py | 59 ++++++++++++++++--- .../google_assistant/test_report_state.py | 38 ++++++++++-- 2 files changed, 85 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/google_assistant/report_state.py b/homeassistant/components/google_assistant/report_state.py index cdfb06c5c39..f7c57732876 100644 --- a/homeassistant/components/google_assistant/report_state.py +++ b/homeassistant/components/google_assistant/report_state.py @@ -1,8 +1,11 @@ """Google Report State implementation.""" +from __future__ import annotations + +from collections import deque import logging from homeassistant.const import MATCH_ALL -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import CALLBACK_TYPE, HassJob, HomeAssistant, callback from homeassistant.helpers.event import async_call_later from homeassistant.helpers.significant_change import create_checker @@ -14,6 +17,8 @@ from .helpers import AbstractConfig, GoogleEntity, async_get_entities # https://github.com/actions-on-google/smart-home-nodejs/issues/196#issuecomment-439156639 INITIAL_REPORT_DELAY = 60 +# Seconds to wait to group states +REPORT_STATE_WINDOW = 1 _LOGGER = logging.getLogger(__name__) @@ -22,8 +27,35 @@ _LOGGER = logging.getLogger(__name__) def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig): """Enable state reporting.""" checker = None + unsub_pending: CALLBACK_TYPE | None = None + pending = deque([{}]) + + async def report_states(now=None): + """Report the states.""" + nonlocal pending + nonlocal unsub_pending + + pending.append({}) + + # We will report all batches except last one because those are finalized. + while len(pending) > 1: + await google_config.async_report_state_all( + {"devices": {"states": pending.popleft()}} + ) + + # If things got queued up in last batch while we were reporting, schedule ourselves again + if pending[0]: + unsub_pending = async_call_later( + hass, REPORT_STATE_WINDOW, report_states_job + ) + else: + unsub_pending = None + + report_states_job = HassJob(report_states) async def async_entity_state_listener(changed_entity, old_state, new_state): + nonlocal unsub_pending + if not hass.is_running: return @@ -47,11 +79,19 @@ def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig if not checker.async_is_significant_change(new_state, extra_arg=entity_data): return - _LOGGER.debug("Reporting state for %s: %s", changed_entity, entity_data) + _LOGGER.debug("Scheduling report state for %s: %s", changed_entity, entity_data) - await google_config.async_report_state_all( - {"devices": {"states": {changed_entity: entity_data}}} - ) + # If a significant change is already scheduled and we have another significant one, + # let's create a new batch of changes + if changed_entity in pending[-1]: + pending.append({}) + + pending[-1][changed_entity] = entity_data + + if unsub_pending is None: + unsub_pending = async_call_later( + hass, REPORT_STATE_WINDOW, report_states_job + ) @callback def extra_significant_check( @@ -102,5 +142,10 @@ def async_enable_report_state(hass: HomeAssistant, google_config: AbstractConfig unsub = async_call_later(hass, INITIAL_REPORT_DELAY, inital_report) - # pylint: disable=unnecessary-lambda - return lambda: unsub() + @callback + def unsub_all(): + unsub() + if unsub_pending: + unsub_pending() # pylint: disable=not-callable + + return unsub_all diff --git a/tests/components/google_assistant/test_report_state.py b/tests/components/google_assistant/test_report_state.py index f464be60bb9..542a971c5a7 100644 --- a/tests/components/google_assistant/test_report_state.py +++ b/tests/components/google_assistant/test_report_state.py @@ -1,4 +1,5 @@ """Test Google report state.""" +from datetime import timedelta from unittest.mock import AsyncMock, patch from homeassistant.components.google_assistant import error, report_state @@ -41,10 +42,25 @@ async def test_report_state(hass, caplog, legacy_patchable_time): hass.states.async_set("light.kitchen", "on") await hass.async_block_till_done() - assert len(mock_report.mock_calls) == 1 - assert mock_report.mock_calls[0][1][0] == { - "devices": {"states": {"light.kitchen": {"on": True, "online": True}}} - } + hass.states.async_set("light.kitchen_2", "on") + await hass.async_block_till_done() + + assert len(mock_report.mock_calls) == 0 + + async_fire_time_changed( + hass, utcnow() + timedelta(seconds=report_state.REPORT_STATE_WINDOW) + ) + await hass.async_block_till_done() + + assert len(mock_report.mock_calls) == 1 + assert mock_report.mock_calls[0][1][0] == { + "devices": { + "states": { + "light.kitchen": {"on": True, "online": True}, + "light.kitchen_2": {"on": True, "online": True}, + }, + } + } # Test that if serialize returns same value, we don't send with patch( @@ -57,6 +73,9 @@ async def test_report_state(hass, caplog, legacy_patchable_time): # Changed, but serialize is same, so filtered out by extra check hass.states.async_set("light.double_report", "off") + async_fire_time_changed( + hass, utcnow() + timedelta(seconds=report_state.REPORT_STATE_WINDOW) + ) await hass.async_block_till_done() assert len(mock_report.mock_calls) == 1 @@ -69,6 +88,9 @@ async def test_report_state(hass, caplog, legacy_patchable_time): BASIC_CONFIG, "async_report_state_all", AsyncMock() ) as mock_report: hass.states.async_set("switch.ac", "on", {"something": "else"}) + async_fire_time_changed( + hass, utcnow() + timedelta(seconds=report_state.REPORT_STATE_WINDOW) + ) await hass.async_block_till_done() assert len(mock_report.mock_calls) == 0 @@ -81,9 +103,12 @@ async def test_report_state(hass, caplog, legacy_patchable_time): side_effect=error.SmartHomeError("mock-error", "mock-msg"), ): hass.states.async_set("light.kitchen", "off") + async_fire_time_changed( + hass, utcnow() + timedelta(seconds=report_state.REPORT_STATE_WINDOW) + ) await hass.async_block_till_done() - assert "Not reporting state for light.kitchen: mock-error" + assert "Not reporting state for light.kitchen: mock-error" in caplog.text assert len(mock_report.mock_calls) == 0 unsub() @@ -92,6 +117,9 @@ async def test_report_state(hass, caplog, legacy_patchable_time): BASIC_CONFIG, "async_report_state_all", AsyncMock() ) as mock_report: hass.states.async_set("light.kitchen", "on") + async_fire_time_changed( + hass, utcnow() + timedelta(seconds=report_state.REPORT_STATE_WINDOW) + ) await hass.async_block_till_done() assert len(mock_report.mock_calls) == 0