rst-tu-dortmund / teb_local_planner

An optimal trajectory planner considering distinctive topologies for mobile robots based on Timed-Elastic-Bands (ROS Package)
http://wiki.ros.org/teb_local_planner
BSD 3-Clause "New" or "Revised" License
984 stars 545 forks source link

[Bugfix] fixed init teb not clear poses vec #383

Closed 2linkthefire closed 11 months ago

2linkthefire commented 1 year ago
  1. fixed init when teb time diff vec is empty, but pose vec may has pose causing core dump.
2linkthefire commented 1 year ago

@corot

corot commented 1 year ago
  1. fixed init when teb time diff vec is empty, but pose vec may has pose causing core dump.

Hi, thanks for the fix. I never experience that bug. How can I reproduce it?

2linkthefire commented 12 months ago
  1. fixed init when teb time diff vec is empty, but pose vec may has pose causing core dump.

Hi, thanks for the fix. I never experience that bug. How can I reproduce it?

Okay, in general, teb will first add a pose when initializing, and then add a pose and a time_diff in pairs, so the number of time_diff will be one less than a pose; Because teb provides interfaces similar to deletePose() and deleteTimeDiff(), there may be only one pose left, which will be judged as False by TimedElasticBand::isInit(), and the last pose is not cleared when reinitializing, and this dirty data will cause the difference between the number of poses and the time_diff greater than 1; When using pose and time_diff in pairs, it may crash;

corot commented 12 months ago

Okay, in general, teb will first add a pose when initializing, and then add a pose and a time_diff in pairs, so the number of time_diff will be one less than a pose; Because teb provides interfaces similar to deletePose() and deleteTimeDiff(), there may be only one pose left, which will be judged as False by TimedElasticBand::isInit(), and the last pose is not cleared when reinitializing, and this dirty data will cause the difference between the number of poses and the time_diff greater than 1; When using pose and time_diff in pairs, it may crash;

Sounds reasonable. But still I don't know how to reproduce. Even better would be a unit test that reproduces the problem. Would it be possible to add one together with the fix? If not, clear instructions to reproduce it would do.

2linkthefire commented 11 months ago

Sounds reasonable. But still I don't know how to reproduce. Even better would be a unit test that reproduces the problem. Would it be possible to add one together with the fix? If not, clear instructions to reproduce it would do.

Thanks, I took a closer look at the comments on the min_samples parameter and the related deletions from native code, the number of poses should not be removed to less than min_samples number, so the problem does not exist in origin code.

corot commented 11 months ago

Thanks, I took a closer look at the comments on the min_samples parameter and the related deletions from native code, the number of poses should not be removed to less than min_samples number, so the problem does not exist in origin code.

Nice! can we close this PR, then? And thanks for the investigation!