Add Hysteresis to pid controllers
Add hysteresis to pid controllers to lower pwm changes.
It is defaulted to 0 so it should be transparent
to any controller that choses not to implement it.
This is the same pattern used by the stepwise controller.
Tested-by: Unit tests passed
Change-Id: Ib47114285b0017258b7f77eaf067d310f95a0c60
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/dbus/dbusconfiguration.cpp b/dbus/dbusconfiguration.cpp
index 3121623..7d01650 100644
--- a/dbus/dbusconfiguration.cpp
+++ b/dbus/dbusconfiguration.cpp
@@ -428,6 +428,24 @@
VariantToDoubleVisitor(), base.at("SlewNeg"));
info.pidInfo.slew_pos = variant_ns::visit(
VariantToDoubleVisitor(), base.at("SlewPos"));
+ double negativeHysteresis = 0;
+ double positiveHysteresis = 0;
+
+ auto findNeg = base.find("NegativeHysteresis");
+ auto findPos = base.find("PositiveHysteresis");
+ if (findNeg != base.end())
+ {
+ negativeHysteresis = variant_ns::visit(
+ VariantToDoubleVisitor(), findNeg->second);
+ }
+
+ if (findPos != base.end())
+ {
+ positiveHysteresis = variant_ns::visit(
+ VariantToDoubleVisitor(), findPos->second);
+ }
+ info.pidInfo.negativeHysteresis = negativeHysteresis;
+ info.pidInfo.positiveHysteresis = positiveHysteresis;
}
}
auto findStepwise =
diff --git a/pid/ec/pid.hpp b/pid/ec/pid.hpp
index 779ced5..3ab64cd 100644
--- a/pid/ec/pid.hpp
+++ b/pid/ec/pid.hpp
@@ -31,6 +31,8 @@
limits_t out_lim; // clamp of output
double slew_neg;
double slew_pos;
+ double positiveHysteresis;
+ double negativeHysteresis;
} pid_info_t;
double pid(pid_info_t* pidinfoptr, double input, double setpoint);
@@ -47,6 +49,8 @@
ec::limits_t out_lim; // clamp of output
double slew_neg;
double slew_pos;
+ double positiveHysteresis;
+ double negativeHysteresis;
};
} // namespace ec
diff --git a/pid/pidcontroller.cpp b/pid/pidcontroller.cpp
index 7be6ceb..e3eaaff 100644
--- a/pid/pidcontroller.cpp
+++ b/pid/pidcontroller.cpp
@@ -20,6 +20,7 @@
#include <algorithm>
#include <chrono>
+#include <cmath>
#include <iostream>
#include <map>
#include <memory>
@@ -38,8 +39,39 @@
// Get input value
input = inputProc();
- // Calculate new output
- output = ec::pid(getPIDInfo(), input, setpt);
+ auto info = getPIDInfo();
+
+ // if no hysteresis, maintain previous behavior
+ if (info->positiveHysteresis == 0 && info->negativeHysteresis == 0)
+ {
+ // Calculate new output
+ output = ec::pid(info, input, setpt);
+
+ // this variable isn't actually used in this context, but we're setting
+ // it here incase somebody uses it later it's the correct value
+ lastInput = input;
+ }
+ else
+ {
+ // initialize if not set yet
+ if (std::isnan(lastInput))
+ {
+ lastInput = input;
+ }
+
+ // if reading is outside of hysteresis bounds, use it for reading,
+ // otherwise use last reading without updating it first
+ else if ((input - lastInput) > info->positiveHysteresis)
+ {
+ lastInput = input;
+ }
+ else if ((lastInput - input) > info->negativeHysteresis)
+ {
+ lastInput = input;
+ }
+
+ output = ec::pid(info, lastInput, setpt);
+ }
// Output new value
outputProc(output);
diff --git a/pid/pidcontroller.hpp b/pid/pidcontroller.hpp
index 9ed3be2..3d38c2a 100644
--- a/pid/pidcontroller.hpp
+++ b/pid/pidcontroller.hpp
@@ -49,6 +49,11 @@
return &_pid_info;
}
+ double getLastInput(void)
+ {
+ return lastInput;
+ }
+
protected:
ZoneInterface* _owner;
@@ -57,4 +62,5 @@
ec::pid_info_t _pid_info;
double _setpoint;
std::string _id;
+ double lastInput = std::numeric_limits<double>::quiet_NaN();
};
diff --git a/pid/util.cpp b/pid/util.cpp
index 9d666ef..a5936b0 100644
--- a/pid/util.cpp
+++ b/pid/util.cpp
@@ -34,6 +34,8 @@
info->out_lim.max = initial.out_lim.max;
info->slew_neg = initial.slew_neg;
info->slew_pos = initial.slew_pos;
+ info->negativeHysteresis = initial.negativeHysteresis;
+ info->positiveHysteresis = initial.positiveHysteresis;
}
void dumpPIDStruct(ec::pid_info_t* info)
diff --git a/scripts/writepid.mako.cpp b/scripts/writepid.mako.cpp
index 1718bd1..852f4c0 100644
--- a/scripts/writepid.mako.cpp
+++ b/scripts/writepid.mako.cpp
@@ -27,6 +27,13 @@
setpoint = 0
else:
setpoint = details['set-point']
+
+ neg_hysteresis = 0
+ pos_hysteresis = 0
+ if 'neg_hysteresis' in details['pid']:
+ neg_hysteresis = details['pid']['neg_hysteresis']
+ if 'pos_hysteresis' in details['pid']:
+ pos_hysteresis = details['pid']['pos_hysteresis']
%>
${setpoint},
{${details['pid']['sampleperiod']},
@@ -37,7 +44,9 @@
{${details['pid']['i_limit']['min']}, ${details['pid']['i_limit']['max']}},
{${details['pid']['out_limit']['min']}, ${details['pid']['out_limit']['max']}},
${details['pid']['slew_neg']},
- ${details['pid']['slew_pos']}},
+ ${details['pid']['slew_pos']},
+ ${neg_hysteresis},
+ ${pos_hysteresis}},
},
},
% endfor
diff --git a/test/pid_thermalcontroller_unittest.cpp b/test/pid_thermalcontroller_unittest.cpp
index 804d84a..ae5c769 100644
--- a/test/pid_thermalcontroller_unittest.cpp
+++ b/test/pid_thermalcontroller_unittest.cpp
@@ -8,6 +8,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+using ::testing::_;
using ::testing::Return;
using ::testing::StrEq;
@@ -143,4 +144,71 @@
EXPECT_CALL(z, getCachedValue(StrEq("fleeting1"))).WillOnce(Return(10.0));
EXPECT_EQ(5.0, p->inputProc());
+}
+
+TEST(ThermalControllerTest, NegHysteresis_BehavesAsExpected)
+{
+
+ // This test verifies Negative hysteresis behaves as expected by
+ // crossing the setpoint and noticing readings don't change until past the
+ // hysteresis value
+
+ ZoneMock z;
+
+ std::vector<std::string> inputs = {"fleeting0"};
+ double setpoint = 10.0;
+ ec::pidinfo initial;
+ initial.negativeHysteresis = 4.0;
+
+ std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
+ EXPECT_FALSE(p == nullptr);
+
+ EXPECT_CALL(z, getCachedValue(StrEq("fleeting0")))
+ .Times(3)
+ .WillOnce(Return(12.0))
+ .WillOnce(Return(9.0))
+ .WillOnce(Return(7.0));
+
+ EXPECT_CALL(z, addRPMSetPoint(_)).Times(3);
+
+ std::vector<double> lastReadings = {12.0, 12.0, 7.0};
+ for (auto& reading : lastReadings)
+ {
+ p->process();
+ EXPECT_EQ(p->getLastInput(), reading);
+ }
+}
+
+TEST(ThermalControllerTest, PosHysteresis_BehavesAsExpected)
+{
+ // This test verifies Positive hysteresis behaves as expected by
+ // crossing the setpoint and noticing readings don't change until past the
+ // hysteresis value
+
+ ZoneMock z;
+
+ std::vector<std::string> inputs = {"fleeting0"};
+ double setpoint = 10.0;
+ ec::pidinfo initial;
+ initial.positiveHysteresis = 5.0;
+
+ std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
+ &z, "therm1", inputs, setpoint, initial, ThermalType::margin);
+ EXPECT_FALSE(p == nullptr);
+
+ EXPECT_CALL(z, getCachedValue(StrEq("fleeting0")))
+ .Times(3)
+ .WillOnce(Return(8.0))
+ .WillOnce(Return(13.0))
+ .WillOnce(Return(14.0));
+
+ EXPECT_CALL(z, addRPMSetPoint(_)).Times(3);
+
+ std::vector<double> lastReadings = {8.0, 8.0, 14.0};
+ for (auto& reading : lastReadings)
+ {
+ p->process();
+ EXPECT_EQ(p->getLastInput(), reading);
+ }
}
\ No newline at end of file