search

uos / sick_tim

A ROS driver for the SICK TiM series of laser scanners.
http://wiki.ros.org/sick_tim
47 stars 90 forks source link

added parameters for the name, the laser_link and the ros topic #14

Closed tigelbri closed 10 years ago

tigelbri commented 10 years ago

made the parameters for the name, the laser_link and the ros topic in the sick_tim3xx.urdf file dynamic. So the package can be used for multi laser approaches.

jspricke commented 10 years ago

@mintar should we provide a version without the extra parameters for compatibility, or do we just assume the those using catkin already will get the change? Otherwise I'm fine with merging.

mintar commented 10 years ago

First off, I'd recommend cherry-picking the change to all other branches, as we've done until now. So we should pick a solution that's right for all branches, not just hydro_catkin.

That said, I'd say we merge this pull request. It will break other people's URDFs that include sick_tim3xx.urdf.xacro (if anybody does), but it should be easy enough to track down and fix.

Also, example.urdf in this repo should be fixed so that the include works.

jspricke commented 10 years ago

I'm against adding the topic as a parameter, as it's common to do it through topic remapping. Also, can you please open a new pull requests for different topics?

mintar commented 10 years ago

I'm also for dropping e335938 for the reasons that @jspricke mentioned.

I'm working on a modified version of @tigelbri 's other changes and will commit them soon, so don't bother fixing it yet.

mintar commented 10 years ago

I've incorporated a modified version of the proposed changes (configurable ros_topic name, TiM 551 support) into the URDFs. Thanks, @tigelbri !