Improve service by allowing to reference entity id instead of deconz id #11862

Kane610 · 2018-01-22T21:26:03Z

Description:

It is cumbersome to get ahold of the deconz_id, this allows the user to specify the entity_id instead.

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4494

Checklist:

The code change is tested and works locally.

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

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

balloob

It's not clear to me why this is necessary to begin with?

balloob · 2018-01-23T08:10:49Z

homeassistant/components/deconz/__init__.py

-        yield from deconz.async_put_state(field, data)
+        entity_id = call.data.get(SERVICE_ENTITY)
+        data_string = call.data.get(SERVICE_DATA)
+        data = ast.literal_eval(data_string)  # String to dict


Eval is not allowed, too dangerous.

Do you have suggestion about how I can get a dictionary here?

The Issue here is that data is expected to be json compliant, which it unfortunately isn't since it get single quotes, where as json.loads expects double quotes. So there is a translation going on from the service entry in the gui to the service in the backend; {"name": "TRADFRI motion sensor 1"} arrives as {'name': 'TRADFRI motion sensor 1'}

balloob · 2018-01-23T08:11:48Z

homeassistant/components/binary_sensor/deconz.py

@@ -29,6 +30,7 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
        if sensor and sensor.type in DECONZ_BINARY_SENSOR:
            entities.append(DeconzBinarySensor(sensor))
    async_add_devices(entities, True)
+    hass.data[DECONZ_ENTITIES] = hass.data[DECONZ_ENTITIES] + entities


This is not allowed. Main component should not have access to the entity objects.

I guess I could just do a translation dict between entity name and deconz id instead

Kane610 · 2018-01-23T08:44:43Z

The real issue is that there is no easy way for the user to know the deconZ_id. I could add it as an attribute to each entity or do you have an alternative to how I can do this? The important thing is to help the user being able to use the service with all devices in deconz.

Kane610 · 2018-01-24T16:56:31Z

@balloob I'd be happy to implement according to your wishes or redesign when your entity registry has been delivered, can we move forward with this?

houndci-bot · 2018-01-31T20:57:00Z

homeassistant/components/sensor/deconz.py

@@ -37,6 +38,7 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
                    entities.append(DeconzBattery(sensor))
            else:
                entities.append(DeconzSensor(sensor))
+                #entities.append(DeconzSensor(sensor, entity_registry))


block comment should start with '# '

houndci-bot · 2018-01-31T20:57:00Z

homeassistant/components/sensor/deconz.py

@@ -26,6 +26,7 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None):

    from pydeconz.sensor import DECONZ_SENSOR, SWITCH as DECONZ_REMOTE
    sensors = hass.data[DECONZ_DATA].sensors
+    #entity_registry = hass.data[DECONZ_ENTITIES]


block comment should start with '# '

balloob · 2018-02-02T00:29:29Z

homeassistant/components/deconz/__init__.py

+            groups = list(deconz.groups.values())
+            lights = list(deconz.lights.values())
+            sensors = list(deconz.sensors.values())
+            devices = groups + lights + sensors


I think what you're looking for is chain, that way you don't have to create a list but can just iterate over it.

from itertools import chain for device in chain(deconz.groups.values(), deconz.lights.values(), …):

balloob · 2018-02-02T00:30:19Z

homeassistant/components/deconz/__init__.py

+        deconz = hass.data[DOMAIN]
+
+        registry = hass.data.get(DATA_REGISTRY)
+        if registry.async_is_registered(entity_id):


Not needed here, just get the entry. Do make sure to use guard clause pattern:

entry = registry.async_get_entry(…) if entry is None: _LOGGER.error(…) return

houndci-bot · 2018-02-02T17:34:27Z

homeassistant/components/deconz/__init__.py

+            registry = hass.data.get(DATA_REGISTRY)
+            entity = registry.async_get_entry(entity_id)
+            for device in chain(
+                    deconz.groups.values(), deconz.lights.values(), deconz.sensors.values()):


line too long (93 > 79 characters)

…gether with deconz id

Fixed call to protected member

balloob

I'm sorry. I've thought about it more and I think that it's better to keep the Entity Registry private for now. So let have deconz keep track of unique id's.

You could do something like this fairly easy by leveraging the async_added_to_hass callback:

class DeconzLight(Light):

    @asyncio.coroutine
    def async_added_to_hass(self):
        """Called when entity added to hass."""
        self.hass.data[DATA_DECONZ_ID][self.entity_id] = self.unique_id

(self.hass and self.entity_id are guaranteed to exist when async_added_to_hass is called)

Kane610 · 2018-02-13T06:52:33Z

@balloob alright, should I remove the new method for registry? Yes that was the original way I was doing it, just thought the registry would remove the need for a local registry.

balloob · 2018-02-13T07:03:05Z

I know. Sorry for not realizing this earlier.

Let's remove it.

Kane610 · 2018-02-13T07:07:18Z

No worries, it is not always easy to have a plan ready for new functionality. I will change these things tonight.

houndci-bot · 2018-02-13T17:36:12Z

homeassistant/components/deconz/__init__.py

 import asyncio
 import logging
+from itertools import chain


'itertools.chain' imported but unused

houndci-bot · 2018-02-13T17:36:12Z

homeassistant/components/deconz/__init__.py

 import asyncio
 import logging
+from itertools import chain


'itertools.chain' imported but unused

Kane610 · 2018-02-13T19:51:34Z

@balloob better like this?

MartinHjelmare · 2018-02-13T19:54:44Z

homeassistant/components/binary_sensor/deconz.py

    async_add_devices(entities, True)


 class DeconzBinarySensor(BinarySensorDevice):
    """Representation of a binary sensor."""

-    def __init__(self, sensor):
+    def __init__(self, hass, sensor):


Don't pass in hass. It will be set on the entity when the entity has been added to home assistant. This means you can access hass with self.hass, after the entity has been added to home assistant.

MartinHjelmare · 2018-02-13T19:54:56Z

homeassistant/components/binary_sensor/deconz.py

        """Set up sensor and add update callback to get data from websocket."""
+        self.hass = hass


Remove this.

MartinHjelmare · 2018-02-13T19:56:58Z

homeassistant/components/light/deconz.py

    async_add_devices(entities, True)


 class DeconzLight(Light):
    """Representation of a deCONZ light."""

-    def __init__(self, light):
+    def __init__(self, hass, light):


Don't pass in hass. It will be set on the entity when the entity has been added to home assistant. This means you can access hass with self.hass, after the entity has been added to home assistant.

MartinHjelmare · 2018-02-13T19:57:08Z

homeassistant/components/light/deconz.py

        """Set up light and add update callback to get data from websocket."""
+        self.hass = hass


Please remove.

MartinHjelmare · 2018-02-13T19:57:23Z

homeassistant/components/scene/deconz.py

    async_add_devices(entities)


 class DeconzScene(Scene):
    """Representation of a deCONZ scene."""

-    def __init__(self, scene):
+    def __init__(self, hass, scene):


MartinHjelmare · 2018-02-13T19:57:29Z

homeassistant/components/scene/deconz.py

        """Set up a scene."""
+        self.hass = hass


MartinHjelmare · 2018-02-13T19:57:41Z

homeassistant/components/sensor/deconz.py

    async_add_devices(entities, True)


 class DeconzSensor(Entity):
    """Representation of a sensor."""

-    def __init__(self, sensor):
+    def __init__(self, hass, sensor):


MartinHjelmare · 2018-02-13T19:57:48Z

homeassistant/components/sensor/deconz.py

        """Set up sensor and add update callback to get data from websocket."""
+        self.hass = hass


MartinHjelmare · 2018-02-13T19:57:58Z

homeassistant/components/sensor/deconz.py

@@ -116,8 +119,9 @@ def device_state_attributes(self):
 class DeconzBattery(Entity):
    """Battery class for when a device is only represented as an event."""

-    def __init__(self, device):
+    def __init__(self, hass, device):


MartinHjelmare · 2018-02-13T19:58:03Z

homeassistant/components/sensor/deconz.py

        """Register dispatcher callback for update of battery state."""
+        self.hass = hass


MartinHjelmare

Looks good to me!

Kane610 · 2018-02-13T20:14:33Z

Thanks @MartinHjelmare

Kane610 requested a review from andrey-git as a code owner January 22, 2018 21:26

homeassistant added cla-signed integration: deconz integration: scene core platform: binary_sensor.deconz labels Jan 22, 2018

Kane610 force-pushed the deconz-simplify-service branch from cbc4091 to 84d3e57 Compare January 22, 2018 21:31

homeassistant added the cla-signed label Jan 22, 2018

Kane610 mentioned this pull request Jan 22, 2018

Update deconz.markdown home-assistant/home-assistant.io#4494

Merged

2 tasks

balloob requested changes Jan 23, 2018

View reviewed changes

Kane610 force-pushed the deconz-simplify-service branch from facbb09 to 9da45a6 Compare January 29, 2018 20:30

homeassistant added the cla-signed label Jan 29, 2018

Kane610 force-pushed the deconz-simplify-service branch from bcea029 to 9c902b9 Compare January 30, 2018 20:14

homeassistant added the cla-signed label Jan 30, 2018

Kane610 force-pushed the deconz-simplify-service branch from 9c902b9 to f5b54fe Compare January 31, 2018 19:08

homeassistant added the cla-signed label Jan 31, 2018

Kane610 requested a review from a team as a code owner January 31, 2018 20:56

homeassistant added the cla-signed label Jan 31, 2018

houndci-bot reviewed Jan 31, 2018

View reviewed changes

balloob reviewed Feb 2, 2018

View reviewed changes

houndci-bot reviewed Feb 2, 2018

View reviewed changes

Kane610 added 4 commits February 12, 2018 21:15

Improve service by allowing to reference entity id instead of deconz id

b974607

Change from having access to full entities to only store entity id to…

5685ec1

…gether with deconz id

Don't use eval, there is a dict type for voluptuous

e89d70e

Use entity registry instead of keeping a local registry over entity ids

4562701

Kane610 added 5 commits February 12, 2018 21:18

Bump dependency to v28

ba378fd

Fixed call to protected member

Use chain to iterate over dict values

fc12bfe

Cleanup

df4b1f4

Fix hound comment

a324785

Cleanup

757e1c2

Kane610 force-pushed the deconz-simplify-service branch from 74a1b1d to 757e1c2 Compare February 12, 2018 20:21

homeassistant added the cla-signed label Feb 12, 2018

Follow refactoring of entity

24f099f

balloob requested changes Feb 13, 2018

View reviewed changes

Revert to using a local registry

478efd3

homeassistant added the cla-signed label Feb 13, 2018

houndci-bot reviewed Feb 13, 2018

View reviewed changes

Remove unused import

7e0f481

MartinHjelmare requested changes Feb 13, 2018

View reviewed changes

self.hass is automatically available when entity is registered in hass

042ffe6

homeassistant added the cla-signed label Feb 13, 2018

MartinHjelmare approved these changes Feb 13, 2018

View reviewed changes

balloob merged commit 8bff813 into home-assistant:dev Feb 14, 2018

Kane610 deleted the deconz-simplify-service branch February 14, 2018 17:03

balloob mentioned this pull request Feb 22, 2018

0.64.0 #12609

Merged

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

ghost removed platform: binary_sensor.deconz labels Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve service by allowing to reference entity id instead of deconz id #11862

Improve service by allowing to reference entity id instead of deconz id #11862

Kane610 commented Jan 22, 2018 •

edited

Loading

balloob left a comment

balloob Jan 23, 2018

Kane610 Jan 23, 2018

Kane610 Jan 23, 2018

balloob Jan 23, 2018

Kane610 Jan 23, 2018

Kane610 commented Jan 23, 2018

Kane610 commented Jan 24, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

balloob Feb 2, 2018

balloob Feb 2, 2018

houndci-bot Feb 2, 2018

balloob left a comment •

edited

Loading

Kane610 commented Feb 13, 2018 •

edited

Loading

balloob commented Feb 13, 2018

Kane610 commented Feb 13, 2018

houndci-bot Feb 13, 2018

houndci-bot Feb 13, 2018

Kane610 commented Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare Feb 13, 2018

MartinHjelmare left a comment

Kane610 commented Feb 13, 2018

		"""Set up sensor and add update callback to get data from websocket."""
		self.hass = hass

		"""Set up light and add update callback to get data from websocket."""
		self.hass = hass

		"""Register dispatcher callback for update of battery state."""
		self.hass = hass

Improve service by allowing to reference entity id instead of deconz id #11862

Improve service by allowing to reference entity id instead of deconz id #11862

Conversation

Kane610 commented Jan 22, 2018 • edited Loading

Description:

Checklist:

balloob 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

Kane610 commented Jan 23, 2018

Kane610 commented Jan 24, 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

balloob left a comment • edited Loading

Choose a reason for hiding this comment

Kane610 commented Feb 13, 2018 • edited Loading

balloob commented Feb 13, 2018

Kane610 commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kane610 commented Feb 13, 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

MartinHjelmare left a comment

Choose a reason for hiding this comment

Kane610 commented Feb 13, 2018

Kane610 commented Jan 22, 2018 •

edited

Loading

balloob left a comment •

edited

Loading

Kane610 commented Feb 13, 2018 •

edited

Loading