Skip to content

Blink token recovery#6696

Closed
fronzbot wants to merge 3 commits into
home-assistant:devfrom
fronzbot:blink-authorization
Closed

Blink token recovery#6696
fronzbot wants to merge 3 commits into
home-assistant:devfrom
fronzbot:blink-authorization

Conversation

@fronzbot
Copy link
Copy Markdown
Contributor

@fronzbot fronzbot commented Mar 19, 2017

Description:

After 24hrs auth token expires. This change allows for the system to grab a new token. I also increased the timeout for auto updates to 180 seconds, but added in the ability to force an update (for example, right after a new photo is taken). This allowed me to move all update/throttling behavior into the main component rather than in any of the sub-modules (more concise in my opinion)

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

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

@fronzbot
Copy link
Copy Markdown
Contributor Author

@pvizeli Any chance someone could review this before 0.41.0 (I ask you since you reviewed the initial platform and for some reason mention-bot didn't comment on this PR)? Right now without this fix, the Blink platform fails to communicate after 24hrs, something I missed in the original release.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Mar 22, 2017

Yeah, I do it. Do you have look to helpers/event.py for async_track_point_in_utc_time?

@fronzbot
Copy link
Copy Markdown
Contributor Author

No, i don't use that file in the code. The way I track if a token expired is if I get a non-200 response code and I attempt to login again to retrieve a new token. There's no guarantee that the token will expire exactly after 24 hrs, so I do it this way to protect against the expiration time from changing in the future.

Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Please remove polling from sensor/binary and use with track_point_in_utc_time or track_point_in_time (with that you need lock to make sure that you not overtask it) for update the data. So you need not call data.update() on entity updates. On all other you can access to this cached data and need no poll.

@fronzbot
Copy link
Copy Markdown
Contributor Author

I'm not sure I need to use either the track_point_in_utc_time nor the track_point_in_time, unless I am misunderstanding you? I can remove the call to the update() function in both the binary_sensor and sensor submodule, and should be able to rely on the cached properties since every time the camera requests a new image, the sensor data will also be fetched. Is this what you are ultimately requesting?

@fronzbot
Copy link
Copy Markdown
Contributor Author

@pvizeli Do I need to change anything else?

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Mar 27, 2017

Look into andorid ip Webcam. You have a object on component and access from platform to this object (data holding object). Instead you floot this object with update request, add you own update handling to this object and access only to cached value from platform.

You need to do:

  • Set polling to False on entity
  • Remove all udpates from platform code
  • Add a own update routine to you data handling object with (track_point_in_utc_time, use it like recursive call)

@fronzbot
Copy link
Copy Markdown
Contributor Author

That's a pretty massive change, though, no? I understand what you're getting at, and it makes perfect sense, but functionally, this PR is exactly the same as my original implementation. If my original implementation was not adequate then why did it get merged? Unfortunately, I don't really have time to make a change like that, so if you don't want to merge this PR (despite it being functionally equivalent to the original implementation) I'll close this and open an issue so someone else can come in and fix it. Let me know.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Apr 4, 2017

Yeah it is a bit wired. I will help you to improve your code for handle that stable. With this code you have a data object and all sensor read data from this cache. Thas is okay but it is no polling device anymore. But you call exesive the update from you cache and slow done homeassistant without any real reason.

So the handle that correct, you have a data object (cache) and they update itself with track_point_in_utc_time. All other entity read this cache and need not call update. So you have 1 call instead > 15 calls.

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.

4 participants