Clean up power state handling and fix voltage events
Seperate isPowerOn into two functions, one to set up the
match and one to poll the boolean. This way a dbus connection
object isn't needed in the sensor. Use this new function to
allow the ADCSensor to only signal threshold crosses if the
sensor is in the correct state.
Tested-by: Verified no SEL events for ADC sensors were created
during power cycling.
Change-Id: Ida800ab478b85ac2cb5976fa3471411c5d4bdc88
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/src/ADCSensor.cpp b/src/ADCSensor.cpp
index 4a61655..eb2587c 100644
--- a/src/ADCSensor.cpp
+++ b/src/ADCSensor.cpp
@@ -41,13 +41,14 @@
std::shared_ptr<sdbusplus::asio::connection> &conn,
boost::asio::io_service &io, const std::string &sensorName,
std::vector<thresholds::Threshold> &&_thresholds,
- const double scaleFactor,
+ const double scaleFactor, PowerState readState,
const std::string &sensorConfiguration) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"), path,
std::move(_thresholds), sensorConfiguration,
"xyz.openbmc_project.Configuration.ADC", maxReading, minReading),
objServer(objectServer), scaleFactor(scaleFactor),
- inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), errCount(0)
+ readState(std::move(readState)), inputDev(io, open(path.c_str(), O_RDONLY)),
+ waitTimer(io), errCount(0)
{
sensorInterface = objectServer.add_interface(
"/xyz/openbmc_project/sensors/voltage/" + name,
@@ -66,6 +67,9 @@
}
setInitialProperties(conn);
setupRead();
+
+ // setup match
+ setupPowerMatch(conn);
}
ADCSensor::~ADCSensor()
@@ -160,5 +164,9 @@
void ADCSensor::checkThresholds(void)
{
+ if (readState == PowerState::on && !isPowerOn())
+ {
+ return;
+ }
thresholds::checkThresholds(this);
}
diff --git a/src/ADCSensorMain.cpp b/src/ADCSensorMain.cpp
index 4fff81a..8bd28b9 100644
--- a/src/ADCSensorMain.cpp
+++ b/src/ADCSensorMain.cpp
@@ -183,9 +183,23 @@
scaleFactor = variant_ns::visit(VariantToFloatVisitor(),
findScaleFactor->second);
}
+
+ auto findPowerOn = baseConfiguration->second.find("PowerState");
+ PowerState readState = PowerState::always;
+ if (findPowerOn != baseConfiguration->second.end())
+ {
+ std::string powerState = variant_ns::visit(VariantToStringVisitor(),
+ findPowerOn->second);
+ if (powerState == "On")
+ {
+ readState = PowerState::on;
+ };
+ }
+
sensors[sensorName] = std::make_unique<ADCSensor>(
path.string(), objectServer, dbusConnection, io, sensorName,
- std::move(sensorThresholds), scaleFactor, *interfacePath);
+ std::move(sensorThresholds), scaleFactor, readState,
+ *interfacePath);
}
}
diff --git a/src/CPUSensor.cpp b/src/CPUSensor.cpp
index d54263a..969a449 100644
--- a/src/CPUSensor.cpp
+++ b/src/CPUSensor.cpp
@@ -40,8 +40,8 @@
Sensor(boost::replace_all_copy(sensorName, " ", "_"), path,
std::move(_thresholds), sensorConfiguration, objectType, maxReading,
minReading),
- objServer(objectServer), dbusConnection(conn),
- inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), errCount(0)
+ objServer(objectServer), inputDev(io, open(path.c_str(), O_RDONLY)),
+ waitTimer(io), errCount(0)
{
sensorInterface = objectServer.add_interface(
@@ -60,7 +60,7 @@
"xyz.openbmc_project.Sensor.Threshold.Critical");
}
setInitialProperties(conn);
- isPowerOn(dbusConnection); // first call initializes
+ setupPowerMatch(conn);
setupRead();
}
@@ -123,7 +123,7 @@
if (errCount >= warnAfterErrorCount)
{
// only an error if power is on
- if (isPowerOn(dbusConnection))
+ if (isPowerOn())
{
// only print once
if (errCount == warnAfterErrorCount)
diff --git a/src/ExitAirTempSensor.cpp b/src/ExitAirTempSensor.cpp
index 510e595..c7e6e8e 100644
--- a/src/ExitAirTempSensor.cpp
+++ b/src/ExitAirTempSensor.cpp
@@ -271,6 +271,7 @@
}
setInitialProperties(conn);
setupMatches();
+ setupPowerMatch(conn);
}
ExitAirTempSensor::~ExitAirTempSensor()
@@ -351,7 +352,7 @@
}
// if fans are off, just make the exit temp equal to inlet
- if (!isPowerOn(dbusConnection))
+ if (!isPowerOn())
{
val = inletTemp;
return true;
diff --git a/src/TachSensor.cpp b/src/TachSensor.cpp
index b42062c..132ca69 100644
--- a/src/TachSensor.cpp
+++ b/src/TachSensor.cpp
@@ -43,9 +43,9 @@
Sensor(boost::replace_all_copy(fanName, " ", "_"), path,
std::move(_thresholds), sensorConfiguration, objectType,
limits.second, limits.first),
- objServer(objectServer), dbusConnection(conn),
- presence(std::move(presence)), redundancy(redundancy),
- inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), errCount(0)
+ objServer(objectServer), presence(std::move(presence)),
+ redundancy(redundancy), inputDev(io, open(path.c_str(), O_RDONLY)),
+ waitTimer(io), errCount(0)
{
sensorInterface = objectServer.add_interface(
"/xyz/openbmc_project/sensors/fan_tach/" + name,
@@ -64,7 +64,7 @@
"xyz.openbmc_project.Sensor.Threshold.Critical");
}
setInitialProperties(conn);
- isPowerOn(dbusConnection); // first call initializes
+ setupPowerMatch(conn); // first call initializes
setupRead();
}
@@ -131,28 +131,27 @@
}
else
{
- pollTime = sensorFailedPollTimeMs;
- errCount++;
- }
- if (errCount >= warnAfterErrorCount)
- {
- // only an error if power is on
- if (isPowerOn(dbusConnection))
+ if (!isPowerOn())
{
- // only print once
- if (errCount == warnAfterErrorCount)
- {
- std::cerr << "Failure to read sensor " << name << " at "
- << path << " ec:" << err << "\n";
- }
- updateValue(0);
+ errCount = 0;
+ updateValue(std::numeric_limits<double>::quiet_NaN());
}
else
{
- errCount = 0; // check power again in 10 cycles
- updateValue(std::numeric_limits<double>::quiet_NaN());
+ pollTime = sensorFailedPollTimeMs;
+ errCount++;
}
}
+ if (errCount >= warnAfterErrorCount)
+ {
+ // only print once
+ if (errCount == warnAfterErrorCount)
+ {
+ std::cerr << "Failure to read sensor " << name << " at " << path
+ << " ec:" << err << "\n";
+ }
+ updateValue(0);
+ }
}
responseStream.clear();
inputDev.close();
diff --git a/src/Utils.cpp b/src/Utils.cpp
index e4de9e0..c52f88f 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -28,6 +28,9 @@
const static constexpr char* powerObjectName =
"/xyz/openbmc_project/Chassis/Control/Power0";
+bool powerStatusOn = false;
+std::unique_ptr<sdbusplus::bus::match::match> powerMatch = nullptr;
+
bool getSensorConfiguration(
const std::string& type,
const std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
@@ -105,20 +108,20 @@
return true;
}
-// initially returns false, then sets up matches and returns status
-// should be called once first to initialize
-bool isPowerOn(const std::shared_ptr<sdbusplus::asio::connection>& conn)
+bool isPowerOn(void)
{
- static std::unique_ptr<sdbusplus::bus::match::match> powerMatch = nullptr;
- static bool powerStatusOn = false;
-
- if (powerMatch != nullptr)
+ if (!powerMatch)
{
- return powerStatusOn;
+ throw std::runtime_error("Power Match Not Created");
}
+ return powerStatusOn;
+}
+
+void setupPowerMatch(const std::shared_ptr<sdbusplus::asio::connection>& conn)
+{
// create a match for powergood changes, first time do a method call to
- // return the correct value
+ // cache the correct value
std::function<void(sdbusplus::message::message & message)> eventHandler =
[](sdbusplus::message::message& message) {
std::string objectName;
@@ -153,8 +156,6 @@
},
powerInterfaceName, powerObjectName, "org.freedesktop.DBus.Properties",
"Get", powerInterfaceName, "pgood");
-
- return powerStatusOn;
}
// replaces limits if MinReading and MaxReading are found.