Skip to content

Restart SNTP client when we have an IPv4 or IPv6 address#26

Merged
d-a-v merged 1 commit into
d-a-v:masterfrom
davefiddes:sntp-restart-fix
Jan 27, 2019
Merged

Restart SNTP client when we have an IPv4 or IPv6 address#26
d-a-v merged 1 commit into
d-a-v:masterfrom
davefiddes:sntp-restart-fix

Conversation

@davefiddes
Copy link
Copy Markdown
Contributor

This fixes the restarting of the SNTP client when the network
interface has a valid IP address. This was broken in changeset
9246ef for IPv4.

Note that for IPv6 the SNTP server can take up to 15 seconds to
be used due to the async nature of stateless DHCPv6 and SLAAC.
For IPv4 it takes effect immediately as the address and SNTP
server come in the same packet.

Tests:

  • Verify operation on dual-stack IPv4/IPv6:
    • DHCPv6 only network (no DHCPv4 server present)
    • DHCPv4 and DHCPv6 network
  • Verify operation on IPv4 only stack
  • Check SNTP restart with LwIP logging enabled
  • Verify time is requested and received via Wireshark
  • Verify system time is set

Signed-off-by: David J. Fiddes D.J@fiddes.net

This fixes the restarting of the SNTP client when the network
interface has a valid IP address. This was broken in changeset
9246ef for IPv4.

Note that for IPv6 the SNTP server can take up to 15 seconds to
be used due to the async nature of stateless DHCPv6 and SLAAC.
For IPv4 it takes effect immediately as the address and SNTP
server come in the same packet.

Tests:
 - Verify operation on dual-stack IPv4/IPv6:
   - DHCPv6 only network (no DHCPv4 server present)
   - DHCPv4 and DHCPv6 network
 - Verify operation on IPv4 only stack
 - Check SNTP restart with LwIP logging enabled
 - Verify time is requested and received via Wireshark
 - Verify system time is set

Signed-off-by: David J. Fiddes <D.J@fiddes.net>
@davefiddes
Copy link
Copy Markdown
Contributor Author

Sorry ti took a while to get this fix together. I spent too long chasing a nasty looking stack corruption because my lwip2-src was based off the tip rather than the last stable release as you use. Not quite sure why that would make a difference. I'll try and investigate if I get time.

@davefiddes
Copy link
Copy Markdown
Contributor Author

SNTP app patch submitted to the LwIP project for review: https://savannah.nongnu.org/patch/index.php?9749

@d-a-v d-a-v merged commit d2f388b into d-a-v:master Jan 27, 2019
@d-a-v
Copy link
Copy Markdown
Owner

d-a-v commented Jan 27, 2019

Thanks ! I will commit this to Arduino asap

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.

2 participants