-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
time: import IANA timezone definitions, expose SNTP API #6373
Conversation
- `configTime("timezone", "ntp servers...")` added - timezone definitions by country/cities (TZ.h) - script to update timezone definitions - updated example
After this morning's example update, using scheduled functions, here's the new output with my timezone:
(watch |
@bperrybap @Tech-TX @Meinsda @whyameye @mcspr ref:#4637,#5681 SNTP API additions (in example):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in #5681, this works for me in both the proposed example and in an existing sketch.
As a sidenote, since we are changing sntp a bit.
Should startup delay also be configurable? (ref the old #5564)
And is there a reason not to do something like this, and user then defines those in a sketch when needed? https://gist.github.com/mcspr/a68cd059304c14f487a4653f7b03e091
(besides exposing another function that runs in a system context)
@mcspr Following lwIP's logic, I added a weak function (and only after I followed your link to see that it was what you did for both parameters (even the former which is proposed as a constant in lwIP)). |
Yes, sorry, I would've changed linklayer repo directly to show a proper patch. I somehow missed that arduino lwipopts.h is also there (edit: re-reading the sentence, now I understand that we thought about the same thing :)) tbh, I liked the idea to have them both weak functions for consistency sake. even if the other macro called _func, they both are used without calling() |
update example fix for lwip1.4
@mcspr, both are now weak functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. long names seem to solve the documentation problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor technical debt and default demo issues, but OTW LGTM.
New output for the example, I did my best to show that everything is handled by newlib and lwIP/sntp,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, and thanks for the LWIP build fix slipstream, too.
Good work on this! |
configTime("timezone", "ntp servers...")
addedCloses #4637
Closes #5681
Fixes #6562