Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT/TEST] Evaluate cost of flash if switching to a templated abstract iterator on DataModel::Provider #36889

Draft
wants to merge 58 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
7c8ec0f
Start defining some iterator functions that may be used ... it is sti…
andreilitvin Dec 17, 2024
a3fc9e3
Start defining iterator-based implementations of things
andreilitvin Dec 17, 2024
c86d7d3
more comments, no implementation. This WILL be scary
andreilitvin Dec 17, 2024
4436e81
Make tests pass by providing a default NULL implementation of thins
andreilitvin Dec 17, 2024
dee40a1
Flat iterator of device types works
andreilitvin Dec 17, 2024
2fc1276
Flip code for device types to use iterator getters
andy31415 Dec 18, 2024
f7aba74
Restyle
andy31415 Dec 18, 2024
3f20fdf
Fix includes
andy31415 Dec 18, 2024
529f262
Fix includes
andy31415 Dec 18, 2024
f56a010
Fix typo
andy31415 Dec 18, 2024
a3edec0
Fix includes
andy31415 Dec 18, 2024
aa7cc8f
Fix lint
andy31415 Dec 18, 2024
d1a87c5
Make GetDeviceTypes pure virtual since we have an impl now
andy31415 Dec 18, 2024
0d0f93c
Prototype semantic tags implementation as well, to see if code size s…
andy31415 Dec 18, 2024
3d41548
Restyle
andy31415 Dec 18, 2024
27b9ca0
One more attempt to decrease flash usage by less instantiation of things
andy31415 Dec 18, 2024
ef07811
Move client cluster to use iterators, see how flash behaves after that
andy31415 Dec 18, 2024
970908f
Fix includes
andy31415 Dec 18, 2024
4190eac
One more code simplification
andy31415 Dec 18, 2024
5d69b4e
Cleanup server client iteration hint as it is not needed anymore
andy31415 Dec 18, 2024
54469a9
Fix android/dynamic dispatch signature
andy31415 Dec 18, 2024
f4041d9
Fix implementation ... was missing function not type
andy31415 Dec 18, 2024
b17caa2
Restyle
andy31415 Dec 18, 2024
0c640bc
Have an implementation of endpoint iterator. AttributePathExpander do…
andy31415 Dec 18, 2024
ba5e046
Restyle
andy31415 Dec 18, 2024
ea91053
Remove one more unused method
andy31415 Dec 18, 2024
51fcd67
Code compile but TESTS DO NOT PASS
andy31415 Dec 18, 2024
f9242b6
Restyle
andy31415 Dec 18, 2024
37073c8
Unit tests for attributepathexpanditerator exists, however we need mo…
andy31415 Dec 18, 2024
4c402f0
Unit tests pass now
andy31415 Dec 18, 2024
20c5550
Make attribute path expand iterator next usage more sane ... this may…
andy31415 Dec 18, 2024
68ec79c
Restyle
andy31415 Dec 18, 2024
0c36454
Stronger tests for session reset for AttributePathExpandIterator
andy31415 Dec 18, 2024
6addabd
Fix up descriptor cluster
andy31415 Dec 18, 2024
9534473
Fix cast ... int promotion is annoying
andy31415 Dec 18, 2024
3022dff
Hackish way to save RAM usage on AttributePathExpandIterator
andy31415 Dec 18, 2024
35af481
Restyle
andy31415 Dec 18, 2024
d0f3069
Fix includes
andy31415 Dec 18, 2024
d64421f
Generated commands implementation
andy31415 Dec 18, 2024
035a759
Restyle
andy31415 Dec 18, 2024
b864597
Fix some complains in compilers about shadowing
andy31415 Dec 18, 2024
1ac6576
Fix one more shadowing complaint
andy31415 Dec 18, 2024
255d458
More notlint to make compilers happy ... tests should not have these...
andy31415 Dec 18, 2024
8df1a35
Fix up some includes
andreilitvin Dec 19, 2024
86d9199
Convert accepted commands to iterators as well
andreilitvin Dec 19, 2024
95a242f
Fix include
andreilitvin Dec 19, 2024
4360210
Converted server clusters to iterators, unit tests pass
andreilitvin Dec 19, 2024
3a18e52
Restyle
andreilitvin Dec 19, 2024
6d809ce
Fix format specifiers
andreilitvin Dec 19, 2024
a813df9
Restyled by clang-format
restyled-commits Dec 19, 2024
74b9e29
Convert attributes to iterators ... conversion now fully done and uni…
andreilitvin Dec 19, 2024
dff921a
Fix includes
andreilitvin Dec 19, 2024
c954d21
Restyled by clang-format
restyled-commits Dec 19, 2024
0dc4363
Add clang-tidy ignores for tests where we actually do validate optionals
andreilitvin Dec 19, 2024
246fd32
One more fix
andreilitvin Dec 19, 2024
44ad069
Fix tidy warning: tidy was right
andreilitvin Dec 19, 2024
85215fd
Merge branch 'master' into iterator-update
andreilitvin Dec 19, 2024
3967e2f
Merge branch 'master' into iterator-update
andreilitvin Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion examples/common/pigweed/rpc_services/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,14 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
request.operationFlags.Set(app::DataModel::OperationFlags::kInternal);
request.subjectDescriptor = &subjectDescriptor;

std::optional<app::DataModel::ClusterInfo> info = provider->GetServerClusterInfo(path);
std::optional<app::DataModel::ClusterInfo> info;

auto clusters = provider->GetServerClusters(path.mEndpointId);
if (clusters->SeekTo(path.mClusterId))
{
info = clusters->GetMetadata();
}

if (!info.has_value())
{
return ::pw::Status::NotFound();
Expand Down
4 changes: 2 additions & 2 deletions src/access/ProviderDeviceTypeResolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ class DynamicProviderDeviceTypeResolver : public chip::Access::AccessControl::De

bool IsDeviceTypeOnEndpoint(chip::DeviceTypeId deviceType, chip::EndpointId endpoint) override
{
app::DataModel::Provider * model = mModelGetter();
for (auto type = model->FirstDeviceType(endpoint); type.has_value(); type = model->NextDeviceType(endpoint, *type))
auto it = mModelGetter()->GetDeviceTypes(endpoint);
for (auto type = it->Next(); type.has_value(); type = it->Next())
{
if (type->deviceTypeId == deviceType)
{
Expand Down
158 changes: 99 additions & 59 deletions src/app/AttributePathExpandIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
#include <app/AttributePathExpandIterator.h>

#include <app/GlobalAttributes.h>
#include <lib/core/DataModelTypes.h>
#include <lib/support/CodeUtils.h>
#include <optional>

using namespace chip::app::DataModel;

Expand All @@ -28,13 +30,9 @@ AttributePathExpandIterator::AttributePathExpandIterator(DataModel::Provider * p
SingleLinkedListNode<AttributePathParams> * attributePath) :
mDataModelProvider(provider),
mpAttributePath(attributePath), mOutputPath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId)

{
mOutputPath.mExpanded = true; // this is reset in 'next' if needed

// Make the iterator ready to emit the first valid path in the list.
// TODO: the bool return value here is completely unchecked
Next();
mOutputPath.mNeedsInitialization = true; // Ensure a Next is called on all public API
mOutputPath.mExpanded = true; // this is reset in 'next' if needed
}

bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId)
Expand All @@ -49,31 +47,26 @@ bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId)
break;
}

const ConcreteAttributePath attributePath(mOutputPath.mEndpointId, mOutputPath.mClusterId, attributeId);
return mDataModelProvider->GetAttributeInfo(attributePath).has_value();
return mDataModelProvider->GetAttributes(mOutputPath)->SeekTo(attributeId);
}

std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId(SearchSession & session)
{
if (mOutputPath.mAttributeId == kInvalidAttributeId)
{
if (mpAttributePath->mValue.HasWildcardAttributeId())
{
AttributeEntry entry = mDataModelProvider->FirstAttribute(mOutputPath);
return entry.IsValid() //
? entry.path.mAttributeId //
: Clusters::Globals::Attributes::GeneratedCommandList::Id; //
}

// We allow fixed attribute IDs if and only if they are valid:
// - they may be GLOBAL attributes OR
// - they are valid attributes for this cluster
if (IsValidAttributeId(mpAttributePath->mValue.mAttributeId))
if (!mpAttributePath->mValue.HasWildcardAttributeId())
{
// We allow fixed attribute IDs if and only if they are valid:
// - they may be GLOBAL attributes OR
// - they are valid attributes for this cluster
if (!IsValidAttributeId(mpAttributePath->mValue.mAttributeId))
{
return std::nullopt;
}
return mpAttributePath->mValue.mAttributeId;
}

return std::nullopt;
session.attributes = mDataModelProvider->GetAttributes(mOutputPath);
}

// advance the existing attribute id if it can be advanced
Expand All @@ -97,76 +90,120 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
return std::nullopt;
}

AttributeEntry entry = mDataModelProvider->NextAttribute(mOutputPath);
if (entry.IsValid())
auto id = session.attributes->Next();
if (id.has_value())
{
return entry.path.mAttributeId;
return id;
}

// Finished the data model, start with global attributes
static_assert(ArraySize(GlobalAttributesNotInMetadata) > 0);
return GlobalAttributesNotInMetadata[0];
}

std::optional<ClusterId> AttributePathExpandIterator::NextClusterId()
std::optional<ClusterId> AttributePathExpandIterator::NextClusterId(SearchSession & session)
{

if (mOutputPath.mClusterId == kInvalidClusterId)
{
if (mpAttributePath->mValue.HasWildcardClusterId())
if (!mpAttributePath->mValue.HasWildcardClusterId())
{
ClusterEntry entry = mDataModelProvider->FirstServerCluster(mOutputPath.mEndpointId);
return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt;
return mpAttributePath->mValue.mClusterId;
}

// only return a cluster if it is valid
const ConcreteClusterPath clusterPath(mOutputPath.mEndpointId, mpAttributePath->mValue.mClusterId);
if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value())
// restart iteration over the clusters of the current endpoint
session.clusters = mDataModelProvider->GetServerClusters(mOutputPath.mEndpointId);
}

VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardClusterId(), std::nullopt);

return session.clusters->Next();
}

AttributePathExpandIterator::SearchSession AttributePathExpandIterator::PrepareSearch()
{
SearchSession session;

session.endpoints = mDataModelProvider->GetEndpoints();

// position on the current endpoint to look up
if (mOutputPath.mEndpointId != kInvalidEndpointId)
{
// we are already positioned on a specific endpoint, so start from there
if (!session.endpoints->SeekTo(mOutputPath.mEndpointId))
{
return std::nullopt;
ChipLogError(InteractionModel, "Endpoint id " ChipLogFormatMEI " is not valid anymore",
ChipLogValueMEI(mOutputPath.mEndpointId));
}
}

return mpAttributePath->mValue.mClusterId;
if (mOutputPath.mEndpointId != kInvalidEndpointId)
{
session.clusters = mDataModelProvider->GetServerClusters(mOutputPath.mEndpointId);
if (mOutputPath.mClusterId != kInvalidClusterId)
{
// we are already positioned on a specific cluster, so start from there
if (!session.clusters->SeekTo(mOutputPath.mClusterId))
{
ChipLogError(InteractionModel,
"Cluster id " ChipLogFormatMEI " is not valid anymore (in endpoint " ChipLogFormatMEI ")",
ChipLogValueMEI(mOutputPath.mClusterId), ChipLogValueMEI(mOutputPath.mEndpointId));
}
}
}

VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardClusterId(), std::nullopt);
if ((mOutputPath.mEndpointId != kInvalidEndpointId) && (mOutputPath.mClusterId != kInvalidClusterId))
{
session.attributes = mDataModelProvider->GetAttributes(mOutputPath);
if (mOutputPath.mAttributeId != kInvalidAttributeId)
{
// we are already positioned on a specific cluster, so start from there
if (!session.attributes->SeekTo(mOutputPath.mAttributeId))
{
ChipLogError(InteractionModel,
"Attribute id " ChipLogFormatMEI " is not valid anymore (in " ChipLogFormatMEI "/" ChipLogFormatMEI
")",
ChipLogValueMEI(mOutputPath.mAttributeId), ChipLogValueMEI(mOutputPath.mEndpointId),
ChipLogValueMEI(mOutputPath.mClusterId));
}
}
}

ClusterEntry entry = mDataModelProvider->NextServerCluster(mOutputPath);
return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt;
return session;
}

std::optional<ClusterId> AttributePathExpandIterator::NextEndpointId()
std::optional<ClusterId> AttributePathExpandIterator::NextEndpointId(SearchSession & session)
{
if (mOutputPath.mEndpointId == kInvalidEndpointId)
{
if (mpAttributePath->mValue.HasWildcardEndpointId())
if (!mpAttributePath->mValue.HasWildcardEndpointId())
{
EndpointEntry ep = mDataModelProvider->FirstEndpoint();
return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
// use existing value, we are not wildcarding
return mpAttributePath->mValue.mEndpointId;
}

return mpAttributePath->mValue.mEndpointId;
// start a new iteration
session.endpoints = mDataModelProvider->GetEndpoints();
}

// to advance, we need to be asked for wildcards
VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardEndpointId(), std::nullopt);

EndpointEntry ep = mDataModelProvider->NextEndpoint(mOutputPath.mEndpointId);
return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
return session.endpoints->Next();
}

void AttributePathExpandIterator::ResetCurrentCluster()
void AttributePathExpandIterator::ResetCurrentCluster(SearchSession & session)
{
EnsureFirstNextCalled(session);

// If this is a null iterator, or the attribute id of current cluster info is not a wildcard attribute id, then this function
// will do nothing, since we won't be expanding the wildcard attribute ids under a cluster.
VerifyOrReturn(mpAttributePath != nullptr && mpAttributePath->mValue.HasWildcardAttributeId());

// Reset path expansion to ask for the first attribute of the current cluster
mOutputPath.mAttributeId = kInvalidAttributeId;
mOutputPath.mExpanded = true; // we know this is a wildcard attribute
Next();
Next(session);
}

bool AttributePathExpandIterator::AdvanceOutputPath()
bool AttributePathExpandIterator::AdvanceOutputPath(SearchSession & session)
{
if (!mpAttributePath->mValue.IsWildcardPath())
{
Expand All @@ -187,7 +224,7 @@ bool AttributePathExpandIterator::AdvanceOutputPath()
if (mOutputPath.mClusterId != kInvalidClusterId)
{

std::optional<AttributeId> nextAttribute = NextAttributeId();
std::optional<AttributeId> nextAttribute = NextAttributeId(session);
if (nextAttribute.has_value())
{
mOutputPath.mAttributeId = *nextAttribute;
Expand All @@ -198,7 +235,7 @@ bool AttributePathExpandIterator::AdvanceOutputPath()
// no valid attribute, try to advance the cluster, see if a suitable one exists
if (mOutputPath.mEndpointId != kInvalidEndpointId)
{
std::optional<ClusterId> nextCluster = NextClusterId();
std::optional<ClusterId> nextCluster = NextClusterId(session);
if (nextCluster.has_value())
{
mOutputPath.mClusterId = *nextCluster;
Expand All @@ -208,7 +245,7 @@ bool AttributePathExpandIterator::AdvanceOutputPath()
}

// no valid cluster, try advance the endpoint, see if a suitable on exists
std::optional<EndpointId> nextEndpoint = NextEndpointId();
std::optional<EndpointId> nextEndpoint = NextEndpointId(session);
if (nextEndpoint.has_value())
{
mOutputPath.mEndpointId = *nextEndpoint;
Expand All @@ -219,20 +256,23 @@ bool AttributePathExpandIterator::AdvanceOutputPath()
}
}

bool AttributePathExpandIterator::Next()
bool AttributePathExpandIterator::Next(SearchSession & session)
{
EnsureFirstNextCalled(session);
while (mpAttributePath != nullptr)
{
if (AdvanceOutputPath())
if (AdvanceOutputPath(session))
{
return true;
}
mpAttributePath = mpAttributePath->mpNext;
mOutputPath = ConcreteReadAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId);
mOutputPath.mExpanded = true; // this is reset to false on advancement if needed
mpAttributePath = mpAttributePath->mpNext;
mOutputPath = ConcreteReadAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId);
mOutputPath.mNeedsInitialization = false; // it is final
mOutputPath.mExpanded = true; // this is reset to false on advancement if needed
}

mOutputPath = ConcreteReadAttributePath();
mOutputPath = ConcreteReadAttributePath();
mOutputPath.mNeedsInitialization = false; // it is final
return false;
}

Expand Down
Loading
Loading