search

ROBOTIS-GIT / hls_lfcd_lds_driver

ROS package for HLDS HLS-LFCD LDS driver
http://turtlebot3.robotis.com
BSD 3-Clause "New" or "Revised" License
90 stars 70 forks source link

Corrected max-range to avoid fencepost error. #27

Closed skasperski closed 6 years ago

skasperski commented 6 years ago

The issue here is that in a laser scan the angle_min should refer to the direction of the first beam and angle_max to the direction of the last beam in the scan. If you assume 0 ~ 2pi, the first beam is counted twice, resulting in 361 beams that should be in the scan.

kijongGil commented 6 years ago

Hi :) Thank you for your contribution.

However, lines 91-95 in hlds_laser_publisher.cpp and lines 67-71 in hlds_laser_segment_publisher.cpp have not real values. This code just notices data type like as comment.

Thanks Gilbert

skasperski commented 6 years ago

These values are important for algorithms that make use of the laser-scans, e.g. mapping algorithms. They use these values to transform the range readings into coordinates and cannot work correctly if the scan's meta-data contains wrong values.

kijongGil commented 6 years ago

I agree with your opinion. Correctly scan data is important for algorithms. But, Your PR has not function. Because I said that these lines are just notice to developer as like http://docs.ros.org/api/sensor_msgs/html/msg/LaserScan.html I know what you are worried about. here are values https://github.com/ROBOTIS-GIT/hls_lfcd_lds_driver/blob/25b2778bfe86c8002a5afb1d8675f03ccf67b323/src/hlds_laser_publisher.cpp#L125

Thanks Gilbert

skasperski commented 6 years ago

The scan values are not the problem, they are alright. The meta-data in the ROS message is needed for algorithms to make correct use of the scan values. It is indeed important that they are correct, so that other modules can work correctly. Especially the min- and max angles are required to create 3D points from the array of range measurements.

kijongGil commented 6 years ago

Thanks for your contribution. :) I checked again this issue. I'm sorry to have told you the wrong thing. The data are included in ROS message. It is very important as you said. I changed the branch to develop. Thanks!

robotpilot commented 6 years ago

Hi @skasperski,

Thank you for your consideration and supports. I've tested Karto SLAM in addition to the Gmapping and Cartographer we mainly used, and there was a big problem as you said. I have merged your PR changes with our master branch #28 and will release it in the next ROS binary update ASAP.

Thank you so much!