From 609212642c83460ed87af6eb75f96ab98cfd245a Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Thu, 7 Sep 2023 09:17:27 -0400 Subject: [PATCH] [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 <67972863+jmartinez-silabs@users.noreply.github.com> --------- Co-authored-by: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> --- .../clusters/level-control/level-control.cpp | 19 +++++++++++++------ .../clusters/on-off-server/on-off-server.cpp | 5 +++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp index b61d2c34156570..28f7c16573d86f 100644 --- 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::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(static_cast(timeMs / 100)), - chip::Optional>(), chip::Optional>(), INVALID_STORED_LEVEL); + status = moveToLevelHandler(endpoint, command, level, app::DataModel::MakeNullable(static_cast(timeMs / 100)), + chip::Optional>(), chip::Optional>(), + 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 index 16a7e60664d658..a3ebc06d740b7c 100644 --- 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())))