Renovate load controller tests

[bmagyar]

Should fix ros-controls/ros2_control#825

[codecov-commenter]

Codecov Report

Merging #569 (16e73cf) into master (e7f9962) will decrease coverage by 3.36%.
The diff coverage is 26.44%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #569      +/-   ##
==========================================
- Coverage   35.78%   32.43%   -3.36%     
==========================================
  Files         189        7     -182     
  Lines       17570      666   -16904     
  Branches    11592      358   -11234     
==========================================
- Hits         6287      216    -6071     
+ Misses        994      157     -837     
+ Partials    10289      293    -9996     

Flag	Coverage Δ
unittests	32.43% <26.44%> (-3.36%)	⬇️

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

Impacted Files	Coverage Δ
...ontroller/test/test_load_diff_drive_controller.cpp	11.11% <0.00%> (ø)
diff_drive_controller/src/odometry.cpp	42.16% <11.11%> (ø)
diff_drive_controller/src/speed_limiter.cpp	46.55% <11.11%> (ø)
...ive_controller/test/test_diff_drive_controller.cpp	17.62% <12.08%> (ø)
...troller/include/diff_drive_controller/odometry.hpp	20.00% <20.00%> (ø)
...iff_drive_controller/src/diff_drive_controller.cpp	39.22% <35.50%> (ø)
...de/diff_drive_controller/diff_drive_controller.hpp	100.00% <100.00%> (ø)

... and 189 files with indirect coverage changes

[bmagyar]

So what this test renovation uncovered is that the load admittance controller was never going to work with this setup or has been broken for quite a while.

• A bunch of params that need to be set are not set in the config. It's funny that the config for the loading test even says # contains minimal parameters that need to be set to load controller :D
• Once using the full test_admittance_controller config for the load test, things still fail as it is now missing the robot description. Is it really necessary to require all these things at load time ? I mean the robot description is an easy ask but grabbing parameters etc are meant to be in configure, not load.

[destogl]

Once using the full test_admittance_controller config for the load test, things still fail as it is now missing the robot description. Is it really necessary to require all these things at load time ?

Yes, yes and yes, we have caught this few weeks ago, and it should be solved herein #547.

Isn't it?

In #560 @guihomework was updating tests and afaik with fix from #547 things should be fine.

[destogl]

To clarify the process.

1. Parameter structures are created in on_init.
2. Parameters are read / updated in on_configure.

What here may be confusing (but I am not sure about it). generate_parameter_library is potentially expecting all parameters to be available when parameter object is created. I think that then all internal parameters checks are done. Maybe we have to fix this logic there and do parameters check when we update them.

[bmagyar]

#547 was merged a week ago and my branch is up to date. It did not fix the issue, maybe only the first part.

[bmagyar]

The gen params lib issues can easily be fixed by using the non-minimal config for the load test. I didn't have time to investigate the missing robot_description error.

…

On Thu, 13 Apr 2023, 13:38 Dr. Denis, ***@***.***> wrote: To clarify the process. 1. Parameter structures are created in on_init. 2. Parameters are read / updated in on_configure. What here may be confusing (but I am not sure about it). generate_parameter_library is potentially expecting all parameters to be available when parameter object is created. I think that then all internal parameters checks are done. Maybe we have to fix this logic there and do parameters check when we update them. — Reply to this email directly, view it on GitHub <#569 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA24PYLOAIPYYXKCMSCLTCTXA7XVZANCNFSM6AAAAAAW4HQ3QY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[gwalck]

To clarify the process.

1. Parameter structures are created in `on_init`.

2. Parameters are read / updated in `on_configure`.

What here may be confusing (but I am not sure about it). generate_parameter_library is potentially expecting all parameters to be available when parameter object is created. I think that then all internal parameters checks are done. Maybe we have to fix this logic there and do parameters check when we update them.

Hi, I am @guihomework with a new account. While working on a new controller generation using RosTeamWorkspace, I faced the issue of a controller not passing the renovated load test due to a mandatory parameter missing.

My opinion on the problem is that the previous ASSERT_NO_THROW did not catch the param loading issues, but it still asserted that the plugin was found and could be loaded, otherwise there is a throw (I just faced pluginlib not finding the lib due a badly set xml lib path). The missing param generates an exception that is caught in the on_init which then fails with a nullptr but does not throw.

I don't think the load test should require the minimum mandatory parameters and additionally succeed the on_init call, since the loading of the plugin is what we want to test there, and on_init is checked in a second test that has the mandatory parameters.

[christophfroehlich]

@bmagyar should this be backported also to humble? #670 has conflicts because of this PR missing in humble.

[bmagyar]

let's try!

[bmagyar]

@Mergifyio backport humble

[mergify]

backport humble

✅ Backports have been created

• #677 Renovate load controller tests (backport #569) has been created for branch humble