Skip to content

Commit

Permalink
Fix logic error in codegen data model: write privileges should only e…
Browse files Browse the repository at this point in the history
…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 <andreilitvin@google.com>
  • Loading branch information
andy31415 and andreilitvin authored Jul 3, 2024
1 parent d31cb54 commit e5acdc9
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/app/codegen-data-model/CodegenDataModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
22 changes: 21 additions & 1 deletion src/app/codegen-data-model/tests/TestCodegenModelViaMocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e5acdc9

Please sign in to comment.