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.

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

Delay zwave updates for 100ms to group them. #6420

Merged
merged 14 commits into from Mar 5, 2017
20 changes: 18 additions & 2 deletions homeassistant/components/zwave/__init__.py
Expand Up @@ -12,6 +12,7 @@

import voluptuous as vol

from homeassistant.core import callback
from homeassistant.loader import get_platform
from homeassistant.helpers import discovery
from homeassistant.const import (
Expand Down Expand Up @@ -769,6 +770,7 @@ def __init__(self, value, domain):
self._wakeup_value_id = None
self._battery_value_id = None
self._power_value_id = None
self._scheduled_update = False
Copy link
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, maybe rename this to _update_scheduled to avoid mixup with _schedule_update method.

Copy link
Member

Choose a reason for hiding this comment

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

That's a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #6434

self._update_attributes()

dispatcher.connect(
Expand All @@ -793,8 +795,10 @@ def value_changed(self):
self.update_properties()
# If value changed after device was created but before setup_platform
# was called - skip updating state.
if self.hass:
self.schedule_update_ha_state()
if self.hass and not self._scheduled_update:
self._schedule_update()
Copy link
Member

Choose a reason for hiding this comment

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

This should become hass.async_add_job(schedule)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to hass.add_job as async_add_job is only supposed to be called from a loop

else:
_LOGGER.info('Skip update for %s. It is already scheduled', self.entity_id)

Choose a reason for hiding this comment

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

line too long (87 > 79 characters)


def _update_ids(self):
"""Update value_ids from which to pull attributes."""
Expand Down Expand Up @@ -916,3 +920,15 @@ def refresh_from_network(self):
return
for value_id in dependent_ids + [self._value.value_id]:
self._value.node.refresh_value(value_id)

def _schedule_update(self):
Copy link
Member

Choose a reason for hiding this comment

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

We should actually make this a callback and check at start if function if an update has already been scheduled. Otherwise because of race conditions we might schedule it twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"""Schedule delayed update."""
@callback
def do_update():
"""Really update."""
self.hass.add_job(self.async_update_ha_state())
Copy link
Member

Choose a reason for hiding this comment

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

This should be async_add_job

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

self._scheduled_update = False

self.hass.loop.call_soon_threadsafe(
self.hass.loop.call_later, 0.1, do_update)
self._scheduled_update = True