PEL: Change method of asserting a fault LED
There was a recent change in direction on how PELs should request that
fault LEDs be turned on. Previously, the code would talk to the LED
group objects directly. The new direction is to set the Functional
property on the OperationalStatus interface on the inventory objects in
question to false, and the LED manager code will watch that to know when
to turn on the LEDs.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: Ieebb09ba002843cf863359a09aba26540356aa91
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index 518df8f..adea6c9 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -67,8 +67,9 @@
constexpr auto compatible =
"xyz.openbmc_project.Configuration.IBMCompatibleSystem";
constexpr auto vpdManager = "com.ibm.VPD.Manager";
-constexpr auto association = "xyz.openbmc_project.Association";
constexpr auto ledGroup = "xyz.openbmc_project.Led.Group";
+constexpr auto operationalStatus =
+ "xyz.openbmc_project.State.Decorator.OperationalStatus";
} // namespace interface
using namespace sdbusplus::xyz::openbmc_project::State::Boot::server;
@@ -409,24 +410,6 @@
return shortest;
}
-std::string
- DataInterface::getFaultLEDGroup(const std::string& inventoryPath) const
-{
- auto associationPath = inventoryPath + "/" + "fault_led_group";
- auto service = getService(associationPath, interface::association);
-
- DBusValue endpoints;
- getProperty(service, associationPath, interface::association, "endpoints",
- endpoints);
- auto paths = std::get<std::vector<std::string>>(endpoints);
- if (paths.empty())
- {
- throw std::runtime_error("Association endpoints property empty");
- }
-
- return paths[0];
-}
-
void DataInterface::assertLEDGroup(const std::string& ledGroup,
bool value) const
{
@@ -438,6 +421,19 @@
_bus.call(method);
}
+void DataInterface::setFunctional(const std::string& objectPath,
+ bool value) const
+{
+ DBusValue variant = value;
+ auto service = getService(objectPath, interface::operationalStatus);
+
+ auto method = _bus.new_method_call(service.c_str(), objectPath.c_str(),
+ interface::dbusProperty, "Set");
+
+ method.append(interface::operationalStatus, "Functional", variant);
+ _bus.call(method);
+}
+
std::vector<std::string> DataInterface::getSystemNames() const
{
DBusSubTree subtree;
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index 9a48721..385b07a 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -279,17 +279,6 @@
bool expanded) const = 0;
/**
- * @brief Returns the fault LED group D-Bus path for the inventory
- * D-Bus path passed in.
- *
- * @param[in] inventoryPath - The inventory D-Bus path
- *
- * @return std::string - The fault LED group D-Bus path
- */
- virtual std::string
- getFaultLEDGroup(const std::string& inventoryPath) const = 0;
-
- /**
* @brief Sets the Asserted property on the LED group passed in.
*
* @param[in] ledGroup - The LED group D-Bus path
@@ -298,6 +287,16 @@
virtual void assertLEDGroup(const std::string& ledGroup,
bool value) const = 0;
+ /**
+ * @brief Sets the Functional property on the OperationalStatus
+ * interface on a D-Bus object.
+ *
+ * @param[in] objectPath - The D-Bus object path
+ * @param[in] functional - The value
+ */
+ virtual void setFunctional(const std::string& objectPath,
+ bool functional) const = 0;
+
protected:
/**
* @brief Sets the host on/off state and runs any
@@ -528,17 +527,6 @@
bool expanded) const override;
/**
- * @brief Returns the fault LED group D-Bus path for the inventory
- * D-Bus path passed in.
- *
- * @param[in] inventoryPath - The inventory D-Bus path
- *
- * @return std::string - The fault LED group D-Bus path
- */
- std::string
- getFaultLEDGroup(const std::string& inventoryPath) const override;
-
- /**
* @brief Sets the Asserted property on the LED group passed in.
*
* @param[in] ledGroup - The LED group D-Bus path
@@ -546,6 +534,16 @@
*/
void assertLEDGroup(const std::string& ledGroup, bool value) const override;
+ /**
+ * @brief Sets the Functional property on the OperationalStatus
+ * interface on a D-Bus object.
+ *
+ * @param[in] objectPath - The D-Bus object path
+ * @param[in] functional - The value
+ */
+ void setFunctional(const std::string& objectPath,
+ bool functional) const override;
+
private:
/**
* @brief Reads the BMC firmware version string and puts it into
diff --git a/extensions/openpower-pels/service_indicators.cpp b/extensions/openpower-pels/service_indicators.cpp
index 7f795f9..1c9eff0 100644
--- a/extensions/openpower-pels/service_indicators.cpp
+++ b/extensions/openpower-pels/service_indicators.cpp
@@ -75,12 +75,11 @@
auto locCodes = getLocationCodes(callouts);
if (!locCodes.empty())
{
- // Find the LED groups for those location codes.
- auto ledPaths = getLEDGroupPaths(locCodes);
- if (!ledPaths.empty())
+ // Find the inventory paths for those location codes.
+ auto paths = getInventoryPaths(locCodes);
+ if (!paths.empty())
{
- // Tell the LED groups to assert their LEDs.
- assertLEDs(ledPaths);
+ setNotFunctional(paths);
sai = false;
}
}
@@ -193,18 +192,19 @@
return false;
}
-std::vector<std::string> LightPath::getLEDGroupPaths(
+std::vector<std::string> LightPath::getInventoryPaths(
const std::vector<std::string>& locationCodes) const
{
- std::vector<std::string> ledGroups;
+ std::vector<std::string> paths;
std::string inventoryPath;
for (const auto& locCode : locationCodes)
{
try
{
- inventoryPath =
+ auto inventoryPath =
_dataIface.getInventoryFromLocCode(locCode, 0, true);
+ paths.push_back(std::move(inventoryPath));
}
catch (const std::exception& e)
{
@@ -213,43 +213,31 @@
locCode, e.what())
.c_str());
- // Unless we can get the LEDs for all FRUs, we can't turn
+ // Unless we can set the LEDs for all FRUs, we can't turn
// on any of them, so clear the list and quit.
- ledGroups.clear();
- break;
- }
-
- try
- {
- ledGroups.push_back(_dataIface.getFaultLEDGroup(inventoryPath));
- }
- catch (const std::exception& e)
- {
- log<level::ERR>(fmt::format("Could not get LED group path for "
- "inventory path {} ({}).",
- inventoryPath, e.what())
- .c_str());
- ledGroups.clear();
+ paths.clear();
break;
}
}
- return ledGroups;
+ return paths;
}
-void LightPath::assertLEDs(const std::vector<std::string>& ledGroups) const
+void LightPath::setNotFunctional(
+ const std::vector<std::string>& inventoryPaths) const
{
- for (const auto& ledGroup : ledGroups)
+ for (const auto& path : inventoryPaths)
{
try
{
- _dataIface.assertLEDGroup(ledGroup, true);
+ _dataIface.setFunctional(path, false);
}
catch (const std::exception& e)
{
- log<level::ERR>(fmt::format("Failed to assert LED group {} ({})",
- ledGroup, e.what())
- .c_str());
+ log<level::INFO>(
+ fmt::format("Could not write Functional property on {} ({})",
+ path, e.what())
+ .c_str());
}
}
}
diff --git a/extensions/openpower-pels/service_indicators.hpp b/extensions/openpower-pels/service_indicators.hpp
index 522f3db..85adf2b 100644
--- a/extensions/openpower-pels/service_indicators.hpp
+++ b/extensions/openpower-pels/service_indicators.hpp
@@ -123,22 +123,26 @@
private:
/**
- * @brief Returns the LED group D-Bus paths to use to turn on the
- * LEDs for the passed in location codes.
+ * @brief Returns the inventory D-Bus paths for the passed
+ * in location codes.
*
* @param[in] locationCodes - The location codes
*
- * @return std::vector<std::string> - The LED group D-Bus paths
+ * @return std::vector<std::string> - The inventory D-Bus paths
*/
std::vector<std::string>
- getLEDGroupPaths(const std::vector<std::string>& locationCodes) const;
+ getInventoryPaths(const std::vector<std::string>& locationCodes) const;
/**
- * @brief Sets the Assert property on the LED group D-Bus objects
+ * @brief Sets the Functional property on the passed in
+ * inventory paths to false.
*
- * @param[in] std::vector<std::string> - The LED group D-Bus paths
+ * There is code watching for this that will then turn on
+ * any LEDs for that FRU.
+ *
+ * @param[in] inventoryPaths - The inventory D-Bus paths
*/
- void assertLEDs(const std::vector<std::string>& ledGroups) const;
+ void setNotFunctional(const std::vector<std::string>& inventoryPaths) const;
/**
* @brief Checks if the callout priority is one that the policy