From 38a5b47f0d24c50fd088552bc1450a797a8d21bf Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 10 Oct 2024 13:00:23 +0530 Subject: [PATCH 01/25] esp32 diagnostic trace - Working backend with metric, trace and counter diagnostics - Diagnostic interface implementation with ring buffer storage - Added option ENABLE_ESP_DIAGNOSTICS_TRACE in chip KConfig - Added required options for enabling matter diagnostic trace in project Kconfig --- config/esp32/components/chip/CMakeLists.txt | 14 ++ config/esp32/components/chip/Kconfig | 26 ++- .../esp32/main/Kconfig.projbuild | 14 ++ ...diagnostic-logs-provider-delegate-impl.cpp | 39 +++- .../diagnostic-logs-provider-delegate-impl.h | 10 + .../esp32/main/main.cpp | 15 ++ src/tracing/esp32_diagnostic_trace/BUILD.gn | 47 +++++ .../esp32_diagnostic_trace/Counter.cpp | 68 ++++++ src/tracing/esp32_diagnostic_trace/Counter.h | 58 +++++ .../DiagnosticStorageManager.cpp | 154 ++++++++++++++ .../DiagnosticStorageManager.h | 54 +++++ .../DiagnosticTracing.cpp | 178 ++++++++++++++++ .../DiagnosticTracing.h | 71 +++++++ .../esp32_diagnostic_trace/Diagnostics.h | 198 ++++++++++++++++++ .../include/matter/tracing/macros_impl.h | 52 +++++ 15 files changed, 985 insertions(+), 13 deletions(-) create mode 100644 src/tracing/esp32_diagnostic_trace/BUILD.gn create mode 100644 src/tracing/esp32_diagnostic_trace/Counter.cpp create mode 100644 src/tracing/esp32_diagnostic_trace/Counter.h create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h create mode 100644 src/tracing/esp32_diagnostic_trace/Diagnostics.h create mode 100644 src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt index c1b0e0b6440cbe..975bc3812802fa 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -302,6 +302,11 @@ if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE) chip_gn_arg_append("matter_trace_config" "\"${CHIP_ROOT}/src/tracing/esp32_trace:esp32_trace_tracing\"") endif() +if (CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE) + chip_gn_arg_bool("matter_enable_tracing_support" "true") + chip_gn_arg_append("matter_trace_config" "\"${CHIP_ROOT}/src/tracing/esp32_diagnostic_trace:esp32_diagnostic_tracing\"") +endif() + if (CONFIG_ENABLE_ESP_INSIGHTS_SYSTEM_STATS) chip_gn_arg_append("matter_enable_esp_insights_system_stats" "true") endif() @@ -314,6 +319,10 @@ if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE) target_include_directories(${COMPONENT_LIB} INTERFACE "${CHIP_ROOT}/src/tracing/esp32_trace/include") endif() +if (CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE) + target_include_directories(${COMPONENT_LIB} INTERFACE "${CHIP_ROOT}/src/tracing/esp32_diagnostic_trace/include") +endif() + if (CONFIG_CHIP_DEVICE_ENABLE_DYNAMIC_SERVER) chip_gn_arg_append("chip_build_controller_dynamic_server" "true") endif() @@ -379,6 +388,11 @@ if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE) 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") +endif() + + # When using the pregenerated files, there is a edge case where an error appears for # undeclared argument chip_code_pre_generated_directory. To get around with it we are # disabling the --fail-on-unused-args flag. diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index 96ad4229635097..b402832e6dd979 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -860,6 +860,15 @@ menu "CHIP Device Layer" NVS namespace. If this option is enabled, the application can use an API to set a CD, the configured CD will be used for subsequent CD reads. + config ENABLE_ESP_DIAGNOSTICS_TRACE + bool "Enable ESP Platform Diagnostics for Matter" + depends on ESP_DIAGNOSTICS_ENABLED + default y + help + Enables the ESP Diagnostics platform to collect, store, and retrieve diagnostic data for the Matter protocol. + This feature helps monitor system health and performance by providing insights through diagnostics logs. + Requires ESP_DIAGNOSTICS_ENABLED to be activated. + config ENABLE_ESP_INSIGHTS_TRACE bool "Enable Matter ESP Insights" depends on ESP_INSIGHTS_ENABLED @@ -876,15 +885,14 @@ menu "CHIP Device Layer" help This option enables the system statistics to be sent to the insights cloud. - config MAX_PERMIT_LIST_SIZE - int "Set permit list size for Insights traces" - range 5 30 - depends on ESP_INSIGHTS_ENABLED - default 20 - help - Maximum number of group entries that can be included in the permit list for reporting - the traces to insights. - + config MAX_PERMIT_LIST_SIZE + int "Set permit list size for Insights traces" + range 5 30 + depends on ESP_INSIGHTS_ENABLED || ESP_DIAGNOSTICS_ENABLED + default 20 + help + Set the maximum number of group entries that can be included in the permit list for reporting + traces to Insights or diagnostics. This ensures proper management of trace reporting capacity. endmenu diff --git a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild index 4ec2e859c0b1f5..767b269d995432 100644 --- a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild +++ b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild @@ -83,3 +83,17 @@ depends on ENABLE_PW_RPC about available pin numbers for UART. endmenu + +menu "Platform Diagnostics" + config ESP_DIAGNOSTICS_ENABLED + bool "Enable ESP Diagnostics" + default n + + 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. +endmenu 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 index 3160bf71e12574..5659cce93e1579 100644 --- 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 @@ -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) { @@ -75,7 +80,14 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) switch (intent) { case IntentEnum::kEndUserSupport: - return static_cast(endUserSupportLogEnd - endUserSupportLogStart); + { + #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + return RETRIEVAL_BUFFER_SIZE; + #else + return static_cast(endUserSupportLogEnd - endUserSupportLogStart); + #endif + } + break; case IntentEnum::kNetworkDiag: return static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart); case IntentEnum::kCrashLogs: @@ -107,11 +119,30 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE switch (intent) { case IntentEnum::kEndUserSupport: { - context->EndUserSupport.span = - ByteSpan(&endUserSupportLogStart[0], static_cast(endUserSupportLogEnd - endUserSupportLogStart)); + #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + + if (diagnosticStorage.IsEmptyBuffer()) + { + printf("Buffer is empty\n"); + ChipLogError(DeviceLayer, "Empty Diagnostic buffer"); + return CHIP_ERROR_NOT_FOUND; + } + // Retrieve data from the diagnostic storage + CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan); + if (err != CHIP_NO_ERROR) + { + 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 + context->EndUserSupport.span = + ByteSpan(&endUserSupportLogStart[0], static_cast(endUserSupportLogEnd - endUserSupportLogStart)); + #endif } break; - case IntentEnum::kNetworkDiag: { context->NetworkDiag.span = ByteSpan(&networkDiagnosticLogStart[0], static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart)); diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index fc1350ed3ec265..4c53b8ed3d1139 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -19,12 +19,22 @@ #pragma once #include + +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include +#endif + #include #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #include #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_END_USER_BUFFER_SIZE +using namespace chip::Tracing::Diagnostics; +#endif + namespace chip { namespace app { namespace Clusters { diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 9f7a73011a19a8..d8d7018c7ba3dc 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -52,6 +52,10 @@ #include #endif // CONFIG_ENABLE_ESP32_DEVICE_INFO_PROVIDER +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include +#endif + namespace { #if CONFIG_ENABLE_ESP32_FACTORY_DATA_PROVIDER chip::DeviceLayer::ESP32FactoryDataProvider sFactoryDataProvider; @@ -72,6 +76,12 @@ 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 @@ -83,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. diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn new file mode 100644 index 00000000000000..60d425af9497cb --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/BUILD.gn @@ -0,0 +1,47 @@ +# +#Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import("//build_overrides/build.gni") +import("//build_overrides/chip.gni") + +config("tracing") { + include_dirs = [ "include" ] +} + +static_library("backend") { + output_name = "libEsp32DiagnosticsBackend" + output_dir = "${root_out_dir}/lib" + + sources = [ + "Counter.cpp", + "Counter.h", + "DiagnosticTracing.cpp", + "DiagnosticTracing.h", + "DiagnosticStorageManager.cpp", + "DiagnosticStorageManager.h", + "Diagnostics.h", + ] + + public_deps = [ + "${chip_root}/src/lib/core", + "${chip_root}/src/tracing", + ] +} + +source_set("esp32_diagnostic_tracing") { + public = [ "include/matter/tracing/macros_impl.h" ] + public_configs = [ ":tracing" ] + deps = [ ":backend" ] +} diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp new file mode 100644 index 00000000000000..77948129a8e8fe --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -0,0 +1,68 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +using namespace chip; + +namespace Diagnostics { + +// This is a one time allocation for counters. It is not supposed to be freed. +ESPDiagnosticCounter * ESPDiagnosticCounter::mHead = nullptr; + +ESPDiagnosticCounter * ESPDiagnosticCounter::GetInstance(const char * label) +{ + ESPDiagnosticCounter * current = mHead; + + while (current != nullptr) + { + if (strcmp(current->label, label) == 0) + { + current->instanceCount++; + return current; + } + current = current->mNext; + } + + // Allocate a new instance if counter is not present in the list. + void * ptr = Platform::MemoryAlloc(sizeof(ESPDiagnosticCounter)); + VerifyOrDie(ptr != nullptr); + + ESPDiagnosticCounter * newInstance = new (ptr) ESPDiagnosticCounter(label); + newInstance->mNext = mHead; + mHead = newInstance; + + return newInstance; +} + +int32_t ESPDiagnosticCounter::GetInstanceCount() const +{ + return instanceCount; +} + +void ESPDiagnosticCounter::ReportMetrics() +{ + CHIP_ERROR err = CHIP_NO_ERROR; + Counter counter(label, instanceCount, esp_log_timestamp()); + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + err = diagnosticStorage.Store(counter); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); +} + +} // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h new file mode 100644 index 00000000000000..4e58975999712f --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -0,0 +1,58 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" + +using namespace chip::Tracing::Diagnostics; + +namespace Diagnostics { + +/** + * This class is used to monotonically increment the counters as per the label of the counter macro + * '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. + */ + +class ESPDiagnosticCounter +{ +private: + static ESPDiagnosticCounter * mHead; // head of the counter list + const char * label; // unique key ,it is used as a static string. + int32_t instanceCount; + ESPDiagnosticCounter * mNext; // pointer to point to the next entry in the list + + ESPDiagnosticCounter(const char * labelParam) : label(labelParam), instanceCount(1), mNext(nullptr) {} + +public: + static ESPDiagnosticCounter * GetInstance(const char * label); + + int32_t GetInstanceCount() const; + + void ReportMetrics(); +}; + +} // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp new file mode 100644 index 00000000000000..98046d5523582b --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -0,0 +1,154 @@ + +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include + +namespace chip { +namespace Tracing { + +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() {} + +CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + + CircularTLVWriter writer; + writer.Init(mEndUserCircularBuffer); + + // Start a TLV structure container (Anonymous) + 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", ErrorStr(err))); + + err = diagnostic.Encode(writer); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DeviceLayer, "Failed to encode diagnostic data : %s", ErrorStr(err)); + err = CHIP_ERROR_INVALID_ARGUMENT; + writer.EndContainer(outerContainer); + writer.Finalize(); + return err; + } + err = writer.EndContainer(outerContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, 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")); + return CHIP_NO_ERROR; +} + +CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + CircularTLVReader reader; + reader.Init(mEndUserCircularBuffer); + + TLVWriter writer; + writer.Init(payload); + + TLVType outWriterContainer; + err = writer.StartContainer(AnonymousTag(), kTLVType_List, outWriterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container")); + + while (true) + { + err = reader.Next(); + if (err == CHIP_ERROR_END_OF_TLV) + { + ChipLogProgress(DeviceLayer, "No more data to read"); + break; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err))); + + if (reader.GetType() == kTLVType_Structure && reader.GetTag() == AnonymousTag()) + { + 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))); + + // Check if the current element is a METRIC or TRACE container + if ((reader.GetType() == kTLVType_Structure) && + (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == 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; + } + // Exit the outer container + 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"); + } + } + + err = writer.EndContainer(outWriterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to close outer container")); + // Finalize the writing process + err = writer.Finalize(); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); + payload.reduce_size(writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "Retrieval successful"); + return CHIP_NO_ERROR; +} + +bool DiagnosticStorageImpl::IsEmptyBuffer() +{ + return mEndUserCircularBuffer.DataLength() == 0; +} +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h new file mode 100644 index 00000000000000..1b21a6cb54ed5c --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -0,0 +1,54 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "Diagnostics.h" +#include +#include + +namespace chip { +namespace Tracing { +namespace Diagnostics { +using namespace chip::Platform; +using chip::TLV::TLVType; +class DiagnosticStorageImpl : public DiagnosticStorageInterface +{ +public: + + static DiagnosticStorageImpl& GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); + + DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; + DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete; + + CHIP_ERROR Store(DiagnosticEntry & diagnostic) override; + + CHIP_ERROR Retrieve(MutableByteSpan &payload) override; + + bool IsEmptyBuffer(); + +private: + DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize); + DiagnosticStorageImpl(); + ~DiagnosticStorageImpl(); + + TLVCircularBuffer mEndUserCircularBuffer; +}; +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp new file mode 100644 index 00000000000000..174eac0625f44f --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -0,0 +1,178 @@ + +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace chip { +namespace Tracing { +namespace Diagnostics { + +#define LOG_HEAP_INFO(label, group, entry_exit) \ + do \ + { \ + ESP_DIAG_EVENT("MTR_TRC", "%s - %s - %s", entry_exit, label, group); \ + } while (0) + +constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE; +using HashValue = uint32_t; + +// Implements a murmurhash with 0 seed. +uint32_t MurmurHash(const void * key) +{ + const uint32_t kMultiplier = 0x5bd1e995; + const uint32_t kShift = 24; + const unsigned char * data = (const unsigned char *) key; + uint32_t hash = 0; + + while (*data) + { + uint32_t value = *data++; + value *= kMultiplier; + value ^= value >> kShift; + value *= kMultiplier; + hash *= kMultiplier; + hash ^= value; + } + + hash ^= hash >> 13; + hash *= kMultiplier; + hash ^= hash >> 15; + + if (hash == 0) + { + ESP_LOGW("Tracing", "MurmurHash resulted in a hash value of 0"); + } + + return hash; +} + +HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), + MurmurHash("CASESession"), + MurmurHash("NetworkCommissioning"), + MurmurHash("GeneralCommissioning"), + MurmurHash("OperationalCredentials"), + MurmurHash("CASEServer"), + MurmurHash("BLE"), + MurmurHash("BLE_Error"), + MurmurHash("Wifi"), + MurmurHash("Wifi_Error"), + MurmurHash("Fabric") }; // namespace + +bool IsPermitted(HashValue hashValue) +{ + for (HashValue permitted : gPermitList) + { + if (permitted == 0) + { + break; + } + if (hashValue == permitted) + { + return true; + } + } + return false; +} + +void ESP32Diagnostics::LogMessageReceived(MessageReceivedInfo & info) {} + +void ESP32Diagnostics::LogMessageSend(MessageSendInfo & info) {} + +void ESP32Diagnostics::LogNodeLookup(NodeLookupInfo & info) {} + +void ESP32Diagnostics::LogNodeDiscovered(NodeDiscoveredInfo & info) {} + +void ESP32Diagnostics::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {} + +void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) +{ + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + CHIP_ERROR err = CHIP_NO_ERROR; + switch (event.ValueType()) + { + case ValueType::kInt32: { + ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32()); + Metric metric(event.key(), event.ValueInt32(), esp_log_timestamp()); + err = diagnosticStorage.Store(metric); + } + break; + + case ValueType::kUInt32: { + ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); + Metric metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); + err = diagnosticStorage.Store(metric); + } + break; + + case ValueType::kChipErrorCode: + ESP_LOGI("mtr", "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()); + break; + + default: + ESP_LOGI("mtr", "The value of %s is of an UNKNOWN TYPE", event.key()); + break; + } + + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Metric Diagnostic data")); +} + +void ESP32Diagnostics::TraceCounter(const char * label) +{ + ::Diagnostics::ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); +} + +void ESP32Diagnostics::TraceBegin(const char * label, const char * group) { + StoreDiagnostics(label, group); +} + +void ESP32Diagnostics::TraceEnd(const char * label, const char * group) { + StoreDiagnostics(label, group); +} + +void ESP32Diagnostics::TraceInstant(const char * label, const char * group) { + StoreDiagnostics(label, group); +} + +void ESP32Diagnostics::StoreDiagnostics(const char* label, const char* group) { + CHIP_ERROR err = CHIP_NO_ERROR; + HashValue hashValue = MurmurHash(group); + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + if (IsPermitted(hashValue)) + { + Trace trace(label, group, esp_log_timestamp()); + err = diagnosticStorage.Store(trace); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); + } +} + +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h new file mode 100644 index 00000000000000..6188f7b6b9c95e --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -0,0 +1,71 @@ +#pragma once + +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include + + +#include +namespace chip { +namespace Tracing { +namespace Diagnostics { +/// A Backend that outputs data to chip logging. +/// +/// Structured data is formatted as json strings. +class ESP32Diagnostics : public ::chip::Tracing::Backend +{ +public: + ESP32Diagnostics(uint8_t *buffer, size_t buffer_size) + { + DiagnosticStorageImpl::GetInstance(buffer, buffer_size); + } + + // Deleted copy constructor and assignment operator to prevent copying + ESP32Diagnostics(const ESP32Diagnostics&) = delete; + ESP32Diagnostics& operator=(const ESP32Diagnostics&) = delete; + + void TraceBegin(const char * label, const char * group) override; + + void TraceEnd(const char * label, const char * group) override; + + /// Trace a zero-sized event + void TraceInstant(const char * label, const char * group) override; + + void TraceCounter(const char * label) override; + + void LogMessageSend(MessageSendInfo &) override; + void LogMessageReceived(MessageReceivedInfo &) override; + + void LogNodeLookup(NodeLookupInfo &) override; + 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; +}; + +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip \ No newline at end of file diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h new file mode 100644 index 00000000000000..a824f4a0457559 --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -0,0 +1,198 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +namespace chip { +namespace Tracing { + +namespace Diagnostics { +using namespace chip::TLV; + +enum class DIAGNOSTICS_TAG +{ + METRIC = 0, + TRACE = 1, + COUNTER = 2, + LABEL = 3, + GROUP = 4, + VALUE = 5, + TIMESTAMP = 6 +}; + +class DiagnosticEntry { +public: + virtual ~DiagnosticEntry() = default; + virtual CHIP_ERROR Encode(CircularTLVWriter &writer) = 0; +}; + +template +class Metric : public DiagnosticEntry { +public: + Metric(const char* label, T value, uint32_t timestamp) + : label_(label), value_(value), timestamp_(timestamp) {} + + Metric() {} + + const char* GetLabel() const { return label_; } + T GetValue() const { return value_; } + uint32_t GetTimestamp() const { return timestamp_; } + + CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR err = CHIP_NO_ERROR; + TLVType metricContainer; + err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::METRIC), kTLVType_Structure, metricContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err))); + + // TIMESTAMP + err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", ErrorStr(err))); + + // LABEL + err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write LABEL for METRIC : %s", ErrorStr(err))); + + // VALUE + err = writer.Put(ContextTag(DIAGNOSTICS_TAG::VALUE), value_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write VALUE for METRIC : %s", ErrorStr(err))); + + 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))); + return err; + } + +private: + const char* label_; + T value_; + uint32_t timestamp_; +}; + +class Trace : public DiagnosticEntry { +public: + Trace(const char* label, const char* group, uint32_t timestamp) + : label_(label), group_(group), timestamp_(timestamp) {} + + Trace() {} + + const char* GetLabel() const { return label_; } + uint32_t GetTimestamp() const { return timestamp_; } + const char* GetGroup() const { return group_; } + + CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR err = CHIP_NO_ERROR; + TLVType traceContainer; + err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::TRACE), kTLVType_Structure, traceContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for Trace: %s", ErrorStr(err))); + + // TIMESTAMP + err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", ErrorStr(err))); + + // GROUP + err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::GROUP), group_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write GROUP for TRACE : %s", ErrorStr(err))); + + // LABEL + err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write LABEL for TRACE : %s", ErrorStr(err))); + + 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))); + return err; + } + +private: + const char* label_; + const char* group_; + uint32_t timestamp_; +}; + +class Counter : public DiagnosticEntry { +public: + Counter(const char* label, uint32_t count, uint32_t timestamp) + : label_(label), count_(count), timestamp_(timestamp) {} + + Counter() {} + + uint32_t GetCount() const { return count_; } + + uint32_t GetTimestamp() const { return timestamp_; } + + CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR err = CHIP_NO_ERROR; + TLVType counterContainer; + err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::COUNTER), kTLVType_Structure, counterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for Counter: %s", ErrorStr(err))); + + // TIMESTAMP + err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for COUNTER : %s", ErrorStr(err))); + + // LABEL + err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write LABEL for COUNTER : %s", ErrorStr(err))); + + // COUNT + err = writer.Put(ContextTag(DIAGNOSTICS_TAG::COUNTER), count_); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to write VALUE for COUNTER : %s", ErrorStr(err))); + + 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))); + return err; + } + +private: + const char* label_; + uint32_t count_; + uint32_t timestamp_; +}; + +class DiagnosticStorageInterface { +public: + virtual ~DiagnosticStorageInterface() = default; + + virtual CHIP_ERROR Store(DiagnosticEntry& diagnostic) = 0; + + virtual CHIP_ERROR Retrieve(MutableByteSpan &payload) = 0; +}; + +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h b/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h new file mode 100644 index 00000000000000..23231d1da38c8f --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h @@ -0,0 +1,52 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +/* Ensure we do not have double tracing macros defined */ +#if defined(MATTER_TRACE_BEGIN) +#error "Tracing macros seem to be double defined" +#endif + +#include + +// This gets forwarded to the multiplexed instance +#define MATTER_TRACE_BEGIN(label, group) ::chip::Tracing::Internal::Begin(label, group) +#define MATTER_TRACE_END(label, group) ::chip::Tracing::Internal::End(label, group) +#define MATTER_TRACE_INSTANT(label, group) ::chip::Tracing::Internal::Instant(label, group) +#define MATTER_TRACE_COUNTER(label) ::chip::Tracing::Internal::Counter(label) + +namespace chip { +namespace Tracing { +namespace Diagnostics { +class Scoped +{ +public: + inline Scoped(const char * label, const char * group) : mLabel(label), mGroup(group) { MATTER_TRACE_BEGIN(label, group); } + inline ~Scoped() {} + +private: + const char * mLabel; + const char * mGroup; +}; +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip +#define _CONCAT_IMPL(a, b) a##b +#define _MACRO_CONCAT(a, b) _CONCAT_IMPL(a, b) + +#define MATTER_TRACE_SCOPE(label, group) ::chip::Tracing::Diagnostics::Scoped _MACRO_CONCAT(_trace_scope, __COUNTER__)(label, group) From 54a2e6ec8a1115a3a71bac53d74890e66aa681fb Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 14 Nov 2024 11:53:06 +0530 Subject: [PATCH 02/25] example/temperature-measurement-app - Resolve buffer issues - Use single buffer for store and retrieve of diagnostics - Resolve data loss issue --- .../main/diagnostic-logs-provider-delegate-impl.cpp | 10 +++------- .../include/diagnostic-logs-provider-delegate-impl.h | 4 +++- .../temperature-measurement-app/esp32/main/main.cpp | 8 +------- 3 files changed, 7 insertions(+), 15 deletions(-) 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 index 5659cce93e1579..6fe5b43460aa72 100644 --- 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 @@ -29,11 +29,6 @@ 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) { @@ -82,7 +77,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - return RETRIEVAL_BUFFER_SIZE; + return DIAGNOSTIC_BUFFER_SIZE; #else return static_cast(endUserSupportLogEnd - endUserSupportLogStart); #endif @@ -121,10 +116,10 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + MutableByteSpan endUserSupportSpan(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); if (diagnosticStorage.IsEmptyBuffer()) { - printf("Buffer is empty\n"); ChipLogError(DeviceLayer, "Empty Diagnostic buffer"); return CHIP_ERROR_NOT_FOUND; } @@ -143,6 +138,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE #endif } break; + case IntentEnum::kNetworkDiag: { context->NetworkDiag.span = ByteSpan(&networkDiagnosticLogStart[0], static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart)); diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index 4c53b8ed3d1139..a0e9fb4e18012b 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -31,7 +31,9 @@ #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_END_USER_BUFFER_SIZE +#define DIAGNOSTIC_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE +static uint8_t endUserBuffer[DIAGNOSTIC_BUFFER_SIZE]; + using namespace chip::Tracing::Diagnostics; #endif diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index d8d7018c7ba3dc..59df5c5345440d 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -76,12 +76,6 @@ 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 @@ -94,7 +88,7 @@ extern "C" void app_main() #endif #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - static ESP32Diagnostics diagnosticBackend(endUserBuffer, buffer_size); + static ESP32Diagnostics diagnosticBackend(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); Tracing::Register(diagnosticBackend); #endif From 167bd6a618a636f9d8db7396c3fde7dc14f62d5a Mon Sep 17 00:00:00 2001 From: mahesh Date: Fri, 15 Nov 2024 13:03:10 +0530 Subject: [PATCH 03/25] backend: Add description for diagnosticstorage interface, remove unncessary comments, format files, namespace changes --- ...diagnostic-logs-provider-delegate-impl.cpp | 61 +++++---- src/tracing/esp32_diagnostic_trace/BUILD.gn | 4 +- .../esp32_diagnostic_trace/Counter.cpp | 12 +- src/tracing/esp32_diagnostic_trace/Counter.h | 10 +- .../DiagnosticStorageManager.cpp | 29 ++-- .../DiagnosticStorageManager.h | 13 +- .../DiagnosticTracing.cpp | 20 +-- .../DiagnosticTracing.h | 18 +-- .../esp32_diagnostic_trace/Diagnostics.h | 125 +++++++++++------- 9 files changed, 159 insertions(+), 133 deletions(-) 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 index 6fe5b43460aa72..597cc78a6e0888 100644 --- 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 @@ -74,15 +74,14 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) { switch (intent) { - case IntentEnum::kEndUserSupport: - { - #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - return DIAGNOSTIC_BUFFER_SIZE; - #else - return static_cast(endUserSupportLogEnd - endUserSupportLogStart); - #endif - } - break; + case IntentEnum::kEndUserSupport: { +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + return DIAGNOSTIC_BUFFER_SIZE; +#else + return static_cast(endUserSupportLogEnd - endUserSupportLogStart); +#endif + } + break; case IntentEnum::kNetworkDiag: return static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart); case IntentEnum::kCrashLogs: @@ -114,28 +113,28 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE switch (intent) { case IntentEnum::kEndUserSupport: { - #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - MutableByteSpan endUserSupportSpan(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); - - if (diagnosticStorage.IsEmptyBuffer()) - { - ChipLogError(DeviceLayer, "Empty Diagnostic buffer"); - return CHIP_ERROR_NOT_FOUND; - } - // Retrieve data from the diagnostic storage - CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan); - if (err != CHIP_NO_ERROR) - { - 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 - context->EndUserSupport.span = - ByteSpan(&endUserSupportLogStart[0], static_cast(endUserSupportLogEnd - endUserSupportLogStart)); - #endif +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + MutableByteSpan endUserSupportSpan(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); + + if (diagnosticStorage.IsEmptyBuffer()) + { + ChipLogError(DeviceLayer, "Empty Diagnostic buffer"); + return CHIP_ERROR_NOT_FOUND; + } + // Retrieve data from the diagnostic storage + CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan); + if (err != CHIP_NO_ERROR) + { + 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 + context->EndUserSupport.span = + ByteSpan(&endUserSupportLogStart[0], static_cast(endUserSupportLogEnd - endUserSupportLogStart)); +#endif } break; diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn index 60d425af9497cb..c0b9e845c17b5a 100644 --- a/src/tracing/esp32_diagnostic_trace/BUILD.gn +++ b/src/tracing/esp32_diagnostic_trace/BUILD.gn @@ -27,10 +27,10 @@ static_library("backend") { sources = [ "Counter.cpp", "Counter.h", - "DiagnosticTracing.cpp", - "DiagnosticTracing.h", "DiagnosticStorageManager.cpp", "DiagnosticStorageManager.h", + "DiagnosticTracing.cpp", + "DiagnosticTracing.h", "Diagnostics.h", ] diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 77948129a8e8fe..73116ed9a04acd 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -19,8 +19,8 @@ #include #include -using namespace chip; - +namespace chip { +namespace Tracing { namespace Diagnostics { // This is a one time allocation for counters. It is not supposed to be freed. @@ -45,8 +45,8 @@ ESPDiagnosticCounter * ESPDiagnosticCounter::GetInstance(const char * label) VerifyOrDie(ptr != nullptr); ESPDiagnosticCounter * newInstance = new (ptr) ESPDiagnosticCounter(label); - newInstance->mNext = mHead; - mHead = newInstance; + newInstance->mNext = mHead; + mHead = newInstance; return newInstance; } @@ -61,8 +61,10 @@ void ESPDiagnosticCounter::ReportMetrics() CHIP_ERROR err = CHIP_NO_ERROR; Counter counter(label, instanceCount, esp_log_timestamp()); DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - err = diagnosticStorage.Store(counter); + err = diagnosticStorage.Store(counter); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); } } // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index 4e58975999712f..6d8e23bd6d85e6 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -18,15 +18,15 @@ #pragma once +#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" #include #include #include #include #include -#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" - -using namespace chip::Tracing::Diagnostics; +namespace chip { +namespace Tracing { namespace Diagnostics { /** @@ -41,7 +41,7 @@ class ESPDiagnosticCounter { private: static ESPDiagnosticCounter * mHead; // head of the counter list - const char * label; // unique key ,it is used as a static string. + const char * label; // unique key ,it is used as a static string. int32_t instanceCount; ESPDiagnosticCounter * mNext; // pointer to point to the next entry in the list @@ -56,3 +56,5 @@ class ESPDiagnosticCounter }; } // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp index 98046d5523582b..d16430ce2f4ffe 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -23,12 +23,13 @@ namespace chip { namespace Tracing { +using namespace chip::TLV; namespace Diagnostics { -DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize) - : mEndUserCircularBuffer(buffer, bufferSize) {} +DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize) : mEndUserCircularBuffer(buffer, bufferSize) {} -DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) { +DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) +{ static DiagnosticStorageImpl instance(buffer, bufferSize); return instance; } @@ -42,7 +43,6 @@ CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) CircularTLVWriter writer; writer.Init(mEndUserCircularBuffer); - // Start a TLV structure container (Anonymous) TLVType outerContainer; err = writer.StartContainer(AnonymousTag(), kTLVType_Structure, outerContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, @@ -98,21 +98,25 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) 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))); + 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() == kTLVType_Structure) && - (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) + (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || + reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) { + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) + { err = writer.CopyElement(reader); - if (err == CHIP_ERROR_BUFFER_TOO_SMALL) { + 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 { + else + { ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); break; } @@ -123,7 +127,6 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) reader.ExitContainer(outerReaderContainer); return CHIP_ERROR_WRONG_TLV_TYPE; } - // Exit the outer container err = reader.ExitContainer(outerReaderContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err))); @@ -136,11 +139,11 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) err = writer.EndContainer(outWriterContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to close outer container")); - // Finalize the writing process err = writer.Finalize(); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); payload.reduce_size(writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "---------------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; } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 1b21a6cb54ed5c..d35f745130f343 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -19,26 +19,23 @@ #pragma once #include "Diagnostics.h" -#include #include +#include namespace chip { namespace Tracing { namespace Diagnostics { -using namespace chip::Platform; -using chip::TLV::TLVType; class DiagnosticStorageImpl : public DiagnosticStorageInterface { public: + static DiagnosticStorageImpl & GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); - static DiagnosticStorageImpl& GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); - - DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; + DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete; CHIP_ERROR Store(DiagnosticEntry & diagnostic) override; - CHIP_ERROR Retrieve(MutableByteSpan &payload) override; + CHIP_ERROR Retrieve(MutableByteSpan & payload) override; bool IsEmptyBuffer(); @@ -47,7 +44,7 @@ class DiagnosticStorageImpl : public DiagnosticStorageInterface DiagnosticStorageImpl(); ~DiagnosticStorageImpl(); - TLVCircularBuffer mEndUserCircularBuffer; + chip::TLV::TLVCircularBuffer mEndUserCircularBuffer; }; } // namespace Diagnostics } // namespace Tracing diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 174eac0625f44f..f620abf65407a6 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -111,7 +111,7 @@ void ESP32Diagnostics::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {} void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; switch (event.ValueType()) { case ValueType::kInt32: { @@ -146,24 +146,28 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ::Diagnostics::ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); + ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); } -void ESP32Diagnostics::TraceBegin(const char * label, const char * group) { +void ESP32Diagnostics::TraceBegin(const char * label, const char * group) +{ StoreDiagnostics(label, group); } -void ESP32Diagnostics::TraceEnd(const char * label, const char * group) { +void ESP32Diagnostics::TraceEnd(const char * label, const char * group) +{ StoreDiagnostics(label, group); } -void ESP32Diagnostics::TraceInstant(const char * label, const char * group) { +void ESP32Diagnostics::TraceInstant(const char * label, const char * group) +{ StoreDiagnostics(label, group); } -void ESP32Diagnostics::StoreDiagnostics(const char* label, const char* group) { - CHIP_ERROR err = CHIP_NO_ERROR; - HashValue hashValue = MurmurHash(group); +void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + HashValue hashValue = MurmurHash(group); DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); if (IsPermitted(hashValue)) { diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index 6188f7b6b9c95e..a94aa2d232d8a8 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -18,12 +18,11 @@ * limitations under the License. */ +#include #include #include -#include #include -#include - +#include #include namespace chip { @@ -35,14 +34,11 @@ namespace Diagnostics { class ESP32Diagnostics : public ::chip::Tracing::Backend { public: - ESP32Diagnostics(uint8_t *buffer, size_t buffer_size) - { - DiagnosticStorageImpl::GetInstance(buffer, buffer_size); - } + ESP32Diagnostics(uint8_t * buffer, size_t buffer_size) { DiagnosticStorageImpl::GetInstance(buffer, buffer_size); } // Deleted copy constructor and assignment operator to prevent copying - ESP32Diagnostics(const ESP32Diagnostics&) = delete; - ESP32Diagnostics& operator=(const ESP32Diagnostics&) = delete; + ESP32Diagnostics(const ESP32Diagnostics &) = delete; + ESP32Diagnostics & operator=(const ESP32Diagnostics &) = delete; void TraceBegin(const char * label, const char * group) override; @@ -60,7 +56,7 @@ 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); + void StoreDiagnostics(const char * label, const char * group); private: using ValueType = MetricEvent::Value::Type; @@ -68,4 +64,4 @@ class ESP32Diagnostics : public ::chip::Tracing::Backend } // namespace Diagnostics } // namespace Tracing -} // namespace chip \ No newline at end of file +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index a824f4a0457559..c4b31dd34c84e5 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -19,63 +19,64 @@ #pragma once #include -#include #include +#include namespace chip { namespace Tracing { namespace Diagnostics { -using namespace chip::TLV; enum class DIAGNOSTICS_TAG { - METRIC = 0, - TRACE = 1, - COUNTER = 2, - LABEL = 3, - GROUP = 4, - VALUE = 5, - TIMESTAMP = 6 + METRIC = 0, + TRACE = 1, + COUNTER = 2, + LABEL = 3, + GROUP = 4, + VALUE = 5, + TIMESTAMP = 6 }; -class DiagnosticEntry { +class DiagnosticEntry +{ public: - virtual ~DiagnosticEntry() = default; - virtual CHIP_ERROR Encode(CircularTLVWriter &writer) = 0; + virtual ~DiagnosticEntry() = default; + virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) = 0; }; -template -class Metric : public DiagnosticEntry { +template +class Metric : public DiagnosticEntry +{ public: - Metric(const char* label, T value, uint32_t timestamp) - : label_(label), value_(value), timestamp_(timestamp) {} + Metric(const char * label, T value, uint32_t timestamp) : label_(label), value_(value), timestamp_(timestamp) {} Metric() {} - const char* GetLabel() const { return label_; } + const char * GetLabel() const { return label_; } T GetValue() const { return value_; } uint32_t GetTimestamp() const { return timestamp_; } - CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + { CHIP_ERROR err = CHIP_NO_ERROR; - TLVType metricContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::METRIC), kTLVType_Structure, metricContainer); + 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))); // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), 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))); // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write LABEL for METRIC : %s", ErrorStr(err))); // VALUE - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::VALUE), value_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write VALUE for METRIC : %s", ErrorStr(err))); @@ -87,41 +88,42 @@ class Metric : public DiagnosticEntry { } private: - const char* label_; + const char * label_; T value_; uint32_t timestamp_; }; -class Trace : public DiagnosticEntry { +class Trace : public DiagnosticEntry +{ public: - Trace(const char* label, const char* group, uint32_t timestamp) - : label_(label), group_(group), timestamp_(timestamp) {} + Trace(const char * label, const char * group, uint32_t timestamp) : label_(label), group_(group), timestamp_(timestamp) {} Trace() {} - const char* GetLabel() const { return label_; } + const char * GetLabel() const { return label_; } uint32_t GetTimestamp() const { return timestamp_; } - const char* GetGroup() const { return group_; } + const char * GetGroup() const { return group_; } - CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + { CHIP_ERROR err = CHIP_NO_ERROR; - TLVType traceContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::TRACE), kTLVType_Structure, traceContainer); + 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))); // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), 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))); // GROUP - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::GROUP), group_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::GROUP), group_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write GROUP for TRACE : %s", ErrorStr(err))); // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write LABEL for TRACE : %s", ErrorStr(err))); @@ -133,15 +135,15 @@ class Trace : public DiagnosticEntry { } private: - const char* label_; - const char* group_; + const char * label_; + const char * group_; uint32_t timestamp_; }; -class Counter : public DiagnosticEntry { +class Counter : public DiagnosticEntry +{ public: - Counter(const char* label, uint32_t count, uint32_t timestamp) - : label_(label), count_(count), timestamp_(timestamp) {} + Counter(const char * label, uint32_t count, uint32_t timestamp) : label_(label), count_(count), timestamp_(timestamp) {} Counter() {} @@ -149,25 +151,27 @@ class Counter : public DiagnosticEntry { uint32_t GetTimestamp() const { return timestamp_; } - CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + { CHIP_ERROR err = CHIP_NO_ERROR; - TLVType counterContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::COUNTER), kTLVType_Structure, counterContainer); + 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))); // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for COUNTER : %s", ErrorStr(err))); // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write LABEL for COUNTER : %s", ErrorStr(err))); // COUNT - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::COUNTER), count_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), count_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write VALUE for COUNTER : %s", ErrorStr(err))); @@ -179,18 +183,37 @@ class Counter : public DiagnosticEntry { } private: - const char* label_; + const char * label_; uint32_t count_; uint32_t timestamp_; }; -class DiagnosticStorageInterface { +/** + * @brief Interface for storing and retrieving diagnostic data. + */ +class DiagnosticStorageInterface +{ public: + /** + * @brief Virtual destructor for the interface. + */ virtual ~DiagnosticStorageInterface() = default; - virtual CHIP_ERROR Store(DiagnosticEntry& diagnostic) = 0; - - virtual CHIP_ERROR Retrieve(MutableByteSpan &payload) = 0; + /** + * @brief Stores a diagnostic entry. + * @param diagnostic Reference to a DiagnosticEntry object containing the + * diagnostic data to store. + * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. + */ + virtual CHIP_ERROR Store(DiagnosticEntry & diagnostic) = 0; + + /** + * @brief Retrieves diagnostic data as a payload. + * @param payload Reference to a MutableByteSpan where the retrieved + * diagnostic data will be stored. + * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. + */ + virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0; }; } // namespace Diagnostics From 640911ba156b1afa197fab682558edaada8dd8b6 Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 19 Nov 2024 17:37:19 +0530 Subject: [PATCH 04/25] temperature_measurement_app: Review changes --- config/esp32/components/chip/Kconfig | 6 ++---- .../esp32/main/Kconfig.projbuild | 5 +---- .../esp32/main/diagnostic-logs-provider-delegate-impl.cpp | 5 +++-- .../main/include/diagnostic-logs-provider-delegate-impl.h | 6 ++---- examples/temperature-measurement-app/esp32/main/main.cpp | 2 +- 5 files changed, 9 insertions(+), 15 deletions(-) diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index b402832e6dd979..9d9eb716db1d52 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -862,12 +862,10 @@ menu "CHIP Device Layer" config ENABLE_ESP_DIAGNOSTICS_TRACE bool "Enable ESP Platform Diagnostics for Matter" - depends on ESP_DIAGNOSTICS_ENABLED - default y + default n help Enables the ESP Diagnostics platform to collect, store, and retrieve diagnostic data for the Matter protocol. This feature helps monitor system health and performance by providing insights through diagnostics logs. - Requires ESP_DIAGNOSTICS_ENABLED to be activated. config ENABLE_ESP_INSIGHTS_TRACE bool "Enable Matter ESP Insights" @@ -888,7 +886,7 @@ menu "CHIP Device Layer" config MAX_PERMIT_LIST_SIZE int "Set permit list size for Insights traces" range 5 30 - depends on ESP_INSIGHTS_ENABLED || ESP_DIAGNOSTICS_ENABLED + depends on ESP_INSIGHTS_ENABLED || ENABLE_ESP_DIAGNOSTICS_TRACE default 20 help Set the maximum number of group entries that can be included in the permit list for reporting diff --git a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild index 767b269d995432..ac3965d63fdd5a 100644 --- a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild +++ b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild @@ -85,13 +85,10 @@ depends on ENABLE_PW_RPC endmenu menu "Platform Diagnostics" - config ESP_DIAGNOSTICS_ENABLED - bool "Enable ESP Diagnostics" - default n config END_USER_BUFFER_SIZE int "Set buffer size for end user diagnostic data" - depends on ESP_DIAGNOSTICS_ENABLED + depends on ENABLE_ESP_DIAGNOSTICS_TRACE default 4096 help Defines the buffer size (in bytes) for storing diagnostic data related to end user activity. 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 index 597cc78a6e0888..73a31dc5312f25 100644 --- 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 @@ -76,7 +76,8 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - return DIAGNOSTIC_BUFFER_SIZE; + // Returning buffer_size > 1024 bytes to transfer data over BDX otherwise it uses Response Payload. + return CONFIG_END_USER_BUFFER_SIZE; #else return static_cast(endUserSupportLogEnd - endUserSupportLogStart); #endif @@ -115,7 +116,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - MutableByteSpan endUserSupportSpan(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); + MutableByteSpan endUserSupportSpan(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); if (diagnosticStorage.IsEmptyBuffer()) { diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index a0e9fb4e18012b..a759aa946b2d3e 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -21,7 +21,7 @@ #include #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include +#include #endif #include @@ -31,9 +31,7 @@ #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#define DIAGNOSTIC_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE -static uint8_t endUserBuffer[DIAGNOSTIC_BUFFER_SIZE]; - +static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; using namespace chip::Tracing::Diagnostics; #endif diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 59df5c5345440d..39a4be0d726493 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -88,7 +88,7 @@ extern "C" void app_main() #endif #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - static ESP32Diagnostics diagnosticBackend(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); + static ESP32Diagnostics diagnosticBackend(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); Tracing::Register(diagnosticBackend); #endif From 8cc00d8f59ec2636d5fb99578e6bdd24412cc07c Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 26 Nov 2024 15:33:41 +0530 Subject: [PATCH 05/25] esp32_diagnostic_trace: Review changes - backend: Replace linkedlist with map for counter diagnostics - backend: Pass diagnostic storage as a parameter to backend - esp32_diagnostic_trace: Return actual data size --- ...diagnostic-logs-provider-delegate-impl.cpp | 3 +- .../esp32/main/main.cpp | 3 +- scripts/tools/check_includes_config.py | 3 ++ .../esp32_diagnostic_trace/Counter.cpp | 41 ++++++------------- src/tracing/esp32_diagnostic_trace/Counter.h | 27 ++++++------ .../DiagnosticStorageManager.cpp | 9 +++- .../DiagnosticStorageManager.h | 4 +- .../DiagnosticTracing.cpp | 16 ++++---- .../DiagnosticTracing.h | 5 ++- .../esp32_diagnostic_trace/Diagnostics.h | 1 - 10 files changed, 55 insertions(+), 57 deletions(-) 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 index 73a31dc5312f25..271958813160a7 100644 --- 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 @@ -76,8 +76,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - // Returning buffer_size > 1024 bytes to transfer data over BDX otherwise it uses Response Payload. - return CONFIG_END_USER_BUFFER_SIZE; + return DiagnosticStorageImpl::GetInstance().GetDataSize(); #else return static_cast(endUserSupportLogEnd - endUserSupportLogStart); #endif diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 39a4be0d726493..dbb99429e94b54 100644 --- 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 - static ESP32Diagnostics diagnosticBackend(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); + static ESP32Diagnostics diagnosticBackend(diagnosticStorage); Tracing::Register(diagnosticBackend); #endif diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py index eacc3274c3ba05..b06868506a1895 100644 --- a/scripts/tools/check_includes_config.py +++ b/scripts/tools/check_includes_config.py @@ -168,6 +168,9 @@ 'src/tracing/json/json_tracing.cpp': {'string', 'sstream'}, 'src/tracing/json/json_tracing.h': {'fstream', 'unordered_map', 'string'}, + # esp32 diagnostic tracing + 'src/tracing/esp32_diagnostic_trace/Counter.h': {'map'}, + # esp32 tracing 'src/tracing/esp32_trace/esp32_tracing.h': {'unordered_map'}, diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 73116ed9a04acd..e9043823f6d06c 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -23,45 +23,30 @@ namespace chip { namespace Tracing { namespace Diagnostics { -// This is a one time allocation for counters. It is not supposed to be freed. -ESPDiagnosticCounter * ESPDiagnosticCounter::mHead = nullptr; +std::map ESPDiagnosticCounter::mCounterList; -ESPDiagnosticCounter * ESPDiagnosticCounter::GetInstance(const char * label) +void ESPDiagnosticCounter::CountInit(const char * label) { - ESPDiagnosticCounter * current = mHead; - - while (current != nullptr) + if (mCounterList.find(label) != mCounterList.end()) { - if (strcmp(current->label, label) == 0) - { - current->instanceCount++; - return current; - } - current = current->mNext; + mCounterList[label]++; + } + else + { + mCounterList[label] = 1; } - - // Allocate a new instance if counter is not present in the list. - void * ptr = Platform::MemoryAlloc(sizeof(ESPDiagnosticCounter)); - VerifyOrDie(ptr != nullptr); - - ESPDiagnosticCounter * newInstance = new (ptr) ESPDiagnosticCounter(label); - newInstance->mNext = mHead; - mHead = newInstance; - - return newInstance; } -int32_t ESPDiagnosticCounter::GetInstanceCount() const +uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const { - return instanceCount; + return mCounterList[label]; } -void ESPDiagnosticCounter::ReportMetrics() +void ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance) { CHIP_ERROR err = CHIP_NO_ERROR; - Counter counter(label, instanceCount, esp_log_timestamp()); - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - err = diagnosticStorage.Store(counter); + Counter counter(label, GetInstanceCount(label), esp_log_timestamp()); + err = mStorageInstance.Store(counter); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); } diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index 6d8e23bd6d85e6..ea882b3487d3b3 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -18,11 +18,12 @@ #pragma once -#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" +#include "tracing/esp32_diagnostic_trace/Diagnostics.h" #include #include #include #include +#include #include namespace chip { @@ -39,20 +40,22 @@ namespace Diagnostics { class ESPDiagnosticCounter { -private: - static ESPDiagnosticCounter * mHead; // head of the counter list - const char * label; // unique key ,it is used as a static string. - int32_t instanceCount; - ESPDiagnosticCounter * mNext; // pointer to point to the next entry in the list - - ESPDiagnosticCounter(const char * labelParam) : label(labelParam), instanceCount(1), mNext(nullptr) {} - public: - static ESPDiagnosticCounter * GetInstance(const char * label); + static ESPDiagnosticCounter & GetInstance(const char * label) + { + static ESPDiagnosticCounter instance; + CountInit(label); + return instance; + } - int32_t GetInstanceCount() const; + uint32_t GetInstanceCount(const char * label) const; - void ReportMetrics(); + void ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance); + +private: + ESPDiagnosticCounter() {} + static std::map mCounterList; + static void CountInit(const char * label); }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp index d16430ce2f4ffe..bc3367d0e2e3a0 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -21,6 +21,8 @@ #include #include +#define TLV_CLOSING_BYTES 4 + namespace chip { namespace Tracing { using namespace chip::TLV; @@ -105,7 +107,7 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < (writer.GetRemainingFreeLength() + TLV_CLOSING_BYTES)) { err = writer.CopyElement(reader); if (err == CHIP_ERROR_BUFFER_TOO_SMALL) @@ -152,6 +154,11 @@ bool DiagnosticStorageImpl::IsEmptyBuffer() { return mEndUserCircularBuffer.DataLength() == 0; } + +uint32_t DiagnosticStorageImpl::GetDataSize() +{ + return mEndUserCircularBuffer.DataLength(); +} } // namespace Diagnostics } // namespace Tracing } // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index d35f745130f343..647a552495b9f2 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -18,9 +18,9 @@ #pragma once -#include "Diagnostics.h" #include #include +#include namespace chip { namespace Tracing { @@ -39,6 +39,8 @@ class DiagnosticStorageImpl : public DiagnosticStorageInterface bool IsEmptyBuffer(); + uint32_t GetDataSize(); + private: DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize); DiagnosticStorageImpl(); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index f620abf65407a6..2ebc9084a05504 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -110,21 +110,20 @@ void ESP32Diagnostics::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {} void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; switch (event.ValueType()) { case ValueType::kInt32: { ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32()); Metric metric(event.key(), event.ValueInt32(), esp_log_timestamp()); - err = diagnosticStorage.Store(metric); + err = mStorageInstance.Store(metric); } break; case ValueType::kUInt32: { ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); Metric metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); - err = diagnosticStorage.Store(metric); + err = mStorageInstance.Store(metric); } break; @@ -146,7 +145,7 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); + ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance); } void ESP32Diagnostics::TraceBegin(const char * label, const char * group) @@ -166,13 +165,12 @@ 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); - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + CHIP_ERROR err = CHIP_NO_ERROR; + HashValue hashValue = MurmurHash(group); if (IsPermitted(hashValue)) { Trace trace(label, group, esp_log_timestamp()); - err = diagnosticStorage.Store(trace); + err = mStorageInstance.Store(trace); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); } } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index a94aa2d232d8a8..5ab60a31457447 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -21,7 +21,7 @@ #include #include #include -#include +#include #include #include @@ -34,7 +34,7 @@ namespace Diagnostics { class ESP32Diagnostics : public ::chip::Tracing::Backend { public: - ESP32Diagnostics(uint8_t * buffer, size_t buffer_size) { DiagnosticStorageImpl::GetInstance(buffer, buffer_size); } + ESP32Diagnostics(DiagnosticStorageInterface & storageInstance) : mStorageInstance(storageInstance) {} // Deleted copy constructor and assignment operator to prevent copying ESP32Diagnostics(const ESP32Diagnostics &) = delete; @@ -60,6 +60,7 @@ class ESP32Diagnostics : public ::chip::Tracing::Backend private: using ValueType = MetricEvent::Value::Type; + DiagnosticStorageInterface & mStorageInstance; }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index c4b31dd34c84e5..256d73d041e4f1 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -17,7 +17,6 @@ */ #pragma once - #include #include #include From 029d53465222da7089c9befa3faff7f77c92e82b Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 19 Nov 2024 17:37:19 +0530 Subject: [PATCH 06/25] temperature_measurement_app: Review changes --- .../esp32/main/diagnostic-logs-provider-delegate-impl.cpp | 5 +++++ 1 file changed, 5 insertions(+) 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 index 271958813160a7..0770b940d130c0 100644 --- 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 @@ -29,6 +29,10 @@ using namespace chip::app::Clusters::DiagnosticLogs; LogProvider LogProvider::sInstance; LogProvider::CrashLogContext LogProvider::sCrashLogContext; +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +static uint32_t sIntentSize = CONFIG_END_USER_BUFFER_SIZE; +#endif + namespace { bool IsValidIntent(IntentEnum intent) { @@ -129,6 +133,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); return err; } + sIntentSize = endUserSupportSpan.size(); // Now, assign the span to the EndUserSupport object or whatever is required context->EndUserSupport.span = endUserSupportSpan; #else From da9e7f16276dd36071877a254a920b2c730e092d Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 21 Nov 2024 14:10:32 +0530 Subject: [PATCH 07/25] backend: map for counter diagnostics related changes - esp32_diagnostic_trace: improve storage class design - add extra tlv closing bytes check before copying diagnostic - unify diagnostic entries into a single type --- ...diagnostic-logs-provider-delegate-impl.cpp | 4 +- .../esp32/main/main.cpp | 3 +- src/tracing/esp32_diagnostic_trace/BUILD.gn | 1 - .../esp32_diagnostic_trace/Counter.cpp | 6 +- src/tracing/esp32_diagnostic_trace/Counter.h | 9 +- .../DiagnosticStorageManager.cpp | 164 --------------- .../DiagnosticStorageManager.h | 93 +++++++-- .../DiagnosticTracing.cpp | 52 ++--- .../DiagnosticTracing.h | 2 - .../esp32_diagnostic_trace/Diagnostics.h | 193 +++++------------- .../include/matter/tracing/macros_impl.h | 2 +- 11 files changed, 157 insertions(+), 372 deletions(-) delete mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp 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 index 0770b940d130c0..2a7c40fe4526a7 100644 --- 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 @@ -80,7 +80,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(endUserSupportLogEnd - endUserSupportLogStart); #endif @@ -118,7 +118,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 index dbb99429e94b54..63673094b35b87 100644 --- 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 index c0b9e845c17b5a..5516968ea639af 100644 --- 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.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index e9043823f6d06c..11e2bb2b1430ce 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -16,7 +16,7 @@ * limitations under the License. */ -#include +#include #include namespace chip { @@ -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()) { @@ -45,7 +45,7 @@ uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const void ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance) { CHIP_ERROR err = CHIP_NO_ERROR; - Counter counter(label, GetInstanceCount(label), esp_log_timestamp()); + Diagnostic counter(label, GetInstanceCount(label), esp_log_timestamp()); err = mStorageInstance.Store(counter); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); } 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.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp deleted file mode 100644 index bc3367d0e2e3a0..00000000000000 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ /dev/null @@ -1,164 +0,0 @@ - -/* - * - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include -#include - -#define TLV_CLOSING_BYTES 4 - -namespace chip { -namespace Tracing { -using namespace chip::TLV; - -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() {} - -CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - - CircularTLVWriter writer; - writer.Init(mEndUserCircularBuffer); - - 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", ErrorStr(err))); - - err = diagnostic.Encode(writer); - if (err != CHIP_NO_ERROR) - { - ChipLogError(DeviceLayer, "Failed to encode diagnostic data : %s", ErrorStr(err)); - err = CHIP_ERROR_INVALID_ARGUMENT; - writer.EndContainer(outerContainer); - writer.Finalize(); - return err; - } - err = writer.EndContainer(outerContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, 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")); - return CHIP_NO_ERROR; -} - -CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - CircularTLVReader reader; - reader.Init(mEndUserCircularBuffer); - - TLVWriter writer; - writer.Init(payload); - - TLVType outWriterContainer; - err = writer.StartContainer(AnonymousTag(), kTLVType_List, outWriterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container")); - - while (true) - { - err = reader.Next(); - if (err == CHIP_ERROR_END_OF_TLV) - { - ChipLogProgress(DeviceLayer, "No more data to read"); - break; - } - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err))); - - if (reader.GetType() == kTLVType_Structure && reader.GetTag() == AnonymousTag()) - { - 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() == kTLVType_Structure) && - (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || - reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) - { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < (writer.GetRemainingFreeLength() + TLV_CLOSING_BYTES)) - { - 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"); - } - } - - 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")); - payload.reduce_size(writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", - writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "Retrieval successful"); - return CHIP_NO_ERROR; -} - -bool DiagnosticStorageImpl::IsEmptyBuffer() -{ - return mEndUserCircularBuffer.DataLength() == 0; -} - -uint32_t DiagnosticStorageImpl::GetDataSize() -{ - return mEndUserCircularBuffer.DataLength(); -} -} // namespace Diagnostics -} // namespace Tracing -} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 647a552495b9f2..723a25e9654c9e 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -18,36 +18,99 @@ #pragma once -#include -#include #include +#define TLV_CLOSING_BYTE 1 + 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 = chip::TLV::kTLVType_NotSpecified; + ReturnErrorOnFailure(writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_List, outWriterContainer)); + + 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()) + { + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < ((writer.GetRemainingFreeLength() + TLV_CLOSING_BYTE))) + { + err = writer.CopyElement(reader); + if (err == CHIP_ERROR_BUFFER_TOO_SMALL) + { + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); + break; + } + } + else + { + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); + break; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); + } + else + { + ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container"); + } + } - chip::TLV::TLVCircularBuffer mEndUserCircularBuffer; + ReturnErrorOnFailure(writer.EndContainer(outWriterContainer)); + ReturnErrorOnFailure(writer.Finalize()); + span.reduce_size(writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", writer.GetLengthWritten()); + 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/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 2ebc9084a05504..05c22bb53bfdcb 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -19,24 +19,14 @@ #include #include -#include #include -#include -#include #include #include -#include namespace chip { namespace Tracing { namespace Diagnostics { -#define LOG_HEAP_INFO(label, group, entry_exit) \ - do \ - { \ - ESP_DIAG_EVENT("MTR_TRC", "%s - %s - %s", entry_exit, label, group); \ - } while (0) - constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE; using HashValue = uint32_t; @@ -76,14 +66,12 @@ HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("GeneralCommissioning"), MurmurHash("OperationalCredentials"), MurmurHash("CASEServer"), - MurmurHash("BLE"), - MurmurHash("BLE_Error"), - MurmurHash("Wifi"), - MurmurHash("Wifi_Error"), - MurmurHash("Fabric") }; // namespace + MurmurHash("Fabric"), + MurmurHash("Resolver") }; // namespace -bool IsPermitted(HashValue hashValue) +bool IsPermitted(const char * str) { + HashValue hashValue = MurmurHash(str); for (HashValue permitted : gPermitList) { if (permitted == 0) @@ -115,14 +103,14 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { case ValueType::kInt32: { ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32()); - Metric metric(event.key(), event.ValueInt32(), esp_log_timestamp()); + Diagnostic metric(event.key(), event.ValueInt32(), esp_log_timestamp()); err = mStorageInstance.Store(metric); } break; case ValueType::kUInt32: { ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); - Metric metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); + Diagnostic metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); err = mStorageInstance.Store(metric); } break; @@ -150,29 +138,27 @@ 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); -} +void ESP32Diagnostics::TraceEnd(const char * label, const char * group) {} -void ESP32Diagnostics::TraceInstant(const char * label, const char * group) +void ESP32Diagnostics::TraceInstant(const char * label, const char * value) { - StoreDiagnostics(label, group); + if (!IsPermitted(value)) + { + StoreDiagnostics(label, value); + } } void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) { - CHIP_ERROR err = CHIP_NO_ERROR; - HashValue hashValue = MurmurHash(group); - if (IsPermitted(hashValue)) - { - Trace 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 256d73d041e4f1..d035467d8909ca 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -17,73 +17,63 @@ */ #pragma once -#include + #include -#include namespace chip { namespace Tracing { namespace Diagnostics { -enum class DIAGNOSTICS_TAG -{ - METRIC = 0, - TRACE = 1, - COUNTER = 2, - LABEL = 3, - GROUP = 4, - VALUE = 5, - TIMESTAMP = 6 -}; - +/** + * @class DiagnosticEntry + * @brief Abstract base class for encoding diagnostic entries into TLV format. + * + */ class DiagnosticEntry { public: - virtual ~DiagnosticEntry() = default; + /** + * @brief Virtual destructor for proper cleanup in derived classes. + */ + virtual ~DiagnosticEntry() = default; + + /** + * @brief Pure virtual method to encode diagnostic data into a TLV structure. + * + * @param writer A reference to the `chip::TLV::CircularTLVWriter` instance + * used to encode the TLV data. + * @return CHIP_ERROR Returns an error code indicating the success or + * failure of the encoding operation. + */ virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) = 0; }; template -class Metric : public DiagnosticEntry +class Diagnostic : public DiagnosticEntry { public: - Metric(const char * label, T value, uint32_t timestamp) : label_(label), value_(value), timestamp_(timestamp) {} - - Metric() {} - - const char * GetLabel() const { return label_; } - T GetValue() const { return value_; } - uint32_t GetTimestamp() const { return timestamp_; } + Diagnostic(const char * label, T value, uint32_t timestamp) : label_(label), value_(value), timestamp_(timestamp) {} CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override { - CHIP_ERROR err = CHIP_NO_ERROR; - 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))); - - // 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))); - - // LABEL - err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write LABEL for METRIC : %s", ErrorStr(err))); - - // VALUE - err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write VALUE for METRIC : %s", ErrorStr(err))); - - 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))); - return err; + 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(0), timestamp_)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(1), label_)); + if constexpr (std::is_same_v) + { + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(2), value_)); + } + else + { + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(2), value_)); + } + ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); + ReturnErrorOnFailure(writer.Finalize()); + ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", label_); + return CHIP_NO_ERROR; } private: @@ -92,101 +82,6 @@ class Metric : public DiagnosticEntry uint32_t timestamp_; }; -class Trace : public DiagnosticEntry -{ -public: - Trace(const char * label, const char * group, uint32_t timestamp) : label_(label), group_(group), timestamp_(timestamp) {} - - Trace() {} - - const char * GetLabel() const { return label_; } - uint32_t GetTimestamp() const { return timestamp_; } - const char * GetGroup() const { return group_; } - - CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override - { - CHIP_ERROR err = CHIP_NO_ERROR; - 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))); - - // 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))); - - // GROUP - err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::GROUP), group_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write GROUP for TRACE : %s", ErrorStr(err))); - - // LABEL - err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write LABEL for TRACE : %s", ErrorStr(err))); - - 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))); - return err; - } - -private: - const char * label_; - const char * group_; - uint32_t timestamp_; -}; - -class Counter : public DiagnosticEntry -{ -public: - Counter(const char * label, uint32_t count, uint32_t timestamp) : label_(label), count_(count), timestamp_(timestamp) {} - - Counter() {} - - uint32_t GetCount() const { return count_; } - - uint32_t GetTimestamp() const { return timestamp_; } - - CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override - { - CHIP_ERROR err = CHIP_NO_ERROR; - 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))); - - // TIMESTAMP - err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for COUNTER : %s", ErrorStr(err))); - - // LABEL - err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write LABEL for COUNTER : %s", ErrorStr(err))); - - // COUNT - err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), count_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write VALUE for COUNTER : %s", ErrorStr(err))); - - 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))); - return err; - } - -private: - const char * label_; - uint32_t count_; - uint32_t timestamp_; -}; - /** * @brief Interface for storing and retrieving diagnostic data. */ @@ -213,6 +108,18 @@ 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; + + /** + * @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; }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h b/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h index 23231d1da38c8f..2c96e80dedbea2 100644 --- a/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h +++ b/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h @@ -37,7 +37,7 @@ class Scoped { public: inline Scoped(const char * label, const char * group) : mLabel(label), mGroup(group) { MATTER_TRACE_BEGIN(label, group); } - inline ~Scoped() {} + inline ~Scoped() { MATTER_TRACE_END(mLabel, mGroup); } private: const char * mLabel; From 03f8f7a8eefe9a43f21b32ab67851b2e6d765e91 Mon Sep 17 00:00:00 2001 From: mahesh Date: Wed, 11 Dec 2024 12:01:55 +0530 Subject: [PATCH 08/25] example: Remove redundant code --- ...diagnostic-logs-provider-delegate-impl.cpp | 24 +++++-------------- .../diagnostic-logs-provider-delegate-impl.h | 7 ++---- .../esp32/main/main.cpp | 16 ++++++------- 3 files changed, 15 insertions(+), 32 deletions(-) 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 index 2a7c40fe4526a7..8b6795549ff6ee 100644 --- 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 @@ -29,10 +29,6 @@ using namespace chip::app::Clusters::DiagnosticLogs; LogProvider LogProvider::sInstance; LogProvider::CrashLogContext LogProvider::sCrashLogContext; -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -static uint32_t sIntentSize = CONFIG_END_USER_BUFFER_SIZE; -#endif - namespace { bool IsValidIntent(IntentEnum intent) { @@ -83,7 +79,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) return CircularDiagnosticBuffer::GetInstance().GetDataSize(); #else return static_cast(endUserSupportLogEnd - endUserSupportLogStart); -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE } break; case IntentEnum::kNetworkDiag: @@ -121,25 +117,17 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); MutableByteSpan endUserSupportSpan(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); - if (diagnosticStorage.IsEmptyBuffer()) - { - ChipLogError(DeviceLayer, "Empty Diagnostic buffer"); - return CHIP_ERROR_NOT_FOUND; - } + VerifyOrReturnError(!diagnosticStorage.IsEmptyBuffer(), CHIP_ERROR_NOT_FOUND, + ChipLogError(DeviceLayer, "Empty Diagnostic buffer")); // Retrieve data from the diagnostic storage CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan); - if (err != CHIP_NO_ERROR) - { - ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); - return err; - } - sIntentSize = endUserSupportSpan.size(); - // Now, assign the span to the EndUserSupport object or whatever is required + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err))); context->EndUserSupport.span = endUserSupportSpan; #else context->EndUserSupport.span = ByteSpan(&endUserSupportLogStart[0], static_cast(endUserSupportLogEnd - endUserSupportLogStart)); -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE } break; diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index a759aa946b2d3e..b748f4a3a61b5b 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -20,10 +20,6 @@ #include -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include -#endif - #include #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) @@ -31,9 +27,10 @@ #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; using namespace chip::Tracing::Diagnostics; -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace chip { namespace app { diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 63673094b35b87..35a385dc3ec74e 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -54,7 +54,7 @@ #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace { #if CONFIG_ENABLE_ESP32_FACTORY_DATA_PROVIDER @@ -79,6 +79,12 @@ static AppDeviceCallbacks EchoCallbacks; 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 + CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); + diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); + static ESP32Diagnostics diagnosticBackend(diagnosticStorage); + Tracing::Register(diagnosticBackend); +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE } extern "C" void app_main() @@ -86,14 +92,6 @@ extern "C" void app_main() #if CONFIG_ENABLE_PW_RPC chip::rpc::Init(); #endif - -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); - diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); - static ESP32Diagnostics diagnosticBackend(diagnosticStorage); - Tracing::Register(diagnosticBackend); -#endif - ESP_LOGI(TAG, "Temperature sensor!"); // Initialize the ESP NVS layer. From c69857379faed02e0cda51603b9877828a5317da Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 23 Dec 2024 12:49:53 +0530 Subject: [PATCH 09/25] diagnostic-buffer: clear buffer after successful bdx log transfer - Add related logic in temperature-measurement-app --- ...diagnostic-logs-provider-delegate-impl.cpp | 28 ++++++++--- .../DiagnosticStorageManager.h | 50 ++++++++++++------- .../esp32_diagnostic_trace/Diagnostics.h | 2 +- 3 files changed, 54 insertions(+), 26 deletions(-) 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 index 8b6795549ff6ee..56659933d61517 100644 --- 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 @@ -29,6 +29,10 @@ using namespace chip::app::Clusters::DiagnosticLogs; LogProvider LogProvider::sInstance; LogProvider::CrashLogContext LogProvider::sCrashLogContext; +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +static uint32_t read_entries = 0; +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + namespace { bool IsValidIntent(IntentEnum intent) { @@ -115,14 +119,17 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); - MutableByteSpan endUserSupportSpan(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); + MutableByteSpan endUserSupportSpan(endUserBuffer, 2048); VerifyOrReturnError(!diagnosticStorage.IsEmptyBuffer(), CHIP_ERROR_NOT_FOUND, ChipLogError(DeviceLayer, "Empty Diagnostic buffer")); // Retrieve data from the diagnostic storage - CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err))); + CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan, read_entries); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); + return err; + } context->EndUserSupport.span = endUserSupportSpan; #else context->EndUserSupport.span = @@ -296,11 +303,18 @@ CHIP_ERROR LogProvider::StartLogCollection(IntentEnum intent, LogSessionHandle & CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error) { - if (error != CHIP_NO_ERROR) +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + if (error == CHIP_NO_ERROR) { - // Handle the error - ChipLogProgress(DeviceLayer, "End log collection reason: %s", ErrorStr(error)); + CHIP_ERROR err = CircularDiagnosticBuffer::GetInstance().ClearReadMemory(read_entries); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DeviceLayer, "Failed to clear diagnostic read entries"); + } + ChipLogProgress(DeviceLayer, "Clear all read diagnostics successfully"); } +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + VerifyOrReturnValue(sessionHandle != kInvalidLogSessionHandle, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnValue(mSessionContextMap.count(sessionHandle), CHIP_ERROR_INVALID_ARGUMENT); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 723a25e9654c9e..2b2d7381302b67 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -20,8 +20,6 @@ #include -#define TLV_CLOSING_BYTE 1 - namespace chip { namespace Tracing { namespace Diagnostics { @@ -54,7 +52,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia return err; } - CHIP_ERROR Retrieve(MutableByteSpan & span) override + CHIP_ERROR Retrieve(MutableByteSpan & span, uint32_t & read_entries) override { CHIP_ERROR err = CHIP_NO_ERROR; chip::TLV::TLVReader reader; @@ -62,43 +60,45 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia chip::TLV::TLVWriter writer; writer.Init(span.data(), span.size()); + read_entries = 0; + + bool close_success = true; // To check if the last TLV is copied successfully. + uint32_t successful_written_bytes = 0; // Store temporary writer length in case last TLV is not copied successfully. chip::TLV::TLVType outWriterContainer = chip::TLV::kTLVType_NotSpecified; ReturnErrorOnFailure(writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_List, outWriterContainer)); 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()) { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < ((writer.GetRemainingFreeLength() + TLV_CLOSING_BYTE))) + err = writer.CopyElement(reader); + if (err == CHIP_NO_ERROR) { - err = writer.CopyElement(reader); - if (err == CHIP_ERROR_BUFFER_TOO_SMALL) - { - ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); - break; - } + successful_written_bytes = writer.GetLengthWritten(); + read_entries++; } else { - ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); + close_success = false; + ChipLogError(DeviceLayer, "Failed to copy TLV element: %s", ErrorStr(err)); break; } - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); } else { - ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container"); + ChipLogError(DeviceLayer, "Skipping unexpected TLV element"); } } ReturnErrorOnFailure(writer.EndContainer(outWriterContainer)); ReturnErrorOnFailure(writer.Finalize()); - span.reduce_size(writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", writer.GetLengthWritten()); + if (close_success) + { + successful_written_bytes = writer.GetLengthWritten(); + } + span.reduce_size(successful_written_bytes); + ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes); return CHIP_NO_ERROR; } @@ -106,6 +106,20 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia uint32_t GetDataSize() override { return DataLength(); } + CHIP_ERROR ClearReadMemory(uint32_t entries) + { + CHIP_ERROR err = CHIP_NO_ERROR; + while (entries--) + { + err = EvictHead(); + if (err != CHIP_NO_ERROR) + { + break; + } + } + return err; + } + private: CircularDiagnosticBuffer() : chip::TLV::TLVCircularBuffer(nullptr, 0) {} ~CircularDiagnosticBuffer() override = default; diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index d035467d8909ca..44361c9d2f5db0 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -107,7 +107,7 @@ class DiagnosticStorageInterface * diagnostic data will be stored. * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. */ - virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0; + virtual CHIP_ERROR Retrieve(MutableByteSpan & payload, uint32_t & read_entries) = 0; /** * @brief Checks if the storage buffer is empty. From 692abf0c47d9bc1659ebf613daa126e6f34f517e Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 26 Dec 2024 11:50:01 +0530 Subject: [PATCH 10/25] diagnostic-storage: Remove singltone design for storage instance - Add private CircularTLVReader and CircularTLVWriter - Remove redundant outer container for tlv's from Retrieve method - Maintain TAG's for diagnostic entry elements - Make diagnostic entry as a constant param to store method - Move Murmurhash to utils namespace --- .../DiagnosticStorageManager.h | 35 ++++++------------- .../DiagnosticTracing.cpp | 9 +++-- .../esp32_diagnostic_trace/Diagnostics.h | 22 ++++++++---- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 2b2d7381302b67..649337f0e6e2e3 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -26,25 +26,17 @@ namespace Diagnostics { class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public DiagnosticStorageInterface { public: - // Singleton instance getter - static CircularDiagnosticBuffer & GetInstance() - { - static CircularDiagnosticBuffer instance; - return instance; - } + 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; - void Init(uint8_t * buffer, size_t bufferLength) { chip::TLV::TLVCircularBuffer::Init(buffer, bufferLength); } - - CHIP_ERROR Store(DiagnosticEntry & entry) override + CHIP_ERROR Store(const DiagnosticEntry & entry) override { - chip::TLV::CircularTLVWriter writer; - writer.Init(*this); + mWriter.Init(*this); - CHIP_ERROR err = entry.Encode(writer); + CHIP_ERROR err = entry.Encode(mWriter); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "Failed to write entry: %s", chip::ErrorStr(err)); @@ -54,9 +46,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia CHIP_ERROR Retrieve(MutableByteSpan & span, uint32_t & read_entries) override { - CHIP_ERROR err = CHIP_NO_ERROR; - chip::TLV::TLVReader reader; - reader.Init(*this); + mReader.Init(*this); chip::TLV::TLVWriter writer; writer.Init(span.data(), span.size()); @@ -65,14 +55,11 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia bool close_success = true; // To check if the last TLV is copied successfully. uint32_t successful_written_bytes = 0; // Store temporary writer length in case last TLV is not copied successfully. - chip::TLV::TLVType outWriterContainer = chip::TLV::kTLVType_NotSpecified; - ReturnErrorOnFailure(writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_List, outWriterContainer)); - - while (CHIP_NO_ERROR == reader.Next()) + while (CHIP_NO_ERROR == mReader.Next()) { - if (reader.GetType() == chip::TLV::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag()) + if (mReader.GetType() == chip::TLV::kTLVType_Structure && mReader.GetTag() == chip::TLV::AnonymousTag()) { - err = writer.CopyElement(reader); + CHIP_ERROR err = writer.CopyElement(mReader); if (err == CHIP_NO_ERROR) { successful_written_bytes = writer.GetLengthWritten(); @@ -90,8 +77,6 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia ChipLogError(DeviceLayer, "Skipping unexpected TLV element"); } } - - ReturnErrorOnFailure(writer.EndContainer(outWriterContainer)); ReturnErrorOnFailure(writer.Finalize()); if (close_success) { @@ -121,8 +106,8 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia } private: - CircularDiagnosticBuffer() : chip::TLV::TLVCircularBuffer(nullptr, 0) {} - ~CircularDiagnosticBuffer() override = default; + chip::TLV::CircularTLVReader mReader; + chip::TLV::CircularTLVWriter mWriter; }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 05c22bb53bfdcb..bf1221158f7240 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -27,9 +27,7 @@ namespace chip { namespace Tracing { namespace Diagnostics { -constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE; -using HashValue = uint32_t; - +namespace Utils { // Implements a murmurhash with 0 seed. uint32_t MurmurHash(const void * key) { @@ -59,6 +57,11 @@ uint32_t MurmurHash(const void * key) return hash; } +} // namespace Utils + +constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE; +using HashValue = uint32_t; +using namespace Utils; HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("CASESession"), diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 44361c9d2f5db0..4f4d3aa5864fd8 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -25,6 +25,14 @@ namespace Tracing { namespace Diagnostics { +// Diagnostic TAGs +enum D_TAG +{ + TIMESTAMP = 0, + LABEL, + VALUE +}; + /** * @class DiagnosticEntry * @brief Abstract base class for encoding diagnostic entries into TLV format. @@ -46,7 +54,7 @@ class DiagnosticEntry * @return CHIP_ERROR Returns an error code indicating the success or * failure of the encoding operation. */ - virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) = 0; + virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) const = 0; }; template @@ -55,20 +63,20 @@ class Diagnostic : public DiagnosticEntry public: Diagnostic(const char * label, T value, uint32_t timestamp) : label_(label), value_(value), timestamp_(timestamp) {} - CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) const override { 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(0), timestamp_)); - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(1), label_)); + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(D_TAG::TIMESTAMP), timestamp_)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(D_TAG::LABEL), label_)); if constexpr (std::is_same_v) { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(2), value_)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(D_TAG::VALUE), value_)); } else { - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(2), value_)); + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(D_TAG::VALUE), value_)); } ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); ReturnErrorOnFailure(writer.Finalize()); @@ -99,7 +107,7 @@ class DiagnosticStorageInterface * diagnostic data to store. * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. */ - virtual CHIP_ERROR Store(DiagnosticEntry & diagnostic) = 0; + virtual CHIP_ERROR Store(const DiagnosticEntry & diagnostic) = 0; /** * @brief Retrieves diagnostic data as a payload. From e0ce37a0dd818ce8dde983e611ecdb2bc62bdad6 Mon Sep 17 00:00:00 2001 From: mahesh Date: Fri, 27 Dec 2024 11:14:56 +0530 Subject: [PATCH 11/25] temperature-measurement-app: use seperate buffer for retrieval of diagnostics - move diagnostic storage buffer to main --- .../diagnostic-logs-provider-delegate-impl.cpp | 17 +++++++++++------ .../diagnostic-logs-provider-delegate-impl.h | 3 +-- .../esp32/main/main.cpp | 4 +++- 3 files changed, 15 insertions(+), 9 deletions(-) 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 index 56659933d61517..7bc838a344d2b2 100644 --- 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 @@ -31,6 +31,10 @@ LogProvider::CrashLogContext LogProvider::sCrashLogContext; #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE static uint32_t read_entries = 0; +#define BUFFER_SIZE 2048 + +// Buffer is used to retrieve diagnostics from diagnostics storage +static uint8_t retrievalBuffer[BUFFER_SIZE]; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace { @@ -80,7 +84,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - return CircularDiagnosticBuffer::GetInstance().GetDataSize(); + return diagnosticStorage.GetDataSize(); #else return static_cast(endUserSupportLogEnd - endUserSupportLogStart); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -118,9 +122,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); - MutableByteSpan endUserSupportSpan(endUserBuffer, 2048); - + MutableByteSpan endUserSupportSpan(retrievalBuffer, BUFFER_SIZE); VerifyOrReturnError(!diagnosticStorage.IsEmptyBuffer(), CHIP_ERROR_NOT_FOUND, ChipLogError(DeviceLayer, "Empty Diagnostic buffer")); // Retrieve data from the diagnostic storage @@ -306,12 +308,15 @@ CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ER #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE if (error == CHIP_NO_ERROR) { - CHIP_ERROR err = CircularDiagnosticBuffer::GetInstance().ClearReadMemory(read_entries); + CHIP_ERROR err = diagnosticStorage.ClearReadMemory(read_entries); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "Failed to clear diagnostic read entries"); } - ChipLogProgress(DeviceLayer, "Clear all read diagnostics successfully"); + else + { + ChipLogProgress(DeviceLayer, "Clear all read diagnostics successfully"); + } } #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index b748f4a3a61b5b..4c95a0ca5692c1 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -28,8 +28,7 @@ #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include -static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; -using namespace chip::Tracing::Diagnostics; +extern chip::Tracing::Diagnostics::CircularDiagnosticBuffer diagnosticStorage; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace chip { diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 35a385dc3ec74e..238333ae4bc3db 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -54,6 +54,9 @@ #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include +static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; // Global static buffer used to store diagnostics +using namespace chip::Tracing::Diagnostics; +CircularDiagnosticBuffer diagnosticStorage(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace { @@ -80,7 +83,6 @@ 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 - CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); static ESP32Diagnostics diagnosticBackend(diagnosticStorage); Tracing::Register(diagnosticBackend); From b238f918a3d414d3c2b9aff31688e3f24688d9f3 Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 6 Jan 2025 10:54:25 +0530 Subject: [PATCH 12/25] temperature-measurement-app: pass diagnostic storage Instance as pointer to LogProvider - add kconfig option for retrieval buffer size --- .../esp32/main/Kconfig.projbuild | 7 ++++++ ...diagnostic-logs-provider-delegate-impl.cpp | 25 +++++++++++-------- .../diagnostic-logs-provider-delegate-impl.h | 15 ++++++++++- .../esp32/main/main.cpp | 4 +++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild index ac3965d63fdd5a..c17b5b4ae2e780 100644 --- a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild +++ b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild @@ -93,4 +93,11 @@ menu "Platform Diagnostics" 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. + + config RETRIEVAL_BUFFER_SIZE + int "Set buffer size for retrieval of diagnostics from diagnostic storage" + depends on ENABLE_ESP_DIAGNOSTICS_TRACE + default 4096 + help + Defines the buffer size (in bytes) for retrieval of diagnostic data. endmenu 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 index 7bc838a344d2b2..b0db91a36399a0 100644 --- 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 @@ -30,11 +30,8 @@ LogProvider LogProvider::sInstance; LogProvider::CrashLogContext LogProvider::sCrashLogContext; #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -static uint32_t read_entries = 0; -#define BUFFER_SIZE 2048 - -// Buffer is used to retrieve diagnostics from diagnostics storage -static uint8_t retrievalBuffer[BUFFER_SIZE]; +static uint32_t sReadEntries = 0; +static uint8_t retrievalBuffer[CONFIG_RETRIEVAL_BUFFER_SIZE]; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace { @@ -84,7 +81,9 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - return diagnosticStorage.GetDataSize(); + VerifyOrReturnError(mStorageInstance != nullptr, 0, + ChipLogError(DeviceLayer, "Diagnostic Storage instance cannot be null.")); + return mStorageInstance->GetDataSize(); #else return static_cast(endUserSupportLogEnd - endUserSupportLogStart); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -122,11 +121,13 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - MutableByteSpan endUserSupportSpan(retrievalBuffer, BUFFER_SIZE); - VerifyOrReturnError(!diagnosticStorage.IsEmptyBuffer(), CHIP_ERROR_NOT_FOUND, + 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, ChipLogError(DeviceLayer, "Empty Diagnostic buffer")); // Retrieve data from the diagnostic storage - CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan, read_entries); + CHIP_ERROR err = mStorageInstance->Retrieve(endUserSupportSpan, sReadEntries); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); @@ -306,16 +307,18 @@ CHIP_ERROR LogProvider::StartLogCollection(IntentEnum intent, LogSessionHandle & CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error) { #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INTERNAL, + ChipLogError(DeviceLayer, "Diagnostic Storage instance cannot be null.")); if (error == CHIP_NO_ERROR) { - CHIP_ERROR err = diagnosticStorage.ClearReadMemory(read_entries); + CHIP_ERROR err = mStorageInstance->ClearReadMemory(sReadEntries); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "Failed to clear diagnostic read entries"); } else { - ChipLogProgress(DeviceLayer, "Clear all read diagnostics successfully"); + ChipLogProgress(DeviceLayer, "Cleared all read diagnostics successfully"); } } #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index 4c95a0ca5692c1..d112a6a007a512 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -28,7 +28,7 @@ #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include -extern chip::Tracing::Diagnostics::CircularDiagnosticBuffer diagnosticStorage; +using namespace chip::Tracing::Diagnostics; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace chip { @@ -45,7 +45,16 @@ namespace DiagnosticLogs { class LogProvider : public DiagnosticLogsProviderDelegate { public: +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + static inline LogProvider & GetInstance(CircularDiagnosticBuffer * bufferInstance) + { + VerifyOrReturnValue(bufferInstance != nullptr, sInstance); + sInstance.mStorageInstance = bufferInstance; + return sInstance; + } +#else static inline LogProvider & GetInstance() { return sInstance; } +#endif /////////// DiagnosticLogsProviderDelegate Interface ///////// CHIP_ERROR StartLogCollection(IntentEnum intent, LogSessionHandle & outHandle, Optional & outTimeStamp, @@ -64,6 +73,10 @@ class LogProvider : public DiagnosticLogsProviderDelegate LogProvider(const LogProvider &) = delete; LogProvider & operator=(const LogProvider &) = delete; +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + CircularDiagnosticBuffer * mStorageInstance = nullptr; +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + struct CrashLogContext { #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 238333ae4bc3db..7429312e441d05 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -144,6 +144,10 @@ extern "C" void app_main() using namespace chip::app::Clusters::DiagnosticLogs; void emberAfDiagnosticLogsClusterInitCallback(chip::EndpointId endpoint) { +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + auto & logProvider = LogProvider::GetInstance(&diagnosticStorage); +#else auto & logProvider = LogProvider::GetInstance(); +#endif DiagnosticLogsServer::Instance().SetDiagnosticLogsProviderDelegate(endpoint, &logProvider); } From b875ddf2ac13e416cc728f48347fedbb510b35f5 Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 6 Jan 2025 11:50:31 +0530 Subject: [PATCH 13/25] backend: update methods to propagate error to level up - 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 --- ...diagnostic-logs-provider-delegate-impl.cpp | 2 +- .../esp32/main/main.cpp | 2 +- .../esp32_diagnostic_trace/Counter.cpp | 7 ++-- src/tracing/esp32_diagnostic_trace/Counter.h | 2 +- .../DiagnosticStorageManager.h | 14 ++------ .../DiagnosticTracing.cpp | 32 +++++++++++-------- .../DiagnosticTracing.h | 6 ++-- .../esp32_diagnostic_trace/Diagnostics.h | 12 +++---- 8 files changed, 38 insertions(+), 39 deletions(-) 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 index b0db91a36399a0..2aa6e10e501502 100644 --- 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 index 7429312e441d05..600003838af5f8 100644 --- 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 index 11e2bb2b1430ce..e3296e714a36e1 100644 --- 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 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 index 38a83b41234cfb..c2fd0b0155fa73 100644 --- 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 index 649337f0e6e2e3..18b84940af3e2c 100644 --- 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 index bf1221158f7240..89194728799c86 100644 --- 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 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 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 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 index 54cabef7fd55bb..0f793ac01482ac 100644 --- 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 index 4f4d3aa5864fd8..1f8474f4d69a80 100644 --- 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) { - 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. From 542cee85654fd9c6f09be006a3aa7c29868c1a00 Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 23 Jan 2025 12:42:45 +0530 Subject: [PATCH 14/25] temperature-measurement-app: review changes --- .../diagnostic-logs-provider-delegate-impl.cpp | 10 +++++----- .../diagnostic-logs-provider-delegate-impl.h | 17 +++++++---------- .../esp32/main/main.cpp | 10 ++++------ 3 files changed, 16 insertions(+), 21 deletions(-) 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 index 2aa6e10e501502..4767aee50d6fd1 100644 --- 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 @@ -29,7 +29,7 @@ using namespace chip::app::Clusters::DiagnosticLogs; LogProvider LogProvider::sInstance; LogProvider::CrashLogContext LogProvider::sCrashLogContext; -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE static uint32_t sReadEntries = 0; static uint8_t retrievalBuffer[CONFIG_RETRIEVAL_BUFFER_SIZE]; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -80,7 +80,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) switch (intent) { case IntentEnum::kEndUserSupport: { -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE VerifyOrReturnError(mStorageInstance != nullptr, 0, ChipLogError(DeviceLayer, "Diagnostic Storage instance cannot be null.")); return mStorageInstance->GetDataSize(); @@ -120,7 +120,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE switch (intent) { case IntentEnum::kEndUserSupport: { -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "Diagnostic Storage instance cannot be null.")); MutableByteSpan endUserSupportSpan(retrievalBuffer, CONFIG_RETRIEVAL_BUFFER_SIZE); @@ -130,7 +130,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE CHIP_ERROR err = mStorageInstance->Retrieve(endUserSupportSpan, sReadEntries); if (err != CHIP_NO_ERROR) { - ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); + ChipLogError(DeviceLayer, "Failed to retrieve data: %" CHIP_ERROR_FORMAT, err.Format()); return err; } context->EndUserSupport.span = endUserSupportSpan; @@ -318,7 +318,7 @@ CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ER } else { - ChipLogProgress(DeviceLayer, "Cleared all read diagnostics successfully"); + ChipLogDetail(DeviceLayer, "Cleared all read diagnostics successfully"); } } #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index d112a6a007a512..17240f1f2c2e38 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -26,7 +26,7 @@ #include #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include using namespace chip::Tracing::Diagnostics; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -45,16 +45,7 @@ namespace DiagnosticLogs { class LogProvider : public DiagnosticLogsProviderDelegate { public: -#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - static inline LogProvider & GetInstance(CircularDiagnosticBuffer * bufferInstance) - { - VerifyOrReturnValue(bufferInstance != nullptr, sInstance); - sInstance.mStorageInstance = bufferInstance; - return sInstance; - } -#else static inline LogProvider & GetInstance() { return sInstance; } -#endif /////////// DiagnosticLogsProviderDelegate Interface ///////// CHIP_ERROR StartLogCollection(IntentEnum intent, LogSessionHandle & outHandle, Optional & outTimeStamp, @@ -64,6 +55,11 @@ class LogProvider : public DiagnosticLogsProviderDelegate size_t GetSizeForIntent(IntentEnum intent) override; CHIP_ERROR GetLogForIntent(IntentEnum intent, MutableByteSpan & outBuffer, Optional & outTimeStamp, Optional & outTimeSinceBoot) override; +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + void SetDiagnosticStorageInstance(CircularDiagnosticBuffer * bufferInstance) { + mStorageInstance = bufferInstance; + } +#endif private: static LogProvider sInstance; @@ -74,6 +70,7 @@ class LogProvider : public DiagnosticLogsProviderDelegate LogProvider & operator=(const LogProvider &) = delete; #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + // If mStorageInstance is nullptr then operations related to diagnostic storage will be skipped. CircularDiagnosticBuffer * mStorageInstance = nullptr; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 600003838af5f8..8451ed6fb8f303 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -52,7 +52,7 @@ #include #endif // CONFIG_ENABLE_ESP32_DEVICE_INFO_PROVIDER -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; // Global static buffer used to store diagnostics using namespace chip::Tracing::Diagnostics; @@ -82,8 +82,7 @@ static AppDeviceCallbacks EchoCallbacks; 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); +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE static ESP32Diagnostics diagnosticBackend(&diagnosticStorage); Tracing::Register(diagnosticBackend); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -144,10 +143,9 @@ extern "C" void app_main() using namespace chip::app::Clusters::DiagnosticLogs; void emberAfDiagnosticLogsClusterInitCallback(chip::EndpointId endpoint) { -#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - auto & logProvider = LogProvider::GetInstance(&diagnosticStorage); -#else auto & logProvider = LogProvider::GetInstance(); +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + logProvider.SetDiagnosticStorageInstance(&diagnosticStorage); #endif DiagnosticLogsServer::Instance().SetDiagnosticLogsProviderDelegate(endpoint, &logProvider); } From b4f2001e1b8b81a22b3e268a18f83fb93f410b5b Mon Sep 17 00:00:00 2001 From: mahesh Date: Fri, 24 Jan 2025 11:53:56 +0530 Subject: [PATCH 15/25] esp32_diagnostic_trace: review changes --- .../esp32_diagnostic_trace/Counter.cpp | 8 ++- src/tracing/esp32_diagnostic_trace/Counter.h | 2 +- .../DiagnosticTracing.cpp | 51 ++++++++----------- 3 files changed, 26 insertions(+), 35 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index e3296e714a36e1..1f0853673752b8 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -42,13 +42,11 @@ uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const return mCounterList[label]; } -CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface * mStorageInstance) +CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface * storageInstance) { - CHIP_ERROR err = CHIP_NO_ERROR; - VerifyOrReturnError(mStorageInstance != nullptr, err, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); + VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); Diagnostic counter(label, GetInstanceCount(label), esp_log_timestamp()); - err = mStorageInstance->Store(counter); - return err; + return storageInstance->Store(counter); } } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index c2fd0b0155fa73..ce2697c14147a8 100644 --- 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; - CHIP_ERROR ReportMetrics(const char * label, DiagnosticStorageInterface * mStorageInstance); + CHIP_ERROR ReportMetrics(const char * label, DiagnosticStorageInterface * storageInstance); private: ESPDiagnosticCounter() {} diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 89194728799c86..32ac7f28acb2fd 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -63,25 +63,30 @@ constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE; using HashValue = uint32_t; using namespace Utils; +// Only traces with scope in gPermitList are allowed. +// Used for MATTER_TRACE_SCOPE() HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("CASESession"), MurmurHash("NetworkCommissioning"), MurmurHash("GeneralCommissioning"), MurmurHash("OperationalCredentials"), MurmurHash("CASEServer"), - MurmurHash("Fabric"), - MurmurHash("Resolver") }; // namespace + MurmurHash("Fabric") }; // namespace -bool IsPermitted(const char * str) +// All traces with value from gSkipList are skipped. +// Used for MATTER_TRACE_INSTANT() +HashValue gSkipList[kPermitListMaxSize] = { + MurmurHash("Resolver"), +}; + +bool IsPresent(const char * str, HashValue * list) { - HashValue hashValue = MurmurHash(str); - for (HashValue permitted : gPermitList) - { - if (permitted == 0) + for (int i = 0; i < kPermitListMaxSize; i++) { + if (list[i] == 0) { break; } - if (hashValue == permitted) + if (MurmurHash(str) == list[i]) { return true; } @@ -101,21 +106,20 @@ 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: { ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32()); Diagnostic metric(event.key(), event.ValueInt32(), esp_log_timestamp()); - err = mStorageInstance->Store(metric); + ReturnOnFailure(mStorageInstance->Store(metric)); } break; case ValueType::kUInt32: { ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32()); Diagnostic metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); - err = mStorageInstance->Store(metric); + ReturnOnFailure(mStorageInstance->Store(metric)); } break; @@ -131,43 +135,32 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) ChipLogProgress(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key()); break; } - - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Metric Diagnostic data")); } void ESP32Diagnostics::TraceCounter(const char * label) { - CHIP_ERROR err = ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter Diagnostic data")); + ReturnOnFailure(ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance)); } void ESP32Diagnostics::TraceBegin(const char * label, const char * group) { - if (IsPermitted(group)) - { - CHIP_ERROR err = StoreDiagnostics(label, group); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); - } + VerifyOrReturn(IsPresent(group, gPermitList)); + ReturnOnFailure(StoreDiagnostics(label, group)); } void ESP32Diagnostics::TraceEnd(const char * label, const char * group) {} void ESP32Diagnostics::TraceInstant(const char * label, const char * value) { - if (!IsPermitted(value)) - { - CHIP_ERROR err = StoreDiagnostics(label, value); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); - } + VerifyOrReturn(IsPresent(value, gSkipList)); + ReturnOnFailure(StoreDiagnostics(label, value)); } 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")); + VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); Diagnostic trace(label, group, esp_log_timestamp()); - err = mStorageInstance->Store(trace); - return err; + return mStorageInstance->Store(trace); } } // namespace Diagnostics From 4cc21a32c7d5b3ccc1adc7110197aa90f683d04c Mon Sep 17 00:00:00 2001 From: mahesh Date: Fri, 24 Jan 2025 14:19:30 +0530 Subject: [PATCH 16/25] esp32_diagnostic_trace: Add clearBuffer() method, update api documentation --- ...diagnostic-logs-provider-delegate-impl.cpp | 2 +- .../diagnostic-logs-provider-delegate-impl.h | 4 +- .../esp32_diagnostic_trace/Counter.cpp | 3 +- .../DiagnosticStorageManager.h | 28 ++++++----- .../DiagnosticTracing.cpp | 6 ++- .../esp32_diagnostic_trace/Diagnostics.h | 49 ++++++++++++++----- 6 files changed, 61 insertions(+), 31 deletions(-) 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 index 4767aee50d6fd1..8502751cc2424d 100644 --- 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 @@ -311,7 +311,7 @@ CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ER ChipLogError(DeviceLayer, "Diagnostic Storage instance cannot be null.")); if (error == CHIP_NO_ERROR) { - CHIP_ERROR err = mStorageInstance->ClearReadMemory(sReadEntries); + CHIP_ERROR err = mStorageInstance->ClearBuffer(sReadEntries); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "Failed to clear diagnostic read entries"); diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index 17240f1f2c2e38..5062b617262377 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -56,9 +56,7 @@ class LogProvider : public DiagnosticLogsProviderDelegate CHIP_ERROR GetLogForIntent(IntentEnum intent, MutableByteSpan & outBuffer, Optional & outTimeStamp, Optional & outTimeSinceBoot) override; #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - void SetDiagnosticStorageInstance(CircularDiagnosticBuffer * bufferInstance) { - mStorageInstance = bufferInstance; - } + void SetDiagnosticStorageInstance(CircularDiagnosticBuffer * bufferInstance) { mStorageInstance = bufferInstance; } #endif private: diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 1f0853673752b8..523e31f960fcc3 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -44,7 +44,8 @@ uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface * storageInstance) { - VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); + VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, + ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); Diagnostic counter(label, GetInstanceCount(label), esp_log_timestamp()); return storageInstance->Store(counter); } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 18b84940af3e2c..aa60ea6f9dbb52 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -38,6 +38,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia CHIP_ERROR Retrieve(MutableByteSpan & span, uint32_t & read_entries) override { + CHIP_ERROR err = CHIP_NO_ERROR; mReader.Init(*this); chip::TLV::TLVWriter writer; @@ -47,11 +48,11 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia bool close_success = true; // To check if the last TLV is copied successfully. uint32_t successful_written_bytes = 0; // Store temporary writer length in case last TLV is not copied successfully. - while (CHIP_NO_ERROR == mReader.Next()) + while ((err = mReader.Next()) == CHIP_NO_ERROR) { if (mReader.GetType() == chip::TLV::kTLVType_Structure && mReader.GetTag() == chip::TLV::AnonymousTag()) { - CHIP_ERROR err = writer.CopyElement(mReader); + err = writer.CopyElement(mReader); if (err == CHIP_NO_ERROR) { successful_written_bytes = writer.GetLengthWritten(); @@ -60,13 +61,12 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia else { close_success = false; - ChipLogError(DeviceLayer, "Failed to copy TLV element: %s", ErrorStr(err)); break; } } else { - ChipLogError(DeviceLayer, "Skipping unexpected TLV element"); + ChipLogDetail(DeviceLayer, "Skipping unexpected TLV element"); } } ReturnErrorOnFailure(writer.Finalize()); @@ -83,18 +83,22 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia uint32_t GetDataSize() override { return DataLength(); } - CHIP_ERROR ClearReadMemory(uint32_t entries) + CHIP_ERROR ClearBuffer() override + { + while (!IsBufferEmpty()) + { + ReturnErrorOnFailure(EvictHead()); + } + return CHIP_NO_ERROR; + } + + CHIP_ERROR ClearBuffer(uint32_t entries) { - CHIP_ERROR err = CHIP_NO_ERROR; while (entries--) { - err = EvictHead(); - if (err != CHIP_NO_ERROR) - { - break; - } + ReturnErrorOnFailure(EvictHead()); } - return err; + return CHIP_NO_ERROR; } private: diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 32ac7f28acb2fd..4993c2c3b9bab9 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -81,7 +81,8 @@ HashValue gSkipList[kPermitListMaxSize] = { bool IsPresent(const char * str, HashValue * list) { - for (int i = 0; i < kPermitListMaxSize; i++) { + for (int i = 0; i < kPermitListMaxSize; i++) + { if (list[i] == 0) { break; @@ -158,7 +159,8 @@ void ESP32Diagnostics::TraceInstant(const char * label, const char * value) CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) { - VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); + VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, + ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); Diagnostic trace(label, group, esp_log_timestamp()); return mStorageInstance->Store(trace); } diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 1f8474f4d69a80..ee70ef3b6a72ed 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -102,32 +102,57 @@ class DiagnosticStorageInterface virtual ~DiagnosticStorageInterface() = default; /** - * @brief Stores a diagnostic entry. - * @param diagnostic Reference to a DiagnosticEntry object containing the - * diagnostic data to store. - * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. + * @brief Stores a diagnostic entry in the diagnostic storage buffer. + * @param diagnostic A reference to a `DiagnosticEntry` object that contains + * the diagnostic data to be stored. + * @return CHIP_ERROR Returns `CHIP_NO_ERROR` if the data is successfully stored, + * or an appropriate error code in case of failure. */ virtual CHIP_ERROR Store(const DiagnosticEntry & diagnostic) = 0; /** - * @brief Retrieves diagnostic data as a payload. - * @param payload Reference to a MutableByteSpan where the retrieved - * diagnostic data will be stored. - * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. + * @brief Copies diagnostic data from the storage buffer to a payload. + * + * This method retrieves the stored diagnostic data and copies it into the + * provided `payload` buffer. If the buffer is too small to hold all the data, + * the method returns the successfully copied entries along with an error code + * indicating that the buffer was insufficient. + * + * @param payload A reference to a `MutableByteSpan` where the retrieved + * diagnostic data will be copied. + * @param read_entries A reference to an integer that will hold the total + * number of successfully read diagnostic entries. + * + * @retval CHIP_NO_ERROR If the operation succeeded and all data was copied. + * @retval CHIP_ERROR_BUFFER_TOO_SMALL If the buffer was not large enough to hold all data. + * @retval CHIP_ERROR If any other failure occurred during the operation. */ virtual CHIP_ERROR Retrieve(MutableByteSpan & payload, uint32_t & read_entries) = 0; /** - * @brief Checks if the storage buffer is empty. - * @return bool true if the buffer contains no stored diagnostic data, otherwise false. + * @brief Checks if the diagnostic storage buffer is empty. + * + * This method checks whether the buffer contains any stored diagnostic data. + * + * @return bool Returns `true` if the buffer contains no stored data, + * or `false` if the buffer has data. */ virtual bool IsBufferEmpty() = 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. + * @brief Retrieves the size of the data currently stored in the diagnostic buffer. + * + * This method returns the total size (in bytes) of all diagnostic data that is + * currently stored in the buffer. + * + * @return uint32_t The size (in bytes) of the stored diagnostic data. */ virtual uint32_t GetDataSize() = 0; + + /** + * @brief Clears entire buffer + */ + virtual CHIP_ERROR ClearBuffer() = 0; }; } // namespace Diagnostics From e5ff131a61ba4b2ff559d919319f5fd08d74c232 Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 11 Feb 2025 11:46:45 +0530 Subject: [PATCH 17/25] fixed trace instant filter --- src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 4993c2c3b9bab9..c9a969c1517681 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -153,7 +153,7 @@ void ESP32Diagnostics::TraceEnd(const char * label, const char * group) {} void ESP32Diagnostics::TraceInstant(const char * label, const char * value) { - VerifyOrReturn(IsPresent(value, gSkipList)); + VerifyOrReturn(!IsPresent(value, gSkipList)); ReturnOnFailure(StoreDiagnostics(label, value)); } From 427ae7b1400e269f11029018c54fe17f299da5c2 Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 11 Feb 2025 11:36:12 +0530 Subject: [PATCH 18/25] esp_diagnostic_trace: add clearbuffer method to diagnostic interface --- .../esp32_diagnostic_trace/DiagnosticStorageManager.h | 2 +- src/tracing/esp32_diagnostic_trace/Diagnostics.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index aa60ea6f9dbb52..745f5a9bd8cd3f 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -92,7 +92,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia return CHIP_NO_ERROR; } - CHIP_ERROR ClearBuffer(uint32_t entries) + CHIP_ERROR ClearBuffer(uint32_t entries) override { while (entries--) { diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index ee70ef3b6a72ed..a111072ee0732e 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -153,6 +153,11 @@ class DiagnosticStorageInterface * @brief Clears entire buffer */ virtual CHIP_ERROR ClearBuffer() = 0; + + /** + * @brief Clears buffer up to the specified number of entries + */ + virtual CHIP_ERROR ClearBuffer(uint32_t entries) = 0; }; } // namespace Diagnostics From 09165625c3ec3aa5f4ff26b294520b1ac71bbf48 Mon Sep 17 00:00:00 2001 From: mahesh Date: Wed, 5 Mar 2025 17:53:50 +0530 Subject: [PATCH 19/25] diagnostics: add decode method --- .../esp32_diagnostic_trace/Counter.cpp | 2 +- .../DiagnosticTracing.cpp | 6 +- .../esp32_diagnostic_trace/Diagnostics.h | 100 ++++++++++++++++-- 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 523e31f960fcc3..48ee85924b1f3d 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -46,7 +46,7 @@ CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticSto { VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); - Diagnostic counter(label, GetInstanceCount(label), esp_log_timestamp()); + Diagnostic counter(const_cast(label), GetInstanceCount(label), esp_log_timestamp()); return storageInstance->Store(counter); } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index c9a969c1517681..3aa4777ba962b2 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -112,14 +112,14 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { case ValueType::kInt32: { ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32()); - Diagnostic metric(event.key(), event.ValueInt32(), esp_log_timestamp()); + Diagnostic metric(const_cast(event.key()), event.ValueInt32(), esp_log_timestamp()); ReturnOnFailure(mStorageInstance->Store(metric)); } break; case ValueType::kUInt32: { ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32()); - Diagnostic metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); + Diagnostic metric(const_cast(event.key()), event.ValueUInt32(), esp_log_timestamp()); ReturnOnFailure(mStorageInstance->Store(metric)); } break; @@ -161,7 +161,7 @@ CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * g { VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); - Diagnostic trace(label, group, esp_log_timestamp()); + Diagnostic trace(const_cast(label), const_cast(group), esp_log_timestamp()); return mStorageInstance->Store(trace); } diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index a111072ee0732e..c6df25acc0702e 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -19,6 +19,7 @@ #pragma once #include +#define kMaxStringValueSize 128 namespace chip { namespace Tracing { @@ -30,7 +31,7 @@ enum class DiagTag : uint8_t { TIMESTAMP = 0, LABEL, - VALUE + VALUE, }; /** @@ -55,39 +56,122 @@ class DiagnosticEntry * failure of the encoding operation. */ virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) const = 0; + + virtual CHIP_ERROR Decode(chip::TLV::TLVReader & reader) = 0; }; template class Diagnostic : public DiagnosticEntry { public: - Diagnostic(const char * label, T value, uint32_t timestamp) : label_(label), value_(value), timestamp_(timestamp) {} + Diagnostic(char * label = nullptr, T value = T{}, uint32_t timestamp = 0) : label_(label), value_(value), timestamp_(timestamp) + {} CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) const override { chip::TLV::TLVType DiagnosticOuterContainer = chip::TLV::kTLVType_NotSpecified; ReturnErrorOnFailure( writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, DiagnosticOuterContainer)); + + // Write timestamp ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::TIMESTAMP), timestamp_)); - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), label_)); - if constexpr (std::is_same_v) + + // Write label + if (strlen(label_) > kMaxStringValueSize) { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), value_)); + char labelBuffer[kMaxStringValueSize + 1]; + memcpy(labelBuffer, label_, kMaxStringValueSize); + labelBuffer[kMaxStringValueSize] = '\0'; + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), labelBuffer)); } else + { + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), label_)); + } + + // Write value + if constexpr (std::is_same_v) + { + if (strlen(value_) > kMaxStringValueSize) + { + char valueBuffer[kMaxStringValueSize + 1]; + memcpy(valueBuffer, value_, kMaxStringValueSize); + valueBuffer[kMaxStringValueSize] = '\0'; + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), valueBuffer)); + } + else + { + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), value_)); + } + } + else if constexpr (std::is_same_v) { ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), value_)); } + else if constexpr (std::is_same_v) + { + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), value_)); + } + else + { + return CHIP_ERROR_INVALID_ARGUMENT; + } ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); ReturnErrorOnFailure(writer.Finalize()); ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", label_); return CHIP_NO_ERROR; } + CHIP_ERROR Decode(chip::TLV::TLVReader & reader) override + { + TLV::TLVType containerType; + ReturnErrorOnFailure(reader.EnterContainer(containerType)); + // Read timestamp + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(DiagTag::TIMESTAMP))); + ReturnErrorOnFailure(reader.Get(timestamp_)); + + // Read label + ReturnErrorOnFailure(reader.Next(TLV::ContextTag(DiagTag::LABEL))); + uint32_t labelSize = reader.GetLength(); + char labelBuffer[kMaxStringValueSize + 1]; + ReturnErrorOnFailure(reader.GetString(labelBuffer, kMaxStringValueSize + 1)); + memcpy(label_, labelBuffer, labelSize + 1); + + // Read value + ReturnErrorOnFailure(reader.Next()); + if constexpr (std::is_same_v) + { + uint32_t valueSize = reader.GetLength(); + char valueBuffer[kMaxStringValueSize + 1]; + ReturnErrorOnFailure(reader.GetString(valueBuffer, kMaxStringValueSize + 1)); + memcpy(value_, valueBuffer, valueSize + 1); + } + else if constexpr (std::is_same_v) + { + ReturnErrorOnFailure(reader.Get(value_)); + } + else if constexpr (std::is_same_v) + { + ReturnErrorOnFailure(reader.Get(value_)); + } + else + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + + ReturnErrorOnFailure(reader.ExitContainer(containerType)); + return CHIP_NO_ERROR; + } + + // Getters + char * GetLabel() const { return label_; } + T GetValue() const { return value_; } + uint32_t GetTimestamp() const { return timestamp_; } + private: - const char * label_; - T value_; - uint32_t timestamp_; + char * label_ = nullptr; + T value_{}; + uint32_t timestamp_ = 0; }; /** From b4f724e0629f99bef4a9b9072afc8e3964c272c9 Mon Sep 17 00:00:00 2001 From: mahesh Date: Wed, 12 Mar 2025 10:47:31 +0530 Subject: [PATCH 20/25] readme update --- .../temperature-measurement-app/esp32/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/examples/temperature-measurement-app/esp32/README.md b/examples/temperature-measurement-app/esp32/README.md index a3276ae37299a6..0fa56a400b07b3 100644 --- a/examples/temperature-measurement-app/esp32/README.md +++ b/examples/temperature-measurement-app/esp32/README.md @@ -40,6 +40,21 @@ main/diagnostic_logs These files contain dummy data. +#### To use diagnostic tracing + +Open menuconfig + +``` + idf.py menuconfig +``` + +Enable `ENABLE_ESP_DIAGNOSTICS_TRACE` option from menuconfig + +Set diagnostic storage buffer size from `Platform Diagnostics` menu + +- End user buffer default size 4096 +- Retrieval buffer default size 4096 + #### To test the diagnostic logs cluster ``` From 52da1f92dfb9ecff4751bcae21441dcbcf6b82da Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 17 Mar 2025 15:00:18 +0530 Subject: [PATCH 21/25] Review changes --- .../diagnostic-logs-provider-delegate-impl.h | 2 +- src/tracing/esp32_diagnostic_trace/BUILD.gn | 6 +- .../esp32_diagnostic_trace/Counter.cpp | 17 +- src/tracing/esp32_diagnostic_trace/Counter.h | 5 +- .../DiagnosticEntry.cpp | 114 ++++++++ .../esp32_diagnostic_trace/DiagnosticEntry.h | 60 +++++ .../DiagnosticStorage.cpp | 106 ++++++++ .../DiagnosticStorage.h | 83 ++++++ .../DiagnosticStorageManager.h | 111 -------- .../DiagnosticTracing.cpp | 45 +++- .../DiagnosticTracing.h | 7 +- .../esp32_diagnostic_trace/Diagnostics.h | 249 ------------------ 12 files changed, 427 insertions(+), 378 deletions(-) create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp create mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h delete mode 100644 src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h delete mode 100644 src/tracing/esp32_diagnostic_trace/Diagnostics.h diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index 5062b617262377..a32b60060ab439 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -27,7 +27,7 @@ #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include +#include using namespace chip::Tracing::Diagnostics; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn index 5516968ea639af..56ae6dfd96a29f 100644 --- a/src/tracing/esp32_diagnostic_trace/BUILD.gn +++ b/src/tracing/esp32_diagnostic_trace/BUILD.gn @@ -27,10 +27,12 @@ static_library("backend") { sources = [ "Counter.cpp", "Counter.h", - "DiagnosticStorageManager.h", + "DiagnosticStorage.cpp", + "DiagnosticStorage.h", "DiagnosticTracing.cpp", "DiagnosticTracing.h", - "Diagnostics.h", + "DiagnosticEntry.h", + "DiagnosticEntry.cpp", ] public_deps = [ diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 48ee85924b1f3d..606bf69f4bd1f0 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -42,12 +42,23 @@ uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const return mCounterList[label]; } -CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface * storageInstance) +CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, CircularDiagnosticBuffer * storageInstance) { VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); - Diagnostic counter(const_cast(label), GetInstanceCount(label), esp_log_timestamp()); - return storageInstance->Store(counter); + + // Allocate buffers for the diagnostic entry + char labelBuffer[kMaxStringValueSize]; + strncpy(labelBuffer, label, kMaxStringValueSize); + labelBuffer[kMaxStringValueSize - 1] = '\0'; + + // Create diagnostic entry + DiagnosticEntry entry = { .label = labelBuffer, + .uintValue = GetInstanceCount(label), + .type = ValueType::kUnsignedInteger, + .timestamp = esp_log_timestamp() }; + + return storageInstance->Store(entry); } } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index ce2697c14147a8..c186ff66ea6363 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -18,7 +18,8 @@ #pragma once -#include "tracing/esp32_diagnostic_trace/Diagnostics.h" +#include "tracing/esp32_diagnostic_trace/DiagnosticStorage.h" +#include "tracing/esp32_diagnostic_trace/DiagnosticEntry.h" #include namespace chip { @@ -45,7 +46,7 @@ class ESPDiagnosticCounter uint32_t GetInstanceCount(const char * label) const; - CHIP_ERROR ReportMetrics(const char * label, DiagnosticStorageInterface * storageInstance); + CHIP_ERROR ReportMetrics(const char * label, CircularDiagnosticBuffer * storageInstance); private: ESPDiagnosticCounter() {} diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp new file mode 100644 index 00000000000000..98eda94816a06c --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp @@ -0,0 +1,114 @@ +#include +using namespace chip::TLV; + +namespace chip { +namespace Tracing { +namespace Diagnostics { + +CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) +{ + TLVType DiagnosticOuterContainer = kTLVType_NotSpecified; + ReturnErrorOnFailure(writer.StartContainer(AnonymousTag(), kTLVType_Structure, DiagnosticOuterContainer)); + + // Write timestamp + ReturnErrorOnFailure(writer.Put(ContextTag(DiagTag::TIMESTAMP), entry.timestamp)); + + // Write label + if (entry.label != nullptr) + { + if (strlen(entry.label) > kMaxStringValueSize) + { + char labelBuffer[kMaxStringValueSize + 1]; + memcpy(labelBuffer, entry.label, kMaxStringValueSize); + labelBuffer[kMaxStringValueSize] = '\0'; + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), labelBuffer)); + } + else + { + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), entry.label)); + } + } + + // Write value based on type + switch (entry.type) + { + case ValueType::kCharString: + if (entry.stringValue != nullptr) + { + if (strlen(entry.stringValue) > kMaxStringValueSize) + { + char valueBuffer[kMaxStringValueSize + 1]; + memcpy(valueBuffer, entry.stringValue, kMaxStringValueSize); + valueBuffer[kMaxStringValueSize] = '\0'; + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), valueBuffer)); + } + else + { + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), entry.stringValue)); + } + } + break; + + case ValueType::kUnsignedInteger: + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), entry.uintValue)); + break; + + case ValueType::kSignedInteger: + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), entry.intValue)); + break; + } + + ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); + ReturnErrorOnFailure(writer.Finalize()); + ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", entry.label); + return CHIP_NO_ERROR; +} + +CHIP_ERROR Decode(CircularTLVReader & reader, DiagnosticEntry & entry) +{ + TLVType containerType; + ReturnErrorOnFailure(reader.EnterContainer(containerType)); + + // Read timestamp + ReturnErrorOnFailure(reader.Next(ContextTag(DiagTag::TIMESTAMP))); + ReturnErrorOnFailure(reader.Get(entry.timestamp)); + + // Read label + ReturnErrorOnFailure(reader.Next(ContextTag(DiagTag::LABEL))); + uint32_t labelSize = reader.GetLength(); + if (labelSize > kMaxStringValueSize) + { + return CHIP_ERROR_BUFFER_TOO_SMALL; + } + ReturnErrorOnFailure(reader.GetString(entry.label, kMaxStringValueSize + 1)); + + // Read value + ReturnErrorOnFailure(reader.Next()); + switch (entry.type) + { + case ValueType::kCharString: { + uint32_t valueSize = reader.GetLength(); + if (valueSize > kMaxStringValueSize) + { + return CHIP_ERROR_BUFFER_TOO_SMALL; + } + ReturnErrorOnFailure(reader.GetString(entry.stringValue, kMaxStringValueSize + 1)); + break; + } + + case ValueType::kUnsignedInteger: + ReturnErrorOnFailure(reader.Get(entry.uintValue)); + break; + + case ValueType::kSignedInteger: + ReturnErrorOnFailure(reader.Get(entry.intValue)); + break; + } + + ReturnErrorOnFailure(reader.ExitContainer(containerType)); + return CHIP_NO_ERROR; +} + +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h new file mode 100644 index 00000000000000..6dab0cfea5cb63 --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h @@ -0,0 +1,60 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#include + +namespace chip { +namespace Tracing { +namespace Diagnostics { + +static constexpr size_t kMaxStringValueSize = 128; + +enum class ValueType +{ + kCharString, + kUnsignedInteger, + kSignedInteger +}; + +struct DiagnosticEntry +{ + char * label; + union + { + char * stringValue; + uint32_t uintValue; + int32_t intValue; + }; + ValueType type; + uint32_t timestamp; +}; + +enum class DiagTag : uint8_t +{ + TIMESTAMP = 1, + LABEL = 2, + VALUE = 3 +}; + +CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer, const DiagnosticEntry & entry); +CHIP_ERROR Decode(chip::TLV::CircularTLVReader & reader, DiagnosticEntry & entry); + +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp new file mode 100644 index 00000000000000..688132f7ea8a35 --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp @@ -0,0 +1,106 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +using namespace chip::TLV; + +namespace chip { +namespace Tracing { +namespace Diagnostics { + +CHIP_ERROR CircularDiagnosticBuffer::Store(const DiagnosticEntry & entry) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + mWriter.Init(*this); + ReturnLogErrorOnFailure(Encode(mWriter, entry)); + return err; +} + +CHIP_ERROR CircularDiagnosticBuffer::Retrieve(MutableByteSpan & span, uint32_t & read_entries) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + mReader.Init(*this); + + TLVWriter writer; + writer.Init(span.data(), span.size()); + read_entries = 0; + + bool close_success = true; // To check if the last TLV is copied successfully. + uint32_t successful_written_bytes = 0; // Store temporary writer length in case last TLV is not copied successfully. + + while ((err = mReader.Next()) == CHIP_NO_ERROR) + { + if (mReader.GetType() == kTLVType_Structure && mReader.GetTag() == AnonymousTag()) + { + err = writer.CopyElement(mReader); + if (err == CHIP_NO_ERROR) + { + successful_written_bytes = writer.GetLengthWritten(); + read_entries++; + } + else + { + close_success = false; + break; + } + } + else + { + ChipLogDetail(DeviceLayer, "Skipping unexpected TLV element"); + } + } + ReturnErrorOnFailure(writer.Finalize()); + if (close_success) + { + successful_written_bytes = writer.GetLengthWritten(); + } + span.reduce_size(successful_written_bytes); + ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes); + return CHIP_NO_ERROR; +} + +bool CircularDiagnosticBuffer::IsBufferEmpty() +{ + return DataLength() == 0; +} + +uint32_t CircularDiagnosticBuffer::GetDataSize() +{ + return DataLength(); +} + +CHIP_ERROR CircularDiagnosticBuffer::ClearBuffer() +{ + while (!IsBufferEmpty()) + { + ReturnErrorOnFailure(EvictHead()); + } + return CHIP_NO_ERROR; +} + +CHIP_ERROR CircularDiagnosticBuffer::ClearBuffer(uint32_t entries) +{ + while (entries--) + { + ReturnErrorOnFailure(EvictHead()); + } + return CHIP_NO_ERROR; +} +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h new file mode 100644 index 00000000000000..ead96a5ba8e42e --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h @@ -0,0 +1,83 @@ +#pragma once +#include +#include +#include + +namespace chip { +namespace Tracing { +namespace Diagnostics { + +/** + * @brief Diagnostic storage class + */ +class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer +{ +public: + CircularDiagnosticBuffer(uint8_t * buffer, size_t bufferLength) : chip::TLV::TLVCircularBuffer(buffer, bufferLength) {} + + /** + * @brief Stores a diagnostic entry in the diagnostic storage buffer. + * @param diagnostic A reference to a `DiagnosticEntry` object that contains + * the diagnostic data to be stored. + * @return CHIP_ERROR Returns `CHIP_NO_ERROR` if the data is successfully stored, + * or an appropriate error code in case of failure. + */ + CHIP_ERROR Store(const DiagnosticEntry & diagnostic); + + /** + * @brief Copies diagnostic data from the storage buffer to a payload. + * + * This method retrieves the stored diagnostic data and copies it into the + * provided `payload` buffer. If the buffer is too small to hold all the data, + * the method returns the successfully copied entries along with an error code + * indicating that the buffer was insufficient. + * + * @param payload A reference to a `MutableByteSpan` where the retrieved + * diagnostic data will be copied. + * @param read_entries A reference to an integer that will hold the total + * number of successfully read diagnostic entries. + * + * @retval CHIP_NO_ERROR If the operation succeeded and all data was copied. + * @retval CHIP_ERROR_BUFFER_TOO_SMALL If the buffer was not large enough to hold all data. + * @retval CHIP_ERROR If any other failure occurred during the operation. + */ + CHIP_ERROR Retrieve(MutableByteSpan & payload, uint32_t & read_entries); + + /** + * @brief Checks if the diagnostic storage buffer is empty. + * + * This method checks whether the buffer contains any stored diagnostic data. + * + * @return bool Returns `true` if the buffer contains no stored data, + * or `false` if the buffer has data. + */ + bool IsBufferEmpty(); + + /** + * @brief Retrieves the size of the data currently stored in the diagnostic buffer. + * + * This method returns the total size (in bytes) of all diagnostic data that is + * currently stored in the buffer. + * + * @return uint32_t The size (in bytes) of the stored diagnostic data. + */ + uint32_t GetDataSize(); + + /** + * @brief Clears entire buffer + */ + CHIP_ERROR ClearBuffer(); + + /** + * @brief Clears buffer up to the specified number of entries + */ + CHIP_ERROR ClearBuffer(uint32_t entries); + +private: + chip::TLV::CircularTLVReader mReader; + chip::TLV::CircularTLVWriter mWriter; +}; + +} // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h deleted file mode 100644 index 745f5a9bd8cd3f..00000000000000 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ /dev/null @@ -1,111 +0,0 @@ -/* - * - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include - -namespace chip { -namespace Tracing { -namespace Diagnostics { -class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public DiagnosticStorageInterface -{ -public: - CircularDiagnosticBuffer(uint8_t * buffer, size_t bufferLength) : chip::TLV::TLVCircularBuffer(buffer, bufferLength) {} - - CHIP_ERROR Store(const DiagnosticEntry & entry) override - { - CHIP_ERROR err = CHIP_NO_ERROR; - mWriter.Init(*this); - ReturnLogErrorOnFailure(entry.Encode(mWriter)); - return err; - } - - CHIP_ERROR Retrieve(MutableByteSpan & span, uint32_t & read_entries) override - { - CHIP_ERROR err = CHIP_NO_ERROR; - mReader.Init(*this); - - chip::TLV::TLVWriter writer; - writer.Init(span.data(), span.size()); - read_entries = 0; - - bool close_success = true; // To check if the last TLV is copied successfully. - uint32_t successful_written_bytes = 0; // Store temporary writer length in case last TLV is not copied successfully. - - while ((err = mReader.Next()) == CHIP_NO_ERROR) - { - if (mReader.GetType() == chip::TLV::kTLVType_Structure && mReader.GetTag() == chip::TLV::AnonymousTag()) - { - err = writer.CopyElement(mReader); - if (err == CHIP_NO_ERROR) - { - successful_written_bytes = writer.GetLengthWritten(); - read_entries++; - } - else - { - close_success = false; - break; - } - } - else - { - ChipLogDetail(DeviceLayer, "Skipping unexpected TLV element"); - } - } - ReturnErrorOnFailure(writer.Finalize()); - if (close_success) - { - successful_written_bytes = writer.GetLengthWritten(); - } - span.reduce_size(successful_written_bytes); - ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes); - return CHIP_NO_ERROR; - } - - bool IsBufferEmpty() override { return DataLength() == 0; } - - uint32_t GetDataSize() override { return DataLength(); } - - CHIP_ERROR ClearBuffer() override - { - while (!IsBufferEmpty()) - { - ReturnErrorOnFailure(EvictHead()); - } - return CHIP_NO_ERROR; - } - - CHIP_ERROR ClearBuffer(uint32_t entries) override - { - while (entries--) - { - ReturnErrorOnFailure(EvictHead()); - } - return CHIP_NO_ERROR; - } - -private: - chip::TLV::CircularTLVReader mReader; - chip::TLV::CircularTLVWriter mWriter; -}; - -} // namespace Diagnostics -} // namespace Tracing -} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 3aa4777ba962b2..2127f4d6292428 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -112,15 +112,33 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { case ValueType::kInt32: { ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32()); - Diagnostic metric(const_cast(event.key()), event.ValueInt32(), esp_log_timestamp()); - ReturnOnFailure(mStorageInstance->Store(metric)); + + // Allocate buffers for the diagnostic entry + char labelBuffer[kMaxStringValueSize]; + strncpy(labelBuffer, event.key(), kMaxStringValueSize); + labelBuffer[kMaxStringValueSize - 1] = '\0'; + + DiagnosticEntry entry = { .label = labelBuffer, + .intValue = event.ValueInt32(), + .type = Diagnostics::ValueType::kSignedInteger, + .timestamp = esp_log_timestamp() }; + ReturnOnFailure(mStorageInstance->Store(entry)); } break; case ValueType::kUInt32: { ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32()); - Diagnostic metric(const_cast(event.key()), event.ValueUInt32(), esp_log_timestamp()); - ReturnOnFailure(mStorageInstance->Store(metric)); + + // Allocate buffers for the diagnostic entry + char labelBuffer[kMaxStringValueSize]; + strncpy(labelBuffer, event.key(), kMaxStringValueSize); + labelBuffer[kMaxStringValueSize - 1] = '\0'; + + DiagnosticEntry entry = { .label = labelBuffer, + .uintValue = event.ValueUInt32(), + .type = Diagnostics::ValueType::kUnsignedInteger, + .timestamp = esp_log_timestamp() }; + ReturnOnFailure(mStorageInstance->Store(entry)); } break; @@ -161,8 +179,23 @@ CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * g { VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); - Diagnostic trace(const_cast(label), const_cast(group), esp_log_timestamp()); - return mStorageInstance->Store(trace); + + // Allocate buffers for the diagnostic entry + char labelBuffer[kMaxStringValueSize]; + strncpy(labelBuffer, label, kMaxStringValueSize); + labelBuffer[kMaxStringValueSize - 1] = '\0'; + + char groupBuffer[kMaxStringValueSize]; + strncpy(groupBuffer, group, kMaxStringValueSize); + groupBuffer[kMaxStringValueSize - 1] = '\0'; + + // Create diagnostic entry + DiagnosticEntry entry = { .label = labelBuffer, + .stringValue = groupBuffer, + .type = Diagnostics::ValueType::kCharString, + .timestamp = esp_log_timestamp() }; + + return mStorageInstance->Store(entry); } } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index 0f793ac01482ac..92d0bbbd1db69a 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -19,10 +19,9 @@ */ #include -#include +#include #include -#include namespace chip { namespace Tracing { namespace Diagnostics { @@ -32,7 +31,7 @@ namespace Diagnostics { class ESP32Diagnostics : public ::chip::Tracing::Backend { public: - ESP32Diagnostics(DiagnosticStorageInterface * storageInstance) : mStorageInstance(storageInstance) {} + ESP32Diagnostics(CircularDiagnosticBuffer * storageInstance) : mStorageInstance(storageInstance) {} // Deleted copy constructor and assignment operator to prevent copying ESP32Diagnostics(const ESP32Diagnostics &) = delete; @@ -57,7 +56,7 @@ class ESP32Diagnostics : public ::chip::Tracing::Backend private: using ValueType = MetricEvent::Value::Type; - DiagnosticStorageInterface * mStorageInstance; + CircularDiagnosticBuffer * mStorageInstance; CHIP_ERROR StoreDiagnostics(const char * label, const char * group); }; diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h deleted file mode 100644 index c6df25acc0702e..00000000000000 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ /dev/null @@ -1,249 +0,0 @@ -/* - * - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#define kMaxStringValueSize 128 - -namespace chip { -namespace Tracing { - -namespace Diagnostics { - -// Diagnostic TAGs -enum class DiagTag : uint8_t -{ - TIMESTAMP = 0, - LABEL, - VALUE, -}; - -/** - * @class DiagnosticEntry - * @brief Abstract base class for encoding diagnostic entries into TLV format. - * - */ -class DiagnosticEntry -{ -public: - /** - * @brief Virtual destructor for proper cleanup in derived classes. - */ - virtual ~DiagnosticEntry() = default; - - /** - * @brief Pure virtual method to encode diagnostic data into a TLV structure. - * - * @param writer A reference to the `chip::TLV::CircularTLVWriter` instance - * used to encode the TLV data. - * @return CHIP_ERROR Returns an error code indicating the success or - * failure of the encoding operation. - */ - virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) const = 0; - - virtual CHIP_ERROR Decode(chip::TLV::TLVReader & reader) = 0; -}; - -template -class Diagnostic : public DiagnosticEntry -{ -public: - Diagnostic(char * label = nullptr, T value = T{}, uint32_t timestamp = 0) : label_(label), value_(value), timestamp_(timestamp) - {} - - CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) const override - { - chip::TLV::TLVType DiagnosticOuterContainer = chip::TLV::kTLVType_NotSpecified; - ReturnErrorOnFailure( - writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, DiagnosticOuterContainer)); - - // Write timestamp - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::TIMESTAMP), timestamp_)); - - // Write label - if (strlen(label_) > kMaxStringValueSize) - { - char labelBuffer[kMaxStringValueSize + 1]; - memcpy(labelBuffer, label_, kMaxStringValueSize); - labelBuffer[kMaxStringValueSize] = '\0'; - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), labelBuffer)); - } - else - { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), label_)); - } - - // Write value - if constexpr (std::is_same_v) - { - if (strlen(value_) > kMaxStringValueSize) - { - char valueBuffer[kMaxStringValueSize + 1]; - memcpy(valueBuffer, value_, kMaxStringValueSize); - valueBuffer[kMaxStringValueSize] = '\0'; - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), valueBuffer)); - } - else - { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), value_)); - } - } - else if constexpr (std::is_same_v) - { - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), value_)); - } - else if constexpr (std::is_same_v) - { - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), value_)); - } - else - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); - ReturnErrorOnFailure(writer.Finalize()); - ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", label_); - return CHIP_NO_ERROR; - } - - CHIP_ERROR Decode(chip::TLV::TLVReader & reader) override - { - TLV::TLVType containerType; - ReturnErrorOnFailure(reader.EnterContainer(containerType)); - // Read timestamp - ReturnErrorOnFailure(reader.Next(TLV::ContextTag(DiagTag::TIMESTAMP))); - ReturnErrorOnFailure(reader.Get(timestamp_)); - - // Read label - ReturnErrorOnFailure(reader.Next(TLV::ContextTag(DiagTag::LABEL))); - uint32_t labelSize = reader.GetLength(); - char labelBuffer[kMaxStringValueSize + 1]; - ReturnErrorOnFailure(reader.GetString(labelBuffer, kMaxStringValueSize + 1)); - memcpy(label_, labelBuffer, labelSize + 1); - - // Read value - ReturnErrorOnFailure(reader.Next()); - if constexpr (std::is_same_v) - { - uint32_t valueSize = reader.GetLength(); - char valueBuffer[kMaxStringValueSize + 1]; - ReturnErrorOnFailure(reader.GetString(valueBuffer, kMaxStringValueSize + 1)); - memcpy(value_, valueBuffer, valueSize + 1); - } - else if constexpr (std::is_same_v) - { - ReturnErrorOnFailure(reader.Get(value_)); - } - else if constexpr (std::is_same_v) - { - ReturnErrorOnFailure(reader.Get(value_)); - } - else - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - ReturnErrorOnFailure(reader.ExitContainer(containerType)); - return CHIP_NO_ERROR; - } - - // Getters - char * GetLabel() const { return label_; } - T GetValue() const { return value_; } - uint32_t GetTimestamp() const { return timestamp_; } - -private: - char * label_ = nullptr; - T value_{}; - uint32_t timestamp_ = 0; -}; - -/** - * @brief Interface for storing and retrieving diagnostic data. - */ -class DiagnosticStorageInterface -{ -public: - /** - * @brief Virtual destructor for the interface. - */ - virtual ~DiagnosticStorageInterface() = default; - - /** - * @brief Stores a diagnostic entry in the diagnostic storage buffer. - * @param diagnostic A reference to a `DiagnosticEntry` object that contains - * the diagnostic data to be stored. - * @return CHIP_ERROR Returns `CHIP_NO_ERROR` if the data is successfully stored, - * or an appropriate error code in case of failure. - */ - virtual CHIP_ERROR Store(const DiagnosticEntry & diagnostic) = 0; - - /** - * @brief Copies diagnostic data from the storage buffer to a payload. - * - * This method retrieves the stored diagnostic data and copies it into the - * provided `payload` buffer. If the buffer is too small to hold all the data, - * the method returns the successfully copied entries along with an error code - * indicating that the buffer was insufficient. - * - * @param payload A reference to a `MutableByteSpan` where the retrieved - * diagnostic data will be copied. - * @param read_entries A reference to an integer that will hold the total - * number of successfully read diagnostic entries. - * - * @retval CHIP_NO_ERROR If the operation succeeded and all data was copied. - * @retval CHIP_ERROR_BUFFER_TOO_SMALL If the buffer was not large enough to hold all data. - * @retval CHIP_ERROR If any other failure occurred during the operation. - */ - virtual CHIP_ERROR Retrieve(MutableByteSpan & payload, uint32_t & read_entries) = 0; - - /** - * @brief Checks if the diagnostic storage buffer is empty. - * - * This method checks whether the buffer contains any stored diagnostic data. - * - * @return bool Returns `true` if the buffer contains no stored data, - * or `false` if the buffer has data. - */ - virtual bool IsBufferEmpty() = 0; - - /** - * @brief Retrieves the size of the data currently stored in the diagnostic buffer. - * - * This method returns the total size (in bytes) of all diagnostic data that is - * currently stored in the buffer. - * - * @return uint32_t The size (in bytes) of the stored diagnostic data. - */ - virtual uint32_t GetDataSize() = 0; - - /** - * @brief Clears entire buffer - */ - virtual CHIP_ERROR ClearBuffer() = 0; - - /** - * @brief Clears buffer up to the specified number of entries - */ - virtual CHIP_ERROR ClearBuffer(uint32_t entries) = 0; -}; - -} // namespace Diagnostics -} // namespace Tracing -} // namespace chip From 1ef047bb073d554f6809e4130af46991e576de3f Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 17 Mar 2025 16:37:04 +0530 Subject: [PATCH 22/25] Modified chip log statements, make string values mutable --- .../esp32_diagnostic_trace/Counter.cpp | 8 +-- .../DiagnosticEntry.cpp | 16 +++--- .../esp32_diagnostic_trace/DiagnosticEntry.h | 14 +++-- .../DiagnosticStorage.cpp | 2 +- .../DiagnosticTracing.cpp | 56 +++++++------------ .../DiagnosticTracing.h | 4 +- 6 files changed, 41 insertions(+), 59 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 606bf69f4bd1f0..929fc0e08dab44 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -46,14 +46,8 @@ CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, CircularDiagn { VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); - - // Allocate buffers for the diagnostic entry - char labelBuffer[kMaxStringValueSize]; - strncpy(labelBuffer, label, kMaxStringValueSize); - labelBuffer[kMaxStringValueSize - 1] = '\0'; - // Create diagnostic entry - DiagnosticEntry entry = { .label = labelBuffer, + DiagnosticEntry entry = { .label = const_cast(label), .uintValue = GetInstanceCount(label), .type = ValueType::kUnsignedInteger, .timestamp = esp_log_timestamp() }; diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp index 98eda94816a06c..273993fef25c0e 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp @@ -5,7 +5,7 @@ namespace chip { namespace Tracing { namespace Diagnostics { -CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) +CHIP_ERROR Encode(TLVWriter & writer, const DiagnosticEntry & entry) { TLVType DiagnosticOuterContainer = kTLVType_NotSpecified; ReturnErrorOnFailure(writer.StartContainer(AnonymousTag(), kTLVType_Structure, DiagnosticOuterContainer)); @@ -16,7 +16,8 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) // Write label if (entry.label != nullptr) { - if (strlen(entry.label) > kMaxStringValueSize) + uint32_t labelSize = strlen(entry.label) > kMaxStringValueSize ? kMaxStringValueSize : strlen(entry.label); + if (labelSize >= kMaxStringValueSize) { char labelBuffer[kMaxStringValueSize + 1]; memcpy(labelBuffer, entry.label, kMaxStringValueSize); @@ -25,7 +26,7 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) } else { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), entry.label)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), entry.label, labelSize)); } } @@ -35,7 +36,8 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) case ValueType::kCharString: if (entry.stringValue != nullptr) { - if (strlen(entry.stringValue) > kMaxStringValueSize) + uint32_t valueSize = strlen(entry.stringValue) > kMaxStringValueSize ? kMaxStringValueSize : strlen(entry.stringValue); + if (valueSize >= kMaxStringValueSize) { char valueBuffer[kMaxStringValueSize + 1]; memcpy(valueBuffer, entry.stringValue, kMaxStringValueSize); @@ -44,7 +46,7 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) } else { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), entry.stringValue)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), entry.stringValue, valueSize)); } } break; @@ -60,11 +62,11 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry) ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); ReturnErrorOnFailure(writer.Finalize()); - ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", entry.label); + ChipLogDetail(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", entry.label); return CHIP_NO_ERROR; } -CHIP_ERROR Decode(CircularTLVReader & reader, DiagnosticEntry & entry) +CHIP_ERROR Decode(TLVReader & reader, DiagnosticEntry & entry) { TLVType containerType; ReturnErrorOnFailure(reader.EnterContainer(containerType)); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h index 6dab0cfea5cb63..1d00751eb4175a 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h @@ -23,7 +23,7 @@ namespace chip { namespace Tracing { namespace Diagnostics { -static constexpr size_t kMaxStringValueSize = 128; +static constexpr size_t kMaxStringValueSize = 64; enum class ValueType { @@ -34,9 +34,11 @@ enum class ValueType struct DiagnosticEntry { + // mutable label because modified in decode char * label; union { + // mutable stringValue because modified in decode char * stringValue; uint32_t uintValue; int32_t intValue; @@ -47,13 +49,13 @@ struct DiagnosticEntry enum class DiagTag : uint8_t { - TIMESTAMP = 1, - LABEL = 2, - VALUE = 3 + TIMESTAMP = 0, + LABEL = 1, + VALUE = 2, }; -CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer, const DiagnosticEntry & entry); -CHIP_ERROR Decode(chip::TLV::CircularTLVReader & reader, DiagnosticEntry & entry); +CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, const DiagnosticEntry & entry); +CHIP_ERROR Decode(chip::TLV::TLVReader & reader, DiagnosticEntry & entry); } // namespace Diagnostics } // namespace Tracing diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp index 688132f7ea8a35..7b6e102fbbc5be 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp @@ -70,7 +70,7 @@ CHIP_ERROR CircularDiagnosticBuffer::Retrieve(MutableByteSpan & span, uint32_t & successful_written_bytes = writer.GetLengthWritten(); } span.reduce_size(successful_written_bytes); - ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes); + ChipLogDetail(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes); return CHIP_NO_ERROR; } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 2127f4d6292428..9097b73e200ec5 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -52,7 +52,7 @@ uint32_t MurmurHash(const void * key) if (hash == 0) { - ESP_LOGW("Tracing", "MurmurHash resulted in a hash value of 0"); + ChipLogError(DeviceLayer, "MurmurHash resulted in a hash value of 0"); } return hash; @@ -63,18 +63,23 @@ constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE; using HashValue = uint32_t; using namespace Utils; -// Only traces with scope in gPermitList are allowed. -// Used for MATTER_TRACE_SCOPE() +/* + * gPermitList will allow the following traces to be stored in the storage instance while other traces are skipped. + * Only traces with scope in gPermitList are allowed. + * Used for MATTER_TRACE_SCOPE() + */ HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("CASESession"), MurmurHash("NetworkCommissioning"), MurmurHash("GeneralCommissioning"), MurmurHash("OperationalCredentials"), MurmurHash("CASEServer"), - MurmurHash("Fabric") }; // namespace + MurmurHash("Fabric") }; -// All traces with value from gSkipList are skipped. -// Used for MATTER_TRACE_INSTANT() +/* + * gSkipList will skip the following traces from being stored in the storage instance while other traces are stored. + * Used for MATTER_TRACE_INSTANT() + */ HashValue gSkipList[kPermitListMaxSize] = { MurmurHash("Resolver"), }; @@ -111,14 +116,8 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) switch (event.ValueType()) { case ValueType::kInt32: { - ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32()); - - // Allocate buffers for the diagnostic entry - char labelBuffer[kMaxStringValueSize]; - strncpy(labelBuffer, event.key(), kMaxStringValueSize); - labelBuffer[kMaxStringValueSize - 1] = '\0'; - - DiagnosticEntry entry = { .label = labelBuffer, + ChipLogDetail(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32()); + DiagnosticEntry entry = { .label = const_cast(event.key()), .intValue = event.ValueInt32(), .type = Diagnostics::ValueType::kSignedInteger, .timestamp = esp_log_timestamp() }; @@ -127,14 +126,8 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) break; case ValueType::kUInt32: { - ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32()); - - // Allocate buffers for the diagnostic entry - char labelBuffer[kMaxStringValueSize]; - strncpy(labelBuffer, event.key(), kMaxStringValueSize); - labelBuffer[kMaxStringValueSize - 1] = '\0'; - - DiagnosticEntry entry = { .label = labelBuffer, + ChipLogDetail(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32()); + DiagnosticEntry entry = { .label = const_cast(event.key()), .uintValue = event.ValueUInt32(), .type = Diagnostics::ValueType::kUnsignedInteger, .timestamp = esp_log_timestamp() }; @@ -143,15 +136,15 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) break; case ValueType::kChipErrorCode: - ChipLogProgress(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode()); + ChipLogDetail(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode()); break; case ValueType::kUndefined: - ChipLogProgress(DeviceLayer, "The value of %s is undefined", event.key()); + ChipLogDetail(DeviceLayer, "The value of %s is undefined", event.key()); break; default: - ChipLogProgress(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key()); + ChipLogDetail(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key()); break; } } @@ -180,18 +173,9 @@ CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * g VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); - // Allocate buffers for the diagnostic entry - char labelBuffer[kMaxStringValueSize]; - strncpy(labelBuffer, label, kMaxStringValueSize); - labelBuffer[kMaxStringValueSize - 1] = '\0'; - - char groupBuffer[kMaxStringValueSize]; - strncpy(groupBuffer, group, kMaxStringValueSize); - groupBuffer[kMaxStringValueSize - 1] = '\0'; - // Create diagnostic entry - DiagnosticEntry entry = { .label = labelBuffer, - .stringValue = groupBuffer, + DiagnosticEntry entry = { .label = const_cast(label), + .stringValue = const_cast(group), .type = Diagnostics::ValueType::kCharString, .timestamp = esp_log_timestamp() }; diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index 92d0bbbd1db69a..fc8e0eca00e1fc 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -25,9 +25,9 @@ namespace chip { namespace Tracing { namespace Diagnostics { -/// A Backend that outputs data to chip logging. +/// A Backend that stores data to storage instance /// -/// Structured data is formatted as json strings. +/// Structured data is formatted as TLV. class ESP32Diagnostics : public ::chip::Tracing::Backend { public: From 66f50db658d8b099dcc6feed14b1dc1d69197cb4 Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 17 Mar 2025 12:07:52 +0000 Subject: [PATCH 23/25] Restyled changes --- src/tracing/esp32_diagnostic_trace/BUILD.gn | 4 ++-- src/tracing/esp32_diagnostic_trace/Counter.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn index 56ae6dfd96a29f..4115cc1eaa33d5 100644 --- a/src/tracing/esp32_diagnostic_trace/BUILD.gn +++ b/src/tracing/esp32_diagnostic_trace/BUILD.gn @@ -27,12 +27,12 @@ static_library("backend") { sources = [ "Counter.cpp", "Counter.h", + "DiagnosticEntry.cpp", + "DiagnosticEntry.h", "DiagnosticStorage.cpp", "DiagnosticStorage.h", "DiagnosticTracing.cpp", "DiagnosticTracing.h", - "DiagnosticEntry.h", - "DiagnosticEntry.cpp", ] public_deps = [ diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index c186ff66ea6363..af7cff814ba7e5 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -18,8 +18,8 @@ #pragma once -#include "tracing/esp32_diagnostic_trace/DiagnosticStorage.h" #include "tracing/esp32_diagnostic_trace/DiagnosticEntry.h" +#include "tracing/esp32_diagnostic_trace/DiagnosticStorage.h" #include namespace chip { From 4236f3c52cb7051622c0b9275ebbb1bb3124c448 Mon Sep 17 00:00:00 2001 From: mahesh Date: Wed, 19 Mar 2025 14:44:22 +0530 Subject: [PATCH 24/25] backend: copyright header change, retrieve api return value changed --- ...diagnostic-logs-provider-delegate-impl.cpp | 4 ++-- src/tracing/esp32_diagnostic_trace/BUILD.gn | 2 +- .../esp32_diagnostic_trace/Counter.cpp | 2 +- src/tracing/esp32_diagnostic_trace/Counter.h | 2 +- .../DiagnosticEntry.cpp | 17 ++++++++++++++++ .../esp32_diagnostic_trace/DiagnosticEntry.h | 2 +- .../DiagnosticStorage.cpp | 4 ++-- .../DiagnosticStorage.h | 20 ++++++++++++++++++- .../DiagnosticTracing.cpp | 2 +- .../DiagnosticTracing.h | 2 +- .../include/matter/tracing/macros_impl.h | 2 +- 11 files changed, 47 insertions(+), 12 deletions(-) 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 index 8502751cc2424d..d3766e294dc62b 100644 --- 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 @@ -128,9 +128,9 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE ChipLogError(DeviceLayer, "Empty Diagnostic buffer")); // Retrieve data from the diagnostic storage CHIP_ERROR err = mStorageInstance->Retrieve(endUserSupportSpan, sReadEntries); - if (err != CHIP_NO_ERROR) + if (err != CHIP_NO_ERROR && err != CHIP_ERROR_BUFFER_TOO_SMALL && err != CHIP_ERROR_END_OF_TLV) { - ChipLogError(DeviceLayer, "Failed to retrieve data: %" CHIP_ERROR_FORMAT, err.Format()); + ChipLogError(DeviceLayer, "Failed to retrieve data: %s", ErrorStr(err)); return err; } context->EndUserSupport.span = endUserSupportSpan; diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn index 4115cc1eaa33d5..5915aa0556836a 100644 --- a/src/tracing/esp32_diagnostic_trace/BUILD.gn +++ b/src/tracing/esp32_diagnostic_trace/BUILD.gn @@ -1,5 +1,5 @@ # -#Copyright (c) 2024 Project CHIP Authors +#Copyright (c) 2025 Project CHIP Authors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 929fc0e08dab44..fbc4446663545b 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index af7cff814ba7e5..670b905da68f54 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp index 273993fef25c0e..a4083ac2bbd504 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp @@ -1,3 +1,20 @@ +/* + * + * Copyright (c) 2025 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ #include using namespace chip::TLV; diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h index 1d00751eb4175a..510c25fa3dc322 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp index 7b6e102fbbc5be..20fc60d47b9f8f 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -71,7 +71,7 @@ CHIP_ERROR CircularDiagnosticBuffer::Retrieve(MutableByteSpan & span, uint32_t & } span.reduce_size(successful_written_bytes); ChipLogDetail(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes); - return CHIP_NO_ERROR; + return err; } bool CircularDiagnosticBuffer::IsBufferEmpty() diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h index ead96a5ba8e42e..28f6553b882d0c 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorage.h @@ -1,3 +1,20 @@ +/* + * + * Copyright (c) 2025 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ #pragma once #include #include @@ -29,7 +46,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer * * This method retrieves the stored diagnostic data and copies it into the * provided `payload` buffer. If the buffer is too small to hold all the data, - * the method returns the successfully copied entries along with an error code + * still method returns the successfully copied entries along with an error code * indicating that the buffer was insufficient. * * @param payload A reference to a `MutableByteSpan` where the retrieved @@ -39,6 +56,7 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer * * @retval CHIP_NO_ERROR If the operation succeeded and all data was copied. * @retval CHIP_ERROR_BUFFER_TOO_SMALL If the buffer was not large enough to hold all data. + * @retval CHIP_ERROR_END_OF_TLV If the end of the TLV stream is reached. * @retval CHIP_ERROR If any other failure occurred during the operation. */ CHIP_ERROR Retrieve(MutableByteSpan & payload, uint32_t & read_entries); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 9097b73e200ec5..31e11e5dbd2742 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -1,7 +1,7 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index fc8e0eca00e1fc..b9a6be73ec5e79 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -2,7 +2,7 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h b/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h index 2c96e80dedbea2..567b0d2da808fc 100644 --- a/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h +++ b/src/tracing/esp32_diagnostic_trace/include/matter/tracing/macros_impl.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2024 Project CHIP Authors + * Copyright (c) 2025 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); From a7fec5a0b1b3b766284e6403dcfa0171c8f5638c Mon Sep 17 00:00:00 2001 From: mahesh Date: Wed, 26 Mar 2025 12:02:29 +0530 Subject: [PATCH 25/25] diagnosticEntry: add invalid valuetype --- src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp | 4 ++++ src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp index a4083ac2bbd504..730c21dac752a6 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp @@ -50,6 +50,8 @@ CHIP_ERROR Encode(TLVWriter & writer, const DiagnosticEntry & entry) // Write value based on type switch (entry.type) { + case ValueType::kInvalidType: + return CHIP_ERROR_INVALID_ARGUMENT; case ValueType::kCharString: if (entry.stringValue != nullptr) { @@ -105,6 +107,8 @@ CHIP_ERROR Decode(TLVReader & reader, DiagnosticEntry & entry) ReturnErrorOnFailure(reader.Next()); switch (entry.type) { + case ValueType::kInvalidType: + return CHIP_ERROR_INVALID_ARGUMENT; case ValueType::kCharString: { uint32_t valueSize = reader.GetLength(); if (valueSize > kMaxStringValueSize) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h index 510c25fa3dc322..3d2db5ba129802 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h @@ -27,6 +27,7 @@ static constexpr size_t kMaxStringValueSize = 64; enum class ValueType { + kInvalidType = 0, kCharString, kUnsignedInteger, kSignedInteger