Entity registry #11979

balloob · 2018-01-28T00:49:18Z

Description:

Initial implementation of an entity registry:

Ensures that objects with a unique ID will be given the same entity id each time.
Allows users to update the entity registry file to rename entity ids (requires restart for now)
Entity Component will enforce that registered entity IDs are not given to other entities
Entity registry is written to disk 10 seconds after a new entry is registered. Registering another entry will reset that timer.

To do:

tests
check all existing unique ID implementations are fine
Entities that have no unique ID will never be given an entity ID that has previously been registered.

Breaking change:
It's no longer supported to add the same entity twice to Home Assistant. In the past Home Assistant would filter it out based on unique id, now it's up to the platform to make sure this doesn't happen.

Related issue (if applicable): fixes #11533

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for `configuration.yaml` (if applicable):

Checklist:

The code change is tested and works locally.

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

balloob · 2018-01-28T00:50:26Z

homeassistant/components/group/__init__.py

@@ -381,13 +380,9 @@ def _async_process_config(hass, config, component):

        # Don't create tasks and await them all. The order is important as
        # groups get a number based on creation order.
-        group = yield from Group.async_create_group(
+        yield from Group.async_create_group(


async_create_group already adds the group. Old code was adding each group twice.

MartinHjelmare · 2018-01-28T01:53:47Z

Looks good so far 👍.

I'm wondering about the relative imports. Is that now preferred, or do we not care about mixing absolute and relative imports?

balloob · 2018-01-28T02:09:38Z

I... don't know about relative imports

MartinHjelmare · 2018-01-28T02:19:49Z

homeassistant/helpers/entity_component.py

@@ -19,11 +19,13 @@
 from homeassistant.util.async import (
    run_callback_threadsafe, run_coroutine_threadsafe)
 import homeassistant.util.dt as dt_util
+from .entity_registry import EntityRegistry


This is a relative import.

Sorry, I meant I don't know if I like them more

Ok, then I'll employ a lax attitude until someone says otherwise, if it shows up in another PR. 🍹

andrey-git · 2018-01-28T08:58:24Z

Why did you removed unique_id implementations in some cases and left it in others?

andrey-git · 2018-01-28T08:51:35Z

homeassistant/helpers/entity_registry.py

+    @callback
+    def async_get_or_create(self, domain, platform, unique_id, *,
+                            suggested_object_id=None):
+        """Get entity. Creat if it doesn't exist."""


Creat -> Create

houndci-bot · 2018-01-28T09:54:58Z

tests/helpers/test_entity_component.py

@@ -14,15 +14,15 @@
 from homeassistant.helpers.entity import Entity, generate_entity_id
 from homeassistant.helpers.entity_component import (
    EntityComponent, DEFAULT_SCAN_INTERVAL, SLOW_SETUP_WARNING)
-from homeassistant.helpers import entity_component
+from homeassistant.helpers import entity_component, entity_registry


'homeassistant.helpers.entity_registry' imported but unused

balloob · 2018-01-28T09:57:42Z

@andrey-git I have removed several implementations of unique ID that were not unique. For example, they would depend on the name of a device.

Unique ID has to be unique no matter the configuration. Serial numbers, device IDs, Mac addresses and BT addresses are great unique IDs. IP address is not perfect and I was in doubt if they should stay as part of unique id.

balloob · 2018-01-28T10:05:44Z

I've done one more review and restored some unique ID implementations that I removed earlier.

rytilahti

Added a couple of comments, but I have a couple of questions:

IP addresses should not be used as unique IDs in general, considering how common address renewals can be?
Does or should this play somehow together with discovered devices? Most of the time you either get a mac address (id or mac on my devices) over mdns, or an uuid (udn is part of the upnp standard). Should autodetected components be adapted accordingly to use this information?

rytilahti · 2018-01-28T18:27:16Z

homeassistant/components/climate/daikin.py

@@ -180,7 +180,7 @@ def set(self, settings):
    @property
    def unique_id(self):
        """Return the ID of this AC."""
-        return "{}.{}".format(self.__class__, self._api.ip_address)
+        return self._api.ip_address


Shouldn't this be something else than the IP address?

rytilahti · 2018-01-28T18:28:21Z

homeassistant/components/light/flux_led.py

@@ -170,7 +170,7 @@ def available(self) -> bool:
    @property
    def unique_id(self):
        """Return the ID of this light."""
-        return '{}.{}'.format(self.__class__, self._ipaddr)
+        return self._ipaddr


IP address also here.

rytilahti · 2018-01-28T18:28:51Z

homeassistant/components/light/tikteck.py

@@ -70,7 +70,7 @@ def __init__(self, device):
    @property
    def unique_id(self):
        """Return the ID of this light."""
-        return "{}.{}".format(self.__class__, self._address)
+        return self._address


This probably also should be removed?

This is a bluetooth address.

rytilahti · 2018-01-28T18:36:43Z

homeassistant/components/light/yeelight.py

@@ -178,7 +178,7 @@ def effect_list(self):
    @property
    def unique_id(self) -> str:
        """Return the ID of this light."""
-        return "{}.{}".format(self.__class__, self._ipaddr)
+        return self._ipaddr


IP address. These devices would have two ways to obtain a unique identifier (either by using miio's proprietary api, a bit no-go as the current component uses a different one) or via mdns (mac field).

Yeah I agree, IP address is not a good thing to see as uniqueness. Will remove them.

rytilahti · 2018-01-28T18:39:59Z

homeassistant/helpers/entity_registry.py

+PATH_REGISTRY = 'entity_registry.yaml'
+SAVE_DELAY = 10
+Entry = namedtuple('EntityRegistryEntry',
+                   'entity_id,unique_id,platform,domain')


Would it be worth using attrs instead of namedtuples in general, see http://www.attrs.org/en/stable/why.html#namedtuples

I don't want to add another dependency. If anything, we would use a backport of the data classes that are being added to Python 3.7.

For the current use case namedtuples will be enough.

balloob · 2018-01-28T19:06:47Z

@rytilahti discovered entities should use the same source for unique ID as configured entities. I have another proposal to better deal with discovered entities: #11947

pvizeli

I like it.

For homematic, we have a unique ID, but they is unique for the device a not for the entity. I.e. a switch with two channel create 2 switch entity but they have the same device ID. what is the idea of that to solve it? Actual we generate unique entity_id but only copy this is not the nice way. We discovery also all this device.

balloob · 2018-01-29T20:48:51Z

If a device has a unique ID and 2 channels, you can make a unique id by combining the unique ID and the channel id.

Kane610 · 2018-01-29T20:57:58Z

Would it be possible to add functionality to query the registry to do a lookup of entity_id or unique_id?

balloob · 2018-01-29T21:03:19Z

There is the get_or_create function and there is an is_registered function.

balloob · 2018-01-30T00:22:06Z

So I see a lot of reviews and "I like it" statements. Is it ok to merge?

andrey-git · 2018-01-30T07:34:53Z

I think it is important that eventually IDs can be changed without restarting.
If the current Entity Registry design doesn't hinder that - I'm ok with merging.

balloob · 2018-01-30T07:58:54Z

That will be possible with the current design. It's something I want to be allow people to do via the UI.

Rough Implementation:

Entity Platform will register with EntityRegistry as a rename listener
Add method EntityRegistry#async_rename_entity_id

Entity Platform gets notified:

hass.states.async_remove(old_entity_id)
entity.entity_id = new_entity_id
yield from entity.async_update_ha_state()

pvizeli · 2018-01-30T10:44:37Z

I will implement unique_id as entity_id on homematic

balloob · 2018-01-30T17:58:32Z

You don't have to set entity_id. When registering an entity id, the registry will use the available device name or a combination of platform and unique id.

pvizeli · 2018-01-31T08:02:05Z

Yes, I don't set it, I set only the name :)

balloob requested a review from a team as a code owner January 28, 2018 00:49

homeassistant added cla-signed integration: group core platform: climate.nest labels Jan 28, 2018

balloob commented Jan 28, 2018

View reviewed changes

MartinHjelmare reviewed Jan 28, 2018

View reviewed changes

balloob requested review from rytilahti and a team as code owners January 28, 2018 08:17

homeassistant added the cla-signed label Jan 28, 2018

andrey-git reviewed Jan 28, 2018

View reviewed changes

houndci-bot reviewed Jan 28, 2018

View reviewed changes

homeassistant added the cla-signed label Jan 28, 2018

rytilahti mentioned this pull request Jan 28, 2018

Add manufacturer to SSDP discoverable data home-assistant-libs/netdisco#165

Merged

rytilahti reviewed Jan 28, 2018

View reviewed changes

homeassistant added the cla-signed label Jan 28, 2018

balloob force-pushed the entity-registry branch from ccba3de to 49799c6 Compare January 29, 2018 00:59

homeassistant added the cla-signed label Jan 29, 2018

balloob added 6 commits January 28, 2018 17:14

Entity#unique_id defaults to None

6a77c89

Initial commit entity registry

c330c96

Clean up unique_id property

43da9f2

Lint

2cddce9

Add tests to entity component

cd8f6ae

Lint

383011b

Fix tests

1369ea4

balloob force-pushed the entity-registry branch from 49799c6 to 1369ea4 Compare January 29, 2018 01:14

homeassistant added the cla-signed label Jan 29, 2018

balloob added 3 commits January 28, 2018 20:35

Add some docs

0d8b4ee

Add one more test

7275247

Fix new test…

a7e0bfd

pvizeli reviewed Jan 29, 2018

View reviewed changes

balloob mentioned this pull request Jan 30, 2018

RFC: Entity Registry #11533

Closed

pvizeli approved these changes Jan 30, 2018

View reviewed changes

pvizeli merged commit e51427b into dev Jan 30, 2018

pvizeli deleted the entity-registry branch January 30, 2018 10:44

molobrakos mentioned this pull request Jan 30, 2018

Squeezebox Fix duplicate server from discovery #12063

Merged

2 tasks

amelchio mentioned this pull request Jan 30, 2018

Some of the ZWave entities not showing up in 0.62 #11997

Closed

covrig mentioned this pull request Jan 31, 2018

Aeotec DSC11 Smart Strip doesn't show all switches #12098

Closed

This was referenced Feb 7, 2018

zha: Allow custom entity names #12217

Closed

0.63 #12267

Merged

arsaboo mentioned this pull request Feb 11, 2018

homeassistant.exceptions.HomeAssistantError: Entity id already exists error in 0.63 #12287

Closed

OttoWinter mentioned this pull request Feb 25, 2018

Hue light groups #11920

Closed

home-assistant locked and limited conversation to collaborators May 29, 2018

ghost added integration: nest and removed platform: climate.nest labels Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Entity registry #11979

Entity registry #11979

balloob commented Jan 28, 2018 •

edited

Loading

balloob Jan 28, 2018

MartinHjelmare commented Jan 28, 2018

balloob commented Jan 28, 2018

MartinHjelmare Jan 28, 2018

balloob Jan 28, 2018

MartinHjelmare Jan 28, 2018

andrey-git commented Jan 28, 2018

andrey-git Jan 28, 2018

houndci-bot Jan 28, 2018

balloob commented Jan 28, 2018

balloob commented Jan 28, 2018

rytilahti left a comment

rytilahti Jan 28, 2018

rytilahti Jan 28, 2018

rytilahti Jan 28, 2018

balloob Jan 28, 2018

rytilahti Jan 28, 2018

balloob Jan 28, 2018

rytilahti Jan 28, 2018

balloob Jan 28, 2018

balloob commented Jan 28, 2018 •

edited

Loading

pvizeli left a comment

balloob commented Jan 29, 2018

Kane610 commented Jan 29, 2018

balloob commented Jan 29, 2018

balloob commented Jan 30, 2018

andrey-git commented Jan 30, 2018

balloob commented Jan 30, 2018

pvizeli commented Jan 30, 2018

balloob commented Jan 30, 2018

pvizeli commented Jan 31, 2018

Entity registry #11979

Entity registry #11979

Conversation

balloob commented Jan 28, 2018 • edited Loading

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

MartinHjelmare commented Jan 28, 2018

balloob commented Jan 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-git commented Jan 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob commented Jan 28, 2018

balloob commented Jan 28, 2018

rytilahti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob commented Jan 28, 2018 • edited Loading

pvizeli left a comment

Choose a reason for hiding this comment

balloob commented Jan 29, 2018

Kane610 commented Jan 29, 2018

balloob commented Jan 29, 2018

balloob commented Jan 30, 2018

andrey-git commented Jan 30, 2018

balloob commented Jan 30, 2018

pvizeli commented Jan 30, 2018

balloob commented Jan 30, 2018

pvizeli commented Jan 31, 2018

balloob commented Jan 28, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

balloob commented Jan 28, 2018 •

edited

Loading