Adding OpenWeatherMap Minutely forecast action #128799

10100011 · 2024-10-19T18:51:00Z

Proposed change

This PR adds functionality to the OpenWeatherMap integration to expose Minutely foreasts as an Action. Minutely forecasts compliment Daily and Hourly forecasts by providing precipitation (rain or snow) levels for each minute of the next hour.

Instead of creating 60 separate sensors, or even 60 state attributes of a single sensor, this PR creates a new Service accessible as an OpenWeatherMap Action. It returns a response in the same format as weather.get_forecasts, with 60 objects containing a datetime in UTC (starting now) and precipitation in mm/h.

Key changes:

Upgrade to pyopenweathermap v0.2.1 library (comparison), which added Minutely functionality
New Service handle_get_minutely_forecast in services.py
Extraction of existing minutely_forecast data in coordinator.py (no additional API data is requested)

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: Adding Minutely forecast for Home Assistant freekode/pyopenweathermap#14
This PR is related to issue:
Link to documentation pull request: Adding OpenWeatherMap Minutely forecast action documentation home-assistant.io#35298

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

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

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant

Hi @10100011

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2024-10-19T18:51:04Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

home-assistant · 2024-10-19T18:51:07Z

Hey there @fabaff, @freekode, @nzapponi, mind taking a look at this pull request as it has been labeled with an integration (openweathermap) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of openweathermap can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign openweathermap Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

home-assistant

Hi @10100011

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Stale

bouwew

Is Minutely a proper word?

Better to use the OpenWeatherMap lingo: Minute forecast.

I now see that they use minutely as a parameter name. It's fine it use that word in the code.
But I suggest using Minute in texts that a User will see.

joostlek · 2024-10-20T13:25:50Z

homeassistant/components/openweathermap/coordinator.py

@@ -104,6 +111,14 @@ def _convert_weather_response(self, weather_report: WeatherReport):
            ],
        }

+    def _get_minutely_weather_data(self, minutely_forecast):


Can we add typing to the function?

joostlek · 2024-10-20T13:26:11Z

homeassistant/components/openweathermap/coordinator.py

+            for item in minutely_forecast
+        ]
+
+        return {f"{Platform.WEATHER}.{DOMAIN}": {"forecast": forecasts}}


Not sure how much this adds if its static

joostlek · 2024-10-20T13:26:32Z

homeassistant/components/openweathermap/coordinator.py

-            return round(precipitation["3h"], 2)
-        if "1h" in precipitation:
-            return round(precipitation["1h"], 2)
+        if precipitation is not None:


When would it be none?

The pyopenweathermap library >0.2.0 seems to have introduced the possibility of None values being included for rain and snow: wind_gust=14.92, wind_bearing=140, rain=None, snow=None.

I'll raise, and possibly fix, the issue on pyopenweathermap.

Edit: rain and snow only appear in the response from the API if they are due / above zero, which appears to cause the None. Previously, they were empty dictionaries.

joostlek · 2024-10-20T13:26:44Z

homeassistant/components/openweathermap/manifest.json

@@ -6,5 +6,5 @@
  "documentation": "https://www.home-assistant.io/integrations/openweathermap",
  "iot_class": "cloud_polling",
  "loggers": ["pyopenweathermap"],
-  "requirements": ["pyopenweathermap==0.1.1"]
+  "requirements": ["pyopenweathermap==0.2.1"]


Can we bump this in a separate preliminary PR?

Will do, pending resolution of the comment above (None values) as it may require a new version.

joostlek · 2024-10-20T13:27:16Z

homeassistant/components/openweathermap/services.py

+    if mode == OWM_MODE_V30:
+        hass.services.async_register(
+            domain=DOMAIN,
+            service=SERVICE_GET_MINUTELY_FORECAST,
+            service_func=handle_get_minutely_forecasts,
+            supports_response=SupportsResponse.ONLY,
+        )
+    else:
+        hass.services.async_remove(
+            domain=DOMAIN,
+            service=SERVICE_GET_MINUTELY_FORECAST,
+        )


Services should be registered in async_setup and should not be removed on runtime

This is only called when the user makes a change to the integration configuration (Settings -> Devices & Services -> OpenWeatherMap -> Configure) and changes the forecast mode of the integration to a mode which does not support Minute forecasting. As such, this isn't really on runtime: it's only when the user reconfigures the integration without first deleting it.

If the service isn't removed when using an unsupported mode, the user will simply be returned an empty forecast - with no feedback as to the reason. I would argue that is worse UX.

If the mode of the user is incorrect, you can raise a ServiceValidationError, explaining why it failed. More info about this can be found for now in https://github.com/home-assistant/developers.home-assistant/pull/2400/files

joostlek · 2024-10-20T13:27:34Z

homeassistant/components/openweathermap/services.yaml

@@ -0,0 +1 @@
+get_minutely_forecasts:


Don't we need to add a location or anything? What if I have OWM setup twice?

10100011 · 2024-10-20T14:32:37Z

Is Minutely a proper word?

Better to use the OpenWeatherMap lingo: Minute forecast.

I now see that they use minutely as a parameter name. It's fine it use that word in the code. But I suggest using Minute in texts that a User will see.

Agreed. I had thought that it fitted neatly with "daily" and "hourly", but you're right: it does not mean "minute-by-minute". Will change.

Adding OpenWeatherMap Minutely forecast action

81c1239

10100011 requested a review from fabaff as a code owner October 19, 2024 18:51

home-assistant bot previously requested changes Oct 19, 2024

View reviewed changes

home-assistant bot added cla-needed Hacktoberfest integration: openweathermap new-feature labels Oct 19, 2024

home-assistant bot marked this pull request as draft October 19, 2024 18:51

home-assistant bot assigned fabaff Oct 19, 2024

home-assistant bot added the Quality Scale: No score label Oct 19, 2024

Merge branch 'dev' into minutely-action

9026d16

home-assistant bot previously requested changes Oct 19, 2024

View reviewed changes

home-assistant bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Oct 19, 2024

10100011 marked this pull request as ready for review October 19, 2024 19:10

This was referenced Oct 19, 2024

Minutely action home-assistant/home-assistant.io#35296

Closed

Adding OpenWeatherMap Minutely forecast action documentation home-assistant/home-assistant.io#35298

Open

Merge branch 'dev' into minutely-action

01debb8

bouwew reviewed Oct 20, 2024

View reviewed changes

joostlek requested changes Oct 20, 2024

View reviewed changes

home-assistant bot marked this pull request as draft October 20, 2024 13:27

10100011 mentioned this pull request Oct 20, 2024

Rain and snow are returned as None when neither is forecast freekode/pyopenweathermap#16

Closed

Merge branch 'home-assistant:dev' into minutely-action

0b77ff1

10100011 added 5 commits October 21, 2024 13:37

Actioning review comments

fdf3958

Merge branch 'home-assistant:dev' into minutely-action

81a897b

Merge branch 'home-assistant:dev' into minutely-action

2b22638

Check for correct mode per action call

ede911f

Merge branch 'home-assistant:dev' into minutely-action

5e2a3dc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding OpenWeatherMap Minutely forecast action #128799

Adding OpenWeatherMap Minutely forecast action #128799

10100011 commented Oct 19, 2024 •

edited

Loading

home-assistant bot left a comment

home-assistant bot commented Oct 19, 2024

home-assistant bot commented Oct 19, 2024

home-assistant bot left a comment

bouwew left a comment •

edited

Loading

joostlek Oct 20, 2024

joostlek Oct 20, 2024

joostlek Oct 20, 2024

10100011 Oct 20, 2024 •

edited

Loading

joostlek Oct 20, 2024

10100011 Oct 20, 2024 •

edited

Loading

joostlek Oct 20, 2024

10100011 Oct 20, 2024 •

edited

Loading

joostlek Oct 20, 2024

joostlek Oct 20, 2024

10100011 commented Oct 20, 2024

		@@ -0,0 +1 @@
		get_minutely_forecasts:

Adding OpenWeatherMap Minutely forecast action #128799

Are you sure you want to change the base?

Adding OpenWeatherMap Minutely forecast action #128799

Conversation

10100011 commented Oct 19, 2024 • edited Loading

Proposed change

Type of change

Additional information

Checklist

home-assistant bot left a comment

Choose a reason for hiding this comment

home-assistant bot commented Oct 19, 2024

home-assistant bot commented Oct 19, 2024

home-assistant bot left a comment

Choose a reason for hiding this comment

bouwew left a comment • edited Loading

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

10100011 Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

10100011 Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

10100011 Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

joostlek Oct 20, 2024

Choose a reason for hiding this comment

10100011 commented Oct 20, 2024

10100011 commented Oct 19, 2024 •

edited

Loading

bouwew left a comment •

edited

Loading

10100011 Oct 20, 2024 •

edited

Loading

10100011 Oct 20, 2024 •

edited

Loading

10100011 Oct 20, 2024 •

edited

Loading