Fix costmap on_cleanup issues [eloquent-devel] #1431
Conversation
…and reset param_subscriber pointer
|
Looks like a unit test needs updating if removing undeclare. Otherwise seems good |
The plugins' |
|
Look at the CI test results. You need to update a unit test that is failing from your change. |
CircleCI doesn't work on the release branches yet. |
Codecov Report
@@ Coverage Diff @@
## eloquent-devel #1431 +/- ##
================================================
Coverage ? 40.2%
================================================
Files ? 231
Lines ? 11848
Branches ? 5116
================================================
Hits ? 4764
Misses ? 3717
Partials ? 3367
Continue to review full report at Codecov.
|
|
|
|
@SteveMacenski Just updated the test. Like I mentioned in description, I don't think we want/need to undeclare parameters during a layer reset. I could be onboard for undeclaring parameters in the node in the destructor for each plugin though, if it is worth cleaning up parameters in the case that different plugins are swapped in a re-configure of the Costmap2DROS object. |
ghost
mentioned this pull request
* -fix for #1363. -created clearMap function for each layer. -use clearMap instead of reset to clear the costmaps * -function name change -infilation layer clearMap change -make resetMap a pure virtual function * Changing clearMaps back to reset * Fix uncrustify error * Fixing lifecycle transitions and tests now that reset doesn't act as a cleanup function * Updating this based on the approach taken in PR #1431 * Moving voxel_grid reset to the resetMaps function. * Missed adding resetMaps back to header. * Adding some comments to help navigate the inheritance hierarchy
* -fix for #1363. -created clearMap function for each layer. -use clearMap instead of reset to clear the costmaps * -function name change -infilation layer clearMap change -make resetMap a pure virtual function * Changing clearMaps back to reset * Fix uncrustify error * Fixing lifecycle transitions and tests now that reset doesn't act as a cleanup function * Updating this based on the approach taken in PR #1431 * Moving voxel_grid reset to the resetMaps function. * Missed adding resetMaps back to header. * Adding some comments to help navigate the inheritance hierarchy
2 participants
The
nav2_system_testswere failing in large part due to the parameter callbacks being called erroneously during theon_cleanupof theCostmap2DROSobject. It is a pattern that theresetof costmap plugins will reset/deactivate and also re-initialize. In this PR I'm maintaining that behavior since I believe that is intended for use when clearing the costmap during run-time, but here I'm removing theresetLayersinside ofon_cleanup, especially since deleting thelayered_costmapwill destroy the plugins anyway. Also, since the parameters are beingdeclare_if_not_declared, there's no real need to undeclare the parameters when resetting.Future work that may be required in bullet points