topic_tools: throttling when rostime jump backward #1397
Conversation
|
Should there be some kind of threshold for the jump, or at least a flag to disable this behaviour? I'm thinking a default that the jump-back-in-time logic is only triggered for a delta > 1s or something. Admittedly, I may be grasping at straws here, but I'm imagining a scenario where two separate machines are publishing the same topic with slightly offset clocks, or even with the same clock but network lag which causes the messages to interleave. |
There was a problem hiding this comment.
If time moves backwards it's definitely an event. Realistically you should never have two clocks publishing, that's basically guarenteed to cause bad behavior due to interleaving. And non-monotonic clocks will wreak havoc hear and elsewhere.
This is the same logic that tf users to clear the buffer: https://github.com/ros/geometry2/blob/melodic-devel/tf2_ros/src/transform_listener.cpp#L105
tools/topic_tools/src/throttle.cpp
Outdated
| @@ -126,6 +126,11 @@ void in_cb(const ros::MessageEvent<ShapeShifter>& msg_event) | |||
| now.fromSec(ros::WallTime::now().toSec()); | |||
| else | |||
| now = ros::Time::now(); | |||
| if (g_last_time > now) | |||
| { | |||
| ROS_WARN("Detected jump back in time."); | |||
There was a problem hiding this comment.
This should state what it's doing, not just that it detected. Something like "Detected jump back in time, resetting throttle period to now for ***."
There was a problem hiding this comment.
@mikepurvis @tfoote Thank you very much for your review!
@tfoote I updated the message. Please check. 👍
furushchev
force-pushed
the
throttle_backward
branch
from
e5131d1 to
0bbc1fe
Compare
|
lgtm, we don't really have a way to address the throttle instance so at least it's know that some throttle will be updated. |
|
Thank you for the patch. |
Closes #1396