Fix ESP32-S2 sim test

[bettio]

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]

AMP wants number 1 here, less code so perhaps preferably.. imho 2 can be ignored - and 3 might as well be added..

either way issue is mem pressure - something like CONFIG_ESP_WIFI_STATIC_RX_BUFFER_NUM=6 in src/platforms/esp32/test/sdkconfig.ci.wokwi - makes CI pass

I have tested locally (on wokwi) with number 1 and the sdkconfig..

Review of 160fa1e5 — ESP32 network teardown

Verdict: changes requested

The new port-driver destroy hook is the right ownership layer for a teardown
backstop, and the try ... after change makes test_wifi_managed more robust.
However, the destroy hook only unregisters AtomVM's callbacks. It leaves the
ESP-IDF Wi-Fi driver and fixed-key default netifs alive, so a later
GlobalContext can abort when it tries to create WIFI_STA_DEF again. The
test change also always follows the normal network:stop() path and therefore
does not exercise the new fallback.

Findings

1. High — Driver destruction leaves process-global Wi-Fi resources alive

Location: src/platforms/esp32/components/avm_builtins/network_driver.c:1995-2004

network_driver_destroy() unregisters the event handlers but does not perform
the rest of stop_network()'s teardown: stopping/deinitializing Wi-Fi and
destroying WIFI_STA_DEF / WIFI_AP_DEF.

This matters because the ESP32 test process creates multiple GlobalContexts
while the ESP-IDF default event loop and Wi-Fi objects are process-global. If a
VM reaches this fallback without calling network:stop(), the next call to
esp_netif_create_default_wifi_sta() attempts to create the same fixed netif
key. On ESP-IDF 5.5.4 (the default simulator CI version), the convenience API
asserts when esp_netif_new() rejects that duplicate key. Thus the new fallback
does not leave the process reusable and can turn one failed/aborted network test
into a crash in the following test.

The existing stop_network() implementation is already tolerant of absent
handlers, uninitialized Wi-Fi, and missing netifs. Reuse it instead of
duplicating only part of the teardown. Unregister the per-scan callback first,
because stop_network() does not currently handle it.

diff --git a/src/platforms/esp32/components/avm_builtins/network_driver.c b/src/platforms/esp32/components/avm_builtins/network_driver.c
--- a/src/platforms/esp32/components/avm_builtins/network_driver.c
+++ b/src/platforms/esp32/components/avm_builtins/network_driver.c
@@
-static void stop_network(Context *ctx)
+static void stop_network(void)
 {
@@
-                stop_network(ctx);
+                stop_network();
                 break;
@@
 static void network_driver_destroy(GlobalContext *global)
 {
     UNUSED(global);
 
-    esp_event_handler_unregister(WIFI_EVENT, ESP_EVENT_ANY_ID, &event_handler);
-    esp_event_handler_unregister(IP_EVENT, IP_EVENT_STA_GOT_IP, &event_handler);
-    esp_event_handler_unregister(IP_EVENT, IP_EVENT_AP_STAIPASSIGNED, &event_handler);
-    esp_event_handler_unregister(sntp_event_base, SNTP_EVENT_BASE_SYNC, &event_handler);
     esp_event_handler_unregister(WIFI_EVENT, WIFI_EVENT_SCAN_DONE, &scan_done_handler);
+    stop_network();
 }

Context *ctx can be removed because stop_network() does not use it.

2. Medium — The modified test does not cover the new no-stop teardown path

Locations:

• src/platforms/esp32/test/main/test_erl_sources/test_wifi_managed.erl:25-37
• src/platforms/esp32/test/main/test_main.c:173-178

The production fix is specifically for destruction when network:stop() was
not called, but the changed test now guarantees that network:stop() runs in
its after block. By the time port_driver_destroy_all() invokes the new
callback, normal network teardown has already removed the handlers, stopped
Wi-Fi, and destroyed the netifs. The test therefore passes even if
network_driver_destroy() is removed or is incomplete as in finding 1.

Keep the try ... after hardening, but add a dedicated teardown regression that:

1. starts Wi-Fi and returns without calling network:stop();
2. lets avm_test_case() destroy the port drivers and GlobalContext; and
3. starts Wi-Fi again in a second avm_test_case() invocation in the same Unity
   test.

Running the scenario twice is important: it checks both that late callbacks do
not reach the old VM and that persistent ESP-IDF Wi-Fi/netif state was actually
released. This is a multi-file test-fixture addition, so no misleadingly partial
patch is proposed here.

3. Low — The cleanup result is ignored, allowing teardown failure to pass

Location: src/platforms/esp32/test/main/test_erl_sources/test_wifi_managed.erl:35-36

network:stop/0 can return {error, timeout} or another error, but an after
expression's value is discarded. The test can therefore return ok even when
its cleanup did not complete—the exact state in which later tests become
unreliable. Match the documented success result so cleanup failures fail this
test at their source.

diff --git a/src/platforms/esp32/test/main/test_erl_sources/test_wifi_managed.erl b/src/platforms/esp32/test/main/test_erl_sources/test_wifi_managed.erl
--- a/src/platforms/esp32/test/main/test_erl_sources/test_wifi_managed.erl
+++ b/src/platforms/esp32/test/main/test_erl_sources/test_wifi_managed.erl
@@
             after
-                network:stop()
+                ok = network:stop()
             end;

Additional concurrency note

ESP-IDF's public unregister API does not document a general callback-join
guarantee. In ESP-IDF 5.5.4, dispatch holds the event-loop mutex while invoking
handlers, so an unregister from the AtomVM/test task waits for a callback
currently running on the separate event-loop task. A re-entrant unregister from
the event-loop task instead uses deferred removal and is not a join. The current
destroy callback is invoked cross-task, so this is not raised as a separate
blocker, but future cleanup code should not free callback arguments from inside
an event callback merely because unregister returned.

Positive observations

• Registering cleanup through REGISTER_PORT_DRIVER puts the fallback in the
  correct lifecycle layer, before globalcontext_destroy() frees VM-global
  state.
• try ... after is the correct way to preserve cleanup when a managed-network
  assertion fails.
• Increasing simulated DHCP waits from 20 to 30 seconds is narrowly scoped and
  does not change production behavior.
• git show --check and git diff --check HEAD^ HEAD report no whitespace
  errors.

[petermm]

test_wifi_teardown didn't make it in, I'm fine without it..

[bettio]

Fixed.