From 12522881fdc5438293cde987b46c9e4f913eb8b3 Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 10 Dec 2024 19:10:20 +0530 Subject: [PATCH] diagnostic_storage: remove redundant code --- .../esp32_diagnostic_trace/Counter.cpp | 4 +-- src/tracing/esp32_diagnostic_trace/Counter.h | 9 ++---- .../DiagnosticStorageManager.h | 2 -- .../DiagnosticTracing.cpp | 31 +++++++------------ .../DiagnosticTracing.h | 2 -- .../esp32_diagnostic_trace/Diagnostics.h | 29 ++++++++--------- 6 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 0317fbaa492d1a..6eda7076a5e347 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -16,8 +16,8 @@ * limitations under the License. */ -#include #include +#include namespace chip { namespace Tracing { @@ -25,7 +25,7 @@ namespace Diagnostics { std::map 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 index ea882b3487d3b3..38a83b41234cfb 100644 --- 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 -#include -#include -#include #include -#include 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 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 index 22713a70c6322c..723a25e9654c9e 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -18,8 +18,6 @@ #pragma once -#include -#include #include #define TLV_CLOSING_BYTE 1 diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 9f1d2ed64d8ef1..b161c16d079770 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -19,13 +19,9 @@ #include #include -#include #include -#include -#include #include #include -#include 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 trace(label, group, esp_log_timestamp()); - err = mStorageInstance.Store(trace); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); - } + Diagnostic 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 index 5ab60a31457447..54cabef7fd55bb 100644 --- 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 -#include #include #include #include diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 2025ba8d599a6e..d035467d8909ca 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -17,22 +17,14 @@ */ #pragma once -#include + #include -#include 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) { - 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; };