Corrected wrong define usage and applied a fix to attribute-storage f…

…unction
project-chip · Oct 11, 2024 · 8e4c7f1 · 8e4c7f1
1 parent dc1162f
commit 8e4c7f1
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 42 deletions.
diff --git a/src/app/clusters/mode-select-server/mode-select-server.cpp b/src/app/clusters/mode-select-server/mode-select-server.cpp
@@ -33,6 +33,10 @@
 #include <platform/DiagnosticDataProvider.h>
 #include <tracing/macros.h>
 
+#ifdef MATTER_DM_PLUGIN_SCENES_MANAGEMENT
+#include <app/clusters/scenes-server/scenes-server.h>
+#endif // MATTER_DM_PLUGIN_SCENES_MANAGEMENT
+
 #ifdef MATTER_DM_PLUGIN_ON_OFF
 #include <app/clusters/on-off-server/on-off-server.h>
 #endif // MATTER_DM_PLUGIN_ON_OFF
@@ -113,23 +117,21 @@ Status ChangeToMode(EndpointId endpointId, uint8_t newMode)
 {
     MATTER_TRACE_SCOPE("ChangeToMode", "ModeSelect");
     ChipLogProgress(Zcl, "ModeSelect: Entering emberAfModeSelectClusterChangeToModeCallback");
-    EndpointId endpointId = commandPath.mEndpointId;
-    uint8_t newMode       = commandData.newMode;
+
     // Check that the newMode matches one of the supported options
     const ModeSelect::Structs::ModeOptionStruct::Type * modeOptionPtr;
     const ModeSelect::SupportedModesManager * gSupportedModeManager = ModeSelect::getSupportedModesManager();
     if (gSupportedModeManager == nullptr)
     {
         ChipLogError(Zcl, "ModeSelect: SupportedModesManager is NULL");
-        commandHandler->AddStatus(commandPath, Status::Failure);
-        return true;
+        return Status::Failure;
     }
     Status checkSupportedModeStatus = gSupportedModeManager->getModeOptionByMode(endpointId, newMode, &modeOptionPtr);
     if (Status::Success != checkSupportedModeStatus)
     {
+        ChipLogProgress(Zcl, "ModeSelect: Failed to find the option with mode %u", newMode);
         return checkSupportedModeStatus;
     }
-
     ModeSelect::Attributes::CurrentMode::Set(endpointId, newMode);
 
     return Status::Success;
@@ -146,7 +148,7 @@ static void sceneModeSelectCallback(EndpointId endpoint);
 using ModeSelectEndPointPair = scenes::DefaultSceneHandlerImpl::EndpointStatePair<uint8_t>;
 using ModeSelectTransitionTimeInterface =
     scenes::DefaultSceneHandlerImpl::TransitionTimeInterface<kModeSelectMaxEnpointCount,
-                                                             EMBER_AF_ON_OFF_CLUSTER_SERVER_ENDPOINT_COUNT>;
+                                                             EMBER_AF_MODE_SELECT_CLUSTER_SERVER_ENDPOINT_COUNT>;
 
 class DefaultModeSelectSceneHandler : public scenes::DefaultSceneHandlerImpl
 {
@@ -181,13 +183,13 @@ class DefaultModeSelectSceneHandler : public scenes::DefaultSceneHandlerImpl
     }
 
     /// @brief Serialize the Cluster's EFS value
-    /// @param endpoint target endpoint
-    /// @param cluster  target cluster
-    /// @param serializedBytes data to serialize into EFS
+    /// @param [in] endpoint target endpoint
+    /// @param [in] cluster  target cluster
+    /// @param [out] serializedBytes data to serialize into EFS
     /// @return CHIP_NO_ERROR if successfully serialized the data, CHIP_ERROR_INVALID_ARGUMENT otherwise
     CHIP_ERROR SerializeSave(EndpointId endpoint, ClusterId cluster, MutableByteSpan & serializedBytes) override
     {
-        using AttributeValuePair = Scenes::Structs::AttributeValuePair::Type;
+        using AttributeValuePair = ScenesManagement::Structs::AttributeValuePairStruct::Type;
 
         uint8_t currentMode;
         // read CurrentMode value
@@ -217,7 +219,7 @@ class DefaultModeSelectSceneHandler : public scenes::DefaultSceneHandlerImpl
     CHIP_ERROR ApplyScene(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes,
                           scenes::TransitionTimeMs timeMs) override
     {
-        app::DataModel::DecodableList<Scenes::Structs::AttributeValuePair::DecodableType> attributeValueList;
+        app::DataModel::DecodableList<ScenesManagement::Structs::AttributeValuePairStruct::DecodableType> attributeValueList;
 
         VerifyOrReturnError(cluster == ModeSelect::Id, CHIP_ERROR_INVALID_ARGUMENT);
 
@@ -232,8 +234,9 @@ class DefaultModeSelectSceneHandler : public scenes::DefaultSceneHandlerImpl
         {
             auto & decodePair = pair_iterator.GetValue();
             VerifyOrReturnError(decodePair.attributeID == Attributes::CurrentMode::Id, CHIP_ERROR_INVALID_ARGUMENT);
+            VerifyOrReturnError(decodePair.valueUnsigned8.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
             ReturnErrorOnFailure(mSceneEndpointStatePairs.InsertPair(
-                ModeSelectEndPointPair(endpoint, static_cast<uint8_t>(decodePair.attributeValue))));
+                ModeSelectEndPointPair(endpoint, static_cast<uint8_t>(decodePair.valueUnsigned8.HasValue()))));
         }
         // Verify that the EFS was completely read
         CHIP_ERROR err = pair_iterator.GetStatus();
@@ -243,6 +246,7 @@ class DefaultModeSelectSceneHandler : public scenes::DefaultSceneHandlerImpl
             return err;
         }
 
+        VerifyOrReturnError(mTransitionTimeInterface.sceneEventControl(endpoint) != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
         DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(timeMs), timerCallback,
                                               mTransitionTimeInterface.sceneEventControl(endpoint));
 
@@ -251,7 +255,7 @@ class DefaultModeSelectSceneHandler : public scenes::DefaultSceneHandlerImpl
 
 private:
     ModeSelectTransitionTimeInterface mTransitionTimeInterface =
-        ModeSelectTransitionTimeInterface(Attributes::CurrentMode::Id, sceneModeSelectCallback);
+        ModeSelectTransitionTimeInterface(ModeSelect::Id, sceneModeSelectCallback);
 };
 static DefaultModeSelectSceneHandler sModeSelectSceneHandler;
 
@@ -261,6 +265,13 @@ static void timerCallback(System::Layer *, void * callbackContext)
     (control->callback)(control->endpoint);
 }
 
+/**
+ * @brief This function is a callback to apply the mode that was saved when the ApplyScene was called with a transition time greater
+ * than 0.
+ *
+ * @param endpoint The endpoint ID that the scene mode selection is associated with.
+ *
+ */
 static void sceneModeSelectCallback(EndpointId endpoint)
 {
     ModeSelectEndPointPair savedState;
@@ -291,13 +302,12 @@ bool emberAfModeSelectClusterChangeToModeCallback(CommandHandler * commandHandle
 
     if (Status::Success != status)
     {
-        ChipLogProgress(Zcl, "ModeSelect: Failed to find the option with mode %u", commandData.newMode);
         commandHandler->AddStatus(commandPath, status);
         return true;
     }
 
     ChipLogProgress(Zcl, "ModeSelect: ChangeToMode successful");
-    commandHandler->AddStatus(commandPath, Status::Success);
+    commandHandler->AddStatus(commandPath, status);
     return true;
 }
 
@@ -321,23 +331,18 @@ void emberAfModeSelectClusterServerInitCallback(EndpointId endpointId)
         Status status = Attributes::StartUpMode::Get(endpointId, startUpMode);
         if (status == Status::Success && !startUpMode.IsNull())
         {
-<<<<<<< HEAD
-#ifdef MATTER_DM_PLUGIN_ON_OFF
-            == == == =
-
 #if defined(EMBER_AF_PLUGIN_SCENES) && CHIP_CONFIG_SCENES_USE_DEFAULT_HANDLERS
-                         app::Clusters::Scenes::ScenesServer::Instance().RegisterSceneHandler(endpointId, &sModeSelectSceneHandler);
+            ScenesManagement::ScenesServer::Instance().RegisterSceneHandler(endpointId, &sModeSelectSceneHandler);
 #endif // defined(EMBER_AF_PLUGIN_SCENES) && CHIP_CONFIG_SCENES_USE_DEFAULT_HANDLERS
 
-#ifdef EMBER_AF_PLUGIN_ON_OFF
-            >>>>>>> 908c492c15(Added scene handler to mode select cluster)
-                // 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) &&
-                    emberAfContainsAttribute(endpointId, ModeSelect::Id, ModeSelect::Attributes::OnMode::Id))
+#ifdef MATTER_DM_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) &&
+                emberAfContainsAttribute(endpointId, ModeSelect::Id, ModeSelect::Attributes::OnMode::Id))
             {
                 Attributes::OnMode::TypeInfo::Type onMode;
                 bool onOffValueForStartUp = false;

diff --git a/src/app/tests/suites/TestScenesFabricSceneInfo.yaml b/src/app/tests/suites/TestScenesFabricSceneInfo.yaml
@@ -253,6 +253,11 @@ tests:
                                     },
                                 ],
                         },
+                        {
+                            ClusterID: 0x0050,
+                            AttributeValueList:
+                                [{ AttributeID: 0x0003, ValueUnsigned8: 0x01}],
+                        },
                     ]
       response:
           values:
@@ -349,6 +354,11 @@ tests:
                                     },
                                 ],
                         },
+                        {
+                            ClusterID: 0x0050,
+                            AttributeValueList:
+                                [{ AttributeID: 0x0003, ValueUnsigned8: 0x01}],
+                        },
                     ]
 
     - label: "Read the FabricSceneInfo attribute (0x0007) "

diff --git a/src/app/tests/suites/certification/Test_TC_S_2_2.yaml b/src/app/tests/suites/certification/Test_TC_S_2_2.yaml
@@ -618,16 +618,6 @@ tests:
                                         },
                                     ],
                             },
-                            {
-                                ClusterID: 0x0050,
-                                AttributeValueList:
-                                    [
-                                        {
-                                            AttributeID: 0x0003,
-                                            AttributeValue: 0x00,
-                                        },
-                                    ],
-                            },
                         ]
 
     - label:

diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h
@@ -1485,9 +1485,9 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
 #endif
 
 /**
- * @brief The maximum number of clusters per scene, we recommend using 3 for a typical use case (onOff + level control + color
- * control cluster). Needs to be changed in case a greater number of clusters is chosen. Now set to 4 to allow for the addition of
- * the mode select cluster.
+ * @brief The maximum number of clusters per scene, we recommend using 4 for a typical use case (onOff + level control + color
+ * control cluster + mode selec cluster). Needs to be changed in case a greater number of clusters is chosen. In the event the
+ * device does not need to support the mode select cluster, the maximum number of clusters per scene should be set to 3.
  */
 #ifndef CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE
 #define CHIP_CONFIG_SCENES_MAX_CLUSTERS_PER_SCENE 4