Wink pubnub subscription support and base Wink device #2324
Conversation
| from homeassistant.helpers import validate_config, discovery | ||
| from homeassistant.helpers.entity import ToggleEntity | ||
| from homeassistant import bootstrap |
There was a problem hiding this comment.
- 1: F401 'bootstrap' imported but unused
w1ll1am23
force-pushed
the
wink_pubnub
branch
3 times, most recently
from
204c372 to
7e20f4c
Compare
| REQUIREMENTS = ['python-wink==0.7.7'] | ||
| REQUIREMENTS = ['python-wink==0.7.8', 'pubnub==3.7.8'] | ||
|
|
||
| DISCOVER_LIGHTS = "wink.lights" |
There was a problem hiding this comment.
These are no longer needed and should be removed.
w1ll1am23
force-pushed
the
wink_pubnub
branch
2 times, most recently
from
9ae1f1c to
33a1a64
Compare
|
When adding the should_poll and removing the if check in the update, that still results in update getting called. This doesn't seem to work for locks or anything that takes more then a couple seconds to update. The pubnub update comes in, then update is called which polls the wink api which then changes the state in HA. So if Wink hasn't received the update yet, then HA will reflect the wrong state. Is there any way around this? Would it be acceptable to add the if back to the update and check if should_poll == True? Update. @jsg4 has been testing this and indicated that state changes aren't reliable with this current code. I believe it has to do with the additional/unnecessary request made to wink. |
|
Found the problem, I was calling self.update_ha_state(True) which was forcing the call to Wink. Removed the True and it seems to work perfect now. |
|
Cool, so are we good to go? 👍 |
|
Issue 2 in the PR description still seems to be a problem even with the latest version of pubnub. I have a ticket opened with Pubnub on it to see what their thoughts are. Other than that it is good to go. Any thoughts on why that might be happening/how to fix it? |
|
Does pubnub offer some sort of ping functionality that we can use to verify On Sat, Jun 25, 2016, 07:52 William Scanlon [email protected]
|
|
I saw this on their site. "Timeout events are triggered when the server does not hear a hearbeat from the client within a default timeout time of 280 seconds." This made me think it was already setup to do this, but after looking at their code I think maybe it isn't doing anything by default. I will test out setting it to 120 seconds with a heartbeat every 59 seconds and see what happens. |
|
Do they have some commands without effects, like to query the status of something, that can be used to verify that the connection is alive? |
|
I am not sure, it looks like the heatbeat options is working though, @jsg4 is going to test it out as well and we can see how it goes. Based on what I have read the pubnub library should keep the connection active and reconnect if it gets dropped. When I am seeing this issue I still see the active TCP connection to their server. Also I haven't heard back from pubnub support yet so we will see what they have to say once they get back to me. |
|
Could it be that the connection with us and pubnub is fine but between wink and pubnub is broken ? If that is the case there is not much we can do… |
|
Can we just have it disconnect and reconnect after a timed interval? 4 |
|
@balloob Good question I guess that is possible, but maybe this heartbeat keeps that side active as well? This would be another question for pubnub I guess. @jsg4 not sure how that would work because each device would have to re-subscribe after the main pubnub object was "recreated." Not saying it couldn't be done but I am not sure. We will have to see how the heartbeat works for you. Without the heartbeat when I would get home from work my door sensors wouldn't alert me the door was opened. I have the heartbeat running now so we will see in about 4 hours if I get an alert. Fingers crossed! |
|
I have been running with pubnub heartbeat (without reboots) for just under 2 days and haven't had any unresponsive wink devices. I think this may have solved the problem. |
|
Awesome, merging this 🐬 |
Description:
This will migrate all Wink devices from the current polling method of updating to a push based update. This results in faster updates. This PR also creates a base Wink device that all other devices extend from which should make future updates easier.
I have tested powerstrips, lights, binary sensors, sensors, and locks. I don't own a garage door, or any non-wink z-wave devices so if anyone could test those out that would be cool.
There are two minor issues I have noticed.
Should a configuration option be added to switch back to polling based updates?
I will be out of town starting tomorrow 6/19-6/22 so it may be awhile before I can make any updates that may be required for this PR.
Related issue (if applicable): fixes #2314
Checklist:
If code communicates with devices:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py.