OctoMap / octomap_mapping

ROS stack for mapping with OctoMap, contains octomap_server package
http://www.ros.org/wiki/octomap_mapping
335 stars 280 forks source link

Fix parameter name in launch file #26

Closed andre-nguyen closed 8 years ago

wxmerkt commented 8 years ago

I'm uncertain whether this was broken. Looking at OctomapServer.cpp#L95, it should indeed be sensor_model/max_range for the parameter as in the launch file. The value you've changed it to is used in the dynamic reconfigure. If a value in the dynamic reconfigure is changed, it is then set to m_maxRange here.

Did you see any issues with the current launch file?

andre-nguyen commented 8 years ago

Interesting, I was running a simulation in gazebo and noticed some points being taken very far away and the performance was very slow. After changing this value it worked right.

I guess I could close this if its working right for everyone else.

wxmerkt commented 8 years ago

Interesting, I might be missing something here. I'll test and investigate tomorrow morning and also look forward to hear what insights @ahornung has.

andre-nguyen commented 8 years ago

Just to be clear here, I'm running on indigo (the PR is against indigo-devel).

Here's my full launch file so we're sure there aren't any shenanigans going on here.

<!--
  Example launch file for octomap_server mapping:
  Listens to incoming PointCloud2 data and incrementally builds an octomap.
  The data is sent out in different representations.

  Copy this file into your workspace and adjust as needed, see
  www.ros.org/wiki/octomap_server for details
-->
<launch>
    <node pkg="octomap_server" type="octomap_server_node" name="octomap_server">
        <param name="resolution" value="0.1" />

        <!-- fixed map frame (set to 'map' if SLAM or localization running!) -->
        <param name="frame_id" type="string" value="world" />

        <!-- maximum range to integrate (speedup!) -->
        <param name="sensor_model_max_range" value="10.0" />

        <!-- data source to integrate (PointCloud2) -->
        <remap from="cloud_in" to="/velodyne_pointcloud2" />

    </node>
    <node pkg="point_cloud_converter" name="point_cloud_converter" type="point_cloud_converter_node" >
        <remap from="points_in" to="/velodyne_pointcloud"/>
        <remap from="points2_out" to="/velodyne_pointcloud2" />
    </node>

    <node pkg="tf" type="static_transform_publisher" name="pointcloud2robot"
args="0 0 0 0 3.14159 0 firefly/velodyne/velodyne_link firefly/velodyne/velodyne_link_data 10" />
</launch>

and I'm using the Gazebo ros block laser plugin

ahornung commented 8 years ago

Yes, as iamwolf wrote the parameter name is definitely sensor_model/max_range. The only explanation I can think of is that you start dynamic_reconfigure on top. This could overwrite the parameter value with whatever is set in dynamic_reconfigure.

jvgomez commented 8 years ago

This is interesting. When I was doing the changes to the dynamic reconfigure I posted this question But then, for some reason, it started working for me, at least at launch time.

Some weeks ago I discovered that it is actually possible to update the dynamic reconfigure from cpp but it is a bit tedious in my opinion. In fact, it is a limitation of dynamic reconfigure.

I will take a look at it again to try to modify the dynamic reconfigure values with the launchfile values.

Anyway, is this PR actually fixing something? That parameter does not exist within the node.

andre-nguyen commented 8 years ago

@jvgomez This change allowed my server to actually limit the laser range whereas before it seemed to be taking all the points it received.

I guess if it works for everyone, I must have messed up something on my end; maybe with my simulation.

Closing this PR.

jvgomez commented 8 years ago

I cannot tell if it is working for me at this moment. I will test tomorrow.

ahornung commented 8 years ago

Conflict between dynamic_reconfigure and params should be fixed now in indigo-devel, as reported in #27.

andre-nguyen commented 8 years ago

Nice! Should be able to test tomorrow in the afternoon.

andre-nguyen commented 8 years ago

So I changed my launch file back to sensor_model/max_range and it looks like it works.

Thanks everyone! @iamwolf @ahornung @jvgomez