Restyle [im] Fix wildcard path support per spec

src/app/ClusterInfo.h

@@ -25,96 +25,37 @@
 
 namespace chip {
 namespace app {
-static constexpr AttributeId kRootAttributeId = 0xFFFFFFFF;
 
+/**
+ * ClusterInfo is the representation of an attribute path or an event path used by ReacHandler, ReadClient, WriteHandler,
+ * Report::Engine etc, it contains a mpNext field so it can be used as a linked list. It uses some invalid values for representing
+ * the wildcard value for its field.
+ */
+// TODO: The cluster info should be breaked into AttributeInfo and EventInfo.
+// Note: The change will happen after #11171 with a better linked list.
 struct ClusterInfo
 {
-    enum class Flags : uint8_t
-    {
-        kFieldIdValid   = 0x01,
-        kListIndexValid = 0x02,
-        kEventIdValid   = 0x03,
-    };
-
     bool IsAttributePathSupersetOf(const ClusterInfo & other) const
     {
-        if ((other.mEndpointId != mEndpointId) || (other.mClusterId != mClusterId))
-        {
-            return false;
-        }
-
-        Optional<AttributeId> myFieldId =
-            mFlags.Has(Flags::kFieldIdValid) ? Optional<AttributeId>::Value(mFieldId) : Optional<AttributeId>::Missing();
-
-        Optional<AttributeId> otherFieldId = other.mFlags.Has(Flags::kFieldIdValid) ? Optional<AttributeId>::Value(other.mFieldId)
-                                                                                    : Optional<AttributeId>::Missing();
-
-        Optional<ListIndex> myListIndex =
-            mFlags.Has(Flags::kListIndexValid) ? Optional<ListIndex>::Value(mListIndex) : Optional<ListIndex>::Missing();
-
-        Optional<ListIndex> otherListIndex = other.mFlags.Has(Flags::kListIndexValid) ? Optional<ListIndex>::Value(other.mListIndex)
-                                                                                      : Optional<ListIndex>::Missing();
+        VerifyOrReturnError(!mEndpointId.HasValue() || mEndpointId == other.mEndpointId, false);
+        VerifyOrReturnError(!mClusterId.HasValue() || mClusterId == other.mClusterId, false);
+        VerifyOrReturnError(!mFieldId.HasValue() || mFieldId == other.mFieldId, false);
+        VerifyOrReturnError(!mListIndex.HasValue() || mListIndex == other.mListIndex, false);
 
-        // If list index exists, field index must exist
-        // Field 0xFFFFFFF (any) &  listindex set is invalid
-        assert(!(myListIndex.HasValue() && !myFieldId.HasValue()));
-        assert(!(otherListIndex.HasValue() && !otherFieldId.HasValue()));
-        assert(!(myFieldId == Optional<AttributeId>::Value(kRootAttributeId) && myListIndex.HasValue()));
-        assert(!(otherFieldId == Optional<AttributeId>::Value(kRootAttributeId) && otherListIndex.HasValue()));
-
-        if (myFieldId == Optional<AttributeId>::Value(kRootAttributeId))
-        {
-            return true;
-        }
-
-        if (myFieldId != otherFieldId)
-        {
-            return false;
-        }
-
-        // We only support top layer for attribute representation, either FieldId or FieldId + ListIndex
-        // Combination: if myFieldId == otherFieldId, ListIndex cannot exist without FieldId
-        // 1. myListIndex and otherListIndex both missing or both exactly the same, then current is superset of other
-        // 2. myListIndex is missing, no matter if otherListIndex is missing or not, then current is superset of other
-        if (myListIndex == otherListIndex)
-        {
-            // either both missing or both exactly the same
-            return true;
-        }
-
-        if (!myListIndex.HasValue())
-        {
-            // difference is ok only if myListIndex is missing
-            return true;
-        }
-
-        return false;
+        return true;
     }
 
+    bool HasWildcard() const { return !mEndpointId.HasValue() || !mClusterId.HasValue() || !mFieldId.HasValue(); }
+
     ClusterInfo() {}
-    NodeId mNodeId         = 0;
-    ClusterId mClusterId   = 0;
-    ListIndex mListIndex   = 0;
-    AttributeId mFieldId   = 0;
-    EndpointId mEndpointId = 0;
-    BitFlags<Flags> mFlags;
-    ClusterInfo * mpNext = nullptr;
-    EventId mEventId     = 0;
-    /* For better structure alignment
-     * Above ordering is by bit-size to ensure least amount of memory alignment padding.
-     * Changing order to something more natural (e.g. clusterid before nodeid) will result
-     * in extra memory alignment padding.
-     * uint64 mNodeId
-     * uint16_t mClusterId
-     * uint16_t mListIndex
-     * uint8_t FieldId
-     * uint8_t EndpointId
-     * uint8_t mDirty
-     * uint8_t mType
-     * uint32_t mpNext
-     * uint16_t EventId
-     * padding 2 bytes
-     */
+
+    Optional<NodeId> mNodeId;
+    Optional<EndpointId> mEndpointId;
+    Optional<ClusterId> mClusterId;
+    Optional<AttributeId> mFieldId;
+    Optional<EventId> mEventId;
+    Optional<ListIndex> mListIndex;
+    ClusterInfo * mpNext = nullptr; // pointer width (uint32 or uint64)
 };
 } // namespace app
 } // namespace chip

src/app/EventManagement.cpp

@@ -618,8 +618,10 @@ static bool IsInterestedEventPaths(EventLoadOutContext * eventLoadOutContext, co
     }
     while (interestedEventPaths != nullptr)
     {
-        if (interestedEventPaths->mNodeId == event.mNodeId && interestedEventPaths->mEndpointId == event.mEndpointId &&
-            interestedEventPaths->mClusterId == event.mClusterId && interestedEventPaths->mEventId == event.mEventId)
+        if (interestedEventPaths->mNodeId == Optional<NodeId>(event.mNodeId) &&
+            interestedEventPaths->mEndpointId == Optional<EndpointId>(event.mEndpointId) &&
+            interestedEventPaths->mClusterId == Optional<ClusterId>(event.mClusterId) &&
+            interestedEventPaths->mEventId == Optional<EventId>(event.mEventId))
         {
             return true;
         }

src/app/InteractionModelEngine.cpp

@@ -471,7 +471,6 @@ void InteractionModelEngine::ReleaseClusterInfoList(ClusterInfo *& aClusterInfo)
     {
         lastClusterInfo = lastClusterInfo->mpNext;
     }
-    lastClusterInfo->mFlags.ClearAll();
     lastClusterInfo->mpNext    = mpNextAvailableClusterInfo;
     mpNextAvailableClusterInfo = aClusterInfo;
     aClusterInfo               = nullptr;
@@ -508,7 +507,6 @@ bool InteractionModelEngine::MergeOverlappedAttributePath(ClusterInfo * apAttrib
         {
             runner->mListIndex = aAttributePath.mListIndex;
             runner->mFieldId   = aAttributePath.mFieldId;
-            runner->mFlags     = aAttributePath.mFlags;
             return true;
         }
         runner = runner->mpNext;

src/app/MessageDef/AttributePath.cpp

@@ -29,6 +29,7 @@
 #include <stdio.h>
 
 #include <app/AppBuildConfig.h>
+#include <lib/core/Optional.h>
 
 using namespace chip;
 using namespace chip::TLV;
@@ -193,6 +194,31 @@ CHIP_ERROR AttributePath::Parser::GetListIndex(chip::ListIndex * const apListInd
     return GetUnsignedInteger(kCsTag_ListIndex, apListIndex);
 }
 
+CHIP_ERROR AttributePath::Parser::GetNodeId(chip::Optional<NodeId> & aNodeId) const
+{
+    return GetUnsignedInteger(kCsTag_NodeId, aNodeId);
+}
+
+CHIP_ERROR AttributePath::Parser::GetEndpointId(chip::Optional<EndpointId> & aEndpointId) const
+{
+    return GetUnsignedInteger(kCsTag_EndpointId, aEndpointId);
+}
+
+CHIP_ERROR AttributePath::Parser::GetClusterId(chip::Optional<ClusterId> & aClusterId) const
+{
+    return GetUnsignedInteger(kCsTag_ClusterId, aClusterId);
+}
+
+CHIP_ERROR AttributePath::Parser::GetFieldId(chip::Optional<FieldId> & aFieldId) const
+{
+    return GetUnsignedInteger(kCsTag_FieldId, aFieldId);
+}
+
+CHIP_ERROR AttributePath::Parser::GetListIndex(chip::Optional<ListIndex> & aListIndex) const
+{
+    return GetUnsignedInteger(kCsTag_ListIndex, aListIndex);
+}
+
 CHIP_ERROR AttributePath::Builder::_Init(chip::TLV::TLVWriter * const apWriter, const Tag aTag)
 {
     mpWriter = apWriter;

src/app/MessageDef/AttributePath.h

@@ -30,6 +30,7 @@
 #include <app/util/basic-types.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/CHIPTLV.h>
+#include <lib/core/Optional.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
 
@@ -84,6 +85,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetNodeId(chip::NodeId * const apNodeId) const;
 
+    /**
+     *  @brief Get NodeId field, will set the aNodeId to Optional::Missing when value does not exist.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetNodeId(chip::Optional<NodeId> & aNodeId) const;
+
     /**
      *  @brief Get a TLVReader for the EndpointId. Next() must be called before accessing them.
      *
@@ -95,6 +106,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetEndpointId(chip::EndpointId * const apEndpointId) const;
 
+    /**
+     *  @brief Gets EndpointId, will set the aEndpointId to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetEndpointId(chip::Optional<EndpointId> & aEndpointId) const;
+
     /**
      *  @brief Get a TLVReader for the ClusterId. Next() must be called before accessing them.
      *
@@ -106,6 +127,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetClusterId(chip::ClusterId * const apClusterId) const;
 
+    /**
+     *  @brief Gets ClusterId, will set the aClusterId to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetClusterId(chip::Optional<ClusterId> & aClusterId) const;
+
     /**
      *  @brief Get a TLVReader for the FieldId. Next() must be called before accessing them.
      *
@@ -117,6 +148,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetFieldId(chip::AttributeId * const apFieldId) const;
 
+    /**
+     *  @brief Gets FieldId, will set the aFieldId to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetFieldId(chip::Optional<AttributeId> & aFieldId) const;
+
     /**
      *  @brief Get a TLVReader for the ListIndex. Next() must be called before accessing them.
      *
@@ -127,6 +168,16 @@ class Parser : public chip::app::Parser
      *          #CHIP_END_OF_TLV if there is no such element
      */
     CHIP_ERROR GetListIndex(chip::ListIndex * const apListIndex) const;
+
+    /**
+     *  @brief Gets ListIndex, will set the aListIndex to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetListIndex(chip::Optional<ListIndex> & aListIndex) const;
 };
 
 class Builder : public chip::app::Builder

src/app/MessageDef/EventPath.cpp

@@ -176,6 +176,26 @@ CHIP_ERROR EventPath::Parser::GetEventId(chip::EventId * const apEventId) const
     return GetUnsignedInteger(kCsTag_EventId, apEventId);
 }
 
+CHIP_ERROR EventPath::Parser::GetNodeId(chip::Optional<NodeId> & aNodeId) const
+{
+    return GetUnsignedInteger(kCsTag_NodeId, aNodeId);
+}
+
+CHIP_ERROR EventPath::Parser::GetEndpointId(chip::Optional<EndpointId> & aEndpointId) const
+{
+    return GetUnsignedInteger(kCsTag_EndpointId, aEndpointId);
+}
+
+CHIP_ERROR EventPath::Parser::GetClusterId(chip::Optional<ClusterId> & aClusterId) const
+{
+    return GetUnsignedInteger(kCsTag_ClusterId, aClusterId);
+}
+
+CHIP_ERROR EventPath::Parser::GetEventId(chip::Optional<EventId> & aFieldId) const
+{
+    return GetUnsignedInteger(kCsTag_EventId, aFieldId);
+}
+
 CHIP_ERROR EventPath::Builder::_Init(chip::TLV::TLVWriter * const apWriter, const Tag aTag)
 {
     mpWriter = apWriter;

src/app/MessageDef/EventPath.h

@@ -84,6 +84,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetNodeId(chip::NodeId * const apNodeId) const;
 
+    /**
+     *  @brief Get NodeId field, will set the aNodeId to Optional::Missing when value does not exist.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetNodeId(chip::Optional<NodeId> & aNodeId) const;
+
     /**
      *  @brief Get a TLVReader for the EndpointId. Next() must be called before accessing them.
      *
@@ -95,6 +105,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetEndpointId(chip::EndpointId * const apEndpointId) const;
 
+    /*
+     *  @brief Gets EndpointId, will set the aEndpointId to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetEndpointId(chip::Optional<EndpointId> & aEndpointId) const;
+
     /**
      *  @brief Get a TLVReader for the ClusterId. Next() must be called before accessing them.
      *
@@ -106,6 +126,16 @@ class Parser : public chip::app::Parser
      */
     CHIP_ERROR GetClusterId(chip::ClusterId * const apClusterId) const;
 
+    /**
+     *  @brief Gets ClusterId, will set the aClusterId to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetClusterId(chip::Optional<ClusterId> & aClusterId) const;
+
     /**
      *  @brief Get a TLVReader for the EventId. Next() must be called before accessing them.
      *
@@ -116,6 +146,16 @@ class Parser : public chip::app::Parser
      *          #CHIP_END_OF_TLV if there is no such element
      */
     CHIP_ERROR GetEventId(chip::EventId * const apEventId) const;
+
+    /**
+     *  @brief Gets FieldId, will set the aFieldId to Optional::Missing when the field is missing.
+     *
+     *  @param [in] aListIndex    The value for getting list index field.
+     *
+     *  @return #CHIP_NO_ERROR on success
+     *          #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
+     */
+    CHIP_ERROR GetEventId(chip::Optional<EventId> & aFieldId) const;
 };
 
 class Builder : public chip::app::Builder