Skip to content

Commit

Permalink
[ESP32]: increment the total operational hours inline without log (#3…
Browse files Browse the repository at this point in the history
…7058)

* [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 <[email protected]>
  • Loading branch information
shubhamdp and restyled-commits committed Jan 15, 2025
1 parent 5d21af8 commit 45ce5cb
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 10 deletions.
23 changes: 14 additions & 9 deletions src/platform/ESP32/ConfigurationManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <platform/ConfigurationManager.h>
#include <platform/ESP32/ESP32Config.h>
#include <platform/ESP32/ESP32Utils.h>
#include <platform/ESP32/ScopedNvsHandle.h>
#include <platform/internal/GenericConfigurationManagerImpl.ipp>

#if CHIP_DEVICE_CONFIG_ENABLE_ETHERNET
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion src/platform/ESP32/ESP32Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ class ESP32Config

static void RunConfigUnitTest(void);

private:
static const char * GetPartitionLabelByNamespace(const char * ns);
};

Expand Down

0 comments on commit 45ce5cb

Please sign in to comment.