Skip to content

Commit

Permalink
[Scenes] Level control handler bugfix (project-chip#29076)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
2 people authored and HunsupJung committed Oct 23, 2023
1 parent 5de3f0c commit c6c44ef
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
19 changes: 13 additions & 6 deletions src/app/clusters/level-control/level-control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
5 changes: 5 additions & 0 deletions src/app/clusters/on-off-server/on-off-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())))
Expand Down

0 comments on commit c6c44ef

Please sign in to comment.