diff --git a/src/app/GlobalAttributes.cpp b/src/app/GlobalAttributes.cpp index a65f79b22bd870..ac6c00ab447d58 100644 --- a/src/app/GlobalAttributes.cpp +++ b/src/app/GlobalAttributes.cpp @@ -97,14 +97,6 @@ DataModel::ActionReturnStatus ReadGlobalAttributeFromMetadata(DataModel::Provide // and this reduces template variants for Encode, saving flash. ReturnErrorOnFailure(listEncodeHelper.Encode(static_cast(entry.attributeId))); } - - for (auto id : GlobalAttributesNotInMetadata) - { - // NOTE: cast to u64 because TLV encodes all numbers the same (no TLV sideffects) - // and this reduces template variants for Encode, saving flash. - ReturnErrorOnFailure(listEncodeHelper.Encode(static_cast(id))); - } - return CHIP_NO_ERROR; }); } diff --git a/src/app/data-model-provider/ProviderMetadataTree.h b/src/app/data-model-provider/ProviderMetadataTree.h index 3b0820e6efb73e..8e7763dc489020 100644 --- a/src/app/data-model-provider/ProviderMetadataTree.h +++ b/src/app/data-model-provider/ProviderMetadataTree.h @@ -60,14 +60,14 @@ class ProviderMetadataTree virtual CHIP_ERROR ClientClusters(EndpointId endpointId, ListBuilder & builder) = 0; virtual CHIP_ERROR ServerClusters(EndpointId endpointId, ListBuilder & builder) = 0; - /// Attribute lists contain all attributes EXCEPT the list attributes that - /// are part of metadata. The output from this method MUST NOT contain: - /// - AttributeList::Id + /// Attribute lists contain all attributes. This MUST include all global + /// attributes (See SPEC 7.13 Global Elements / Global Attributes Table). + /// In particular this MUST contain: /// - AcceptedCommandList::Id - /// - GeneratedCommandList::Id - /// However it MUST ALWAYS contain: + /// - AttributeList::Id /// - ClusterRevision::Id /// - FeatureMap::Id + /// - GeneratedCommandList::Id virtual CHIP_ERROR Attributes(const ConcreteClusterPath & path, ListBuilder & builder) = 0; virtual CHIP_ERROR GeneratedCommands(const ConcreteClusterPath & path, ListBuilder & builder) = 0; virtual CHIP_ERROR AcceptedCommands(const ConcreteClusterPath & path, ListBuilder & builder) = 0; diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp index b891346d170432..f8dc0c22b750b4 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -24,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -267,8 +269,15 @@ CHIP_ERROR CodegenDataModelProvider::Attributes(const ConcreteClusterPath & path VerifyOrReturnValue(cluster->attributeCount > 0, CHIP_NO_ERROR); VerifyOrReturnValue(cluster->attributes != nullptr, CHIP_NO_ERROR); - // TODO: if ember would encode data in AttributeEntry form, we could reference things directly - ReturnErrorOnFailure(builder.EnsureAppendCapacity(cluster->attributeCount)); + // TODO: if ember would encode data in AttributeEntry form, we could reference things directly (shorter code, + // although still allocation overhead due to global attributes not in metadata) + // + // We have Attributes from ember + global attributes that are NOT in ember metadata. + // We have to report them all + constexpr size_t kGlobalAttributeNotInMetadataCount = + ArraySize(GlobalAttributesNotInMetadata); + + ReturnErrorOnFailure(builder.EnsureAppendCapacity(cluster->attributeCount + kGlobalAttributeNotInMetadataCount)); Span attributeSpan(cluster->attributes, cluster->attributeCount); @@ -277,6 +286,18 @@ CHIP_ERROR CodegenDataModelProvider::Attributes(const ConcreteClusterPath & path ReturnErrorOnFailure(builder.Append(AttributeEntryFrom(path, attribute))); } + DataModel::AttributeEntry globalListEntry; + + globalListEntry.readPrivilege = Access::Privilege::kView; + globalListEntry.flags.Set(DataModel::AttributeQualityFlags::kListAttribute); + // these entries should also be `Fixed` however no such flag is propagated (we do not track) + + for (auto & attribute : GlobalAttributesNotInMetadata) + { + globalListEntry.attributeId = attribute; + ReturnErrorOnFailure(builder.Append(globalListEntry)); + } + return CHIP_NO_ERROR; } diff --git a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp index 2505fdd87d0edf..a0b0dcd7ba1a15 100644 --- a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp +++ b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp @@ -1073,7 +1073,7 @@ TEST_F(TestCodegenModelViaMocks, IterateOverAttributes) EXPECT_EQ(model.Attributes(ConcreteClusterPath(kMockEndpoint2, MockClusterId(2)), builder), CHIP_NO_ERROR); auto attributes = builder.TakeBuffer(); - ASSERT_EQ(attributes.size(), 4u); + ASSERT_EQ(attributes.size(), 7u); ASSERT_EQ(attributes[0].attributeId, ClusterRevision::Id); ASSERT_FALSE(attributes[0].flags.Has(AttributeQualityFlags::kListAttribute)); @@ -1086,6 +1086,16 @@ TEST_F(TestCodegenModelViaMocks, IterateOverAttributes) ASSERT_EQ(attributes[3].attributeId, MockAttributeId(2)); ASSERT_TRUE(attributes[3].flags.Has(AttributeQualityFlags::kListAttribute)); + + // Ends with global list attributes + ASSERT_EQ(attributes[4].attributeId, GeneratedCommandList::Id); + ASSERT_TRUE(attributes[4].flags.Has(AttributeQualityFlags::kListAttribute)); + + ASSERT_EQ(attributes[5].attributeId, AcceptedCommandList::Id); + ASSERT_TRUE(attributes[5].flags.Has(AttributeQualityFlags::kListAttribute)); + + ASSERT_EQ(attributes[6].attributeId, AttributeList::Id); + ASSERT_TRUE(attributes[6].flags.Has(AttributeQualityFlags::kListAttribute)); } TEST_F(TestCodegenModelViaMocks, FindAttribute) @@ -1127,7 +1137,7 @@ TEST_F(TestCodegenModelViaMocks, FindAttribute) EXPECT_FALSE(info->writePrivilege.has_value()); // NOLINT(bugprone-unchecked-optional-access) } -// global attributes are EXPLICITLY not supported +// global attributes are EXPLICITLY supported TEST_F(TestCodegenModelViaMocks, GlobalAttributeInfo) { UseMockNodeConfig config(gTestNodeConfig); @@ -1138,10 +1148,14 @@ TEST_F(TestCodegenModelViaMocks, GlobalAttributeInfo) std::optional info = finder.Find( ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::GeneratedCommandList::Id)); - ASSERT_FALSE(info.has_value()); + ASSERT_TRUE(info.has_value()); info = finder.Find(ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id)); - ASSERT_FALSE(info.has_value()); + ASSERT_TRUE(info.has_value()); + + info = finder.Find( + ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AcceptedCommandList::Id)); + ASSERT_TRUE(info.has_value()); } TEST_F(TestCodegenModelViaMocks, IterateOverAcceptedCommands)