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

Examples update, add a note for configTime() #5343

Merged
merged 3 commits into from
Jul 22, 2021

Conversation

vortigont
Copy link
Contributor

lwip lib bundled with esp32 Arduino supports only one ntp server. Any additional servers set are just silently ignored.
This default is different from esp8266 Arduino core and very confusing. Most of the examples provided uses 3 different ntp servers for redundancy while only the first one is used actually.
Addressing issue #4964

… is supported by lwip

Signed-off-by: Emil Muratov <gpm@hotplug.ru>
@CLAassistant
Copy link

CLAassistant commented Jul 1, 2021

CLA assistant check
All committers have signed the CLA.

vortigont added a commit to vortigont/EmbUI that referenced this pull request Jul 4, 2021
+ TimeProcessor is a singleton now
* TimeSync event now works for esp32 platform also
- Obsolete all WorldTimeApi methods, left it as a sepparate class for compatibility only

Known issues:
esp32 platform uses one ntp server only - espressif/arduino-esp32#5343

Signed-off-by: Emil Muratov <gpm@hotplug.ru>
@me-no-dev
Copy link
Member

Hi @vortigont :) Thanks for taking the time! LITTLEFS was recently renamed to LittleFS. Could you please adjust your pull request?

@vortigont
Copy link
Contributor Author

@me-no-dev merged with recent master changes. Tnx for the hint.
I was thinking that maybe lwip build defines should be changed to match esp8266? So that by default all 3 ntp servers options could be used? esp8266 Arduino core bundle contains several versions of lwip, like full-featured, small-mss, IPv6. But esp2 offers only one version, and rebuilding your own version doesn't look that easy for everyone's. So, maybe it would be better to match the defauls? I can prepare a PR for the build-scripts changes instead of this PR. How do you think?

@me-no-dev
Copy link
Member

@vortigont if this is an IDF menuconfig option, then change should be added in the lib-builder sdkconfigs, which will then propagate to arduino the next time that IDF has some updates for us :)

@vortigont
Copy link
Contributor Author

OK, then I will check for lib-builder's defines and make a PR, then you can choose between this PR or lwip fix :)
Tnx!

@vortigont
Copy link
Contributor Author

@me-no-dev looks like lib-builder won't help here since esp-idf's lwip Kconfig and lwipopts.h doesn't have proper defines to adjust SNTP_MAX_SERVERS at build time.
Would appreciate if this PR for examples could be accepted, meanwhile I'll try to prepare PR for esp-idf to implement required menuconfig option and maybe someday we could get back to lib-builder's default override for arduino-esp32
Tnx!

@me-no-dev me-no-dev merged commit 34125ce into espressif:master Jul 22, 2021
@me-no-dev
Copy link
Member

Merged @vortigont thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants