commit 734f953519fbb21a277313152399971744df158f
author James Feist <james.feist@linux.intel.com> Thu Nov 15 12:13:18 2018 -0800
committer James Feist <james.feist@linux.intel.com> Thu Nov 15 22:51:42 2018 +0000
tree eff44cd74de69925808666cef515c391cfb75312
parent 4a0c0611ea872efffc4dc6fc8c8b57e87fa3605d

Allow multiple inputs to thermal and stepwise controllers

Use std::max to determine which input value to apply.
Also start throwing when inputs are empty as otherwise
there will be a nullptr dereference.

Tested-by: Added multiple inputs and application no longer
segfaults and verifed max was being used. Also added unit
tests.

Change-Id: I7c8eda45b99247b8e92e629f036c9a46c98d9fe2
Signed-off-by: James Feist <james.feist@linux.intel.com>

pid/builder.cpp

diff --git a/pid/builder.cpp b/pid/builder.cpp
index 55f3e35..261a165 100644
--- a/pid/builder.cpp
+++ b/pid/builder.cpp
@@ -102,8 +102,19 @@
                     zone->addThermalInput(i);
                 }
 
+                ThermalType type;
+                if (info->type == "temp")
+                {
+                    type = ThermalType::absolute;
+                }
+                else
+                {
+                    type = ThermalType::margin;
+                }
+
                 auto pid = ThermalController::createThermalPid(
-                    zone.get(), name, inputs, info->setpoint, info->pidInfo);
+                    zone.get(), name, inputs, info->setpoint, info->pidInfo,
+                    type);
 
                 zone->addThermalPID(std::move(pid));
             }

pid/stepwisecontroller.cpp

diff --git a/pid/stepwisecontroller.cpp b/pid/stepwisecontroller.cpp
index b81f8b1..43fd241 100644
--- a/pid/stepwisecontroller.cpp
+++ b/pid/stepwisecontroller.cpp
@@ -17,6 +17,7 @@
 #include "stepwisecontroller.hpp"
 
 #include "ec/stepwise.hpp"
+#include "errors/exception.hpp"
 #include "util.hpp"
 #include "zone.hpp"
 
@@ -66,9 +67,10 @@
     ZoneInterface* owner, const std::string& id,
     const std::vector<std::string>& inputs, const ec::StepwiseInfo& initial)
 {
-    // StepwiseController currently only supports precisely one input.
-    if (inputs.size() != 1)
+    // StepwiseController requires at least 1 input
+    if (inputs.empty())
     {
+        throw ControllerBuildException("Stepwise controller missing inputs");
         return nullptr;
     }
 
@@ -83,7 +85,11 @@
 
 double StepwiseController::inputProc(void)
 {
-    double value = _owner->getCachedValue(_inputs.at(0));
+    double value = std::numeric_limits<double>::lowest();
+    for (const auto& in : _inputs)
+    {
+        value = std::max(value, _owner->getCachedValue(in));
+    }
     return value;
 }
 

pid/thermalcontroller.cpp

diff --git a/pid/thermalcontroller.cpp b/pid/thermalcontroller.cpp
index 485688a..ff2c7bc 100644
--- a/pid/thermalcontroller.cpp
+++ b/pid/thermalcontroller.cpp
@@ -16,21 +16,23 @@
 
 #include "thermalcontroller.hpp"
 
+#include "errors/exception.hpp"
 #include "util.hpp"
 #include "zone.hpp"
 
 std::unique_ptr<PIDController> ThermalController::createThermalPid(
     ZoneInterface* owner, const std::string& id,
     const std::vector<std::string>& inputs, double setpoint,
-    const ec::pidinfo& initial)
+    const ec::pidinfo& initial, const ThermalType& type)
 {
-    // ThermalController currently only supports precisely one input.
-    if (inputs.size() != 1)
+    // ThermalController requires at least 1 input
+    if (inputs.empty())
     {
+        throw ControllerBuildException("Thermal controller missing inputs");
         return nullptr;
     }
 
-    auto thermal = std::make_unique<ThermalController>(id, inputs, owner);
+    auto thermal = std::make_unique<ThermalController>(id, inputs, type, owner);
 
     ec::pid_info_t* info = thermal->getPIDInfo();
     thermal->setSetpoint(setpoint);
@@ -43,11 +45,24 @@
 // bmc_host_sensor_value_double
 double ThermalController::inputProc(void)
 {
-    /*
-     * This only supports one thermal input because it doesn't yet know how to
-     * handle merging them, probably max?
-     */
-    double value = _owner->getCachedValue(_inputs.at(0));
+    double value;
+    const double& (*compare)(const double&, const double&);
+    if (type == ThermalType::margin)
+    {
+        value = std::numeric_limits<double>::max();
+        compare = std::min<double>;
+    }
+    else
+    {
+        value = std::numeric_limits<double>::lowest();
+        compare = std::max<double>;
+    }
+
+    for (const auto& in : _inputs)
+    {
+        value = compare(value, _owner->getCachedValue(in));
+    }
+
     return value;
 }
 

pid/thermalcontroller.hpp

diff --git a/pid/thermalcontroller.hpp b/pid/thermalcontroller.hpp
index 8089da1..317219c 100644
--- a/pid/thermalcontroller.hpp
+++ b/pid/thermalcontroller.hpp
@@ -11,19 +11,27 @@
  * A ThermalController is a PID controller that reads a number of sensors and
  * provides the set-points for the fans.
  */
+
+enum class ThermalType
+{
+    margin,
+    absolute
+};
+
 class ThermalController : public PIDController
 {
   public:
     static std::unique_ptr<PIDController>
         createThermalPid(ZoneInterface* owner, const std::string& id,
                          const std::vector<std::string>& inputs,
-                         double setpoint, const ec::pidinfo& initial);
+                         double setpoint, const ec::pidinfo& initial,
+                         const ThermalType& type);
 
     ThermalController(const std::string& id,
                       const std::vector<std::string>& inputs,
-                      ZoneInterface* owner) :
+                      const ThermalType& type, ZoneInterface* owner) :
         PIDController(id, owner),
-        _inputs(inputs)
+        _inputs(inputs), type(type)
     {
     }
 
@@ -33,4 +41,5 @@
 
   private:
     std::vector<std::string> _inputs;
+    ThermalType type;
 };