From ee8624e31f29d48fecace26a582868dd082dfbdd Mon Sep 17 00:00:00 2001 From: mahesh 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 0047cfecc70a1c..db859e5d2949e5 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -298,6 +298,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() @@ -310,6 +315,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. @@ -604,4 +618,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 2d8127900eed69..b97dbf955d0aea 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -999,6 +999,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 @@ -1015,15 +1024,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 @@ -1403,4 +1411,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 55336ea76030dd..b72e2536e54186 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(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(endUserSupportLogEnd - endUserSupportLogStart)); + #endif } break; - case IntentEnum::kNetworkDiag: { context->NetworkDiag.span = ByteSpan(&networkDiagnosticLogStart[0], static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart)); diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index 3431a54adc86a8..533ec2b4f01244 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -19,12 +19,22 @@ #pragma once #include + +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include +#endif + #include #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #include #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#define RETRIEVAL_BUFFER_SIZE CONFIG_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 #endif // CONFIG_ENABLE_ESP32_DEVICE_INFO_PROVIDER +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include +#endif + namespace { #if CONFIG_ENABLE_ESP32_FACTORY_DATA_PROVIDER chip::DeviceLayer::ESP32FactoryDataProvider sFactoryDataProvider; @@ -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 +#include + +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 +#include +#include +#include +#include +#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 +#include +#include +#include + +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 +#include + +#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 +#include +#include +#include +#include +#include +#include +#include +#include + +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 metric(event.key(), event.ValueInt32(), esp_log_timestamp()); + err = diagnosticStorage.Store(metric); + } + break; + + case ValueType::kUInt32: { + ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); + Metric metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); + err = diagnosticStorage.Store(metric); + } + break; + + case ValueType::kChipErrorCode: + ESP_LOGI("mtr", "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode()); + break; + + case ValueType::kUndefined: + ESP_LOGI("mtr", "The value of %s is undefined", event.key()); + break; + + default: + ESP_LOGI("mtr", "The value of %s is of an UNKNOWN TYPE", event.key()); + break; + } + + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Metric Diagnostic data")); +} + +void ESP32Diagnostics::TraceCounter(const char * label) +{ + ::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 +#include +#include +#include +#include + + +#include +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 +#include +#include + +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 +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 + +// 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 06ead62ccf80f384efda42c2d3ed01a378e229af Mon Sep 17 00:00:00 2001 From: mahesh 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 db859e5d2949e5..35815c12fd87fc 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() @@ -618,4 +618,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 b97dbf955d0aea..a6373398f36a71 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -1411,4 +1411,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 b72e2536e54186..2c0906d1ce7b27 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(endUserSupportLogEnd - endUserSupportLogStart); + { + #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + return RETRIEVAL_BUFFER_SIZE; + #else + return static_cast(endUserSupportLogEnd - endUserSupportLogStart); + #endif + } + break; case IntentEnum::kNetworkDiag: return static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart); case IntentEnum::kCrashLogs: @@ -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 533ec2b4f01244..fc10485072a13f 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 #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 #include #include #include @@ -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 #include -#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 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 6175cdf3a12c0c4aa9f9f25b5af2b2ea27d17e95 Mon Sep 17 00:00:00 2001 From: mahesh 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 2c0906d1ce7b27..16df89e9b8d837 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(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(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 fc10485072a13f..310c5b4b124aeb 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 1bd882ae8c96ed2c3df2c49e553ab463dacf1117 Mon Sep 17 00:00:00 2001 From: mahesh 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 16df89e9b8d837..a0046804e82167 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(endUserSupportLogEnd - endUserSupportLogStart); - #endif - } - break; + case IntentEnum::kEndUserSupport: { +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + return DIAGNOSTIC_BUFFER_SIZE; +#else + return static_cast(endUserSupportLogEnd - endUserSupportLogStart); +#endif + } + break; case IntentEnum::kNetworkDiag: return static_cast(networkDiagnosticLogEnd - networkDiagnosticLogStart); case IntentEnum::kCrashLogs: @@ -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(endUserSupportLogEnd - endUserSupportLogStart)); - #endif +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); + MutableByteSpan endUserSupportSpan(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); + + if (diagnosticStorage.IsEmptyBuffer()) + { + ChipLogError(DeviceLayer, "Empty Diagnostic buffer"); + return CHIP_ERROR_NOT_FOUND; + } + // Retrieve data from the diagnostic storage + CHIP_ERROR err = diagnosticStorage.Retrieve(endUserSupportSpan); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DeviceLayer, "Failed to retrieve data: %s", chip::ErrorStr(err)); + return err; + } + // Now, assign the span to the EndUserSupport object or whatever is required + context->EndUserSupport.span = endUserSupportSpan; +#else + context->EndUserSupport.span = + ByteSpan(&endUserSupportLogStart[0], static_cast(endUserSupportLogEnd - endUserSupportLogStart)); +#endif } break; diff --git a/src/tracing/esp32_diagnostic_trace/BUILD.gn b/src/tracing/esp32_diagnostic_trace/BUILD.gn index 60d425af9497cb..c0b9e845c17b5a 100644 --- a/src/tracing/esp32_diagnostic_trace/BUILD.gn +++ b/src/tracing/esp32_diagnostic_trace/BUILD.gn @@ -27,10 +27,10 @@ static_library("backend") { sources = [ "Counter.cpp", "Counter.h", - "DiagnosticTracing.cpp", - "DiagnosticTracing.h", "DiagnosticStorageManager.cpp", "DiagnosticStorageManager.h", + "DiagnosticTracing.cpp", + "DiagnosticTracing.h", "Diagnostics.h", ] diff --git a/src/tracing/esp32_diagnostic_trace/Counter.cpp b/src/tracing/esp32_diagnostic_trace/Counter.cpp index 77948129a8e8fe..73116ed9a04acd 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.cpp +++ b/src/tracing/esp32_diagnostic_trace/Counter.cpp @@ -19,8 +19,8 @@ #include #include -using namespace chip; - +namespace chip { +namespace Tracing { namespace Diagnostics { // This is a one time allocation for counters. It is not supposed to be freed. @@ -45,8 +45,8 @@ ESPDiagnosticCounter * ESPDiagnosticCounter::GetInstance(const char * label) VerifyOrDie(ptr != nullptr); ESPDiagnosticCounter * newInstance = new (ptr) ESPDiagnosticCounter(label); - newInstance->mNext = mHead; - mHead = newInstance; + newInstance->mNext = mHead; + mHead = newInstance; return newInstance; } @@ -61,8 +61,10 @@ void ESPDiagnosticCounter::ReportMetrics() CHIP_ERROR err = CHIP_NO_ERROR; Counter counter(label, instanceCount, esp_log_timestamp()); DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - err = diagnosticStorage.Store(counter); + err = diagnosticStorage.Store(counter); VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Counter diagnostic data")); } } // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index 4e58975999712f..6d8e23bd6d85e6 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -18,15 +18,15 @@ #pragma once +#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" #include #include #include #include #include -#include "tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h" - -using namespace chip::Tracing::Diagnostics; +namespace chip { +namespace Tracing { namespace Diagnostics { /** @@ -41,7 +41,7 @@ class ESPDiagnosticCounter { private: static ESPDiagnosticCounter * mHead; // head of the counter list - const char * label; // unique key ,it is used as a static string. + const char * label; // unique key ,it is used as a static string. int32_t instanceCount; ESPDiagnosticCounter * mNext; // pointer to point to the next entry in the list @@ -56,3 +56,5 @@ class ESPDiagnosticCounter }; } // namespace Diagnostics +} // namespace Tracing +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp index 98046d5523582b..d16430ce2f4ffe 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.cpp @@ -23,12 +23,13 @@ namespace chip { namespace Tracing { +using namespace chip::TLV; namespace Diagnostics { -DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize) - : mEndUserCircularBuffer(buffer, bufferSize) {} +DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize) : mEndUserCircularBuffer(buffer, bufferSize) {} -DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) { +DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) +{ static DiagnosticStorageImpl instance(buffer, bufferSize); return instance; } @@ -42,7 +43,6 @@ CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) CircularTLVWriter writer; writer.Init(mEndUserCircularBuffer); - // Start a TLV structure container (Anonymous) TLVType outerContainer; err = writer.StartContainer(AnonymousTag(), kTLVType_Structure, outerContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, @@ -98,21 +98,25 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", ErrorStr(err))); err = reader.Next(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err))); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, + ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err))); - // Check if the current element is a METRIC or TRACE container if ((reader.GetType() == kTLVType_Structure) && - (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) + (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || + reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) { + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) + { err = writer.CopyElement(reader); - if (err == CHIP_ERROR_BUFFER_TOO_SMALL) { + if (err == CHIP_ERROR_BUFFER_TOO_SMALL) + { ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); break; } VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); } - else { + else + { ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); break; } @@ -123,7 +127,6 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) reader.ExitContainer(outerReaderContainer); return CHIP_ERROR_WRONG_TLV_TYPE; } - // Exit the outer container err = reader.ExitContainer(outerReaderContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err))); @@ -136,11 +139,11 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) err = writer.EndContainer(outWriterContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to close outer container")); - // Finalize the writing process err = writer.Finalize(); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); payload.reduce_size(writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", writer.GetLengthWritten()); + ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", + writer.GetLengthWritten()); ChipLogProgress(DeviceLayer, "Retrieval successful"); return CHIP_NO_ERROR; } diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 1b21a6cb54ed5c..d35f745130f343 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h @@ -19,26 +19,23 @@ #pragma once #include "Diagnostics.h" -#include #include +#include namespace chip { namespace Tracing { namespace Diagnostics { -using namespace chip::Platform; -using chip::TLV::TLVType; class DiagnosticStorageImpl : public DiagnosticStorageInterface { public: + static DiagnosticStorageImpl & GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); - static DiagnosticStorageImpl& GetInstance(uint8_t * buffer = nullptr, size_t bufferSize = 0); - - DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; + DiagnosticStorageImpl(const DiagnosticStorageImpl &) = delete; DiagnosticStorageImpl & operator=(const DiagnosticStorageImpl &) = delete; CHIP_ERROR Store(DiagnosticEntry & diagnostic) override; - CHIP_ERROR Retrieve(MutableByteSpan &payload) override; + CHIP_ERROR Retrieve(MutableByteSpan & payload) override; bool IsEmptyBuffer(); @@ -47,7 +44,7 @@ class DiagnosticStorageImpl : public DiagnosticStorageInterface DiagnosticStorageImpl(); ~DiagnosticStorageImpl(); - TLVCircularBuffer mEndUserCircularBuffer; + chip::TLV::TLVCircularBuffer mEndUserCircularBuffer; }; } // namespace Diagnostics } // namespace Tracing diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp index 174eac0625f44f..f620abf65407a6 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp @@ -111,7 +111,7 @@ void ESP32Diagnostics::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info) {} void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) { DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); - CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR err = CHIP_NO_ERROR; switch (event.ValueType()) { case ValueType::kInt32: { @@ -146,24 +146,28 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ::Diagnostics::ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); + ESPDiagnosticCounter::GetInstance(label)->ReportMetrics(); } -void ESP32Diagnostics::TraceBegin(const char * label, const char * group) { +void ESP32Diagnostics::TraceBegin(const char * label, const char * group) +{ StoreDiagnostics(label, group); } -void ESP32Diagnostics::TraceEnd(const char * label, const char * group) { +void ESP32Diagnostics::TraceEnd(const char * label, const char * group) +{ StoreDiagnostics(label, group); } -void ESP32Diagnostics::TraceInstant(const char * label, const char * group) { +void ESP32Diagnostics::TraceInstant(const char * label, const char * group) +{ StoreDiagnostics(label, group); } -void ESP32Diagnostics::StoreDiagnostics(const char* label, const char* group) { - CHIP_ERROR err = CHIP_NO_ERROR; - HashValue hashValue = MurmurHash(group); +void ESP32Diagnostics::StoreDiagnostics(const char * label, const char * group) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + HashValue hashValue = MurmurHash(group); DiagnosticStorageImpl & diagnosticStorage = DiagnosticStorageImpl::GetInstance(); if (IsPermitted(hashValue)) { diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index 6188f7b6b9c95e..a94aa2d232d8a8 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -18,12 +18,11 @@ * limitations under the License. */ +#include #include #include -#include #include -#include - +#include #include namespace chip { @@ -35,14 +34,11 @@ namespace Diagnostics { class ESP32Diagnostics : public ::chip::Tracing::Backend { public: - ESP32Diagnostics(uint8_t *buffer, size_t buffer_size) - { - DiagnosticStorageImpl::GetInstance(buffer, buffer_size); - } + ESP32Diagnostics(uint8_t * buffer, size_t buffer_size) { DiagnosticStorageImpl::GetInstance(buffer, buffer_size); } // Deleted copy constructor and assignment operator to prevent copying - ESP32Diagnostics(const ESP32Diagnostics&) = delete; - ESP32Diagnostics& operator=(const ESP32Diagnostics&) = delete; + ESP32Diagnostics(const ESP32Diagnostics &) = delete; + ESP32Diagnostics & operator=(const ESP32Diagnostics &) = delete; void TraceBegin(const char * label, const char * group) override; @@ -60,7 +56,7 @@ class ESP32Diagnostics : public ::chip::Tracing::Backend void LogNodeDiscovered(NodeDiscoveredInfo &) override; void LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo &) override; void LogMetricEvent(const MetricEvent &) override; - void StoreDiagnostics(const char* label, const char* group); + void StoreDiagnostics(const char * label, const char * group); private: using ValueType = MetricEvent::Value::Type; @@ -68,4 +64,4 @@ class ESP32Diagnostics : public ::chip::Tracing::Backend } // namespace Diagnostics } // namespace Tracing -} // namespace chip \ No newline at end of file +} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index a824f4a0457559..c4b31dd34c84e5 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -19,63 +19,64 @@ #pragma once #include -#include #include +#include namespace chip { namespace Tracing { namespace Diagnostics { -using namespace chip::TLV; enum class DIAGNOSTICS_TAG { - METRIC = 0, - TRACE = 1, - COUNTER = 2, - LABEL = 3, - GROUP = 4, - VALUE = 5, - TIMESTAMP = 6 + METRIC = 0, + TRACE = 1, + COUNTER = 2, + LABEL = 3, + GROUP = 4, + VALUE = 5, + TIMESTAMP = 6 }; -class DiagnosticEntry { +class DiagnosticEntry +{ public: - virtual ~DiagnosticEntry() = default; - virtual CHIP_ERROR Encode(CircularTLVWriter &writer) = 0; + virtual ~DiagnosticEntry() = default; + virtual CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) = 0; }; -template -class Metric : public DiagnosticEntry { +template +class Metric : public DiagnosticEntry +{ public: - Metric(const char* label, T value, uint32_t timestamp) - : label_(label), value_(value), timestamp_(timestamp) {} + Metric(const char * label, T value, uint32_t timestamp) : label_(label), value_(value), timestamp_(timestamp) {} Metric() {} - const char* GetLabel() const { return label_; } + const char * GetLabel() const { return label_; } T GetValue() const { return value_; } uint32_t GetTimestamp() const { return timestamp_; } - CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + { CHIP_ERROR err = CHIP_NO_ERROR; - TLVType metricContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::METRIC), kTLVType_Structure, metricContainer); + chip::TLV::TLVType metricContainer; + err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::METRIC), chip::TLV::kTLVType_Structure, metricContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err))); // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", ErrorStr(err))); // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write LABEL for METRIC : %s", ErrorStr(err))); // VALUE - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::VALUE), value_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::VALUE), value_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write VALUE for METRIC : %s", ErrorStr(err))); @@ -87,41 +88,42 @@ class Metric : public DiagnosticEntry { } private: - const char* label_; + const char * label_; T value_; uint32_t timestamp_; }; -class Trace : public DiagnosticEntry { +class Trace : public DiagnosticEntry +{ public: - Trace(const char* label, const char* group, uint32_t timestamp) - : label_(label), group_(group), timestamp_(timestamp) {} + Trace(const char * label, const char * group, uint32_t timestamp) : label_(label), group_(group), timestamp_(timestamp) {} Trace() {} - const char* GetLabel() const { return label_; } + const char * GetLabel() const { return label_; } uint32_t GetTimestamp() const { return timestamp_; } - const char* GetGroup() const { return group_; } + const char * GetGroup() const { return group_; } - CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + { CHIP_ERROR err = CHIP_NO_ERROR; - TLVType traceContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::TRACE), kTLVType_Structure, traceContainer); + chip::TLV::TLVType traceContainer; + err = writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TRACE), chip::TLV::kTLVType_Structure, traceContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start TLV container for Trace: %s", ErrorStr(err))); // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for METRIC : %s", ErrorStr(err))); // GROUP - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::GROUP), group_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::GROUP), group_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write GROUP for TRACE : %s", ErrorStr(err))); // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write LABEL for TRACE : %s", ErrorStr(err))); @@ -133,15 +135,15 @@ class Trace : public DiagnosticEntry { } private: - const char* label_; - const char* group_; + const char * label_; + const char * group_; uint32_t timestamp_; }; -class Counter : public DiagnosticEntry { +class Counter : public DiagnosticEntry +{ public: - Counter(const char* label, uint32_t count, uint32_t timestamp) - : label_(label), count_(count), timestamp_(timestamp) {} + Counter(const char * label, uint32_t count, uint32_t timestamp) : label_(label), count_(count), timestamp_(timestamp) {} Counter() {} @@ -149,25 +151,27 @@ class Counter : public DiagnosticEntry { uint32_t GetTimestamp() const { return timestamp_; } - CHIP_ERROR Encode(CircularTLVWriter &writer) override { + CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer) override + { CHIP_ERROR err = CHIP_NO_ERROR; - TLVType counterContainer; - err = writer.StartContainer(ContextTag(DIAGNOSTICS_TAG::COUNTER), kTLVType_Structure, counterContainer); + chip::TLV::TLVType counterContainer; + err = + writer.StartContainer(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), chip::TLV::kTLVType_Structure, counterContainer); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start TLV container for Counter: %s", ErrorStr(err))); // TIMESTAMP - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::TIMESTAMP), timestamp_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write TIMESTAMP for COUNTER : %s", ErrorStr(err))); // LABEL - err = writer.PutString(ContextTag(DIAGNOSTICS_TAG::LABEL), label_); + err = writer.PutString(chip::TLV::ContextTag(DIAGNOSTICS_TAG::LABEL), label_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write LABEL for COUNTER : %s", ErrorStr(err))); // COUNT - err = writer.Put(ContextTag(DIAGNOSTICS_TAG::COUNTER), count_); + err = writer.Put(chip::TLV::ContextTag(DIAGNOSTICS_TAG::COUNTER), count_); VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to write VALUE for COUNTER : %s", ErrorStr(err))); @@ -179,18 +183,37 @@ class Counter : public DiagnosticEntry { } private: - const char* label_; + const char * label_; uint32_t count_; uint32_t timestamp_; }; -class DiagnosticStorageInterface { +/** + * @brief Interface for storing and retrieving diagnostic data. + */ +class DiagnosticStorageInterface +{ public: + /** + * @brief Virtual destructor for the interface. + */ virtual ~DiagnosticStorageInterface() = default; - virtual CHIP_ERROR Store(DiagnosticEntry& diagnostic) = 0; - - virtual CHIP_ERROR Retrieve(MutableByteSpan &payload) = 0; + /** + * @brief Stores a diagnostic entry. + * @param diagnostic Reference to a DiagnosticEntry object containing the + * diagnostic data to store. + * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. + */ + virtual CHIP_ERROR Store(DiagnosticEntry & diagnostic) = 0; + + /** + * @brief Retrieves diagnostic data as a payload. + * @param payload Reference to a MutableByteSpan where the retrieved + * diagnostic data will be stored. + * @return CHIP_ERROR Returns CHIP_NO_ERROR on success, or an appropriate error code on failure. + */ + virtual CHIP_ERROR Retrieve(MutableByteSpan & payload) = 0; }; } // namespace Diagnostics From d83dc21d2d330968b59e8efda97746ac85b833f5 Mon Sep 17 00:00:00 2001 From: mahesh 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 a6373398f36a71..4d84891e8d6a3a 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -1001,12 +1001,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" @@ -1027,7 +1025,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 a0046804e82167..f8553ffefaae31 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(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 310c5b4b124aeb..7f9eb3b5339f2a 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -21,7 +21,7 @@ #include #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include +#include #endif #include @@ -31,9 +31,7 @@ #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#define DIAGNOSTIC_BUFFER_SIZE CONFIG_END_USER_BUFFER_SIZE -static uint8_t endUserBuffer[DIAGNOSTIC_BUFFER_SIZE]; - +static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; using namespace chip::Tracing::Diagnostics; #endif diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 59df5c5345440d..39a4be0d726493 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -88,7 +88,7 @@ extern "C" void app_main() #endif #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - static ESP32Diagnostics diagnosticBackend(endUserBuffer, DIAGNOSTIC_BUFFER_SIZE); + static ESP32Diagnostics diagnosticBackend(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); Tracing::Register(diagnosticBackend); #endif From 168a5acce39822b04600faef516b59dbba5ba117 Mon Sep 17 00:00:00 2001 From: mahesh 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 #include +#define TLV_CLOSING_BYTES 4 + namespace chip { namespace Tracing { using namespace chip::TLV; @@ -105,7 +107,7 @@ CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < writer.GetRemainingFreeLength()) + if ((reader.GetLengthRead() - writer.GetLengthWritten()) < (writer.GetRemainingFreeLength() + TLV_CLOSING_BYTES)) { err = writer.CopyElement(reader); if (err == CHIP_ERROR_BUFFER_TOO_SMALL) 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 #include +#include 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 #include #include -#include +#include #include #include diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index c4b31dd34c84e5..256d73d041e4f1 100644 --- a/src/tracing/esp32_diagnostic_trace/Diagnostics.h +++ b/src/tracing/esp32_diagnostic_trace/Diagnostics.h @@ -17,7 +17,6 @@ */ #pragma once - #include #include #include From a98b1f8db9ca10f09333844b646ae50d908cdefb Mon Sep 17 00:00:00 2001 From: mahesh 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 2e79c6f8f9cfa9..47406d15e683b1 100644 --- a/scripts/tools/check_includes_config.py +++ b/scripts/tools/check_includes_config.py @@ -167,6 +167,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 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 #include #include +#include #include 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 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 51b07727f1c51ba8f5786a9dae6a9106545ad576 Mon Sep 17 00:00:00 2001 From: mahesh 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 #include #include @@ -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 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 metric(event.key(), event.ValueInt32(), esp_log_timestamp()); - err = diagnosticStorage.Store(metric); + err = mStorageInstance.Store(metric); } break; case ValueType::kUInt32: { ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); Metric metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); - err = diagnosticStorage.Store(metric); + err = mStorageInstance.Store(metric); } break; @@ -146,7 +145,7 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event) void ESP32Diagnostics::TraceCounter(const char * label) { - ESPDiagnosticCounter::GetInstance(label).ReportMetrics(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 3338979ceba160b409f48f14ef94ecac7a11086a Mon Sep 17 00:00:00 2001 From: mahesh 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 f8553ffefaae31..d46843a41f6ebd 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(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 321c392b9c35f4b37326bbd4e7871b5a30a5e1a7 Mon Sep 17 00:00:00 2001 From: mahesh 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 d46843a41f6ebd..2130072c5958ce 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 5a80c25b3d45606a96ecbce3c1d12b78b78efe3e Mon Sep 17 00:00:00 2001 From: mahesh 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 #include #include +#include namespace chip { namespace Tracing { From ec041b9c9dab4263dc10de13d347995ef10e392f Mon Sep 17 00:00:00 2001 From: mahesh 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 2130072c5958ce..299d2685c615b6 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(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 #include #include -#include 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 -#include -#include - -#define TLV_CLOSING_BYTES 4 - -namespace chip { -namespace Tracing { -using namespace chip::TLV; - -namespace Diagnostics { -DiagnosticStorageImpl::DiagnosticStorageImpl(uint8_t * buffer, size_t bufferSize) : mEndUserCircularBuffer(buffer, bufferSize) {} - -DiagnosticStorageImpl & DiagnosticStorageImpl::GetInstance(uint8_t * buffer, size_t bufferSize) -{ - static DiagnosticStorageImpl instance(buffer, bufferSize); - return instance; -} - -DiagnosticStorageImpl::~DiagnosticStorageImpl() {} - -CHIP_ERROR DiagnosticStorageImpl::Store(DiagnosticEntry & diagnostic) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - - CircularTLVWriter writer; - writer.Init(mEndUserCircularBuffer); - - TLVType outerContainer; - err = writer.StartContainer(AnonymousTag(), kTLVType_Structure, outerContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to start TLV container for metric : %s", ErrorStr(err))); - - err = diagnostic.Encode(writer); - if (err != CHIP_NO_ERROR) - { - ChipLogError(DeviceLayer, "Failed to encode diagnostic data : %s", ErrorStr(err)); - err = CHIP_ERROR_INVALID_ARGUMENT; - writer.EndContainer(outerContainer); - writer.Finalize(); - return err; - } - err = writer.EndContainer(outerContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to end TLV container for metric : %s", ErrorStr(err))); - - err = writer.Finalize(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); - return CHIP_NO_ERROR; -} - -CHIP_ERROR DiagnosticStorageImpl::Retrieve(MutableByteSpan & payload) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - CircularTLVReader reader; - reader.Init(mEndUserCircularBuffer); - - TLVWriter writer; - writer.Init(payload); - - TLVType outWriterContainer; - err = writer.StartContainer(AnonymousTag(), kTLVType_List, outWriterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to start container")); - - while (true) - { - err = reader.Next(); - if (err == CHIP_ERROR_END_OF_TLV) - { - ChipLogProgress(DeviceLayer, "No more data to read"); - break; - } - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to read next TLV element: %s", ErrorStr(err))); - - if (reader.GetType() == kTLVType_Structure && reader.GetTag() == AnonymousTag()) - { - TLVType outerReaderContainer; - err = reader.EnterContainer(outerReaderContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to enter outer TLV container: %s", ErrorStr(err))); - - err = reader.Next(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to read next TLV element in outer container: %s", ErrorStr(err))); - - if ((reader.GetType() == kTLVType_Structure) && - (reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::METRIC) || reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::TRACE) || - reader.GetTag() == ContextTag(DIAGNOSTICS_TAG::COUNTER))) - { - if ((reader.GetLengthRead() - writer.GetLengthWritten()) < (writer.GetRemainingFreeLength() + TLV_CLOSING_BYTES)) - { - err = writer.CopyElement(reader); - if (err == CHIP_ERROR_BUFFER_TOO_SMALL) - { - ChipLogProgress(DeviceLayer, "Buffer too small to occupy current element"); - break; - } - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to copy TLV element")); - } - else - { - ChipLogProgress(DeviceLayer, "Buffer too small to occupy current TLV"); - break; - } - } - else - { - ChipLogError(DeviceLayer, "Unexpected TLV element in outer container"); - reader.ExitContainer(outerReaderContainer); - return CHIP_ERROR_WRONG_TLV_TYPE; - } - err = reader.ExitContainer(outerReaderContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, - ChipLogError(DeviceLayer, "Failed to exit outer TLV container: %s", ErrorStr(err))); - } - else - { - ChipLogError(DeviceLayer, "Unexpected TLV element type or tag in outer container"); - } - } - - err = writer.EndContainer(outWriterContainer); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to close outer container")); - err = writer.Finalize(); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to finalize TLV writing")); - payload.reduce_size(writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "---------------Total written bytes successfully : %ld----------------\n", - writer.GetLengthWritten()); - ChipLogProgress(DeviceLayer, "Retrieval successful"); - return CHIP_NO_ERROR; -} - -bool DiagnosticStorageImpl::IsEmptyBuffer() -{ - return mEndUserCircularBuffer.DataLength() == 0; -} - -uint32_t DiagnosticStorageImpl::GetDataSize() -{ - return mEndUserCircularBuffer.DataLength(); -} -} // namespace Diagnostics -} // namespace Tracing -} // namespace chip diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h b/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h index 647a552495b9f2..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 0befb4f4ff4921d82913a7bd987e4f59ad5c401f Mon Sep 17 00:00:00 2001 From: mahesh 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 #include +#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 7e88a79c4af8690834e66abb8358c9af88ee4ca2 Mon Sep 17 00:00:00 2001 From: mahesh 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 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 #include -#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 metric(event.key(), event.ValueInt32(), esp_log_timestamp()); + Diagnostic metric(event.key(), event.ValueInt32(), esp_log_timestamp()); err = mStorageInstance.Store(metric); } break; case ValueType::kUInt32: { ESP_LOGI("mtr", "The value of %s is %lu ", event.key(), event.ValueUInt32()); - Metric metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); + Diagnostic metric(event.key(), event.ValueUInt32(), esp_log_timestamp()); err = mStorageInstance.Store(metric); } break; @@ -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 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 -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) + { + 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 4ce5d090267cc7da46aeb79a318acf8ba9124859 Mon Sep 17 00:00:00 2001 From: mahesh 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 #include +#include namespace chip { namespace Tracing { @@ -25,7 +25,7 @@ namespace Diagnostics { std::map ESPDiagnosticCounter::mCounterList; -void ESPDiagnosticCounter::CountInit(const char * label) +void ESPDiagnosticCounter::IncreaseCount(const char * label) { if (mCounterList.find(label) != mCounterList.end()) { diff --git a/src/tracing/esp32_diagnostic_trace/Counter.h b/src/tracing/esp32_diagnostic_trace/Counter.h index ea882b3487d3b3..38a83b41234cfb 100644 --- a/src/tracing/esp32_diagnostic_trace/Counter.h +++ b/src/tracing/esp32_diagnostic_trace/Counter.h @@ -19,12 +19,7 @@ #pragma once #include "tracing/esp32_diagnostic_trace/Diagnostics.h" -#include -#include -#include -#include #include -#include namespace chip { namespace Tracing { @@ -44,7 +39,7 @@ class ESPDiagnosticCounter static ESPDiagnosticCounter & GetInstance(const char * label) { static ESPDiagnosticCounter instance; - CountInit(label); + IncreaseCount(label); return instance; } @@ -55,7 +50,7 @@ class ESPDiagnosticCounter private: ESPDiagnosticCounter() {} static std::map mCounterList; - static void CountInit(const char * label); + static void IncreaseCount(const char * label); }; } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticStorageManager.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 -#include #include #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 #include -#include #include -#include -#include #include #include -#include 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 trace(label, group, esp_log_timestamp()); - err = mStorageInstance.Store(trace); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); - } + Diagnostic trace(label, group, esp_log_timestamp()); + VerifyOrReturn(mStorageInstance.Store(trace) == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to store Trace Diagnostic data")); } } // namespace Diagnostics diff --git a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h index 5ab60a31457447..54cabef7fd55bb 100644 --- a/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h +++ b/src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h @@ -18,8 +18,6 @@ * limitations under the License. */ -#include -#include #include #include #include diff --git a/src/tracing/esp32_diagnostic_trace/Diagnostics.h b/src/tracing/esp32_diagnostic_trace/Diagnostics.h index 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 + #include -#include 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) { - 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 48ad096e085a079e120a9ace34cdbcdefec534e0 Mon Sep 17 00:00:00 2001 From: mahesh 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 #include +#include 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 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 e8db459a646a5703772cf76ed27d10922b3ad5ed Mon Sep 17 00:00:00 2001 From: mahesh 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 299d2685c615b6..5b01902a3ee536 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(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(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 7f9eb3b5339f2a..85ca647e84f2d0 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -20,10 +20,6 @@ #include -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -#include -#endif - #include #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) @@ -31,9 +27,10 @@ #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; using namespace chip::Tracing::Diagnostics; -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace chip { namespace app { diff --git a/examples/temperature-measurement-app/esp32/main/main.cpp b/examples/temperature-measurement-app/esp32/main/main.cpp index 63673094b35b87..35a385dc3ec74e 100644 --- a/examples/temperature-measurement-app/esp32/main/main.cpp +++ b/examples/temperature-measurement-app/esp32/main/main.cpp @@ -54,7 +54,7 @@ #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include -#endif +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE namespace { #if CONFIG_ENABLE_ESP32_FACTORY_DATA_PROVIDER @@ -79,6 +79,12 @@ static AppDeviceCallbacks EchoCallbacks; static void InitServer(intptr_t context) { Esp32AppServer::Init(); // Init ZCL Data Model and CHIP App Server AND Initialize device attestation config +#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); + diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); + static ESP32Diagnostics diagnosticBackend(diagnosticStorage); + Tracing::Register(diagnosticBackend); +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE } extern "C" void app_main() @@ -86,14 +92,6 @@ extern "C" void app_main() #if CONFIG_ENABLE_PW_RPC chip::rpc::Init(); #endif - -#if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE - CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); - diagnosticStorage.Init(endUserBuffer, CONFIG_END_USER_BUFFER_SIZE); - static ESP32Diagnostics diagnosticBackend(diagnosticStorage); - Tracing::Register(diagnosticBackend); -#endif - ESP_LOGI(TAG, "Temperature sensor!"); // Initialize the ESP NVS layer. From 14cceaed6c83ac735fbca594865ec00b5f9db688 Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 28 Nov 2024 11:54:22 +0530 Subject: [PATCH 18/22] esp32: add ble related diagnostics to platform --- src/platform/ESP32/BUILD.gn | 1 + src/platform/ESP32/nimble/BLEManagerImpl.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/src/platform/ESP32/BUILD.gn b/src/platform/ESP32/BUILD.gn index 1ffae2593e8b9d..e9d70c5f5936e0 100644 --- a/src/platform/ESP32/BUILD.gn +++ b/src/platform/ESP32/BUILD.gn @@ -68,6 +68,7 @@ static_library("ESP32") { "${chip_root}/src/lib/dnssd:platform_header", "${chip_root}/src/platform/logging:headers", "${chip_root}/src/setup_payload", + "${chip_root}/src/tracing:macros", ] public = [ diff --git a/src/platform/ESP32/nimble/BLEManagerImpl.cpp b/src/platform/ESP32/nimble/BLEManagerImpl.cpp index 176fd81ee9f2c9..348766d85ec966 100644 --- a/src/platform/ESP32/nimble/BLEManagerImpl.cpp +++ b/src/platform/ESP32/nimble/BLEManagerImpl.cpp @@ -69,6 +69,8 @@ #include "nimble/nimble_port_freertos.h" #include "services/gap/ble_svc_gap.h" #include "services/gatt/ble_svc_gatt.h" +#include +#include #define MAX_ADV_DATA_LEN 31 #define CHIP_ADV_DATA_TYPE_FLAGS 0x01 @@ -209,6 +211,7 @@ void BLEManagerImpl::ConnectDevice(const ble_addr_t & addr, uint16_t timeout) CHIP_ERROR BLEManagerImpl::_Init() { CHIP_ERROR err; + MATTER_TRACE_INSTANT("BLE_mode", "Nimble"); // Initialize the Chip BleLayer. #ifdef CONFIG_ENABLE_ESP32_BLE_CONTROLLER @@ -244,7 +247,10 @@ CHIP_ERROR BLEManagerImpl::_Init() void BLEManagerImpl::_Shutdown() { + constexpr chip::Tracing::MetricKey kMetricBluetoothConnections = "ble_con_count"; + MATTER_LOG_METRIC(kMetricBluetoothConnections, mNumGAPCons); CancelBleAdvTimeoutTimer(); + MATTER_TRACE_INSTANT("BLE", "Shutdown"); BleLayer::Shutdown(); @@ -280,6 +286,7 @@ void BLEManagerImpl::BleAdvTimeoutHandler(System::Layer *, void *) if (BLEMgrImpl().mFlags.Has(Flags::kFastAdvertisingEnabled)) { ChipLogProgress(DeviceLayer, "bleAdv Timeout : Start slow advertisement"); + MATTER_TRACE_INSTANT("BLE_adv", "Slow"); BLEMgrImpl().mFlags.Set(Flags::kFastAdvertisingEnabled, 0); BLEMgrImpl().mFlags.Set(Flags::kAdvertisingRefreshNeeded, 1); @@ -292,6 +299,7 @@ void BLEManagerImpl::BleAdvTimeoutHandler(System::Layer *, void *) else { ChipLogProgress(DeviceLayer, "bleAdv Timeout : Start extended advertisement"); + MATTER_TRACE_INSTANT("BLE_adv", "Extended"); BLEMgrImpl().mFlags.Set(Flags::kAdvertising); BLEMgrImpl().mFlags.Set(Flags::kExtAdvertisingEnabled); BLEMgr().SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising); @@ -1011,6 +1019,7 @@ void BLEManagerImpl::ClaimBLEMemory(System::Layer *, void *) CHIP_ERROR BLEManagerImpl::DeinitBLE() { esp_err_t err = ESP_OK; + MATTER_TRACE_INSTANT("BLE", "Deinit"); VerifyOrReturnError(ble_hs_is_enabled(), CHIP_ERROR_INCORRECT_STATE, ChipLogProgress(DeviceLayer, "BLE already deinited")); VerifyOrReturnError(0 == nimble_port_stop(), MapBLEError(ESP_FAIL), ChipLogError(DeviceLayer, "nimble_port_stop() failed")); @@ -1788,6 +1797,7 @@ CHIP_ERROR BLEManagerImpl::ConfigureBle(uint32_t aAdapterId, bool aIsCentral) mIsCentral = aIsCentral; if (mIsCentral) { + MATTER_TRACE_INSTANT("BLE_role", "Central"); int rc = peer_init(MYNEWT_VAL(BLE_MAX_CONNECTIONS), 64, 64, 64); assert(rc == 0); } From 792af96fd7462989df14b69f65b1ea3a51f9a5cf Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 28 Nov 2024 11:58:32 +0530 Subject: [PATCH 19/22] esp32: add wifi related diagnostics to platform --- src/platform/ESP32/ConnectivityManagerImpl.h | 4 + .../ESP32/ConnectivityManagerImpl_WiFi.cpp | 134 +++++++++++++++++- src/platform/ESP32/ESP32Utils.cpp | 51 +++++++ src/platform/ESP32/ESP32Utils.h | 1 + 4 files changed, 189 insertions(+), 1 deletion(-) diff --git a/src/platform/ESP32/ConnectivityManagerImpl.h b/src/platform/ESP32/ConnectivityManagerImpl.h index e2940173b135ce..2e0758c8d46243 100644 --- a/src/platform/ESP32/ConnectivityManagerImpl.h +++ b/src/platform/ESP32/ConnectivityManagerImpl.h @@ -139,6 +139,9 @@ class ConnectivityManagerImpl final : public ConnectivityManager, void OnStationDisconnected(void); void ChangeWiFiStationState(WiFiStationState newState); static void DriveStationState(::chip::System::Layer * aLayer, void * aAppState); +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + void LogWiFiInfo(void); +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #if CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP WiFiAPMode _GetWiFiAPMode(void); @@ -176,6 +179,7 @@ class ConnectivityManagerImpl final : public ConnectivityManager, // ===== Members for internal use by the following friends. + friend ConnectivityManager & ConnectivityMgr(void); friend ConnectivityManagerImpl & ConnectivityMgrImpl(void); diff --git a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp index 380edfd3e7adc9..fc83a8459f20ba 100644 --- a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp +++ b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp @@ -41,6 +41,9 @@ #include #include +#include +#include + #if CHIP_DEVICE_CONFIG_ENABLE_WIFI using namespace ::chip; @@ -48,6 +51,14 @@ using namespace ::chip::Inet; using namespace ::chip::System; using chip::DeviceLayer::Internal::ESP32Utils; +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +static int max_rssi = -100; +static int current_rssi = 1; +static int min_rssi = 0; +constexpr chip::Tracing::MetricKey kMetricWiFiMaxRSSI = "max_rssi"; +constexpr chip::Tracing::MetricKey kMetricWiFiMinRSSI = "min_rssi"; +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + namespace chip { namespace DeviceLayer { @@ -554,6 +565,20 @@ void ConnectivityManagerImpl::_OnWiFiStationProvisionChange() void ConnectivityManagerImpl::DriveStationState() { +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + esp_wifi_sta_get_rssi(¤t_rssi); + if (current_rssi != 1 && current_rssi > max_rssi) + { + max_rssi = current_rssi; + MATTER_LOG_METRIC(kMetricWiFiMaxRSSI, static_cast(max_rssi)); + } + else if (current_rssi < min_rssi) + { + min_rssi = current_rssi; + MATTER_LOG_METRIC(kMetricWiFiMinRSSI, static_cast(min_rssi)); + } +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + bool stationConnected; // Refresh the current station mode. Specifically, this reads the ESP auto_connect flag, @@ -598,6 +623,7 @@ void ConnectivityManagerImpl::DriveStationState() if (err != ESP_OK) { ChipLogError(DeviceLayer, "esp_wifi_disconnect() failed: %s", esp_err_to_name(err)); + MATTER_TRACE_INSTANT("WiFi_Error", esp_err_to_name(err)); return; } @@ -641,12 +667,14 @@ void ConnectivityManagerImpl::DriveStationState() { ChipLogProgress(DeviceLayer, "Attempting to connect WiFi station interface"); esp_err_t err = esp_wifi_connect(); + MATTER_TRACE_COUNTER("WiFi_con_attempt"); if (err != ESP_OK) { ChipLogError(DeviceLayer, "esp_wifi_connect() failed: %s", esp_err_to_name(err)); + MATTER_TRACE_INSTANT("WiFi_Error", esp_err_to_name(err)); + MATTER_TRACE_COUNTER("WiFi_fail_attempt"); return; } - ChangeWiFiStationState(kWiFiStationState_Connecting); } @@ -670,6 +698,10 @@ void ConnectivityManagerImpl::DriveStationState() void ConnectivityManagerImpl::OnStationConnected() { + MATTER_TRACE_INSTANT("WiFi", "Station Connected"); +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + LogWiFiInfo(); +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE // Assign an IPv6 link local address to the station interface. esp_err_t err = esp_netif_create_ip6_linklocal(esp_netif_get_handle_from_ifkey(ESP32Utils::kDefaultWiFiStationNetifKey)); if (err != ESP_OK) @@ -698,6 +730,8 @@ void ConnectivityManagerImpl::OnStationConnected() void ConnectivityManagerImpl::OnStationDisconnected() { + MATTER_TRACE_INSTANT("WiFi", "Station Disconnected"); + // TODO Invoke WARM to perform actions that occur when the WiFi station interface goes down. // Alert other components of the new state. @@ -707,6 +741,7 @@ void ConnectivityManagerImpl::OnStationDisconnected() PlatformMgr().PostEventOrDie(&event); WiFiDiagnosticsDelegate * delegate = GetDiagnosticDataProvider().GetWiFiDiagnosticsDelegate(); uint16_t reason = NetworkCommissioning::ESPWiFiDriver::GetInstance().GetLastDisconnectReason(); + MATTER_TRACE_INSTANT("WiFi_disconnect_Reason", ESP32Utils::WiFiDisconnectReasonToStr(reason)); uint8_t associationFailureCause = chip::to_underlying(chip::app::Clusters::WiFiNetworkDiagnostics::AssociationFailureCauseEnum::kUnknown); @@ -1153,6 +1188,103 @@ CHIP_ERROR ConnectivityManagerImpl::_SetPollingInterval(System::Clock::Milliseco } #endif // CHIP_CONFIG_ENABLE_ICD_SERVER +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +void ConnectivityManagerImpl::LogWiFiInfo() +{ + esp_err_t err; + uint8_t protocol_bitmap; + wifi_bandwidth_t bandwidth; + wifi_ps_type_t ps_type; + wifi_second_chan_t secondary_channel; + + // Get AP protocol + err = esp_wifi_get_protocol(WIFI_IF_STA, &protocol_bitmap); + if (err == ESP_OK) + { + const char * protocol; + if (protocol_bitmap & WIFI_PROTOCOL_11B) + protocol = "11b"; + else if (protocol_bitmap & WIFI_PROTOCOL_11G) + protocol = "11g"; + else if (protocol_bitmap & WIFI_PROTOCOL_11N) + protocol = "11n"; + else + protocol = "Unknown"; + MATTER_TRACE_INSTANT("AP_protocol", protocol); + } + else + { + ESP_LOGE("LogWiFiInfo", "Failed to get AP protocol: %d", err); + } + + // Get bandwidth + err = esp_wifi_get_bandwidth(WIFI_IF_STA, &bandwidth); + if (err == ESP_OK) + { + const char * bandwidth_str = (bandwidth == WIFI_BW_HT20) ? "HT20" : "HT40"; + MATTER_TRACE_INSTANT("Bandwidth", bandwidth_str); + } + else + { + ESP_LOGE("LogWiFiInfo", "Failed to get bandwidth: %d", err); + } + + // Get power save type + err = esp_wifi_get_ps(&ps_type); + if (err == ESP_OK) + { + const char * ps_type_str; + switch (ps_type) + { + case WIFI_PS_NONE: + ps_type_str = "No power save"; + break; + case WIFI_PS_MIN_MODEM: + ps_type_str = "Min modem power save"; + break; + case WIFI_PS_MAX_MODEM: + ps_type_str = "Max modem power save"; + break; + default: + ps_type_str = "Unknown"; + break; + } + MATTER_TRACE_INSTANT("Power_save_type", ps_type_str); + } + else + { + ESP_LOGE("LogWiFiInfo", "Failed to get power save type: %d", err); + } + + // Get secondary channel + err = esp_wifi_get_channel(NULL, &secondary_channel); + if (err == ESP_OK) + { + const char * second_ch; + switch (secondary_channel) + { + case WIFI_SECOND_CHAN_NONE: + second_ch = "HT20: No channel"; + break; + case WIFI_SECOND_CHAN_ABOVE: + second_ch = "HT40: above primary"; + break; + case WIFI_SECOND_CHAN_BELOW: + second_ch = "HT40: below primary"; + break; + default: + second_ch = "Unknown"; + break; + } + MATTER_TRACE_INSTANT("Secondary_channel", second_ch); + } + else + { + ESP_LOGE("LogWiFiInfo", "Failed to get secondary channel: %d", err); + } +} +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + } // namespace DeviceLayer } // namespace chip diff --git a/src/platform/ESP32/ESP32Utils.cpp b/src/platform/ESP32/ESP32Utils.cpp index e66007a0a85c24..3e83202bd44b80 100644 --- a/src/platform/ESP32/ESP32Utils.cpp +++ b/src/platform/ESP32/ESP32Utils.cpp @@ -221,6 +221,57 @@ const char * ESP32Utils::WiFiModeToStr(wifi_mode_t wifiMode) } } +const char * ESP32Utils::WiFiDisconnectReasonToStr(uint16_t reason) +{ + switch (reason) + { + case WIFI_REASON_ASSOC_TOOMANY: + return "WIFI_REASON_ASSOC_TOOMANY"; + case WIFI_REASON_NOT_ASSOCED: + return "WIFI_REASON_NOT_ASSOCED"; + case WIFI_REASON_ASSOC_NOT_AUTHED: + return "WIFI_REASON_ASSOC_NOT_AUTHED"; + case WIFI_REASON_4WAY_HANDSHAKE_TIMEOUT: + return "WIFI_REASON_4WAY_HANDSHAKE_TIMEOUT"; + case WIFI_REASON_GROUP_CIPHER_INVALID: + return "WIFI_REASON_GROUP_CIPHER_INVALID"; + case WIFI_REASON_UNSUPP_RSN_IE_VERSION: + return "WIFI_REASON_UNSUPP_RSN_IE_VERSION"; + case WIFI_REASON_AKMP_INVALID: + return "WIFI_REASON_AKMP_INVALID"; + case WIFI_REASON_CIPHER_SUITE_REJECTED: + return "WIFI_REASON_CIPHER_SUITE_REJECTED"; + case WIFI_REASON_PAIRWISE_CIPHER_INVALID: + return "WIFI_REASON_PAIRWISE_CIPHER_INVALID"; + case WIFI_REASON_NOT_AUTHED: + return "WIFI_REASON_NOT_AUTHED"; + case WIFI_REASON_MIC_FAILURE: + return "WIFI_REASON_MIC_FAILURE"; + case WIFI_REASON_IE_IN_4WAY_DIFFERS: + return "WIFI_REASON_IE_IN_4WAY_DIFFERS"; + case WIFI_REASON_INVALID_RSN_IE_CAP: + return "WIFI_REASON_INVALID_RSN_IE_CAP"; + case WIFI_REASON_INVALID_PMKID: + return "WIFI_REASON_INVALID_PMKID"; + case WIFI_REASON_802_1X_AUTH_FAILED: + return "WIFI_REASON_802_1X_AUTH_FAILED"; + case WIFI_REASON_NO_AP_FOUND: + return "WIFI_REASON_NO_AP_FOUND"; + case WIFI_REASON_BEACON_TIMEOUT: + return "WIFI_REASON_BEACON_TIMEOUT"; + case WIFI_REASON_AUTH_EXPIRE: + return "WIFI_REASON_AUTH_EXPIRE"; + case WIFI_REASON_AUTH_LEAVE: + return "WIFI_REASON_AUTH_LEAVE"; + case WIFI_REASON_ASSOC_LEAVE: + return "WIFI_REASON_ASSOC_LEAVE"; + case WIFI_REASON_ASSOC_EXPIRE: + return "WIFI_REASON_ASSOC_EXPIRE"; + default: + return "Unknown Reason"; + } +} + struct netif * ESP32Utils::GetStationNetif(void) { return GetNetif(kDefaultWiFiStationNetifKey); diff --git a/src/platform/ESP32/ESP32Utils.h b/src/platform/ESP32/ESP32Utils.h index b82734c540da20..24871a2432c701 100644 --- a/src/platform/ESP32/ESP32Utils.h +++ b/src/platform/ESP32/ESP32Utils.h @@ -38,6 +38,7 @@ class ESP32Utils static CHIP_ERROR SetAPMode(bool enabled); static int OrderScanResultsByRSSI(const void * _res1, const void * _res2); static const char * WiFiModeToStr(wifi_mode_t wifiMode); + static const char * WiFiDisconnectReasonToStr(uint16_t reason); static struct netif * GetNetif(const char * ifKey); static struct netif * GetStationNetif(void); static bool IsInterfaceUp(const char * ifKey); From fd99413c026ba55b0e651ba736276303c61aa103 Mon Sep 17 00:00:00 2001 From: mahesh Date: Mon, 9 Dec 2024 15:41:49 +0530 Subject: [PATCH 20/22] esp32: add heap related diagnostics to platform --- src/platform/ESP32/ESP32Utils.cpp | 75 +++++++++++++++++++++++++++++++ src/platform/ESP32/ESP32Utils.h | 4 ++ 2 files changed, 79 insertions(+) diff --git a/src/platform/ESP32/ESP32Utils.cpp b/src/platform/ESP32/ESP32Utils.cpp index 3e83202bd44b80..a62fd460c7b14c 100644 --- a/src/platform/ESP32/ESP32Utils.cpp +++ b/src/platform/ESP32/ESP32Utils.cpp @@ -35,8 +35,33 @@ #include "esp_wifi.h" #include "nvs.h" +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +#include "esp_heap_caps.h" +#include +#include +#include + +using namespace chip::DeviceLayer; +using namespace chip::Tracing; + +// Heap Diagnostics (internal) +constexpr MetricKey kMetricHeapInternalFree = "internal_free"; +constexpr MetricKey kMetricHeapInternalMinFree = "internal_min_free"; +constexpr MetricKey kMetricHeapInternalLargestBlock = "internal_largest_free"; + +// Heap Diagnostics (external) +constexpr MetricKey kMetricHeapExternalFree = "external_free"; +constexpr MetricKey kMetricHeapExternalMinFree = "external_min_free"; +constexpr MetricKey kMetricHeapExternalLargestBlock = "external_largest_block"; +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + using namespace ::chip::DeviceLayer::Internal; using chip::DeviceLayer::Internal::DeviceNetworkInfo; + +#ifndef CONFIG_HEAP_LOG_INTERVAL +#define CONFIG_HEAP_LOG_INTERVAL 300000 +#endif // CONFIG_HEAP_LOG_INTERVAL + #if CHIP_DEVICE_CONFIG_ENABLE_WIFI CHIP_ERROR ESP32Utils::IsAPEnabled(bool & apEnabled) { @@ -422,6 +447,56 @@ CHIP_ERROR ESP32Utils::InitWiFiStack(void) } #endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE +void LogHeapDataCallback(chip::System::Layer * systemLayer, void * appState) +{ + // Internal RAM (default heap) + uint32_t internal_free = heap_caps_get_free_size(MALLOC_CAP_INTERNAL); + uint32_t internal_largest_free_block = heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL); + uint32_t internal_min_free = heap_caps_get_minimum_free_size(MALLOC_CAP_INTERNAL); + + MATTER_LOG_METRIC(kMetricHeapInternalFree, internal_free); + MATTER_LOG_METRIC(kMetricHeapInternalMinFree, internal_min_free); + MATTER_LOG_METRIC(kMetricHeapInternalLargestBlock, internal_largest_free_block); + +#ifdef CONFIG_SPIRAM + // External RAM (if PSRAM is enabled) + uint32_t external_free = heap_caps_get_free_size(MALLOC_CAP_SPIRAM); + uint32_t external_largest_free_block = heap_caps_get_largest_free_block(MALLOC_CAP_SPIRAM); + uint32_t external_min_free = heap_caps_get_minimum_free_size(MALLOC_CAP_SPIRAM); + + MATTER_LOG_METRIC(kMetricHeapExternalFree, external_free); + MATTER_LOG_METRIC(kMetricHeapExternalMinFree, external_min_free); + MATTER_LOG_METRIC(kMetricHeapExternalLargestBlock, external_largest_free_block); +#endif + + // Reschedule the timer for the next interval + systemLayer->StartTimer(chip::System::Clock::Milliseconds32(CONFIG_HEAP_LOG_INTERVAL), LogHeapDataCallback, nullptr); +} + +void FailedAllocCallback(size_t size, uint32_t caps, const char * function_name) +{ + MATTER_TRACE_COUNTER("Failed_memory_allocations"); + ESP_LOGE(TAG, "Memory allocation failed!"); + ESP_LOGE(TAG, "Requested Size: %zu, Caps: %lu, Function: %s", size, caps, function_name); +} + +// Function to initialize and start periodic heap logging +void ESP32Utils::LogHeapInfo() +{ + esp_err_t err = heap_caps_register_failed_alloc_callback(FailedAllocCallback); + if (err != ESP_OK) + { + ESP_LOGE(TAG, "Failed to register callback. Error: 0x%08x", err); + } + // Log immediately + LogHeapDataCallback(&SystemLayer(), nullptr); + + // Start the periodic logging using SystemLayer + SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(CONFIG_HEAP_LOG_INTERVAL), LogHeapDataCallback, nullptr); +} +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + struct netif * ESP32Utils::GetNetif(const char * ifKey) { struct netif * netif = NULL; diff --git a/src/platform/ESP32/ESP32Utils.h b/src/platform/ESP32/ESP32Utils.h index 24871a2432c701..294cf76d1e9c6d 100644 --- a/src/platform/ESP32/ESP32Utils.h +++ b/src/platform/ESP32/ESP32Utils.h @@ -49,6 +49,10 @@ class ESP32Utils static CHIP_ERROR ClearWiFiStationProvision(void); static CHIP_ERROR InitWiFiStack(void); +#ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + static void LogHeapInfo(); +#endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE + static CHIP_ERROR MapError(esp_err_t error); static void RegisterESP32ErrorFormatter(); static bool FormatError(char * buf, uint16_t bufSize, CHIP_ERROR err); From a3e478dbe0b0920385123411e7786a8c5dea0e40 Mon Sep 17 00:00:00 2001 From: mahesh Date: Tue, 17 Dec 2024 16:24:58 +0530 Subject: [PATCH 21/22] esp32: add freertos task related diagnostics to platform --- src/platform/ESP32/ESP32Utils.cpp | 58 ++++++++++++++++++++++++++++++- src/platform/ESP32/ESP32Utils.h | 1 + 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/platform/ESP32/ESP32Utils.cpp b/src/platform/ESP32/ESP32Utils.cpp index a62fd460c7b14c..f2bde483dbad25 100644 --- a/src/platform/ESP32/ESP32Utils.cpp +++ b/src/platform/ESP32/ESP32Utils.cpp @@ -37,6 +37,8 @@ #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE #include "esp_heap_caps.h" +#include +#include #include #include #include @@ -53,6 +55,9 @@ constexpr MetricKey kMetricHeapInternalLargestBlock = "internal_largest_free"; constexpr MetricKey kMetricHeapExternalFree = "external_free"; constexpr MetricKey kMetricHeapExternalMinFree = "external_min_free"; constexpr MetricKey kMetricHeapExternalLargestBlock = "external_largest_block"; + +// Task runtime +constexpr MetricKey kMetricTaskName = "runtime"; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE using namespace ::chip::DeviceLayer::Internal; @@ -489,12 +494,63 @@ void ESP32Utils::LogHeapInfo() { ESP_LOGE(TAG, "Failed to register callback. Error: 0x%08x", err); } - // Log immediately LogHeapDataCallback(&SystemLayer(), nullptr); // Start the periodic logging using SystemLayer SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(CONFIG_HEAP_LOG_INTERVAL), LogHeapDataCallback, nullptr); } + + +const char * StateToString(eTaskState state) +{ + switch (state) + { + case eRunning: + return "Running"; + case eReady: + return "Ready"; + case eBlocked: + return "Blocked"; + case eSuspended: + return "Suspended"; + case eDeleted: + return "Deleted"; + default: + return "Unknown"; + } +} + +CHIP_ERROR ESP32Utils::LogTaskSnapshotInfo() +{ +#ifdef CONFIG_FREERTOS_USE_TRACE_FACILITY + uint32_t arraySize = uxTaskGetNumberOfTasks(); + + chip::Platform::ScopedMemoryBuffer taskStatusArray; + VerifyOrReturnError(taskStatusArray.Calloc(arraySize), CHIP_ERROR_NO_MEMORY); + + uint32_t dummyRunTimeCounter; + arraySize = uxTaskGetSystemState(taskStatusArray.Get(), arraySize, &dummyRunTimeCounter); + + auto GenerateMetricName = [&](const char * task_name, const char * suffix) { + std::string metricName = task_name; + metricName += "_"; + metricName += suffix; + return metricName; + }; + + for (int i = 0; i < arraySize; i++) + { + TaskStatus_t task = taskStatusArray[i]; + MATTER_TRACE_INSTANT(GenerateMetricName(task.pcTaskName, "state").c_str(), StateToString(task.eCurrentState)); + MATTER_TRACE_INSTANT(GenerateMetricName(task.pcTaskName, "stack_start_address").c_str(), + std::to_string(reinterpret_cast(task.pxStackBase)).c_str()); +#ifdef CONFIG_FREERTOS_GENERATE_RUN_TIME_STATS + MATTER_LOG_METRIC(kMetricTaskName, static_cast(task.ulRunTimeCounter)); +#endif // CONFIG_FREERTOS_GENERATE_RUN_TIME_STATS + } +#endif // CONFIG_FREERTOS_USE_TRACE_FACILITY + return CHIP_NO_ERROR; +} #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE struct netif * ESP32Utils::GetNetif(const char * ifKey) diff --git a/src/platform/ESP32/ESP32Utils.h b/src/platform/ESP32/ESP32Utils.h index 294cf76d1e9c6d..34fce1a1cc2933 100644 --- a/src/platform/ESP32/ESP32Utils.h +++ b/src/platform/ESP32/ESP32Utils.h @@ -51,6 +51,7 @@ class ESP32Utils #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE static void LogHeapInfo(); + static CHIP_ERROR LogTaskSnapshotInfo(); #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE static CHIP_ERROR MapError(esp_err_t error); From da1f8b077737a26c49c3da666ed71c0b2fa8f090 Mon Sep 17 00:00:00 2001 From: mahesh Date: Wed, 18 Dec 2024 08:11:41 +0530 Subject: [PATCH 22/22] restyle changes --- src/platform/ESP32/ConnectivityManagerImpl.h | 1 - src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp | 6 +++--- src/platform/ESP32/ESP32Utils.cpp | 9 ++++----- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/platform/ESP32/ConnectivityManagerImpl.h b/src/platform/ESP32/ConnectivityManagerImpl.h index 2e0758c8d46243..48e00fcc1c39ff 100644 --- a/src/platform/ESP32/ConnectivityManagerImpl.h +++ b/src/platform/ESP32/ConnectivityManagerImpl.h @@ -179,7 +179,6 @@ class ConnectivityManagerImpl final : public ConnectivityManager, // ===== Members for internal use by the following friends. - friend ConnectivityManager & ConnectivityMgr(void); friend ConnectivityManagerImpl & ConnectivityMgrImpl(void); diff --git a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp index fc83a8459f20ba..f994cc934028f5 100644 --- a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp +++ b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp @@ -52,9 +52,9 @@ using namespace ::chip::System; using chip::DeviceLayer::Internal::ESP32Utils; #ifdef CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE -static int max_rssi = -100; -static int current_rssi = 1; -static int min_rssi = 0; +static int max_rssi = -100; +static int current_rssi = 1; +static int min_rssi = 0; constexpr chip::Tracing::MetricKey kMetricWiFiMaxRSSI = "max_rssi"; constexpr chip::Tracing::MetricKey kMetricWiFiMinRSSI = "min_rssi"; #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE diff --git a/src/platform/ESP32/ESP32Utils.cpp b/src/platform/ESP32/ESP32Utils.cpp index f2bde483dbad25..0a40d72f1f7ce3 100644 --- a/src/platform/ESP32/ESP32Utils.cpp +++ b/src/platform/ESP32/ESP32Utils.cpp @@ -47,13 +47,13 @@ using namespace chip::DeviceLayer; using namespace chip::Tracing; // Heap Diagnostics (internal) -constexpr MetricKey kMetricHeapInternalFree = "internal_free"; -constexpr MetricKey kMetricHeapInternalMinFree = "internal_min_free"; +constexpr MetricKey kMetricHeapInternalFree = "internal_free"; +constexpr MetricKey kMetricHeapInternalMinFree = "internal_min_free"; constexpr MetricKey kMetricHeapInternalLargestBlock = "internal_largest_free"; // Heap Diagnostics (external) -constexpr MetricKey kMetricHeapExternalFree = "external_free"; -constexpr MetricKey kMetricHeapExternalMinFree = "external_min_free"; +constexpr MetricKey kMetricHeapExternalFree = "external_free"; +constexpr MetricKey kMetricHeapExternalMinFree = "external_min_free"; constexpr MetricKey kMetricHeapExternalLargestBlock = "external_largest_block"; // Task runtime @@ -500,7 +500,6 @@ void ESP32Utils::LogHeapInfo() SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(CONFIG_HEAP_LOG_INTERVAL), LogHeapDataCallback, nullptr); } - const char * StateToString(eTaskState state) { switch (state)