diff --git a/homeassistant/components/alexa/state_report.py b/homeassistant/components/alexa/state_report.py index aa4110ea686..d66906810b2 100644 --- a/homeassistant/components/alexa/state_report.py +++ b/homeassistant/components/alexa/state_report.py @@ -8,7 +8,7 @@ import aiohttp import async_timeout from homeassistant.const import HTTP_ACCEPTED, MATCH_ALL, STATE_ON -from homeassistant.core import State +from homeassistant.core import HomeAssistant, State, callback from homeassistant.helpers.significant_change import create_checker import homeassistant.util.dt as dt_util @@ -28,7 +28,20 @@ async def async_enable_proactive_mode(hass, smart_home_config): # Validate we can get access token. await smart_home_config.async_get_access_token() - checker = await create_checker(hass, DOMAIN) + @callback + def extra_significant_check( + hass: HomeAssistant, + old_state: str, + old_attrs: dict, + old_extra_arg: dict, + new_state: str, + new_attrs: dict, + new_extra_arg: dict, + ): + """Check if the serialized data has changed.""" + return old_extra_arg is not None and old_extra_arg != new_extra_arg + + checker = await create_checker(hass, DOMAIN, extra_significant_check) async def async_entity_state_listener( changed_entity: str, @@ -70,15 +83,22 @@ async def async_enable_proactive_mode(hass, smart_home_config): if not should_report and not should_doorbell: return - if not checker.async_is_significant_change(new_state): - return - if should_doorbell: should_report = False + if should_report: + alexa_properties = list(alexa_changed_entity.serialize_properties()) + else: + alexa_properties = None + + if not checker.async_is_significant_change( + new_state, extra_arg=alexa_properties + ): + return + if should_report: await async_send_changereport_message( - hass, smart_home_config, alexa_changed_entity + hass, smart_home_config, alexa_changed_entity, alexa_properties ) elif should_doorbell: @@ -92,7 +112,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, alexa_properties, *, invalidate_access_token=True ): """Send a ChangeReport message for an Alexa entity. @@ -107,7 +127,7 @@ async def async_send_changereport_message( payload = { API_CHANGE: { "cause": {"type": Cause.APP_INTERACTION}, - "properties": list(alexa_entity.serialize_properties()), + "properties": alexa_properties, } } @@ -146,7 +166,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, alexa_properties, invalidate_access_token=False ) _LOGGER.error( diff --git a/homeassistant/components/google_assistant/report_state.py b/homeassistant/components/google_assistant/report_state.py index 8943d4d211e..cdfb06c5c39 100644 --- a/homeassistant/components/google_assistant/report_state.py +++ b/homeassistant/components/google_assistant/report_state.py @@ -38,42 +38,59 @@ 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 not checker.async_is_significant_change(new_state, extra_arg=entity_data): + return + _LOGGER.debug("Reporting state for %s: %s", changed_entity, entity_data) await google_config.async_report_state_all( {"devices": {"states": {changed_entity: entity_data}}} ) + @callback + def extra_significant_check( + hass: HomeAssistant, + old_state: str, + old_attrs: dict, + old_extra_arg: dict, + new_state: str, + new_attrs: dict, + new_extra_arg: dict, + ): + """Check if the serialized data has changed.""" + return old_extra_arg != new_extra_arg + async def inital_report(_now): """Report initially all states.""" nonlocal unsub, checker entities = {} - checker = await create_checker(hass, DOMAIN) + checker = await create_checker(hass, DOMAIN, extra_significant_check) 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() + entity_data = entity.query_serialize() except SmartHomeError: 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, extra_arg=entity_data + ): + continue + + entities[entity.entity_id] = entity_data + if not entities: return diff --git a/homeassistant/helpers/significant_change.py b/homeassistant/helpers/significant_change.py index 0a5b6aae10d..694acfcf2bd 100644 --- a/homeassistant/helpers/significant_change.py +++ b/homeassistant/helpers/significant_change.py @@ -27,7 +27,7 @@ The following cases will never be passed to your function: - state adding/removing """ from types import MappingProxyType -from typing import Any, Callable, Dict, Optional, Union +from typing import Any, Callable, Dict, Optional, Tuple, Union from homeassistant.const import STATE_UNAVAILABLE, STATE_UNKNOWN from homeassistant.core import HomeAssistant, State, callback @@ -47,13 +47,28 @@ CheckTypeFunc = Callable[ Optional[bool], ] +ExtraCheckTypeFunc = Callable[ + [ + HomeAssistant, + str, + Union[dict, MappingProxyType], + Any, + str, + Union[dict, MappingProxyType], + Any, + ], + Optional[bool], +] + async def create_checker( - hass: HomeAssistant, _domain: str + hass: HomeAssistant, + _domain: str, + extra_significant_check: Optional[ExtraCheckTypeFunc] = None, ) -> "SignificantlyChangedChecker": """Create a significantly changed checker for a domain.""" await _initialize(hass) - return SignificantlyChangedChecker(hass) + return SignificantlyChangedChecker(hass, extra_significant_check) # Marked as singleton so multiple calls all wait for same output. @@ -105,34 +120,46 @@ class SignificantlyChangedChecker: Will always compare the entity to the last entity that was considered significant. """ - def __init__(self, hass: HomeAssistant) -> None: + def __init__( + self, + hass: HomeAssistant, + extra_significant_check: Optional[ExtraCheckTypeFunc] = None, + ) -> None: """Test if an entity has significantly changed.""" self.hass = hass - self.last_approved_entities: Dict[str, State] = {} + self.last_approved_entities: Dict[str, Tuple[State, Any]] = {} + self.extra_significant_check = extra_significant_check @callback - def async_is_significant_change(self, new_state: State) -> bool: - """Return if this was a significant change.""" - old_state: Optional[State] = self.last_approved_entities.get( + def async_is_significant_change( + self, new_state: State, *, extra_arg: Optional[Any] = None + ) -> bool: + """Return if this was a significant change. + + Extra kwargs are passed to the extra significant checker. + """ + old_data: Optional[Tuple[State, Any]] = self.last_approved_entities.get( new_state.entity_id ) # First state change is always ok to report - if old_state is None: - self.last_approved_entities[new_state.entity_id] = new_state + if old_data is None: + self.last_approved_entities[new_state.entity_id] = (new_state, extra_arg) return True + old_state, old_extra_arg = old_data + # Handle state unknown or unavailable if new_state.state in (STATE_UNKNOWN, STATE_UNAVAILABLE): if new_state.state == old_state.state: return False - self.last_approved_entities[new_state.entity_id] = new_state + self.last_approved_entities[new_state.entity_id] = (new_state, extra_arg) return True # If last state was unknown/unavailable, also significant. if old_state.state in (STATE_UNKNOWN, STATE_UNAVAILABLE): - self.last_approved_entities[new_state.entity_id] = new_state + self.last_approved_entities[new_state.entity_id] = (new_state, extra_arg) return True functions: Optional[Dict[str, CheckTypeFunc]] = self.hass.data.get( @@ -144,23 +171,36 @@ class SignificantlyChangedChecker: check_significantly_changed = functions.get(new_state.domain) - # No platform available means always true. - if check_significantly_changed is None: - self.last_approved_entities[new_state.entity_id] = new_state - return True + if check_significantly_changed is not None: + result = check_significantly_changed( + self.hass, + old_state.state, + old_state.attributes, + new_state.state, + new_state.attributes, + ) - result = check_significantly_changed( - self.hass, - old_state.state, - old_state.attributes, - new_state.state, - new_state.attributes, - ) + if result is False: + return False - if result is False: - return False + if self.extra_significant_check is not None: + result = self.extra_significant_check( + self.hass, + old_state.state, + old_state.attributes, + old_extra_arg, + new_state.state, + new_state.attributes, + extra_arg, + ) + + if result is False: + return False # Result is either True or None. # None means the function doesn't know. For now assume it's True - self.last_approved_entities[new_state.entity_id] = new_state + self.last_approved_entities[new_state.entity_id] = ( + new_state, + extra_arg, + ) return True diff --git a/tests/components/alexa/test_state_report.py b/tests/components/alexa/test_state_report.py index 809bca5638b..a057eada531 100644 --- a/tests/components/alexa/test_state_report.py +++ b/tests/components/alexa/test_state_report.py @@ -178,6 +178,7 @@ async def test_doorbell_event(hass, aioclient_mock): async def test_proactive_mode_filter_states(hass, aioclient_mock): """Test all the cases that filter states.""" + aioclient_mock.post(TEST_URL, text="", status=202) await state_report.async_enable_proactive_mode(hass, DEFAULT_CONFIG) # First state should report @@ -186,7 +187,8 @@ async def test_proactive_mode_filter_states(hass, aioclient_mock): "on", {"friendly_name": "Test Contact Sensor", "device_class": "door"}, ) - assert len(aioclient_mock.mock_calls) == 0 + await hass.async_block_till_done() + assert len(aioclient_mock.mock_calls) == 1 aioclient_mock.clear_requests() @@ -238,3 +240,24 @@ async def test_proactive_mode_filter_states(hass, aioclient_mock): await hass.async_block_till_done() await hass.async_block_till_done() assert len(aioclient_mock.mock_calls) == 0 + + # If serializes to same properties, it should not report + aioclient_mock.post(TEST_URL, text="", status=202) + with patch( + "homeassistant.components.alexa.entities.AlexaEntity.serialize_properties", + return_value=[{"same": "info"}], + ): + hass.states.async_set( + "binary_sensor.same_serialize", + "off", + {"friendly_name": "Test Contact Sensor", "device_class": "door"}, + ) + await hass.async_block_till_done() + hass.states.async_set( + "binary_sensor.same_serialize", + "off", + {"friendly_name": "Test Contact Sensor", "device_class": "door"}, + ) + + await hass.async_block_till_done() + assert len(aioclient_mock.mock_calls) == 1 diff --git a/tests/components/google_assistant/test_report_state.py b/tests/components/google_assistant/test_report_state.py index 72130dbfdb9..f464be60bb9 100644 --- a/tests/components/google_assistant/test_report_state.py +++ b/tests/components/google_assistant/test_report_state.py @@ -46,6 +46,24 @@ async def test_report_state(hass, caplog, legacy_patchable_time): "devices": {"states": {"light.kitchen": {"on": True, "online": True}}} } + # Test that if serialize returns same value, we don't send + with patch( + "homeassistant.components.google_assistant.report_state.GoogleEntity.query_serialize", + return_value={"same": "info"}, + ), patch.object(BASIC_CONFIG, "async_report_state_all", AsyncMock()) as mock_report: + # New state, so reported + hass.states.async_set("light.double_report", "on") + await hass.async_block_till_done() + + # Changed, but serialize is same, so filtered out by extra check + hass.states.async_set("light.double_report", "off") + await hass.async_block_till_done() + + assert len(mock_report.mock_calls) == 1 + assert mock_report.mock_calls[0][1][0] == { + "devices": {"states": {"light.double_report": {"same": "info"}}} + } + # Test that only significant state changes are reported with patch.object( BASIC_CONFIG, "async_report_state_all", AsyncMock() diff --git a/tests/helpers/test_significant_change.py b/tests/helpers/test_significant_change.py index e72951d36dd..79f3dd3fe3e 100644 --- a/tests/helpers/test_significant_change.py +++ b/tests/helpers/test_significant_change.py @@ -5,7 +5,6 @@ from homeassistant.components.sensor import DEVICE_CLASS_BATTERY from homeassistant.const import ATTR_DEVICE_CLASS, STATE_UNAVAILABLE, STATE_UNKNOWN from homeassistant.core import State from homeassistant.helpers import significant_change -from homeassistant.setup import async_setup_component @pytest.fixture(name="checker") @@ -26,8 +25,6 @@ async def checker_fixture(hass): async def test_signicant_change(hass, checker): """Test initialize helper works.""" - assert await async_setup_component(hass, "sensor", {}) - ent_id = "test_domain.test_entity" attrs = {ATTR_DEVICE_CLASS: DEVICE_CLASS_BATTERY} @@ -48,3 +45,30 @@ async def test_signicant_change(hass, checker): # State turned unavailable assert checker.async_is_significant_change(State(ent_id, "100", attrs)) assert checker.async_is_significant_change(State(ent_id, STATE_UNAVAILABLE, attrs)) + + +async def test_significant_change_extra(hass, checker): + """Test extra significant checker works.""" + ent_id = "test_domain.test_entity" + attrs = {ATTR_DEVICE_CLASS: DEVICE_CLASS_BATTERY} + + assert checker.async_is_significant_change(State(ent_id, "100", attrs), extra_arg=1) + assert checker.async_is_significant_change(State(ent_id, "200", attrs), extra_arg=1) + + # Reset the last significiant change to 100 to repeat test but with + # extra checker installed. + assert checker.async_is_significant_change(State(ent_id, "100", attrs), extra_arg=1) + + def extra_significant_check( + hass, old_state, old_attrs, old_extra_arg, new_state, new_attrs, new_extra_arg + ): + return old_extra_arg != new_extra_arg + + checker.extra_significant_check = extra_significant_check + + # This is normally a significant change (100 -> 200), but the extra arg check marks it + # as insignificant. + assert not checker.async_is_significant_change( + State(ent_id, "200", attrs), extra_arg=1 + ) + assert checker.async_is_significant_change(State(ent_id, "200", attrs), extra_arg=2)