Skip to content

Commit

Permalink
Use ProviderChangeListener directly to report attrbiute change
Browse files Browse the repository at this point in the history
  • Loading branch information
yufengwangca committed Dec 6, 2024
1 parent 45f544b commit f65773e
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 42 deletions.
17 changes: 16 additions & 1 deletion src/app/codegen-data-model-provider/CodegenDataModelProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,17 @@ 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
Expand Down Expand Up @@ -126,6 +137,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()
Expand Down Expand Up @@ -185,7 +198,7 @@ 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.
Expand Down Expand Up @@ -221,6 +234,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
18 changes: 0 additions & 18 deletions src/app/data-model-provider/MetadataTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/app/data-model-provider/Provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
};
Expand Down
14 changes: 9 additions & 5 deletions src/app/reporting/reporting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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));
}
14 changes: 14 additions & 0 deletions src/app/tests/test-interaction-model-api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/test-interaction-model-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
14 changes: 14 additions & 0 deletions src/controller/tests/data_model/DataModelFixtures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/controller/tests/data_model/DataModelFixtures.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down

0 comments on commit f65773e

Please sign in to comment.