-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
Fix wind speed change in NWS #37222
8000New 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
Fix wind speed change in NWS #37222
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nws changing the units in the api without some type of notification (https://www.weather.gov/documentation/services-web-api) is not great.
It would nice if the pypi package took care of all the conversions and presented the wind speed in a reliable unit so we didn't have to handle any changes in home assistant itself in case they make changes to the api again. Its probably hard to predict what they will change next though.
The ideal solution would be if NWS would publish a full specification of the data, then we could map any incoming unit to the one we need. Since this isn't the case: I'm going to implement logic in the pypi library to supply the units being reported and the known units (for wind speed this would be km_hr-1 and m_s-1). This still would require some overhead on the home assistant side, but only a mapping from NWS unit to home assistant unit would be required, i.e. this current change would be a one line change if an additional unit is introduced. As it is now, if it reverts back to m/s it will be an issue again. This is not good. I don't like the idea of converting units in the pypi library specifically for home assistant. The ideal state, IMO, is that it would be used by more than one downstream package, which would make it more robust and have more potential contributors. Also, it would require additional code burden for unit conversion or additional dependencies like |
The thought was to provide reliable units for anything consuming the pypi package regardless of |
Should we tag this for 0.112? |
You make a good point and this is actually easier all around. The unit conversion burden is also not super high. I will use the current units as the provided ones. It fixes a bug, so it seems like a good idea to get into a 0.112 release. |
Proposed change
NWS changes reported units of wind speed from m/s to km/hr. See #37203 .
Type of change
Example entry for
configuration.yaml
:Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale: