esp32 diagnostic trace changes

- Resolve buffer related issues
- Add store diagnostic call for trace_end and trace_instant
- Update scoped macro
- Remove extra namespace values, spaces and print statements
- Remove evicthead call for circular buffer after read successful

Author: pimpalemahesh

config/esp32/components/chip/CMakeLists.txt

@@ -373,11 +373,11 @@ if(CONFIG_ENABLE_PW_RPC)
 endif()
 
 if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE)
-    list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32TracingBackend.a")
+  list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32TracingBackend.a")
 endif()
 
 if (CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE)
-    list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32DiagnosticsBackend.a")
+  list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32DiagnosticsBackend.a")
 endif()
 
 
@@ -607,4 +607,4 @@ if (CONFIG_CHIP_OTA_IMAGE_BUILD)
     )
     # Adding dependecy as app target so that this runs after images are ready
     add_dependencies(chip-ota-image app)
-endif()
+endif()

config/esp32/components/chip/Kconfig

@@ -1404,4 +1404,4 @@ menu "CHIP Device Layer"
 
     endmenu
 
-endmenu
+endmenu

examples/temperature-measurement-app/esp32/main/Kconfig.projbuild

@@ -89,29 +89,11 @@ menu "Platform Diagnostics"
         bool "Enable ESP Diagnostics"
         default n
 
-    config DIAGNOSTIC_RETRIEVAL_BUFFER_SIZE
-        int "Set buffer size to retrieve diagnostic data"
-        depends on ESP_DIAGNOSTICS_ENABLED
-        default 1024
-        help
-            Defines the buffer size (in bytes) for retrieving diagnostic data through diagnostic logs cluster.
-            Increase this size if the diagnostic data generated by the application requires more space.
-
     config END_USER_BUFFER_SIZE
         int "Set buffer size for end user diagnostic data"
         depends on ESP_DIAGNOSTICS_ENABLED
         default 4096
         help
             Defines the buffer size (in bytes) for storing diagnostic data related to end user activity.
             This buffer will hold logs and traces relevant to user interactions with the Matter protocol.
-            Increase this size if the diagnostic data generated by the application requires more space.
-
-    config NETWORK_BUFFER_SIZE
-        int "Set buffer size for network diagnostic data"
-        depends on ESP_DIAGNOSTICS_ENABLED
-        default 2048
-        help
-            Defines the buffer size (in bytes) for storing network-related diagnostic data.
-            This buffer will store logs and traces related to network events and communication for the Matter protocol.
-            Adjust this size based on the expected network diagnostics requirements.
 endmenu

examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp

@@ -29,6 +29,11 @@ using namespace chip::app::Clusters::DiagnosticLogs;
 LogProvider LogProvider::sInstance;
 LogProvider::CrashLogContext LogProvider::sCrashLogContext;
 
+#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
+    static uint8_t retrieveBuffer[RETRIEVAL_BUFFER_SIZE];
+    MutableByteSpan endUserSupportSpan(retrieveBuffer, sizeof(retrieveBuffer));
+#endif
+
 namespace {
 bool IsValidIntent(IntentEnum intent)
 {
@@ -76,7 +81,14 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent)
     switch (intent)
     {
     case IntentEnum::kEndUserSupport:
-        return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart);
+        {
+            #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
+                return RETRIEVAL_BUFFER_SIZE;
+            #else
+                return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart);
+            #endif
+        }
+        break;
     case IntentEnum::kNetworkDiag:
         return static_cast<size_t>(networkDiagnosticLogEnd - networkDiagnosticLogStart);
     case IntentEnum::kCrashLogs:
@@ -105,17 +117,12 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE
 {
     context->intent = intent;
 
-    #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
-        DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance();
-
-        static uint8_t retrieveBuffer[RETRIEVAL_BUFFER_SIZE];
-        MutableByteSpan endUserSupportSpan(retrieveBuffer, sizeof(retrieveBuffer));
-    #endif
-
     switch (intent)
     {
     case IntentEnum::kEndUserSupport: {
         #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
+            DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance();
+
             if (diagnosticStorage.IsEmptyBuffer())
             {
                 printf("Buffer is empty\n");
@@ -129,7 +136,6 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE
                 ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err));
                 return err;
             }
-
             // Now, assign the span to the EndUserSupport object or whatever is required
             context->EndUserSupport.span = endUserSupportSpan;
         #else

examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h

@@ -31,8 +31,8 @@
 #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF)
 
 #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
-#define RETRIEVAL_BUFFER_SIZE CONFIG_DIAGNOSTIC_RETRIEVAL_BUFFER_SIZE
-using namespace chip::Tracing;
+#define RETRIEVAL_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE
+using namespace chip::Tracing::Diagnostics;
 #endif
 
 namespace chip {

examples/temperature-measurement-app/esp32/main/main.cpp

@@ -76,14 +76,15 @@ extern const char TAG[] = "temperature-measurement-app";
 
 static AppDeviceCallbacks EchoCallbacks;
 
+#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
+using namespace chip::Tracing::Diagnostics;
+static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE];
+static size_t buffer_size = CONFIG_END_USER_BUFFER_SIZE;
+#endif
+
 static void InitServer(intptr_t context)
 {
     Esp32AppServer::Init(); // Init ZCL Data Model and CHIP App Server AND Initialize device attestation config
-
-#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
-    static Tracing::Insights::ESP32Diagnostics diagnosticBackend;
-    Tracing::Register(diagnosticBackend);
-#endif
 }
 
 extern "C" void app_main()
@@ -92,6 +93,11 @@ extern "C" void app_main()
     chip::rpc::Init();
 #endif
 
+#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
+    static ESP32Diagnostics diagnosticBackend(endUserBuffer, buffer_size);
+    Tracing::Register(diagnosticBackend);
+#endif
+
     ESP_LOGI(TAG, "Temperature sensor!");
 
     // Initialize the ESP NVS layer.

src/tracing/esp32_diagnostic_trace/Counter.cpp

@@ -21,7 +21,7 @@
 
 using namespace chip;
 
-namespace Insights {
+namespace Diagnostics {
 
 // This is a one time allocation for counters. It is not supposed to be freed.
 ESPDiagnosticCounter * ESPDiagnosticCounter::mHead = nullptr;
@@ -65,4 +65,4 @@ void ESPDiagnosticCounter::ReportMetrics()
     VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data"));
 }
 
-} // namespace Insights
+} // namespace Diagnostics

src/tracing/esp32_diagnostic_trace/Counter.h

@@ -25,13 +25,13 @@
 #include <string.h>
 #include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h"
 
-using namespace chip::Tracing;
+using namespace chip::Tracing::Diagnostics;
 
-namespace Insights {
+namespace Diagnostics {
 
 /**
  * This class is used to monotonically increment the counters as per the label of the counter macro
- * 'MATTER_TRACE_COUNTER(label)' and report the metrics to esp-insights.
+ * 'MATTER_TRACE_COUNTER(label)'
  * As per the label of the counter macro, it adds the counter in the linked list with the name label if not
  * present and returns the same instance and increments the value if the counter is already present
  * in the list.
@@ -55,4 +55,4 @@ class ESPDiagnosticCounter
     void ReportMetrics();
 };
 
-} // namespace Insights
+} // namespace Diagnostics

src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp

@@ -17,17 +17,21 @@
  *    limitations under the License.
  */
 
-#include <esp_log.h>
 #include <lib/core/CHIPError.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h>
 
 namespace chip {
 namespace Tracing {
 
-DiagnosticStorageImpl::DiagnosticStorageImpl() :
-    mEndUserCircularBuffer(mEndUserBuffer, END_USER_BUFFER_SIZE), mNetworkCircularBuffer(mNetworkBuffer, NETWORK_BUFFER_SIZE)
-{}
+namespace Diagnostics {
+DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize)
+    : mEndUserCircularBuffer(buffer, bufferSize) {}
+
+DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) {
+    static DiagnosticStorageImpl instance(buffer, bufferSize);
+    return instance;
+}
 
 DiagnosticStorageImpl::~DiagnosticStorageImpl() {}
 
@@ -39,10 +43,10 @@ CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic)
     writer.Init(mEndUserCircularBuffer);
 
     // Start a TLV structure container (Anonymous)
-    chip::TLV::TLVType outerContainer;
-    err = writer.StartContainer(AnonymousTag(), chip::TLV::TLVType::kTLVType_Structure, outerContainer);
+    TLVType outerContainer;
+    err = writer.StartContainer(AnonymousTag(), kTLVType_Structure, outerContainer);
     VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                        ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", chip::ErrorStr(err)));
+                        ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err)));
 
     err = diagnostic.Encode(writer);
     if (err != CHIP_NO_ERROR)
@@ -55,32 +59,24 @@ CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic)
     }
     err = writer.EndContainer(outerContainer);
     VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                        ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", chip::ErrorStr(err)));
+                        ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", ErrorStr(err)));
 
     err = writer.Finalize();
     VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing"));
-
-    printf("Total Data Length in Buffer : %lu\n Total available length in buffer : %lu\nTotal buffer length : %lu\n",
-           mEndUserCircularBuffer.DataLength(), mEndUserCircularBuffer.AvailableDataLength(),
-           mEndUserCircularBuffer.GetTotalDataLength());
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload)
 {
-    printf("***************************************************************************RETRIEVAL "
-           "STARTED**********************************************************\n");
     CHIP_ERROR err = CHIP_NO_ERROR;
-    chip::TLV::TLVReader reader;
+    CircularTLVReader reader;
     reader.Init(mEndUserCircularBuffer);
 
-    chip::TLV::TLVWriter writer;
+    TLVWriter writer;
     writer.Init(payload);
 
-    uint32_t totalBufferSize = 0;
-
-    chip::TLV::TLVType outWriterContainer;
-    err = writer.StartContainer(AnonymousTag(), chip::TLV::TLVType::kTLVType_List, outWriterContainer);
+    TLVType outWriterContainer;
+    err = writer.StartContainer(AnonymousTag(), kTLVType_List, outWriterContainer);
     VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container"));
 
     while (true)
@@ -92,35 +88,29 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload)
             break;
         }
         VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                            ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", chip::ErrorStr(err)));
+                            ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err)));
 
-        if (reader.GetType() == chip::TLV::TLVType::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag())
+        if (reader.GetType() == kTLVType_Structure && reader.GetTag() == AnonymousTag())
         {
-            chip::TLV::TLVType outerReaderContainer;
+            TLVType outerReaderContainer;
             err = reader.EnterContainer(outerReaderContainer);
             VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                                ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", chip::ErrorStr(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", chip::ErrorStr(err)));
+            VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err)));
 
             // Check if the current element is a METRIC or TRACE container
-            if ((reader.GetType() == chip::TLV::kTLVType_Structure) &&
+            if ((reader.GetType() == kTLVType_Structure) &&
                 (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER)))
             {
-                ESP_LOGW("SIZE", "Total read till now: %ld Total write till now: %ld", reader.GetLengthRead(), writer.GetLengthWritten());
-
                 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"));
-                    ChipLogProgress(DeviceLayer, "Read metric container successfully");
-                    mEndUserCircularBuffer.EvictHead();
                 }
                 else {
                     ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV");
@@ -136,18 +126,12 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload)
             // Exit the outer container
             err = reader.ExitContainer(outerReaderContainer);
             VerifyOrReturnError(err == CHIP_NO_ERROR, err,
-                                ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", chip::ErrorStr(err)));
-
-            
+                                ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err)));
         }
         else
         {
             ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container");
         }
-
-        printf("Total Data Length in Buffer: %lu\n Total available length in buffer: %lu\nTotal buffer length: %lu\n",
-               mEndUserCircularBuffer.DataLength(), mEndUserCircularBuffer.AvailableDataLength(),
-               mEndUserCircularBuffer.GetTotalDataLength());
     }
 
     err = writer.EndContainer(outWriterContainer);
@@ -156,7 +140,7 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload)
     err = writer.Finalize();
     VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing"));
     payload.reduce_size(writer.GetLengthWritten());
-    printf("---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten());
+    ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten());
     ChipLogProgress(DeviceLayer, "Retrieval successful");
     return CHIP_NO_ERROR;
 }
@@ -165,6 +149,6 @@ bool DiagnosticStorageImpl::IsEmptyBuffer()
 {
     return mEndUserCircularBuffer.DataLength() == 0;
 }
-
+} // namespace Diagnostics
 } // namespace Tracing
 } // namespace chip

src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h

@@ -22,22 +22,16 @@
 #include <lib/support/CHIPMem.h>
 #include <lib/core/CHIPError.h>
 
-#define END_USER_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE
-#define NETWORK_BUFFER_SIZE CONFIG_NETWORK_BUFFER_SIZE
-
 namespace chip {
 namespace Tracing {
+namespace Diagnostics {
 using namespace chip::Platform;
-
+using chip::TLV::TLVType;
 class DiagnosticStorageImpl : public DiagnosticStorageInterface
 {
 public:
 
-    static DiagnosticStorageImpl& GetInstance()
-    {
-        static DiagnosticStorageImpl instance;
-        return instance;
-    }
+    static DiagnosticStorageImpl& GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0);
 
     DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete;
     DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete;
@@ -49,14 +43,12 @@ class DiagnosticStorageImpl : public DiagnosticStorageInterface
     bool IsEmptyBuffer();
 
 private:
+    DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize);
     DiagnosticStorageImpl();
     ~DiagnosticStorageImpl();
 
     TLVCircularBuffer mEndUserCircularBuffer;
-    TLVCircularBuffer mNetworkCircularBuffer;
-    uint8_t mEndUserBuffer[END_USER_BUFFER_SIZE];
-    uint8_t mNetworkBuffer[NETWORK_BUFFER_SIZE];
 };
-
+} // namespace Diagnostics
 } // namespace Tracing
 } // namespace chip