Improve extras/sntp -- Issue #562

SuperHouse / esp-open-rtos

Open source FreeRTOS-based ESP8266 software framework

BSD 3-Clause "New" or "Revised" License

1.53k stars 491 forks source link

Improve extras/sntp -- Issue #562 #563

Closed jeffsf closed 6 years ago

jeffsf commented 6 years ago

Primary change is correcting the missing scaling of the microseconds calculation Resolves #562

Other changes address:

Capture RTC count before any logging
No "obligatory" printf() logging
Change logged message to show:
- adjustment about to be made in seconds
- time since last SNTP update
- ratio of the two as ppm
- "cal" value (raw as int, as before)
Initialize "cal" value from sdk_system_rtc_clock_cali_proc()

I'm more than happy to squash this, or split into mutiple commits

jeffsf commented 6 years ago

As I "peel the onion" on the SNTP implementation, it looks like there are other issues to resolve as well (Edit: which the new LWIP code addresses). ~~I'm planning on rebasing this into two commits~~

~~Resolve calculation error~~
~~Improve logging~~ ~~so that various portions could be cherry-picked, if appropriate.~~

~~Please hold off on merge until I can make those changes, as well as~~ a "simple" #define to enable printf()-based logging. #define SNTP_LOGD(FMT, ...) printf(FMT "\n", ##__VA_ARGS__) in the form of a compiler flag is rather awkward)

jeffsf commented 6 years ago

86fa1ad represents the compiler flag

I tried rebasing into a functional-correctness commit and a debug-output commit and wasn't completely successful. I could give it another try, or just go ahead and squash this entire thing if you choose to accept it.

The new LWIP drop of #554 seems to contain a near rewrite of the LWIP SNTP code that now seems to:

Consider RTT in setting time
Handle broadcast NTP
Accept NTP server from DHCP

Rather than reinvent the wheel on the obsolete LWIP core of extras/sntp, I'm going to continue development off a merge between master and #554.