[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>
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 @@
_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 @@
_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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
_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 @@
{
_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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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));
};