Skip to content
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.

Publish messages using frame timestamp #105

Open
wants to merge 5 commits into
base: eloquent
Choose a base branch
from

Conversation

ruffsl
Copy link

@ruffsl ruffsl commented Jan 24, 2020

By convention, ROS drivers attempt to preserve timestamps from the sensor. This implements that by transparently setting the message timestamp wrt. the set timestamp domain selected by the SDK.

https://github.com/IntelRealSense/librealsense/blob/9f99fa9a509555f85bffc15ce27531aaa6db6f7e/include/librealsense2/hpp/rs_frame.hpp#L467

Related: #84

Copy link

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

Conversion ratio seems off to me, alternative conversion suggested inline

realsense_ros/src/rs_base.cpp Outdated Show resolved Hide resolved
@ruffsl ruffsl force-pushed the frame_timestamp branch 3 times, most recently from c7c16fe to 628c5ac Compare January 24, 2020 17:59
Copy link

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

Main remark was about the missing CMake change, everything else is nitpicky and opinionated so maybe maintainers here would have a different opinion

realsense_ros/include/realsense/rs_base.hpp Outdated Show resolved Hide resolved
realsense_ros/package.xml Outdated Show resolved Hide resolved
realsense_ros/CMakeLists.txt Show resolved Hide resolved
@ruffsl
Copy link
Author

ruffsl commented Jan 25, 2020

After some thinking, this may need some more work to respect the clock from rcl, e.g. initializing the sensor time with that of the node's clock, like that which is done for the ROS1 realsence driver.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants