From 02adda88d18cb4249fd51e76aa487daaaf1c8fb3 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 11 Jan 2022 23:56:07 +0100 Subject: [PATCH 1/7] Fix string comparison issues for device disabled_by attribute This fixes string comparisons introduced by "Make device entry disabled by an enum (#60239)." --- homeassistant/helpers/device_registry.py | 2 +- homeassistant/helpers/entity_registry.py | 2 +- tests/helpers/test_device_registry.py | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/homeassistant/helpers/device_registry.py b/homeassistant/helpers/device_registry.py index 6bec4e592e63a3..d84c55c51eeee2 100644 --- a/homeassistant/helpers/device_registry.py +++ b/homeassistant/helpers/device_registry.py @@ -732,7 +732,7 @@ def async_config_entry_disabled_by_changed( if not config_entry.disabled_by: for device in devices: - if device.disabled_by is not DeviceEntryDisabler.CONFIG_ENTRY: + if device.disabled_by != DeviceEntryDisabler.CONFIG_ENTRY: continue registry.async_update_device(device.id, disabled_by=None) return diff --git a/homeassistant/helpers/entity_registry.py b/homeassistant/helpers/entity_registry.py index 2a4e14ba0508f0..1651f7ba7d18f3 100644 --- a/homeassistant/helpers/entity_registry.py +++ b/homeassistant/helpers/entity_registry.py @@ -447,7 +447,7 @@ def async_device_modified(self, event: Event) -> None: self.async_update_entity(entity.entity_id, disabled_by=None) return - if device.disabled_by is dr.DeviceEntryDisabler.CONFIG_ENTRY: + if device.disabled_by == dr.DeviceEntryDisabler.CONFIG_ENTRY: # Handled by async_config_entry_disabled return diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index e9a1506e71d855..0b6aa977f3940c 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -218,7 +218,7 @@ async def test_loading_from_storage(hass, hass_storage): assert entry.name_by_user == "Test Friendly Name" assert entry.hw_version == "hw_version" assert entry.entry_type is device_registry.DeviceEntryType.SERVICE - assert entry.disabled_by is device_registry.DeviceEntryDisabler.USER + assert entry.disabled_by == device_registry.DeviceEntryDisabler.USER assert isinstance(entry.config_entries, set) assert isinstance(entry.connections, set) assert isinstance(entry.identifiers, set) @@ -913,7 +913,7 @@ async def test_update(registry): assert updated_entry.name_by_user == "Test Friendly Name" assert updated_entry.identifiers == new_identifiers assert updated_entry.via_device_id == "98765B" - assert updated_entry.disabled_by is device_registry.DeviceEntryDisabler.USER + assert updated_entry.disabled_by == device_registry.DeviceEntryDisabler.USER assert registry.async_get_device({("hue", "456")}) is None assert registry.async_get_device({("bla", "123")}) is None @@ -1508,10 +1508,10 @@ async def test_disable_config_entry_disables_devices(hass, registry): entry1 = registry.async_get(entry1.id) assert entry1.disabled - assert entry1.disabled_by is device_registry.DeviceEntryDisabler.CONFIG_ENTRY + assert entry1.disabled_by == device_registry.DeviceEntryDisabler.CONFIG_ENTRY entry2 = registry.async_get(entry2.id) assert entry2.disabled - assert entry2.disabled_by is device_registry.DeviceEntryDisabler.USER + assert entry2.disabled_by == device_registry.DeviceEntryDisabler.USER await hass.config_entries.async_set_disabled_by(config_entry.entry_id, None) await hass.async_block_till_done() @@ -1520,7 +1520,7 @@ async def test_disable_config_entry_disables_devices(hass, registry): assert not entry1.disabled entry2 = registry.async_get(entry2.id) assert entry2.disabled - assert entry2.disabled_by is device_registry.DeviceEntryDisabler.USER + assert entry2.disabled_by == device_registry.DeviceEntryDisabler.USER async def test_only_disable_device_if_all_config_entries_are_disabled(hass, registry): @@ -1556,7 +1556,7 @@ async def test_only_disable_device_if_all_config_entries_are_disabled(hass, regi entry1 = registry.async_get(entry1.id) assert entry1.disabled - assert entry1.disabled_by is device_registry.DeviceEntryDisabler.CONFIG_ENTRY + assert entry1.disabled_by == device_registry.DeviceEntryDisabler.CONFIG_ENTRY await hass.config_entries.async_set_disabled_by(config_entry1.entry_id, None) await hass.async_block_till_done() From 58361c6d3ae6ad5d5c7499817e742438cbefab97 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 11 Jan 2022 23:59:50 +0100 Subject: [PATCH 2/7] Fix string comparison issues with DeviceEntryType --- tests/components/deconz/test_gateway.py | 2 +- tests/components/picnic/test_sensor.py | 2 +- tests/helpers/test_device_registry.py | 2 +- tests/helpers/test_entity_platform.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/components/deconz/test_gateway.py b/tests/components/deconz/test_gateway.py index 30473814f26473..c0e8b4b1704c5c 100644 --- a/tests/components/deconz/test_gateway.py +++ b/tests/components/deconz/test_gateway.py @@ -176,7 +176,7 @@ async def test_gateway_setup(hass, aioclient_mock): ) assert gateway_entry.configuration_url == f"http://{HOST}:{PORT}" - assert gateway_entry.entry_type is dr.DeviceEntryType.SERVICE + assert gateway_entry.entry_type == dr.DeviceEntryType.SERVICE async def test_gateway_device_configuration_url_when_addon(hass, aioclient_mock): diff --git a/tests/components/picnic/test_sensor.py b/tests/components/picnic/test_sensor.py index a4a52e50453eb8..638f5c80873b9e 100644 --- a/tests/components/picnic/test_sensor.py +++ b/tests/components/picnic/test_sensor.py @@ -434,7 +434,7 @@ async def test_device_registry_entry(self): ) assert picnic_service.model == DEFAULT_USER_RESPONSE["user_id"] assert picnic_service.name == "Picnic: Commonstreet 123a" - assert picnic_service.entry_type is DeviceEntryType.SERVICE + assert picnic_service.entry_type == DeviceEntryType.SERVICE async def test_auth_token_is_saved_on_update(self): """Test that auth-token changes in the session object are reflected by the config entry.""" diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index 0b6aa977f3940c..b0c0a1ea7c3f21 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -217,7 +217,7 @@ async def test_loading_from_storage(hass, hass_storage): assert entry.area_id == "12345A" assert entry.name_by_user == "Test Friendly Name" assert entry.hw_version == "hw_version" - assert entry.entry_type is device_registry.DeviceEntryType.SERVICE + assert entry.entry_type == device_registry.DeviceEntryType.SERVICE assert entry.disabled_by == device_registry.DeviceEntryDisabler.USER assert isinstance(entry.config_entries, set) assert isinstance(entry.connections, set) diff --git a/tests/helpers/test_entity_platform.py b/tests/helpers/test_entity_platform.py index 9aa0a849e5a875..41c846fbeaf24a 100644 --- a/tests/helpers/test_entity_platform.py +++ b/tests/helpers/test_entity_platform.py @@ -864,7 +864,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): assert device.identifiers == {("hue", "1234")} assert device.configuration_url == "http://192.168.0.100/config" assert device.connections == {(dr.CONNECTION_NETWORK_MAC, "abcd")} - assert device.entry_type is dr.DeviceEntryType.SERVICE + assert device.entry_type == dr.DeviceEntryType.SERVICE assert device.manufacturer == "test-manuf" assert device.model == "test-model" assert device.name == "test-name" From 7c8a661db6c52e35a529837e69d83b172bf6728a Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 12 Jan 2022 01:09:28 +0100 Subject: [PATCH 3/7] Revert "Fix string comparison issues with DeviceEntryType" This reverts commit 58361c6d3ae6ad5d5c7499817e742438cbefab97. --- tests/components/deconz/test_gateway.py | 2 +- tests/components/picnic/test_sensor.py | 2 +- tests/helpers/test_device_registry.py | 2 +- tests/helpers/test_entity_platform.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/components/deconz/test_gateway.py b/tests/components/deconz/test_gateway.py index c0e8b4b1704c5c..30473814f26473 100644 --- a/tests/components/deconz/test_gateway.py +++ b/tests/components/deconz/test_gateway.py @@ -176,7 +176,7 @@ async def test_gateway_setup(hass, aioclient_mock): ) assert gateway_entry.configuration_url == f"http://{HOST}:{PORT}" - assert gateway_entry.entry_type == dr.DeviceEntryType.SERVICE + assert gateway_entry.entry_type is dr.DeviceEntryType.SERVICE async def test_gateway_device_configuration_url_when_addon(hass, aioclient_mock): diff --git a/tests/components/picnic/test_sensor.py b/tests/components/picnic/test_sensor.py index 638f5c80873b9e..a4a52e50453eb8 100644 --- a/tests/components/picnic/test_sensor.py +++ b/tests/components/picnic/test_sensor.py @@ -434,7 +434,7 @@ async def test_device_registry_entry(self): ) assert picnic_service.model == DEFAULT_USER_RESPONSE["user_id"] assert picnic_service.name == "Picnic: Commonstreet 123a" - assert picnic_service.entry_type == DeviceEntryType.SERVICE + assert picnic_service.entry_type is DeviceEntryType.SERVICE async def test_auth_token_is_saved_on_update(self): """Test that auth-token changes in the session object are reflected by the config entry.""" diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index b0c0a1ea7c3f21..0b6aa977f3940c 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -217,7 +217,7 @@ async def test_loading_from_storage(hass, hass_storage): assert entry.area_id == "12345A" assert entry.name_by_user == "Test Friendly Name" assert entry.hw_version == "hw_version" - assert entry.entry_type == device_registry.DeviceEntryType.SERVICE + assert entry.entry_type is device_registry.DeviceEntryType.SERVICE assert entry.disabled_by == device_registry.DeviceEntryDisabler.USER assert isinstance(entry.config_entries, set) assert isinstance(entry.connections, set) diff --git a/tests/helpers/test_entity_platform.py b/tests/helpers/test_entity_platform.py index 41c846fbeaf24a..9aa0a849e5a875 100644 --- a/tests/helpers/test_entity_platform.py +++ b/tests/helpers/test_entity_platform.py @@ -864,7 +864,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): assert device.identifiers == {("hue", "1234")} assert device.configuration_url == "http://192.168.0.100/config" assert device.connections == {(dr.CONNECTION_NETWORK_MAC, "abcd")} - assert device.entry_type == dr.DeviceEntryType.SERVICE + assert device.entry_type is dr.DeviceEntryType.SERVICE assert device.manufacturer == "test-manuf" assert device.model == "test-model" assert device.name == "test-name" From f142b0bf71bcddbd7b2fe6f1076e7af3d49e6c14 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 12 Jan 2022 01:09:31 +0100 Subject: [PATCH 4/7] Revert "Fix string comparison issues for device disabled_by attribute" This reverts commit 02adda88d18cb4249fd51e76aa487daaaf1c8fb3. --- homeassistant/helpers/device_registry.py | 2 +- homeassistant/helpers/entity_registry.py | 2 +- tests/helpers/test_device_registry.py | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/homeassistant/helpers/device_registry.py b/homeassistant/helpers/device_registry.py index d84c55c51eeee2..6bec4e592e63a3 100644 --- a/homeassistant/helpers/device_registry.py +++ b/homeassistant/helpers/device_registry.py @@ -732,7 +732,7 @@ def async_config_entry_disabled_by_changed( if not config_entry.disabled_by: for device in devices: - if device.disabled_by != DeviceEntryDisabler.CONFIG_ENTRY: + if device.disabled_by is not DeviceEntryDisabler.CONFIG_ENTRY: continue registry.async_update_device(device.id, disabled_by=None) return diff --git a/homeassistant/helpers/entity_registry.py b/homeassistant/helpers/entity_registry.py index 1651f7ba7d18f3..2a4e14ba0508f0 100644 --- a/homeassistant/helpers/entity_registry.py +++ b/homeassistant/helpers/entity_registry.py @@ -447,7 +447,7 @@ def async_device_modified(self, event: Event) -> None: self.async_update_entity(entity.entity_id, disabled_by=None) return - if device.disabled_by == dr.DeviceEntryDisabler.CONFIG_ENTRY: + if device.disabled_by is dr.DeviceEntryDisabler.CONFIG_ENTRY: # Handled by async_config_entry_disabled return diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index 0b6aa977f3940c..e9a1506e71d855 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -218,7 +218,7 @@ async def test_loading_from_storage(hass, hass_storage): assert entry.name_by_user == "Test Friendly Name" assert entry.hw_version == "hw_version" assert entry.entry_type is device_registry.DeviceEntryType.SERVICE - assert entry.disabled_by == device_registry.DeviceEntryDisabler.USER + assert entry.disabled_by is device_registry.DeviceEntryDisabler.USER assert isinstance(entry.config_entries, set) assert isinstance(entry.connections, set) assert isinstance(entry.identifiers, set) @@ -913,7 +913,7 @@ async def test_update(registry): assert updated_entry.name_by_user == "Test Friendly Name" assert updated_entry.identifiers == new_identifiers assert updated_entry.via_device_id == "98765B" - assert updated_entry.disabled_by == device_registry.DeviceEntryDisabler.USER + assert updated_entry.disabled_by is device_registry.DeviceEntryDisabler.USER assert registry.async_get_device({("hue", "456")}) is None assert registry.async_get_device({("bla", "123")}) is None @@ -1508,10 +1508,10 @@ async def test_disable_config_entry_disables_devices(hass, registry): entry1 = registry.async_get(entry1.id) assert entry1.disabled - assert entry1.disabled_by == device_registry.DeviceEntryDisabler.CONFIG_ENTRY + assert entry1.disabled_by is device_registry.DeviceEntryDisabler.CONFIG_ENTRY entry2 = registry.async_get(entry2.id) assert entry2.disabled - assert entry2.disabled_by == device_registry.DeviceEntryDisabler.USER + assert entry2.disabled_by is device_registry.DeviceEntryDisabler.USER await hass.config_entries.async_set_disabled_by(config_entry.entry_id, None) await hass.async_block_till_done() @@ -1520,7 +1520,7 @@ async def test_disable_config_entry_disables_devices(hass, registry): assert not entry1.disabled entry2 = registry.async_get(entry2.id) assert entry2.disabled - assert entry2.disabled_by == device_registry.DeviceEntryDisabler.USER + assert entry2.disabled_by is device_registry.DeviceEntryDisabler.USER async def test_only_disable_device_if_all_config_entries_are_disabled(hass, registry): @@ -1556,7 +1556,7 @@ async def test_only_disable_device_if_all_config_entries_are_disabled(hass, regi entry1 = registry.async_get(entry1.id) assert entry1.disabled - assert entry1.disabled_by == device_registry.DeviceEntryDisabler.CONFIG_ENTRY + assert entry1.disabled_by is device_registry.DeviceEntryDisabler.CONFIG_ENTRY await hass.config_entries.async_set_disabled_by(config_entry1.entry_id, None) await hass.async_block_till_done() From ae1fc537d4aeaea4f3b10a60c459a3ff6d96b494 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 12 Jan 2022 01:15:08 +0100 Subject: [PATCH 5/7] Convert disabled_by to DeviceEntryDisabler on load --- homeassistant/helpers/device_registry.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/homeassistant/helpers/device_registry.py b/homeassistant/helpers/device_registry.py index 6bec4e592e63a3..96425b2ea93bc7 100644 --- a/homeassistant/helpers/device_registry.py +++ b/homeassistant/helpers/device_registry.py @@ -553,7 +553,9 @@ async def async_load(self) -> None: configuration_url=device["configuration_url"], # type ignores (if tuple arg was cast): likely https://github.com/python/mypy/issues/8625 connections={tuple(conn) for conn in device["connections"]}, # type: ignore[misc] - disabled_by=device["disabled_by"], + disabled_by=DeviceEntryDisabler(device["disabled_by"]) + if device["disabled_by"] + else None, entry_type=DeviceEntryType(device["entry_type"]) if device["entry_type"] else None, From 9b1862c7597b2a23e6b3106b1a21db496dd3ba60 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 11 Jan 2022 16:34:43 -0800 Subject: [PATCH 6/7] Add test --- tests/helpers/test_device_registry.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index e9a1506e71d855..521b1f03bf0a7e 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -782,6 +782,7 @@ async def test_loading_saving_data(hass, registry, area_registry): identifiers={("hue", "abc")}, manufacturer="manufacturer", model="light", + entry_type=device_registry.DeviceEntryType.SERVICE, ) assert orig_light4.id == orig_light3.id @@ -816,11 +817,21 @@ async def test_loading_saving_data(hass, registry, area_registry): new_via = registry2.async_get_device({("hue", "0123")}) new_light = registry2.async_get_device({("hue", "456")}) new_light4 = registry2.async_get_device({("hue", "abc")}) + new_light4 = registry2.async_get_device({("hue", "abc")}) assert orig_via == new_via assert orig_light == new_light assert orig_light4 == new_light4 + # Ensure enums converted + for (old, new) in ( + (orig_via, new_via), + (orig_light, new_light), + (orig_light4, new_light4), + ): + assert old.disabled_by is new.disabled_by + assert old.entry_type is new.entry_type + # Ensure a save/load cycle does not keep suggested area new_kitchen_light = registry2.async_get_device({("hue", "999")}) assert orig_kitchen_light.suggested_area == "Kitchen" From 70d06e9691ee73159ff41eb03da9f962968764ff Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 11 Jan 2022 16:35:20 -0800 Subject: [PATCH 7/7] Double assignment --- tests/helpers/test_device_registry.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/helpers/test_device_registry.py b/tests/helpers/test_device_registry.py index 521b1f03bf0a7e..a0949bad03ce01 100644 --- a/tests/helpers/test_device_registry.py +++ b/tests/helpers/test_device_registry.py @@ -817,7 +817,6 @@ async def test_loading_saving_data(hass, registry, area_registry): new_via = registry2.async_get_device({("hue", "0123")}) new_light = registry2.async_get_device({("hue", "456")}) new_light4 = registry2.async_get_device({("hue", "abc")}) - new_light4 = registry2.async_get_device({("hue", "abc")}) assert orig_via == new_via assert orig_light == new_light