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 for custom timestamp #97

Open
LastStarDust opened this issue May 22, 2023 · 8 comments
Open

Support for custom timestamp #97

LastStarDust opened this issue May 22, 2023 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@LastStarDust
Copy link

Hello!
Thank you for developing such a useful package. It has become an irreplaceable piece of our system.

In our system, the timestamp is expressed as a single uint64 number with nanosecond precision. Even if converting from nanosec to sec + nanosec is perfectly feasible, it would be cleaner if we could just use the timestamp as is.

Hence my question: would it be possible to use a custom definition of std_msgs/msg/Header Message for time synchronization with TimeSynchronizer or ApproximateTimeSynchronizer?

Concretely, I would like to use a header whose timestamp is a single uint64 stamp with nanosecond precision, instead of two int32 and uint32 stamps with second and nanosecond precision respectively, as in the default ROS2 definition.

If this feature is not implemented yet, I would like to contribute with a pull request. Just let me know if there is any request or suggestion on your side.

Regards
Giorgio

@clalancette
Copy link
Contributor

Can you give some more details on exactly what you are trying to do and how the current code doesn't meet your needs? As far as I can tell the current code doesn't directly depend on std_msgs/msg/Header anywhere, so it isn't clear to me what would need to change.

@clalancette clalancette added the more-information-needed Further information is required label May 22, 2023
@LastStarDust
Copy link
Author

@clalancette Sorry for not being clearer.

I am referring to the TimeSynchronizer class, which uses the message timestamp contained in the header to synchronize multiple ROS messages.

Please refer to here and here,

@clalancette
Copy link
Contributor

Sorry, now I see what you mean.

I think that as it stands, that won't really work because the code expects those fields to be a particular type. I think you'd have to reimplement parts of https://github.com/ros2/message_filters/blob/rolling/include/message_filters/message_traits.h to make it work properly.

@clalancette clalancette added help wanted Extra attention is needed and removed more-information-needed Further information is required labels Aug 3, 2023
@LastStarDust
Copy link
Author

@clalancette
Thank you for the hint.
Assuming I make it work, how gladly would you accept a pull request about this feature?

@clalancette
Copy link
Contributor

Assuming I make it work, how gladly would you accept a pull request about this feature?

Please feel free to open a PR if you implement something. As long as it doesn't break backwards compatibility (i.e. the default is still to use std_msgs/msg/Header), I think we can probably take it.

@LastStarDust
Copy link
Author

I do not plan on working on it any time soon, but I am happy to hear that you are open to including this feature.
I might open a PR someday. Until then feel free to close this issue if you prefer.

@lix19937
Copy link

lix19937 commented Aug 31, 2024

I also need this feature.

If I donot use rclcpp::Time stamp define timestamp, just use uint64_t stamp, then use synchronizer is still works ? @clalancette

@lix19937
Copy link

It seems user override the TimeStamp will works.

namespace message_filters
{
namespace message_traits
{
template<>
struct TimeStamp<Msg>
{
  static rclcpp::Time value(const Msg & m)
  {
    return m.user_stamp;
  }
};
}  // namespace message_traits
}  // namespace message_filters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants