From aa56b4dd30b459f9c45b62870e00dc8ee9904478 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 22 Aug 2019 14:12:24 -0700 Subject: [PATCH] Log warning if disabled entities receive updates. (#26143) * Log warning if disabled entities receive updates. * Fix test * Always set entity ID on disabled entities --- homeassistant/helpers/entity.py | 13 ++++++++ homeassistant/helpers/entity_platform.py | 6 ++-- .../components/config/test_entity_registry.py | 21 ++++++++++--- tests/helpers/test_entity.py | 31 +++++++++++++++++-- tests/helpers/test_entity_platform.py | 2 +- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/homeassistant/helpers/entity.py b/homeassistant/helpers/entity.py index aecdf45dde5..7de41415f08 100644 --- a/homeassistant/helpers/entity.py +++ b/homeassistant/helpers/entity.py @@ -99,6 +99,9 @@ class Entity: # If we reported if this entity was slow _slow_reported = False + # If we reported this entity is updated while disabled + _disabled_reported = False + # Protect for multiple updates _update_staged = False @@ -273,6 +276,16 @@ class Entity: @callback def _async_write_ha_state(self): """Write the state to the state machine.""" + if self.registry_entry and self.registry_entry.disabled_by: + if not self._disabled_reported: + self._disabled_reported = True + _LOGGER.warning( + "Entity %s is incorrectly being triggered for updates while it is disabled. This is a bug in the %s integration.", + self.entity_id, + self.platform.platform_name, + ) + return + start = timer() attr = {} diff --git a/homeassistant/helpers/entity_platform.py b/homeassistant/helpers/entity_platform.py index 74351ac50af..4a6a3038fd0 100644 --- a/homeassistant/helpers/entity_platform.py +++ b/homeassistant/helpers/entity_platform.py @@ -349,6 +349,9 @@ class EntityPlatform: disabled_by=disabled_by, ) + entity.registry_entry = entry + entity.entity_id = entry.entity_id + if entry.disabled: self.logger.info( "Not adding entity %s because it's disabled", @@ -358,9 +361,6 @@ class EntityPlatform: ) return - entity.registry_entry = entry - entity.entity_id = entry.entity_id - # We won't generate an entity ID if the platform has already set one # We will however make sure that platform cannot pick a registered ID elif entity.entity_id is not None and entity_registry.async_is_registered( diff --git a/tests/components/config/test_entity_registry.py b/tests/components/config/test_entity_registry.py index f18abe9b0e2..64328a0c8c5 100644 --- a/tests/components/config/test_entity_registry.py +++ b/tests/components/config/test_entity_registry.py @@ -127,13 +127,13 @@ async def test_update_entity(hass, client): assert state is not None assert state.name == "before update" + # UPDATE NAME await client.send_json( { "id": 6, "type": "config/entity_registry/update", "entity_id": "test_domain.world", "name": "after update", - "disabled_by": "user", } ) @@ -142,7 +142,7 @@ async def test_update_entity(hass, client): assert msg["result"] == { "config_entry_id": None, "device_id": None, - "disabled_by": "user", + "disabled_by": None, "platform": "test_platform", "entity_id": "test_domain.world", "name": "after update", @@ -151,13 +151,26 @@ async def test_update_entity(hass, client): state = hass.states.get("test_domain.world") assert state.name == "after update" - assert registry.entities["test_domain.world"].disabled_by == "user" - + # UPDATE DISABLED_BY TO USER await client.send_json( { "id": 7, "type": "config/entity_registry/update", "entity_id": "test_domain.world", + "disabled_by": "user", + } + ) + + msg = await client.receive_json() + + assert registry.entities["test_domain.world"].disabled_by == "user" + + # UPDATE DISABLED_BY TO NONE + await client.send_json( + { + "id": 8, + "type": "config/entity_registry/update", + "entity_id": "test_domain.world", "disabled_by": None, } ) diff --git a/tests/helpers/test_entity.py b/tests/helpers/test_entity.py index 58f76d396c1..94650592d8e 100644 --- a/tests/helpers/test_entity.py +++ b/tests/helpers/test_entity.py @@ -7,13 +7,13 @@ from unittest.mock import MagicMock, patch, PropertyMock import pytest -import homeassistant.helpers.entity as entity +from homeassistant.helpers import entity, entity_registry from homeassistant.core import Context from homeassistant.const import ATTR_HIDDEN, ATTR_DEVICE_CLASS from homeassistant.config import DATA_CUSTOMIZE from homeassistant.helpers.entity_values import EntityValues -from tests.common import get_test_home_assistant +from tests.common import get_test_home_assistant, mock_registry def test_generate_entity_id_requires_hass_or_ids(): @@ -499,3 +499,30 @@ async def test_set_context_expired(hass): assert hass.states.get("hello.world").context != context assert ent._context is None assert ent._context_set is None + + +async def test_warn_disabled(hass, caplog): + """Test we warn once if we write to a disabled entity.""" + entry = entity_registry.RegistryEntry( + entity_id="hello.world", + unique_id="test-unique-id", + platform="test-platform", + disabled_by="user", + ) + mock_registry(hass, {"hello.world": entry}) + + ent = entity.Entity() + ent.hass = hass + ent.entity_id = "hello.world" + ent.registry_entry = entry + ent.platform = MagicMock(platform_name="test-platform") + + caplog.clear() + ent.async_write_ha_state() + assert hass.states.get("hello.world") is None + assert "Entity hello.world is incorrectly being triggered" in caplog.text + + caplog.clear() + ent.async_write_ha_state() + assert hass.states.get("hello.world") is None + assert caplog.text == "" diff --git a/tests/helpers/test_entity_platform.py b/tests/helpers/test_entity_platform.py index 606a4c82096..caf8bb702af 100644 --- a/tests/helpers/test_entity_platform.py +++ b/tests/helpers/test_entity_platform.py @@ -491,7 +491,7 @@ async def test_registry_respect_entity_disabled(hass): platform = MockEntityPlatform(hass) entity = MockEntity(unique_id="1234") await platform.async_add_entities([entity]) - assert entity.entity_id is None + assert entity.entity_id == "test_domain.world" assert hass.states.async_entity_ids() == []