From edd3b8689ba48a7d9c12d56ee344b3a79ee15769 Mon Sep 17 00:00:00 2001
From: paul Jewell <paul@teulu.org>
Date: Sat, 10 Dec 2016 12:43:50 +0100
Subject: [PATCH] LP-289 Further clean up of PIDControlDown::UpdateParameters()

---
 flight/libraries/pid/pidcontroldown.cpp | 31 ++++++++++++-------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/flight/libraries/pid/pidcontroldown.cpp b/flight/libraries/pid/pidcontroldown.cpp
index ee8f33d60..253eaa682 100644
--- a/flight/libraries/pid/pidcontroldown.cpp
+++ b/flight/libraries/pid/pidcontroldown.cpp
@@ -7,7 +7,8 @@
  * @{
  *
  * @file       pidcontroldown.h
- * @author     The OpenPilot Team, http://www.openpilot.org Copyright (C) 2015.
+ * @author     The LibrePilot Project, http://www.librepilot.org Copyright (C) 2016.
+ *             The OpenPilot Team, http://www.openpilot.org Copyright (C) 2015.
  * @brief      Executes PID control for down direction
  *
  * @see        The GNU Public License (GPL) Version 3
@@ -90,13 +91,13 @@ void PIDControlDown::Activate()
 
 void PIDControlDown::UpdateParameters(float kp, float ki, float kd, float beta, float dT, float velocityMax)
 {
-    // pid_configure(&PID, kp, ki, kd, ilimit);
-    float Td;
-    float Ti;
-    float Tt;
-    float kt;
-    float N  = 10.0f;
-    float Tf;
+    float Td; // Derivative time constant
+    float Ti; // Integral time constant
+    float kt; // Feedback gain for integral windup avoidance
+    float N = 10.0f; // N is the factor used to determine the
+                     // time constant for derivative filtering
+                     // Why 10? Maybe should be configurable?
+    float Tf; // Low pass filter time constant for derivative filtering
 
     // Define Td, handling zero kp term (for I or ID controller)
     if (kp < 1e-6f) {
@@ -107,23 +108,21 @@ void PIDControlDown::UpdateParameters(float kp, float ki, float kd, float beta,
 
     // Define Ti, Tt and kt, handling zero ki term (for P or PD controller)
     if (ki < 1e-6f) { // Avoid Ti being infinite
-        Ti = 1e6f;
-        // Tt antiwindup time constant - we don't need antiwindup with no I term
-        Tt = 1e6f;
         kt = 0.0f;
     } else {
         Ti = kp / ki;
-        Tt = (Ti + Td) / 2.0f;
-        kt = 1.0f / Tt;
+        kt = 1.0f / Ti;
     }
-    // Define Tf, according to controller type 
+
+    // Define Tf, according to controller type
     if (kd < 1e-6f) {
         // PI Controller or P Controller
-        Tf = Ti / N;
+        Tf = 0; // Originally this: (Ti / N) - which creates a D term from
+                // the integral time constant!
     } else {
         Tf = Td / N;
     }
-    
+
     if (beta > 1.0f) {
         beta = 1.0f;
     } else if (beta < 0.4f) {