Export pid gains as reference

[kumar-sanjeeev]

This PR includes the following changes:

• Added a new parameter export_gain_references to enable or disable the export of gains (p, i, d) as references.
• Updated the update_and_write_commands function based on the value of export_gain_references.
• Updated test cases for both scenarios i.e export_gain_references=true and export_gain_references=false
• Updated the documentation to include the option for exporting PID gains as a reference.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 68.25397% with 20 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 85.06%. Comparing base (ec198b1) to head (7c836d3). :warning: Report is 15 commits behind head on master.

Files with missing lines	Patch %	Lines
pid_controller/src/pid_controller.cpp	56.81%	15 Missing and 4 partials :warning:
pid_controller/test/test_pid_controller.cpp	93.75%	0 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2020      +/-   ##
==========================================
- Coverage   85.13%   85.06%   -0.07%     
==========================================
  Files         144      144              
  Lines       13968    14021      +53     
  Branches     1201     1211      +10     
==========================================
+ Hits        11891    11927      +36     
- Misses       1670     1685      +15     
- Partials      407      409       +2     

Flag	Coverage Δ
unittests	85.06% <68.25%> (-0.07%)	:arrow_down:

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

Files with missing lines	Coverage Δ
pid_controller/test/test_pid_controller.hpp	87.50% <ø> (ø)
..._controller/test/test_pid_controller_preceding.cpp	100.00% <100.00%> (ø)
pid_controller/test/test_pid_controller.cpp	99.45% <93.75%> (-0.55%)	:arrow_down:
pid_controller/src/pid_controller.cpp	66.42% <56.81%> (-1.50%)	:arrow_down:

:rocket: New features to boost your workflow:

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

[kumar-sanjeeev]

In principle, this looks fine for me.

understood. I am trying to understand one thing, in my current implementation, when I set export_gain_references=true, the reference_interfaces_ vector allocates additional memory for the PID gains for each DOF, as expected:

  if (params_.export_gain_references)
  {
    total_reference_size += dof_ * GAIN_INTERFACES.size();
  }
  reference_interfaces_.resize(total_reference_size, std::numeric_limits<double>::quiet_NaN());

Because of this, some of the existing tests are failing. If this implementation is correct, should I update the tests so that they handle both cases (export_gain_references=true and export_gain_references=false)? Or is there something else I might be missing? Or is there any other recommended way to cater this ?

Test config that I used to reproduce the failure case, along with the corresponding log:

test_pid_controller:
  ros__parameters:
    dof_names:
      - joint1

    command_interface: position

    reference_and_state_interfaces: ["position"]

    export_gain_references: true

    gains:
      joint1: {p: 1.0, i: 2.0, d: 3.0, u_clamp_max: 5.0, u_clamp_min: -5.0}

/workspaces/ros2_control_rolling_dev_docker_ws/src/ros-controls/ros2_controllers/pid_controller/test/test_pid_controller.cpp:88: Failure
Expected equality of these values:
  ref_if_conf.size()
    Which is: 4
  dof_state_values_.size()
    Which is: 1

[  FAILED  ] PidControllerTest.check_exported_interfaces (6 ms)

[thedevmystic]

I also think implementation is correct @kumar-sanjeeev, But if implementation is correct then, why are we failing tests?

Overall, I think some tests need to be updated, but this is just my assumption.

[kumar-sanjeeev]

hi @christophfroehlich, I have tried few different ways to fix the ABI check, but still haven’t succeeded. Do you have any recommendations? Or is it okay in this feature request, if it breaks ?

[thedevmystic]

Phenomenal work, @kumar-sanjeeev. And I'm very glad you completed this, and now this is ready for a review. Good luck for ahead!

[kumar-sanjeeev]

Hi @christophfroehlich,

The implementation is complete from my side. Whenever you have time to review it, please let me know if there is anything I may have missed or should improve.