sensor: Refactor get sensor thresholds command

Using GetManagedObjects to get the thresholds could take upto
15~20 sec for the core temperatures. The time taken to fetch
threshold properties via GetManagedObjects is proportional to the
number of objects under the path. Since the dbus object path is
available in the yaml it is performant to read the threshold
properties directly.

Change-Id: Ic22d8f4d73d8ce4eabdffd6fc7af23f73b981f66
Signed-off-by: Tom Joseph <tomjoseph@in.ibm.com>

3 files changed, 84 insertions, 119 deletions

diff --git a/sensorhandler.cpp b/sensorhandler.cpp
index 1bd10b7..e52ab69 100644
--- a/sensorhandler.cpp
+++ b/sensorhandler.cpp
@@ -675,64 +675,87 @@ ipmi_ret_t ipmi_sen_get_sensor_reading(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
     return rc;
 }
 
-ipmi_ret_t ipmi_sen_get_sensor_thresholds(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
-        ipmi_request_t request, ipmi_response_t response,
-        ipmi_data_len_t data_len, ipmi_context_t context)
+void getSensorThresholds(uint8_t sensorNum,
+                         get_sdr::GetSensorThresholdsResponse* response)
 {
-    constexpr auto warningThresholdInterface =
+    constexpr auto warningThreshIntf =
         "xyz.openbmc_project.Sensor.Threshold.Warning";
-    constexpr auto criticalThresholdInterface =
+    constexpr auto criticalThreshIntf =
         "xyz.openbmc_project.Sensor.Threshold.Critical";
-    constexpr auto valueInterface =
-        "xyz.openbmc_project.Sensor.Value";
-    constexpr auto sensorRoot = "/xyz/openbmc_project/sensors";
 
-    ipmi::sensor::Thresholds thresholds =
+    sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
+
+    const auto iter = sensors.find(sensorNum);
+    const auto info = iter->second;
+
+    auto service = ipmi::getService(bus, info.sensorInterface, info.sensorPath);
+
+    auto warnThresholds = ipmi::getAllDbusProperties(bus,
+                                                     service,
+                                                     info.sensorPath,
+                                                     warningThreshIntf);
+
+    double warnLow = warnThresholds["WarningLow"].get<int64_t>();
+    double warnHigh = warnThresholds["WarningHigh"].get<int64_t>();
+
+    if (warnLow != 0)
     {
-        {
-            warningThresholdInterface,
-            {
-                {
-                    "WarningLow",
-                    ipmi::sensor::ThresholdMask::NON_CRITICAL_LOW_MASK,
-                    ipmi::sensor::ThresholdIndex::NON_CRITICAL_LOW_IDX
-                },
-                {
-                    "WarningHigh",
-                    ipmi::sensor::ThresholdMask::NON_CRITICAL_HIGH_MASK,
-                    ipmi::sensor::ThresholdIndex::NON_CRITICAL_HIGH_IDX
-                }
-            }
-        },
-        {
-            criticalThresholdInterface,
-            {
-               {
-                    "CriticalLow",
-                    ipmi::sensor::ThresholdMask::CRITICAL_LOW_MASK,
-                    ipmi::sensor::ThresholdIndex::CRITICAL_LOW_IDX
-                },
-                {
-                    "CriticalHigh",
-                    ipmi::sensor::ThresholdMask::CRITICAL_HIGH_MASK,
-                    ipmi::sensor::ThresholdIndex::CRITICAL_HIGH_IDX
-                }
-            }
-        }
-    };
+        warnLow *= pow(10, info.scale - info.exponentR);
+        response->lowerNonCritical = static_cast<uint8_t>((
+            warnLow - info.scaledOffset) / info.coefficientM);
+        response->validMask |= static_cast<uint8_t>(
+                ipmi::sensor::ThresholdMask::NON_CRITICAL_LOW_MASK);
+    }
 
-    sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
+    if (warnHigh != 0)
+    {
+        warnHigh *= pow(10, info.scale - info.exponentR);
+        response->upperNonCritical = static_cast<uint8_t>((
+            warnHigh - info.scaledOffset) / info.coefficientM);
+        response->validMask |= static_cast<uint8_t>(
+                ipmi::sensor::ThresholdMask::NON_CRITICAL_HIGH_MASK);
+    }
+
+    auto critThresholds = ipmi::getAllDbusProperties(bus,
+                                                     service,
+                                                     info.sensorPath,
+                                                     criticalThreshIntf);
+    double critLow = critThresholds["CriticalLow"].get<int64_t>();
+    double critHigh = critThresholds["CriticalHigh"].get<int64_t>();
+
+    if (critLow != 0)
+    {
+        critLow *= pow(10, info.scale - info.exponentR);
+        response->lowerCritical = static_cast<uint8_t>((
+            critLow - info.scaledOffset) / info.coefficientM);
+        response->validMask |= static_cast<uint8_t>(
+                ipmi::sensor::ThresholdMask::CRITICAL_LOW_MASK);
+    }
+
+    if (critHigh != 0)
+    {
+        critHigh *= pow(10, info.scale - info.exponentR);
+        response->upperCritical = static_cast<uint8_t>((
+            critHigh - info.scaledOffset)/ info.coefficientM);
+        response->validMask |= static_cast<uint8_t>(
+                ipmi::sensor::ThresholdMask::CRITICAL_HIGH_MASK);
+    }
+}
+
+ipmi_ret_t ipmi_sen_get_sensor_thresholds(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
+                             ipmi_request_t request, ipmi_response_t response,
+                             ipmi_data_len_t data_len, ipmi_context_t context)
+{
+    constexpr auto valueInterface = "xyz.openbmc_project.Sensor.Value";
 
     if (*data_len != sizeof(uint8_t))
     {
+        *data_len = 0;
         return IPMI_CC_REQ_DATA_LEN_INVALID;
     }
-    auto sensorNum = *(reinterpret_cast<uint8_t*>(request));
-
-    auto responseData =
-        reinterpret_cast<get_sdr::GetSensorThresholdsResponse*>(response);
 
-    responseData->validMask = 0;
+    auto sensorNum = *(reinterpret_cast<const uint8_t *>(request));
+    *data_len = 0;
 
     const auto iter = sensors.find(sensorNum);
     if (iter == sensors.end())
@@ -740,69 +763,26 @@ ipmi_ret_t ipmi_sen_get_sensor_thresholds(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
         return IPMI_CC_SENSOR_INVALID;
     }
 
-    const auto sensorInfo = iter->second;
+    const auto info = iter->second;
 
     //Proceed only if the sensor value interface is implemented.
-    if (sensorInfo.propertyInterfaces.find(valueInterface) ==
-        sensorInfo.propertyInterfaces.end())
+    if (info.propertyInterfaces.find(valueInterface) ==
+        info.propertyInterfaces.end())
     {
         //return with valid mask as 0
         return IPMI_CC_OK;
     }
 
-    std::string service;
-    try
-    {
-        service = ipmi::getService(bus,
-                                   sensorInfo.sensorInterface,
-                                   sensorInfo.sensorPath);
-    }
-    catch (const std::runtime_error& e)
-    {
-        log<level::ERR>(e.what());
-        return IPMI_CC_UNSPECIFIED_ERROR;
-    }
-
-    //prevent divide by 0
-    auto coefficientM =
-        sensorInfo.coefficientM ? sensorInfo.coefficientM : 1;
+    auto responseData =
+        reinterpret_cast<get_sdr::GetSensorThresholdsResponse*>(response);
 
     try
     {
-        auto mngObjects = ipmi::getManagedObjects(bus,
-                          service,
-                          sensorRoot);
-
-        auto senIter = mngObjects.find(sensorInfo.sensorPath);
-        if (senIter == mngObjects.end())
-        {
-            return IPMI_CC_SENSOR_INVALID;
-        }
-
-        for (const auto& threshold : thresholds)
-        {
-            auto thresholdType = senIter->second.find(threshold.first);
-            if (thresholdType != senIter->second.end())
-            {
-                for (const auto& threshLevel : threshold.second)
-                {
-                    auto val = thresholdType->
-                        second[threshLevel.property].get<int64_t>();
-                    if (val != 0)
-                    {
-                        auto idx = static_cast<uint8_t>(threshLevel.idx);
-                        responseData->data[idx] = static_cast<uint8_t>(
-                            (val - sensorInfo.scaledOffset) / coefficientM);
-                        responseData->validMask |=
-                            static_cast<uint8_t>(threshLevel.maskValue);
-                    }
-                }
-            }
-        }
+        getSensorThresholds(sensorNum, responseData);
     }
-    catch (InternalFailure& e)
+    catch (std::exception& e)
     {
-        //Not able to get the values, reset the mask.
+        //Mask if the property is not present
         responseData->validMask = 0;
     }
 
diff --git a/sensorhandler.h b/sensorhandler.h
index e32405d..f5b5c94 100644
--- a/sensorhandler.h
+++ b/sensorhandler.h
@@ -241,8 +241,13 @@ inline void set_owner_lun_channel(uint8_t channel, SensorDataRecordKey* key)
  */
 struct GetSensorThresholdsResponse
 {
-    uint8_t validMask; //Indicates which values are valid
-    uint8_t data[6];   //Container for threshold values
+    uint8_t validMask;          //!< valid mask
+    uint8_t lowerNonCritical;   //!< lower non-critical threshold
+    uint8_t lowerCritical;      //!< lower critical threshold
+    uint8_t lowerNonRecoverable;//!< lower non-recoverable threshold
+    uint8_t upperNonCritical;   //!< upper non-critical threshold
+    uint8_t upperCritical;      //!< upper critical threshold
+    uint8_t upperNonRecoverable;//!< upper non-recoverable threshold
 } __attribute__((packed));
 
 // Body - full record
diff --git a/types.hpp b/types.hpp
index e77dc8e..eb4cf8c 100644
--- a/types.hpp
+++ b/types.hpp
@@ -197,26 +197,6 @@ enum class ThresholdMask
     CRITICAL_HIGH_MASK = 0x10,
 };
 
-enum class ThresholdIndex
-{
-    NON_CRITICAL_LOW_IDX = 0,
-    CRITICAL_LOW_IDX = 1,
-    NON_RECOVERABLE_LOW_IDX = 2,
-    NON_CRITICAL_HIGH_IDX = 3,
-    CRITICAL_HIGH_IDX = 4,
-    NON_RECOVERABLE_HIGH_IDX = 5,
-};
-
-struct ThresholdLevel
-{
-    std::string property;
-    ThresholdMask maskValue;
-    ThresholdIndex idx;
-};
-
-using SensorThresholds = std::vector<ThresholdLevel>;
-using Thresholds = std::map<std::string, SensorThresholds>;
-
 }// namespace sensor
 
 namespace network