Skip to content

Add precipitation to weather card#1098

Closed
escoand wants to merge 4 commits intohome-assistant:masterfrom
escoand:add-weather-precipitation
Closed

Add precipitation to weather card#1098
escoand wants to merge 4 commits intohome-assistant:masterfrom
escoand:add-weather-precipitation

Conversation

@escoand
Copy link
Copy Markdown

@escoand escoand commented Apr 17, 2018

I've added the precipitation into the forecast chart.

Pull-request for OpenWeatherMap component: home-assistant/core#13971

@c727 c727 mentioned this pull request Apr 22, 2018
@c727
Copy link
Copy Markdown
Contributor

c727 commented Apr 24, 2018

stupid question: no weather component has data for this in its forecast. so where do you get the data from?

@c727
Copy link
Copy Markdown
Contributor

c727 commented Apr 24, 2018

please test your stuff before you create a PR

@c727 c727 closed this Apr 24, 2018
@escoand
Copy link
Copy Markdown
Author

escoand commented Apr 24, 2018

Which came first, chicken or egg? In this case both: home-assistant/core#13971

@c727
Copy link
Copy Markdown
Contributor

c727 commented Apr 24, 2018

Link the PR in the description please and add screenshots

@c727 c727 reopened this Apr 24, 2018
@escoand
Copy link
Copy Markdown
Author

escoand commented Apr 24, 2018

Thanks for reopening. I've referenced the other way around and this is visible on the website. But you're right, it's not visible ie in the OctoDroid Android App.

@escoand
Copy link
Copy Markdown
Author

escoand commented Apr 29, 2018

Currently there is no unit for the values, but this is a problem for the temperatures as well. The openweathermap precipitation is in mm. Should we at this to the HTML?

@c727
Copy link
Copy Markdown
Contributor

c727 commented Apr 29, 2018

we have units defined here hass.config.core.unit_system -> length. for metric we use [mm] for precipitation

@c727
Copy link
Copy Markdown
Contributor

c727 commented Apr 29, 2018

... and imperial uses inch/in.
the conversion (if required) should be done by the backend part

@c727 c727 changed the title add precipitation Add precipitation to weather card Apr 29, 2018
@escoand
Copy link
Copy Markdown
Author

escoand commented Apr 29, 2018

Ok, thanks I'll look into it. But I meant if the unit should be visible.

@escoand
Copy link
Copy Markdown
Author

escoand commented Apr 30, 2018

Backend PR was merged. Maybe we do the unit thing in a second PR.

@c727
Copy link
Copy Markdown
Contributor

c727 commented Apr 30, 2018

getUnit(measure) {
  if (measure === 'precipitation') {
    return this.getUnit('length') === 'km' ? 'mm' : 'in';
  }
  return this.hass.config.core.unit_system[measure] || '';
}

what about the more-info-card? https://github.com/home-assistant/home-assistant-polymer/blob/master/src/dialogs/more-info/controls/more-info-weather.html

@escoand
Copy link
Copy Markdown
Author

escoand commented May 2, 2018

Don't know if every weather provider gives this value as mm/inch or if we get rain probability and therefore should pass the unit together with the data.

@c727
Copy link
Copy Markdown
Contributor

c727 commented May 2, 2018

Then you should check this

@c727
Copy link
Copy Markdown
Contributor

c727 commented May 29, 2018

I solved the merge conflicts in #1221, If we get problems with incorrect units that should be fixed in main repo components

@c727 c727 closed this May 29, 2018
@escoand escoand deleted the add-weather-precipitation branch May 29, 2018 04:48
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 8, 2022
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.

3 participants