Fix logic error in codegen data model: write privileges should only e…

…xist if attribute is NOT read-only (#34161) * Tests update * Restyle * Make clang-tidy happy * Restyle and fix one more clang-tidy error --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Nov 29, 2024 · 0008826 · 0008826
1 parent ca5a148
commit 0008826
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 2 deletions.
diff --git a/src/app/codegen-data-model/CodegenDataModel.cpp b/src/app/codegen-data-model/CodegenDataModel.cpp
@@ -115,7 +115,7 @@ void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttribut
                        InteractionModel::AttributeInfo * info)
 {
     info->readPrivilege = RequiredPrivilege::ForReadAttribute(path);
-    if (attribute.IsReadOnly())
+    if (!attribute.IsReadOnly())
     {
         info->writePrivilege = RequiredPrivilege::ForWriteAttribute(path);
     }

diff --git a/src/app/codegen-data-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model/tests/TestCodegenModelViaMocks.cpp
@@ -59,6 +59,8 @@ constexpr NodeId kTestNodeId           = 0xFFFF'1234'ABCD'4321;
 
 constexpr EndpointId kEndpointIdThatIsMissing = kMockEndpointMin - 1;
 
+constexpr AttributeId kReadOnlyAttributeId = 0x5001;
+
 static_assert(kEndpointIdThatIsMissing != kInvalidEndpointId);
 static_assert(kEndpointIdThatIsMissing != kMockEndpoint1);
 static_assert(kEndpointIdThatIsMissing != kMockEndpoint2);
@@ -190,6 +192,11 @@ const MockNodeConfig gTestNodeConfig({
         }),
         MockClusterConfig(MockClusterId(3), {
             ClusterRevision::Id, FeatureMap::Id,
+            MockAttributeConfig(
+              kReadOnlyAttributeId,
+              ZCL_INT32U_ATTRIBUTE_TYPE,
+              ATTRIBUTE_MASK_NULLABLE    // NOTE: explicltly NOT ATTRIBUTE_MASK_WRITABLE
+            )
         }),
         MockClusterConfig(MockClusterId(4), {
             ClusterRevision::Id,
@@ -809,9 +816,22 @@ TEST(TestCodegenModelViaMocks, GetAttributeInfo)
     ASSERT_TRUE(info.has_value());
     EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
 
+    // Mocks always set everything as R/W with administrative privileges
+    EXPECT_EQ(info->readPrivilege, chip::Access::Privilege::kAdminister);  // NOLINT(bugprone-unchecked-optional-access)
+    EXPECT_EQ(info->writePrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
+
     info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint2, MockClusterId(2), MockAttributeId(2)));
     ASSERT_TRUE(info.has_value());
-    EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
+    EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute));   // NOLINT(bugprone-unchecked-optional-access)
+    EXPECT_EQ(info->readPrivilege, chip::Access::Privilege::kAdminister);  // NOLINT(bugprone-unchecked-optional-access)
+    EXPECT_EQ(info->writePrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
+
+    // test a read-only attribute, which will not have a write privilege
+    info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint3, MockClusterId(3), kReadOnlyAttributeId));
+    ASSERT_TRUE(info.has_value());
+    EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
+    EXPECT_EQ(info->readPrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
+    EXPECT_FALSE(info->writePrivilege.has_value());                       // NOLINT(bugprone-unchecked-optional-access)
 }
 
 // global attributes are EXPLICITLY not supported