Add plugins Parameter to BT XML for Selective Clearing of Costmap Layers

[BCKSELFDRIVEWORLD]

PR #5400 was broken due to my mistake

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	(add tickets here #1)
Primary OS tested on	(Ubuntu, MacOS, Windows)
Robotic platform tested on	(Steve's Robot, gazebo simulation of Tally, hardware turtlebot)
Does this PR contain AI generated software?	(No; Yes and it is marked inline in the code)
Was this PR description generated by AI software?	Out of respect for maintainers, AI for human-to-human communications are banned

---

Description of contribution in a few bullet points

Description of documentation updates required from your changes

Description of how this change was tested

---

Future work that may be required in bullet points

For Maintainers:

• Check that any new parameters added are updated in docs.nav2.org
• Check that any significant change is added to the migration guide
• Check that any new features OR changes to existing behaviors are reflected in the tuning guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists
• Should this be backported to current distributions? If so, tag with backport-*.

[codecov]

Codecov Report

❌ Patch coverage is 24.75248% with 76 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
nav2_costmap_2d/src/clear_costmap_service.cpp	8.10%	68 Missing ⚠️
...vior_tree/plugins/action/clear_costmap_service.cpp	60.00%	8 Missing ⚠️

Files with missing lines	Coverage Δ
...vior_tree/plugins/action/clear_costmap_service.hpp	100.00% <100.00%> (ø)
...vior_tree/plugins/action/clear_costmap_service.cpp	83.67% <60.00%> (-16.33%)	⬇️
nav2_costmap_2d/src/clear_costmap_service.cpp	20.80% <8.10%> (-15.03%)	⬇️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[BCKSELFDRIVEWORLD]

thanks for reviews @Sushant-Chavan

[Sushant-Chavan]

LGTM.

Awaiting Steve's comment regarding the response from the service in case a requested costmap plugin could not be cleared.

[SteveMacenski]

A number of tests related to code changes are now failing

[BCKSELFDRIVEWORLD]

If a requested plugin is not clearable, is it enough to skip it and reset the master, since I already log it? @Sushant-Chavan I’d appreciate your review on this

[Sushant-Chavan]

If a requested plugin is not clearable, is it enough to skip it and reset the master, since I already log it?

I think if atleast one layer was reset, we should reset the master costmap. Otherwise we shouldn't.

I also think that once you find a non-clearable plugin that was requested to be cleared by the client, you should stop clearing any further requested plugins. Then reset the master costmap (if any previous plugins were reset), and return a failure response. Do you agree @SteveMacenski ?

[BCKSELFDRIVEWORLD]

I think if atleast one layer was reset, we should reset the master costmap. Otherwise we shouldn't.

i agree too

I also think that once you find a non-clearable plugin that was requested to be cleared by the client, you should stop clearing any further requested plugins. Then reset the master costmap (if any previous plugins were reset), and return a failure response

I think the service response should only return false if a plugin is clearable but fails to clear.

• If a plugin is not clearable, we just log a warning and skip it.
• If a plugin is clearable but could not be cleared, then the service should return false.

[SteveMacenski]

I also think that once you find a non-clearable plugin that was requested to be cleared by the client, you should stop clearing any further requested plugins. Then reset the master costmap (if any previous plugins were reset), and return a failure response. Do you agree @SteveMacenski ?

I agree.

I believe that @BCKSELFDRIVEWORLD implemented this in the last few commits, please re-review @Sushant-Chavan when you have a moment. Thanks for your time!

[BCKSELFDRIVEWORLD]

Summary i belive that true implementation:

Return false when:

• A clearable layer fails to clear.

• All requested plugins are non-clearable.(In this case, no reset is triggered and no actual clearing occurs, so the service should indicate failure.)

Return true when:

• At least one clearable layer was successfully cleared and the master costmap reset succeeded.

• Some plugins were skipped as non-clearable, but at least one clearable plugin was successfully cleared.

Skipping non-clearable layers should not by itself cause the service to return false. user may provide a mixed list of plugins, and as long as at least one clearable layer was cleared, the request is considered successful. Our only additional responsibility in these cases is to log or inform that some plugins were skipped, without treating that as a failure condition.

[SteveMacenski]

I think I agree with @Sushant-Chavan that if a user requests layers to clear explicitly (rather than 'clear all') all of those should be valid. If its not valid, its not a valid request and that should be made clear to a caller in more than just logging.

Whether we fail immediately or continue (and log all errors) and then return fail is up for debate. I would generally say 'do everything asked' rather than failing fast so that the state at the end of the service call is known for as much as it can be - there's no question about which was executed and which was not. However, I agree with the failure return as the application requesting specific layers should know better and that's a failure that's worth notifying the caller.

[SteveMacenski]

@BCKSELFDRIVEWORLD any update? I'd love to get this in this week!

[BCKSELFDRIVEWORLD]

@SteveMacenski I’ve completed all the requested changes and attached a few screenshots.
Some of the tests were failing because of the boolean conversions, so I made a small fix for that.
I’m not entirely sure if this is the correct implementation since I don’t have much experience with this part, but the tests are now passing.Please let me know if you have any suggestions for improving it.

[BCKSELFDRIVEWORLD]

another case: