Fix the error code returned when writing global list attributes. (#31454

) * Fix the error code returned when writing global list attributes. It should be UNSUPPORTED_WRITE, not UNSUPPORTED_ATTRIBUTE. Fixes #31448 * Address review comment.
project-chip · Jan 17, 2024 · 87e6da0 · 87e6da0
1 parent a8df984
commit 87e6da0
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 829 deletions.
diff --git a/examples/darwin-framework-tool/templates/tests/ciTests.json b/examples/darwin-framework-tool/templates/tests/ciTests.json
@@ -3,6 +3,8 @@
     "disable": [
         "Test_TC_SC_4_1",
         "Test_TC_SC_5_2",
+        "TestBasicInformation disabled because codegen test don't support writing readonly attributes",
+        "TestBasicInformation",
         "TestClusterComplexTypes",
         "TestEvents",
         "TestDiscovery",

diff --git a/src/app/tests/suites/TestBasicInformation.yaml b/src/app/tests/suites/TestBasicInformation.yaml
@@ -215,3 +215,43 @@ tests:
       response:
           # For now all-clusters-app only supports 1 max paths per invoke.
           value: 1
+
+    - label: "Write global ClusterRevision attribute"
+      command: "writeAttribute"
+      attribute: "ClusterRevision"
+      arguments:
+          value: 1
+      response:
+          error: UNSUPPORTED_WRITE
+
+    - label: "Write global FeatureMap attribute"
+      command: "writeAttribute"
+      attribute: "FeatureMap"
+      arguments:
+          value: 1
+      response:
+          error: UNSUPPORTED_WRITE
+
+    - label: "Write global AttributeList attribute"
+      command: "writeAttribute"
+      attribute: "AttributeList"
+      arguments:
+          value: []
+      response:
+          error: UNSUPPORTED_WRITE
+
+    - label: "Write global GeneratedCommandList attribute"
+      command: "writeAttribute"
+      attribute: "GeneratedCommandList"
+      arguments:
+          value: []
+      response:
+          error: UNSUPPORTED_WRITE
+
+    - label: "Write global AcceptedCommandList attribute"
+      command: "writeAttribute"
+      attribute: "AcceptedCommandList"
+      arguments:
+          value: []
+      response:
+          error: UNSUPPORTED_WRITE
diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
@@ -503,6 +503,28 @@ Protocols::InteractionModel::Status UnsupportedAttributeStatus(const ConcreteAtt
     return Status::UnsupportedAttribute;
 }
 
+// Will set at most one of the out-params (aAttributeCluster or
+// aAttributeMetadata) to non-null.  Both null means attribute not supported,
+// aAttributeCluster non-null means this is a supported global attribute that
+// does not have metadata.
+void FindAttributeMetadata(const ConcreteAttributePath & aPath, const EmberAfCluster ** aAttributeCluster,
+                           const EmberAfAttributeMetadata ** aAttributeMetadata)
+{
+    *aAttributeCluster  = nullptr;
+    *aAttributeMetadata = nullptr;
+
+    for (auto & attr : GlobalAttributesNotInMetadata)
+    {
+        if (attr == aPath.mAttributeId)
+        {
+            *aAttributeCluster = emberAfFindServerCluster(aPath.mEndpointId, aPath.mClusterId);
+            return;
+        }
+    }
+
+    *aAttributeMetadata = emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId);
+}
+
 } // anonymous namespace
 
 bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
@@ -530,22 +552,7 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b
 
     const EmberAfCluster * attributeCluster            = nullptr;
     const EmberAfAttributeMetadata * attributeMetadata = nullptr;
-
-    bool isGlobalAttributeNotInMetadata = false;
-    for (auto & attr : GlobalAttributesNotInMetadata)
-    {
-        if (attr == aPath.mAttributeId)
-        {
-            isGlobalAttributeNotInMetadata = true;
-            attributeCluster               = emberAfFindServerCluster(aPath.mEndpointId, aPath.mClusterId);
-            break;
-        }
-    }
-
-    if (!isGlobalAttributeNotInMetadata)
-    {
-        attributeMetadata = emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId);
-    }
+    FindAttributeMetadata(aPath, &attributeCluster, &attributeMetadata);
 
     if (attributeCluster == nullptr && attributeMetadata == nullptr)
     {
@@ -972,14 +979,19 @@ const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePat
 CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, const ConcreteDataAttributePath & aPath,
                                   TLV::TLVReader & aReader, WriteHandler * apWriteHandler)
 {
-    const EmberAfAttributeMetadata * attributeMetadata = GetAttributeMetadata(aPath);
+    // Check attribute existence. This includes attributes with registered metadata, but also specially handled
+    // mandatory global attributes (which just check for cluster on endpoint).
+    const EmberAfCluster * attributeCluster            = nullptr;
+    const EmberAfAttributeMetadata * attributeMetadata = nullptr;
+    FindAttributeMetadata(aPath, &attributeCluster, &attributeMetadata);
 
-    if (attributeMetadata == nullptr)
+    if (attributeCluster == nullptr && attributeMetadata == nullptr)
     {
         return apWriteHandler->AddStatus(aPath, UnsupportedAttributeStatus(aPath));
     }
 
-    if (attributeMetadata->IsReadOnly())
+    // All the global attributes we don't have metadata for are readonly.
+    if (attributeMetadata == nullptr || attributeMetadata->IsReadOnly())
     {
         return apWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::UnsupportedWrite);
     }