Skip to content

Commit 92919b0

Browse files
[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 <commits@restyled.io>
1 parent 14b3329 commit 92919b0

File tree

2 files changed

+14
-10
lines changed

2 files changed

+14
-10
lines changed

src/platform/ESP32/ConfigurationManagerImpl.cpp

+14-9
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <platform/ConfigurationManager.h>
3131
#include <platform/ESP32/ESP32Config.h>
3232
#include <platform/ESP32/ESP32Utils.h>
33+
#include <platform/ESP32/ScopedNvsHandle.h>
3334
#include <platform/internal/GenericConfigurationManagerImpl.ipp>
3435

3536
#if CHIP_DEVICE_CONFIG_ENABLE_ETHERNET
@@ -55,14 +56,9 @@ uint32_t ConfigurationManagerImpl::mTotalOperationalHours = 0;
5556

5657
void ConfigurationManagerImpl::TotalOperationalHoursTimerCallback(TimerHandle_t timer)
5758
{
58-
mTotalOperationalHours++;
59-
60-
CHIP_ERROR err = ConfigurationMgrImpl().StoreTotalOperationalHours(mTotalOperationalHours);
61-
62-
if (err != CHIP_NO_ERROR)
63-
{
64-
ChipLogError(DeviceLayer, "Failed to store total operational hours: %" CHIP_ERROR_FORMAT, err.Format());
65-
}
59+
// This function is called from the FreeRTOS timer task. Since the task stack is limited,
60+
// we avoid logging error messages here to prevent stack overflows.
61+
(void) ConfigurationMgrImpl().StoreTotalOperationalHours(++mTotalOperationalHours);
6662
}
6763

6864
CHIP_ERROR ConfigurationManagerImpl::Init()
@@ -180,6 +176,9 @@ CHIP_ERROR ConfigurationManagerImpl::Init()
180176
}
181177

182178
{
179+
// The total-operational-hours is critical information. It intentionally uses the FreeRTOS timer
180+
// to increment the value, this ensures it is not affected by PostEvent failures.
181+
183182
// Start a timer which reloads every one hour and bumps the total operational hours
184183
TickType_t reloadPeriod = (1000 * 60 * 60) / portTICK_PERIOD_MS;
185184
TimerHandle_t timerHandle = xTimerCreate("tOpHrs", reloadPeriod, pdPASS, nullptr, TotalOperationalHoursTimerCallback);
@@ -224,7 +223,13 @@ CHIP_ERROR ConfigurationManagerImpl::GetTotalOperationalHours(uint32_t & totalOp
224223

225224
CHIP_ERROR ConfigurationManagerImpl::StoreTotalOperationalHours(uint32_t totalOperationalHours)
226225
{
227-
return WriteConfigValue(ESP32Config::kCounterKey_TotalOperationalHours, totalOperationalHours);
226+
ScopedNvsHandle handle;
227+
ESP32Config::Key key = ESP32Config::kCounterKey_TotalOperationalHours;
228+
229+
ReturnErrorOnFailure(handle.Open(key.Namespace, NVS_READWRITE, ESP32Config::GetPartitionLabelByNamespace(key.Namespace)));
230+
ReturnMappedErrorOnFailure(nvs_set_u32(handle, key.Name, totalOperationalHours));
231+
ReturnMappedErrorOnFailure(nvs_commit(handle));
232+
return CHIP_NO_ERROR;
228233
}
229234

230235
CHIP_ERROR ConfigurationManagerImpl::GetSoftwareVersionString(char * buf, size_t bufSize)

src/platform/ESP32/ESP32Config.h

-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ class ESP32Config
128128

129129
static void RunConfigUnitTest(void);
130130

131-
private:
132131
static const char * GetPartitionLabelByNamespace(const char * ns);
133132
};
134133

0 commit comments

Comments
 (0)