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

Update vera cover refresh logic #6897

Merged
merged 3 commits into from Apr 4, 2017
Merged

Conversation

pavoni
Copy link
Contributor

@pavoni pavoni commented Apr 2, 2017

Description:

This PR mainly bumps the vera version (which has an update that changes how thermostats are set - plus a change to how covers work - which fixes a bug where open and set_position didn't play well together.

I added some extra calls to self.schedule_update_ha_state() because I think some of the vera devices might not send async position updates correctly.

I'm not 100% these are needed (does the base cover code already schedule an update after a command).?

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

Checklist:

If user exposed functionality or configuration variables are added/changed:

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
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link

@pavoni, thanks for your PR! By analyzing the history of the files in this pull request, we identified @robjohnson189, @arjenfvellinga and @balloob to be potential reviewers.

@balloob
Copy link
Member

balloob commented Apr 3, 2017

Home Assistant will automatically update the entity in the state machine if should_poll returns True for the entity. If it is False, we expect the component/platform to push a new state once it is known using schedule_update_ha_state.

@pavoni
Copy link
Contributor Author

pavoni commented Apr 3, 2017

Thanks @balloob - VeraCover inherits from VeraDevice which implements should_poll and returns False. So this should be GTG.

@balloob balloob merged commit 5b9d995 into dev Apr 4, 2017
@balloob balloob deleted the update_vera_cover_refresh_logic branch April 4, 2017 05:44
@andrey-git
Copy link
Contributor

Why add schedule_update_ha_state?

It takes data from device and updates the state machine. If something changed ozw would call the value_updated which will call schedule_update_ha_state anyway.

@andrey-git
Copy link
Contributor

Disregard, I confused vera with ozw

@pavoni
Copy link
Contributor Author

pavoni commented Apr 6, 2017

It's a good point though- and should be true with vera too - but seems like it might not always be with some devices (which is what this PR should fix)!

@fabaff fabaff mentioned this pull request Apr 6, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Jul 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants