From 2c98093312b1b6de1cc90d383190f0dd9782bf2b Mon Sep 17 00:00:00 2001 From: Moises Terrones Date: Tue, 11 Feb 2025 11:28:27 -0600 Subject: [PATCH 1/6] Add ListNotifications on DataModelProvider --- src/app/WriteHandler.cpp | 9 ++++---- src/app/data-model-provider/OperationTypes.h | 7 ++++++ src/app/data-model-provider/Provider.h | 8 +++++++ .../codegen/CodegenDataModelProvider.h | 1 + .../CodegenDataModelProvider_Write.cpp | 22 +++++++++++++++++++ 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 09a9ec0079e901..a6506d4bc6d927 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -258,17 +258,18 @@ CHIP_ERROR WriteHandler::SendWriteResponse(System::PacketBufferTLVWriter && aMes void WriteHandler::DeliverListWriteBegin(const ConcreteAttributePath & aPath) { - if (auto * attrOverride = AttributeAccessInterfaceRegistry::Instance().Get(aPath.mEndpointId, aPath.mClusterId)) + if(mDataModelProvider != nullptr) { - attrOverride->OnListWriteBegin(aPath); + mDataModelProvider->ListAttributeWriteNotification(aPath, DataModel::ListWriteOperation::kListWriteBegin); } } void WriteHandler::DeliverListWriteEnd(const ConcreteAttributePath & aPath, bool writeWasSuccessful) { - if (auto * attrOverride = AttributeAccessInterfaceRegistry::Instance().Get(aPath.mEndpointId, aPath.mClusterId)) + if(mDataModelProvider != nullptr) { - attrOverride->OnListWriteEnd(aPath, writeWasSuccessful); + mDataModelProvider->ListAttributeWriteNotification(aPath, writeWasSuccessful ? + DataModel::ListWriteOperation::kListWriteEndFinal : DataModel::ListWriteOperation::kListWriteEnd); } } diff --git a/src/app/data-model-provider/OperationTypes.h b/src/app/data-model-provider/OperationTypes.h index 35d1654ac12568..f71de6946a97c9 100644 --- a/src/app/data-model-provider/OperationTypes.h +++ b/src/app/data-model-provider/OperationTypes.h @@ -73,6 +73,13 @@ enum class ReadFlags : uint32_t kFabricFiltered = 0x0001, // reading is performed fabric-filtered }; +enum class ListWriteOperation : uint8_t +{ + kListWriteBegin = 0, + kListWriteEnd, + kListWriteEndFinal +}; + struct ReadAttributeRequest : OperationRequest { ConcreteAttributePath path; diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index fc5a204c869785..08447323cf53e9 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -79,6 +79,14 @@ class Provider : public ProviderMetadataTree /// - validation of ACL/timed interaction flags/writability, if those checks are desired. virtual ActionReturnStatus WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) = 0; + /// Indicates the start/end of a series of list operations. This function will be called either before the first + /// Write operation or after the last one of a series of consequence attribute data of the same attribute. + /// + /// 1) This function will be called if the client tries to set a nullable list attribute to null. + /// 2) This function will only be called once for a series of consequent attribute data (regardless the kind of list operation) + /// of the same attribute. + virtual void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) = 0; + /// `handler` is used to send back the reply. /// - returning `std::nullopt` means that return value was placed in handler directly. /// This includes cases where command handling and value return will be done asynchronously. diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.h b/src/data-model-providers/codegen/CodegenDataModelProvider.h index 764f5f74874673..8b966ec78a4f4c 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.h +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.h @@ -63,6 +63,7 @@ class CodegenDataModelProvider : public DataModel::Provider AttributeValueEncoder & encoder) override; DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; + void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) override; std::optional Invoke(const DataModel::InvokeRequest & request, TLV::TLVReader & input_arguments, CommandHandler * handler) override; diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp index 3160bbdaf26722..0d86d31c814c1a 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp @@ -198,6 +198,28 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat return CHIP_NO_ERROR; } +void CodegenDataModelProvider::ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) { + AttributeAccessInterface * aai = + AttributeAccessInterfaceRegistry::Instance().Get(aPath.mEndpointId, aPath.mClusterId); + + if(aai != nullptr) + { + switch(opType) + { + case DataModel::ListWriteOperation::kListWriteBegin: + aai->OnListWriteBegin(aPath); + break; + case DataModel::ListWriteOperation::kListWriteEnd: + aai->OnListWriteEnd(aPath, false); + break; + case DataModel::ListWriteOperation::kListWriteEndFinal: + aai->OnListWriteEnd(aPath, true); + break; + } + } + +} + void CodegenDataModelProvider::Temporary_ReportAttributeChanged(const AttributePathParams & path) { ContextAttributesChangeListener change_listener(CurrentContext()); From 664454ff441a4e85225cff171203c7c17f42860c Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 11 Feb 2025 21:55:21 +0000 Subject: [PATCH 2/6] Restyled by whitespace --- src/app/WriteHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index a6506d4bc6d927..6bfa334b0cc401 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -268,7 +268,7 @@ void WriteHandler::DeliverListWriteEnd(const ConcreteAttributePath & aPath, bool { if(mDataModelProvider != nullptr) { - mDataModelProvider->ListAttributeWriteNotification(aPath, writeWasSuccessful ? + mDataModelProvider->ListAttributeWriteNotification(aPath, writeWasSuccessful ? DataModel::ListWriteOperation::kListWriteEndFinal : DataModel::ListWriteOperation::kListWriteEnd); } } From 5d233a2107683b64c20d85e2300653a30889a00e Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 11 Feb 2025 21:55:25 +0000 Subject: [PATCH 3/6] Restyled by clang-format --- src/app/WriteHandler.cpp | 9 +++--- src/app/data-model-provider/Provider.h | 4 +-- .../codegen/CodegenDataModelProvider.h | 3 +- .../CodegenDataModelProvider_Write.cpp | 30 +++++++++---------- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 6bfa334b0cc401..00a7a5e7c6b7d3 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -258,7 +258,7 @@ CHIP_ERROR WriteHandler::SendWriteResponse(System::PacketBufferTLVWriter && aMes void WriteHandler::DeliverListWriteBegin(const ConcreteAttributePath & aPath) { - if(mDataModelProvider != nullptr) + if (mDataModelProvider != nullptr) { mDataModelProvider->ListAttributeWriteNotification(aPath, DataModel::ListWriteOperation::kListWriteBegin); } @@ -266,10 +266,11 @@ void WriteHandler::DeliverListWriteBegin(const ConcreteAttributePath & aPath) void WriteHandler::DeliverListWriteEnd(const ConcreteAttributePath & aPath, bool writeWasSuccessful) { - if(mDataModelProvider != nullptr) + if (mDataModelProvider != nullptr) { - mDataModelProvider->ListAttributeWriteNotification(aPath, writeWasSuccessful ? - DataModel::ListWriteOperation::kListWriteEndFinal : DataModel::ListWriteOperation::kListWriteEnd); + mDataModelProvider->ListAttributeWriteNotification(aPath, + writeWasSuccessful ? DataModel::ListWriteOperation::kListWriteEndFinal + : DataModel::ListWriteOperation::kListWriteEnd); } } diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index 08447323cf53e9..2237315827d49d 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -83,8 +83,8 @@ class Provider : public ProviderMetadataTree /// Write operation or after the last one of a series of consequence attribute data of the same attribute. /// /// 1) This function will be called if the client tries to set a nullable list attribute to null. - /// 2) This function will only be called once for a series of consequent attribute data (regardless the kind of list operation) - /// of the same attribute. + /// 2) This function will only be called once for a series of consequent attribute data (regardless the kind of list + /// operation) of the same attribute. virtual void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) = 0; /// `handler` is used to send back the reply. diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.h b/src/data-model-providers/codegen/CodegenDataModelProvider.h index 8b966ec78a4f4c..5a916480917ee8 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.h +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.h @@ -63,7 +63,8 @@ class CodegenDataModelProvider : public DataModel::Provider AttributeValueEncoder & encoder) override; DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; - void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) override; + void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, + BitFlags opType) override; std::optional Invoke(const DataModel::InvokeRequest & request, TLV::TLVReader & input_arguments, CommandHandler * handler) override; diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp index 0d86d31c814c1a..518c173becaa53 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp @@ -198,26 +198,26 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat return CHIP_NO_ERROR; } -void CodegenDataModelProvider::ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) { - AttributeAccessInterface * aai = - AttributeAccessInterfaceRegistry::Instance().Get(aPath.mEndpointId, aPath.mClusterId); +void CodegenDataModelProvider::ListAttributeWriteNotification(const ConcreteAttributePath & aPath, + BitFlags opType) +{ + AttributeAccessInterface * aai = AttributeAccessInterfaceRegistry::Instance().Get(aPath.mEndpointId, aPath.mClusterId); - if(aai != nullptr) + if (aai != nullptr) { - switch(opType) + switch (opType) { - case DataModel::ListWriteOperation::kListWriteBegin: - aai->OnListWriteBegin(aPath); - break; - case DataModel::ListWriteOperation::kListWriteEnd: - aai->OnListWriteEnd(aPath, false); - break; - case DataModel::ListWriteOperation::kListWriteEndFinal: - aai->OnListWriteEnd(aPath, true); - break; + case DataModel::ListWriteOperation::kListWriteBegin: + aai->OnListWriteBegin(aPath); + break; + case DataModel::ListWriteOperation::kListWriteEnd: + aai->OnListWriteEnd(aPath, false); + break; + case DataModel::ListWriteOperation::kListWriteEndFinal: + aai->OnListWriteEnd(aPath, true); + break; } } - } void CodegenDataModelProvider::Temporary_ReportAttributeChanged(const AttributePathParams & path) From 1a61b044e1050dee2d259940550e41bafdb9e5b2 Mon Sep 17 00:00:00 2001 From: Moises Terrones Date: Wed, 12 Feb 2025 21:16:01 +0000 Subject: [PATCH 4/6] Rename enum values and add more information in comments. --- src/app/WriteHandler.cpp | 4 ++-- src/app/data-model-provider/OperationTypes.h | 4 ++-- src/app/data-model-provider/Provider.h | 7 ++++--- .../codegen/CodegenDataModelProvider_Write.cpp | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 00a7a5e7c6b7d3..1974d7e40ce71b 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -269,8 +269,8 @@ void WriteHandler::DeliverListWriteEnd(const ConcreteAttributePath & aPath, bool if (mDataModelProvider != nullptr) { mDataModelProvider->ListAttributeWriteNotification(aPath, - writeWasSuccessful ? DataModel::ListWriteOperation::kListWriteEndFinal - : DataModel::ListWriteOperation::kListWriteEnd); + writeWasSuccessful ? DataModel::ListWriteOperation::kListWriteSuccess + : DataModel::ListWriteOperation::kListWriteFailure); } } diff --git a/src/app/data-model-provider/OperationTypes.h b/src/app/data-model-provider/OperationTypes.h index f71de6946a97c9..51b8056c6fb7c6 100644 --- a/src/app/data-model-provider/OperationTypes.h +++ b/src/app/data-model-provider/OperationTypes.h @@ -76,8 +76,8 @@ enum class ReadFlags : uint32_t enum class ListWriteOperation : uint8_t { kListWriteBegin = 0, - kListWriteEnd, - kListWriteEndFinal + kListWriteSuccess, + kListWriteFailure }; struct ReadAttributeRequest : OperationRequest diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index 2237315827d49d..96bf4570bc42fc 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -80,11 +80,12 @@ class Provider : public ProviderMetadataTree virtual ActionReturnStatus WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) = 0; /// Indicates the start/end of a series of list operations. This function will be called either before the first - /// Write operation or after the last one of a series of consequence attribute data of the same attribute. + /// Write operation or after the last one of a series of consecutive attribute data of the same attribute. /// /// 1) This function will be called if the client tries to set a nullable list attribute to null. - /// 2) This function will only be called once for a series of consequent attribute data (regardless the kind of list - /// operation) of the same attribute. + /// 2) This function will only be called for a series of consecutive attribute data (regardless the kind of list + /// operation) of the same attribute at the beggining or end of the Write operations. + /// 3) The opType argument indicates the type of notification (Start, Failure, Success). virtual void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) = 0; /// `handler` is used to send back the reply. diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp index 518c173becaa53..9070f76313b5d4 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp @@ -210,10 +210,10 @@ void CodegenDataModelProvider::ListAttributeWriteNotification(const ConcreteAttr case DataModel::ListWriteOperation::kListWriteBegin: aai->OnListWriteBegin(aPath); break; - case DataModel::ListWriteOperation::kListWriteEnd: + case DataModel::ListWriteOperation::kListWriteFailure: aai->OnListWriteEnd(aPath, false); break; - case DataModel::ListWriteOperation::kListWriteEndFinal: + case DataModel::ListWriteOperation::kListWriteSuccess: aai->OnListWriteEnd(aPath, true); break; } From e8612b60a6cba7d114e5528593fcca95f406da35 Mon Sep 17 00:00:00 2001 From: Moises Terrones Date: Thu, 27 Feb 2025 10:31:24 -0600 Subject: [PATCH 5/6] Update src/app/data-model-provider/Provider.h Co-authored-by: Boris Zbarsky --- src/app/data-model-provider/Provider.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index 96bf4570bc42fc..66ad1a6a2629a4 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -83,8 +83,8 @@ class Provider : public ProviderMetadataTree /// Write operation or after the last one of a series of consecutive attribute data of the same attribute. /// /// 1) This function will be called if the client tries to set a nullable list attribute to null. - /// 2) This function will only be called for a series of consecutive attribute data (regardless the kind of list - /// operation) of the same attribute at the beggining or end of the Write operations. + /// 2) This function will only be called at the beginning and end of a series of consecutive attribute data + /// blocks for the same attribute, no matter what list operations those data blocks represent. /// 3) The opType argument indicates the type of notification (Start, Failure, Success). virtual void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) = 0; From f6d0f1cda0ee76c394c7451f8365fa889a6c1533 Mon Sep 17 00:00:00 2001 From: Moises Terrones Date: Thu, 27 Feb 2025 17:44:13 +0000 Subject: [PATCH 6/6] Remove BitFlags from Provider --- src/app/data-model-provider/Provider.h | 4 ++-- src/data-model-providers/codegen/CodegenDataModelProvider.h | 3 +-- .../codegen/CodegenDataModelProvider_Write.cpp | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index 66ad1a6a2629a4..27a86b63fa0cf2 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -83,10 +83,10 @@ class Provider : public ProviderMetadataTree /// Write operation or after the last one of a series of consecutive attribute data of the same attribute. /// /// 1) This function will be called if the client tries to set a nullable list attribute to null. - /// 2) This function will only be called at the beginning and end of a series of consecutive attribute data + /// 2) This function will only be called at the beginning and end of a series of consecutive attribute data /// blocks for the same attribute, no matter what list operations those data blocks represent. /// 3) The opType argument indicates the type of notification (Start, Failure, Success). - virtual void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, BitFlags opType) = 0; + virtual void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, ListWriteOperation opType) = 0; /// `handler` is used to send back the reply. /// - returning `std::nullopt` means that return value was placed in handler directly. diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.h b/src/data-model-providers/codegen/CodegenDataModelProvider.h index 5a916480917ee8..9edea20ed6a7a7 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.h +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.h @@ -63,8 +63,7 @@ class CodegenDataModelProvider : public DataModel::Provider AttributeValueEncoder & encoder) override; DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; - void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, - BitFlags opType) override; + void ListAttributeWriteNotification(const ConcreteAttributePath & aPath, DataModel::ListWriteOperation opType) override; std::optional Invoke(const DataModel::InvokeRequest & request, TLV::TLVReader & input_arguments, CommandHandler * handler) override; diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp index 9070f76313b5d4..647ea9d4c0e65b 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp @@ -199,7 +199,7 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat } void CodegenDataModelProvider::ListAttributeWriteNotification(const ConcreteAttributePath & aPath, - BitFlags opType) + DataModel::ListWriteOperation opType) { AttributeAccessInterface * aai = AttributeAccessInterfaceRegistry::Instance().Get(aPath.mEndpointId, aPath.mClusterId);