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.

Sign up for GitHub

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

Jump to bottom

Conversions from Unix time to CHIP Epoch are wrong #30990

Open
tcarmelveilleux opened this issue Dec 13, 2023 · 2 comments
Open

Conversions from Unix time to CHIP Epoch are wrong #30990

tcarmelveilleux opened this issue Dec 13, 2023 · 2 comments
Labels
IM Event Epoch Timestamp testing tests

Comments

@tcarmelveilleux
Copy link
Contributor

  • UTC time has leap seconds
  • Unix/POSIX TAI time does not.

See https://www.nist.gov/pml/time-and-frequency-division/time-realization/leap-seconds

As of Dec 13, 2023, there are 37s of difference between a given POSIX/TAI time (earlier) and UTC time (later).

All of our conversions from POSIX time to CHIP Epoch time are made assuming TAI timeline, while the spec says:

7.19.2.5. Epoch Time in Microseconds
This type represents an offset, in microseconds, from 0 hours, 0 minutes, 0 seconds, on the 1st of January, 2000 UTC (the Epoch), encoded as an unsigned 64-bit scalar value.

This offset is the sum of two parts: time elapsed, not counting leap-seconds, and a local time offset. The local time offset MAY include a timezone offset and a MAY include a DST offset.

Any use of this type SHALL indicate how the associated local time offset is determined in the specific context of that use. This MAY be done, for example, by simply saying the time is a UTC time, in which case the local time offset is 0.

A given Epoch Time value MAY be interpreted in at least two ways:

The value can be converted to a local clock date/time (year, month, day, hours, minutes, seconds, microseconds) by treating the local time offset as 0 and finding the UTC (year, month, day, hours, minutes, seconds, microseconds) tuple that corresponds to an elapsed time since the epoch time equal to the given value. The value then represents that tuple, but interpreted in the specific timezone and DST situation associated with the value. This procedure does not require knowing the local time offset of the value.

The value can be converted to a UTC time by subtracting the associated local time offset from the Epoch Time value and then treating the resulting value as an elapsed count of microseconds since the epoch time.

It was found, while adding unit tests to src/lib/support/tests/TestTimeUtils.cpp that there are no "absolute" well-known values used to validate the time scales, and that the Time Sync cluster's implementation of POSIX to CHIP Epoch time is not correctly handled.
@tcarmelveilleux tcarmelveilleux added testing IM Event Epoch Timestamp tests time sync Implementation of the Time Synchronization cluster labels Dec 13, 2023
@github-actions github-actions bot removed the time sync Implementation of the Time Synchronization cluster label Dec 13, 2023
@ksperling-apple
Copy link
Contributor

The text says "time elapsed, not counting leap-seconds" which means we're effectively using TAI or a smeared definition of UTC?

@tcarmelveilleux tcarmelveilleux mentioned this issue Dec 14, 2023
Merged
tcarmelveilleux pushed a commit to tcarmelveilleux/connectedhomeip that referenced this issue Dec 14, 2023
@tennessee-google
Clean-up time conversions locations
c3bbb99 
- Test conversions in Time Sync cluster were not in a unit-testable
  location.
- TestTimeUtils was not using nl-unit-test
- Time sync cluster server build rules were missing dependencies
- Documentation for many time conversion methods was not accurate
- `secondsToMilliseconds` did not match coding style
- Cleaned-up cut and paste errors in some adjacent tests.

Issue project-chip#30990

Testing done:
- Unit tests and integration tests still pass.
- Started tests for edge conditions (which found project-chip#30990)
@tcarmelveilleux tcarmelveilleux mentioned this issue Dec 14, 2023
Merged
@github-project-automation github-project-automation bot moved this from Todo to Done in [Feature] Time Sync Dec 15, 2023
thivya-amazon pushed a commit to thivya-amazon/connectedhomeip that referenced this issue Dec 18, 2023
@tcarmelveilleux @tennessee-google
@restyled-commits @thivya-amazon
Clean-up time conversions on the way to fix project-chip#30990 (proje…
4604051 
…ct-chip#31021)

* Clean-up time conversions locations

- Test conversions in Time Sync cluster were not in a unit-testable
  location.
- TestTimeUtils was not using nl-unit-test
- Time sync cluster server build rules were missing dependencies
- Documentation for many time conversion methods was not accurate
- `secondsToMilliseconds` did not match coding style
- Cleaned-up cut and paste errors in some adjacent tests.

Issue project-chip#30990

Testing done:
- Unit tests and integration tests still pass.
- Started tests for edge conditions (which found project-chip#30990)

* Restyled by clang-format

* Add backwards compatibility

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
@tcarmelveilleux
Copy link
Contributor Author

This is not completed, re-opening

@github-project-automation github-project-automation bot moved this from Done to In Progress in [Feature] Time Sync Dec 22, 2023
@tcarmelveilleux tcarmelveilleux mentioned this issue Dec 22, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
IM Event Epoch Timestamp testing tests
Projects
[Feature] Time Sync
Status: In Progress
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tcarmelveilleux @ksperling-apple