From 849fcd6be8e0acc83b09e3d41ecddf50e9932dd5 Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Mon, 7 Aug 2023 23:46:27 +0200 Subject: [PATCH 1/5] ESP32/NVS: fix memory allocation Use term_binary_heap_size(size) macro in order to calculate required heap size for binary. Signed-off-by: Davide Bettio --- src/platforms/esp32/components/avm_builtins/nvs_nif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platforms/esp32/components/avm_builtins/nvs_nif.c b/src/platforms/esp32/components/avm_builtins/nvs_nif.c index 8f136a9a6..b61d0d4c6 100644 --- a/src/platforms/esp32/components/avm_builtins/nvs_nif.c +++ b/src/platforms/esp32/components/avm_builtins/nvs_nif.c @@ -84,7 +84,7 @@ static term nif_esp_nvs_get_binary(Context *ctx, int argc, term argv[]) RAISE_ERROR(term_from_int(err)); } - if (UNLIKELY(memory_ensure_free(ctx, size + BINARY_HEADER_SIZE) != MEMORY_GC_OK)) { + if (UNLIKELY(memory_ensure_free(ctx, term_binary_heap_size(size)) != MEMORY_GC_OK)) { TRACE("Unabled to ensure free space for binary. namespace='%s' key='%s' size=%i\n", namespace, key, size); RAISE_ERROR(OUT_OF_MEMORY_ATOM); } From 2025e40108b82da945b1ef85a5f5d7ff532155ba Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Tue, 8 Aug 2023 14:51:20 +0200 Subject: [PATCH 2/5] ESP32/NVS: add fetch that returns tagged tuples instead of just the result New fetch function returns either `{ok, result}` or `error`, instead of `binary()` or `undefined`. Signed-off-by: Davide Bettio --- libs/eavmlib/src/esp.erl | 22 ++++++++++- .../esp32/components/avm_builtins/nvs_nif.c | 39 ++++++++++++++----- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/libs/eavmlib/src/esp.erl b/libs/eavmlib/src/esp.erl index 2a872bb92..722c20466 100644 --- a/libs/eavmlib/src/esp.erl +++ b/libs/eavmlib/src/esp.erl @@ -32,6 +32,7 @@ wakeup_cause/0, sleep_enable_ext0_wakeup/2, sleep_enable_ext1_wakeup/2, + nvs_fetch_binary/2, nvs_get_binary/1, nvs_get_binary/2, nvs_get_binary/3, nvs_set_binary/2, nvs_set_binary/3, nvs_erase_key/1, nvs_erase_key/2, @@ -132,6 +133,21 @@ sleep_enable_ext0_wakeup(_Pin, _Level) -> sleep_enable_ext1_wakeup(_Mask, _Mode) -> erlang:nif_error(undefined). +%%----------------------------------------------------------------------------- +%% @param Namespace NVS namespace +%% @param Key NVS key +%% @returns tagged tuple with binary value associated with this key in NV +%% storage, {error, not_found} if there is no value associated with +%% this key, or in general {error, Reason} for any other error. +%% @doc Get the binary value associated with a key, or undefined, if +%% there is no value associated with this key. +%% @end +%%----------------------------------------------------------------------------- +-spec nvs_fetch_binary(Namespace :: atom(), Key :: atom()) -> + {ok, binary()} | {error, not_found} | {error, atom()}. +nvs_fetch_binary(Namespace, Key) when is_atom(Namespace) andalso is_atom(Key) -> + erlang:nif_error(undefined). + %%----------------------------------------------------------------------------- %% @doc Equivalent to nvs_get_binary(?ATOMVM_NVS_NS, Key). %% @end @@ -151,7 +167,11 @@ nvs_get_binary(Key) when is_atom(Key) -> %%----------------------------------------------------------------------------- -spec nvs_get_binary(Namespace :: atom(), Key :: atom()) -> binary() | undefined. nvs_get_binary(Namespace, Key) when is_atom(Namespace) andalso is_atom(Key) -> - erlang:nif_error(undefined). + case nvs_fetch_binary(Namespace, Key) of + {ok, Result} -> Result; + {errror, not_found} -> undefined; + {error, OtherError} -> throw(OtherError) + end. %%----------------------------------------------------------------------------- %% @param Namespace NVS namespace diff --git a/src/platforms/esp32/components/avm_builtins/nvs_nif.c b/src/platforms/esp32/components/avm_builtins/nvs_nif.c index b61d0d4c6..0ad9c27a5 100644 --- a/src/platforms/esp32/components/avm_builtins/nvs_nif.c +++ b/src/platforms/esp32/components/avm_builtins/nvs_nif.c @@ -63,12 +63,20 @@ static term nif_esp_nvs_get_binary(Context *ctx, int argc, term argv[]) switch (err) { case ESP_OK: break; - case ESP_ERR_NVS_NOT_FOUND: + case ESP_ERR_NVS_NOT_FOUND: { TRACE("No such namespace. namespace='%s'\n", namespace); - return UNDEFINED_ATOM; + if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { + RAISE_ERROR(OUT_OF_MEMORY_ATOM); + } + term error_tuple = term_alloc_tuple(2, &ctx->heap); + term ns_not_found = globalcontext_make_atom(ctx->global, ATOM_STR("\x13", "namespace_not_found")); + term_put_tuple_element(error_tuple, 0, OK_ATOM); + term_put_tuple_element(error_tuple, 1, ns_not_found); + return error_tuple; + } default: - fprintf(stderr, "Unable to open NVS. namespace '%s' err=%i\n", namespace, err); - RAISE_ERROR(term_from_int(err)); + TRACE("Unable to open NVS. namespace '%s' err=%i\n", namespace, err); + RAISE_ERROR(esp_err_to_term(ctx->global, err)); } size_t size = 0; @@ -76,15 +84,23 @@ static term nif_esp_nvs_get_binary(Context *ctx, int argc, term argv[]) switch (err) { case ESP_OK: break; - case ESP_ERR_NVS_NOT_FOUND: + case ESP_ERR_NVS_NOT_FOUND: { TRACE("No such entry. namespace='%s' key='%s'\n", namespace, key); - return UNDEFINED_ATOM; + if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { + RAISE_ERROR(OUT_OF_MEMORY_ATOM); + } + term error_tuple = term_alloc_tuple(2, &ctx->heap); + term not_found = globalcontext_make_atom(ctx->global, ATOM_STR("\x9", "not_found")); + term_put_tuple_element(error_tuple, 0, OK_ATOM); + term_put_tuple_element(error_tuple, 1, not_found); + return error_tuple; + } default: - fprintf(stderr, "Unable to get NVS blob size. namespace '%s' key='%s' err=%i\n", namespace, key, err); - RAISE_ERROR(term_from_int(err)); + TRACE("Unable to get NVS blob size. namespace '%s' key='%s' err=%i\n", namespace, key, err); + RAISE_ERROR(esp_err_to_term(ctx->global, err)); } - if (UNLIKELY(memory_ensure_free(ctx, term_binary_heap_size(size)) != MEMORY_GC_OK)) { + if (UNLIKELY(memory_ensure_free(ctx, term_binary_heap_size(size) + TUPLE_SIZE(2)) != MEMORY_GC_OK)) { TRACE("Unabled to ensure free space for binary. namespace='%s' key='%s' size=%i\n", namespace, key, size); RAISE_ERROR(OUT_OF_MEMORY_ATOM); } @@ -95,7 +111,10 @@ static term nif_esp_nvs_get_binary(Context *ctx, int argc, term argv[]) switch (err) { case ESP_OK: TRACE("Found data for key. namespace='%s' key='%s' size='%i'\n", namespace, key, size); - return binary; + term result_tuple = term_alloc_tuple(2, &ctx->heap); + term_put_tuple_element(result_tuple, 0, OK_ATOM); + term_put_tuple_element(result_tuple, 1, binary); + return result_tuple; default: fprintf(stderr, "Unable to get NVS blob. namespace='%s' key='%s' err=%i\n", namespace, key, err); RAISE_ERROR(term_from_int(err)); From b36fbbec11bc85c2c80bd8080eea43ce58d0c4cb Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Tue, 8 Aug 2023 21:19:46 +0200 Subject: [PATCH 3/5] ESP32/NVS: use `put` rather than `set` Right now there are `fetch` and `get` function, `put` name matches better current naming. Signed-off-by: Davide Bettio --- doc/src/network-programming-guide.md | 8 ++++---- libs/eavmlib/src/esp.erl | 20 +++++++++++++++++++ .../esp32/components/avm_builtins/nvs_nif.c | 10 +++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/doc/src/network-programming-guide.md b/doc/src/network-programming-guide.md index e11cb5956..016188345 100644 --- a/doc/src/network-programming-guide.md +++ b/doc/src/network-programming-guide.md @@ -231,13 +231,13 @@ If set in NVS storage, you may remove the corresponding `ssid` and `psk` paramet You can set these credentials once, as follows: - esp:nvs_set_binary(atomvm, sta_ssid, <<"myssid">>). - esp:nvs_set_binary(atomvm, sta_psk, <<"mypsk">>). + esp:nvs_put_binary(atomvm, sta_ssid, <<"myssid">>). + esp:nvs_put_binary(atomvm, sta_psk, <<"mypsk">>). or - esp:nvs_set_binary(atomvm, ap_ssid, <<"myssid">>). - esp:nvs_set_binary(atomvm, ap_psk, <<"mypsk">>). + esp:nvs_put_binary(atomvm, ap_ssid, <<"myssid">>). + esp:nvs_put_binary(atomvm, ap_psk, <<"mypsk">>). With these settings, you can run ESP programs that initialize the network without configuring your SSID and PSK explicitly in source code. diff --git a/libs/eavmlib/src/esp.erl b/libs/eavmlib/src/esp.erl index 722c20466..9d197a064 100644 --- a/libs/eavmlib/src/esp.erl +++ b/libs/eavmlib/src/esp.erl @@ -35,6 +35,7 @@ nvs_fetch_binary/2, nvs_get_binary/1, nvs_get_binary/2, nvs_get_binary/3, nvs_set_binary/2, nvs_set_binary/3, + nvs_put_binary/3, nvs_erase_key/1, nvs_erase_key/2, nvs_erase_all/0, nvs_erase_all/1, nvs_reformat/0, @@ -45,6 +46,8 @@ get_mac/1 ]). +-deprecated([{nvs_set_binary, 2, next_version}, {nvs_set_binary, 3, next_version}]). + -type esp_reset_reason() :: esp_rst_unknown | esp_rst_poweron @@ -197,6 +200,7 @@ nvs_get_binary(Namespace, Key, Default) when %%----------------------------------------------------------------------------- %% @doc Equivalent to nvs_set_binary(?ATOMVM_NVS_NS, Key, Value). +%% @deprecated Please use nvs_put_binary instead. %% @end %%----------------------------------------------------------------------------- -spec nvs_set_binary(Key :: atom(), Value :: binary()) -> ok. @@ -210,11 +214,27 @@ nvs_set_binary(Key, Value) when is_atom(Key) andalso is_binary(Value) -> %% @returns ok %% @doc Set an binary value associated with a key. If a value exists %% for the specified key, it is over-written. +%% @deprecated Please use nvs_put_binary instead. %% @end %%----------------------------------------------------------------------------- -spec nvs_set_binary(Namespace :: atom(), Key :: atom(), Value :: binary()) -> ok. nvs_set_binary(Namespace, Key, Value) when is_atom(Namespace) andalso is_atom(Key) andalso is_binary(Value) +-> + nvs_put_binary(Namespace, Key, Value). + +%%----------------------------------------------------------------------------- +%% @param Namespace NVS namespace +%% @param Key NVS key +%% @param Value binary value +%% @returns ok +%% @doc Set an binary value associated with a key. If a value exists +%% for the specified key, it is over-written. +%% @end +%%----------------------------------------------------------------------------- +-spec nvs_put_binary(Namespace :: atom(), Key :: atom(), Value :: binary()) -> ok. +nvs_put_binary(Namespace, Key, Value) when + is_atom(Namespace) andalso is_atom(Key) andalso is_binary(Value) -> erlang:nif_error(undefined). diff --git a/src/platforms/esp32/components/avm_builtins/nvs_nif.c b/src/platforms/esp32/components/avm_builtins/nvs_nif.c index 0ad9c27a5..d0c3d33bb 100644 --- a/src/platforms/esp32/components/avm_builtins/nvs_nif.c +++ b/src/platforms/esp32/components/avm_builtins/nvs_nif.c @@ -121,7 +121,7 @@ static term nif_esp_nvs_get_binary(Context *ctx, int argc, term argv[]) } } -static term nif_esp_nvs_set_binary(Context *ctx, int argc, term argv[]) +static term nif_esp_nvs_put_binary(Context *ctx, int argc, term argv[]) { UNUSED(argc); VALIDATE_VALUE(argv[0], term_is_atom); @@ -260,9 +260,9 @@ static const struct Nif esp_nvs_get_binary_nif = { .base.type = NIFFunctionType, .nif_ptr = nif_esp_nvs_get_binary }; -static const struct Nif esp_nvs_set_binary_nif = { +static const struct Nif esp_nvs_put_binary_nif = { .base.type = NIFFunctionType, - .nif_ptr = nif_esp_nvs_set_binary + .nif_ptr = nif_esp_nvs_put_binary }; static const struct Nif esp_nvs_erase_key_nif = { .base.type = NIFFunctionType, @@ -294,9 +294,9 @@ const struct Nif *nvs_nif_get_nif(const char *nifname) TRACE("Resolved platform nif %s ...\n", nifname); return &esp_nvs_get_binary_nif; } - if (strcmp("esp:nvs_set_binary/3", nifname) == 0) { + if (strcmp("esp:nvs_put_binary/3", nifname) == 0) { TRACE("Resolved platform nif %s ...\n", nifname); - return &esp_nvs_set_binary_nif; + return &esp_nvs_put_binary_nif; } if (strcmp("esp:nvs_erase_key/2", nifname) == 0) { TRACE("Resolved platform nif %s ...\n", nifname); From 4a69949083d1f27080403cc5080e23671f20c6b3 Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Tue, 8 Aug 2023 21:23:46 +0200 Subject: [PATCH 4/5] ESP32/NVS: deprecate NVS functions that default to ?ATOMVM_NVS_NS Let's explicitly mention the namespace, rather having a default one, so applications are encouraged using their own namespace, rather a kitchen sink one. Signed-off-by: Davide Bettio --- libs/eavmlib/src/esp.erl | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libs/eavmlib/src/esp.erl b/libs/eavmlib/src/esp.erl index 9d197a064..a8d765a76 100644 --- a/libs/eavmlib/src/esp.erl +++ b/libs/eavmlib/src/esp.erl @@ -46,7 +46,13 @@ get_mac/1 ]). --deprecated([{nvs_set_binary, 2, next_version}, {nvs_set_binary, 3, next_version}]). +-deprecated([ + {nvs_get_binary, 1, next_version}, + {nvs_erase_key, 1, next_version}, + {nvs_erase_all, 0, next_version}, + {nvs_set_binary, 2, next_version}, + {nvs_set_binary, 3, next_version} +]). -type esp_reset_reason() :: esp_rst_unknown @@ -153,6 +159,7 @@ nvs_fetch_binary(Namespace, Key) when is_atom(Namespace) andalso is_atom(Key) -> %%----------------------------------------------------------------------------- %% @doc Equivalent to nvs_get_binary(?ATOMVM_NVS_NS, Key). +%% @deprecated Please do not use this function. %% @end %%----------------------------------------------------------------------------- -spec nvs_get_binary(Key :: atom()) -> binary() | undefined. @@ -242,6 +249,7 @@ nvs_put_binary(Namespace, Key, Value) when %% @param Key NVS key %% @returns ok %% @doc Equivalent to nvs_erase_key(?ATOMVM_NVS_NS, Key). +%% @deprecated Please do not use this function. %% @end %%----------------------------------------------------------------------------- -spec nvs_erase_key(Key :: atom()) -> ok. @@ -262,6 +270,7 @@ nvs_erase_key(Namespace, Key) when is_atom(Namespace) andalso is_atom(Key) -> %%----------------------------------------------------------------------------- %% @doc Equivalent to nvs_erase_all(?ATOMVM_NVS_NS). +%% @deprecated Please do not use this function. %% @end %%----------------------------------------------------------------------------- -spec nvs_erase_all() -> ok. From db31c660de3d6238e9654eda9b0bb7ec95a2357d Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Wed, 9 Aug 2023 13:00:51 +0200 Subject: [PATCH 5/5] CHANGELOG.md: add ESP32 NVS changes Signed-off-by: Davide Bettio --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d614b097..3151349c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for nodejs with Wasm - Added support for a subset of the OTP logger interface - Added `esp:partition_list/0` function +- Added `esp:nvs_fetch_binary/2` and `nvs_put_binary/3` functions (`esp:nvs_set_binary` and +functions that default to `?ATOMVM_NVS_NS` are deprecated now). ### Fixed - Fixed issue with formatting integers with io:format() on STM32 platform