Skip to content

Conversation

@vortigont
Copy link
Contributor

LWIP has two define's for setting max number of sntp servers:

  • total number of handled servers
  • max number of sntp's picked via DHCP

by default both values are equal to 1, but could be set separately.
This is a followup for espressif/arduino-esp32#5343. Having this menuconfig option allows building lwip for arduino-esp32 with multiple sntps's supported

LWIP has two definess for setting max number of sntp servers:
 - Total number of handled servers
 - max number of sntp's picked via DHCP

by default both values are equal to 1, but could be set separately

Signed-off-by: Emil Muratov <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Jul 27, 2021

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 27, 2021
@github-actions github-actions bot changed the title lwip: menuconfig option for max number of SNTP servers lwip: menuconfig option for max number of SNTP servers (IDFGH-5616) Jul 27, 2021
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a question and an optional suggestion.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jul 30, 2021
This could be toggled on/off, off is the default.
SNTP debug option. Example update for ntp via DHCP

Signed-off-by: Emil Muratov <[email protected]>
@vortigont
Copy link
Contributor Author

@david-cermak I've updated notes for menuoption, also added NTP debug option and sntp example to utilize SNTP via DHCP if build-in.
Would appreciate if you could review changes.
Thanks!

@david-cermak
Copy link
Collaborator

@vortigont Thanks for the prompt update and addressing the concern of misconfiguration.
This PR looks very good to me and has been added to the internal round of testing and reviews (this usually takes some time though till it gets merged to master).

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Aug 25, 2021
@espressif-bot espressif-bot merged commit e545237 into espressif:master Aug 29, 2021
@vortigont vortigont deleted the sntpmenuopt branch September 3, 2021 11:14
vortigont added a commit to vortigont/esp32-arduino-lib-builder that referenced this pull request Nov 2, 2021
 - increase default number of NTP servers up to 3 (match with Arduino esp8266)
 - activate SNTP over DHCP requests (match with Arduino esp8266)

addressing issue  espressif/arduino-esp32#4964
provided via  espressif/esp-idf#7336
me-no-dev pushed a commit to espressif/esp32-arduino-lib-builder that referenced this pull request Dec 20, 2021
- increase default number of NTP servers up to 3 (match with Arduino esp8266)
 - activate SNTP over DHCP requests (match with Arduino esp8266)

addressing issue  espressif/arduino-esp32#4964
provided via  espressif/esp-idf#7336
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Resolution: Done Issue is done internally Status: Done Issue is done internally

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants