Device Registry #15980

Kane610 · 2018-08-14T20:33:33Z

Description:

Device registry based on home-assistant/architecture#36

Related issue (if applicable): fixes #

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>

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code does not interact with devices:

Tests have been added to verify that the new code works.

houndci-bot · 2018-08-14T20:33:40Z

homeassistant/helpers/entity_platform.py

        registry = await async_get_registry(hass)

        tasks = [
            self._async_add_entity(entity, update_before_add,
-                                   component_entities, registry)
+                                   component_entities, registry, device_registry)


line too long (81 > 79 characters)

Let's rename registry -> entity_registry so we keep our sanity :)

houndci-bot · 2018-08-14T20:33:41Z

homeassistant/helpers/entity_platform.py

@@ -209,11 +210,13 @@ def add_entities(self, new_entities, update_before_add=False):
        hass = self.hass
        component_entities = set(hass.states.async_entity_ids(self.domain))

+
+        device_registry = async_get_device_registry(self.hass)


too many blank lines (2)

awarecan · 2018-08-15T06:16:34Z

How can I get all entities of one device?

I feel that a Demo device included demo light, demo switch, demo sensor will better than use deconz as demo. And also easy to write unit test for demo device.

balloob · 2018-08-15T08:15:38Z

homeassistant/helpers/device_registry.py

+class DeviceEntry:
+    """Device Registry Entry."""
+
+    unique_id = attr.ib(type=str)


Let's name this id and use a factory to automatically generate it

id = attr.ib(type=str, default=attr.Factory(lambda: uuid.uuid4().hex))

Nevermind, read up on unique_id. Drop this comment.

So after reading everything, let's circle back here:

We should add the id property to DeviceEntry, it will be how we can point at this device from the entity registry.

For unique_id and serial, I would expect us to have a tuple of identifiers. Like connection, but with just identifying things:

identifiers = ( ('serial', 'abcdefgh'), # if we can read the device serial ('nest', 'czxcz'), # if the developer wants to pass an identifier. They can use their domain to scope it. )

Then we would only do identification on identifiers, or connections are still viable due to discovery?

balloob · 2018-08-15T08:16:45Z

homeassistant/helpers/device_registry.py

+    serial = attr.ib(type=str)
+    manufacturer = attr.ib(type=str)
+    model = attr.ib(type=str)
+    connection = attr.ib(type=tuple)


I think the idea was for this to be a tuple of tuples. A device can have multiple connections. For example a Raspberry Pi has WiFi and LAN connection.

Should we use some kind of standardisation for the types?

CONNECTION_ZIGBEE = 'zigbee' CONNECTION_MAC = 'mac'

I guess we can also have duplicates (wifi + lan = 2 macs ?)

I wonder how often you use both wifi and lan for a device that is connected to hass

balloob · 2018-08-15T08:18:29Z

homeassistant/helpers/device_registry.py

+    model = attr.ib(type=str)
+    connection = attr.ib(type=tuple)
+    sw_version = attr.ib(type=str, default=None)
+    config_entry_id = attr.ib(type=set, default=attr.Factory(set))


I realize now that this is not the right place.

Instead, this will be linked via the Entity Registry which will contain a reference to the device registry and to the config entry.

MartinHjelmare · 2018-08-15T08:20:44Z

homeassistant/components/binary_sensor/deconz.py

+            serial = self._sensor.uniqueid.split('-', 1)[0]
+        else:
+            return None
+        dev = {


Do we want this to be a loosely defined dictionary?

As opposed to using get_create_device in each entity.device? I started in this way, not set in stone

We could return an instance of a class for this purpose, similar to the DeviceEntry class.

we could use async_get_create_device directly, then entity_platform would only do the coupling between entity_registry and device_registry.

balloob · 2018-08-15T08:25:04Z

homeassistant/helpers/device_registry.py

+    def async_get_device(self, serial: str, connection: tuple):
+        """Check if device is registered."""
+        for device in self.devices.values():
+            if (device.serial == serial or


Now we're treating serial as unique ID?

Following the discussion up top originally; first it was unique id, it was then moved to serial. Regardless, gonna redo it all to your suggested identifier tuple

balloob · 2018-08-15T08:26:07Z

homeassistant/helpers/entity_platform.py

@@ -209,11 +210,13 @@ def add_entities(self, new_entities, update_before_add=False):
        hass = self.hass
        component_entities = set(hass.states.async_entity_ids(self.domain))

+
+        device_registry = async_get_device_registry(self.hass)


Instead of this, you could, albeit more lines, type:

device_registry = hass.helpers.device_registry.async_get_registry()

Should I do the same for entity registry?

balloob · 2018-08-15T08:27:25Z

homeassistant/helpers/entity_platform.py

@@ -269,10 +272,22 @@ def add_entities(self, new_entities, update_before_add=False):
            else:
                config_entry_id = None

+            if entity.device is not None:


entity.device will actually execute code so better to just request it once instead of 8 times.

device = entity.device if device is not None:

balloob · 2018-08-15T08:35:08Z

homeassistant/helpers/entity_platform.py

+                    entity.device['connection'],
+                    sw_version=entity.device.get('sw_version'),
+                    config_entry_id=config_entry_id)
+                device_id = device.unique_id


Should be device.id, we need to generate our own IDs for devices and not trust developers.

balloob · 2018-08-15T08:39:03Z

This is off to a great start !

Left some comments. Other suggestions:

split out Deconz from the PR.
Add Storage helper for persistence
Add tests

Kane610 · 2018-08-15T11:38:33Z

Thanks for the feedback! Everything but tests I should be able to get fixed tonight.

Kane610 · 2018-08-15T17:04:21Z

@balloob if we do persistent storage, should I cover removal of entities -> devices with the initial implementation?

houndci-bot · 2018-08-15T20:28:30Z

homeassistant/helpers/entity_platform.py

+        device_registry = (await \
+            hass.helpers.device_registry.async_get_registry())
+        entity_registry = (await \
+            hass.helpers.entity_registry.async_get_registry())


continuation line under-indented for visual indent

houndci-bot · 2018-08-15T20:28:30Z

homeassistant/helpers/entity_platform.py

-        registry = await async_get_registry(hass)
+        device_registry = (await \
+            hass.helpers.device_registry.async_get_registry())
+        entity_registry = (await \


the backslash is redundant between brackets

houndci-bot · 2018-08-15T20:28:30Z

homeassistant/helpers/entity_platform.py

@@ -209,11 +208,15 @@ def add_entities(self, new_entities, update_before_add=False):
        hass = self.hass
        component_entities = set(hass.states.async_entity_ids(self.domain))

-        registry = await async_get_registry(hass)
+        device_registry = (await \
+            hass.helpers.device_registry.async_get_registry())


continuation line under-indented for visual indent

houndci-bot · 2018-08-15T20:28:30Z

homeassistant/helpers/entity_platform.py

@@ -209,11 +208,15 @@ def add_entities(self, new_entities, update_before_add=False):
        hass = self.hass
        component_entities = set(hass.states.async_entity_ids(self.domain))

-        registry = await async_get_registry(hass)
+        device_registry = (await \


the backslash is redundant between brackets

houndci-bot · 2018-08-15T20:28:30Z

homeassistant/helpers/device_registry.py

+        return None
+
+    async def async_get_or_create(self, identifiers, manufacturer, model,
+                            connection, *, sw_version=None):


continuation line under-indented for visual indent

houndci-bot · 2018-08-15T20:28:31Z

homeassistant/components/deconz/__init__.py

@@ -12,6 +12,8 @@
    CONF_ID, CONF_PORT, EVENT_HOMEASSISTANT_STOP)
 from homeassistant.core import EventOrigin, callback
 from homeassistant.helpers import aiohttp_client, config_validation as cv
+from homeassistant.helpers.device_registry import (


'homeassistant.helpers.device_registry.async_get_registry as async_get_device_registry' imported but unused

balloob · 2018-08-16T08:17:58Z

Let's not cover removal, we do that in a future PR.

balloob · 2018-08-17T12:57:44Z

For inspiration on how to handle storage, see #16018

houndci-bot · 2018-08-19T15:29:57Z

homeassistant/helpers/entity_registry.py

@@ -210,6 +218,7 @@ def _async_update_entity(self, entity_id, *, name=_UNDEF,
                entities[entity['entity_id']] = RegistryEntry(
                    entity_id=entity['entity_id'],
                    config_entry_id=entity.get('config_entry_id'),
+                    device_id=info.get('device_id'),


undefined name 'info'

syssi

Cool!

houndci-bot · 2018-08-22T04:19:56Z

homeassistant/helpers/device_registry.py

+            if any(identifier in device.identifiers
+                   for identifier in identifiers) or \
+               any(connection in device.connection
+                       for connection in connections):


continuation line over-indented for visual indent

Kane610 · 2018-08-22T09:14:37Z

🎉

MartinHjelmare · 2018-08-22T11:55:10Z

Now we need to update the developer docs, so we can show developers how to use this.

Kane610 requested a review from a team as a code owner August 14, 2018 20:33

homeassistant added the cla-signed label Aug 14, 2018

ghost assigned Kane610 Aug 14, 2018

ghost added the in progress label Aug 14, 2018

houndci-bot reviewed Aug 14, 2018

View reviewed changes

balloob reviewed Aug 15, 2018

View reviewed changes

MartinHjelmare reviewed Aug 15, 2018

View reviewed changes

balloob reviewed Aug 15, 2018

View reviewed changes

homeassistant added the cla-signed label Aug 15, 2018

houndci-bot reviewed Aug 15, 2018

View reviewed changes

Kane610 added 5 commits August 19, 2018 17:29

First draft

4d4b7c2

Generate device id

7215e74

No obscure registry

2403769

Dont store config_entry_id in device

90c1bc5

Storage

59f5787

Kane610 force-pushed the device-concept branch from 19a6e30 to 59f5787 Compare August 19, 2018 15:29

homeassistant added the cla-signed label Aug 19, 2018

houndci-bot reviewed Aug 19, 2018

View reviewed changes

Small mistake on rebase

08a2504

Kane610 added 2 commits August 21, 2018 22:35

Fix Knx to not use self.device

eff02a5

Clean up a small mistake in soundtouch

552e5c2

Kane610 force-pushed the device-concept branch from 35d0f8e to 552e5c2 Compare August 21, 2018 20:41

Kane610 requested review from kellerza, molobrakos, rytilahti and syssi as code owners August 21, 2018 20:41

homeassistant added the cla-signed label Aug 21, 2018

Fix comment from Ballob

cb842b8

Kane610 changed the title ~~WIP: Device Registry~~ Device Registry Aug 21, 2018

syssi approved these changes Aug 21, 2018

View reviewed changes

Fix bad indentation

e6d588c

homeassistant added the cla-signed label Aug 21, 2018

Kane610 mentioned this pull request Aug 21, 2018

deCONZ - Support device registry #16115

Merged

4 tasks

Fix indentatin

b6e13f4

houndci-bot reviewed Aug 22, 2018

View reviewed changes

balloob and others added 3 commits August 22, 2018 09:30

Lint

b493a81

Remove unused variable

575f649

Lint

c3dbb0f

balloob merged commit 0009be5 into home-assistant:dev Aug 22, 2018

ghost removed the in progress label Aug 22, 2018

Kane610 deleted the device-concept branch August 23, 2018 21:41

bartvm mentioned this pull request Aug 25, 2018

Ecovacs authentification and device registry issues #16186

Closed

This was referenced Aug 27, 2018

Fix device telldus #16224

Merged

0.77 #16256

Merged

amelchio mentioned this pull request Aug 30, 2018

Fix LIFX effects #16309

Merged

6 tasks

home-assistant locked and limited conversation to collaborators Dec 10, 2018

Device Registry #15980

Device Registry #15980

Conversation

Kane610 commented Aug 14, 2018 • edited Loading

Description:

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awarecan commented Aug 15, 2018

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob commented Aug 15, 2018

Kane610 commented Aug 15, 2018

Kane610 commented Aug 15, 2018

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 Aug 16, 2018

balloob commented Aug 17, 2018

Choose a reason for hiding this comment

syssi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kane610 commented Aug 22, 2018

MartinHjelmare commented Aug 22, 2018

Kane610 commented Aug 14, 2018 •

edited

Loading