Handle ROS Time Shifts and C++14 Build Issue

[IntelligentJackal]

Overview

Author: @chacalnoir, @IntelligentJackal

Issue

• ROS time updates are not always consistent with c++ steady_clock. Even when ROS time is running real-time without pausing, starting, stopping, etc., it has the ability to jump forward and backward. Since the converters use steady_clock, this means there is a time offset that often results in future timestamps and typically results in incorrectly located detections for a moving sensor.
• When building on an Ubuntu 20.04 system with ROS noetic, std::make_unique was not defined. std::make_unique is a C++14 capability, and the c++11 flag seemed to be messing up the build. Changing the flag to c++14 fixed the issue.

Changes

• This PR adds the ability to reset the ROS base time by expecting that the ROS time update will be consistent with the steady_clock update. If they are not, the ROS base time is adjusted to result in a consistent time at the moment of call.
• If a user does not wish to use this capability, never calling this function results in the code acting the same as it always has.
• The build flag was updated from C++11 to C++14 to fix the build issue with std::make_unique

Testing

This has been tested on an Ubuntu 20.04 system, ROS noetic, with the new function being called at regular intervals. The results were that the ROS base time was updated as expected.

[saching13]

Thanks for the PR. LGTM.

1. Can you adjust the styling ?
2. Can you include the change in stereo_inertial_node with the option to disable it based on parameters?

[saching13]

Do you by any chance have an idea if this is the case with ROS2 timer too?

[IntelligentJackal]

Do you by any chance have an idea if this is the case with ROS2 timer too?

I do not know for sure, having not used ROS 2 directly. However, based on reading this (https://design.ros2.org/articles/clock_and_time.html) design info concerning ROS 2 clock, it looks like ROS 2 clock has the ability to parallel several clock implementations, with one being steady_clock. As such, I would think the problem likely exists for ROS 2 as well.

[IntelligentJackal]

Thanks for the PR. LGTM.

1. Can you adjust the styling  ?

2. Can you include the change in `stereo_inertial_node` with the option to disable it based on parameters?

(1) Done (2) Working on this

[IntelligentJackal]

Thanks for the PR. LGTM.

1. Can you adjust the styling  ?

2. Can you include the change in `stereo_inertial_node` with the option to disable it based on parameters?

(1) Done (2) Working on this

The stereo_inertial_node appears to primarily operate on callbacks, which makes these functions a bit harder to call. Likewise, the BridgePublisher acts similarly. So, I added a setting into each of the converters for automatically updating ROS base time on each toRosMsg call. May be called more or less often depending on how many messages are flowing. But, the updates to the ROS base time can now fully reside within the converters. This is controlled via a launch parameter in stereo_inertial_node.

[saching13]

LGTM. thanks for the PR.

[borongyuan]

A better solution is using realtime_clock from realtime_tools.

[IntelligentJackal]

A better solution is using realtime_clock from realtime_tools.

I agree that better solves the problem when using a real-time clock. However, I did not want to leverage the requirement on this software and users thereof that they have either the steady clock or the real-time clock. So, there was the option to implement all ROS clock options or implement this solution, which enables an external user to update this clock to match what they are doing. Adding a real-time clock option could be useful for when users of this software are using a real-time clock (they'll get exactly the desired behavior), while keeping this more generic option to capture all potential clock-type usages.