Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

deCONZ - Support device registry #16115

Merged
merged 14 commits into from
Aug 24, 2018
Merged

deCONZ - Support device registry #16115

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8dbda5a
Support device registry
Kane610 Aug 21, 2018
82c0745
Fix hound comment
Kane610 Aug 21, 2018
38b51a1
Fix Balloobs comments
Kane610 Aug 22, 2018
bb8eb38
Update tests
Kane610 Aug 22, 2018
454c2da
Add missing pydoc string
Kane610 Aug 22, 2018
7069980
Partial test fix
Kane610 Aug 22, 2018
15ba6ab
Update tests
Kane610 Aug 22, 2018
16a7191
Fix styling comments
Kane610 Aug 22, 2018
7fcfc74
Fix test
Kane610 Aug 23, 2018
7b48826
Improve readability of platform device descriptions
Kane610 Aug 23, 2018
4d3c28c
Fix identifiers
Kane610 Aug 23, 2018
df79e77
Inverse check in device property
Kane610 Aug 24, 2018
33405d8
Added second half of logic
Kane610 Aug 24, 2018
067c0f8
Lint
Kane610 Aug 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions homeassistant/components/binary_sensor/deconz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
DATA_DECONZ_ID, DATA_DECONZ_UNSUB)
from homeassistant.const import ATTR_BATTERY_LEVEL
from homeassistant.core import callback
from homeassistant.helpers.device_registry import (
CONNECTION_ZIGBEE, IDENTIFIER_MAC)
from homeassistant.helpers.dispatcher import async_dispatcher_connect

DEPENDENCIES = ['deconz']
Expand Down Expand Up @@ -113,3 +115,21 @@ def device_state_attributes(self):
if self._sensor.type in PRESENCE and self._sensor.dark is not None:
attr[ATTR_DARK] = self._sensor.dark
return attr

@property
def device(self):
"""Description for device registry."""
if (self._sensor.uniqueid is not None and
Copy link
Member

@balloob balloob Aug 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Can you inverse this logic to make the method a bit easier to read

if (self._sensor.uniqueid is None or
        self._sensor.uniqueid.count(':') != 7):
    return None

return {
    …
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I can evaluate positive on the counter instead. Then leave return None to be at the end of the method instead.

self._sensor.uniqueid.count(':') == 7):
mac = self._sensor.uniqueid.split('-', 1)[0]
else:
return None
dev = {
'connection': [[CONNECTION_ZIGBEE, mac]],
'identifiers': [[IDENTIFIER_MAC, mac]],
Copy link
Member

Choose a reason for hiding this comment

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

There should not be an identifier MAC. There is already a connection one.

Is there a serial number for this sensor? In that case you could add [DECONZ_DOMAIN, serial]

Also MAC should never be used as just a standalone type, as there is Network MAC, Zigbee MAC etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point with DECONZ_DOMAIN since it should only map inside of deconz context 👍

Previously you asked if the sensor had a mac, I will go back to using serial :)

Should I just call them NETWORK_MAC and ZIGBEE_MAC?

Copy link
Member

Choose a reason for hiding this comment

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

yes

'manufacturer': self._sensor.manufacturer,
'model': self._sensor.modelid,
'name': self._sensor.name,
'sw_version': self._sensor.swversion,
}
return dev
11 changes: 10 additions & 1 deletion homeassistant/components/deconz/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
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 CONNECTION_MAC
from homeassistant.helpers.dispatcher import (
async_dispatcher_connect, async_dispatcher_send)
from homeassistant.util import slugify
Expand All @@ -23,7 +24,7 @@
CONF_ALLOW_CLIP_SENSOR, CONFIG_FILE, DATA_DECONZ_EVENT,
DATA_DECONZ_ID, DATA_DECONZ_UNSUB, DOMAIN, _LOGGER)

REQUIREMENTS = ['pydeconz==43']
REQUIREMENTS = ['pydeconz==44']

CONFIG_SCHEMA = vol.Schema({
DOMAIN: vol.Schema({
Expand Down Expand Up @@ -119,6 +120,14 @@ def async_add_remote(sensors):

deconz.start()

device_registry = await \
hass.helpers.device_registry.async_get_registry()
device_registry.async_get_or_create(
Copy link

Choose a reason for hiding this comment

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

indentation contains mixed spaces and tabs

Copy link
Member

Choose a reason for hiding this comment

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

As this example might be copied a lot, I would prefer that we use named arguments to clarify what is what:

device_registry.async_get_or_create(
  identifiers=[[DOMAIN, deconz.config.bridgeid]],
  manufacturer='Dresden Elektronik',
  # etc
)

connection=[[CONNECTION_MAC, deconz.config.mac]],
identifiers=[[DOMAIN, deconz.config.bridgeid]],
manufacturer='Dresden Elektronik', model=deconz.config.modelid,
name=deconz.config.name, sw_version=deconz.config.swversion)

async def async_configure(call):
"""Set attribute of device in deCONZ.

Expand Down
20 changes: 20 additions & 0 deletions homeassistant/components/light/deconz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
SUPPORT_BRIGHTNESS, SUPPORT_COLOR, SUPPORT_COLOR_TEMP, SUPPORT_EFFECT,
SUPPORT_FLASH, SUPPORT_TRANSITION, Light)
from homeassistant.core import callback
from homeassistant.helpers.device_registry import (
CONNECTION_ZIGBEE, IDENTIFIER_MAC)
from homeassistant.helpers.dispatcher import async_dispatcher_connect
import homeassistant.util.color as color_util

Expand Down Expand Up @@ -199,3 +201,21 @@ def device_state_attributes(self):
if self._light.type == 'LightGroup':
attributes['all_on'] = self._light.all_on
return attributes

@property
def device(self):
"""Description for device registry."""
if (self._light.uniqueid is not None and
Copy link
Member

Choose a reason for hiding this comment

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

Same

self._light.uniqueid.count(':') == 7):
mac = self._light.uniqueid.split('-', 1)[0]
else:
return None
dev = {
'connection': [[CONNECTION_ZIGBEE, mac]],
'identifiers': [[IDENTIFIER_MAC, mac]],
'manufacturer': self._light.manufacturer,
'model': self._light.modelid,
'name': self._light.name,
'sw_version': self._light.swversion,
}
return dev
38 changes: 38 additions & 0 deletions homeassistant/components/sensor/deconz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from homeassistant.const import (
ATTR_BATTERY_LEVEL, ATTR_VOLTAGE, DEVICE_CLASS_BATTERY)
from homeassistant.core import callback
from homeassistant.helpers.device_registry import (
CONNECTION_ZIGBEE, IDENTIFIER_MAC)
from homeassistant.helpers.dispatcher import async_dispatcher_connect
from homeassistant.helpers.entity import Entity
from homeassistant.util import slugify
Expand Down Expand Up @@ -134,6 +136,24 @@ def device_state_attributes(self):
attr[ATTR_DAYLIGHT] = self._sensor.daylight
return attr

@property
def device(self):
"""Description for device registry."""
if (self._sensor.uniqueid is not None and
self._sensor.uniqueid.count(':') == 7):
mac = self._sensor.uniqueid.split('-', 1)[0]
else:
return None
dev = {
'connection': [[CONNECTION_ZIGBEE, mac]],
'identifiers': [[IDENTIFIER_MAC, mac]],
'manufacturer': self._sensor.manufacturer,
'model': self._sensor.modelid,
'name': self._sensor.name,
'sw_version': self._sensor.swversion,
}
return dev


class DeconzBattery(Entity):
"""Battery class for when a device is only represented as an event."""
Expand Down Expand Up @@ -192,3 +212,21 @@ def device_state_attributes(self):
ATTR_EVENT_ID: slugify(self._device.name),
}
return attr

@property
def device(self):
"""Description for device registry."""
if (self._device.uniqueid is not None and
Copy link
Member

Choose a reason for hiding this comment

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

Same

self._device.uniqueid.count(':') == 7):
mac = self._device.uniqueid.split('-', 1)[0]
else:
return None
dev = {
'connection': [[CONNECTION_ZIGBEE, mac]],
'identifiers': [[IDENTIFIER_MAC, mac]],
'manufacturer': self._device.manufacturer,
'model': self._device.modelid,
'name': self._device.name,
'sw_version': self._device.swversion,
}
return dev
20 changes: 20 additions & 0 deletions homeassistant/components/switch/deconz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
POWER_PLUGS, SIRENS)
from homeassistant.components.switch import SwitchDevice
from homeassistant.core import callback
from homeassistant.helpers.device_registry import (
CONNECTION_ZIGBEE, IDENTIFIER_MAC)
from homeassistant.helpers.dispatcher import async_dispatcher_connect

DEPENDENCIES = ['deconz']
Expand Down Expand Up @@ -79,6 +81,24 @@ def should_poll(self):
"""No polling needed."""
return False

@property
def device(self):
"""Description for device registry."""
if (self._switch.uniqueid is not None and
Copy link
Member

Choose a reason for hiding this comment

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

Same

self._switch.uniqueid.count(':') == 7):
mac = self._switch.uniqueid.split('-', 1)[0]
else:
return None
dev = {
'connection': [[CONNECTION_ZIGBEE, mac]],
'identifiers': [[IDENTIFIER_MAC, mac]],
'manufacturer': self._switch.manufacturer,
'model': self._switch.modelid,
'name': self._switch.name,
'sw_version': self._switch.swversion,
}
return dev


class DeconzPowerPlug(DeconzSwitch):
"""Representation of power plugs from deCONZ."""
Expand Down
15 changes: 10 additions & 5 deletions homeassistant/helpers/device_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,20 @@
STORAGE_VERSION = 1
SAVE_DELAY = 10

CONNECTION_MAC = 'mac'
CONNECTION_ZIGBEE = 'zigbee'

IDENTIFIER_MAC = 'mac'


@attr.s(slots=True, frozen=True)
class DeviceEntry:
"""Device Registry Entry."""

connection = attr.ib(type=list)
identifiers = attr.ib(type=list)
manufacturer = attr.ib(type=str)
model = attr.ib(type=str)
connection = attr.ib(type=list)
name = attr.ib(type=str, default=None)
sw_version = attr.ib(type=str, default=None)
id = attr.ib(type=str, default=attr.Factory(lambda: uuid.uuid4().hex))
Expand All @@ -48,19 +53,19 @@ def async_get_device(self, identifiers: str, connections: tuple):
return None

@callback
def async_get_or_create(self, identifiers, manufacturer, model,
connection, *, name=None, sw_version=None):
def async_get_or_create(self, *, connection, identifiers, manufacturer,
model, name=None, sw_version=None):
"""Get device. Create if it doesn't exist."""
device = self.async_get_device(identifiers, connection)

if device is not None:
return device

device = DeviceEntry(
connection=connection,
identifiers=identifiers,
manufacturer=manufacturer,
model=model,
connection=connection,
name=name,
sw_version=sw_version
)
Expand Down Expand Up @@ -93,10 +98,10 @@ def _data_to_save(self):
data['devices'] = [
{
'id': entry.id,
'connection': entry.connection,
'identifiers': entry.identifiers,
'manufacturer': entry.manufacturer,
'model': entry.model,
'connection': entry.connection,
'name': entry.name,
'sw_version': entry.sw_version,
} for entry in self.devices
Expand Down
7 changes: 5 additions & 2 deletions homeassistant/helpers/entity_platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,11 @@ async def _async_add_entity(self, entity, update_before_add,
device = entity.device
if device is not None:
device = device_registry.async_get_or_create(
device['identifiers'], device['manufacturer'],
device['model'], device['connection'],
connection=device['connection'],
identifiers=device['identifiers'],
manufacturer=device['manufacturer'],
model=device['model'],
name=device.get('name'),
sw_version=device.get('sw_version'))
device_id = device.id
else:
Expand Down
2 changes: 1 addition & 1 deletion requirements_all.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ pycsspeechtts==1.0.2
pydaikin==0.4

# homeassistant.components.deconz
pydeconz==43
pydeconz==44

# homeassistant.components.zwave
pydispatcher==2.0.5
Expand Down
2 changes: 1 addition & 1 deletion requirements_test_all.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ py-canary==0.5.0
pyblackbird==0.5

# homeassistant.components.deconz
pydeconz==43
pydeconz==44

# homeassistant.components.zwave
pydispatcher==2.0.5
Expand Down
55 changes: 37 additions & 18 deletions tests/components/deconz/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,17 @@
from homeassistant.setup import async_setup_component
from homeassistant.components import deconz

from tests.common import mock_coro

from tests.common import mock_coro, MockConfigEntry

CONFIG = {
"config": {
"bridgeid": "0123456789ABCDEF",
"mac": "12:34:56:78:90:ab",
"modelid": "deCONZ",
"name": "Phoscon",
"swversion": "2.05.35"
}
}

async def test_config_with_host_passed_to_config_entry(hass):
"""Test that configured options for a host are loaded via config entry."""
Expand Down Expand Up @@ -93,8 +102,11 @@ async def test_setup_entry_successful(hass):
entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'}
with patch.object(hass, 'async_create_task') as mock_add_job, \
patch.object(hass, 'config_entries') as mock_config_entries, \
patch('pydeconz.DeconzSession.async_load_parameters',
return_value=mock_coro(True)):
patch('pydeconz.DeconzSession.async_get_state',
return_value=mock_coro(CONFIG)), \
patch('pydeconz.DeconzSession.start', return_value=True), \
patch('homeassistant.helpers.device_registry.async_get_registry',
return_value=mock_coro(Mock())):
assert await deconz.async_setup_entry(hass, entry) is True
assert hass.data[deconz.DOMAIN]
assert hass.data[deconz.DATA_DECONZ_ID] == {}
Expand All @@ -115,10 +127,13 @@ async def test_setup_entry_successful(hass):

async def test_unload_entry(hass):
"""Test being able to unload an entry."""
entry = Mock()
entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'}
with patch('pydeconz.DeconzSession.async_load_parameters',
return_value=mock_coro(True)):
entry = MockConfigEntry(domain=deconz.DOMAIN, data={
'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'
})
entry.add_to_hass(hass)
with patch('pydeconz.DeconzSession.async_get_state',
return_value=mock_coro(CONFIG)), \
patch('pydeconz.DeconzSession.start', return_value=True):
Copy link

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

assert await deconz.async_setup_entry(hass, entry) is True
assert deconz.DATA_DECONZ_EVENT in hass.data
hass.data[deconz.DATA_DECONZ_EVENT].append(Mock())
Expand All @@ -132,6 +147,9 @@ async def test_unload_entry(hass):

async def test_add_new_device(hass):
"""Test adding a new device generates a signal for platforms."""
entry = Mock()
entry.data = {'host': '1.2.3.4', 'port': 80,
'api_key': '1234567890ABCDEF', 'allow_clip_sensor': False}
new_event = {
"t": "event",
"e": "added",
Expand All @@ -147,11 +165,10 @@ async def test_add_new_device(hass):
"type": "ZHASwitch"
}
}
entry = Mock()
entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'}
with patch.object(deconz, 'async_dispatcher_send') as mock_dispatch_send, \
patch('pydeconz.DeconzSession.async_load_parameters',
return_value=mock_coro(True)):
patch('pydeconz.DeconzSession.async_get_state',
Copy link

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

return_value=mock_coro(CONFIG)), \
patch('pydeconz.DeconzSession.start', return_value=True):
Copy link

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

assert await deconz.async_setup_entry(hass, entry) is True
hass.data[deconz.DOMAIN].async_event_handler(new_event)
await hass.async_block_till_done()
Expand All @@ -162,15 +179,16 @@ async def test_add_new_device(hass):
async def test_add_new_remote(hass):
"""Test new added device creates a new remote."""
entry = Mock()
entry.data = {'host': '1.2.3.4', 'port': 80, 'api_key': '1234567890ABCDEF'}
entry.data = {'host': '1.2.3.4', 'port': 80,
'api_key': '1234567890ABCDEF', 'allow_clip_sensor': False}
remote = Mock()
remote.name = 'name'
remote.type = 'ZHASwitch'
remote.register_async_callback = Mock()
with patch('pydeconz.DeconzSession.async_load_parameters',
return_value=mock_coro(True)):
with patch('pydeconz.DeconzSession.async_get_state',
return_value=mock_coro(CONFIG)), \
patch('pydeconz.DeconzSession.start', return_value=True):
assert await deconz.async_setup_entry(hass, entry) is True

async_dispatcher_send(hass, 'deconz_new_sensor', [remote])
await hass.async_block_till_done()
assert len(hass.data[deconz.DATA_DECONZ_EVENT]) == 1
Expand All @@ -185,8 +203,9 @@ async def test_do_not_allow_clip_sensor(hass):
remote.name = 'name'
remote.type = 'CLIPSwitch'
remote.register_async_callback = Mock()
with patch('pydeconz.DeconzSession.async_load_parameters',
return_value=mock_coro(True)):
with patch('pydeconz.DeconzSession.async_get_state',
return_value=mock_coro(CONFIG)), \
patch('pydeconz.DeconzSession.start', return_value=True):
Copy link

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

assert await deconz.async_setup_entry(hass, entry) is True

async_dispatcher_send(hass, 'deconz_new_sensor', [remote])
Expand Down
Loading