From 82680ba93bc3b6dc80858e81dbc22455071f459e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 14 Feb 2022 21:56:25 -0500 Subject: [PATCH] Fix wildcard handling of global attributes not present in metadata. (#15013) * Fix wildcard handling of global attributes not present in metadata. Wildcard expansion was not including AttributeList, ServerGeneratedCommandList, ClientGeneratedCommandList. Also, AttributeList was not including ServerGeneratedCommandList and ClientGeneratedCommandList. Fixes https://github.com/project-chip/connectedhomeip/issues/14063 * Address review comments --- .github/workflows/build.yaml | 2 +- src/app/AttributePathExpandIterator.cpp | 51 +++++++++-- src/app/AttributePathExpandIterator.h | 3 + src/app/BUILD.gn | 1 + src/app/GlobalAttributes.h | 39 ++++++++ .../tests/TestAttributePathExpandIterator.cpp | 88 +++++++++++++++++++ .../tests/suites/TestBasicInformation.yaml | 9 +- .../util/ember-compatibility-functions.cpp | 35 +++++--- src/controller/tests/TestReadChunking.cpp | 44 ++++++++-- .../Framework/CHIPTests/CHIPClustersTests.m | 8 +- src/lib/support/CodeUtils.h | 31 +++++++ .../chip-tool/zap-generated/test/Commands.h | 10 ++- 12 files changed, 287 insertions(+), 34 deletions(-) create mode 100644 src/app/GlobalAttributes.h diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 43372ee113de26..9c28f3948f81f4 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -70,7 +70,7 @@ jobs: timeout-minutes: 20 run: scripts/run_in_build_env.sh "ninja -C ./out" - name: Run Tests - timeout-minutes: 2 + timeout-minutes: 5 run: scripts/tests/gn_tests.sh # TODO Log Upload https://github.com/project-chip/connectedhomeip/issues/2227 # TODO https://github.com/project-chip/connectedhomeip/issues/1512 diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index 80920627cacf71..0d893c5886a98a 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,12 @@ AttributePathExpandIterator::AttributePathExpandIterator(ClusterInfo * aClusterI mClusterIndex = UINT8_MAX; mAttributeIndex = UINT16_MAX; + static_assert(std::numeric_limits::max() >= ArraySize(GlobalAttributesNotInMetadata), + "Our index won't be able to hold the value we need to hold."); + static_assert(std::is_same::value, + "If this changes audit all uses where we set to UINT8_MAX"); + mGlobalAttributeIndex = UINT8_MAX; + // Make the iterator ready to emit the first valid path in the list. Next(); } @@ -101,8 +108,10 @@ void AttributePathExpandIterator::PrepareAttributeIndexRange(const ClusterInfo & { if (aClusterInfo.HasWildcardAttributeId()) { - mAttributeIndex = 0; - mEndAttributeIndex = emberAfGetServerAttributeCount(aEndpointId, aClusterId); + mAttributeIndex = 0; + mEndAttributeIndex = emberAfGetServerAttributeCount(aEndpointId, aClusterId); + mGlobalAttributeIndex = 0; + mGlobalAttributeEndIndex = ArraySize(GlobalAttributesNotInMetadata); } else { @@ -110,6 +119,24 @@ void AttributePathExpandIterator::PrepareAttributeIndexRange(const ClusterInfo & // 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(mAttributeIndex + 1); + if (mAttributeIndex == UINT16_MAX) + { + // Check whether this is a non-metadata global attribute. + // + // Default to the max value, which will correspond (after we add 1 + // and overflow to 0 for the max index) to us not going through + // non-metadata global attributes for this attribute. + mGlobalAttributeIndex = UINT8_MAX; + for (uint8_t idx = 0; idx < ArraySize(GlobalAttributesNotInMetadata); ++idx) + { + if (GlobalAttributesNotInMetadata[idx] == aClusterInfo.mAttributeId) + { + mGlobalAttributeIndex = idx; + break; + } + } + mGlobalAttributeEndIndex = static_cast(mGlobalAttributeIndex + 1); + } } } @@ -137,7 +164,8 @@ bool AttributePathExpandIterator::Next() mClusterIndex = UINT8_MAX; } - for (; mEndpointIndex < mEndEndpointIndex; (mEndpointIndex++, mClusterIndex = UINT8_MAX, mAttributeIndex = UINT16_MAX)) + for (; mEndpointIndex < mEndEndpointIndex; + (mEndpointIndex++, mClusterIndex = UINT8_MAX, mAttributeIndex = UINT16_MAX, mGlobalAttributeIndex = UINT8_MAX)) { if (!emberAfEndpointIndexIsEnabled(mEndpointIndex)) { @@ -150,15 +178,17 @@ bool AttributePathExpandIterator::Next() if (mClusterIndex == UINT8_MAX) { PrepareClusterIndexRange(*mpClusterInfo, endpointId); - mAttributeIndex = UINT16_MAX; + mAttributeIndex = UINT16_MAX; + mGlobalAttributeIndex = UINT8_MAX; } - for (; mClusterIndex < mEndClusterIndex; (mClusterIndex++, mAttributeIndex = UINT16_MAX)) + for (; mClusterIndex < mEndClusterIndex; + (mClusterIndex++, mAttributeIndex = UINT16_MAX, mGlobalAttributeIndex = UINT8_MAX)) { // emberAfGetNthClusterId must return a valid cluster id here since we have verified the mClusterIndex does // not exceed the mEndClusterIndex. ClusterId clusterId = emberAfGetNthClusterId(endpointId, mClusterIndex, true /* server */).Value(); - if (mAttributeIndex == UINT16_MAX) + if (mAttributeIndex == UINT16_MAX && mGlobalAttributeIndex == UINT8_MAX) { PrepareAttributeIndexRange(*mpClusterInfo, endpointId, clusterId); } @@ -175,6 +205,15 @@ bool AttributePathExpandIterator::Next() // Return true will skip the increment of mClusterIndex, mEndpointIndex and mpClusterInfo. return true; } + else if (mGlobalAttributeIndex < mGlobalAttributeEndIndex) + { + // Return a path pointing to the next global attribute. + mOutputPath.mAttributeId = GlobalAttributesNotInMetadata[mGlobalAttributeIndex]; + mOutputPath.mClusterId = clusterId; + mOutputPath.mEndpointId = endpointId; + mGlobalAttributeIndex++; + return true; + } // We have exhausted all attributes of this cluster, continue iterating over attributes of next cluster. } // We have exhausted all clusters of this endpoint, continue iterating over clusters of next endpoint. diff --git a/src/app/AttributePathExpandIterator.h b/src/app/AttributePathExpandIterator.h index 00cbda62dd6412..75463aeb823ad4 100644 --- a/src/app/AttributePathExpandIterator.h +++ b/src/app/AttributePathExpandIterator.h @@ -101,6 +101,9 @@ class AttributePathExpandIterator // Note: should use decltype(EmberAfEndpointType::clusterCount) here, but af-types is including app specific generated files. uint8_t mClusterIndex, mEndClusterIndex; uint16_t mAttributeIndex, mEndAttributeIndex; + // For dealing with global attributes that are not part of the attribute + // metadata. + uint8_t mGlobalAttributeIndex, mGlobalAttributeEndIndex; ConcreteAttributePath mOutputPath; diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 04bc6b105ebd5a..ecaae7a7db0e27 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -58,6 +58,7 @@ static_library("app") { "DeviceProxy.h", "EventManagement.cpp", "EventPathParams.h", + "GlobalAttributes.h", "InteractionModelEngine.cpp", "InteractionModelRevision.h", "InteractionModelTimeout.h", diff --git a/src/app/GlobalAttributes.h b/src/app/GlobalAttributes.h new file mode 100644 index 00000000000000..8276528599cbdc --- /dev/null +++ b/src/app/GlobalAttributes.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2022 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace chip { +namespace app { + +/** + * List of attribute ids of attributes that appear on every cluster and have + * values that are always produced via code, hence do not appear in attribute + * metadata to save space. These _must_ appear in order. + */ +constexpr AttributeId GlobalAttributesNotInMetadata[] = { + Clusters::Globals::Attributes::ServerGeneratedCommandList::Id, + Clusters::Globals::Attributes::ClientGeneratedCommandList::Id, + Clusters::Globals::Attributes::AttributeList::Id, +}; + +static_assert(ArrayIsSorted(GlobalAttributesNotInMetadata), "Array of global attribute ids must be sorted"); + +} // namespace app +} // namespace chip diff --git a/src/app/tests/TestAttributePathExpandIterator.cpp b/src/app/tests/TestAttributePathExpandIterator.cpp index 1cf20238a37583..570c42cd09a78b 100644 --- a/src/app/tests/TestAttributePathExpandIterator.cpp +++ b/src/app/tests/TestAttributePathExpandIterator.cpp @@ -47,33 +47,60 @@ void TestAllWildcard(nlTestSuite * apSuite, void * apContext) P paths[] = { { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint1, MockClusterId(2), MockAttributeId(1) }, + { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint2, MockClusterId(2), MockAttributeId(1) }, { kMockEndpoint2, MockClusterId(2), MockAttributeId(2) }, + { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(1) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(2) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint3, MockClusterId(1), MockAttributeId(1) }, + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(1) }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(2) }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(3) }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(4) }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::AttributeList::Id }, }; size_t index = 0; @@ -137,6 +164,32 @@ void TestWildcardCluster(nlTestSuite * apSuite, void * apContext) NL_TEST_ASSERT(apSuite, index == ArraySize(paths)); } +void TestWildcardClusterGlobalAttributeNotInMetadata(nlTestSuite * apSuite, void * apContext) +{ + app::ClusterInfo clusInfo; + clusInfo.mEndpointId = Test::kMockEndpoint3; + clusInfo.mAttributeId = app::Clusters::Globals::Attributes::AttributeList::Id; + + app::ConcreteAttributePath path; + P paths[] = { + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::AttributeList::Id }, + }; + + size_t index = 0; + + for (app::AttributePathExpandIterator iter(&clusInfo); iter.Get(path); iter.Next()) + { + 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); + index++; + } + NL_TEST_ASSERT(apSuite, index == ArraySize(paths)); +} + void TestWildcardAttribute(nlTestSuite * apSuite, void * apContext) { app::ClusterInfo clusInfo; @@ -150,6 +203,9 @@ void TestWildcardAttribute(nlTestSuite * apSuite, void * apContext) { kMockEndpoint2, MockClusterId(3), MockAttributeId(1) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(2) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, }; size_t index = 0; @@ -219,33 +275,60 @@ void TestMultipleClusInfo(nlTestSuite * apSuite, void * apContext) P paths[] = { { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint1, MockClusterId(2), MockAttributeId(1) }, + { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint1, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint2, MockClusterId(2), MockAttributeId(1) }, { kMockEndpoint2, MockClusterId(2), MockAttributeId(2) }, + { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(1) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(2) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint3, MockClusterId(1), MockAttributeId(1) }, + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::FeatureMap::Id }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(1) }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(2) }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(3) }, { kMockEndpoint3, MockClusterId(2), MockAttributeId(4) }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::FeatureMap::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint3, MockClusterId(4), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, { kMockEndpoint3, MockClusterId(1), Clusters::Globals::Attributes::ClusterRevision::Id }, { kMockEndpoint3, MockClusterId(2), Clusters::Globals::Attributes::ClusterRevision::Id }, @@ -256,6 +339,9 @@ void TestMultipleClusInfo(nlTestSuite * apSuite, void * apContext) { kMockEndpoint2, MockClusterId(3), MockAttributeId(1) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(2) }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ServerGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::ClientGeneratedCommandList::Id }, + { kMockEndpoint2, MockClusterId(3), Clusters::Globals::Attributes::AttributeList::Id }, { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, }; @@ -294,6 +380,8 @@ const nlTest sTests[] = NL_TEST_DEF("TestAllWildcard", TestAllWildcard), NL_TEST_DEF("TestWildcardEndpoint", TestWildcardEndpoint), NL_TEST_DEF("TestWildcardCluster", TestWildcardCluster), + NL_TEST_DEF("TestWildcardClusterGlobalAttributeNotInMetadata", + TestWildcardClusterGlobalAttributeNotInMetadata), NL_TEST_DEF("TestWildcardAttribute", TestWildcardAttribute), NL_TEST_DEF("TestNoWildcard", TestNoWildcard), NL_TEST_DEF("TestMultipleClusInfo", TestMultipleClusInfo), diff --git a/src/app/tests/suites/TestBasicInformation.yaml b/src/app/tests/suites/TestBasicInformation.yaml index d165d12b6366ec..ec4df7fa4d7fd9 100644 --- a/src/app/tests/suites/TestBasicInformation.yaml +++ b/src/app/tests/suites/TestBasicInformation.yaml @@ -56,8 +56,7 @@ tests: command: "readAttribute" attribute: "AttributeList" response: - value: - [ + value: [ 0, 1, 2, @@ -77,6 +76,8 @@ tests: 16, 17, 18, - 0xFFFB, - 0xFFFD, + 0xFFF8, # ServerGeneratedCommandList + 0xFFF9, # ClientGeneratedCommandList + 0xFFFB, # AttributeList + 0xFFFD, # ClusterRevision ] diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index 833c057eb40ae9..e8d4fe138be76e 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -346,29 +347,39 @@ class GlobalAttributeReader : public MandatoryGlobalAttributeReader CHIP_ERROR GlobalAttributeReader::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { using namespace Clusters::Globals::Attributes; - // The id of the attributes below is not in the attribute metadata. - // TODO: This does not play nicely with wildcard reads. Need to fix ZAP to - // put it in the metadata, or fix wildcard expansion to add it. switch (aPath.mAttributeId) { case AttributeList::Id: return aEncoder.EncodeList([this](const auto & encoder) { - constexpr AttributeId ourId = Clusters::Globals::Attributes::AttributeList::Id; - const size_t count = mCluster->attributeCount; - bool addedOurId = false; + const size_t count = mCluster->attributeCount; + bool addedExtraGlobals = false; for (size_t i = 0; i < count; ++i) { - AttributeId id = mCluster->attributes[i].attributeId; - if (!addedOurId && id > ourId) + AttributeId id = mCluster->attributes[i].attributeId; + constexpr auto lastGlobalId = GlobalAttributesNotInMetadata[ArraySize(GlobalAttributesNotInMetadata) - 1]; + // We are relying on GlobalAttributesNotInMetadata not having + // any gaps in their ids here, but for now they do because we + // have no EventList support. So this assert is not quite + // right. There should be a "- 1" on the right-hand side of the + // equals sign. + static_assert(lastGlobalId - GlobalAttributesNotInMetadata[0] == ArraySize(GlobalAttributesNotInMetadata), + "Ids in GlobalAttributesNotInMetadata not consecutive (except EventList)"); + if (!addedExtraGlobals && id > lastGlobalId) { - ReturnErrorOnFailure(encoder.Encode(ourId)); - addedOurId = true; + for (const auto & globalId : GlobalAttributesNotInMetadata) + { + ReturnErrorOnFailure(encoder.Encode(globalId)); + } + addedExtraGlobals = true; } ReturnErrorOnFailure(encoder.Encode(id)); } - if (!addedOurId) + if (!addedExtraGlobals) { - ReturnErrorOnFailure(encoder.Encode(ourId)); + for (const auto & globalId : GlobalAttributesNotInMetadata) + { + ReturnErrorOnFailure(encoder.Encode(globalId)); + } } return CHIP_NO_ERROR; }); diff --git a/src/controller/tests/TestReadChunking.cpp b/src/controller/tests/TestReadChunking.cpp index 9baab882aa9cbd..d575e69b8ca145 100644 --- a/src/controller/tests/TestReadChunking.cpp +++ b/src/controller/tests/TestReadChunking.cpp @@ -111,7 +111,39 @@ class TestReadCallback : public app::ReadClient::Callback void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const app::StatusIB & aStatus) { - if (aPath.mAttributeId != kTestListAttribute) + if (aPath.mAttributeId == Globals::Attributes::ServerGeneratedCommandList::Id) + { + app::DataModel::DecodableList v; + NL_TEST_ASSERT(gSuite, app::DataModel::Decode(*apData, v) == CHIP_NO_ERROR); + auto it = v.begin(); + size_t arraySize = 0; + while (it.Next()) + { + NL_TEST_ASSERT(gSuite, false); + } + NL_TEST_ASSERT(gSuite, it.GetStatus() == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, v.ComputeSize(&arraySize) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, arraySize == 0); + } + else if (aPath.mAttributeId == Globals::Attributes::ClientGeneratedCommandList::Id) + { + app::DataModel::DecodableList v; + NL_TEST_ASSERT(gSuite, app::DataModel::Decode(*apData, v) == CHIP_NO_ERROR); + auto it = v.begin(); + size_t arraySize = 0; + while (it.Next()) + { + NL_TEST_ASSERT(gSuite, false); + } + NL_TEST_ASSERT(gSuite, it.GetStatus() == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, v.ComputeSize(&arraySize) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, arraySize == 0); + } + else if (aPath.mAttributeId == Globals::Attributes::AttributeList::Id) + { + // Nothing to check for this one; depends on the endpoint. + } + else if (aPath.mAttributeId != kTestListAttribute) { uint8_t v; NL_TEST_ASSERT(gSuite, app::DataModel::Decode(*apData, v) == CHIP_NO_ERROR); @@ -129,7 +161,7 @@ void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aP } NL_TEST_ASSERT(gSuite, it.GetStatus() == CHIP_NO_ERROR); NL_TEST_ASSERT(gSuite, v.ComputeSize(&arraySize) == CHIP_NO_ERROR); - NL_TEST_ASSERT(gSuite, arraySize = 5); + NL_TEST_ASSERT(gSuite, arraySize == 5); } mAttributeCount++; } @@ -240,7 +272,7 @@ void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContex // Since bugs can happen, we don't want this test to never stop, so create a ceiling for how many // times this can run without seeing expected results. // - for (int j = 0; j < 10 && !readCallback.mOnReportEnd; j++) + for (int j = 0; j < 20 && !readCallback.mOnReportEnd; j++) { ctx.DrainAndServiceIO(); chip::app::InteractionModelEngine::GetInstance()->GetReportingEngine().Run(); @@ -248,9 +280,11 @@ void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContex } // - // Always returns the same number of attributes read (5 + revision = 6). + // Always returns the same number of attributes read (5 + revision + + // AttributeList + ClientGeneratedCommandList + + // ServerGeneratedCommandList = 9). // - NL_TEST_ASSERT(apSuite, readCallback.mAttributeCount == 6); + NL_TEST_ASSERT(apSuite, readCallback.mAttributeCount == 9); readCallback.mAttributeCount = 0; NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); diff --git a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m index fbf5954a455a2a..34a5c0aad2a901 100644 --- a/src/darwin/Framework/CHIPTests/CHIPClustersTests.m +++ b/src/darwin/Framework/CHIPTests/CHIPClustersTests.m @@ -43358,7 +43358,7 @@ - (void)testSendClusterTestBasicInformation_000005_ReadAttribute { id actualValue = value; - XCTAssertEqual([actualValue count], 21); + XCTAssertEqual([actualValue count], 23); XCTAssertEqual([actualValue[0] unsignedIntValue], 0UL); XCTAssertEqual([actualValue[1] unsignedIntValue], 1UL); XCTAssertEqual([actualValue[2] unsignedIntValue], 2UL); @@ -43378,8 +43378,10 @@ - (void)testSendClusterTestBasicInformation_000005_ReadAttribute XCTAssertEqual([actualValue[16] unsignedIntValue], 16UL); XCTAssertEqual([actualValue[17] unsignedIntValue], 17UL); XCTAssertEqual([actualValue[18] unsignedIntValue], 18UL); - XCTAssertEqual([actualValue[19] unsignedIntValue], 65531UL); - XCTAssertEqual([actualValue[20] unsignedIntValue], 65533UL); + XCTAssertEqual([actualValue[19] unsignedIntValue], 65528UL); + XCTAssertEqual([actualValue[20] unsignedIntValue], 65529UL); + XCTAssertEqual([actualValue[21] unsignedIntValue], 65531UL); + XCTAssertEqual([actualValue[22] unsignedIntValue], 65533UL); } [expectation fulfill]; diff --git a/src/lib/support/CodeUtils.h b/src/lib/support/CodeUtils.h index bbbf95c1c00ca8..aa06454b9bdc6e 100644 --- a/src/lib/support/CodeUtils.h +++ b/src/lib/support/CodeUtils.h @@ -669,3 +669,34 @@ inline void chipDie(void) * thing in C++ as well. */ #define ArraySize(a) (sizeof(a) / sizeof((a)[0])) + +namespace chip { + +/** + * Utility for checking, at compile time if the array is constexpr, whether an + * array is sorted. Can be used for static_asserts. + */ + +template +constexpr bool ArrayIsSorted(const T * aArray, size_t aLength) +{ + if (aLength == 0 || aLength == 1) + { + return true; + } + + if (aArray[0] > aArray[1]) + { + return false; + } + + return ArrayIsSorted(aArray + 1, aLength - 1); +} + +template +constexpr bool ArrayIsSorted(const T (&aArray)[N]) +{ + return ArrayIsSorted(aArray, N); +} + +} // namespace chip diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 4b7f6f0345bc80..fe03e6fff94802 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -79887,10 +79887,14 @@ class TestBasicInformation : public TestCommand VerifyOrReturn(CheckNextListItemDecodes("attributeList", iter_0, 18)); VerifyOrReturn(CheckValue("attributeList[18]", iter_0.GetValue(), 18UL)); VerifyOrReturn(CheckNextListItemDecodes("attributeList", iter_0, 19)); - VerifyOrReturn(CheckValue("attributeList[19]", iter_0.GetValue(), 65531UL)); + VerifyOrReturn(CheckValue("attributeList[19]", iter_0.GetValue(), 65528UL)); VerifyOrReturn(CheckNextListItemDecodes("attributeList", iter_0, 20)); - VerifyOrReturn(CheckValue("attributeList[20]", iter_0.GetValue(), 65533UL)); - VerifyOrReturn(CheckNoMoreListItems("attributeList", iter_0, 21)); + VerifyOrReturn(CheckValue("attributeList[20]", iter_0.GetValue(), 65529UL)); + VerifyOrReturn(CheckNextListItemDecodes("attributeList", iter_0, 21)); + VerifyOrReturn(CheckValue("attributeList[21]", iter_0.GetValue(), 65531UL)); + VerifyOrReturn(CheckNextListItemDecodes("attributeList", iter_0, 22)); + VerifyOrReturn(CheckValue("attributeList[22]", iter_0.GetValue(), 65533UL)); + VerifyOrReturn(CheckNoMoreListItems("attributeList", iter_0, 23)); } NextTest();