From e5acdc9a8b5bd4bd7a56642b7bb3c811488d56d1 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 3 Jul 2024 10:38:23 -0400 Subject: [PATCH] Fix logic error in codegen data model: write privileges should only exist 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 --- .../codegen-data-model/CodegenDataModel.cpp | 2 +- .../tests/TestCodegenModelViaMocks.cpp | 22 ++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/app/codegen-data-model/CodegenDataModel.cpp b/src/app/codegen-data-model/CodegenDataModel.cpp index d46deeeddaa1e0..ec7b13af357287 100644 --- 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 index 11264bbd33a36e..df759e78666659 100644 --- 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