For each zone log sensor name with max setpoint
Add sensor name that has the maximum setpoint for a PID zone.
Log a debug message when the sensor is changed.
The name is also added to the log file for each log record.
Tested:
Override one CPU temperature sensor
busctl set-property xyz.openbmc_project.CPUSensor /xyz/openbmc_project/sensors/temperature/DTS_CPU1 xyz.openbmc_project.Sensor.Value Value d 82
Observed log message:
swampd[443]: PID Zone 0 max SetPoint 34.5546 requested by DTS_CPU1
Signed-off-by: Nirav Shah <nirav.j2.shah@intel.com>
Signed-off-by: Zhikui Ren <zhikui.ren@intel.com>
Change-Id: Ifc12cb9a106da1bf41dd35697210f74ba1b589db
diff --git a/pid/pidcontroller.hpp b/pid/pidcontroller.hpp
index cb92377..c3bbc2e 100644
--- a/pid/pidcontroller.hpp
+++ b/pid/pidcontroller.hpp
@@ -21,7 +21,7 @@
{
public:
PIDController(const std::string& id, ZoneInterface* owner) :
- Controller(), _owner(owner), _setpoint(0), _id(id)
+ Controller(), _owner(owner), _id(id)
{}
virtual ~PIDController()
@@ -58,12 +58,12 @@
protected:
ZoneInterface* _owner;
+ std::string _id;
private:
// parameters
ec::pid_info_t _pid_info;
- double _setpoint;
- std::string _id;
+ double _setpoint = 0;
double lastInput = std::numeric_limits<double>::quiet_NaN();
};
diff --git a/pid/stepwisecontroller.cpp b/pid/stepwisecontroller.cpp
index c8ec219..5c36778 100644
--- a/pid/stepwisecontroller.cpp
+++ b/pid/stepwisecontroller.cpp
@@ -101,7 +101,7 @@
}
else
{
- _owner->addSetPoint(value);
+ _owner->addSetPoint(value, _id);
}
return;
}
diff --git a/pid/thermalcontroller.cpp b/pid/thermalcontroller.cpp
index 8125349..71374a1 100644
--- a/pid/thermalcontroller.cpp
+++ b/pid/thermalcontroller.cpp
@@ -123,7 +123,7 @@
// bmc_set_pid_output
void ThermalController::outputProc(double value)
{
- _owner->addSetPoint(value);
+ _owner->addSetPoint(value, _id);
return;
}
diff --git a/pid/zone.cpp b/pid/zone.cpp
index af40a2b..1eda992 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -101,9 +101,18 @@
return _zoneId;
}
-void DbusPidZone::addSetPoint(double setpoint)
+void DbusPidZone::addSetPoint(double setPoint, const std::string& name)
{
- _SetPoints.push_back(setpoint);
+ _SetPoints.push_back(setPoint);
+ /*
+ * if there are multiple thermal controllers with the same
+ * value, pick the first one in the iterator
+ */
+ if (_maximumSetPoint < setPoint)
+ {
+ _maximumSetPoint = setPoint;
+ _maximumSetPointName = name;
+ }
}
void DbusPidZone::addRPMCeiling(double ceiling)
@@ -119,6 +128,7 @@
void DbusPidZone::clearSetPoints(void)
{
_SetPoints.clear();
+ _maximumSetPoint = 0;
}
double DbusPidZone::getFailSafePercent(void) const
@@ -126,7 +136,7 @@
return _failSafePercent;
}
-double DbusPidZone::getMinThermalSetpoint(void) const
+double DbusPidZone::getMinThermalSetPoint(void) const
{
return _minThermalOutputSetPt;
}
@@ -210,27 +220,46 @@
void DbusPidZone::determineMaxSetPointRequest(void)
{
- double max = 0;
std::vector<double>::iterator result;
-
- if (_SetPoints.size() > 0)
- {
- result = std::max_element(_SetPoints.begin(), _SetPoints.end());
- max = *result;
- }
+ double minThermalThreshold = getMinThermalSetPoint();
if (_RPMCeilings.size() > 0)
{
result = std::min_element(_RPMCeilings.begin(), _RPMCeilings.end());
- max = std::min(max, *result);
+ // if Max set point is larger than the lowest ceiling, reset to lowest
+ // ceiling.
+ if (*result < _maximumSetPoint)
+ {
+ _maximumSetPoint = *result;
+ // When using lowest ceiling, controller name is ceiling.
+ _maximumSetPointName = "Ceiling";
+ }
}
/*
* If the maximum RPM setpoint output is below the minimum RPM
* setpoint, set it to the minimum.
*/
- max = std::max(getMinThermalSetpoint(), max);
-
+ if (minThermalThreshold >= _maximumSetPoint)
+ {
+ _maximumSetPoint = minThermalThreshold;
+ _maximumSetPointName = "";
+ }
+ else if (_maximumSetPointName.compare(_maximumSetPointNamePrev))
+ {
+ std::cerr << "PID Zone " << _zoneId << " max SetPoint "
+ << _maximumSetPoint << " requested by "
+ << _maximumSetPointName;
+ for (const auto& sensor : _failSafeSensors)
+ {
+ if (sensor.find("Fan") == std::string::npos)
+ {
+ std::cerr << " " << sensor;
+ }
+ }
+ std::cerr << "\n";
+ _maximumSetPointNamePrev.assign(_maximumSetPointName);
+ }
if (tuningEnabled)
{
/*
@@ -240,17 +269,15 @@
*/
static constexpr auto setpointpath = "/etc/thermal.d/setpoint";
- fileParseRpm(setpointpath, max);
+ fileParseRpm(setpointpath, _maximumSetPoint);
// Allow per-zone setpoint files to override overall setpoint file
std::ostringstream zoneSuffix;
zoneSuffix << ".zone" << _zoneId;
std::string zoneSetpointPath = setpointpath + zoneSuffix.str();
- fileParseRpm(zoneSetpointPath, max);
+ fileParseRpm(zoneSetpointPath, _maximumSetPoint);
}
-
- _maximumSetPoint = max;
return;
}
@@ -260,7 +287,7 @@
* epoch_ms,setpt,fan1,fan2,fanN,sensor1,sensor2,sensorN,failsafe
*/
- _log << "epoch_ms,setpt";
+ _log << "epoch_ms,setpt,requester";
for (const auto& f : _fanInputs)
{
@@ -308,6 +335,7 @@
now.time_since_epoch())
.count();
_log << "," << _maximumSetPoint;
+ _log << "," << _maximumSetPointName;
}
for (const auto& f : _fanInputs)
diff --git a/pid/zone.hpp b/pid/zone.hpp
index 01dc62e..c73ad39 100644
--- a/pid/zone.hpp
+++ b/pid/zone.hpp
@@ -61,13 +61,13 @@
bool getFailSafeMode(void) const override;
int64_t getZoneID(void) const;
- void addSetPoint(double setpoint) override;
+ void addSetPoint(double setPoint, const std::string& name) override;
double getMaxSetPointRequest(void) const override;
void addRPMCeiling(double ceiling) override;
void clearSetPoints(void) override;
void clearRPMCeilings(void) override;
double getFailSafePercent(void) const override;
- double getMinThermalSetpoint(void) const;
+ double getMinThermalSetPoint(void) const;
Sensor* getSensor(const std::string& name) override;
void determineMaxSetPointRequest(void) override;
@@ -98,6 +98,8 @@
const int64_t _zoneId;
double _maximumSetPoint = 0;
+ std::string _maximumSetPointName;
+ std::string _maximumSetPointNamePrev;
bool _manualMode = false;
bool _redundantWrite = false;
const double _minThermalOutputSetPt;
diff --git a/pid/zone_interface.hpp b/pid/zone_interface.hpp
index b8da048..982e051 100644
--- a/pid/zone_interface.hpp
+++ b/pid/zone_interface.hpp
@@ -46,7 +46,7 @@
virtual double getCachedValue(const std::string& name) = 0;
/** Add a set point value for the Max Set Point computation. */
- virtual void addSetPoint(double setpoint) = 0;
+ virtual void addSetPoint(double setpoint, const std::string& name) = 0;
/** Clear all set points specified via addSetPoint */
virtual void clearSetPoints(void) = 0;
diff --git a/test/pid_stepwisecontroller_unittest.cpp b/test/pid_stepwisecontroller_unittest.cpp
index d9a7728..a832ed7 100644
--- a/test/pid_stepwisecontroller_unittest.cpp
+++ b/test/pid_stepwisecontroller_unittest.cpp
@@ -44,8 +44,8 @@
.WillOnce(Return(31.0)) // return 40
.WillOnce(Return(32.0)); // return 60
- EXPECT_CALL(z, addSetPoint(40.0)).Times(2);
- EXPECT_CALL(z, addSetPoint(60.0)).Times(1);
+ EXPECT_CALL(z, addSetPoint(40.0, "foo")).Times(2);
+ EXPECT_CALL(z, addSetPoint(60.0, "foo")).Times(1);
for (int ii = 0; ii < 3; ii++)
{
@@ -80,8 +80,8 @@
.WillOnce(Return(27.0)) // return 60
.WillOnce(Return(26.0)); // return 40
- EXPECT_CALL(z, addSetPoint(40.0)).Times(1);
- EXPECT_CALL(z, addSetPoint(60.0)).Times(2);
+ EXPECT_CALL(z, addSetPoint(40.0, "foo")).Times(1);
+ EXPECT_CALL(z, addSetPoint(60.0, "foo")).Times(2);
for (int ii = 0; ii < 3; ii++)
{
diff --git a/test/pid_thermalcontroller_unittest.cpp b/test/pid_thermalcontroller_unittest.cpp
index ef4de66..57a1b9f 100644
--- a/test/pid_thermalcontroller_unittest.cpp
+++ b/test/pid_thermalcontroller_unittest.cpp
@@ -104,7 +104,7 @@
EXPECT_FALSE(p == nullptr);
double value = 90.0;
- EXPECT_CALL(z, addSetPoint(value));
+ EXPECT_CALL(z, addSetPoint(value, "therm1"));
p->outputProc(value);
}
@@ -175,7 +175,7 @@
.WillOnce(Return(9.0))
.WillOnce(Return(7.0));
- EXPECT_CALL(z, addSetPoint(_)).Times(3);
+ EXPECT_CALL(z, addSetPoint(_, "therm1")).Times(3);
std::vector<double> lastReadings = {12.0, 12.0, 7.0};
for (auto& reading : lastReadings)
@@ -208,7 +208,7 @@
.WillOnce(Return(13.0))
.WillOnce(Return(14.0));
- EXPECT_CALL(z, addSetPoint(_)).Times(3);
+ EXPECT_CALL(z, addSetPoint(_, "therm1")).Times(3);
std::vector<double> lastReadings = {8.0, 8.0, 14.0};
for (auto& reading : lastReadings)
diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp
index 90d2a2f..172c818 100644
--- a/test/pid_zone_unittest.cpp
+++ b/test/pid_zone_unittest.cpp
@@ -161,14 +161,14 @@
TEST_F(PidZoneTest, RpmSetPoints_AddMaxClear_BehaveAsExpected)
{
// Tests addSetPoint, clearSetPoints, determineMaxSetPointRequest
- // and getMinThermalSetpoint.
+ // and getMinThermalSetPoint.
// At least one value must be above the minimum thermal setpoint used in
// the constructor otherwise it'll choose that value
std::vector<double> values = {100, 200, 300, 400, 500, 5000};
for (auto v : values)
{
- zone->addSetPoint(v);
+ zone->addSetPoint(v, "");
}
// This will pull the maximum RPM setpoint request.
@@ -180,7 +180,7 @@
// This will go through the RPM set point values and grab the maximum.
zone->determineMaxSetPointRequest();
- EXPECT_EQ(zone->getMinThermalSetpoint(), zone->getMaxSetPointRequest());
+ EXPECT_EQ(zone->getMinThermalSetPoint(), zone->getMaxSetPointRequest());
}
TEST_F(PidZoneTest, RpmSetPoints_AddBelowMinimum_BehavesAsExpected)
@@ -191,14 +191,14 @@
std::vector<double> values = {100, 200, 300, 400, 500};
for (auto v : values)
{
- zone->addSetPoint(v);
+ zone->addSetPoint(v, "");
}
// This will pull the maximum RPM setpoint request.
zone->determineMaxSetPointRequest();
// Verifies the value returned in the minimal thermal rpm set point.
- EXPECT_EQ(zone->getMinThermalSetpoint(), zone->getMaxSetPointRequest());
+ EXPECT_EQ(zone->getMinThermalSetPoint(), zone->getMaxSetPointRequest());
}
TEST_F(PidZoneTest, GetFailSafePercent_ReturnsExpected)
diff --git a/test/zone_mock.hpp b/test/zone_mock.hpp
index bab77e3..dd98c15 100644
--- a/test/zone_mock.hpp
+++ b/test/zone_mock.hpp
@@ -19,7 +19,7 @@
MOCK_METHOD0(initializeCache, void());
MOCK_METHOD1(getCachedValue, double(const std::string&));
MOCK_CONST_METHOD0(getRedundantWrite, bool(void));
- MOCK_METHOD1(addSetPoint, void(double));
+ MOCK_METHOD2(addSetPoint, void(double, const std::string&));
MOCK_METHOD0(clearSetPoints, void());
MOCK_METHOD1(addRPMCeiling, void(double));
MOCK_METHOD0(clearRPMCeilings, void());