Get version directly on PSU plugged in
When PSU is plugged in, get the version directly instead of waiting for
the VERSION interface change on the inventory object.
This simplies the code a lot in onPsuInventoryChanged(), and it does not
depend on the VERSION interface on inventory object anymore.
Tested: Verify on Witherspoon that when PSU is plugged out and in, the
PSU software object is removed and created as expected.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: I2770049baf8ffa8f1d91391fd9225a237acb24dc
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index cf691d7..50c1a26 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -199,8 +199,6 @@
auto versionId = utils::getVersionId(psuVersion);
auto path = std::string(SOFTWARE_OBJPATH) + "/" + versionId;
- psuStatusMap[psuInventoryPath] = {true, psuVersion};
-
auto it = activations.find(versionId);
if (it != activations.end())
{
@@ -238,7 +236,6 @@
void ItemUpdater::removePsuObject(const std::string& psuInventoryPath)
{
- psuStatusMap[psuInventoryPath] = {false, ""};
auto it = psuPathActivationMap.find(psuInventoryPath);
if (it == psuPathActivationMap.end())
{
@@ -300,52 +297,27 @@
void ItemUpdater::onPsuInventoryChanged(const std::string& psuPath,
const Properties& properties)
{
- std::optional<bool> present;
- std::optional<std::string> version;
+ bool present;
+ std::string version;
- // The code was expecting to get callback on mutliple properties changed.
- // But in practice, the callback is received one-by-one for each property.
- // So it has to handle Present and Version property separately.
+ // Only present property is interested
auto p = properties.find(PRESENT);
- if (p != properties.end())
- {
- present = sdbusplus::message::variant_ns::get<bool>(p->second);
- psuStatusMap[psuPath].present = *present;
- }
- p = properties.find(VERSION);
- if (p != properties.end())
- {
- version = sdbusplus::message::variant_ns::get<std::string>(p->second);
- psuStatusMap[psuPath].version = *version;
- }
-
- // If present or version is not changed, ignore
- if (!present.has_value() && !version.has_value())
+ if (p == properties.end())
{
return;
}
+ present = sdbusplus::message::variant_ns::get<bool>(p->second);
- if (psuStatusMap[psuPath].present)
+ if (present)
{
- // If version is not updated, let's wait for it
- if (psuStatusMap[psuPath].version.empty())
+ version = utils::getVersion(psuPath);
+ if (!version.empty())
{
- log<level::DEBUG>("Waiting for version to be updated");
- return;
+ createPsuObject(psuPath, version);
}
- // Create object or association based on the version and psu inventory
- // path
- createPsuObject(psuPath, psuStatusMap[psuPath].version);
}
else
{
- if (!present.has_value())
- {
- // If a PSU is plugged out, version property is update to empty as
- // well, and we get callback here, but ignore that because it is
- // handled by PRESENT callback.
- return;
- }
// Remove object or association
removePsuObject(psuPath);
}
@@ -366,10 +338,7 @@
}
// Add matches for PSU Inventory's property changes
psuMatches.emplace_back(
- bus,
- MatchRules::type::signal() + MatchRules::path(p) +
- MatchRules::member("PropertiesChanged") +
- MatchRules::interface("org.freedesktop.DBus.Properties"),
+ bus, MatchRules::propertiesChanged(p, ITEM_IFACE),
std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
std::placeholders::_1));
}
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index f6fefff..eabe66f 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -163,16 +163,6 @@
std::map<std::string, const std::unique_ptr<Activation>&>
psuPathActivationMap;
- /** @brief A struct to hold the PSU present status and version */
- struct psuStatus
- {
- bool present;
- std::string version;
- };
-
- /** @brief The map of PSU inventory path and the psuStatus */
- std::map<std::string, psuStatus> psuStatusMap;
-
/** @brief sdbusplus signal match for PSU Software*/
sdbusplus::bus::match_t versionMatch;
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
index 0ae9c76..9f05ed2 100644
--- a/test/test_item_updater.cpp
+++ b/test/test_item_updater.cpp
@@ -271,8 +271,9 @@
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
// The PSU is present and version is added in a single call
- Properties propAdded{{PRESENT, PropertyType(true)},
- {VERSION, PropertyType(std::string(version))}};
+ Properties propAdded{{PRESENT, PropertyType(true)}};
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
+ .WillOnce(Return(std::string(version)));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
.Times(2);
onPsuInventoryChanged(psuPath, propAdded);
@@ -310,11 +311,11 @@
onPsuInventoryChanged(psuPath, propRemoved);
Properties propAdded{{PRESENT, PropertyType(true)}};
- Properties propVersion{{VERSION, PropertyType(std::string(version))}};
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
+ .WillOnce(Return(std::string(version)));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
.Times(2);
onPsuInventoryChanged(psuPath, propAdded);
- onPsuInventoryChanged(psuPath, propVersion);
// on exit, objects are removed
EXPECT_CALL(sdbusMock, sd_bus_emit_object_removed(_, StrEq(objPath)))
@@ -384,10 +385,12 @@
// Add PSU0 and PSU1 back, but PSU1 with a different version
version1 = "version1";
objPath1 = getObjPath(version1);
- Properties propAdded0{{PRESENT, PropertyType(true)},
- {VERSION, PropertyType(std::string(version0))}};
- Properties propAdded1{{PRESENT, PropertyType(true)},
- {VERSION, PropertyType(std::string(version1))}};
+ Properties propAdded0{{PRESENT, PropertyType(true)}};
+ Properties propAdded1{{PRESENT, PropertyType(true)}};
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psu0)))
+ .WillOnce(Return(std::string(version0)));
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
+ .WillOnce(Return(std::string(version1)));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath0)))
.Times(2);
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath1)))