Skip to content
New 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

Add ability to set sitemap chart period to a future period #2518

Closed
1 of 4 tasks
higgers opened this issue Apr 1, 2024 · 22 comments · Fixed by openhab/openhab-core#4172
Closed
1 of 4 tasks

Add ability to set sitemap chart period to a future period #2518

higgers opened this issue Apr 1, 2024 · 22 comments · Fixed by openhab/openhab-core#4172
Labels
basic ui Basic UI enhancement New feature or request

Comments

@higgers
Copy link

higgers commented Apr 1, 2024

Which UI are you reporting an issue for?

  • Basic UI
  • HABPanel
  • HABot
  • CometVisu

The problem

I have changed my electricity supply tariff to the Agile tariff with Octopus in the UK. This is a dynamic tariff with a unit price per each 30 minute period of the day. The future (the prices for the next day are released at 16:00GMT every day) prices can be read via the Octopus API, for instance the prices for the North West can be found here.

I have persisted the future prices with some rub code with the help of JimT using a timeseries item and would like to be able to show the future prices in a chart in a sitemap but can only configure the chart to show historic prices.

Your suggestion

Allow charts in sitemaps to be configured to show future periods as well as past periods. Maybe allow future periods for the same durations as the current past periods.

Your environment

OH 4.1.2

Additional information

I opened a thread in the OH community about this here and was advised to raise an issue here.

@higgers higgers added the enhancement New feature or request label Apr 1, 2024
@lolodomo
Copy link
Contributor

lolodomo commented Apr 1, 2024

We just need to define the sitemap syntax to request a chart in future
We could use a negative period value to request in the future. It has the advantage there is nothing to change in the syntax I believe, but this is not really intuitive.
Doing the reverse, positive for the future and negative for the past would be more intuitive, but this will be an unexpected breaking change.
Or we could add another attribute in addition to the "period" to choose between past and future with a default being past for backward compatibility, something like "direction" with value "past" or "future" ?

Waiting for the best proposal.

@openhab/android-maintainers for info.

The change in Basic UI will then be easy.

@lolodomo lolodomo added the basic ui Basic UI label Apr 1, 2024
@lolodomo
Copy link
Contributor

lolodomo commented Apr 1, 2024

I just checked, a value starting by a minus for the period would be rejected by the current syntax.
So in any case we have to enhance the sitemap syntax first.

@lolodomo
Copy link
Contributor

lolodomo commented Apr 2, 2024

Ok, I believe I found an easy and relatively intuitive solution that is also backward compatible. The idea is to extend the value of the period attribute to accept an optional prefix, either "last:" or "next:".
So "2D" or "last:2D" for 2 days in the past and "next:2D" for 2 days in the future.

@mherwege
Copy link
Contributor

mherwege commented Apr 2, 2024

Maybe a stretch, but what if someone comes up with the request to have a timerange from past to future? I don't have a use for it, but I could see it being useful. The way this is going we would need yet another syntax to make that possible. I am not saying we should support this, but we may create something that would allow a reasonable syntax when we would do something like that. And in such a case I don't think we can get away from 2 (or even 3) arguments: "history", "future" and "resolution". You could skip "resolution" if you expect the user to always use the same for history and future (and give an error when not).

@andrewfg
Copy link

andrewfg commented Apr 2, 2024

a value starting by a minus for the period would be rejected by the current syntax.
"2D" or "last:2D" for 2 days in the past and "next:2D" for 2 days in the future
request to have a timerange from past to future

I suggest that we should not consider '-' as 'minus' symbol but consider rather as a 'range-until' marker. The new syntax would be X-Y meaning "from-x-in-past-until-y-in-future". Examples of such a range being as follows..

  • existing day range (always past) => D one day in past until now
  • future day range (always future) => -D now until one day in future
  • combined day range (past and future) => D-D one day in past until one day in future
  • combined week range (past and future) => W-W one week in past until one week in future
// last one day
Chart item=g_Boiler_Setpoint_Monitoring refresh=60000 period=D service="rrd4j"

// next one day
Chart item=g_Boiler_Setpoint_Monitoring refresh=60000 period=-D service="rrd4j"

// last one day until next one day
Chart item=g_Boiler_Setpoint_Monitoring refresh=60000 period=D-D service="rrd4j"

@higgers
Copy link
Author

higgers commented Apr 2, 2024

Maybe a stretch, but what if someone comes up with the request to have a timerange from past to future? I don't have a use for it, but I could see it being useful. The way this is going we would need yet another syntax to make that possible. I am not saying we should support this, but we may create something that would allow a reasonable syntax when we would do something like that. And in such a case I don't think we can get away from 2 (or even 3) arguments: "history", "future" and "resolution". You could skip "resolution" if you expect the user to always use the same for history and future (and give an error when not).

This is a good point, what if we want a chart that spans a period from the past to the future?

I like the suggestion of "last:" and "next:" to indicate past and future periods though I think the terms "past:" and "future:" are possibly more self documenting/intuitive.

What if we used symbols such as brackets or square backets to indicate that new syntax is being used? The existing syntax could be left as is and a new syntax introduced such as:

[past:2d-future:1W]

To indicate a period of two days into the past and one week into the future. The previous syntax could maybe be deprecated at a future point.

@mueller-ma
Copy link
Member

What about something like an offset?

// last one day
Chart item=g_Boiler_Setpoint_Monitoring refresh=60000 period=D service="rrd4j"

// next one day
Chart item=g_Boiler_Setpoint_Monitoring refresh=60000 period=-D service="rrd4j"

// last one day until next one day (from one day in the past, two days)
Chart item=g_Boiler_Setpoint_Monitoring refresh=60000 period=2D offset=-D service="rrd4j"

@andrewfg
Copy link

andrewfg commented Apr 2, 2024

[past:2d-future:1W]

As I suggested in my prior post, you do not need the prefixes. The syntax for the above range can be simplified to '2D-W' ..

@lolodomo
Copy link
Contributor

lolodomo commented Apr 2, 2024

I am ok with @andrewfg proposal.

This would also require a change in the chart servlet to support this new syntax and compute automatically start and end date/times from this period if not provided.

@higgers
Copy link
Author

higgers commented Apr 2, 2024

[past:2d-future:1W]

As I suggested in my prior post, you do not need the prefixes. The syntax for the above range can be simplified to '2D-W' ..

Indeed it can be simplified, yes. There are pros and cons to both verbose intuitive syntax and concise less intuitive syntax for users with a variety of skill levels and... ability to read documentation. I know, because I don't always read the documentation fully myself :)

@andrewfg
Copy link

andrewfg commented Apr 3, 2024

don't always read the documentation fully

Apropos this, @lolodomo, can you make a note that any eventual PR should also update the doc. :)

@lolodomo
Copy link
Contributor

lolodomo commented Apr 3, 2024

Apropos this, @lolodomo, can you make a note that any eventual PR should also update the doc. :)

Yes, obviously.

lolodomo added a commit to lolodomo/openhab-core that referenced this issue Apr 5, 2024
lolodomo added a commit to lolodomo/openhab-core that referenced this issue Apr 6, 2024
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Apr 6, 2024
kaikreuzer pushed a commit to openhab/openhab-core that referenced this issue Apr 7, 2024
* [sitemap] Extend chart periods to cover past and future

Closes openhab/openhab-webui#2518

Signed-off-by: Laurent Garnier <[email protected]>
kaikreuzer pushed a commit that referenced this issue Apr 7, 2024
@lolodomo
Copy link
Contributor

lolodomo commented Apr 7, 2024

Remains to do an update of openHAB documentation.

@mherwege
Copy link
Contributor

mherwege commented Apr 7, 2024

Also remains to adjust sitemap UI configuration.

@lolodomo
Copy link
Contributor

lolodomo commented Apr 7, 2024

Also remains to adjust sitemap UI configuration.

You mean sitemap generator in Main UI ?

@lolodomo lolodomo reopened this Apr 7, 2024
@mherwege
Copy link
Contributor

mherwege commented Apr 7, 2024

Also remains to adjust sitemap UI configuration.

You mean sitemap generator in Main UI ?

Yes, indeed, parser and configuration UI. I will have a look into that.

@andrewfg
Copy link

andrewfg commented Apr 7, 2024

@lolodomo maybe also the dropdown list for the time series button ??

image

@lolodomo
Copy link
Contributor

lolodomo commented Apr 7, 2024

@lolodomo maybe also the dropdown list for the time series button ??

Already done and merged.

florian-h05 pushed a commit that referenced this issue Apr 12, 2024
Chart widget sitemap configuration has been extended with support for
future periods in openhab/openhab-core#4172.
This adds configuration in the UI.

See #2518.

Signed-off-by: Mark Herwege <[email protected]>
lolodomo added a commit to lolodomo/openhab-docs that referenced this issue Apr 14, 2024
@lolodomo
Copy link
Contributor

I have now proposed the change in the documentation.

@openhab/ios-maintainers
@openhab/android-maintainers
Any change required in the iOS and Android app ?

@mueller-ma
Copy link
Member

The Android app handles the chart period as string, but doesn't try to parse it. So I guess it should work with the new syntax.

https://github.com/openhab/openhab-android/blob/main/mobile/src/main/java/org/openhab/habdroid/model/Widget.kt#L219

@lolodomo
Copy link
Contributor

The Android app handles the chart period as string, but doesn't try to parse it. So I guess it should work with the new syntax.

Fine.

Through the "menu" to update the chart period, you could now also propose predefined periods in the future, like I did in Basic UI.

stefan-hoehn pushed a commit to openhab/openhab-docs that referenced this issue Apr 19, 2024
@lolodomo
Copy link
Contributor

Documentation now updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
basic ui Basic UI enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants