Repurpose the checkSchemaValidity to PrettyPrint only

--Remove the necessary elemnt check in im pretty print --Rename CheckSchemaValidity to PrettyPrint --Don't process the returned error for prettyPrint in IM client/server code
project-chip · Nov 16, 2022 · 1fe5d5f · 1fe5d5f
1 parent 361c74d
commit 1fe5d5f
Show file tree

Hide file tree

Showing 93 changed files with 732 additions and 1,641 deletions.
diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt
@@ -119,8 +119,8 @@ if (CONFIG_BUILD_CHIP_TESTS)
     chip_gn_arg_bool("chip_build_tests"     "true")
 endif()
 
-if (CONFIG_CHIP_ENABLE_SCHEMA_CHECK)
-    chip_gn_arg_bool("chip_enable_schema_check"	"true")
+if (CONFIG_IM_PRETTY_PRINT)
+    chip_gn_arg_bool("enable_im_pretty_print"	"true")
 endif()
 
 if (NOT CONFIG_USE_MINIMAL_MDNS)

diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig
@@ -787,12 +787,11 @@ menu "CHIP Device Layer"
                 for the debug level events are disabled.
 
 
-	config CHIP_ENABLE_SCHEMA_CHECK
-	   bool "Enable Schema Check"
+	config CHIP_CONFIG_IM_PRETTY_PRINT
+	   bool "Enable IM Pretty Print"
 	   default y
 	   help
-	     If enabled, it checks incoming messages are following the expected schema,
-             and incoming message payloads are logged in detail.
+	     If enabled, incoming message payloads are logged in detail.
 	     To see detailed logging please set default log level to Debug.
 	     (Component config --> Log output --> Default log verbosity --> Debug)
 

diff --git a/examples/common/tracing/decoder/interaction_model/Decoder.cpp b/examples/common/tracing/decoder/interaction_model/Decoder.cpp
@@ -132,12 +132,12 @@ CHIP_ERROR LogAsProtocolMessage(uint8_t protocolCode, const uint8_t * data, size
 
 CHIP_ERROR DecodeStatusResponse(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::StatusResponseMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        parser.PrettyPrint();
     }
 #endif
 
@@ -146,12 +146,12 @@ CHIP_ERROR DecodeStatusResponse(TLV::TLVReader & reader, bool decode)
 
 CHIP_ERROR DecodeReadRequest(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::ReadRequestMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        return parser.PrettyPrint();
     }
 #endif
 
@@ -160,12 +160,12 @@ CHIP_ERROR DecodeReadRequest(TLV::TLVReader & reader, bool decode)
 
 CHIP_ERROR DecodeSubscribeRequest(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::SubscribeRequestMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        return parser.PrettyPrint();
     }
 #endif
 
@@ -174,12 +174,12 @@ CHIP_ERROR DecodeSubscribeRequest(TLV::TLVReader & reader, bool decode)
 
 CHIP_ERROR DecodeSubscribeResponse(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::SubscribeResponseMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        parser.PrettyPrint();
     }
 #endif
 
@@ -190,12 +190,12 @@ CHIP_ERROR DecodeReportData(TLV::TLVReader & reader, bool decode)
 {
     ReturnErrorOnFailure(MaybeDecodeNestedReadResponse(reader.GetReadPoint(), reader.GetTotalLength()));
 
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::ReportDataMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        parser.PrettyPrint();
     }
 #endif
 
@@ -204,12 +204,12 @@ CHIP_ERROR DecodeReportData(TLV::TLVReader & reader, bool decode)
 
 CHIP_ERROR DecodeWriteRequest(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::WriteRequestMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        return parser.PrettyPrint();
     }
 #endif
 
@@ -218,12 +218,12 @@ CHIP_ERROR DecodeWriteRequest(TLV::TLVReader & reader, bool decode)
 
 CHIP_ERROR DecodeWriteResponse(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::WriteResponseMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        return parser.PrettyPrint();
     }
 #endif
 
@@ -234,12 +234,12 @@ CHIP_ERROR DecodeInvokeCommandRequest(TLV::TLVReader & reader, bool decode)
 {
     ReturnErrorOnFailure(MaybeDecodeNestedCommandRequest(reader.GetReadPoint(), reader.GetTotalLength()));
 
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::InvokeRequestMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        return parser.PrettyPrint();
     }
 #endif
 
@@ -250,12 +250,12 @@ CHIP_ERROR DecodeInvokeCommandResponse(TLV::TLVReader & reader, bool decode)
 {
     ReturnErrorOnFailure(MaybeDecodeNestedCommandResponse(reader.GetReadPoint(), reader.GetTotalLength()));
 
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::InvokeResponseMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        parser.PrettyPrint();
     }
 #endif
 
@@ -264,12 +264,12 @@ CHIP_ERROR DecodeInvokeCommandResponse(TLV::TLVReader & reader, bool decode)
 
 CHIP_ERROR DecodeTimedRequest(TLV::TLVReader & reader, bool decode)
 {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#if CHIP_CONFIG_IM_PRETTY_PRINT
     if (decode)
     {
         app::TimedRequestMessage::Parser parser;
         ReturnErrorOnFailure(parser.Init(reader));
-        return parser.CheckSchemaValidity();
+        parser.PrettyPrint();
     }
 #endif
 

diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -20,7 +20,7 @@ import("common_flags.gni")
 
 declare_args() {
   # Enable strict schema checks.
-  chip_enable_schema_check =
+  enable_im_pretty_print =
       is_debug && (current_os == "linux" || current_os == "mac" ||
                    current_os == "ios" || current_os == "android")
 
@@ -46,7 +46,7 @@ buildconfig_header("app_buildconfig") {
   header_dir = "app"
 
   defines = [
-    "CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK=${chip_enable_schema_check}",
+    "CHIP_CONFIG_IM_PRETTY_PRINT=${enable_im_pretty_print}",
     "CHIP_CONFIG_IM_FORCE_FABRIC_QUOTA_CHECK=${chip_im_force_fabric_quota_check}",
     "CHIP_CONFIG_ENABLE_SESSION_RESUMPTION=${chip_enable_session_resumption}",
     "CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY=${chip_access_control_policy_logging_verbosity}",

diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
@@ -104,8 +104,8 @@ Status CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && payloa
     InvokeRequests::Parser invokeRequests;
     reader.Init(std::move(payload));
     VerifyOrReturnError(invokeRequestMessage.Init(reader) == CHIP_NO_ERROR, Status::InvalidAction);
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
-    VerifyOrReturnError(invokeRequestMessage.CheckSchemaValidity() == CHIP_NO_ERROR, Status::InvalidAction);
+#if CHIP_CONFIG_IM_PRETTY_PRINT
+    invokeRequestMessage.PrettyPrint();
 #endif
 
     VerifyOrReturnError(invokeRequestMessage.GetSuppressResponse(&mSuppressResponse) == CHIP_NO_ERROR, Status::InvalidAction);

diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp
@@ -213,8 +213,8 @@ CHIP_ERROR CommandSender::ProcessInvokeResponse(System::PacketBufferHandle && pa
     reader.Init(std::move(payload));
     ReturnErrorOnFailure(invokeResponseMessage.Init(reader));
 
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
-    ReturnErrorOnFailure(invokeResponseMessage.CheckSchemaValidity());
+#if CHIP_CONFIG_IM_PRETTY_PRINT
+    invokeResponseMessage.PrettyPrint();
 #endif
 
     ReturnErrorOnFailure(invokeResponseMessage.GetSuppressResponse(&suppressResponse));

diff --git a/src/app/MessageDef/AttributeDataIB.cpp b/src/app/MessageDef/AttributeDataIB.cpp
@@ -28,11 +28,10 @@
 
 namespace chip {
 namespace app {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
-CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
+#if CHIP_CONFIG_IM_PRETTY_PRINT
+CHIP_ERROR AttributeDataIB::Parser::PrettyPrint() const
 {
-    CHIP_ERROR err      = CHIP_NO_ERROR;
-    int tagPresenceMask = 0;
+    CHIP_ERROR err = CHIP_NO_ERROR;
     TLV::TLVReader reader;
 
     PRETTY_PRINT("AttributeDataIB =");
@@ -51,11 +50,7 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
         switch (tagNum)
         {
         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(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
-
 #if CHIP_DETAIL_LOGGING
             {
                 chip::DataVersion version;
@@ -64,24 +59,16 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
             }
 #endif // CHIP_DETAIL_LOGGING
             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));
-            {
-                AttributePathIB::Parser path;
-                ReturnErrorOnFailure(path.Init(reader));
+        case to_underlying(Tag::kPath): {
+            AttributePathIB::Parser path;
+            ReturnErrorOnFailure(path.Init(reader));
 
-                PRETTY_PRINT_INCDEPTH();
-                ReturnErrorOnFailure(path.CheckSchemaValidity());
-                PRETTY_PRINT_DECDEPTH();
-            }
-            break;
+            PRETTY_PRINT_INCDEPTH();
+            ReturnErrorOnFailure(path.PrettyPrint());
+            PRETTY_PRINT_DECDEPTH();
+        }
+        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));
-
             PRETTY_PRINT_INCDEPTH();
             ReturnErrorOnFailure(CheckIMPayload(reader, 0, "Data"));
             PRETTY_PRINT_DECDEPTH();
@@ -97,15 +84,12 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
     // if we have exhausted this container
     if (CHIP_END_OF_TLV == err)
     {
-        // check for required fields:
-        const int requiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));
-
-        err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_DATA_IB;
+        err = CHIP_NO_ERROR;
     }
     ReturnErrorOnFailure(err);
     return reader.ExitContainer(mOuterContainerType);
 }
-#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#endif // CHIP_CONFIG_IM_PRETTY_PRINT
 
 CHIP_ERROR AttributeDataIB::Parser::GetPath(AttributePathIB::Parser * const apPath) const
 {

diff --git a/src/app/MessageDef/AttributeDataIB.h b/src/app/MessageDef/AttributeDataIB.h
@@ -42,23 +42,9 @@ enum class Tag : uint8_t
 class Parser : public StructParser
 {
 public:
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
-    /**
-     *  @brief Roughly verify the message is correctly formed
-     *   1) all mandatory tags are present
-     *   2) all elements have expected data type
-     *   3) any tag can only appear once
-     *   4) At the top level of the structure, unknown tags are ignored for forward compatibility
-     *  @note The main use of this function is to print out what we're
-     *    receiving during protocol development and debugging.
-     *    The encoding rule has changed in IM encoding spec so this
-     *    check is only "roughly" conformant now.
-     *
-     *  @return #CHIP_NO_ERROR on success
-     */
-    CHIP_ERROR CheckSchemaValidity() const;
-#endif
-
+#if CHIP_CONFIG_IM_PRETTY_PRINT
+    CHIP_ERROR PrettyPrint() const;
+#endif // CHIP_CONFIG_IM_PRETTY_PRINT
     /**
      *  @brief Get the DataVersion.
      *

diff --git a/src/app/MessageDef/AttributeDataIBs.cpp b/src/app/MessageDef/AttributeDataIBs.cpp
@@ -36,8 +36,8 @@ using namespace chip::TLV;
 
 namespace chip {
 namespace app {
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
-CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
+#if CHIP_CONFIG_IM_PRETTY_PRINT
+CHIP_ERROR AttributeDataIBs::Parser::PrettyPrint() const
 {
     CHIP_ERROR err        = CHIP_NO_ERROR;
     size_t numDataElement = 0;
@@ -59,7 +59,7 @@ CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
             ReturnErrorOnFailure(data.Init(reader));
 
             PRETTY_PRINT_INCDEPTH();
-            ReturnErrorOnFailure(data.CheckSchemaValidity());
+            ReturnErrorOnFailure(data.PrettyPrint());
             PRETTY_PRINT_DECDEPTH();
         }
 
@@ -81,7 +81,7 @@ CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
     ReturnErrorOnFailure(err);
     return reader.ExitContainer(mOuterContainerType);
 }
-#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
+#endif // CHIP_CONFIG_IM_PRETTY_PRINT
 
 AttributeDataIB::Builder & AttributeDataIBs::Builder::CreateAttributeDataIBBuilder()
 {

diff --git a/src/app/MessageDef/AttributeDataIBs.h b/src/app/MessageDef/AttributeDataIBs.h
@@ -40,22 +40,9 @@ namespace AttributeDataIBs {
 class Parser : public ArrayParser
 {
 public:
-#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
-    /**
-     *  @brief Roughly verify the message is correctly formed
-     *   1) all mandatory tags are present
-     *   2) all elements have expected data type
-     *   3) any tag can only appear once
-     *   4) At the top level of the structure, unknown tags are ignored for forward compatibility
-     *  @note The main use of this function is to print out what we're
-     *    receiving during protocol development and debugging.
-     *    The encoding rule has changed in IM encoding spec so this
-     *    check is only "roughly" conformant now.
-     *
-     *  @return #CHIP_NO_ERROR on success
-     */
-    CHIP_ERROR CheckSchemaValidity() const;
-#endif
+#if CHIP_CONFIG_IM_PRETTY_PRINT
+    CHIP_ERROR PrettyPrint() const;
+#endif // CHIP_CONFIG_IM_PRETTY_PRINT
 };
 
 class Builder : public ArrayBuilder