Improve service by allowing to reference entity id instead of deconz id by Kane610 · Pull Request #11862 · home-assistant/core

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

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

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

block comment should start with '# '

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

block comment should start with '# '

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

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

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

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

+
 import asyncio
 import logging
+from itertools import chain


'itertools.chain' imported but unused

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

+
 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

    """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

-    def __init__(self, sensor):
+    def __init__(self, hass, sensor):
        """Set up sensor and add update callback to get data from websocket."""
+        self.hass = hass


Remove this.

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

    """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

-    def __init__(self, light):
+    def __init__(self, hass, light):
        """Set up light and add update callback to get data from websocket."""
+        self.hass = hass


Please remove.

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

    """Representation of a deCONZ scene."""

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


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

-    def __init__(self, scene):
+    def __init__(self, hass, scene):
        """Set up a scene."""
+        self.hass = hass


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

    """Representation of a sensor."""

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


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

-    def __init__(self, sensor):
+    def __init__(self, hass, sensor):
        """Set up sensor and add update callback to get data from websocket."""
+        self.hass = hass


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

    """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

-    def __init__(self, device):
+    def __init__(self, hass, device):
        """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

Comment thread homeassistant/components/deconz/__init__.py Outdated

Copy link
Copy Markdown

houndci-bot Feb 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (93 > 79 characters)

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

Uh oh!

Conversation

Kane610 commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Uh oh!

balloob left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kane610 commented Jan 23, 2018

Uh oh!

Kane610 commented Jan 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kane610 commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balloob commented Feb 13, 2018

Uh oh!

Kane610 commented Feb 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kane610 commented Feb 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Kane610 commented Feb 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Kane610 commented Jan 22, 2018 •

edited

Loading

balloob left a comment •

edited

Loading

Kane610 commented Feb 13, 2018 •

edited

Loading