fix(nvs): std::string reads no longer include the zero terminator in size() (#854)

components/nvs/example/README.md

@@ -78,3 +78,38 @@ Restarting now.
 Restart counter will increment on each run.
 
 To reset the counter, erase the contents of flash memory using `idf.py erase-flash`, then upload the program again as described above.
+
+## NVS Validation Output
+
+The last section of the example exercises the NVS operations (integer, bool,
+float and string round trips through `get_or_set_var` / `set_var` / `get_var`,
+key and namespace erase) with explicit pass / fail checks in a `validate`
+namespace that is erased before and after, so the same sequence runs on every
+boot. Each check prints its own line and a summary closes the section:
+
+```
+Validating NVS operations...
+  [PASS] uint8 get_or_set_var on a missing key returns the default
+  [PASS] uint8 default was stored
+  [PASS] int32 set_var / get_var round trip
+  [PASS] bool set_var / get_var round trip
+  [PASS] float set_var / get_var round trip
+  [PASS] string get_or_set_var on a missing key returns the default
+  [PASS] string get_or_set_var on an existing key returns the stored value
+  [PASS] string set_var / get_var round trip
+  [PASS] shorter string overwrite reads back exactly
+  [PASS] empty string set_var / get_var round trip
+  [PASS] 64 character string round trip
+  [PASS] string c_str() has no embedded terminator
+  [PASS] erase key succeeds
+  [PASS] erased key is not found
+  [PASS] other keys survive erasing one key
+  [PASS] reading a missing string key fails and leaves the value alone
+  [PASS] erase namespace succeeds
+  [PASS] keys are not found after erasing the namespace
+NVS validation: 18/18 checks passed - PASS
+```
+
+Any failing check is listed again after the summary line. The string checks
+assert that `size()` is the stored string's length (no zero terminator in the
+value, see [#854](https://github.com/esp-cpp/espp/issues/854)).

components/nvs/example/main/nvs_example.cpp

@@ -1,5 +1,7 @@
 #include <bitset>
 #include <chrono>
+#include <cstring>
+#include <string>
 #include <thread>
 #include <vector>
 
@@ -88,7 +90,7 @@ extern "C" void app_main(void) {
     if (ec) {
       fmt::print("Error: {}\n", ec.message());
     } else {
-      fmt::print("String = '{}'\n", str);
+      fmt::print("String = '{}' ({} bytes)\n", str, str.size());
     }
     ec.clear();
 
@@ -107,7 +109,7 @@ extern "C" void app_main(void) {
     if (ec) {
       fmt::print("Error: {}\n", ec.message());
     } else {
-      fmt::print("String = '{}'\n", str);
+      fmt::print("String = '{}' ({} bytes)\n", str, str.size());
     }
     ec.clear();
 
@@ -209,6 +211,157 @@ extern "C" void app_main(void) {
     //! [nvshandle example]
   }
 
+  {
+    //! [nvs validation]
+    // Validate the NVS operations above with explicit pass / fail checks and
+    // print a summary at the end. The checks run in their own namespace, which
+    // is erased before and after, so every boot runs the same sequence.
+    std::error_code ec;
+    espp::Nvs nvs;
+    nvs.init(ec);
+    ec.clear();
+
+    int passed = 0;
+    int failed = 0;
+    std::vector<std::string> failures;
+    auto check = [&](const char *name, bool ok) {
+      fmt::print("  [{}] {}\n", ok ? "PASS" : "FAIL", name);
+      if (ok) {
+        passed++;
+      } else {
+        failed++;
+        failures.emplace_back(name);
+      }
+    };
+    // a key is "missing" when reading it fails with Key_Not_Found
+    auto key_missing = [&](const char *key) {
+      std::error_code read_ec;
+      int32_t dummy = 0;
+      nvs.get_var("validate", key, dummy, read_ec);
+      return read_ec && read_ec.value() == static_cast<int>(NvsErrc::Key_Not_Found);
+    };
+    auto has_nul = [](const std::string &s) { return s.find('\0') != std::string::npos; };
+
+    fmt::print("\nValidating NVS operations...\n");
+    // start from a clean namespace (erasing a namespace that does not exist is
+    // fine - the first boot after a flash erase gets here with nothing stored)
+    nvs.erase("validate", ec);
+    ec.clear();
+
+    // integers: get_or_set_var on a missing key stores and returns the default
+    uint8_t u8 = 0;
+    nvs.get_or_set_var("validate", "u8", u8, uint8_t{42}, ec);
+    check("uint8 get_or_set_var on a missing key returns the default", !ec && u8 == 42);
+    ec.clear();
+    u8 = 0;
+    nvs.get_var("validate", "u8", u8, ec);
+    check("uint8 default was stored", !ec && u8 == 42);
+    ec.clear();
+
+    int32_t i32 = 0;
+    nvs.set_var("validate", "i32", int32_t{-123456}, ec);
+    nvs.get_var("validate", "i32", i32, ec);
+    check("int32 set_var / get_var round trip", !ec && i32 == -123456);
+    ec.clear();
+
+    // bool: set_var / get_var round trip in both directions
+    bool b = false;
+    nvs.set_var("validate", "bool", true, ec);
+    nvs.get_var("validate", "bool", b, ec);
+    bool bool_true_ok = !ec && b;
+    nvs.set_var("validate", "bool", false, ec);
+    nvs.get_var("validate", "bool", b, ec);
+    check("bool set_var / get_var round trip", bool_true_ok && !ec && !b);
+    ec.clear();
+
+    // float: stored bit-exact
+    float f = 0.0f;
+    nvs.set_var("validate", "float", 3.14159f, ec);
+    nvs.get_var("validate", "float", f, ec);
+    check("float set_var / get_var round trip", !ec && f == 3.14159f);
+    ec.clear();
+
+    // strings: get_or_set_var on a missing key stores and returns the default,
+    // and size() is the string's length (no zero terminator in the value)
+    std::string s;
+    nvs.get_or_set_var("validate", "str", s, std::string("default"), ec);
+    check("string get_or_set_var on a missing key returns the default",
+          !ec && s == "default" && !has_nul(s));
+    ec.clear();
+
+    // get_or_set_var on an existing key returns the stored value, not the default
+    s.clear();
+    nvs.get_or_set_var("validate", "str", s, std::string("other"), ec);
+    check("string get_or_set_var on an existing key returns the stored value",
+          !ec && s == "default" && !has_nul(s));
+    ec.clear();
+
+    // set_var / get_var round trip: equal, exact size, last character intact
+    nvs.set_var("validate", "str", std::string("hello world"), ec);
+    s.clear();
+    nvs.get_var("validate", "str", s, ec);
+    check("string set_var / get_var round trip",
+          !ec && s == "hello world" && s.back() == 'd' && !has_nul(s));
+    ec.clear();
+
+    // overwriting with a shorter string: the read is the new string only,
+    // even when the destination already holds something longer
+    nvs.set_var("validate", "str", std::string("hi"), ec);
+    s = std::string(32, 'x');
+    nvs.get_var("validate", "str", s, ec);
+    check("shorter string overwrite reads back exactly", !ec && s == "hi");
+    ec.clear();
+
+    // an empty string is stored and read back as empty
+    nvs.set_var("validate", "str", std::string(""), ec);
+    s = "not empty";
+    nvs.get_var("validate", "str", s, ec);
+    check("empty string set_var / get_var round trip", !ec && s.empty());
+    ec.clear();
+
+    // the longest value a string key holds comfortably (NVS strings are limited
+    // by the page size; 64 characters is well inside that)
+    std::string long_str(64, 'a');
+    nvs.set_var("validate", "str", long_str, ec);
+    s.clear();
+    nvs.get_var("validate", "str", s, ec);
+    check("64 character string round trip", !ec && s == long_str && s.size() == 64);
+    ec.clear();
+
+    // c-string comparison: the stored string ends where the value ends
+    check("string c_str() has no embedded terminator", strlen(s.c_str()) == s.size());
+
+    // erasing a key: the key is gone, the others survive
+    nvs.erase("validate", "str", ec);
+    check("erase key succeeds", !ec);
+    ec.clear();
+    check("erased key is not found", key_missing("str"));
+    nvs.get_var("validate", "i32", i32, ec);
+    check("other keys survive erasing one key", !ec && i32 == -123456);
+    ec.clear();
+
+    // reading a missing key reports Key_Not_Found and leaves the value alone
+    s = "untouched";
+    nvs.get_var("validate", "missing", s, ec);
+    check("reading a missing string key fails and leaves the value alone", ec && s == "untouched");
+    ec.clear();
+
+    // erasing the namespace: everything is gone
+    nvs.erase("validate", ec);
+    check("erase namespace succeeds", !ec);
+    ec.clear();
+    check("keys are not found after erasing the namespace",
+          key_missing("u8") && key_missing("i32") && key_missing("bool"));
+
+    fmt::print("NVS validation: {}/{} checks passed - {}\n", passed, passed + failed,
+               failed == 0 ? "PASS" : "FAIL");
+    for (const auto &name : failures) {
+      fmt::print("  failed: {}\n", name);
+    }
+    fflush(stdout);
+    //! [nvs validation]
+  }
+
   while (true) {
     std::this_thread::sleep_for(1s);
   }

components/nvs/include/nvs_handle_espp.hpp

@@ -164,7 +164,8 @@ class NvsHandle : public BaseComponent {
 
   /// @brief Reads a string from the NVS
   /// @param[in] key NVS Key of the string to read
-  /// @param[out] value string to read
+  /// @param[out] value string to read; its size() is the stored string's length
+  ///             (the zero terminator NVS keeps with it is not part of the value)
   /// @param[out] ec Saves a std::error_code representing success or failure
   void get(const char *key, std::string &value, std::error_code &ec) {
     if (!check_handle_initialized(ec))
@@ -188,6 +189,9 @@ class NvsHandle : public BaseComponent {
       logger_.error("Error {} reading from NVS!", esp_err_to_name(err));
       return;
     }
+    // get_item_size() counts the zero terminator for strings and get_string()
+    // writes it: drop it, so value.size() is the stored string's length
+    value.resize(len > 0 ? len - 1 : 0);
     return;
   }
 
@@ -295,7 +299,8 @@ class NvsHandle : public BaseComponent {
 
   /// @brief Reads a string from the NVS
   /// @param[in] key NVS Key of the string to read
-  /// @param[out] value string to read
+  /// @param[out] value string to read; its size() is the stored string's length
+  ///             (the zero terminator NVS keeps with it is not part of the value)
   /// @param[in] default_value Default value to return if key is not found
   /// @param[out] ec Saves a std::error_code representing success or failure
   void get(const char *key, std::string &value, const std::string &default_value,
@@ -323,6 +328,9 @@ class NvsHandle : public BaseComponent {
         logger_.error("Error {} reading from NVS!", esp_err_to_name(err));
         return;
       }
+      // get_item_size() counts the zero terminator for strings and get_string()
+      // writes it: drop it, so value.size() is the stored string's length
+      value.resize(len - 1);
     }
   }