Support to accumulate PWM of different controllers for same sensor
Description:
1. Add one property: accumulateSetPoint in zone of fan table that could
be used to enable accumulation of output PWM of different controllers
with same sensor.
2. Add one property: checkHysterWithSetpt in pid info of fan table to
select to compare current input and setpoint to check hysteresis.
3. The purpose of accumulate the stepwise output and PID output for
one sensor is that the setting of stepwise could use to prevent
the fan speed from suddenly increasing from a very low speed to a
very high speed due to reaching the setpoint.
Use stepwise before setpoint could also keep the PWM steady at
low ambient temperature.
Design:
1. Search "accumulateSetPoint" field in fan table.
If the value was true, accumulate the output PWM of different
controllers with same profile name.
2. Support two method to calculate PID output that could be chosen by
setting the "checkHysterWithSetpt" to true in pid info of fan table.
If the flag was set to true, it won't calculate PWM output if the
input lower than setpoint.
Test Case:
1. Check the output PWM of different controllers with same profile
name could be accumulated - pass.
2. Set "checkHysterWithSetpt" to true and check PID output would not be
calculated if the input temperature was lower than setpoint - pass.
Please see more details in gist:
https://gist.github.com/DelphineCCChiu/a6170d3e1a12fc4ee76fad324382fba3
Change-Id: I9f38f250d72545784c6c11be2fde7d45f0b385c4
Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
diff --git a/conf.hpp b/conf.hpp
index 265da09..8a9ea38 100644
--- a/conf.hpp
+++ b/conf.hpp
@@ -84,6 +84,10 @@
/* Customize time settings for every cycle */
CycleTime cycleTime;
+
+ /* Enable accumulation of the output PWM of different controllers with same
+ * sensor */
+ bool accumulateSetPoint;
};
using PIDConf = std::map<std::string, ControllerInfo>;
diff --git a/dbus/dbusconfiguration.cpp b/dbus/dbusconfiguration.cpp
index 7b78280..f9c3460 100644
--- a/dbus/dbusconfiguration.cpp
+++ b/dbus/dbusconfiguration.cpp
@@ -385,14 +385,20 @@
info.pidInfo.slewPos = std::visit(VariantToDoubleVisitor(),
getPIDAttribute(base, "SlewPos"));
+ bool checkHysterWithSetpt = false;
double negativeHysteresis = 0;
double positiveHysteresis = 0;
double derivativeCoeff = 0;
+ auto findCheckHysterFlag = base.find("CheckHysterWithSetpt");
auto findNeg = base.find("NegativeHysteresis");
auto findPos = base.find("PositiveHysteresis");
auto findDerivative = base.find("DCoefficient");
+ if (findCheckHysterFlag != base.end())
+ {
+ checkHysterWithSetpt = std::get<bool>(findCheckHysterFlag->second);
+ }
if (findNeg != base.end())
{
negativeHysteresis = std::visit(VariantToDoubleVisitor(),
@@ -409,6 +415,7 @@
findDerivative->second);
}
+ info.pidInfo.checkHysterWithSetpt = checkHysterWithSetpt;
info.pidInfo.negativeHysteresis = negativeHysteresis;
info.pidInfo.positiveHysteresis = positiveHysteresis;
info.pidInfo.derivativeCoeff = derivativeCoeff;
@@ -649,6 +656,14 @@
details.cycleTime.cycleIntervalTimeMS);
getCycleTimeSetting(zone, index, "UpdateThermalsTimeMS",
details.cycleTime.updateThermalsTimeMS);
+
+ bool accumulateSetPoint = false;
+ auto findAccSetPoint = zone.find("AccumulateSetPoint");
+ if (findAccSetPoint != zone.end())
+ {
+ accumulateSetPoint = std::get<bool>(findAccSetPoint->second);
+ }
+ details.accumulateSetPoint = accumulateSetPoint;
}
auto findBase = configuration.second.find(pidConfigurationInterface);
// loop through all the PID configurations and fill out a sensor config
diff --git a/pid/builder.cpp b/pid/builder.cpp
index 39d0076..d133662 100644
--- a/pid/builder.cpp
+++ b/pid/builder.cpp
@@ -76,7 +76,8 @@
auto zone = std::make_shared<DbusPidZone>(
zoneId, zoneConf->second.minThermalOutput,
zoneConf->second.failsafePercent, zoneConf->second.cycleTime, mgr,
- modeControlBus, getControlPath(zoneId).c_str(), deferSignals);
+ modeControlBus, getControlPath(zoneId).c_str(), deferSignals,
+ zoneConf->second.accumulateSetPoint);
std::cerr << "Zone Id: " << zone->getZoneID() << "\n";
diff --git a/pid/buildjson.cpp b/pid/buildjson.cpp
index dd7d2a3..57fabbf 100644
--- a/pid/buildjson.cpp
+++ b/pid/buildjson.cpp
@@ -66,12 +66,18 @@
*/
auto p = j.at("pid");
+ auto checkHysterWithSetpt = p.find("checkHysterWithSetpt");
auto positiveHysteresis = p.find("positiveHysteresis");
auto negativeHysteresis = p.find("negativeHysteresis");
auto derivativeCoeff = p.find("derivativeCoeff");
+ auto checkHysterWithSetptValue = false;
auto positiveHysteresisValue = 0.0;
auto negativeHysteresisValue = 0.0;
auto derivativeCoeffValue = 0.0;
+ if (checkHysterWithSetpt != p.end())
+ {
+ checkHysterWithSetpt->get_to(checkHysterWithSetptValue);
+ }
if (positiveHysteresis != p.end())
{
positiveHysteresis->get_to(positiveHysteresisValue);
@@ -113,6 +119,7 @@
c.pidInfo.positiveHysteresis = positiveHysteresisValue;
c.pidInfo.negativeHysteresis = negativeHysteresisValue;
c.pidInfo.derivativeCoeff = derivativeCoeffValue;
+ c.pidInfo.checkHysterWithSetpt = checkHysterWithSetptValue;
}
else
{
@@ -213,6 +220,14 @@
getCycleTimeSetting(zone, id, "updateThermalsTimeMS",
thisZoneConfig.cycleTime.updateThermalsTimeMS);
+ bool accumulateSetPoint = false;
+ auto findAccSetPoint = zone.find("accumulateSetPoint");
+ if (findAccSetPoint != zone.end())
+ {
+ findAccSetPoint->get_to(accumulateSetPoint);
+ }
+ thisZoneConfig.accumulateSetPoint = accumulateSetPoint;
+
auto pids = zone["pids"];
for (const auto& pid : pids)
{
diff --git a/pid/ec/pid.hpp b/pid/ec/pid.hpp
index 9dac6a4..9db08fd 100644
--- a/pid/ec/pid.hpp
+++ b/pid/ec/pid.hpp
@@ -19,21 +19,23 @@
*/
typedef struct
{
- bool initialized; // has pid been initialized
+ bool initialized; // has pid been initialized
+ bool checkHysterWithSetpt; // compare current input and setpoint to check
+ // hysteresis
- double ts; // sample time in seconds
- double integral; // integral of error
- double lastOutput; // value of last output
- double lastError; // value of last error
+ double ts; // sample time in seconds
+ double integral; // integral of error
+ double lastOutput; // value of last output
+ double lastError; // value of last error
- double proportionalCoeff; // coeff for P
- double integralCoeff; // coeff for I
- double derivativeCoeff; // coeff for D
- double feedFwdOffset; // offset coeff for feed-forward term
- double feedFwdGain; // gain for feed-forward term
+ double proportionalCoeff; // coeff for P
+ double integralCoeff; // coeff for I
+ double derivativeCoeff; // coeff for D
+ double feedFwdOffset; // offset coeff for feed-forward term
+ double feedFwdGain; // gain for feed-forward term
- limits_t integralLimit; // clamp of integral
- limits_t outLim; // clamp of output
+ limits_t integralLimit; // clamp of integral
+ limits_t outLim; // clamp of output
double slewNeg;
double slewPos;
double positiveHysteresis;
@@ -46,6 +48,9 @@
/* Condensed version for use by the configuration. */
struct pidinfo
{
+ bool checkHysterWithSetpt; // compare current input and setpoint to check
+ // hysteresis
+
double ts; // sample time in seconds
double proportionalCoeff; // coeff for P
double integralCoeff; // coeff for I
diff --git a/pid/pidcontroller.cpp b/pid/pidcontroller.cpp
index fa8d6f6..b33037f 100644
--- a/pid/pidcontroller.cpp
+++ b/pid/pidcontroller.cpp
@@ -30,6 +30,81 @@
namespace pid_control
{
+double PIDController::calPIDOutput(double setpt, double input,
+ ec::pid_info_t* info)
+{
+ double output;
+ auto name = getID();
+
+ if (info->checkHysterWithSetpt)
+ {
+ // Over the hysteresis bounds, keep counting pid
+ if (input > (setpt + info->positiveHysteresis))
+ {
+ // Calculate new output
+ output = ec::pid(info, input, setpt, &name);
+
+ // 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;
+ }
+ // Under the hysteresis bounds, initialize pid
+ else if (input < (setpt - info->negativeHysteresis))
+ {
+ lastInput = setpt;
+ info->integral = 0;
+ output = 0;
+ }
+ // inside the hysteresis bounds, keep last output
+ else
+ {
+ lastInput = input;
+ output = info->lastOutput;
+ }
+
+ info->lastOutput = output;
+ }
+ else
+ {
+ // if no hysteresis, maintain previous behavior
+ if (info->positiveHysteresis == 0 && info->negativeHysteresis == 0)
+ {
+ // Calculate new output
+ output = ec::pid(info, input, setpt, &name);
+
+ // 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 the value is not set (NAN) or abnormal (+INF or
+ // -INF)
+ if (!(std::isfinite(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, &name);
+ }
+ }
+
+ return output;
+}
+
void PIDController::process(void)
{
double input;
@@ -43,39 +118,11 @@
input = inputProc();
auto info = getPIDInfo();
- auto name = getID();
- // if no hysteresis, maintain previous behavior
- if (info->positiveHysteresis == 0 && info->negativeHysteresis == 0)
- {
- // Calculate new output
- output = ec::pid(info, input, setpt, &name);
+ // Calculate output value
+ output = calPIDOutput(setpt, input, info);
- // 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, &name);
- }
+ info->lastOutput = output;
// Output new value
outputProc(output);
diff --git a/pid/pidcontroller.hpp b/pid/pidcontroller.hpp
index 05cbd71..91147d9 100644
--- a/pid/pidcontroller.hpp
+++ b/pid/pidcontroller.hpp
@@ -24,6 +24,7 @@
Controller(), _owner(owner), _id(id)
{
_pid_info.initialized = false;
+ _pid_info.checkHysterWithSetpt = false;
_pid_info.ts = static_cast<double>(0.0);
_pid_info.integral = static_cast<double>(0.0);
_pid_info.lastOutput = static_cast<double>(0.0);
@@ -73,6 +74,8 @@
return lastInput;
}
+ double calPIDOutput(double setpt, double input, ec::pid_info_t* info);
+
protected:
ZoneInterface* _owner;
std::string _id;
diff --git a/pid/util.cpp b/pid/util.cpp
index ac6edb1..dbad60e 100644
--- a/pid/util.cpp
+++ b/pid/util.cpp
@@ -26,6 +26,7 @@
void initializePIDStruct(ec::pid_info_t* info, const ec::pidinfo& initial)
{
+ info->checkHysterWithSetpt = initial.checkHysterWithSetpt;
info->ts = initial.ts;
info->proportionalCoeff = initial.proportionalCoeff;
info->integralCoeff = initial.integralCoeff;
diff --git a/pid/zone.cpp b/pid/zone.cpp
index e5eddca..5332efe 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -120,15 +120,32 @@
return;
}
- _SetPoints.push_back(setPoint);
+ auto profileName = name;
+ if (getAccSetPoint())
+ {
+ /*
+ * If the name of controller is Linear_Temp_CPU0.
+ * The profile name will be Temp_CPU0.
+ */
+ profileName = name.substr(name.find("_") + 1);
+ _SetPoints[profileName] += setPoint;
+ }
+ else
+ {
+ if (_SetPoints[profileName] < setPoint)
+ {
+ _SetPoints[profileName] = setPoint;
+ }
+ }
+
/*
* if there are multiple thermal controllers with the same
* value, pick the first one in the iterator
*/
- if (_maximumSetPoint < setPoint)
+ if (_maximumSetPoint < _SetPoints[profileName])
{
- _maximumSetPoint = setPoint;
- _maximumSetPointName = name;
+ _maximumSetPoint = _SetPoints[profileName];
+ _maximumSetPointName = profileName;
}
}
@@ -208,7 +225,16 @@
void DbusPidZone::addThermalInput(const std::string& therm,
bool missingAcceptable)
{
- _thermalInputs.push_back(therm);
+ /*
+ * One sensor may have stepwise and PID at the same time.
+ * Searching the sensor name before inserting it to avoid duplicated sensor
+ * names.
+ */
+ if (std::find(_thermalInputs.begin(), _thermalInputs.end(), therm) ==
+ _thermalInputs.end())
+ {
+ _thermalInputs.push_back(therm);
+ }
if (missingAcceptable)
{
@@ -287,6 +313,33 @@
}
/*
+ * Combine the maximum SetPoint Name if the controllers have same profile
+ * name. e.g., PID_BB_INLET_TEMP_C + Stepwise_BB_INLET_TEMP_C.
+ */
+ if (getAccSetPoint())
+ {
+ auto profileName = _maximumSetPointName;
+ _maximumSetPointName = "";
+
+ for (auto& p : _thermals)
+ {
+ auto controllerID = p->getID();
+ auto found = controllerID.find(profileName);
+ if (found != std::string::npos)
+ {
+ if (_maximumSetPointName.empty())
+ {
+ _maximumSetPointName = controllerID;
+ }
+ else
+ {
+ _maximumSetPointName += " + " + controllerID;
+ }
+ }
+ }
+ }
+
+ /*
* If the maximum RPM setpoint output is below the minimum RPM
* setpoint, set it to the minimum.
*/
@@ -579,4 +632,9 @@
}
}
+bool DbusPidZone::getAccSetPoint(void) const
+{
+ return _accumulateSetPoint;
+}
+
} // namespace pid_control
diff --git a/pid/zone.hpp b/pid/zone.hpp
index 464e672..52180a5 100644
--- a/pid/zone.hpp
+++ b/pid/zone.hpp
@@ -49,11 +49,13 @@
public:
DbusPidZone(int64_t zone, double minThermalOutput, double failSafePercent,
conf::CycleTime cycleTime, const SensorManager& mgr,
- sdbusplus::bus_t& bus, const char* objPath, bool defer) :
+ sdbusplus::bus_t& bus, const char* objPath, bool defer,
+ bool accumulateSetPoint) :
ModeObject(bus, objPath,
defer ? ModeObject::action::defer_emit
: ModeObject::action::emit_object_added),
_zoneId(zone), _maximumSetPoint(),
+ _accumulateSetPoint(accumulateSetPoint),
_minThermalOutputSetPt(minThermalOutput),
_zoneFailSafePercent(failSafePercent), _cycleTime(cycleTime), _mgr(mgr)
{
@@ -72,6 +74,7 @@
void setManualMode(bool mode);
bool getFailSafeMode(void) const override;
void markSensorMissing(const std::string& name);
+ bool getAccSetPoint(void) const override;
int64_t getZoneID(void) const override;
void addSetPoint(double setPoint, const std::string& name) override;
@@ -209,6 +212,7 @@
std::string _maximumSetPointNamePrev;
bool _manualMode = false;
bool _redundantWrite = false;
+ bool _accumulateSetPoint = false;
const double _minThermalOutputSetPt;
// Current fail safe Percent.
double _failSafePercent;
@@ -219,7 +223,7 @@
std::set<std::string> _failSafeSensors;
std::set<std::string> _missingAcceptable;
- std::vector<double> _SetPoints;
+ std::map<std::string, double> _SetPoints;
std::vector<double> _RPMCeilings;
std::vector<std::string> _fanInputs;
std::vector<std::string> _thermalInputs;
diff --git a/pid/zone_interface.hpp b/pid/zone_interface.hpp
index 7797740..31f3256 100644
--- a/pid/zone_interface.hpp
+++ b/pid/zone_interface.hpp
@@ -104,6 +104,11 @@
*/
virtual bool getRedundantWrite(void) const = 0;
+ /** Returns true if user wants to accumulate the output PWM of different
+ * controllers with same sensor
+ */
+ virtual bool getAccSetPoint(void) const = 0;
+
/** For each fan pid, do processing. */
virtual void processFans(void) = 0;
/** For each thermal pid, do processing. */
diff --git a/test/pid_thermalcontroller_unittest.cpp b/test/pid_thermalcontroller_unittest.cpp
index 7aee1e1..8abedf5 100644
--- a/test/pid_thermalcontroller_unittest.cpp
+++ b/test/pid_thermalcontroller_unittest.cpp
@@ -211,6 +211,7 @@
std::vector<pid_control::conf::SensorInput> inputs = {{"fleeting0"}};
double setpoint = 10.0;
ec::pidinfo initial;
+ initial.checkHysterWithSetpt = false;
initial.negativeHysteresis = 4.0;
std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
@@ -244,6 +245,7 @@
std::vector<pid_control::conf::SensorInput> inputs = {{"fleeting0"}};
double setpoint = 10.0;
ec::pidinfo initial;
+ initial.checkHysterWithSetpt = false;
initial.positiveHysteresis = 5.0;
std::unique_ptr<PIDController> p = ThermalController::createThermalPid(
diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp
index 6c570cc..1f6e672 100644
--- a/test/pid_zone_unittest.cpp
+++ b/test/pid_zone_unittest.cpp
@@ -53,6 +53,7 @@
SensorManager m(bus_mock_passive, bus_mock_host);
bool defer = true;
+ bool accSetPoint = false;
const char* objPath = "/path/";
int64_t zone = 1;
double minThermalOutput = 1000.0;
@@ -83,7 +84,7 @@
.WillOnce(Return(0));
DbusPidZone p(zone, minThermalOutput, failSafePercent, cycleTime, m,
- bus_mock_mode, objPath, defer);
+ bus_mock_mode, objPath, defer, accSetPoint);
// Success.
}
@@ -126,9 +127,9 @@
.Times(::testing::AnyNumber())
.WillOnce(Return(0));
- zone = std::make_unique<DbusPidZone>(zoneId, minThermalOutput,
- failSafePercent, cycleTime, mgr,
- bus_mock_mode, objPath, defer);
+ zone = std::make_unique<DbusPidZone>(
+ zoneId, minThermalOutput, failSafePercent, cycleTime, mgr,
+ bus_mock_mode, objPath, defer, accSetPoint);
}
// unused
@@ -146,6 +147,7 @@
double failSafePercent = 0;
double setpoint = 50.0;
bool defer = true;
+ bool accSetPoint = false;
const char* objPath = "/path/";
SensorManager mgr;
conf::CycleTime cycleTime;
diff --git a/test/zone_mock.hpp b/test/zone_mock.hpp
index a3d6be9..4e34890 100644
--- a/test/zone_mock.hpp
+++ b/test/zone_mock.hpp
@@ -46,6 +46,7 @@
MOCK_CONST_METHOD0(getCycleIntervalTime, uint64_t());
MOCK_CONST_METHOD0(getUpdateThermalsCycle, uint64_t());
+ MOCK_CONST_METHOD0(getAccSetPoint, bool());
MOCK_METHOD1(getSensor, Sensor*(const std::string&));