Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SteadyTime and SteadyTimer for bond timeouts #18

Merged
merged 3 commits into from Jul 20, 2017

Conversation

flixr
Copy link
Contributor

@flixr flixr commented Mar 6, 2017

Fixes #16

Depends on ros/roscpp_core#57 and ros/ros_comm#1014

Tested and working on Ubuntu14.04/Indigo.

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

These changes lgtm, but it might require that this repository create a new branch for lunar, if the upstream API addition only gets merged there. It would need to be backported to all supported ROS distributions to avoid any branching in this repository.

It would also be possible to use the preprocessor to conditionally use SteadyTimer when it's available from roscpp, but that's up to @mikaelarguedas.

@mikaelarguedas
Copy link
Member

@flixr I merged some cosmetics fixups on master so I took the liberty of rebasing this branch.
It looks good so far, I have some more testing to do though.
I think the fix is critical enough to be made available for every ros distribution. I'll wait to know if SteadyTimer will be backported for all active ros distros and make the appropriate changes based on this information.

Thanks for providing a fix for this! That's a bug that has certainly been impacting many users

Copy link
Member

@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.

LGTM, waiting on approval on ros/ros_comm#1014 and ros/roscpp_core#57 to decide if this can me merged as is or need a distro specific segregation (preprocessor or branching out)

@mikaelarguedas
Copy link
Member

@mikaelarguedas to either branch out or ifdef to use the new timer only on Lunar

@mikaelarguedas mikaelarguedas changed the base branch from master to lunar-devel July 13, 2017 23:41
@mikaelarguedas
Copy link
Member

ros/ros_comm#1014 and ros/roscpp_core#57 have been merged so I'm going to merge this for lunar. thanks @flixr for the valuable contribution, that's a great improvement!

@mikaelarguedas mikaelarguedas merged commit 38fccce into ros:lunar-devel Jul 20, 2017
@flixr flixr deleted the steady_time branch January 4, 2018 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants