Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
152 changes: 149 additions & 3 deletions homeassistant/components/binary_sensor/zha.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,21 @@ async def async_setup_platform(hass, config, async_add_devices,
if discovery_info is None:
return

from zigpy.zcl.clusters.general import OnOff
from zigpy.zcl.clusters.security import IasZone
if IasZone.cluster_id in discovery_info['in_clusters']:
await _async_setup_iaszone(hass, config, async_add_devices,
discovery_info)
elif OnOff.cluster_id in discovery_info['out_clusters']:
await _async_setup_remote(hass, config, async_add_devices,
discovery_info)

in_clusters = discovery_info['in_clusters']

async def _async_setup_iaszone(hass, config, async_add_devices,
discovery_info):
device_class = None
cluster = in_clusters[IasZone.cluster_id]
from zigpy.zcl.clusters.security import IasZone
cluster = discovery_info['in_clusters'][IasZone.cluster_id]
if discovery_info['new_join']:
await cluster.bind()
ieee = cluster.endpoint.device.application.ieee
Expand All @@ -53,8 +62,34 @@ async def async_setup_platform(hass, config, async_add_devices,
async_add_devices([sensor], update_before_add=True)


async def _async_setup_remote(hass, config, async_add_devices, discovery_info):

async def safe(coro):
"""Run coro, catching ZigBee delivery errors, and ignoring them."""
import zigpy.exceptions
try:
await coro
except zigpy.exceptions.DeliveryError as exc:
_LOGGER.warning("Ignoring error during setup: %s", exc)

if discovery_info['new_join']:
from zigpy.zcl.clusters.general import OnOff, LevelControl
out_clusters = discovery_info['out_clusters']
if OnOff.cluster_id in out_clusters:
cluster = out_clusters[OnOff.cluster_id]
await safe(cluster.bind())
await safe(cluster.configure_reporting(0, 0, 600, 1))
if LevelControl.cluster_id in out_clusters:
cluster = out_clusters[LevelControl.cluster_id]
await safe(cluster.bind())
await safe(cluster.configure_reporting(0, 1, 600, 1))

sensor = Switch(**discovery_info)
async_add_devices([sensor], update_before_add=True)


class BinarySensor(zha.Entity, BinarySensorDevice):
"""THe ZHA Binary Sensor."""
"""The ZHA Binary Sensor."""

_domain = DOMAIN

Expand Down Expand Up @@ -102,3 +137,114 @@ async def async_update(self):
state = result.get('zone_status', self._state)
if isinstance(state, (int, uint16_t)):
self._state = result.get('zone_status', self._state) & 3


class Switch(zha.Entity, BinarySensorDevice):
"""ZHA switch/remote controller/button."""

_domain = DOMAIN

class OnOffListener:
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.

Why nest the listener classes?

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.

The idea was that they are pretty tightly bound to the Switch class, and are only classes because of the zigpy API. shrug

"""Listener for the OnOff ZigBee cluster."""

def __init__(self, entity):
"""Initialize OnOffListener."""
self._entity = entity

def cluster_command(self, tsn, command_id, args):
"""Handle commands received to this cluster."""
if command_id in (0x0000, 0x0040):
self._entity.set_state(False)
elif command_id in (0x0001, 0x0041, 0x0042):
self._entity.set_state(True)
elif command_id == 0x0002:
self._entity.set_state(not self._entity.is_on)

def attribute_updated(self, attrid, value):
"""Handle attribute updates on this cluster."""
if attrid == 0:
self._entity.set_state(value)
self._entity.schedule_update_ha_state()

def zdo_command(self, *args, **kwargs):
"""Handle ZDO commands on this cluster."""
pass

class LevelListener:
"""Listener for the LevelControl ZigBee cluster."""

def __init__(self, entity):
"""Initialize LevelListener."""
self._entity = entity

def cluster_command(self, tsn, command_id, args):
"""Handle commands received to this cluster."""
if command_id in (0x0000, 0x0004): # move_to_level, -with_on_off
self._entity.set_level(args[0])
elif command_id in (0x0001, 0x0005): # move, -with_on_off
# We should dim slowly -- for now, just step once
rate = args[1]
if args[0] == 0xff:
rate = 10 # Should read default move rate
self._entity.move_level(-rate if args[0] else rate)
elif command_id == 0x0002: # step
# Step (technically shouldn't change on/off)
self._entity.move_level(-args[1] if args[0] else args[1])

def attribute_update(self, attrid, value):
"""Handle attribute updates on this cluster."""
if attrid == 0:
self._entity.set_level(value)

def zdo_command(self, *args, **kwargs):
"""Handle ZDO commands on this cluster."""
pass

def __init__(self, **kwargs):
"""Initialize Switch."""
self._state = True
self._level = 255
from zigpy.zcl.clusters import general
self._out_listeners = {
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Apr 30, 2018

Choose a reason for hiding this comment

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

It's safer to register state update callbacks in async_added_to_hass entity coroutine. If a callback is called before the entity has been added to home assistant, the call to schedule_update_ha_state will error.

I suggest you define and initialize the listener dicts (_in_listeners and _out_listeners) as instance attributes on the parent class, instead of class attributes. And set the listener items in async_added_to_hass in this child class.

general.OnOff.cluster_id: self.OnOffListener(self),
general.LevelControl.cluster_id: self.LevelListener(self),
}
super().__init__(**kwargs)

@property
def is_on(self) -> bool:
"""Return true if the binary sensor is on."""
return self._state

@property
def device_state_attributes(self):
"""Return the device state attributes."""
return {'level': self._state and self._level or 0}

def move_level(self, change):
"""Increment the level, setting state if appropriate."""
if not self._state and change > 0:
self._level = 0
self._level = min(255, max(0, self._level + change))
self._state = bool(self._level)
self.schedule_update_ha_state()
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Apr 30, 2018

Choose a reason for hiding this comment

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

Will these callbacks move_level, set_level etc, be called from a thread and not from within the event loop, ie that's why we're using the sync version of this method?

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.

None of the current radios run on a different thread. And I think it's reasonable to expect them not to. I'll update.


def set_level(self, level):
"""Set the level, setting state if appropriate."""
self._level = level
self._state = bool(self._level)
self.schedule_update_ha_state()

def set_state(self, state):
"""Set the state."""
self._state = state
if self._level == 0:
self._level = 255
self.schedule_update_ha_state()

async def async_update(self):
"""Retrieve latest state."""
from zigpy.zcl.clusters.general import OnOff
result = await zha.safe_read(
self._endpoint.out_clusters[OnOff.cluster_id], ['on_off'])
self._state = result.get('on_off', self._state)
97 changes: 66 additions & 31 deletions homeassistant/components/zha/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,44 +221,78 @@ async def async_device_initialized(self, device, join):
self._config,
)

for cluster_id, cluster in endpoint.in_clusters.items():
cluster_type = type(cluster)
if cluster_id in profile_clusters[0]:
continue
if cluster_type not in zha_const.SINGLE_CLUSTER_DEVICE_CLASS:
continue

component = zha_const.SINGLE_CLUSTER_DEVICE_CLASS[cluster_type]
cluster_key = "{}-{}".format(device_key, cluster_id)
discovery_info = {
'application_listener': self,
'endpoint': endpoint,
'in_clusters': {cluster.cluster_id: cluster},
'out_clusters': {},
'new_join': join,
'unique_id': cluster_key,
'entity_suffix': '_{}'.format(cluster_id),
}
discovery_info.update(discovered_info)
self._hass.data[DISCOVERY_KEY][cluster_key] = discovery_info
for cluster in endpoint.in_clusters.values():
await self._attempt_single_cluster_device(
endpoint,
cluster,
profile_clusters[0],
device_key,
zha_const.SINGLE_INPUT_CLUSTER_DEVICE_CLASS,
'in_clusters',
discovered_info,
join,
)

await discovery.async_load_platform(
self._hass,
component,
DOMAIN,
{'discovery_key': cluster_key},
self._config,
for cluster in endpoint.out_clusters.values():
await self._attempt_single_cluster_device(
endpoint,
cluster,
profile_clusters[1],
device_key,
zha_const.SINGLE_OUTPUT_CLUSTER_DEVICE_CLASS,
'out_clusters',
discovered_info,
join,
)

def register_entity(self, ieee, entity_obj):
"""Record the creation of a hass entity associated with ieee."""
self._device_registry[ieee].append(entity_obj)

async def _attempt_single_cluster_device(self, endpoint, cluster,
profile_clusters, device_key,
device_classes, discovery_attr,
entity_info, is_new_join):
"""Try to set up an entity from a "bare" cluster."""
if cluster.cluster_id in profile_clusters:
return
# pylint: disable=unidiomatic-typecheck
if type(cluster) not in device_classes:
return

component = device_classes[type(cluster)]
cluster_key = "{}-{}".format(device_key, cluster.cluster_id)
discovery_info = {
'application_listener': self,
'endpoint': endpoint,
'in_clusters': {},
'out_clusters': {},
'new_join': is_new_join,
'unique_id': cluster_key,
'entity_suffix': '_{}'.format(cluster.cluster_id),
}
discovery_info[discovery_attr] = {cluster.cluster_id: cluster}
discovery_info.update(entity_info)
self._hass.data[DISCOVERY_KEY][cluster_key] = discovery_info

await discovery.async_load_platform(
self._hass,
component,
DOMAIN,
{'discovery_key': cluster_key},
self._config,
)


class Entity(entity.Entity):
"""A base class for ZHA entities."""

_domain = None # Must be overridden by subclasses
# Normally the entity itself is the listener. Base classes may set this to
# a dict of cluster ID -> listener to receive messages for specific
# clusters separately
_in_listeners = {}
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.

Having mutable objects as class attributes is dangerous and prone to bugs.

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.

Looks like these are used as a class wide registry of listeners. Will there never be a problem of one instance overwriting a listener of another instance? Are cluster_id s unique to each entity?

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Apr 30, 2018

Choose a reason for hiding this comment

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

So it looks like we replace the class attribute with an instance attribute of the same name when instantiating the child entity. So then it's not a class wide registry anymore, which is good. But I'd suggest to rewrite this logic anyway. See my comment about a safer way to handle state update callbacks.

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.

It's not ever replaced.

~/src/home-assistant/homeassistant/components$ egrep -r '(in|out)_listeners' .
./zha/__init__.py:    _in_listeners = {}
./zha/__init__.py:    _out_listeners = {}
./zha/__init__.py:            cluster.add_listener(self._in_listeners.get(cluster_id, self))
./zha/__init__.py:            cluster.add_listener(self._out_listeners.get(cluster_id, self))
./binary_sensor/zha.py:        self._out_listeners = {

Anyways, doing in in async_added_to_hass would be better.

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.

The last line in the grep replaces the class attribute with an instance attribute in the entity name space.

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.

Uh, I see what you mean. Yes, you're right (except it doesn't need to be an instance attribute).

_out_listeners = {}

def __init__(self, endpoint, in_clusters, out_clusters, manufacturer,
model, application_listener, unique_id, **kwargs):
Expand Down Expand Up @@ -287,10 +321,11 @@ def __init__(self, endpoint, in_clusters, out_clusters, manufacturer,
kwargs.get('entity_suffix', ''),
)

for cluster in in_clusters.values():
cluster.add_listener(self)
for cluster in out_clusters.values():
cluster.add_listener(self)
for cluster_id, cluster in in_clusters.items():
cluster.add_listener(self._in_listeners.get(cluster_id, self))
for cluster_id, cluster in out_clusters.items():
cluster.add_listener(self._out_listeners.get(cluster_id, self))

self._endpoint = endpoint
self._in_clusters = in_clusters
self._out_clusters = out_clusters
Expand Down Expand Up @@ -379,7 +414,7 @@ async def safe_read(cluster, attributes):
try:
result, _ = await cluster.read_attributes(
attributes,
allow_cache=False,
allow_cache=True,
)
return result
except Exception: # pylint: disable=broad-except
Expand Down
19 changes: 17 additions & 2 deletions homeassistant/components/zha/const.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"""All constants related to the ZHA component."""

DEVICE_CLASS = {}
SINGLE_CLUSTER_DEVICE_CLASS = {}
SINGLE_INPUT_CLUSTER_DEVICE_CLASS = {}
SINGLE_OUTPUT_CLUSTER_DEVICE_CLASS = {}
COMPONENT_CLUSTERS = {}


Expand All @@ -15,11 +16,17 @@ def populate_data():
from zigpy.profiles import PROFILES, zha, zll

DEVICE_CLASS[zha.PROFILE_ID] = {
zha.DeviceType.ON_OFF_SWITCH: 'binary_sensor',
zha.DeviceType.LEVEL_CONTROL_SWITCH: 'binary_sensor',
zha.DeviceType.REMOTE_CONTROL: 'binary_sensor',
zha.DeviceType.SMART_PLUG: 'switch',

zha.DeviceType.ON_OFF_LIGHT: 'light',
zha.DeviceType.DIMMABLE_LIGHT: 'light',
zha.DeviceType.COLOR_DIMMABLE_LIGHT: 'light',
zha.DeviceType.ON_OFF_LIGHT_SWITCH: 'binary_sensor',
zha.DeviceType.DIMMER_SWITCH: 'binary_sensor',
zha.DeviceType.COLOR_DIMMER_SWITCH: 'binary_sensor',
}
DEVICE_CLASS[zll.PROFILE_ID] = {
zll.DeviceType.ON_OFF_LIGHT: 'light',
Expand All @@ -29,15 +36,23 @@ def populate_data():
zll.DeviceType.COLOR_LIGHT: 'light',
zll.DeviceType.EXTENDED_COLOR_LIGHT: 'light',
zll.DeviceType.COLOR_TEMPERATURE_LIGHT: 'light',
zll.DeviceType.COLOR_CONTROLLER: 'binary_sensor',
zll.DeviceType.COLOR_SCENE_CONTROLLER: 'binary_sensor',
zll.DeviceType.CONTROLLER: 'binary_sensor',
zll.DeviceType.SCENE_CONTROLLER: 'binary_sensor',
zll.DeviceType.ON_OFF_SENSOR: 'binary_sensor',
}

SINGLE_CLUSTER_DEVICE_CLASS.update({
SINGLE_INPUT_CLUSTER_DEVICE_CLASS.update({
zcl.clusters.general.OnOff: 'switch',
zcl.clusters.measurement.RelativeHumidity: 'sensor',
zcl.clusters.measurement.TemperatureMeasurement: 'sensor',
zcl.clusters.security.IasZone: 'binary_sensor',
zcl.clusters.hvac.Fan: 'fan',
})
SINGLE_OUTPUT_CLUSTER_DEVICE_CLASS.update({
zcl.clusters.general.OnOff: 'binary_sensor',
})

# A map of hass components to all Zigbee clusters it could use
for profile_id, classes in DEVICE_CLASS.items():
Expand Down