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.

Sign up for GitHub

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

Zha switch schedule update state #16621

Merged
merged 2 commits into from
Sep 21, 2018
Merged

Zha switch schedule update state #16621

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c074f7
switch.zha: Schedule state update.
Adminiuga Sep 14, 2018
de31673
switch.zha: Update debug logging.
Adminiuga Sep 14, 2018
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
19 changes: 14 additions & 5 deletions homeassistant/components/switch/zha.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ class Switch(zha.Entity, SwitchDevice):

def attribute_updated(self, attribute, value):
"""Handle attribute update from device."""
_LOGGER.debug("Attribute updated: %s %s %s", self, attribute, value)
cluster = self._endpoint.on_off
attr_name = cluster.attributes.get(attribute, [attribute])[0]
_LOGGER.debug("%s: Attribute '%s' on cluster '%s' updated to %s",
self.entity_id, attr_name, cluster.ep_attribute, value)
if attribute == self.value_attribute:
self._state = value
self.async_schedule_update_ha_state()
Expand All @@ -65,23 +68,29 @@ async def async_turn_on(self, **kwargs):
"""Turn the entity on."""
from zigpy.exceptions import DeliveryError
try:
await self._endpoint.on_off.on()
res = await self._endpoint.on_off.on()
_LOGGER.debug("%s: turned 'on': %s", self.entity_id, res[1])
except DeliveryError as ex:
_LOGGER.error("Unable to turn the switch on: %s", ex)
_LOGGER.error("%s: Unable to turn the switch on: %s",
self.entity_id, ex)
return

self._state = 1
self.async_schedule_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.

We shouldn't do this, and neither should we change self._state. We depend on attribute_updated to push the state change to us

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure relying on attribute_updated is a proper channel for this kind of state updated, especially when we're triggering the change.
We do send a ZCL command and we do know the transmission hasn't timed out, in other words device has acknowledge reception of the command.
The problem with attribute_updated it relies on attribute reporting configuration, so if for whatever reason zha.configure_reporting() was unsuccessful, this makes the switch unusable, as I never can change the state (because the feedback loop through attribute_updated is broken) even though I can control the switch and switch can successfully receive my commands.
attribute_updated channel is more for externally originated events, like user toggling the switch from the button on the actual physical device.

@rcloran what do you think?

Copy link
Contributor Author

@Adminiuga Adminiuga Sep 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I also want to implement attribute_updated() method for fan.zha.ZhaFan class, because it is nice for HA state to be in sync with the actual state of the device, since the fan is often operated externally from the remote/wall switch.
I'd like to understand why it is fundamentally wrong with updating self._state in the service calls and in attribute_updated() method. Both are executed in the event loop, so there shouldn't be any race conditions. I'm not always getting attribute report triggering the attribute_update() with King of Fans zigbee fan, so relying solely on atribute_updated() to update self._state would likely render Fan inoperable from HA


async def async_turn_off(self, **kwargs):
"""Turn the entity off."""
from zigpy.exceptions import DeliveryError
try:
await self._endpoint.on_off.off()
res = await self._endpoint.on_off.off()
_LOGGER.debug("%s: turned 'off': %s", self.entity_id, res[1])
except DeliveryError as ex:
_LOGGER.error("Unable to turn the switch off: %s", ex)
_LOGGER.error("%s: Unable to turn the switch off: %s",
self.entity_id, ex)
return

self._state = 0
self.async_schedule_update_ha_state()

async def async_update(self):
"""Retrieve latest state."""
Expand Down