Set inique dhcp_hostname, or configured name on pico_w

Correctly set `dhcp_hostname` from `sta_config_options()` and/or `ap_config_options()`, or, if
undefined, sets a unique hostname to the default device name of `atomvm-${DEVICE_MAC}`. Formerly
all pico_w devices tried to register to an access point with the factory default `PicoW` hostname,
which would make all subsequent pico devices to connect to an access point, after the first one,
unreachable by hostname, and cause unpredictable problems for some routers.

Fixes #1094 on release-0.6 branch

IMPORTANT: DO NOT FORWARD this commit to main branch. Breaking changes for rp2 (formerly rp2040)
platform necessitate a seaprate PR to fix this issue for release-0.6, a fix has already been
submitted to main branch (PR #1173).

Signed-off-by: Winford <[email protected]>

Author: UncleGrumpy

CHANGELOG.md

@@ -16,6 +16,7 @@ with nodejs and emscripten)
 - Added `uart:read/2` with a timeout parameter.
 - Missing `erlang:is_function/2` BIF
 - Added `erlang:is_record/2`
+- Added ability to set per-interface `dhcp_hostname` on Pico W if present in config.
 
 ### Fixed
 
@@ -67,6 +68,9 @@ memory error
 - Fixed nif_atomvm_posix_read GC bug
 - Fixed `erlang:is_number/1` function, now returns true also for floats
 - Fixed unlink protocol and add support for `link/1` on ports
+- Correctly set Pico-W unique dhcp hostname when using the default, previously all rp2040 devices
+used the same "PicoW" dhcp hostname, causing collisions when multiple rp2040 are on the same
+network. (See issue #1094)
 
 ### Changed
 

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

@@ -35,16 +35,20 @@
 #include <hardware/rtc.h>
 #include <lwip/apps/sntp.h>
 #include <pico/cyw43_arch.h>
+#include <string.h>
 
 #pragma GCC diagnostic pop
 
 #define PORT_REPLY_SIZE (TUPLE_SIZE(2) + REF_SIZE)
+#define DEFAULT_HOSTNAME_FMT "atomvm-%02x%02x%02x%02x%02x%02x"
+#define DEFAULT_HOSTNAME_SIZE (strlen("atomvm-") + 12 + 1)
 
 static const char *const ap_atom = ATOM_STR("\x2", "ap");
 static const char *const ap_channel_atom = ATOM_STR("\xA", "ap_channel");
 static const char *const ap_sta_connected_atom = ATOM_STR("\x10", "ap_sta_connected");
 static const char *const ap_sta_disconnected_atom = ATOM_STR("\x13", "ap_sta_disconnected");
 static const char *const ap_started_atom = ATOM_STR("\xA", "ap_started");
+static const char *const dhcp_hostname_atom = ATOM_STR("\xD", "dhcp_hostname");
 static const char *const host_atom = ATOM_STR("\x4", "host");
 static const char *const psk_atom = ATOM_STR("\x3", "psk");
 static const char *const sntp_atom = ATOM_STR("\x4", "sntp");
@@ -79,6 +83,8 @@ struct NetworkDriverData
     int stas_count;
     uint8_t *stas_mac;
     struct dhcp_config *dhcp_config;
+    char *hostname;
+    char *ap_hostname;
     queue_t queue;
 };
 
@@ -100,6 +106,14 @@ struct NetworkDriverEvent
     };
 };
 
+enum DriverErrorCodeType
+{
+    DriverOK,
+    DriverBADARG,
+    DriverMACError,
+    DriverOOM
+};
+
 // Callbacks do not allow for user data
 // netif->state is actually pointing to &cyw43_state
 static struct NetworkDriverData *driver_data;
@@ -120,6 +134,26 @@ static term tuple_from_addr(Heap *heap, uint32_t addr)
     return port_heap_create_tuple_n(heap, 4, terms);
 }
 
+static term error_code_to_term(int error, GlobalContext *global)
+{
+    switch (error) {
+        case DriverOK:
+            return OK_ATOM;
+            break;
+        case DriverBADARG:
+            return BADARG_ATOM;
+            break;
+        case DriverMACError:
+            return globalcontext_make_atom(global, ATOM_STR("\x10", "device_mac_error"));
+            break;
+        case DriverOOM:
+            return OUT_OF_MEMORY_ATOM;
+            break;
+        default:
+            return BADARG_ATOM;
+    }
+}
+
 static void send_term(Heap *heap, term t)
 {
     term ref = term_from_ref_ticks(driver_data->ref_ticks, heap);
@@ -211,15 +245,62 @@ static void send_sntp_sync(struct timeval *tv)
     END_WITH_STACK_HEAP(heap, driver_data->global);
 }
 
+static enum DriverErrorCodeType write_default_device_name(size_t size, char **out)
+{
+    uint8_t mac[6];
+    // Device name is used for AP mode ssid (if undefined), and for the
+    // default dhcp_hostname on both interfaces.  It seems the interface
+    // parameter is ignored and both interfaces have the same MAC address.
+    int err = cyw43_wifi_get_mac(&cyw43_state, CYW43_ITF_STA, mac);
+    if (UNLIKELY(err)) {
+        return DriverMACError;
+    }
+    *out = malloc(size);
+    if (IS_NULL_PTR(out)) {
+        return DriverOOM;
+    }
+    snprintf(*out, size,
+        DEFAULT_HOSTNAME_FMT, mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
+    //*out = strndup(buf, size);
+    //free(buf);
+    return DriverOK;
+}
+
+static enum DriverErrorCodeType set_interface_dhcp_name(term dhcp_name, char **out)
+{
+    if (term_is_invalid_term(dhcp_name)) {
+        enum DriverErrorCodeType ok_ret = write_default_device_name(DEFAULT_HOSTNAME_SIZE, out);
+        if (UNLIKELY(ok_ret != DriverOK)) {
+            free(out);
+            cyw43_arch_disable_sta_mode();
+            return ok_ret;
+        }
+    } else {
+        int ok = 0;
+        *out = interop_term_to_string(dhcp_name, &ok);
+        if (!ok || IS_NULL_PTR(out)) {
+            if (out != NULL) {
+                free(out);
+                cyw43_arch_disable_sta_mode();
+                return DriverBADARG;
+            }
+            cyw43_arch_disable_sta_mode();
+            return DriverOOM;
+        }
+    }
+    return DriverOK;
+}
+
 static term start_sta(term sta_config, GlobalContext *global)
 {
     term ssid_term = interop_kv_get_value(sta_config, ssid_atom, global);
     term pass_term = interop_kv_get_value(sta_config, psk_atom, global);
+    term hostname_term = interop_kv_get_value(sta_config, dhcp_hostname_atom, global);
 
     //
     // Check parameters
     //
-    if (term_is_invalid_term(ssid_term)) {
+    if (UNLIKELY(term_is_invalid_term(ssid_term))) {
         return BADARG_ATOM;
     }
     int ok = 0;
@@ -230,13 +311,21 @@ static term start_sta(term sta_config, GlobalContext *global)
     char *psk = NULL;
     if (!term_is_invalid_term(pass_term)) {
         psk = interop_term_to_string(pass_term, &ok);
-        if (!ok) {
+        if (UNLIKELY(!ok)) {
             free(ssid);
             return BADARG_ATOM;
         }
     }
 
-    cyw43_arch_enable_sta_mode();
+    enum DriverErrorCodeType ret = set_interface_dhcp_name(hostname_term, &driver_data->hostname);
+    if (UNLIKELY(ret != DriverOK)) {
+        free(ssid);
+        free(psk);
+        return error_code_to_term(ret, global);
+    }
+
+    netif_set_hostname(&cyw43_state.netif[CYW43_ITF_STA], driver_data->hostname);
+
     uint32_t auth = (psk == NULL) ? CYW43_AUTH_OPEN : CYW43_AUTH_WPA2_MIXED_PSK;
     int result = cyw43_arch_wifi_connect_async(ssid, psk, auth);
     // We need to set the callback after calling connect async because it's
@@ -247,32 +336,14 @@ static term start_sta(term sta_config, GlobalContext *global)
     free(ssid);
     free(psk);
     if (result != 0) {
+        free(driver_data->hostname);
+        cyw43_arch_disable_sta_mode();
         return BADARG_ATOM;
     }
 
     return OK_ATOM;
 }
 
-static char *get_default_device_name()
-{
-    uint8_t mac[6];
-    // Device name is used for AP mode. It seems the interface parameter is
-    // ignored and both interfaces have the same MAC address.
-    int err = cyw43_wifi_get_mac(&cyw43_state, CYW43_ITF_AP, mac);
-    if (err) {
-        return NULL;
-    }
-
-    size_t buf_size = strlen("atomvm-") + 12 + 1;
-    char *buf = malloc(buf_size);
-    if (IS_NULL_PTR(buf)) {
-        return NULL;
-    }
-    snprintf(buf, buf_size,
-        "atomvm-%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
-    return buf;
-}
-
 static void network_driver_do_cyw43_assoc(GlobalContext *glb)
 {
     int max_stas;
@@ -384,13 +455,18 @@ static term start_ap(term ap_config, GlobalContext *global)
     term ssid_term = interop_kv_get_value(ap_config, ssid_atom, global);
     term pass_term = interop_kv_get_value(ap_config, psk_atom, global);
     term channel_term = interop_kv_get_value(ap_config, ap_channel_atom, global);
+    term hostname_term = interop_kv_get_value(ap_config, dhcp_hostname_atom, global);
 
     //
     // Check parameters
     //
     char *ssid = NULL;
     if (term_is_invalid_term(ssid_term)) {
-        ssid = get_default_device_name();
+        enum DriverErrorCodeType ret = write_default_device_name(DEFAULT_HOSTNAME_SIZE, &ssid);
+        if (UNLIKELY(ret != DriverOK)) {
+            free(ssid);
+            return error_code_to_term(ret, global);
+        }
     } else {
         int ok = 0;
         ssid = interop_term_to_string(ssid_term, &ok);
@@ -402,11 +478,11 @@ static term start_ap(term ap_config, GlobalContext *global)
     if (!term_is_invalid_term(pass_term)) {
         int ok = 0;
         psk = interop_term_to_string(pass_term, &ok);
-        if (strlen(psk) < 8) {
+        if (UNLIKELY(strlen(psk) < 8)) {
             free(ssid);
             return BADARG_ATOM;
         }
-        if (!ok) {
+        if (UNLIKELY(!ok)) {
             free(ssid);
             return BADARG_ATOM;
         }
@@ -419,9 +495,18 @@ static term start_ap(term ap_config, GlobalContext *global)
         }
     }
 
+    enum DriverErrorCodeType ret = set_interface_dhcp_name(hostname_term, &driver_data->ap_hostname);
+    if (UNLIKELY(ret != DriverOK)) {
+        free(ssid);
+        free(psk);
+        return error_code_to_term(ret, global);
+    }
+
     uint32_t auth = (psk == NULL) ? CYW43_AUTH_OPEN : CYW43_AUTH_WPA2_AES_PSK;
     cyw43_state.assoc_cb = network_driver_cyw43_assoc_cb;
     cyw43_arch_enable_ap_mode(ssid, psk, auth);
+    // Set hostname after enabling AP mode otherwise hostname will revert to "PicoW"
+    netif_set_hostname(&cyw43_state.netif[CYW43_ITF_AP], driver_data->ap_hostname);
     send_ap_started(global);
     free(ssid);
     free(psk);
@@ -574,17 +659,18 @@ static void start_network(Context *ctx, term pid, term ref, term config)
         return;
     }
 
+    // Always enable sta mode so the bus is initialized and we get a MAC
+    // address. This is done before configuring the interface because the
+    // MAC is added to the default hostname, and default ssid in ap mode.
+    // (i.e. atomvm-0123456789ab)
+    cyw43_arch_enable_sta_mode();
     if (!term_is_invalid_term(sta_config)) {
         term result_atom = start_sta(sta_config, ctx->global);
         if (result_atom != OK_ATOM) {
             term error = port_create_error_tuple(ctx, result_atom);
             port_send_reply(ctx, pid, ref, error);
             return;
         }
-    } else {
-        // Always enable sta mode so the bus is initialized and we get a MAC
-        // address.
-        cyw43_arch_enable_sta_mode();
     }
 
     if (!term_is_invalid_term(ap_config)) {
@@ -705,6 +791,10 @@ void network_driver_init(GlobalContext *global)
 void network_driver_destroy(GlobalContext *global)
 {
     if (driver_data) {
+        free(driver_data->hostname);
+        if (driver_data->ap_hostname) {
+            free(driver_data->ap_hostname);
+        }
         free(driver_data->sntp_hostname);
         free(driver_data->stas_mac);
         if (driver_data->dhcp_config) {