From 45ce5cb4c789ec2fd20eae62b523fa0504d990ba Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Wed, 15 Jan 2025 12:02:48 +0530 Subject: [PATCH] [ESP32]: increment the total operational hours inline without log (#37058) * [ESP32]: increment the total operational hours inline without log Since total-operational-hours is a critical information, it intentionally uses the FreeRTOS timer to increment the values, so that it should not be a victim of PostEvent failures. Earlier it used to call WriteConfigValues which has the ChipLogProgress and logging from the timer stack may overflow the timer stack. So, inlined the implementation which do not log. * Restyled by clang-format * add reasoning behind the change as comment --------- Co-authored-by: Restyled.io --- .../ESP32/ConfigurationManagerImpl.cpp | 23 +++++++++++-------- src/platform/ESP32/ESP32Config.h | 1 - 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/platform/ESP32/ConfigurationManagerImpl.cpp b/src/platform/ESP32/ConfigurationManagerImpl.cpp index 00b07420704f5d..eea0b5e66125b5 100644 --- a/src/platform/ESP32/ConfigurationManagerImpl.cpp +++ b/src/platform/ESP32/ConfigurationManagerImpl.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #if CHIP_DEVICE_CONFIG_ENABLE_ETHERNET @@ -57,14 +58,9 @@ uint32_t ConfigurationManagerImpl::mTotalOperationalHours = 0; void ConfigurationManagerImpl::TotalOperationalHoursTimerCallback(TimerHandle_t timer) { - mTotalOperationalHours++; - - CHIP_ERROR err = ConfigurationMgrImpl().StoreTotalOperationalHours(mTotalOperationalHours); - - if (err != CHIP_NO_ERROR) - { - ChipLogError(DeviceLayer, "Failed to store total operational hours: %" CHIP_ERROR_FORMAT, err.Format()); - } + // This function is called from the FreeRTOS timer task. Since the task stack is limited, + // we avoid logging error messages here to prevent stack overflows. + (void) ConfigurationMgrImpl().StoreTotalOperationalHours(++mTotalOperationalHours); } CHIP_ERROR ConfigurationManagerImpl::Init() @@ -182,6 +178,9 @@ CHIP_ERROR ConfigurationManagerImpl::Init() } { + // The total-operational-hours is critical information. It intentionally uses the FreeRTOS timer + // to increment the value, this ensures it is not affected by PostEvent failures. + // Start a timer which reloads every one hour and bumps the total operational hours TickType_t reloadPeriod = (1000 * 60 * 60) / portTICK_PERIOD_MS; TimerHandle_t timerHandle = xTimerCreate("tOpHrs", reloadPeriod, pdPASS, nullptr, TotalOperationalHoursTimerCallback); @@ -226,7 +225,13 @@ CHIP_ERROR ConfigurationManagerImpl::GetTotalOperationalHours(uint32_t & totalOp CHIP_ERROR ConfigurationManagerImpl::StoreTotalOperationalHours(uint32_t totalOperationalHours) { - return WriteConfigValue(ESP32Config::kCounterKey_TotalOperationalHours, totalOperationalHours); + ScopedNvsHandle handle; + ESP32Config::Key key = ESP32Config::kCounterKey_TotalOperationalHours; + + ReturnErrorOnFailure(handle.Open(key.Namespace, NVS_READWRITE, ESP32Config::GetPartitionLabelByNamespace(key.Namespace))); + ReturnMappedErrorOnFailure(nvs_set_u32(handle, key.Name, totalOperationalHours)); + ReturnMappedErrorOnFailure(nvs_commit(handle)); + return CHIP_NO_ERROR; } CHIP_ERROR ConfigurationManagerImpl::GetSoftwareVersionString(char * buf, size_t bufSize) diff --git a/src/platform/ESP32/ESP32Config.h b/src/platform/ESP32/ESP32Config.h index f2f03cf7f9074b..570066e93b3d78 100644 --- a/src/platform/ESP32/ESP32Config.h +++ b/src/platform/ESP32/ESP32Config.h @@ -132,7 +132,6 @@ class ESP32Config static void RunConfigUnitTest(void); -private: static const char * GetPartitionLabelByNamespace(const char * ns); };