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

Wink Rollershutter #2294

Merged
merged 2 commits into from
Jun 18, 2016
Merged

Wink Rollershutter #2294

merged 2 commits into from
Jun 18, 2016

Conversation

philk
Copy link
Contributor

@philk philk commented Jun 14, 2016

Description:

Adds support for Wink shades/blinds using the rollershutter component.

Requires additions from python-wink 0.7.7 and it seemed like having a different version would probably break things so I updated REQUIREMENTS in all the Wink components. I can squash that or put it in a separate PR if that's the way you all want to do things.

I didn't see any tests for the other Wink components. If there's a good example of a similar component I could take a stab at that.

Pull request in home-assistant.io with documentation (if applicable):
home-assistant/home-assistant.io#559

Checklist:

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

If code communicates with devices:

  • 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 have been added to requirements_all.txt by running script/gen_requirements_all.py.

If the code does not interact with devices:

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

@philk philk force-pushed the wink_rollershutter branch 2 times, most recently from 244f2a1 to b4ddbf4 Compare June 14, 2016 04:58
@w1ll1am23
Copy link
Contributor

You stated that the current position isn't returned from the Wink API. In your python-wink json for the device it shows null as the position value. Are you getting null back? Do you know the position of the shades in the official Wink App? If so I think you are going to want to bring this up as a bug with Wink so they can get it fixed. Not knowing the position really sucks :(

@philk philk force-pushed the wink_rollershutter branch from b4ddbf4 to 0cbfc5b Compare June 14, 2016 21:55
@philk
Copy link
Contributor Author

philk commented Jun 14, 2016

Unfortunately it's a problem further upstream. The Somfy ZRTSI doesn't track and report status (since RTS is a unidirectional protocol) so there's no way for Wink to know it either. I can't find any confirmation about the Lutron Serena's (the only other shades supported by Wink) reporting status one way or the other. If you open the Wink app all blinds always show as closed. The Wink app internally keeps track of the shades (unless the app is closed/crashed/force quit, in which case it defaults back to closed) and will be inaccurate if you use the actual shade remotes. Note that in the app this is super annoying since you have to swipe the blinds open, then closed to close them. Whee! Wink... 😐

I did originally consider making this PR track state internally like the Wink app does but knowing how annoying that is in the Wink app when it's wrong it felt bad.

You do make a good point that there's no reason to swallow the state at the home-assistant level (in case the Serena's are different). python-wink returns valid state if it exists so we might as well respect that. I've updated the PR with current_position implemented.

@property
def should_poll(self):
"""Wink Shades don't track their position."""
return False
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 True right? You have an update method that actually updates the state and you implement current_position

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It certainly could be True.
I've shot out a message to Wink support to see if the position will always be null from the API. If so I'd say this should remain as False since polling is just a waste.

@balloob
Copy link
Member

balloob commented Jun 18, 2016

Looks good! 🐬

@balloob balloob merged commit 4084004 into home-assistant:dev Jun 18, 2016
@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants