From 04adc529179d7c3a80e6b3a14fa598950aacb905 Mon Sep 17 00:00:00 2001
From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com>
Date: Sun, 19 Feb 2023 01:23:24 -0500
Subject: [PATCH] [OnOff] Fix OffWaitTime delayed state off (#25172)

* Fix issue where OnTime was set to 0 and the updateOnOffTimeCommand callback was called before OnOff off State was set causing OffWaitTime to wrongly be reseted.

* Fix expected string in Cirque MobileDeviceTest since The log changed in the on off server
---
 .../clusters/on-off-server/on-off-server.cpp  | 23 ++++++++-----------
 .../linux-cirque/MobileDeviceTest.py          |  4 ++--
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp
index 2c0c58f17739da..3954db8cb704de 100644
--- a/src/app/clusters/on-off-server/on-off-server.cpp
+++ b/src/app/clusters/on-off-server/on-off-server.cpp
@@ -110,8 +110,6 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
     EmberAfStatus status;
     bool currentValue, newValue;
 
-    emberAfOnOffClusterPrintln("On/Off set value: %x %x", endpoint, static_cast<uint8_t>(command));
-
     // read current on/off value
     status = Attributes::OnOff::Get(endpoint, &currentValue);
     if (status != EMBER_ZCL_STATUS_SUCCESS)
@@ -123,14 +121,14 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
     // if the value is already what we want to set it to then do nothing
     if ((!currentValue && command == Commands::Off::Id) || (currentValue && command == Commands::On::Id))
     {
-        emberAfOnOffClusterPrintln("On/off already set to new value");
+        emberAfOnOffClusterPrintln("Endpoint %x On/off already set to new value", endpoint);
         return EMBER_ZCL_STATUS_SUCCESS;
     }
 
     // we either got a toggle, or an on when off, or an off when on,
     // so we need to swap the value
     newValue = !currentValue;
-    emberAfOnOffClusterPrintln("Toggle on/off from %x to %x", currentValue, newValue);
+    emberAfOnOffClusterPrintln("Toggle ep%x on/off from state %x to %x", endpoint, currentValue, newValue);
 
     // the sequence of updating on/off attribute and kick off level change effect should
     // be depend on whether we are turning on or off. If we are turning on the light, we
@@ -193,12 +191,6 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
     }
     else // Set Off
     {
-        if (SupportsLightingApplications(endpoint))
-        {
-            emberAfOnOffClusterPrintln("Off Command - OnTime :  0");
-            Attributes::OnTime::Set(endpoint, 0); // Reset onTime
-        }
-
 #ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
         // If initiatedByLevelChange is false, then we assume that the level change
         // ZCL stuff has not happened and we do it here
@@ -207,8 +199,8 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
             emberAfOnOffClusterLevelControlEffectCallback(endpoint, newValue);
         }
         else
-        {
 #endif
+        {
             // write the new on/off value
             status = Attributes::OnOff::Set(endpoint, newValue);
             if (status != EMBER_ZCL_STATUS_SUCCESS)
@@ -216,9 +208,13 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
                 emberAfOnOffClusterPrintln("ERR: writing on/off %x", status);
                 return status;
             }
-#ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
+
+            if (SupportsLightingApplications(endpoint))
+            {
+                emberAfOnOffClusterPrintln("Off completed. reset OnTime to  0");
+                Attributes::OnTime::Set(endpoint, 0); // Reset onTime
+            }
         }
-#endif
     }
 
 #ifdef EMBER_AF_PLUGIN_SCENES
@@ -383,7 +379,6 @@ bool OnOffServer::offWithEffectCommand(app::CommandHandler * commandObj, const a
 #endif // EMBER_AF_PLUGIN_SCENES
 
             OnOff::Attributes::GlobalSceneControl::Set(endpoint, false);
-            Attributes::OnTime::Set(endpoint, 0);
         }
 
         // Only apply effect if OnOff is on
diff --git a/src/test_driver/linux-cirque/MobileDeviceTest.py b/src/test_driver/linux-cirque/MobileDeviceTest.py
index bb7a9a4f69b525..c5d29a1c2feff0 100755
--- a/src/test_driver/linux-cirque/MobileDeviceTest.py
+++ b/src/test_driver/linux-cirque/MobileDeviceTest.py
@@ -121,9 +121,9 @@ def run_controller_test(self):
                 self.get_device_pretty_id(device_id)))
             self.assertTrue(self.sequenceMatch(self.get_device_log(device_id).decode('utf-8'), [
                 "Received command for Endpoint=1 Cluster=0x0000_0006 Command=0x0000_0001",
-                "Toggle on/off from 0 to 1",
+                "Toggle ep1 on/off from state 0 to 1",
                 "Received command for Endpoint=1 Cluster=0x0000_0006 Command=0x0000_0000",
-                "Toggle on/off from 1 to 0",
+                "Toggle ep1 on/off from state 1 to 0",
                 "No command 0x0000_0001 in Cluster 0x0000_0006 on Endpoint 0xe9"]),
                 "Datamodel test failed: cannot find matching string from device {}".format(device_id))