diagnostic_storage: remove redundant code

project-chip · Dec 13, 2024 · 1252288 · 1252288
1 parent 8512a1f
commit 1252288
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 48 deletions.
diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp
@@ -16,16 +16,16 @@
  *    limitations under the License.
  */
 
-#include <string.h>
 #include <tracing/esp32_diagnostic_trace/Counter.h>
+#include <esp_log.h>
 
 namespace chip {
 namespace Tracing {
 namespace Diagnostics {
 
 std::map<const char *, uint32_t> ESPDiagnosticCounter::mCounterList;
 
-void ESPDiagnosticCounter::CountInit(const char * label)
+void ESPDiagnosticCounter::IncreaseCount(const char * label)
 {
     if (mCounterList.find(label) != mCounterList.end())
     {

diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h
@@ -19,12 +19,7 @@
 #pragma once
 
 #include "tracing/esp32_diagnostic_trace/Diagnostics.h"
-#include <esp_diagnostics_metrics.h>
-#include <esp_log.h>
-#include <lib/support/CHIPMem.h>
-#include <lib/support/CHIPMemString.h>
 #include <map>
-#include <string.h>
 
 namespace chip {
 namespace Tracing {
@@ -44,7 +39,7 @@ class ESPDiagnosticCounter
     static ESPDiagnosticCounter & GetInstance(const char * label)
     {
         static ESPDiagnosticCounter instance;
-        CountInit(label);
+        IncreaseCount(label);
         return instance;
     }
 
@@ -55,7 +50,7 @@ class ESPDiagnosticCounter
 private:
     ESPDiagnosticCounter() {}
     static std::map<const char *, uint32_t> mCounterList;
-    static void CountInit(const char * label);
+    static void IncreaseCount(const char * label);
 };
 
 } // namespace Diagnostics

diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h
@@ -18,8 +18,6 @@
 
 #pragma once
 
-#include <lib/core/CHIPError.h>
-#include <lib/support/CHIPMem.h>
 #include <tracing/esp32_diagnostic_trace/Diagnostics.h>
 
 #define TLV_CLOSING_BYTE 1

diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp
@@ -19,13 +19,9 @@
 
 #include <algorithm>
 #include <esp_err.h>
-#include <esp_heap_caps.h>
 #include <esp_log.h>
-#include <memory>
-#include <tracing/backend.h>
 #include <tracing/esp32_diagnostic_trace/Counter.h>
 #include <tracing/esp32_diagnostic_trace/DiagnosticTracing.h>
-#include <tracing/metric_event.h>
 
 namespace chip {
 namespace Tracing {
@@ -76,14 +72,11 @@ HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"),
                                               MurmurHash("GeneralCommissioning"),
                                               MurmurHash("OperationalCredentials"),
                                               MurmurHash("CASEServer"),
-                                              MurmurHash("BLE"),
-                                              MurmurHash("BLE_Error"),
-                                              MurmurHash("Wifi"),
-                                              MurmurHash("Wifi_Error"),
                                               MurmurHash("Fabric") }; // namespace
 
-bool IsPermitted(HashValue hashValue)
+bool IsPermitted(const char * str)
 {
+    HashValue hashValue = MurmurHash(str);
     for (HashValue permitted : gPermitList)
     {
         if (permitted == 0)
@@ -150,12 +143,18 @@ void ESP32Diagnostics::TraceCounter(const char * label)
 
 void ESP32Diagnostics::TraceBegin(const char * label, const char * group)
 {
-    StoreDiagnostics(label, group);
+    if (IsPermitted(group))
+    {
+        StoreDiagnostics(label, group);
+    }
 }
 
 void ESP32Diagnostics::TraceEnd(const char * label, const char * group)
 {
-    StoreDiagnostics(label, group);
+    if (IsPermitted(group))
+    {
+        StoreDiagnostics(label, group);
+    }
 }
 
 void ESP32Diagnostics::TraceInstant(const char * label, const char * group)
@@ -165,14 +164,8 @@ void ESP32Diagnostics::TraceInstant(const char * label, const char * group)
 
 void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group)
 {
-    CHIP_ERROR err      = CHIP_NO_ERROR;
-    HashValue hashValue = MurmurHash(group);
-    if (IsPermitted(hashValue))
-    {
-        Diagnostic<const char *> trace(label, group, esp_log_timestamp());
-        err = mStorageInstance.Store(trace);
-        VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));
-    }
+    Diagnostic<const char *> trace(label, group, esp_log_timestamp());
+    VerifyOrReturn(mStorageInstance.Store(trace) == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));
 }
 
 } // namespace Diagnostics

diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h
@@ -18,8 +18,6 @@
  *    limitations under the License.
  */
 
-#include <esp_log.h>
-#include <lib/core/CHIPError.h>
 #include <tracing/backend.h>
 #include <tracing/esp32_diagnostic_trace/Diagnostics.h>
 #include <tracing/metric_event.h>

diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h
@@ -17,22 +17,14 @@
  */
 
 #pragma once
-#include <lib/core/CHIPError.h>
+
 #include <lib/core/TLVCircularBuffer.h>
-#include <lib/support/Span.h>
 
 namespace chip {
 namespace Tracing {
 
 namespace Diagnostics {
 
-enum class DIAGNOSTICS_TAG
-{
-    LABEL = 0,
-    VALUE,
-    TIMESTAMP
-};
-
 /**
  * @class DiagnosticEntry
  * @brief Abstract base class for encoding diagnostic entries into TLV format.
@@ -68,18 +60,15 @@ class Diagnostic : public DiagnosticEntry
         chip::TLV::TLVType DiagnosticOuterContainer = chip::TLV::kTLVType_NotSpecified;
         ReturnErrorOnFailure(
             writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, DiagnosticOuterContainer));
-        // TIMESTAMP
-        ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_));
-        // LABEL
-        ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_));
-        // VALUE
+        ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(0), timestamp_));
+        ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(1), label_));
         if constexpr (std::is_same_v<T, const char *>)
         {
-            ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_));
+            ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(2), value_));
         }
         else
         {
-            ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_));
+            ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(2), value_));
         }
         ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer));
         ReturnErrorOnFailure(writer.Finalize());
@@ -120,8 +109,16 @@ class DiagnosticStorageInterface
      */
     virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0;
 
+    /**
+     * @brief Checks if the storage buffer is empty.
+     * @return bool true if the buffer contains no stored diagnostic data, otherwise false.
+     */
     virtual bool IsEmptyBuffer() = 0;
 
+    /**
+     * @brief Retrieves the size of the data currently stored in the buffer.
+     * @return uint32_t The size (in bytes) of the stored diagnostic data.
+     */
     virtual uint32_t GetDataSize() = 0;
 };