From ee8624e31f29d48fecace26a582868dd082dfbdd Mon Sep 17 00:00:00 2001 From: mahesh Date: Thu, 10 Oct 2024 13:00:23 +0530 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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