src/epoch.rs: fix gh 187

[gwbres]

• fix conversion from and to elapsed weeks

Signed-off-by: Guillaume W. Bres guillaume.bressaix@gmail.com

[gwbres]

Hello Chris,

with the following src/epoch.rs modifications, I can unlock reciprocal tests of every time_of_week existing tests.

[codecov-commenter]

Codecov Report

Base: 80.63% // Head: 80.37% // Decreases project coverage by -0.26% ⚠️

Coverage data is based on head (5ec7c80) compared to base (aa1ea21).
Patch coverage: 68.29% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
- Coverage   80.63%   80.37%   -0.27%     
==========================================
  Files          10       10              
  Lines        2794     2792       -2     
==========================================
- Hits         2253     2244       -9     
- Misses        541      548       +7     

Impacted Files	Coverage Δ
src/weekday.rs	83.54% <ø> (ø)
src/epoch.rs	87.57% <68.29%> (-0.39%)	⬇️
src/timescale.rs	90.90% <0.00%> (-2.03%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[gwbres]

when working with self.to_duration().to_unit(x) i'm always facing rounding issues right here.
So I decided to move to total_nanoseconds() ..

[ChristopherRabotin]

Thanks Guillaume ! How big of a rounding error do you see with UNIX? There is a rounding error with TDB and ET because they're necessarily initialized with an iteration, but the code accepts something like 150 nanoseconds of error as reasonable. In general, I try to avoid operations on i128 because that's a very big type and will be very slow on embedded systems (most are 32 bit, so it needs four registers per operation).

…

On Tue, Dec 6, 2022, 09:46 gwbres ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/epoch.rs <#188 (comment)>: > - let mut start_of_week = self.previous_weekday_at_midnight(Weekday::Sunday); - let ref_epoch = self.time_scale.ref_epoch(); - // restrict start of week/sunday to the start of the time scale - if start_of_week < ref_epoch { - start_of_week = ref_epoch; - } - let dw = *self - start_of_week; // difference in weekdays [0..6] - (wk as u32, dw.nanoseconds) + let total_nanoseconds = self.to_duration().total_nanoseconds(); + let weeks = + total_nanoseconds / NANOSECONDS_PER_DAY as i128 / Weekday::DAYS_PER_WEEK_I64 as i128; + // elapsed nanoseconds in current week: + // remove previously determined nb of weeks + // get residual nanoseconds + let nanoseconds = total_nanoseconds + - weeks * NANOSECONDS_PER_DAY as i128 * Weekday::DAYS_PER_WEEK_I64 as i128; when working with self.to_duration().to_unit(x) i'm always facing rounding issues right here. So I decided to move to total_nanoseconds() .. — Reply to this email directly, view it on GitHub <#188 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABEZV2BOM3NSE2J3V2SBESTWL5UVLANCNFSM6AAAAAASVXHBPY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[gwbres]

In general, I try to avoid operations on i128 because that's a very big
type and will be very slow on embedded systems (most are 32 bit, so it
needs four registers per operation).

Yeah I know, that is why I was trying something like this

2253     #[must_use]
2254     /// Converts this epoch into the time of week, represented as a rolling week counter into that time scale
2255     /// and the number of nanoseconds elapsed in current week (since closest Sunday midnight).
2256     /// This is usually how GNSS receivers describe a timestamp.
2257     pub fn to_time_of_week(&self) -> (u32, u64) {
2258         let days = self.to_duration().to_unit(Unit::Day);
2260         // nb of weeks
2261         let weeks = days / Weekday::DAYS_PER_WEEK;
2262         // nanoseconds within current week
2263         //   is total number of nanoseconds
2264         //   - number of nanoseconds contained in whole days
2264         let nanoseconds = self.to_duration().to_unit(Unit::Nanosecond) //total
2265             - weeks /7.0 /24.0 /3600.0 /1.0E9; // remove those already contained in whole "weeks"
2267         (weeks.trunc() as u32, nanoseconds as u64)
2268     }

[gwbres]

I'll remove this,
since the current proposal does not use it