Refactoring threshold properties code.
Multiple threshold interfaces like Hardshutdown and Softshutdown
needs to be created for sensors. Therefore, array has been created
to store multiple properties of thresholds.
Threshold level, direction and properties can be accessed based on
the array created. Moreover, lines of code has been reduced by
refactoring existing threshold code.
TESTED: Tested on Facebook YosemiteV2 hardware. Verified that the
Warning and Critical interfaces and properties are created and
displaying in dbus objects.
Signed-off-by: Jayashree Dhanapal <jayashree-d@hcl.com>
Change-Id: I98be6dd723a6f66336147ec572f161a6c5df3691
diff --git a/include/Thresholds.hpp b/include/Thresholds.hpp
index 209d68e..528fc95 100644
--- a/include/Thresholds.hpp
+++ b/include/Thresholds.hpp
@@ -13,15 +13,17 @@
struct Sensor;
namespace thresholds
{
-enum Level
+enum class Level
{
WARNING,
- CRITICAL
+ CRITICAL,
+ ERROR
};
-enum Direction
+enum class Direction
{
HIGH,
- LOW
+ LOW,
+ ERROR
};
struct Threshold
{
diff --git a/include/sensor.hpp b/include/sensor.hpp
index d38bcde..ff8bf8e 100644
--- a/include/sensor.hpp
+++ b/include/sensor.hpp
@@ -96,6 +96,26 @@
// construction of your Sensor subclass. See ExternalSensor for example.
std::function<void()> externalSetHook;
+ struct ThresholdProperty
+ {
+ thresholds::Level level;
+ thresholds::Direction direction;
+ uint8_t sevOrder;
+ const char* levelProperty;
+ const char* alarmProperty;
+ const char* dirOrder;
+ };
+
+ constexpr static std::array<ThresholdProperty, 4> thresProp = {
+ {{thresholds::Level::WARNING, thresholds::Direction::HIGH, 0,
+ "WarningHigh", "WarningAlarmHigh", "greater than"},
+ {thresholds::Level::WARNING, thresholds::Direction::LOW, 0,
+ "WarningLow", "WarningAlarmLow", "less than"},
+ {thresholds::Level::CRITICAL, thresholds::Direction::HIGH, 1,
+ "CriticalHigh", "CriticalAlarmHigh", "greater than"},
+ {thresholds::Level::CRITICAL, thresholds::Direction::LOW, 1,
+ "CriticalLow", "CriticalAlarmLow", "less than"}}};
+
void updateInstrumentation(double readValue)
{
// Do nothing if this feature is not enabled
@@ -245,40 +265,18 @@
threshold.hysteresis = hysteresisTrigger;
}
std::shared_ptr<sdbusplus::asio::dbus_interface> iface;
- std::string level;
- std::string alarm;
if (threshold.level == thresholds::Level::CRITICAL)
{
iface = thresholdInterfaceCritical;
- if (threshold.direction == thresholds::Direction::HIGH)
- {
- level = "CriticalHigh";
- alarm = "CriticalAlarmHigh";
- }
- else
- {
- level = "CriticalLow";
- alarm = "CriticalAlarmLow";
- }
}
else if (threshold.level == thresholds::Level::WARNING)
{
iface = thresholdInterfaceWarning;
- if (threshold.direction == thresholds::Direction::HIGH)
- {
- level = "WarningHigh";
- alarm = "WarningAlarmHigh";
- }
- else
- {
- level = "WarningLow";
- alarm = "WarningAlarmLow";
- }
}
else
{
- std::cerr << "Unknown threshold level" << threshold.level
- << "\n";
+ std::cerr << "Unknown threshold level"
+ << static_cast<int>(threshold.level) << "\n";
continue;
}
if (!iface)
@@ -287,6 +285,15 @@
continue;
}
+ std::string level =
+ propertyLevel(threshold.level, threshold.direction);
+ std::string alarm =
+ propertyAlarm(threshold.level, threshold.direction);
+
+ if ((level.empty()) || (alarm.empty()))
+ {
+ continue;
+ }
size_t thresSize =
label.empty() ? thresholds.size() : thresholdSize;
iface->register_property(
@@ -373,6 +380,32 @@
}
}
+ std::string propertyLevel(const thresholds::Level lev,
+ const thresholds::Direction dir)
+ {
+ for (ThresholdProperty prop : thresProp)
+ {
+ if ((prop.level == lev) && (prop.direction == dir))
+ {
+ return prop.levelProperty;
+ }
+ }
+ return "";
+ }
+
+ std::string propertyAlarm(const thresholds::Level lev,
+ const thresholds::Direction dir)
+ {
+ for (ThresholdProperty prop : thresProp)
+ {
+ if ((prop.level == lev) && (prop.direction == dir))
+ {
+ return prop.alarmProperty;
+ }
+ }
+ return "";
+ }
+
bool readingStateGood()
{
if (readState == PowerState::on && !isPowerOn())
diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp
index 0581f21..20f7949 100644
--- a/src/Thresholds.cpp
+++ b/src/Thresholds.cpp
@@ -17,44 +17,31 @@
#include <vector>
static constexpr bool debug = false;
+static constexpr int thresLevel = 2;
namespace thresholds
{
-unsigned int toBusValue(const Level& level)
+Level findThresholdLevel(uint8_t sev, const std::string& direct)
{
- switch (level)
+ for (Sensor::ThresholdProperty prop : Sensor::thresProp)
{
- case (Level::WARNING):
+ if ((prop.sevOrder == sev) && (prop.dirOrder == direct))
{
- return 0;
- }
- case (Level::CRITICAL):
- {
- return 1;
- }
- default:
- {
- return -1;
+ return prop.level;
}
}
+ return Level::ERROR;
}
-std::string toBusValue(const Direction& direction)
+Direction findThresholdDirection(uint8_t sev, const std::string& direct)
{
- switch (direction)
+ for (Sensor::ThresholdProperty prop : Sensor::thresProp)
{
- case (Direction::LOW):
+ if ((prop.sevOrder == sev) && (prop.dirOrder == direct))
{
- return "less than";
- }
- case (Direction::HIGH):
- {
- return "greater than";
- }
- default:
- {
- return "err";
+ return prop.direction;
}
}
+ return Direction::ERROR;
}
bool parseThresholdsFromConfig(
@@ -119,25 +106,19 @@
<< item.first << "\n";
return false;
}
- Level level;
- Direction direction;
- if (std::visit(VariantToUnsignedIntVisitor(), severityFind->second) ==
- 0)
+ uint8_t severity =
+ std::visit(VariantToUnsignedIntVisitor(), severityFind->second);
+
+ std::string directions =
+ std::visit(VariantToStringVisitor(), directionFind->second);
+
+ Level level = findThresholdLevel(severity, directions);
+ Direction direction = findThresholdDirection(severity, directions);
+
+ if ((static_cast<int>(level) == thresLevel) ||
+ (static_cast<int>(direction) == thresLevel))
{
- level = Level::WARNING;
- }
- else
- {
- level = Level::CRITICAL;
- }
- if (std::visit(VariantToStringVisitor(), directionFind->second) ==
- "less than")
- {
- direction = Direction::LOW;
- }
- else
- {
- direction = Direction::HIGH;
+ continue;
}
double val = std::visit(VariantToDoubleVisitor(), valueFind->second);
@@ -195,8 +176,9 @@
std::string dir =
std::visit(VariantToStringVisitor(), directionFind->second);
- if ((toBusValue(threshold.level) != level) ||
- (toBusValue(threshold.direction) != dir))
+ if (((findThresholdLevel(level, dir)) != threshold.level) ||
+ ((findThresholdDirection(level, dir)) !=
+ threshold.direction))
{
return; // not the droid we're looking for
}
@@ -228,30 +210,13 @@
for (const auto& threshold : sensor->thresholds)
{
std::shared_ptr<sdbusplus::asio::dbus_interface> interface;
- std::string property;
if (threshold.level == thresholds::Level::CRITICAL)
{
interface = sensor->thresholdInterfaceCritical;
- if (threshold.direction == thresholds::Direction::HIGH)
- {
- property = "CriticalHigh";
- }
- else
- {
- property = "CriticalLow";
- }
}
else if (threshold.level == thresholds::Level::WARNING)
{
interface = sensor->thresholdInterfaceWarning;
- if (threshold.direction == thresholds::Direction::HIGH)
- {
- property = "WarningHigh";
- }
- else
- {
- property = "WarningLow";
- }
}
else
{
@@ -261,6 +226,13 @@
{
continue;
}
+
+ std::string property =
+ sensor->propertyLevel(threshold.level, threshold.direction);
+ if (property.empty())
+ {
+ continue;
+ }
interface->set_property(property, threshold.value);
}
}
@@ -483,36 +455,19 @@
thresholds::Level level, thresholds::Direction direction,
bool assert)
{
- std::string property;
std::shared_ptr<sdbusplus::asio::dbus_interface> interface;
- if (level == thresholds::Level::WARNING &&
- direction == thresholds::Direction::HIGH)
+ if (level == thresholds::Level::WARNING)
{
- property = "WarningAlarmHigh";
interface = sensor->thresholdInterfaceWarning;
}
- else if (level == thresholds::Level::WARNING &&
- direction == thresholds::Direction::LOW)
+ else if (level == thresholds::Level::CRITICAL)
{
- property = "WarningAlarmLow";
- interface = sensor->thresholdInterfaceWarning;
- }
- else if (level == thresholds::Level::CRITICAL &&
- direction == thresholds::Direction::HIGH)
- {
- property = "CriticalAlarmHigh";
- interface = sensor->thresholdInterfaceCritical;
- }
- else if (level == thresholds::Level::CRITICAL &&
- direction == thresholds::Direction::LOW)
- {
- property = "CriticalAlarmLow";
interface = sensor->thresholdInterfaceCritical;
}
else
{
- std::cerr << "Unknown threshold, level " << level << "direction "
- << direction << "\n";
+ std::cerr << "Unknown threshold, level " << static_cast<int>(level)
+ << "direction " << static_cast<int>(direction) << "\n";
return;
}
if (!interface)
@@ -521,6 +476,12 @@
return;
}
+ std::string property = sensor->propertyAlarm(level, direction);
+ if (property.empty())
+ {
+ std::cout << "Alarm property is empty \n";
+ return;
+ }
if (interface->set_property<bool, true>(property, assert))
{
try