Update Ring to 0.6.0 by balloob · Pull Request #30748 · home-assistant/core

balloob · 2020-01-14T04:53:03Z

Description:

Ring dependency has been updated to 0.6.0 which significantly reduces the number of requests.
Tweak update code in Home Assistant to reduce number of requests
Support multiple Ring accounts
Add device classes to sensors
Fix device identifiers and model names

Thanks to @arsaboo for allowing me to use his account for testing.

Added myself as code owner because at this point, well 🤷‍♂

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist

If the code does not interact with devices:

Tests have been added to verify that the new code works.

dshokouhi · 2020-01-14T07:36:28Z

Tested this PR and all expected entities get loaded properly including the cameras. I am still facing the issue where binary sensors are not updating but the other sensors for the device is updating. I am willing to share my account as well if it helps to troubleshoot this issue. I know @arsaboo is not experiencing this issue but based on this issue it seems I am not alone: python-ring-doorbell/python-ring-doorbell#177

dshokouhi · 2020-01-14T14:45:20Z

If it is helpful I am also willing to share my account. I have 4 ring camera devices and also the ring smart lighting system which is not supported by HA yet. Please get in touch with me on discord :)

tchellomello · 2020-01-14T15:20:26Z

@dshokouhi I'm going to test it tonight and report it back.

MartinHjelmare

Looks good!

MartinHjelmare · 2020-01-14T20:07:39Z

            logger.warning,
-            "Setup of platform %s is taking over %s seconds.",
+            "Setup of %s platform %s is taking over %s seconds.",
+            self.domain,


Should this change be included in this PR?

Well, I could open a new PR, but I used it here, so thought I would just include it.

Ok. It's not our regular practice, so I wanted to make sure it wasn't a change that was committed by accident.

MartinHjelmare · 2020-01-14T20:09:21Z

+        self.hass = hass
+        self.cache = {}
+
+    async def async_get_history(self, config_entry_id, device):


Is this better than using a dict cache and our Throttle helper to throttle updating the cache?

Yeah, because with the Throttle decorator, if a call is in process, it will return None instead of waiting for the call to finish.

So at worst we would then get 10 second stale data?

What kind of data is this?

Last 10 events for a device.

Throttle returns None, not the data. The implementation would become then data.update(); handle(data.data)

I think that if this approach seems to work, we might want to turn that into a generic asyncio throttle function. We do a similar thing in Hue.

It's still not clear to me why a central update and cache with signals telling the entities to update would be worse than this central update with a timed cache where entities pull data, in this situation.

The entities per device that are involved are fixed, except for the camera where only cameras that have subscription are added. So there won't be less or more entities per device pulling data except for the camera case. Maybe the camera entity is the reason?

My point is, that if we know what devices we have and nothing about the features of those devices changes, we should be able to update all devices regardless of platforms.

Oh my bad. I thought you asked why not use X, instead of how does it work with the existing dispatch update that Ring uses.

So I don't have a good reason. And it is indeed kinda complicated now. We now have a health tracker that has an opt-in signal. We could have used the same approach for history. hmm.

Want to rewrite it ?

I think it's ok like this. I mostly want to understand the reasons for the different designs so I know when to recommend what approach. And I like as few choices as possible for my recommendations.

Sorry about my last comment tone. I meant to say want ME to rewrite it.

I thought about it more and I'll do one more pass cleaning up data fetching. I can do better here.

balloob · 2020-01-14T20:54:41Z

Going to merge this now so I can cut the beta and go for lunch. I will fix any comments that come up.

* Update Ring to 0.6.0 * Update sensor tests * update -> async_update * Delete temp files * Address comments * Final tweaks * Remove stale print

* upstream/dev: (82 commits) Add support for vacuums to Alexa. (home-assistant#30764) Refactor Ring data handling (home-assistant#30777) Restore unit_of_measurement from entity registry (home-assistant#30780) Update pyubee to 0.8 (home-assistant#30785) Update emulated_roku to 0.1.9 (home-assistant#30791) Add Config Flow support, Device Registry support, available property to vizio component (home-assistant#30653) Allow input_* and timer component setup without config (home-assistant#30772) Search: Add search to default config and don't resolve area (home-assistant#30762) [ci skip] Translation update Use storage based collections for Timer platform (home-assistant#30765) Upgrade youtube_dl to version 2020.01.15 (home-assistant#30767) Whitelist Frenck for release Hass.io allow to reset password with CLI (home-assistant#30755) Revert home-assistant#29701 (home-assistant#30766) Add Safe Mode (home-assistant#30723) Update Ring to 0.6.0 (home-assistant#30748) Add support for the voltage sensor on the greeneye GEM (home-assistant#30484) Fix supported_features in MQTT fan (home-assistant#28680) Fix small typo in alarmdotcom component (home-assistant#30758) bump aiokef to 0.2.5 which uses locks (home-assistant#30753) ...

Update Ring to 0.6.0

f3e0c78

balloob added this to the 0.104.0 milestone Jan 14, 2020

homeassistant added the cla-signed label Jan 14, 2020

probot-home-assistant Bot added core has-tests integration: ring labels Jan 14, 2020

balloob added 3 commits January 13, 2020 21:55

Update sensor tests

c6ec1b6

update -> async_update

aa85275

Delete temp files

e57db9a

MartinHjelmare reviewed Jan 14, 2020

View reviewed changes

Address comments

ee3ddc8

MartinHjelmare approved these changes Jan 14, 2020

View reviewed changes

Final tweaks

30394d3

balloob requested a review from a team as a code owner January 14, 2020 19:32

Remove stale print

34a22dc

MartinHjelmare reviewed Jan 14, 2020

View reviewed changes

balloob merged commit c4673dd into dev Jan 14, 2020

delete-merged-branch Bot deleted the update-ring-2 branch January 14, 2020 20:54

balloob added the cherry-picked label Jan 14, 2020

balloob added a commit that referenced this pull request Jan 14, 2020

Update Ring to 0.6.0 (#30748)

4bc520c

* Update Ring to 0.6.0 * Update sensor tests * update -> async_update * Delete temp files * Address comments * Final tweaks * Remove stale print

lock Bot locked and limited conversation to collaborators Jan 16, 2020

Uh oh!

Conversation

balloob commented Jan 14, 2020

Description:

Checklist:

Uh oh!

dshokouhi commented Jan 14, 2020

Uh oh!

dshokouhi commented Jan 14, 2020

Uh oh!

tchellomello commented Jan 14, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob commented Jan 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

balloob Jan 14, 2020 •

edited

Loading