Add new function to obtain connection RSSI info #1138

UncleGrumpy · 2024-04-23T19:43:10Z

These changes add support for both ESP32 and Pico-W to obtain the current signal strength (in decibels) of the connection to the associated access point using network:sta_rssi/0.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

petermm · 2024-04-23T20:50:19Z

Nice, I wonder if an percentage signal_percent should be added, similar to nerves networking: https://github.com/nerves-networking/vintage_net_wifi/blob/4496d2b86166c4f0255a5a9d8e5f9ab5a9d1feef/lib/vintage_net_wifi/signal_info.ex#L21

see dbm_to_percent:
https://github.com/nerves-networking/vintage_net_wifi/blob/main/lib/vintage_net_wifi/utils.ex#L17

eg. it's somewhat hard to reason between -25 or -45 which is better and are any of them of marginal quality?

UncleGrumpy · 2024-04-24T00:37:39Z

With RF signals a smaller deviation from the theoretical perfect 0 represents a stronger signal to noise ratio, the point where the network becomes unusable varies by equipment and environmental factors, but typically below -70 the network will be unusable... in fact the esp-idf has a default lower limit where it will terminate the connection and emit a network disconnected event. An interesting future feature to add would be the ability to adjust the RSSI threshold.

The ESP-IDF documentation and the function name itself is misleading... it actually returns the dBm of the signal, not a true RSSI, but I stuck with the IDF naming so that it would be familiar. The same is true for Pico-SDK.

pguyot · 2024-05-14T14:41:43Z

libs/eavmlib/src/network.erl

+%% @doc     Get the rssi information of AP to which the device is associated with.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec sta_rssi() -> Rssi :: db() | {error, Reason :: term()}.


I'd rather have {ok, db()} than just db(). It's easier to match.

pguyot · 2024-05-14T14:43:18Z

src/platforms/esp32/components/avm_builtins/network_driver.c

-        term config = term_get_tuple_element(cmd, 1);
+        if (term_is_atom(cmd)) {
+            cmd_term = cmd;
+            UNUSED(config);


I'm not sure what this UNUSED is here for.

pguyot · 2024-05-14T14:44:09Z

src/platforms/esp32/components/avm_builtins/network_driver.c

@@ -761,6 +763,32 @@ static void start_network(Context *ctx, term pid, term ref, term config)
    port_send_reply(ctx, pid, ref, OK_ATOM);
 }

+static void get_sta_rssi(Context *ctx, term pid, term ref)
+{
+    int sta_rssi = -999;


0 seems a better way to silence esp-idf compiler.

Indeed. That makes sense, I had set it to 0 originally, but changed that to an impossible value during early testing to be sure the value was being changed, because 0 being returned is an indication of error, even if the actual error returned is ESP_OK.

pguyot · 2024-05-14T14:47:01Z

src/platforms/rp2040/src/lib/networkdriver.c

@@ -31,6 +31,7 @@
 #pragma GCC diagnostic ignored "-Wpedantic"

 #include <cyw43.h>
+#include <cyw43_ll.h>


Instead of including this header to access low-level interface (what ll stands for I believe) because code needs CYW43_IOCTL_GET_RSSI, get_sta_rssi probably could call cyw43_wifi_get_rssi that is defined in cyw43.h.

I don't know how I missed that! Thanks!

pguyot · 2024-05-14T14:49:30Z

src/platforms/rp2040/src/lib/networkdriver.c

+        return;
+    }
+    // {Ref, {rssi, -25}}
+    BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE + TUPLE_SIZE(2) + sizeof(int), heap);


See comments on esp32 driver.

pguyot · 2024-05-14T14:50:20Z

src/platforms/rp2040/src/lib/networkdriver.c

+    if ((term_is_tuple(cmd) && term_get_tuple_arity(cmd) == 2) || term_is_atom(cmd)) {
+        if (term_is_atom(cmd)) {
+            cmd_term = cmd;
+            UNUSED(config);


See comments on esp32 driver.

pguyot · 2024-05-14T14:50:30Z

src/platforms/rp2040/src/lib/networkdriver.c

-        enum network_cmd cmd = interop_atom_term_select_int(cmd_table, cmd_term, ctx->global);
-        switch (cmd) {
-            case NetworkStartCmd:
+        enum network_cmd command = interop_atom_term_select_int(cmd_table, cmd_term, ctx->global);


Thank you for this.

pguyot · 2024-05-14T14:51:11Z

src/platforms/esp32/components/avm_builtins/network_driver.c

+        return;
+    }
+    // {Ref, {rssi, -25}}
+    BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE + TUPLE_SIZE(2) + sizeof(int), heap);


The message is {reference(), {rssi, integer()}, so the required size is PORT_REPLY_SIZE + TUPLE_SIZE(2).

Here, the function could also use port_ensure_available. The current code (a) allocates sizeof(int) which does not make sense, (b) builds a tuple on a stack heap and then another one on the context heap (port_send_reply does create a tuple), without ensuring there is space in the context heap : it may crash.

pguyot · 2024-05-14T14:52:26Z

src/platforms/esp32/components/avm_builtins/network_driver.c

+    esp_err_t err = esp_wifi_sta_get_rssi(&sta_rssi);
+    if (UNLIKELY(err != ESP_OK)) {
+        size_t heap_size = TUPLE_SIZE(2);
+        if (UNLIKELY(memory_ensure_free(ctx, heap_size) != MEMORY_GC_OK)) {


Reviewing port.h and port.c, I think :

function get_sta_rssi takes a Context *ctx as a parameter, so it should only be called directly from consume_mailbox function (which it is)

as a result, it can and probably should use the ctx' heap to send messages

this function should call port_ensure_available instead of memory_ensure_free, but this doesn't hurt. port_ensure_available doesn't GC, but the function is not referring to anything that could be moved by GC. The driver already called memory_ensure_free so I think it's ok to keep that.

the actual message is {reference(), {error, integer()}}, so heap_size should be TUPLE_SIZE(2) + REF_SIZE + TUPLE_SIZE(2) or PORT_REPLY_SIZE + TUPLE_SIZE(2) (PORT_REPLY_SIZE probably should eventually go to port.h).

pguyot · 2024-05-14T14:52:42Z

src/platforms/rp2040/src/lib/networkdriver.c

+
+    if (UNLIKELY(err != 0)) {
+        size_t heap_size = TUPLE_SIZE(2);
+        if (UNLIKELY(memory_ensure_free(ctx, heap_size) != MEMORY_GC_OK)) {


See comments on esp32 driver.

Adds the ability to get the signal strength, in decibels, of the connection to the access point the device is connected to. Signed-off-by: Winford <winford@object.stream>

Adds the ability to get the signal strength, in decibels, of the connection to the access point the pico-w is connected to. Signed-off-by: Winford <winford@object.stream>

UncleGrumpy requested a review from pguyot April 23, 2024 19:44

pguyot requested changes May 14, 2024

View reviewed changes

Add network:sta_rssi/0 to ESP32 network driver

487263a

Adds the ability to get the signal strength, in decibels, of the connection to the access point the device is connected to. Signed-off-by: Winford <winford@object.stream>

UncleGrumpy force-pushed the rssi branch from 4f527fa to 2e41ba2 Compare May 16, 2024 23:20

UncleGrumpy requested a review from pguyot May 17, 2024 19:17

Add support for network:sta_rssi/0 to pico-w network driver

b4fe0a6

Adds the ability to get the signal strength, in decibels, of the connection to the access point the pico-w is connected to. Signed-off-by: Winford <winford@object.stream>

UncleGrumpy force-pushed the rssi branch from 2e41ba2 to b4fe0a6 Compare May 17, 2024 19:20

pguyot approved these changes May 19, 2024

View reviewed changes

bettio approved these changes May 20, 2024

View reviewed changes

bettio merged commit aad2d60 into atomvm:release-0.6 May 20, 2024
87 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new function to obtain connection RSSI info #1138

Add new function to obtain connection RSSI info #1138

UncleGrumpy commented Apr 23, 2024

petermm commented Apr 23, 2024

UncleGrumpy commented Apr 24, 2024 •

edited

pguyot May 14, 2024

pguyot May 14, 2024

pguyot May 14, 2024

UncleGrumpy May 14, 2024

pguyot May 14, 2024

UncleGrumpy May 15, 2024

pguyot May 14, 2024

pguyot May 14, 2024

pguyot May 14, 2024

pguyot May 14, 2024

pguyot May 14, 2024

pguyot May 14, 2024

Add new function to obtain connection RSSI info #1138

Add new function to obtain connection RSSI info #1138

Conversation

UncleGrumpy commented Apr 23, 2024

petermm commented Apr 23, 2024

UncleGrumpy commented Apr 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UncleGrumpy commented Apr 24, 2024 •

edited