Skip to content

Commit

Permalink
Move Level Control OnOff feature check into OnOff cluster. (#23820)
Browse files Browse the repository at this point in the history
This simplifies the coupling a bit by doing this runtime feature check in the
same place where we are already doing the runtime check for existence of a
relevant Level Control cluster.  This allows us to remove some code in Level
Control that was duplicating existing code in On/Off.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 16, 2023
1 parent f7c9e8e commit 3ee653d
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 21 deletions.
16 changes: 1 addition & 15 deletions src/app/clusters/level-control/level-control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1084,20 +1084,6 @@ void emberAfOnOffClusterLevelControlEffectCallback(EndpointId endpoint, bool new
return;
}

// if the OnOff feature is not supported, the effect on LevelControl is ignored
if (!HasFeature(endpoint, chip::app::Clusters::LevelControl::LevelControlFeature::kOnOff))
{
emberAfLevelControlClusterPrintln("OnOff feature not supported, ignore LevelControlEffect");
if (!newValue)
{
// OnOff server expects LevelControl to handle the OnOff attribute change,
// when going to off state. The attribute is set directly rather
// than using setOnOff function to avoid misleading comments in log.
OnOff::Attributes::OnOff::Set(endpoint, OnOff::Commands::Off::Id);
}
return;
}

uint8_t minimumLevelAllowedForTheDevice = state->minLevel;

// "Temporarily store CurrentLevel."
Expand Down Expand Up @@ -1312,7 +1298,7 @@ static bool areStartUpLevelControlServerAttributesNonVolatile(EndpointId endpoin

void emberAfPluginLevelControlClusterServerPostInitCallback(EndpointId endpoint) {}

bool HasFeature(chip::EndpointId endpoint, chip::app::Clusters::LevelControl::LevelControlFeature feature)
bool LevelControlHasFeature(EndpointId endpoint, LevelControlFeature feature)
{
bool success;
uint32_t featureMap;
Expand Down
13 changes: 9 additions & 4 deletions src/app/clusters/level-control/level-control.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,21 @@

#include <stdint.h>

#include <app-common/zap-generated/cluster-objects.h>
#include <app-common/zap-generated/cluster-enums.h>
#include <app/util/basic-types.h>

/** @brief On/off Cluster Server Post Init
/** @brief Level Control Cluster Server Post Init
*
* Following resolution of the On/Off state at startup for this endpoint, perform any
* Following resolution of the Level Control state at startup for this endpoint, perform any
* additional initialization needed; e.g., synchronize hardware state.
*
* @param endpoint Endpoint that is being initialized Ver.: always
*/
void emberAfPluginLevelControlClusterServerPostInitCallback(chip::EndpointId endpoint);

bool HasFeature(chip::EndpointId endpoint, chip::app::Clusters::LevelControl::LevelControlFeature feature);
/**
* Check whether the instance of the Level Control cluster on the given endpoint
* has the given feature. The implementation is allowed to assume there is in
* fact an instance of Level Control on the given endpoint.
*/
bool LevelControlHasFeature(chip::EndpointId endpoint, chip::app::Clusters::LevelControl::LevelControlFeature feature);
20 changes: 18 additions & 2 deletions src/app/clusters/on-off-server/on-off-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
#include <app/clusters/scenes/scenes.h>
#endif // EMBER_AF_PLUGIN_SCENES

#ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
#include <app/clusters/level-control/level-control.h>
#endif // EMBER_AF_PLUGIN_LEVEL_CONTROL

using namespace chip;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::OnOff;
Expand Down Expand Up @@ -77,6 +81,18 @@ EmberAfStatus OnOffServer::getOnOffValue(chip::EndpointId endpoint, bool * curre
return status;
}

#ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
static bool LevelControlWithOnOffFeaturePresent(EndpointId endpoint)
{
if (!emberAfContainsServer(endpoint, LevelControl::Id))
{
return false;
}

return LevelControlHasFeature(endpoint, LevelControl::LevelControlFeature::kOnOff);
}
#endif // EMBER_AF_PLUGIN_LEVEL_CONTROL

/** @brief On/off Cluster Set Value
*
* This function is called when the on/off value needs to be set, either through
Expand Down Expand Up @@ -153,7 +169,7 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
#ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
// If initiatedByLevelChange is false, then we assume that the level change
// ZCL stuff has not happened and we do it here
if (!initiatedByLevelChange && emberAfContainsServer(endpoint, LevelControl::Id))
if (!initiatedByLevelChange && LevelControlWithOnOffFeaturePresent(endpoint))
{
emberAfOnOffClusterLevelControlEffectCallback(endpoint, newValue);
}
Expand Down Expand Up @@ -183,7 +199,7 @@ EmberAfStatus OnOffServer::setOnOffValue(chip::EndpointId endpoint, chip::Comman
#ifdef EMBER_AF_PLUGIN_LEVEL_CONTROL
// If initiatedByLevelChange is false, then we assume that the level change
// ZCL stuff has not happened and we do it here
if (!initiatedByLevelChange && emberAfContainsServer(endpoint, LevelControl::Id))
if (!initiatedByLevelChange && LevelControlWithOnOffFeaturePresent(endpoint))
{
emberAfOnOffClusterLevelControlEffectCallback(endpoint, newValue);
}
Expand Down

0 comments on commit 3ee653d

Please sign in to comment.