Fix: on_shutdown callback of controllers never get executed

[Wiktor-99]

As discussed with @christophfroehlich, I added a shutdown call instead of calling cleanup.

[saikishor]

@Wiktor-99 Looks good. Better not to add a new method only for this, just change it within the unload_controller method itself.

Can you add some tests?

Thank you

[Wiktor-99]

@saikishor Why method shouldn't be added? Currently the unload_controller method is really long, it might improve readability.

About the test I see that some of them are failing, so fixing them will also cover new code.

[saikishor]

@Wiktor-99 Sounds fair. Let's leave it like that.

[saikishor]

About the test I see that some of them are failing, so fixing them will also cover new code.

Thank you

[saikishor]

I think it is missing a parenthesis

[codecov]

Codecov Report

Attention: Patch coverage is 63.63636% with 8 lines in your changes missing coverage. Please review.

Project coverage is 89.27%. Comparing base (f4c9dd0) to head (6366451).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
controller_manager/src/controller_manager.cpp	50.00%	5 Missing and 1 partial ⚠️
...ith_interfaces/test_controller_with_interfaces.cpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1995      +/-   ##
==========================================
- Coverage   89.32%   89.27%   -0.05%     
==========================================
  Files         130      130              
  Lines       14522    14533      +11     
  Branches     1258     1257       -1     
==========================================
+ Hits        12972    12975       +3     
- Misses       1083     1092       +9     
+ Partials      467      466       -1     

Flag	Coverage Δ
unittests	89.27% <63.63%> (-0.05%)	⬇️

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

Files with missing lines	Coverage Δ
.../include/controller_manager/controller_manager.hpp	100.00% <ø> (ø)
...r_manager/test/test_controller/test_controller.cpp	88.73% <100.00%> (+0.67%)	⬆️
...r_manager/test/test_controller/test_controller.hpp	100.00% <ø> (ø)
...ontroller_manager/test/test_controller_manager.cpp	96.10% <ø> (ø)
...ller_manager/test/test_controller_manager_srvs.cpp	99.26% <100.00%> (ø)
...ith_interfaces/test_controller_with_interfaces.hpp	100.00% <ø> (ø)
...ith_interfaces/test_controller_with_interfaces.cpp	66.66% <0.00%> (-13.34%)	⬇️
controller_manager/src/controller_manager.cpp	76.35% <50.00%> (+0.13%)	⬆️

... and 3 files with indirect coverage changes

[christophfroehlich]

LGTM

[saikishor]

LGTM

[christophfroehlich]

@bmagyar should we backport this to humble? The current behavior is wrong, but it might crash (badly configured) systems.