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

Add mode (daily/hourly) to darksky #16719

Merged
merged 2 commits into from Sep 24, 2018

Conversation

Projects
None yet
4 participants
@randellhodges
Contributor

randellhodges commented Sep 19, 2018

Description:

Added a daily mode (like openweathermap) as an option. Also included wind_bearing, ozone, and visibility to current conditions.

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

weather:
  - platform: darksky
    mode: daily

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

@wafflebot wafflebot bot added the in progress label Sep 19, 2018

@randellhodges randellhodges referenced this pull request Sep 19, 2018

Merged

Add mode to darksky #6317

2 of 2 tasks complete

@randellhodges randellhodges changed the title from Added daily mode to darksky and wind_bearing, ozone, and visibility to Added daily mode to darksky Sep 19, 2018

@randellhodges randellhodges changed the title from Added daily mode to darksky to Add mode (daily/hourly) to darksky Sep 19, 2018

ATTR_FORECAST_WIND_SPEED = 'wind_speed'
ATTR_FORECAST_WIND_BEARING = 'wind_bearing'
ATTR_FORECAST_DEW_POINT = 'dew_point'
ATTR_FORECAST_PRESSURE = 'pressure'

This comment has been minimized.

@pvizeli

pvizeli Sep 20, 2018

Member

This component follow a internal standard. If you want extend this, you need first make a request for this in: https://github.com/home-assistant/architecture

After that you need extend the base class, demo and UI and update the documentation about this entity.

This comment has been minimized.

@randellhodges

randellhodges Sep 20, 2018

Contributor

Are you referring to dew point and pressure? I can remove those and discuss extending later. wind_speed and wind_bearing are already on the base class as properties and already in use by openweathermap as part of the forecast.

This comment has been minimized.

@randellhodges

randellhodges Sep 21, 2018

Contributor

I removed dew point and pressure. I noticed the ecobee is setting pressure as an attribute, so there might need to be a larger effort here, unless I'm missing the point (which I might be).

I'll submit something to the standard for those 2. I think having them available as both current and forecast properties is a value add for a weather component. Any other weather property you can think of that would be good to have available?

This comment has been minimized.

@fabaff

fabaff Sep 24, 2018

Member

The weather platforms should be on the "lowest common denominator" to make them exchangeable. It might be that one third-party service is delivering a value but none of the others. Thus it doesn't make much sense to include it.

For the full flexibility we have the weather sensors which can be used if you want a specific value.

Feel free to start a discussion on https://github.com/home-assistant/architecture because what was the case yesterday could have changed today.

@fabaff

fabaff approved these changes Sep 24, 2018

Thanks 🐦

@fabaff fabaff merged commit b52e852 into home-assistant:dev Sep 24, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 93.544%
Details

@wafflebot wafflebot bot removed the in progress label Sep 24, 2018

@balloob balloob referenced this pull request Oct 12, 2018

Merged

0.80.0 #17361

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