commit 9154caabf34d0dc26bddf9507913f2b752800f51
author Willy Tu <wltu@google.com> Thu Dec 02 02:28:54 2021 -0800
committer Willy Tu <wltu@google.com> Wed Nov 16 05:30:43 2022 +0000
tree 76b1a66dda4616fcfa4b6e962b374b31703b4280
parent 0679e4b174822cd5801f320f631b2f4aa45debed

sensordatahandler: clamp the values instead of removing the sensor

Clamping the sensor values to prevent sensors from going missing.

Based on
https://github.com/openbmc/phosphor-host-ipmid/blob/5aae092cab08279d45c7914c466314b356164b7c/dbus-sdr/sensorutils.cpp#L258-L304

Clamping the threshold value to correct the result of negative threshold
value.

Tested:
Set 'sensorUnits1: 0x80' in config for sensor cpu0_abcd_cur which has
low threshold -1 and high threshold 7.6.
Before:
~# ipmitool sensor | grep cpu0_abcd_cur
cpu0_abcd_cur | 0.022      | Amps       | ok    | na        | 4.020     | 4.020     | 7.587     | 7.587     | na
After:
~# ipmitool sensor | grep cpu0_abcd_cur
cpu0_abcd_cur | -0.135     | Amps       | ok    | na        | -0.998    | -0.998    | 7.587     | 7.587     | na

Change-Id: Ie5af79ccbe8ab9660755f1c9d281e49a08d01309
Signed-off-by: Willy Tu <wltu@google.com>
Signed-off-by: JeffLin <JeffLin2@quantatw.com>
Signed-off-by: Willy Tu <wltu@google.com>

sensordatahandler.hpp

diff --git a/sensordatahandler.hpp b/sensordatahandler.hpp
index 89d00d0..601f5cd 100644
--- a/sensordatahandler.hpp
+++ b/sensordatahandler.hpp
@@ -258,26 +258,20 @@
     constexpr uint8_t sensorUnitsSignedBits = 2 << 6;
     constexpr uint8_t signedDataFormat = 0x80;
     // if sensorUnits1 [7:6] = 10b, sensor is signed
+    int32_t minClamp;
+    int32_t maxClamp;
     if ((sensorInfo.sensorUnits1 & sensorUnitsSignedBits) == signedDataFormat)
     {
-        if (rawData > std::numeric_limits<int8_t>::max() ||
-            rawData < std::numeric_limits<int8_t>::lowest())
-        {
-            log<level::ERR>("Value out of range");
-            throw std::out_of_range("Value out of range");
-        }
-        setReading(static_cast<int8_t>(rawData), &response);
+        minClamp = std::numeric_limits<int8_t>::lowest();
+        maxClamp = std::numeric_limits<int8_t>::max();
     }
     else
     {
-        if (rawData > std::numeric_limits<uint8_t>::max() ||
-            rawData < std::numeric_limits<uint8_t>::lowest())
-        {
-            log<level::ERR>("Value out of range");
-            throw std::out_of_range("Value out of range");
-        }
-        setReading(static_cast<uint8_t>(rawData), &response);
+        minClamp = std::numeric_limits<uint8_t>::lowest();
+        maxClamp = std::numeric_limits<uint8_t>::max();
     }
+    setReading(static_cast<uint8_t>(std::clamp(rawData, minClamp, maxClamp)),
+               &response);
 
     if (!std::isfinite(value))
     {

sensorhandler.cpp

diff --git a/sensorhandler.cpp b/sensorhandler.cpp
index 30cee8b..e04319c 100644
--- a/sensorhandler.cpp
+++ b/sensorhandler.cpp
@@ -690,6 +690,21 @@
     ipmi::PropertyMap warnThresholds;
     ec = ipmi::getAllDbusProperties(ctx, service, info.sensorPath,
                                     warningThreshIntf, warnThresholds);
+    int32_t minClamp;
+    int32_t maxClamp;
+    int32_t rawData;
+    constexpr uint8_t sensorUnitsSignedBits = 2 << 6;
+    constexpr uint8_t signedDataFormat = 0x80;
+    if ((info.sensorUnits1 & sensorUnitsSignedBits) == signedDataFormat)
+    {
+        minClamp = std::numeric_limits<int8_t>::lowest();
+        maxClamp = std::numeric_limits<int8_t>::max();
+    }
+    else
+    {
+        minClamp = std::numeric_limits<uint8_t>::lowest();
+        maxClamp = std::numeric_limits<uint8_t>::max();
+    }
     if (!ec)
     {
         double warnLow = ipmi::mappedVariant<double>(
@@ -702,8 +717,9 @@
         if (std::isfinite(warnLow))
         {
             warnLow *= std::pow(10, info.scale - info.exponentR);
-            resp.lowerNonCritical = static_cast<uint8_t>(
-                round((warnLow - info.scaledOffset) / info.coefficientM));
+            rawData = round((warnLow - info.scaledOffset) / info.coefficientM);
+            resp.lowerNonCritical =
+                static_cast<uint8_t>(std::clamp(rawData, minClamp, maxClamp));
             resp.validMask |= static_cast<uint8_t>(
                 ipmi::sensor::ThresholdMask::NON_CRITICAL_LOW_MASK);
         }
@@ -711,8 +727,9 @@
         if (std::isfinite(warnHigh))
         {
             warnHigh *= std::pow(10, info.scale - info.exponentR);
-            resp.upperNonCritical = static_cast<uint8_t>(
-                round((warnHigh - info.scaledOffset) / info.coefficientM));
+            rawData = round((warnHigh - info.scaledOffset) / info.coefficientM);
+            resp.upperNonCritical =
+                static_cast<uint8_t>(std::clamp(rawData, minClamp, maxClamp));
             resp.validMask |= static_cast<uint8_t>(
                 ipmi::sensor::ThresholdMask::NON_CRITICAL_HIGH_MASK);
         }
@@ -733,8 +750,9 @@
         if (std::isfinite(critLow))
         {
             critLow *= std::pow(10, info.scale - info.exponentR);
-            resp.lowerCritical = static_cast<uint8_t>(
-                round((critLow - info.scaledOffset) / info.coefficientM));
+            rawData = round((critLow - info.scaledOffset) / info.coefficientM);
+            resp.lowerCritical =
+                static_cast<uint8_t>(std::clamp(rawData, minClamp, maxClamp));
             resp.validMask |= static_cast<uint8_t>(
                 ipmi::sensor::ThresholdMask::CRITICAL_LOW_MASK);
         }
@@ -742,8 +760,9 @@
         if (std::isfinite(critHigh))
         {
             critHigh *= std::pow(10, info.scale - info.exponentR);
-            resp.upperCritical = static_cast<uint8_t>(
-                round((critHigh - info.scaledOffset) / info.coefficientM));
+            rawData = round((critHigh - info.scaledOffset) / info.coefficientM);
+            resp.upperCritical =
+                static_cast<uint8_t>(std::clamp(rawData, minClamp, maxClamp));
             resp.validMask |= static_cast<uint8_t>(
                 ipmi::sensor::ThresholdMask::CRITICAL_HIGH_MASK);
         }