esp32_diagnostic_trace: add CircularDiagnosticBuffer class to improve design

esp · esp · commit 153bce3c567f · 2024-12-06T16:36:55.000+05:30
diff --git a/examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp b/examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp
@@ -81,7 +81,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent)
     {
     case IntentEnum::kEndUserSupport: {
 #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
-        return DiagnosticStorageImpl::GetInstance().GetDataSize();
+        return CircularDiagnosticBuffer::GetInstance().GetDataSize();
 #else
         return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart);
 #endif
@@ -119,7 +119,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE
     {
     case IntentEnum::kEndUserSupport: {
 #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
-        DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance();
+        CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance();
         MutableByteSpan endUserSupportSpan(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE);
 
         if (diagnosticStorage.IsEmptyBuffer())
diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp
@@ -88,7 +88,8 @@ extern "C" void app_main()
 #endif
 
 #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
-    DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE);
+    CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance();
+    diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE);
     static ESP32Diagnostics diagnosticBackend(diagnosticStorage);
     Tracing::Register(diagnosticBackend);
 #endif
diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn
@@ -27,7 +27,6 @@ static_library("backend") {
   sources = [
     "Counter.cpp",
     "Counter.h",
-    "DiagnosticStorageManager.cpp",
     "DiagnosticStorageManager.h",
     "DiagnosticTracing.cpp",
     "DiagnosticTracing.h",
diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h
@@ -25,7 +25,6 @@
 #include <lib/support/CHIPMemString.h>
 #include <map>
 #include <string.h>
-#include <map>
 
 namespace chip {
 namespace Tracing {
diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h
@@ -25,29 +25,122 @@
 namespace chip {
 namespace Tracing {
 namespace Diagnostics {
-class DiagnosticStorageImpl : public DiagnosticStorageInterface
+class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public DiagnosticStorageInterface
 {
 public:
-    static DiagnosticStorageImpl & GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0);
+    // Singleton instance getter
+    static CircularDiagnosticBuffer & GetInstance()
+    {
+        static CircularDiagnosticBuffer instance;
+        return instance;
+    }
 
-    DiagnosticStorageImpl(const DiagnosticStorageImpl &)             = delete;
-    DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete;
+    // Delete copy constructor and assignment operator to ensure singleton
+    CircularDiagnosticBuffer(const CircularDiagnosticBuffer &)             = delete;
+    CircularDiagnosticBuffer & operator=(const CircularDiagnosticBuffer &) = delete;
 
-    CHIP_ERROR Store(DiagnosticEntry & diagnostic) override;
+    void Init(uint8_t * buffer, size_t bufferLength) { chip::TLV::TLVCircularBuffer::Init(buffer, bufferLength); }
 
-    CHIP_ERROR Retrieve(MutableByteSpan & payload) override;
+    CHIP_ERROR Store(DiagnosticEntry & entry) override
+    {
+        chip::TLV::CircularTLVWriter writer;
+        writer.Init(*this);
 
-    bool IsEmptyBuffer();
+        CHIP_ERROR err = entry.Encode(writer);
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(DeviceLayer, "Failed to write entry: %s", chip::ErrorStr(err));
+        }
+        return err;
+    }
 
-    uint32_t GetDataSize();
+    CHIP_ERROR Retrieve(MutableByteSpan & span) override
+    {
+        CHIP_ERROR err = CHIP_NO_ERROR;
+        chip::TLV::TLVReader reader;
+        reader.Init(*this);
 
-private:
-    DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize);
-    DiagnosticStorageImpl();
-    ~DiagnosticStorageImpl();
+        chip::TLV::TLVWriter writer;
+        writer.Init(span.data(), span.size());
+
+        chip::TLV::TLVType outWriterContainer;
+        err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_List, outWriterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container"));
+
+        while (CHIP_NO_ERROR == reader.Next())
+        {
+            VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                                ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err)));
+
+            if (reader.GetType() == chip::TLV::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag())
+            {
+                chip::TLV::TLVType outerReaderContainer;
+                err = reader.EnterContainer(outerReaderContainer);
+                VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                                    ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", ErrorStr(err)));
+
+                err = reader.Next();
+                VerifyOrReturnError(
+                    err == CHIP_NO_ERROR, err,
+                    ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err)));
+
+                if ((reader.GetType() == chip::TLV::kTLVType_Structure) &&
+                    (reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC) ||
+                     reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE) ||
+                     reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER)))
+                {
+                    if ((reader.GetLengthRead() - writer.GetLengthWritten()) < (writer.GetRemainingFreeLength()))
+                    {
+                        err = writer.CopyElement(reader);
+                        if (err == CHIP_ERROR_BUFFER_TOO_SMALL)
+                        {
+                            ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element");
+                            break;
+                        }
+                        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element"));
+                    }
+                    else
+                    {
+                        ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV");
+                        break;
+                    }
+                }
+                else
+                {
+                    ChipLogError(DeviceLayer, "Unexpected TLV element in outer container");
+                    reader.ExitContainer(outerReaderContainer);
+                    return CHIP_ERROR_WRONG_TLV_TYPE;
+                }
+                err = reader.ExitContainer(outerReaderContainer);
+                VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                                    ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err)));
+            }
+            else
+            {
+                ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container");
+            }
+        }
 
-    chip::TLV::TLVCircularBuffer mEndUserCircularBuffer;
+        err = writer.EndContainer(outWriterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to close outer container"));
+        err = writer.Finalize();
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing"));
+        span.reduce_size(writer.GetLengthWritten());
+        ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n",
+                        writer.GetLengthWritten());
+        ChipLogProgress(DeviceLayer, "Retrieval successful");
+        return CHIP_NO_ERROR;
+    }
+
+    bool IsEmptyBuffer() override { return DataLength() == 0; }
+
+    uint32_t GetDataSize() override { return DataLength(); }
+
+private:
+    CircularDiagnosticBuffer() : chip::TLV::TLVCircularBuffer(nullptr, 0) {}
+    ~CircularDiagnosticBuffer() override = default;
 };
+
 } // namespace Diagnostics
 } // namespace Tracing
 } // namespace chip
diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h
@@ -59,10 +59,14 @@ class Metric : public DiagnosticEntry
     CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override
     {
         CHIP_ERROR err = CHIP_NO_ERROR;
+        chip::TLV::TLVType metricOuterContainer;
+        err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, metricOuterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                            ChipLogError(DeviceLayer, "Failed to Start outer metric container: %s", ErrorStr(err)));
         chip::TLV::TLVType metricContainer;
         err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC), chip::TLV::kTLVType_Structure, metricContainer);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to Start inner metric container: %s", ErrorStr(err)));
 
         // TIMESTAMP
         err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_);
@@ -82,7 +86,13 @@ class Metric : public DiagnosticEntry
         ChipLogProgress(DeviceLayer, "Metric Value written to storage successfully. label: %s\n", label_);
         err = writer.EndContainer(metricContainer);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to end inner TLV container for Metric : %s", ErrorStr(err)));
+        err = writer.EndContainer(metricOuterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                            ChipLogError(DeviceLayer, "Failed to end outer TLV container for Metric : %s", ErrorStr(err)));
+        err = writer.Finalize();
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err)));
+        ReturnErrorOnFailure(writer.Finalize());
         return err;
     }
 
@@ -106,15 +116,18 @@ class Trace : public DiagnosticEntry
     CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override
     {
         CHIP_ERROR err = CHIP_NO_ERROR;
+        chip::TLV::TLVType traceOuterContainer;
+        err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, traceOuterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                            ChipLogError(DeviceLayer, "Failed to Start outer trace container: %s", ErrorStr(err)));
         chip::TLV::TLVType traceContainer;
         err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE), chip::TLV::kTLVType_Structure, traceContainer);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to start TLV container for Trace: %s", ErrorStr(err)));
-
+                            ChipLogError(DeviceLayer, "Failed to Start inner trace container: %s", ErrorStr(err)));
         // TIMESTAMP
         err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for TRACE : %s", ErrorStr(err)));
 
         // GROUP
         err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::GROUP), group_);
@@ -129,7 +142,13 @@ class Trace : public DiagnosticEntry
         ChipLogProgress(DeviceLayer, "Trace Value written to storage successfully. label: %s value: %s\n", label_, group_);
         err = writer.EndContainer(traceContainer);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to end TLV container for Trace : %s", ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to end inner TLV container for Trace : %s", ErrorStr(err)));
+        err = writer.EndContainer(traceOuterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                            ChipLogError(DeviceLayer, "Failed to end outer TLV container for Trace : %s", ErrorStr(err)));
+        err = writer.Finalize();
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err)));
+        ReturnErrorOnFailure(writer.Finalize());
         return err;
     }
 
@@ -153,11 +172,15 @@ class Counter : public DiagnosticEntry
     CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override
     {
         CHIP_ERROR err = CHIP_NO_ERROR;
+        chip::TLV::TLVType counterOuterContainer;
+        err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, counterOuterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                            ChipLogError(DeviceLayer, "Failed to Start outer counter container: %s", ErrorStr(err)));
         chip::TLV::TLVType counterContainer;
         err =
             writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), chip::TLV::kTLVType_Structure, counterContainer);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to start TLV container for Counter: %s", ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to Start inner counter container: %s", ErrorStr(err)));
 
         // TIMESTAMP
         err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_);
@@ -177,7 +200,12 @@ class Counter : public DiagnosticEntry
         ChipLogProgress(DeviceLayer, "Counter Value written to storage successfully  label: %s count: %ld\n", label_, count_);
         err = writer.EndContainer(counterContainer);
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to end TLV container for counter : %s", ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to end inner TLV container for Counter : %s", ErrorStr(err)));
+        err = writer.EndContainer(counterOuterContainer);
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err,
+                            ChipLogError(DeviceLayer, "Failed to end outer TLV container for Counter : %s", ErrorStr(err)));
+        err = writer.Finalize();
+        VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err)));
         return err;
     }
 
@@ -213,6 +241,10 @@ class DiagnosticStorageInterface
      * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure.
      */
     virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0;
+
+    virtual bool IsEmptyBuffer() = 0;
+
+    virtual uint32_t GetDataSize() = 0;
 };
 
 } // namespace Diagnostics
