Resolve issues in lamp test feature
This commit implements the following changes in lamp test
1. Reset lamp test timer on assert to assert case
2. Reject stopping lamp test when there is a request to
deassert lamp test.
Test:
Case 1: Start lamp test (False to True).
Result: 4mins of lamp test execution and Asserted is set back to
false after 4mins.
Case 2: Stop lamp test (False to False).
Result: Lamp test is not performed and Asserted remains false.
Case 3: Stop request when lamp test is still on (True to false)
Result: Stop request not allowed and lamp test continues to execute.
Asserted stays true until the lamp test ends.
Case 4: Retrigger lamp test (True to True).
Result: 4mins timer restarts. Asserted should be back to false after
4mins.
Change-Id: Ib6086f223d5c5ce80b872ed5f35645893ce79cf9
Signed-off-by: Priyanga Ramasamy <priyanga24@in.ibm.com>
diff --git a/manager/group.cpp b/manager/group.cpp
index c9ad161..93be9a5 100644
--- a/manager/group.cpp
+++ b/manager/group.cpp
@@ -13,6 +13,23 @@
/** @brief Overloaded Property Setter function */
bool Group::asserted(bool value)
{
+ if (customCallBack != nullptr)
+ {
+ // Custom callback method tells if the lamptest request is handled
+ // successfully or not.
+ if (customCallBack(this, value))
+ {
+ // If the lamp test request is handled successfully, update the
+ // asserted property.
+ return sdbusplus::xyz::openbmc_project::Led::server::Group::
+ asserted(value);
+ }
+
+ // If the lamp test request is not handled successfully, return the
+ // existing asserted value without any change.
+ return sdbusplus::xyz::openbmc_project::Led::server::Group::asserted();
+ }
+
// If the value is already what is before, return right away
if (value ==
sdbusplus::xyz::openbmc_project::Led::server::Group::asserted())
@@ -20,15 +37,6 @@
return value;
}
- if (customCallBack != nullptr)
- {
- // Call the custom callback method
- customCallBack(this, value);
-
- return sdbusplus::xyz::openbmc_project::Led::server::Group::asserted(
- value);
- }
-
// Introducing these to enable gtest.
ActionSet ledsAssert{};
ActionSet ledsDeAssert{};
diff --git a/manager/group.hpp b/manager/group.hpp
index e3717ca..2febfbb 100644
--- a/manager/group.hpp
+++ b/manager/group.hpp
@@ -43,7 +43,7 @@
*/
Group(sdbusplus::bus_t& bus, const std::string& objPath, Manager& manager,
std::shared_ptr<Serialize> serializePtr,
- std::function<void(Group*, bool)> callBack = nullptr) :
+ std::function<bool(Group*, bool)> callBack = nullptr) :
GroupInherit(bus, objPath.c_str(), GroupInherit::action::defer_emit),
path(objPath), manager(manager), serializePtr(serializePtr),
@@ -77,8 +77,14 @@
std::shared_ptr<Serialize> serializePtr;
/** @brief Custom callback when LED group is asserted
+ * Callback that holds LED group method which handles lamp test request.
+ *
+ * @param[in] Group object - Pointer to Group object
+ * @param[in] bool - Input value (true/false)
+ *
+ * @return bool which tells if execution succeeds(true) or fails(false).
*/
- std::function<void(Group*, bool)> customCallBack;
+ std::function<bool(Group*, bool)> customCallBack;
};
} // namespace led
diff --git a/manager/lamptest/lamptest.cpp b/manager/lamptest/lamptest.cpp
index 408f832..3dfaaa5 100644
--- a/manager/lamptest/lamptest.cpp
+++ b/manager/lamptest/lamptest.cpp
@@ -167,6 +167,10 @@
{
// reset the timer and then return
timer.restart(std::chrono::seconds(LAMP_TEST_TIMEOUT_IN_SECS));
+
+ // Notify host to reset the timer
+ doHostLampTest(true);
+
return;
}
@@ -190,7 +194,7 @@
timer.restart(std::chrono::seconds(LAMP_TEST_TIMEOUT_IN_SECS));
isLampTestRunning = true;
- // Notify PHYP to start the lamp test
+ // Notify host to start the lamp test
doHostLampTest(true);
// Set all the Physical action to On for lamp test
@@ -222,7 +226,7 @@
groupObj->asserted(false);
}
-void LampTest::requestHandler(Group* group, bool value)
+bool LampTest::requestHandler(Group* group, bool value)
{
if (groupObj == NULL)
{
@@ -232,10 +236,30 @@
if (value)
{
start();
+
+ // Return true in both cases (F -> T && T -> T)
+ return true;
}
else
{
- stop();
+ if (timer.hasExpired())
+ {
+ stop();
+
+ // Return true as the request to stop the lamptest is handled
+ // successfully.
+ return true;
+ }
+ else if (timer.isEnabled())
+ {
+ lg2::info(
+ "Lamp test is still running. Cannot force stop the lamp test. Asserted is set back to true.");
+
+ // Return false as the request to stop lamptest is not handled as
+ // the lamptest is still running.
+ return false;
+ }
+ return false;
}
}
@@ -304,6 +328,5 @@
}
return;
}
-
} // namespace led
} // namespace phosphor
diff --git a/manager/lamptest/lamptest.hpp b/manager/lamptest/lamptest.hpp
index 30d6eb2..0de9082 100644
--- a/manager/lamptest/lamptest.hpp
+++ b/manager/lamptest/lamptest.hpp
@@ -48,12 +48,18 @@
/** @brief the lamp test request handler
*
+ * If lamp test is running (Asserted=true) and if user requests to stop lamp
+ * test (Asserted input=false), Stop operation will not take place and set
+ * the Asserted to true itself. LampTest Asserted is/can be set to false
+ * only when the lamptest timer expires.
+ *
* @param[in] group - Pointer to Group object
* @param[in] value - true: start lamptest
* false: stop lamptest
- * @return
+ *
+ * @return Whether lamp test request is handled successfully or not.
*/
- void requestHandler(Group* group, bool value);
+ bool requestHandler(Group* group, bool value);
/** @brief Update physical LEDs states during lamp test and the lamp test is
* running
@@ -125,7 +131,7 @@
*/
Layout::Action getActionFromString(const std::string& str);
- /** @brief Notify PHYP to start / stop the lamp test
+ /** @brief Notify host to start / stop the lamp test
*
* @param[in] value - the Asserted property value
*/