Make datamodel-provider inteface logging optional/configurable (proje…

…ct-chip#36249) * make more detailed logging optional in the codegen data model and enable it only on known large platforms * Update src/app/common_flags.gni Co-authored-by: Boris Zbarsky <[email protected]> * Restyled by gn --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Restyled.io <[email protected]>
yyzhong-g · Dec 11, 2024 · 53fb0de · 53fb0de
1 parent 5b36f16
commit 53fb0de
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 4 deletions.
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -76,6 +76,7 @@ buildconfig_header("app_buildconfig") {
     "CHIP_DEVICE_CONFIG_DYNAMIC_SERVER=${chip_build_controller_dynamic_server}",
     "CHIP_CONFIG_ENABLE_BUSY_HANDLING_FOR_OPERATIONAL_SESSION_SETUP=${chip_enable_busy_handling_for_operational_session_setup}",
     "CHIP_CONFIG_DATA_MODEL_CHECK_DIE_ON_FAILURE=${chip_data_model_check_die_on_failure}",
+    "CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING=${chip_data_model_extra_logging}",
   ]
 
   if (chip_use_data_model_interface == "disabled") {

diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp
@@ -108,8 +108,13 @@ void WriteHandler::Close()
 std::optional<bool> WriteHandler::IsListAttributePath(const ConcreteAttributePath & path)
 {
 #if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
-    VerifyOrReturnValue(mDataModelProvider != nullptr, std::nullopt,
-                        ChipLogError(DataManagement, "Null data model while checking attribute properties."));
+    if (mDataModelProvider == nullptr)
+    {
+#if CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING
+        ChipLogError(DataManagement, "Null data model while checking attribute properties.");
+#endif
+        return std::nullopt;
+    }
 
     auto info = mDataModelProvider->GetAttributeInfo(path);
     if (!info.has_value())

diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp
@@ -140,9 +140,11 @@ std::optional<CommandId> EnumeratorCommandFinder::FindCommandId(Operation operat
 
     if (err != CHIP_NO_ERROR)
     {
+#if CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING
         // Report the error here since we lose actual error. This generally should NOT be possible as CommandHandlerInterface
         // usually returns unimplemented or should just work for our use case (our callback never fails)
         ChipLogError(DataManagement, "Enumerate error: %" CHIP_ERROR_FORMAT, err.Format());
+#endif
         return kInvalidCommandId;
     }
 
@@ -155,8 +157,10 @@ std::variant<CHIP_ERROR, DataModel::ClusterInfo> LoadClusterInfo(const ConcreteC
     DataVersion * versionPtr = emberAfDataVersionStorage(path);
     if (versionPtr == nullptr)
     {
+#if CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING
         ChipLogError(AppServer, "Failed to get data version for %d/" ChipLogFormatMEI, static_cast<int>(path.mEndpointId),
                      ChipLogValueMEI(cluster.clusterId));
+#endif
         return CHIP_ERROR_NOT_FOUND;
     }
 
@@ -211,7 +215,7 @@ DataModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const Emb
             return *entryValue;
         }
 
-#if CHIP_ERROR_LOGGING
+#if CHIP_ERROR_LOGGING && CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING
         if (CHIP_ERROR * errValue = std::get_if<CHIP_ERROR>(&entry))
         {
             ChipLogError(AppServer, "Failed to load cluster entry: %" CHIP_ERROR_FORMAT, errValue->Format());
@@ -571,7 +575,7 @@ std::optional<DataModel::ClusterInfo> CodegenDataModelProvider::GetClusterInfo(c
 
     if (CHIP_ERROR * err = std::get_if<CHIP_ERROR>(&info))
     {
-#if CHIP_ERROR_LOGGING
+#if CHIP_ERROR_LOGGING && CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING
         ChipLogError(AppServer, "Failed to load cluster info: %" CHIP_ERROR_FORMAT, err->Format());
 #else
         (void) err->Format();

diff --git a/src/app/common_flags.gni b/src/app/common_flags.gni
@@ -40,4 +40,11 @@ declare_args() {
   # If/once the chip_use_data_model_interface flag is removed or does not support
   # a `check` option, this should alwo be removed
   chip_data_model_check_die_on_failure = false
+
+  # This controls if more logging is supposed to be enabled into the data models.
+  # This is an optimization for small-flash size devices as extra logging requires
+  # additional flash for messages & code for formatting.
+  chip_data_model_extra_logging =
+      current_os == "linux" || current_os == "ios" || current_os == "mac" ||
+      current_os == "android"
 }
diff --git a/src/app/reporting/Read-DataModel.cpp b/src/app/reporting/Read-DataModel.cpp
@@ -97,7 +97,9 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
     if (!status.IsOutOfSpaceEncodingResponse())
     {
         DataModel::ActionReturnStatus::StringStorage storage;
+#if CHIP_CONFIG_DATA_MODEL_EXTRA_LOGGING
         ChipLogError(DataManagement, "Failed to read attribute: %s", status.c_str(storage));
+#endif
     }
     return status;
 }