manager: add timer for failed LED actions
The current manager performs LED ActionSet one-time only when LED group
asserted property changes, so if the physical LED object path is not
ready when it is set, the LED will be in an error state. Most of time
this error occurs at BMC booting due phospohr-led-manager starts before
LED driver is probed.
In order to correct LED state after physical LED is ready, add a timer
to repeatedly setting failed LED after 1 second.
Change-Id: I0bb46189c79c961cdaa501a7386346c2bb351252
Signed-off-by: Potin Lai <potin.lai@quantatw.com>
diff --git a/manager/led-main.cpp b/manager/led-main.cpp
index f12dd20..281cac9 100644
--- a/manager/led-main.cpp
+++ b/manager/led-main.cpp
@@ -42,7 +42,7 @@
#endif
/** @brief Group manager object */
- phosphor::led::Manager manager(bus, systemLedMap);
+ phosphor::led::Manager manager(bus, systemLedMap, event);
/** @brief sd_bus object manager */
sdbusplus::server::manager_t objManager(bus,
diff --git a/manager/manager.cpp b/manager/manager.cpp
index a9fd21e..6d223b8 100644
--- a/manager/manager.cpp
+++ b/manager/manager.cpp
@@ -118,34 +118,41 @@
return;
}
#endif
- // This order of LED operation is important.
- if (ledsDeAssert.size())
+ ActionSet newReqChangedLeds;
+ std::vector<std::pair<ActionSet&, ActionSet&>> actionsVec = {
+ {reqLedsAssert, ledsAssert}, {reqLedsDeAssert, ledsDeAssert}};
+
+ timer.setEnabled(false);
+ std::set_union(ledsAssert.begin(), ledsAssert.end(), ledsDeAssert.begin(),
+ ledsDeAssert.end(),
+ std::inserter(newReqChangedLeds, newReqChangedLeds.begin()),
+ ledLess);
+
+ // prepare reqLedsAssert & reqLedsDeAssert
+ for (auto pair : actionsVec)
{
- for (const auto& it : ledsDeAssert)
- {
- std::string objPath = std::string(phyLedPath) + it.name;
- lg2::debug("De-Asserting LED, NAME = {NAME}", "NAME", it.name);
- drivePhysicalLED(objPath, Layout::Action::Off, it.dutyOn,
- it.period);
- }
+ ActionSet tmpSet;
+
+ // Discard current required LED actions, if these LEDs have new actions
+ // in newReqChangedLeds.
+ std::set_difference(pair.first.begin(), pair.first.end(),
+ newReqChangedLeds.begin(), newReqChangedLeds.end(),
+ std::inserter(tmpSet, tmpSet.begin()), ledLess);
+
+ // Union the remaining LED actions with new LED actions.
+ pair.first.clear();
+ std::set_union(tmpSet.begin(), tmpSet.end(), pair.second.begin(),
+ pair.second.end(),
+ std::inserter(pair.first, pair.first.begin()), ledLess);
}
- if (ledsAssert.size())
- {
- for (const auto& it : ledsAssert)
- {
- std::string objPath = std::string(phyLedPath) + it.name;
- lg2::debug("Asserting LED, NAME = {NAME}", "NAME", it.name);
- drivePhysicalLED(objPath, it.action, it.dutyOn, it.period);
- }
- }
+ driveLedsHandler();
return;
}
// Calls into driving physical LED post choosing the action
-void Manager::drivePhysicalLED(const std::string& objPath,
- Layout::Action action, uint8_t dutyOn,
- const uint16_t period)
+int Manager::drivePhysicalLED(const std::string& objPath, Layout::Action action,
+ uint8_t dutyOn, const uint16_t period)
{
try
{
@@ -167,9 +174,10 @@
lg2::error(
"Error setting property for physical LED, ERROR = {ERROR}, OBJECT_PATH = {PATH}",
"ERROR", e, "PATH", objPath);
+ return -1;
}
- return;
+ return 0;
}
/** @brief Returns action string based on enum */
@@ -194,5 +202,55 @@
}
}
+void Manager::driveLedsHandler(void)
+{
+ ActionSet failedLedsAssert;
+ ActionSet failedLedsDeAssert;
+
+ // This order of LED operation is important.
+ if (reqLedsDeAssert.size())
+ {
+ for (const auto& it : reqLedsDeAssert)
+ {
+ std::string objPath = std::string(phyLedPath) + it.name;
+ lg2::debug("De-Asserting LED, NAME = {NAME}, ACTION = {ACTION}",
+ "NAME", it.name, "ACTION", it.action);
+ if (drivePhysicalLED(objPath, Layout::Action::Off, it.dutyOn,
+ it.period))
+ {
+ failedLedsDeAssert.insert(it);
+ }
+ }
+ }
+
+ if (reqLedsAssert.size())
+ {
+ for (const auto& it : reqLedsAssert)
+ {
+ std::string objPath = std::string(phyLedPath) + it.name;
+ lg2::debug("Asserting LED, NAME = {NAME}, ACTION = {ACTION}",
+ "NAME", it.name, "ACTION", it.action);
+ if (drivePhysicalLED(objPath, it.action, it.dutyOn, it.period))
+ {
+ failedLedsAssert.insert(it);
+ }
+ }
+ }
+
+ reqLedsAssert = failedLedsAssert;
+ reqLedsDeAssert = failedLedsDeAssert;
+
+ if (reqLedsDeAssert.empty() && reqLedsAssert.empty())
+ {
+ timer.setEnabled(false);
+ }
+ else
+ {
+ timer.restartOnce(std::chrono::seconds(1));
+ }
+
+ return;
+}
+
} // namespace led
} // namespace phosphor
diff --git a/manager/manager.hpp b/manager/manager.hpp
index d8573b6..caa7ba8 100644
--- a/manager/manager.hpp
+++ b/manager/manager.hpp
@@ -3,6 +3,9 @@
#include "ledlayout.hpp"
#include "utils.hpp"
+#include <sdeventplus/event.hpp>
+#include <sdeventplus/utility/timer.hpp>
+
#include <set>
#include <string>
#include <unordered_map>
@@ -75,9 +78,13 @@
*
* @param [in] bus - sdbusplus handler
* @param [in] GroupMap - LEDs group layout
+ * @param [in] Event - sd event handler
*/
- Manager(sdbusplus::bus_t& bus, const GroupMap& ledLayout) :
- ledMap(ledLayout), bus(bus)
+ Manager(
+ sdbusplus::bus_t& bus, const GroupMap& ledLayout,
+ const sdeventplus::Event& event = sdeventplus::Event::get_default()) :
+ ledMap(ledLayout),
+ bus(bus), timer(event, [this](auto&) { driveLedsHandler(); })
{
// Nothing here
}
@@ -112,9 +119,11 @@
* @param[in] action - Intended action to be triggered
* @param[in] dutyOn - Duty Cycle ON percentage
* @param[in] period - Time taken for one blink cycle
+ *
+ * @return: - 0: success, -1: LED set failed
*/
- void drivePhysicalLED(const std::string& objPath, Layout::Action action,
- uint8_t dutyOn, const uint16_t period);
+ int drivePhysicalLED(const std::string& objPath, Layout::Action action,
+ uint8_t dutyOn, const uint16_t period);
/** @brief Set lamp test callback when enabled lamp test.
*
@@ -149,6 +158,18 @@
std::function<bool(ActionSet& ledsAssert, ActionSet& ledsDeAssert)>
lampTestCallBack;
+ /** @brief Timer used for LEDs handler callback*/
+ sdeventplus::utility::Timer<sdeventplus::ClockId::Monotonic> timer;
+
+ /** @brief Contains the required set of assert LEDs action */
+ ActionSet reqLedsAssert;
+
+ /** @brief Contains the required set of deassert LEDs action */
+ ActionSet reqLedsDeAssert;
+
+ /** @brief LEDs handler callback */
+ void driveLedsHandler();
+
/** @brief Returns action string based on enum
*
* @param[in] action - Action enum