From fe425640c286cad4a4bbe91c399e791b45195dbb Mon Sep 17 00:00:00 2001 From: beamholder Date: Sat, 19 Jun 2021 10:44:19 -0600 Subject: [PATCH 1/2] Include nvs_commit() on three methods In [Preferences.cpp](https://github.com/espressif/arduino-esp32/blob/master/libraries/Preferences/src/Preferences.cpp), the functions: ``` Preferences::clear() Preferences::remove() Preferences::end() ``` should be revised to include a call to `nvs_commit()` as required per [Non-volatile storage library](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/storage/nvs_flash.html) when using ``` nvs_erase_all() nvs_erase_key() nvs_close() ``` --- libraries/Preferences/src/Preferences.cpp | 26 ++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/libraries/Preferences/src/Preferences.cpp b/libraries/Preferences/src/Preferences.cpp index 3c1928b820f..ae6b71eebaf 100644 --- a/libraries/Preferences/src/Preferences.cpp +++ b/libraries/Preferences/src/Preferences.cpp @@ -53,10 +53,14 @@ bool Preferences::begin(const char * name, bool readOnly, const char* partition_ return true; } -void Preferences::end(){ +void Preferences::end(){ // modified to add an nvs_commit() if(!_started){ return; } + esp_err_t err = nvs_commit(_handle); // to undo changes: delete the lines from here... + if(err){ + log_e("nvs_commit fail: %s %s", key, nvs_error(err)); + } // ... to here. nvs_close(_handle); _started = false; } @@ -65,7 +69,7 @@ void Preferences::end(){ * Clear all keys in opened preferences * */ -bool Preferences::clear(){ +bool Preferences::clear(){ // modified to add an nvs_commit() if(!_started || _readOnly){ return false; } @@ -74,14 +78,20 @@ bool Preferences::clear(){ log_e("nvs_erase_all fail: %s", nvs_error(err)); return false; } - return true; + // return true; // to undo changes: uncomment this line and... + err = nvs_commit(_handle); // ... delete the lines from from here... + if(err){ + log_e("nvs_commit fail: %s", nvs_error(err)); + return false; + } + return true; // ... to here. } /* * Remove a key * */ -bool Preferences::remove(const char * key){ +bool Preferences::remove(const char * key){ // modified to add an nvs_commit() if(!_started || !key || _readOnly){ return false; } @@ -90,7 +100,13 @@ bool Preferences::remove(const char * key){ log_e("nvs_erase_key fail: %s %s", key, nvs_error(err)); return false; } - return true; + // return true; // to undo changes: uncomment this line and... + err = nvs_commit(_handle); // ... delete the lines from from here... + if(err){ + log_e("nvs_commit fail: %s %s", key, nvs_error(err)); + return false; + } + return true; // ... to here. } /* From e49eae0679f6cae2b27316e6b3c6785e47eeaca2 Mon Sep 17 00:00:00 2001 From: beamholder Date: Mon, 5 Jul 2021 11:08:35 -0600 Subject: [PATCH 2/2] Remove comments as requested during review. --- libraries/Preferences/src/Preferences.cpp | 25 +++++++++-------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/libraries/Preferences/src/Preferences.cpp b/libraries/Preferences/src/Preferences.cpp index ae6b71eebaf..81397c80140 100644 --- a/libraries/Preferences/src/Preferences.cpp +++ b/libraries/Preferences/src/Preferences.cpp @@ -1,8 +1,9 @@ -// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD +// Copyright 2015-2021 Espressif Systems (Shanghai) PTE LTD // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at + // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software @@ -43,7 +44,7 @@ bool Preferences::begin(const char * name, bool readOnly, const char* partition_ } err = nvs_open_from_partition(partition_label, name, readOnly ? NVS_READONLY : NVS_READWRITE, &_handle); } else { - err = nvs_open(name, readOnly?NVS_READONLY:NVS_READWRITE, &_handle); + err = nvs_open(name, readOnly ? NVS_READONLY : NVS_READWRITE, &_handle); } if(err){ log_e("nvs_open failed: %s", nvs_error(err)); @@ -53,14 +54,10 @@ bool Preferences::begin(const char * name, bool readOnly, const char* partition_ return true; } -void Preferences::end(){ // modified to add an nvs_commit() +void Preferences::end(){ if(!_started){ return; } - esp_err_t err = nvs_commit(_handle); // to undo changes: delete the lines from here... - if(err){ - log_e("nvs_commit fail: %s %s", key, nvs_error(err)); - } // ... to here. nvs_close(_handle); _started = false; } @@ -69,7 +66,7 @@ void Preferences::end(){ // modified to * Clear all keys in opened preferences * */ -bool Preferences::clear(){ // modified to add an nvs_commit() +bool Preferences::clear(){ if(!_started || _readOnly){ return false; } @@ -78,20 +75,19 @@ bool Preferences::clear(){ // modified to add a log_e("nvs_erase_all fail: %s", nvs_error(err)); return false; } - // return true; // to undo changes: uncomment this line and... - err = nvs_commit(_handle); // ... delete the lines from from here... + err = nvs_commit(_handle); if(err){ log_e("nvs_commit fail: %s", nvs_error(err)); return false; } - return true; // ... to here. + return true; } /* * Remove a key * */ -bool Preferences::remove(const char * key){ // modified to add an nvs_commit() +bool Preferences::remove(const char * key){ if(!_started || !key || _readOnly){ return false; } @@ -100,13 +96,12 @@ bool Preferences::remove(const char * key){ // modified to add a log_e("nvs_erase_key fail: %s %s", key, nvs_error(err)); return false; } - // return true; // to undo changes: uncomment this line and... - err = nvs_commit(_handle); // ... delete the lines from from here... + err = nvs_commit(_handle); if(err){ log_e("nvs_commit fail: %s %s", key, nvs_error(err)); return false; } - return true; // ... to here. + return true; } /*