Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

Introduces a self._extended_properties cache to the ArloCamera object. #52

Merged
merged 2 commits into from
Oct 10, 2017

Conversation

tchellomello
Copy link
Owner

@tchellomello tchellomello commented Oct 10, 2017

Description:
Introduces a self._extended_properties cache to the ArloCamera object.

  • This will guarantee the data will be cached locally avoid a new query every time
    an extended property is requested.

    During some tests, the time to load all properties were considerably faster due to the local cache. If the data demands/requires an update, it can be easily updated by calling the update() method for the ArloCamera object.

    This will also contribute to HASS for I/O when reporting the Arlo attributes.

Fixes #31

  * This will guarante the data will be cached locally avoid a new query every time
    an extended property is requested.

    During some tests, the time to load all properties were considerally faster
    due to the local cache. If the data demands/requires update, it can be easily
    updated by calling the update() method for the ArloCamera object.

    This will also contribute to HASS for I/O when reporting the Arlo attributes.
@tchellomello tchellomello added this to the 0.0.8 milestone Oct 10, 2017
@tchellomello tchellomello self-assigned this Oct 10, 2017
@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage increased (+6.2%) to 85.854% when pulling 271aa3d on cached_cam_properties into 1827b49 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+6.2%) to 85.854% when pulling 8bb5b32 on cached_cam_properties into 1827b49 on master.

1 similar comment
@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage increased (+6.2%) to 85.854% when pulling 8bb5b32 on cached_cam_properties into 1827b49 on master.

@tchellomello
Copy link
Owner Author

Once if we got this approved, I think we are ready for the next version.
Do you guys want to add any other patch on 0.0.8?

@jwillaz
Copy link
Collaborator

jwillaz commented Oct 10, 2017

@tchellomello I'm working on some fixes myself. The merge they did earlier today for the dev branch HASS components still throws a lot of errors. I like what you did with the extended properties, but my concern is that it's still making some unnecessary calls to get_camera_properties. A single call gets ALL camera properties, so they should be updated all at once - that's the implementation I'm working on.

@tchellomello
Copy link
Owner Author

tchellomello commented Oct 10, 2017

@jwillaz Nice!! That is a great idea!! So do you think we can merge this and then you can work on top of this PR to simplify this in a unique call or should we wait for your PR and close this one?

I've tested HASS with this implementation and I've seen a huge performance improvement. I'm already excited with a single call then the user's experience will be fantastic!

@jwillaz
Copy link
Collaborator

jwillaz commented Oct 10, 2017

@tchellomello Yep, go ahead and merge. I'll do a comparison and update accordingly.

@tchellomello
Copy link
Owner Author

@jwillaz awesome So I'll merge and then I'll wait for your PR before publishing the 0.0.8

@tchellomello tchellomello merged commit 93e8790 into master Oct 10, 2017
@tchellomello tchellomello deleted the cached_cam_properties branch October 10, 2017 07:21
@jwillaz
Copy link
Collaborator

jwillaz commented Oct 10, 2017

@tchellomello Allright, we're in business! I just wrapped up the finishing touches and testing. Will submit PR in a few hours. There are definitely changes for the HASS camera and sensor components, so I'll be submitting PRs there as well.

@tchellomello
Copy link
Owner Author

@jwillaz sounds good. As soon as we get you PR for pyarlo merged, we can publish a new version and then you can go ahead and submit the PR's that you have been working on for HASS.

Thank you!!!

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

3 participants