Rework PID class API

[christophfroehlich]

While fixing the clang errors I realized that lots of implicit casts to computeCommand were used, and the typical usage was to pass something like period.nanoseconds() and convert it internally to seconds. Because introducing an overload with double dt as argument is ambiguous and might silently break user code, I decided to convert the class methods from camelCase to snake_case (the ros2_control standard), remove the uint64_t version but introduce the following new overloads which converts dt to seconds in a consistent way

• double dt
• rcl_duration_value_t dt_ns
• rclcpp::Duration dt
• std::chrono::nanoseconds dt_ns

I converted the method names of PidROS accordingly as well, but left the only rclcpp::Duration version there.

The old methods remain deprecated, this should not break anything in downstream packages.
AFAIK adding non-virtual methods should not be a problem regarding ABI break?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 92.85714% with 13 lines in your changes missing coverage. Please review.

Project coverage is 75.87%. Comparing base (74510fd) to head (3d3bb50).

Files with missing lines	Patch %	Lines
src/pid_ros.cpp	80.35%	11 Missing ⚠️
src/pid.cpp	93.75%	2 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           ros2-master     #246      +/-   ##
===============================================
+ Coverage        74.78%   75.87%   +1.08%     
===============================================
  Files               22       22              
  Lines             1071     1119      +48     
  Branches            85       85              
===============================================
+ Hits               801      849      +48     
  Misses             222      222              
  Partials            48       48              

Flag	Coverage Δ
unittests	75.87% <92.85%> (+1.08%)	⬆️

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

Files with missing lines	Coverage Δ
include/control_toolbox/pid.hpp	86.66% <100.00%> (+8.88%)	⬆️
include/control_toolbox/pid_ros.hpp	100.00% <ø> (ø)
test/pid_parameters_tests.cpp	100.00% <100.00%> (ø)
test/pid_publisher_tests.cpp	94.87% <100.00%> (ø)
test/pid_tests.cpp	100.00% <100.00%> (ø)
src/pid.cpp	93.67% <93.75%> (+1.02%)	⬆️
src/pid_ros.cpp	72.41% <80.35%> (ø)

[mergify]

This pull request is in conflict. Could you fix it @christophfroehlich?

[saikishor]

In general looks great to me.

Compatibility tests look spot on. Super good work.

Just a few things, i thought that might be worth discussing.

[saikishor]

LGTM.

Thanks for the changes

[saikishor]

Rest looks good