Enforce cleanup_controller on more exit points of configure_controller

[vectorcrumb]

Two of the possible early return paths on controller_manager::configure_controller led to controllers being left in an INACTIVE state instead of rolling back to the UNCONFIGURED state. This specifically affected controllers that were chainable that:

• didn't export any state nor reference interfaces
• raised an exception while querying their state/reference interfaces

Two tests were added to the controller_manager/test_cleanup_controller test suite for these cases.

This PR should solve #3153

---

To send us a pull request, please:

• Fork the repository.
• Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
• Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
• Commit to your fork using clear commit messages.
• Send a pull request, answering any default questions in the pull request interface.
• Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.10%. Comparing base (bb07fdd) to head (99b5dbf).

Files with missing lines	Patch %	Lines
...ontroller_manager/test/test_cleanup_controller.cpp	83.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3192      +/-   ##
==========================================
- Coverage   89.12%   89.10%   -0.03%     
==========================================
  Files         158      158              
  Lines       19587    19601      +14     
  Branches     1589     1590       +1     
==========================================
+ Hits        17457    17465       +8     
- Misses       1471     1476       +5     
- Partials      659      660       +1     

Flag	Coverage Δ
unittests	89.10% <85.71%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
controller_manager/src/controller_manager.cpp	75.85% <100.00%> (-0.27%)	⬇️
...ontroller_manager/test/test_cleanup_controller.cpp	94.59% <83.33%> (-5.41%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[vectorcrumb]

Hi, I have some tests failing due to timing issues, could somebody lend me a hand?

[christophfroehlich]

Hi, I have some tests failing due to timing issues, could somebody lend me a hand?

These are known flaky tests and not related, don't worry.

[vectorcrumb]

These are known flaky tests and not related, don't worry.

Understood, thanks! Anything else I should be doing (eg reaching out to people some channel)? I didn't find any discord or similar in the contribution guide.

[christophfroehlich]

There is no need to tag anyone, we will come back when we will have time. You always can help in reviewing one of the many other open PRs ;)

[saikishor]

Rest LGTM. Just one minor nit in the tests

[saikishor]

LGTM.

P.S. Do not force push the PRs again, this makes the maintainers to review all the codebase again. Moreover, we will squash the commits

[vectorcrumb]

Got it, I'll refrain from doing that. What if I want to rebase on top of master though? Do you need anything else from my side to get this merged? Thanks for the feedback!

[saikishor]

Got it, I'll refrain from doing that. What if I want to rebase on top of master though? Do you need anything else from my side to get this merged? Thanks for the feedback!

You can just sync those commits using the GitHub UI from your fork or from the PR

[christophfroehlich]

Got it, I'll refrain from doing that. What if I want to rebase on top of master though? Do you need anything else from my side to get this merged? Thanks for the feedback!

You can just sync those commits using the GitHub UI from your fork or from the PR

• using merge strategy instead of rebase

[christophfroehlich]

Please fix the failing pre-commit job, I suggest to install pre-commit permanently in this repo.