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

Darksky: Expose missing conditions for day 0 forecast #18312

Merged
merged 1 commit into from Nov 19, 2018

Conversation

Projects
None yet
4 participants
@leppa
Contributor

leppa commented Nov 7, 2018

Description:

See #18205 for a detailed background on this issue. This PR implements proposal 1 form there.

Dark Sky Sensor didn't expose conditions for day 0 (today forecast) that had the same name as current conditions. With this change all conditions form day 0 (today) forecast are exposed the same way as conditions for the rest of the days (1..7): as dark_sky_<condition>_<day>. As a consequence, conditions for day 0 that were already exposed now have _0 suffix. This actually improves the code by removing most of special handling, based on condition name.

To get today forecast the user now has to add - 0 to forecast configuration parameter.

Conditions, for which suffix _0 appeared: precip_accumulation, temperature_high, temperature_low, apparent_temperature_high, apparent_temperature_low, precip_intensity_max, moon_phase.

This is a breaking change!

Related issue (if applicable): fixes #18205

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: darksky
    api_key: YOUR_API_KEY
    forecast:
      - 0
    monitored_conditions:
      - <any>

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:

Darksky: Expose missing conditions for day 0 forecast
Dark Sky Sensor didn't expose conditions for day 0 (today forecast) that
had the same name as current conditions. With this change all conditions
form day 0 (today) forecast are exposed the same way as conditions for
the rest of the days (1..7): as `dark_sky_<condition>_<day>`. As a
consequence, conditions for day 0 that were already exposed now have
`_0` suffix. This actually improves the code by removing most of
special handling, based on condition name.

To get day 0 conditions the user now has to add `- 0` to `forecast`
configuration parameter.

Conditions, for which suffix `_0` appeared: `precip_accumulation`,
`temperature_high`, `temperature_low`, `apparent_temperature_high`,
`apparent_temperature_low`, `precip_intensity_max`, `moon_phase`.

This is a breaking change!

Closes #18205

@leppa leppa requested a review from fabaff as a code owner Nov 7, 2018

@wafflebot wafflebot bot added the in progress label Nov 7, 2018

@leppa leppa referenced this pull request Nov 7, 2018

Merged

Update Dark Sky Sensor documentation #7424

2 of 2 tasks complete

@pvizeli pvizeli merged commit 01953ab into home-assistant:dev Nov 19, 2018

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.0005%) to 93.038%
Details

@wafflebot wafflebot bot removed the in progress label Nov 19, 2018

@leppa leppa deleted the leppa:darksky branch Nov 19, 2018

mxworm added a commit to mxworm/home-assistant that referenced this pull request Nov 19, 2018

Merge branch 'dev' of https://github.com/home-assistant/home-assistant
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant:
  Bumped ghlocalapi to 0.1.0 (home-assistant#18584)
  Prefix all xiaomi_aqara events (home-assistant#17354)
  Fix MQTT async_added_to_hass (home-assistant#18575)
  Add mikrotik SSL support (home-assistant#17898)
  Reconfigure MQTT binary_sensor component if discovery info is changed (home-assistant#18169)
  Darksky: Expose missing conditions for day 0 forecast (home-assistant#18312)
  Update pyhomematic to 0.1.52 and add features for lights (home-assistant#18499)
  Fix for epson state not updating (home-assistant#18357)
  Support for Point component (home-assistant#17466)
  Template binary sensor to not track all state changes (home-assistant#18573)
  Correct cached stale device tracker handling (home-assistant#18572)
  Add support for sessions (home-assistant#18518)
  Remove turn_on and turn_off feature for clients (home-assistant#18234)
  Log delay and wait_template steps in scripts (home-assistant#18448)
  Logbook speedup (home-assistant#18376)
  Avoid race in entity_platform.async_add_entities() (home-assistant#18445)
  Fix small issue related to topic prefix (home-assistant#18512)
  Enable native support + ADB authentication for Fire TV (home-assistant#17767)
  Re-adding the season attribute (home-assistant#18523)
@luca-angemi

This comment has been minimized.

Contributor

luca-angemi commented Nov 22, 2018

I've tested this in 83.b0
It seems that also humidity and temperature are affected as I have those sensors now with and without _0

image
image

my config:

- platform: darksky
  api_key: !secret darksky_secret
  language: es
  forecast:
    - 0
  monitored_conditions:
    - temperature
    - humidity 
    - temperature_low
    - temperature_high
    - hourly_summary
  update_interval: 
    minutes: 5
@leppa

This comment has been minimized.

Contributor

leppa commented Nov 22, 2018

@luca-angemi, take a look at table in #18205. The logic is:

  • conditions that had X in column 1 didn't change name.
  • conditions that had ? in column 2 were not available before. Now they're exposed as <condition>_0.
  • values that had - in column 1 and X in column 2 changed their name to <condition>_0.

So what you see now is: sensor.dark_sky_temperature is current air temperature, sensor.dark_sky_temperature_0 is forecasted air temperature for today. Same for humidity.

@leppa

This comment has been minimized.

Contributor

leppa commented Nov 22, 2018

By the way, regarding unknown value: API documentation for temperature states "optional, not on minutely". So, while they state that they provide it on all forecasts except minutely, the value is optional and they don't seem to provide it on daily forecast either.

@luca-angemi

This comment has been minimized.

Contributor

luca-angemi commented Nov 22, 2018

@leppa thanks very much for the quick response. It makes sense.

Cheers!

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