search

xoseperez / espurna

Home automation firmware for ESP8266-based devices
http://tinkerman.cat
GNU General Public License v3.0
3k stars 636 forks source link

NTP - UI should warn when the timezone in incorrect #2627

Open brunetton opened 3 weeks ago

brunetton commented 3 weeks ago

Device

itead-sonoff-dual-r2

Version

1.18.0-gita518080a+github240830

Bug description

Today it's the day when I have to review my automatic door schedules. But here in France it's winter time (CET) and for now I was using CEST timezone for summer time. So it's time to change timezone.

If I understand well, it's possible to define a CET timezone so during winter local time will be CET, and during summer local time will be CEST.

Last time I was struggling with timezone (#2608) I was able to see the local time in logs (that was my lifesaver as I was able to check it the timezone is parsed and local time is correct). But I can't see it anymore in v1.18 so I'm "in the dark":

When I intentionally put a wrong timezone (random chars) and save the changes, the only feedback from UI is "Changes saved". As in #2626 I've no way to know if my input is correct or not.

It would be really useful to let user be aware of errors on save. At least instead of the "Changes saved" alert, it would be great to have "Incorrect timezone". Then user can search and find the error.

I didn't jump onto the code but if I could at least see something in logs it would be more than useful !

Thanks !

Steps to reproduce

No response

Build tools used

No response

Any relevant log output (when available)

No response

Decoded stack trace (when available)

No response

brunetton commented 3 weeks ago

Update: I found the DATE command in terminal that gives me the local time.

It allowed me to test and realize that, contrary to what I understood reading the doc:

Anyway, I still think user should be warned when an incorrect timezone is provided.

Would you be opened to a PR for documentation ? Or is Europe/Paris supposed to work ?

Thank you

mcspr commented 3 weeks ago

Europe/Paris

Not using zoneinfo names (i.e. TZ=:Europe/Paris, as it would be used on linux)

TZ_Europe_Paris

This is intended to be used when building from source, either in code/espurna/config/general.h or via custom.h provided by the user (provided by the SDK header)

CET-1CEST,M3.5.0,M10.5.0/3 (I don't think so)

This is a valid TZ variable with the DST info, generated by this script by looking at '/usr/share/zoneinfo'-like directory structure and extracting the very last line from every file there https://github.com/esp8266/Arduino/blob/master/tools/format_tzdata.py

This is a trade-off between dragging every TZ file into the firmware, or including a parser on top of the existing one that would be used to parse that zoneinfo blob (e.g. uploaded by the user), or forcing some kind of ui to include DST / SDT manually

Last time I was struggling with timezone (https://github.com/xoseperez/espurna/issues/2608) I was able to see the local time in logs (that was my lifesaver as I was able to check it the timezone is parsed and local time is correct). But I can't see it anymore in v1.18 so I'm "in the dark":

Try typing date or ntp plus enter, so there is no need to wait for the ntp sync log dump (serial, telnet or webui debug panel)

mcspr commented 3 weeks ago

Quoting https://github.com/xoseperez/espurna/issues/2608#issuecomment-2187335260

ntpTZ is not really a TZ name, but a specification string for either plain offset or DST settings

s/ntpTZ/ntpTZ value/

Anyway, I still think user should be warned when an incorrect timezone is provided.

Also returning to #2608, libc time.h is a fairly limited standard implementation. I could grab timezone offset or 4char TZ tag name, but not much else. Actual implementation struct is hidden and internal to the libc (probably every libc, for that matter). No errno after tzset() or environment updates, no return values either :shrug:

Similar to systemd-analyze calendar ... suggested in the #2626, env TZ=... date DATE STRING can be used to check current or specific date as-if TZ=... was set on the device

edit:

Would you be opened to a PR for documentation ? Or is Europe/Paris supposed to work ?

Wiki page table corrected (hopefully). #define move to build flag column, ntpTZ => Value reference to the table below