Use Instant in HistoricItem.getTimestamp instead of ZonedDateTime?

Development
1

Hi,
i would like to change the return type of the getTimestamp method in the HistoricItem. I think it is better to use a Instant here, as the persistence implementations usually work without timezone and use the system default timezone. At the end this is serialized to a unix timestamp before sending to the client, dropping the timezome again. So alot of not needed conversion happens here. Computations with this LocalDateTime is complex to, e.g. adding some seconds (like the persistence does alot) does check the transition of DST is needed every time.

Do you think i should wait for this PR for openhab 5, as this is a change in the API or is it okay to create a PR for 4.4? Another PR will adopt this change in the addons and should be merged at the same time.

Or should i create a new method to make this a none breaking change? The default implementation of this would call getTimestamp and convert it to a Instant. The getTimestamp could be deprecated and implementations could be changed one by one. I am missing a good name for this new method returning the Instant.

2

getInstant

3

See these relevant issues and PRs:

If it’s an API or user facing breaking change it’s supposed to wait for OH 5. However, some exceptions are made periodically, most recently with all the changes to persistence to hamonize with the fact that persistence can now store future values too. :person_shrugging: I’d ask in a in issue and see what the maintainers say.

I’m always in favor of this approach over a breaking change when it’s possible. I agree with @JimT , I’d expect it to be names getInstant.