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>
diff --git a/errors/exception.hpp b/errors/exception.hpp
index 69d63be..c1c789a 100644
--- a/errors/exception.hpp
+++ b/errors/exception.hpp
@@ -18,3 +18,19 @@
private:
std::string message;
};
+
+class ControllerBuildException : public std::exception
+{
+ public:
+ ControllerBuildException(const std::string& message) : message(message)
+ {
+ }
+
+ virtual const char* what() const noexcept override
+ {
+ return message.c_str();
+ }
+
+ private:
+ std::string message;
+};
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));
}
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;
}
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;
}
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;
};
diff --git a/test/pid_thermalcontroller_unittest.cpp b/test/pid_thermalcontroller_unittest.cpp
index 6da309f..804d84a 100644
--- a/test/pid_thermalcontroller_unittest.cpp
+++ b/test/pid_thermalcontroller_unittest.cpp
@@ -23,7 +23,7 @@
ec::pidinfo initial;
std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
- &z, "therm1", inputs, setpoint, initial);
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
// Success
EXPECT_FALSE(p == nullptr);
}
@@ -37,25 +37,13 @@
std::vector<std::string> inputs = {};
double setpoint = 10.0;
ec::pidinfo initial;
-
- std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
- &z, "therm1", inputs, setpoint, initial);
- EXPECT_TRUE(p == nullptr);
-}
-
-TEST(ThermalControllerTest, VerifyFactoryFailsForMoreThanOneInput)
-{
- // ThermalControllers currently only support one input, so don't let
- // someone accidentally specify more.
-
- ZoneMock z;
-
- std::vector<std::string> inputs = {"fleeting0", "asdf"};
- double setpoint = 10.0;
- ec::pidinfo initial;
-
- std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
- &z, "therm1", inputs, setpoint, initial);
+ std::unique_ptr<PIDController> p;
+ EXPECT_THROW(
+ {
+ p = ThermalController::createThermalPid(
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
+ },
+ std::exception);
EXPECT_TRUE(p == nullptr);
}
@@ -70,7 +58,7 @@
ec::pidinfo initial;
std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
- &z, "therm1", inputs, setpoint, initial);
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
EXPECT_FALSE(p == nullptr);
EXPECT_CALL(z, getCachedValue(StrEq("fleeting0"))).WillOnce(Return(5.0));
@@ -89,7 +77,7 @@
ec::pidinfo initial;
std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
- &z, "therm1", inputs, setpoint, initial);
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
EXPECT_FALSE(p == nullptr);
EXPECT_EQ(setpoint, p->setptProc());
@@ -97,7 +85,7 @@
TEST(ThermalControllerTest, OutputProc_BehavesAsExpected)
{
- // This test just verifies inputProc behaves as expected.
+ // This test just verifies outputProc behaves as expected.
ZoneMock z;
@@ -106,7 +94,7 @@
ec::pidinfo initial;
std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
- &z, "therm1", inputs, setpoint, initial);
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
EXPECT_FALSE(p == nullptr);
double value = 90.0;
@@ -114,3 +102,45 @@
p->outputProc(value);
}
+
+TEST(ThermalControllerTest, InputProc_MultipleInputsAbsolute)
+{
+ // This test verifies inputProc behaves as expected with multiple absolute
+ // inputs.
+
+ ZoneMock z;
+
+ std::vector<std::string> inputs = {"fleeting0", "fleeting1"};
+ double setpoint = 10.0;
+ ec::pidinfo initial;
+
+ std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
+ &z, "therm1", inputs, setpoint, initial, ThermalType::absolute);
+ EXPECT_FALSE(p == nullptr);
+
+ EXPECT_CALL(z, getCachedValue(StrEq("fleeting0"))).WillOnce(Return(5.0));
+ EXPECT_CALL(z, getCachedValue(StrEq("fleeting1"))).WillOnce(Return(10.0));
+
+ EXPECT_EQ(10.0, p->inputProc());
+}
+
+TEST(ThermalControllerTest, InputProc_MultipleInputsMargin)
+{
+ // This test verifies inputProc behaves as expected with multiple margin
+ // inputs.
+
+ ZoneMock z;
+
+ std::vector<std::string> inputs = {"fleeting0", "fleeting1"};
+ double setpoint = 10.0;
+ ec::pidinfo initial;
+
+ std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
+ EXPECT_FALSE(p == nullptr);
+
+ EXPECT_CALL(z, getCachedValue(StrEq("fleeting0"))).WillOnce(Return(5.0));
+ EXPECT_CALL(z, getCachedValue(StrEq("fleeting1"))).WillOnce(Return(10.0));
+
+ EXPECT_EQ(5.0, p->inputProc());
+}
\ No newline at end of file