Adding useful error messages when sensors ignored
Useful output for every case in which a configured sensor is skipped over
Adding DEBUG global boolean constant, similar to other dbus-sensors usage
Also useful output along success path when DEBUG true
Reformatted some long std::cerr statements to pass clang-format
Tested: With corresponding patches elsewhere,
all ISL68137 devices on my system were instantiated,
and periodically read from, by PSUSensor,
and showed up in the output of busctl.
Change-Id: I95f56216a9c5d7719503c82c0e4eb8815cdb2c08
Signed-off-by: Josh Lehan <krellan@google.com>
diff --git a/src/PSUSensor.cpp b/src/PSUSensor.cpp
index 7c9e109..e18f739 100644
--- a/src/PSUSensor.cpp
+++ b/src/PSUSensor.cpp
@@ -28,6 +28,8 @@
static constexpr const char* sensorPathPrefix = "/xyz/openbmc_project/sensors/";
+static constexpr bool DEBUG = false;
+
PSUSensor::PSUSensor(const std::string& path, const std::string& objectType,
sdbusplus::asio::object_server& objectServer,
std::shared_ptr<sdbusplus::asio::connection>& conn,
@@ -42,6 +44,15 @@
inputDev(io, open(path.c_str(), O_RDONLY)), waitTimer(io), errCount(0),
sensorFactor(factor)
{
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Constructed sensor: path " << path << " type "
+ << objectType << " config " << sensorConfiguration
+ << " typename " << sensorTypeName << " factor " << factor
+ << " min " << min << " max " << max << " name \""
+ << sensorName << "\"\n";
+ }
+
std::string dbusPath = sensorPathPrefix + sensorTypeName + name;
sensorInterface = objectServer.add_interface(
@@ -86,6 +97,7 @@
{
if (err == boost::system::errc::bad_file_descriptor)
{
+ std::cerr << "Bad file descriptor from " << path << "\n";
return;
}
std::istream responseStream(&readBuf);
@@ -98,6 +110,12 @@
float nvalue = std::stof(response);
responseStream.clear();
nvalue /= sensorFactor;
+
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Read " << path << " scale " << sensorFactor
+ << " value " << nvalue << "\n";
+ }
if (static_cast<double>(nvalue) != value)
{
updateValue(nvalue);
@@ -106,11 +124,14 @@
}
catch (const std::invalid_argument&)
{
+ std::cerr << "Could not parse " << response << " from path " << path
+ << "\n";
errCount++;
}
}
else
{
+ std::cerr << "System error " << err << " from path " << path << "\n";
errCount++;
}
@@ -130,6 +151,7 @@
int fd = open(path.c_str(), O_RDONLY);
if (fd < 0)
{
+ std::cerr << "Failed to open path " << path << "\n";
return;
}
inputDev.assign(fd);
@@ -137,6 +159,7 @@
waitTimer.async_wait([&](const boost::system::error_code& ec) {
if (ec == boost::asio::error::operation_aborted)
{
+ std::cerr << "Failed to reschedule wait for " << path << "\n";
return;
}
setupRead();
diff --git a/src/PSUSensorMain.cpp b/src/PSUSensorMain.cpp
index 41fc21c..8a940b8 100644
--- a/src/PSUSensorMain.cpp
+++ b/src/PSUSensorMain.cpp
@@ -27,6 +27,8 @@
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>
+static constexpr bool DEBUG = false;
+
static constexpr std::array<const char*, 3> sensorTypes = {
"xyz.openbmc_project.Configuration.pmbus",
"xyz.openbmc_project.Configuration.MAX34451",
@@ -148,6 +150,7 @@
{
ManagedObjectType sensorConfigs;
+ int numCreated = 0;
bool useCache = false;
// TODO may need only modify the ones that need to be changed.
@@ -180,7 +183,7 @@
std::ifstream nameFile(pmbusPath);
if (!nameFile.good())
{
- std::cerr << "Failure reading " << pmbusPath << "\n";
+ std::cerr << "Failure finding pmbus path " << pmbusPath << "\n";
continue;
}
@@ -191,6 +194,10 @@
if (std::find(pmbusNames.begin(), pmbusNames.end(), pmbusName) ==
pmbusNames.end())
{
+ // To avoid this error message, add your driver name to
+ // the pmbusNames vector at the top of this file.
+ std::cerr << "Driver name " << pmbusName
+ << " not found in sensor whitelist\n";
continue;
}
@@ -200,6 +207,7 @@
auto ret = directories.insert(directory.string());
if (!ret.second)
{
+ std::cerr << "Duplicate path " << directory.string() << "\n";
continue; // check if path has already been searched
}
@@ -224,6 +232,8 @@
}
catch (std::invalid_argument&)
{
+ std::cerr << "Error parsing bus " << busStr << " addr " << addrStr
+ << "\n";
continue;
}
@@ -271,12 +281,13 @@
!(confAddr = std::get_if<uint64_t>(&(configAddress->second))))
{
std::cerr
- << "Canot get bus or address, invalid configuration\n";
+ << "Cannot get bus or address, invalid configuration\n";
continue;
}
if ((*confBus != bus) || (*confAddr != addr))
{
+ std::cerr << "Configuration mismatch of bus or addr\n";
continue;
}
@@ -285,6 +296,8 @@
}
if (interfacePath == nullptr)
{
+ // To avoid this error message, add your export map entry,
+ // from Entity Manager, to sensorTypes at the top of this file.
std::cerr << "failed to find match for " << deviceName << "\n";
continue;
}
@@ -309,6 +322,7 @@
std::vector<std::string> psuNames;
do
{
+ // Individual string fields: Name, Name1, Name2, Name3, ...
psuNames.push_back(std::get<std::string>(findPSUName->second));
findPSUName = baseConfig->second.find("Name" + std::to_string(i++));
} while (findPSUName != baseConfig->second.end());
@@ -372,6 +386,11 @@
labelHead = label.substr(0, label.find(" "));
}
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Sensor label head " << labelHead << "\n";
+ }
+
checkPWMSensor(sensorPath, labelHead, *interfacePath, objectServer,
psuNames[0]);
@@ -430,6 +449,12 @@
std::visit(VariantToIntVisitor(), findScaleFactor->second);
}
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Sensor scaling factor " << factor << " string "
+ << strScaleFactor << "\n";
+ }
+
std::vector<thresholds::Threshold> sensorThresholds;
if (!parseThresholdsFromConfig(*sensorData, sensorThresholds))
@@ -449,6 +474,13 @@
std::string sensorName =
psuNames[nameIndex] + " " + findProperty->second.labelTypeName;
+ ++numCreated;
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Created " << numCreated
+ << " sensors so far: " << sensorName << "\n";
+ }
+
sensors[sensorName] = std::make_unique<PSUSensor>(
sensorPathStr, sensorType, objectServer, dbusConnection, io,
sensorName, std::move(sensorThresholds), *interfacePath,
@@ -462,6 +494,11 @@
std::make_unique<PSUCombineEvent>(
objectServer, io, *psuName, eventPathList, "OperationalStatus");
}
+
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Created total of " << numCreated << " sensors\n";
+ }
return;
}