monitor: Enhance tracing of fan & tach sensor states

Moved the determination of the allowed range of speeds of a tach sensor
from the fan to the tach sensor. Also changed the check for too many
nonfunctional tach sensors to return the count of nonfunctional tach
sensors of a fan so that this could be added to the tracing for when a
fan's functional state is updated.

Signed-off-by: Matthew Barth <msbarth@us.ibm.com>
Change-Id: Id65b8064d203bcc63a70f33d4783706e5cc07d2f
diff --git a/monitor/fan.cpp b/monitor/fan.cpp
index 9fe63c1..69a23b0 100644
--- a/monitor/fan.cpp
+++ b/monitor/fan.cpp
@@ -237,31 +237,18 @@
     return target;
 }
 
-bool Fan::tooManySensorsNonfunctional()
+size_t Fan::countNonFunctionalSensors()
 {
-    size_t numFailed =
-        std::count_if(_sensors.begin(), _sensors.end(),
-                      [](const auto& s) { return !s->functional(); });
-
-    return (numFailed >= _numSensorFailsForNonFunc);
+    return std::count_if(_sensors.begin(), _sensors.end(),
+                         [](const auto& s) { return !s->functional(); });
 }
 
 bool Fan::outOfRange(const TachSensor& sensor)
 {
     auto actual = static_cast<uint64_t>(sensor.getInput());
-    auto target = sensor.getTarget();
-    auto factor = sensor.getFactor();
-    auto offset = sensor.getOffset();
+    auto range = sensor.getRange(_deviation);
 
-    uint64_t min = target * (100 - _deviation) / 100;
-    uint64_t max = target * (100 + _deviation) / 100;
-
-    // TODO: openbmc/openbmc#2937 enhance this function
-    // either by making it virtual, or by predefining different
-    // outOfRange ops and selecting by yaml config
-    min = min * factor + offset;
-    max = max * factor + offset;
-    if ((actual < min) || (actual > max))
+    if ((actual < range.first) || (actual > range.second))
     {
         return true;
     }
@@ -271,39 +258,37 @@
 
 void Fan::updateState(TachSensor& sensor)
 {
+    auto range = sensor.getRange(_deviation);
     sensor.setFunctional(!sensor.functional());
-
     getLogger().log(
         fmt::format("Setting tach sensor {} functional state to {}. "
-                    "Actual speed: {} Target speed: {}",
-                    sensor.name(), sensor.functional(), sensor.getInput(),
-                    sensor.getTarget()));
+                    "[target = {}, input = {}, allowed range = ({} - {})]",
+                    sensor.name(), sensor.functional(), sensor.getTarget(),
+                    sensor.getInput(), range.first, range.second));
 
     // A zero value for _numSensorFailsForNonFunc means we aren't dealing
     // with fan FRU functional status, only sensor functional status.
     if (_numSensorFailsForNonFunc)
     {
+        auto numNonFuncSensors = countNonFunctionalSensors();
         // If the fan was nonfunctional and enough sensors are now OK,
-        // the fan can go back to functional
-        if (!_functional && !tooManySensorsNonfunctional())
+        // the fan can be set to functional
+        if (!_functional && !(numNonFuncSensors >= _numSensorFailsForNonFunc))
         {
-            getLogger().log(
-                fmt::format("Setting fan {} back to functional", _name));
-
+            getLogger().log(fmt::format("Setting fan {} to functional, number "
+                                        "of nonfunctional sensors = {}",
+                                        _name, numNonFuncSensors));
             updateInventory(true);
         }
 
         // If the fan is currently functional, but too many
         // contained sensors are now nonfunctional, update
-        // the whole fan nonfunctional.
-        if (_functional && tooManySensorsNonfunctional())
+        // the fan to nonfunctional.
+        if (_functional && (numNonFuncSensors >= _numSensorFailsForNonFunc))
         {
-            getLogger().log(fmt::format("Setting fan {} to nonfunctional "
-                                        "Sensor: {} "
-                                        "Actual speed: {} "
-                                        "Target speed: {}",
-                                        _name, sensor.name(), sensor.getInput(),
-                                        sensor.getTarget()));
+            getLogger().log(fmt::format("Setting fan {} to nonfunctional, "
+                                        "number of nonfunctional sensors = {}",
+                                        _name, numNonFuncSensors));
             updateInventory(false);
         }
     }
diff --git a/monitor/fan.hpp b/monitor/fan.hpp
index 91ea395..62514f6 100644
--- a/monitor/fan.hpp
+++ b/monitor/fan.hpp
@@ -176,10 +176,9 @@
     bool outOfRange(const TachSensor& sensor);
 
     /**
-     * @brief Returns true if too many sensors are nonfunctional
-     *        as defined by _numSensorFailsForNonFunc
+     * @brief Returns the number sensors that are nonfunctional
      */
-    bool tooManySensorsNonfunctional();
+    size_t countNonFunctionalSensors();
 
     /**
      * @brief Updates the Functional property in the inventory
diff --git a/monitor/tach_sensor.cpp b/monitor/tach_sensor.cpp
index dc02ade..0e30a10 100644
--- a/monitor/tach_sensor.cpp
+++ b/monitor/tach_sensor.cpp
@@ -26,6 +26,7 @@
 
 #include <experimental/filesystem>
 #include <functional>
+#include <utility>
 
 namespace phosphor
 {
@@ -153,6 +154,19 @@
     return _tachTarget;
 }
 
+std::pair<uint64_t, uint64_t> TachSensor::getRange(const size_t deviation) const
+{
+    // Determine min/max range applying the deviation
+    uint64_t min = getTarget() * (100 - deviation) / 100;
+    uint64_t max = getTarget() * (100 + deviation) / 100;
+
+    // Adjust the min/max range by applying the factor & offset
+    min = min * _factor + _offset;
+    max = max * _factor + _offset;
+
+    return std::make_pair(min, max);
+}
+
 void TachSensor::setFunctional(bool functional)
 {
     _functional = functional;
diff --git a/monitor/tach_sensor.hpp b/monitor/tach_sensor.hpp
index 88c8547..4a09f17 100644
--- a/monitor/tach_sensor.hpp
+++ b/monitor/tach_sensor.hpp
@@ -7,6 +7,7 @@
 #include <sdeventplus/utility/timer.hpp>
 
 #include <chrono>
+#include <utility>
 
 namespace phosphor
 {
@@ -278,6 +279,15 @@
         return false;
     }
 
+    /**
+     * @brief Get the current allowed range of speeds
+     *
+     * @param[in] deviation - The configured deviation(in percent) allowed
+     *
+     * @return pair - Min/Max range of speeds allowed
+     */
+    std::pair<uint64_t, uint64_t> getRange(const size_t deviation) const;
+
   private:
     /**
      * @brief Returns the match string to use for matching