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

[wpilib] Timer: Add getLoopTimestamp() #7364

Closed

Conversation

PeterJohnson
Copy link
Member

@PeterJohnson PeterJohnson commented Nov 8, 2024

This is updated at the start of each periodic function in TimedRobot, so is stable throughout the callback function execution.

TODO:

  • Finalize naming (GetLoopTimestamp? GetPeriodicTimestamp? GetLoopStartTimestamp?)
  • Provide access for testability in C++ (Java can use introspection)
  • Update WPILib classes to use this where appropriate (should MathShared use it? should Timer use it?)

This is updated at the start of each periodic function in TimedRobot, so is
stable throughout the callback function execution.
@PeterJohnson PeterJohnson requested a review from a team as a code owner November 8, 2024 08:27
@oh-yes-0-fps
Copy link
Contributor

oh-yes-0-fps commented Nov 11, 2024

You should mention that its loop start in the function ident I feel. When I first read getLoopTimestamp I was thinking time since the start of the loop. Also maybe an atomic bool that is flipped when a TimedRobot is initialized so you can more confidently fallback to getFPGATimestamp if the user isn't using a TimedRobot. This is especially important if its meant for libraries as the robot type the user uses is out of their control.

@thenetworkgrinch
Copy link
Contributor

This would make sensor reading cache's so much easier

@PeterJohnson
Copy link
Member Author

Closing in favor of #7378.

@PeterJohnson PeterJohnson deleted the add-loop-timestamp branch November 16, 2024 15:26
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.

3 participants