Watchy icon indicating copy to clipboard operation
Watchy copied to clipboard

Set NTP Time with offset from weather API

Open therealmitchconnors opened this issue 3 years ago • 20 comments

Fixes #51.

Because ntp sync requires network connectivity, it is done only at startup time, and then whenever the weather is checked. This minimizes impact to battery life.

Geolocation is detected by ip address using ip-api.com. This may report invalid results if the connection is proxies, so we should probably add a menu option to disable this, but I'm not sure how to do that yet.

therealmitchconnors avatar Jun 25 '21 18:06 therealmitchconnors

One problem with using IP-API is that it appears to give incorrect results for mobile connections. In my case, the result is in the correct timezone, but in a city 1000 miles away...

therealmitchconnors avatar Jun 25 '21 18:06 therealmitchconnors

I like the idea of doing it at the same time as the weather… we could even use the Date from the weather server response to synchronize, using even less power.

schodet avatar Jun 25 '21 19:06 schodet

I have reduced the scope of this API to get the timezone offset from the weather API, and sync ntp every time weather is synced. I still like the idea of setting the location by the ip-api, but that should be a separate pull request.

therealmitchconnors avatar Jul 08 '21 17:07 therealmitchconnors

Just to confirm, is this a complete replacement for #70?

brianmay avatar Aug 02 '21 23:08 brianmay

Tested and works well, particularly in conjunction with #62 to set the location.

shtrom avatar Aug 03 '21 10:08 shtrom

Odd, not seeing this myself. I haven't applied #62, so would expect it to get the wrong time zone (it looks like it defaults to New York). But not even seeing a single NTP packet. Wonder if I am doing something wrong. Still investigating.

brianmay avatar Aug 03 '21 22:08 brianmay

Disregard, turns out I will still using the old version of Watchy. As per the supplied instructions. I think I worked out how to build it correctly now, and it works fine.

brianmay avatar Aug 03 '21 23:08 brianmay

Ok, after a couple of days on this, it seems that the watch quickly gets too fast (20 minutes at the moment). I tried setting the time back manually, and it promptly went back to the previous time, so I suspect there's an incorrect offset calculated and/or stored somewhere.

shtrom avatar Aug 05 '21 00:08 shtrom

@shtrom Yes, I definitely noticed it getting very fast after only several hours. There are a number of possible reasons, haven't investigated yet.

brianmay avatar Aug 05 '21 00:08 brianmay

I see that there are calls to Serial.println, but I don't see anything on the usb/serial connection.

Later: Need to use:

Serial.begin(921600, SERIAL_8N1, 3, 1); 

This seems to output a bit of garbage. Don't care.

I have kludged together something that will print the RTC at the start and end of every call to getWeatherData, hoping this might tell me something.

brianmay avatar Aug 06 '21 01:08 brianmay

My tests showed that it was tracking time reasonably well, until it polls ntp for the first time (after 30 minutes).

At this point, getLocalTime returns a value 20 seconds into the future. Some of this time is because getLocalTime takes a few seconds, but not that long. Now the clock is at least 10 seconds fast.

brianmay avatar Aug 06 '21 04:08 brianmay

Seems like every call to getLocalTime adds almost 20 seconds to the time. Here are same sample times shown.

17:05:03 6/8/2021
17:05:03 6/8/2021
Friday, August 06 2021 17:05:20

The first two times are the RTC before and after calling getLocalTime, and the next time is the new time returned. So we expected to get a time of 17:05:03, but we got a time of 17:05:20 instead.

In 30 minutes time, I expect the same thing to happen again. So the time will then be almost 40 seconds out.

So something really screwy with this getLocalTime call.

brianmay avatar Aug 06 '21 07:08 brianmay

Not sure I like this design. Probably would be better to always store UTC in the RTC clock, and translate to local time when retrieving. Maybe even using timezone rules that should be saved in the image (not sure how easy this is). That way we automatically do the right thing with daylight savings starting/ending.

But I would like to get working as is first. I can't find any documentation or code for the getLocalTime() function though - where can I get a reference for this?

I have one theory that it might be getting confused with us deep shutting and/or wifi sleeping between calls. Will try to test.

Maybe we should skip this function - it appears to be broken, and just use a 3rd party ntp library? e.g. https://github.com/taranais/NTPClient - no idea if this will work.

brianmay avatar Aug 09 '21 23:08 brianmay

I also have observed serious time drift over several days when not connected to WiFi, which should mean no NTP calls. Are we sure this issue is unique to this PR?

therealmitchconnors avatar Aug 10 '21 17:08 therealmitchconnors

@therealmitchconnors I can't comment on your issue exactly. But the more I investigate the more I think this PR is somewhat broken. As per my debugging above which shows with every getLocalTime request it gains almost 20 seconds.

The problem here with this PR is getLocalTime doesn't translate to "get ntp time". It translates to "get system time (and translate to local time)" which is configured to be synchronised with ntp. But this synchronisation is probably hampered because we keep shutting down the wifi network and also deep sleeping. So we end up just retrieving the system clock instead and using that to program the RTC. i.e. we end up programming an accurate DS3232RTC from the inaccurate software based ESP32 clock. Note: I am still conducting tests on some of the claims made here.

For more information on the system clock - see https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/system/system_time.html

I think for this application, were we have an accurate DS3232RTC we need to be making direct ntp calls ourselves, and not rely on the system clock in any way. Having two clocks competing with each other seems like a recipe for problems.

Your case is different though, because if the weather http request fails, it won't even call getLocalTime, and should rely 100% on the DS3232RTC. But to be sure, does the clock still drift without this PR? The DS3232RTC clock looked reasonable accurate to me, but admittedly I haven't run it for long.

brianmay avatar Aug 10 '21 22:08 brianmay

Just to confirm the above, at 08:24 it was due to poll the ntp time. It wakes up the network:

08:24:06.015785 IP 192.168.5.1.bootps > 192.168.5.145.bootpc: BOOTP/DHCP, Reply, length 300
08:24:06.019682 ARP, Request who-has 192.168.5.145 tell 192.168.5.145, length 46
08:24:06.358788 ARP, Request who-has 192.168.5.1 tell 192.168.5.145, length 46
08:24:06.358810 ARP, Reply 192.168.5.1 is-at 80:2a:a8:4d:af:8f (oui Unknown), length 28

Looks up the weather service:

08:24:06.361054 IP 192.168.5.145.26833 > 192.168.3.38.domain: 57352+ A? api.openweathermap.org. (40)
08:24:06.363485 IP 192.168.3.38.domain > 192.168.5.145.26833: 57352 2/4/1 A 178.128.25.248, A 178.128.122.9 (225)
08:24:06.367490 IP 192.168.5.145.62012 > 178.128.25.248.http: Flags [S], seq 2899399650, win 5744, options [mss 1436], length 0
08:24:06.457586 IP 178.128.25.248.http > 192.168.5.145.62012: Flags [S.], seq 2146925651, ack 2899399651, win 29200, options [mss 1420], length 0
08:24:06.601467 IP 192.168.5.145.62012 > 178.128.25.248.http: Flags [.], ack 1, win 5744, length 0
08:24:06.693739 IP 178.128.25.248.http > 192.168.5.145.62012: Flags [.], ack 236, win 30016, length 0

Then it did a DNS request for the ntp server:

08:24:07.221126 IP 192.168.5.145.1688 > 192.168.3.38.domain: 8480+ A? pool.ntp.org. (30)
08:24:07.547265 IP 192.168.3.38.domain > 192.168.5.145.1688: 8480 4/9/0 A 162.159.200.1, A 162.159.200.123, A 13.55.50.68, A 27.124.125.250 (244)

But it never actually contacted the NTP server.

Then it shuts down the wifi again:

08:24:11.434281 ARP, Request who-has 192.168.5.145 tell 192.168.5.1, length 28

Regardless, the getLocalTime returned a value into the future.

08:24:03 11/8/2021
08:24:03 11/8/2021
Wednesday, August 11 2021 08:24:19

Actually it is odd that the DS3232RTC said the time was 08:24:03 when calling getLocalTime. This happens after the http weather request. But the timestamps on the tcpdumps - from a trusted ntp synchronised server - say the network wasn't up yet at this time. Wonder if that is a sign that the DS3232RTC is loosing time.

brianmay avatar Aug 10 '21 22:08 brianmay

The DS3232RTC clock by itself does appear loosing time, and becoming increasingly slow :-(

Not sure if this is a hardware of software issue. My guess is might be hardware.

...said the software engineer. :-)

brianmay avatar Aug 11 '21 04:08 brianmay

I can see two ways of getting NTP working correctly.

  • Don't shutdown the network or enter deep sleep. Then I believe getLocalTime will sync correctly to NTP. If you do this you can simplify the code note to use the RTC, it isn't needed. But this will end up using more battery power. Still need to test this sufficiently, although perhaps not a good option really.
  • Use an NTP client library such as https://github.com/arduino-libraries/NTPClient which gives you precise control over when NTP requests are made. See the forceUpdate function (not tested!). This way you can make sure ntp requests are only made after the network is configured. Based on the NTP result you can set the RTC.

Bit disappointing that the watch doesn't seem to be able to keep time correctly without NTP however.

brianmay avatar Aug 15 '21 06:08 brianmay

This comment might explain the behaviour I have been seeing also: https://github.com/sqfmi/Watchy/pull/70#issuecomment-869416826

brianmay avatar Aug 15 '21 06:08 brianmay

I have test code at https://github.com/brianmay/watchy-remote/tree/test_time. My code does things a little bit better (IMHO) then some of the other code, I trust it a bit more.

This will:

  • Connect to wifi.
  • Sync ntp time, and write to ntp.
  • Disconnect wifi
  • Every 10 seconds retrieve time using getLocalTime and RTC clocks, display, and print to serial.

Assumptions:

  • Wifi must be configured already with another firmware.
  • All times displayed are UTC.
  • Deep sleep not supported (yet).
  • Build with platformio.

At the moment, the local time appears to be tracking the correct time pretty closely. But the RTC clock is seriously loosing time rapidly.

updating...
local time: Monday, August 16 2021 04:31:35
rtc time: Monday, August 16 2021 04:31:12

My feeling so far, subject to further testing:

  • rtc clock loses time, and can't be relied on.
  • esp32 system clock seems to be OK if deep sleep turned off.
  • If deep sleep mode used, the esp32 system clock will gain time.

This does seem a little bit surprising and unexpected.

brianmay avatar Aug 16 '21 04:08 brianmay

Release 1.4.3 adds feature to NTP sync during weather API call, based on the city's time zone. There has been changes to library so I can't merge this PR directly. Check out the new release and let us know if there are any issues. Thanks for the PR @therealmitchconnors!

sqfmi avatar Oct 15 '22 02:10 sqfmi