Transition to simpler trigger interface
Using this simpler interface allows us to simplify the unpacking code,
and remove the use of a variant containing a variant, which has caused
bugs in the past. Splitting these apart allows us to replicate the
Redfish interfaces with less code.
Tested:
Discrete and Numeric triggers both create correctly
```
curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X POST https://192.168.7.2/redfish/v1/TelemetryService/Triggers -d '{"Name": "eds", "NumericThresholds": {"LowerCritical": {"Reading": 1.0, "Activation": "Increasing", "DwellTime": "P1S"}}}'
curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X POST https://192.168.7.2/redfish/v1/TelemetryService/Triggers -d '{"Name": "eds", "DiscreteTriggers": [{"DwellTime": "P1S", "Severity": "OK", "Value": "1234"}]}
```
Change-Id: If898e2285f90f78b22e47cc670e4206ba4368665
Signed-off-by: Ed Tanous <etanous@nvidia.com>
diff --git a/include/dbus_utility.hpp b/include/dbus_utility.hpp
index 50d75c5..89cda89 100644
--- a/include/dbus_utility.hpp
+++ b/include/dbus_utility.hpp
@@ -71,13 +71,8 @@
std::vector<std::tuple<sdbusplus::message::object_path, std::string>>,
std::string, std::string, uint64_t>>,
std::vector<std::pair<sdbusplus::message::object_path, std::string>>,
-
- // TODO This needs looked at. It's used in the trigger system, but a
- // variant of a variant seems really odd
- std::variant<
- std::vector<std::tuple<std::string, uint64_t, std::string, double>>,
- std::vector<std::tuple<std::string, std::string, uint64_t, std::string>>
- >
+ std::vector<std::tuple<std::string, uint64_t, std::string, double>>,
+ std::vector<std::tuple<std::string, std::string, uint64_t, std::string>>
>;
// clang-format on
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index c35a2be..f165c5a 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -66,19 +66,6 @@
#include <variant>
#include <vector>
-namespace nlohmann
-{
-template <typename... Args>
-struct adl_serializer<std::variant<Args...>>
-{
- // NOLINTNEXTLINE(readability-identifier-naming)
- static void to_json(json& j, const std::variant<Args...>& args)
- {
- std::visit([&j](auto&& val) { j = val; }, args);
- }
-};
-} // namespace nlohmann
-
namespace crow
{
namespace openbmc_mapper
diff --git a/redfish-core/lib/trigger.hpp b/redfish-core/lib/trigger.hpp
index 047fbb7..c8559eb 100644
--- a/redfish-core/lib/trigger.hpp
+++ b/redfish-core/lib/trigger.hpp
@@ -37,10 +37,6 @@
using DiscreteThresholdParams =
std::tuple<std::string, std::string, uint64_t, std::string>;
-using TriggerThresholdParams =
- std::variant<std::vector<NumericThresholdParams>,
- std::vector<DiscreteThresholdParams>>;
-
using TriggerSensorsParams =
std::vector<std::pair<sdbusplus::message::object_path, std::string>>;
@@ -204,8 +200,8 @@
std::vector<std::pair<sdbusplus::message::object_path, std::string>>
sensors;
std::vector<sdbusplus::message::object_path> reports;
- TriggerThresholdParams thresholds;
-
+ std::vector<NumericThresholdParams> numericThresholds;
+ std::vector<DiscreteThresholdParams> discreteThresholds;
std::optional<DiscreteCondition> discreteCondition;
std::optional<MetricType> metricType;
std::optional<std::vector<std::string>> metricProperties;
@@ -361,7 +357,7 @@
}
}
- ctx.thresholds = std::move(parsedParams);
+ ctx.numericThresholds = std::move(parsedParams);
return true;
}
@@ -373,7 +369,7 @@
std::vector<DiscreteThresholdParams> parsedParams;
if (!discreteTriggers)
{
- ctx.thresholds = std::move(parsedParams);
+ ctx.discreteThresholds = std::move(parsedParams);
return true;
}
@@ -416,7 +412,7 @@
value);
}
- ctx.thresholds = std::move(parsedParams);
+ ctx.discreteThresholds = std::move(parsedParams);
return true;
}
@@ -730,19 +726,11 @@
return triggerActions;
}
-inline std::optional<nlohmann::json::array_t>
- getDiscreteTriggers(const TriggerThresholdParams& thresholdParams)
+inline std::optional<nlohmann::json::array_t> getDiscreteTriggers(
+ const std::vector<DiscreteThresholdParams>& discreteParams)
{
nlohmann::json::array_t triggers;
- const std::vector<DiscreteThresholdParams>* discreteParams =
- std::get_if<std::vector<DiscreteThresholdParams>>(&thresholdParams);
-
- if (discreteParams == nullptr)
- {
- return std::nullopt;
- }
-
- for (const auto& [name, severity, dwellTime, value] : *discreteParams)
+ for (const auto& [name, severity, dwellTime, value] : discreteParams)
{
std::optional<std::string> duration =
time_utils::toDurationStringFromUint(dwellTime);
@@ -762,19 +750,12 @@
return triggers;
}
-inline std::optional<nlohmann::json>
- getNumericThresholds(const TriggerThresholdParams& thresholdParams)
+inline std::optional<nlohmann::json::object_t> getNumericThresholds(
+ const std::vector<NumericThresholdParams>& numericParams)
{
nlohmann::json::object_t thresholds;
- const std::vector<NumericThresholdParams>* numericParams =
- std::get_if<std::vector<NumericThresholdParams>>(&thresholdParams);
- if (numericParams == nullptr)
- {
- return std::nullopt;
- }
-
- for (const auto& [type, dwellTime, activation, reading] : *numericParams)
+ for (const auto& [type, dwellTime, activation, reading] : numericParams)
{
std::optional<std::string> duration =
time_utils::toDurationStringFromUint(dwellTime);
@@ -840,12 +821,15 @@
const TriggerSensorsParams* sensors = nullptr;
const std::vector<sdbusplus::message::object_path>* reports = nullptr;
const std::vector<std::string>* triggerActions = nullptr;
- const TriggerThresholdParams* thresholds = nullptr;
+
+ const std::vector<DiscreteThresholdParams>* discreteThresholds = nullptr;
+ const std::vector<NumericThresholdParams>* numericThresholds = nullptr;
const bool success = sdbusplus::unpackPropertiesNoThrow(
dbus_utils::UnpackErrorPrinter(), properties, "Name", name, "Discrete",
discrete, "Sensors", sensors, "Reports", reports, "TriggerActions",
- triggerActions, "Thresholds", thresholds);
+ triggerActions, "DiscreteThresholds", discreteThresholds,
+ "NumericThresholds", numericThresholds);
if (!success)
{
@@ -877,42 +861,39 @@
json["Links"]["MetricReportDefinitions"] = *linkedReports;
}
- if (discrete != nullptr)
+ if (discreteThresholds != nullptr)
{
- if (*discrete)
+ std::optional<nlohmann::json::array_t> discreteTriggers =
+ getDiscreteTriggers(*discreteThresholds);
+
+ if (!discreteTriggers)
{
- std::optional<nlohmann::json::array_t> discreteTriggers =
- getDiscreteTriggers(*thresholds);
-
- if (!discreteTriggers)
- {
- BMCWEB_LOG_ERROR("Property Thresholds is invalid for discrete "
- "triggers in Trigger: {}",
- id);
- return false;
- }
-
- json["DiscreteTriggers"] = *discreteTriggers;
- json["DiscreteTriggerCondition"] =
- discreteTriggers->empty() ? "Changed" : "Specified";
- json["MetricType"] = metric_definition::MetricType::Discrete;
+ BMCWEB_LOG_ERROR("Property Thresholds is invalid for discrete "
+ "triggers in Trigger: {}",
+ id);
+ return false;
}
- else
+
+ json["DiscreteTriggers"] = *discreteTriggers;
+ json["DiscreteTriggerCondition"] =
+ discreteTriggers->empty() ? "Changed" : "Specified";
+ json["MetricType"] = metric_definition::MetricType::Discrete;
+ }
+ if (numericThresholds != nullptr)
+ {
+ std::optional<nlohmann::json::object_t> jnumericThresholds =
+ getNumericThresholds(*numericThresholds);
+
+ if (!jnumericThresholds)
{
- std::optional<nlohmann::json> numericThresholds =
- getNumericThresholds(*thresholds);
-
- if (!numericThresholds)
- {
- BMCWEB_LOG_ERROR("Property Thresholds is invalid for numeric "
- "thresholds in Trigger: {}",
- id);
- return false;
- }
-
- json["NumericThresholds"] = *numericThresholds;
- json["MetricType"] = metric_definition::MetricType::Numeric;
+ BMCWEB_LOG_ERROR("Property Thresholds is invalid for numeric "
+ "thresholds in Trigger: {}",
+ id);
+ return false;
}
+
+ json["NumericThresholds"] = *jnumericThresholds;
+ json["MetricType"] = metric_definition::MetricType::Numeric;
}
if (name != nullptr)
@@ -956,7 +937,7 @@
service, "/xyz/openbmc_project/Telemetry/Triggers",
"xyz.openbmc_project.Telemetry.TriggerManager", "AddTrigger",
"TelemetryService/" + ctx.id, ctx.name, ctx.actions, ctx.sensors,
- ctx.reports, ctx.thresholds);
+ ctx.reports, ctx.numericThresholds, ctx.discreteThresholds);
}
} // namespace telemetry