commit a327923d4ba7b47b8ff07fe42ccb025fd9c894b9
author Chris Sides <christopher.sides@hpe.com> Mon Apr 24 16:08:13 2023 -0500
committer Christopher Sides <christopher.sides@hpe.com> Thu Mar 21 20:09:01 2024 +0000
tree 64cf2a2c4f1b88549a038c48ec93a443861c5f78
parent b2a0f2e0bfb5bc28fce737339f67df6a842cb726

add hysteresis param to parseThresholdsFromAttr()

Parameter 'hysteresis' with a default = NaN, is added to the header for
parseThresholdsFromAttr(), and existing calls to it now explicitly
specify a hysteresis value.

This is an alternative to having calls to parseThresholdFromAttr() use
a hardcoded Intel-specific value of '0.'

The 'NaN' value was chosen as default because it reflects the value
that was being written before a device-specific value was hardcoded in
to solve a specific issue.

Change was created based on the discussion here:
https://gerrit.openbmc.org/c/openbmc/dbus-sensors/+/61744/comments/38f6a
221_58a39817a

Tested: Confirmed the code builds and doesn't throw errors with OBMC
'local CI' testing. I believe heavier testing with actual Intel HW
isn't needed because this change simple moves where a value is specified
(from hardcode in function body to passed-in parameter) without
affecting any existing in-use functionality.

Change-Id: I1240b951d6e37adc1aad5ad3b19144f6baa64998
Signed-off-by: Chris Sides <christopher.sides@hpe.com>

src/IntelCPUSensor.cpp

diff --git a/src/IntelCPUSensor.cpp b/src/IntelCPUSensor.cpp
index 2bebcd8..e10b8ea 100644
--- a/src/IntelCPUSensor.cpp
+++ b/src/IntelCPUSensor.cpp
@@ -288,7 +288,7 @@
                     std::vector<thresholds::Threshold> newThresholds;
                     if (parseThresholdsFromAttr(
                             newThresholds, path,
-                            IntelCPUSensor::sensorScaleFactor, dtsOffset))
+                            IntelCPUSensor::sensorScaleFactor, dtsOffset, 0))
                     {
                         if (!std::equal(thresholds.begin(), thresholds.end(),
                                         newThresholds.begin(),

src/IntelCPUSensorMain.cpp

diff --git a/src/IntelCPUSensorMain.cpp b/src/IntelCPUSensorMain.cpp
index 005ed7e..b9ee586 100644
--- a/src/IntelCPUSensorMain.cpp
+++ b/src/IntelCPUSensorMain.cpp
@@ -362,7 +362,7 @@
             {
                 if (!parseThresholdsFromAttr(sensorThresholds, inputPathStr,
                                              IntelCPUSensor::sensorScaleFactor,
-                                             dtsOffset))
+                                             dtsOffset, 0))
                 {
                     std::cerr << "error populating thresholds for "
                               << sensorName << "\n";

src/Thresholds.cpp

diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp
index 6829041..7a64d1e 100644
--- a/src/Thresholds.cpp
+++ b/src/Thresholds.cpp
@@ -475,7 +475,7 @@
 bool parseThresholdsFromAttr(
     std::vector<thresholds::Threshold>& thresholdVector,
     const std::string& inputPath, const double& scaleFactor,
-    const double& offset)
+    const double& offset, const double& hysteresis)
 {
     const boost::container::flat_map<
         std::string, std::vector<std::tuple<const char*, thresholds::Level,
@@ -516,7 +516,8 @@
                         std::cout << "Threshold: " << attrPath << ": " << *val
                                   << "\n";
                     }
-                    thresholdVector.emplace_back(level, direction, *val, 0);
+                    thresholdVector.emplace_back(level, direction, *val,
+                                                 hysteresis);
                 }
             }
         }

src/Thresholds.hpp

diff --git a/src/Thresholds.hpp b/src/Thresholds.hpp
index fa7ce0c..352c55c 100644
--- a/src/Thresholds.hpp
+++ b/src/Thresholds.hpp
@@ -119,10 +119,15 @@
     std::vector<thresholds::Threshold>& thresholdVector,
     const std::string* matchLabel = nullptr, const int* sensorIndex = nullptr);
 
+// Sensors touched by parseThresholdFromAttr() are forcibly updated with given
+// parameters, so callers are encouraged to specify a sane hysteresis value for
+// their HW. For reference, the hysteresis fomula used in Sensor.hpp is:
+//  hysteresis.trigger = (max_val - min_val) * 0.01
 bool parseThresholdsFromAttr(
     std::vector<thresholds::Threshold>& thresholdVector,
     const std::string& inputPath, const double& scaleFactor,
-    const double& offset = 0);
+    const double& offset = 0,
+    const double& hysteresis = std::numeric_limits<double>::quiet_NaN());
 
 struct ThresholdDefinition
 {