ray_ground_filter can not remove ground points

[storrrrrrrrm]

Checklist

• [X] I've read the contribution guidelines.
• [X] I've searched other issues and no duplicate issues were found.
• [X] I'm convinced that this is not my fault but a bug.

Description

test with pointcloud whose frame is not base_link

when set frame_id to base_link https://github.com/autowarefoundation/autoware.universe/blob/main/perception/ground_segmentation/src/ray_ground_filter_nodelet.cpp#L62

it removes ground points correctly

when set frame_id to velodyne_top it can not remove ground points correctly

Expected behavior

remove ground points correctly when start ray_ground_filter with param frame_id set to a value which is not base_link

Actual behavior

it can not remove ground points correctly if do not transform pointcloud to base_link frame

Steps to reproduce

currently there is no a launch file like scan_ground_filter.launch.py in autoware. i found this issue when writing test program for ground_segmentation.

if u wanna reproduce this, u can test with a bag whose frame is not base_link, and start ray_ground_filter with frame_id setted same with frame in bag.

Possible causes

when compare current_height with general_height_threshold, https://github.com/autowarefoundation/autoware.universe/blob/main/perception/ground_segmentation/src/ray_ground_filter_nodelet.cpp#L244-L245 there is an implicit assumption, current_height is height from a fake flat ground。

however now current_height comes from z. https://github.com/autowarefoundation/autoware.universe/blob/main/perception/ground_segmentation/src/ray_ground_filter_nodelet.cpp#L223

so if z is value in base_link frame which is actually on ground, it classify ground points correctly。 if z is value in velodyne_top which is a frame on top of car, it can not classify correctly.

Additional context

we can add a param like z_offset to tell the height of input pointcloud frame to the flat ground. and refer to z_offset when calculate current_height

[miursh]

@storrrrrrrrm Thanks for creating this issue. As you mentioned, ray ground filter requires the output frame to be base_link implicitly. Even though, it seems a bit odd to add a z_offset parameter since tf should be used for coordinate transformation.　

[xmfcx]

I think this is the expected behavior of the ray_ground_filter. Why do you want to run it in a different frame?

The way it uses the frame is, it knows where to initialize the ground plane from.

[storrrrrrrrm]

I think this is the expected behavior of the ray_ground_filter. Why do you want to run it in a different frame?

The way it uses the frame is, it knows where to initialize the ground plane from.

it is ok for me that if ray_ground_filter works only when frmae_id is base_link. just found that scan_ground_filter has no such limitiation when add test case. you can have a look at https://github.com/autowarefoundation/autoware.universe/pull/1401

if designed to be worked with base_link only, maybe it is better to set base_frame to a const value in code rather than a ros parameter, or just add some explanation in ray-ground-filter.md :smile:

[badai-nguyen]

@storrrrrrrrm There is another way If you want to test separately ground filter with your non-base_link rosbag. You can run autoware with your pointcloud rosbag and remap pointcloud topic to the ground_filter's input topic like: ros2 bag play your_rosbag --remap /your/velodyne_top/pointcloud =/perception/obstacle_segmentation/range_cropped/pointcloud Since current ground_filter processes pointcloud after crop_box so it is better to remap to crob_box_filter's input: ros2 bag play your_rosbag --remap /your/velodyne_top/pointcloud:=/sensing/lidar/concatenated/pointcloud

you can select ray_ground_filter by replace ScanGroundFilterComponent here: https://github.com/autowarefoundation/autoware.universe/blob/abaea407741d347f6a77761d9b71f31f66db722c/launch/tier4_perception_launch/config/obstacle_segmentation/ground_segmentation/ground_segmentation.param.yaml#L17

Please make sure adding base_frame parameter into setting file.

As my testing on the top lidar extracted from sample-rosbag , ray_ground_filter also worked.
Ray ground filter test:

Scan_ground_filter test:

Note: using current ray_ground_filter without crop_box_filter will show quite different result:

[miursh]

@storrrrrrrrm

if designed to be worked with base_link only, maybe it is better to set base_frame to a const value in code rather than a ros parameter

I agree with this. Could you make change for it in this PR

[storrrrrrrrm]

@storrrrrrrrm

if designed to be worked with base_link only, maybe it is better to set base_frame to a const value in code rather than a ros parameter

I agree with this. Could you make change for it in this PR

sure，just one question,where is 'this PR',when i click this,it jumps to https://github.com/autowarefoundation/autoware.universe/issues/url

[miursh]

oops, sorry. I mean your PR. https://github.com/autowarefoundation/autoware.universe/pull/1401

[xmfcx]

@storrrrrrrrm The mentioned PR only adds the test and this issue still needs to be resolved right?

[ralwing]

if designed to be worked with base_link only, maybe it is better to set base_frame to a const value in code rather than a ros parameter

base_link could have a different name. e.g. in Carla simulator it's hero. So IMHO this parameter should be configurable.

[storrrrrrrrm]

@storrrrrrrrm The mentioned PR only adds the test and this issue still needs to be resolved right?

no, i added a z_offset and solved this issue already in #1401, but @miursh thought it is odd to add a z_offset and it is better to set base_frame to a const value,@badai-nguyen pointed a way to test ray_ground_filter without using unit test,i did not try this since recently i am busy with somthing else.

[yukkysaito]

I checked the code, but this base_frame parameter is not originally needed. because it is supported in filter.cpp as input_frame .

Also, ray ground filter and scan ground filter assume to work on base_link frame. So, we need to remove base_frame and input_frame is fixed base_link (need to add this assumption).