Skip to content

Create zwave devices on OZW thread and only add them during discovery #6096

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

Merged
balloob merged 10 commits into from
Feb 23, 2017
Merged

Create zwave devices on OZW thread and only add them during discovery #6096

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9b09960
Create zwave devices on OZW thread and only add them during discovery.
andrey-git Feb 19, 2017
5a247eb
Merge branch 'dev' of https://github.com/home-assistant/home-assistan…
andrey-git Feb 19, 2017
d277e9c
Read and write devices dict from loop thread.
andrey-git Feb 19, 2017
4660b8e
More async
andrey-git Feb 20, 2017
61a3c19
Merge branch 'dev' of https://github.com/home-assistant/home-assistan…
andrey-git Feb 20, 2017
0ea85b8
replace callback with coroutine
andrey-git Feb 20, 2017
b823d08
Merge branch 'dev' of https://github.com/home-assistant/home-assistan…
andrey-git Feb 20, 2017
456eabc
import common function instead of callin git
andrey-git Feb 22, 2017
2fab8ff
Merge branch 'dev' of https://github.com/home-assistant/home-assistan…
andrey-git Feb 22, 2017
cf67e77
Merge branch 'dev' of https://github.com/home-assistant/home-assistan…
andrey-git Feb 23, 2017
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
29 changes: 14 additions & 15 deletions homeassistant/components/binary_sensor/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,30 @@
DEPENDENCIES = []


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup the Z-Wave platform for binary sensors."""
if discovery_info is None or zwave.NETWORK is None:
return

node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]
def get_device(value, hass, **kwargs):
"""Create zwave entity device."""
value.set_change_verified(False)

device_mapping = workaround.get_device_mapping(value)
if device_mapping == workaround.WORKAROUND_NO_OFF_EVENT:
# Default the multiplier to 4
re_arm_multiplier = (zwave.get_config_value(value.node, 9) or 4)
add_devices([
ZWaveTriggerSensor(value, "motion",
hass, re_arm_multiplier * 8)
])
return
return ZWaveTriggerSensor(value, "motion", hass, re_arm_multiplier * 8)

if workaround.get_device_component_mapping(value) == DOMAIN:
add_devices([ZWaveBinarySensor(value, None)])
return
return ZWaveBinarySensor(value, None)

if value.command_class == zwave.const.COMMAND_CLASS_SENSOR_BINARY:
add_devices([ZWaveBinarySensor(value, None)])
return ZWaveBinarySensor(value, None)
return None


def setup_platform(hass, config, add_devices, discovery_info=None):

@balloob balloob Feb 19, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that you can make a generic setup that you just import for each platform. We should make it async for improved speed:

(to be placed in zwave/__init__.py)

@asyncio.coroutine
def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
    """Generic Z-Wave platform setup."""
    if discovery_info is None or zwave.NETWORK is None:
        return False
    device = zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])
    if device:
        yield from async_add_devices([device])
        return True
    else:
        return False

In the case of the lock component, you could then wrap this method and register the services.

from homeassistant.components.zwave import async_setup_platform as zwave_async_setup_platform
@asyncio.coroutine
def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
    result = yield from zwave_async_setup_platform(…)
    if not result or hass.services.has_service('lock', SERVICE_XX):
        return
    # Register services

"""Setup the Z-Wave platform for binary sensors."""
if discovery_info is None or zwave.NETWORK is None:
return
add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


class ZWaveBinarySensor(BinarySensorDevice, zwave.ZWaveDeviceEntity):
Expand Down
15 changes: 8 additions & 7 deletions homeassistant/components/climate/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,17 @@
def setup_platform(hass, config, add_devices, discovery_info=None):
"""Set up the Z-Wave Climate devices."""
if discovery_info is None or zwave.NETWORK is None:
_LOGGER.debug("No discovery_info=%s or no NETWORK=%s",
discovery_info, zwave.NETWORK)
return

add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


def get_device(hass, value, **kwargs):
"""Create zwave entity device."""
temp_unit = hass.config.units.temperature_unit
node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]
value.set_change_verified(False)
add_devices([ZWaveClimate(value, temp_unit)])
_LOGGER.debug("discovery_info=%s and zwave.NETWORK=%s",
discovery_info, zwave.NETWORK)
return ZWaveClimate(value, temp_unit)


class ZWaveClimate(ZWaveDeviceEntity, ClimateDevice):
Expand Down
26 changes: 14 additions & 12 deletions homeassistant/components/cover/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,30 @@
SUPPORT_GARAGE = SUPPORT_OPEN | SUPPORT_CLOSE


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Find and return Z-Wave covers."""
if discovery_info is None or zwave.NETWORK is None:
return

node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]

def get_device(value, **kwargs):
"""Create zwave entity device."""
if (value.command_class == zwave.const.COMMAND_CLASS_SWITCH_MULTILEVEL
and value.index == 0):
value.set_change_verified(False)
add_devices([ZwaveRollershutter(value)])
return ZwaveRollershutter(value)
elif (value.command_class == zwave.const.COMMAND_CLASS_SWITCH_BINARY or
value.command_class == zwave.const.COMMAND_CLASS_BARRIER_OPERATOR):
if (value.type != zwave.const.TYPE_BOOL and
value.genre != zwave.const.GENRE_USER):
return
return None
value.set_change_verified(False)
add_devices([ZwaveGarageDoor(value)])
else:
return ZwaveGarageDoor(value)
return None


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Find and return Z-Wave covers."""
if discovery_info is None or zwave.NETWORK is None:
return

add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


class ZwaveRollershutter(zwave.ZWaveDeviceEntity, CoverDevice):
"""Representation of an Zwave roller shutter."""
Expand Down
19 changes: 11 additions & 8 deletions homeassistant/components/light/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,28 +52,31 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
"""Find and add Z-Wave lights."""
if discovery_info is None or zwave.NETWORK is None:
return
node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]
add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


def get_device(node, value, node_config, **kwargs):
"""Create zwave entity device."""
name = '{}.{}'.format(DOMAIN, zwave.object_id(value))
node_config = hass.data[zwave.DATA_DEVICE_CONFIG].get(name)
refresh = node_config.get(zwave.CONF_REFRESH_VALUE)
delay = node_config.get(zwave.CONF_REFRESH_DELAY)
_LOGGER.debug('name=%s node_config=%s CONF_REFRESH_VALUE=%s'
' CONF_REFRESH_DELAY=%s', name, node_config,
refresh, delay)
if value.command_class != zwave.const.COMMAND_CLASS_SWITCH_MULTILEVEL:

@balloob balloob Feb 19, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(for a future PR)

Should this filtering happening inside these methods or should get_device only be called with the correct values? (And thus the filtering done before)

@andrey-git andrey-git Feb 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Some filtering can be removed as it is already done in component. I'll look into it in another PR

return
return None
if value.type != zwave.const.TYPE_BYTE:
return
return None
if value.genre != zwave.const.GENRE_USER:
return
return None

value.set_change_verified(False)

if node.has_command_class(zwave.const.COMMAND_CLASS_SWITCH_COLOR):
add_devices([ZwaveColorLight(value, refresh, delay)])
return ZwaveColorLight(value, refresh, delay)
else:
add_devices([ZwaveDimmer(value, refresh, delay)])
return ZwaveDimmer(value, refresh, delay)


def brightness_state(value):
Expand Down
15 changes: 9 additions & 6 deletions homeassistant/components/lock/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,12 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
if discovery_info is None or zwave.NETWORK is None:
return

node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]
add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


def get_device(hass, node, value, **kwargs):
"""Create zwave entity device."""
descriptions = load_yaml_config_file(
path.join(path.dirname(__file__), 'services.yaml'))

Expand Down Expand Up @@ -182,11 +185,11 @@ def clear_usercode(service):
break

if value.command_class != zwave.const.COMMAND_CLASS_DOOR_LOCK:
return
return None
if value.type != zwave.const.TYPE_BOOL:
return
return None
if value.genre != zwave.const.GENRE_USER:
return
return None
if node.has_command_class(zwave.const.COMMAND_CLASS_USER_CODE):

@balloob balloob Feb 19, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Registering these services should stay in setup_platform

@andrey-git andrey-git Feb 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are registered conditionally by accessing node, so must be done on OZW thread.

hass.services.register(DOMAIN,
SERVICE_SET_USERCODE,
Expand All @@ -204,7 +207,7 @@ def clear_usercode(service):
descriptions.get(SERVICE_CLEAR_USERCODE),
schema=CLEAR_USERCODE_SCHEMA)
value.set_change_verified(False)
add_devices([ZwaveLock(value)])
return ZwaveLock(value)


class ZwaveLock(zwave.ZWaveDeviceEntity, LockDevice):
Expand Down
38 changes: 18 additions & 20 deletions homeassistant/components/sensor/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,22 @@
_LOGGER = logging.getLogger(__name__)


def get_device(node, value, **kwargs):
"""Create zwave entity device."""
value.set_change_verified(False)

# Generic Device mappings
if node.has_command_class(zwave.const.COMMAND_CLASS_SENSOR_MULTILEVEL):
return ZWaveMultilevelSensor(value)
if node.has_command_class(zwave.const.COMMAND_CLASS_METER) and \
value.type == zwave.const.TYPE_DECIMAL:
return ZWaveMultilevelSensor(value)
if node.has_command_class(zwave.const.COMMAND_CLASS_ALARM) or \
node.has_command_class(zwave.const.COMMAND_CLASS_SENSOR_ALARM):
return ZWaveAlarmSensor(value)
return None


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup Z-Wave sensors."""
# Return on empty `discovery_info`. Given you configure HA with:
Expand All @@ -25,26 +41,8 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
if discovery_info is None or zwave.NETWORK is None:
return

node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]

value.set_change_verified(False)

# if 1 in groups and (NETWORK.controller.node_id not in
# groups[1].associations):
# node.groups[1].add_association(NETWORK.controller.node_id)

# Generic Device mappings
if node.has_command_class(zwave.const.COMMAND_CLASS_SENSOR_MULTILEVEL):
add_devices([ZWaveMultilevelSensor(value)])

elif node.has_command_class(zwave.const.COMMAND_CLASS_METER) and \
value.type == zwave.const.TYPE_DECIMAL:
add_devices([ZWaveMultilevelSensor(value)])

elif node.has_command_class(zwave.const.COMMAND_CLASS_ALARM) or \
node.has_command_class(zwave.const.COMMAND_CLASS_SENSOR_ALARM):
add_devices([ZWaveAlarmSensor(value)])
add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


class ZWaveSensor(zwave.ZWaveDeviceEntity):
Expand Down
25 changes: 14 additions & 11 deletions homeassistant/components/switch/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,26 @@
_LOGGER = logging.getLogger(__name__)


# pylint: disable=unused-argument
def get_device(node, value, **kwargs):
"""Create zwave entity device."""
if not node.has_command_class(zwave.const.COMMAND_CLASS_SWITCH_BINARY):
return None
if value.type != zwave.const.TYPE_BOOL or value.genre != \
zwave.const.GENRE_USER:
return None
value.set_change_verified(False)
return ZwaveSwitch(value)


# pylint: disable=unused-argument
def setup_platform(hass, config, add_devices, discovery_info=None):
"""Setup the Z-Wave platform."""
if discovery_info is None or zwave.NETWORK is None:
return

node = zwave.NETWORK.nodes[discovery_info[zwave.const.ATTR_NODE_ID]]
value = node.values[discovery_info[zwave.const.ATTR_VALUE_ID]]

if not node.has_command_class(zwave.const.COMMAND_CLASS_SWITCH_BINARY):
return
if value.type != zwave.const.TYPE_BOOL or value.genre != \
zwave.const.GENRE_USER:
return

value.set_change_verified(False)
add_devices([ZwaveSwitch(value)])
add_devices(
[zwave.get_device(hass, discovery_info[zwave.const.DISCOVERY_DEVICE])])


class ZwaveSwitch(zwave.ZWaveDeviceEntity, SwitchDevice):
Expand Down
34 changes: 29 additions & 5 deletions homeassistant/components/zwave/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import voluptuous as vol

from homeassistant.core import callback
from homeassistant.loader import get_platform
from homeassistant.helpers import discovery
from homeassistant.const import (
ATTR_BATTERY_LEVEL, ATTR_LOCATION, ATTR_ENTITY_ID, ATTR_WAKEUP,
Expand Down Expand Up @@ -54,7 +56,11 @@
DEFAULT_CONF_REFRESH_DELAY = 2
DOMAIN = 'zwave'

DATA_ZWAVE_DICT = 'zwave_devices'

NETWORK = None


DATA_DEVICE_CONFIG = 'zwave_device_config'

# List of tuple (DOMAIN, discovered service, supported command classes,
Expand Down Expand Up @@ -264,6 +270,11 @@ def get_config_value(node, value_index, tries=5):
return None


def get_device(hass, dict_id):
"""Return Zwave Entity device."""
return hass.data[DATA_ZWAVE_DICT].pop(dict_id)


# pylint: disable=R0914
def setup(hass, config):
"""Setup Z-Wave.
Expand Down Expand Up @@ -310,6 +321,7 @@ def setup(hass, config):
options.lock()

NETWORK = ZWaveNetwork(options, autostart=False)
hass.data[DATA_ZWAVE_DICT] = {}

if use_debug:
def log_all(signal, value=None):
Expand Down Expand Up @@ -399,11 +411,20 @@ def value_added(node, value):
value.enable_poll(polling_intensity)
else:
value.disable_poll()
platform = get_platform(component, DOMAIN)
device = platform.get_device(
node=node, value=value, node_config=node_config, hass=hass)

@balloob balloob Feb 19, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can move node_config out of hass.data since we pass it in here.

I am not a big fan of using **kwargs for get_device. I think that we should standardize it.

get_config(node, value, node_config):

There is no hass there because this all about creating the device. Hass will be set on the entity once added to hass.

@andrey-git andrey-git Feb 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Climate needs hass to pull the default units.

@andrey-git andrey-git Feb 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also most platforms don't need all 4 - won't it look better if each one would "declare" only what it needs?

if device:

@balloob balloob Feb 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use a guard clause.

if not device:
    return

@andrey-git andrey-git Feb 22, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

dict_id = value.value_id

@callback

@andrey-git andrey-git Feb 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When this was a callback it was never executed

def discover_device(component, device, dict_id):
"""Put device in a dictionary and call discovery on it."""
hass.data[DATA_ZWAVE_DICT][dict_id] = device
discovery.load_platform(hass, component, DOMAIN, {

@emlove emlove Feb 19, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should probably use the async equivalent here, since we're already in the event loop at this point. No sense in creating a new job: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/helpers/discovery.py#L136 There's nothing waiting for discover_device to return.

@balloob balloob Feb 19, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

load_platform will blow up when used inside the event loop, so yes. We should.

@andrey-git andrey-git Feb 20, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

const.DISCOVERY_DEVICE: dict_id}, config)

discovery.load_platform(hass, component, DOMAIN, {
const.ATTR_NODE_ID: node.node_id,
const.ATTR_VALUE_ID: value.value_id,
}, config)
hass.add_job(discover_device, component, device, dict_id)

def scene_activated(node, scene_id):
"""Called when a scene is activated on any node in the network."""
Expand Down Expand Up @@ -694,7 +715,10 @@ def value_changed(self, value):
"""Called when a value for this entity's node has changed."""
self._update_attributes()
self.update_properties()
self.schedule_update_ha_state()
# If value changed after device was createed but before setup_platform
# was called - skip updating state.
if self.hass:
self.schedule_update_ha_state()

def _update_attributes(self):
"""Update the node attributes. May only be used inside callback."""
Expand Down
2 changes: 2 additions & 0 deletions homeassistant/components/zwave/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
ATTR_CONFIG_VALUE = "value"
NETWORK_READY_WAIT_SECS = 30

DISCOVERY_DEVICE = 'device'

SERVICE_CHANGE_ASSOCIATION = "change_association"
SERVICE_ADD_NODE = "add_node"
SERVICE_ADD_NODE_SECURE = "add_node_secure"
Expand Down