Add NWS weather

[MatthewFlamm]

Description:

Adds NWS weather platform. NWS weather.

There are two forecast modes supported by NWS, one is a separate day and night forecast (two forecasts a day) or hourly. Hourly works well with the standard card. Day and night displays the hour of the forecast which is confusing. This platform has an additional forecast attribute to tag which forecast is day or night. A precipitation probability is also exposed. A modified weather card PR in the frontend is planned to utilize these features to better support day and night forecasts.

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

Example entry for configuration.yaml (if applicable):

weather:
  - platform: nws
    api_key: my_email

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:

• Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly (example).
• New dependencies have been added to requirements in the manifest (example).
• New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
• New files were added to .coveragerc.

If the code does not interact with devices:

• Tests have been added to verify that the new code works.

[fabaff]

Just some comments to get the review started.

[MatthewFlamm]

I'm going to remove the TODO from the PR. My frontend dev environment is totally hosed, and I'm not sure how to fix it. My frontend changes did work several versions ago, but I can't test that it works today. I don't want to bog down this PR with these issues.

[MatthewFlamm]

#23875 is a similar PR for a NWS/NOAA sensor platform, which uses the same data source and library. These should be considered together, as we have a few inconsistencies across the two implementations. I'm proposing that we work together to resolve the inconsistencies, unless there is a preference to implement the weather platform or the sensor platform first. Then we can resolve the differences during the second PR.

[MatthewFlamm]

I have added a fall back to parsing the metar code if the regular data is missing. This is to be consistent with #23875, as well as fixes issues with some stations having no formatted data but valid metar codes sometimes. I also changed it to only retrieve the latest observation since we aren't using prior observations.

The biggest mismatch in approach I see now with #23875 , is with obtaining the condition to convert to HA. This PR uses the icon url which has a published catalog of possible values and meanings. The sensor PR parses a text description.

[rewsssiestedbo]

I have added a fall back to parsing the metar code if the regular data is missing. This is to be consistent with #23875, as well as fixes issues with some stations having no formatted data but valid metar codes sometimes. I also changed it to only retrieve the latest observation since we aren't using prior observations.

The biggest mismatch in approach I see now with #23875 , is with obtaining the condition to convert to HA. This PR uses the icon url which has a published catalog of possible values and meanings. The sensor PR parses a text description.

It is probably best if I just remove the code to translate the NWS textDescription values to HA condition codes. None of the other weather sensors use the HA condition codes, and unlike for the weather platforms, there is nothing that depends upon the specific HA condition codes. I had mainly put it in as a basis for a future weather platform module, but since you have that module completed, there is no reason to keep it.

[rewsssiestedbo]

The code doesn't handle the exceptions that can be raised from the pynws methods: if the longitude/latitude is outside the area covered by the NWS, the API will return an HTTP 404 error, which pynws turns into a ClientReponseError exception. The same thing happens if the station code is not valid. These should be turned into a ConfigEntryNotReady exception. Other types of errors from the pynws methods typically indicate an issue with the connection to the API (or between the Akamai edge server and the NWS backend), and should turn into a PlatformNotReady (on any call during the initial setup), or treated as a missed poll during normal updates.

[rewsssiestedbo]

I starting running this component yesterday. Today it logged a number of errors:

  File "/home/homeassistant/.homeassistant/custom_components/nws/weather.py", line 268, in wind_bearing
    wind_bearing = self._metar_obs.wind_dir.value()
AttributeError: 'NoneType' object has no attribute 'value'

This appears to be a result of the wind direction value in the JSON being null, and the field in the METAR data saying VRB (variable), which results in no value from the parsed metar record either. An example of this is the observation data for KPHL at "https://api.weather.gov/stations/KPHL/observations/2019-05-23T20:02:00+00:00"

[MatthewFlamm]

AttributeError: 'NoneType' object has no attribute 'value'

This should be fixed now. I'll work on handling errors especially during platform setup as these seem to be fatal.

[MatthewFlamm]

@fabaff I feel that the code here has ballooned too much. I'm working on pulling a lot of the messy logic into the upstream pypi library to provide a cleaner interface. This is almost done. Once that is complete, should I push here, or open a new PR?

[MartinHjelmare]

I would say, continue here, thanks!

[MartinHjelmare]

If it's going to take a while until the new changes are ready, please close the PR, then reopen it when you're ready for a review. We're trying to decrease our open PR buffer. Thanks!

[MatthewFlamm]

Thanks, Changes are very close to being finished, I'll continue here

[MartinHjelmare]

Good! Now it's just the tests left to either adjust per my comment above, or remove them and exclude the integration in .coveragerc.

[MatthewFlamm]

Thanks! I found the ipma integration had tests in pytest form that I can try working off of. I will give it a shot, but if I'm making no progress, I will take the second route.

[sofar]

I'm still really looking forward to using this from stock. I've been using it as a local integration for a while without issues and while I'm probably not running the latest, it's been well worth it - this beats many of the other services that require a subscription.

[MatthewFlamm]

Is there anything else needed here?

@sofar If this doesn't move, I'll update the custom component repo to support HACS so it is easier to stay on top of versions.

[MatthewFlamm]

At risk of setting this PR back even farther, I have a working config flow setup for this integration that basically just as asks for all the fields in the configuration. I'm trying to figure out how to get some of the options into an option flow witha list of possible values.

For example,.I set lat/long in the config flow. Then I set up my entity with the default weather station for the lat/lon. The options flow ideally would get a list of nearly stations either using API call, or even better taking it from the entity directly, and provide a list to the user. I cannot find any examples of this usage flow however. I only see text forms and check boxes. It doesn't even seem possible to dynamically update the entry names on the data flow form, which would be a hack method anyway.

[MartinHjelmare]

Let's not add anything more to this PR. That can come in the next PR.

[MartinHjelmare]

Great!

[MatthewFlamm]

Thank you for your reviewing time and support @MartinHjelmare, @fabaff, and @rewsssiestedbo.