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

[Doc bug] Units of NtpResult.seconds_fraction ambiguous #21

Closed
svet-b opened this issue Apr 18, 2023 · 2 comments
Closed

[Doc bug] Units of NtpResult.seconds_fraction ambiguous #21

svet-b opened this issue Apr 18, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@svet-b
Copy link

svet-b commented Apr 18, 2023

Thanks for a really helpful library! Though I ran into an issue trying to parse NtpResult.seconds_fraction (resp. NtpResult.sec_fraction()):

  1. This states that the value is in microseconds: https://github.com/vpetrigo/sntpc/blob/master/src/lib.rs#L280
  2. This states nanoseconds: https://github.com/vpetrigo/sntpc/blob/master/src/lib.rs#L292
  3. In the example it's in units of 1/u32::MAX of a second (if my reading is right) - i.e. about a quarter of a nanosecond: https://github.com/vpetrigo/sntpc/blob/master/examples/simple_request.rs#L87

I gather that (1) is definitely not the case (since the values are too large), and I suspect that (3) is the real answer, but it would be good to have this clear in the docs. Or perhaps convert the result to micro or nanoseconds, since 1/u32::MAX is a bit awkward to deal with.

Also - and this is minor - but I think in the example using time.sec_fraction() as u64 * 1_000_000 / u32::MAX as u64 to get the decimal points will show the wrong value if the result has fewer than 6 digits. E.g. it would show 1.23 instead of 1.000023. Might be better to just convert to float, e.g.

println!("Got time: {}", time.sec() as f64 + time.sec_fraction() as f64 / u32::MAX as f64);
@svet-b svet-b added the bug Something isn't working label Apr 18, 2023
@vpetrigo
Copy link
Owner

vpetrigo commented May 8, 2023

Hello @svet-b,
Sorry for the late reply!

Thank you for pointing out. Since we have a second fractional value in the low 32 bit we can convert it to the fractional part representation by dividing it by 2^32 - single bit in the second fraction value is ~232 picosecond. E.g. example from the internet:

  • we get value 1329481807 in the second fraction value from the server which is something like that 0.30954410484991967678070068359375
  • if the seconds value is 123456497 we would get 123456497.30954410484991967678070068359375

Whatever your application requires, you are free to "strip" fraction value to milliseconds/microseconds by using your formulae:

let microseconds = time.sec_fraction() as u64 * 1_000_000 / u32::MAX as u64

For our example that would lead to something like that:

let seconds = time.sec() // 123456497
let fractions = time.sec_fraction() as f64 / u32::MAX as f64 // 0.30954410484991967678070068359375
let microsec_fractions = time.sec_fraction() as u64 * 1_000_000 / u32::MAX as u64 // 309544

println!("Got time: {}", seconds as f64 + fractions); // 123456497.30954410484991967678070068359375
println!("Got time: {}.{}", seconds, microsec_fractions); // 123456497.309544

vpetrigo added a commit that referenced this issue May 8, 2023
vpetrigo added a commit that referenced this issue May 8, 2023
vpetrigo added a commit that referenced this issue May 8, 2023
@vpetrigo
Copy link
Owner

vpetrigo commented May 9, 2023

Should be fixed in 0.3.3+. Feel free to reopen that issue if your question is still unanswered. 😄

@vpetrigo vpetrigo closed this as completed May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants