Make global attributes be part of IM instead of part of DataModel::Provider

.github/workflows/lint.yml

@@ -114,7 +114,6 @@ jobs:
                      --known-failure app/util/config.h \
                      --known-failure app/util/DataModelHandler.cpp \
                      --known-failure app/util/DataModelHandler.h \
-                     --known-failure app/util/ember-global-attribute-access-interface.h \
                      --known-failure app/util/ember-io-storage.h \
                      --known-failure app/util/endpoint-config-api.h \
                      --known-failure app/util/generic-callbacks.h \
@@ -133,7 +132,6 @@ jobs:
                   # for them. Keeping them as a list as they still need review ...
                   #   --known-failure app/util/attribute-table.cpp \
                   #   --known-failure app/util/ember-io-storage.cpp \
-                  #   --known-failure app/util/ember-global-attribute-access-interface.cpp \
                   #   --known-failure app/util/attribute-storage.cpp \
             - name: Check for matter lint errors
               if: always()

examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt

@@ -58,10 +58,12 @@ class PairOnNetworkLongImReadCommand(
       event.getJson().toString() == """{"0:STRUCT":{"0:UINT":1}}"""
 
     fun checkAllAttributesJsonForFixedLabel(cluster: String): Boolean {
+      // TODO: this hard-codes the array and as a result it is order-dependend. This should
+      //       be changed to be order-independent.
       val expected =
         """{"65528:ARRAY-?":[],"0:ARRAY-STRUCT":[{"0:STRING":"room","1:STRING":"bedroom 2"},""" +
           """{"0:STRING":"orientation","1:STRING":"North"},{"0:STRING":"floor","1:STRING":"2"},""" +
-          """{"0:STRING":"direction","1:STRING":"up"}],"65531:ARRAY-UINT":[0,65528,65529,65531,65532,65533],""" +
+          """{"0:STRING":"direction","1:STRING":"up"}],"65531:ARRAY-UINT":[0,65532,65533,65528,65529,65531],""" +
           """"65533:UINT":1,"65529:ARRAY-?":[],"65532:UINT":0}"""
       return cluster.equals(expected)
     }

kotlin-detect-config.yaml

@@ -164,6 +164,7 @@ style:
             - "**/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/MultiAdminClientFragment.kt"
             - "**/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/ClusterInteractionFragment.kt"
             - "**/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/AddressCommissioningFragment.kt"
+            - "**/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairOnNetworkLongImReadCommand.kt"
             - "**/src/controller/java/src/matter/onboardingpayload/QRCodeOnboardingPayloadParser.kt"
     ExplicitItLambdaParameter:
         excludes:

scripts/build_coverage.sh

@@ -176,9 +176,9 @@ if [ "$skip_gn" == false ]; then
 fi
 
 mkdir -p "$COVERAGE_ROOT"
-lcov --initial --capture --directory "$OUTPUT_ROOT/obj/src" --exclude="$PWD"/zzz_generated/* --exclude="$PWD"/third_party/* --exclude=/usr/include/* --output-file "$COVERAGE_ROOT/lcov_base.info"
-lcov --capture --directory "$OUTPUT_ROOT/obj/src" --exclude="$PWD"/zzz_generated/* --exclude="$PWD"/third_party/* --exclude=/usr/include/* --output-file "$COVERAGE_ROOT/lcov_test.info"
-lcov --add-tracefile "$COVERAGE_ROOT/lcov_base.info" --add-tracefile "$COVERAGE_ROOT/lcov_test.info" --output-file "$COVERAGE_ROOT/lcov_final.info"
+lcov --initial --capture --ignore-errors inconsistent --directory "$OUTPUT_ROOT/obj/src" --exclude="$PWD"/zzz_generated/* --exclude="$PWD"/third_party/* --exclude=/usr/include/* --output-file "$COVERAGE_ROOT/lcov_base.info"
+lcov --capture --ignore-errors inconsistent --directory "$OUTPUT_ROOT/obj/src" --exclude="$PWD"/zzz_generated/* --exclude="$PWD"/third_party/* --exclude=/usr/include/* --output-file "$COVERAGE_ROOT/lcov_test.info"
+lcov --ignore-errors inconsistent --add-tracefile "$COVERAGE_ROOT/lcov_base.info" --add-tracefile "$COVERAGE_ROOT/lcov_test.info" --output-file "$COVERAGE_ROOT/lcov_final.info"
 genhtml "$COVERAGE_ROOT/lcov_final.info" --output-directory "$COVERAGE_ROOT/html" --title "SHA:$(git rev-parse HEAD)" --header-title "Matter SDK Coverage Report"
 
 # Copy webapp's YAML file to the coverage output directory

src/app/BUILD.gn

@@ -107,17 +107,22 @@ source_set("paths") {
     "${chip_root}/src/app/util:types",
     "${chip_root}/src/lib/core",
     "${chip_root}/src/lib/core:types",
+    "${chip_root}/src/lib/support",
   ]
 }
 
 source_set("global-attributes") {
-  sources = [ "GlobalAttributes.h" ]
+  sources = [
+    "GlobalAttributes.cpp",
+    "GlobalAttributes.h",
+  ]
 
   # This also depends on zap-generated code which is currently impossible to split out
   # as a dependency
   public_deps = [
     ":app_config",
     "${chip_root}/src/app/common:ids",
+    "${chip_root}/src/app/data-model-provider",
     "${chip_root}/src/lib/support",
   ]
 }
@@ -263,7 +268,6 @@ static_library("interaction-model") {
     # We likely should formalize and change this with a proper DataModel::Provider that
     # is consistent instead
     sources += [
-      "${chip_root}/src/app/util/ember-global-attribute-access-interface.cpp",
       "${chip_root}/src/app/util/ember-io-storage.cpp",
       "dynamic_server/DynamicDispatcher.cpp",
     ]

src/app/GlobalAttributes.cpp

@@ -0,0 +1,132 @@
+/*
+ *    Copyright (c) 2022-2025 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+#include <app/GlobalAttributes.h>
+#include <app/data-model-provider/MetadataLookup.h>
+#include <protocols/interaction_model/StatusCode.h>
+
+using chip::Protocols::InteractionModel::Status;
+
+namespace chip {
+namespace app {
+
+bool IsSupportedGlobalAttributeNotInMetadata(AttributeId attributeId)
+{
+    for (auto & attr : GlobalAttributesNotInMetadata)
+    {
+        if (attr == attributeId)
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+DataModel::ActionReturnStatus ReadGlobalAttributeFromMetadata(DataModel::Provider * provider, const ConcreteAttributePath & path,
+                                                              AttributeValueEncoder & encoder)
+{
+    CHIP_ERROR err;
+
+    switch (path.mAttributeId)
+    {
+    case Clusters::Globals::Attributes::GeneratedCommandList::Id: {
+        DataModel::ListBuilder<CommandId> builder;
+        err = provider->GeneratedCommands(path, builder);
+        if (err != CHIP_NO_ERROR)
+        {
+            break;
+        }
+        auto buffer = builder.TakeBuffer();
+
+        return encoder.EncodeList([&buffer](const auto & listEncodeHelper) {
+            for (auto id : buffer)
+            {
+                // 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<uint64_t>(id)));
+            }
+            return CHIP_NO_ERROR;
+        });
+    }
+    case Clusters::Globals::Attributes::AcceptedCommandList::Id: {
+        DataModel::ListBuilder<DataModel::AcceptedCommandEntry> builder;
+        err = provider->AcceptedCommands(path, builder);
+        if (err != CHIP_NO_ERROR)
+        {
+            break;
+        }
+        auto buffer = builder.TakeBuffer();
+
+        return encoder.EncodeList([&buffer](const auto & listEncodeHelper) {
+            for (auto entry : buffer)
+            {
+                // 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<uint64_t>(entry.commandId)));
+            }
+            return CHIP_NO_ERROR;
+        });
+    }
+    case Clusters::Globals::Attributes::AttributeList::Id: {
+        DataModel::ListBuilder<DataModel::AttributeEntry> builder;
+        err = provider->Attributes(path, builder);
+        if (err != CHIP_NO_ERROR)
+        {
+            break;
+        }
+        auto buffer = builder.TakeBuffer();
+
+        return encoder.EncodeList([&buffer](const auto & listEncodeHelper) {
+            for (auto entry : buffer)
+            {
+                // 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<uint64_t>(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<uint64_t>(id)));
+            }
+
+            return CHIP_NO_ERROR;
+        });
+    }
+    default:
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
+    // if we get here, the path was NOT valid
+    if (err == CHIP_ERROR_NOT_FOUND)
+    {
+        // The `Failure` here is arbitrary: we expect ReadGlobalAttributeFromMetadata to be
+        // an internal API used for global attributes only and call preconditions say that
+        // should never happen.
+        //
+        // Code only takes this path if one of
+        // `GeneratedCommands`/`AcceptedCommands`/`Attribute` return a NOT_FOUND and
+        // that would indicate an invalid cluster (which should have been pre-validated by
+        // the caller).
+        return DataModel::ValidateClusterPath(provider, path, Status::Failure);
+    }
+    return err;
+}
+
+} // namespace app
+} // namespace chip

src/app/GlobalAttributes.h

@@ -19,6 +19,11 @@
 
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app/AppConfig.h>
+#include <app/AttributeValueEncoder.h>
+#include <app/ConcreteAttributePath.h>
+#include <app/data-model-provider/ActionReturnStatus.h>
+#include <app/data-model-provider/Provider.h>
+#include <lib/core/DataModelTypes.h>
 #include <lib/support/CodeUtils.h>
 
 namespace chip {
@@ -37,18 +42,17 @@ constexpr AttributeId GlobalAttributesNotInMetadata[] = {
 
 static_assert(ArrayIsSorted(GlobalAttributesNotInMetadata), "Array of global attribute ids must be sorted");
 
-inline bool IsSupportedGlobalAttributeNotInMetadata(AttributeId attributeId)
-{
-    for (auto & attr : GlobalAttributesNotInMetadata)
-    {
-        if (attr == attributeId)
-        {
-            return true;
-        }
-    }
-
-    return false;
-}
+bool IsSupportedGlobalAttributeNotInMetadata(AttributeId attributeId);
+
+/**
+ * Reads a `IsSupportedGlobalAttributeNotInMetadata` attribute into `encoder`.
+ *
+ * Preconditions:
+ *   - `path` MUST be a valid cluster path inside `provider` and its mAttributeID
+ *     MUST be `IsSupportedGlobalAttributeNotInMetadata`
+ */
+DataModel::ActionReturnStatus ReadGlobalAttributeFromMetadata(DataModel::Provider * provider, const ConcreteAttributePath & path,
+                                                              AttributeValueEncoder & encoder);
 
 } // namespace app
 } // namespace chip

src/app/chip_data_model.cmake

@@ -162,7 +162,6 @@ function(chip_configure_data_model APP_TARGET)
         ${CHIP_APP_BASE_DIR}/util/attribute-table.cpp
         ${CHIP_APP_BASE_DIR}/util/binding-table.cpp
         ${CHIP_APP_BASE_DIR}/util/DataModelHandler.cpp
-        ${CHIP_APP_BASE_DIR}/util/ember-global-attribute-access-interface.cpp
         ${CHIP_APP_BASE_DIR}/util/ember-io-storage.cpp
         ${CHIP_APP_BASE_DIR}/util/generic-callback-stubs.cpp
         ${CHIP_APP_BASE_DIR}/util/privilege-storage.cpp

src/app/chip_data_model.gni

@@ -209,7 +209,6 @@ template("chip_data_model") {
         "${_app_root}/util/DataModelHandler.cpp",
         "${_app_root}/util/attribute-storage.cpp",
         "${_app_root}/util/attribute-table.cpp",
-        "${_app_root}/util/ember-global-attribute-access-interface.cpp",
         "${_app_root}/util/ember-io-storage.cpp",
         "${_app_root}/util/util.cpp",
       ]

src/app/data-model-provider/MetadataLookup.cpp

@@ -82,6 +82,27 @@ std::optional<EndpointEntry> EndpointFinder::Find(EndpointId endpointId)
     return std::nullopt;
 }
 
+Protocols::InteractionModel::Status ValidateClusterPath(ProviderMetadataTree * provider, const ConcreteClusterPath & path,
+                                                        Protocols::InteractionModel::Status successStatus)
+{
+    if (ServerClusterFinder(provider).Find(path).has_value())
+    {
+        return successStatus;
+    }
+
+    auto endpoints = provider->EndpointsIgnoreError();
+    for (auto & endpointEntry : endpoints)
+    {
+        if (endpointEntry.id == path.mEndpointId)
+        {
+            // endpoint is valid
+            return Protocols::InteractionModel::Status::UnsupportedCluster;
+        }
+    }
+
+    return Protocols::InteractionModel::Status::UnsupportedEndpoint;
+}
+
 } // namespace DataModel
 } // namespace app
 } // namespace chip

src/app/data-model-provider/MetadataLookup.h

@@ -80,6 +80,14 @@ class EndpointFinder
     ReadOnlyBuffer<EndpointEntry> mEndpoints;
 };
 
+/// Validates that the cluster identified by `path` exists within the given provider.
+/// If the endpoint does not exist, will return Status::UnsupportedEndpoint.
+/// If the endpoint exists but does not have the cluster identified by the path, will return Status::UnsupportedCluster.
+///
+/// Otherwise, will return successStatus.
+Protocols::InteractionModel::Status ValidateClusterPath(ProviderMetadataTree * provider, const ConcreteClusterPath & path,
+                                                        Protocols::InteractionModel::Status successStatus);
+
 } // namespace DataModel
 } // namespace app
 } // namespace chip

src/app/data-model-provider/Provider.h

@@ -58,8 +58,12 @@ class Provider : public ProviderMetadataTree
     // event emitting, path marking and other operations
     virtual InteractionModelContext CurrentContext() const { return mContext; }
 
-    /// TEMPORARY/TRANSITIONAL requirement for transitioning from ember-specific code
-    ///   ReadAttribute is REQUIRED to respond to GlobalAttribute read requests
+    /// NOTE: this code is NOT required to handle `List` global attributes:
+    ///       AcceptedCommandsList, GeneratedCommandsList OR AttributeList
+    ///
+    ///       Users of DataModel::Provider are expected to get these lists
+    ///       from ProviderMetadataTree (in particular IM Reads of these
+    ///       attributes will the automatically filled from metadata).
     ///
     /// Return value notes:
     ///   ActionReturnStatus::IsOutOfSpaceEncodingResponse

src/app/data-model-provider/ProviderMetadataTree.h

@@ -60,6 +60,14 @@ class ProviderMetadataTree
     virtual CHIP_ERROR ClientClusters(EndpointId endpointId, ListBuilder<ClusterId> & builder)          = 0;
     virtual CHIP_ERROR ServerClusters(EndpointId endpointId, ListBuilder<ServerClusterEntry> & 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
+    ///    - AcceptedCommandList::Id
+    ///    - GeneratedCommandList::Id
+    /// However it MUST ALWAYS contain:
+    ///    - ClusterRevision::Id
+    ///    - FeatureMap::Id
     virtual CHIP_ERROR Attributes(const ConcreteClusterPath & path, ListBuilder<AttributeEntry> & builder)             = 0;
     virtual CHIP_ERROR GeneratedCommands(const ConcreteClusterPath & path, ListBuilder<CommandId> & builder)           = 0;
     virtual CHIP_ERROR AcceptedCommands(const ConcreteClusterPath & path, ListBuilder<AcceptedCommandEntry> & builder) = 0;

src/app/data-model-provider/StringBuilderAdapters.cpp

@@ -28,3 +28,15 @@ StatusWithSize ToString<chip::app::DataModel::ActionReturnStatus>(const chip::ap
 }
 
 } // namespace pw
+//
+#if CHIP_CONFIG_TEST_GOOGLETEST
+namespace chip {
+
+void PrintTo(const chip::app::DataModel::ActionReturnStatus & status, std::ostream * os)
+{
+    chip::app::DataModel::ActionReturnStatus::StringStorage storage;
+    *os << "ActionReturnStatus<" << status.c_str(storage) << ">";
+}
+
+} // namespace chip
+#endif // CHIP_CONFIG_TEST_GOOGLETEST

src/app/data-model-provider/StringBuilderAdapters.h

@@ -34,6 +34,7 @@
 /// which is not as helpful as a full formatted output.
 
 #include <pw_string/string_builder.h>
+#include <pw_unit_test/framework.h>
 
 #include <app/data-model-provider/ActionReturnStatus.h>
 
@@ -44,3 +45,11 @@ StatusWithSize ToString<chip::app::DataModel::ActionReturnStatus>(const chip::ap
                                                                   pw::span<char> buffer);
 
 } // namespace pw
+
+#if CHIP_CONFIG_TEST_GOOGLETEST
+namespace chip {
+
+void PrintTo(const chip::app::DataModel::ActionReturnStatus & status, std::ostream * os);
+
+} // namespace chip
+#endif // CHIP_CONFIG_TEST_GOOGLETEST