Standardize read errors
Each sensor handled read errors their own way,
making it inconsistant. This helps to align them all
in the base class, so that error handling happens the
same for each sensor. It also aligns the power state
change handling.
Tested: Tested DC Cycling and Enabling/Disabling ME to
make sure functional change correctly
Change-Id: I1a191d27629602e1ca3871d933af07b15bf9f331
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/src/IpmbSensor.cpp b/src/IpmbSensor.cpp
index f36e836..d8b0d4f 100644
--- a/src/IpmbSensor.cpp
+++ b/src/IpmbSensor.cpp
@@ -69,9 +69,9 @@
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(thresholdData), sensorConfiguration,
"xyz.openbmc_project.Configuration.ExitAirTemp", ipmbMaxReading,
- ipmbMinReading),
- deviceAddress(deviceAddress), readState(PowerState::on),
- objectServer(objectServer), dbusConnection(conn), waitTimer(io)
+ ipmbMinReading, PowerState::on),
+ deviceAddress(deviceAddress), objectServer(objectServer),
+ dbusConnection(conn), waitTimer(io)
{
std::string dbusPath = sensorPathPrefix + sensorTypeName + "/" + name;
@@ -89,7 +89,6 @@
dbusPath, "xyz.openbmc_project.Sensor.Threshold.Critical");
}
association = objectServer.add_interface(dbusPath, association::interface);
- setupPowerMatch(conn);
}
IpmbSensor::~IpmbSensor()
@@ -226,68 +225,55 @@
void IpmbSensor::checkThresholds(void)
{
- if (readState == PowerState::on && !isPowerOn())
- {
- return;
- }
- else if (readState == PowerState::biosPost && !hasBiosPost())
- {
- return;
- }
thresholds::checkThresholds(this);
}
-void IpmbSensor::processError(void)
-{
- static constexpr size_t errorFilter = 2;
-
- if (!errorCount)
- {
- std::cerr << "Invalid data from device: " << name << "\n";
- }
- else if (errorCount > errorFilter)
- {
- updateValue(0);
- }
- if (errorCount < std::numeric_limits<uint8_t>::max())
- {
- errorCount++;
- }
-}
-
-double IpmbSensor::processReading(const std::vector<uint8_t>& data)
+bool IpmbSensor::processReading(const std::vector<uint8_t>& data, double& resp)
{
switch (readingFormat)
{
case (ReadingFormat::byte0):
- return data[0];
+ resp = data[0];
+ return true;
case (ReadingFormat::byte3):
if (data.size() < 4)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
- return 0;
+ if (!errCount)
+ {
+ std::cerr << "Invalid data length returned for " << name
+ << "\n";
+ }
+ return false;
}
- return data[3];
+ resp = data[3];
+ return true;
case (ReadingFormat::elevenBit):
if (data.size() < 5)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
- return 0;
+ if (!errCount)
+ {
+ std::cerr << "Invalid data length returned for " << name
+ << "\n";
+ }
+ return false;
}
- return ((data[4] << 8) | data[3]);
+ resp = ((data[4] << 8) | data[3]);
+ return true;
case (ReadingFormat::elevenBitShift):
if (data.size() < 5)
{
- std::cerr << "Invalid data length returned for " << name
- << "\n";
- return 0;
+ if (!errCount)
+ {
+ std::cerr << "Invalid data length returned for " << name
+ << "\n";
+ }
+ return false;
}
- return ((data[4] << 8) | data[3]) >> 3;
+ resp = ((data[4] << 8) | data[3]) >> 3;
+ return true;
default:
throw std::runtime_error("Invalid reading type");
}
@@ -305,7 +291,6 @@
}
if (!isPowerOn() && readState != PowerState::always)
{
- updateValue(0);
read();
return;
}
@@ -315,14 +300,7 @@
const int& status = std::get<0>(response);
if (ec || status)
{
- processError();
- updateValue(0);
- read();
- return;
- }
- if (!isPowerOn() && readState != PowerState::always)
- {
- updateValue(0);
+ incrementError();
read();
return;
}
@@ -338,17 +316,24 @@
}
if (data.empty())
{
- processError();
+ incrementError();
read();
return;
}
- double value = processReading(data);
+
+ double value = 0;
+
+ if (!processReading(data, value))
+ {
+ incrementError();
+ read();
+ return;
+ }
/* Adjust value as per scale and offset */
value = (value * scaleVal) + offsetVal;
updateValue(value);
read();
- errorCount = 0; // success
},
"xyz.openbmc_project.Ipmi.Channel.Ipmb",
"/xyz/openbmc_project/Ipmi/Channel/Ipmb", "org.openbmc.Ipmb",