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

Fix #429, OS_time_t with single tick counter #735

Merged

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Jan 4, 2021

Describe the contribution

Use a single 64-bit tick counter to implement OS_time_t, rather than a split 32 bit seconds + 32 bit microseconds counter.

This benefits in several ways:

  • increases the timing precision by 10x (0.1us ticks)
  • increases the representable range by 400x (+/-14000 yrs)
  • simplifies addition/subtraction (no carry over)
  • avoids "year 2038" bug w/32-bit timestamps

Fixes #429

Testing performed
Build and run all unit tests, sanity check CFE

Expected behavior changes
None

System(s) tested on
Ubuntu 20.04 (native)
RTEMS 4.11 + pc686 (qemu)

Additional context
This is the final step listed in issue #429, and it depends on several dependencies being merged already:

It is submitted as a separate PR from #723 due to the dependencies, so it doesn't necessarily all have to be merged at once. However if CCB wants to accelerate the rollout this can be all done in a single merge cycle.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) dependency labels Jan 4, 2021
@wal004
Copy link

wal004 commented Jan 5, 2021

This is the final step listed in issue #429, and it depends on several dependencies being merged already:

Would you be up for automatically managing dependent PRs? I built Dpulls to handle that - could be helpful if you often deal with dependent PRs - totally free for open source.

@astrogeco
Copy link
Contributor

astrogeco commented Jan 13, 2021

CCB:2021-01-13 APPROVED

@ejtimmon please comment on App impact

@astrogeco astrogeco added CCB:2021-01-13 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jan 13, 2021
Use a single 64-bit tick counter as OS_time_t, rather than
a split 32 bit seconds + 32 bit microseconds counter.

This benefits in several ways:

- increases the timing precision by 10x (0.1us ticks)
- increases the representable range by 400x (+/-14000 yrs)
- simplifies addition/subtraction (no carry over)
- avoids "year 2038" bug w/32-bit timestamps
@jphickey
Copy link
Contributor Author

Rebased to current "main" and confirmed this builds OK against the current "main" branch of cfe/psp. (i.e. dependencies resolved, everything looks OK).

Also added an explicit cast to uint32 in the fractional getters (milliseconds/microseconds/nanoseconds) to avoid triggering a conversion warning in the event that some app is being compiled with -Wconversion.

This should be ready to go now.

@astrogeco astrogeco changed the base branch from main to integration-candidate January 25, 2021 16:57
@astrogeco astrogeco merged commit 0867182 into nasa:integration-candidate Jan 25, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jan 25, 2021
@jphickey jphickey deleted the fix-429-expand-ostimet-part2 branch January 27, 2021 14:09
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
…nators

Fix nasa#735, add comment if null terminated or not.
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.

Use a better time representation in OS_stat call
4 participants