backend: update methods to propagate error to level up

pimpalemahesh · esp · commit d6e27ed94f5a · 2025-01-28T11:03:04.000+05:30
- pass diagnostic storage instance as a pointer to backend
- replace ESPLog statement with ChipLog for logging
diagnostic-storage: update IsEmptyBuffer method to IsBufferEmpty
- change diagnostic TAG enum class
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
@@ -124,7 +124,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE
         VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INTERNAL,
                             ChipLogError(DeviceLayer, "Diagnostic Storage instance cannot be null."));
         MutableByteSpan endUserSupportSpan(retrievalBuffer, CONFIG_RETRIEVAL_BUFFER_SIZE);
-        VerifyOrReturnError(!mStorageInstance->IsEmptyBuffer(), CHIP_ERROR_NOT_FOUND,
+        VerifyOrReturnError(!mStorageInstance->IsBufferEmpty(), CHIP_ERROR_NOT_FOUND,
                             ChipLogError(DeviceLayer, "Empty Diagnostic buffer"));
         // Retrieve data from the diagnostic storage
         CHIP_ERROR err = mStorageInstance->Retrieve(endUserSupportSpan, sReadEntries);
diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp
@@ -84,7 +84,7 @@ 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
     diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE);
-    static ESP32Diagnostics diagnosticBackend(diagnosticStorage);
+    static ESP32Diagnostics diagnosticBackend(&diagnosticStorage);
     Tracing::Register(diagnosticBackend);
 #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE
 }
diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp
@@ -42,12 +42,13 @@ uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const
     return mCounterList[label];
 }
 
-void ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance)
+CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface * mStorageInstance)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
+    VerifyOrReturnError(mStorageInstance != nullptr, err, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));
     Diagnostic<uint32_t> counter(label, GetInstanceCount(label), esp_log_timestamp());
-    err = mStorageInstance.Store(counter);
-    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data"));
+    err = mStorageInstance->Store(counter);
+    return err;
 }
 
 } // namespace Diagnostics
diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h
@@ -45,7 +45,7 @@ class ESPDiagnosticCounter
 
     uint32_t GetInstanceCount(const char * label) const;
 
-    void ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance);
+    CHIP_ERROR ReportMetrics(const char * label, DiagnosticStorageInterface * mStorageInstance);
 
 private:
     ESPDiagnosticCounter() {}
diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h
@@ -28,19 +28,11 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia
 public:
     CircularDiagnosticBuffer(uint8_t * buffer, size_t bufferLength) : chip::TLV::TLVCircularBuffer(buffer, bufferLength) {}
 
-    // Delete copy constructor and assignment operator to ensure singleton
-    CircularDiagnosticBuffer(const CircularDiagnosticBuffer &)             = delete;
-    CircularDiagnosticBuffer & operator=(const CircularDiagnosticBuffer &) = delete;
-
     CHIP_ERROR Store(const DiagnosticEntry & entry) override
     {
+        CHIP_ERROR err = CHIP_NO_ERROR;
         mWriter.Init(*this);
-
-        CHIP_ERROR err = entry.Encode(mWriter);
-        if (err != CHIP_NO_ERROR)
-        {
-            ChipLogError(DeviceLayer, "Failed to write entry: %s", chip::ErrorStr(err));
-        }
+        ReturnLogErrorOnFailure(entry.Encode(mWriter));
         return err;
     }
 
@@ -87,7 +79,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia
         return CHIP_NO_ERROR;
     }
 
-    bool IsEmptyBuffer() override { return DataLength() == 0; }
+    bool IsBufferEmpty() override { return DataLength() == 0; }
 
     uint32_t GetDataSize() override { return DataLength(); }
 
diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp
@@ -102,32 +102,33 @@ void ESP32Diagnostics::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {}
 void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
+    VerifyOrReturn(mStorageInstance != nullptr, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));
     switch (event.ValueType())
     {
     case ValueType::kInt32: {
-        ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32());
+        ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32());
         Diagnostic<int32_t> metric(event.key(), event.ValueInt32(), esp_log_timestamp());
-        err = mStorageInstance.Store(metric);
+        err = mStorageInstance->Store(metric);
     }
     break;
 
     case ValueType::kUInt32: {
-        ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32());
+        ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32());
         Diagnostic<uint32_t> metric(event.key(), event.ValueUInt32(), esp_log_timestamp());
-        err = mStorageInstance.Store(metric);
+        err = mStorageInstance->Store(metric);
     }
     break;
 
     case ValueType::kChipErrorCode:
-        ESP_LOGI("mtr", "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode());
+        ChipLogProgress(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode());
         break;
 
     case ValueType::kUndefined:
-        ESP_LOGI("mtr", "The value of %s is undefined", event.key());
+        ChipLogProgress(DeviceLayer, "The value of %s is undefined", event.key());
         break;
 
     default:
-        ESP_LOGI("mtr", "The value of %s is of an UNKNOWN TYPE", event.key());
+        ChipLogProgress(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key());
         break;
     }
 
@@ -136,14 +137,16 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)
 
 void ESP32Diagnostics::TraceCounter(const char * label)
 {
-    ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance);
+    CHIP_ERROR err = ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance);
+    VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter Diagnostic data"));
 }
 
 void ESP32Diagnostics::TraceBegin(const char * label, const char * group)
 {
     if (IsPermitted(group))
     {
-        StoreDiagnostics(label, group);
+        CHIP_ERROR err = StoreDiagnostics(label, group);
+        VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));
     }
 }
 
@@ -153,15 +156,18 @@ void ESP32Diagnostics::TraceInstant(const char * label, const char * value)
 {
     if (!IsPermitted(value))
     {
-        StoreDiagnostics(label, value);
+        CHIP_ERROR err = StoreDiagnostics(label, value);
+        VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));
     }
 }
 
-void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group)
+CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group)
 {
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    VerifyOrReturnError(mStorageInstance != nullptr, err, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));
     Diagnostic<const char *> trace(label, group, esp_log_timestamp());
-    VerifyOrReturn(mStorageInstance.Store(trace) == CHIP_NO_ERROR,
-                   ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));
+    err = mStorageInstance->Store(trace);
+    return err;
 }
 
 } // namespace Diagnostics
diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h
@@ -32,7 +32,7 @@ namespace Diagnostics {
 class ESP32Diagnostics : public ::chip::Tracing::Backend
 {
 public:
-    ESP32Diagnostics(DiagnosticStorageInterface & storageInstance) : mStorageInstance(storageInstance) {}
+    ESP32Diagnostics(DiagnosticStorageInterface * storageInstance) : mStorageInstance(storageInstance) {}
 
     // Deleted copy constructor and assignment operator to prevent copying
     ESP32Diagnostics(const ESP32Diagnostics &)             = delete;
@@ -54,11 +54,11 @@ class ESP32Diagnostics : public ::chip::Tracing::Backend
     void LogNodeDiscovered(NodeDiscoveredInfo &) override;
     void LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo &) override;
     void LogMetricEvent(const MetricEvent &) override;
-    void StoreDiagnostics(const char * label, const char * group);
 
 private:
     using ValueType = MetricEvent::Value::Type;
-    DiagnosticStorageInterface & mStorageInstance;
+    DiagnosticStorageInterface * mStorageInstance;
+    CHIP_ERROR StoreDiagnostics(const char * label, const char * group);
 };
 
 } // namespace Diagnostics
diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h
@@ -26,7 +26,7 @@ namespace Tracing {
 namespace Diagnostics {
 
 // Diagnostic TAGs
-enum D_TAG
+enum class DiagTag : uint8_t
 {
     TIMESTAMP = 0,
     LABEL,
@@ -68,15 +68,15 @@ class Diagnostic : public DiagnosticEntry
         chip::TLV::TLVType DiagnosticOuterContainer = chip::TLV::kTLVType_NotSpecified;
         ReturnErrorOnFailure(
             writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, DiagnosticOuterContainer));
-        ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(D_TAG::TIMESTAMP), timestamp_));
-        ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(D_TAG::LABEL), label_));
+        ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::TIMESTAMP), timestamp_));
+        ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), label_));
         if constexpr (std::is_same_v<T, const char *>)
         {
-            ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(D_TAG::VALUE), value_));
+            ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), value_));
         }
         else
         {
-            ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(D_TAG::VALUE), value_));
+            ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), value_));
         }
         ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer));
         ReturnErrorOnFailure(writer.Finalize());
@@ -121,7 +121,7 @@ class DiagnosticStorageInterface
      * @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;
+    virtual bool IsBufferEmpty() = 0;
 
     /**
      * @brief Retrieves the size of the data currently stored in the buffer.

Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ static void InitServer(intptr_t context)`
`84`	`84`	`Esp32AppServer::Init(); // Init ZCL Data Model and CHIP App Server AND Initialize device attestation config`
`85`	`85`	`#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE`
`86`	`86`	`diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE);`
`87`		`- static ESP32Diagnostics diagnosticBackend(diagnosticStorage);`
	`87`	`+ static ESP32Diagnostics diagnosticBackend(&diagnosticStorage);`
`88`	`88`	`Tracing::Register(diagnosticBackend);`
`89`	`89`	`#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE`
`90`	`90`	`}`
Original file line number	Diff line number	Diff line change
`@@ -42,12 +42,13 @@ uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const`
`42`	`42`	`return mCounterList[label];`
`43`	`43`	`}`
`44`	`44`
`45`		`-void ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance)`
	`45`	`+CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface * mStorageInstance)`
`46`	`46`	`{`
`47`	`47`	`CHIP_ERROR err = CHIP_NO_ERROR;`
	`48`	`+ VerifyOrReturnError(mStorageInstance != nullptr, err, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));`
`48`	`49`	`Diagnostic<uint32_t> counter(label, GetInstanceCount(label), esp_log_timestamp());`
`49`		`- err = mStorageInstance.Store(counter);`
`50`		`- VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data"));`
	`50`	`+ err = mStorageInstance->Store(counter);`
	`51`	`+ return err;`
`51`	`52`	`}`
`52`	`53`
`53`	`54`	`} // namespace Diagnostics`
Original file line number	Diff line number	Diff line change
`@@ -102,32 +102,33 @@ void ESP32Diagnostics::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {}`
`102`	`102`	`void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)`
`103`	`103`	`{`
`104`	`104`	`CHIP_ERROR err = CHIP_NO_ERROR;`
	`105`	`+ VerifyOrReturn(mStorageInstance != nullptr, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));`
`105`	`106`	`switch (event.ValueType())`
`106`	`107`	`{`
`107`	`108`	`case ValueType::kInt32: {`
`108`		`- ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32());`
	`109`	`+ ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32());`
`109`	`110`	`Diagnostic<int32_t> metric(event.key(), event.ValueInt32(), esp_log_timestamp());`
`110`		`- err = mStorageInstance.Store(metric);`
	`111`	`+ err = mStorageInstance->Store(metric);`
`111`	`112`	`}`
`112`	`113`	`break;`
`113`	`114`
`114`	`115`	`case ValueType::kUInt32: {`
`115`		`- ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32());`
	`116`	`+ ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32());`
`116`	`117`	`Diagnostic<uint32_t> metric(event.key(), event.ValueUInt32(), esp_log_timestamp());`
`117`		`- err = mStorageInstance.Store(metric);`
	`118`	`+ err = mStorageInstance->Store(metric);`
`118`	`119`	`}`
`119`	`120`	`break;`
`120`	`121`
`121`	`122`	`case ValueType::kChipErrorCode:`
`122`		`- ESP_LOGI("mtr", "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode());`
	`123`	`+ ChipLogProgress(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode());`
`123`	`124`	`break;`
`124`	`125`
`125`	`126`	`case ValueType::kUndefined:`
`126`		`- ESP_LOGI("mtr", "The value of %s is undefined", event.key());`
	`127`	`+ ChipLogProgress(DeviceLayer, "The value of %s is undefined", event.key());`
`127`	`128`	`break;`
`128`	`129`
`129`	`130`	`default:`
`130`		`- ESP_LOGI("mtr", "The value of %s is of an UNKNOWN TYPE", event.key());`
	`131`	`+ ChipLogProgress(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key());`
`131`	`132`	`break;`
`132`	`133`	`}`
`133`	`134`
`@@ -136,14 +137,16 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)`
`136`	`137`
`137`	`138`	`void ESP32Diagnostics::TraceCounter(const char * label)`
`138`	`139`	`{`
`139`		`- ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance);`
	`140`	`+ CHIP_ERROR err = ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance);`
	`141`	`+ VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter Diagnostic data"));`
`140`	`142`	`}`
`141`	`143`
`142`	`144`	`void ESP32Diagnostics::TraceBegin(const char * label, const char * group)`
`143`	`145`	`{`
`144`	`146`	`if (IsPermitted(group))`
`145`	`147`	`{`
`146`		`- StoreDiagnostics(label, group);`
	`148`	`+ CHIP_ERROR err = StoreDiagnostics(label, group);`
	`149`	`+ VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));`
`147`	`150`	`}`
`148`	`151`	`}`
`149`	`152`
`@@ -153,15 +156,18 @@ void ESP32Diagnostics::TraceInstant(const char * label, const char * value)`
`153`	`156`	`{`
`154`	`157`	`if (!IsPermitted(value))`
`155`	`158`	`{`
`156`		`- StoreDiagnostics(label, value);`
	`159`	`+ CHIP_ERROR err = StoreDiagnostics(label, value);`
	`160`	`+ VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));`
`157`	`161`	`}`
`158`	`162`	`}`
`159`	`163`
`160`		`-void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group)`
	`164`	`+CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group)`
`161`	`165`	`{`
	`166`	`+ CHIP_ERROR err = CHIP_NO_ERROR;`
	`167`	`+ VerifyOrReturnError(mStorageInstance != nullptr, err, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));`
`162`	`168`	`Diagnostic<const char *> trace(label, group, esp_log_timestamp());`
`163`		`- VerifyOrReturn(mStorageInstance.Store(trace) == CHIP_NO_ERROR,`
`164`		`- ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data"));`
	`169`	`+ err = mStorageInstance->Store(trace);`
	`170`	`+ return err;`
`165`	`171`	`}`
`166`	`172`
`167`	`173`	`} // namespace Diagnostics`