ira_laser_tools icon indicating copy to clipboard operation
ira_laser_tools copied to clipboard

Published 20 hours ago • verified publisher icon iralabdisco

Add support for ROS2 humble

Open nakai-omer opened this issue 2 years ago • 12 comments

Hi,

We have ported this lib to ROS2 humble, could probably work on older ROS2 versions as well, just the static transform arguments have changed in humble. We tried to make only small changes, without major refactoring, in order to be able to easily migrate fixes from current implementation to this branch.

nakai-omer avatar Jul 13 '22 11:07 nakai-omer

it should be merged into a humble branch to match ros guidelines

nakai-omer avatar Jul 13 '22 11:07 nakai-omer

@nakai-omer I had to change Line 219 of laserscan_multi_merger.cpp otherwise the range was limited. See here. Just wondering if you have the same issue?

ryanpennings avatar Aug 07 '22 05:08 ryanpennings

@ryanpennings Thanks for the feedback. In our implementation we use the transformLaserScanToPointCloud override which doesn't include the range_cutoff param at all. This to my understanding, should cause the range_max from the laserscan message to be used instead. Make sure the value of max range in your scan message is correct.

nakai-omer avatar Aug 09 '22 13:08 nakai-omer

I had the same limited range issue as @ryanpennings and the fix worked.

Also, I had to set QoS for the publisher to be reliable in order for the merged scan to show up in rviz2.

samiamlabs avatar Aug 23 '22 11:08 samiamlabs

@samiamlabs Can you please check what is the max_range param from your laser scan message, and whether is matches your configured range_max in this package? I will merge this line fix if needed.

nakai-omer avatar Aug 28 '22 12:08 nakai-omer

I have already checked that. I could not find a transformLaserScanToPointCloud that matched the arguments you were using.

samiamlabs avatar Aug 29 '22 08:08 samiamlabs

@samiamlabs Thanks for the update. I have added @ryanpennings fix to this PR.

nakai-omer avatar Aug 29 '22 10:08 nakai-omer

Hi maintainers and author @nakai-omer Are there any other changes that are needed to be made on this branch? Can it be merged so we can have the option for ROS2?

vineet131 avatar Apr 15 '23 14:04 vineet131

@vineet131 From my point of view, it can be merged, it is working for us with ROS2 Humble.

nakai-omer avatar Apr 15 '23 20:04 nakai-omer

@vineet131 From my point of view, it can be merged, it is working for us with ROS2 Humble.

Thank you! So maintainers @axelfurlan and @trigal can we have this PR merged?

Edit: Asking again including @iralab

vineet131 avatar Apr 15 '23 23:04 vineet131

Hello authors @JackFrost67 @axelfurlan @trigal @iralab

Once again I wanted to ask if this PR can be accepted so that ROS2 upstream support for this repo can be obtained. It would be of help to us. Thank you.

vineet131 avatar Jul 06 '23 07:07 vineet131

Hello mantainers @JackFrost67 @axelfurlan @iralab any update about the possibility to merge the PR, for us it is working well in ROS Humble!

andrycapod avatar Oct 27 '23 12:10 andrycapod