Skip to content

Add new integration here_weather#28910

Closed
eifinger wants to merge 56 commits into
home-assistant:devfrom
eifinger:here_weather
Closed

Add new integration here_weather#28910
eifinger wants to merge 56 commits into
home-assistant:devfrom
eifinger:here_weather

Conversation

@eifinger
Copy link
Copy Markdown
Contributor

@eifinger eifinger commented Nov 20, 2019

Proposed change

Add a new integration using the HERE Destination Weather API to provide weather and sensor entities.

The integration provides the following four modes:

  • Astronomy: Sunrise, Sunset and Moonphase
  • Hourly: Weather forecast in an hourly format
  • Daily: Weather forecast in a dailyformat
  • Daily Simple: Like Daily but with high/low temp, UV-index and pressure
  • Observation: Detailed precipitation for the next 24h

All sensors and all weather entities but the Daily Simple are disabled by default.
The information is refreshed every 300s or less if the number of active integrations is too high. This way it is automatically made sure that the free limit of 250k request per month ist not exceeded.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Nov 28, 2019

@eifinger Does this also obtain the moon data (moonrise and set times)? Here is one of the few providers that provide this information. I think the herepy library provides that information.

@eifinger
Copy link
Copy Markdown
Contributor Author

@arsaboo with the following config:

sensor:
- platform: here_weather
  name: Home
  app_id: "<APP_ID>"
  app_code: "<APP_CODE>"
  mode: forecast_astronomy

you will get the following sensors:

"sunrise": {"name": "Sunrise", "unit_of_measurement": None},
 "sunset": {"name": "Sunset", "unit_of_measurement": None},
"moonrise": {"name": "Moonrise", "unit_of_measurement": None},
"moonset": {"name": "Moonset", "unit_of_measurement": None},
"moonPhase": {"name": "Moon Phase", "unit_of_measurement": "%"},
"moonPhaseDesc": {"name": "Moon Phase Description", "unit_of_measurement": None},
"city": {"name": "City", "unit_of_measurement": None},
"latitude": {"name": "Latitude", "unit_of_measurement": None},
"longitude": {"name": "Longitude", "unit_of_measurement": None},
"utcTime": {"name": "Sunrise", "unit_of_measurement": "timestamp"}

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Nov 28, 2019

Does it include the forecast information as well? We can have today's value as the value of the sensor and forecast as attributes.

@eifinger
Copy link
Copy Markdown
Contributor Author

The current implementation (which I am not all too happy with) creates a sensor for each information (e.g. moonrise). If you want to have the information for a specific point in the future you can also define offset: <DAYS> to get the information up to 7 days in the future.

Currently this means if you want to have the information for today and the information for 7 days in the future you would have to have the following config:

sensor:
- platform: here_weather
  name: Home
  app_id: "<APP_ID>"
  app_code: "<APP_CODE>"
  mode: forecast_astronomy
- platform: here_weather
  name: Home in 7 days
  app_id: "<APP_ID>"
  app_code: "<APP_CODE>"
  mode: forecast_astronomy
  offset: 7

I as I have said I am gladly taking ideas on how to to this differently.

@arsaboo
Copy link
Copy Markdown
Contributor

arsaboo commented Nov 29, 2019

I created a custom sensor just for the moon data. It creates one sensor with the state as the current moon phase and all the forecast information in attributes.

I don't want to pollute this PR with all the feature request discussion. I am discord (arsaboo) if you want to discuss it there.

@eifinger eifinger changed the title Add new platform here_weather WIP Add new platform here_weather Dec 11, 2019
@eifinger eifinger mentioned this pull request Dec 12, 2019
9 tasks
@eifinger eifinger changed the title WIP Add new platform here_weather Add new platform here_weather Dec 16, 2019
Comment thread requirements_all.txt
@stale

This comment has been minimized.

@stale stale Bot added the stale label Mar 21, 2020
@eifinger
Copy link
Copy Markdown
Contributor Author

Still open

@stale stale Bot removed the stale label Mar 21, 2020
@stale

This comment has been minimized.

@stale stale Bot added the stale label Apr 23, 2020
@eifinger
Copy link
Copy Markdown
Contributor Author

...

@stale stale Bot removed the stale label Apr 24, 2020
Comment thread homeassistant/components/here_weather/sensor.py Outdated
@stale

This comment has been minimized.

Comment thread homeassistant/components/here_weather/config_flow.py Outdated
Comment thread homeassistant/components/here_weather/const.py Outdated
Comment thread homeassistant/components/here_weather/sensor.py
Comment thread homeassistant/components/here_weather/const.py Outdated
Comment thread tests/components/here_weather/test_init.py Outdated
Comment thread homeassistant/components/here_weather/config_flow.py Outdated
@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Jul 23, 2021

Will this integration use the same api key as here_travel_time? If that's the case, I still think it's a really bad idea to add a new integration that uses the same connection credentials and api limits etc as an existing integration.

@eifinger
Copy link
Copy Markdown
Contributor Author

Yes, it uses same API keys and request limits. The HERE API does not differentiate between them

@eifinger
Copy link
Copy Markdown
Contributor Author

eifinger commented Sep 4, 2021

As this PR is now open for nearly 2 years (I think the all time record 😅) and a lot of people spent a lot of time and effort on it I am currently debating whether it would be best for all if I close the PR and publish this integration as a custom component. This way we avoid spending even more time.

@MartinHjelmare you spent the most time with me on this PR what do you think?

@MartinHjelmare
Copy link
Copy Markdown
Member

The main problem is that this new integration would share api key and api limits with another integration. I don't think we should do that.

A custom integration is a good alternative until we have a solution for the above problem, I think.

Thanks for your work and contributions! 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants