whitecatboard / Lua-RTOS-ESP32

Lua RTOS for ESP32
Other
1.19k stars 221 forks source link

nvs read behavior on missing namespace #256

Closed xopxe closed 5 years ago

xopxe commented 5 years ago

In the current version if do a nvs.read("nonexisting", "akey", 100) you get a

stdin:1: 261:key not found

error (which is confusing). We made the following change to nvs.c so we get the default value not only on missing key but also namespace. What do you think?

--- a/components/lua/modules/sys/nvs.c
+++ b/components/lua/modules/sys/nvs.c
@@ -213,13 +213,6 @@ static int l_nvs_read(lua_State *L) {
     // Open
     err = nvs_open(nspace, NVS_READONLY, &handle_to_settings);
     if (err != ESP_OK) {
-
-               if (err == ESP_ERR_NVS_NOT_FOUND && total == 3) {
-                       lua_pushvalue(L, 3);
-           return 1;
-               }
-
-
        nvs_error(L, err);
     }
the0ne commented 5 years ago

@xopxe can you create a PR for your change, so that it can be reviewed?

xopxe commented 5 years ago

PR here:

265

the0ne commented 5 years ago

@xopxe thanks a lot, looks great! @jolivepetrus do you see an impact on the wcb-ide?

jolivepetrus commented 5 years ago

@the0ne, @xopxe,

No, this PR has not impact in The Whitecat IDE. Can be merged without risk.

the0ne commented 5 years ago

@xopxe thanks for your PR! As it has been merged already I'm closing this issue.