From b94bb606008f93f7bd4026bcd7e9a95c2e4cb284 Mon Sep 17 00:00:00 2001 From: shripad621git Date: Wed, 13 Sep 2023 13:03:30 +0530 Subject: [PATCH] Addressed review comments --- src/tracing/esp32_trace/BUILD.gn | 15 ++++++ .../matter/tracing/insights_sys_stats.cpp | 46 +++++++++---------- .../matter/tracing/insights_sys_stats.h | 4 +- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/tracing/esp32_trace/BUILD.gn b/src/tracing/esp32_trace/BUILD.gn index cb41004a74289b..7548894ab072ce 100644 --- a/src/tracing/esp32_trace/BUILD.gn +++ b/src/tracing/esp32_trace/BUILD.gn @@ -20,6 +20,10 @@ config("tracing") { include_dirs = [ "include" ] } +declare_args() { + matter_enable_esp_insights_system_stats = false +} + static_library("backend") { output_name = "libEsp32TracingBackend" output_dir = "${root_out_dir}/lib" @@ -28,6 +32,17 @@ static_library("backend") { "esp32_tracing.cpp", "esp32_tracing.h", ] + if (matter_enable_esp_insights_system_stats) { + sources += [ + "insights_sys_stats.cpp", + "insights_sys_stats.h", + ] + deps = [ + "${chip_root}/src/lib/core", + "${chip_root}/src/platform", + "${chip_root}/src/system", + ] + } public_deps = [ "${chip_root}/src/tracing" ] } diff --git a/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.cpp b/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.cpp index cb46e2ca5daf28..1b944fd7747d22 100644 --- a/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.cpp +++ b/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.cpp @@ -33,19 +33,19 @@ InsightsSystemMetrics & InsightsSystemMetrics::GetInstance() return instance; } -void InsightsSystemMetrics::SamplingIntervalHandler(Layer * systemLayer, void * context) +void InsightsSystemMetrics::SamplingHandler(Layer * systemLayer, void * context) { auto instance = static_cast(context); count_t * highwatermarks = GetHighWatermarks(); for (int i = 0; i < System::Stats::kNumEntries; i++) { - esp_err_t err = esp_diag_metrics_add_uint(instance->mLabels[i], static_cast(highwatermarks[i])); + esp_err_t err = esp_diag_metrics_add_uint(instance->mLabels[i], static_cast(highwatermarks[i])); if (err != ESP_OK) { ESP_LOGE(kTag, "Failed to add the metric:%s, err:%d", instance->mLabels[i], err); } } - DeviceLayer::SystemLayer().StartTimer(instance->GetSamplingInterval(), SamplingIntervalHandler, instance); + DeviceLayer::SystemLayer().StartTimer(instance->GetSamplingInterval(), SamplingHandler, instance); } CHIP_ERROR InsightsSystemMetrics::Unregister() @@ -67,39 +67,38 @@ CHIP_ERROR InsightsSystemMetrics::Unregister() mLabels[i] = nullptr; } } + mRegistered = false; return CHIP_NO_ERROR; } -void InsightsSystemMetrics::CancelSamplingInterval(intptr_t arg) +void InsightsSystemMetrics::SetSamplingHandler(intptr_t arg) { - auto instance = static_cast(reinterpret_cast(arg)); - DeviceLayer::SystemLayer().CancelTimer(SamplingIntervalHandler, instance); -} - -CHIP_ERROR InsightsSystemMetrics::SetSamplingInterval(chip::System::Clock::Timeout aTimeout) -{ - if (!mRegistered) - { - return CHIP_ERROR_INCORRECT_STATE; - } + InsightsSystemMetrics * instance = reinterpret_cast(arg); - if (aTimeout == System::Clock::kZero) + if (instance->mTimeout == System::Clock::kZero) { - DeviceLayer::PlatformMgr().ScheduleWork(CancelSamplingInterval, reinterpret_cast(this)); + DeviceLayer::SystemLayer().CancelTimer(SamplingHandler, instance); } - else if (aTimeout != mTimeout) + else { - DeviceLayer::PlatformMgr().ScheduleWork(CancelSamplingInterval, reinterpret_cast(this)); - mTimeout = aTimeout; - CHIP_ERROR err = DeviceLayer::SystemLayer().StartTimer(mTimeout, SamplingIntervalHandler, this); + DeviceLayer::SystemLayer().CancelTimer(SamplingHandler, instance); + CHIP_ERROR err = DeviceLayer::SystemLayer().StartTimer(instance->mTimeout, SamplingHandler, instance); if (err != CHIP_NO_ERROR) { ESP_LOGE(kTag, "Failed to start the new timer %" CHIP_ERROR_FORMAT, err.Format()); - return err; } } +} - return CHIP_NO_ERROR; +CHIP_ERROR InsightsSystemMetrics::SetSamplingInterval(chip::System::Clock::Timeout aTimeout) +{ + if (!mRegistered) + { + return CHIP_ERROR_INCORRECT_STATE; + } + mTimeout = aTimeout; + + return DeviceLayer::PlatformMgr().ScheduleWork(SetSamplingHandler, reinterpret_cast(this)); } CHIP_ERROR InsightsSystemMetrics::RegisterAndEnable(chip::System::Clock::Timeout aTimeout) @@ -138,13 +137,14 @@ CHIP_ERROR InsightsSystemMetrics::RegisterAndEnable(chip::System::Clock::Timeout if (err != ESP_OK) { ESP_LOGE(kTag, "Failed to register metric:%s, err:%d", mLabels[i], err); + Unregister(); return CHIP_ERROR_INCORRECT_STATE; } } mTimeout = System::Clock::Milliseconds32(aTimeout); - CHIP_ERROR err = DeviceLayer::SystemLayer().StartTimer(GetSamplingInterval(), SamplingIntervalHandler, this); + CHIP_ERROR err = DeviceLayer::SystemLayer().StartTimer(GetSamplingInterval(), SamplingHandler, this); if (err != CHIP_NO_ERROR) { ESP_LOGE(kTag, "Failed to start the timer, err:%" CHIP_ERROR_FORMAT, err.Format()); diff --git a/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.h b/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.h index 76769b38b14b0c..cb95ce54b196e1 100644 --- a/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.h +++ b/src/tracing/esp32_trace/include/matter/tracing/insights_sys_stats.h @@ -60,8 +60,8 @@ class InsightsSystemMetrics System::Clock::Timeout mTimeout; char * mLabels[chip::System::Stats::kNumEntries]; - static void SamplingIntervalHandler(System::Layer * systemLayer, void * context); - static void CancelSamplingInterval(intptr_t arg); + static void SamplingHandler(System::Layer * systemLayer, void * context); + static void SetSamplingHandler(intptr_t arg); }; } // namespace Stats