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

Extension to lwip sntp to provide a weak callback notifying us the ti… (IDFGH-991) #1668

Closed
wants to merge 1 commit into from

Conversation

@markwj
Copy link

@markwj markwj commented Mar 1, 2018

Regarding:

https://www.esp32.com/viewtopic.php?f=13&t=4833
and
https://www.esp32.com/viewtopic.php?f=2&t=4695

A few users (vonnieda, loboris, and myself) have asked for a way of overriding, or getting notified of, the SNTP time being set. The use case here is to be able to delay the main application code until we have an accurate clock. The application flow then becomes:

1] Wait for network to come up

  • sntp_setoperatingmode(SNTP_OPMODE_POLL);
  • sntp_setservername(0, (char*)"pool.ntp.org");
  • sntp_init();

2] Wait for SNTP time to be set

  • Continue with application code

Following forum discussions with ESP_Sprite, and ESP_Angus, this patch implements a mechanism for [2], using weak linked overridable functions.

User code can then override
void sntp_setsystemtime_us(u32_t t, u32_t us)
to be notified of, and set, system time.

For compatibility and completeness, we have also provided a void sntp_setsystemtime(u32_t t), although this does not seem to be required for the standard ESP32 environment.

…me has been set by sntp
@CLAassistant
Copy link

@CLAassistant CLAassistant commented Mar 1, 2018

CLA assistant check
All committers have signed the CLA.

@igrr
Copy link
Member

@igrr igrr commented Mar 1, 2018

Hi @markwj, at the moment we are working on a similar feature that allows installing custom callback for the SNTP update function, at run time.
Would run-time registration satisfy your requirements?

@projectgus
Copy link
Member

@projectgus projectgus commented Mar 1, 2018

@igrr Oops, that might be on me - I'd recommended @markwj use weak linking.

@markwj
Copy link
Author

@markwj markwj commented Mar 8, 2018

Runtime registration of custom callback would also be fine. That was my original approach, but @projectgus suggested weak linking may be more appropriate. Both work just fine for me. So long as I can (a) get notified when the time is set, and (b) be allowed to set the time myself.

@igrr
Copy link
Member

@igrr igrr commented Mar 8, 2018

Ok, sorry for the extra work you had to do @markwj. The change to LwIP SNTP module which allows runtime registration of an update hook (and also smooth time adjustment via adjtime) is in review at the moment.

@mwick83
Copy link

@mwick83 mwick83 commented Mar 10, 2018

I'm glad to see, I'm not the only one needing this feature. I was thinking of implementing something similar myself. @igrr could you provide a link to the runtime registration review/pull request?

@igrr
Copy link
Member

@igrr igrr commented Mar 10, 2018

@mwick83 that would be on our internal code review system, sorry.

@mwick83
Copy link

@mwick83 mwick83 commented Mar 10, 2018

@igrr Oh, I see. Do you have an ETA when we can expect to see it on master?

@projectgus
Copy link
Member

@projectgus projectgus commented Apr 23, 2018

Hi @mwick83 ,

Sorry noone got back to you for so long. The change to add a runtime hook to LWIP SNTP has been held back behind a major update of LWIP to a newer upstream version. Once that merges then we plan to apply the patch to add this hook (which we then plan to send upstream as well).

Because the LWIP update is fairly major it's taken longer than expected to review and update. Sorry for the long delay.

Angus

@X-Ryl669
Copy link
Contributor

@X-Ryl669 X-Ryl669 commented Nov 19, 2018

What about a callback in esp/settimeofday function, that way, it does not depends on LWIP version at all ?
You set a pointer to the callback, and it's checked in settimeofday and called if not null. That's enough for what you need, IMHO:

static void (*esp_time_updated_cb)(void *) = 0; static void * esp_time_updated_cb_arg = 0;
void set_time_updated_cb(void(*callback)(void*), void * arg) 
{
    esp_time_updated_cb = callback;
    esp_time_updated_cb_arg = arg;
}

int settimeofday(const struct timeval *tv, const struct timezone *tz)
{
    (void) tz;
#if defined( WITH_FRC ) || defined( WITH_RTC )
    if (tv) {
        adjtime_corr_stop();
        uint64_t now = ((uint64_t) tv->tv_sec) * 1000000LL + tv->tv_usec;
        uint64_t since_boot = get_time_since_boot();
        set_boot_time(now - since_boot);
        if (esp_time_updated_cb) esp_time_updated_cb(esp_time_updated_cb_arg);
    }
    return 0;
#else
    errno = ENOSYS;
    return -1;
#endif
}

@olegantonyan
Copy link
Contributor

@olegantonyan olegantonyan commented Dec 6, 2018

Any news here?

@igrr igrr closed this in 7e5be1b Apr 17, 2019
@pctj101
Copy link

@pctj101 pctj101 commented Apr 17, 2019

nice! 7e5be1b

@github-actions github-actions bot changed the title Extension to lwip sntp to provide a weak callback notifying us the ti… Extension to lwip sntp to provide a weak callback notifying us the ti… (IDFGH-991) Apr 17, 2019
igrr pushed a commit that referenced this pull request Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants