Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

use SteadyTimer in message_filters#1247

Merged
dirk-thomas merged 1 commit intoros:lunar-develfrom
flixr:message_filters_steady_timer
Dec 19, 2017
Merged

use SteadyTimer in message_filters#1247
dirk-thomas merged 1 commit intoros:lunar-develfrom
flixr:message_filters_steady_timer

Conversation

@flixr
Copy link
Copy Markdown
Contributor

@flixr flixr commented Nov 24, 2017

To make sure that the message filters also work if system time is set back, use SteadyTimer instead of the normal Timer

void init()
{
update_timer_ = nh_.createTimer(update_rate_, &TimeSequencer::update, this);
update_timer_ = nh_.createSteadyTimer(ros::WallDuration(update_rate_.toSec()), &TimeSequencer::update, this);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can WallDuration really not be initialized from a Duration? That seems like an API fail.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IIRC it can't... but I didn't check that again and did it explicitly just to be sure...

@mikepurvis
Copy link
Copy Markdown
Member

LGTM. It's a behaviour change, but I'd vote for this go into Lunar. It's extremely unlikely someone would be depending on the current broken behaviour.

That said, there's a potential ABI issue would be if a third party had message_filter class members, those will now change in type (though not in size, so maybe it wouldn't actually be a problem, since there's nothing to link?).

@dirk-thomas
Copy link
Copy Markdown
Member

Thank you for the patch.

@dirk-thomas dirk-thomas merged commit ba502fa into ros:lunar-devel Dec 19, 2017
@flixr flixr deleted the message_filters_steady_timer branch December 19, 2017 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants