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

POSIX TZ variable for NTP differs from Wiki. #2418

Closed
davebuk opened this issue Jan 17, 2021 · 8 comments
Closed

POSIX TZ variable for NTP differs from Wiki. #2418

davebuk opened this issue Jan 17, 2021 · 8 comments

Comments

@davebuk
Copy link
Contributor

davebuk commented Jan 17, 2021

I have been updating a lot of my devices and noticed the NTP section changed a while ago for the time zone setting. A lot of my devices currently show: CET0CEST,M3.5.0,M10.5.0/3. Looking at: https://github.com/xoseperez/espurna/wiki/NTP I set: #define NTP_TIMEZONE TZ_Europe_London to one of my build settings. I thought this should change the time zone to: GMT0BST,M3.5.0/1,M10.5.0. I can put that time zone data in manually and espurna gets the correct time, but I wanted to specify it correctly in my custom.h builds

@mcspr
Copy link
Collaborator

mcspr commented Jan 18, 2021

This must be part of the conversion happening here:

void _ntpConvertLegacyOffsets() {
bool found { false };
bool europe { true };
bool dst { true };
int offset { 60 };
settings::kv_store.foreach([&](settings::kvs_type::KeyValueResult&& kv) {
const auto key = kv.key.read();
if (key == F("ntpOffset")) {
offset = kv.value.read().toInt();
found = true;
} else if (key == F("ntpDST")) {
dst = (1 == kv.value.read().toInt());
found = true;
} else if (key == F("ntpRegion")) {
europe = (0 == kv.value.read().toInt());
found = true;
}
});
if (!found) {
return;
}
// XXX: only expect offsets in hours
String custom { europe ? F("CET") : F("CST") };
custom.reserve(32);
if (offset > 0) {
custom += '-';
}
custom += abs(offset) / 60;
if (dst) {
custom += europe ? F("CEST") : F("EDT");
if (europe) {
custom += F(",M3.5.0,M10.5.0/3");
} else {
custom += F(",M3.2.0,M11.1.0");
}
}
delSetting("ntpOffset");
delSetting("ntpDST");
delSetting("ntpRegion");
setSetting("ntpTZ", custom);
}

If either of ntpOffset, ntpDST or ntpRegion are found in settings, it will construct a ntpTZ value based on those + old defaults for the NTP configuration and override the existing setting. del ntpTZ should restore the build-time default after the reboot, but I guess it should not overwrite the existing ntpTZ value (can't check for build-time though due to the way settings work with custom.h)

@davebuk
Copy link
Contributor Author

davebuk commented Jan 18, 2021

I'll try a loading a build to a factory reset device and check it loads the correct values.

@davebuk
Copy link
Contributor Author

davebuk commented Jan 20, 2021

#2414 (comment)

The NTP server has always worked and been set to the default, but after loading a firmware and defining the time zone in the custom.h, the server address has been cleared. I can manually put it back in and it saves.

@mcspr
Copy link
Collaborator

mcspr commented Jan 21, 2021

Current dev now includes these:
c179467
e8e58b8

Which should:

  • avoid removing existing ntpTZ when old cfg vars are detected. I wonder though what is better - somewhat broken conversion (note the transition hour specified after the /, when missing default is 2:00) or just remove old settings and use build default, forcing to convert manually through TZ.h
  • detect bad values from DHCP and disabling this feature via ntpDhcp setting (although, idk if it was just empty ntpServer?)
  • detect changes to both tz and server settings

@davebuk
Copy link
Contributor Author

davebuk commented Jan 21, 2021

After clearing out the Time Zone data from webUI and building my tuya dimmer device using #define NTP_TIMEZONE TZ_Europe_London after boot, NTP keys still show:

> ntpServer => "pool.ntp.org"
> ntpTZ => ""

@mcspr
Copy link
Collaborator

mcspr commented Jan 21, 2021

That's a weird one. WebUI sets the key to an empty string instead of deleting.

@mcspr
Copy link
Collaborator

mcspr commented Jan 22, 2021

...which was always the case for the web? I don't think I changed that between 1.14.0 and now (at least I hope so, can't find any delSetting in the web source code history). Technically, the expected 'erase-all' interaction is factory reset, which will erase the settings storage forcing each module to use build-time flags as defaults

@davebuk
Copy link
Contributor Author

davebuk commented Jan 23, 2021

A full factory reset has sorted it. It's cleared out all the keys and filled in the NTP settings I assumed it would correctly.

@davebuk davebuk closed this as completed Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants