Skip to content

Conversation

@georgeboot
Copy link
Contributor

@georgeboot georgeboot commented May 20, 2025

This pull requests adds the Epoch::from_ptp_seconds and Epoch::from_ptp_milliseconds helper methods to deal with IEEE 1588-2008 time being in TAI but using the unix reference epoch.

I'm not 100% happpy with the naming of the methods however. By default IEEE 1588-2008 uses TAI with unix epoch, but the standard does not enforce this. In practice, (practically) all implementations use it however.

Fixes #378

@ChristopherRabotin
Copy link
Member

Thanks for the PR! Could you also add a from_ptp_duration constructor, similar to https://docs.rs/hifitime/latest/hifitime/struct.Epoch.html#method.from_tai_duration.

Thanks

@codecov
Copy link

codecov bot commented May 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.87%. Comparing base (1311c4f) to head (109ee42).
Report is 52 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #379      +/-   ##
==========================================
- Coverage   84.32%   83.87%   -0.46%     
==========================================
  Files          23       24       +1     
  Lines        3720     3813      +93     
==========================================
+ Hits         3137     3198      +61     
- Misses        583      615      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ChristopherRabotin
Copy link
Member

Hmm, I can tackle the dependency error. It seems like the MSRV may need updating again.

#[must_use]
/// Initialize an Epoch from the provided IEEE 1588-2008 (PTPv2) millisecond timestamp since TAI midnight 1970 January 01.
/// PTP uses the TAI timescale but with the Unix Epoch for compatibility with unix systems.
pub fn from_ptp_milliseconds(millisecond: f64) -> Self {
Copy link
Collaborator

@gwbres gwbres May 20, 2025

Choose a reason for hiding this comment

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

Minor thought,

(I also like having construction methods like these a lot, they make life much easier for the end user)
but since PTP applications are usually located around tens of nanoseconds, I would rather have from_ptp_nanoseconds instead of milliseconds ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair point. added

@georgeboot georgeboot requested a review from gwbres May 20, 2025 19:13
@georgeboot
Copy link
Contributor Author

Hmm, I can tackle the dependency error. It seems like the MSRV may need updating again.

Yeah not too sure what to do about that.

@ChristopherRabotin
Copy link
Member

Thanks @georgeboot , I'll merge this now. I'm working on fixing the lints on another branch. It'll be released with version 4.1.0 in the coming days.

@ChristopherRabotin ChristopherRabotin merged commit 36f699f into nyx-space:master May 24, 2025
21 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support PTP TAI epoch

3 participants