used sdbusplus::unpackPropertiesNoThrow part 3
used sdbusplus::unpackPropertiesNoThrow in systems.hpp, also replaced
all usages of "GetAll" with sdbusplus::asio::getAllProperties
bmcweb size: 2672984 -> 2668888 (-4096)
compressed size: 1128611 -> 1127280 (-1331)
Tested:
Performed get on:
- /redfish/v1/Systems/system
- /redfish/v1/Systems/system/Memory/dimm0
Get result before and after the change was the same
Change-Id: I11ca70bae07bdb17edd1935c539c68814d6ec172
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp
index 0c2c53c..60bdbb9 100644
--- a/redfish-core/lib/systems.hpp
+++ b/redfish-core/lib/systems.hpp
@@ -28,6 +28,8 @@
#include <dbus_utility.hpp>
#include <registries/privilege_registry.hpp>
#include <sdbusplus/asio/property.hpp>
+#include <sdbusplus/unpack_properties.hpp>
+#include <utils/dbus_utils.hpp>
#include <utils/json_utils.hpp>
#include <utils/sw_utils.hpp>
@@ -164,37 +166,32 @@
"xyz.openbmc_project.State.Decorator.OperationalStatus", "Functional",
std::move(getCpuFunctionalState));
- for (const auto& property : properties)
+ // TODO: Get Model
+
+ const uint16_t* coreCount = nullptr;
+
+ const bool success = sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(), properties, "CoreCount", coreCount);
+
+ if (!success)
{
+ messages::internalError(aResp->res);
+ return;
+ }
- // TODO: Get Model
+ if (coreCount != nullptr)
+ {
+ nlohmann::json& coreCountJson =
+ aResp->res.jsonValue["ProcessorSummary"]["CoreCount"];
+ uint64_t* coreCountJsonPtr = coreCountJson.get_ptr<uint64_t*>();
- // Get CoreCount
- if (property.first == "CoreCount")
+ if (coreCountJsonPtr == nullptr)
{
-
- // Get CPU CoreCount and add it to the total
- const uint16_t* coreCountVal =
- std::get_if<uint16_t>(&property.second);
-
- if (coreCountVal == nullptr)
- {
- messages::internalError(aResp->res);
- return;
- }
-
- nlohmann::json& coreCount =
- aResp->res.jsonValue["ProcessorSummary"]["CoreCount"];
- uint64_t* coreCountPtr = coreCount.get_ptr<uint64_t*>();
-
- if (coreCountPtr == nullptr)
- {
- coreCount = *coreCountVal;
- }
- else
- {
- *coreCountPtr += *coreCountVal;
- }
+ coreCountJson = *coreCount;
+ }
+ else
+ {
+ *coreCountJsonPtr += *coreCount;
}
}
}
@@ -213,7 +210,9 @@
const std::string& path)
{
- crow::connections::systemBus->async_method_call(
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, service, path,
+ "xyz.openbmc_project.Inventory.Item.Cpu",
[aResp, service,
path](const boost::system::error_code ec2,
const dbus::utility::DBusPropertiesMap& properties) {
@@ -224,9 +223,7 @@
return;
}
getProcessorProperties(aResp, service, path, properties);
- },
- service, path, "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.Inventory.Item.Cpu");
+ });
}
/*
@@ -289,7 +286,9 @@
BMCWEB_LOG_DEBUG
<< "Found Dimm, now get its properties.";
- crow::connections::systemBus->async_method_call(
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, connection.first,
+ path, "xyz.openbmc_project.Inventory.Item.Dimm",
[aResp, service{connection.first},
path](const boost::system::error_code ec2,
const dbus::utility::DBusPropertiesMap&
@@ -304,45 +303,7 @@
BMCWEB_LOG_DEBUG << "Got " << properties.size()
<< " Dimm properties.";
- if (!properties.empty())
- {
- for (const std::pair<
- std::string,
- dbus::utility::DbusVariantType>&
- property : properties)
- {
- if (property.first != "MemorySizeInKB")
- {
- continue;
- }
- const uint32_t* value =
- std::get_if<uint32_t>(&property.second);
- if (value == nullptr)
- {
- BMCWEB_LOG_DEBUG
- << "Find incorrect type of MemorySize";
- continue;
- }
- nlohmann::json& totalMemory =
- aResp->res
- .jsonValue["MemorySummary"]
- ["TotalSystemMemoryGiB"];
- const uint64_t* preValue =
- totalMemory.get_ptr<const uint64_t*>();
- if (preValue == nullptr)
- {
- continue;
- }
- aResp->res
- .jsonValue["MemorySummary"]
- ["TotalSystemMemoryGiB"] =
- *value / (1024 * 1024) + *preValue;
- aResp->res.jsonValue["MemorySummary"]
- ["Status"]["State"] =
- "Enabled";
- }
- }
- else
+ if (properties.empty())
{
sdbusplus::asio::getProperty<bool>(
*crow::connections::systemBus, service,
@@ -360,11 +321,50 @@
}
updateDimmProperties(aResp, dimmState);
});
+ return;
}
- },
- connection.first, path,
- "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.Inventory.Item.Dimm");
+
+ const uint32_t* memorySizeInKB = nullptr;
+
+ const bool success =
+ sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(),
+ properties, "MemorySizeInKB",
+ memorySizeInKB);
+
+ if (!success)
+ {
+ messages::internalError(aResp->res);
+ return;
+ }
+
+ if (memorySizeInKB != nullptr)
+ {
+ nlohmann::json& totalMemory =
+ aResp->res
+ .jsonValue["MemorySummary"]
+ ["TotalSystemMemoryGiB"];
+ const uint64_t* preValue =
+ totalMemory.get_ptr<const uint64_t*>();
+ if (preValue == nullptr)
+ {
+ aResp->res
+ .jsonValue["MemorySummary"]
+ ["TotalSystemMemoryGiB"] =
+ *memorySizeInKB / (1024 * 1024);
+ }
+ else
+ {
+ aResp->res
+ .jsonValue["MemorySummary"]
+ ["TotalSystemMemoryGiB"] =
+ *memorySizeInKB / (1024 * 1024) +
+ *preValue;
+ }
+ aResp->res.jsonValue["MemorySummary"]["Status"]
+ ["State"] = "Enabled";
+ }
+ });
memoryHealth->inventory.emplace_back(path);
}
@@ -382,7 +382,10 @@
{
BMCWEB_LOG_DEBUG
<< "Found UUID, now get its properties.";
- crow::connections::systemBus->async_method_call(
+
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, connection.first,
+ path, "xyz.openbmc_project.Common.UUID",
[aResp](const boost::system::error_code ec3,
const dbus::utility::DBusPropertiesMap&
properties) {
@@ -395,42 +398,42 @@
}
BMCWEB_LOG_DEBUG << "Got " << properties.size()
<< " UUID properties.";
- for (const std::pair<
- std::string,
- dbus::utility::DbusVariantType>& property :
- properties)
- {
- if (property.first == "UUID")
- {
- const std::string* value =
- std::get_if<std::string>(
- &property.second);
- if (value != nullptr)
- {
- std::string valueStr = *value;
- if (valueStr.size() == 32)
- {
- valueStr.insert(8, 1, '-');
- valueStr.insert(13, 1, '-');
- valueStr.insert(18, 1, '-');
- valueStr.insert(23, 1, '-');
- }
- BMCWEB_LOG_DEBUG << "UUID = "
- << valueStr;
- aResp->res.jsonValue["UUID"] = valueStr;
- }
- }
+ const std::string* uUID = nullptr;
+
+ const bool success =
+ sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(),
+ properties, "UUID", uUID);
+
+ if (!success)
+ {
+ messages::internalError(aResp->res);
+ return;
}
- },
- connection.first, path,
- "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.Common.UUID");
+
+ if (uUID != nullptr)
+ {
+ std::string valueStr = *uUID;
+ if (valueStr.size() == 32)
+ {
+ valueStr.insert(8, 1, '-');
+ valueStr.insert(13, 1, '-');
+ valueStr.insert(18, 1, '-');
+ valueStr.insert(23, 1, '-');
+ }
+ BMCWEB_LOG_DEBUG << "UUID = " << valueStr;
+ aResp->res.jsonValue["UUID"] = valueStr;
+ }
+ });
}
else if (interfaceName ==
"xyz.openbmc_project.Inventory.Item.System")
{
- crow::connections::systemBus->async_method_call(
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, connection.first,
+ path,
+ "xyz.openbmc_project.Inventory.Decorator.Asset",
[aResp](const boost::system::error_code ec2,
const dbus::utility::DBusPropertiesMap&
propertiesList) {
@@ -442,38 +445,60 @@
}
BMCWEB_LOG_DEBUG << "Got " << propertiesList.size()
<< " properties for system";
- for (const std::pair<
- std::string,
- dbus::utility::DbusVariantType>& property :
- propertiesList)
+
+ const std::string* partNumber = nullptr;
+ const std::string* serialNumber = nullptr;
+ const std::string* manufacturer = nullptr;
+ const std::string* model = nullptr;
+ const std::string* subModel = nullptr;
+
+ const bool success =
+ sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(),
+ propertiesList, "PartNumber", partNumber,
+ "SerialNumber", serialNumber,
+ "Manufacturer", manufacturer, "Model",
+ model, "SubModel", subModel);
+
+ if (!success)
{
- const std::string& propertyName =
- property.first;
- if ((propertyName == "PartNumber") ||
- (propertyName == "SerialNumber") ||
- (propertyName == "Manufacturer") ||
- (propertyName == "Model") ||
- (propertyName == "SubModel"))
- {
- const std::string* value =
- std::get_if<std::string>(
- &property.second);
- if (value != nullptr)
- {
- aResp->res.jsonValue[propertyName] =
- *value;
- }
- }
+ messages::internalError(aResp->res);
+ return;
+ }
+
+ if (partNumber != nullptr)
+ {
+ aResp->res.jsonValue["PartNumber"] =
+ *partNumber;
+ }
+
+ if (serialNumber != nullptr)
+ {
+ aResp->res.jsonValue["SerialNumber"] =
+ *serialNumber;
+ }
+
+ if (manufacturer != nullptr)
+ {
+ aResp->res.jsonValue["Manufacturer"] =
+ *manufacturer;
+ }
+
+ if (model != nullptr)
+ {
+ aResp->res.jsonValue["Model"] = *model;
+ }
+
+ if (subModel != nullptr)
+ {
+ aResp->res.jsonValue["SubModel"] = *subModel;
}
// Grab the bios version
sw_util::populateSoftwareInformation(
aResp, sw_util::biosPurpose, "BiosVersion",
false);
- },
- connection.first, path,
- "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.Inventory.Decorator.Asset");
+ });
sdbusplus::asio::getProperty<std::string>(
*crow::connections::systemBus, connection.first,
@@ -494,6 +519,7 @@
});
}
}
+ break;
}
}
},
@@ -1794,7 +1820,9 @@
inline void getProvisioningStatus(std::shared_ptr<bmcweb::AsyncResp> aResp)
{
BMCWEB_LOG_DEBUG << "Get OEM information.";
- crow::connections::systemBus->async_method_call(
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, "xyz.openbmc_project.PFR.Manager",
+ "/xyz/openbmc_project/pfr", "xyz.openbmc_project.PFR.Attributes",
[aResp](const boost::system::error_code ec,
const dbus::utility::DBusPropertiesMap& propertiesList) {
nlohmann::json& oemPFR =
@@ -1813,17 +1841,15 @@
const bool* provState = nullptr;
const bool* lockState = nullptr;
- for (const std::pair<std::string, dbus::utility::DbusVariantType>&
- property : propertiesList)
+
+ const bool success = sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(), "UfmProvisioned", provState,
+ "UfmLocked", lockState);
+
+ if (!success)
{
- if (property.first == "UfmProvisioned")
- {
- provState = std::get_if<bool>(&property.second);
- }
- else if (property.first == "UfmLocked")
- {
- lockState = std::get_if<bool>(&property.second);
- }
+ messages::internalError(aResp->res);
+ return;
}
if ((provState == nullptr) || (lockState == nullptr))
@@ -1848,10 +1874,7 @@
{
oemPFR["ProvisioningStatus"] = "NotProvisioned";
}
- },
- "xyz.openbmc_project.PFR.Manager", "/xyz/openbmc_project/pfr",
- "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.PFR.Attributes");
+ });
}
#endif
@@ -2167,7 +2190,10 @@
getHostWatchdogTimer(const std::shared_ptr<bmcweb::AsyncResp>& aResp)
{
BMCWEB_LOG_DEBUG << "Get host watchodg";
- crow::connections::systemBus->async_method_call(
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, "xyz.openbmc_project.Watchdog",
+ "/xyz/openbmc_project/watchdog/host0",
+ "xyz.openbmc_project.State.Watchdog",
[aResp](const boost::system::error_code ec,
const dbus::utility::DBusPropertiesMap& properties) {
if (ec)
@@ -2185,44 +2211,35 @@
// watchdog service is running/enabled
hostWatchdogTimer["Status"]["State"] = "Enabled";
- for (const auto& property : properties)
+ const bool* enabled = nullptr;
+ const std::string* expireAction = nullptr;
+
+ const bool success = sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(), properties, "Enabled", enabled,
+ "ExpireAction", expireAction);
+
+ if (!success)
{
- BMCWEB_LOG_DEBUG << "prop=" << property.first;
- if (property.first == "Enabled")
- {
- const bool* state = std::get_if<bool>(&property.second);
-
- if (state == nullptr)
- {
- messages::internalError(aResp->res);
- return;
- }
-
- hostWatchdogTimer["FunctionEnabled"] = *state;
- }
- else if (property.first == "ExpireAction")
- {
- const std::string* s =
- std::get_if<std::string>(&property.second);
- if (s == nullptr)
- {
- messages::internalError(aResp->res);
- return;
- }
-
- std::string action = dbusToRfWatchdogAction(*s);
- if (action.empty())
- {
- messages::internalError(aResp->res);
- return;
- }
- hostWatchdogTimer["TimeoutAction"] = action;
- }
+ messages::internalError(aResp->res);
+ return;
}
- },
- "xyz.openbmc_project.Watchdog", "/xyz/openbmc_project/watchdog/host0",
- "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.State.Watchdog");
+
+ if (enabled != nullptr)
+ {
+ hostWatchdogTimer["FunctionEnabled"] = *enabled;
+ }
+
+ if (expireAction != nullptr)
+ {
+ std::string action = dbusToRfWatchdogAction(*expireAction);
+ if (action.empty())
+ {
+ messages::internalError(aResp->res);
+ return;
+ }
+ hostWatchdogTimer["TimeoutAction"] = action;
+ }
+ });
}
/**
@@ -2301,70 +2318,54 @@
parseIpsProperties(const std::shared_ptr<bmcweb::AsyncResp>& aResp,
const dbus::utility::DBusPropertiesMap& properties)
{
- for (const auto& property : properties)
+ const bool* enabled = nullptr;
+ const uint8_t* enterUtilizationPercent = nullptr;
+ const uint64_t* enterDwellTime = nullptr;
+ const uint8_t* exitUtilizationPercent = nullptr;
+ const uint64_t* exitDwellTime = nullptr;
+
+ const bool success = sdbusplus::unpackPropertiesNoThrow(
+ dbus_utils::UnpackErrorPrinter(), properties, "Enabled", enabled,
+ "EnterUtilizationPercent", enterUtilizationPercent,
+ "ExitUtilizationPercent", exitUtilizationPercent, "ExitDwellTime",
+ exitDwellTime);
+
+ if (!success)
{
- if (property.first == "Enabled")
- {
- const bool* state = std::get_if<bool>(&property.second);
- if (state == nullptr)
- {
- return false;
- }
- aResp->res.jsonValue["IdlePowerSaver"][property.first] = *state;
- }
- else if (property.first == "EnterUtilizationPercent")
- {
- const uint8_t* util = std::get_if<uint8_t>(&property.second);
- if (util == nullptr)
- {
- return false;
- }
- aResp->res.jsonValue["IdlePowerSaver"][property.first] = *util;
- }
- else if (property.first == "EnterDwellTime")
- {
- // Convert Dbus time from milliseconds to seconds
- const uint64_t* timeMilliseconds =
- std::get_if<uint64_t>(&property.second);
- if (timeMilliseconds == nullptr)
- {
- return false;
- }
- const std::chrono::duration<uint64_t, std::milli> ms(
- *timeMilliseconds);
- aResp->res.jsonValue["IdlePowerSaver"]["EnterDwellTimeSeconds"] =
- std::chrono::duration_cast<std::chrono::duration<uint64_t>>(ms)
- .count();
- }
- else if (property.first == "ExitUtilizationPercent")
- {
- const uint8_t* util = std::get_if<uint8_t>(&property.second);
- if (util == nullptr)
- {
- return false;
- }
- aResp->res.jsonValue["IdlePowerSaver"][property.first] = *util;
- }
- else if (property.first == "ExitDwellTime")
- {
- // Convert Dbus time from milliseconds to seconds
- const uint64_t* timeMilliseconds =
- std::get_if<uint64_t>(&property.second);
- if (timeMilliseconds == nullptr)
- {
- return false;
- }
- const std::chrono::duration<uint64_t, std::milli> ms(
- *timeMilliseconds);
- aResp->res.jsonValue["IdlePowerSaver"]["ExitDwellTimeSeconds"] =
- std::chrono::duration_cast<std::chrono::duration<uint64_t>>(ms)
- .count();
- }
- else
- {
- BMCWEB_LOG_WARNING << "Unexpected IdlePowerSaver property: "
- << property.first;
- }
+ return false;
+ }
+
+ if (enabled != nullptr)
+ {
+ aResp->res.jsonValue["IdlePowerSaver"]["Enabled"] = *enabled;
+ }
+
+ if (enterUtilizationPercent != nullptr)
+ {
+ aResp->res.jsonValue["IdlePowerSaver"]["EnterUtilizationPercent"] =
+ *enterUtilizationPercent;
+ }
+
+ if (enterDwellTime != nullptr)
+ {
+ const std::chrono::duration<uint64_t, std::milli> ms(*enterDwellTime);
+ aResp->res.jsonValue["IdlePowerSaver"]["EnterDwellTimeSeconds"] =
+ std::chrono::duration_cast<std::chrono::duration<uint64_t>>(ms)
+ .count();
+ }
+
+ if (exitUtilizationPercent != nullptr)
+ {
+ aResp->res.jsonValue["IdlePowerSaver"]["ExitUtilizationPercent"] =
+ *exitUtilizationPercent;
+ }
+
+ if (exitDwellTime != nullptr)
+ {
+ const std::chrono::duration<uint64_t, std::milli> ms(*exitDwellTime);
+ aResp->res.jsonValue["IdlePowerSaver"]["ExitDwellTimeSeconds"] =
+ std::chrono::duration_cast<std::chrono::duration<uint64_t>>(ms)
+ .count();
}
return true;
@@ -2426,7 +2427,9 @@
}
// Valid IdlePowerSaver object found, now read the current values
- crow::connections::systemBus->async_method_call(
+ sdbusplus::asio::getAllProperties(
+ *crow::connections::systemBus, service, path,
+ "xyz.openbmc_project.Control.Power.IdlePowerSaver",
[aResp](const boost::system::error_code ec2,
const dbus::utility::DBusPropertiesMap& properties) {
if (ec2)
@@ -2442,9 +2445,7 @@
messages::internalError(aResp->res);
return;
}
- },
- service, path, "org.freedesktop.DBus.Properties", "GetAll",
- "xyz.openbmc_project.Control.Power.IdlePowerSaver");
+ });
},
"xyz.openbmc_project.ObjectMapper",
"/xyz/openbmc_project/object_mapper",