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

Adjust OpenUV integration for upcoming API limit changes #19949

Merged
merged 12 commits into from Jan 14, 2019

Conversation

Projects
None yet
7 participants
@bachya
Copy link
Contributor

bachya commented Jan 11, 2019

Description:

Starting February 1st, OpenUV will be reducing the free tier of its API to 50 requests per day. The current integration will likely overrun this. To accommodate all possible scenarios, this PR removes automatic polling of the API and, instead, gives users the ability to update sensors manually via the openuv.update_data service.

Additionally, this PR fixes a bug (caused by a recent OpenUV change) where users would see "Invalid API Key" when configuring the component, even if the API key was valid.

BREAKING CHANGE: users will now need to use the openuv.update_data service to request new data from the API.

Related issue (if applicable): fixes #19931 and fixes #19950

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

Example entry for configuration.yaml (if applicable):

openuv:
  api_key: !secret openuv_api_key

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.

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

@bachya bachya self-assigned this Jan 11, 2019

@wafflebot wafflebot bot added the in progress label Jan 11, 2019

@bachya bachya changed the title WIP: Adjust OpenUV integration for upcoming API limit changes Adjust OpenUV integration for upcoming API limit changes Jan 11, 2019

@balloob

This comment has been minimized.

Copy link
Member

balloob commented Jan 11, 2019

What if openUV reports on a different location than the sun?

@bachya

This comment has been minimized.

Copy link
Contributor Author

bachya commented Jan 11, 2019

Great point – won't help to halt the API on EST while tracking GMT. 😆

How would you feel about altering homeassistant.helpers.sun.get_astral_location to take an optional latitude/longitude (using the values defined in configuration.yaml if not provided)? In that case, we'd have to add/alter the other astral helpers to handle those coordinates.

@Gamester17

This comment has been minimized.

Copy link

Gamester17 commented Jan 11, 2019

@bachya While its only semantics since 50 requests per day is more one request per 30-minute for the 24-hours of a day I would still just like to point out that Home Assistant used worldwide around the globe so there should be no "typical amount of UV hours from the sun per day" in this context. Sure you could calculate an average but it will, in reality, most likely be irrelevant in this use-case. For example, in the northern part of Sweden the sun never goes even down at all during the summer solstice (the longest day of the year), and in the southern part of Sweden the sun goes up at around 4:20 AM and goes down at around 22:00 on the same day, the summer solstice (which is on the 21st of June this year but it happens on a different day each year).

@bachya

This comment has been minimized.

Copy link
Contributor Author

bachya commented Jan 11, 2019

@Gamester17: appreciate that perspective. I’m open to alternate implementations if you feel this one cuts it too close in locations that are "extra sunny" (including sticking with this implementation and encouraging users in those locales to reduce their scan intervals).

@bachya

This comment has been minimized.

Copy link
Contributor Author

bachya commented Jan 11, 2019

Another option would be to remove polling altogether and create a service to update the data manually; then, users would have control (and responsibility) over how often updates occur.

This route would also eliminate needing to make any changes to the sun helper.

EDIT: after thinking about it, I like this approach, as it doesn't snake into the sun helper and gives everyone the freedom to do what they wish.

bachya added some commits Jan 11, 2019

@bachya bachya force-pushed the bachya:openuv-api-update branch from a5639e8 to f41e408 Jan 12, 2019

bachya added some commits Jan 12, 2019

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Looks good!

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Jan 13, 2019

Can be merged when build passes.

@bachya bachya referenced this pull request Jan 13, 2019

Merged

Embed OpenUV platforms into the component #20072

3 of 3 tasks complete

@fabaff fabaff merged commit ef79566 into home-assistant:dev Jan 14, 2019

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.002%) to 93.029%
Details

@wafflebot wafflebot bot removed the in progress label Jan 14, 2019

@bachya bachya deleted the bachya:openuv-api-update branch Jan 14, 2019

@balloob balloob referenced this pull request Jan 23, 2019

Merged

0.86.0 #20354

alandtse added a commit to alandtse/home-assistant that referenced this pull request Feb 12, 2019

Adjust OpenUV integration for upcoming API limit changes (home-assist…
…ant#19949)

* Adjust OpenUV integration for upcoming API limit changes

* Added fix for "Invalid API Key"

* Bugfix

* Add initial nighttime check

* Move from polling to a service-based model

* Fixed test

* Removed unnecessary scan interval

* Fixed test

* Moving test imports

* Member comments

* Hound

* Removed unused import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment