Merge branch 'attribute-path-expand-state' of github.com:andy31415/connectedhomeip into attribute-path-expand-state

Author: andy31415

src/app/AttributePathExpandIterator.cpp

@@ -31,17 +31,17 @@ bool AttributePathExpandIterator::AdvanceOutputPath()
     /// Output path invariants
     ///    - kInvalid* constants are used to define "no value available (yet)" and
     ///      iteration loop will fill the first value when such a value is seen (fixed for non-wildcard
-    ///      or iteration-based in case of wildcadrs)
-    ///    - Fill of the output path is done in order: first endpoint, then cluster, then attribute.
+    ///      or iteration-based in case of wildcards).
+    ///    - Iteration of the output path is done in breadth-first order: first endpoint, then cluster, then attribute.
     /// Processing works like:
-    ///    - First state will start of as kInvalidEndpointId/kInvalidClusterId/kInvalidAttributeId
-    ///    - First loop pass fills in endointID, followed by clusterId, followed by attributeID
-    ///    - whenever one item fails to continue iterating (there is no "next") the following
+    ///    - Initial state is kInvalidEndpointId/kInvalidClusterId/kInvalidAttributeId
+    ///    - First loop pass fills-in endointID, followed by clusterID, followed by attributeID
+    ///    - Whenever one level is done iterating (there is no "next") the following
     ///      "higher path component" is updated:
     ///         - once a valid path exists, try to advance attributeID
     ///         - if attributeID fails to advance, try to advance clusterID (and restart attributeID)
     ///         - if clusterID fails to advance, try to advance endpointID (and restart clusterID)
-    ///         - if endpointID fails to advance, assume iteration done
+    ///         - if endpointID fails to advance, iteration is done
     while (true)
     {
         if (mPosition.mOutputPath.mClusterId != kInvalidClusterId)

src/app/AttributePathExpandIterator.h

@@ -31,31 +31,30 @@ namespace app {
 ///
 /// - Start iterating by creating an iteration state
 ///
-///      mPosition =  AttributePathExpandIterator::Position::StartIterating(path);
+///      AttributePathExpandIterator::Position position = AttributePathExpandIterator::Position::StartIterating(path);
 ///
 /// - Use the iteration state in a for loop:
 ///
 ///      ConcreteAttributePath path;
-///      for (AttributePathExpandIterator iterator(mPosition); iterator->Next(path);) {
+///      for (AttributePathExpandIterator iterator(position); iterator->Next(path);) {
 ///         // use `path` here`
 ///      }
 ///
 ///   OR:
 ///
 ///      ConcreteAttributePath path;
-///      AttributePathExpandIterator iterator(mPosition);
+///      AttributePathExpandIterator iterator(position);
 ///
 ///      while (iterator.Next(path)) {
 ///         // use `path` here`
 ///      }
 ///
-/// USAGE requirements and assumptions:
+/// Usage requirements and assumptions:
 ///
-///    - There should be only one single AttributePathExpandIterator for a state  at a time.
-///
-///    - `Position` is automatically updated by the AttributePathExpandIterator, so
-///      calling `Next` on the iterator will update the state variable.
+///    - An ` AttributePathExpandIterator::Position` can only be used by a single AttributePathExpandIterator at a time.
 ///
+///    - `position` is automatically updated by the AttributePathExpandIterator, so
+///      calling `Next` on the iterator will update the position cursor variable.
 ///
 class AttributePathExpandIterator
 {
@@ -65,7 +64,7 @@ class AttributePathExpandIterator
     public:
         // Position is treated as a direct member access by the AttributePathExpandIterator, however it is opaque (except copying)
         // for external code. We allow friendship here to not have specific get/set for methods (clearer interface and less
-        // likelyhood of extra code usage).
+        // likelihood of extra code usage).
         friend class AttributePathExpandIterator;
 
         /// External callers can only ever start iterating on a new path from the beginning
@@ -100,7 +99,7 @@ class AttributePathExpandIterator
         ConcreteAttributePath mOutputPath;
     };
 
-    AttributePathExpandIterator(DataModel::Provider * dataModel, Position & state) : mDataModelProvider(dataModel), mPosition(state)
+    AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position) : mDataModelProvider(dataModel), mPosition(position)
     {}
 
     // This class may not be copied. A new one should be created when needed and they
@@ -151,8 +150,10 @@ class AttributePathExpandIterator
     bool IsValidAttributeId(AttributeId attributeId);
 };
 
-/// Wraps around an AttributePathExpandIterator however it rolls back Next() to one
-/// step back whenever Next() is not run to completion (until it returns false)
+/// PeekAttributePathExpandIterator is an AttributePathExpandIterator wrapper that rolls back the Next()
+/// call whenever a new `MarkCompleted()` method is not called (until Next() returns false). This is useful
+/// to allow pausing iteration in cases the next path was not ready to be used yet and iteration needs to
+/// continue later.
 ///
 /// Example use cases:
 ///

src/app/ReadHandler.cpp

@@ -857,7 +857,7 @@ void ReadHandler::AttributePathIsDirty(DataModel::Provider * apDataModel, const
 {
     mDirtyGeneration = mManagementCallback.GetInteractionModelEngine()->GetReportingEngine().GetDirtySetGeneration();
 
-    // we want to get the value, but not advance the state
+    // We want to get the value, but not advance the iterator position.
     AttributePathExpandIterator::Position tempPosition = mAttributePathExpandPosition;
     ConcreteAttributePath path;
 

src/app/ReadHandler.h

@@ -566,7 +566,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
     // The last schedule event number snapshoted in the beginning when preparing to fill new events to reports
     EventNumber mLastScheduledEventNumber = 0;
 
-    /// State for any on-going path expansion for handling wildcard reads/subscriptions
+    /// Iterator position state for any on-going path expansion for handling wildcard reads/subscriptions.
     AttributePathExpandIterator::Position mAttributePathExpandPosition;
 
     Messaging::ExchangeHolder mExchangeCtx;