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
47 changes: 39 additions & 8 deletions homeassistant/components/zwave/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,46 @@ def value_added(node, value):
def node_added(node):
"""Handle a new node on the network."""
entity = ZWaveNodeEntity(node, network)
name = node_name(node)
generated_id = generate_entity_id(DOMAIN + '.{}', name, [])
node_config = device_config.get(generated_id)
if node_config.get(CONF_IGNORED):
_LOGGER.info(
"Ignoring node entity %s due to device settings",
generated_id)

def _add_node_to_component():
name = node_name(node)
generated_id = generate_entity_id(DOMAIN + '.{}', name, [])
node_config = device_config.get(generated_id)
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 realize it's existing logic)

This is not entirely correct anymore, as based on the unique ID we might choose to give them a different entity id. The correct implementation would be to leverage the disabling of entities via the entity registry. If an entity is disabled, we will never call async_added_to_hass (where subscriptions should happen)

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.

Yes, this bug is what got me working on zwave again in the first place.

I'll will address this in a future PR.

In any case, is the registry the right place for ignoring entities? Do other platforms use it for "ignore"?

Also - imaging a zombie zwave node that can't be parsed at all. This means it won't get unique_id, so it can't be ignored via the registry.

I propose to add a a list of ignored IDs to (root) zwave config to handle this case.

As for ignoring specific values - while entity registry would work here, I find it very convenient to ignore via zwave config globs, i.e:

sensor.*energy*:
  ignored: true

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.

You and your globs 😉

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 don't mind keeping it here.

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.

Btw a useful subset of globs can be used from the UI - a checkbox "ignore energy meters", "ignore power meters", etc.

if node_config.get(CONF_IGNORED):
_LOGGER.info(
"Ignoring node entity %s due to device settings",
generated_id)
return
component.add_entities([entity])

if entity.unique_id:
_add_node_to_component()
return
component.add_entities([entity])

async def _check_node_ready():
"""Wait for node to be parsed."""
start_time = dt_util.utcnow()
while True:
waited = int((dt_util.utcnow()-start_time).total_seconds())

if entity.unique_id:
_LOGGER.info("Z-Wave node %d ready after %d seconds",
entity.node_id, waited)
break
elif waited >= const.NODE_READY_WAIT_SECS:
# Wait up to NODE_READY_WAIT_SECS seconds for the Z-Wave
# node to be ready.
_LOGGER.warning(
"Z-Wave node %d not ready after %d seconds, "
"continuing anyway",
entity.node_id, waited)
break
else:
await asyncio.sleep(1, loop=hass.loop)

hass.async_add_job(_add_node_to_component)

hass.add_job(_check_node_ready)

def network_ready():
"""Handle the query of all awake nodes."""
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/zwave/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ATTR_VALUE_INDEX = "value_index"
ATTR_VALUE_INSTANCE = "value_instance"
NETWORK_READY_WAIT_SECS = 300
NODE_READY_WAIT_SECS = 30

DISCOVERY_DEVICE = 'device'

Expand Down
17 changes: 17 additions & 0 deletions homeassistant/components/zwave/node_entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def __init__(self, node, network):
self._name = node_name(self.node)
self._product_name = node.product_name
self._manufacturer_name = node.manufacturer_name
self._unique_id = self._compute_unique_id()
self._attributes = {}
self.wakeup_interval = None
self.location = None
Expand All @@ -95,6 +96,11 @@ def __init__(self, node, network):
dispatcher.connect(
self.network_scene_activated, ZWaveNetwork.SIGNAL_SCENE_EVENT)

@property
def unique_id(self):
"""Unique ID of Z-wave node."""
return self._unique_id

def network_node_changed(self, node=None, value=None, args=None):
"""Handle a changed node on the network."""
if node and node.node_id != self.node_id:
Expand Down Expand Up @@ -138,8 +144,14 @@ def node_changed(self):
self.wakeup_interval = None

self.battery_level = self.node.get_battery_level()
self._product_name = self.node.product_name
self._manufacturer_name = self.node.manufacturer_name
self._name = node_name(self.node)
self._attributes = attributes

if not self._unique_id:
self._unique_id = self._compute_unique_id()

self.maybe_schedule_update()

def network_node_event(self, node, value):
Expand Down Expand Up @@ -229,3 +241,8 @@ def device_state_attributes(self):
attrs[ATTR_WAKEUP] = self.wakeup_interval

return attrs

def _compute_unique_id(self):
if self._manufacturer_name and self._product_name:
return 'node-{}'.format(self.node_id)
return None
41 changes: 41 additions & 0 deletions tests/components/zwave/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,47 @@ def mock_connect(receiver, signal, *args, **kwargs):
assert hass.states.get('zwave.mock_node').state is 'unknown'


async def test_unparsed_node_discovery(hass, mock_openzwave):
"""Test discovery of a node."""
mock_receivers = []

def mock_connect(receiver, signal, *args, **kwargs):
if signal == MockNetwork.SIGNAL_NODE_ADDED:
mock_receivers.append(receiver)

with patch('pydispatch.dispatcher.connect', new=mock_connect):
await async_setup_component(hass, 'zwave', {'zwave': {}})

assert len(mock_receivers) == 1

node = MockNode(node_id=14, manufacturer_name=None)

sleeps = []

def utcnow():
return datetime.fromtimestamp(len(sleeps))

asyncio_sleep = asyncio.sleep

async def sleep(duration, loop):
if duration > 0:
sleeps.append(duration)
await asyncio_sleep(0, loop=loop)

with patch('homeassistant.components.zwave.dt_util.utcnow', new=utcnow):
with patch('asyncio.sleep', new=sleep):
with patch.object(zwave, '_LOGGER') as mock_logger:
hass.async_add_job(mock_receivers[0], node)
await hass.async_block_till_done()

assert len(sleeps) == const.NODE_READY_WAIT_SECS
assert mock_logger.warning.called
assert len(mock_logger.warning.mock_calls) == 1
assert mock_logger.warning.mock_calls[0][1][1:] == \
(14, const.NODE_READY_WAIT_SECS)
assert hass.states.get('zwave.mock_node').state is 'unknown'


@asyncio.coroutine
def test_node_ignored(hass, mock_openzwave):
"""Test discovery of a node."""
Expand Down
13 changes: 11 additions & 2 deletions tests/components/zwave/test_node_entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ def setUp(self):
query_stage='Dynamic', is_awake=True, is_ready=False,
is_failed=False, is_info_received=True, max_baud_rate=40000,
is_zwave_plus=False, capabilities=[], neighbors=[], location=None)
self.node.manufacturer_name = 'Test Manufacturer'
self.node.product_name = 'Test Product'
self.entity = node_entity.ZWaveNodeEntity(self.node,
self.zwave_network)

Expand Down Expand Up @@ -357,3 +355,14 @@ def test_state_ready(self):
def test_not_polled(self):
"""Test should_poll property."""
self.assertFalse(self.entity.should_poll)

def test_unique_id(self):
"""Test unique_id."""
self.assertEqual('node-567', self.entity.unique_id)

def test_unique_id_missing_data(self):
"""Test unique_id."""
self.node.manufacturer_name = None
entity = node_entity.ZWaveNodeEntity(self.node, self.zwave_network)

self.assertIsNone(entity.unique_id)
4 changes: 4 additions & 0 deletions tests/mock/zwave.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ def __init__(self, *,
product_type='678',
command_classes=None,
can_wake_up_value=True,
manufacturer_name='Test Manufacturer',
product_name='Test Product',
network=None,
**kwargs):
"""Initialize a Z-Wave mock node."""
Expand All @@ -128,6 +130,8 @@ def __init__(self, *,
self.manufacturer_id = manufacturer_id
self.product_id = product_id
self.product_type = product_type
self.manufacturer_name = manufacturer_name
self.product_name = product_name
self.can_wake_up_value = can_wake_up_value
self._command_classes = command_classes or []
if network is not None:
Expand Down