From 7d8f74526427e5494e206c157fbec2ed89b90712 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20Fr=C3=B6hlich?=
 <christophfroehlich@users.noreply.github.com>
Date: Thu, 18 Jul 2024 10:48:57 +0200
Subject: [PATCH] PID: Improve the API docs and change default value of
 antiwindup (#202)

* Improve the API docs

* Set antiwindup to true in this constructor specialization

---------

Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>
---
 include/control_toolbox/pid.hpp | 47 ++++++++++++++++++++++++++++-----
 test/pid_tests.cpp              |  2 +-
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/include/control_toolbox/pid.hpp b/include/control_toolbox/pid.hpp
index 13f8553a..877edb9f 100644
--- a/include/control_toolbox/pid.hpp
+++ b/include/control_toolbox/pid.hpp
@@ -113,12 +113,34 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    */
   struct Gains
   {
-    // Optional constructor for passing in values without antiwindup
+  /*!
+   * \brief Optional constructor for passing in values without antiwindup
+   *
+   * \param p The proportional gain.
+   * \param i The integral gain.
+   * \param d The derivative gain.
+   * \param i_max The max integral windup.
+   * \param i_min The min integral windup.
+   *
+   * \throws An std::invalid_argument exception is thrown if i_min > i_max
+   */
     Gains(double p, double i, double d, double i_max, double i_min)
-    : p_gain_(p), i_gain_(i), d_gain_(d), i_max_(i_max), i_min_(i_min), antiwindup_(false)
+    : p_gain_(p), i_gain_(i), d_gain_(d), i_max_(i_max), i_min_(i_min), antiwindup_(true)
     {
     }
-    // Optional constructor for passing in values
+
+  /*!
+   * \brief Optional constructor for passing in values
+   *
+   * \param p The proportional gain.
+   * \param i The integral gain.
+   * \param d The derivative gain.
+   * \param i_max The max integral windup.
+   * \param i_min The min integral windup.
+   * \param antiwindup If true, antiwindup is enabled and i_max/i_min are enforced
+   *
+   * \throws An std::invalid_argument exception is thrown if i_min > i_max
+   */
     Gains(double p, double i, double d, double i_max, double i_min, bool antiwindup)
     : p_gain_(p), i_gain_(i), d_gain_(d), i_max_(i_max), i_min_(i_min), antiwindup_(antiwindup)
     {
@@ -145,9 +167,9 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    * \param d The derivative gain.
    * \param i_max The max integral windup.
    * \param i_min The min integral windup.
+   * \param antiwindup If true, antiwindup is enabled and i_max/i_min are enforced
    *
-   * \note
-   *  An std::invalid_argument exception is thrown if i_min > i_max
+   * \throws An std::invalid_argument exception is thrown if i_min > i_max
    */
   Pid(
     double p = 0.0, double i = 0.0, double d = 0.0, double i_max = 0.0, double i_min = -0.0,
@@ -173,6 +195,7 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    * \param d The derivative gain.
    * \param i_max The max integral windup.
    * \param i_min The min integral windup.
+   * \param antiwindup If true, antiwindup is enabled and i_max/i_min are enforced
    *
    * \note New gains are not applied if i_min_ > i_max_
    */
@@ -192,6 +215,15 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    * \param i_min The min integral windup.
    */
   void getGains(double & p, double & i, double & d, double & i_max, double & i_min);
+  /*!
+   * \brief Get PID gains for the controller.
+   * \param p The proportional gain.
+   * \param i The integral gain.
+   * \param d The derivative gain.
+   * \param i_max The max integral windup.
+   * \param i_min The min integral windup.
+   * \param antiwindup If true, antiwindup is enabled and i_max/i_min are enforced
+   */
   void getGains(
     double & p, double & i, double & d, double & i_max, double & i_min, bool & antiwindup);
 
@@ -208,6 +240,7 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    * \param d The derivative gain.
    * \param i_max The max integral windup.
    * \param i_min The min integral windup.
+   * \param antiwindup If true, antiwindup is enabled and i_max/i_min are enforced
    *
    * \note New gains are not applied if i_min > i_max
    */
@@ -231,7 +264,7 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    *
    * \returns PID command
    */
-  double computeCommand(double error, uint64_t dt);
+  [[nodiscard]] double computeCommand(double error, uint64_t dt);
 
   /*!
    * \brief Set the PID error and compute the PID command with nonuniform
@@ -244,7 +277,7 @@ class CONTROL_TOOLBOX_PUBLIC Pid
    *
    * \returns PID command
    */
-  double computeCommand(double error, double error_dot, uint64_t dt);
+  [[nodiscard]] double computeCommand(double error, double error_dot, uint64_t dt);
 
   /*!
    * \brief Set current command for this PID controller
diff --git a/test/pid_tests.cpp b/test/pid_tests.cpp
index b8ef5635..22e3e3af 100644
--- a/test/pid_tests.cpp
+++ b/test/pid_tests.cpp
@@ -223,7 +223,7 @@ TEST(ParameterTest, gainSettingCopyPIDTest)
 
   // Send update command to populate errors -------------------------------------------------
   pid1.setCurrentCmd(10);
-  pid1.computeCommand(20, 1.0 * 1e9);
+  (void) pid1.computeCommand(20, 1.0 * 1e9);
 
   // Test copy constructor -------------------------------------------------
   Pid pid2(pid1);