Fix wildcard handling of global attributes not present in metadata. (p…

…roject-chip#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 project-chip#14063

* Address review comments

.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

src/app/AttributePathExpandIterator.cpp

@@ -22,6 +22,7 @@
 #include <app/ClusterInfo.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/EventManagement.h>
+#include <app/GlobalAttributes.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/CHIPTLVDebug.hpp>
 #include <lib/support/CodeUtils.h>
@@ -60,6 +61,12 @@ AttributePathExpandIterator::AttributePathExpandIterator(ClusterInfo * aClusterI
     mClusterIndex   = UINT8_MAX;
     mAttributeIndex = UINT16_MAX;
 
+    static_assert(std::numeric_limits<decltype(mGlobalAttributeIndex)>::max() >= ArraySize(GlobalAttributesNotInMetadata),
+                  "Our index won't be able to hold the value we need to hold.");
+    static_assert(std::is_same<decltype(mGlobalAttributeIndex), uint8_t>::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,15 +108,35 @@ 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
     {
         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<uint16_t>(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<uint8_t>(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.

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;
 

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",

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 <app-common/zap-generated/ids/Attributes.h>
+
+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