openhab-core
openhab-core copied to clipboard
[REST API] Provide time zone in rest/persistence/items/ response
The /rest/persistence/items/<item> endpoint provides a list of data points, where each data point consists of a timestamp and state. The timestamp is provided as long without time zone information. This becomes a problem if the client and servee time zone do not match, as (as far as I can tell) there's no way of finding out the time zone the server is set to. If the client uses the data points to draw a chart, it has no chance of matching the output of the chart servlet.
Therefore I think it would be beneficial to provide the time zone as part of the REST response, as it would allow the client to restore the actual (local) time from the UTC timestamp.
See https://github.com/openhab/openhab-android/issues/3902 for the motivation behind this request ;-)
The long value implies unix epoch. Won't that provide enough information to reconstruct it? or should we change it to iso8601?
afaik, this information isn't yet being used in the Mainui / basicui, so changing it now shouldn't cause problems. @florian-h05, @lolodomo, @spacemanspiff2007?
@jimtng Thanks for thinking of HABApp! It's currently is using and processing this timestamp so that would be a breaking change which I would have to incorporate. I definitely support the addition of a timezone, preferable as an ISO timestamp.
Couldn't the solution be to provide a timezone parameter when requesting the chart servlet? Instead of using the server timezone, if the parameter is provided, it will return the chart with the requested timezone. That way clients can keep it consistent.
Won't that provide enough information to reconstruct it?
I don't see how, given there's no way (AFAICT) to get the server time zone.
afaik, this information isn't yet being used in the Mainui / basicui
MainUI uses this endpoint for its charts.
Couldn't the solution be to provide a timezone parameter when requesting the chart servlet?
Yes, that's also a possible option, but feels a bit 'indirect', as still there wouldn't be a way to get the local timestamp of persistence data.
I definitely support the addition of a timezone, preferable as an ISO timestamp.
You mean converting all of the timestamps from long to a timestamp formatted as string? My concern there is that there can be a lot of timestamp-value combinations in the response [1], and using strings would increase both JSON response and client side effort for parsing the string back to some numeric value.
My thought in the issue description was more in the direction of some field like "timezone": "Europe/Berlin" or something among those lines.
[1] In the Android app I try to limit response size to 500k items, but still, we're talking about a ~20 MB JSON response in the current format already.
http://openhab_server/rest/services/org.openhab.i18n/config will give you
{
"language": "en",
"location": "x,y",
"measurementSystem": "SI",
"region": "AU",
"timezone": "Australia/Brisbane"
}
I don't know much about this area. Vaguely remembered something about timezone provider, so this may not even be the server's timezone itself.
Yes, that's also a possible option, but feels a bit 'indirect', as still there wouldn't be a way to get the local timestamp of persistence data.
Is that relevant? The timestamp returned is a UNIX epoch, so always UTC. It is for visualisation it needs to be converted to a specific timezone. Logically, that could be the server timezone or the UI timezone. The challenge at the moment is you have no control over the chart servlet conversion to a specific timezone. It is always the server timezone. So it forces all else in the UI to also be in the server timezone to be consistent. If you have a specific option to get the chartlet in a specific timezone, it gives control to the UI and you can use the UI timezone. Adding the option to get the timezone with persistence data is just a proxy to retrieve the server timezone.
I don't know much about this area. Vaguely remembered something about timezone provider, so this may not even be the server's timezone itself.
It would be the timezone configured in OH, or the server timezone if there is none configured. But I believe chartlets should use this configured (or server) timezone. If not, that would be a bug in my view.
You mean converting all of the timestamps from long to a timestamp formatted as string? My concern there is that there can be a lot of timestamp-value combinations in the response [1], and using strings would increase both JSON response and client side effort for parsing the string back to some numeric value. My thought in the issue description was more in the direction of some field like "timezone": "Europe/Berlin" or something among those lines.
The response will be gzipped so the dates with timezone will deflate nicely and add almost nothing in overhead. When providing an ISO timestamp an off the shelf library can be used to parse these ISO timestamps and create the corresponding objects on the client side. These libraries are always faster then a custom implementation where the offset is fixed. Also this would work properly for edge cases, e.g. what is if there is a DST switch during the persistence interval? Which timezone would you report?
Reporting ISO timestamps would also align with how the item timestamps are reported.
In the end I am both happy with how it currently is or with ISO timestamps. But please no custom timezone handling (e.g. a TZ field that applies to the complete dataset).
I also agree with @mherwege: Currently there is no ambiguity since UNIX epoch is UTC. So either the server is not properly returning the UNIX epoch or there is a missing timezone switch on the client side.
I don't understand what's the problem with having UNIX epochs. You can easily convert this to the local timezone, or request the server zime zone. As the config endpoint is only accessible to admins, I have created #4826.
If you have a specific option to get the chartlet in a specific timezone, it gives control to the UI and you can use the UI timezone. Adding the option to get the timezone with persistence data is just a proxy to retrieve the server timezone.
I actually fully agree. We ideally should do both I think, an option to retrieve server time zone and a way to set time zone for chart servlet.
or there is a missing timezone switch on the client side.
@spacemanspiff2007 Not sure what kind of switch you're referring to here? If you mean a toggle between 'use client zone' and 'use server zone', then at the moment it's not possible to properly do either for the reasons below.
I don't understand what's the problem with having UNIX epochs. You can easily convert this to the local timezone, or request the server zime zone.
As I have mentioned, the problem with the former approach is consistency to chart servlet rendered charts. As you have noticed, the problem with the latter approach is that there's no general way to request the server time zone 😉
Thanks for that PR! It effectively solves the issue at hand.
Thanks for that PR! It effectively solves the issue at hand.
So what‘s remaining is that the chart serviert should support specifying the timezone to generate the chart for, correct?
@openhab/core-maintainers I think this can be closed.