Filter some Alexa reports that are duplicate (#45093)

* Filter some Alexa reports that are duplicate

* When state changes during reporting, only report last state, not all state changes

homeassistant/components/alexa/state_report.py

@@ -2,15 +2,17 @@
 import asyncio
 import json
 import logging
+from typing import Dict, Optional
 
 import aiohttp
 import async_timeout
 
 from homeassistant.const import HTTP_ACCEPTED, MATCH_ALL, STATE_ON
+from homeassistant.core import State
 import homeassistant.util.dt as dt_util
 
 from .const import API_CHANGE, Cause
-from .entities import ENTITY_ADAPTERS, generate_alexa_id
+from .entities import ENTITY_ADAPTERS, AlexaEntity, generate_alexa_id
 from .messages import AlexaResponse
 
 _LOGGER = logging.getLogger(__name__)
@@ -25,7 +27,13 @@ async def async_enable_proactive_mode(hass, smart_home_config):
     # Validate we can get access token.
     await smart_home_config.async_get_access_token()
 
-    async def async_entity_state_listener(changed_entity, old_state, new_state):
+    progress: Dict[str, AlexaEntity] = {}
+
+    async def async_entity_state_listener(
+        changed_entity: str,
+        old_state: Optional[State],
+        new_state: Optional[State],
+    ):
         if not hass.is_running:
             return
 
@@ -39,24 +47,79 @@ async def async_enable_proactive_mode(hass, smart_home_config):
             _LOGGER.debug("Not exposing %s because filtered by config", changed_entity)
             return
 
-        alexa_changed_entity = ENTITY_ADAPTERS[new_state.domain](
+        alexa_changed_entity: AlexaEntity = ENTITY_ADAPTERS[new_state.domain](
             hass, smart_home_config, new_state
         )
 
-        for interface in alexa_changed_entity.interfaces():
+        # Queue up entity to be sent later.
-            if interface.properties_proactively_reported():
+        # If two states come in while we are reporting the state, only the last one will be reported.
-                await async_send_changereport_message(
+        if changed_entity in progress:
-                    hass, smart_home_config, alexa_changed_entity
+            progress[changed_entity] = alexa_changed_entity
-                )
             return
+
+        # Determine how entity should be reported on
+        should_report = False
+        should_doorbell = False
+
+        for interface in alexa_changed_entity.interfaces():
+            if not should_report and interface.properties_proactively_reported():
+                should_report = True
+
             if (
                 interface.name() == "Alexa.DoorbellEventSource"
                 and new_state.state == STATE_ON
             ):
+                should_doorbell = True
+                break
+
+        if not should_report and not should_doorbell:
+            return
+
+        if should_doorbell:
+            should_report = False
+
+        # Store current state change information
+        last_state: Optional[AlexaEntity] = None
+        if old_state:
+            last_state = ENTITY_ADAPTERS[old_state.domain](
+                hass, smart_home_config, old_state
+            )
+        progress[changed_entity] = alexa_changed_entity
+
+        # Start reporting on entity. Keep reporting as long as new states come in
+        # while we were reporting a state.
+        while last_state != progress[changed_entity]:
+            to_report = progress[changed_entity]
+            alexa_properties = None
+
+            if should_report:
+                # this sends all the properties of the Alexa Entity, whether they have
+                # changed or not. this should be improved, and properties that have not
+                # changed should be moved to the 'context' object
+                alexa_properties = list(alexa_changed_entity.serialize_properties())
+
+                if last_state and last_state.entity.state == to_report.entity.state:
+                    old_alexa_properties = list(last_state.serialize_properties())
+                    if old_alexa_properties == alexa_properties:
+                        return
+
+            try:
+                if should_report:
+                    await async_send_changereport_message(
+                        hass, smart_home_config, alexa_changed_entity, alexa_properties
+                    )
+
+                elif should_doorbell:
                     await async_send_doorbell_event_message(
                         hass, smart_home_config, alexa_changed_entity
                     )
-                return
+            except Exception:
+                progress.pop(changed_entity)
+                raise
+
+            last_state = to_report
+
+        progress.pop(changed_entity)
 
     return hass.helpers.event.async_track_state_change(
         MATCH_ALL, async_entity_state_listener
@@ -64,7 +127,7 @@ async def async_enable_proactive_mode(hass, smart_home_config):
 
 
 async def async_send_changereport_message(
-    hass, config, alexa_entity, *, invalidate_access_token=True
+    hass, config, alexa_entity, properties, *, invalidate_access_token=True
 ):
     """Send a ChangeReport message for an Alexa entity.
 
@@ -76,11 +139,6 @@ async def async_send_changereport_message(
 
     endpoint = alexa_entity.alexa_id()
 
-    # this sends all the properties of the Alexa Entity, whether they have
-    # changed or not. this should be improved, and properties that have not
-    # changed should be moved to the 'context' object
-    properties = list(alexa_entity.serialize_properties())
-
     payload = {
         API_CHANGE: {"cause": {"type": Cause.APP_INTERACTION}, "properties": properties}
     }
@@ -120,7 +178,7 @@ async def async_send_changereport_message(
     ):
         config.async_invalidate_access_token()
         return await async_send_changereport_message(
-            hass, config, alexa_entity, invalidate_access_token=False
+            hass, config, alexa_entity, properties, invalidate_access_token=False
         )
 
     _LOGGER.error(

tests/components/alexa/test_state_report.py

@@ -1,4 +1,8 @@
 """Test report state."""
+import asyncio
+from unittest.mock import Mock, patch
+
+from homeassistant import core
 from homeassistant.components.alexa import state_report
 
 from . import DEFAULT_CONFIG, TEST_URL
@@ -171,3 +175,141 @@ async def test_doorbell_event(hass, aioclient_mock):
     assert call_json["event"]["header"]["name"] == "DoorbellPress"
     assert call_json["event"]["payload"]["cause"]["type"] == "PHYSICAL_INTERACTION"
     assert call_json["event"]["endpoint"]["endpointId"] == "binary_sensor#test_doorbell"
+
+
+async def test_proactive_mode_filter_states(hass, aioclient_mock):
+    """Test all the cases that filter states."""
+    hass.states.async_set(
+        "binary_sensor.test_contact",
+        "on",
+        {"friendly_name": "Test Contact Sensor", "device_class": "door"},
+    )
+
+    await state_report.async_enable_proactive_mode(hass, DEFAULT_CONFIG)
+
+    # Force update should not report
+    hass.states.async_set(
+        "binary_sensor.test_contact",
+        "on",
+        {"friendly_name": "Test Contact Sensor", "device_class": "door"},
+        force_update=True,
+    )
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    assert len(aioclient_mock.mock_calls) == 0
+
+    # hass not running should not report
+    hass.states.async_set(
+        "binary_sensor.test_contact",
+        "off",
+        {"friendly_name": "Test Contact Sensor", "device_class": "door"},
+    )
+    with patch.object(hass, "state", core.CoreState.stopping):
+        await hass.async_block_till_done()
+        await hass.async_block_till_done()
+    assert len(aioclient_mock.mock_calls) == 0
+
+    # unsupported entity should not report
+    hass.states.async_set(
+        "binary_sensor.test_contact",
+        "on",
+        {"friendly_name": "Test Contact Sensor", "device_class": "door"},
+    )
+    with patch.dict(
+        "homeassistant.components.alexa.state_report.ENTITY_ADAPTERS", {}, clear=True
+    ):
+        await hass.async_block_till_done()
+        await hass.async_block_till_done()
+    assert len(aioclient_mock.mock_calls) == 0
+
+    # Not exposed by config should not report
+    hass.states.async_set(
+        "binary_sensor.test_contact",
+        "off",
+        {"friendly_name": "Test Contact Sensor", "device_class": "door"},
+    )
+    with patch.object(DEFAULT_CONFIG, "should_expose", return_value=False):
+        await hass.async_block_till_done()
+        await hass.async_block_till_done()
+    assert len(aioclient_mock.mock_calls) == 0
+
+    # Removing an entity
+    hass.states.async_remove("binary_sensor.test_contact")
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    assert len(aioclient_mock.mock_calls) == 0
+
+
+async def test_proactive_mode_filter_in_progress(hass, aioclient_mock):
+    """When in progress, queue up state."""
+    hass.states.async_set(
+        "binary_sensor.test_contact",
+        "off",
+        {"friendly_name": "Test Contact Sensor", "device_class": "door"},
+    )
+
+    await state_report.async_enable_proactive_mode(hass, DEFAULT_CONFIG)
+
+    # Progress should filter out the 2nd event.
+    long_sendchange = asyncio.Event()
+
+    with patch(
+        "homeassistant.components.alexa.state_report.async_send_changereport_message",
+        Mock(side_effect=lambda *args: long_sendchange.wait()),
+    ) as mock_report:
+        hass.states.async_set(
+            "binary_sensor.test_contact",
+            "on",
+            {
+                "friendly_name": "Test Contact Sensor",
+                "device_class": "door",
+                "update": 1,
+            },
+        )
+
+        await asyncio.sleep(0)
+        await asyncio.sleep(0)
+        await asyncio.sleep(0)
+
+    assert len(mock_report.mock_calls) == 1
+
+    with patch(
+        "homeassistant.components.alexa.state_report.async_send_changereport_message",
+    ) as mock_report_2:
+        hass.states.async_set(
+            "binary_sensor.test_contact",
+            "off",
+            {
+                "friendly_name": "Test Contact Sensor",
+                "device_class": "door",
+                "update": 2,
+            },
+        )
+        hass.states.async_set(
+            "binary_sensor.test_contact",
+            "on",
+            {
+                "friendly_name": "Test Contact Sensor",
+                "device_class": "door",
+                "update": 3,
+            },
+        )
+        hass.states.async_set(
+            "binary_sensor.test_contact",
+            "off",
+            {
+                "friendly_name": "Test Contact Sensor",
+                "device_class": "door",
+                "update": 4,
+            },
+        )
+
+        await asyncio.sleep(0)
+        await asyncio.sleep(0)
+        await asyncio.sleep(0)
+        long_sendchange.set()
+        await hass.async_block_till_done()
+
+    # Should be 1 because the 4rd state change
+    assert len(mock_report_2.mock_calls) == 1
+    mock_report_2.mock_calls[0][1][2].entity.attributes["update"] == 4