home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
72.03k stars 30.17k forks source link

Weather Dark Sky, wrong unit for wind speed #16793

Closed endor-force closed 5 years ago

endor-force commented 6 years ago

Home Assistant release with the issue:

0.78.2

Last working Home Assistant release (if known): N/A

Operating environment (Hass.io/Docker/Windows/etc.):

Hass.io

Component/platform:

https://www.home-assistant.io/components/weather.darksky

Description of problem: Setting up weather darksky and looking at the value it is clear that the wind speed is reported in the wrong unit. For metric it is reported in km/h even if forcing configuration option units to si

We need to introduce wind speed unit in the weather or dark sky sensors

Looking at darksky web site it is clear how it should be:

<select class="units" tabindex="-1">
      <option value="us">˚F,&nbsp;mph</option>
      <option value="si">˚C,&nbsp;m/s</option>
      <option value="ca">˚C,&nbsp;km/h</option>
      <option value="uk2">˚C,&nbsp;mph</option>
    </select>

As a reference, sensor/darksky.py looks correct

SENSOR_TYPES = {

'wind_speed': ['Wind Speed', 'm/s', 'mph', 'km/h', 'mph', 'mph',
                   'mdi:weather-windy', ['currently', 'hourly', 'daily']],

}

    def update_unit_of_measurement(self):
        """Update units based on unit system."""
        unit_index = {
            'si': 1,
            'us': 2,
            'ca': 3,
            'uk': 4,
            'uk2': 5
        }.get(self.unit_system, 1)
        self._unit_of_measurement = SENSOR_TYPES[self.type][unit_index]

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

weather:
  - platform: darksky
    api_key: !secret darksky_api
    units: si

Traceback (if applicable):

N/A

Additional information:

DubhAd commented 6 years ago

See this other issue that I raised too. The problem here is that the units for the weather component don't seem to be stored anywhere accessible.

gurbyz commented 5 years ago

Same is happening for weather.buienradar component. #18558 Here also the unit for windspeed is shown as 'km/h' but should be 'm/s'. And here also the unit is correct in the separate Buienradar Sensor for weather.

Maybe the issue is on a higher level than these components (darksky, buienradar) are?

allgeyer commented 5 years ago

This has been solved in the weather.smhi component by converting 'm/s' to 'km/h'. So a solution would be to change the code to:

    @property
    def wind_speed(self):
        """Return the wind speed."""
        # Convert from m/s to km/h
        return round(self._ds_currently.get('windSpeed') * 18 / 5)
endor-force commented 5 years ago

That's one way to do workaround it. Unfortunately for long term, measuring wind speed in km/h is just wierd. It should better be an option in the weather component to specify unit of measure and hass to follow..

Mariusthvdb commented 5 years ago

somehow this seems a non-discussion. Since all platform sensors are correctly showing their windspeed in m/s, why do we even have to discuss this here? This simply is a bug that could and should be taken out quite easily. Darksky and Buienradar are incorrect, Open weather Map is correct (albeit showing in km/hr, but nicely multiplied by 3.6) Didn't test others, so this might not be conclusive...;-)

balloob commented 5 years ago

The developer documentation states that it is km/h, so that's what it should be.

If you don't like that, and you're willing to champion this issue and make all the required changes needed once a consensus has been reached, open an architecture issue.

Mariusthvdb commented 5 years ago

Km/h would be fine if the unit would match the speed... Right now m/s is the speed and unit shows km/h

Btw, architecture issue is raised.

Petro31 commented 5 years ago

Is this still an issue? Have you upgraded to the latest version?

Rantaki commented 5 years ago

Don’t know, not using it anymore...

reevery commented 5 years ago

Hi, yes it is still an issue. The value is correct (in mph, same for visibility and nearestStormDistance in miles) but the unit is incorrect (kph/km, should be miles and mph). Whilst we're on this component, can we change the default to auto, not us?

configuaration.yaml:

  - platform: darksky
    api_key: !secret darksky_key
    units: uk2

image

For reference, when calling the API using postman:

https://api.darksky.net/forecast/<api_key>/<lat>,<lon>?units=uk2
{
    ...
    "currently": {
        "nearestStormDistance": 102,
        "windSpeed": 9.66,
        "visibility": 6.61,
    }.
    "flags": {
        "units": "uk2"
    },
}
https://api.darksky.net/forecast/<api_key>/<lat>,<lon>?units=ca
{
    ...
    "currently": {
        "nearestStormDistance": 164,
        "windSpeed": 15.55,
        "visibility": 10.59,
    }.
    "flags": {
        "units": "ca"
    },
}

Also for reference, here is OpenWeatherMap which was not configured for units (I think it's hard-coded to metric): image

tanus10 commented 5 years ago

This is still an issue. It has been for a couple of years actually. (Yes it has been reported before)

To clarify, Let say the actual wind speed is 3.2 m/s (reported by Dark Sky with units: si) Currently, Homeassistant wrongly displays it as 3.2 km/h

For each 'units' parameter in configuration.yaml under various weather platform, corresponding weather services report appropriately converted values per requests.

For Dark Sky, units:si means reporting wind speed in meter for second or m/s. Receiving this data, Homeassistant displays it as km/h WITHOUT any conversion! HA just attaches km/h text string at the end of given value. (Of course, we made sure all HA user-side configurations are correct, Dark Sky is sending correct value, etc)

It is just a bug which has not been fixed for years, probably because some people think it is just a discussion of partial preference of units.

It is not.

Even if Homeassistant is designed to display wind speed only in km/h or mph (which is not), then it needs to do a mathematical conversion first. Not just swapping units text string like HA is doing right now.