Use ProviderChangeListener directly to report attrbiute change

Author: yufengwangca

src/app/codegen-data-model-provider/CodegenDataModelProvider.h

@@ -80,6 +80,18 @@ class EnumeratorCommandFinder
     }
 };
 
+class DefaultProviderChangeListener : public DataModel::ProviderChangeListener
+{
+public:
+    explicit DefaultProviderChangeListener(DataModel::Provider & provider) : mProvider(provider) 
+    {}
+    
+    void MarkDirty(const AttributePathParams & path) override;
+
+private:
+    DataModel::Provider & mProvider;    
+};    
+
 } // namespace detail
 
 /// An implementation of `InteractionModel::Model` that relies on code-generation
@@ -126,6 +138,8 @@ class CodegenDataModelProvider : public DataModel::Provider
     };
 
 public:
+    CodegenDataModelProvider() : mChangeListener(*this) {}
+
     /// clears out internal caching. Especially useful in unit tests,
     /// where path caching does not really apply (the same path may result in different outcomes)
     void Reset()
@@ -185,7 +199,10 @@ class CodegenDataModelProvider : public DataModel::Provider
     ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) override;
     ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) override;
 
-    void Temporary_ReportAttributeChanged(const AttributePathParams & path) override;
+    DataModel::ProviderChangeListener & GetAttributeChangeReporter() override
+    {
+        return mChangeListener;
+    }    
 
 private:
     // Iteration is often done in a tight loop going through all values.
@@ -221,6 +238,8 @@ class CodegenDataModelProvider : public DataModel::Provider
     // Ember requires a persistence provider, so we make sure we can always have something
     PersistentStorageDelegate * mPersistentStorageDelegate = nullptr;
 
+    detail::DefaultProviderChangeListener mChangeListener;
+
     /// Finds the specified ember cluster
     ///
     /// Effectively the same as `emberAfFindServerCluster` except with some caching capabilities

src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp

@@ -88,6 +88,26 @@ std::optional<CHIP_ERROR> TryWriteViaAccessInterface(const ConcreteDataAttribute
 
 } // namespace
 
+namespace detail {
+
+void DefaultProviderChangeListener::MarkDirty(const AttributePathParams & path)
+{
+    ContextAttributesChangeListener change_listener(mProvider.CurrentContext());
+    if (path.mClusterId != kInvalidClusterId)
+    {
+        emberAfAttributeChanged(path.mEndpointId, path.mClusterId, path.mAttributeId, &change_listener);
+    }
+    else
+    {
+        // When the path has wildcard cluster Id, call the emberAfEndpointChanged to mark attributes on the given endpoint
+        // as having changing, but do NOT increase/alter any cluster data versions, as this happens when a bridged endpoint is
+        // added or removed from a bridge and the cluster data is not changed during the process.
+        emberAfEndpointChanged(path.mEndpointId, &change_listener);
+    }
+}
+
+} // namespace detail
+
 DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const DataModel::WriteAttributeRequest & request,
                                                                        AttributeValueDecoder & decoder)
 {
@@ -261,21 +281,5 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat
     return CHIP_NO_ERROR;
 }
 
-void CodegenDataModelProvider::Temporary_ReportAttributeChanged(const AttributePathParams & path)
-{
-    ContextAttributesChangeListener change_listener(CurrentContext());
-    if (path.mClusterId != kInvalidClusterId)
-    {
-        emberAfAttributeChanged(path.mEndpointId, path.mClusterId, path.mAttributeId, &change_listener);
-    }
-    else
-    {
-        // When the path has wildcard cluster Id, call the emberAfEndpointChanged to mark attributes on the given endpoint
-        // as having changing, but do NOT increase/alter any cluster data versions, as this happens when a bridged endpoint is
-        // added or removed from a bridge and the cluster data is not changed during the process.
-        emberAfEndpointChanged(path.mEndpointId, &change_listener);
-    }
-}
-
 } // namespace app
 } // namespace chip

src/app/data-model-provider/MetadataTypes.h

@@ -221,24 +221,6 @@ class ProviderMetadataTree
     // returned as responses.
     virtual ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) = 0;
     virtual ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before)   = 0;
-
-    /// Workaround function to report attribute change.
-    ///
-    /// When this is invoked, the caller is expected to increment the cluster data version, and the attribute path
-    /// should be marked as `dirty` by the data model provider listener so that the reporter can notify the subscriber
-    /// of attribute changes.
-    /// This function should be invoked when attribute managed by attribute access interface is modified but not
-    /// through an actual Write interaction.
-    /// For example, if the LastNetworkingStatus attribute changes because the NetworkCommissioning driver detects a
-    /// network connection status change and calls SetLastNetworkingStatusValue(). The data model provider can recognize
-    /// this change by invoking this function at the point of change.
-    ///
-    /// This is a workaround function as we cannot notify the attribute change to the data model provider. The provider
-    /// should own its data and versions.
-    ///
-    /// TODO: We should remove this function when the AttributeAccessInterface/CommandHandlerInterface is able to report
-    /// the attribute changes.
-    virtual void Temporary_ReportAttributeChanged(const AttributePathParams & path) = 0;
 };
 
 } // namespace DataModel

src/app/data-model-provider/Provider.h

@@ -97,6 +97,15 @@ class Provider : public ProviderMetadataTree
     virtual std::optional<ActionReturnStatus> Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments,
                                                      CommandHandler * handler) = 0;
 
+    /// Returns a reference to the ProviderChangeListener used for reporting attribute changes.
+    ///
+    /// This is used to notify listeners about changes in attributes managed by the provider.
+    /// Listeners can be used to react to updates in the underlying data model.
+    ///
+    /// It is expected that implementations provide a valid reference, ensuring that
+    /// attribute change reporting is functional throughout the lifecycle of the Provider.
+    virtual ProviderChangeListener & GetAttributeChangeReporter() = 0;
+
 private:
     InteractionModelContext mContext = { nullptr };
 };

src/app/reporting/reporting.cpp

@@ -30,8 +30,9 @@ void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clust
     // applications notifying about changes from their end.
     assertChipStackLockedByCurrentThread();
 
-    InteractionModelEngine::GetInstance()->GetDataModelProvider()->Temporary_ReportAttributeChanged(
-        AttributePathParams(endpoint, clusterId, attributeId));
+    DataModel::ProviderChangeListener & changeListener =
+        InteractionModelEngine::GetInstance()->GetDataModelProvider()->GetAttributeChangeReporter();
+    changeListener.MarkDirty(AttributePathParams(endpoint, clusterId, attributeId));
 }
 
 void MatterReportingAttributeChangeCallback(const ConcreteAttributePath & aPath)
@@ -40,8 +41,9 @@ void MatterReportingAttributeChangeCallback(const ConcreteAttributePath & aPath)
     // applications notifying about changes from their end.
     assertChipStackLockedByCurrentThread();
 
-    InteractionModelEngine::GetInstance()->GetDataModelProvider()->Temporary_ReportAttributeChanged(
-        AttributePathParams(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId));
+    DataModel::ProviderChangeListener & changeListener =
+        InteractionModelEngine::GetInstance()->GetDataModelProvider()->GetAttributeChangeReporter();
+    changeListener.MarkDirty(AttributePathParams(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId));
 }
 
 void MatterReportingAttributeChangeCallback(EndpointId endpoint)
@@ -50,5 +52,7 @@ void MatterReportingAttributeChangeCallback(EndpointId endpoint)
     // applications notifying about changes from their end.
     assertChipStackLockedByCurrentThread();
 
-    InteractionModelEngine::GetInstance()->GetDataModelProvider()->Temporary_ReportAttributeChanged(AttributePathParams(endpoint));
+    DataModel::ProviderChangeListener & changeListener =
+        InteractionModelEngine::GetInstance()->GetDataModelProvider()->GetAttributeChangeReporter();
+    changeListener.MarkDirty(AttributePathParams(endpoint));
 }

src/app/tests/test-interaction-model-api.cpp

@@ -60,6 +60,14 @@ class TestOnlyAttributeValueDecoderAccessor
     AttributeValueDecoder & mDecoder;
 };
 
+class TestProviderChangeListener : public DataModel::ProviderChangeListener
+{
+public:
+    explicit TestProviderChangeListener() {}
+
+    void MarkDirty(const AttributePathParams & path) override {}
+};
+
 // strong defintion in TestCommandInteraction.cpp
 __attribute__((weak)) void DispatchSingleClusterCommand(const ConcreteCommandPath & aRequestCommandPath,
                                                         chip::TLV::TLVReader & aReader, CommandHandler * apCommandObj)
@@ -149,6 +157,12 @@ std::optional<ActionReturnStatus> TestImCustomDataModel::Invoke(const InvokeRequ
     return std::make_optional<ActionReturnStatus>(CHIP_ERROR_NOT_IMPLEMENTED);
 }
 
+ProviderChangeListener & TestImCustomDataModel::GetAttributeChangeReporter()
+{
+    static TestProviderChangeListener changeListener;
+    return changeListener;
+}
+
 DataModel::EndpointEntry TestImCustomDataModel::FirstEndpoint()
 {
     return CodegenDataModelProviderInstance(nullptr /* delegate */)->FirstEndpoint();

src/app/tests/test-interaction-model-api.h

@@ -109,6 +109,7 @@ class TestImCustomDataModel : public DataModel::Provider
                                                  AttributeValueDecoder & decoder) override;
     std::optional<DataModel::ActionReturnStatus> Invoke(const DataModel::InvokeRequest & request,
                                                         chip::TLV::TLVReader & input_arguments, CommandHandler * handler) override;
+    DataModel::ProviderChangeListener & GetAttributeChangeReporter() override;
 
     DataModel::EndpointEntry FirstEndpoint() override;
     DataModel::EndpointEntry NextEndpoint(EndpointId before) override;
@@ -131,7 +132,6 @@ class TestImCustomDataModel : public DataModel::Provider
     std::optional<DataModel::CommandInfo> GetAcceptedCommandInfo(const ConcreteCommandPath & path) override;
     ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) override;
     ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) override;
-    void Temporary_ReportAttributeChanged(const AttributePathParams & path) override {}
 };
 
 } // namespace app

src/controller/tests/data_model/DataModelFixtures.cpp

@@ -67,6 +67,14 @@ class TestOnlyAttributeValueDecoderAccessor
     AttributeValueDecoder & mDecoder;
 };
 
+class TestProviderChangeListener : public DataModel::ProviderChangeListener
+{
+public:
+    explicit TestProviderChangeListener() {}
+
+    void MarkDirty(const AttributePathParams & path) override {}
+};
+
 namespace DataModelTests {
 
 ScopedChangeOnly<ReadResponseDirective> gReadResponseDirective(ReadResponseDirective::kSendDataResponse);
@@ -474,6 +482,12 @@ std::optional<ActionReturnStatus> CustomDataModel::Invoke(const InvokeRequest &
     return std::nullopt; // handler status is set by the dispatch
 }
 
+ProviderChangeListener & CustomDataModel::GetAttributeChangeReporter()
+{
+    static TestProviderChangeListener changeListener;
+    return changeListener;
+}
+
 DataModel::EndpointEntry CustomDataModel::FirstEndpoint()
 {
     return CodegenDataModelProviderInstance(nullptr /* delegate */)->FirstEndpoint();

src/controller/tests/data_model/DataModelFixtures.h

@@ -121,6 +121,7 @@ class CustomDataModel : public DataModel::Provider
                                                  AttributeValueDecoder & decoder) override;
     std::optional<DataModel::ActionReturnStatus> Invoke(const DataModel::InvokeRequest & request,
                                                         chip::TLV::TLVReader & input_arguments, CommandHandler * handler) override;
+    DataModel::ProviderChangeListener & GetAttributeChangeReporter() override;
 
     DataModel::EndpointEntry FirstEndpoint() override;
     DataModel::EndpointEntry NextEndpoint(EndpointId before) override;
@@ -143,7 +144,6 @@ class CustomDataModel : public DataModel::Provider
     std::optional<DataModel::CommandInfo> GetAcceptedCommandInfo(const ConcreteCommandPath & path) override;
     ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) override;
     ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) override;
-    void Temporary_ReportAttributeChanged(const AttributePathParams & path) override {}
 };
 
 } // namespace DataModelTests