[DateTimeType] ZonedDateTime, Instant or neither?

[jlaur]

I'm creating this issue in order to discuss how DateTimeType should work. It seems like currently it internally stores a ZonedDateTime stripped of ZoneId, but preserving a ZoneOffset:

https://github.com/openhab/openhab-core/blob/cabb3f73150d13f44d647f46af8362e396684b92/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/DateTimeType.java#L75-L77

This means that information about Daylight Savings Time is lost. Expressed as DSL rule:

val zdt1 = now
val zdt2 = new DateTimeType().getZonedDateTime()
logInfo("zdt1", "toString: " + zdt1.toString() + ", zone: " + zdt1.getZone() + ", dst: " + zdt1.getZone().getRules().isDaylightSavings(zdt1.toInstant()))
logInfo("zdt2", "toString: " + zdt2.toString() + ", zone: " + zdt2.getZone() + ", dst: " + zdt2.getZone().getRules().isDaylightSavings(zdt2.toInstant()))

Result:

2022-04-06 22:11:29.194 [INFO ] [org.openhab.core.model.script.zdt1  ] - toString: 2022-04-06T22:11:29.183384+02:00[Europe/Berlin], zone: Europe/Berlin, dst: true
2022-04-06 22:11:29.203 [INFO ] [org.openhab.core.model.script.zdt2  ] - toString: 2022-04-06T22:11:29.183905+02:00, zone: +02:00, dst: false

(side note: I have [Europe/Copenhagen] configured in openHAB, so DSL rule engine doesn't seem to use TimeZoneProvider for now)

This has been a problem for at least openhab/openhab-addons#12546.

I'm wondering if ZoneId being stripped is a feature or a bug? It seems intended as withFixedOffsetZone() is explicitly called, I'm just not sure why.

Instant?

Second, a bigger topic: I'm wondering if it even makes sense to store a ZonedDateTime for DateTimeType. It's documented here: https://www.openhab.org/docs/concepts/items.html#datetimetype

When constructing a DateTimeType for a channel, we need an unambiguous timestamp which could be expressed as an Instant. Any logic that would need time-zone information could obtain this from another source, namely TimeZoneProvider. This of course must be possible anywhere needed, including within rules running in a scripting engine. The advantage would be that the logic is applied for the currently configured time-zone.

Currently when DateTimeType is displayed in any UI without any custom formatting for the channel or item, the raw ZonedDateTime string is displayed, for example: "2022-04-06T22:11:29.183905+02:00". In my opinion, this should be converted to local time. It is also worth noting that this string currently depends on the time-zone from the system or openHAB at the moment when the DateTimeType was created, not according to the currently configured time-zone. When using a pattern like "%1$tY-%1$tm-%1$td %1$tH:%1$tM:%1$tS" to convert to local time, it doesn't really matter which time-zone the system was in. All we need is an Instant and current ZoneId so we can display it in the local time-zone currently configured. And in fact when using mentioned pattern, the two DateTimes 2022-02-28T21:24:52.000+0000 and 2022-02-28T22:24:52.000+0100 are both displayed as 2022-02-28 22:24:52 for my time-zone.

[fwolter]

@doandzhi are you still around? Can you remember why you stripped the DST information when refactoring DateTimeType? See https://github.com/eclipse-archived/smarthome/pull/4259/files#diff-71d82b24bb4161c7b20eded1d3be769b77da8cdaeabeace24666ad8af7106461R64

Some unstructured thoughts to start a discussion:

I can't think of any use case for which it would be necessary to store the time zone information of the source of the DateTimeType. I would therefore agree that it makes sense to store the time as Instant in DateTimeType. There could be convenience methods to convert the DateTimeType into a LocalDateTime by a given time zone or the time zone provided by TimeZoneProvider.

When a binding creates a DateTimeType it should know whether the date & time retrieved from some device is a. UTC, b. a custom time zone or c. the local timezone (TimeZoneProvider). Depending on these use cases, DateTimeType could provide different constructors or a dedicated factory to keep the type clean. Side note: a custom source time zone could be configureable in the Thing configuration.

Modern browsers can provide the client's time zone upon request. I would expect date & time displayed in the MainUI to be in the client's local time by default. The offset of the displayed date & time should be independent from any time zone setting when the DateTimeType was created, as you already wrote.

The system time zone shouldn't play any role on this entire handling, since there might be use cases where you cannot configure the system time zone as @wborn pointed out (e.g. OH running in cloud).

I think the majority of user interactions with DateTimeType is in local time. When enhancing the interface of DateTimeType we should lead the user into this direction primarily. If you use date & time, you simply want to specify a time. Period. What you don't want is to understand the Java time concept, i.e. the difference between Instant, ZonedDateTime, LocalDate, LocalTime and LocalDateTime and/or decorated with a ZoneId. So, the interface must be easy to grasp and the documentation must be concise.

There must not be any breaking change to the interface of DateTimeType, because it is also used in user rules.

[J-N-K]

IMO the implementation should be changed to Instant. .getZonedDateTime() should then return that instant in the currently configured timezone. I can't think of a use-case where the actual timezone should be relevant and if so, then .toZone(String zone) should be used. However, this might be breaking in some cases, so probably this should be postponed until OH4.

[jlaur]

@J-N-K - what would be the correct way of making TimeZoneProvider available to DateTimeType? That would probably be the first step to provide compatibility. Then it could be be refactored internally to use Instant, and a constructor taking an Instant could also be added. And after that UI's could be reworked if needed (this part would be totally new terrain for me).

[openhab-bot]

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-0-wishlist/142388/106

[J-N-K]

Difficult. I have no good idea yet.

[jlaur]

@J-N-K, @fwolter - thinking about this again, does DateTimeType strictly have to use TimeZoneProvider? This seems like a "convenience conversion" if we would actually like to move time-zone out of DateTime. The question is if this is what we want.

If we would refactor all usages into using getInstant() instead of getZonedDateTime(), they could use TimeZoneProvider on their own for applying currently configured time-zone. Main UI might even in some cases prefer the time-zone from the browser (e.g. Intl.DateTimeFormat().resolvedOptions().timeZone) over the time-zone configured in openHAB, so in these cases it wouldn't be a loss not having this time-zone automatically returned from DateTimeType.

For backwards compatibility we could still use ZoneId.systemDefault() for getZonedDateTime(). This is not as good as TimeZoneProvider.getTimeZone() (e.g. for cloud installations), but is it worse than the time-zone which happened to be provided when the object was initialized? For DSL rules it seems like it might be worse when source time-zone is currently assumed.

Maybe I'm just trying to avoid facing the fact that I have no clue how to inject a provider into a type.

In any case, as preparation, I believe some usages of DateTimeType.getZonedDateDate() could already be refactored into using own time-zone (e.g. from TimeZoneProvider or browser) before we even start changing this class. This would also fix the issues locally in these places by using the current time-zone rather than the source time-zone.

Btw, the default constructor currently uses the system default time-zone (through ZonedDateTime.now()).

WDYT?

@ghys - perhaps you can give some advise how/where to approach this in Main UI, and/or if it would be viable to switch to browser time-zone here?

[fwolter]

thinking about this again, does DateTimeType strictly have to use TimeZoneProvider? This seems like a "convenience conversion" if we would actually like to move time-zone out of DateTime.

I agree that this can be seen as a convenience method. I'd further say that the use case of a timezone conversion in rules is pretty small as most OH installations are probably located in a single time zone. (Time zone conversion e.g. in MainUI could be done in the UI and time zone conversions from a physical device should be done in the binding, IMHO)

For DSL rules it seems like it might be worse when source time-zone is currently assumed.

Do you mean for backward compatibility? I think the source time zone is quite irrelevant, once converted to Instant. Because from this point on you can convert the Instant to any time zone you want. Or do I miss something?

For backwards compatibility we could still use ZoneId.systemDefault() for getZonedDateTime().

I think this would be a good compromise. It doesn't feel good to introduce a dependency to TimeZoneProvider in DateTimeType for such a tiny use case. If you have a cloud installation, you could do ZonedDateTime.of(dateTimeType.getInstant(), zoneId) to create a ZonedDateTime in a different time zone than the system time zone (you'd probably want to use LocalDateTime, then, but ZonedDateTime is heavily used in rules, biased from the current DateTimeType implementation)

There's another use case having TimeZoneProvider in DateTimeType: a binding gets the local time from a physical device without any time zone information (I've seen this in many bindings). To create an Instant, you need the local time zone, which should be acquired via TimeZoneProvider. So, a convencience method (or constructor) of DateTimeType like ofLocalDateTime(LocalDateTime local) could make sense. Otherwise the binding developer needs to know there's a TimeZoneProvider and write the code for injecting it. I'm not sure if there's an elegant way to keep DateTimeType clean from a dependency to TimeZoneProvider and have a convenient interface for bindings.

EDIT: A compromise would be to have a "convenience" constructor like DateTimeType(TimeZoneProvider p, LocalDatetime l) to internally create an Instant. The binding developer still needs to take care to inject TimeZoneProvider, but is guided into the right direction due to the signature of the constructor.

[jlaur]

For DSL rules it seems like it might be worse when source time-zone is currently assumed.

Do you mean for backward compatibility? I think the source time zone is quite irrelevant, once converted to Instant. Because from this point on you can convert the Instant to any time zone you want. Or do I miss something?

Yes, I meant for backwards compatibility. Currently contributors are encouraged to use TimeZoneProvider when constructing DateTimeTypes for state updates in bindings. This is exactly the reason why this issue came to life, because it didn't make sense to me to go through all that trouble of getting the currently configured time-zone just to update a channel, when the time-zone could be changed at any moment after that. Therefore I thought it would be better to apply the time-zone on retrieval, i.e. at the moment of using/showing the DateTimeType.

For a conforming binding this means the correct time-zone can be assumed in rules. If we would strip time-zone from provided ZonedDateTimes - like suggested in order to migrate to Instant internally - getZonedDateTime() would then return a ZonedDateTime with ZoneId.systemDefault() rather than the previously source time-zone from TimeZoneProvider. So it would be a little worse, at least for cloud installations having a different configured time-zone than the system time-zone. Such rules would need adaptation.

For backwards compatibility we could still use ZoneId.systemDefault() for getZonedDateTime().

I think this would be a good compromise. It doesn't feel good to introduce a dependency to TimeZoneProvider in DateTimeType for such a tiny use case. If you have a cloud installation, you could do ZonedDateTime.of(dateTimeType.getInstant(), zoneId) to create a ZonedDateTime in a different time zone than the system time zone (you'd probably want to use LocalDateTime, then, but ZonedDateTime is heavily used in rules, biased from the current DateTimeType implementation)

Agreed.

There's another use case having TimeZoneProvider in DateTimeType: a binding gets the local time from a physical device without any time zone information (I've seen this in many bindings). To create an Instant, you need the local time zone, which should be acquired via TimeZoneProvider. So, a convencience method (or constructor) of DateTimeType like ofLocalDateTime(LocalDateTime local) could make sense. Otherwise the binding developer needs to know there's a TimeZoneProvider and write the code for injecting it. I'm not sure if there's an elegant way to keep DateTimeType clean from a dependency to TimeZoneProvider and have a convenient interface for bindings.

EDIT: A compromise would be to have a "convenience" constructor like DateTimeType(TimeZoneProvider p, LocalDatetime l) to internally create an Instant. The binding developer still needs to take care to inject TimeZoneProvider, but is guided into the right direction due to the signature of the constructor.

I agree with this compromise as otherwise the type would still be entangled with the provider, leading us back to the injection problem.

[openhab-bot]

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/dishwasher-price-calculation-automation/139207/10