Access control cluster server improvements (#12690)

* Access control cluster server improvements - move implementation into anonymous namespace - handle more errors during list encode - move extension attribute storage to external (still not implemented) - move cluster revision storage to external * Regen and restyle again * Regen and restyle again again * Fix iteration in ACL read Sentinel error is expected when iteration ends.
project-chip · May 16, 2022 · 4162170 · 4162170
1 parent 7cb1a74
commit 4162170
Show file tree

Hide file tree

Showing 6 changed files with 468 additions and 464 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap
@@ -979,7 +979,7 @@
               "mfgCode": null,
               "side": "server",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "",
@@ -994,7 +994,7 @@
               "mfgCode": null,
               "side": "server",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "1",

diff --git a/src/app/clusters/access-control-server/access-control-server.cpp b/src/app/clusters/access-control-server/access-control-server.cpp
@@ -35,6 +35,8 @@ using namespace chip::Access;
 
 namespace AccessControlCluster = chip::app::Clusters::AccessControl;
 
+namespace {
+
 struct AccessControlEntryCodec
 {
     static CHIP_ERROR Convert(AuthMode from, AccessControlCluster::AuthMode & to)
@@ -318,25 +320,31 @@ struct AccessControlEntryCodec
 class AccessControlAttribute : public chip::app::AttributeAccessInterface
 {
 public:
-    AccessControlAttribute() : AttributeAccessInterface(Optional<EndpointId>(0), Clusters::AccessControl::Id) {}
+    AccessControlAttribute() : AttributeAccessInterface(Optional<EndpointId>(0), AccessControlCluster::Id) {}
 
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
     CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;
 
+    static constexpr uint16_t ClusterRevision = 1;
+
 private:
     CHIP_ERROR ReadAcl(AttributeValueEncoder & aEncoder);
     CHIP_ERROR ReadExtension(AttributeValueEncoder & aEncoder);
     CHIP_ERROR WriteAcl(AttributeValueDecoder & aDecoder);
     CHIP_ERROR WriteExtension(AttributeValueDecoder & aDecoder);
 };
 
+constexpr uint16_t AccessControlAttribute::ClusterRevision;
+
 CHIP_ERROR AccessControlAttribute::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     switch (aPath.mAttributeId)
     {
-    case Clusters::AccessControl::Attributes::Acl::Id:
+    case AccessControlCluster::Attributes::Acl::Id:
         return ReadAcl(aEncoder);
-    case Clusters::AccessControl::Attributes::Extension::Id:
+    case AccessControlCluster::Attributes::ClusterRevision::Id:
+        return aEncoder.Encode(ClusterRevision);
+    case AccessControlCluster::Attributes::Extension::Id:
         return ReadExtension(aEncoder);
     }
 
@@ -348,31 +356,31 @@ CHIP_ERROR AccessControlAttribute::ReadAcl(AttributeValueEncoder & aEncoder)
     AccessControlEntryCodec codec;
     AccessControl::EntryIterator iterator;
 
-    GetAccessControl().Entries(iterator);
+    ReturnErrorOnFailure(GetAccessControl().Entries(iterator));
 
-    CHIP_ERROR err = aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR {
-        while (iterator.Next(codec.entry) == CHIP_NO_ERROR)
+    return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR {
+        CHIP_ERROR err;
+        while ((err = iterator.Next(codec.entry)) == CHIP_NO_ERROR)
         {
-            encoder.Encode(codec);
+            ReturnErrorOnFailure(encoder.Encode(codec));
         }
+        ReturnErrorCodeIf(err != CHIP_NO_ERROR && err != CHIP_ERROR_SENTINEL, err);
         return CHIP_NO_ERROR;
     });
-
-    return err;
 }
 
 CHIP_ERROR AccessControlAttribute::ReadExtension(AttributeValueEncoder & aEncoder)
 {
-    return CHIP_NO_ERROR;
+    return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR { return CHIP_NO_ERROR; });
 }
 
 CHIP_ERROR AccessControlAttribute::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
 {
     switch (aPath.mAttributeId)
     {
-    case Clusters::AccessControl::Attributes::Acl::Id:
+    case AccessControlCluster::Attributes::Acl::Id:
         return WriteAcl(aDecoder);
-    case Clusters::AccessControl::Attributes::Extension::Id:
+    case AccessControlCluster::Attributes::Extension::Id:
         return WriteExtension(aDecoder);
     }
 
@@ -419,13 +427,17 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(AttributeValueDecoder & aDecoder)
 
 CHIP_ERROR AccessControlAttribute::WriteExtension(AttributeValueDecoder & aDecoder)
 {
+    DataModel::DecodableList<AccessControlCluster::Structs::ExtensionEntry::DecodableType> list;
+    ReturnErrorOnFailure(aDecoder.Decode(list));
     return CHIP_NO_ERROR;
 }
 
 AccessControlAttribute gAttribute;
 
 AccessControl gAccessControl(Examples::GetAccessControlDelegate());
 
+} // namespace
+
 void MatterAccessControlPluginServerInitCallback()
 {
     registerAttributeAccessOverride(&gAttribute);

diff --git a/src/app/zap-templates/zcl/data-model/chip/access-control-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/access-control-cluster.xml
@@ -73,7 +73,7 @@ limitations under the License.
         <access op="write" privilege="administer"/>        
         <access modifier="fabric-scoped"/>
     </attribute>
-    <attribute side="server" code="0x0001" define="EXTENSION" type="ARRAY" length="4" entryType="ExtensionEntry" writable="true">
+    <attribute side="server" code="0x0001" define="EXTENSION" type="ARRAY" entryType="ExtensionEntry" writable="true">
         <description>Extension</description>
         <access op="read" privilege="administer"/>
         <access op="write" privilege="administer"/>        

diff --git a/src/controller/data_model/controller-clusters.zap b/src/controller/data_model/controller-clusters.zap
@@ -1427,7 +1427,7 @@
               "mfgCode": null,
               "side": "client",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "1",
@@ -1468,7 +1468,7 @@
               "mfgCode": null,
               "side": "server",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "",
@@ -1483,7 +1483,7 @@
               "mfgCode": null,
               "side": "server",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
               "defaultValue": "1",