From 3b8a2a72043c383568cabb999fca5244608c9282 Mon Sep 17 00:00:00 2001 From: Unknown Date: Wed, 15 Nov 2017 13:39:36 -0800 Subject: [PATCH 01/16] Fire events for ISY994 control events This allows hass to react directly to Insteon button presses (on switches and remotes), including presses, double-presses, and long holds --- homeassistant/components/isy994.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index 7686eb7dc7de89..3942d61702f1f8 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -231,11 +231,22 @@ def __init__(self, node) -> None: self._change_handler = self._node.status.subscribe( 'changed', self.on_update) + if hasattr(self._node, 'controlEvents'): + self._control_handler = self._node.controlEvents.subscribe( + self.on_control) + # pylint: disable=unused-argument def on_update(self, event: object) -> None: """Handle the update event from the ISY994 Node.""" self.schedule_update_ha_state() + def on_control(self, event: object) -> None: + """Handle a control event from the ISY994 Node.""" + self.hass.bus.fire('isy994_control', { + 'entity_id': self.entity_id, + 'control': event + }) + @property def domain(self) -> str: """Get the domain of the device.""" From 72bbd5d8464bd6b50c96e2f318c180c485a729de Mon Sep 17 00:00:00 2001 From: Unknown Date: Sat, 25 Nov 2017 21:51:59 -0800 Subject: [PATCH 02/16] Move change event subscription to after entity is added to hass The event handler method requires `self.hass` to exist, which doesn't have a value until the async_added_to_hass method is called. Should eliminate a race condition. --- homeassistant/components/isy994.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index 3942d61702f1f8..dbdb294912f933 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -4,6 +4,7 @@ For configuration details please visit the documentation for this component at https://home-assistant.io/components/isy994/ """ +import asyncio from collections import namedtuple import logging from urllib.parse import urlparse @@ -227,7 +228,12 @@ class ISYDevice(Entity): def __init__(self, node) -> None: """Initialize the insteon device.""" self._node = node + self._change_handler = None + self._control_handler = None + @asyncio.coroutine + def async_added_to_hass(self) -> None: + """Subscribe to the node change events.""" self._change_handler = self._node.status.subscribe( 'changed', self.on_update) From f10822fd320d66e0117fbec2f9506c8fc85e5766 Mon Sep 17 00:00:00 2001 From: Unknown Date: Wed, 22 Nov 2017 23:24:27 -0800 Subject: [PATCH 03/16] Overhaul binary sensors in ISY994 to be functional "out of the box" We now smash all of the subnodes from the ISY994 in to one Hass binary_sensor, and automatically support both paradigms of state reporting that Insteon sensors can do. Sometimes a single node's state represents the sensor's state, other times two nodes are used and only "ON" events are sent from each. The logic between the two forunately do not conflict so we can support both without knowing which mode the device is in. This also allows us to handle the heartbeat functionality that certain sensors have - we simply store the timestamp of the heartbeat as an attribute on the sensor device. It defaults to Unknown on bootup if and only if the device supports heartbeats, due to the presence of subnode 4. --- .../components/binary_sensor/isy994.py | 125 +++++++++++++++++- homeassistant/components/isy994.py | 12 +- 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index fd6269e3630267..f147243aef60ff 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -5,11 +5,12 @@ https://home-assistant.io/components/binary_sensor.isy994/ """ import logging +from datetime import datetime from typing import Callable # noqa from homeassistant.components.binary_sensor import BinarySensorDevice, DOMAIN import homeassistant.components.isy994 as isy -from homeassistant.const import STATE_ON, STATE_OFF +from homeassistant.const import STATE_ON, STATE_OFF, STATE_UNKNOWN from homeassistant.helpers.typing import ConfigType _LOGGER = logging.getLogger(__name__) @@ -32,10 +33,27 @@ def setup_platform(hass, config: ConfigType, return False devices = [] + devices_by_nid = {} + child_nodes = [] for node in isy.filter_nodes(isy.SENSOR_NODES, units=UOM, states=STATES): - devices.append(ISYBinarySensorDevice(node)) + if node.parent_node is None: + device = ISYBinarySensorDevice(node) + devices.append(device) + devices_by_nid[node.nid] = device + else: + # We'll process the child nodes last, to ensure all parent nodes + # have been processed + child_nodes.append(node) + + for node in child_nodes: + try: + devices_by_nid[node.parent_node.nid].add_child_node(node) + except KeyError: + _LOGGER.warning("Node %s has a parent node %s, but no device " + "was created for the parent. Skipping.", + node.nid, node.parent_nid) for program in isy.PROGRAMS.get(DOMAIN, []): try: @@ -49,22 +67,115 @@ def setup_platform(hass, config: ConfigType, class ISYBinarySensorDevice(isy.ISYDevice, BinarySensorDevice): - """Representation of an ISY994 binary sensor device.""" + """Representation of an ISY994 binary sensor device. + + Often times, a single device is represented by multiple nodes in the ISY, + allowing for different nuances in how those devices report their on and + off events. This class turns those multiple nodes in to a single Hass + entity and handles both ways that ISY binary sensors can work. + """ def __init__(self, node) -> None: """Initialize the ISY994 binary sensor device.""" isy.ISYDevice.__init__(self, node) + self._child_nodes = [] + self._off_node = None + self._heartbeat_node = None + self._heartbeat_timestamp = None + # pylint: disable=protected-access + self._computed_state = bool(self._node.status._val) + node.controlEvents.subscribe(self._positive_node_control_handler) + + def add_child_node(self, child): + """Add a child node to this binary sensor device. + + The child node can either be a node that receives the 'off' events, or + a heartbeat node for reporting that this device is still alive + """ + subnode_id = int(child.nid[-1]) + if subnode_id == 2: + # "Negative" node that can be used to represent a negative state + # when it reports "On" + child.controlEvents.subscribe(self._negative_node_control_handler) + self._off_node = child + elif subnode_id == 4: + # Heartbeat node that just reports "On" every 24 hours + child.controlEvents.subscribe(self._heartbeat_node_control_handler) + self._heartbeat_timestamp = STATE_UNKNOWN + self._heartbeat_node = child + + def _negative_node_control_handler(self, event: object) -> None: + """Handle an "On" control event from the "negative" node.""" + if event == 'DON': + _LOGGER.debug("Sensor %s turning Off via the Negative node " + "sending a DON command", self.name) + self._computed_state = False + self.schedule_update_ha_state() + + def _positive_node_control_handler(self, event: object) -> None: + """Handle On and Off control event coming from the primary node. + + Depending on device configuration, sometimes only On events + will come to this node, with the negative node representing Off + events + """ + if event == 'DON': + _LOGGER.debug("Sensor %s turning On via the Primary node " + "sending a DON command", self.name) + self._computed_state = True + self.schedule_update_ha_state() + if event == 'DOF': + _LOGGER.debug("Sensor %s turning Off via the Primary node " + "sending a DOF command", self.name) + self._computed_state = False + self.schedule_update_ha_state() + + def _heartbeat_node_control_handler(self, event: object) -> None: + """Update the heartbeat timestamp when an On event is sent.""" + if event == 'DON': + self._heartbeat_timestamp = datetime.now().isoformat() + + # pylint: disable=unused-argument + def on_update(self, event: object) -> None: + """Ignore primary node status updates. + + We listen directly to the Control events on all nodes for this + device. + """ + pass + + @property + def value(self) -> bool: + """Get the current value of the device.""" + return self._computed_state @property def is_on(self) -> bool: """Get whether the ISY994 binary sensor device is on.""" - return bool(self.value) + return self.value + + @property + def device_state_attributes(self): + """Get the state attributes for the device.""" + attr = super(ISYBinarySensorDevice, self).device_state_attributes + if self._heartbeat_timestamp is not None: + attr['last_heartbeat'] = self._heartbeat_timestamp + return attr + +class ISYBinarySensorProgram(isy.ISYDevice, BinarySensorDevice): + """Representation of an ISY994 binary sensor program. -class ISYBinarySensorProgram(ISYBinarySensorDevice): - """Representation of an ISY994 binary sensor program.""" + This does not need all of the subnode logic in the device version of binary + sensors. + """ def __init__(self, name, node) -> None: """Initialize the ISY994 binary sensor program.""" - ISYBinarySensorDevice.__init__(self, node) + isy.ISYDevice.__init__(self, node) self._name = name + + @property + def is_on(self) -> bool: + """Get whether the ISY994 binary sensor device is on.""" + return bool(self.value) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index dbdb294912f933..e74aa99d140040 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -92,6 +92,16 @@ def filter_nodes(nodes: list, units: list=None, states: list=None) -> list: return filtered_nodes +def _is_node_a_sensor(node, path: str, sensor_identifier: str) -> bool: + if sensor_identifier in path or sensor_identifier in node.name: + return True + + try: + return node.node_def_id == 'BinaryAlarm' + except AttributeError: + return False + + def _categorize_nodes(hidden_identifier: str, sensor_identifier: str) -> None: """Categorize the ISY994 nodes.""" global SENSOR_NODES @@ -107,7 +117,7 @@ def _categorize_nodes(hidden_identifier: str, sensor_identifier: str) -> None: hidden = hidden_identifier in path or hidden_identifier in node.name if hidden: node.name += hidden_identifier - if sensor_identifier in path or sensor_identifier in node.name: + if _is_node_a_sensor(node, path, sensor_identifier): SENSOR_NODES.append(node) elif isinstance(node, PYISY.Nodes.Node): NODES.append(node) From 6cbe3b7ae944077dfd3f2c7df90286a5c99de615 Mon Sep 17 00:00:00 2001 From: Unknown Date: Sat, 25 Nov 2017 15:54:55 -0800 Subject: [PATCH 04/16] Parse the binary sensor device class from the ISY's device "type" Now we automatically know which sensors are moisture, motion, and openings! (We also reverse the moisture sensor state, because Insteon reports ON for dry on the primary node.) --- .../components/binary_sensor/isy994.py | 58 ++++++++++++++++--- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index f147243aef60ff..1cd890e0abf0aa 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -15,14 +15,15 @@ _LOGGER = logging.getLogger(__name__) -VALUE_TO_STATE = { - False: STATE_OFF, - True: STATE_ON, -} - UOM = ['2', '78'] STATES = [STATE_OFF, STATE_ON, 'true', 'false'] +ISY_DEVICE_TYPES = { + 'moisture': ['16.8', '16.13', '16.14'], + 'opening': ['16.9', '16.6', '16.7', '16.2', '16.17', '16.20', '16.21'], + 'motion': ['16.1', '16.4', '16.5', '16.3'] +} + # pylint: disable=unused-argument def setup_platform(hass, config: ConfigType, @@ -51,9 +52,9 @@ def setup_platform(hass, config: ConfigType, try: devices_by_nid[node.parent_node.nid].add_child_node(node) except KeyError: - _LOGGER.warning("Node %s has a parent node %s, but no device " - "was created for the parent. Skipping.", - node.nid, node.parent_nid) + _LOGGER.error("Node %s has a parent node %s, but no device " + "was created for the parent. Skipping.", + node.nid, node.parent_nid) for program in isy.PROGRAMS.get(DOMAIN, []): try: @@ -82,10 +83,29 @@ def __init__(self, node) -> None: self._off_node = None self._heartbeat_node = None self._heartbeat_timestamp = None + self._device_class_from_type = self._detect_device_type() # pylint: disable=protected-access self._computed_state = bool(self._node.status._val) node.controlEvents.subscribe(self._positive_node_control_handler) + def _detect_device_type(self) -> str: + try: + device_type = self._node.type + except AttributeError: + self._device_class_from_type = None + return None + + split_type = device_type.split('.') + _LOGGER.debug(split_type) + for cls, ids in ISY_DEVICE_TYPES.items(): + _LOGGER.debug(split_type[0] + '.' + split_type[1]) + if split_type[0] + '.' + split_type[1] in ids: + self._device_class_from_type = cls + return cls + + self._device_class_from_type = None + return None + def add_child_node(self, child): """Add a child node to this binary sensor device. @@ -134,6 +154,7 @@ def _heartbeat_node_control_handler(self, event: object) -> None: """Update the heartbeat timestamp when an On event is sent.""" if event == 'DON': self._heartbeat_timestamp = datetime.now().isoformat() + self.schedule_update_ha_state() # pylint: disable=unused-argument def on_update(self, event: object) -> None: @@ -146,7 +167,18 @@ def on_update(self, event: object) -> None: @property def value(self) -> bool: - """Get the current value of the device.""" + """Get the current value of the device. + + Insteon leak sensors set their primary node to On when the state is + DRY, not WET, so we invert the binary state if the user indicates + that it is a moisture sensor. + """ + try: + if self.device_class == 'moisture': + return not self._computed_state + except AttributeError: + pass + return self._computed_state @property @@ -154,6 +186,14 @@ def is_on(self) -> bool: """Get whether the ISY994 binary sensor device is on.""" return self.value + @property + def device_class(self) -> str: + """Return the class of this device. + + This was discovered by parsing the device type code during init + """ + return self._device_class_from_type + @property def device_state_attributes(self): """Get the state attributes for the device.""" From 7f9189c8c7d2ab265cd6ded4bc05489443804637 Mon Sep 17 00:00:00 2001 From: Unknown Date: Sat, 25 Nov 2017 21:42:00 -0800 Subject: [PATCH 05/16] Code review tweaks The one material change here is that the event subscribers were moved to the `async_added_to_hass` method, as the handlers depend on things that only exist after the entity has been added. --- .../components/binary_sensor/isy994.py | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index 1cd890e0abf0aa..59b128c6282c7a 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -4,6 +4,8 @@ For more details about this platform, please refer to the documentation at https://home-assistant.io/components/binary_sensor.isy994/ """ + +import asyncio import logging from datetime import datetime from typing import Callable # noqa @@ -78,32 +80,46 @@ class ISYBinarySensorDevice(isy.ISYDevice, BinarySensorDevice): def __init__(self, node) -> None: """Initialize the ISY994 binary sensor device.""" - isy.ISYDevice.__init__(self, node) - self._child_nodes = [] - self._off_node = None + super().__init__(node) + self._negative_node = None self._heartbeat_node = None self._heartbeat_timestamp = None self._device_class_from_type = self._detect_device_type() # pylint: disable=protected-access self._computed_state = bool(self._node.status._val) - node.controlEvents.subscribe(self._positive_node_control_handler) + + @asyncio.coroutine + def async_added_to_hass(self) -> None: + """Subscribe to the node and subnode event emitters.""" + super().async_added_to_hass() + + self._node.controlEvents.subscribe(self._positive_node_control_handler) + + try: + self._negative_node.controlEvents.subscribe( + self._negative_node_control_handler) + except AttributeError: + # Heartbeat node doesn't exist + pass + + try: + self._heartbeat_node.controlEvents.subscribe( + self._heartbeat_node_control_handler) + except AttributeError: + # Heartbeat node doesn't exist + pass def _detect_device_type(self) -> str: try: device_type = self._node.type except AttributeError: - self._device_class_from_type = None return None split_type = device_type.split('.') - _LOGGER.debug(split_type) - for cls, ids in ISY_DEVICE_TYPES.items(): - _LOGGER.debug(split_type[0] + '.' + split_type[1]) + for device_class, ids in ISY_DEVICE_TYPES.items(): if split_type[0] + '.' + split_type[1] in ids: - self._device_class_from_type = cls - return cls + return device_class - self._device_class_from_type = None return None def add_child_node(self, child): @@ -116,11 +132,9 @@ def add_child_node(self, child): if subnode_id == 2: # "Negative" node that can be used to represent a negative state # when it reports "On" - child.controlEvents.subscribe(self._negative_node_control_handler) - self._off_node = child + self._negative_node = child elif subnode_id == 4: # Heartbeat node that just reports "On" every 24 hours - child.controlEvents.subscribe(self._heartbeat_node_control_handler) self._heartbeat_timestamp = STATE_UNKNOWN self._heartbeat_node = child @@ -197,7 +211,7 @@ def device_class(self) -> str: @property def device_state_attributes(self): """Get the state attributes for the device.""" - attr = super(ISYBinarySensorDevice, self).device_state_attributes + attr = super().device_state_attributes if self._heartbeat_timestamp is not None: attr['last_heartbeat'] = self._heartbeat_timestamp return attr @@ -212,7 +226,7 @@ class ISYBinarySensorProgram(isy.ISYDevice, BinarySensorDevice): def __init__(self, name, node) -> None: """Initialize the ISY994 binary sensor program.""" - isy.ISYDevice.__init__(self, node) + super().__init__(node) self._name = name @property From 489fbbc68ee260248c6334f36c679be57ac6731c Mon Sep 17 00:00:00 2001 From: Unknown Date: Sat, 25 Nov 2017 22:27:32 -0800 Subject: [PATCH 06/16] Handle cases where a sensor's state is unknown When the ISY first boots up, if a battery-powered sensor has not reported in yet (due to heartbeat or a change in state), the state is unknown until it does. --- .../components/binary_sensor/isy994.py | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index 59b128c6282c7a..ff6b3ee8b70f2d 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -86,7 +86,10 @@ def __init__(self, node) -> None: self._heartbeat_timestamp = None self._device_class_from_type = self._detect_device_type() # pylint: disable=protected-access - self._computed_state = bool(self._node.status._val) + if self._node.status._val == -1: + self._computed_state = None + else: + self._computed_state = bool(self._node.status._val) @asyncio.coroutine def async_added_to_hass(self) -> None: @@ -180,13 +183,17 @@ def on_update(self, event: object) -> None: pass @property - def value(self) -> bool: + def value(self) -> object: """Get the current value of the device. Insteon leak sensors set their primary node to On when the state is DRY, not WET, so we invert the binary state if the user indicates that it is a moisture sensor. """ + if self._computed_state is None: + # Do this first so we don't invert None on moisture sensors + return None + try: if self.device_class == 'moisture': return not self._computed_state @@ -197,8 +204,18 @@ def value(self) -> bool: @property def is_on(self) -> bool: - """Get whether the ISY994 binary sensor device is on.""" - return self.value + """Get whether the ISY994 binary sensor device is on. + + Note: This method will return false if the current state is UNKNOWN + """ + return bool(self.value) + + @property + def state(self): + """Return the state of the binary sensor.""" + if self._computed_state is None: + return STATE_UNKNOWN + return STATE_ON if self.is_on else STATE_OFF @property def device_class(self) -> str: From 4de13c4712d2af9e7212c124ef34cd954d191c3d Mon Sep 17 00:00:00 2001 From: Unknown Date: Sun, 26 Nov 2017 23:07:44 -0800 Subject: [PATCH 07/16] Clean up from code review Fix coroutine await, remove unnecessary exception check, and return None when state is unknown --- homeassistant/components/binary_sensor/isy994.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index ff6b3ee8b70f2d..66ea5ea7c3284c 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -94,7 +94,7 @@ def __init__(self, node) -> None: @asyncio.coroutine def async_added_to_hass(self) -> None: """Subscribe to the node and subnode event emitters.""" - super().async_added_to_hass() + yield from super().async_added_to_hass() self._node.controlEvents.subscribe(self._positive_node_control_handler) @@ -194,11 +194,8 @@ def value(self) -> object: # Do this first so we don't invert None on moisture sensors return None - try: - if self.device_class == 'moisture': - return not self._computed_state - except AttributeError: - pass + if self.device_class == 'moisture': + return not self._computed_state return self._computed_state @@ -214,7 +211,7 @@ def is_on(self) -> bool: def state(self): """Return the state of the binary sensor.""" if self._computed_state is None: - return STATE_UNKNOWN + return None return STATE_ON if self.is_on else STATE_OFF @property From efb000ac7a3db9e9525b96eb22dfb0e577bdb504 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Thu, 30 Nov 2017 13:23:55 -0800 Subject: [PATCH 08/16] Unknown value from PyISY is now -inf rather than -1 --- homeassistant/components/binary_sensor/isy994.py | 2 +- homeassistant/components/isy994.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index 66ea5ea7c3284c..042c6dc554693b 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -86,7 +86,7 @@ def __init__(self, node) -> None: self._heartbeat_timestamp = None self._device_class_from_type = self._detect_device_type() # pylint: disable=protected-access - if self._node.status._val == -1: + if self._node.status._val == -1*float('inf'): self._computed_state = None else: self._computed_state = bool(self._node.status._val) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index e74aa99d140040..449892b301fa16 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -295,6 +295,9 @@ def should_poll(self) -> bool: def value(self) -> object: """Get the current value of the device.""" # pylint: disable=protected-access + if self._node.status._val == -1*float('inf'): + return None + return self._node.status._val @property From 76b9299a7bf4d3b0ae942ac8e1f6ceed9ef2476b Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Mon, 4 Dec 2017 16:43:03 -0800 Subject: [PATCH 09/16] Move heartbeat handling to a separate sensor Now all heartbeat-compatible sensors will have a separate `binary_sensor` device that represents the battery state (on = dead) --- .../components/binary_sensor/isy994.py | 172 ++++++++++++++---- 1 file changed, 139 insertions(+), 33 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index 042c6dc554693b..16e885f87a93eb 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -7,13 +7,16 @@ import asyncio import logging -from datetime import datetime +from datetime import timedelta from typing import Callable # noqa +from homeassistant.core import callback from homeassistant.components.binary_sensor import BinarySensorDevice, DOMAIN import homeassistant.components.isy994 as isy -from homeassistant.const import STATE_ON, STATE_OFF, STATE_UNKNOWN +from homeassistant.const import STATE_ON, STATE_OFF from homeassistant.helpers.typing import ConfigType +from homeassistant.helpers.event import async_track_point_in_utc_time +from homeassistant.util import dt as dt_util _LOGGER = logging.getLogger(__name__) @@ -52,11 +55,21 @@ def setup_platform(hass, config: ConfigType, for node in child_nodes: try: - devices_by_nid[node.parent_node.nid].add_child_node(node) + parent_device = devices_by_nid[node.parent_node.nid] except KeyError: _LOGGER.error("Node %s has a parent node %s, but no device " "was created for the parent. Skipping.", node.nid, node.parent_nid) + else: + subnode_id = int(node.nid[-1]) + if subnode_id == 4: + # Subnode 4 is the heartbeat node, which we will represent + # as a separate binary_sensor + device = ISYBinarySensorHeartbeat(node, parent_device) + parent_device.add_heartbeat_device(device) + devices.append(device) + elif subnode_id == 2: + parent_device.add_negative_node(node) for program in isy.PROGRAMS.get(DOMAIN, []): try: @@ -82,8 +95,7 @@ def __init__(self, node) -> None: """Initialize the ISY994 binary sensor device.""" super().__init__(node) self._negative_node = None - self._heartbeat_node = None - self._heartbeat_timestamp = None + self._heartbeat_device = None self._device_class_from_type = self._detect_device_type() # pylint: disable=protected-access if self._node.status._val == -1*float('inf'): @@ -105,13 +117,6 @@ def async_added_to_hass(self) -> None: # Heartbeat node doesn't exist pass - try: - self._heartbeat_node.controlEvents.subscribe( - self._heartbeat_node_control_handler) - except AttributeError: - # Heartbeat node doesn't exist - pass - def _detect_device_type(self) -> str: try: device_type = self._node.type @@ -125,21 +130,30 @@ def _detect_device_type(self) -> str: return None - def add_child_node(self, child): - """Add a child node to this binary sensor device. + def add_heartbeat_device(self, device) -> None: + """Register a heartbeat device for this sensor. - The child node can either be a node that receives the 'off' events, or - a heartbeat node for reporting that this device is still alive + The heartbeat node beats on its own, but we can gain a little + reliability by considering any node activity for this sensor + to be a heartbeat as well. """ - subnode_id = int(child.nid[-1]) - if subnode_id == 2: - # "Negative" node that can be used to represent a negative state - # when it reports "On" - self._negative_node = child - elif subnode_id == 4: - # Heartbeat node that just reports "On" every 24 hours - self._heartbeat_timestamp = STATE_UNKNOWN - self._heartbeat_node = child + self._heartbeat_device = device + + def _heartbeat(self) -> None: + """Send a heartbeat to our heartbeat device, if we have one.""" + try: + self._heartbeat_device.heartbeat() + except AttributeError: + # No heartbeat device exists + pass + + def add_negative_node(self, child) -> None: + """Add a negative node to this binary sensor device. + + The negative node is a node that can receive the 'off' events + for the sensor, depending on device configuration and type. + """ + self._negative_node = child def _negative_node_control_handler(self, event: object) -> None: """Handle an "On" control event from the "negative" node.""" @@ -148,6 +162,7 @@ def _negative_node_control_handler(self, event: object) -> None: "sending a DON command", self.name) self._computed_state = False self.schedule_update_ha_state() + self._heartbeat() def _positive_node_control_handler(self, event: object) -> None: """Handle On and Off control event coming from the primary node. @@ -161,17 +176,13 @@ def _positive_node_control_handler(self, event: object) -> None: "sending a DON command", self.name) self._computed_state = True self.schedule_update_ha_state() + self._heartbeat() if event == 'DOF': _LOGGER.debug("Sensor %s turning Off via the Primary node " "sending a DOF command", self.name) self._computed_state = False self.schedule_update_ha_state() - - def _heartbeat_node_control_handler(self, event: object) -> None: - """Update the heartbeat timestamp when an On event is sent.""" - if event == 'DON': - self._heartbeat_timestamp = datetime.now().isoformat() - self.schedule_update_ha_state() + self._heartbeat() # pylint: disable=unused-argument def on_update(self, event: object) -> None: @@ -222,12 +233,107 @@ def device_class(self) -> str: """ return self._device_class_from_type + +class ISYBinarySensorHeartbeat(isy.ISYDevice, BinarySensorDevice): + """Representation of the battery state of an ISY994 sensor.""" + + def __init__(self, node, parent_device) -> None: + """Initialize the ISY994 binary sensor device.""" + super().__init__(node) + self._computed_state = None + self._parent_device = parent_device + self._heartbeat_timer = None + + @asyncio.coroutine + def async_added_to_hass(self) -> None: + """Subscribe to the node and subnode event emitters.""" + yield from super().async_added_to_hass() + + self._node.controlEvents.subscribe( + self._heartbeat_node_control_handler) + + # Start the timer on bootup, so we can change from UNKNOWN to ON + self._restart_timer() + + def _heartbeat_node_control_handler(self, event: object) -> None: + """Update the heartbeat timestamp when an On event is sent.""" + if event == 'DON': + self.heartbeat() + + def heartbeat(self): + """Mark the device as online, and restart the 25 hour timer. + + This gets called when the heartbeat node beats, but also when the + parent sensor sends any events, as we can trust that to mean the device + is online. This mitigates the risk of false positives due to a single + missed heartbeat event. + """ + self._computed_state = False + self._restart_timer() + self.schedule_update_ha_state() + + def _restart_timer(self): + """Restart the 25 hour timer.""" + try: + self._heartbeat_timer() + self._heartbeat_timer = None + except TypeError: + # No heartbeat timer is active + pass + + # pylint: disable=unused-argument + @callback + def timer_elapsed(now) -> None: + """Heartbeat missed; set state to indicate dead battery.""" + self._computed_state = True + self._heartbeat_timer = None + self.schedule_update_ha_state() + + point_in_time = dt_util.utcnow() + timedelta(hours=25) + _LOGGER.debug("Timer starting. Now: %s Then: %s", + dt_util.utcnow(), point_in_time) + + self._heartbeat_timer = async_track_point_in_utc_time( + self.hass, timer_elapsed, point_in_time) + + # pylint: disable=unused-argument + def on_update(self, event: object) -> None: + """Ignore node status updates. + + We listen directly to the Control events for this device. + """ + pass + + @property + def value(self) -> object: + """Get the current value of this sensor.""" + return self._computed_state + + @property + def is_on(self) -> bool: + """Get whether the ISY994 binary sensor device is on. + + Note: This method will return false if the current state is UNKNOWN + """ + return bool(self.value) + + @property + def state(self): + """Return the state of the binary sensor.""" + if self._computed_state is None: + return None + return STATE_ON if self.is_on else STATE_OFF + + @property + def device_class(self) -> str: + """Get the class of this device.""" + return 'battery' + @property def device_state_attributes(self): """Get the state attributes for the device.""" attr = super().device_state_attributes - if self._heartbeat_timestamp is not None: - attr['last_heartbeat'] = self._heartbeat_timestamp + attr['parent_entity_id'] = self._parent_device.entity_id return attr From 624ee066474e7df9bf60b7631fa221d7d33435c5 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Wed, 6 Dec 2017 14:22:37 -0800 Subject: [PATCH 10/16] Add support for Unknown state, which is being added in next PyISY PyISY will report unknown states as the number "-inf". This is implemented in the base ISY994 component, but subcomponents that override the `state` method needed some extra logic to handle it as well. --- homeassistant/components/cover/isy994.py | 5 ++++- homeassistant/components/isy994.py | 18 +++++++++++++++++- homeassistant/components/lock/isy994.py | 5 ++++- homeassistant/components/sensor/isy994.py | 3 +++ homeassistant/components/switch/isy994.py | 5 ++++- 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/cover/isy994.py b/homeassistant/components/cover/isy994.py index 1e83038278ceae..4dd1c9be364a11 100644 --- a/homeassistant/components/cover/isy994.py +++ b/homeassistant/components/cover/isy994.py @@ -69,7 +69,10 @@ def is_closed(self) -> bool: @property def state(self) -> str: """Get the state of the ISY994 cover device.""" - return VALUE_TO_STATE.get(self.value, STATE_OPEN) + if self.is_unknown(): + return None + else: + return VALUE_TO_STATE.get(self.value, STATE_OPEN) def open_cover(self, **kwargs) -> None: """Send the open cover command to the ISY994 cover device.""" diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index dbdb294912f933..b034e9439e70ef 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -107,7 +107,8 @@ def _categorize_nodes(hidden_identifier: str, sensor_identifier: str) -> None: hidden = hidden_identifier in path or hidden_identifier in node.name if hidden: node.name += hidden_identifier - if sensor_identifier in path or sensor_identifier in node.name: + if (sensor_identifier in path or sensor_identifier in node.name) \ + and isinstance(node, PYISY.Nodes.Node): SENSOR_NODES.append(node) elif isinstance(node, PYISY.Nodes.Node): NODES.append(node) @@ -287,6 +288,21 @@ def value(self) -> object: # pylint: disable=protected-access return self._node.status._val + def is_unknown(self) -> bool: + """Get whether or not the value of this Entity's node is unknown. + + PyISY reports unknown values as -inf + """ + return self.value == -1 * float('inf') + + @property + def state(self): + """Return the state of the ISY device.""" + if self.is_unknown(): + return None + else: + return super().state + @property def device_state_attributes(self) -> Dict: """Get the state attributes for the device.""" diff --git a/homeassistant/components/lock/isy994.py b/homeassistant/components/lock/isy994.py index edbb8a34f240f9..63272b90b1fa80 100644 --- a/homeassistant/components/lock/isy994.py +++ b/homeassistant/components/lock/isy994.py @@ -66,7 +66,10 @@ def is_locked(self) -> bool: @property def state(self) -> str: """Get the state of the lock.""" - return VALUE_TO_STATE.get(self.value, STATE_UNKNOWN) + if self.is_unknown(): + return None + else: + return VALUE_TO_STATE.get(self.value, STATE_UNKNOWN) def lock(self, **kwargs) -> None: """Send the lock command to the ISY994 device.""" diff --git a/homeassistant/components/sensor/isy994.py b/homeassistant/components/sensor/isy994.py index f64fa6191e2253..e961c63a1b511c 100644 --- a/homeassistant/components/sensor/isy994.py +++ b/homeassistant/components/sensor/isy994.py @@ -282,6 +282,9 @@ def raw_unit_of_measurement(self) -> str: @property def state(self) -> str: """Get the state of the ISY994 sensor device.""" + if self.is_unknown(): + return None + if len(self._node.uom) == 1: if self._node.uom[0] in UOM_TO_STATES: states = UOM_TO_STATES.get(self._node.uom[0]) diff --git a/homeassistant/components/switch/isy994.py b/homeassistant/components/switch/isy994.py index b930bedc2c7a4e..0f1ec62eaeef04 100644 --- a/homeassistant/components/switch/isy994.py +++ b/homeassistant/components/switch/isy994.py @@ -69,7 +69,10 @@ def is_on(self) -> bool: @property def state(self) -> str: """Get the state of the ISY994 device.""" - return VALUE_TO_STATE.get(bool(self.value), STATE_UNKNOWN) + if self.is_unknown(): + return None + else: + return VALUE_TO_STATE.get(bool(self.value), STATE_UNKNOWN) def turn_off(self, **kwargs) -> None: """Send the turn on command to the ISY994 switch.""" From b9f842264b23c1230247e9d4487a8eade8b31f04 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Thu, 7 Dec 2017 12:39:52 -0800 Subject: [PATCH 11/16] Change a couple try blocks to explicit None checks --- homeassistant/components/binary_sensor/isy994.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index 16e885f87a93eb..ddd0aa79451f03 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -110,17 +110,15 @@ def async_added_to_hass(self) -> None: self._node.controlEvents.subscribe(self._positive_node_control_handler) - try: + if self._negative_node is not None: self._negative_node.controlEvents.subscribe( self._negative_node_control_handler) - except AttributeError: - # Heartbeat node doesn't exist - pass def _detect_device_type(self) -> str: try: device_type = self._node.type except AttributeError: + # The type attribute didn't exist in the ISY's API response return None split_type = device_type.split('.') @@ -141,11 +139,8 @@ def add_heartbeat_device(self, device) -> None: def _heartbeat(self) -> None: """Send a heartbeat to our heartbeat device, if we have one.""" - try: + if self._heartbeat_device is not None: self._heartbeat_device.heartbeat() - except AttributeError: - # No heartbeat device exists - pass def add_negative_node(self, child) -> None: """Add a negative node to this binary sensor device. From e5da5bce3ca14ee5282d7d1c5bf5130dd76e7ed1 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Thu, 7 Dec 2017 23:52:58 -0800 Subject: [PATCH 12/16] Bump PyISY to 1.1.0, now that it has been published! --- homeassistant/components/isy994.py | 2 +- requirements_all.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index b034e9439e70ef..9012989e180eef 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -18,7 +18,7 @@ from homeassistant.helpers.entity import Entity from homeassistant.helpers.typing import ConfigType, Dict # noqa -REQUIREMENTS = ['PyISY==1.0.8'] +REQUIREMENTS = ['PyISY==1.1.0'] _LOGGER = logging.getLogger(__name__) diff --git a/requirements_all.txt b/requirements_all.txt index 840ed5a834ac7c..3d1d99180b1ca5 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -23,7 +23,7 @@ certifi>=2017.4.17 DoorBirdPy==0.1.0 # homeassistant.components.isy994 -PyISY==1.0.8 +PyISY==1.1.0 # homeassistant.components.notify.html5 PyJWT==1.5.3 From ccf3a9e68c239ce246dc182b7ccd3e3e8a824ff8 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Thu, 7 Dec 2017 23:57:24 -0800 Subject: [PATCH 13/16] Remove -inf checking from base component The implementation of the -inf checking was improved in another branch which has been merged in to this branch already. --- homeassistant/components/isy994.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index 0b1ed71951eb42..b5ef8e5ca16660 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -298,9 +298,6 @@ def should_poll(self) -> bool: def value(self) -> object: """Get the current value of the device.""" # pylint: disable=protected-access - if self._node.status._val == -1*float('inf'): - return None - return self._node.status._val def is_unknown(self) -> bool: From 92d8dd4f3596e114de006c80158ca02592bfbbda Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Mon, 11 Dec 2017 23:40:09 -0800 Subject: [PATCH 14/16] Restrict negative-node and heartbeat support to known compatible types Not all Insteon sensors use the same subnode IDs for the same things, so we need to use different logic depending on device type. Negative node and heartbeat support is now only used for leak sensors and open/close sensors. --- .../components/binary_sensor/isy994.py | 69 ++++++++++++------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index ddd0aa79451f03..6aed165f336c98 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -61,15 +61,24 @@ def setup_platform(hass, config: ConfigType, "was created for the parent. Skipping.", node.nid, node.parent_nid) else: - subnode_id = int(node.nid[-1]) - if subnode_id == 4: - # Subnode 4 is the heartbeat node, which we will represent - # as a separate binary_sensor - device = ISYBinarySensorHeartbeat(node, parent_device) - parent_device.add_heartbeat_device(device) + device_type = _detect_device_type(node) + if device_type in ['moisture', 'opening']: + subnode_id = int(node.nid[-1]) + # Leak and door/window sensors work the same way with negative + # nodes and heartbeat nodes + if subnode_id == 4: + # Subnode 4 is the heartbeat node, which we will represent + # as a separate binary_sensor + device = ISYBinarySensorHeartbeat(node, parent_device) + parent_device.add_heartbeat_device(device) + devices.append(device) + elif subnode_id == 2: + parent_device.add_negative_node(node) + else: + # We don't yet have any special logic for other sensor types, + # so add the nodes as individual devices + device = ISYBinarySensorDevice(node) devices.append(device) - elif subnode_id == 2: - parent_device.add_negative_node(node) for program in isy.PROGRAMS.get(DOMAIN, []): try: @@ -82,6 +91,26 @@ def setup_platform(hass, config: ConfigType, add_devices(devices) +def _detect_device_type(node) -> str: + try: + device_type = node.type + except AttributeError: + # The type attribute didn't exist in the ISY's API response + return None + + split_type = device_type.split('.') + for device_class, ids in ISY_DEVICE_TYPES.items(): + if split_type[0] + '.' + split_type[1] in ids: + return device_class + + return None + + +def _is_val_unknown(val): + """Determine if a number value represents UNKNOWN from PyISY.""" + return val == -1*float('inf') + + class ISYBinarySensorDevice(isy.ISYDevice, BinarySensorDevice): """Representation of an ISY994 binary sensor device. @@ -96,9 +125,9 @@ def __init__(self, node) -> None: super().__init__(node) self._negative_node = None self._heartbeat_device = None - self._device_class_from_type = self._detect_device_type() + self._device_class_from_type = _detect_device_type(self._node) # pylint: disable=protected-access - if self._node.status._val == -1*float('inf'): + if _is_val_unknown(self._node.status._val): self._computed_state = None else: self._computed_state = bool(self._node.status._val) @@ -114,20 +143,6 @@ def async_added_to_hass(self) -> None: self._negative_node.controlEvents.subscribe( self._negative_node_control_handler) - def _detect_device_type(self) -> str: - try: - device_type = self._node.type - except AttributeError: - # The type attribute didn't exist in the ISY's API response - return None - - split_type = device_type.split('.') - for device_class, ids in ISY_DEVICE_TYPES.items(): - if split_type[0] + '.' + split_type[1] in ids: - return device_class - - return None - def add_heartbeat_device(self, device) -> None: """Register a heartbeat device for this sensor. @@ -150,6 +165,12 @@ def add_negative_node(self, child) -> None: """ self._negative_node = child + if not _is_val_unknown(self._negative_node): + # If the negative node has a value, it means the negative node is + # in use for this device. Therefore, we cannot determine the state + # of the sensor until we receive our first ON event. + self._computed_state = None + def _negative_node_control_handler(self, event: object) -> None: """Handle an "On" control event from the "negative" node.""" if event == 'DON': From e12ee0f6e9105822ed6c027f450964657025f7b0 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Tue, 12 Dec 2017 10:40:42 -0800 Subject: [PATCH 15/16] Use new style string formatting --- homeassistant/components/binary_sensor/isy994.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/binary_sensor/isy994.py b/homeassistant/components/binary_sensor/isy994.py index 6aed165f336c98..a5b61c9ffedee5 100644 --- a/homeassistant/components/binary_sensor/isy994.py +++ b/homeassistant/components/binary_sensor/isy994.py @@ -100,7 +100,7 @@ def _detect_device_type(node) -> str: split_type = device_type.split('.') for device_class, ids in ISY_DEVICE_TYPES.items(): - if split_type[0] + '.' + split_type[1] in ids: + if '{}.{}'.format(split_type[0], split_type[1]) in ids: return device_class return None From fef351554e8663360db2645834ce19da97ad6517 Mon Sep 17 00:00:00 2001 From: Greg Laabs Date: Tue, 12 Dec 2017 12:10:35 -0800 Subject: [PATCH 16/16] Add binary sensor detection for pre-5.x firmware Meant to do this originally; writing documentation revealed that this requirement was missed! --- homeassistant/components/isy994.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/isy994.py b/homeassistant/components/isy994.py index b5ef8e5ca16660..af1846c7bf8173 100644 --- a/homeassistant/components/isy994.py +++ b/homeassistant/components/isy994.py @@ -93,16 +93,31 @@ def filter_nodes(nodes: list, units: list=None, states: list=None) -> list: def _is_node_a_sensor(node, path: str, sensor_identifier: str) -> bool: + """Determine if the given node is a sensor.""" if not isinstance(node, PYISY.Nodes.Node): return False if sensor_identifier in path or sensor_identifier in node.name: return True + # This method is most reliable but only works on 5.x firmware try: - return node.node_def_id == 'BinaryAlarm' + if node.node_def_id == 'BinaryAlarm': + return True except AttributeError: - return False + pass + + # This method works on all firmwares, but only for Insteon devices + try: + device_type = node.type + except AttributeError: + # Node has no type; most likely not an Insteon device + pass + else: + split_type = device_type.split('.') + return split_type[0] == '16' # 16 represents Insteon binary sensors + + return False def _categorize_nodes(hidden_identifier: str, sensor_identifier: str) -> None: