commit a3e8f2a391f389ffb2c379ca0c181e67de43824e
author Yong Zhao <yozhao@google.com> Sat Jan 09 02:22:43 2021 +0000
committer Yong Zhao <yozhao@google.com> Fri Jan 15 00:53:35 2021 +0000
tree 31252d4e6b17daf3c03fbabe1356b3345ff0830e
parent 6e6ac0f850e94c4729456c41ba53f5a84437e59b

Fan: Eliminate one unnecessary loop by using known fanType

Given we have known the fanType, we don't need to loop through all fan
types. In order to use fanType as the index for the sensorTypes array,
we need to change "enum class FanTypes" to "enum FanTypes". Otherwise,
we will get a compiling error.

This also fixes an issue that an I2C fan could be incorrectly processed
as a Nuvoton or Aspee fan when they are used together.

Tested: the fan sensors are seen in the ipmitool sensor list.

Signed-off-by: Yong Zhao <yozhao@google.com>
Change-Id: I474a149e01b06a5a2e883aaa840486ef97965775

src/FanMain.cpp

diff --git a/src/FanMain.cpp b/src/FanMain.cpp
index 1853bb8..4306f16 100644
--- a/src/FanMain.cpp
+++ b/src/FanMain.cpp
@@ -44,21 +44,27 @@
 
 namespace fs = std::filesystem;
 
+// The following two structures need to be consistent
 static constexpr std::array<const char*, 3> sensorTypes = {
     "xyz.openbmc_project.Configuration.AspeedFan",
     "xyz.openbmc_project.Configuration.I2CFan",
     "xyz.openbmc_project.Configuration.NuvotonFan"};
+
+enum FanTypes
+{
+    aspeed = 0,
+    i2c,
+    nuvoton,
+    max,
+};
+
+static_assert(std::tuple_size<decltype(sensorTypes)>::value == FanTypes::max,
+              "sensorTypes element number is not equal to FanTypes number");
+
 constexpr const char* redundancyConfiguration =
     "xyz.openbmc_project.Configuration.FanRedundancy";
 static std::regex inputRegex(R"(fan(\d+)_input)");
 
-enum class FanTypes
-{
-    aspeed,
-    i2c,
-    nuvoton
-};
-
 // todo: power supply fan redundancy
 std::optional<RedundancySensor> systemRedundancy;
 
@@ -141,7 +147,6 @@
         sensorsChanged,
     size_t retries = 0)
 {
-
     auto getter = std::make_shared<GetSensorConfiguration>(
         dbusConnection,
         std::move([&io, &objectServer, &tachSensors, &pwmSensors,
@@ -200,22 +205,17 @@
                 {
                     // find the base of the configuration to see if indexes
                     // match
-                    for (const char* type : sensorTypes)
-                    {
-                        auto sensorBaseFind = sensor.second.find(type);
-                        if (sensorBaseFind != sensor.second.end())
-                        {
-                            baseConfiguration = &(*sensorBaseFind);
-                            interfacePath = &(sensor.first.str);
-                            baseType = type;
-                            break;
-                        }
-                    }
-                    if (baseConfiguration == nullptr)
+                    auto sensorBaseFind =
+                        sensor.second.find(sensorTypes[fanType]);
+                    if (sensorBaseFind == sensor.second.end())
                     {
                         continue;
                     }
 
+                    baseConfiguration = &(*sensorBaseFind);
+                    interfacePath = &(sensor.first.str);
+                    baseType = sensorTypes[fanType];
+
                     auto findIndex = baseConfiguration->second.find("Index");
                     if (findIndex == baseConfiguration->second.end())
                     {
@@ -237,9 +237,7 @@
                         sensorData = &(sensor.second);
                         break;
                     }
-                    else if (baseType ==
-                             std::string(
-                                 "xyz.openbmc_project.Configuration.I2CFan"))
+                    else if (fanType == FanTypes::i2c)
                     {
                         auto findBus = baseConfiguration->second.find("Bus");
                         auto findAddress =