Update documentation of PID class

[ViktorCVS]

Documentation PR post-PR #298 merge

[ViktorCVS]

@christophfroehlich I’ve created a PR to update the documentation, but I’m not sure if there are other repositories or files I need to update. Can you help me with that?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.38%. Comparing base (b2b3425) to head (fd7d000).

Additional details and impacted files

@@             Coverage Diff              @@
##           ros2-master     #388   +/-   ##
============================================
  Coverage        78.38%   78.38%           
============================================
  Files               30       30           
  Lines             1888     1888           
  Branches           114      114           
============================================
  Hits              1480     1480           
  Misses             338      338           
  Partials            70       70           

Flag	Coverage Δ
unittests	78.38% <ø> (ø)

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

Files with missing lines	Coverage Δ
control_toolbox/include/control_toolbox/pid.hpp	66.98% <ø> (ø)

🚀 New features to boost your workflow:

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

[christophfroehlich]

@christophfroehlich I’ve created a PR to update the documentation, but I’m not sure if there are other repositories or files I need to update. Can you help me with that?

I suggest that you add the documentation here
https://github.com/ros-controls/control_toolbox/blob/ros2-master/doc/control_toolbox.md
which will be published here
https://control.ros.org/rolling/doc/control_toolbox/doc/index.html

Add a new section for the PID, copy basis information from the docstring and add your antiwindup description from the other PR there.

[ViktorCVS]

@christophfroehlich I’ve created a PR to update the documentation, but I’m not sure if there are other repositories or files I need to update. Can you help me with that?

I suggest that you add the documentation here https://github.com/ros-controls/control_toolbox/blob/ros2-master/doc/control_toolbox.md which will be published here https://control.ros.org/rolling/doc/control_toolbox/doc/index.html

Add a new section for the PID, copy basis information from the docstring and add your antiwindup description from the other PR there.

Done! I changed it from draft to open. If there is still documentation to add, I’ll add more commits.

[christophfroehlich]

Thanks for this addition. Some details got outdated, comments see below.

please update also the docstring of the Pid Class in pid.hpp accordingly (remove i_clamp etc).

control_toolbox/control_toolbox/include/control_toolbox/pid.hpp

Line 205 in b2b3425

/*! \class Pid

And could you please also add the references to literature?

[christophfroehlich]

Suggested change

	* The parameters :paramref:`antiwindup`, :paramref:`i_clamp_max`, and :paramref:`i_clamp_min` have been removed. The anti-windup behavior is now configured via the :paramref:`AntiwindupStrategy` enum. (`#298 <https://github.com/ros-controls/control_toolbox/pull/298>`_).
	* The parameters :paramref:`antiwindup`, :paramref:`i_clamp_max`, and :paramref:`i_clamp_min` have been removed. The anti-windup behavior is now configured via the :paramref:`AntiWindupStrategy` enum. (`#298 <https://github.com/ros-controls/control_toolbox/pull/298>`_).

[ViktorCVS]

Thanks! Solved in commit eaf1721

[ViktorCVS]

Thanks for this addition. Some details got outdated, comments see below.

please update also the docstring of the Pid Class in pid.hpp accordingly (remove i_clamp etc).

control_toolbox/control_toolbox/include/control_toolbox/pid.hpp

Line 205 in b2b3425

/*! \class Pid

And could you please also add the references to literature?

Done!

[christophfroehlich]

Thanks a lot. Next time, please use the github UI to apply suggested changes if you fully agree, this makes the re-review faster for us ;)