Fix LED action priority inconsistency

When certain LED is first set to "Blink" and is set to
"ON" again, LED needs to stay "Blink" as opposed to
turning "ON". This is the default behavior needed on all
systems unless overridden.

Change-Id: Ied52bb60c538516802b05eb72c139a31499178f5
Signed-off-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com>

3 files changed, 136 insertions, 7 deletions

diff --git a/manager.cpp b/manager.cpp
index 26f00a5..4ebc024 100644
--- a/manager.cpp
+++ b/manager.cpp
@@ -38,7 +38,8 @@ bool Manager::setGroupState(const std::string& path, bool assert,
     group transient {};
     std::set_difference(currentState.begin(), currentState.end(),
                         desiredState.begin(), desiredState.end(),
-                        std::inserter(transient, transient.begin()));
+                        std::inserter(transient, transient.begin()),
+                        ledComp);
     if(transient.size())
     {
         // We really do not want the Manager to know how a particular LED
@@ -68,7 +69,8 @@ bool Manager::setGroupState(const std::string& path, bool assert,
     // Turn on these
     std::set_difference(desiredState.begin(), desiredState.end(),
                         currentState.begin(), currentState.end(),
-                        std::inserter(ledsAssert, ledsAssert.begin()));
+                        std::inserter(ledsAssert, ledsAssert.begin()),
+                        ledComp);
 
 
     // Done.. Save the latest and greatest.
diff --git a/test/led-test-map.hpp b/test/led-test-map.hpp
index 067bb47..a8a62af 100644
--- a/test/led-test-map.hpp
+++ b/test/led-test-map.hpp
@@ -132,3 +132,22 @@ static const std::map<std::string,
         }
     },
 };
+
+static const std::map<std::string,
+    std::set<phosphor::led::Layout::LedAction>>
+        twoGroupsWithMultipleComonLEDInDifferentStateBlinkAgain = {
+    {   "/xyz/openbmc_project/ledmanager/groups/MultipleLedsASet",{
+            {"One",phosphor::led::Layout::On},
+            {"Two",phosphor::led::Layout::Blink},
+            {"Three",phosphor::led::Layout::On},
+            {"Four",phosphor::led::Layout::On},
+        }
+    },
+    {   "/xyz/openbmc_project/ledmanager/groups/MultipleLedsBSet",{
+            {"Two",phosphor::led::Layout::Blink},
+            {"Three",phosphor::led::Layout::Blink},
+            {"Five",phosphor::led::Layout::On},
+            {"Six",phosphor::led::Layout::On},
+        }
+    },
+};
diff --git a/test/utest.cpp b/test/utest.cpp
index 7450f09..19c54a2 100644
--- a/test/utest.cpp
+++ b/test/utest.cpp
@@ -4,7 +4,6 @@
 #include <sdbusplus/bus.hpp>
 #include "manager.hpp"
 #include "led-test-map.hpp"
-
 using namespace phosphor::led;
 class LedTest : public ::testing::Test
 {
@@ -664,9 +663,10 @@ TEST_F(LedTest, assertTwoGroupsWithMultipleComonLEDInDifferentStates)
                                             ledsDeAssert, ledsUpdate);
         EXPECT_EQ(true, result);
 
-        // Need just the ledsAssserted populated with these.
+        // Need just the ledsAssserted populated with these.'Two' gets to Blink
+        // due to higher priority.
         std::set<Layout::LedAction> refAssert = {
-            {"Two",phosphor::led::Layout::On},
+            {"Two",phosphor::led::Layout::Blink},
             {"Three",phosphor::led::Layout::Blink},
             {"Five",phosphor::led::Layout::On},
             {"Six",phosphor::led::Layout::On},
@@ -818,11 +818,12 @@ TEST_F(LedTest, assertTwoGroupsWithMultipleComonLEDInDifferentStateDeAssertOne)
                                             ledsDeAssert, ledsUpdate);
         EXPECT_EQ(true, result);
 
-        // Need just the ledsAssserted populated with these.
+        // Need just the ledsAssserted populated with these. 'Three` remains at
+        // Blink due to increased priority.
         std::set<Layout::LedAction> refAssert = {
             {"One",phosphor::led::Layout::On},
             {"Two",phosphor::led::Layout::Blink},
-            {"Three",phosphor::led::Layout::On},
+            {"Three",phosphor::led::Layout::Blink},
             {"Four",phosphor::led::Layout::On},
         };
         EXPECT_EQ(refAssert.size(), ledsAssert.size());
@@ -877,3 +878,110 @@ TEST_F(LedTest, assertTwoGroupsWithMultipleComonLEDInDifferentStateDeAssertOne)
         EXPECT_EQ(0, temp.size());
     }
 }
+
+/** @brief Assert 2 groups having multiple LEDs common but in
+ *         different state and De-Assert one and make sure the
+ *         state is changed back to Blink
+ */
+TEST_F(LedTest, assertTwoGroupsWithMultipleComonLEDInDifferentStateDeAssertOneGetBlink)
+{
+    Manager manager(bus, twoGroupsWithMultipleComonLEDInDifferentStateBlinkAgain);
+    {
+        // Assert Set-B
+        Manager::group ledsAssert {};
+        Manager::group ledsDeAssert {};
+        Manager::group ledsUpdate {};
+
+        auto group ="/xyz/openbmc_project/ledmanager/groups/MultipleLedsBSet";
+        auto result = manager.setGroupState(group, true, ledsAssert,
+                                            ledsDeAssert, ledsUpdate);
+        EXPECT_EQ(true, result);
+
+        // Need just the ledsAssserted populated with these.
+        std::set<Layout::LedAction> refAssert = {
+            {"Two",phosphor::led::Layout::Blink},
+            {"Three",phosphor::led::Layout::Blink},
+            {"Five",phosphor::led::Layout::On},
+            {"Six",phosphor::led::Layout::On},
+        };
+        EXPECT_EQ(refAssert.size(), ledsAssert.size());
+        EXPECT_EQ(0, ledsDeAssert.size());
+        EXPECT_EQ(0, ledsUpdate.size());
+
+        // difference of refAssert and ledsAssert must be null.
+        Manager::group temp {};
+        std::set_difference(ledsAssert.begin(), ledsAssert.end(),
+                            refAssert.begin(), refAssert.end(),
+                            std::inserter(temp, temp.begin()));
+        EXPECT_EQ(0, temp.size());
+    }
+    {
+        // Assert Set-A
+        Manager::group ledsAssert {};
+        Manager::group ledsDeAssert {};
+        Manager::group ledsUpdate {};
+
+        auto group ="/xyz/openbmc_project/ledmanager/groups/MultipleLedsASet";
+        auto result = manager.setGroupState(group, true, ledsAssert,
+                                            ledsDeAssert, ledsUpdate);
+        EXPECT_EQ(true, result);
+
+        // Need just the ledsAssserted populated with these. 'Three` remains at
+        // Blink due to increased priority. Two is already in asserted state and
+        // it wont be asserted again.
+        std::set<Layout::LedAction> refAssert = {
+            {"One",phosphor::led::Layout::On},
+            {"Three",phosphor::led::Layout::Blink},
+            {"Four",phosphor::led::Layout::On},
+        };
+        EXPECT_EQ(refAssert.size(), ledsAssert.size());
+        EXPECT_EQ(0, ledsDeAssert.size());
+        EXPECT_EQ(0, ledsUpdate.size());
+
+        // difference of refAssert and ledsAssert must be null.
+        Manager::group temp {};
+        std::set_difference(ledsAssert.begin(), ledsAssert.end(),
+                            refAssert.begin(), refAssert.end(),
+                            std::inserter(temp, temp.begin()));
+        EXPECT_EQ(0, temp.size());
+    }
+    {
+        // DeAssert Set-A
+        Manager::group ledsAssert {};
+        Manager::group ledsDeAssert {};
+        Manager::group ledsUpdate {};
+
+        auto group ="/xyz/openbmc_project/ledmanager/groups/MultipleLedsASet";
+        auto result = manager.setGroupState(group, false, ledsAssert,
+                                            ledsDeAssert, ledsUpdate);
+        EXPECT_EQ(false, result);
+
+        // Need just the ledsUpdated populated with these.
+        std::set<Layout::LedAction> refUpdate = {
+            {"Three",phosphor::led::Layout::Blink},
+        };
+        EXPECT_EQ(refUpdate.size(), ledsUpdate.size());
+        EXPECT_EQ(0, ledsAssert.size());
+
+        // difference of refAssert and ledsAssert must be null.
+        Manager::group temp {};
+        std::set_difference(ledsUpdate.begin(), ledsUpdate.end(),
+                            refUpdate.begin(), refUpdate.end(),
+                            std::inserter(temp, temp.begin()));
+        EXPECT_EQ(0, temp.size());
+
+        // Need just the ledsDeAsserted populated with these.
+        std::set<Layout::LedAction> refDeAssert = {
+            {"One",phosphor::led::Layout::On},
+            {"Four",phosphor::led::Layout::On},
+        };
+        EXPECT_EQ(refDeAssert.size(), ledsDeAssert.size());
+
+        // difference of refAssert and ledsAssert must be null.
+        temp.clear();
+        std::set_difference(ledsDeAssert.begin(), ledsDeAssert.end(),
+                            refDeAssert.begin(), refDeAssert.end(),
+                            std::inserter(temp, temp.begin()));
+        EXPECT_EQ(0, temp.size());
+    }
+}