[openweathermap] Add support for persisting OneCall API forecasts

[florian-h05]

Implement time series support introduced by openhab/openhab-core#3597 for all forecast channels of the OneCall API weather and forecast Thing.

Also change the behaviour of the handler, so that setting forecastMinutes, forecastHours and forecastDays to 0 does not exclude the respective data from the API response. So these config parameters can be used to disable the individual channel groups for each time-step.

[florian-h05]

@jlaur I guess I need to use update instructions here to make those new channel groups available for "old" Things. Can you probably provide some help here since I have never worked with update instructions before?

[jlaur]

@jlaur I guess I need to use update instructions here to make those new channel groups available for "old" Things. Can you probably provide some help here since I have never worked with update instructions before?

You could have a look at https://www.openhab.org/docs/developer/bindings/thing-xml.html#updating-thing-types and:

openhab-addons/bundles/org.openhab.binding.danfossairunit/src/main/resources/OH-INF/update/instructions.xml

Lines 12 to 20 in a4da986

	<instruction-set targetVersion="2">
	<update-channel id="supply_fan_speed" groupIds="main">
	<type>danfossairunit:supplyFanSpeed</type>
	</update-channel>
	<update-channel id="extract_fan_speed" groupIds="main">
	<type>danfossairunit:extractFanSpeed</type>
	</update-channel>
	</instruction-set>

[florian-h05]

Thanks.

Is there a way to add a whole channel group in one step? Or do I have to add each individual channel?

[florian-h05]

@jlaur I got the Thing update instructions working, so from my POV this is ready for merging.

[jlaur]

Thanks for implementing time series support! LGTM, although I didn't go into full depth - I assume you already tested this as usual. 🙂 Only a few minor comments/questions.

Also, can you regenerate the I18N properties file?

[lolodomo]

I am a little off topic in this message.

In Main UI, the only way to show these values in future will then be to look at the chart ?

We have still no way with sitemap UIs ? I believe at a certain time @J-N-K mentioned the need for a way to request chart period in the future for charts built by server. Is the chart servlet already compatible with these new time series ?

[jlaur]

In Main UI, the only way to show these values in future will then be to look at the chart ?

Yes.

We have still no way with sitemap UIs ? I believe at a certain time @J-N-K mentioned the need for a way to request chart period in the future for charts built by server. Is the chart servlet already compatible with these new time series ?

Sitemaps are actually still my primary way of interacting with openHAB from the Android app, but it's configured only for my production system which is still 4.0. I don't remember if I tried to configure a sitemap with a chart to inspect in BasicUI.

If you want to try it out you can simply install Energi Data Service binding. It doesn't require any authentication, and only one required parameter: Price area. Set that to one of the suggested values (e.g. DK1), and you'll get future prices for channel spot-price. Let me know if I can help. It seems that sitemaps in the app are quite retrospective (looking back), so maybe this was missed. Which means that actually the feature would be quite useless for my use-case, which would be viewing future prices in a sitemap chart in the app. Would be cool if anything is still possible to enhance in this area, but I guess it also requires changes in the app?

[jlaur]

Which means that actually the feature would be quite useless for my use-case, which would be viewing future prices in a sitemap chart in the app. Would be cool if anything is still possible to enhance in this area, but I guess it also requires changes in the app?

@lolodomo - seems not supported, this is how it looks from BasicUI, only options for looking back in time:

[florian-h05]

@jlaur Thanks for the review.

LGTM, although I didn't go into full depth - I assume you already tested this as usual.

Yes, I have tested this on my dev system. I haven't tested all channels, but at least one minutely, one hourly and one daily to make sure the new code for creating time series works properly (the code to get the state is the same as before, just moved to a new private method - GitHub diff view is really bad a moved code ...). The update instructions also work fine.

Also, can you regenerate the I18N properties file?

Done.

For the other changes I've done, please have a look at my answers to your comments.

[florian-h05]

In Main UI, the only way to show these values in future will then be to look at the chart ?

Yes. Unfortunately, there it is not possible (yet) to set a chart to a future time range as default, you always have to manually go to the future.

@lolodomo - seems not supported, this is how it looks from BasicUI, only options for looking back in time:

BasicUI would need to provide options for selecting future time ranges, but the chart servlet would also need to support future time ranges. Not sure if it does, but this should be easy to find out. Just open the browser dev tools when viewing BasicUI and find the request to the chart servlet. I would guess time range is passed as query param, so you should be able to adjust the URL and try to request a a future chart.

[lolodomo]

The chart servlet calls the default chart provider with two dates, the start time and the end time. Here is the method in the chart provider
finally retrieving the data. Is this method compatible with the new feature?
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/chart/defaultchartprovider/DefaultChartProvider.java#L308

Thr chart servlet already supports with its calling parameters the request of a chart in future. You just have to set "begin" parameter to now and the "period" parameter as usual. So nothing to change here.

Sitemap UIs need a new input to choose between past and future. Then it is just a small change of the chart URL to be done by each UI.
The syntax of the Chart sitemap element would require a new parameter to choose between past and future. Default would be past for backward compatibility.

[lolodomo]

In fact, UIs should always set either begin param or end param with the current date & time.
begin -> chart in future
end -> chart in past

[lolodomo]

As a quick fix in Basic UI, I can add a new switch in the header row to switch between past and future.

But the user will still not be able to request directly a chart in future until we change the sitemap syntax. The parameter to add could be named "inFuture" ? Another option that is more a workaround would be to use a negative period for charts in future like for example -D for the day in future.

[jlaur]

@lolodomo - maybe a new issue should be created for adding support for future states in sitemap charts?

[florian-h05]

@jlaur I have addressed your review and fixed DCO (Sign-Off was missing because one of my commits this morning was from GitHub web), therefore the force push.

[lolodomo]

Just to be clear, does this PR break something ? If a user simply upgrades OH without changing anything, will he loose some features or not ?

[florian-h05]

With the current implementation, this does not break anything and there will be no loss of functionality. The README just suggests/encourages to use the time series channels instead of the current channels.

[jlaur]

LGTM