test: pid: fancontroller

Adds unit-tests for the fancontroller.
Bugfix: set point not initialized to 0, although bug has no impact.
Tested: Ran on quanta-q71l board and it behaved as expected.

Change-Id: I516833d8c9ed806b765ff9333801f3d57932a17b
Signed-off-by: Patrick Venture <venture@google.com>

5 files changed, 255 insertions, 1 deletions

diff --git a/pid/controller.hpp b/pid/controller.hpp
index fa89ad1..4220e07 100644
--- a/pid/controller.hpp
+++ b/pid/controller.hpp
@@ -17,6 +17,7 @@ class PIDController
     public:
         PIDController(const std::string& id, ZoneInterface* owner)
             : _owner(owner),
+              _setpoint(0),
               _id(id)
         { }
 
diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp
index c2dee74..9407689 100644
--- a/pid/fancontroller.cpp
+++ b/pid/fancontroller.cpp
@@ -27,6 +27,10 @@ std::unique_ptr<PIDController> FanController::CreateFanPid(
     std::vector<std::string>& inputs,
     ec::pidinfo initial)
 {
+    if (inputs.size() == 0)
+    {
+        return nullptr;
+    }
     auto fan = std::make_unique<FanController>(id, inputs, owner);
     ec::pid_info_t* info = fan->get_pid_info();
 
@@ -68,6 +72,8 @@ float FanController::input_proc(void)
         throw;
     }
 
+    /* Reset the value from the above loop. */
+    value = 0;
     if (values.size() > 0)
     {
         /* When this is a configuration option in a later update, this code
diff --git a/pid/fancontroller.hpp b/pid/fancontroller.hpp
index b9b015c..3354bb6 100644
--- a/pid/fancontroller.hpp
+++ b/pid/fancontroller.hpp
@@ -35,6 +35,11 @@ class FanController : public PIDController
         float setpt_proc(void) override;
         void output_proc(float value) override;
 
+        FanSpeedDirection getFanDirection(void) const
+        {
+            return _direction;
+        }
+
         void setFanDirection(FanSpeedDirection direction)
         {
             _direction = direction;
diff --git a/test/Makefile.am b/test/Makefile.am
index 6bd5138..4d78bc6 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -13,7 +13,7 @@ AM_LDFLAGS = \
 # Run all 'check' test programs
 check_PROGRAMS = sensor_manager_unittest sensor_pluggable_unittest \
  sensor_host_unittest util_unittest pid_zone_unittest \
- pid_thermalcontroller_unittest
+ pid_thermalcontroller_unittest pid_fancontroller_unittest
 TESTS = $(check_PROGRAMS)
 
 # Until libconfig is mocked out or replaced, include it.
@@ -38,3 +38,8 @@ pid_thermalcontroller_unittest_SOURCES = pid_thermalcontroller_unittest.cpp
 pid_thermalcontroller_unittest_LDADD = $(top_builddir)/pid/ec/pid.o \
  $(top_builddir)/pid/util.o $(top_builddir)/pid/controller.o \
  $(top_builddir)/pid/thermalcontroller.o
+
+pid_fancontroller_unittest_SOURCES = pid_fancontroller_unittest.cpp
+pid_fancontroller_unittest_LDADD = $(top_builddir)/pid/ec/pid.o \
+ $(top_builddir)/pid/util.o $(top_builddir)/pid/controller.o \
+ $(top_builddir)/pid/fancontroller.o
diff --git a/test/pid_fancontroller_unittest.cpp b/test/pid_fancontroller_unittest.cpp
new file mode 100644
index 0000000..b127929
--- /dev/null
+++ b/test/pid_fancontroller_unittest.cpp
@@ -0,0 +1,237 @@
+#include "pid/fancontroller.hpp"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <string>
+#include <vector>
+
+#include "pid/ec/pid.hpp"
+#include "test/sensor_mock.hpp"
+#include "test/zone_mock.hpp"
+
+using ::testing::DoubleEq;
+using ::testing::Invoke;
+using ::testing::Return;
+using ::testing::StrEq;
+using ::testing::_;
+
+TEST(FanControllerTest, BoringFactoryTest) {
+    // Verify the factory will properly build the FanPIDController in the
+    // boring (uninteresting) case.
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    // Success
+    EXPECT_FALSE(p == nullptr);
+}
+
+TEST(FanControllerTest, VerifyFactoryFailsWithZeroInputs) {
+    // A fan controller needs at least one input.
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_TRUE(p == nullptr);
+}
+
+TEST(FanControllerTest, InputProc_AllSensorsReturnZero) {
+    // If all your inputs are 0, return 0.
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0", "fan1"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+
+    EXPECT_CALL(z, getCachedValue(StrEq("fan0"))).WillOnce(Return(0));
+    EXPECT_CALL(z, getCachedValue(StrEq("fan1"))).WillOnce(Return(0));
+
+    EXPECT_EQ(0.0, p->input_proc());
+}
+
+TEST(FanControllerTest, InputProc_IfSensorNegativeIsIgnored) {
+    // A sensor value returning sub-zero is ignored as an error.
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0", "fan1"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+
+    EXPECT_CALL(z, getCachedValue(StrEq("fan0"))).WillOnce(Return(-1));
+    EXPECT_CALL(z, getCachedValue(StrEq("fan1"))).WillOnce(Return(-1));
+
+    EXPECT_EQ(0.0, p->input_proc());
+}
+
+TEST(FanControllerTest, InputProc_ChoosesMinimumValue) {
+    // Verify it selects the minimum value from its inputs.
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0", "fan1", "fan2"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+
+    EXPECT_CALL(z, getCachedValue(StrEq("fan0"))).WillOnce(Return(10.0));
+    EXPECT_CALL(z, getCachedValue(StrEq("fan1"))).WillOnce(Return(30.0));
+    EXPECT_CALL(z, getCachedValue(StrEq("fan2"))).WillOnce(Return(5.0));
+
+    EXPECT_EQ(5.0, p->input_proc());
+}
+
+// The direction is unused presently, but these tests validate the logic.
+TEST(FanControllerTest, SetPtProc_SpeedChanges_VerifyDirection) {
+    // The fan direction defaults to neutral, because we have no data.  Verify
+    // that after this point it appropriately will indicate speeding up or
+    // slowing down based on the RPM values specified.
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0", "fan1"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+    // Grab pointer for mocking.
+    FanController *fp = reinterpret_cast<FanController*>(p.get());
+
+    // Fanspeed starts are Neutral.
+    EXPECT_EQ(FanSpeedDirection::NEUTRAL, fp->getFanDirection());
+
+    // getMaxRPMRequest returns a higher value than 0, so the fans should be
+    // marked as speeding up.
+    EXPECT_CALL(z, getMaxRPMRequest()).WillOnce(Return(10.0));
+    EXPECT_EQ(10.0, p->setpt_proc());
+    EXPECT_EQ(FanSpeedDirection::UP, fp->getFanDirection());
+
+    // getMaxRPMRequest returns a lower value than 10, so the fans should be
+    // marked as slowing down.
+    EXPECT_CALL(z, getMaxRPMRequest()).WillOnce(Return(5.0));
+    EXPECT_EQ(5.0, p->setpt_proc());
+    EXPECT_EQ(FanSpeedDirection::DOWN, fp->getFanDirection());
+
+    // getMaxRPMRequest returns the same value, so the fans should be marked as
+    // neutral.
+    EXPECT_CALL(z, getMaxRPMRequest()).WillOnce(Return(5.0));
+    EXPECT_EQ(5.0, p->setpt_proc());
+    EXPECT_EQ(FanSpeedDirection::NEUTRAL, fp->getFanDirection());
+}
+
+TEST(FanControllerTest, OutputProc_VerifiesIfFailsafeEnabledInputIsIgnored) {
+    // Verify that if failsafe mode is enabled and the input value for the fans
+    // is below the failsafe minimum value, the input is not used and the fans
+    // are driven at failsafe RPM.
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0", "fan1"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+
+    EXPECT_CALL(z, getFailSafeMode()).WillOnce(Return(true));
+    EXPECT_CALL(z, getFailSafePercent()).Times(2).WillRepeatedly(Return(75.0));
+
+    int64_t timeout = 0;
+    std::unique_ptr<Sensor> s1 = std::make_unique<SensorMock>("fan0", timeout);
+    std::unique_ptr<Sensor> s2 = std::make_unique<SensorMock>("fan1", timeout);
+    // Grab pointers for mocking.
+    SensorMock *sm1 = reinterpret_cast<SensorMock*>(s1.get());
+    SensorMock *sm2 = reinterpret_cast<SensorMock*>(s2.get());
+
+    EXPECT_CALL(z, getSensor(StrEq("fan0"))).WillOnce(Return(s1.get()));
+    EXPECT_CALL(*sm1, write(0.75));
+    EXPECT_CALL(z, getSensor(StrEq("fan1"))).WillOnce(Return(s2.get()));
+    EXPECT_CALL(*sm2, write(0.75));
+
+    // This is a fan PID, so calling output_proc will try to write this value
+    // to the sensors.
+
+    // Setting 50%, will end up being 75% because the sensors are in failsafe mode.
+    p->output_proc(50.0);
+}
+
+TEST(FanControllerTest, OutputProc_BehavesAsExpected) {
+    // Verifies that when the system is not in failsafe mode, the input value
+    // to output_proc is used to drive the sensors (fans).
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0", "fan1"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+
+    EXPECT_CALL(z, getFailSafeMode()).WillOnce(Return(false));
+
+    int64_t timeout = 0;
+    std::unique_ptr<Sensor> s1 = std::make_unique<SensorMock>("fan0", timeout);
+    std::unique_ptr<Sensor> s2 = std::make_unique<SensorMock>("fan1", timeout);
+    // Grab pointers for mocking.
+    SensorMock *sm1 = reinterpret_cast<SensorMock*>(s1.get());
+    SensorMock *sm2 = reinterpret_cast<SensorMock*>(s2.get());
+
+    EXPECT_CALL(z, getSensor(StrEq("fan0"))).WillOnce(Return(s1.get()));
+    EXPECT_CALL(*sm1, write(0.5));
+    EXPECT_CALL(z, getSensor(StrEq("fan1"))).WillOnce(Return(s2.get()));
+    EXPECT_CALL(*sm2, write(0.5));
+
+    // This is a fan PID, so calling output_proc will try to write this value
+    // to the sensors.
+    p->output_proc(50.0);
+}
+
+TEST(FanControllerTest, OutputProc_VerifyFailSafeIgnoredIfInputHigher) {
+    // If the requested output is higher than the failsafe value, then use the
+    // value provided to output_proc.
+
+    ZoneMock z;
+
+    std::vector<std::string> inputs = {"fan0"};
+    ec::pidinfo initial;
+
+    std::unique_ptr<PIDController> p =
+        FanController::CreateFanPid(&z, "fan1", inputs, initial);
+    EXPECT_FALSE(p == nullptr);
+
+    EXPECT_CALL(z, getFailSafeMode()).WillOnce(Return(true));
+    EXPECT_CALL(z, getFailSafePercent()).WillOnce(Return(75.0));
+
+    int64_t timeout = 0;
+    std::unique_ptr<Sensor> s1 = std::make_unique<SensorMock>("fan0", timeout);
+    // Grab pointer for mocking.
+    SensorMock *sm1 = reinterpret_cast<SensorMock*>(s1.get());
+
+    // Converting from float to double for expectation.
+    float percent = 80;
+    double value = static_cast<double>(percent / 100);
+
+    EXPECT_CALL(z, getSensor(StrEq("fan0"))).WillOnce(Return(s1.get()));
+    EXPECT_CALL(*sm1, write(value));
+
+    // This is a fan PID, so calling output_proc will try to write this value
+    // to the sensors.
+    p->output_proc(percent);
+}