Improve IM Message encoding (#19016)

project-chip · Aug 25, 2023 · 5002275 · 5002275
1 parent 5d174f8
commit 5002275
Show file tree

Hide file tree

Showing 66 changed files with 629 additions and 868 deletions.
diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
@@ -302,7 +302,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
         return AddStatus(concretePath, Protocols::InteractionModel::Status::NeedsTimedInteraction);
     }
 
-    err = aCommandElement.GetData(&commandDataReader);
+    err = aCommandElement.GetFields(&commandDataReader);
     if (CHIP_END_OF_TLV == err)
     {
         ChipLogDetail(DataManagement,
@@ -359,7 +359,7 @@ CHIP_ERROR CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCo
     ChipLogDetail(DataManagement, "Received group command for Group=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI,
                   groupId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId));
 
-    err = aCommandElement.GetData(&commandDataReader);
+    err = aCommandElement.GetFields(&commandDataReader);
     if (CHIP_END_OF_TLV == err)
     {
         ChipLogDetail(DataManagement,
@@ -488,7 +488,7 @@ CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aCommandPa
     ReturnErrorOnFailure(path.Encode(aCommandPath));
     if (aStartDataStruct)
     {
-        ReturnErrorOnFailure(commandData.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)),
+        ReturnErrorOnFailure(commandData.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)),
                                                                      TLV::kTLVType_Structure, mDataElementContainerType));
     }
     MoveToState(State::AddingCommand);

diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h
@@ -319,7 +319,7 @@ class CommandHandler
         ReturnErrorOnFailure(PrepareCommand(path, false));
         TLV::TLVWriter * writer = GetCommandDataIBTLVWriter();
         VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), aData));
+        ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)), aData));
 
         return FinishCommand(/* aEndDataStruct = */ false);
     }

diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp
@@ -274,7 +274,7 @@ CHIP_ERROR CommandSender::ProcessInvokeResponseIB(InvokeResponseIB::Parser & aIn
             ReturnErrorOnFailure(commandPath.GetEndpointId(&endpointId));
             ReturnErrorOnFailure(commandPath.GetClusterId(&clusterId));
             ReturnErrorOnFailure(commandPath.GetCommandId(&commandId));
-            commandData.GetData(&commandDataReader);
+            commandData.GetFields(&commandDataReader);
             err             = CHIP_NO_ERROR;
             hasDataResponse = true;
         }
@@ -336,7 +336,7 @@ CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathP
 
     if (aStartDataStruct)
     {
-        ReturnErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)),
+        ReturnErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)),
                                                                        TLV::kTLVType_Structure, mDataElementContainerType));
     }
 

diff --git a/src/app/CommandSender.h b/src/app/CommandSender.h
@@ -196,7 +196,7 @@ class CommandSender final : public Messaging::ExchangeDelegate
         ReturnErrorOnFailure(PrepareCommand(aCommandPath, /* aStartDataStruct = */ false));
         TLV::TLVWriter * writer = GetCommandDataIBTLVWriter();
         VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), aData));
+        ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)), aData));
         return FinishCommand(aTimedInvokeTimeoutMs);
     }
 

diff --git a/src/app/MessageDef/ArrayParser.cpp b/src/app/MessageDef/ArrayParser.cpp
@@ -24,8 +24,7 @@ CHIP_ERROR ArrayParser::Init(const TLV::TLVReader & aReader)
 {
     mReader.Init(aReader);
     VerifyOrReturnError(TLV::kTLVType_Array == mReader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
-    ReturnErrorOnFailure(mReader.EnterContainer(mOuterContainerType));
-    return CHIP_NO_ERROR;
+    return mReader.EnterContainer(mOuterContainerType);
 }
 } // namespace app
 } // namespace chip
diff --git a/src/app/MessageDef/AttributeDataIB.cpp b/src/app/MessageDef/AttributeDataIB.cpp
@@ -35,7 +35,7 @@ AttributeDataIB::Parser::ParseData(TLV::TLVReader & aReader, int aDepth) const
 
     if (aDepth == 0)
     {
-        PRETTY_PRINT("\tData = ");
+        PRETTY_PRINT("Data = ");
     }
     else
     {
@@ -187,7 +187,7 @@ AttributeDataIB::Parser::ParseData(TLV::TLVReader & aReader, int aDepth) const
 CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
 {
     CHIP_ERROR err      = CHIP_NO_ERROR;
-    int TagPresenceMask = 0;
+    int tagPresenceMask = 0;
     TLV::TLVReader reader;
 
     PRETTY_PRINT("AttributeDataIB =");
@@ -207,8 +207,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
         {
         case to_underlying(Tag::kDataVersion):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kDataVersion))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kDataVersion));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kDataVersion))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kDataVersion));
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
 
 #if CHIP_DETAIL_LOGGING
@@ -221,8 +221,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
             break;
         case to_underlying(Tag::kPath):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kPath));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kPath));
             {
                 AttributePathIB::Parser path;
                 ReturnErrorOnFailure(path.Init(reader));
@@ -234,8 +234,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
             break;
         case to_underlying(Tag::kData):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kData))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kData));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kData))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kData));
 
             PRETTY_PRINT_INCDEPTH();
             ReturnErrorOnFailure(ParseData(reader, 0));
@@ -253,29 +253,20 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
     if (CHIP_END_OF_TLV == err)
     {
         // check for required fields:
-        const int RequiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));
+        const int requiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));
 
-        if ((TagPresenceMask & RequiredFields) == RequiredFields)
-        {
-            err = CHIP_NO_ERROR;
-        }
-        else
-        {
-            err = CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT;
-        }
+        err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_DATA_IB;
     }
     ReturnErrorOnFailure(err);
-    ReturnErrorOnFailure(reader.ExitContainer(mOuterContainerType));
-    return CHIP_NO_ERROR;
+    return reader.ExitContainer(mOuterContainerType);
 }
 #endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
 
 CHIP_ERROR AttributeDataIB::Parser::GetPath(AttributePathIB::Parser * const apPath) const
 {
     TLV::TLVReader reader;
     ReturnErrorOnFailure(mReader.FindElementWithTag(TLV::ContextTag(to_underlying(Tag::kPath)), reader));
-    ReturnErrorOnFailure(apPath->Init(reader));
-    return CHIP_NO_ERROR;
+    return apPath->Init(reader);
 }
 
 CHIP_ERROR AttributeDataIB::Parser::GetDataVersion(chip::DataVersion * const apVersion) const

diff --git a/src/app/MessageDef/AttributeDataIBs.cpp b/src/app/MessageDef/AttributeDataIBs.cpp
@@ -40,7 +40,7 @@ namespace app {
 CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
 {
     CHIP_ERROR err        = CHIP_NO_ERROR;
-    size_t NumDataElement = 0;
+    size_t numDataElement = 0;
     chip::TLV::TLVReader reader;
 
     PRETTY_PRINT("AttributeDataIBs =");
@@ -51,21 +51,19 @@ CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
 
     while (CHIP_NO_ERROR == (err = reader.Next()))
     {
-        VerifyOrExit(chip::TLV::AnonymousTag() == reader.GetTag(), err = CHIP_ERROR_INVALID_TLV_TAG);
-        VerifyOrExit(chip::TLV::kTLVType_Structure == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);
+        VerifyOrReturnError(TLV::AnonymousTag() == reader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG);
+        VerifyOrReturnError(TLV::kTLVType_Structure == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
 
         {
             AttributeDataIB::Parser data;
-            err = data.Init(reader);
-            SuccessOrExit(err);
+            ReturnErrorOnFailure(data.Init(reader));
 
             PRETTY_PRINT_INCDEPTH();
-            err = data.CheckSchemaValidity();
-            SuccessOrExit(err);
+            ReturnErrorOnFailure(data.CheckSchemaValidity());
             PRETTY_PRINT_DECDEPTH();
         }
 
-        ++NumDataElement;
+        ++numDataElement;
     }
 
     PRETTY_PRINT("],");
@@ -75,17 +73,13 @@ CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
     if (CHIP_END_OF_TLV == err)
     {
         // if we have at least one data element
-        if (NumDataElement > 0)
+        if (numDataElement > 0)
         {
             err = CHIP_NO_ERROR;
         }
     }
-    SuccessOrExit(err);
-    err = reader.ExitContainer(mOuterContainerType);
-
-exit:
-
-    return err;
+    ReturnErrorOnFailure(err);
+    return reader.ExitContainer(mOuterContainerType);
 }
 #endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
 

diff --git a/src/app/MessageDef/AttributePathIB.cpp b/src/app/MessageDef/AttributePathIB.cpp
@@ -33,7 +33,7 @@ namespace app {
 CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
 {
     CHIP_ERROR err      = CHIP_NO_ERROR;
-    int TagPresenceMask = 0;
+    int tagPresenceMask = 0;
     TLV::TLVReader reader;
 
     PRETTY_PRINT("AttributePathIB =");
@@ -53,8 +53,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
         {
         case to_underlying(Tag::kEnableTagCompression):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kEnableTagCompression))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kEnableTagCompression));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kEnableTagCompression))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kEnableTagCompression));
 #if CHIP_DETAIL_LOGGING
             {
                 bool enableTagCompression;
@@ -66,8 +66,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
         case to_underlying(Tag::kNode):
             // check if this tag has appeared before
 
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kNode))), err = CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kNode));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kNode))), err = CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kNode));
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);
 
 #if CHIP_DETAIL_LOGGING
@@ -80,8 +80,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
             break;
         case to_underlying(Tag::kEndpoint):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kEndpoint))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kEndpoint));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kEndpoint))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kEndpoint));
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
 #if CHIP_DETAIL_LOGGING
             {
@@ -93,8 +93,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
             break;
         case to_underlying(Tag::kCluster):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kCluster))), err = CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kCluster));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kCluster))), err = CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kCluster));
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);
 
 #if CHIP_DETAIL_LOGGING
@@ -107,8 +107,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
             break;
         case to_underlying(Tag::kAttribute):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kAttribute))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kAttribute));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kAttribute))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kAttribute));
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
 #if CHIP_DETAIL_LOGGING
             {
@@ -120,8 +120,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
             break;
         case to_underlying(Tag::kListIndex):
             // check if this tag has appeared before
-            VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kListIndex))), CHIP_ERROR_INVALID_TLV_TAG);
-            TagPresenceMask |= (1 << to_underlying(Tag::kListIndex));
+            VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kListIndex))), CHIP_ERROR_INVALID_TLV_TAG);
+            tagPresenceMask |= (1 << to_underlying(Tag::kListIndex));
             VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType() || TLV::kTLVType_Null == reader.GetType(),
                                 CHIP_ERROR_WRONG_TLV_TYPE);
 #if CHIP_DETAIL_LOGGING
@@ -149,10 +149,10 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
     // if we have exhausted this container
     if (CHIP_END_OF_TLV == err)
     {
-        if ((TagPresenceMask & (1 << to_underlying(Tag::kAttribute))) == 0 &&
-            (TagPresenceMask & (1 << to_underlying(Tag::kListIndex))) != 0)
+        if ((tagPresenceMask & (1 << to_underlying(Tag::kAttribute))) == 0 &&
+            (tagPresenceMask & (1 << to_underlying(Tag::kListIndex))) != 0)
         {
-            err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
+            err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB;
         }
         else
         {
@@ -161,8 +161,7 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
     }
 
     ReturnErrorOnFailure(err);
-    ReturnErrorOnFailure(reader.ExitContainer(mOuterContainerType));
-    return CHIP_NO_ERROR;
+    return reader.ExitContainer(mOuterContainerType);
 }
 #endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
 
@@ -215,7 +214,7 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAt
         else
         {
             // TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
-            err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
+            err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB;
         }
     }
     else if (CHIP_END_OF_TLV == err)

diff --git a/src/app/MessageDef/AttributePathIBs.cpp b/src/app/MessageDef/AttributePathIBs.cpp
@@ -68,8 +68,7 @@ CHIP_ERROR AttributePathIBs::Parser::CheckSchemaValidity() const
         }
     }
     ReturnErrorOnFailure(err);
-    ReturnErrorOnFailure(reader.ExitContainer(mOuterContainerType));
-    return CHIP_NO_ERROR;
+    return reader.ExitContainer(mOuterContainerType);
 }
 #endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK