-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
timeofweek: introduce time of week construction and conversion #180
Conversation
Hello @ChristopherRabotin, I'd like to introduce these two methods to deal with GNSS receivers easily. The If you look at the test method, we expect |
ee8c197
to
0e79e1a
Compare
* (wk, tow) is usually how an epoch is expressed by a GNSS receiver, usually in a GNSS timescale. Wk is a rolling counter, and tow is the amount of seconds since Sunday midnight of that week. `tow` is usually expressed in [s] or [ms] by receivers, we use [ns] here, to maintain crate consistency and never be limited. Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Sure, that looks like it could be helpful, so thanks for starting this work. |
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Hello Chris, I improved the PR description and tried to improve the autodocs.
For example, GNSS vehicles describe an epoch in their timescale, with a rolling week counter ("wk" - maybe we should also improve some variables name), and "tow" is the offset between the current instant and the closest sunday midnight. I managed to create the constructor, currently named For the reverse operation See this new method pub fn Epoch::weekday_tai(&self) -> u8 {} which gives an unsigned integer number wrapping at 6, which will help describe a weekday, starting on 0: Monday Then I introduced pub fn closest_utc_start_of_week(&self) -> Self which for Self, returns the closest Sunday Midnight UTC epoch. This is the main information I need to evaluate what I notated in pub fn Epoch::to_timeofweek_utc(&self) -> (u32, u64) the rolling week counter is easily determined. |
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Understood, thanks Guillaume! I'll start the review soon and hope to be able to help on the bug this weekend. This looks very helpful even to other issues like #183 . |
Yep! indeed. I decided not to introduce a new Unit or timeserie, to keep things simple. But a Week granularity would help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments for when I can contribute back to this branch. This is great works, thanks Guillaume!
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
probably because leap seconds are accounted not only once but twice, due to a timescale mix up somewhere |
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
This is good! Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Okay, I've started the review, and I'll push to your branch in a bit. This is quite good work, thanks! |
Also add `with_hms` and `with_hms_strict` to set the time of a given Epoch Closes nyx-space#183 Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Needs more testing for sure Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Okay, I fixed the time of week calculation. I think it needs more testing, most notably reciprocity testing. Otherwise, this looks good! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #180 +/- ##
==========================================
+ Coverage 79.32% 80.63% +1.30%
==========================================
Files 9 10 +1
Lines 2588 2794 +206
==========================================
+ Hits 2053 2253 +200
- Misses 535 541 +6 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
I don't think Edit: Actually, I guess you made it work when it's in the same time scale. I've added a test that does not work, but maybe it's because the test I added is incorrect. |
Found bug in time_of_week initialization? Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
This is the first step in nyx-space#182 Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
Hello Chris, To determine the position of a satellite vehicle in the sky, you need to solve Kepler's equations. |
I cannot seem to figure out where this time of week bug with UTC time scales comes from. I'm pretty sure that something is wrong in the calculation of |
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
I ended up implementing a hot fix. Sorry for the delay in getting this merged. |
wk
is a rolling counter, counting the elapsed weeks since the first epoch of that time scale.tow
is the amount of seconds since Sunday midnight of that week.tow
is usually expressed in [s] or [ms] by receivers, we use [ns] in our case, to maintain crate consistency and never be limited.Signed-off-by: Guillaume W. Bres guillaume.bressaix@gmail.com