added: settings for customizing time zone and DST #191

AlexxBoo · 2020-10-06T13:30:55Z

I went ahead and added all Time Zones for selection in a dropdown list, as well as a setting for daylight saving.

jasoncoon · 2020-10-09T12:58:05Z

Awesome! I've been debating doing that, with a new field type and just putting the tz list in the web app. But this works as well, thank you!

AlexxBoo · 2020-10-09T14:26:34Z

Yeah, I did not think of this solution. 🤔 Having the list in the web-app would probably relieve communication to the backend, and building and maintaining the list would not use up program memory and processing resources. Hence that would arguably be the proper way I guess. But on the other hand then the web-app would have to deal with validation and value ranges. There would be no way for the backend to figure if a provided value would be valid, right?

henrygab

Timezones are terribly complex.

I strongly recommend instead storing the following:

int32_t corresponding to the UTC offset in seconds
an enumeration value corresponding to a list of immutable DST algorithms (new algorithms == new index)
int32_t indicating the offset to apply at "start" and to remove at "end" ... because we like countries in the southern hemisphere.

Value	Enum	Start	End
`0`	None	N/A	N/A
`1`	US_2021	2AM the second Sunday in March	2AM the first Sunday in November
`2`	Old_CA_Labrador	12:01AM the second Sunday in March	12:01AM the first Sunday in November
`3`	MX_2021	2AM on first Sunday in April	2AM last Sunday in October
`4`	Old_Cuba	Midnight (CUST) the second Sunday in March	1AM (CUDT) the first Sunday in November
`5`	EU_2021	1AM *GMT* last Sunday in March	1AM *GMT* last Sunday in October

Critically, note that algorithm 5 uses GMT to define when the shift occurs, regardless of the local time zone applied.

henrygab · 2021-10-03T21:38:07Z

TimeZones.h

+  String utcOffset;
+  String region;
+} TimeZone;
+typedef TimeZone TimeZoneList[];


I recommend removing this typedef. You call it a "list", but it's actually an array, which is a different data structure....

henrygab · 2021-10-03T23:12:43Z

TimeZones.h

+} TimeZone;
+typedef TimeZone TimeZoneList[];
+
+const TimeZoneList timeZones = {


I would recommend not storing string values in this data structure.
Instead, accept any provided value (in range), and have the HTML page list them explicitly. Below, I recommend storing only the offset on the device (server).

henrygab · 2021-10-03T23:23:57Z

esp8266-fastled-webserver.ino

@@ -511,6 +514,18 @@ void setup() {
    sendInt(clockBackgroundFade);
  });

+  webServer.on("/timeZone", HTTP_POST, []() {


On the server side, I would recommend AGAINST storing the setting as an index into the array. Instead, I would recommend allowing the user to indicate any offset from UTC they want. Storing that value == stability across releases, and when geopolitical things change.

Although not required here, if wanting to display multiple options for the same time zone offset (e.g., list London separately from Dakar), can also simply store a stable differentiator (e.g., int) that the client can use to find/display the proper string.

Why do this?

Allows the time zone to be set to any value, including time zones not listed.

e.g., Newfoundland (Canada) is UTC-4:30

Allows updates due to changes in world geopolitics to be done entirely in HTML pages, not in server code

henrygab · 2021-10-03T23:27:29Z

esp8266-fastled-webserver.ino

+  webServer.on("/dst", HTTP_POST, []() {
+    String value = webServer.arg("value");
+    setDst(value.toInt());
+    sendInt(dst);
+  });


Daylight Saving Time is not a boolean True/False. Currently, those timezones that use DST all happen to offset by exactly one hour. However, when DST begins and ends is not always the same.

Recommend to change this to store the following data:

DST Start (datetime)

DST End (datetime)

DST Offset (default == 1 hour)

Want to learn more? See http://ontimezone.com/exceptions.php.

henrygab · 2021-10-03T23:32:21Z

TimeZones.h

@@ -0,0 +1,49 @@
+typedef struct {


I strongly recommend against storing the strings in the server. Rather, store only in the client (e.g., HTML webpage). In addition, I would not store the index to the array, but rather the utcOffsetInSeconds itself. More reasons given below.

For fun, see video of why timezones are hard.

henrygab · 2021-10-03T23:40:54Z

Fields.h

+String getTimeZone() {
+  return String(currentTimeZoneIndex);
+}
+
+String getTimeZones() {
+  String json = "";
+
+  for (uint8_t i = 0; i < timeZoneCount; i++) {
+    json += "\"" + timeZones[i].utcOffset + " – " + timeZones[i].region + "\"";
+    if (i < timeZoneCount - 1)
+      json += ",";
+  }
+
+  return json;
+}
+
+String setTimeZone(uint8_t value)
+{
+  if (value >= timeZoneCount)
+    value = timeZoneCount - 1;
+
+  currentTimeZoneIndex = value;
+
+  EEPROM.write(11, currentTimeZoneIndex);
+  EEPROM.commit();
+
+  timeClient.setTimeOffset(timeZones[currentTimeZoneIndex].utcOffsetInSeconds + (dst ? 3600 : 0));
+
+  broadcastInt("timeZone", currentTimeZoneIndex);
+}
+
+String getDst() {
+  return String(dst);
+}
+
+String setDst(uint8_t value)
+{
+  dst = value == 0 ? 0 : 1;
+
+  EEPROM.write(12, dst);
+  EEPROM.commit();
+
+  timeClient.setTimeOffset(timeZones[currentTimeZoneIndex].utcOffsetInSeconds + (dst ? 3600 : 0));
+
+  broadcastInt("dst", dst);
+}


These would all change, if accepting my recommendations below.

henrygab · 2021-10-03T23:52:04Z

esp8266-fastled-webserver.ino

+  webServer.on("/timeZone", HTTP_POST, []() {
+    String value = webServer.arg("value");
+    setTimeZone(value.toInt());
+    sendInt(currentTimeZoneIndex);
+  });


This should not be an index into an array, because the first change in the list (adding/removing a timezone) will either make the list not sorted, or will cause any stored values on existing devices to change timezone on upgrade. That's just a bug waiting to happen.

henrygab · 2021-10-04T00:01:19Z

esp8266-fastled-webserver.ino

+  webServer.on("/dst", HTTP_POST, []() {
+    String value = webServer.arg("value");
+    setDst(value.toInt());
+    sendInt(dst);
+  });


Daylight Saving Time is, unfortunately, not a boolean. Different areas of the world start / stop DST at different dates and/or times.

Existing rules that I am aware of include:
0. No DST -- Yay!

US_Canada_2021 -- starts at 2AM second Sunday in March, ends 2AM first Sunday in November

2010_Newfoundland -- starst at 12:01AM, ends 12:01AM (same dates as US) ... NO LONGER USED AS OF 2011

Mexico_2021 -- starts at 2AM on first Sunday in April, ends 2AM last Sunday in October

Cuba_2013 -- Starts at midnight (standard time) ... same dates as US.

EU_Britain -- Starts 1AM GMT (regardless of timezone) last Sunday in March, Ends last Sunday in October. NOTE THE USE OF GMT FOR CALCULATION OF START/STOP

See, for example, http://ontimezone.com/exceptions.php for some differences, or view the fun video on timezones linked above.

henrygab · 2021-10-04T00:05:18Z

esp8266-fastled-webserver.ino

@@ -120,6 +120,8 @@ unsigned long autoPlayTimeout = 0;

 uint8_t showClock = 0;
 uint8_t clockBackgroundFade = 240;
+uint8_t currentTimeZoneIndex = 0;


This would now be called currentTimeZoneAlgorithm, for example.

henrygab · 2021-10-04T00:13:48Z

esp8266-fastled-webserver.ino

@@ -921,6 +936,14 @@ void loadSettings()

  showClock = EEPROM.read(9);
  clockBackgroundFade = EEPROM.read(10);
+
+  currentTimeZoneIndex = EEPROM.read(11);


This would need updates if accepting the above recommendations.

In short:
Store UTC offset directly.
Store DST as an enumeration corresponding to a fixed list of algorithms.

henrygab · 2021-12-13T01:31:32Z

@AlexxBoo ... I think we should close this PR, in view of #213. Let us know if you disagree.

AlexxBoo · 2021-12-14T07:54:21Z

Hi @henrygab, thank you so much for the feedback (sorry for my late response...)
I'm totally fine with closing this PR, seeing that the initially intended functionality is properly finding its way into the software.

added: settings for customizing time zone and DST

bc20177

henrygab requested changes Oct 4, 2021

View reviewed changes

henrygab added the waiting for info label Dec 13, 2021

AlexxBoo closed this Dec 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added: settings for customizing time zone and DST #191

added: settings for customizing time zone and DST #191

AlexxBoo commented Oct 6, 2020

jasoncoon commented Oct 9, 2020

AlexxBoo commented Oct 9, 2020 •

edited

Loading

henrygab left a comment

henrygab Oct 3, 2021

henrygab Oct 3, 2021

henrygab Oct 3, 2021

henrygab Oct 3, 2021

henrygab Oct 3, 2021

henrygab Oct 3, 2021

henrygab Oct 3, 2021

henrygab Oct 4, 2021

henrygab Oct 4, 2021

henrygab Oct 4, 2021

henrygab commented Dec 13, 2021

AlexxBoo commented Dec 14, 2021

added: settings for customizing time zone and DST #191

added: settings for customizing time zone and DST #191

Conversation

AlexxBoo commented Oct 6, 2020

jasoncoon commented Oct 9, 2020

AlexxBoo commented Oct 9, 2020 • edited Loading

henrygab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henrygab commented Dec 13, 2021

AlexxBoo commented Dec 14, 2021

AlexxBoo commented Oct 9, 2020 •

edited

Loading