Move Level Control OnOff feature check into OnOff cluster. (#23820)
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.
diff --git a/src/app/clusters/level-control/level-control.cpp b/src/app/clusters/level-control/level-control.cpp
index 22db98f..723cc3a 100644
--- a/src/app/clusters/level-control/level-control.cpp
+++ b/src/app/clusters/level-control/level-control.cpp
@@ -1084,20 +1084,6 @@
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."
@@ -1312,7 +1298,7 @@
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;
diff --git a/src/app/clusters/level-control/level-control.h b/src/app/clusters/level-control/level-control.h
index d87d41d..0ae5105 100644
--- a/src/app/clusters/level-control/level-control.h
+++ b/src/app/clusters/level-control/level-control.h
@@ -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);
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 24acfcb..260051d 100644
--- a/src/app/clusters/on-off-server/on-off-server.cpp
+++ b/src/app/clusters/on-off-server/on-off-server.cpp
@@ -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;
@@ -77,6 +81,18 @@
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
@@ -153,7 +169,7 @@
#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);
}
@@ -183,7 +199,7 @@
#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);
}