[Scenes] Level control handler bugfix (#29076)

* Used NumericAttributesTraits for storage null values, removed un-necessary typing and added doc on on-off and level control handlers interactions * Update src/app/clusters/on-off-server/on-off-server.cpp Co-authored-by: Junior Martinez <[email protected]> --------- Co-authored-by: Junior Martinez <[email protected]>
project-chip · Nov 22, 2023 · 6092126 · 6092126
1 parent 36d766e
commit 6092126
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 6 deletions.
diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp
@@ -171,9 +171,16 @@ class DefaultLevelControlSceneHandler : public scenes::DefaultSceneHandlerImpl
         uint8_t maxLevel;
         VerifyOrReturnError(EMBER_ZCL_STATUS_SUCCESS == Attributes::MaxLevel::Get(endpoint, &maxLevel), CHIP_ERROR_READ_FAILED);
 
-        pairs[0].attributeID    = Attributes::CurrentLevel::Id;
-        pairs[0].attributeValue = (!level.IsNull()) ? level.Value() : maxLevel + 1;
-        size_t attributeCount   = 1;
+        pairs[0].attributeID = Attributes::CurrentLevel::Id;
+        if (!level.IsNull())
+        {
+            pairs[0].attributeValue = level.Value();
+        }
+        else
+        {
+            chip::app::NumericAttributeTraits<uint32_t>::SetNull(pairs[0].attributeValue);
+        }
+        size_t attributeCount = 1;
         if (LevelControlHasFeature(endpoint, LevelControl::Feature::kFrequency))
         {
             uint16_t frequency;
@@ -238,9 +245,9 @@ class DefaultLevelControlSceneHandler : public scenes::DefaultSceneHandlerImpl
         CommandId command = LevelControlHasFeature(endpoint, LevelControl::Feature::kOnOff) ? Commands::MoveToLevelWithOnOff::Id
                                                                                             : Commands::MoveToLevel::Id;
 
-        status = moveToLevelHandler(
-            endpoint, command, level, app::DataModel::MakeNullable<uint16_t>(static_cast<uint16_t>(timeMs / 100)),
-            chip::Optional<BitMask<LevelControlOptions>>(), chip::Optional<BitMask<LevelControlOptions>>(), INVALID_STORED_LEVEL);
+        status = moveToLevelHandler(endpoint, command, level, app::DataModel::MakeNullable(static_cast<uint16_t>(timeMs / 100)),
+                                    chip::Optional<BitMask<LevelControlOptions>>(), chip::Optional<BitMask<LevelControlOptions>>(),
+                                    INVALID_STORED_LEVEL);
 
         if (status != Status::Success)
         {

diff --git a/src/app/clusters/on-off-server/on-off-server.cpp b/src/app/clusters/on-off-server/on-off-server.cpp
@@ -289,6 +289,11 @@ class DefaultOnOffSceneHandler : public scenes::DefaultSceneHandlerImpl
             return err;
         }
 
+        // This handler assumes it is being used with the default handler for the level control. Therefore if the level control
+        // cluster with on off feature is present on the endpoint and the level control handler is registered, it assumes this
+        // handler will take action on the on-off state. This assumes the level control attributes were also saved in the scene.
+        // This is to prevent a behavior where the on off state is set by this handler, and then the level control handler or vice
+        // versa.
 #ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
         if (!(LevelControlWithOnOffFeaturePresent(endpoint) &&
               Scenes::ScenesServer::Instance().IsHandlerRegistered(endpoint, LevelControlServer::GetSceneHandler())))