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

support qzss time scale #228

Merged
merged 5 commits into from
May 23, 2023
Merged

Conversation

gwbres
Copy link
Collaborator

@gwbres gwbres commented May 20, 2023

Introduces QZSS time scale, which is straight forward in the sense it behaves like GPST strictly, but comes from a set of dedicated clocks.

Partial answer to issue #210

Signed-off-by: Guillaume W. Bres <[email protected]>
@codecov
Copy link

codecov bot commented May 20, 2023

Codecov Report

Patch coverage: 90.47% and project coverage change: +0.63 🎉

Comparison is base (692b222) 80.46% compared to head (4d0f6f0) 81.09%.

Additional details and impacted files
@@              Coverage Diff              @@
##           4.0.0-dev     #228      +/-   ##
=============================================
+ Coverage      80.46%   81.09%   +0.63%     
=============================================
  Files             16       16              
  Lines           3731     3783      +52     
=============================================
+ Hits            3002     3068      +66     
+ Misses           729      715      -14     
Impacted Files Coverage Δ
src/asn1der.rs 84.04% <75.00%> (+0.34%) ⬆️
src/timescale.rs 97.43% <90.00%> (+2.94%) ⬆️
src/epoch.rs 89.05% <91.83%> (+1.06%) ⬆️

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

Signed-off-by: Guillaume W. Bres <[email protected]>
Signed-off-by: Guillaume W. Bres <[email protected]>
@ChristopherRabotin ChristopherRabotin changed the base branch from master to 4.0.0-dev May 20, 2023 20:53
@ChristopherRabotin
Copy link
Member

Woohoo ! Great, thanks, I plan to pick up hifitime this week so I'll have a look soon. I changed the merge branch to 4.0.0-dev which will be the development branch for version 4.0.0. There are about 400 downloads of hifitime per day now, so I don't want to introduce breaking changes on the version 3.x releases.

@gwbres
Copy link
Collaborator Author

gwbres commented May 20, 2023

Hello Chris,
it picked up a few lines that are not covered by a test case, i'll submit that tomorrow i'm heading to bed right now :)
see you

@gwbres gwbres force-pushed the qzsst branch 2 times, most recently from e85c7b9 to 15b3849 Compare May 21, 2023 10:02
@gwbres
Copy link
Collaborator Author

gwbres commented May 21, 2023

@ChristopherRabotin ,

I forgot to increment a loop in a testbench somewhere.
These loops iterate from 0.. to N , where N is the highest integer number representing the "latest GNSS" system known by this library.
This is easy to forget since we don't have a macro to figure this number out easily (previously 7, now incremented to 8).

But that triggers src/asn1der::test_encdec correctly now which says a QZSS timescale was not preserved through the test.
Obviously, the problem comes from what happens in Epoch::from_der[u8 buffer] but I have no idea what function this inherited feature relies on.
Currently, all Integer or Str interpretation are properly coded IMO.

[Edit]
Well, the answer was kind of simple.
I had matched GPST and QZSST in all duration calculations (to and from), since they're identical.
But in some of these operartions we fix a timescale, and it has to be "independent" now.
So it's easier to provide dedicated methods like any other timescale, but unfortunately the code quantity keeps growing.
In the end, it's a whole new set of to_, from_ and python constructors

@gwbres gwbres force-pushed the qzsst branch 5 times, most recently from d0bb02c to f97f2bc Compare May 21, 2023 11:45
Signed-off-by: Guillaume W. Bres <[email protected]>
@@ -64,6 +64,7 @@ pub const UNIX_REF_EPOCH: Epoch = Epoch::from_tai_duration(Duration {
});

/// Enum of the different time systems available
#[non_exhaustive]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants