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

Separate out AttributePathExpandIterator::Position #36980

Merged
merged 62 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
222d5fc
Copied over the new AttributePathExpandIterator and will incrementall…
andreilitvin Jan 7, 2025
8edc026
Rename AttributePathExpandIterator to legacy
andreilitvin Jan 7, 2025
2d4e3ad
Prepare for using new style iterators ... checking NOT YET enabled th…
andreilitvin Jan 7, 2025
3b2c14d
Enabled checks ... and unit tests fail, but this now can be debugged
andreilitvin Jan 7, 2025
27f6fdc
Fix some of the underlying bugs: read handling logic assumes we are o…
andreilitvin Jan 7, 2025
a458140
Unit tests pass now
andreilitvin Jan 7, 2025
26a5b69
Restyle
andreilitvin Jan 7, 2025
b72fc9b
Use new iterator in IME
andreilitvin Jan 7, 2025
e764906
Update logic to use the new iterator on testRead
andreilitvin Jan 7, 2025
bb3be30
more updates
andreilitvin Jan 7, 2025
5b5a612
Restyle
andreilitvin Jan 7, 2025
c566826
Remove the legacy attribute path expand iterator
andreilitvin Jan 7, 2025
04d6626
Update naming
andreilitvin Jan 7, 2025
fef425f
Restyle
andreilitvin Jan 7, 2025
56b1307
Remove extra argument for ReadHandler constructor
andreilitvin Jan 7, 2025
bf8c4ba
Restyle
andreilitvin Jan 7, 2025
7d4475b
Slight flash improvement
andreilitvin Jan 7, 2025
652e389
Fix up includes
andreilitvin Jan 7, 2025
4fa0fc8
Removed empty line
andreilitvin Jan 7, 2025
4a4ec3e
added comment on why state is a friend class
andreilitvin Jan 7, 2025
bc52596
Comment updates
andreilitvin Jan 7, 2025
93079a2
Restyle, add some comments and add extra checks on validity check onl…
andreilitvin Jan 7, 2025
c05d9f9
Merge branch 'master' into attribute-path-expand-state
andy31415 Jan 8, 2025
75bb1c1
Remove an include
andy31415 Jan 8, 2025
851928a
Comment updates, renamed mLastOutputPath to mOutputPath
andy31415 Jan 8, 2025
5024a4a
Fix one typo
andy31415 Jan 8, 2025
6266f96
Re-arrange members of ReadHandler to optimize for memory layout. This…
andy31415 Jan 8, 2025
a080dcf
Restyle
andy31415 Jan 8, 2025
b9a5d1d
Rename State to Position
andy31415 Jan 8, 2025
6fe8461
One more rename
andy31415 Jan 8, 2025
255cffe
Remove redundant assigment ...we are at a net 0 txt increase now on qpg
andy31415 Jan 8, 2025
ed950cf
Add more unit tests for non-obvious requirement that wildcard expansi…
andy31415 Jan 8, 2025
2e2ebd3
Update src/app/AttributePathExpandIterator.cpp
andy31415 Jan 8, 2025
130c1f6
Update src/app/AttributePathExpandIterator.h
andy31415 Jan 8, 2025
a610782
Update src/app/AttributePathExpandIterator.h
andy31415 Jan 8, 2025
e30ae74
Update src/app/AttributePathExpandIterator.h
andy31415 Jan 8, 2025
f9cd8f5
Update src/app/ReadHandler.h
andy31415 Jan 8, 2025
aeecc5c
Update src/app/ReadHandler.cpp
andy31415 Jan 8, 2025
57198a6
Update src/app/AttributePathExpandIterator.h
andy31415 Jan 8, 2025
75173d3
Use different values for the cluster ids for testing
andy31415 Jan 8, 2025
0bbdee6
Merge branch 'attribute-path-expand-state' of github.com:andy31415/co…
andy31415 Jan 8, 2025
04d27ba
One more state to position change
andy31415 Jan 8, 2025
75564bf
mExpanded is now set during output path returning. Removed 2 more set…
andy31415 Jan 8, 2025
f2470b6
Merge branch 'master' into attribute-path-expand-state
andreilitvin Jan 9, 2025
91e8547
Remove some tests that seem redundant, keep only one
andreilitvin Jan 9, 2025
f96d388
Update src/app/AttributePathExpandIterator.cpp
andy31415 Jan 10, 2025
5962c1c
Update src/app/AttributePathExpandIterator.cpp
andy31415 Jan 10, 2025
3b6557d
Update src/app/AttributePathExpandIterator.cpp
andy31415 Jan 10, 2025
c2e23a2
Update src/app/AttributePathExpandIterator.cpp
andy31415 Jan 10, 2025
740d1c7
Update src/app/InteractionModelEngine.cpp
andy31415 Jan 10, 2025
321bfc3
Update src/app/ReadHandler.h
andy31415 Jan 10, 2025
a131e01
Update src/app/AttributePathExpandIterator.h
andy31415 Jan 10, 2025
78bdf32
Update src/app/ReadHandler.h
andy31415 Jan 10, 2025
1c0cf18
Use mCompletePosition
andy31415 Jan 10, 2025
efa903f
Another rename
andy31415 Jan 10, 2025
5310cf8
Merge branch 'master' into attribute-path-expand-state
andy31415 Jan 10, 2025
b06a268
Undo submodule update
andy31415 Jan 10, 2025
917d4a8
Restyle
andy31415 Jan 10, 2025
a4f3d29
Update comment text to not sound like graph parsing
andy31415 Jan 10, 2025
e6b8003
Rename method to be more descriptive
andy31415 Jan 10, 2025
9de15ec
Update peek attribute iterator to rollback and update code logic a bi…
andy31415 Jan 10, 2025
13eef77
Merge branch 'master' into attribute-path-expand-state
andy31415 Jan 10, 2025
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
235 changes: 114 additions & 121 deletions src/app/AttributePathExpandIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,86 @@
#include <app/GlobalAttributes.h>
#include <lib/support/CodeUtils.h>

#include <optional>

using namespace chip::app::DataModel;

namespace chip {
namespace app {

AttributePathExpandIterator::AttributePathExpandIterator(DataModel::Provider * provider,
SingleLinkedListNode<AttributePathParams> * attributePath) :
mDataModelProvider(provider),
mpAttributePath(attributePath), mOutputPath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId)
bool AttributePathExpandIterator::AdvanceOutputPath()
{
/// Output path invariants
/// - kInvalid* constants are used to define "no value available (yet)" and
/// iteration loop will fill the first value when such a value is seen (fixed for non-wildcard
/// or iteration-based in case of wildcards).
/// - Iteration of the output path is done in order: first endpoint, then cluster, then attribute.
/// Processing works like:
/// - Initial state is kInvalidEndpointId/kInvalidClusterId/kInvalidAttributeId
/// - First loop pass fills-in endpointID, followed by clusterID, followed by attributeID
/// - Whenever one level is done iterating (there is no "next") the following
/// "higher path component" is updated:
/// - once a valid path exists, try to advance attributeID
/// - if attributeID fails to advance, try to advance clusterID (and restart attributeID)
/// - if clusterID fails to advance, try to advance endpointID (and restart clusterID)
/// - if endpointID fails to advance, iteration is done
while (true)
{
if (mPosition.mOutputPath.mClusterId != kInvalidClusterId)
{
std::optional<AttributeId> nextAttribute = NextAttributeId();
if (nextAttribute.has_value())
{
mPosition.mOutputPath.mAttributeId = *nextAttribute;
mPosition.mOutputPath.mExpanded = mPosition.mAttributePath->mValue.IsWildcardPath();
return true;
}
}

// no valid attribute, try to advance the cluster, see if a suitable one exists
if (mPosition.mOutputPath.mEndpointId != kInvalidEndpointId)
{
std::optional<ClusterId> nextCluster = NextClusterId();
if (nextCluster.has_value())
{
// A new cluster ID is to be processed. This sets the cluster ID to the new value and
// ALSO resets the attribute ID to "invalid", to trigger an attribute set/expansion from
// the beginning.
mPosition.mOutputPath.mClusterId = *nextCluster;
mPosition.mOutputPath.mAttributeId = kInvalidAttributeId;
continue;
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
}
}

// No valid cluster, try advance the endpoint, see if a suitable one exists.
std::optional<EndpointId> nextEndpoint = NextEndpointId();
if (nextEndpoint.has_value())
{
// A new endpoint ID is to be processed. This sets the endpoint ID to the new value and
// ALSO resets the cluster ID to "invalid", to trigger a cluster set/expansion from
// the beginning.
mPosition.mOutputPath.mEndpointId = *nextEndpoint;
mPosition.mOutputPath.mClusterId = kInvalidClusterId;
continue;
}
return false;
}
}

bool AttributePathExpandIterator::Next(ConcreteAttributePath & path)
{
mOutputPath.mExpanded = true; // this is reset in 'next' if needed
while (mPosition.mAttributePath != nullptr)
{
if (AdvanceOutputPath())
{
path = mPosition.mOutputPath;
return true;
}
mPosition.mAttributePath = mPosition.mAttributePath->mpNext;
mPosition.mOutputPath = ConcreteReadAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId);
}

// Make the iterator ready to emit the first valid path in the list.
// TODO: the bool return value here is completely unchecked
Next();
return false;
}

bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId)
Expand All @@ -49,40 +113,43 @@ bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId)
break;
}

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

std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
{
if (mOutputPath.mAttributeId == kInvalidAttributeId)
if (mPosition.mOutputPath.mAttributeId == kInvalidAttributeId)
{
if (mpAttributePath->mValue.HasWildcardAttributeId())
if (mPosition.mAttributePath->mValue.HasWildcardAttributeId())
{
AttributeEntry entry = mDataModelProvider->FirstAttribute(mOutputPath);
AttributeEntry entry = mDataModelProvider->FirstAttribute(mPosition.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))
// At this point, the attributeID is NOT a wildcard (i.e. it is fixed).
//
// For wildcard expansion, we validate that this is a valid attribute for the given
// cluster on the given endpoint. If not a wildcard expansion, return it as-is.
if (mPosition.mAttributePath->mValue.IsWildcardPath())
{
return mpAttributePath->mValue.mAttributeId;
if (!IsValidAttributeId(mPosition.mAttributePath->mValue.mAttributeId))
{
return std::nullopt;
}
}

return std::nullopt;
return mPosition.mAttributePath->mValue.mAttributeId;
}

// advance the existing attribute id if it can be advanced
VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardAttributeId(), std::nullopt);
// Advance the existing attribute id if it can be advanced.
VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardAttributeId(), std::nullopt);

// Ensure (including ordering) that GlobalAttributesNotInMetadata is reported as needed
for (unsigned i = 0; i < ArraySize(GlobalAttributesNotInMetadata); i++)
{
if (GlobalAttributesNotInMetadata[i] != mOutputPath.mAttributeId)
if (GlobalAttributesNotInMetadata[i] != mPosition.mOutputPath.mAttributeId)
{
continue;
}
Expand All @@ -93,11 +160,12 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
return GlobalAttributesNotInMetadata[nextAttributeIndex];
}

// reached the end of global attributes
// Reached the end of global attributes. Since global attributes are
// reported last, finishing global attributes means everything completed.
return std::nullopt;
}

AttributeEntry entry = mDataModelProvider->NextAttribute(mOutputPath);
AttributeEntry entry = mDataModelProvider->NextAttribute(mPosition.mOutputPath);
if (entry.IsValid())
{
return entry.path.mAttributeId;
Expand All @@ -111,130 +179,55 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
std::optional<ClusterId> AttributePathExpandIterator::NextClusterId()
{

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

// only return a cluster if it is valid
const ConcreteClusterPath clusterPath(mOutputPath.mEndpointId, mpAttributePath->mValue.mClusterId);
if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value())
// At this point, the clusterID is NOT a wildcard (i.e. is fixed).
//
// For wildcard expansion, we validate that this is a valid cluster for the endpoint.
// If non-wildcard expansion, we return as-is.
if (mPosition.mAttributePath->mValue.IsWildcardPath())
{
return std::nullopt;
const ConcreteClusterPath clusterPath(mPosition.mOutputPath.mEndpointId, mPosition.mAttributePath->mValue.mClusterId);
if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value())
{
return std::nullopt;
}
}

return mpAttributePath->mValue.mClusterId;
return mPosition.mAttributePath->mValue.mClusterId;
}

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

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

std::optional<ClusterId> AttributePathExpandIterator::NextEndpointId()
{
if (mOutputPath.mEndpointId == kInvalidEndpointId)
if (mPosition.mOutputPath.mEndpointId == kInvalidEndpointId)
{
if (mpAttributePath->mValue.HasWildcardEndpointId())
if (mPosition.mAttributePath->mValue.HasWildcardEndpointId())
{
EndpointEntry ep = mDataModelProvider->FirstEndpoint();
return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
}

return mpAttributePath->mValue.mEndpointId;
return mPosition.mAttributePath->mValue.mEndpointId;
}

VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardEndpointId(), std::nullopt);
// Expand endpoints only if it is a wildcard on the endpoint specifically.
VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardEndpointId(), std::nullopt);

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

void AttributePathExpandIterator::ResetCurrentCluster()
{
// 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();
}

bool AttributePathExpandIterator::AdvanceOutputPath()
{
if (!mpAttributePath->mValue.IsWildcardPath())
{
if (mOutputPath.mEndpointId != kInvalidEndpointId)
{
return false; // cannot expand non-wildcard path
}

mOutputPath.mEndpointId = mpAttributePath->mValue.mEndpointId;
mOutputPath.mClusterId = mpAttributePath->mValue.mClusterId;
mOutputPath.mAttributeId = mpAttributePath->mValue.mAttributeId;
mOutputPath.mExpanded = false;
return true;
}

while (true)
{
if (mOutputPath.mClusterId != kInvalidClusterId)
{

std::optional<AttributeId> nextAttribute = NextAttributeId();
if (nextAttribute.has_value())
{
mOutputPath.mAttributeId = *nextAttribute;
return true;
}
}

// no valid attribute, try to advance the cluster, see if a suitable one exists
if (mOutputPath.mEndpointId != kInvalidEndpointId)
{
std::optional<ClusterId> nextCluster = NextClusterId();
if (nextCluster.has_value())
{
mOutputPath.mClusterId = *nextCluster;
mOutputPath.mAttributeId = kInvalidAttributeId; // restarts attributes
continue;
}
}

// no valid cluster, try advance the endpoint, see if a suitable on exists
std::optional<EndpointId> nextEndpoint = NextEndpointId();
if (nextEndpoint.has_value())
{
mOutputPath.mEndpointId = *nextEndpoint;
mOutputPath.mClusterId = kInvalidClusterId; // restarts clusters
continue;
}
return false;
}
}

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

mOutputPath = ConcreteReadAttributePath();
return false;
}

} // namespace app
} // namespace chip
Loading
Loading