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

Get zha switch and binary_sensor state on startup #11672

Merged
merged 10 commits into from Mar 9, 2018

Conversation

Projects
None yet
8 participants
@SteveEasley
Copy link
Contributor

commented Jan 15, 2018

Description:

Fix: Ensures the zha switch and binary_sensor platform reads the current state of the switch on HA startup. This mirrors how the zha light works.

Note: This is a recreation of the closed #11429.
@balloob I addressed your review request from that PR in this PR (https://github.com/home-assistant/home-assistant/pull/11429/files/c69e9e40fafc5061ab6601ca8827b6152210dd93#r161091226)

Checklist:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
@SteveEasley

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2018

Thanks to @dmulcahey for the help on this.

result = yield from safe_read(self._endpoint.light_color,
['current_x', 'current_y'])
result = yield from zha.get_attributes(self._endpoint.light_color,
['current_x', 'current_y'])

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 16, 2018

continuation line over-indented for visual indent

result = yield from safe_read(self._endpoint.light_color,
['color_temperature'])
result = yield from zha.get_attributes(self._endpoint.light_color,
['color_temperature'])

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 16, 2018

continuation line over-indented for visual indent

result = yield from safe_read(self._endpoint.level,
['current_level'])
result = yield from zha.get_attributes(self._endpoint.level,
['current_level'])

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 16, 2018

continuation line over-indented for visual indent

@dmulcahey

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2018

#11670 take a peek at that. @rcloran (creator of ZHA) submitted that and it is already merged. You'll probably want to pull that inline with your changes or start a discussion to determine an outcome about the function name.

SteveEasley added some commits Jan 16, 2018

@SteveEasley

This comment has been minimized.

Copy link
Contributor Author

commented Jan 16, 2018

Ready for review

@dshokouhi

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2018

Without this PR, will the device's state ever get updated if it has not changed or will it remain until a change has been detected? I have some contact sensors and sometimes my automations fail when things get out of sync. Want to make sure I need to be aware of this upon restart or if it will eventually fix itself lol.

@@ -15,19 +15,39 @@
DEPENDENCIES = ['zha']


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Set up Zigbee Home Automation switches."""
@asyncio.coroutine

This comment has been minimized.

Copy link
@balloob

balloob Mar 1, 2018

Member

We've migrated to async/await syntax. Please drop this decorator and prefix async methods with async like async def async_setup_platform. Replace any yield from with await.

@balloob

This comment has been minimized.

Copy link
Member

commented Mar 1, 2018

One tiny comment. Ok to merge when comment addressed 👍


@asyncio.coroutine
def async_turn_off(self, **kwargs):
"""Turn the entity off."""
yield from self._endpoint.on_off.off()
self._state = 0
self.schedule_update_ha_state()

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Mar 2, 2018

Member

If you really need this use async_schedule_update_ha_state(). But since this is a non polling entity, doesn't the device feedback the state change to the entity via async_update? Then this isn't needed.

_LOGGER.debug("Attribute updated: %s %s %s", self, attribute, value)
if attribute == self.value_attribute:
self._state = value
self.schedule_update_ha_state()

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Mar 2, 2018

Member

If this is run on the event loop, use async_schedule_update_ha_state().

SteveEasley added some commits Mar 5, 2018

@SteveEasley

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2018

All review comments were addressed. Ready for review.

@@ -83,7 +87,17 @@ def cluster_command(self, tsn, command_id, args):
if command_id == 0:
self._state = args[0] & 3
_LOGGER.debug("Updated alarm state: %s", self._state)
self.schedule_update_ha_state()
self.async_schedule_update_ha_state()

This comment has been minimized.

Copy link
@balloob

balloob Mar 6, 2018

Member

Is cluster_command executed in an async context? Either this line or the call to self.hass.add_job 3 lines below is wrong (should be async_add_job).

This comment has been minimized.

Copy link
@SteveEasley

SteveEasley Mar 6, 2018

Author Contributor

I think it is run in an async context. But that is based on my evidence that when I comment the self.async_schedule_update_ha_state() line my binary sensor no long updates the UI. Does that sound right?

This comment has been minimized.

Copy link
@SteveEasley

SteveEasley Mar 6, 2018

Author Contributor

Added a new commit with the change 3 lines down. Confirmed the changes works fine.

@balloob

balloob approved these changes Mar 9, 2018

@balloob balloob added this to the 0.65 milestone Mar 9, 2018

@balloob balloob merged commit 5e2296f into home-assistant:dev Mar 9, 2018

5 checks passed

WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.01%) to 94.028%
Details
hound No violations found. Woof!

balloob added a commit that referenced this pull request Mar 9, 2018

Get zha switch and binary_sensor state on startup (#11672)
* Get zha switch and binary_sensor state on startup

* Removed unused var

* Make zha switch report status

* Use right method name

* Formatting fix

* Updates to match latest dev

* PR feedback updates

* Use async for cluster commands

@balloob balloob referenced this pull request Mar 9, 2018

Merged

0.65 #12995

@ryanwinter

This comment has been minimized.

Copy link

commented Mar 10, 2018

The smartthings multipurpose sensor status is still incorrect after restart (defaults to closed, even if its open).

Does this patch no apply for that binary sensor?

@SteveEasley

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2018

Do you have other switches to test? I just confirmed again that my CentraLite 3320-L switch detects state after both a restart and reboot.

@ryanwinter

This comment has been minimized.

Copy link

commented Mar 13, 2018

Tested a smartthings outlet. The device actually gets switched off during reset which is weird. But I guess that means it is reporting correctly...

@dshokouhi

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2018

Hi just chiming in that my Iris Contact Sensors are not updating their status correctly when I restart home assistant, they still show as off instead of on when they are open and I do a restart.

Edit: I am on version 0.65.6

@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.