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

Refine Timezones #296

Closed
proddy opened this issue Aug 2, 2019 · 11 comments
Closed

Refine Timezones #296

proddy opened this issue Aug 2, 2019 · 11 comments

Comments

@proddy
Copy link

proddy commented Aug 2, 2019

The timezone pull-down selection doesn't work for me so I started to look into the code. The timezone selected from the Web UI and stored in the SPIFFS config file is never used in the code, so device time will always be 1 hour + UTC. Is this intentional or is the implementation not finished?

@omersiar
Copy link
Collaborator

omersiar commented Aug 2, 2019

Hmm, this sounds completely not intentional, on the other hand I remember debugging a Browser issue, I might be forgot to revert code after testing.

Good finding, I will check it. Thanks

@proddy
Copy link
Author

proddy commented Aug 2, 2019

ok! thought it was me. Thanks and let me know if I can help out with testing or coding.

@proddy proddy changed the title question about timezone? timezone setting not working Aug 11, 2019
@proddy
Copy link
Author

proddy commented Aug 16, 2019

Looking at the code again the timezone is taken from the browser (in the JS) so you don't really need to explicitly set a timezone. I think the whole section in the html and config can be removed. Do you agree?

@omersiar
Copy link
Collaborator

Internally, time is being tracked as Unix Time which is then adjusted to User's timezone on Web UI. You need to set timezone in order to list entries in the log correctly.

@proddy
Copy link
Author

proddy commented Aug 18, 2019

Yes, time from NTP is in UTC and when viewed in the event log it's adjusted on the fly to the web browsers's timezone with the code:

var comp = new Date();
value = Math.floor(value + ((comp.getTimezoneOffset() * 60) * -1));

So you don't need to explicitly set a timezone. In fact it's not even used anywhere in the code so can be removed from the web interface and config file.

@omersiar
Copy link
Collaborator

You are right for the frontend, on firmware level timezone setting is used.

timezone = tz;

@omersiar
Copy link
Collaborator

#116

@proddy
Copy link
Author

proddy commented Aug 18, 2019

yes it's set in ntp.cpp but never used - that's my point. Sorry if I wasn't clear in the beginning. The time is always saved in UTC.

However if you do want to convert the UTC to the correct timezone and handle DST (daylight saving times) in the firmware, I have extended the NTP code to use this library which works. But now I'm thinking its not really necessary since the only time the Date is ever displayed is in the web interface and JavaScript can do the real-time conversions quite nicely.

@omersiar
Copy link
Collaborator

Oh, now I understand, sorry for the inconvenience. I just need to refactor these functions and libraries.

@omersiar omersiar changed the title timezone setting not working Refine Timezones Aug 26, 2019
@peteshoard
Copy link

Time also does not persist during reboot, see images
device_date

@matjack1 matjack1 added this to the V2 milestone Mar 9, 2023
@matjack1
Copy link
Collaborator

Fixed by #604

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants