Rearrange thresholds to be pair based
It was pointed out in another review that the way the dbus interfaces
are set up, they are all pair-wise, with a positive and negative end
of the interface. This changes the data structures to match that
reality, inventing a "ThresholdDefinition" object. This reduces the
code, and allows us to promote the level definitions up to a higher
scope in the structure.
This greatly simplifies the thresProp structure to remove a number of
elements that were redundant, and allows adding new thresholds easier
in the future.
Tested: Tested on Vegman system; Thresholds work as designed.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic9d5ba8282f78fe928689cb7fbd98d575a570200
diff --git a/include/Thresholds.hpp b/include/Thresholds.hpp
index 50075a2..fb266d0 100644
--- a/include/Thresholds.hpp
+++ b/include/Thresholds.hpp
@@ -112,7 +112,7 @@
std::list<TimerPair> timers;
};
-bool findOrder(Level lev, Direction dir);
+bool isValidLevel(Level lev);
bool parseThresholdsFromConfig(
const SensorData& sensorData,
@@ -124,6 +124,19 @@
const double& scaleFactor,
const double& offset = 0);
+struct ThresholdDefinition
+{
+ Level level;
+ uint8_t sevOrder;
+ const char* levelName;
+};
+
+constexpr static std::array<thresholds::ThresholdDefinition, 4> thresProp = {
+ {{Level::WARNING, 0, "Warning"},
+ {Level::CRITICAL, 1, "Critical"},
+ {Level::SOFTSHUTDOWN, 2, "SoftShutdown"},
+ {Level::HARDSHUTDOWN, 3, "HardShutdown"}}};
+
std::string getInterface(const Level level);
void persistThreshold(const std::string& baseInterface, const std::string& path,
diff --git a/include/sensor.hpp b/include/sensor.hpp
index 25e28fd..40f4223 100644
--- a/include/sensor.hpp
+++ b/include/sensor.hpp
@@ -94,39 +94,15 @@
// 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;
- };
+ using Level = thresholds::Level;
+ using Direction = thresholds::Direction;
- constexpr static std::array<ThresholdProperty, 8> 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"},
- {thresholds::Level::SOFTSHUTDOWN, thresholds::Direction::HIGH, 2,
- "SoftShutdownHigh", "SoftShutdownAlarmHigh", "greater than"},
- {thresholds::Level::SOFTSHUTDOWN, thresholds::Direction::LOW, 2,
- "SoftShutdownLow", "SoftShutdownAlarmLow", "less than"},
- {thresholds::Level::HARDSHUTDOWN, thresholds::Direction::HIGH, 3,
- "HardShutdownHigh", "HardShutdownAlarmHigh", "greater than"},
- {thresholds::Level::HARDSHUTDOWN, thresholds::Direction::LOW, 3,
- "HardShutdownLow", "HardShutdownAlarmLow", "less than"}}};
-
- std::array<std::shared_ptr<sdbusplus::asio::dbus_interface>, 4>
+ std::array<std::shared_ptr<sdbusplus::asio::dbus_interface>,
+ thresholds::thresProp.size()>
thresholdInterfaces;
std::shared_ptr<sdbusplus::asio::dbus_interface>
- getThresholdInterface(thresholds::Level lev)
+ getThresholdInterface(Level lev)
{
size_t index = static_cast<size_t>(lev);
if (index >= thresholdInterfaces.size())
@@ -287,7 +263,7 @@
{
threshold.hysteresis = hysteresisTrigger;
}
- if (!(thresholds::findOrder(threshold.level, threshold.direction)))
+ if (!thresholds::isValidLevel(threshold.level))
{
continue;
}
@@ -395,27 +371,41 @@
}
}
- std::string propertyLevel(const thresholds::Level lev,
- const thresholds::Direction dir)
+ std::string propertyLevel(const Level lev, const Direction dir)
{
- for (ThresholdProperty prop : thresProp)
+ for (const thresholds::ThresholdDefinition& prop :
+ thresholds::thresProp)
{
- if ((prop.level == lev) && (prop.direction == dir))
+ if (prop.level == lev)
{
- return prop.levelProperty;
+ if (dir == Direction::HIGH)
+ {
+ return std::string(prop.levelName) + "High";
+ }
+ if (dir == Direction::LOW)
+ {
+ return std::string(prop.levelName) + "Low";
+ }
}
}
return "";
}
- std::string propertyAlarm(const thresholds::Level lev,
- const thresholds::Direction dir)
+ std::string propertyAlarm(const Level lev, const Direction dir)
{
- for (ThresholdProperty prop : thresProp)
+ for (const thresholds::ThresholdDefinition& prop :
+ thresholds::thresProp)
{
- if ((prop.level == lev) && (prop.direction == dir))
+ if (prop.level == lev)
{
- return prop.alarmProperty;
+ if (dir == Direction::HIGH)
+ {
+ return std::string(prop.levelName) + "AlarmHigh";
+ }
+ if (dir == Direction::LOW)
+ {
+ return std::string(prop.levelName) + "AlarmLow";
+ }
}
}
return "";
diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp
index f50905a..89269a2 100644
--- a/src/Thresholds.cpp
+++ b/src/Thresholds.cpp
@@ -19,11 +19,11 @@
static constexpr bool debug = false;
namespace thresholds
{
-Level findThresholdLevel(uint8_t sev, const std::string& direct)
+Level findThresholdLevel(uint8_t sev)
{
- for (Sensor::ThresholdProperty prop : Sensor::thresProp)
+ for (const ThresholdDefinition& prop : thresProp)
{
- if ((prop.sevOrder == sev) && (prop.dirOrder == direct))
+ if (prop.sevOrder == sev)
{
return prop.level;
}
@@ -31,23 +31,24 @@
return Level::ERROR;
}
-Direction findThresholdDirection(uint8_t sev, const std::string& direct)
+Direction findThresholdDirection(const std::string& direct)
{
- for (Sensor::ThresholdProperty prop : Sensor::thresProp)
+ if (direct == "greater than")
{
- if ((prop.sevOrder == sev) && (prop.dirOrder == direct))
- {
- return prop.direction;
- }
+ return Direction::HIGH;
+ }
+ if (direct == "less than")
+ {
+ return Direction::LOW;
}
return Direction::ERROR;
}
-bool findOrder(Level lev, Direction dir)
+bool isValidLevel(Level lev)
{
- for (Sensor::ThresholdProperty prop : Sensor::thresProp)
+ for (const ThresholdDefinition& prop : thresProp)
{
- if ((prop.level == lev) && (prop.direction == dir))
+ if (prop.level == lev)
{
return true;
}
@@ -123,8 +124,8 @@
std::string directions =
std::visit(VariantToStringVisitor(), directionFind->second);
- Level level = findThresholdLevel(severity, directions);
- Direction direction = findThresholdDirection(severity, directions);
+ Level level = findThresholdLevel(severity);
+ Direction direction = findThresholdDirection(directions);
if ((level == Level::ERROR) || (direction == Direction::ERROR))
{
@@ -186,9 +187,8 @@
std::string dir =
std::visit(VariantToStringVisitor(), directionFind->second);
- if (((findThresholdLevel(severity, dir)) != threshold.level) ||
- ((findThresholdDirection(severity, dir)) !=
- threshold.direction))
+ if ((findThresholdLevel(severity) != threshold.level) ||
+ (findThresholdDirection(dir) != threshold.direction))
{
return; // not the droid we're looking for
}
@@ -214,7 +214,7 @@
{
for (const auto& threshold : sensor->thresholds)
{
- if (!findOrder(threshold.level, threshold.direction))
+ if (!isValidLevel(threshold.level))
{
continue;
}
@@ -454,7 +454,7 @@
thresholds::Level level, thresholds::Direction direction,
bool assert)
{
- if (!findOrder(level, direction))
+ if (!isValidLevel(level))
{
return;
}
@@ -548,26 +548,14 @@
std::string getInterface(const Level thresholdLevel)
{
- std::string level;
- switch (thresholdLevel)
+ for (const ThresholdDefinition& thresh : thresProp)
{
- case Level::WARNING:
- level = "Warning";
- break;
- case Level::CRITICAL:
- level = "Critical";
- break;
- case Level::SOFTSHUTDOWN:
- level = "SoftShutdown";
- break;
- case Level::HARDSHUTDOWN:
- level = "HardShutdown";
- break;
- case Level::ERROR:
- level = "Error";
- break;
+ if (thresh.level == thresholdLevel)
+ {
+ return std::string("xyz.openbmc_project.Sensor.Threshold.") +
+ thresh.levelName;
+ }
}
- std::string interface = "xyz.openbmc_project.Sensor.Threshold." + level;
- return interface;
+ return "";
}
} // namespace thresholds