matter: samples: Switch to code-driven Identify cluster on MB

zapgen-driven Identify cluster implementation does not work
anymore for dynamic-generated endpoints. Instead we must
use code-driven implementation for this cluster.
For now switch to the new implementation to build the applicationm
but the MatterBridgedDevice constructor must be refactored later.

Signed-off-by: Arkadiusz Balys <[email protected]>

Author: ArekBalysNordic

applications/matter_bridge/src/bridged_device_types/generic_switch.cpp

@@ -8,8 +8,8 @@
 
 #include <zephyr/logging/log.h>
 
-#include <app-common/zap-generated/ids/Commands.h>
 #include <app-common/zap-generated/attributes/Accessors.h>
+#include <app-common/zap-generated/ids/Commands.h>
 #include <app/clusters/switch-server/switch-server.h>
 
 LOG_MODULE_DECLARE(app, CONFIG_CHIP_APP_LOG_LEVEL);
@@ -18,7 +18,6 @@ namespace
 {
 DESCRIPTOR_CLUSTER_ATTRIBUTES(descriptorAttrs);
 BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_ATTRIBUTES(bridgedDeviceBasicAttrs);
-IDENTIFY_CLUSTER_ATTRIBUTES(identifyAttrs);
 }; /* namespace */
 
 using namespace ::chip;
@@ -36,8 +35,7 @@ DECLARE_DYNAMIC_CLUSTER(Clusters::Switch::Id, switchAttr, ZAP_CLUSTER_MASK(SERVE
 	DECLARE_DYNAMIC_CLUSTER(Clusters::Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs,
 				ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
-	DECLARE_DYNAMIC_CLUSTER(Clusters::Identify::Id, identifyAttrs, ZAP_CLUSTER_MASK(SERVER),
-				sIdentifyIncomingCommands, nullptr) DECLARE_DYNAMIC_CLUSTER_LIST_END;
+	DECLARE_DYNAMIC_CLUSTER_LIST_END;
 
 DECLARE_DYNAMIC_ENDPOINT(bridgedGenericSwitchEndpoint, genericSwitchClusters);
 

applications/matter_bridge/src/bridged_device_types/humidity_sensor.cpp

@@ -10,7 +10,6 @@ namespace
 {
 DESCRIPTOR_CLUSTER_ATTRIBUTES(descriptorAttrs);
 BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_ATTRIBUTES(bridgedDeviceBasicAttrs);
-IDENTIFY_CLUSTER_ATTRIBUTES(identifyAttrs);
 }; /* namespace */
 
 using namespace ::chip;
@@ -32,8 +31,7 @@ DECLARE_DYNAMIC_CLUSTER(Clusters::RelativeHumidityMeasurement::Id, humiSensorAtt
 	DECLARE_DYNAMIC_CLUSTER(Clusters::Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs,
 				ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
-	DECLARE_DYNAMIC_CLUSTER(Clusters::Identify::Id, identifyAttrs, ZAP_CLUSTER_MASK(SERVER),
-				sIdentifyIncomingCommands, nullptr) DECLARE_DYNAMIC_CLUSTER_LIST_END;
+	DECLARE_DYNAMIC_CLUSTER_LIST_END;
 
 DECLARE_DYNAMIC_ENDPOINT(bridgedHumidityEndpoint, bridgedHumidityClusters);
 

applications/matter_bridge/src/bridged_device_types/onoff_light.cpp

@@ -16,19 +16,12 @@ namespace
 {
 DESCRIPTOR_CLUSTER_ATTRIBUTES(descriptorAttrs);
 BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_ATTRIBUTES(bridgedDeviceBasicAttrs);
-IDENTIFY_CLUSTER_ATTRIBUTES(identifyAttrs);
 }; /* namespace */
 
 using namespace ::chip;
 using namespace ::chip::app;
 using namespace Nrf;
 
-constexpr CommandId identifyIncomingCommands[] = {
-	app::Clusters::Identify::Commands::Identify::Id,
-	app::Clusters::Identify::Commands::TriggerEffect::Id,
-	kInvalidCommandId,
-};
-
 DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(onOffAttrs)
 DECLARE_DYNAMIC_ATTRIBUTE(Clusters::OnOff::Attributes::OnOff::Id, BOOLEAN, 1, 0),
 	DECLARE_DYNAMIC_ATTRIBUTE(Clusters::OnOff::Attributes::FeatureMap::Id, BITMAP32, 4, 0),
@@ -80,8 +73,7 @@ DECLARE_DYNAMIC_CLUSTER(Clusters::OnOff::Id, onOffAttrs, ZAP_CLUSTER_MASK(SERVER
 				groupsGeneratedCommands),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs,
 				ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
-	DECLARE_DYNAMIC_CLUSTER(Clusters::Identify::Id, identifyAttrs, ZAP_CLUSTER_MASK(SERVER),
-				identifyIncomingCommands, nullptr) DECLARE_DYNAMIC_CLUSTER_LIST_END;
+	DECLARE_DYNAMIC_CLUSTER_LIST_END;
 
 DECLARE_DYNAMIC_ENDPOINT(bridgedLightEndpoint, bridgedLightClusters);
 

applications/matter_bridge/src/bridged_device_types/onoff_light_switch.cpp

@@ -16,7 +16,6 @@ namespace
 {
 DESCRIPTOR_CLUSTER_ATTRIBUTES(descriptorAttrs);
 BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_ATTRIBUTES(bridgedDeviceBasicAttrs);
-IDENTIFY_CLUSTER_ATTRIBUTES(identifyAttrs);
 }; /* namespace */
 
 using namespace ::chip;
@@ -46,10 +45,6 @@ DECLARE_DYNAMIC_CLUSTER(Clusters::OnOff::Id, onOffClientAttrs, ZAP_CLUSTER_MASK(
 	DECLARE_DYNAMIC_CLUSTER(Clusters::Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs,
 				ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
-	DECLARE_DYNAMIC_CLUSTER(Clusters::Identify::Id, identifyAttrs, ZAP_CLUSTER_MASK(SERVER),
-				sIdentifyIncomingCommands, nullptr),
-	DECLARE_DYNAMIC_CLUSTER(Clusters::Identify::Id, identifyAttrs, ZAP_CLUSTER_MASK(CLIENT),
-				sIdentifyIncomingCommands, nullptr),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::Binding::Id, bindingAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr,
 				nullptr) DECLARE_DYNAMIC_CLUSTER_LIST_END;
 

applications/matter_bridge/src/bridged_device_types/temperature_sensor.cpp

@@ -10,7 +10,6 @@ namespace
 {
 DESCRIPTOR_CLUSTER_ATTRIBUTES(descriptorAttrs);
 BRIDGED_DEVICE_BASIC_INFORMATION_CLUSTER_ATTRIBUTES(bridgedDeviceBasicAttrs);
-IDENTIFY_CLUSTER_ATTRIBUTES(identifyAttrs);
 }; /* namespace */
 
 using namespace ::chip;
@@ -28,8 +27,7 @@ DECLARE_DYNAMIC_CLUSTER_LIST_BEGIN(bridgedTemperatureClusters)
 DECLARE_DYNAMIC_CLUSTER(Clusters::TemperatureMeasurement::Id, tempSensorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::Descriptor::Id, descriptorAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
 	DECLARE_DYNAMIC_CLUSTER(Clusters::BridgedDeviceBasicInformation::Id, bridgedDeviceBasicAttrs, ZAP_CLUSTER_MASK(SERVER), nullptr, nullptr),
-	DECLARE_DYNAMIC_CLUSTER(Clusters::Identify::Id, identifyAttrs, ZAP_CLUSTER_MASK(SERVER), sIdentifyIncomingCommands,
-				nullptr) DECLARE_DYNAMIC_CLUSTER_LIST_END;
+DECLARE_DYNAMIC_CLUSTER_LIST_END;
 
 DECLARE_DYNAMIC_ENDPOINT(bridgedTemperatureEndpoint, bridgedTemperatureClusters);
 

applications/matter_bridge/src/core/matter_bridged_device.cpp

@@ -6,6 +6,11 @@
 
 #include "matter_bridged_device.h"
 #include <app-common/zap-generated/callback.h>
+#include <app/AttributeValueDecoder.h>
+#include <app/clusters/identify-server/IdentifyCluster.h>
+#include <app/data-model/Encode.h>
+#include <lib/core/TLVReader.h>
+#include <lib/core/TLVWriter.h>
 #include <lib/support/ZclString.h>
 
 using namespace ::chip;
@@ -85,12 +90,42 @@ CHIP_ERROR MatterBridgedDevice::HandleWriteIdentify(chip::AttributeId attributeI
 {
 	switch (attributeId) {
 	case Clusters::Identify::Attributes::IdentifyTime::Id:
-		if (data && dataSize == sizeof(mIdentifyTime)) {
-			memcpy(&mIdentifyTime, data, sizeof(mIdentifyTime));
-			/* Externally stored attribute was updated, now we need to notify identify-server to
-			   leverage the Identify cluster implementation */
-			app::ConcreteAttributePath attributePath{ mEndpointId, Clusters::Identify::Id, attributeId };
-			MatterIdentifyClusterServerAttributeChangedCallback(attributePath);
+		if (data && dataSize == sizeof(uint16_t)) {
+			uint16_t identifyTime = *reinterpret_cast<uint16_t *>(data);
+
+			uint8_t tlvBuffer[64];
+			TLV::TLVWriter writer;
+			writer.Init(tlvBuffer, sizeof(tlvBuffer));
+
+			TLV::TLVType outerContainerType;
+			ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure,
+								   outerContainerType));
+			ReturnErrorOnFailure(DataModel::Encode(writer, TLV::ContextTag(1), identifyTime));
+			ReturnErrorOnFailure(writer.EndContainer(outerContainerType));
+			ReturnErrorOnFailure(writer.Finalize());
+
+			TLV::TLVReader reader;
+			reader.Init(tlvBuffer, writer.GetLengthWritten());
+			ReturnErrorOnFailure(reader.Next());
+			ReturnErrorOnFailure(reader.EnterContainer(outerContainerType));
+			ReturnErrorOnFailure(reader.Next());
+
+			Access::SubjectDescriptor subjectDescriptor;
+			subjectDescriptor.fabricIndex = kUndefinedFabricIndex;
+			subjectDescriptor.authMode = Access::AuthMode::kNone;
+
+			AttributeValueDecoder decoder(reader, subjectDescriptor);
+
+			DataModel::WriteAttributeRequest request;
+			request.path.mEndpointId = mEndpointId;
+			request.path.mClusterId = Clusters::Identify::Id;
+			request.path.mAttributeId = attributeId;
+			request.subjectDescriptor = &subjectDescriptor;
+
+			DataModel::ActionReturnStatus status = mIdentifyCluster.Cluster().WriteAttribute(request, decoder);
+			if (!status.IsSuccess()) {
+				return status.GetUnderlyingError();
+			}
 			return CHIP_NO_ERROR;
 		}
 	default:
@@ -111,11 +146,12 @@ CHIP_ERROR MatterBridgedDevice::HandleReadIdentify(chip::AttributeId attributeId
 		return CopyAttribute(&featureMap, sizeof(featureMap), buffer, maxReadLength);
 	}
 	case Clusters::Identify::Attributes::IdentifyType::Id: {
-		MatterBridgedDevice::IdentifyType type = mIdentifyServer.mIdentifyType;
+		MatterBridgedDevice::IdentifyType type = mIdentifyCluster.Cluster().GetIdentifyType();
 		return CopyAttribute(&type, sizeof(type), buffer, maxReadLength);
 	}
 	case Clusters::Identify::Attributes::IdentifyTime::Id: {
-		return CopyAttribute(&mIdentifyTime, sizeof(mIdentifyTime), buffer, maxReadLength);
+		uint16_t identifyTime = mIdentifyCluster.Cluster().GetIdentifyTime();
+		return CopyAttribute(&identifyTime, sizeof(identifyTime), buffer, maxReadLength);
 	}
 	default:
 		return CHIP_ERROR_INVALID_ARGUMENT;

applications/matter_bridge/src/core/matter_bridged_device.h

@@ -6,9 +6,14 @@
 
 #pragma once
 
+#include "app/server-cluster/ServerClusterInterfaceRegistry.h"
+#include "data-model-providers/codegen/CodegenDataModelProvider.h"
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app-common/zap-generated/ids/Clusters.h>
-#include <app/clusters/identify-server/identify-server.h>
+#include <app/AttributeAccessInterfaceRegistry.h>
+#include <app/CommandHandlerInterfaceRegistry.h>
+#include <app/DefaultTimerDelegate.h>
+#include <app/clusters/identify-server/IdentifyCluster.h>
 #include <app/util/attribute-storage.h>
 #include <platform/ConfigurationManager.h>
 
@@ -52,36 +57,33 @@ namespace Nrf
 			0), /* feature map */                                                                          \
 		DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();
 
-/* Declare Bridged Device Identify cluster attributes */
+/* Codedriven delegate for the Identify cluster */
+class IdentifyBridgedDeviceDelegateImpl : public chip::app::Clusters::IdentifyDelegate {
+public:
+	void OnIdentifyStart(chip::app::Clusters::IdentifyCluster &cluster) override
+	{
+		ChipLogError(DeviceLayer, "Starting bridged device identify on endpoint %d",
+			     cluster.GetPaths()[0].mEndpointId);
+	}
 
-constexpr chip::CommandId sIdentifyIncomingCommands[] = {
-	chip::app::Clusters::Identify::Commands::Identify::Id,
-	chip::kInvalidCommandId,
-};
+	void OnIdentifyStop(chip::app::Clusters::IdentifyCluster &cluster) override
+	{
+		ChipLogError(DeviceLayer, "Stopping bridged device identify on endpoint %d",
+			     cluster.GetPaths()[0].mEndpointId);
+		mIsTriggerEffectEnabled = false;
+	}
 
-#define IDENTIFY_CLUSTER_ATTRIBUTES(identifyAttrs)                                                                     \
-	DECLARE_DYNAMIC_ATTRIBUTE_LIST_BEGIN(identifyAttrs)                                                            \
-	DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Identify::Attributes::IdentifyTime::Id, INT16U, 2,              \
-				  ZAP_ATTRIBUTE_MASK(WRITABLE)), /* identify time*/                                    \
-		DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Identify::Attributes::IdentifyType::Id, ENUM8, 1,       \
-					  0), /* identify type */                                                      \
-		DECLARE_DYNAMIC_ATTRIBUTE(chip::app::Clusters::Identify::Attributes::FeatureMap::Id, BITMAP32, 4,      \
-					  0), /* feature map */                                                        \
-		DECLARE_DYNAMIC_ATTRIBUTE_LIST_END();
+	void OnTriggerEffect(chip::app::Clusters::IdentifyCluster &cluster) override
+	{
+		ChipLogError(DeviceLayer, "Triggering effect on endpoint %d", cluster.GetPaths()[0].mEndpointId);
+		mIsTriggerEffectEnabled = true;
+	}
 
-inline void IdentifyStartDefaultCb(Identify *identify)
-{
-	VerifyOrReturn(identify);
-	ChipLogProgress(DeviceLayer, "Starting bridged device identify on endpoint %d",
-			identify->mCluster.Cluster().GetPaths()[0].mEndpointId);
-}
+	bool IsTriggerEffectEnabled() const override { return mIsTriggerEffectEnabled; }
 
-inline void IdentifyStopDefaultCb(Identify *identify)
-{
-	VerifyOrReturn(identify);
-	ChipLogProgress(DeviceLayer, "Stopping bridged device identify on endpoint %d",
-			identify->mCluster.Cluster().GetPaths()[0].mEndpointId);
-}
+private:
+	bool mIsTriggerEffectEnabled = false;
+};
 
 class MatterBridgedDevice {
 public:
@@ -100,8 +102,14 @@ class MatterBridgedDevice {
 	static constexpr uint8_t kDescriptorAttributeArraySize = 254;
 
 	explicit MatterBridgedDevice(const char *uniqueID, const char *nodeLabel)
-		: mIdentifyServer(mEndpointId, IdentifyStartDefaultCb, IdentifyStopDefaultCb,
-				  IdentifyType::kVisibleIndicator)
+		/* TODO: Currently the mEndpointId cannot be used here because it is initialized later.
+		 * We need to refactor this construtor to initialize the mEndpointId here.
+		 * Ref: KRKNWK-21020
+		 */
+		: mIdentifyCluster(
+			  chip::app::Clusters::IdentifyCluster::Config(mEndpointId, mTimerDelegate)
+				  .WithIdentifyType(chip::app::Clusters::Identify::IdentifyTypeEnum::kVisibleIndicator)
+				  .WithDelegate(&mIdentifyDelegate))
 	{
 		if (uniqueID) {
 			memcpy(mUniqueID, uniqueID, strnlen(uniqueID, Nrf::MatterBridgedDevice::kUniqueIDSize));
@@ -112,7 +120,17 @@ class MatterBridgedDevice {
 	}
 	virtual ~MatterBridgedDevice() { chip::Platform::MemoryFree(mDataVersion); }
 
-	void Init(chip::EndpointId endpoint) {}
+	void Init(chip::EndpointId endpoint)
+	{
+		mEndpointId = endpoint;
+
+		CHIP_ERROR err = chip::app::CodegenDataModelProvider::Instance().Registry().Register(
+			mIdentifyCluster.Registration());
+		if (err != CHIP_NO_ERROR) {
+			ChipLogError(DeviceLayer, "Failed to register Identify cluster: %s", ErrorStr(err));
+		}
+	}
+
 	chip::EndpointId GetEndpointId() const { return mEndpointId; }
 
 	virtual uint16_t GetDeviceType() const = 0;
@@ -156,8 +174,9 @@ class MatterBridgedDevice {
 	bool mIsReachable = true;
 	char mUniqueID[kUniqueIDSize] = "";
 	char mNodeLabel[kNodeLabelSize] = "";
-	Identify mIdentifyServer;
-	uint16_t mIdentifyTime{};
+	chip::app::RegisteredServerCluster<chip::app::Clusters::IdentifyCluster> mIdentifyCluster;
+	chip::app::DefaultTimerDelegate mTimerDelegate;
+	IdentifyBridgedDeviceDelegateImpl mIdentifyDelegate;
 };
 
 } /* namespace Nrf */