From b75639d9d13ed2032df8aaa5c9ae34e0e5d1d6b6 Mon Sep 17 00:00:00 2001 From: Bram Kragten Date: Wed, 25 Sep 2019 23:00:18 +0200 Subject: [PATCH] Remove lamps and groups from ha when removed from Hue (#26881) * Remove light when removed from hue * add remove_config_entry_id * Review + bump aiohue * lint * Add tests --- homeassistant/components/hue/light.py | 43 +++++++++++++--- homeassistant/components/hue/manifest.json | 2 +- homeassistant/helpers/device_registry.py | 6 +++ requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/hue/test_light.py | 56 +++++++++++++++++++++ tests/helpers/test_device_registry.py | 58 ++++++++++++++++++++++ 7 files changed, 159 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/hue/light.py b/homeassistant/components/hue/light.py index 96c749a3413..5a3379f71ce 100644 --- a/homeassistant/components/hue/light.py +++ b/homeassistant/components/hue/light.py @@ -8,6 +8,9 @@ import random import aiohue import async_timeout +from homeassistant.helpers.entity_registry import async_get_registry as get_ent_reg +from homeassistant.helpers.device_registry import async_get_registry as get_dev_reg + from homeassistant.components import hue from homeassistant.components.light import ( ATTR_BRIGHTNESS, @@ -147,6 +150,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): tasks.append( async_update_items( hass, + config_entry, bridge, async_add_entities, request_update, @@ -160,6 +164,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): tasks.append( async_update_items( hass, + config_entry, bridge, async_add_entities, request_update, @@ -176,6 +181,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): async def async_update_items( hass, + config_entry, bridge, async_add_entities, request_bridge_update, @@ -204,9 +210,9 @@ async def async_update_items( _LOGGER.error("Unable to reach bridge %s (%s)", bridge.host, err) bridge.available = False - for light_id, light in current.items(): - if light_id not in progress_waiting: - light.async_schedule_update_ha_state() + for item_id, item in current.items(): + if item_id not in progress_waiting: + item.async_schedule_update_ha_state() return @@ -219,7 +225,8 @@ async def async_update_items( _LOGGER.info("Reconnected to bridge %s", bridge.host) bridge.available = True - new_lights = [] + new_items = [] + removed_items = [] for item_id in api: if item_id not in current: @@ -227,12 +234,34 @@ async def async_update_items( api[item_id], request_bridge_update, bridge, is_group ) - new_lights.append(current[item_id]) + new_items.append(current[item_id]) elif item_id not in progress_waiting: current[item_id].async_schedule_update_ha_state() - if new_lights: - async_add_entities(new_lights) + for item_id in current: + if item_id in api: + continue + + # Device is removed from Hue, so we remove it from Home Assistant + entity = current[item_id] + removed_items.append(item_id) + await entity.async_remove() + ent_registry = await get_ent_reg(hass) + if entity.entity_id in ent_registry.entities: + ent_registry.async_remove(entity.entity_id) + dev_registry = await get_dev_reg(hass) + device = dev_registry.async_get_device( + identifiers={(hue.DOMAIN, entity.unique_id)}, connections=set() + ) + dev_registry.async_update_device( + device.id, remove_config_entry_id=config_entry.entry_id + ) + + if new_items: + async_add_entities(new_items) + + for item_id in removed_items: + del current[item_id] class HueLight(Light): diff --git a/homeassistant/components/hue/manifest.json b/homeassistant/components/hue/manifest.json index c0c7c462f90..cb37dd3036f 100644 --- a/homeassistant/components/hue/manifest.json +++ b/homeassistant/components/hue/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/components/hue", "requirements": [ - "aiohue==1.9.1" + "aiohue==1.9.2" ], "ssdp": { "manufacturer": [ diff --git a/homeassistant/helpers/device_registry.py b/homeassistant/helpers/device_registry.py index 19b4a1333b6..456678edac7 100644 --- a/homeassistant/helpers/device_registry.py +++ b/homeassistant/helpers/device_registry.py @@ -157,6 +157,7 @@ class DeviceRegistry: name_by_user=_UNDEF, new_identifiers=_UNDEF, via_device_id=_UNDEF, + remove_config_entry_id=_UNDEF, ): """Update properties of a device.""" return self._async_update_device( @@ -166,6 +167,7 @@ class DeviceRegistry: name_by_user=name_by_user, new_identifiers=new_identifiers, via_device_id=via_device_id, + remove_config_entry_id=remove_config_entry_id, ) @callback @@ -203,6 +205,10 @@ class DeviceRegistry: remove_config_entry_id is not _UNDEF and remove_config_entry_id in config_entries ): + if config_entries == {remove_config_entry_id}: + self.async_remove_device(device_id) + return + config_entries = config_entries - {remove_config_entry_id} if config_entries is not old.config_entries: diff --git a/requirements_all.txt b/requirements_all.txt index 5ca7a7c8160..6481137f3e6 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -152,7 +152,7 @@ aioharmony==0.1.13 aiohttp_cors==0.7.0 # homeassistant.components.hue -aiohue==1.9.1 +aiohue==1.9.2 # homeassistant.components.imap aioimaplib==0.7.15 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index d6c0d8dbbb2..d42120c339e 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -62,7 +62,7 @@ aioesphomeapi==2.2.0 aiohttp_cors==0.7.0 # homeassistant.components.hue -aiohue==1.9.1 +aiohue==1.9.2 # homeassistant.components.notion aionotion==1.1.0 diff --git a/tests/components/hue/test_light.py b/tests/components/hue/test_light.py index 1c891b9c840..582cc185bc8 100644 --- a/tests/components/hue/test_light.py +++ b/tests/components/hue/test_light.py @@ -420,6 +420,62 @@ async def test_new_light_discovered(hass, mock_bridge): assert light.state == "off" +async def test_group_removed(hass, mock_bridge): + """Test if 2nd update has removed group.""" + mock_bridge.allow_groups = True + mock_bridge.mock_light_responses.append({}) + mock_bridge.mock_group_responses.append(GROUP_RESPONSE) + + await setup_bridge(hass, mock_bridge) + assert len(mock_bridge.mock_requests) == 2 + assert len(hass.states.async_all()) == 3 + + mock_bridge.mock_light_responses.append({}) + mock_bridge.mock_group_responses.append({"1": GROUP_RESPONSE["1"]}) + + # Calling a service will trigger the updates to run + await hass.services.async_call( + "light", "turn_on", {"entity_id": "light.group_1"}, blocking=True + ) + + # 2x group update, 2x light update, 1 turn on request + assert len(mock_bridge.mock_requests) == 5 + assert len(hass.states.async_all()) == 2 + + group = hass.states.get("light.group_1") + assert group is not None + + removed_group = hass.states.get("light.group_2") + assert removed_group is None + + +async def test_light_removed(hass, mock_bridge): + """Test if 2nd update has removed light.""" + mock_bridge.mock_light_responses.append(LIGHT_RESPONSE) + + await setup_bridge(hass, mock_bridge) + assert len(mock_bridge.mock_requests) == 1 + assert len(hass.states.async_all()) == 3 + + mock_bridge.mock_light_responses.clear() + mock_bridge.mock_light_responses.append({"1": LIGHT_RESPONSE.get("1")}) + + # Calling a service will trigger the updates to run + await hass.services.async_call( + "light", "turn_on", {"entity_id": "light.hue_lamp_1"}, blocking=True + ) + + # 2x light update, 1 turn on request + assert len(mock_bridge.mock_requests) == 3 + assert len(hass.states.async_all()) == 2 + + light = hass.states.get("light.hue_lamp_1") + assert light is not None + + removed_light = hass.states.get("light.hue_lamp_2") + assert removed_light is None + + async def test_other_group_update(hass, mock_bridge): """Test changing one group that will impact the state of other light.""" mock_bridge.allow_groups = True diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index b854b62853c..1b146e9cb12 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -409,6 +409,64 @@ async def test_update(registry): assert updated_entry.via_device_id == "98765B" +async def test_update_remove_config_entries(hass, registry, update_events): + """Make sure we do not get duplicate entries.""" + entry = registry.async_get_or_create( + config_entry_id="123", + connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")}, + identifiers={("bridgeid", "0123")}, + manufacturer="manufacturer", + model="model", + ) + entry2 = registry.async_get_or_create( + config_entry_id="456", + connections={(device_registry.CONNECTION_NETWORK_MAC, "12:34:56:AB:CD:EF")}, + identifiers={("bridgeid", "0123")}, + manufacturer="manufacturer", + model="model", + ) + entry3 = registry.async_get_or_create( + config_entry_id="123", + connections={(device_registry.CONNECTION_NETWORK_MAC, "34:56:78:CD:EF:12")}, + identifiers={("bridgeid", "4567")}, + manufacturer="manufacturer", + model="model", + ) + + assert len(registry.devices) == 2 + assert entry.id == entry2.id + assert entry.id != entry3.id + assert entry2.config_entries == {"123", "456"} + + updated_entry = registry.async_update_device( + entry2.id, remove_config_entry_id="123" + ) + removed_entry = registry.async_update_device( + entry3.id, remove_config_entry_id="123" + ) + + assert updated_entry.config_entries == {"456"} + assert removed_entry is None + + removed_entry = registry.async_get_device({("bridgeid", "4567")}, set()) + + assert removed_entry is None + + await hass.async_block_till_done() + + assert len(update_events) == 5 + assert update_events[0]["action"] == "create" + assert update_events[0]["device_id"] == entry.id + assert update_events[1]["action"] == "update" + assert update_events[1]["device_id"] == entry2.id + assert update_events[2]["action"] == "create" + assert update_events[2]["device_id"] == entry3.id + assert update_events[3]["action"] == "update" + assert update_events[3]["device_id"] == entry.id + assert update_events[4]["action"] == "remove" + assert update_events[4]["device_id"] == entry3.id + + async def test_loading_race_condition(hass): """Test only one storage load called when concurrent loading occurred .""" with asynctest.patch(