[IM] Update AttributePathParams for encoding a path with wildcard pat…

…hs (#11459)

* Update AttributePathParams

* Codegen

src/app/AttributePathParams.cpp

@@ -0,0 +1,52 @@
+/*
+ *
+ * Copyright (c) 2021 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/AttributePathParams.h>
+#include <app/MessageDef/AttributePathIB.h>
+
+namespace chip {
+namespace app {
+
+CHIP_ERROR AttributePathParams::BuildAttributePath(AttributePathIB::Builder & aBuilder) const
+{
+ if (!HasWildcardEndpointId())
+ {
+ aBuilder.Endpoint(mEndpointId);
+ }
+
+ if (!HasWildcardClusterId())
+ {
+ aBuilder.Cluster(mClusterId);
+ }
+
+ if (!HasWildcardAttributeId())
+ {
+ aBuilder.Attribute(mFieldId);
+ }
+
+ if (!HasWildcardListIndex())
+ {
+ aBuilder.ListIndex(mListIndex);
+ }
+
+ aBuilder.EndOfAttributePathIB();
+
+ return aBuilder.GetError();
+}
+} // namespace app
+} // namespace chip

src/app/AttributePathParams.h

@@ -19,48 +19,54 @@
 #pragma once
 
 #include <app/util/basic-types.h>
-#include <lib/support/BitFlags.h>
+
+#include <app/ClusterInfo.h>
+#include <app/MessageDef/AttributePathIB.h>
 
 namespace chip {
 namespace app {
 struct AttributePathParams
 {
- enum class Flags : uint8_t
- {
- kFieldIdValid = 0x01,
- kListIndexValid = 0x02,
- };
-
  //
  // TODO: (Issue #10596) Need to ensure that we do not encode the NodeId over the wire
  // if it is either not 'set', or is set to a value that matches accessing fabric
  // on which the interaction is undertaken.
  //
- AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId) : AttributePathParams(0, aEndpointId, aClusterId, 0, 0, {}) {}
+ // TODO: (#11420) This class is overlapped with ClusterInfo class, need to do a clean up.
+ AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId) :
+ AttributePathParams(aEndpointId, aClusterId, ClusterInfo::kInvalidAttributeId, ClusterInfo::kInvalidListIndex)
+ {}
 
  AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aFieldId) :
- AttributePathParams(0, aEndpointId, aClusterId, aFieldId, 0, chip::app::AttributePathParams::Flags::kFieldIdValid)
+ AttributePathParams(aEndpointId, aClusterId, aFieldId, ClusterInfo::kInvalidListIndex)
  {}
 
  AttributePathParams(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aFieldId, ListIndex aListIndex) :
- AttributePathParams(0, aEndpointId, aClusterId, aFieldId, aListIndex,
- BitFlags<Flags>(chip::app::AttributePathParams::Flags::kFieldIdValid,
- chip::app::AttributePathParams::Flags::kListIndexValid))
- {}
-
- AttributePathParams(NodeId aNodeId, EndpointId aEndpointId, ClusterId aClusterId, AttributeId aFieldId, ListIndex aListIndex,
- const BitFlags<Flags> aFlags) :
- mNodeId(aNodeId),
- mEndpointId(aEndpointId), mClusterId(aClusterId), mFieldId(aFieldId), mListIndex(aListIndex), mFlags(aFlags)
+ mEndpointId(aEndpointId), mClusterId(aClusterId), mFieldId(aFieldId), mListIndex(aListIndex)
  {}
 
  AttributePathParams() {}
- NodeId mNodeId = 0;
- EndpointId mEndpointId = 0;
- ClusterId mClusterId = 0;
- AttributeId mFieldId = 0;
- ListIndex mListIndex = 0;
- BitFlags<Flags> mFlags;
+
+ CHIP_ERROR BuildAttributePath(AttributePathIB::Builder & aBuilder) const;
+
+ bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); }
+
+ /**
+ * Check that the path meets some basic constraints of an attribute path: If list index is not wildcard, then field id must not
+ * be wildcard. This does not verify that the attribute being targeted is actually of list type when the list index is not
+ * wildcard.
+ */
+ bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); }
+
+ inline bool HasWildcardEndpointId() const { return mEndpointId == ClusterInfo::kInvalidEndpointId; }
+ inline bool HasWildcardClusterId() const { return mClusterId == ClusterInfo::kInvalidClusterId; }
+ inline bool HasWildcardAttributeId() const { return mFieldId == ClusterInfo::kInvalidAttributeId; }
+ inline bool HasWildcardListIndex() const { return mListIndex == ClusterInfo::kInvalidListIndex; }
+
+ EndpointId mEndpointId = ClusterInfo::kInvalidEndpointId;
+ ClusterId mClusterId = ClusterInfo::kInvalidClusterId;
+ AttributeId mFieldId = ClusterInfo::kInvalidAttributeId;
+ ListIndex mListIndex = ClusterInfo::kInvalidListIndex;
 };
 } // namespace app
 } // namespace chip

src/app/BUILD.gn

@@ -35,6 +35,7 @@ static_library("app") {
  output_name = "libCHIPDataModel"
 
  sources = [
+ "AttributePathParams.cpp",
  "Command.cpp",
  "Command.h",
  "CommandHandler.cpp",

src/app/ClusterInfo.h

@@ -35,6 +35,8 @@ namespace app {
 struct ClusterInfo
 {
 private:
+ // Allow AttributePathParams access these constants.
+ friend struct AttributePathParams;
  // Endpoint Id is a uint16 number, and should between 0 and 0xFFFE
  static constexpr EndpointId kInvalidEndpointId = 0xFFFF;
  // The ClusterId, AttributeId and EventId are MEIs,

src/app/ReadClient.cpp

@@ -259,24 +259,8 @@ CHIP_ERROR ReadClient::GenerateAttributePathList(AttributePaths::Builder & aAttr
 {
  for (size_t index = 0; index < aAttributePathParamsListSize; index++)
  {
- AttributePathIB::Builder attributePathBuilder = aAttributePathListBuilder.CreateAttributePath();
- attributePathBuilder.Node(apAttributePathParamsList[index].mNodeId)
- .Endpoint(apAttributePathParamsList[index].mEndpointId)
- .Cluster(apAttributePathParamsList[index].mClusterId);
- if (apAttributePathParamsList[index].mFlags.Has(AttributePathParams::Flags::kFieldIdValid))
- {
- attributePathBuilder.Attribute(apAttributePathParamsList[index].mFieldId);
- }
-
- if (apAttributePathParamsList[index].mFlags.Has(AttributePathParams::Flags::kListIndexValid))
- {
- VerifyOrReturnError(apAttributePathParamsList[index].mFlags.Has(AttributePathParams::Flags::kFieldIdValid),
- CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
- attributePathBuilder.ListIndex(apAttributePathParamsList[index].mListIndex);
- }
-
- attributePathBuilder.EndOfAttributePathIB();
- ReturnErrorOnFailure(attributePathBuilder.GetError());
+ VerifyOrReturnError(apAttributePathParamsList[index].IsValidAttributePath(), CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
+ ReturnErrorOnFailure(apAttributePathParamsList[index].BuildAttributePath(aAttributePathListBuilder.CreateAttributePath()));
  }
  aAttributePathListBuilder.EndOfAttributePaths();
  return aAttributePathListBuilder.GetError();

src/app/WriteClient.cpp

@@ -171,23 +171,10 @@ TLV::TLVWriter * WriteClient::GetAttributeDataElementTLVWriter()
 CHIP_ERROR WriteClient::ConstructAttributePath(const AttributePathParams & aAttributePathParams,
  AttributeDataElement::Builder aAttributeDataElement)
 {
- AttributePathIB::Builder attributePath = aAttributeDataElement.CreateAttributePath();
- if (aAttributePathParams.mFlags.Has(AttributePathParams::Flags::kFieldIdValid))
- {
- attributePath.Attribute(aAttributePathParams.mFieldId);
- }
-
- if (aAttributePathParams.mFlags.Has(AttributePathParams::Flags::kListIndexValid))
- {
- attributePath.ListIndex(aAttributePathParams.mListIndex);
- }
-
- attributePath.Node(aAttributePathParams.mNodeId)
- .Cluster(aAttributePathParams.mClusterId)
- .Endpoint(aAttributePathParams.mEndpointId)
- .EndOfAttributePathIB();
-
- return attributePath.GetError();
+ // We do not support wildcard write now, reject them on client side.
+ VerifyOrReturnError(!aAttributePathParams.HasWildcard() && aAttributePathParams.IsValidAttributePath(),
+ CHIP_ERROR_INVALID_PATH_LIST);
+ return aAttributePathParams.BuildAttributePath(aAttributeDataElement.CreateAttributePath());
 }
 
 CHIP_ERROR WriteClient::FinalizeMessage(System::PacketBufferHandle & aPacket)
@@ -328,31 +315,21 @@ CHIP_ERROR WriteClient::ProcessAttributeStatusIB(AttributeStatusIB::Parser & aAt
  mAttributeStatusIndex++;
  err = aAttributeStatusIB.GetPath(&attributePath);
  SuccessOrExit(err);
- err = attributePath.GetNode(&(attributePathParams.mNodeId));
- SuccessOrExit(err);
  err = attributePath.GetCluster(&(attributePathParams.mClusterId));
  SuccessOrExit(err);
  err = attributePath.GetEndpoint(&(attributePathParams.mEndpointId));
  SuccessOrExit(err);
-
  err = attributePath.GetAttribute(&(attributePathParams.mFieldId));
- if (CHIP_NO_ERROR == err)
- {
- attributePathParams.mFlags.Set(AttributePathParams::Flags::kFieldIdValid);
- }
- else if (CHIP_END_OF_TLV == err)
- {
- err = CHIP_NO_ERROR;
- }
  SuccessOrExit(err);
-
  err = attributePath.GetListIndex(&(attributePathParams.mListIndex));
- if (CHIP_NO_ERROR == err)
+ if (err == CHIP_END_OF_TLV)
  {
- VerifyOrExit(attributePathParams.mFlags.Has(AttributePathParams::Flags::kFieldIdValid),
- err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
- attributePathParams.mFlags.Set(AttributePathParams::Flags::kListIndexValid);
+ err = CHIP_NO_ERROR;
  }
+ // TODO: (#11423) Attribute paths has a pattern of invalid paths, should add a function for checking invalid paths here.
+ // NOTE: We don't support wildcard write for now, reject all wildcard paths.
+ VerifyOrExit(!attributePathParams.HasWildcard() && attributePathParams.IsValidAttributePath(),
+ err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
 
  err = aAttributeStatusIB.GetErrorStatus(&(StatusIBParser));
  if (CHIP_NO_ERROR == err)

src/app/WriteHandler.cpp

@@ -215,22 +215,7 @@ CHIP_ERROR WriteHandler::ConstructAttributePath(const AttributePathParams & aAtt
  AttributeStatusIB::Builder aAttributeStatusIB)
 {
  AttributePathIB::Builder attributePath = aAttributeStatusIB.CreatePath();
- if (aAttributePathParams.mFlags.Has(AttributePathParams::Flags::kFieldIdValid))
- {
- attributePath.Attribute(aAttributePathParams.mFieldId);
- }
-
- if (aAttributePathParams.mFlags.Has(AttributePathParams::Flags::kListIndexValid))
- {
- attributePath.ListIndex(aAttributePathParams.mListIndex);
- }
-
- attributePath.Node(aAttributePathParams.mNodeId)
- .Cluster(aAttributePathParams.mClusterId)
- .Endpoint(aAttributePathParams.mEndpointId)
- .EndOfAttributePathIB();
-
- return attributePath.GetError();
+ return aAttributePathParams.BuildAttributePath(attributePath);
 }
 
 CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathParams,