[dbus-passive] Add threshold fan failure

When a threshold is crossed for a monitored sensor,
assert fan failure.

Tested-by: Changed a sensor threshold so that its current
reading made the threshold asserted and noticed via print
messages that the sensor went into failure state. Also
noticed fans ramp. Wrote unit test to verify sensor can
move in and out of error state correctly.

Change-Id: I83182536e4874eaba97f3f1d48d53ac110fba833
Signed-off-by: James Feist <james.feist@linux.intel.com>

11 files changed, 287 insertions, 3 deletions

diff --git a/dbus/dbuspassive.cpp b/dbus/dbuspassive.cpp
index 702bf8d..a43b024 100644
--- a/dbus/dbuspassive.cpp
+++ b/dbus/dbuspassive.cpp
@@ -58,6 +58,7 @@ DbusPassive::DbusPassive(sdbusplus::bus::bus& bus, const std::string& type,
     _scale = settings.scale;
     _value = settings.value * pow(10, _scale);
     _updated = std::chrono::high_resolution_clock::now();
+    _failed = _helper->ThresholdsAsserted(tempBus, service, path);
 }
 
 ReadReturn DbusPassive::read(void)
@@ -77,6 +78,16 @@ void DbusPassive::setValue(double value)
     _updated = std::chrono::high_resolution_clock::now();
 }
 
+bool DbusPassive::getFailed(void) const
+{
+    return _failed;
+}
+
+void DbusPassive::setFailed(bool value)
+{
+    _failed = value;
+}
+
 int64_t DbusPassive::getScale(void)
 {
     return _scale;
@@ -90,7 +101,8 @@ std::string DbusPassive::getId(void)
 int HandleSensorValue(sdbusplus::message::message& msg, DbusPassive* owner)
 {
     std::string msgSensor;
-    std::map<std::string, sdbusplus::message::variant<int64_t, double>> msgData;
+    std::map<std::string, sdbusplus::message::variant<int64_t, double, bool>>
+        msgData;
 
     msg.read(msgSensor, msgData);
 
@@ -107,6 +119,32 @@ int HandleSensorValue(sdbusplus::message::message& msg, DbusPassive* owner)
             owner->setValue(value);
         }
     }
+    else if (msgSensor == "xyz.openbmc_project.Sensor.Threshold.Critical")
+    {
+        auto criticalAlarmLow = msgData.find("CriticalAlarmLow");
+        auto criticalAlarmHigh = msgData.find("CriticalAlarmHigh");
+        if (criticalAlarmHigh == msgData.end() &&
+            criticalAlarmLow == msgData.end())
+        {
+            return 0;
+        }
+
+        bool asserted = false;
+        if (criticalAlarmLow != msgData.end())
+        {
+            asserted = sdbusplus::message::variant_ns::get<bool>(
+                criticalAlarmLow->second);
+        }
+
+        // checking both as in theory you could de-assert one threshold and
+        // assert the other at the same moment
+        if (!asserted && criticalAlarmHigh != msgData.end())
+        {
+            asserted = sdbusplus::message::variant_ns::get<bool>(
+                criticalAlarmHigh->second);
+        }
+        owner->setFailed(asserted);
+    }
 
     return 0;
 }
diff --git a/dbus/dbuspassive.hpp b/dbus/dbuspassive.hpp
index 87315a0..71ef339 100644
--- a/dbus/dbuspassive.hpp
+++ b/dbus/dbuspassive.hpp
@@ -41,8 +41,10 @@ class DbusPassive : public ReadInterface
                 const std::string& id, DbusHelperInterface* helper);
 
     ReadReturn read(void) override;
+    bool getFailed(void) const override;
 
     void setValue(double value);
+    void setFailed(bool value);
     int64_t getScale(void);
     std::string getId(void);
 
@@ -55,6 +57,7 @@ class DbusPassive : public ReadInterface
 
     std::mutex _lock;
     double _value = 0;
+    bool _failed = false;
     /* The last time the value was refreshed, not necessarily changed. */
     std::chrono::high_resolution_clock::time_point _updated;
 };
diff --git a/dbus/util.cpp b/dbus/util.cpp
index 0d107ab..cdc4bb9 100644
--- a/dbus/util.cpp
+++ b/dbus/util.cpp
@@ -1,10 +1,11 @@
 #include "dbus/util.hpp"
 
+#include <cmath>
 #include <iostream>
 #include <set>
 
 using Property = std::string;
-using Value = sdbusplus::message::variant<int64_t, double, std::string>;
+using Value = sdbusplus::message::variant<int64_t, double, std::string, bool>;
 using PropertyMap = std::map<Property, Value>;
 
 /* TODO(venture): Basically all phosphor apps need this, maybe it should be a
@@ -89,6 +90,50 @@ void DbusHelper::GetProperties(sdbusplus::bus::bus& bus,
     return;
 }
 
+bool DbusHelper::ThresholdsAsserted(sdbusplus::bus::bus& bus,
+                                    const std::string& service,
+                                    const std::string& path)
+{
+
+    auto critical = bus.new_method_call(service.c_str(), path.c_str(),
+                                        propertiesintf.c_str(), "GetAll");
+    critical.append(criticalThreshInf);
+    PropertyMap criticalMap;
+
+    try
+    {
+        auto msg = bus.call(critical);
+        if (!msg.is_method_error())
+        {
+            msg.read(criticalMap);
+        }
+    }
+    catch (sdbusplus::exception_t&)
+    {
+        // do nothing, sensors don't have to expose critical thresholds
+        return false;
+    }
+
+    auto findCriticalLow = criticalMap.find("CriticalAlarmLow");
+    auto findCriticalHigh = criticalMap.find("CriticalAlarmHigh");
+
+    bool asserted = false;
+    if (findCriticalLow != criticalMap.end())
+    {
+        asserted =
+            sdbusplus::message::variant_ns::get<bool>(findCriticalLow->second);
+    }
+
+    // as we are catching properties changed, a sensor could theoretically jump
+    // from one threshold to the other in one event, so check both thresholds
+    if (!asserted && findCriticalHigh != criticalMap.end())
+    {
+        asserted =
+            sdbusplus::message::variant_ns::get<bool>(findCriticalHigh->second);
+    }
+    return asserted;
+}
+
 std::string GetSensorPath(const std::string& type, const std::string& id)
 {
     std::string layer = type;
diff --git a/dbus/util.hpp b/dbus/util.hpp
index e459524..ed7a411 100644
--- a/dbus/util.hpp
+++ b/dbus/util.hpp
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <limits>
 #include <sdbusplus/bus.hpp>
 
 struct SensorProperties
@@ -9,7 +10,15 @@ struct SensorProperties
     std::string unit;
 };
 
+struct SensorThresholds
+{
+    double lowerThreshold = std::numeric_limits<double>::quiet_NaN();
+    double upperThreshold = std::numeric_limits<double>::quiet_NaN();
+};
+
 const std::string sensorintf = "xyz.openbmc_project.Sensor.Value";
+const std::string criticalThreshInf =
+    "xyz.openbmc_project.Sensor.Threshold.Critical";
 const std::string propertiesintf = "org.freedesktop.DBus.Properties";
 
 class DbusHelperInterface
@@ -34,6 +43,16 @@ class DbusHelperInterface
                                const std::string& service,
                                const std::string& path,
                                struct SensorProperties* prop) = 0;
+
+    /** @brief Get Critical Threshold current assert status
+     *
+     * @param[in] bus - A bus to use for the call.
+     * @param[in] service - The service providing the interface.
+     * @param[in] path - The dbus path.
+     */
+    virtual bool ThresholdsAsserted(sdbusplus::bus::bus& bus,
+                                    const std::string& service,
+                                    const std::string& path) = 0;
 };
 
 class DbusHelper : public DbusHelperInterface
@@ -52,6 +71,10 @@ class DbusHelper : public DbusHelperInterface
     void GetProperties(sdbusplus::bus::bus& bus, const std::string& service,
                        const std::string& path,
                        struct SensorProperties* prop) override;
+
+    bool ThresholdsAsserted(sdbusplus::bus::bus& bus,
+                            const std::string& service,
+                            const std::string& path) override;
 };
 
 std::string GetSensorPath(const std::string& type, const std::string& id);
diff --git a/interfaces.hpp b/interfaces.hpp
index bf45545..89ef128 100644
--- a/interfaces.hpp
+++ b/interfaces.hpp
@@ -29,6 +29,11 @@ class ReadInterface
     }
 
     virtual ReadReturn read(void) = 0;
+
+    virtual bool getFailed(void) const
+    {
+        return false;
+    }
 };
 
 /*
diff --git a/pid/zone.cpp b/pid/zone.cpp
index d5e2646..35abb13 100644
--- a/pid/zone.cpp
+++ b/pid/zone.cpp
@@ -251,10 +251,14 @@ void PIDZone::updateSensors(void)
         _cachedValuesByName[t] = r.value;
         tstamp then = r.updated;
 
+        if (sensor->getFailed())
+        {
+            _failSafeSensors.insert(t);
+        }
         /* Only go into failsafe if the timeout is set for
          * the sensor.
          */
-        if (timeout > 0)
+        else if (timeout > 0)
         {
             auto duration =
                 duration_cast<std::chrono::seconds>(now - then).count();
diff --git a/sensors/pluggable.cpp b/sensors/pluggable.cpp
index 8e4d789..eb3eab9 100644
--- a/sensors/pluggable.cpp
+++ b/sensors/pluggable.cpp
@@ -32,3 +32,8 @@ void PluggableSensor::write(double value)
 {
     _writer->write(value);
 }
+
+bool PluggableSensor::getFailed(void)
+{
+    return _reader->getFailed();
+}
\ No newline at end of file
diff --git a/sensors/pluggable.hpp b/sensors/pluggable.hpp
index 005d9ad..396a49c 100644
--- a/sensors/pluggable.hpp
+++ b/sensors/pluggable.hpp
@@ -23,6 +23,7 @@ class PluggableSensor : public Sensor
 
     ReadReturn read(void) override;
     void write(double value) override;
+    bool getFailed(void) override;
 
   private:
     std::unique_ptr<ReadInterface> _reader;
diff --git a/sensors/sensor.hpp b/sensors/sensor.hpp
index 2c287cf..1f13e3a 100644
--- a/sensors/sensor.hpp
+++ b/sensors/sensor.hpp
@@ -22,6 +22,10 @@ class Sensor
 
     virtual ReadReturn read(void) = 0;
     virtual void write(double value) = 0;
+    virtual bool getFailed(void)
+    {
+        return false;
+    };
 
     std::string GetName(void) const
     {
diff --git a/test/dbus_passive_unittest.cpp b/test/dbus_passive_unittest.cpp
index 961b380..43f5cc6 100644
--- a/test/dbus_passive_unittest.cpp
+++ b/test/dbus_passive_unittest.cpp
@@ -57,6 +57,8 @@ TEST(DbusPassiveTest, BoringConstructorTest)
                 prop->value = 10;
                 prop->unit = "x";
             }));
+    EXPECT_CALL(helper, ThresholdsAsserted(_, StrEq("asdf"), StrEq(path)))
+        .WillOnce(Return(false));
 
     DbusPassive(bus_mock, type, id, &helper);
     // Success
@@ -81,6 +83,8 @@ class DbusPassiveTestObj : public ::testing::Test
                     prop->value = _value;
                     prop->unit = "x";
                 }));
+        EXPECT_CALL(helper, ThresholdsAsserted(_, StrEq("asdf"), StrEq(path)))
+            .WillOnce(Return(false));
 
         ri = DbusPassive::CreateDbusPassive(bus_mock, type, id, &helper);
         passive = reinterpret_cast<DbusPassive*>(ri.get());
@@ -274,3 +278,151 @@ TEST_F(DbusPassiveTestObj, VerifyIgnoresOtherPropertySignal)
     ReadReturn r = passive->read();
     EXPECT_EQ(0.01, r.value);
 }
+TEST_F(DbusPassiveTestObj, VerifyCriticalThresholdAssert)
+{
+
+    // Verifies when a threshold is crossed the sensor goes into error state
+    EXPECT_CALL(sdbus_mock, sd_bus_message_ref(IsNull()))
+        .WillOnce(Return(nullptr));
+    sdbusplus::message::message msg(nullptr, &sdbus_mock);
+
+    const char* criticalAlarm = "CriticalAlarmHigh";
+    bool alarm = true;
+    const char* intf = "xyz.openbmc_project.Sensor.Threshold.Critical";
+
+    passive->setFailed(false);
+
+    EXPECT_CALL(sdbus_mock, sd_bus_message_read_basic(IsNull(), 's', NotNull()))
+        .WillOnce(Invoke([&](sd_bus_message* m, char type, void* p) {
+            const char** s = static_cast<const char**>(p);
+            // Read the first parameter, the string.
+            *s = intf;
+            return 0;
+        }))
+        .WillOnce(Invoke([&](sd_bus_message* m, char type, void* p) {
+            const char** s = static_cast<const char**>(p);
+            *s = criticalAlarm;
+            // Read the string in the pair (dictionary).
+            return 0;
+        }));
+
+    // std::map
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_enter_container(IsNull(), 'a', StrEq("{sv}")))
+        .WillOnce(Return(0));
+
+    // while !at_end()
+    EXPECT_CALL(sdbus_mock, sd_bus_message_at_end(IsNull(), 0))
+        .WillOnce(Return(0))
+        .WillOnce(Return(1)); // So it exits the loop after reading one pair.
+
+    // std::pair
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_enter_container(IsNull(), 'e', StrEq("sv")))
+        .WillOnce(Return(0));
+
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_verify_type(IsNull(), 'v', StrEq("x")))
+        .WillOnce(Return(0));
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_verify_type(IsNull(), 'v', StrEq("d")))
+        .WillOnce(Return(0));
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_verify_type(IsNull(), 'v', StrEq("b")))
+        .WillOnce(Return(1));
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_enter_container(IsNull(), 'v', StrEq("b")))
+        .WillOnce(Return(0));
+
+    EXPECT_CALL(sdbus_mock, sd_bus_message_read_basic(IsNull(), 'b', NotNull()))
+        .WillOnce(Invoke([&](sd_bus_message* m, char type, void* p) {
+            bool* s = static_cast<bool*>(p);
+            *s = alarm;
+            return 0;
+        }));
+
+    EXPECT_CALL(sdbus_mock, sd_bus_message_exit_container(IsNull()))
+        .WillOnce(Return(0))  /* variant. */
+        .WillOnce(Return(0))  /* std::pair */
+        .WillOnce(Return(0)); /* std::map */
+
+    int rv = HandleSensorValue(msg, passive);
+    EXPECT_EQ(rv, 0); // It's always 0.
+    bool failed = passive->getFailed();
+    EXPECT_EQ(failed, true);
+}
+
+TEST_F(DbusPassiveTestObj, VerifyCriticalThresholdDeassert)
+{
+
+    // Verifies when a threshold is deasserted a failed sensor goes back into
+    // the normal state
+    EXPECT_CALL(sdbus_mock, sd_bus_message_ref(IsNull()))
+        .WillOnce(Return(nullptr));
+    sdbusplus::message::message msg(nullptr, &sdbus_mock);
+
+    const char* criticalAlarm = "CriticalAlarmHigh";
+    bool alarm = false;
+    const char* intf = "xyz.openbmc_project.Sensor.Threshold.Critical";
+
+    passive->setFailed(true);
+
+    EXPECT_CALL(sdbus_mock, sd_bus_message_read_basic(IsNull(), 's', NotNull()))
+        .WillOnce(Invoke([&](sd_bus_message* m, char type, void* p) {
+            const char** s = static_cast<const char**>(p);
+            // Read the first parameter, the string.
+            *s = intf;
+            return 0;
+        }))
+        .WillOnce(Invoke([&](sd_bus_message* m, char type, void* p) {
+            const char** s = static_cast<const char**>(p);
+            *s = criticalAlarm;
+            // Read the string in the pair (dictionary).
+            return 0;
+        }));
+
+    // std::map
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_enter_container(IsNull(), 'a', StrEq("{sv}")))
+        .WillOnce(Return(0));
+
+    // while !at_end()
+    EXPECT_CALL(sdbus_mock, sd_bus_message_at_end(IsNull(), 0))
+        .WillOnce(Return(0))
+        .WillOnce(Return(1)); // So it exits the loop after reading one pair.
+
+    // std::pair
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_enter_container(IsNull(), 'e', StrEq("sv")))
+        .WillOnce(Return(0));
+
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_verify_type(IsNull(), 'v', StrEq("x")))
+        .WillOnce(Return(0));
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_verify_type(IsNull(), 'v', StrEq("d")))
+        .WillOnce(Return(0));
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_verify_type(IsNull(), 'v', StrEq("b")))
+        .WillOnce(Return(1));
+    EXPECT_CALL(sdbus_mock,
+                sd_bus_message_enter_container(IsNull(), 'v', StrEq("b")))
+        .WillOnce(Return(0));
+
+    EXPECT_CALL(sdbus_mock, sd_bus_message_read_basic(IsNull(), 'b', NotNull()))
+        .WillOnce(Invoke([&](sd_bus_message* m, char type, void* p) {
+            bool* s = static_cast<bool*>(p);
+            *s = alarm;
+            return 0;
+        }));
+
+    EXPECT_CALL(sdbus_mock, sd_bus_message_exit_container(IsNull()))
+        .WillOnce(Return(0))  /* variant. */
+        .WillOnce(Return(0))  /* std::pair */
+        .WillOnce(Return(0)); /* std::map */
+
+    int rv = HandleSensorValue(msg, passive);
+    EXPECT_EQ(rv, 0); // It's always 0.
+    bool failed = passive->getFailed();
+    EXPECT_EQ(failed, false);
+}
\ No newline at end of file
diff --git a/test/dbushelper_mock.hpp b/test/dbushelper_mock.hpp
index 6d3464e..d7dbcbb 100644
--- a/test/dbushelper_mock.hpp
+++ b/test/dbushelper_mock.hpp
@@ -18,4 +18,8 @@ class DbusHelperMock : public DbusHelperInterface
     MOCK_METHOD4(GetProperties,
                  void(sdbusplus::bus::bus&, const std::string&,
                       const std::string&, struct SensorProperties*));
+
+    MOCK_METHOD3(ThresholdsAsserted,
+                 bool(sdbusplus::bus::bus& bus, const std::string& service,
+                      const std::string& path));
 };