Refactor sensor code
Refactor some methods of the sensor code to not rely directly on
SensorAsyncResponse, or to require an ObjectManager response. This is
done to allow later patches to implement individual sensor collection
using GetAll instead of GetManagedObjects for a single sensor
This is done by making the underlying implementations rely on the
properties alone, as we do in other locations.
Behavior should not change with this patchset.
Tested:
Testing in later patch that behavior is the same. Merge at the same
time
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I77838ee14c14dcc9d2bc50375b2a5b0ffbc4573c
diff --git a/redfish-core/lib/sensors.hpp b/redfish-core/lib/sensors.hpp
index 2b1a3c8..ae68a76 100644
--- a/redfish-core/lib/sensors.hpp
+++ b/redfish-core/lib/sensors.hpp
@@ -17,6 +17,8 @@
#include <app.hpp>
#include <boost/algorithm/string/classification.hpp>
+#include <boost/algorithm/string/find.hpp>
+#include <boost/algorithm/string/predicate.hpp>
#include <boost/algorithm/string/split.hpp>
#include <boost/range/algorithm/replace_copy_if.hpp>
#include <dbus_singleton.hpp>
@@ -87,7 +89,7 @@
{node::sensors, std::span<std::string_view>(dbus::sensorPaths)},
{node::thermal, std::span<std::string_view>(dbus::thermalPaths)}}};
-inline const char* toReadingType(const std::string& sensorType)
+inline std::string_view toReadingType(std::string_view sensorType)
{
if (sensorType == "voltage")
{
@@ -132,7 +134,7 @@
return "";
}
-inline const char* toReadingUnits(const std::string& sensorType)
+inline std::string_view toReadingUnits(std::string_view sensorType)
{
if (sensorType == "voltage")
{
@@ -681,15 +683,14 @@
/**
* @brief Returns the Redfish Health value for the specified sensor.
* @param sensorJson Sensor JSON object.
- * @param interfacesDict Map of all sensor interfaces.
+ * @param valuesDict Map of all sensor DBus values.
* @param inventoryItem D-Bus inventory item associated with the sensor. Will
* be nullptr if no associated inventory item was found.
* @return Health value for sensor.
*/
-inline std::string
- getHealth(nlohmann::json& sensorJson,
- const dbus::utility::DBusInteracesMap& interfacesDict,
- const InventoryItem* inventoryItem)
+inline std::string getHealth(nlohmann::json& sensorJson,
+ const dbus::utility::DBusPropertiesMap& valuesDict,
+ const InventoryItem* inventoryItem)
{
// Get current health value (if any) in the sensor JSON object. Some JSON
// objects contain multiple sensors (such as PowerSupplies). We want to set
@@ -718,25 +719,18 @@
// Check if sensor has critical threshold alarm
- for (const auto& [interface, values] : interfacesDict)
+ for (const auto& [valueName, value] : valuesDict)
{
- if (interface == "xyz.openbmc_project.Sensor.Threshold.Critical")
+ if (valueName == "CriticalAlarmHigh" || valueName == "CriticalAlarmLow")
{
- for (const auto& [valueName, value] : values)
+ const bool* asserted = std::get_if<bool>(&value);
+ if (asserted == nullptr)
{
- if (valueName == "CriticalAlarmHigh" ||
- valueName == "CriticalAlarmLow")
- {
- const bool* asserted = std::get_if<bool>(&value);
- if (asserted == nullptr)
- {
- BMCWEB_LOG_ERROR << "Illegal sensor threshold";
- }
- else if (*asserted)
- {
- return "Critical";
- }
- }
+ BMCWEB_LOG_ERROR << "Illegal sensor threshold";
+ }
+ else if (*asserted)
+ {
+ return "Critical";
}
}
}
@@ -755,25 +749,18 @@
}
// Check if sensor has warning threshold alarm
- for (const auto& [interface, values] : interfacesDict)
+ for (const auto& [valueName, value] : valuesDict)
{
- if (interface == "xyz.openbmc_project.Sensor.Threshold.Warning")
+ if (valueName == "WarningAlarmHigh" || valueName == "WarningAlarmLow")
{
- for (const auto& [valueName, value] : values)
+ const bool* asserted = std::get_if<bool>(&value);
+ if (asserted == nullptr)
{
- if (valueName == "WarningAlarmHigh" ||
- valueName == "WarningAlarmLow")
- {
- const bool* asserted = std::get_if<bool>(&value);
- if (asserted == nullptr)
- {
- BMCWEB_LOG_ERROR << "Illegal sensor threshold";
- }
- else if (*asserted)
- {
- return "Warning";
- }
- }
+ BMCWEB_LOG_ERROR << "Illegal sensor threshold";
+ }
+ else if (*asserted)
+ {
+ return "Warning";
}
}
}
@@ -808,45 +795,39 @@
* @param sensorName The name of the sensor to be built
* @param sensorType The type (temperature, fan_tach, etc) of the sensor to
* build
- * @param sensorsAsyncResp Sensor metadata
- * @param interfacesDict A dictionary of the interfaces and properties of said
- * interfaces to be built from
- * @param sensor_json The json object to fill
+ * @param chassisSubNode The subnode (thermal, sensor, ect) of the sensor
+ * @param propertiesDict A dictionary of the properties to build the sensor
+ * from.
+ * @param sensorJson The json object to fill
* @param inventoryItem D-Bus inventory item associated with the sensor. Will
* be nullptr if no associated inventory item was found.
*/
-inline void objectInterfacesToJson(
- const std::string& sensorName, const std::string& sensorType,
- const std::shared_ptr<SensorsAsyncResp>& sensorsAsyncResp,
- const dbus::utility::DBusInteracesMap& interfacesDict,
+inline void objectPropertiesToJson(
+ std::string_view sensorName, std::string_view sensorType,
+ std::string_view chassisSubNode,
+ const dbus::utility::DBusPropertiesMap& propertiesDict,
nlohmann::json& sensorJson, InventoryItem* inventoryItem)
{
// Assume values exist as is (10^0 == 1) if no scale exists
int64_t scaleMultiplier = 0;
- for (const auto& [interface, values] : interfacesDict)
+ for (const auto& [valueName, value] : propertiesDict)
{
- if (interface == "xyz.openbmc_project.Sensor.Value")
+ if (valueName == "Scale")
{
- for (const auto& [valueName, value] : values)
+ const int64_t* int64Value = std::get_if<int64_t>(&value);
+ if (int64Value != nullptr)
{
- if (valueName == "Scale")
- {
- const int64_t* int64Value = std::get_if<int64_t>(&value);
- if (int64Value != nullptr)
- {
- scaleMultiplier = *int64Value;
- }
- }
+ scaleMultiplier = *int64Value;
}
}
}
- if (sensorsAsyncResp->chassisSubNode == sensors::node::sensors)
+ if (chassisSubNode == sensors::node::sensors)
{
- // For sensors in SensorCollection we set Id instead of MemberId,
- // including power sensors.
sensorJson["Id"] = sensorName;
- sensorJson["Name"] = boost::replace_all_copy(sensorName, "_", " ");
+ std::string sensorNameEs(sensorName);
+ std::replace(sensorNameEs.begin(), sensorNameEs.end(), '_', ' ');
+ sensorJson["Name"] = std::move(sensorNameEs);
}
else if (sensorType != "power")
{
@@ -854,12 +835,14 @@
// PowerControl, those properties have more general values because
// multiple sensors can be stored in the same JSON object.
sensorJson["MemberId"] = sensorName;
- sensorJson["Name"] = boost::replace_all_copy(sensorName, "_", " ");
+ std::string sensorNameEs(sensorName);
+ std::replace(sensorNameEs.begin(), sensorNameEs.end(), '_', ' ');
+ sensorJson["Name"] = std::move(sensorNameEs);
}
sensorJson["Status"]["State"] = getState(inventoryItem);
sensorJson["Status"]["Health"] =
- getHealth(sensorJson, interfacesDict, inventoryItem);
+ getHealth(sensorJson, propertiesDict, inventoryItem);
// Parameter to set to override the type we get from dbus, and force it to
// int, regardless of what is available. This is used for schemas like fan,
@@ -867,11 +850,11 @@
bool forceToInt = false;
nlohmann::json::json_pointer unit("/Reading");
- if (sensorsAsyncResp->chassisSubNode == sensors::node::sensors)
+ if (chassisSubNode == sensors::node::sensors)
{
sensorJson["@odata.type"] = "#Sensor.v1_2_0.Sensor";
- const std::string& readingType = sensors::toReadingType(sensorType);
+ std::string_view readingType = sensors::toReadingType(sensorType);
if (readingType.empty())
{
BMCWEB_LOG_ERROR << "Redfish cannot map reading type for "
@@ -882,7 +865,7 @@
sensorJson["ReadingType"] = readingType;
}
- const std::string& readingUnits = sensors::toReadingUnits(sensorType);
+ std::string_view readingUnits = sensors::toReadingUnits(sensorType);
if (readingUnits.empty())
{
BMCWEB_LOG_ERROR << "Redfish cannot map reading unit for "
@@ -923,10 +906,7 @@
}
else if (sensorType == "power")
{
- std::string sensorNameLower =
- boost::algorithm::to_lower_copy(sensorName);
-
- if (sensorName == "total_power")
+ if (boost::iequals(sensorName, "total_power"))
{
sensorJson["@odata.type"] = "#Power.v1_0_0.PowerControl";
// Put multiple "sensors" into a single PowerControl, so have
@@ -935,7 +915,7 @@
sensorJson["Name"] = "Chassis Power Control";
unit = "/PowerConsumedWatts"_json_pointer;
}
- else if (sensorNameLower.find("input") != std::string::npos)
+ else if (boost::ifind_first(sensorName, "input").empty())
{
unit = "/PowerInputWatts"_json_pointer;
}
@@ -957,7 +937,7 @@
properties.emplace_back("xyz.openbmc_project.Sensor.Value", "Value", unit);
- if (sensorsAsyncResp->chassisSubNode == sensors::node::sensors)
+ if (chassisSubNode == sensors::node::sensors)
{
properties.emplace_back(
"xyz.openbmc_project.Sensor.Threshold.Warning", "WarningHigh",
@@ -990,7 +970,7 @@
// TODO Need to get UpperThresholdFatal and LowerThresholdFatal
- if (sensorsAsyncResp->chassisSubNode == sensors::node::sensors)
+ if (chassisSubNode == sensors::node::sensors)
{
properties.emplace_back("xyz.openbmc_project.Sensor.Value", "MinValue",
"/ReadingRangeMin"_json_pointer);
@@ -1015,67 +995,82 @@
for (const std::tuple<const char*, const char*,
nlohmann::json::json_pointer>& p : properties)
{
- for (const auto& [interface, values] : interfacesDict)
+ for (const auto& [valueName, valueVariant] : propertiesDict)
{
- if (interface != std::get<0>(p))
+ if (valueName != std::get<1>(p))
{
continue;
}
- for (const auto& [valueName, valueVariant] : values)
+
+ // The property we want to set may be nested json, so use
+ // a json_pointer for easy indexing into the json structure.
+ const nlohmann::json::json_pointer& key = std::get<2>(p);
+
+ // Attempt to pull the int64 directly
+ const int64_t* int64Value = std::get_if<int64_t>(&valueVariant);
+
+ const double* doubleValue = std::get_if<double>(&valueVariant);
+ const uint32_t* uValue = std::get_if<uint32_t>(&valueVariant);
+ double temp = 0.0;
+ if (int64Value != nullptr)
{
- if (valueName != std::get<1>(p))
- {
- continue;
- }
-
- // The property we want to set may be nested json, so use
- // a json_pointer for easy indexing into the json structure.
- const nlohmann::json::json_pointer& key = std::get<2>(p);
-
- // Attempt to pull the int64 directly
- const int64_t* int64Value = std::get_if<int64_t>(&valueVariant);
-
- const double* doubleValue = std::get_if<double>(&valueVariant);
- const uint32_t* uValue = std::get_if<uint32_t>(&valueVariant);
- double temp = 0.0;
- if (int64Value != nullptr)
- {
- temp = static_cast<double>(*int64Value);
- }
- else if (doubleValue != nullptr)
- {
- temp = *doubleValue;
- }
- else if (uValue != nullptr)
- {
- temp = *uValue;
- }
- else
- {
- BMCWEB_LOG_ERROR
- << "Got value interface that wasn't int or double";
- continue;
- }
- temp = temp * std::pow(10, scaleMultiplier);
- if (forceToInt)
- {
- sensorJson[key] = static_cast<int64_t>(temp);
- }
- else
- {
- sensorJson[key] = temp;
- }
+ temp = static_cast<double>(*int64Value);
+ }
+ else if (doubleValue != nullptr)
+ {
+ temp = *doubleValue;
+ }
+ else if (uValue != nullptr)
+ {
+ temp = *uValue;
+ }
+ else
+ {
+ BMCWEB_LOG_ERROR
+ << "Got value interface that wasn't int or double";
+ continue;
+ }
+ temp = temp * std::pow(10, scaleMultiplier);
+ if (forceToInt)
+ {
+ sensorJson[key] = static_cast<int64_t>(temp);
+ }
+ else
+ {
+ sensorJson[key] = temp;
}
}
}
- sensorsAsyncResp->addMetadata(sensorJson, unit.to_string(),
- "/xyz/openbmc_project/sensors/" + sensorType +
- "/" + sensorName);
-
BMCWEB_LOG_DEBUG << "Added sensor " << sensorName;
}
+/**
+ * @brief Builds a json sensor representation of a sensor.
+ * @param sensorName The name of the sensor to be built
+ * @param sensorType The type (temperature, fan_tach, etc) of the sensor to
+ * build
+ * @param chassisSubNode The subnode (thermal, sensor, ect) of the sensor
+ * @param interfacesDict A dictionary of the interfaces and properties of said
+ * interfaces to be built from
+ * @param sensorJson The json object to fill
+ * @param inventoryItem D-Bus inventory item associated with the sensor. Will
+ * be nullptr if no associated inventory item was found.
+ */
+inline void objectInterfacesToJson(
+ const std::string& sensorName, const std::string& sensorType,
+ const std::string& chassisSubNode,
+ const dbus::utility::DBusInteracesMap& interfacesDict,
+ nlohmann::json& sensorJson, InventoryItem* inventoryItem)
+{
+
+ for (const auto& [interface, valuesDict] : interfacesDict)
+ {
+ objectPropertiesToJson(sensorName, sensorType, chassisSubNode,
+ valuesDict, sensorJson, inventoryItem);
+ }
+}
+
inline void populateFanRedundancy(
const std::shared_ptr<SensorsAsyncResp>& sensorsAsyncResp)
{
@@ -2562,9 +2557,17 @@
if (sensorJson != nullptr)
{
- objectInterfacesToJson(
- sensorName, sensorType, sensorsAsyncResp,
- objDictEntry.second, *sensorJson, inventoryItem);
+ objectInterfacesToJson(sensorName, sensorType,
+ sensorsAsyncResp->chassisSubNode,
+ objDictEntry.second, *sensorJson,
+ inventoryItem);
+
+ std::string path = "/xyz/openbmc_project/sensors/";
+ path += sensorType;
+ path += "/";
+ path += sensorName;
+ sensorsAsyncResp->addMetadata(*sensorJson, sensorType,
+ path);
}
}
if (sensorsAsyncResp.use_count() == 1)