commit ca791156e2594ce7d25b42793a0a1b60d922fefd
author Josh Lehan <krellan@google.com> Sun Sep 20 23:41:39 2020 -0700
committer Patrick Venture <venture@google.com> Fri Oct 09 16:19:02 2020 +0000
tree a37e30b44c2ad581e4026b8be2d1e3eec695c1bc
parent ffa15f6e5706ff434468315d58c359c145f0aecd

FanController/ThermalController: Clean up PID input math

Adding checking against floating-point oddities,
such as NAN, +INF, or -INF, appearing as input,
which would mess up the PID loop math,
causing corrupt output.

If a fan or thermal input value is NAN, +INF, or -INF,
that value will be omitted from contributing to PID loop input.
If all values were omitted for that PID loop,
existing code already hardcodes a value of 0 for fan,
and I added similar code to also hardcode 0 for thermal.

It makes sense to use a placeholder value of 0 degrees of margin,
because this will make the fans spin fast,
if for some reason the zone is not already in failsafe mode by now.
Note that negative values are not allowed for fan,
but they are allowed for thermal.

Tested: Works for me, and PID loops no longer output garbage
when debugging a sensor malfunction while in tuning mode,
making tuning mode much more usable. If not in tuning mode,
the normal failsafe feature would have kicked in,
masking this garbage from appearing in the output anyway.

Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: I7aee812ebaeff209f84cef0db28973696f782ef9

pid/fancontroller.cpp

diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp
index 41f24da..d3c58ca 100644
--- a/pid/fancontroller.cpp
+++ b/pid/fancontroller.cpp
@@ -21,6 +21,7 @@
 #include "zone.hpp"
 
 #include <algorithm>
+#include <cmath>
 #include <iostream>
 
 namespace pid_control
@@ -62,10 +63,16 @@
              * sort of have to guess -- all the other fans are reporting, why
              * not this one?  Maybe it's unable to be read, so it's "bad."
              */
-            if (value > 0)
+            if (!(std::isfinite(value)))
             {
-                values.push_back(value);
+                continue;
             }
+            if (value <= 0)
+            {
+                continue;
+            }
+
+            values.push_back(value);
         }
     }
     catch (const std::exception& e)

pid/thermalcontroller.cpp

diff --git a/pid/thermalcontroller.cpp b/pid/thermalcontroller.cpp
index 35dcd15..8125349 100644
--- a/pid/thermalcontroller.cpp
+++ b/pid/thermalcontroller.cpp
@@ -21,6 +21,8 @@
 #include "zone.hpp"
 
 #include <algorithm>
+#include <cmath>
+#include <iostream>
 
 namespace pid_control
 {
@@ -75,9 +77,25 @@
         compare = std::max<double>;
     }
 
+    bool acceptable = false;
     for (const auto& in : _inputs)
     {
-        value = compare(value, _owner->getCachedValue(in));
+        double cachedValue = _owner->getCachedValue(in);
+
+        // Less than 0 is perfectly OK for temperature, but must not be NAN
+        if (!(std::isfinite(cachedValue)))
+        {
+            continue;
+        }
+
+        value = compare(value, cachedValue);
+        acceptable = true;
+    }
+
+    if (!acceptable)
+    {
+        // While not optimal, zero is better than garbage
+        value = 0;
     }
 
     return value;