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

Publish messages using frame timestamp - #105

Open
ruffsl wants to merge 5 commits into
intel:eloquentfrom
ruffsl:frame_timestamp
Open

Publish messages using frame timestamp#105
ruffsl wants to merge 5 commits into
intel:eloquentfrom
ruffsl:frame_timestamp

Conversation

@ruffsl

@ruffsl ruffsl commented Jan 24, 2020

Copy link
Copy Markdown

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

@mikaelarguedas mikaelarguedas left a comment

Copy link
Copy Markdown

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

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

@mikaelarguedas mikaelarguedas left a comment

Copy link
Copy Markdown

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

Comment thread realsense_ros/include/realsense/rs_base.hpp Outdated
Comment thread realsense_ros/package.xml Outdated
Comment thread realsense_ros/CMakeLists.txt
@ruffsl

ruffsl commented Jan 25, 2020

Copy link
Copy Markdown
Author

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.

2 participants