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

SNTP wire protocol and client #342

Closed
wants to merge 1 commit into from
Closed

SNTP wire protocol and client #342

wants to merge 1 commit into from

Conversation

plorefice
Copy link

Hi, this is an attempt at implementing a Simple Network Time Protocol (SNTP) over UDP sockets.

The client is pretty barebones: only unicast mode with a single server is supported, more advanced stuff like root delay and dispersion are skipped, and no adjustments are performed on the received timestamp.

Even so, I've tested it and it works pretty well, with an accuracy at least in the sub-second range. For timekeeping in embedded applications, which I plan to use it on, should be plenty good. However, let me know if more features need to be added to make it worth merging.

I've also taken the liberty to create an additional apps module for application-layer protocols, in case anyone decides to implement something else like TFTP or similar, to keep the crate more tidy. I'm not married to the idea though.

@whitequark
Copy link
Contributor

Should SNTP be a part of smoltcp? I feel like it would make more sense to publish application-layer stuff as separate crates, so that they wouldn't have to evolve in lockstep with the base stack, nor would they be tied to its release schedule.

@plorefice
Copy link
Author

The way I see it, if this were published as a separate crate, it would still be strongly tied to this particular stack, increasing the risk of it becoming abandonware or at least obsolete.

By including it here, one is "forced" to maintain it, in some way. I also don't see the high-level API that application layer protocols use changing very often, so I don't think it would be much of a burden.

Finally, I agree with you that most application protocols are too complex to live here, but there are a number of "building blocks" that are nice to have out of the box.

This being said, I have no issue whatsoever in moving this code to a dedicated crate, it would be very easy to do, as it doesn't rely on many (if any) internals. I leave the final decision to you.

@whitequark
Copy link
Contributor

By including it here, one is "forced" to maintain it, in some way.

I think I would not like to be "forced" to maintain code by external contributors.

Sorry, something went wrong.

@plorefice
Copy link
Author

I see your point. In that case, I'll close this PR and move the code to an external crate.

Thanks for your time!

Sorry, something went wrong.

@plorefice plorefice closed this May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants