Fix some more clang compiler warnings

[rhaschke]

This depends on ament/googletest#21

[codecov]

Codecov Report

Base: 50.42% // Head: 50.44% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (0dd9670) compared to base (4765028).
Patch coverage: 79.17% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1712      +/-   ##
==========================================
+ Coverage   50.42%   50.44%   +0.03%     
==========================================
  Files         374      374              
  Lines       31335    31328       -7     
==========================================
+ Hits        15797    15801       +4     
+ Misses      15538    15527      -11     

Impacted Files	Coverage Δ
.../collision_detection/test_collision_common_panda.h	98.73% <ø> (ø)
...it/collision_detection/test_collision_common_pr2.h	100.00% <ø> (ø)
moveit_core/robot_state/src/robot_state.cpp	47.91% <0.00%> (-0.03%)	⬇️
...ustrial_motion_planner_testutils/testdata_loader.h	100.00% <ø> (ø)
...s/include/moveit_setup_controllers/controllers.hpp	8.00% <0.00%> (-0.33%)	⬇️
...ramework/include/moveit_setup_framework/config.hpp	32.00% <50.00%> (-1.33%)	⬇️
...pilz_industrial_motion_planner_testutils/basecmd.h	90.48% <75.00%> (-4.26%)	⬇️
...lude/moveit/collision_detection/collision_matrix.h	100.00% <100.00%> (ø)
...veit/trajectory_processing/time_parameterization.h	100.00% <100.00%> (ø)
...strial_motion_planner/src/trajectory_generator.cpp	96.74% <100.00%> (ø)
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[rhaschke]

After a full day of work, all compiler warnings in the moveit2 repo are fixed again. I don't know why we have such an elaborated CI framework if error-checking gets disabled (or downgraded to warnings) as soon as they provide hurdles.
We should really trust the compilers and fix those warnings.

[tylerjw]

Thank you for this change. I don't remember warnings being disabled in CI; I didn't know they were. I'm sorry I didn't see that before.

I was the one who disabled the python test that you re-enabled yesterday. Thank you for that; I didn't how to deal with that one. It looks like it was as simple as removing that import as it was unused.

[tylerjw]

Would you like to merge this now or wait on that pull request to update google test? I think merging now would be nice, but I'm unsure what effect that has on people who build locally.

[ChrisThrasher]

I like a lot about this. My only feedback is about the addition of destructors and how that implicitly deletes move semantics. This won't break user code per se but it can have performance impacts by converting some moves into copies. Do we care enough to = default the move operators as well?

[rhaschke]

Would you like to merge this now or wait on that pull request to update google test?

I suggest merging now to reenable stricter CI. The newer google test doesn't pose a problem locally. Your build log is just more cluttered with warnings (and you can't use -Werror). But that's the current status quo anyway.

[henningkayser]

Why haven't we merged this yet? The only hold-back I see is that we might want to add a note to some documentation to locally switch to gtest1.11, but otherwise I agree with @rhaschke that we should not have to wait for the ament fix. Also, we should backport this to humble, right?