konnected-io / konnected-security

Konnected connects wired sensors and switches to SmartThings, Home Assistant, Hubitat and OpenHAB
https://konnected.io
Apache License 2.0
416 stars 322 forks source link

Enhance time server settings #171

Closed heythisisnate closed 1 year ago

heythisisnate commented 1 year ago

After connecting to WiFi, the device attempts to sync the current date & time from a NTP sever. Currently, the list of NTP servers / hostnames are hardcoded: {gw, 'time.google.com', 'pool.ntp.org'} where gw is the gateway's (router's) IP address. In some rare cases, we have encountered issues where customer routers are not responding correctly to the NTP sync request, and causing time sync to fail.

An enhancement to prevent this issue is to remove gw from the list of default time servers, to sync time from a known valid NTP host by default. Also check for a time_server setting in the device settings to override the hardcoded host list. In the case that the end-user wants to use an alternate NTP host or a locally-hosted NTP server, he can set "time_server": "192.168.1.100" in the settings payload.

Similar feature should also be ported to the Alarm Panel Pro firmware.

h2zero commented 1 year ago

we have encountered issues where customer routers are not responding correctly to the NTP sync request, and causing time sync to fail.

This seems strange to me, if the time sync fails shouldn't it try the other options provided? That's my understanding of the operation, I'm just curious if there is more to this issue.

heythisisnate commented 1 year ago

It should, yes, but in a few isolated cases it isn't. I have not been able to reproduce it. We have a couple customers who have had this problem where the time sync fails on the device, but there seems to be no network issue blocking a sync from time.google.com.

My suspicion is that the fallback time server works properly if the local gateway doesn't respond to the NTP protocol .. it moves on to the next server. But maybe in the case the local gateway responds but times out, or responds with an invalid response, it trips up the fallback mechanism.

My idea was to allow this setting to bypass the local gateway, going directly to time.google.com in these cases. It would be kinda an experimental fix, but I don't know how else to get to the bottom of this issue which is affecting only a few ppl.

h2zero commented 1 year ago

I've created a branch with this addition: https://github.com/konnected-io/konnected-security/tree/custom-time-server

It will use the gw as the first server if there is no time_server setting found as having a nil value there will cause a crash.

heythisisnate commented 1 year ago

This was fixed in #174