dynamically link JTC parameter validation instead of header only

[thedevmystic]

Hello respected maintainers and reviewers! Again this is Surya!

This PR addresses,

• Seperate validate_jtc_parameters into hpp and cpp.
• Added doxygen-style comments.

When I was refactoring tolerances.hpp I got linker error of multiple definitions of this header file. Because it defines functions directly in header without any inline, so I seperated them in .hpp and .cpp

I also added some doxygen-style comments for documentation of these functions.

Regards,
Surya

[codecov]

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.78%. Comparing base (9817475) to head (40bfcf9).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...jectory_controller/src/validate_jtc_parameters.cpp	88.23%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2127      +/-   ##
==========================================
+ Coverage   84.76%   84.78%   +0.01%     
==========================================
  Files         151      151              
  Lines       14619    14629      +10     
  Branches     1269     1269              
==========================================
+ Hits        12392    12403      +11     
+ Misses       1767     1766       -1     
  Partials      460      460              

Flag	Coverage Δ
unittests	84.78% <88.23%> (+0.01%)	⬆️

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

Files with missing lines	Coverage Δ
...jectory_controller/src/validate_jtc_parameters.cpp	88.23% <88.23%> (ø)

🚀 New features to boost your workflow:

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

[bmagyar]

thank you!

[bmagyar]

@christophfroehlich I've removed the backport labels as this may cause issues to people inheriting from the JTC. Happy to discuss though

[christophfroehlich]

I am not sure about this here, as the joint_trajectory_controller_parameters library will be broken now.
People might have been able to use the parameter library without linking against the joint_trajectory_controller, which is now not possible.
Sorry, haven't had a proper look before.

[christophfroehlich]

When I was refactoring tolerances.hpp I got linker error of multiple definitions of this header file. Because it defines functions directly in header without any inline, so I seperated them in .hpp and .cpp

I don't understand why. This means that the generate_parameter_library does not have proper include guards, because we don't include the validation file anywhere else?

[thedevmystic]

When I was refactoring tolerances.hpp I got linker error of multiple definitions of this header file. Because it defines functions directly in header without any inline, so I seperated them in .hpp and .cpp

I don't understand why. This means that the generate_parameter_library does not have proper include guards, because we don't include the validation file anywhere else?

I think you didn't get me, what I said. I meant I'm reorganizing, rewriting and adding something to make the tolerances.hpp faster to compile and better. That's why I got linker error. As validate_jtc_parameters.hpp defines 2 functions without inline this violated ODR (I'm adding tolerances.cpp for better compilation speed and decoupling from .hpp).

[thedevmystic]

So, tolerances.hpp also includes JTC Params header file and joint_trajectory_controller.hpp also includes it. So, viola! We have multiple definitions in 2 .cpp (if we add my Refactored tolerances.cpp).

So, you are 100% correct that:

we don't include the validation file anywhere else?

And I'll say Yet.

[christophfroehlich]

• Idk why we would use the validate_jtc_parameter.hpp file elsewhere?
• We have included joint_trajectory_controller_parameters.hpp twice already but in the same compilation unit, what will you change there?

[thedevmystic]

I'm about to initiate that PR in like a hour or so. So, I think instead of explaining here, just see it!

Edit: PR opened as #2133.