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

[robonect]: Reflect OH2 timezone offset to correctly display DateTimes #7848

Closed
me2000 opened this issue Jun 3, 2020 · 7 comments · Fixed by #7903
Closed

[robonect]: Reflect OH2 timezone offset to correctly display DateTimes #7848

me2000 opened this issue Jun 3, 2020 · 7 comments · Fixed by #7903
Labels
enhancement An enhancement or new feature for an existing add-on

Comments

@me2000
Copy link

me2000 commented Jun 3, 2020

  • Version used: OH2 2.5.5
  • Robonect firmware: V1.1b

Displayed times for example in Habpanel that are being set by the Robonect binding are off by the difference between the local timezone and UTC. From looking at the JSON returned by the module it looks to me that the robonect module is reporting all times and dates in the local timezone (in my case UTC+02:00) but without timezone info. The binding creates a Unix timestamp based DateTime object with the timezone UTC. When this object is now displayed in OH2 the timezone offset is applied.

Example:
The item linked to the channel last-error-date is set by the binding to "2020-06-02T13:17:34.000+0000", a UTC timestamp. When displaying this the time is 15:17 instead of 13:17. The unix timestamp (1591103854) fits the time in UTC, not the local timezone.

A fix could be that when the Thing mower is defined the binding stores the timezone offset between the local OH2 Regional settings and UTC and modifies all times reported from the Robonect module into the correct time so that all displays are correct? My Java days are long gone but I believe this could be done in RobonectHandler.java in convertUnixToDateTimeType(String unixTimeSec).

@me2000 me2000 added the enhancement An enhancement or new feature for an existing add-on label Jun 3, 2020
t2000 added a commit to t2000/openhab2-addons that referenced this issue Jun 12, 2020
Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
@t2000
Copy link
Contributor

t2000 commented Jun 12, 2020

Hey,

since I have the robonect circuit board lieing around here, ready to be installed once my mower arrives, I thought I should fix this right away :)

I took the freedom to use the timezone that the user can specify in the openHAB settings. So whatever the user specifies there will be picked up by robonect with my changes.

I have provided the jar file for testing, see the linked PR.

Regards,

Stefan

@me2000
Copy link
Author

me2000 commented Jun 12, 2020

Hi Stefan!

I have provided the jar file for testing, see the linked PR.

Good idea with taking the time zone info from the regional settings in openHAB, although I failed to understand how you accessed that info (specifically where is the actual time zone read and set in which class?).

Unfortunately your solution is not there yet so I would not approve it. Let me walk you through this (tested on a fresh installation of openHab 2.5.5 with your jar). I used the last error date but this should apply to all DateTimes.

For my mower the last error occurred at June 5th 12:32:53 local time. The mower reports a unix time stamp of 1591360373 which translates to 12:32:53 GMT. Your solution changes the date in the binding to 14:32:53.000+0200. The time zone is correct but the actual time was converted so there is some magic missing. You have to adjust the time when setting the time zone by finding out the current time zone offset and subtracting it from the reported mower timestamp.

I wrote a rule for openHAB to correct the time and posted it in the Robonect forum. It should be also working in winter time but that is untested. The code is this, maybe it helps:

import java.time.Instant
import java.time.ZonedDateTime
import java.time.ZoneId

rule "Mower last error date timezone adjustment"
when
	mowerLastErrorDateTimeRaw changed
then
	val long epochSecs = (mowerLastErrorDateTimeRaw.state as DateTimeType).zonedDateTime.toEpochSecond
	val long adjEpochSecs = epochSecs - ZonedDateTime.now.getOffset.getTotalSeconds
	val Instant instant = Instant.ofEpochMilli(Long.valueOf(adjEpochSecs) * 1000)
	val ZonedDateTime zdt = instant.atZone(ZoneId.of("Europe/Berlin"))
	mowerLastErrorDateTimeAdj.sendCommand(new DateTimeType(zdt))
end

Regards, Steffen

@t2000
Copy link
Contributor

t2000 commented Jun 12, 2020

@me2000 Thank you for your clarification. Sorry, I misunderstood your first posting.

I have now adjusted my PR so the Thing has a new configuration parameter called timezone.

I wanted to have a nice dropdown in paperUI for all timezones, but I figured out that I would have needed to copy&paste everything from the I18nConfigOptionsProvider, so I decided to stick with a string for now.

@me2000
Copy link
Author

me2000 commented Jun 12, 2020

Yep, now it is working, setting the last error date item to the correct time and also showing the correct time on the panel. Excellent!

I would suggest one change though. If there is no timezone set in the Thing definition the binding should not use hard coded "Europe/Berlin" but as in the first commit the configured time zone from the openHAB regional settings. So I suggest the order of preference: Thing configuration -> openHAB regional settings -> hardcoded, either GMT oder Berlin (as I would assume that most Robonect users are German/Austrian/Swiss anyways)

@t2000
Copy link
Contributor

t2000 commented Jun 12, 2020

Thanks for testing.

Good suggestion with using the default openHAB timezone.

However, I cannot do the fallback you suggested:
1.Thing configuration -> openHAB regional settings
2. openHAB regional settings -> hardcoded

I have implemented 1. because the "openHAB regional settings" will ALWAYS return a ZoneId and there is no way to figure out of the user is just using the default system locale or whether he doesn't have configured one.

So part 2., the hardcoded value is just to make the testing code happy for now :)

Feel free to review the code yourself and leave comments on the PR if you like.

@me2000
Copy link
Author

me2000 commented Jun 12, 2020

Understood. This is the best solution, and I even find some of my rule code in it. From what I can remember from my Java time the code looks good so I hope this pull request will be merged.

@t2000
Copy link
Contributor

t2000 commented Jun 12, 2020

Indeed, since i couldn't test it with a device (only with string values provided by you), I wrote the code similar to your rule :)

cpmeister pushed a commit that referenced this issue Jun 17, 2020
* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes #7848

Signed-off-by: Stefan Triller <[email protected]>
knikhilwiz pushed a commit to knikhilwiz/openhab2-addons that referenced this issue Jul 12, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Jul 26, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
Signed-off-by: CSchlipp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this issue Aug 31, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this issue Aug 31, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this issue Aug 31, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this issue Aug 31, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this issue Sep 1, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this issue Sep 19, 2020
…7903)

* [Robonect] Use global openHAB timezone for DateTime objects

Use the timezone that has been configured by the user in openHAB settings
for all robonect messages instead of UTC.

Fixes openhab#7848

Signed-off-by: Stefan Triller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants