commit 49cfba951d35c2512d66e756d0121d1161b46a0e
author Josh Lehan <krellan@google.com> Tue Oct 08 16:50:42 2019 -0700
committer Josh Lehan <krellan@google.com> Thu Oct 10 16:48:37 2019 -0700
tree 7ccb6563fb96a82e59fedfb078de094ebeb5b920
parent 0830c7b8db046b1e3e970b6e1cf42015e0b7e31c

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>

src/PSUSensorMain.cpp

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;
 }