From e73460db47d9f63ac48e319eee1cc0d18d6b367f Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Thu, 10 Oct 2024 13:00:23 +0530 Subject: [PATCH 01/22] 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 - Enabled diagnostic trace for temperature-measurement-app example --- config/esp32/components/chip/CMakeLists.txt | 18 +- config/esp32/components/chip/Kconfig | 28 ++- .../esp32/main/Kconfig.projbuild | 32 +++ ...diagnostic-logs-provider-delegate-impl.cpp | 31 ++- .../diagnostic-logs-provider-delegate-impl.h | 10 + .../esp32/main/main.cpp | 9 + src/tracing/esp32_diagnostic_trace/BUILD.gn | 47 +++++ .../esp32_diagnostic_trace/Counter.cpp | 68 ++++++ src/tracing/esp32_diagnostic_trace/Counter.h | 58 ++++++ .../DiagnosticStorageManager.cpp | 170 +++++++++++++++ .../DiagnosticStorageManager.h | 62 ++++++ .../DiagnosticTracing.cpp | 170 +++++++++++++++ .../DiagnosticTracing.h | 70 +++++++ .../esp32_diagnostic_trace/Diagnostics.h | 196 ++++++++++++++++++ .../include/matter/tracing/macros_impl.h | 52 +++++ 15 files changed, 1006 insertions(+), 15 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 51f75954aae84f..4b3897161c48b8 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -294,6 +294,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() @@ -306,6 +311,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() @@ -368,9 +377,14 @@ if(CONFIG_ENABLE_PW_RPC) endif() if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE) - list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32TracingBackend.a") + list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32TracingBackend.a") endif() +if (CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE) + list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32DiagnosticsBackend.a") +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. @@ -458,4 +472,4 @@ if (CONFIG_CHIP_OTA_IMAGE_BUILD) ) # Adding dependecy as app target so that this runs after images are ready add_dependencies(chip-ota-image app) -endif() +endif() \ No newline at end of file diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index f8885b77e9739f..e38263425b7ea3 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -838,6 +838,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 @@ -854,15 +863,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 @@ -1224,4 +1232,4 @@ menu "CHIP Device Layer" endmenu -endmenu +endmenu \ No newline at end of file diff --git a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild index 4ec2e859c0b1f5..e8ebef3189403a 100644 --- a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild +++ b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild @@ -83,3 +83,35 @@ 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 DIAGNOSTIC_RETRIEVAL_BUFFER_SIZE + int "Set buffer size to retrieve diagnostic data" + depends on ESP_DIAGNOSTICS_ENABLED + default 1024 + help + Defines the buffer size (in bytes) for retrieving diagnostic data through diagnostic logs cluster. + Increase this size if the diagnostic data generated by the application requires more space. + + config END_USER_BUFFER_SIZE + int "Set buffer size for end user diagnostic data" + depends on ESP_DIAGNOSTICS_ENABLED + default 4096 + help + Defines the buffer size (in bytes) for storing diagnostic data related to end user activity. + This buffer will hold logs and traces relevant to user interactions with the Matter protocol. + Increase this size if the diagnostic data generated by the application requires more space. + + config NETWORK_BUFFER_SIZE + int "Set buffer size for network diagnostic data" + depends on ESP_DIAGNOSTICS_ENABLED + default 2048 + help + Defines the buffer size (in bytes) for storing network-related diagnostic data. + This buffer will store logs and traces related to network events and communication for the Matter protocol. + Adjust this size based on the expected network diagnostics requirements. +endmenu 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 705c2f653c9d8f..d4b771cf6783d4 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 @@ -105,14 +105,39 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE { context->intent = intent; + #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + + static uint8_t retrieveBuffer[RETRIEVAL_BUFFER_SIZE]; + MutableByteSpan endUserSupportSpan(retrieveBuffer, sizeof(retrieveBuffer)); + #endif + switch (intent) { case IntentEnum::kEndUserSupport: { - context->EndUserSupport.span = - ByteSpan(&endUserSupportLogStart[0], static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart)); + #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + 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<size_t>(endUserSupportLogEnd - endUserSupportLogStart)); + #endif } break; - case IntentEnum::kNetworkDiag: { context->NetworkDiag.span = ByteSpan(&networkDiagnosticLogStart[0], static_cast<size_t>(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..add85149dc4c73 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 <app/clusters/diagnostic-logs-server/DiagnosticLogsProviderDelegate.h> + +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include <src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> +#endif + #include <map> #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #include <esp_core_dump.h> #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#define RETRIEVAL_BUFFER_SIZE CONFIG_DIAGNOSTIC_RETRIEVAL_BUFFER_SIZE +using namespace chip::Tracing; +#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..fba063f55ef03b 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -52,6 +52,10 @@ #include <DeviceInfoProviderImpl.h> #endif // CONFIG_ENABLE_ESP32_DEVICE_INFO_PROVIDER +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include <tracing/esp32_diagnostic_trace/DiagnosticTracing.h> +#endif + namespace { #if CONFIG_ENABLE_ESP32_FACTORY_DATA_PROVIDER chip::DeviceLayer::ESP32FactoryDataProvider sFactoryDataProvider; @@ -75,6 +79,11 @@ 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 + static Tracing::Insights::ESP32Diagnostics diagnosticBackend; + Tracing::Register(diagnosticBackend); +#endif } extern "C" void app_main() 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..ea3a4b20002b83 --- /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 <string.h> +#include <tracing/esp32_diagnostic_trace/Counter.h> + +using namespace chip; + +namespace Insights { + +// 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 Insights diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h new file mode 100644 index 00000000000000..8a40a56416e7a1 --- /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 <esp_diagnostics_metrics.h> +#include <esp_log.h> +#include <lib/support/CHIPMem.h> +#include <lib/support/CHIPMemString.h> +#include <string.h> +#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" + +using namespace chip::Tracing; + +namespace Insights { + +/** + * This class is used to monotonically increment the counters as per the label of the counter macro + * 'MATTER_TRACE_COUNTER(label)' and report the metrics to esp-insights. + * 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 Insights diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp new file mode 100644 index 00000000000000..5ca8f00529ea09 --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -0,0 +1,170 @@ + +/* + * + * 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 <esp_log.h> +#include <lib/core/CHIPError.h> +#include <lib/support/logging/CHIPLogging.h> +#include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> + +namespace chip { +namespace Tracing { + +DiagnosticStorageImpl::DiagnosticStorageImpl() : + mEndUserCircularBuffer(mEndUserBuffer, END_USER_BUFFER_SIZE), mNetworkCircularBuffer(mNetworkBuffer, NETWORK_BUFFER_SIZE) +{} + +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) + chip::TLV::TLVType outerContainer; + err = writer.StartContainer(AnonymousTag(), chip::TLV::TLVType::kTLVType_Structure, outerContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", chip::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", chip::ErrorStr(err))); + + err = writer.Finalize(); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); + + printf("Total Data Length in Buffer : %lu\n Total available length in buffer : %lu\nTotal buffer length : %lu\n", + mEndUserCircularBuffer.DataLength(), mEndUserCircularBuffer.AvailableDataLength(), + mEndUserCircularBuffer.GetTotalDataLength()); + return CHIP_NO_ERROR; +} + +CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) +{ + printf("***************************************************************************RETRIEVAL " + "STARTED**********************************************************\n"); + CHIP_ERROR err = CHIP_NO_ERROR; + chip::TLV::TLVReader reader; + reader.Init(mEndUserCircularBuffer); + + chip::TLV::TLVWriter writer; + writer.Init(payload); + + uint32_t totalBufferSize = 0; + + chip::TLV::TLVType outWriterContainer; + err = writer.StartContainer(AnonymousTag(), chip::TLV::TLVType::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", chip::ErrorStr(err))); + + if (reader.GetType() == chip::TLV::TLVType::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag()) + { + chip::TLV::TLVType outerReaderContainer; + err = reader.EnterContainer(outerReaderContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", chip::ErrorStr(err))); + + err = reader.Next(); + VerifyOrReturnError( + err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", chip::ErrorStr(err))); + + // Check if the current element is a METRIC or TRACE container + if ((reader.GetType() == chip::TLV::kTLVType_Structure) && + (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) + { + ESP_LOGW("SIZE", "Total read till now: %ld Total write till now: %ld", reader.GetLengthRead(), writer.GetLengthWritten()); + + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) { + err = writer.CopyElement(reader); + if (err == CHIP_ERROR_BUFFER_TOO_SMALL) { + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); + break; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); + ChipLogProgress(DeviceLayer, "Read metric container successfully"); + mEndUserCircularBuffer.EvictHead(); + } + else { + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); + 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", chip::ErrorStr(err))); + + + } + else + { + ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container"); + } + + printf("Total Data Length in Buffer: %lu\n Total available length in buffer: %lu\nTotal buffer length: %lu\n", + mEndUserCircularBuffer.DataLength(), mEndUserCircularBuffer.AvailableDataLength(), + mEndUserCircularBuffer.GetTotalDataLength()); + } + + err = writer.EndContainer(outWriterContainer); + 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()); + printf("---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "Retrieval successful"); + return CHIP_NO_ERROR; +} + +bool DiagnosticStorageImpl::IsEmptyBuffer() +{ + return mEndUserCircularBuffer.DataLength() == 0; +} + +} // 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..8c393f70b32e5a --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -0,0 +1,62 @@ +/* + * + * 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 <lib/support/CHIPMem.h> +#include <lib/core/CHIPError.h> + +#define END_USER_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE +#define NETWORK_BUFFER_SIZE CONFIG_NETWORK_BUFFER_SIZE + +namespace chip { +namespace Tracing { +using namespace chip::Platform; + +class DiagnosticStorageImpl : public DiagnosticStorageInterface +{ +public: + + static DiagnosticStorageImpl& GetInstance() + { + static DiagnosticStorageImpl instance; + return instance; + } + + 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(); + ~DiagnosticStorageImpl(); + + TLVCircularBuffer mEndUserCircularBuffer; + TLVCircularBuffer mNetworkCircularBuffer; + uint8_t mEndUserBuffer[END_USER_BUFFER_SIZE]; + uint8_t mNetworkBuffer[NETWORK_BUFFER_SIZE]; +}; + +} // 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..ce5551e446aab4 --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -0,0 +1,170 @@ + +/* + * + * 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 <algorithm> +#include <esp_err.h> +#include <esp_heap_caps.h> +#include <esp_log.h> +#include <memory> +#include <tracing/backend.h> +#include <tracing/esp32_diagnostic_trace/Counter.h> +#include <tracing/esp32_diagnostic_trace/DiagnosticTracing.h> +#include <tracing/metric_event.h> + +namespace chip { +namespace Tracing { +namespace Insights { + +#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("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; + + printf("LOG MATRIC EVENT CALLED\n"); + + switch (event.ValueType()) + { + case ValueType::kInt32: { + ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32()); + Metric<int32_t> 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<uint32_t> 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) +{ + ::Insights::ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); +} + +void ESP32Diagnostics::TraceBegin(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")); + } +} + +void ESP32Diagnostics::TraceEnd(const char * label, const char * group) {} + +void ESP32Diagnostics::TraceInstant(const char * label, const char * group) {} + +} // namespace Insights +} // 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..eefb0f23de5a3c --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -0,0 +1,70 @@ +#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 <lib/core/CHIPError.h> +#include <tracing/backend.h> +#include <tracing/metric_event.h> +#include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> +#include <esp_log.h> + + +#include <memory> +namespace chip { +namespace Tracing { +namespace Insights { +/// A Backend that outputs data to chip logging. +/// +/// Structured data is formatted as json strings. +class ESP32Diagnostics : public ::chip::Tracing::Backend +{ +public: + ESP32Diagnostics() + { + // Additional initialization if necessary + } + + // 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; + +private: + using ValueType = MetricEvent::Value::Type; +}; + +} // namespace Insights +} // 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..96cb1e4e54c5f1 --- /dev/null +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -0,0 +1,196 @@ +/* + * + * 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 <lib/core/CHIPError.h> +#include <lib/support/Span.h> +#include <lib/core/TLVCircularBuffer.h> + +namespace chip { +namespace Tracing { + +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<typename T> +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; + chip::TLV::TLVType metricContainer; + err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::METRIC), chip::TLV::TLVType::kTLVType_Structure, metricContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", chip::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", chip::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", chip::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", chip::ErrorStr(err))); + + printf("Metric Value written to storage successfully : %s\n", label_); + err = writer.EndContainer(metricContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", chip::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; + chip::TLV::TLVType traceContainer; + err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::TRACE), chip::TLV::TLVType::kTLVType_Structure, traceContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for Trace: %s", chip::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", chip::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", chip::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", chip::ErrorStr(err))); + + printf("Trace Value written to storage successfully : %s\n", label_); + err = writer.EndContainer(traceContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to end TLV container for Trace : %s", chip::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; + chip::TLV::TLVType counterContainer; + err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::COUNTER), chip::TLV::TLVType::kTLVType_Structure, counterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to start TLV container for Counter: %s", chip::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", chip::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", chip::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", chip::ErrorStr(err))); + + printf("Counter Value written to storage successfully : %s\n", label_); + err = writer.EndContainer(counterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to end TLV container for counter : %s", chip::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 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..2ef214ef3e5ff6 --- /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 <tracing/registry.h> + +// 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 Insights { +class Scoped +{ +public: + inline Scoped(const char * label, const char * group) : mLabel(label), mGroup(group) { MATTER_TRACE_BEGIN(label, group); } + inline ~Scoped() { MATTER_TRACE_END(mLabel, mGroup); } + +private: + const char * mLabel; + const char * mGroup; +}; +} // namespace Insights +} // 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::Insights::Scoped _MACRO_CONCAT(_trace_scope, __COUNTER__)(label, group) From 3a65f8819d2450386262b0a23e88491cc1490658 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Fri, 25 Oct 2024 15:31:50 +0530 Subject: [PATCH 02/22] esp32 diagnostic trace changes - Resolve buffer related issues - Add store diagnostic call for trace_end and trace_instant - Update scoped macro - Remove extra namespace values, spaces and print statements - Remove evicthead call for circular buffer after read successful --- config/esp32/components/chip/CMakeLists.txt | 6 +- config/esp32/components/chip/Kconfig | 2 +- .../esp32/main/Kconfig.projbuild | 18 ----- ...diagnostic-logs-provider-delegate-impl.cpp | 24 +++--- .../diagnostic-logs-provider-delegate-impl.h | 4 +- .../esp32/main/main.cpp | 16 ++-- .../esp32_diagnostic_trace/Counter.cpp | 4 +- src/tracing/esp32_diagnostic_trace/Counter.h | 8 +- .../DiagnosticStorageManager.cpp | 66 +++++++---------- .../DiagnosticStorageManager.h | 18 ++--- .../DiagnosticTracing.cpp | 32 +++++--- .../DiagnosticTracing.h | 9 ++- .../esp32_diagnostic_trace/Diagnostics.h | 74 ++++++++++--------- .../include/matter/tracing/macros_impl.h | 8 +- 14 files changed, 135 insertions(+), 154 deletions(-) diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt index 4b3897161c48b8..c3746b3526e860 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -377,11 +377,11 @@ if(CONFIG_ENABLE_PW_RPC) endif() if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE) - list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32TracingBackend.a") + list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32TracingBackend.a") endif() if (CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE) - list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32DiagnosticsBackend.a") + list(APPEND chip_libraries "${CMAKE_CURRENT_BINARY_DIR}/lib/libEsp32DiagnosticsBackend.a") endif() @@ -472,4 +472,4 @@ if (CONFIG_CHIP_OTA_IMAGE_BUILD) ) # Adding dependecy as app target so that this runs after images are ready add_dependencies(chip-ota-image app) -endif() \ No newline at end of file +endif() diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index e38263425b7ea3..4747a3985152a2 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -1232,4 +1232,4 @@ menu "CHIP Device Layer" endmenu -endmenu \ No newline at end of file +endmenu diff --git a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild index e8ebef3189403a..767b269d995432 100644 --- a/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild +++ b/examples/temperature-measurement-app/esp32/main/Kconfig.projbuild @@ -89,14 +89,6 @@ menu "Platform Diagnostics" bool "Enable ESP Diagnostics" default n - config DIAGNOSTIC_RETRIEVAL_BUFFER_SIZE - int "Set buffer size to retrieve diagnostic data" - depends on ESP_DIAGNOSTICS_ENABLED - default 1024 - help - Defines the buffer size (in bytes) for retrieving diagnostic data through diagnostic logs cluster. - Increase this size if the diagnostic data generated by the application requires more space. - config END_USER_BUFFER_SIZE int "Set buffer size for end user diagnostic data" depends on ESP_DIAGNOSTICS_ENABLED @@ -104,14 +96,4 @@ 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. - Increase this size if the diagnostic data generated by the application requires more space. - - config NETWORK_BUFFER_SIZE - int "Set buffer size for network diagnostic data" - depends on ESP_DIAGNOSTICS_ENABLED - default 2048 - help - Defines the buffer size (in bytes) for storing network-related diagnostic data. - This buffer will store logs and traces related to network events and communication for the Matter protocol. - Adjust this size based on the expected network diagnostics requirements. endmenu 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 d4b771cf6783d4..d4ff5d6c6e5a84 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) { @@ -76,7 +81,14 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) switch (intent) { case IntentEnum::kEndUserSupport: - return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart); + { + #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + return RETRIEVAL_BUFFER_SIZE; + #else + return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart); + #endif + } + break; case IntentEnum::kNetworkDiag: return static_cast<size_t>(networkDiagnosticLogEnd - networkDiagnosticLogStart); case IntentEnum::kCrashLogs: @@ -105,17 +117,12 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE { context->intent = intent; - #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - - static uint8_t retrieveBuffer[RETRIEVAL_BUFFER_SIZE]; - MutableByteSpan endUserSupportSpan(retrieveBuffer, sizeof(retrieveBuffer)); - #endif - switch (intent) { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + if (diagnosticStorage.IsEmptyBuffer()) { printf("Buffer is empty\n"); @@ -129,7 +136,6 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); return err; } - // Now, assign the span to the EndUserSupport object or whatever is required context->EndUserSupport.span = endUserSupportSpan; #else 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 add85149dc4c73..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 @@ -31,8 +31,8 @@ #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#define RETRIEVAL_BUFFER_SIZE CONFIG_DIAGNOSTIC_RETRIEVAL_BUFFER_SIZE -using namespace chip::Tracing; +#define RETRIEVAL_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE +using namespace chip::Tracing::Diagnostics; #endif namespace chip { diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index fba063f55ef03b..d8d7018c7ba3dc 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -76,14 +76,15 @@ extern const char TAG[] = "temperature-measurement-app"; static AppDeviceCallbacks EchoCallbacks; +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +using namespace chip::Tracing::Diagnostics; +static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; +static size_t buffer_size = CONFIG_END_USER_BUFFER_SIZE; +#endif + static void InitServer(intptr_t context) { Esp32AppServer::Init(); // Init ZCL Data Model and CHIP App Server AND Initialize device attestation config - -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - static Tracing::Insights::ESP32Diagnostics diagnosticBackend; - Tracing::Register(diagnosticBackend); -#endif } extern "C" void app_main() @@ -92,6 +93,11 @@ extern "C" void app_main() chip::rpc::Init(); #endif +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + static ESP32Diagnostics diagnosticBackend(endUserBuffer, buffer_size); + Tracing::Register(diagnosticBackend); +#endif + ESP_LOGI(TAG, "Temperature sensor!"); // Initialize the ESP NVS layer. diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index ea3a4b20002b83..77948129a8e8fe 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -21,7 +21,7 @@ using namespace chip; -namespace Insights { +namespace Diagnostics { // This is a one time allocation for counters. It is not supposed to be freed. ESPDiagnosticCounter * ESPDiagnosticCounter::mHead = nullptr; @@ -65,4 +65,4 @@ void ESPDiagnosticCounter::ReportMetrics() VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); } -} // namespace Insights +} // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index 8a40a56416e7a1..4e58975999712f 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -25,13 +25,13 @@ #include <string.h> #include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" -using namespace chip::Tracing; +using namespace chip::Tracing::Diagnostics; -namespace Insights { +namespace Diagnostics { /** * This class is used to monotonically increment the counters as per the label of the counter macro - * 'MATTER_TRACE_COUNTER(label)' and report the metrics to esp-insights. + * 'MATTER_TRACE_COUNTER(label)' * As per the label of the counter macro, it adds the counter in the linked list with the name label if not * present and returns the same instance and increments the value if the counter is already present * in the list. @@ -55,4 +55,4 @@ class ESPDiagnosticCounter void ReportMetrics(); }; -} // namespace Insights +} // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp index 5ca8f00529ea09..98046d5523582b 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -17,7 +17,6 @@ * limitations under the License. */ -#include <esp_log.h> #include <lib/core/CHIPError.h> #include <lib/support/logging/CHIPLogging.h> #include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> @@ -25,9 +24,14 @@ namespace chip { namespace Tracing { -DiagnosticStorageImpl::DiagnosticStorageImpl() : - mEndUserCircularBuffer(mEndUserBuffer, END_USER_BUFFER_SIZE), mNetworkCircularBuffer(mNetworkBuffer, NETWORK_BUFFER_SIZE) -{} +namespace Diagnostics { +DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize) + : mEndUserCircularBuffer(buffer, bufferSize) {} + +DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) { + static DiagnosticStorageImpl instance(buffer, bufferSize); + return instance; +} DiagnosticStorageImpl::~DiagnosticStorageImpl() {} @@ -39,10 +43,10 @@ CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) writer.Init(mEndUserCircularBuffer); // Start a TLV structure container (Anonymous) - chip::TLV::TLVType outerContainer; - err = writer.StartContainer(AnonymousTag(), chip::TLV::TLVType::kTLVType_Structure, outerContainer); + TLVType outerContainer; + err = writer.StartContainer(AnonymousTag(), kTLVType_Structure, outerContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err))); err = diagnostic.Encode(writer); if (err != CHIP_NO_ERROR) @@ -55,32 +59,24 @@ CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) } err = writer.EndContainer(outerContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", ErrorStr(err))); err = writer.Finalize(); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); - - printf("Total Data Length in Buffer : %lu\n Total available length in buffer : %lu\nTotal buffer length : %lu\n", - mEndUserCircularBuffer.DataLength(), mEndUserCircularBuffer.AvailableDataLength(), - mEndUserCircularBuffer.GetTotalDataLength()); return CHIP_NO_ERROR; } CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) { - printf("***************************************************************************RETRIEVAL " - "STARTED**********************************************************\n"); CHIP_ERROR err = CHIP_NO_ERROR; - chip::TLV::TLVReader reader; + CircularTLVReader reader; reader.Init(mEndUserCircularBuffer); - chip::TLV::TLVWriter writer; + TLVWriter writer; writer.Init(payload); - uint32_t totalBufferSize = 0; - - chip::TLV::TLVType outWriterContainer; - err = writer.StartContainer(AnonymousTag(), chip::TLV::TLVType::kTLVType_List, outWriterContainer); + TLVType outWriterContainer; + err = writer.StartContainer(AnonymousTag(), kTLVType_List, outWriterContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container")); while (true) @@ -92,26 +88,22 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) break; } VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err))); - if (reader.GetType() == chip::TLV::TLVType::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag()) + if (reader.GetType() == kTLVType_Structure && reader.GetTag() == AnonymousTag()) { - chip::TLV::TLVType outerReaderContainer; + TLVType outerReaderContainer; err = reader.EnterContainer(outerReaderContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", ErrorStr(err))); err = reader.Next(); - VerifyOrReturnError( - err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", chip::ErrorStr(err))); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err))); // Check if the current element is a METRIC or TRACE container - if ((reader.GetType() == chip::TLV::kTLVType_Structure) && + if ((reader.GetType() == kTLVType_Structure) && (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) { - ESP_LOGW("SIZE", "Total read till now: %ld Total write till now: %ld", reader.GetLengthRead(), writer.GetLengthWritten()); - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) { err = writer.CopyElement(reader); if (err == CHIP_ERROR_BUFFER_TOO_SMALL) { @@ -119,8 +111,6 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) break; } VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); - ChipLogProgress(DeviceLayer, "Read metric container successfully"); - mEndUserCircularBuffer.EvictHead(); } else { ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); @@ -136,18 +126,12 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) // Exit the outer container err = reader.ExitContainer(outerReaderContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", chip::ErrorStr(err))); - - + ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err))); } else { ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container"); } - - printf("Total Data Length in Buffer: %lu\n Total available length in buffer: %lu\nTotal buffer length: %lu\n", - mEndUserCircularBuffer.DataLength(), mEndUserCircularBuffer.AvailableDataLength(), - mEndUserCircularBuffer.GetTotalDataLength()); } err = writer.EndContainer(outWriterContainer); @@ -156,7 +140,7 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) err = writer.Finalize(); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); payload.reduce_size(writer.GetLengthWritten()); - printf("---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten()); ChipLogProgress(DeviceLayer, "Retrieval successful"); return CHIP_NO_ERROR; } @@ -165,6 +149,6 @@ bool DiagnosticStorageImpl::IsEmptyBuffer() { return mEndUserCircularBuffer.DataLength() == 0; } - +} // namespace Diagnostics } // namespace Tracing } // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 8c393f70b32e5a..1b21a6cb54ed5c 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -22,22 +22,16 @@ #include <lib/support/CHIPMem.h> #include <lib/core/CHIPError.h> -#define END_USER_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE -#define NETWORK_BUFFER_SIZE CONFIG_NETWORK_BUFFER_SIZE - namespace chip { namespace Tracing { +namespace Diagnostics { using namespace chip::Platform; - +using chip::TLV::TLVType; class DiagnosticStorageImpl : public DiagnosticStorageInterface { public: - static DiagnosticStorageImpl& GetInstance() - { - static DiagnosticStorageImpl instance; - return instance; - } + static DiagnosticStorageImpl& GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete; @@ -49,14 +43,12 @@ class DiagnosticStorageImpl : public DiagnosticStorageInterface bool IsEmptyBuffer(); private: + DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize); DiagnosticStorageImpl(); ~DiagnosticStorageImpl(); TLVCircularBuffer mEndUserCircularBuffer; - TLVCircularBuffer mNetworkCircularBuffer; - uint8_t mEndUserBuffer[END_USER_BUFFER_SIZE]; - uint8_t mNetworkBuffer[NETWORK_BUFFER_SIZE]; }; - +} // namespace Diagnostics } // namespace Tracing } // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index ce5551e446aab4..174eac0625f44f 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -29,7 +29,7 @@ namespace chip { namespace Tracing { -namespace Insights { +namespace Diagnostics { #define LOG_HEAP_INFO(label, group, entry_exit) \ do \ @@ -76,6 +76,10 @@ HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("GeneralCommissioning"), MurmurHash("OperationalCredentials"), MurmurHash("CASEServer"), + MurmurHash("BLE"), + MurmurHash("BLE_Error"), + MurmurHash("Wifi"), + MurmurHash("Wifi_Error"), MurmurHash("Fabric") }; // namespace bool IsPermitted(HashValue hashValue) @@ -108,9 +112,6 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); CHIP_ERROR err = CHIP_NO_ERROR; - - printf("LOG MATRIC EVENT CALLED\n"); - switch (event.ValueType()) { case ValueType::kInt32: { @@ -145,11 +146,22 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ::Insights::ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); + ::Diagnostics::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) { + 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(); @@ -161,10 +173,6 @@ void ESP32Diagnostics::TraceBegin(const char * label, const char * group) } } -void ESP32Diagnostics::TraceEnd(const char * label, const char * group) {} - -void ESP32Diagnostics::TraceInstant(const char * label, const char * group) {} - -} // namespace Insights +} // namespace Diagnostics } // namespace Tracing } // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index eefb0f23de5a3c..6188f7b6b9c95e 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -28,16 +28,16 @@ #include <memory> namespace chip { namespace Tracing { -namespace Insights { +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() + ESP32Diagnostics(uint8_t *buffer, size_t buffer_size) { - // Additional initialization if necessary + DiagnosticStorageImpl::GetInstance(buffer, buffer_size); } // Deleted copy constructor and assignment operator to prevent copying @@ -60,11 +60,12 @@ 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; }; -} // namespace Insights +} // 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 index 96cb1e4e54c5f1..a824f4a0457559 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -25,6 +25,7 @@ namespace chip { namespace Tracing { +namespace Diagnostics { using namespace chip::TLV; enum class DIAGNOSTICS_TAG @@ -58,30 +59,30 @@ class Metric : public DiagnosticEntry { CHIP_ERROR Encode(CircularTLVWriter &writer) override { CHIP_ERROR err = CHIP_NO_ERROR; - chip::TLV::TLVType metricContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::METRIC), chip::TLV::TLVType::kTLVType_Structure, metricContainer); + 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", chip::ErrorStr(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", chip::ErrorStr(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", chip::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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to write VALUE for METRIC : %s", ErrorStr(err))); - printf("Metric Value written to storage successfully : %s\n", label_); + 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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", ErrorStr(err))); return err; } @@ -104,30 +105,30 @@ class Trace : public DiagnosticEntry { CHIP_ERROR Encode(CircularTLVWriter &writer) override { CHIP_ERROR err = CHIP_NO_ERROR; - chip::TLV::TLVType traceContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::TRACE), chip::TLV::TLVType::kTLVType_Structure, traceContainer); + 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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to start TLV container for Trace: %s", ErrorStr(err))); - // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + // TIMESTAMP + err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write LABEL for TRACE : %s", chip::ErrorStr(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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to write GROUP for TRACE : %s", ErrorStr(err))); - // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + // LABEL + err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to write LABEL for TRACE : %s", ErrorStr(err))); - printf("Trace Value written to storage successfully : %s\n", label_); + 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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end TLV container for Trace : %s", ErrorStr(err))); return err; } @@ -150,30 +151,30 @@ class Counter : public DiagnosticEntry { CHIP_ERROR Encode(CircularTLVWriter &writer) override { CHIP_ERROR err = CHIP_NO_ERROR; - chip::TLV::TLVType counterContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::COUNTER), chip::TLV::TLVType::kTLVType_Structure, counterContainer); + 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 start TLV container for Counter: %s", chip::ErrorStr(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", chip::ErrorStr(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", chip::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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to write VALUE for COUNTER : %s", ErrorStr(err))); - printf("Counter Value written to storage successfully : %s\n", label_); + 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", chip::ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end TLV container for counter : %s", ErrorStr(err))); return err; } @@ -192,5 +193,6 @@ class DiagnosticStorageInterface { 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 index 2ef214ef3e5ff6..23231d1da38c8f 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 @@ -32,21 +32,21 @@ namespace chip { namespace Tracing { -namespace Insights { +namespace Diagnostics { class Scoped { public: inline Scoped(const char * label, const char * group) : mLabel(label), mGroup(group) { MATTER_TRACE_BEGIN(label, group); } - inline ~Scoped() { MATTER_TRACE_END(mLabel, mGroup); } + inline ~Scoped() {} private: const char * mLabel; const char * mGroup; }; -} // namespace Insights +} // 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::Insights::Scoped _MACRO_CONCAT(_trace_scope, __COUNTER__)(label, group) +#define MATTER_TRACE_SCOPE(label, group) ::chip::Tracing::Diagnostics::Scoped _MACRO_CONCAT(_trace_scope, __COUNTER__)(label, group) From 018bcc5fcbf87e425f4924b3b799587487c7e92a Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Thu, 14 Nov 2024 11:53:06 +0530 Subject: [PATCH 03/22] 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 d4ff5d6c6e5a84..ba0c9c5f6ee305 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) { @@ -83,7 +78,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<size_t>(endUserSupportLogEnd - endUserSupportLogStart); #endif @@ -122,10 +117,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; } @@ -144,6 +139,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE #endif } break; + case IntentEnum::kNetworkDiag: { context->NetworkDiag.span = ByteSpan(&networkDiagnosticLogStart[0], static_cast<size_t>(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 5a9056ed4038cb2eafe3ae904821fb437b3558f0 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Fri, 15 Nov 2024 13:03:10 +0530 Subject: [PATCH 04/22] 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 ba0c9c5f6ee305..d55497fc6cfc1f 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 @@ -75,15 +75,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<size_t>(endUserSupportLogEnd - endUserSupportLogStart); - #endif - } - break; + case IntentEnum::kEndUserSupport: { +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + return DIAGNOSTIC_BUFFER_SIZE; +#else + return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart); +#endif + } + break; case IntentEnum::kNetworkDiag: return static_cast<size_t>(networkDiagnosticLogEnd - networkDiagnosticLogStart); case IntentEnum::kCrashLogs: @@ -115,28 +114,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<size_t>(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<size_t>(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 <string.h> #include <tracing/esp32_diagnostic_trace/Counter.h> -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 <esp_diagnostics_metrics.h> #include <esp_log.h> #include <lib/support/CHIPMem.h> #include <lib/support/CHIPMemString.h> #include <string.h> -#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 <lib/support/CHIPMem.h> #include <lib/core/CHIPError.h> +#include <lib/support/CHIPMem.h> 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 <esp_log.h> #include <lib/core/CHIPError.h> #include <tracing/backend.h> -#include <tracing/metric_event.h> #include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> -#include <esp_log.h> - +#include <tracing/metric_event.h> #include <memory> 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 <lib/core/CHIPError.h> -#include <lib/support/Span.h> #include <lib/core/TLVCircularBuffer.h> +#include <lib/support/Span.h> 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<typename T> -class Metric : public DiagnosticEntry { +template <typename T> +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 42c7597bf24f8101072086adb70146c6e8d04e78 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Tue, 19 Nov 2024 17:37:19 +0530 Subject: [PATCH 05/22] 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 4747a3985152a2..8c9dff90b199dd 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -840,12 +840,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" @@ -866,7 +864,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 d55497fc6cfc1f..c22cb0ec5a9f44 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 @@ -77,7 +77,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<size_t>(endUserSupportLogEnd - endUserSupportLogStart); #endif @@ -116,7 +117,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 <app/clusters/diagnostic-logs-server/DiagnosticLogsProviderDelegate.h> #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include <src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> +#include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> #endif #include <map> @@ -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 b35d816113fb91820f20c23f0b18764eb4ef6895 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Tue, 26 Nov 2024 15:33:41 +0530 Subject: [PATCH 06/22] esp32_diagnostic_trace: Review changes --- .../esp32_diagnostic_trace/DiagnosticStorageManager.cpp | 4 +++- src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h | 2 +- src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h | 2 +- src/tracing/esp32_diagnostic_trace/Diagnostics.h | 1 - 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp index d16430ce2f4ffe..23cd2e8859b78d 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -21,6 +21,8 @@ #include <lib/support/logging/CHIPLogging.h> #include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> +#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) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index d35f745130f343..3b7e1416ffd24f 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 <lib/core/CHIPError.h> #include <lib/support/CHIPMem.h> +#include <tracing/esp32_diagnostic_trace/Diagnostics.h> namespace chip { namespace Tracing { diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index a94aa2d232d8a8..31270fc38278a9 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -21,7 +21,7 @@ #include <esp_log.h> #include <lib/core/CHIPError.h> #include <tracing/backend.h> -#include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> +#include <tracing/esp32_diagnostic_trace/Diagnostics.h> #include <tracing/metric_event.h> #include <memory> 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 <lib/core/CHIPError.h> #include <lib/core/TLVCircularBuffer.h> #include <lib/support/Span.h> From ba7a6547e0e59ad8406cbc3759c56971ab2bdbe9 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Thu, 21 Nov 2024 14:10:32 +0530 Subject: [PATCH 07/22] backend: Replace linkedlist with map for counter diagnostics --- scripts/tools/check_includes_config.py | 3 ++ .../esp32_diagnostic_trace/Counter.cpp | 38 ++++++------------- src/tracing/esp32_diagnostic_trace/Counter.h | 24 +++++++----- .../DiagnosticTracing.cpp | 2 +- 4 files changed, 30 insertions(+), 37 deletions(-) diff --git a/scripts/tools/check_includes_config.py b/scripts/tools/check_includes_config.py index a7da891ea59c12..2d374c8f8747db 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..74ef5b7505777b 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -23,43 +23,29 @@ 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<const char *, uint32_t> 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 +int32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const { - return instanceCount; + return mCounterList[label]; } -void ESPDiagnosticCounter::ReportMetrics() +void ESPDiagnosticCounter::ReportMetrics(const char * label) { CHIP_ERROR err = CHIP_NO_ERROR; - Counter counter(label, instanceCount, esp_log_timestamp()); + Counter counter(label, GetInstanceCount(label), esp_log_timestamp()); DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); err = diagnosticStorage.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..c9acfe118c400e 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -23,6 +23,7 @@ #include <esp_log.h> #include <lib/support/CHIPMem.h> #include <lib/support/CHIPMemString.h> +#include <map> #include <string.h> namespace chip { @@ -39,20 +40,23 @@ 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 +public: + static ESPDiagnosticCounter & GetInstance(const char * label) + { + static ESPDiagnosticCounter instance; + CountInit(label); + return instance; + } - ESPDiagnosticCounter(const char * labelParam) : label(labelParam), instanceCount(1), mNext(nullptr) {} + int32_t GetInstanceCount(const char * label) const; -public: - static ESPDiagnosticCounter * GetInstance(const char * label); + void ReportMetrics(const char * label); - int32_t GetInstanceCount() const; +private: + ESPDiagnosticCounter() {} - void ReportMetrics(); + static std::map<const char *, uint32_t> mCounterList; + static void CountInit(const char * label); }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index f620abf65407a6..51b3ee2b50ad9e 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -146,7 +146,7 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); + ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label); } void ESP32Diagnostics::TraceBegin(const char * label, const char * group) From 6bfc6c405409c3a1709d81574237bc0f60ae82d8 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Wed, 27 Nov 2024 12:39:47 +0530 Subject: [PATCH 08/22] backend: Pass diagnostic storage as a parameter to backend --- .../esp32/main/main.cpp | 3 ++- src/tracing/esp32_diagnostic_trace/Counter.cpp | 7 +++---- src/tracing/esp32_diagnostic_trace/Counter.h | 7 +++---- .../esp32_diagnostic_trace/DiagnosticTracing.cpp | 16 +++++++--------- .../esp32_diagnostic_trace/DiagnosticTracing.h | 3 ++- 5 files changed, 17 insertions(+), 19 deletions(-) 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/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 74ef5b7505777b..e9043823f6d06c 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -37,17 +37,16 @@ void ESPDiagnosticCounter::CountInit(const char * label) } } -int32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const +uint32_t ESPDiagnosticCounter::GetInstanceCount(const char * label) const { return mCounterList[label]; } -void ESPDiagnosticCounter::ReportMetrics(const char * label) +void ESPDiagnosticCounter::ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance) { CHIP_ERROR err = CHIP_NO_ERROR; Counter counter(label, GetInstanceCount(label), esp_log_timestamp()); - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - err = diagnosticStorage.Store(counter); + 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 c9acfe118c400e..ea882b3487d3b3 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -18,7 +18,7 @@ #pragma once -#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" +#include "tracing/esp32_diagnostic_trace/Diagnostics.h" #include <esp_diagnostics_metrics.h> #include <esp_log.h> #include <lib/support/CHIPMem.h> @@ -48,13 +48,12 @@ class ESPDiagnosticCounter return instance; } - int32_t GetInstanceCount(const char * label) const; + uint32_t GetInstanceCount(const char * label) const; - void ReportMetrics(const char * label); + void ReportMetrics(const char * label, DiagnosticStorageInterface & mStorageInstance); private: ESPDiagnosticCounter() {} - static std::map<const char *, uint32_t> mCounterList; static void CountInit(const char * label); }; diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 51b3ee2b50ad9e..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<int32_t> 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<uint32_t> 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(label); + 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 31270fc38278a9..5ab60a31457447 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -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 From e3f41ec39c2e018caba95eedd6e89cee4e089115 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Mon, 2 Dec 2024 00:20:00 +0530 Subject: [PATCH 09/22] esp32_diagnostic_trace: Return actual data size --- .../esp32/main/diagnostic-logs-provider-delegate-impl.cpp | 3 +-- .../esp32_diagnostic_trace/DiagnosticStorageManager.cpp | 5 +++++ .../esp32_diagnostic_trace/DiagnosticStorageManager.h | 2 ++ 3 files changed, 8 insertions(+), 2 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 c22cb0ec5a9f44..6dd594c16cd402 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 @@ -77,8 +77,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<size_t>(endUserSupportLogEnd - endUserSupportLogStart); #endif diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp index 23cd2e8859b78d..bc3367d0e2e3a0 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -154,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 3b7e1416ffd24f..647a552495b9f2 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -39,6 +39,8 @@ class DiagnosticStorageImpl : public DiagnosticStorageInterface bool IsEmptyBuffer(); + uint32_t GetDataSize(); + private: DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize); DiagnosticStorageImpl(); From 54eb60084550b8b3f0c0d0ae22960282edfd1efa Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Tue, 19 Nov 2024 17:37:19 +0530 Subject: [PATCH 10/22] 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 6dd594c16cd402..de5189bde0419a 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) { @@ -130,6 +134,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 da033781a27994851f7ae836c21cca8d8484f35a Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Thu, 21 Nov 2024 14:10:32 +0530 Subject: [PATCH 11/22] backend: Replace linkedlist with map for counter diagnostics --- src/tracing/esp32_diagnostic_trace/Counter.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index ea882b3487d3b3..2ffb148ecd843f 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -25,6 +25,7 @@ #include <lib/support/CHIPMemString.h> #include <map> #include <string.h> +#include <map> namespace chip { namespace Tracing { From eb403ebb9c9af7f414fbd9374967e0687be5a670 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Fri, 6 Dec 2024 13:00:21 +0530 Subject: [PATCH 12/22] esp32_diagnostic_trace: add CircularDiagnosticBuffer class to improve design --- ...diagnostic-logs-provider-delegate-impl.cpp | 4 +- .../esp32/main/main.cpp | 3 +- src/tracing/esp32_diagnostic_trace/BUILD.gn | 1 - src/tracing/esp32_diagnostic_trace/Counter.h | 1 - .../DiagnosticStorageManager.cpp | 164 ------------------ .../DiagnosticStorageManager.h | 119 +++++++++++-- .../esp32_diagnostic_trace/Diagnostics.h | 48 ++++- 7 files changed, 150 insertions(+), 190 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 de5189bde0419a..2ff9a50765abf0 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 @@ -81,7 +81,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - return DiagnosticStorageImpl::GetInstance().GetDataSize(); + return CircularDiagnosticBuffer::GetInstance().GetDataSize(); #else return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart); #endif @@ -119,7 +119,7 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE { case IntentEnum::kEndUserSupport: { #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); MutableByteSpan endUserSupportSpan(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); if (diagnosticStorage.IsEmptyBuffer()) diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp 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.h b/src/tracing/esp32_diagnostic_trace/Counter.h index 2ffb148ecd843f..ea882b3487d3b3 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -25,7 +25,6 @@ #include <lib/support/CHIPMemString.h> #include <map> #include <string.h> -#include <map> namespace chip { namespace Tracing { diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.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 <lib/core/CHIPError.h> -#include <lib/support/logging/CHIPLogging.h> -#include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> - -#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..982122e66abb6a 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -25,29 +25,122 @@ namespace chip { namespace Tracing { namespace Diagnostics { -class DiagnosticStorageImpl : public DiagnosticStorageInterface +class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public DiagnosticStorageInterface { public: - static DiagnosticStorageImpl & GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); + // Singleton instance getter + static CircularDiagnosticBuffer & GetInstance() + { + static CircularDiagnosticBuffer instance; + return instance; + } - DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; - DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete; + // Delete copy constructor and assignment operator to ensure singleton + CircularDiagnosticBuffer(const CircularDiagnosticBuffer &) = delete; + CircularDiagnosticBuffer & operator=(const CircularDiagnosticBuffer &) = delete; - CHIP_ERROR Store(DiagnosticEntry & diagnostic) override; + void Init(uint8_t * buffer, size_t bufferLength) { chip::TLV::TLVCircularBuffer::Init(buffer, bufferLength); } - CHIP_ERROR Retrieve(MutableByteSpan & payload) override; + CHIP_ERROR Store(DiagnosticEntry & entry) override + { + chip::TLV::CircularTLVWriter writer; + writer.Init(*this); - bool IsEmptyBuffer(); + CHIP_ERROR err = entry.Encode(writer); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DeviceLayer, "Failed to write entry: %s", chip::ErrorStr(err)); + } + return err; + } - uint32_t GetDataSize(); + CHIP_ERROR Retrieve(MutableByteSpan & span) override + { + CHIP_ERROR err = CHIP_NO_ERROR; + chip::TLV::TLVReader reader; + reader.Init(*this); -private: - DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize); - DiagnosticStorageImpl(); - ~DiagnosticStorageImpl(); + chip::TLV::TLVWriter writer; + writer.Init(span.data(), span.size()); + + chip::TLV::TLVType outWriterContainer; + err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_List, outWriterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container")); + + while (CHIP_NO_ERROR == reader.Next()) + { + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err))); + + if (reader.GetType() == chip::TLV::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag()) + { + chip::TLV::TLVType outerReaderContainer; + err = reader.EnterContainer(outerReaderContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", ErrorStr(err))); + + err = reader.Next(); + VerifyOrReturnError( + err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err))); + + if ((reader.GetType() == chip::TLV::kTLVType_Structure) && + (reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC) || + reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE) || + reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER))) + { + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < (writer.GetRemainingFreeLength())) + { + err = writer.CopyElement(reader); + if (err == CHIP_ERROR_BUFFER_TOO_SMALL) + { + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); + break; + } + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); + } + else + { + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); + break; + } + } + else + { + ChipLogError(DeviceLayer, "Unexpected TLV element in outer container"); + reader.ExitContainer(outerReaderContainer); + return CHIP_ERROR_WRONG_TLV_TYPE; + } + err = reader.ExitContainer(outerReaderContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err))); + } + else + { + ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container"); + } + } - chip::TLV::TLVCircularBuffer mEndUserCircularBuffer; + err = writer.EndContainer(outWriterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to close outer container")); + err = writer.Finalize(); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); + span.reduce_size(writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", + writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "Retrieval successful"); + return CHIP_NO_ERROR; + } + + bool IsEmptyBuffer() override { return DataLength() == 0; } + + uint32_t GetDataSize() override { return DataLength(); } + +private: + CircularDiagnosticBuffer() : chip::TLV::TLVCircularBuffer(nullptr, 0) {} + ~CircularDiagnosticBuffer() override = default; }; + } // namespace Diagnostics } // namespace Tracing } // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 256d73d041e4f1..4bd5c74e72a9cf 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -59,10 +59,14 @@ class Metric : public DiagnosticEntry CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override { CHIP_ERROR err = CHIP_NO_ERROR; + chip::TLV::TLVType metricOuterContainer; + err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, metricOuterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to Start outer metric container: %s", ErrorStr(err))); chip::TLV::TLVType metricContainer; err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC), chip::TLV::kTLVType_Structure, metricContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to Start inner metric container: %s", ErrorStr(err))); // TIMESTAMP err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); @@ -82,7 +86,13 @@ class Metric : public DiagnosticEntry ChipLogProgress(DeviceLayer, "Metric Value written to storage successfully. label: %s\n", label_); err = writer.EndContainer(metricContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end inner TLV container for Metric : %s", ErrorStr(err))); + err = writer.EndContainer(metricOuterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to end outer TLV container for Metric : %s", ErrorStr(err))); + err = writer.Finalize(); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err))); + ReturnErrorOnFailure(writer.Finalize()); return err; } @@ -106,15 +116,18 @@ class Trace : public DiagnosticEntry CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override { CHIP_ERROR err = CHIP_NO_ERROR; + chip::TLV::TLVType traceOuterContainer; + err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, traceOuterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to Start outer trace container: %s", ErrorStr(err))); chip::TLV::TLVType traceContainer; err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE), chip::TLV::kTLVType_Structure, traceContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to start TLV container for Trace: %s", ErrorStr(err))); - + ChipLogError(DeviceLayer, "Failed to Start inner trace container: %s", ErrorStr(err))); // TIMESTAMP err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for TRACE : %s", ErrorStr(err))); // GROUP err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::GROUP), group_); @@ -129,7 +142,13 @@ class Trace : public DiagnosticEntry ChipLogProgress(DeviceLayer, "Trace Value written to storage successfully. label: %s value: %s\n", label_, group_); err = writer.EndContainer(traceContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end TLV container for Trace : %s", ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end inner TLV container for Trace : %s", ErrorStr(err))); + err = writer.EndContainer(traceOuterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to end outer TLV container for Trace : %s", ErrorStr(err))); + err = writer.Finalize(); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err))); + ReturnErrorOnFailure(writer.Finalize()); return err; } @@ -153,11 +172,15 @@ class Counter : public DiagnosticEntry CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override { CHIP_ERROR err = CHIP_NO_ERROR; + chip::TLV::TLVType counterOuterContainer; + err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, counterOuterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to Start outer counter container: %s", ErrorStr(err))); chip::TLV::TLVType counterContainer; err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), chip::TLV::kTLVType_Structure, counterContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to start TLV container for Counter: %s", ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to Start inner counter container: %s", ErrorStr(err))); // TIMESTAMP err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); @@ -177,7 +200,12 @@ class Counter : public DiagnosticEntry ChipLogProgress(DeviceLayer, "Counter Value written to storage successfully label: %s count: %ld\n", label_, count_); err = writer.EndContainer(counterContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end TLV container for counter : %s", ErrorStr(err))); + ChipLogError(DeviceLayer, "Failed to end inner TLV container for Counter : %s", ErrorStr(err))); + err = writer.EndContainer(counterOuterContainer); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to end outer TLV container for Counter : %s", ErrorStr(err))); + err = writer.Finalize(); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err))); return err; } @@ -213,6 +241,10 @@ class DiagnosticStorageInterface * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. */ virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0; + + virtual bool IsEmptyBuffer() = 0; + + virtual uint32_t GetDataSize() = 0; }; } // namespace Diagnostics From 0e41be72d74bb28c97e88be50b47be38e2a052b1 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Mon, 9 Dec 2024 11:54:33 +0530 Subject: [PATCH 13/22] esp32_diagnostic_trace: add extra tlv closing bytes check before copying diagnostic --- .../esp32_diagnostic_trace/DiagnosticStorageManager.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 982122e66abb6a..c210642825f683 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -22,6 +22,8 @@ #include <lib/support/CHIPMem.h> #include <tracing/esp32_diagnostic_trace/Diagnostics.h> +#define TLV_CLOSING_BYTES 4 + namespace chip { namespace Tracing { namespace Diagnostics { @@ -89,7 +91,8 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == chip::TLV::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) From b4f0e76749d8f8560d97d6ffc76018f88f9b3a89 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Tue, 10 Dec 2024 18:22:34 +0530 Subject: [PATCH 14/22] diagnostic_storage: unify diagnostic entries into a single type --- .../esp32_diagnostic_trace/Counter.cpp | 2 +- .../DiagnosticStorageManager.h | 57 ++--- .../DiagnosticTracing.cpp | 6 +- .../esp32_diagnostic_trace/Diagnostics.h | 200 ++++-------------- 4 files changed, 56 insertions(+), 209 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index e9043823f6d06c..0317fbaa492d1a 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -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<uint32_t> counter(label, GetInstanceCount(label), esp_log_timestamp()); err = mStorageInstance.Store(counter); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index c210642825f683..22713a70c6322c 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -22,7 +22,7 @@ #include <lib/support/CHIPMem.h> #include <tracing/esp32_diagnostic_trace/Diagnostics.h> -#define TLV_CLOSING_BYTES 4 +#define TLV_CLOSING_BYTE 1 namespace chip { namespace Tracing { @@ -65,9 +65,8 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia chip::TLV::TLVWriter writer; writer.Init(span.data(), span.size()); - chip::TLV::TLVType outWriterContainer; - err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_List, outWriterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container")); + 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()) { @@ -76,47 +75,21 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia if (reader.GetType() == chip::TLV::kTLVType_Structure && reader.GetTag() == chip::TLV::AnonymousTag()) { - chip::TLV::TLVType outerReaderContainer; - err = reader.EnterContainer(outerReaderContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", ErrorStr(err))); - - err = reader.Next(); - VerifyOrReturnError( - err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err))); - - if ((reader.GetType() == chip::TLV::kTLVType_Structure) && - (reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC) || - reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE) || - reader.GetTag() == chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER))) + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < ((writer.GetRemainingFreeLength() + TLV_CLOSING_BYTE))) { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < - ((writer.GetRemainingFreeLength() + TLV_CLOSING_BYTES))) + err = writer.CopyElement(reader); + if (err == CHIP_ERROR_BUFFER_TOO_SMALL) { - 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"); + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); break; } } else { - ChipLogError(DeviceLayer, "Unexpected TLV element in outer container"); - reader.ExitContainer(outerReaderContainer); - return CHIP_ERROR_WRONG_TLV_TYPE; + ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); + break; } - err = reader.ExitContainer(outerReaderContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err))); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); } else { @@ -124,14 +97,10 @@ class CircularDiagnosticBuffer : public chip::TLV::TLVCircularBuffer, public Dia } } - 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")); + ReturnErrorOnFailure(writer.EndContainer(outWriterContainer)); + ReturnErrorOnFailure(writer.Finalize()); span.reduce_size(writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", - writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "Retrieval successful"); + ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", writer.GetLengthWritten()); return CHIP_NO_ERROR; } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 2ebc9084a05504..9f1d2ed64d8ef1 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -115,14 +115,14 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { case ValueType::kInt32: { ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32()); - Metric<int32_t> metric(event.key(), event.ValueInt32(), esp_log_timestamp()); + Diagnostic<int32_t> 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<uint32_t> metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); + Diagnostic<uint32_t> metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); err = mStorageInstance.Store(metric); } break; @@ -169,7 +169,7 @@ void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) HashValue hashValue = MurmurHash(group); if (IsPermitted(hashValue)) { - Trace trace(label, group, esp_log_timestamp()); + Diagnostic<const char *> trace(label, group, esp_log_timestamp()); 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/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 4bd5c74e72a9cf..2025ba8d599a6e 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -28,72 +28,63 @@ namespace Diagnostics { enum class DIAGNOSTICS_TAG { - METRIC = 0, - TRACE = 1, - COUNTER = 2, - LABEL = 3, - GROUP = 4, - VALUE = 5, - TIMESTAMP = 6 + LABEL = 0, + VALUE, + TIMESTAMP }; +/** + * @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 <typename T> -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 metricOuterContainer; - err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, metricOuterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to Start outer metric container: %s", ErrorStr(err))); - chip::TLV::TLVType metricContainer; - err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC), chip::TLV::kTLVType_Structure, metricContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to Start inner metric container: %s", ErrorStr(err))); - + chip::TLV::TLVType DiagnosticOuterContainer = chip::TLV::kTLVType_NotSpecified; + ReturnErrorOnFailure( + writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, DiagnosticOuterContainer)); // 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))); - + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_)); // 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))); - + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_)); // 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 inner TLV container for Metric : %s", ErrorStr(err))); - err = writer.EndContainer(metricOuterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end outer TLV container for Metric : %s", ErrorStr(err))); - err = writer.Finalize(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err))); + if constexpr (std::is_same_v<T, const char *>) + { + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_)); + } + else + { + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_)); + } + ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); ReturnErrorOnFailure(writer.Finalize()); - return err; + ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", label_); + return CHIP_NO_ERROR; } private: @@ -102,119 +93,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 traceOuterContainer; - err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, traceOuterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to Start outer trace container: %s", ErrorStr(err))); - chip::TLV::TLVType traceContainer; - err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE), chip::TLV::kTLVType_Structure, traceContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to Start inner trace container: %s", ErrorStr(err))); - // TIMESTAMP - err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for TRACE : %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 inner TLV container for Trace : %s", ErrorStr(err))); - err = writer.EndContainer(traceOuterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end outer TLV container for Trace : %s", ErrorStr(err))); - err = writer.Finalize(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err))); - ReturnErrorOnFailure(writer.Finalize()); - return err; - } - -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 counterOuterContainer; - err = writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, counterOuterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to Start outer counter container: %s", ErrorStr(err))); - chip::TLV::TLVType counterContainer; - err = - writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), chip::TLV::kTLVType_Structure, counterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to Start inner counter container: %s", ErrorStr(err))); - - // TIMESTAMP - err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for 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 inner TLV container for Counter : %s", ErrorStr(err))); - err = writer.EndContainer(counterOuterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end outer TLV container for Counter : %s", ErrorStr(err))); - err = writer.Finalize(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to Finalize writer : %s", ErrorStr(err))); - return err; - } - -private: - const char * label_; - uint32_t count_; - uint32_t timestamp_; -}; - /** * @brief Interface for storing and retrieving diagnostic data. */ From 3e1958ffc256058e49f32dcf5ad37733d386451c Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Tue, 10 Dec 2024 19:10:20 +0530 Subject: [PATCH 15/22] diagnostic_storage: remove redundant code --- .../esp32_diagnostic_trace/Counter.cpp | 4 +-- src/tracing/esp32_diagnostic_trace/Counter.h | 9 ++---- .../DiagnosticStorageManager.h | 2 -- .../DiagnosticTracing.cpp | 31 +++++++------------ .../DiagnosticTracing.h | 2 -- .../esp32_diagnostic_trace/Diagnostics.h | 29 ++++++++--------- 6 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 0317fbaa492d1a..6eda7076a5e347 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -16,8 +16,8 @@ * limitations under the License. */ -#include <string.h> #include <tracing/esp32_diagnostic_trace/Counter.h> +#include <esp_log.h> namespace chip { namespace Tracing { @@ -25,7 +25,7 @@ namespace Diagnostics { std::map<const char *, uint32_t> ESPDiagnosticCounter::mCounterList; -void ESPDiagnosticCounter::CountInit(const char * label) +void ESPDiagnosticCounter::IncreaseCount(const char * label) { if (mCounterList.find(label) != mCounterList.end()) { diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index ea882b3487d3b3..38a83b41234cfb 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -19,12 +19,7 @@ #pragma once #include "tracing/esp32_diagnostic_trace/Diagnostics.h" -#include <esp_diagnostics_metrics.h> -#include <esp_log.h> -#include <lib/support/CHIPMem.h> -#include <lib/support/CHIPMemString.h> #include <map> -#include <string.h> 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<const char *, uint32_t> mCounterList; - static void CountInit(const char * label); + static void IncreaseCount(const char * label); }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 22713a70c6322c..723a25e9654c9e 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -18,8 +18,6 @@ #pragma once -#include <lib/core/CHIPError.h> -#include <lib/support/CHIPMem.h> #include <tracing/esp32_diagnostic_trace/Diagnostics.h> #define TLV_CLOSING_BYTE 1 diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 9f1d2ed64d8ef1..b161c16d079770 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -19,13 +19,9 @@ #include <algorithm> #include <esp_err.h> -#include <esp_heap_caps.h> #include <esp_log.h> -#include <memory> -#include <tracing/backend.h> #include <tracing/esp32_diagnostic_trace/Counter.h> #include <tracing/esp32_diagnostic_trace/DiagnosticTracing.h> -#include <tracing/metric_event.h> namespace chip { namespace Tracing { @@ -76,14 +72,11 @@ HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("GeneralCommissioning"), MurmurHash("OperationalCredentials"), MurmurHash("CASEServer"), - MurmurHash("BLE"), - MurmurHash("BLE_Error"), - MurmurHash("Wifi"), - MurmurHash("Wifi_Error"), MurmurHash("Fabric") }; // namespace -bool IsPermitted(HashValue hashValue) +bool IsPermitted(const char * str) { + HashValue hashValue = MurmurHash(str); for (HashValue permitted : gPermitList) { if (permitted == 0) @@ -150,12 +143,18 @@ void ESP32Diagnostics::TraceCounter(const char * label) void ESP32Diagnostics::TraceBegin(const char * label, const char * group) { - StoreDiagnostics(label, group); + if (IsPermitted(group)) + { + StoreDiagnostics(label, group); + } } void ESP32Diagnostics::TraceEnd(const char * label, const char * group) { - StoreDiagnostics(label, group); + if (IsPermitted(group)) + { + StoreDiagnostics(label, group); + } } void ESP32Diagnostics::TraceInstant(const char * label, const char * group) @@ -165,14 +164,8 @@ void ESP32Diagnostics::TraceInstant(const char * label, const char * group) void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) { - CHIP_ERROR err = CHIP_NO_ERROR; - HashValue hashValue = MurmurHash(group); - if (IsPermitted(hashValue)) - { - Diagnostic<const char *> trace(label, group, esp_log_timestamp()); - err = mStorageInstance.Store(trace); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); - } + Diagnostic<const char *> 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 <esp_log.h> -#include <lib/core/CHIPError.h> #include <tracing/backend.h> #include <tracing/esp32_diagnostic_trace/Diagnostics.h> #include <tracing/metric_event.h> diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 2025ba8d599a6e..d035467d8909ca 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -17,22 +17,14 @@ */ #pragma once -#include <lib/core/CHIPError.h> + #include <lib/core/TLVCircularBuffer.h> -#include <lib/support/Span.h> namespace chip { namespace Tracing { namespace Diagnostics { -enum class DIAGNOSTICS_TAG -{ - LABEL = 0, - VALUE, - TIMESTAMP -}; - /** * @class DiagnosticEntry * @brief Abstract base class for encoding diagnostic entries into TLV format. @@ -68,18 +60,15 @@ class Diagnostic : public DiagnosticEntry chip::TLV::TLVType DiagnosticOuterContainer = chip::TLV::kTLVType_NotSpecified; ReturnErrorOnFailure( writer.StartContainer(chip::TLV::AnonymousTag(), chip::TLV::kTLVType_Structure, DiagnosticOuterContainer)); - // TIMESTAMP - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_)); - // LABEL - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_)); - // VALUE + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(0), timestamp_)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(1), label_)); if constexpr (std::is_same_v<T, const char *>) { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(2), value_)); } else { - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_)); + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(2), value_)); } ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); ReturnErrorOnFailure(writer.Finalize()); @@ -120,8 +109,16 @@ class DiagnosticStorageInterface */ virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0; + /** + * @brief Checks if the storage buffer is empty. + * @return bool true if the buffer contains no stored diagnostic data, otherwise false. + */ virtual bool IsEmptyBuffer() = 0; + /** + * @brief Retrieves the size of the data currently stored in the buffer. + * @return uint32_t The size (in bytes) of the stored diagnostic data. + */ virtual uint32_t GetDataSize() = 0; }; From 4d2d9e2942d21648e4b9907b02c7d88c5eba0bcb Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Wed, 11 Dec 2024 12:00:45 +0530 Subject: [PATCH 16/22] backend: Remove redundant code, allow only permited value to trace instant --- .../esp32_diagnostic_trace/Counter.cpp | 2 +- .../DiagnosticTracing.cpp | 25 +++++++------------ .../include/matter/tracing/macros_impl.h | 2 +- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 6eda7076a5e347..11e2bb2b1430ce 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -16,8 +16,8 @@ * limitations under the License. */ -#include <tracing/esp32_diagnostic_trace/Counter.h> #include <esp_log.h> +#include <tracing/esp32_diagnostic_trace/Counter.h> namespace chip { namespace Tracing { diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index b161c16d079770..05c22bb53bfdcb 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -27,12 +27,6 @@ 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; @@ -72,7 +66,8 @@ HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"), MurmurHash("GeneralCommissioning"), MurmurHash("OperationalCredentials"), MurmurHash("CASEServer"), - MurmurHash("Fabric") }; // namespace + MurmurHash("Fabric"), + MurmurHash("Resolver") }; // namespace bool IsPermitted(const char * str) { @@ -149,23 +144,21 @@ void ESP32Diagnostics::TraceBegin(const char * label, const char * group) } } -void ESP32Diagnostics::TraceEnd(const char * label, const char * group) +void ESP32Diagnostics::TraceEnd(const char * label, const char * group) {} + +void ESP32Diagnostics::TraceInstant(const char * label, const char * value) { - if (IsPermitted(group)) + if (!IsPermitted(value)) { - StoreDiagnostics(label, group); + StoreDiagnostics(label, value); } } -void ESP32Diagnostics::TraceInstant(const char * label, const char * group) -{ - StoreDiagnostics(label, group); -} - void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) { Diagnostic<const char *> trace(label, group, esp_log_timestamp()); - VerifyOrReturn(mStorageInstance.Store(trace) == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); + 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/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 3e1cf1a2a107286645af929c30eaa03c3d65a784 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Wed, 11 Dec 2024 12:01:55 +0530 Subject: [PATCH 17/22] 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 2ff9a50765abf0..0eb21e349c96fe 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) { @@ -84,7 +80,7 @@ size_t LogProvider::GetSizeForIntent(IntentEnum intent) return CircularDiagnosticBuffer::GetInstance().GetDataSize(); #else return static_cast<size_t>(endUserSupportLogEnd - endUserSupportLogStart); -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE } break; case IntentEnum::kNetworkDiag: @@ -122,25 +118,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<size_t>(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 <app/clusters/diagnostic-logs-server/DiagnosticLogsProviderDelegate.h> -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> -#endif - #include <map> #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 <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> 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 <tracing/esp32_diagnostic_trace/DiagnosticTracing.h> -#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 6835f62097756756853ff4912c8a32ed4b263409 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Mon, 23 Dec 2024 12:49:53 +0530 Subject: [PATCH 18/22] 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 0eb21e349c96fe..c6e3cd52622618 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) { @@ -116,14 +120,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 = @@ -297,11 +304,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 <tracing/esp32_diagnostic_trace/Diagnostics.h> -#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 00cac01139f99260c11cc9180adaf2592ea3c453 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Thu, 26 Dec 2024 11:50:01 +0530 Subject: [PATCH 19/22] 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 <typename T> @@ -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<T, const char *>) { - 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 e848b4eadb36aa125059e48287c8050e6e1f94f6 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Fri, 27 Dec 2024 11:14:56 +0530 Subject: [PATCH 20/22] 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 c6e3cd52622618..5d8b3d586345b9 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 { @@ -81,7 +85,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<size_t>(endUserSupportLogEnd - endUserSupportLogStart); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -119,9 +123,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 @@ -307,12 +309,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 <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> -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 <tracing/esp32_diagnostic_trace/DiagnosticTracing.h> +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 aabef4cb4a2eefa8d000f290e196a0140a6a2479 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Mon, 6 Jan 2025 10:54:25 +0530 Subject: [PATCH 21/22] 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 5d8b3d586345b9..4c091a60bbe0cc 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 { @@ -85,7 +82,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<size_t>(endUserSupportLogEnd - endUserSupportLogStart); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE @@ -123,11 +122,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)); @@ -307,16 +308,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 <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> -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<uint64_t> & 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 d75e86d0ae107ff181b1e047d63fd022bb53f706 Mon Sep 17 00:00:00 2001 From: mahesh <maheshpimple2002@gmail.com> Date: Mon, 6 Jan 2025 11:50:31 +0530 Subject: [PATCH 22/22] 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 4c091a60bbe0cc..b1bdf24ae5b3c0 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 @@ -125,7 +125,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<uint32_t> counter(label, GetInstanceCount(label), esp_log_timestamp()); - err = mStorageInstance.Store(counter); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); + err = mStorageInstance->Store(counter); + return err; } } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h 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<int32_t> metric(event.key(), event.ValueInt32(), esp_log_timestamp()); - err = mStorageInstance.Store(metric); + err = mStorageInstance->Store(metric); } break; case ValueType::kUInt32: { - ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); + ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32()); Diagnostic<uint32_t> metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); - err = mStorageInstance.Store(metric); + err = mStorageInstance->Store(metric); } break; case ValueType::kChipErrorCode: - ESP_LOGI("mtr", "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode()); + ChipLogProgress(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode()); break; case ValueType::kUndefined: - ESP_LOGI("mtr", "The value of %s is undefined", event.key()); + ChipLogProgress(DeviceLayer, "The value of %s is undefined", event.key()); break; default: - ESP_LOGI("mtr", "The value of %s is of an UNKNOWN TYPE", event.key()); + ChipLogProgress(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key()); break; } @@ -136,14 +137,16 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance); + CHIP_ERROR err = ESPDiagnosticCounter::GetInstance(label).ReportMetrics(label, mStorageInstance); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter Diagnostic data")); } void ESP32Diagnostics::TraceBegin(const char * label, const char * group) { if (IsPermitted(group)) { - StoreDiagnostics(label, group); + CHIP_ERROR err = StoreDiagnostics(label, group); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); } } @@ -153,15 +156,18 @@ void ESP32Diagnostics::TraceInstant(const char * label, const char * value) { if (!IsPermitted(value)) { - StoreDiagnostics(label, value); + CHIP_ERROR err = StoreDiagnostics(label, value); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); } } -void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) +CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) { + CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrReturnError(mStorageInstance != nullptr, err, ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL")); Diagnostic<const char *> trace(label, group, esp_log_timestamp()); - VerifyOrReturn(mStorageInstance.Store(trace) == CHIP_NO_ERROR, - ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); + err = mStorageInstance->Store(trace); + return err; } } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h 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<T, const char *>) { - ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(D_TAG::VALUE), value_)); + ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), value_)); } else { - ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(D_TAG::VALUE), value_)); + ReturnErrorOnFailure(writer.Put(chip::TLV::ContextTag(DiagTag::VALUE), value_)); } ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer)); ReturnErrorOnFailure(writer.Finalize()); @@ -121,7 +121,7 @@ class DiagnosticStorageInterface * @brief Checks if the storage buffer is empty. * @return bool true if the buffer contains no stored diagnostic data, otherwise false. */ - virtual bool IsEmptyBuffer() = 0; + virtual bool IsBufferEmpty() = 0; /** * @brief Retrieves the size of the data currently stored in the buffer.