Switch DataModel::Provider to a span-based list iterator - flash and ram savings, simpler interface

examples/common/pigweed/rpc_services/Attributes.h

@@ -27,6 +27,7 @@
 #include <app/InteractionModelEngine.h>
 #include <app/MessageDef/AttributeReportIBs.h>
 #include <app/data-model-provider/ActionReturnStatus.h>
+#include <app/data-model-provider/MetadataLookup.h>
 #include <app/data-model-provider/OperationTypes.h>
 #include <app/data-model-provider/Provider.h>
 #include <app/util/attribute-storage.h>
@@ -221,7 +222,9 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
         request.operationFlags.Set(app::DataModel::OperationFlags::kInternal);
         request.subjectDescriptor = &subjectDescriptor;
 
-        std::optional<app::DataModel::ClusterInfo> info = provider->GetServerClusterInfo(path);
+        app::DataModel::ServerClusterFinder serverClusterFinder(provider);
+        auto info = serverClusterFinder.Find(path);
+
         if (!info.has_value())
         {
             return ::pw::Status::NotFound();

src/access/ProviderDeviceTypeResolver.h

@@ -31,10 +31,10 @@ class DynamicProviderDeviceTypeResolver : public chip::Access::AccessControl::De
 
     bool IsDeviceTypeOnEndpoint(chip::DeviceTypeId deviceType, chip::EndpointId endpoint) override
     {
-        app::DataModel::Provider * model = mModelGetter();
-        for (auto type = model->FirstDeviceType(endpoint); type.has_value(); type = model->NextDeviceType(endpoint, *type))
+        auto deviceTypes = mModelGetter()->DeviceTypes(endpoint);
+        for (auto & type : deviceTypes.GetSpanValidForLifetime())
         {
-            if (type->deviceTypeId == deviceType)
+            if (type.deviceTypeId == deviceType)
             {
                 return true;
             }

src/app/AttributePathExpandIterator.cpp

@@ -17,6 +17,9 @@
 #include <app/AttributePathExpandIterator.h>
 
 #include <app/GlobalAttributes.h>
+#include <app/data-model-provider/MetadataLookup.h>
+#include <app/data-model-provider/MetadataTypes.h>
+#include <lib/core/DataModelTypes.h>
 #include <lib/support/CodeUtils.h>
 
 #include <optional>
@@ -26,6 +29,10 @@ using namespace chip::app::DataModel;
 namespace chip {
 namespace app {
 
+AttributePathExpandIterator::AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position) :
+    mDataModelProvider(dataModel), mPosition(position)
+{}
+
 bool AttributePathExpandIterator::AdvanceOutputPath()
 {
     /// Output path invariants
@@ -113,34 +120,58 @@ bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId)
         break;
     }
 
+    DataModel::AttributeFinder finder(mDataModelProvider);
+
     const ConcreteAttributePath attributePath(mPosition.mOutputPath.mEndpointId, mPosition.mOutputPath.mClusterId, attributeId);
-    return mDataModelProvider->GetAttributeInfo(attributePath).has_value();
+    return finder.Find(attributePath).has_value();
 }
 
 std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
 {
     if (mPosition.mOutputPath.mAttributeId == kInvalidAttributeId)
     {
-        if (mPosition.mAttributePath->mValue.HasWildcardAttributeId())
+        mAttributeIndex = kInvalidIndex;
+    }
+
+    if (mAttributeIndex == kInvalidIndex)
+    {
+        // start a new iteration of attributes on the current cluster path.
+        mAttributes = mDataModelProvider->Attributes(mPosition.mOutputPath);
+
+        if (mPosition.mOutputPath.mAttributeId != kInvalidAttributeId)
         {
-            AttributeEntry entry = mDataModelProvider->FirstAttribute(mPosition.mOutputPath);
-            return entry.IsValid()                                         //
-                ? entry.path.mAttributeId                                  //
-                : Clusters::Globals::Attributes::GeneratedCommandList::Id; //
+            // Position on the correct attribute if we have a start point
+            mAttributeIndex = 0;
+            while ((mAttributeIndex < mAttributes.Size()) &&
+                   (mAttributes[mAttributeIndex].attributeId != mPosition.mOutputPath.mAttributeId))
+            {
+                mAttributeIndex++;
+            }
         }
+    }
 
-        // At this point, the attributeID is NOT a wildcard (i.e. it is fixed).
-        //
-        // For wildcard expansion, we validate that this is a valid attribute for the given
-        // cluster on the given endpoint. If not a wildcard expansion, return it as-is.
-        if (mPosition.mAttributePath->mValue.IsWildcardPath())
+    if (mPosition.mOutputPath.mAttributeId == kInvalidAttributeId)
+    {
+        if (!mPosition.mAttributePath->mValue.HasWildcardAttributeId())
         {
-            if (!IsValidAttributeId(mPosition.mAttributePath->mValue.mAttributeId))
+            // The attributeID is NOT a wildcard (i.e. it is fixed).
+            //
+            // For wildcard expansion, we validate that this is a valid attribute for the given
+            // cluster on the given endpoint. If not a wildcard expansion, return it as-is.
+            if (mPosition.mAttributePath->mValue.IsWildcardPath())
             {
-                return std::nullopt;
+                if (!IsValidAttributeId(mPosition.mAttributePath->mValue.mAttributeId))
+                {
+                    return std::nullopt;
+                }
             }
+            return mPosition.mAttributePath->mValue.mAttributeId;
         }
-        return mPosition.mAttributePath->mValue.mAttributeId;
+        mAttributeIndex = 0;
+    }
+    else
+    {
+        mAttributeIndex++;
     }
 
     // Advance the existing attribute id if it can be advanced.
@@ -165,10 +196,9 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
         return std::nullopt;
     }
 
-    AttributeEntry entry = mDataModelProvider->NextAttribute(mPosition.mOutputPath);
-    if (entry.IsValid())
+    if (mAttributeIndex < mAttributes.Size())
     {
-        return entry.path.mAttributeId;
+        return mAttributes[mAttributeIndex].attributeId;
     }
 
     // Finished the data model, start with global attributes
@@ -178,55 +208,110 @@ std::optional<AttributeId> AttributePathExpandIterator::NextAttributeId()
 
 std::optional<ClusterId> AttributePathExpandIterator::NextClusterId()
 {
-
     if (mPosition.mOutputPath.mClusterId == kInvalidClusterId)
     {
-        if (mPosition.mAttributePath->mValue.HasWildcardClusterId())
+        mClusterIndex = kInvalidIndex;
+    }
+
+    if (mClusterIndex == kInvalidIndex)
+    {
+        // start a new iteration on the current endpoint
+        mClusters = mDataModelProvider->ServerClusters(mPosition.mOutputPath.mEndpointId);
+
+        if (mPosition.mOutputPath.mClusterId != kInvalidClusterId)
         {
-            ClusterEntry entry = mDataModelProvider->FirstServerCluster(mPosition.mOutputPath.mEndpointId);
-            return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt;
+            // Position on the correct cluster if we have a start point
+            mClusterIndex = 0;
+            while ((mClusterIndex < mClusters.Size()) && (mClusters[mClusterIndex].clusterId != mPosition.mOutputPath.mClusterId))
+            {
+                mClusterIndex++;
+            }
         }
+    }
 
-        // At this point, the clusterID is NOT a wildcard (i.e. is fixed).
-        //
-        // For wildcard expansion, we validate that this is a valid cluster for the endpoint.
-        // If non-wildcard expansion, we return as-is.
-        if (mPosition.mAttributePath->mValue.IsWildcardPath())
+    if (mPosition.mOutputPath.mClusterId == kInvalidClusterId)
+    {
+
+        if (!mPosition.mAttributePath->mValue.HasWildcardClusterId())
         {
-            const ConcreteClusterPath clusterPath(mPosition.mOutputPath.mEndpointId, mPosition.mAttributePath->mValue.mClusterId);
-            if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value())
+            // The clusterID is NOT a wildcard (i.e. is fixed).
+            //
+            // For wildcard expansion, we validate that this is a valid cluster for the endpoint.
+            // If non-wildcard expansion, we return as-is.
+            if (mPosition.mAttributePath->mValue.IsWildcardPath())
             {
-                return std::nullopt;
+                const ClusterId clusterId = mPosition.mAttributePath->mValue.mClusterId;
+
+                auto span = mClusters.GetSpanValidForLifetime();
+
+                bool found = false;
+                for (auto & entry : span)
+                {
+                    if (entry.clusterId == clusterId)
+                    {
+                        found = true;
+                        break;
+                    }
+                }
+
+                if (!found)
+                {
+                    return std::nullopt;
+                }
             }
-        }
 
-        return mPosition.mAttributePath->mValue.mClusterId;
+            return mPosition.mAttributePath->mValue.mClusterId;
+        }
+        mClusterIndex = 0;
+    }
+    else
+    {
+        mClusterIndex++;
     }
 
     VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardClusterId(), std::nullopt);
+    VerifyOrReturnValue(mClusterIndex < mClusters.Size(), std::nullopt);
 
-    ClusterEntry entry = mDataModelProvider->NextServerCluster(mPosition.mOutputPath);
-    return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt;
+    return mClusters[mClusterIndex].clusterId;
 }
 
-std::optional<ClusterId> AttributePathExpandIterator::NextEndpointId()
+std::optional<EndpointId> AttributePathExpandIterator::NextEndpointId()
 {
+    if (mEndpointIndex == kInvalidIndex)
+    {
+        // index is missing, have to start a new iteration
+        mEndpoints = mDataModelProvider->Endpoints();
+
+        if (mPosition.mOutputPath.mEndpointId != kInvalidEndpointId)
+        {
+            // Position on the correct endpoint if we have a start point
+            mEndpointIndex = 0;
+            while ((mEndpointIndex < mEndpoints.Size()) && (mEndpoints[mEndpointIndex].id != mPosition.mOutputPath.mEndpointId))
+            {
+                mEndpointIndex++;
+            }
+        }
+    }
+
     if (mPosition.mOutputPath.mEndpointId == kInvalidEndpointId)
     {
-        if (mPosition.mAttributePath->mValue.HasWildcardEndpointId())
+        if (!mPosition.mAttributePath->mValue.HasWildcardEndpointId())
         {
-            EndpointEntry ep = mDataModelProvider->FirstEndpoint();
-            return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
+            return mPosition.mAttributePath->mValue.mEndpointId;
         }
 
-        return mPosition.mAttributePath->mValue.mEndpointId;
+        // start from the beginning
+        mEndpointIndex = 0;
+    }
+    else
+    {
+        mEndpointIndex++;
     }
 
-    // Expand endpoints only if it is a wildcard on the endpoint specifically.
     VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardEndpointId(), std::nullopt);
+    VerifyOrReturnValue(mEndpointIndex < mEndpoints.Size(), std::nullopt);
 
-    EndpointEntry ep = mDataModelProvider->NextEndpoint(mPosition.mOutputPath.mEndpointId);
-    return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt;
+    return mEndpoints[mEndpointIndex].id;
 }
 
 } // namespace app

src/app/AttributePathExpandIterator.h

@@ -19,9 +19,14 @@
 
 #include <app/AttributePathParams.h>
 #include <app/ConcreteAttributePath.h>
+#include <app/data-model-provider/MetadataList.h>
+#include <app/data-model-provider/MetadataTypes.h>
 #include <app/data-model-provider/Provider.h>
 #include <lib/core/DataModelTypes.h>
 #include <lib/support/LinkedList.h>
+#include <lib/support/Span.h>
+
+#include <limits>
 
 namespace chip {
 namespace app {
@@ -96,9 +101,7 @@ class AttributePathExpandIterator
         ConcreteAttributePath mOutputPath;
     };
 
-    AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position) :
-        mDataModelProvider(dataModel), mPosition(position)
-    {}
+    AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position);
 
     // This class may not be copied. A new one should be created when needed and they
     // should not overlap.
@@ -113,9 +116,20 @@ class AttributePathExpandIterator
     bool Next(ConcreteAttributePath & path);
 
 private:
+    static constexpr size_t kInvalidIndex = std::numeric_limits<size_t>::max();
+
     DataModel::Provider * mDataModelProvider;
     Position & mPosition;
 
+    DataModel::MetadataList<DataModel::EndpointEntry> mEndpoints; // all endpoints
+    size_t mEndpointIndex = kInvalidIndex;
+
+    DataModel::MetadataList<DataModel::ServerClusterEntry> mClusters; // all clusters ON THE CURRENT endpoint
+    size_t mClusterIndex = kInvalidIndex;
+
+    DataModel::MetadataList<DataModel::AttributeEntry> mAttributes; // all attributes ON THE CURRENT cluster
+    size_t mAttributeIndex = kInvalidIndex;
+
     /// Move to the next endpoint/cluster/attribute triplet that is valid given
     /// the current mOutputPath and mpAttributePath.
     ///
@@ -140,7 +154,7 @@ class AttributePathExpandIterator
     /// Will start from the beginning if current mOutputPath.mEndpointId is kInvalidEndpointId
     ///
     /// Respects path expansion/values in mpAttributePath
-    std::optional<ClusterId> NextEndpointId();
+    std::optional<EndpointId> NextEndpointId();
 
     /// Checks if the given attributeId is valid for the current mOutputPath(endpoint/cluster)
     ///

src/app/AttributeValueEncoder.h

@@ -64,6 +64,34 @@ class AttributeValueEncoder
             return mAttributeValueEncoder.EncodeListItem(mCheckpoint, aArg);
         }
 
+        // overrides that save flash: no need to care about the extra const
+        // Without this, we have a usage of:
+        //   chip::ChipError chip::app::AttributeValueEncoder::EncodeListItem<unsigned long const&>
+        // Overall we tend to have very similar code expand (from nm):
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned char>(unsigned char&&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned char&>(unsigned char&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned char const&>(unsigned char const&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned short const&>(unsigned short const&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned long&>(unsigned long&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned short&>(unsigned short&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned long long&>(unsigned long long&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned short>(unsigned short&&)
+        //    chip::ChipError chip::app::AttributeValueEncoder::Encode<unsigned long long>(unsigned long long&&)
+        // that we try to reduce
+        //
+        // TODO:
+        //   - we should figure where the extra const override is used
+        //   - we should try to avoid having such footguns. This list template-explosion seems
+        //     dangerous for flash.
+        //
+        // This relies on TLV numbers always being encoded as 64-bit value
+        inline CHIP_ERROR Encode(uint32_t const & aArg) const { return Encode<uint64_t>(aArg); }
+        inline CHIP_ERROR Encode(uint32_t & aArg) const { return Encode<uint64_t>(aArg); }
+        inline CHIP_ERROR Encode(uint16_t const & aArg) const { return Encode<uint64_t>(aArg); }
+        inline CHIP_ERROR Encode(uint16_t & aArg) const { return Encode<uint64_t>(aArg); }
+        inline CHIP_ERROR Encode(uint8_t const & aArg) const { return Encode<uint64_t>(aArg); }
+        inline CHIP_ERROR Encode(uint8_t & aArg) const { return Encode<uint64_t>(aArg); }
+
     private:
         AttributeValueEncoder & mAttributeValueEncoder;
         // Avoid calling the TLVWriter constructor for every instantiation of