Delay zwave updates for 100ms to group them. #6420
Conversation
…into dev Conflicts: homeassistant/components/zwave/__init__.py
|
@andrey-git, thanks for your PR! By analyzing the history of the files in this pull request, we identified @turbokongen, @balloob and @fabaff to be potential reviewers. |
| if self.hass and not self._scheduled_update: | ||
| self._schedule_update() | ||
| else: | ||
| _LOGGER.info('Skip update for %s. It is already scheduled', self.entity_id) |
|
This solves some but not all duplicate updates. |
| if self.hass and not self._scheduled_update: | ||
| self._schedule_update() | ||
| else: | ||
| _LOGGER.info('Skip update for %s. It is already scheduled', |
There was a problem hiding this comment.
This could be debug at most but probably not log at all. It can spam a lot
| @callback | ||
| def do_update(): | ||
| """Really update.""" | ||
| self.hass.add_job(self.async_update_ha_state()) |
| @@ -916,3 +921,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): | |||
There was a problem hiding this comment.
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.
| if self.hass: | ||
| self.schedule_update_ha_state() | ||
| if self.hass and not self._scheduled_update: | ||
| self._schedule_update() |
There was a problem hiding this comment.
This should become hass.async_add_job(schedule)
There was a problem hiding this comment.
Changed to hass.add_job as async_add_job is only supposed to be called from a loop
| self.hass.async_add_job(self.async_update_ha_state) | ||
| self._scheduled_update = False | ||
|
|
||
| if not self._scheduled_update: |
There was a problem hiding this comment.
Move if-statement to first line in function so Python doesn't have to create a function for nothing.
| @@ -794,7 +796,7 @@ def value_changed(self): | |||
| # If value changed after device was created but before setup_platform | |||
| # was called - skip updating state. | |||
| if self.hass: | |||
There was a problem hiding this comment.
We are certain that we don't have to schedule an update if self._scheduled_update:
if self.hass and not self._scheduled_update:(yes, we need a check in both places)
| @callback | ||
| def _schedule_update(self): | ||
| """Schedule delayed update.""" | ||
| if not self._scheduled_update: |
There was a problem hiding this comment.
Should be the inverse. We should not do anything if an update is already scheduled.
| @@ -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 | |||
There was a problem hiding this comment.
For a future PR, maybe rename this to _update_scheduled to avoid mixup with _schedule_update method.
Description:
Delay zwave updates for 100ms to group them.
Related issue (if applicable): #6391
If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass