On mode overwrite (project-chip#16791)

* Let OnMode overwrites StartUpMode.

* Generated Changes.

* Restyled by clang-format

* Restyled by prettier-yaml

* Remove comments in Tests.

Co-authored-by: Restyled.io <[email protected]>

examples/all-clusters-app/all-clusters-common/all-clusters-app.zap

@@ -8023,7 +8023,7 @@
               "mfgCode": null,
               "side": "server",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "NVM",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "0xFF",
@@ -10561,7 +10561,7 @@
               "mfgCode": null,
               "side": "server",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "NVM",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "255",
@@ -21557,4 +21557,4 @@
     }
   ],
   "log": []
-}
+}

src/app/clusters/mode-select-server/mode-select-server.cpp

@@ -26,9 +26,11 @@
 #include <app/CommandHandler.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/clusters/mode-select-server/supported-modes-manager.h>
+#include <app/clusters/on-off-server/on-off-server.h>
 #include <app/util/af.h>
 #include <app/util/attribute-storage.h>
 #include <app/util/error-mapping.h>
+#include <app/util/odd-sized-integers.h>
 #include <app/util/util.h>
 #include <lib/support/CodeUtils.h>
 
@@ -134,6 +136,29 @@ void emberAfModeSelectClusterServerInitCallback(EndpointId endpointId)
             // Initialise currentMode to 0
             uint8_t currentMode = 0;
             status              = Attributes::CurrentMode::Get(endpointId, &currentMode);
+#ifdef EMBER_AF_PLUGIN_ON_OFF
+            // OnMode with Power Up
+            // If the On/Off feature is supported and the On/Off cluster attribute StartUpOnOff is present, with a
+            // value of On (turn on at power up), then the CurrentMode attribute SHALL be set to the OnMode attribute
+            // value when the server is supplied with power, except if the OnMode attribute is null.
+            if (emberAfContainsServer(endpointId, OnOff::Id) &&
+                emberAfContainsAttribute(endpointId, OnOff::Id, OnOff::Attributes::StartUpOnOff::Id, true) &&
+                emberAfContainsAttribute(endpointId, ModeSelect::Id, ModeSelect::Attributes::OnMode::Id, true))
+            {
+                Attributes::OnMode::TypeInfo::Type onMode;
+                bool onOffValueForStartUp = 0;
+                if (Attributes::OnMode::Get(endpointId, onMode) == EMBER_ZCL_STATUS_SUCCESS &&
+                    emberAfIsNonVolatileAttribute(endpointId, OnOff::Id, OnOff::Attributes::StartUpOnOff::Id, true) &&
+                    OnOffServer::Instance().getOnOffValueForStartUp(endpointId, onOffValueForStartUp) == EMBER_ZCL_STATUS_SUCCESS)
+                {
+                    if (onOffValueForStartUp && !onMode.IsNull())
+                    {
+                        emberAfPrintln(EMBER_AF_PRINT_DEBUG, "ModeSelect: CurrentMode is overwritten by OnMode");
+                        return;
+                    }
+                }
+            }
+#endif // EMBER_AF_PLUGIN_ON_OFF
             if (status == EMBER_ZCL_STATUS_SUCCESS && startUpMode.Value() != currentMode)
             {
                 status = Attributes::CurrentMode::Set(endpointId, startUpMode.Value());
@@ -196,6 +221,9 @@ InteractionModel::Status MatterModeSelectClusterServerPreAttributeChangedCallbac
     case ModeSelect::Attributes::StartUpMode::Id:
         result = verifyModeValue(endpointId, *value);
         break;
+    case ModeSelect::Attributes::OnMode::Id:
+        result = verifyModeValue(endpointId, *value);
+        break;
     default:
         result = InteractionModel::Status::Success;
     }
@@ -211,6 +239,10 @@ InteractionModel::Status MatterModeSelectClusterServerPreAttributeChangedCallbac
  */
 static InteractionModel::Status verifyModeValue(const EndpointId endpointId, const uint8_t newMode)
 {
+    if (NumericAttributeTraits<uint8_t>::IsNullValue(newMode)) // This indicates that the new mode is null.
+    {
+        return InteractionModel::Status::Success;
+    }
     const ModeSelect::Structs::ModeOptionStruct::Type * modeOptionPtr;
     EmberAfStatus checkSupportedModeStatus =
         ModeSelect::getSupportedModesManager()->getModeOptionByMode(endpointId, newMode, &modeOptionPtr);

src/app/clusters/on-off-server/on-off-server.cpp

@@ -169,17 +169,15 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, uint8_t comm
 #endif
 #ifdef EMBER_AF_PLUGIN_MODE_SELECT
         // If OnMode is not a null value, then change the current mode to it.
-        ModeSelect::Attributes::OnMode::TypeInfo::Type onMode;
-        status = ModeSelect::Attributes::OnMode::Get(endpoint, onMode);
-        if (status != EMBER_ZCL_STATUS_SUCCESS)
-        {
-            emberAfOnOffClusterPrintln("ERR: reading onMode %x", status);
-            return status;
-        }
-        if (!onMode.IsNull())
+        if (emberAfContainsServer(endpoint, ModeSelect::Id) &&
+            emberAfContainsAttribute(endpoint, ModeSelect::Id, ModeSelect::Attributes::OnMode::Id, true))
         {
-            emberAfOnOffClusterPrintln("Changing Current Mode to %x", onMode.Value());
-            status = ModeSelect::Attributes::CurrentMode::Set(endpoint, onMode.Value());
+            ModeSelect::Attributes::OnMode::TypeInfo::Type onMode;
+            if (ModeSelect::Attributes::OnMode::Get(endpoint, onMode) == EMBER_ZCL_STATUS_SUCCESS && !onMode.IsNull())
+            {
+                emberAfOnOffClusterPrintln("Changing Current Mode to %x", onMode.Value());
+                status = ModeSelect::Attributes::CurrentMode::Set(endpoint, onMode.Value());
+            }
         }
 #endif
     }
@@ -248,41 +246,73 @@ void OnOffServer::initOnOffServer(chip::EndpointId endpoint)
         // 0xff      This value cannot happen.
         // null       Set the OnOff attribute to its previous value.
 
-        app::DataModel::Nullable<OnOff::OnOffStartUpOnOff> startUpOnOff;
-        EmberAfStatus status = Attributes::StartUpOnOff::Get(endpoint, startUpOnOff);
+        bool onOffValueForStartUp = 0;
+        EmberAfStatus status      = getOnOffValueForStartUp(endpoint, onOffValueForStartUp);
         if (status == EMBER_ZCL_STATUS_SUCCESS)
         {
-            // Initialise updated value to 0
-            bool updatedOnOff = 0;
-            status            = Attributes::OnOff::Get(endpoint, &updatedOnOff);
-            if (status == EMBER_ZCL_STATUS_SUCCESS)
+            status = setOnOffValue(endpoint, onOffValueForStartUp, false);
+        }
+
+#ifdef EMBER_AF_PLUGIN_MODE_SELECT
+        // If OnMode is not a null value, then change the current mode to it.
+        if (onOffValueForStartUp && emberAfContainsServer(endpoint, ModeSelect::Id) &&
+            emberAfContainsAttribute(endpoint, ModeSelect::Id, ModeSelect::Attributes::OnMode::Id, true))
+        {
+            ModeSelect::Attributes::OnMode::TypeInfo::Type onMode;
+            if (ModeSelect::Attributes::OnMode::Get(endpoint, onMode) == EMBER_ZCL_STATUS_SUCCESS && !onMode.IsNull())
             {
-                if (!startUpOnOff.IsNull())
-                {
-                    switch (startUpOnOff.Value())
-                    {
-                    case OnOff::OnOffStartUpOnOff::kOff:
-                        updatedOnOff = 0; // Off
-                        break;
-                    case OnOff::OnOffStartUpOnOff::kOn:
-                        updatedOnOff = 1; // On
-                        break;
-                    case OnOff::OnOffStartUpOnOff::kTogglePreviousOnOff:
-                        updatedOnOff = !updatedOnOff;
-                        break;
-                    default:
-                        // All other values 0x03- 0xFE are reserved - no action.
-                        break;
-                    }
-                }
-                status = setOnOffValue(endpoint, updatedOnOff, false);
+                emberAfOnOffClusterPrintln("Changing Current Mode to %x", onMode.Value());
+                status = ModeSelect::Attributes::CurrentMode::Set(endpoint, onMode.Value());
             }
         }
+#endif
     }
 #endif // IGNORE_ON_OFF_CLUSTER_START_UP_ON_OFF
     emberAfPluginOnOffClusterServerPostInitCallback(endpoint);
 }
 
+/** @brief Get the OnOff value when server starts.
+ *
+ * This function determines how StartUpOnOff affects the OnOff value when the server starts.
+ *
+ * @param endpoint   Ver.: always
+ * @param onOffValueForStartUp Ver.: always
+ */
+EmberAfStatus OnOffServer::getOnOffValueForStartUp(chip::EndpointId endpoint, bool & onOffValueForStartUp)
+{
+    app::DataModel::Nullable<OnOff::OnOffStartUpOnOff> startUpOnOff;
+    EmberAfStatus status = Attributes::StartUpOnOff::Get(endpoint, startUpOnOff);
+    if (status == EMBER_ZCL_STATUS_SUCCESS)
+    {
+        // Initialise updated value to 0
+        bool updatedOnOff = 0;
+        status            = Attributes::OnOff::Get(endpoint, &updatedOnOff);
+        if (status == EMBER_ZCL_STATUS_SUCCESS)
+        {
+            if (!startUpOnOff.IsNull())
+            {
+                switch (startUpOnOff.Value())
+                {
+                case OnOff::OnOffStartUpOnOff::kOff:
+                    updatedOnOff = 0; // Off
+                    break;
+                case OnOff::OnOffStartUpOnOff::kOn:
+                    updatedOnOff = 1; // On
+                    break;
+                case OnOff::OnOffStartUpOnOff::kTogglePreviousOnOff:
+                    updatedOnOff = !updatedOnOff;
+                    break;
+                default:
+                    // All other values 0x03- 0xFE are reserved - no action.
+                    break;
+                }
+            }
+            onOffValueForStartUp = updatedOnOff;
+        }
+    }
+    return status;
+}
+
 bool OnOffServer::offCommand(const app::ConcreteCommandPath & commandPath)
 {
     EmberAfStatus status = setOnOffValue(commandPath.mEndpointId, Commands::Off::Id, false);

src/app/clusters/on-off-server/on-off-server.h

@@ -59,6 +59,7 @@ class OnOffServer
                                const chip::app::Clusters::OnOff::Commands::OnWithTimedOff::DecodableType & commandData);
     void updateOnOffTimeCommand(chip::EndpointId endpoint);
     EmberAfStatus setOnOffValue(chip::EndpointId endpoint, uint8_t command, bool initiatedByLevelChange);
+    EmberAfStatus getOnOffValueForStartUp(chip::EndpointId endpoint, bool & onOffValueForStartUp);
 
     bool HasFeature(chip::EndpointId endpoint, OnOffFeature feature);
     inline bool SupportsLightingApplications(chip::EndpointId endpointId)

src/app/tests/suites/TestModeSelectCluster.yaml

@@ -112,6 +112,14 @@ tests:
       response:
           value: currentModeBeforeToggle
 
+    - label: "Change to Unsupported OnMode"
+      command: "writeAttribute"
+      attribute: "OnMode"
+      arguments:
+          value: 2
+      response:
+          error: CONSTRAINT_ERROR
+
     - label: "Change OnMode"
       command: "writeAttribute"
       attribute: "OnMode"
@@ -166,6 +174,49 @@ tests:
               - name: "NewMode"
                 value: 0
 
+    - label: "Change On Mode"
+      command: "writeAttribute"
+      attribute: "OnMode"
+      arguments:
+          value: 4
+
+    - label: "Set StartUpOnOff"
+      cluster: "On/Off"
+      command: "writeAttribute"
+      attribute: "StartUpOnOff"
+      arguments:
+          value: 1
+
+    - label: "Reboot target device"
+      cluster: "SystemCommands"
+      command: "Reboot"
+      arguments:
+          values:
+              - name: "discriminator"
+                value: discriminator
+
+    - label: "Wait for the commissioned device to be retrieved"
+      cluster: "DelayCommands"
+      command: "WaitForCommissionee"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: nodeId
+
+    - label:
+          "Verify Current Mode Change based on OnMode, as it overwrites
+          StartUpMode"
+      command: "readAttribute"
+      attribute: "CurrentMode"
+      response:
+          value: 4
+
+    - label: "Change On Mode to Null"
+      command: "writeAttribute"
+      attribute: "OnMode"
+      arguments:
+          value: null
+
     - label: "Reboot target device"
       cluster: "SystemCommands"
       command: "Reboot"