ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.45k stars 1.24k forks source link

VoxelLayer and ObstacleLayer should raycast for invalid pointcloud points padded inf or 0 #4297

Closed cemonem closed 3 months ago

cemonem commented 4 months ago

Feature request

VoxelLayer and ObstacleLayer should optionally raycast for invalid pointcloud points padded with inf or 0. These invalid points can come from sensors (depth cameras etc.) when they do not hit an obstacle (Gazebo's depth camera plugin pads invalid points with inf, for example, or D435 depth camera optionally pads these points with 0) I don't know if this is already a feature for laserscans with inf_is_valid parameter, but that parameter is not supported for pointcloud data.

Feature description

The observation cloud is the filtered version of the pointcloud with global z value between min_obstacle_height and max_obstacle_height here

https://github.com/open-navigation/navigation2/blob/07fbc50edb75ee97b3650716227caa6925874e84/nav2_costmap_2d/src/observation_buffer.cpp#L135C17-L135C18

The clearing raycast feature only raycasts for points in the observation cloud here, which results in absent obstacles only being cleared if they were previously in front of an existing obstacle. Obviously it should not behave like this.

https://github.com/open-navigation/navigation2/blob/07fbc50edb75ee97b3650716227caa6925874e84/nav2_costmap_2d/plugins/obstacle_layer.cpp#L652

Implementation considerations

This feature could be added as an additional parameter.

SteveMacenski commented 3 months ago

(Gazebo's depth camera plugin pads invalid points with inf, for example, or D435 depth camera optionally pads these points with 0)

So those points are invalid? They shouldn't be used to raycast then. We need some kind of "hit" to know an obstacle is there to raytrace up-to. Laser scanners have the option for inf is valid because those are active laser sensors and no return is better-chance-than-not total clear space past the bounds of its power rating. 3D sensors are frequently passive or with an abundant reasons why you wouldn't get data back for a particular pixel, so the assumption that Inf is a meaningful measurement is mostly untrue in my experience.

cemonem commented 3 months ago

3D sensors are frequently passive or with an abundant reasons why you wouldn't get data back for a particular pixel, so the assumption that Inf is a meaningful measurement is mostly untrue in my experience.

Depth cameras might perform worse than laser scanners in some environments, but this is not a reason to exclude such an important feature. People might have to rely on these solutions due to budget reasons. I am not sure it the correct approach to dismiss a feature simply because it might generate false positives due to low quality sensors. This should be up to the developers of the application, they can simply test it and disable it if it performs worse.

SteveMacenski commented 3 months ago

Its a physical characteristic of the sensor modality and we should provide the appropriate features that represent the model of sensor we're working with. If we expect that a sensor will produce no measurements for some portions of the reading due to how it computes depth, then we can't derive any kind of clearing information from that lack of measurement - it is not necessarily or even likely from valid freespace with passive or even what you might refer to as an active RGB-D cameras.

cemonem commented 3 months ago

I really am not convinced this justifies the omission of this feature, but as I am relatively new, I respect your viewpoint. Thank you for the explanation.

StetroF commented 3 months ago

Here is the part where obstacle layer handle the inf laserscan data:

  float epsilon = 0.0001;  // a tenth of a millimeter
  sensor_msgs::msg::LaserScan message = *raw_message;
  for (size_t i = 0; i < message.ranges.size(); i++) {
      float range = message.ranges[i];
       if (!std::isfinite(range) && range > 0) {
      message.ranges[i] = message.range_max - epsilon;
    }
  }

You can have a try to subscribe the laserscan and camera's pointcloud data. And you will observe that camera will return a empty list rather than a list full of inf value when camera is facing a empty space. Comparing with camera.Laserscan's return data will full of inf value. The code upon show up that you must have a value so that you can change their inf value to max_range.But if the list is empty.It's meaningless to handle them.