diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index 1308d1550c0213..fbad73d4d89993 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -31,9 +31,9 @@ using namespace chip; -// TODO: Here we use forward declaration for these symbols used, there should be some reorganize for code in app/util so they can be -// used with generated files or some mock files. -// Note: including headers from app/util does not work since it will include some app specific generged files. +// TODO: Need to make it so that declarations of things that don't depend on generated files are not intermixed in af.h with +// dependencies on generated files, so we don't have to re-declare things here. +// Note: Some of the generated files that depended by af.h are gen_config.h and gen_tokens.h typedef uint8_t EmberAfClusterMask; extern uint16_t emberAfEndpointCount(void); @@ -68,15 +68,15 @@ void AttributePathExpandIterator::PrepareEndpointIndexRange(const ClusterInfo & { if (aClusterInfo.HasWildcardEndpointId()) { - mBeginEndpointIndex = 0; - mEndEndpointIndex = emberAfEndpointCount(); + mEndpointIndex = 0; + mEndEndpointIndex = emberAfEndpointCount(); } else { - mBeginEndpointIndex = emberAfIndexFromEndpoint(aClusterInfo.mEndpointId); + mEndpointIndex = emberAfIndexFromEndpoint(aClusterInfo.mEndpointId); // If the given cluster id does not exist on the given endpoint, it will return uint16(0xFFFF), then endEndpointIndex // will be 0, means we should iterate a null endpoint set (skip it). - mEndEndpointIndex = static_cast(mBeginEndpointIndex + 1); + mEndEndpointIndex = static_cast(mEndpointIndex + 1); } } @@ -84,15 +84,15 @@ void AttributePathExpandIterator::PrepareClusterIndexRange(const ClusterInfo & a { if (aClusterInfo.HasWildcardClusterId()) { - mBeginClusterIndex = 0; - mEndClusterIndex = emberAfClusterCount(aEndpointId, true /* server */); + mClusterIndex = 0; + mEndClusterIndex = emberAfClusterCount(aEndpointId, true /* server */); } else { - mBeginClusterIndex = emberAfClusterIndex(aEndpointId, aClusterInfo.mClusterId, CLUSTER_MASK_SERVER); + mClusterIndex = emberAfClusterIndex(aEndpointId, aClusterInfo.mClusterId, CLUSTER_MASK_SERVER); // If the given cluster id does not exist on the given endpoint, it will return uint8(0xFF), then endClusterIndex // will be 0, means we should iterate a null cluster set (skip it). - mEndClusterIndex = static_cast(mBeginClusterIndex + 1); + mEndClusterIndex = static_cast(mClusterIndex + 1); } } @@ -101,15 +101,15 @@ void AttributePathExpandIterator::PrepareAttributeIndexRange(const ClusterInfo & { if (aClusterInfo.HasWildcardAttributeId()) { - mBeginAttributeIndex = 0; - mEndAttributeIndex = emberAfGetServerAttributeCount(aEndpointId, aClusterId); + mAttributeIndex = 0; + mEndAttributeIndex = emberAfGetServerAttributeCount(aEndpointId, aClusterId); } else { - mBeginAttributeIndex = emberAfGetServerAttributeIndexByAttributeId(aEndpointId, aClusterId, aClusterInfo.mAttributeId); + mAttributeIndex = emberAfGetServerAttributeIndexByAttributeId(aEndpointId, aClusterId, aClusterInfo.mAttributeId); // If the given attribute id does not exist on the given endpoint, it will return uint16(0xFFFF), then endAttributeIndex // will be 0, means we should iterate a null attribute set (skip it). - mEndAttributeIndex = static_cast(mBeginAttributeIndex + 1); + mEndAttributeIndex = static_cast(mAttributeIndex + 1); } } @@ -117,24 +117,21 @@ bool AttributePathExpandIterator::Next() { for (; mpClusterInfo != nullptr; (mpClusterInfo = mpClusterInfo->mpNext, mEndpointIndex = UINT16_MAX)) { - // Special case: If this is a concrete path, we just return its value as-is. - if (!mpClusterInfo->HasWildcard()) + if (mEndpointIndex == UINT16_MAX) { - mOutputPath.mEndpointId = mpClusterInfo->mEndpointId; - mOutputPath.mClusterId = mpClusterInfo->mClusterId; - mOutputPath.mAttributeId = mpClusterInfo->mAttributeId; + // Special case: If this is a concrete path, we just return its value as-is. + if (!mpClusterInfo->HasWildcard()) + { + mOutputPath.mEndpointId = mpClusterInfo->mEndpointId; + mOutputPath.mClusterId = mpClusterInfo->mClusterId; + mOutputPath.mAttributeId = mpClusterInfo->mAttributeId; - // Prepare for next iteration - (mpClusterInfo = mpClusterInfo->mpNext, mEndpointIndex = UINT16_MAX); - return true; - } + // Prepare for next iteration + mEndpointIndex = mEndEndpointIndex = 0; + } - if (mEndpointIndex == UINT16_MAX) - { PrepareEndpointIndexRange(*mpClusterInfo); - // If we have not started iterating over the endpoints yet. - mEndpointIndex = mBeginEndpointIndex; - mClusterIndex = UINT8_MAX; + mClusterIndex = UINT8_MAX; } for (; mEndpointIndex < mEndEndpointIndex; (mEndpointIndex++, mClusterIndex = UINT8_MAX, mAttributeIndex = UINT16_MAX)) @@ -144,8 +141,6 @@ bool AttributePathExpandIterator::Next() if (mClusterIndex == UINT8_MAX) { PrepareClusterIndexRange(*mpClusterInfo, endpointId); - // If we have not started iterating over the clusters yet. - mClusterIndex = mBeginClusterIndex; mAttributeIndex = UINT16_MAX; } @@ -157,8 +152,6 @@ bool AttributePathExpandIterator::Next() if (mAttributeIndex == UINT16_MAX) { PrepareAttributeIndexRange(*mpClusterInfo, endpointId, clusterId); - // If we have not started iterating over the attributes yet. - mAttributeIndex = mBeginAttributeIndex; } if (mAttributeIndex < mEndAttributeIndex) diff --git a/src/app/AttributePathExpandIterator.h b/src/app/AttributePathExpandIterator.h index f939acb1751936..090928b78f7ba6 100644 --- a/src/app/AttributePathExpandIterator.h +++ b/src/app/AttributePathExpandIterator.h @@ -46,7 +46,7 @@ namespace app { * AttributePathExpandIterator is used to iterate over a linked list of ClusterInfo-s. * The AttributePathExpandIterator is copiable, however, the given cluster info must be valid when calling Next(). * - * AttributePathExpandIterator will expand attribute paths with wildcards, and only emit exist paths for ClusterInfo with + * AttributePathExpandIterator will expand attribute paths with wildcards, and only emit existing paths for ClusterInfo with * wildcards. For ClusterInfo with a concrete path (i.e. does not contain wildcards), AttributePathExpandIterator will emit them * as-is. * @@ -55,7 +55,7 @@ namespace app { * for (AttributePathExpandIterator iterator(clusterInfo); iterator.Get(path); iterator.Next()) {...} * * The iterator does not copy the given ClusterInfo, The given ClusterInfo must be valid when using the iterator. - * If there are new endpoints, clusters or attributes adds, AttributePathExpandIterator must be reinitialized. + * If the set of endpoints, clusters, or attributes that are supported changes, AttributePathExpandIterator must be reinitialized. * * A initialized iterator will return the first valid path, no need to call Next() before calling Get() for the first time. * @@ -74,7 +74,7 @@ class AttributePathExpandIterator /** * Proceed the iterator to the next attribute path in the given cluster info. * - * Feturns false if AttributePathExpandIterator has exhausted all paths in the given ClusterInfo list. + * Returns false if AttributePathExpandIterator has exhausted all paths in the given ClusterInfo list. */ bool Next(); @@ -93,20 +93,15 @@ class AttributePathExpandIterator * - Next() is called after iterating last path. * - Iterator is initialized wirh a null ClusterInfo. */ - inline bool Valid() const - { - // Check if we can emitting a valid path, we are not checking mpClusterInfo since we may just visited a ClusterInfo with - // concrete attribute path, and it is the last one in the linked list. - return mOutputPath.mEndpointId != kInvalidEndpointId; - } + inline bool Valid() const { return mpClusterInfo != nullptr; } private: ClusterInfo * mpClusterInfo; - uint16_t mEndpointIndex, mBeginEndpointIndex, mEndEndpointIndex; + uint16_t mEndpointIndex, mEndEndpointIndex; // Note: should use decltype(EmberAfEndpointType::clusterCount) here, but af-types is including app specific generated files. - uint8_t mClusterIndex, mBeginClusterIndex, mEndClusterIndex; - uint16_t mAttributeIndex, mBeginAttributeIndex, mEndAttributeIndex; + uint8_t mClusterIndex, mEndClusterIndex; + uint16_t mAttributeIndex, mEndAttributeIndex; ConcreteAttributePath mOutputPath; diff --git a/src/app/ClusterInfo.h b/src/app/ClusterInfo.h index f3281ea08040d5..c07d3491623b9f 100644 --- a/src/app/ClusterInfo.h +++ b/src/app/ClusterInfo.h @@ -37,8 +37,6 @@ struct ClusterInfo private: // Allow AttributePathParams access these constants. friend struct AttributePathParams; - // Allow ConcreteAttributePath access these constants. - friend struct ConcreteAttributePath; // The ClusterId, AttributeId and EventId are MEIs, // 0xFFFF is not a valid manufacturer code, thus 0xFFFF'FFFF is not a valid MEI diff --git a/src/app/ConcreteAttributePath.h b/src/app/ConcreteAttributePath.h index 00af507a875a4a..cd81175834d5dc 100644 --- a/src/app/ConcreteAttributePath.h +++ b/src/app/ConcreteAttributePath.h @@ -18,7 +18,6 @@ #pragma once -#include #include namespace chip { namespace app { @@ -38,9 +37,9 @@ struct ConcreteAttributePath return mEndpointId == other.mEndpointId && mClusterId == other.mClusterId && mAttributeId == other.mAttributeId; } - EndpointId mEndpointId = kInvalidEndpointId; - ClusterId mClusterId = ClusterInfo::kInvalidClusterId; - AttributeId mAttributeId = ClusterInfo::kInvalidAttributeId; + EndpointId mEndpointId = 0; + ClusterId mClusterId = 0; + AttributeId mAttributeId = 0; }; } // namespace app } // namespace chip diff --git a/src/app/tests/TestAttributePathExpandIterator.cpp b/src/app/tests/TestAttributePathExpandIterator.cpp index a6b3e3cece4924..b13457c6981e7d 100644 --- a/src/app/tests/TestAttributePathExpandIterator.cpp +++ b/src/app/tests/TestAttributePathExpandIterator.cpp @@ -83,7 +83,7 @@ void TestAllWildcard(nlTestSuite * apSuite, void * apContext) { ChipLogDetail(AppServer, "Visited Attribute: 0x%04" PRIX16 " / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); - // NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); + NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); index++; } NL_TEST_ASSERT(apSuite, index == ArraySize(paths)); @@ -106,7 +106,7 @@ void TestWildcardEndpoint(nlTestSuite * apSuite, void * apContext) { ChipLogDetail(AppServer, "Visited Attribute: 0x%04" PRIX16 " / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); - // NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); + NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); index++; } NL_TEST_ASSERT(apSuite, index == ArraySize(paths)); @@ -132,7 +132,7 @@ void TestWildcardCluster(nlTestSuite * apSuite, void * apContext) { ChipLogDetail(AppServer, "Visited Attribute: 0x%04" PRIX16 " / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); - // NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); + NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); index++; } NL_TEST_ASSERT(apSuite, index == ArraySize(paths)); @@ -159,7 +159,7 @@ void TestWildcardAttribute(nlTestSuite * apSuite, void * apContext) { ChipLogDetail(AppServer, "Visited Attribute: 0x%04" PRIX16 " / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); - // NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); + NL_TEST_ASSERT(apSuite, index < ArraySize(paths) && paths[index] == path); index++; } NL_TEST_ASSERT(apSuite, index == ArraySize(paths));