path: root/meta-rcs/meta-talos/recipes-phosphor/fans/phosphor-pid-control/continue-if-sensor-not-present.patch

diff --git a/dbus/dbuspassive.cpp b/dbus/dbuspassive.cpp
index 806ef40..75b83f5 100644
--- a/dbus/dbuspassive.cpp
+++ b/dbus/dbuspassive.cpp
@@ -26,6 +26,10 @@
 #include <string>
 #include <variant>
 
+using Property = std::string;
+using Value = std::variant<int64_t, double, std::string, bool>;
+using PropertyMap = std::map<Property, Value>;
+
 std::unique_ptr<ReadInterface> DbusPassive::createDbusPassive(
     sdbusplus::bus::bus& bus, const std::string& type, const std::string& id,
     DbusHelperInterface* helper, const conf::SensorConfig* info,
@@ -61,6 +65,41 @@ std::unique_ptr<ReadInterface> DbusPassive::createDbusPassive(
         return nullptr;
     }
 
+    if (failed && (type == "fan"))
+    {
+        std::string path = getInventoryPath(id);
+
+        try
+        {
+            bool present = true;
+
+            auto pimMsg = tempBus.new_method_call("xyz.openbmc_project.Inventory.Manager", path.c_str(), "org.freedesktop.DBus.Properties", "GetAll");
+            pimMsg.append("xyz.openbmc_project.Inventory.Item");
+
+            PropertyMap propMap;
+
+            auto valueResponseMsg = bus.call(pimMsg);
+            valueResponseMsg.read(propMap);
+
+            // If no error was set, the data should all be there.
+            auto findPresent = propMap.find("Present");
+            if (findPresent != propMap.end())
+            {
+                present = std::get<bool>(findPresent->second);
+            }
+
+            // std::cerr << "DbusPassive::createDbusPassive() present: '" << present << "'\n";
+            if (!present)
+            {
+                failed = false;
+            }
+        }
+        catch (const std::exception& e)
+        {
+            // Ignore failure
+        }
+    }
+
     /* if these values are zero, they're ignored. */
     if (info->ignoreDbusMinMax)
     {
@@ -68,6 +107,7 @@ std::unique_ptr<ReadInterface> DbusPassive::createDbusPassive(
         settings.max = 0;
     }
 
+    // std::cerr << "DbusPassive::createDbusPassive() failed: " << failed << "\n";
     return std::make_unique<DbusPassive>(bus, type, id, helper, settings,
                                          failed, path, redundancy);
 }
@@ -78,8 +118,9 @@ DbusPassive::DbusPassive(
     bool failed, const std::string& path,
     const std::shared_ptr<DbusPassiveRedundancy>& redundancy) :
     ReadInterface(),
-    _bus(bus), _signal(bus, getMatch(type, id).c_str(), dbusHandleSignal, this),
-    _id(id), _helper(helper), _failed(failed), path(path),
+    _id(id), _type(type), _helper(helper), _bus(bus),
+    _signal(bus, getMatch(type, id).c_str(), dbusHandleSignal, this),
+    _failed(failed), path(path),
     redundancy(redundancy)
 
 {
@@ -109,19 +150,23 @@ void DbusPassive::setValue(double value)
 
 bool DbusPassive::getFailed(void) const
 {
+    // std::cerr << "DbusPassive::getFailed() redundancy: " << redundancy << "\n";
     if (redundancy)
     {
         const std::set<std::string>& failures = redundancy->getFailed();
         if (failures.find(path) != failures.end())
         {
+            // std::cerr << "DbusPassive::getFailed() redundancy has signalled failure!\n";
             return true;
         }
     }
+    // std::cerr << "DbusPassive::getFailed() _failed: " << _failed << "\n";
     return _failed;
 }
 
 void DbusPassive::setFailed(bool value)
 {
+    // std::cerr << "DbusPassive::setFailed(" << value << ")\n";
     _failed = value;
 }
 
@@ -189,6 +234,43 @@ int handleSensorValue(sdbusplus::message::message& msg, DbusPassive* owner)
         {
             asserted = std::get<bool>(criticalAlarmHigh->second);
         }
+
+        // std::cerr << "handleSensorValue() asserted: " << asserted << " type: '" << owner->_type << "'\n";
+        if (asserted && (owner->_type == "fan"))
+        {
+            std::string path = getInventoryPath(owner->_id);
+
+            try
+            {
+                bool present = true;
+
+                auto pimMsg = owner->_bus.new_method_call("xyz.openbmc_project.Inventory.Manager", path.c_str(), "org.freedesktop.DBus.Properties", "GetAll");
+                pimMsg.append("xyz.openbmc_project.Inventory.Item");
+
+                PropertyMap propMap;
+
+                auto valueResponseMsg = owner->_bus.call(pimMsg);
+                valueResponseMsg.read(propMap);
+
+                // If no error was set, the data should all be there.
+                auto findPresent = propMap.find("Present");
+                if (findPresent != propMap.end())
+                {
+                    present = std::get<bool>(findPresent->second);
+                }
+
+                // std::cerr << "handleSensorValue() present: '" << present << "'\n";
+                if (!present)
+                {
+                    asserted = false;
+                }
+            }
+            catch (const std::exception& e)
+            {
+                // Ignore failure
+            }
+        }
+
         owner->setFailed(asserted);
     }
 
diff --git a/dbus/dbuspassive.hpp b/dbus/dbuspassive.hpp
index f00a2ea..d025ed4 100644
--- a/dbus/dbuspassive.hpp
+++ b/dbus/dbuspassive.hpp
@@ -57,12 +57,14 @@ class DbusPassive : public ReadInterface
     double getMax(void);
     double getMin(void);
 
-  private:
+    std::string _id; // for debug identification
+    std::string _type; // for inventory match
+    DbusHelperInterface* _helper;
     sdbusplus::bus::bus& _bus;
+
+  private:
     sdbusplus::server::match::match _signal;
     int64_t _scale;
-    std::string _id; // for debug identification
-    DbusHelperInterface* _helper;
 
     std::mutex _lock;
     double _value = 0;
diff --git a/dbus/dbuswrite.cpp b/dbus/dbuswrite.cpp
index 502b988..28a04e4 100644
--- a/dbus/dbuswrite.cpp
+++ b/dbus/dbuswrite.cpp
@@ -65,6 +65,8 @@ void DbusWritePercent::write(double value)
                                  "org.freedesktop.DBus.Properties", "Set");
     mesg.append(pwmInterface, "Target", std::variant<uint64_t>(ovalue));
 
+    // std::cerr << "DbusWritePercent::write(" << value << "), writing to connection '" << connectionName.c_str() << "', path '" << path.c_str() << "' xyz.openbmc_project.Control.FanPwm Target '" << ovalue << "'\n";
+
     try
     {
         // TODO: if we don't use the reply, call_noreply()
@@ -111,6 +113,8 @@ void DbusWrite::write(double value)
                                  "org.freedesktop.DBus.Properties", "Set");
     mesg.append(pwmInterface, "Target", std::variant<uint64_t>(value));
 
+    // std::cerr << "DbusWrite::write(" << value << "), writing to connection '" << connectionName.c_str() << "', path '" << path.c_str() << "' xyz.openbmc_project.Control.FanPwm Target '" << value << "'\n";
+
     try
     {
         // TODO: consider call_noreplly
diff --git a/dbus/util.cpp b/dbus/util.cpp
index 33e0985..8a639eb 100644
--- a/dbus/util.cpp
+++ b/dbus/util.cpp
@@ -148,6 +148,7 @@ bool DbusHelper::thresholdsAsserted(sdbusplus::bus::bus& bus,
     {
         asserted = std::get<bool>(findCriticalHigh->second);
     }
+    // std::cerr << "DbusHelper::thresholdsAsserted path: '" << path << "' asserted: " << asserted << "\n";
     return asserted;
 }
 
@@ -170,6 +171,11 @@ std::string getSensorPath(const std::string& type, const std::string& id)
     return std::string("/xyz/openbmc_project/sensors/" + layer + "/" + id);
 }
 
+std::string getInventoryPath(const std::string& id)
+{
+    return std::string("/xyz/openbmc_project/inventory/system/chassis/motherboard/" + id);
+}
+
 std::string getMatch(const std::string& type, const std::string& id)
 {
     return std::string("type='signal',"
diff --git a/pid/ec/pid.cpp b/pid/ec/pid.cpp
index 7d8b403..7a8b91d 100644
--- a/pid/ec/pid.cpp
+++ b/pid/ec/pid.cpp
@@ -16,6 +16,8 @@
 
 #include "pid.hpp"
 
+#include <iostream>
+
 namespace ec
 {
 
@@ -72,6 +74,8 @@ double pid(pid_info_t* pidinfoptr, double input, double setpoint)
     output = proportionalTerm + integralTerm + feedFwdTerm;
     output = clamp(output, pidinfoptr->outLim.min, pidinfoptr->outLim.max);
 
+    // std::cerr << "pid() input: " << input << " error: " << error << " proportionalTerm: " << proportionalTerm << " integralTerm: " << integralTerm << " feedFwdTerm: " << feedFwdTerm << " output: " << output << "\n";
+
     // slew rate
     // TODO(aarena) - Simplify logic as Andy suggested by creating dynamic
     // outLim_min/max that are affected by slew rate control and just clamping
diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp
index dd26d16..3a2c743 100644
--- a/pid/fancontroller.cpp
+++ b/pid/fancontroller.cpp
@@ -106,6 +106,7 @@ double FanController::setptProc(void)
         setFanDirection(FanSpeedDirection::NEUTRAL);
     }
 
+    // std::cerr << "FanController::setptProc() prev: " << prev << " going to return: " << maxRPM << "\n";
     setSetpoint(maxRPM);
 
     return (maxRPM);
@@ -128,12 +129,15 @@ void FanController::outputProc(double value)
         }
     }
 
+    // std::cerr << "FanController::outputProc(" << value << ") _owner->getFailSafeMode(): " << _owner->getFailSafeMode() << " percent: " << percent << "\n";
+
     // value and kFanFailSafeDutyCycle are 10 for 10% so let's fix that.
     percent /= 100;
 
     // PidSensorMap for writing.
     for (const auto& it : _inputs)
     {
+        // std::cerr << "FanController::outputProc(" << value << ") for '" << it << "', writing value '" << percent << "'\n";
         auto sensor = _owner->getSensor(it);
         sensor->write(percent);
     }
diff --git a/pid/pidcontroller.cpp b/pid/pidcontroller.cpp
index e3eaaff..e23149e 100644
--- a/pid/pidcontroller.cpp
+++ b/pid/pidcontroller.cpp
@@ -73,6 +73,8 @@ void PIDController::process(void)
         output = ec::pid(info, lastInput, setpt);
     }
 
+    // std::cerr << "PIDController::process() setpt: " << setpt << " input: " << input << " output: " << output << "\n";
+
     // Output new value
     outputProc(output);
 
diff --git a/pid/zone.cpp b/pid/zone.cpp
index 6a63671..8cdb9f7 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -53,6 +53,11 @@ void PIDZone::setManualMode(bool mode)
 bool PIDZone::getFailSafeMode(void) const
 {
     // If any keys are present at least one sensor is in fail safe mode.
+    // if (!_failSafeSensors.empty())
+    // {
+    //     std::set<std::string>::iterator it = _failSafeSensors.begin();
+    //     std::cerr << "PIDZone::getFailSafeMode sensor '" << *it << "' failed!\n";
+    // }
     return !_failSafeSensors.empty();
 }
 
@@ -139,6 +144,7 @@ void PIDZone::determineMaxSetPointRequest(void)
      */
     max = std::max(getMinThermalSetpoint(), max);
 
+    // std::cerr << "PIDZone::determineMaxSetPointRequest _SetPoints.size(): " << _SetPoints.size() << " tuningEnabled: " << tuningEnabled << " max: " << max << "\n";
     if (tuningEnabled)
     {
         /*
@@ -251,6 +257,7 @@ void PIDZone::updateFanTelemetry(void)
         // check if fan fail.
         if (sensor->getFailed())
         {
+            std::cerr << "Fan '" << f << "' has failed!.\n";
             _failSafeSensors.insert(f);
         }
         else if (timeout != 0 && duration >= period)
@@ -288,6 +295,17 @@ void PIDZone::updateSensors(void)
     for (const auto& t : _thermalInputs)
     {
         auto sensor = _mgr.getSensor(t);
+        if (!sensor)
+        {
+            // std::cerr << "Skipping missing sensor '" << t << "'.\n";
+            // Check if it's in there: remove it.
+            auto kt = _failSafeSensors.find(t);
+            if (kt != _failSafeSensors.end())
+            {
+                _failSafeSensors.erase(kt);
+            }
+            continue;
+        }
         ReadReturn r = sensor->read();
         int64_t timeout = sensor->getTimeout();
 
@@ -303,7 +321,7 @@ void PIDZone::updateSensors(void)
         }
         else if (timeout != 0 && duration >= period)
         {
-            // std::cerr << "Entering fail safe mode.\n";
+            std::cerr << "Entering fail safe mode.\n";
             _failSafeSensors.insert(t);
         }
         else
@@ -315,6 +333,8 @@ void PIDZone::updateSensors(void)
                 _failSafeSensors.erase(kt);
             }
         }
+
+        // std::cerr << "PIDZone::updateSensors for '" << t << "', got value '" << _cachedValuesByName[t] << "'\n";
     }
 
     return;
diff --git a/sensors/builder.cpp b/sensors/builder.cpp
index 4da1cf2..464ec9c 100644
--- a/sensors/builder.cpp
+++ b/sensors/builder.cpp
@@ -87,9 +87,7 @@ SensorManager
                 }
                 if (ri == nullptr)
                 {
-                    throw SensorBuildException(
-                        "Failed to create dbus passive sensor: " + name +
-                        " of type: " + info->type);
+                    continue;
                 }
                 break;
             case IOInterfaceType::EXTERNAL:
diff --git a/sensors/manager.hpp b/sensors/manager.hpp
index 411b302..bd1dd48 100644
--- a/sensors/manager.hpp
+++ b/sensors/manager.hpp
@@ -38,6 +38,8 @@ class SensorManager
     // TODO(venture): Should implement read/write by name.
     Sensor* getSensor(const std::string& name) const
     {
+        if (_sensorMap.count(name) <= 0)
+            return nullptr;
         return _sensorMap.at(name).get();
     }
 
diff --git a/util.hpp b/util.hpp
index e40b61f..5e2feae 100644
--- a/util.hpp
+++ b/util.hpp
@@ -149,6 +149,7 @@ class DbusHelper : public DbusHelperInterface
 };
 
 std::string getSensorPath(const std::string& type, const std::string& id);
+std::string getInventoryPath(const std::string& id);
 std::string getMatch(const std::string& type, const std::string& id);
 void scaleSensorReading(const double min, const double max, double& value);
 bool validType(const std::string& type);