diff --git a/README.md b/README.md index 4bf96a0..87ad40c 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,16 @@ Another optional feature is NVS storage, which saves the configuration permanent - Tilt sensitivity - Last tilt position +### NVS Operation Details +When NVS storage is enabled (CONFIG_RELAY_CHN_ENABLE_NVS=y), the component creates a dedicated background task to manage all NVS write operations. This design has important implications for how you use the NVS-related functions: +- **Asynchronous Writes:** All `set` operations (e.g., `relay_chn_flip_direction()`, `relay_chn_set_run_limit()`) are asynchronous. They add the write request to a queue and return immediately, preventing the calling task from being blocked. +- **Synchronous Reads:** All get operations (e.g., `relay_chn_get_direction()`) are synchronous. They read the value directly from the NVS storage and will block the calling task until the read is complete. +- **Batched Commits:** To optimize performance and minimize flash wear, the NVS task uses a batching mechanism for writes. It collects multiple write requests and commits them to the NVS flash in a single operation after a short period of inactivity (typically 200ms). + +> [!IMPORTANT] +> Due to the asynchronous and batched nature of write operations, a call to a get function may not immediately reflect a value that was just written by a set function. Your application should account for this small delay. + + ## Configuration Configure the component through menuconfig under "Relay Channel Driver Configuration": diff --git a/private_include/relay_chn_nvs.h b/private_include/relay_chn_nvs.h index 6dab6a3..1f5bed3 100644 --- a/private_include/relay_chn_nvs.h +++ b/private_include/relay_chn_nvs.h @@ -32,6 +32,10 @@ esp_err_t relay_chn_nvs_init(void); * * @param[in] ch Channel number. * @param[in] direction Direction to store. + * + * @note This operation is asynchronous. The value is queued to be written + * by a background task. A subsequent `get` call may not immediately + * reflect the new value. * @return ESP_OK on success, error code otherwise. */ esp_err_t relay_chn_nvs_set_direction(uint8_t ch, relay_chn_direction_t direction); @@ -41,16 +45,21 @@ esp_err_t relay_chn_nvs_set_direction(uint8_t ch, relay_chn_direction_t directio * * @param[in] ch Channel number. * @param[out] direction Pointer to store retrieved direction. + * @param[in] default_val Default value to use if not found in NVS. * @return ESP_OK on success, error code otherwise. */ -esp_err_t relay_chn_nvs_get_direction(uint8_t ch, relay_chn_direction_t *direction); +esp_err_t relay_chn_nvs_get_direction(uint8_t ch, relay_chn_direction_t *direction, relay_chn_direction_t default_val); #if CONFIG_RELAY_CHN_ENABLE_RUN_LIMIT /** * @brief Store relay channel run limit in NVS. * * @param[in] ch Channel number. - * @param[in] direction Run limit value to store. + * @param[in] limit_sec Run limit value to store. + * + * @note This operation is asynchronous. The value is queued to be written + * by a background task. A subsequent `get` call may not immediately + * reflect the new value. * @return ESP_OK on success, error code otherwise. */ esp_err_t relay_chn_nvs_set_run_limit(uint8_t ch, uint16_t limit_sec); @@ -59,10 +68,11 @@ esp_err_t relay_chn_nvs_set_run_limit(uint8_t ch, uint16_t limit_sec); * @brief Retrieve relay channel run limit from NVS. * * @param[in] ch Channel number. - * @param[out] direction Pointer to store retrieved run limit value. + * @param[out] limit_sec Pointer to store retrieved run limit value. + * @param[in] default_val Default value to use if not found in NVS. * @return ESP_OK on success, error code otherwise. */ -esp_err_t relay_chn_nvs_get_run_limit(uint8_t ch, uint16_t *limit_sec); +esp_err_t relay_chn_nvs_get_run_limit(uint8_t ch, uint16_t *limit_sec, uint16_t default_val); #endif // CONFIG_RELAY_CHN_ENABLE_RUN_LIMIT == 1 #if CONFIG_RELAY_CHN_ENABLE_TILTING @@ -71,6 +81,10 @@ esp_err_t relay_chn_nvs_get_run_limit(uint8_t ch, uint16_t *limit_sec); * * @param[in] ch Channel number. * @param[in] sensitivity Sensitivity value to store. + * + * @note This operation is asynchronous. The value is queued to be written + * by a background task. A subsequent `get` call may not immediately + * reflect the new value. * @return ESP_OK on success, error code otherwise. */ esp_err_t relay_chn_nvs_set_tilt_sensitivity(uint8_t ch, uint8_t sensitivity); @@ -80,15 +94,20 @@ esp_err_t relay_chn_nvs_set_tilt_sensitivity(uint8_t ch, uint8_t sensitivity); * * @param[in] ch Channel number. * @param[out] sensitivity Pointer to store retrieved sensitivity. + * @param[in] default_val Default value to use if not found in NVS. * @return ESP_OK on success, error code otherwise. */ -esp_err_t relay_chn_nvs_get_tilt_sensitivity(uint8_t ch, uint8_t *sensitivity); +esp_err_t relay_chn_nvs_get_tilt_sensitivity(uint8_t ch, uint8_t *sensitivity, uint8_t default_val); /** * @brief Store tilt counters in NVS. * * @param[in] ch Channel number. * @param[in] tilt_count Tilt count value. + * + * @note This operation is asynchronous. The value is queued to be written + * by a background task. A subsequent `get` call may not immediately + * reflect the new value. * @return ESP_OK on success, error code otherwise. */ esp_err_t relay_chn_nvs_set_tilt_count(uint8_t ch, uint16_t tilt_count); @@ -98,15 +117,17 @@ esp_err_t relay_chn_nvs_set_tilt_count(uint8_t ch, uint16_t tilt_count); * * @param[in] ch Channel number. * @param[out] tilt_count Pointer to store tilt count. + * @param[in] default_val Default value to use if not found in NVS. * @return ESP_OK on success, error code otherwise. */ -esp_err_t relay_chn_nvs_get_tilt_count(uint8_t ch, uint16_t *tilt_count); +esp_err_t relay_chn_nvs_get_tilt_count(uint8_t ch, uint16_t *tilt_count, uint16_t default_val); #endif // CONFIG_RELAY_CHN_ENABLE_TILTING /** * @brief Erase all keys in the NVS namespace. * * This function will erase all key-value pairs in the NVS namespace used by relay channels. + * It will also flush all pending operations in the queue. * * @return ESP_OK on success, error code otherwise. */ @@ -114,10 +135,8 @@ esp_err_t relay_chn_nvs_erase_all(void); /** * @brief Deinitialize NVS storage for relay channels. - * - * @return ESP_OK on success, error code otherwise. */ -esp_err_t relay_chn_nvs_deinit(void); +void relay_chn_nvs_deinit(void); #ifdef __cplusplus } diff --git a/src/relay_chn_ctl_multi.c b/src/relay_chn_ctl_multi.c index 14bf3a4..31122f9 100644 --- a/src/relay_chn_ctl_multi.c +++ b/src/relay_chn_ctl_multi.c @@ -38,10 +38,8 @@ esp_err_t relay_chn_ctl_init(relay_chn_output_t *outputs, relay_chn_run_info_t * uint16_t run_limit_sec = CONFIG_RELAY_CHN_RUN_LIMIT_DEFAULT_SEC; #if CONFIG_RELAY_CHN_ENABLE_NVS // Load run limit value from NVS - ret = relay_chn_nvs_get_run_limit(chn_ctl->id, &run_limit_sec); - if (ret != ESP_OK && ret != ESP_ERR_NVS_NOT_FOUND) { - ESP_LOGE(TAG, "Failed to load run limit from NVS for channel %d with error: %s", i, esp_err_to_name(ret)); - } + ret = relay_chn_nvs_get_run_limit(chn_ctl->id, &run_limit_sec, CONFIG_RELAY_CHN_RUN_LIMIT_DEFAULT_SEC); + ESP_RETURN_ON_ERROR(ret, TAG, "Failed to load run limit from NVS for #%d with error: %s", i, esp_err_to_name(ret)); #endif chn_ctl->run_limit_sec = run_limit_sec; ret = relay_chn_init_run_limit_timer(chn_ctl); diff --git a/src/relay_chn_ctl_single.c b/src/relay_chn_ctl_single.c index af7aba1..c95af90 100644 --- a/src/relay_chn_ctl_single.c +++ b/src/relay_chn_ctl_single.c @@ -32,10 +32,8 @@ esp_err_t relay_chn_ctl_init(relay_chn_output_t *output, relay_chn_run_info_t *r esp_err_t ret; #if CONFIG_RELAY_CHN_ENABLE_NVS // Load run limit value from NVS - ret = relay_chn_nvs_get_run_limit(chn_ctl.id, &run_limit_sec); - if (ret != ESP_OK && ret != ESP_ERR_NVS_NOT_FOUND) { - ESP_LOGE(TAG, "Failed to load run limit from NVS with error: %s", esp_err_to_name(ret)); - } + ret = relay_chn_nvs_get_run_limit(chn_ctl.id, &run_limit_sec, CONFIG_RELAY_CHN_RUN_LIMIT_DEFAULT_SEC); + ESP_RETURN_ON_ERROR(ret, TAG, "Failed to load run limit from NVS with error: %s", esp_err_to_name(ret)); #endif chn_ctl.run_limit_sec = run_limit_sec; ret = relay_chn_init_run_limit_timer(&chn_ctl); diff --git a/src/relay_chn_nvs.c b/src/relay_chn_nvs.c index 7ddd5ae..9f80f10 100644 --- a/src/relay_chn_nvs.c +++ b/src/relay_chn_nvs.c @@ -4,7 +4,12 @@ * SPDX-License-Identifier: MIT */ +#include "freertos/FreeRTOS.h" +#include "freertos/semphr.h" +#include "freertos/task.h" +#include "freertos/queue.h" #include "esp_check.h" +#include "esp_log.h" #include "nvs.h" #include "relay_chn_nvs.h" @@ -26,12 +31,71 @@ #endif #endif +// --- Task and message queue config --- +#define RELAY_CHN_NVS_QUEUE_LEN (8 + CONFIG_RELAY_CHN_COUNT * 8) +#define RELAY_CHN_NVS_TASK_STACK 2048 +#define RELAY_CHN_NVS_COMMIT_TIMEOUT_MS 200 +#define RELAY_CHN_NVS_TASK_PRIO (tskIDLE_PRIORITY + 4) + +typedef enum { + RELAY_CHN_NVS_OP_ERASE_ALL, + RELAY_CHN_NVS_OP_SET_DIRECTION, +#if CONFIG_RELAY_CHN_ENABLE_RUN_LIMIT + RELAY_CHN_NVS_OP_SET_RUN_LIMIT, +#endif +#if CONFIG_RELAY_CHN_ENABLE_TILTING + RELAY_CHN_NVS_OP_SET_TILT_SENSITIVITY, + RELAY_CHN_NVS_OP_SET_TILT_COUNT, +#endif + RELAY_CHN_NVS_OP_DEINIT, +} relay_chn_nvs_op_t; + +typedef struct { + relay_chn_nvs_op_t op; + uint8_t ch; + union { + uint16_t data_u16; + uint8_t data_u8; + } data; +} relay_chn_nvs_msg_t; + static const char *TAG = "RELAY_CHN_NVS"; static nvs_handle_t relay_chn_nvs; +static QueueHandle_t nvs_queue_handle = NULL; +static TaskHandle_t nvs_task_handle = NULL; +static SemaphoreHandle_t deinit_sem = NULL; + + +static void relay_chn_nvs_task(void *arg); esp_err_t relay_chn_nvs_init() { + // Already initialized? + if (nvs_queue_handle != NULL) { + return ESP_OK; + } + + deinit_sem = xSemaphoreCreateBinary(); + if (!deinit_sem) { + ESP_LOGE(TAG, "Failed to create deinit semaphore"); + return ESP_ERR_NO_MEM; + } + + nvs_queue_handle = xQueueCreate(RELAY_CHN_NVS_QUEUE_LEN, sizeof(relay_chn_nvs_msg_t)); + if (!nvs_queue_handle) { + ESP_LOGE(TAG, "Failed to create NVS queue"); + return ESP_ERR_NO_MEM; + } + + BaseType_t res = xTaskCreate(relay_chn_nvs_task, "task_rlch_nvs", + RELAY_CHN_NVS_TASK_STACK, NULL, + RELAY_CHN_NVS_TASK_PRIO, &nvs_task_handle); + if (res != pdPASS) { + ESP_LOGE(TAG, "Failed to create NVS task"); + return ESP_ERR_NO_MEM; + } + esp_err_t ret; #if CONFIG_RELAY_CHN_NVS_CUSTOM_PARTITION ret = nvs_open_from_partition(CONFIG_RELAY_CHN_NVS_CUSTOM_PARTITION_NAME, @@ -52,7 +116,39 @@ esp_err_t relay_chn_nvs_init() return ESP_OK; } +static esp_err_t relay_chn_nvs_enqueue(relay_chn_nvs_msg_t *msg, const char *op_name) +{ + if (!nvs_queue_handle) { + return ESP_ERR_INVALID_STATE; + } + + if (msg->op == RELAY_CHN_NVS_OP_DEINIT || msg->op == RELAY_CHN_NVS_OP_ERASE_ALL) { + // Send DEINIT or ERASE_ALL to the front and wait up to 1 sec if needed + if (xQueueSendToFront(nvs_queue_handle, msg, pdMS_TO_TICKS(1000)) != pdTRUE) { + ESP_LOGW(TAG, "NVS queue is full, dropping %s for #%d", op_name, msg->ch); + return ESP_FAIL; + } + } else { + // Send async + if (xQueueSend(nvs_queue_handle, msg, 0) != pdTRUE) { + ESP_LOGW(TAG, "NVS queue is full, dropping %s for #%d", op_name, msg->ch); + return ESP_FAIL; + } + } + return ESP_OK; +} + esp_err_t relay_chn_nvs_set_direction(uint8_t ch, relay_chn_direction_t direction) +{ + relay_chn_nvs_msg_t msg = { + .op = RELAY_CHN_NVS_OP_SET_DIRECTION, + .ch = ch, + .data.data_u8 = (uint8_t) direction, + }; + return relay_chn_nvs_enqueue(&msg, "SET_DIRECTION"); +} + +static esp_err_t relay_chn_nvs_task_set_direction(uint8_t ch, uint8_t direction) { uint8_t direction_val = 0; esp_err_t ret = nvs_get_u8(relay_chn_nvs, RELAY_CHN_KEY_DIR, &direction_val); @@ -63,24 +159,38 @@ esp_err_t relay_chn_nvs_set_direction(uint8_t ch, relay_chn_direction_t directio direction_val |= (((uint8_t) direction) << ch); // Set the new direction bit ret = nvs_set_u8(relay_chn_nvs, RELAY_CHN_KEY_DIR, direction_val); ESP_RETURN_ON_ERROR(ret, TAG, "Failed to set direction for channel %d", ch); - return nvs_commit(relay_chn_nvs); + return ESP_OK; } -esp_err_t relay_chn_nvs_get_direction(uint8_t ch, relay_chn_direction_t *direction) +esp_err_t relay_chn_nvs_get_direction(uint8_t ch, relay_chn_direction_t *direction, relay_chn_direction_t default_val) { ESP_RETURN_ON_FALSE(direction != NULL, ESP_ERR_INVALID_ARG, TAG, "Direction pointer is NULL"); - + uint8_t direction_val; esp_err_t ret = nvs_get_u8(relay_chn_nvs, RELAY_CHN_KEY_DIR, &direction_val); - if (ret != ESP_OK) { - return ret; // Return error if the key does not exist + if (ret == ESP_ERR_NVS_NOT_FOUND) { + *direction = default_val; + return ESP_OK; + } else if (ret != ESP_OK) { + return ret; // A real error occurred, return it } + // If ret is ESP_OK, direction_val has the stored value. *direction = (relay_chn_direction_t)((direction_val >> ch) & 0x01); return ESP_OK; } #if CONFIG_RELAY_CHN_ENABLE_RUN_LIMIT esp_err_t relay_chn_nvs_set_run_limit(uint8_t ch, uint16_t limit_sec) +{ + relay_chn_nvs_msg_t msg = { + .op = RELAY_CHN_NVS_OP_SET_RUN_LIMIT, + .ch = ch, + .data.data_u16 = limit_sec, + }; + return relay_chn_nvs_enqueue(&msg, "SET_RUN_LIMIT"); +} + +static esp_err_t relay_chn_nvs_task_set_run_limit(uint8_t ch, uint16_t limit_sec) { esp_err_t ret; #if CONFIG_RELAY_CHN_COUNT > 1 @@ -91,25 +201,41 @@ esp_err_t relay_chn_nvs_set_run_limit(uint8_t ch, uint16_t limit_sec) ret = nvs_set_u16(relay_chn_nvs, RELAY_CHN_KEY_RLIM, limit_sec); #endif ESP_RETURN_ON_ERROR(ret, TAG, "Failed to set run limit for channel %d", ch); - return nvs_commit(relay_chn_nvs); + return ESP_OK; } -esp_err_t relay_chn_nvs_get_run_limit(uint8_t ch, uint16_t *limit_sec) +esp_err_t relay_chn_nvs_get_run_limit(uint8_t ch, uint16_t *limit_sec, uint16_t default_val) { ESP_RETURN_ON_FALSE(limit_sec != NULL, ESP_ERR_INVALID_ARG, TAG, "Run limit value pointer is NULL"); + esp_err_t ret; #if CONFIG_RELAY_CHN_COUNT > 1 char key[NVS_KEY_NAME_MAX_SIZE]; snprintf(key, sizeof(key), RELAY_CHN_KEY_RLIM_FMT, ch); - return nvs_get_u16(relay_chn_nvs, key, limit_sec); + ret = nvs_get_u16(relay_chn_nvs, key, limit_sec); #else - return nvs_get_u16(relay_chn_nvs, RELAY_CHN_KEY_RLIM, limit_sec); + ret = nvs_get_u16(relay_chn_nvs, RELAY_CHN_KEY_RLIM, limit_sec); #endif + if (ret == ESP_ERR_NVS_NOT_FOUND) { + *limit_sec = default_val; + return ESP_OK; + } + return ret; } #endif // CONFIG_RELAY_CHN_ENABLE_RUN_LIMIT == 1 #if CONFIG_RELAY_CHN_ENABLE_TILTING esp_err_t relay_chn_nvs_set_tilt_sensitivity(uint8_t ch, uint8_t sensitivity) +{ + relay_chn_nvs_msg_t msg = { + .op = RELAY_CHN_NVS_OP_SET_TILT_SENSITIVITY, + .ch = ch, + .data.data_u8 = sensitivity, + }; + return relay_chn_nvs_enqueue(&msg, "SET_TILT_SENSITIVITY"); +} + +static esp_err_t relay_chn_nvs_task_set_tilt_sensitivity(uint8_t ch, uint8_t sensitivity) { esp_err_t ret; #if CONFIG_RELAY_CHN_COUNT > 1 @@ -120,23 +246,39 @@ esp_err_t relay_chn_nvs_set_tilt_sensitivity(uint8_t ch, uint8_t sensitivity) ret = nvs_set_u8(relay_chn_nvs, RELAY_CHN_KEY_TSENS, sensitivity); #endif ESP_RETURN_ON_ERROR(ret, TAG, "Failed to set tilt sensitivity for channel %d", ch); - return nvs_commit(relay_chn_nvs); + return ESP_OK; } -esp_err_t relay_chn_nvs_get_tilt_sensitivity(uint8_t ch, uint8_t *sensitivity) +esp_err_t relay_chn_nvs_get_tilt_sensitivity(uint8_t ch, uint8_t *sensitivity, uint8_t default_val) { ESP_RETURN_ON_FALSE(sensitivity != NULL, ESP_ERR_INVALID_ARG, TAG, "Sensitivity pointer is NULL"); + esp_err_t ret; #if CONFIG_RELAY_CHN_COUNT > 1 char key[NVS_KEY_NAME_MAX_SIZE]; snprintf(key, sizeof(key), RELAY_CHN_KEY_TSENS_FMT, ch); - return nvs_get_u8(relay_chn_nvs, key, sensitivity); + ret = nvs_get_u8(relay_chn_nvs, key, sensitivity); #else - return nvs_get_u8(relay_chn_nvs, RELAY_CHN_KEY_TSENS, sensitivity); + ret = nvs_get_u8(relay_chn_nvs, RELAY_CHN_KEY_TSENS, sensitivity); #endif + if (ret == ESP_ERR_NVS_NOT_FOUND) { + *sensitivity = default_val; + return ESP_OK; + } + return ret; } esp_err_t relay_chn_nvs_set_tilt_count(uint8_t ch, uint16_t tilt_count) +{ + relay_chn_nvs_msg_t msg = { + .op = RELAY_CHN_NVS_OP_SET_TILT_COUNT, + .ch = ch, + .data.data_u16 = tilt_count, + }; + return relay_chn_nvs_enqueue(&msg, "SET_TILT_COUNT"); +} + +static esp_err_t relay_chn_nvs_task_set_tilt_count(uint8_t ch, uint16_t tilt_count) { esp_err_t ret; #if CONFIG_RELAY_CHN_COUNT > 1 @@ -147,35 +289,164 @@ esp_err_t relay_chn_nvs_set_tilt_count(uint8_t ch, uint16_t tilt_count) ret = nvs_set_u16(relay_chn_nvs, RELAY_CHN_KEY_TCNT, tilt_count); #endif ESP_RETURN_ON_ERROR(ret, TAG, "Failed to save tilt_count tilt counter"); - return nvs_commit(relay_chn_nvs); + return ESP_OK; } -esp_err_t relay_chn_nvs_get_tilt_count(uint8_t ch, uint16_t *tilt_count) +esp_err_t relay_chn_nvs_get_tilt_count(uint8_t ch, uint16_t *tilt_count, uint16_t default_val) { ESP_RETURN_ON_FALSE(tilt_count != NULL, ESP_ERR_INVALID_ARG, TAG, "Counter pointers are NULL"); - + + esp_err_t ret; #if CONFIG_RELAY_CHN_COUNT > 1 char key[NVS_KEY_NAME_MAX_SIZE]; snprintf(key, sizeof(key), RELAY_CHN_KEY_TCNT_FMT, ch); - return nvs_get_u16(relay_chn_nvs, key, tilt_count); + ret = nvs_get_u16(relay_chn_nvs, key, tilt_count); #else - return nvs_get_u16(relay_chn_nvs, RELAY_CHN_KEY_TCNT, tilt_count); + ret = nvs_get_u16(relay_chn_nvs, RELAY_CHN_KEY_TCNT, tilt_count); #endif + if (ret == ESP_ERR_NVS_NOT_FOUND) { + *tilt_count = default_val; + return ESP_OK; + } + return ret; } #endif // CONFIG_RELAY_CHN_ENABLE_TILTING esp_err_t relay_chn_nvs_erase_all() { + relay_chn_nvs_msg_t msg = { + .op = RELAY_CHN_NVS_OP_ERASE_ALL, + }; + return relay_chn_nvs_enqueue(&msg, "ERASE_ALL"); +} + +static esp_err_t do_nvs_deinit() +{ + relay_chn_nvs_msg_t msg = { + .op = RELAY_CHN_NVS_OP_DEINIT, + }; + return relay_chn_nvs_enqueue(&msg, "DEINIT"); +} + +static esp_err_t do_nvs_erase_all() +{ + // Flush all pending SET operations since ERASE_ALL requested + xQueueReset(nvs_queue_handle); // Erase all key-value pairs in the relay_chn NVS namespace esp_err_t ret = nvs_erase_all(relay_chn_nvs); ESP_RETURN_ON_ERROR(ret, TAG, "Failed to erase all keys in NVS namespace '%s'", CONFIG_RELAY_CHN_NVS_NAMESPACE); - - // Commit the changes - return nvs_commit(relay_chn_nvs); -} - -esp_err_t relay_chn_nvs_deinit() -{ - nvs_close(relay_chn_nvs); return ESP_OK; } + +void relay_chn_nvs_deinit() +{ + if (nvs_task_handle) { + if (do_nvs_deinit() == ESP_OK) { + if (deinit_sem && xSemaphoreTake(deinit_sem, pdMS_TO_TICKS(2000)) != pdTRUE) { + ESP_LOGE(TAG, "Failed to get deinit confirmation from NVS task. Forcing deletion."); + vTaskDelete(nvs_task_handle); // Last resort + } + } else { + ESP_LOGE(TAG, "Failed to send deinit message to NVS task. Forcing deletion."); + vTaskDelete(nvs_task_handle); + } + } + if (nvs_queue_handle) { + vQueueDelete(nvs_queue_handle); + nvs_queue_handle = NULL; + } + if (deinit_sem) { + vSemaphoreDelete(deinit_sem); + deinit_sem = NULL; + } + + // Close NVS handle here, after task has stopped and queue is deleted. + nvs_close(relay_chn_nvs); + nvs_task_handle = NULL; +} + +static esp_err_t relay_chn_nvs_task_process_message(const relay_chn_nvs_msg_t *msg, bool *running, bool *dirty) +{ + esp_err_t ret = ESP_OK; + switch (msg->op) { + case RELAY_CHN_NVS_OP_SET_DIRECTION: + ret = relay_chn_nvs_task_set_direction(msg->ch, msg->data.data_u8); + if (ret == ESP_OK) *dirty = true; + break; +#if CONFIG_RELAY_CHN_ENABLE_RUN_LIMIT + case RELAY_CHN_NVS_OP_SET_RUN_LIMIT: + ret = relay_chn_nvs_task_set_run_limit(msg->ch, msg->data.data_u16); + if (ret == ESP_OK) *dirty = true; + break; +#endif +#if CONFIG_RELAY_CHN_ENABLE_TILTING + case RELAY_CHN_NVS_OP_SET_TILT_SENSITIVITY: + ret = relay_chn_nvs_task_set_tilt_sensitivity(msg->ch, msg->data.data_u8); + if (ret == ESP_OK) *dirty = true; + break; + case RELAY_CHN_NVS_OP_SET_TILT_COUNT: + ret = relay_chn_nvs_task_set_tilt_count(msg->ch, msg->data.data_u16); + if (ret == ESP_OK) *dirty = true; + break; +#endif + case RELAY_CHN_NVS_OP_ERASE_ALL: + ret = do_nvs_erase_all(); + if (ret == ESP_OK) *dirty = true; + break; + case RELAY_CHN_NVS_OP_DEINIT: + *running = false; + break; + default: + ESP_LOGE(TAG, "Unknown operation in NVS queue: %d", msg->op); + ret = ESP_ERR_INVALID_ARG; + break; + } + return ret; +} + +/* + * The ESP-IDF NVS functions are protected by an internal mutex. If this task is killed + * while it's holding that mutex, the mutex is never released, which may result in + * deadlocks. This is why this task must be terminated gracefully. + */ +static void relay_chn_nvs_task(void *arg) +{ + relay_chn_nvs_msg_t msg; + bool dirty = false; + bool running = true; + + while (running) { + // Block indefinitely waiting for the first message of a potential batch. + if (xQueueReceive(nvs_queue_handle, &msg, portMAX_DELAY) == pdTRUE) { + // A batch of operations has started. Use a do-while to process the first message + // and any subsequent messages that arrive within the timeout. + do { + esp_err_t ret = relay_chn_nvs_task_process_message(&msg, &running, &dirty); + if (ret != ESP_OK) { + ESP_LOGE(TAG, "Failed to process operation %d for #%d with error %s", msg.op, msg.ch, esp_err_to_name(ret)); + } + } while (running && xQueueReceive(nvs_queue_handle, &msg, pdMS_TO_TICKS(RELAY_CHN_NVS_COMMIT_TIMEOUT_MS)) == pdTRUE); + + // The burst of messages is over (timeout occurred). Commit if anything changed. + if (dirty) { + esp_err_t commit_ret = nvs_commit(relay_chn_nvs); + if (commit_ret == ESP_OK) { + dirty = false; + } else { + ESP_LOGE(TAG, "NVS batch commit failed"); + // Don't reset dirty flag, so we can try to commit again later. + } + } + } + } + + // Before exiting, do one final commit if there are pending changes. + if (dirty) { + if (nvs_commit(relay_chn_nvs) != ESP_OK) { + ESP_LOGE(TAG, "Final NVS commit failed on deinit"); + } + } + xSemaphoreGive(deinit_sem); + nvs_task_handle = NULL; + vTaskDelete(NULL); +} \ No newline at end of file diff --git a/src/relay_chn_output.c b/src/relay_chn_output.c index b0d2c0f..3a608a7 100644 --- a/src/relay_chn_output.c +++ b/src/relay_chn_output.c @@ -75,13 +75,9 @@ static esp_err_t relay_chn_output_ctl_init(relay_chn_output_t *output, #if CONFIG_RELAY_CHN_ENABLE_NVS static esp_err_t relay_chn_output_load_direction(uint8_t ch, relay_chn_direction_t *direction) { - esp_err_t ret = relay_chn_nvs_get_direction(ch, direction); - if (ret == ESP_ERR_NVS_NOT_FOUND) { - // If the key does not exist, use the default direction - *direction = RELAY_CHN_DIRECTION_DEFAULT; - } else if (ret != ESP_OK) { - ESP_RETURN_ON_ERROR(ret, TAG, "Failed to get direction from storage for channel %d: %s", ch, esp_err_to_name(ret)); - } + esp_err_t ret = relay_chn_nvs_get_direction(ch, direction, RELAY_CHN_DIRECTION_DEFAULT); + // relay_chn_nvs_get_direction now handles the NOT_FOUND case and returns a default value. + ESP_RETURN_ON_ERROR(ret, TAG, "Failed to get direction from storage for channel %d: %s", ch, esp_err_to_name(ret)); return ESP_OK; } #endif diff --git a/src/relay_chn_tilt.c b/src/relay_chn_tilt.c index 98c7c7c..6a2da91 100644 --- a/src/relay_chn_tilt.c +++ b/src/relay_chn_tilt.c @@ -636,24 +636,16 @@ static void relay_chn_tilt_timer_cb(void *arg) #if CONFIG_RELAY_CHN_ENABLE_NVS static esp_err_t relay_chn_tilt_load_sensitivity(uint8_t ch, uint8_t *sensitivity) { - esp_err_t ret = relay_chn_nvs_get_tilt_sensitivity(ch, sensitivity); - if (ret == ESP_ERR_NVS_NOT_FOUND) { - *sensitivity = RELAY_CHN_TILT_DEFAULT_SENSITIVITY; - return ESP_OK; - } - ESP_RETURN_ON_ERROR(ret, TAG, "Failed to load tilt sensitivity for channel %d", ch); + ESP_RETURN_ON_ERROR(relay_chn_nvs_get_tilt_sensitivity(ch, sensitivity, RELAY_CHN_TILT_DEFAULT_SENSITIVITY), + TAG, "Failed to load tilt sensitivity for channel %d", ch); return ESP_OK; } static esp_err_t relay_chn_tilt_load_tilt_count(uint8_t ch, uint16_t *tilt_count) { - esp_err_t ret = relay_chn_nvs_get_tilt_count(ch, tilt_count); - if (ret == ESP_ERR_NVS_NOT_FOUND) { - ESP_LOGD(TAG, "relay_chn_tilt_load_tilt_count: No tilt count found in NVS for channel %d, initializing to zero", ch); - tilt_count = 0; - return ESP_OK; - } - ESP_RETURN_ON_ERROR(ret, TAG, "Failed to load tilt counters for channel %d", ch); + ESP_RETURN_ON_ERROR(relay_chn_nvs_get_tilt_count(ch, tilt_count, 0), + TAG, "Failed to load tilt counters for channel %d", ch); + ESP_LOGD(TAG, "Loaded tilt count for channel %d: %d", ch, *tilt_count); return ESP_OK; } #endif // CONFIG_RELAY_CHN_ENABLE_NVS