Add support for the AttributeList attribute. (#12660)

Getting this into the ZAP database as things stand involves modifying every single cluster XML and every single cluster definition in every single .zap file. Instead of that, we just enable it for the controller codegen and manually work around it not being present in endpoint_config.
project-chip · May 31, 2022 · 1547600 · 1547600
1 parent 05128fe
commit 1547600
Show file tree

Hide file tree

Showing 36 changed files with 31,375 additions and 3,925 deletions.
diff --git a/src/app/AttributePathParams.h b/src/app/AttributePathParams.h
@@ -33,7 +33,7 @@ struct AttributePathParams
     //
     // TODO: (#11420) This class is overlapped with ClusterInfo class, need to do a clean up.
     AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId) :
-        AttributePathParams(aEndpointId, aClusterId, ClusterInfo::kInvalidAttributeId, kInvalidListIndex)
+        AttributePathParams(aEndpointId, aClusterId, kInvalidAttributeId, kInvalidListIndex)
     {}
 
     AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId) :
@@ -61,13 +61,13 @@ struct AttributePathParams
     bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); }
 
     inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; }
-    inline bool HasWildcardClusterId() const { return mClusterId == ClusterInfo::kInvalidClusterId; }
-    inline bool HasWildcardAttributeId() const { return mAttributeId == ClusterInfo::kInvalidAttributeId; }
+    inline bool HasWildcardClusterId() const { return mClusterId == kInvalidClusterId; }
+    inline bool HasWildcardAttributeId() const { return mAttributeId == kInvalidAttributeId; }
     inline bool HasWildcardListIndex() const { return mListIndex == kInvalidListIndex; }
 
     EndpointId mEndpointId   = kInvalidEndpointId;
-    ClusterId mClusterId     = ClusterInfo::kInvalidClusterId;
-    AttributeId mAttributeId = ClusterInfo::kInvalidAttributeId;
+    ClusterId mClusterId     = kInvalidClusterId;
+    AttributeId mAttributeId = kInvalidAttributeId;
     ListIndex mListIndex     = kInvalidListIndex;
 };
 } // namespace app

diff --git a/src/app/ClusterInfo.h b/src/app/ClusterInfo.h
@@ -38,17 +38,6 @@ namespace app {
 // Note: The change will happen after #11171 with a better linked list.
 struct ClusterInfo
 {
-private:
-    // Allow AttributePathParams access these constants.
-    friend struct AttributePathParams;
-    friend struct EventPathParams;
-
-    // The ClusterId, AttributeId and EventId are MEIs,
-    // 0xFFFF is not a valid manufacturer code, thus 0xFFFF'FFFF is not a valid MEI
-    static constexpr ClusterId kInvalidClusterId     = 0xFFFF'FFFF;
-    static constexpr AttributeId kInvalidAttributeId = 0xFFFF'FFFF;
-    static constexpr EventId kInvalidEventId         = 0xFFFF'FFFF;
-
 public:
     bool IsAttributePathSupersetOf(const ClusterInfo & other) const
     {

diff --git a/src/app/EventPathParams.h b/src/app/EventPathParams.h
@@ -41,12 +41,12 @@ struct EventPathParams
     bool IsValidEventPath() const { return !(HasWildcardClusterId() && !HasWildcardEventId()); }
 
     inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; }
-    inline bool HasWildcardClusterId() const { return mClusterId == ClusterInfo::kInvalidClusterId; }
-    inline bool HasWildcardEventId() const { return mEventId == ClusterInfo::kInvalidEventId; }
+    inline bool HasWildcardClusterId() const { return mClusterId == kInvalidClusterId; }
+    inline bool HasWildcardEventId() const { return mEventId == kInvalidEventId; }
 
     EndpointId mEndpointId = kInvalidEndpointId;
-    ClusterId mClusterId   = ClusterInfo::kInvalidClusterId;
-    EventId mEventId       = ClusterInfo::kInvalidEventId;
+    ClusterId mClusterId   = kInvalidClusterId;
+    EventId mEventId       = kInvalidEventId;
 };
 } // namespace app
 } // namespace chip
diff --git a/src/app/tests/TestClusterInfo.cpp b/src/app/tests/TestClusterInfo.cpp
@@ -99,11 +99,11 @@ void TestAttributePathIncludedDifferentClusterId(nlTestSuite * apSuite, void * a
 }
 
 /*
-{ClusterInfo::kInvalidEndpointId, ClusterInfo::kInvalidClusterId, ClusterInfo::kInvalidEventId},
-{ClusterInfo::kInvalidEndpointId, MockClusterId(1), ClusterInfo::kInvalidEventId},
-{ClusterInfo::kInvalidEndpointId, MockClusterId(1), MockEventId(1)},
-{kMockEndpoint1, ClusterInfo::kInvalidClusterId, ClusterInfo::kInvalidEventId},
-{kMockEndpoint1, MockClusterId(1), ClusterInfo::kInvalidEventId},
+{kInvalidEndpointId, kInvalidClusterId, kInvalidEventId},
+{kInvalidEndpointId, MockClusterId(1), kInvalidEventId},
+{kInvalidEndpointId, MockClusterId(1), MockEventId(1)},
+{kMockEndpoint1, kInvalidClusterId, kInvalidEventId},
+{kMockEndpoint1, MockClusterId(1), kInvalidEventId},
 {kMockEndpoint1, MockClusterId(1), MockEventId(1)},
 */
 chip::app::ClusterInfo validEventpaths[6];

diff --git a/src/app/tests/suites/TestBasicInformation.yaml b/src/app/tests/suites/TestBasicInformation.yaml
@@ -46,3 +46,32 @@ tests:
       attribute: "location"
       arguments:
           value: ""
+
+    - label: "Read AttributeList value"
+      command: "readAttribute"
+      attribute: "AttributeList"
+      response:
+          value:
+              [
+                  0,
+                  1,
+                  2,
+                  3,
+                  4,
+                  5,
+                  6,
+                  7,
+                  8,
+                  9,
+                  10,
+                  11,
+                  12,
+                  13,
+                  14,
+                  15,
+                  16,
+                  17,
+                  18,
+                  0xFFFB,
+                  0xFFFD,
+              ]
diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
@@ -250,6 +250,84 @@ CHIP_ERROR SendFailureStatus(const ConcreteAttributePath & aPath, AttributeRepor
     return aAttributeReport.EndOfAttributeReportIB().GetError();
 }
 
+// This reader should never actually be registered; we do manual dispatch to it
+// for the one attribute it handles.
+class MandatoryGlobalAttributeReader : public AttributeAccessInterface
+{
+public:
+    MandatoryGlobalAttributeReader(const EmberAfCluster * aCluster) :
+        AttributeAccessInterface(MakeOptional(kInvalidEndpointId), kInvalidClusterId), mCluster(aCluster)
+    {}
+
+protected:
+    const EmberAfCluster * mCluster;
+};
+
+class AttributeListReader : public MandatoryGlobalAttributeReader
+{
+public:
+    AttributeListReader(const EmberAfCluster * aCluster) : MandatoryGlobalAttributeReader(aCluster) {}
+
+    CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
+};
+
+CHIP_ERROR AttributeListReader::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
+{
+    // The id of AttributeList 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.
+    return aEncoder.EncodeList([this](const auto & encoder) {
+        constexpr AttributeId ourId = Clusters::Globals::Attributes::AttributeList::Id;
+        const size_t count          = mCluster->attributeCount;
+        bool addedOurId             = false;
+        for (size_t i = 0; i < count; ++i)
+        {
+            AttributeId id = mCluster->attributes[i].attributeId;
+            if (!addedOurId && id > ourId)
+            {
+                ReturnErrorOnFailure(encoder.Encode(ourId));
+                addedOurId = true;
+            }
+            ReturnErrorOnFailure(encoder.Encode(id));
+        }
+        if (!addedOurId)
+        {
+            ReturnErrorOnFailure(encoder.Encode(ourId));
+        }
+        return CHIP_NO_ERROR;
+    });
+}
+
+// Helper function for trying to read an attribute value via an
+// AttributeAccessInterface.  On failure, the read has failed.  On success, the
+// aTriedEncode outparam is set to whether the AttributeAccessInterface tried to encode a value.
+CHIP_ERROR ReadViaAccessInterface(FabricIndex aAccessingFabricIndex, const ConcreteReadAttributePath & aPath,
+                                  AttributeReportIBs::Builder & aAttributeReports,
+                                  AttributeValueEncoder::AttributeEncodeState * aEncoderState,
+                                  AttributeAccessInterface * aAccessInterface, bool * aTriedEncode)
+{
+    // TODO: We should probably clone the writer and convert failures here
+    // into status responses, unless our caller already does that.
+    AttributeValueEncoder::AttributeEncodeState state =
+        (aEncoderState == nullptr ? AttributeValueEncoder::AttributeEncodeState() : *aEncoderState);
+    AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex, aPath, kTemporaryDataVersion, state);
+    CHIP_ERROR err = aAccessInterface->Read(aPath, valueEncoder);
+
+    if (err != CHIP_NO_ERROR)
+    {
+        // If the err is not CHIP_NO_ERROR, means the encoding was aborted, then the valueEncoder may save its state.
+        // The state is used by list chunking feature for now.
+        if (aEncoderState != nullptr)
+        {
+            *aEncoderState = valueEncoder.GetState();
+        }
+        return err;
+    }
+
+    *aTriedEncode = valueEncoder.TriedEncode();
+    return CHIP_NO_ERROR;
+}
+
 } // anonymous namespace
 
 CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const ConcreteReadAttributePath & aPath,
@@ -260,6 +338,22 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre
                   "Reading attribute: Cluster=" ChipLogFormatMEI " Endpoint=%" PRIx16 " AttributeId=" ChipLogFormatMEI,
                   ChipLogValueMEI(aPath.mClusterId), aPath.mEndpointId, ChipLogValueMEI(aPath.mAttributeId));
 
+    if (aPath.mAttributeId == Clusters::Globals::Attributes::AttributeList::Id)
+    {
+        // This is not in our attribute metadata, so we just check for this
+        // endpoint+cluster existing.
+        EmberAfCluster * cluster = emberAfFindCluster(aPath.mEndpointId, aPath.mClusterId, CLUSTER_MASK_SERVER);
+        if (cluster)
+        {
+            AttributeListReader reader(cluster);
+            bool ignored; // Our reader always tries to encode
+            return ReadViaAccessInterface(aAccessingFabricIndex, aPath, aAttributeReports, apEncoderState, &reader, &ignored);
+        }
+
+        // else to save codesize just fall through and do the metadata search
+        // (which we know will fail and error out);
+    }
+
     EmberAfAttributeMetadata * attributeMetadata =
         emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, CLUSTER_MASK_SERVER, 0);
 
@@ -277,27 +371,11 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre
     // The AttributeValueEncoder may encode more than one AttributeReportIB for the list chunking feature.
     if (attrOverride != nullptr)
     {
-        // TODO: We should probably clone the writer and convert failures here
-        // into status responses, unless our caller already does that.
-        AttributeValueEncoder::AttributeEncodeState state =
-            (apEncoderState == nullptr ? AttributeValueEncoder::AttributeEncodeState() : *apEncoderState);
-        AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex,
-                                           ConcreteAttributePath(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId),
-                                           kTemporaryDataVersion, state);
-        CHIP_ERROR err = attrOverride->Read(aPath, valueEncoder);
-
-        if (err != CHIP_NO_ERROR)
-        {
-            // If the err is not CHIP_NO_ERROR, means the encoding was aborted, then the valueEncoder may save its state.
-            // The state is used by list chunking feature for now.
-            if (apEncoderState != nullptr)
-            {
-                *apEncoderState = valueEncoder.GetState();
-            }
-            return err;
-        }
+        bool triedEncode;
+        ReturnErrorOnFailure(
+            ReadViaAccessInterface(aAccessingFabricIndex, aPath, aAttributeReports, apEncoderState, attrOverride, &triedEncode));
 
-        if (valueEncoder.TriedEncode())
+        if (triedEncode)
         {
             return CHIP_NO_ERROR;
         }

diff --git a/src/app/zap-templates/templates/app/helper.js b/src/app/zap-templates/templates/app/helper.js
@@ -30,6 +30,7 @@ const ChipTypesHelper = require('../../common/ChipTypesHelper.js');
 // This list of attributes is taken from section '11.2. Global Attributes' of the
 // Data Model specification.
 const kGlobalAttributes = [
+  0xfffb, // AttributeList
   0xfffc, // ClusterRevision
   0xfffd, // FeatureMap
 ];

diff --git a/src/app/zap-templates/zcl/data-model/chip/global-attributes.xml b/src/app/zap-templates/zcl/data-model/chip/global-attributes.xml
@@ -20,5 +20,7 @@ limitations under the License.
     <attribute side="server" code="0xFFFD" define="CLUSTER_REVISION_SERVER" type="int16u"   default="1"                 >ClusterRevision</attribute>
     <attribute side="client" code="0xFFFC" define="FEATURE_MAP_CLIENT"      type="bitmap32" default="0" optional="true">FeatureMap</attribute>
     <attribute side="server" code="0xFFFC" define="FEATURE_MAP_SERVER"      type="bitmap32" default="0" optional="true">FeatureMap</attribute>
+    <attribute side="server" code="0xFFFB" define="ATTRIBUTE_LIST_SERVER"   type="array" entryType="attrib_id">AttributeList</attribute>
+
   </global>
 </configurator>