Support derivative term in PID algorithm and support to set cycle interval time from fan table
1. Support to calculate derivative term in PID algorithm.
2. Add two properties: cycleIntervalTimeMS and updateThermalsTimeMS
in fan table that could be used to decide "time interval of PID control loop"
and "time interval to update thermals' cached value".
Tested:
- PID algorithm:
1. Check pid-control-service could calculate output PWM
according to the fan table.
[Test log]
root@greatlakes:~# systemctl status phosphor-pid-control -l
* phosphor-pid-control.service - Phosphor-Pid-Control Margin-based Fan Control Daemon
Loaded: loaded (/lib/systemd/system/phosphor-pid-control.service; enabled; preset: enabled)
Active: active (running) since Fri 2018-03-09 05:09:35 PST; 1min 47s ago
Main PID: 3105 (swampd)
CGroup: /system.slice/phosphor-pid-control.service
`-3105 /usr/bin/swampd -c /usr/share/entity-manager/configurations/fan-table.json
...
Mar 09 05:10:29 greatlakes phosphor-pid-control[3105]: PID Zone 1 max SetPoint 3.75 requested by
PID_NIC_SENSOR_TEMP BMC_SENSOR_FAN0_TACH BMC_SENSOR_FAN2_TACH BMC_SENSOR_FAN4_TACH BMC_SENSOR_FAN6_TACH
- Cycle interval time:
1. Set cycleIntervalTimeMS and updateThermalsTimeMS
to 1000 ms in fan table
2. Check service would update thermal every second from debug log.
[Test log]
root@greatlakes:~# journalctl -u phosphor-pid-control --since "Mar 09 04:52:16"
Mar 09 04:52:16 greatlakes systemd[1]: Started Phosphor-Pid-Control Margin-based Fan Control Daemon.
...
Mar 09 04:53:28 greatlakes phosphor-pid-control[2795]: processThermals
Mar 09 04:53:28 greatlakes phosphor-pid-control[2795]: processFans
Mar 09 04:53:29 greatlakes phosphor-pid-control[2795]: processThermals
Mar 09 04:53:29 greatlakes phosphor-pid-control[2795]: processFans
Mar 09 04:53:30 greatlakes phosphor-pid-control[2795]: processThermals
Mar 09 04:53:30 greatlakes phosphor-pid-control[2795]: processFans
Change-Id: I04e1b440603c3ad66a1e26c96451992785da6fe6
Signed-off-by: Bonnie Lo <Bonnie_Lo@wiwynn.com>
diff --git a/conf.hpp b/conf.hpp
index 9122a28..d7d34f5 100644
--- a/conf.hpp
+++ b/conf.hpp
@@ -38,11 +38,17 @@
std::string type; // fan or margin or temp?
std::vector<std::string> inputs; // one or more sensors.
double setpoint; // initial setpoint for thermal.
- union
- {
- ec::pidinfo pidInfo; // pid details
- ec::StepwiseInfo stepwiseInfo;
- };
+ ec::pidinfo pidInfo; // pid details
+ ec::StepwiseInfo stepwiseInfo;
+};
+
+struct CycleTime
+{
+ /* The time interval every cycle. 0.1 seconds by default */
+ uint64_t cycleIntervalTimeMS = 100; // milliseconds
+
+ /* The interval of updating thermals. 1 second by default */
+ uint64_t updateThermalsTimeMS = 1000; // milliseconds
};
/*
@@ -57,6 +63,9 @@
/* If the sensors are in fail-safe mode, this is the percentage to use. */
double failsafePercent;
+
+ /* Customize time settings for every cycle */
+ CycleTime cycleTime;
};
using PIDConf = std::map<std::string, ControllerInfo>;
diff --git a/pid/builder.cpp b/pid/builder.cpp
index 7be7897..d7a60b4 100644
--- a/pid/builder.cpp
+++ b/pid/builder.cpp
@@ -69,8 +69,8 @@
auto zone = std::make_shared<DbusPidZone>(
zoneId, zoneConf->second.minThermalOutput,
- zoneConf->second.failsafePercent, mgr, modeControlBus,
- getControlPath(zoneId).c_str(), deferSignals);
+ zoneConf->second.failsafePercent, zoneConf->second.cycleTime, mgr,
+ modeControlBus, getControlPath(zoneId).c_str(), deferSignals);
std::cerr << "Zone Id: " << zone->getZoneID() << "\n";
diff --git a/pid/buildjson.cpp b/pid/buildjson.cpp
index e078fdb..cf7df67 100644
--- a/pid/buildjson.cpp
+++ b/pid/buildjson.cpp
@@ -20,6 +20,7 @@
#include <nlohmann/json.hpp>
+#include <iostream>
#include <map>
#include <tuple>
@@ -60,6 +61,7 @@
p.at("samplePeriod").get_to(c.pidInfo.ts);
p.at("proportionalCoeff").get_to(c.pidInfo.proportionalCoeff);
p.at("integralCoeff").get_to(c.pidInfo.integralCoeff);
+ p.at("derivativeCoeff").get_to(c.pidInfo.derivativeCoeff);
p.at("feedFwdOffsetCoeff").get_to(c.pidInfo.feedFwdOffset);
p.at("feedFwdGainCoeff").get_to(c.pidInfo.feedFwdGain);
p.at("integralLimit_min").get_to(c.pidInfo.integralLimit.min);
@@ -139,6 +141,56 @@
thisZoneConfig.minThermalOutput = zone["minThermalOutput"];
thisZoneConfig.failsafePercent = zone["failsafePercent"];
+ auto findTimeInterval = zone.find("cycleIntervalTimeMS");
+ if (findTimeInterval != zone.end())
+ {
+ uint64_t tmp;
+ findTimeInterval->get_to(tmp);
+ if (tmp != 0)
+ {
+ thisZoneConfig.cycleTime.cycleIntervalTimeMS = tmp;
+ }
+ else
+ {
+ std::cerr << "cycleIntervalTimeMS cannot be 0. Use default "
+ << thisZoneConfig.cycleTime.cycleIntervalTimeMS
+ << " ms\n";
+ }
+ }
+
+ auto findUpdateThermalsTime = zone.find("updateThermalsTimeMS");
+ if (findUpdateThermalsTime != zone.end())
+ {
+ uint64_t tmp;
+ findUpdateThermalsTime->get_to(tmp);
+ if (tmp != 0)
+ {
+ thisZoneConfig.cycleTime.updateThermalsTimeMS = tmp;
+ }
+ else
+ {
+ std::cerr << "updateThermalsTimeMS cannot be 0. Use default "
+ << thisZoneConfig.cycleTime.updateThermalsTimeMS
+ << " ms\n";
+ }
+ }
+
+ double updateCount =
+ double(thisZoneConfig.cycleTime.updateThermalsTimeMS) /
+ double(thisZoneConfig.cycleTime.cycleIntervalTimeMS);
+
+ /* Check if updateThermalsTimeMS could be divided by cycleIntervalTimeMS
+ * without leaving a remainder */
+ if (updateCount != std::ceil(updateCount))
+ {
+ std::cerr
+ << "updateThermalsTimeMS cannot be divided by "
+ "cycleIntervalTimeMS without leaving a remainder. Using the "
+ "smallest integer that is not less than the result.\n";
+ updateCount = std::ceil(updateCount);
+ }
+ thisZoneConfig.cycleTime.updateThermalsTimeMS = updateCount;
+
auto pids = zone["pids"];
for (const auto& pid : pids)
{
diff --git a/pid/ec/pid.cpp b/pid/ec/pid.cpp
index 98968f7..10db1dd 100644
--- a/pid/ec/pid.cpp
+++ b/pid/ec/pid.cpp
@@ -48,6 +48,7 @@
double proportionalTerm;
double integralTerm = 0.0f;
+ double derivativeTerm = 0.0f;
double feedFwdTerm = 0.0f;
double output;
@@ -67,11 +68,15 @@
pidinfoptr->integralLimit.max);
}
+ // piD
+ derivativeTerm = pidinfoptr->derivativeCoeff *
+ ((error - pidinfoptr->lastError) / pidinfoptr->ts);
+
// FF
feedFwdTerm =
(setpoint + pidinfoptr->feedFwdOffset) * pidinfoptr->feedFwdGain;
- output = proportionalTerm + integralTerm + feedFwdTerm;
+ output = proportionalTerm + integralTerm + derivativeTerm + feedFwdTerm;
output = clamp(output, pidinfoptr->outLim.min, pidinfoptr->outLim.max);
// slew rate
@@ -115,6 +120,7 @@
pidinfoptr->integralLimit.max);
pidinfoptr->integral = integralTerm;
pidinfoptr->initialized = true;
+ pidinfoptr->lastError = error;
pidinfoptr->lastOutput = output;
return output;
diff --git a/pid/ec/pid.hpp b/pid/ec/pid.hpp
index 29c7bb3..255c911 100644
--- a/pid/ec/pid.hpp
+++ b/pid/ec/pid.hpp
@@ -23,9 +23,11 @@
double ts; // sample time in seconds
double integral; // intergal 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
@@ -45,6 +47,7 @@
double ts; // sample time in seconds
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
ec::limits_t integralLimit; // clamp of integral
diff --git a/pid/pidloop.cpp b/pid/pidloop.cpp
index a5d0daf..aba2b6e 100644
--- a/pid/pidloop.cpp
+++ b/pid/pidloop.cpp
@@ -48,7 +48,7 @@
void pidControlLoop(std::shared_ptr<ZoneInterface> zone,
std::shared_ptr<boost::asio::steady_timer> timer,
- const bool* isCanceling, bool first, int ms100cnt)
+ const bool* isCanceling, bool first, uint64_t cycleCnt)
{
if (*isCanceling)
return;
@@ -64,8 +64,9 @@
processThermals(zone);
}
- timer->expires_after(std::chrono::milliseconds(100));
- timer->async_wait([zone, timer, ms100cnt, isCanceling](
+ timer->expires_after(
+ std::chrono::milliseconds(zone->getCycleIntervalTime()));
+ timer->async_wait([zone, timer, cycleCnt, isCanceling](
const boost::system::error_code& ec) mutable {
if (ec == boost::asio::error::operation_aborted)
{
@@ -102,16 +103,16 @@
// Check if we should just go back to sleep.
if (zone->getManualMode())
{
- pidControlLoop(zone, timer, isCanceling, false, ms100cnt);
+ pidControlLoop(zone, timer, isCanceling, false, cycleCnt);
return;
}
// Get the latest fan speeds.
zone->updateFanTelemetry();
- if (10 <= ms100cnt)
+ if (zone->getUpdateThermalsCycle() <= cycleCnt)
{
- ms100cnt = 0;
+ cycleCnt = 0;
processThermals(zone);
}
@@ -126,9 +127,9 @@
zone->writeLog(out.str());
}
- ms100cnt += 1;
+ cycleCnt += 1;
- pidControlLoop(zone, timer, isCanceling, false, ms100cnt);
+ pidControlLoop(zone, timer, isCanceling, false, cycleCnt);
});
}
diff --git a/pid/pidloop.hpp b/pid/pidloop.hpp
index f9b78b3..0a143e7 100644
--- a/pid/pidloop.hpp
+++ b/pid/pidloop.hpp
@@ -17,11 +17,11 @@
* @param[in] isCanceling - bool ptr to indicate whether pidControlLoop is being
* canceled.
* @param[in] first - boolean to denote if initialization needs to be run.
- * @param[in] ms100cnt - loop timer counter.
+ * @param[in] cycleCnt - loop timer counter.
*/
void pidControlLoop(std::shared_ptr<ZoneInterface> zone,
std::shared_ptr<boost::asio::steady_timer> timer,
const bool* isCanceling, bool first = true,
- int ms100cnt = 0);
+ uint64_t cycleCnt = 0);
} // namespace pid_control
diff --git a/pid/util.cpp b/pid/util.cpp
index 646376d..ac6edb1 100644
--- a/pid/util.cpp
+++ b/pid/util.cpp
@@ -29,6 +29,7 @@
info->ts = initial.ts;
info->proportionalCoeff = initial.proportionalCoeff;
info->integralCoeff = initial.integralCoeff;
+ info->derivativeCoeff = initial.derivativeCoeff;
info->feedFwdOffset = initial.feedFwdOffset;
info->feedFwdGain = initial.feedFwdGain;
info->integralLimit.min = initial.integralLimit.min;
@@ -46,6 +47,7 @@
std::cerr << " ts: " << info->ts
<< " proportionalCoeff: " << info->proportionalCoeff
<< " integralCoeff: " << info->integralCoeff
+ << " derivativeCoeff: " << info->derivativeCoeff
<< " feedFwdOffset: " << info->feedFwdOffset
<< " feedFwdGain: " << info->feedFwdGain
<< " integralLimit.min: " << info->integralLimit.min
@@ -54,6 +56,7 @@
<< " outLim.max: " << info->outLim.max
<< " slewNeg: " << info->slewNeg << " slewPos: " << info->slewPos
<< " last_output: " << info->lastOutput
+ << " last_error: " << info->lastError
<< " integral: " << info->integral << std::endl;
return;
diff --git a/pid/zone.cpp b/pid/zone.cpp
index 585cbeb..cac4577 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -141,6 +141,16 @@
return _minThermalOutputSetPt;
}
+uint64_t DbusPidZone::getCycleIntervalTime(void) const
+{
+ return _cycleTime.cycleIntervalTimeMS;
+}
+
+uint64_t DbusPidZone::getUpdateThermalsCycle(void) const
+{
+ return _cycleTime.updateThermalsTimeMS;
+}
+
void DbusPidZone::addFanPID(std::unique_ptr<Controller> pid)
{
_fans.push_back(std::move(pid));
diff --git a/pid/zone.hpp b/pid/zone.hpp
index 179b24d..41514a7 100644
--- a/pid/zone.hpp
+++ b/pid/zone.hpp
@@ -36,14 +36,14 @@
{
public:
DbusPidZone(int64_t zone, double minThermalOutput, double failSafePercent,
- const SensorManager& mgr, sdbusplus::bus_t& bus,
- const char* objPath, bool defer) :
+ conf::CycleTime cycleTime, const SensorManager& mgr,
+ sdbusplus::bus_t& bus, const char* objPath, bool defer) :
ModeObject(bus, objPath,
defer ? ModeObject::action::defer_emit
: ModeObject::action::emit_object_added),
_zoneId(zone), _maximumSetPoint(),
_minThermalOutputSetPt(minThermalOutput),
- _failSafePercent(failSafePercent), _mgr(mgr)
+ _failSafePercent(failSafePercent), _cycleTime(cycleTime), _mgr(mgr)
{
if (loggingEnabled)
{
@@ -68,6 +68,8 @@
void clearRPMCeilings(void) override;
double getFailSafePercent(void) const override;
double getMinThermalSetPoint(void) const;
+ uint64_t getCycleIntervalTime(void) const override;
+ uint64_t getUpdateThermalsCycle(void) const override;
Sensor* getSensor(const std::string& name) override;
void determineMaxSetPointRequest(void) override;
@@ -107,6 +109,7 @@
bool _redundantWrite = false;
const double _minThermalOutputSetPt;
const double _failSafePercent;
+ const conf::CycleTime _cycleTime;
std::set<std::string> _failSafeSensors;
diff --git a/pid/zone_interface.hpp b/pid/zone_interface.hpp
index 7d59497..d7f73b4 100644
--- a/pid/zone_interface.hpp
+++ b/pid/zone_interface.hpp
@@ -87,6 +87,10 @@
*/
virtual double getFailSafePercent() const = 0;
+ /** Return the zone's cycle time settings */
+ virtual uint64_t getCycleIntervalTime(void) const = 0;
+ virtual uint64_t getUpdateThermalsCycle(void) const = 0;
+
/** Return if the zone is set to manual mode. false equates to automatic
* mode (the default).
*/
diff --git a/test/pid_json_unittest.cpp b/test/pid_json_unittest.cpp
index f243212..b6963cd 100644
--- a/test/pid_json_unittest.cpp
+++ b/test/pid_json_unittest.cpp
@@ -50,6 +50,7 @@
"samplePeriod": 0.1,
"proportionalCoeff": 0.0,
"integralCoeff": 0.0,
+ "derivativeCoeff": 0.0,
"feedFwdOffsetCoeff": 0.0,
"feedFwdGainCoeff": 0.010,
"integralLimit_min": 0.0,
@@ -95,6 +96,7 @@
"samplePeriod": 0.1,
"proportionalCoeff": 0.0,
"integralCoeff": 0.0,
+ "derivativeCoeff": 0.0,
"feedFwdOffsetCoeff": 0.0,
"feedFwdGainCoeff": 0.010,
"integralLimit_min": 0.0,
@@ -206,5 +208,57 @@
EXPECT_DOUBLE_EQ(zoneConfig[1].minThermalOutput, 3000.0);
}
+TEST(ZoneFromJson, getCycleInterval)
+{
+ // Parse a valid configuration with one zone and one PID and the zone have
+ // cycleIntervalTime and updateThermalsTime parameters.
+
+ std::map<int64_t, conf::PIDConf> pidConfig;
+ std::map<int64_t, conf::ZoneConfig> zoneConfig;
+
+ auto j2 = R"(
+ {
+ "zones" : [{
+ "id": 1,
+ "minThermalOutput": 3000.0,
+ "failsafePercent": 75.0,
+ "cycleIntervalTimeMS": 1000.0,
+ "updateThermalsTimeMS": 1000.0,
+ "pids": [{
+ "name": "fan1-5",
+ "type": "fan",
+ "inputs": ["fan1", "fan5"],
+ "setpoint": 90.0,
+ "pid": {
+ "samplePeriod": 0.1,
+ "proportionalCoeff": 0.0,
+ "integralCoeff": 0.0,
+ "derivativeCoeff": 0.0,
+ "feedFwdOffsetCoeff": 0.0,
+ "feedFwdGainCoeff": 0.010,
+ "integralLimit_min": 0.0,
+ "integralLimit_max": 0.0,
+ "outLim_min": 30.0,
+ "outLim_max": 100.0,
+ "slewNeg": 0.0,
+ "slewPos": 0.0
+ }
+ }]
+ }]
+ }
+ )"_json;
+
+ std::tie(pidConfig, zoneConfig) = buildPIDsFromJson(j2);
+ EXPECT_EQ(pidConfig.size(), 1);
+ EXPECT_EQ(zoneConfig.size(), 1);
+
+ EXPECT_EQ(pidConfig[1]["fan1-5"].type, "fan");
+ EXPECT_EQ(zoneConfig[1].cycleTime.cycleIntervalTimeMS, 1000);
+ // updateThermalsTimeMS would be updated as updateThermalsTimeMS /
+ // cycleIntervalTimeMS
+ EXPECT_EQ(zoneConfig[1].cycleTime.updateThermalsTimeMS, 1);
+ EXPECT_DOUBLE_EQ(zoneConfig[1].minThermalOutput, 3000.0);
+}
+
} // namespace
} // namespace pid_control
diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp
index 275dfe6..2c0e147 100644
--- a/test/pid_zone_unittest.cpp
+++ b/test/pid_zone_unittest.cpp
@@ -50,14 +50,15 @@
int64_t zone = 1;
double minThermalOutput = 1000.0;
double failSafePercent = 0.75;
+ conf::CycleTime cycleTime;
double d;
std::vector<std::string> properties;
SetupDbusObject(&sdbus_mock_mode, defer, objPath, modeInterface, properties,
&d);
- DbusPidZone p(zone, minThermalOutput, failSafePercent, m, bus_mock_mode,
- objPath, defer);
+ DbusPidZone p(zone, minThermalOutput, failSafePercent, cycleTime, m,
+ bus_mock_mode, objPath, defer);
// Success.
}
@@ -87,7 +88,7 @@
properties, &property_index);
zone = std::make_unique<DbusPidZone>(zoneId, minThermalOutput,
- failSafePercent, mgr,
+ failSafePercent, cycleTime, mgr,
bus_mock_mode, objPath, defer);
}
@@ -104,6 +105,7 @@
bool defer = true;
const char* objPath = "/path/";
SensorManager mgr;
+ conf::CycleTime cycleTime;
std::unique_ptr<DbusPidZone> zone;
};
diff --git a/test/zone_mock.hpp b/test/zone_mock.hpp
index f2620d5..9c80a50 100644
--- a/test/zone_mock.hpp
+++ b/test/zone_mock.hpp
@@ -36,6 +36,9 @@
MOCK_CONST_METHOD0(getFailSafeMode, bool());
MOCK_CONST_METHOD0(getFailSafePercent, double());
+ MOCK_CONST_METHOD0(getCycleIntervalTime, uint64_t());
+ MOCK_CONST_METHOD0(getUpdateThermalsCycle, uint64_t());
+
MOCK_METHOD1(getSensor, Sensor*(const std::string&));
MOCK_METHOD0(initializeLog, void());