From 76642f66257a2b17cc322f644318be5d2c39bc0b Mon Sep 17 00:00:00 2001 From: Ben Severson Date: Tue, 28 Jul 2026 10:33:15 -0700 Subject: [PATCH 1/3] Keep retrying STA after AP fallback, and defer mode switches off the event handler (#78) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AP fallback was a one-way door: after five failed STA attempts the controller brought up its provisioning AP and never tried the configured network again. A router that reboots — or that is simply slower to boot than the kiln — left the controller stranded on its own AP with no remote access to a device that may be mid-firing, recoverable only by a physical power cycle. Fallback now runs APSTA when STA credentials exist, and a new worker task retries the configured network on a 30 s → 5 min backoff, forever. Bounded so the radio is not churning every few seconds during a firing, persistent so recovery costs at most one backoff interval. Retries are suppressed while a client is associated with the AP — a STA connect drags the shared radio off-channel and would yank the provisioning form out from under whoever is filling it in — and suppression is itself capped at 15 min so a phone that auto-joined and was pocketed cannot strand the kiln. Once STA reconnects and the AP is empty, the AP is dropped and we return to the plain STA steady state. The second half of the issue was start_ap() doing network work inside the Wi-Fi event handler: it called esp_wifi_stop() and, worse, esp_netif_create_default_wifi_ap() there, so a second fallback would abort on the duplicate netif. Both netifs are now created once during init before the radio starts, the event handler is reduced to bookkeeping plus a queue post, and every mode switch happens on the worker. The fallback path also drops ESP_ERROR_CHECK in favour of logged errors and a retry on the next tick — aborting the controller over a Wi-Fi hiccup would take a firing with it. The worker is a low-priority task pinned to core 0 that sleeps on its queue; it cannot delay the firing or safety tasks and never touches the SSR. The retry policy is extracted into wifi_retry_policy.c as a pure function, free of esp_wifi/esp_event/FreeRTOS, and covered by host tests — the same split safety_helpers.c uses, and for the same reason: the rest of wifi_manager.c is not host-buildable. Verified the tests fail against a policy that reproduces today's never-retry behaviour before implementing the real one. Also switches the hardcoded retry limit and AP channel to the APP_WIFI_* constants they were duplicating, and clears WIFI_CONNECTED_BIT when an established STA link drops, so the status LED's "disconnected" state works. Co-Authored-By: Claude Opus 5 --- components/wifi_manager/CMakeLists.txt | 4 +- .../wifi_manager/include/wifi_manager.h | 10 +- .../wifi_manager/include/wifi_retry_policy.h | 60 ++++ components/wifi_manager/wifi_manager.c | 277 +++++++++++++++--- components/wifi_manager/wifi_retry_policy.c | 39 +++ tests/host/CMakeLists.txt | 7 + tests/host/test_wifi_retry_policy.c | 184 ++++++++++++ 7 files changed, 540 insertions(+), 41 deletions(-) create mode 100644 components/wifi_manager/include/wifi_retry_policy.h create mode 100644 components/wifi_manager/wifi_retry_policy.c create mode 100644 tests/host/test_wifi_retry_policy.c diff --git a/components/wifi_manager/CMakeLists.txt b/components/wifi_manager/CMakeLists.txt index e33531f8..ec86fbe6 100644 --- a/components/wifi_manager/CMakeLists.txt +++ b/components/wifi_manager/CMakeLists.txt @@ -1,5 +1,5 @@ idf_component_register( - SRCS "wifi_manager.c" + SRCS "wifi_manager.c" "wifi_retry_policy.c" INCLUDE_DIRS "include" - REQUIRES esp_wifi esp_event esp_netif nvs_flash + REQUIRES esp_wifi esp_event esp_netif esp_timer nvs_flash app_config ) diff --git a/components/wifi_manager/include/wifi_manager.h b/components/wifi_manager/include/wifi_manager.h index cc00a84d..8d7e0019 100644 --- a/components/wifi_manager/include/wifi_manager.h +++ b/components/wifi_manager/include/wifi_manager.h @@ -21,6 +21,13 @@ extern "C" { * Initialize Wi-Fi in STA mode. Falls back to AP mode if STA credentials are empty * or connection fails after retries. * + * The fallback is not permanent: when STA credentials exist the controller runs + * APSTA and keeps retrying the configured network on a 30 s → 5 min backoff, + * so a router that reboots mid-firing recovers without a power cycle. Retries + * are held off while a client is associated with the provisioning AP. Once STA + * reconnects and the AP is empty, the AP is dropped again. All of this runs on + * a dedicated low-priority worker task; nothing blocks the caller. + * * @param sta_ssid Station SSID (empty string = skip STA, go straight to AP) * @param sta_pass Station password * @param ap_ssid AP mode SSID @@ -41,7 +48,8 @@ esp_err_t wifi_manager_wait_connected(uint32_t timeout_ms); bool wifi_manager_is_connected(void); /** - * Check if running in AP mode. + * Check if the provisioning AP is the only way in. False once STA reconnects, + * even during the brief APSTA overlap before the AP is torn down. */ bool wifi_manager_is_ap_mode(void); diff --git a/components/wifi_manager/include/wifi_retry_policy.h b/components/wifi_manager/include/wifi_retry_policy.h new file mode 100644 index 00000000..5ce6cb10 --- /dev/null +++ b/components/wifi_manager/include/wifi_retry_policy.h @@ -0,0 +1,60 @@ +#pragma once + +/* + * Pure STA-reconnect policy for wifi_manager's AP-fallback mode. + * + * Kept free of esp_wifi/esp_event/FreeRTOS so it can be exercised by the host + * test harness (tests/host/test_wifi_retry_policy.c) — same reason + * safety_helpers.c exists. wifi_manager.c owns the radio; this file only + * answers "may I start a STA connect attempt right now?". + */ + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/* First retry lands 30 s after entering AP fallback, then doubles. */ +#define WIFI_RETRY_BASE_MS 30000U + +/* Backoff ceiling. Bounded-but-persistent: the kiln keeps trying forever, but + never more than once every 5 min, so a router that comes back mid-firing + costs at most one backoff interval of lost remote monitoring. */ +#define WIFI_RETRY_MAX_MS 300000U + +/* An associated AP client suppresses retries (a connect attempt makes the STA + scan, which drags the shared radio off the AP channel and would yank the + provisioning form out from under whoever is filling it in). Suppression is + itself bounded: a device that auto-joins "Bisque" and idles there forever + must not strand the kiln on its own AP, so after this long overdue we + attempt anyway. */ +#define WIFI_RETRY_SUPPRESS_MAX_US (15LL * 60 * 1000 * 1000) + +typedef enum { + WIFI_RETRY_NOT_DUE = 0, /* backoff interval has not elapsed */ + WIFI_RETRY_SUPPRESSED, /* due, but an AP client is mid-provisioning */ + WIFI_RETRY_ATTEMPT, /* start a STA connect attempt now */ +} wifi_retry_action_t; + +typedef struct { + uint32_t attempt_count; /* STA attempts made since entering AP fallback */ + int64_t last_attempt_us; /* also the fallback-entry timestamp before the first attempt */ +} wifi_retry_state_t; + +/* Backoff for the (attempt_count)-th retry, in ms: 30 s doubling to a 5 min cap. */ +uint32_t wifi_retry_backoff_ms(uint32_t attempt_count); + +/* Arm the policy on entering AP fallback (or after a successful connect). */ +void wifi_retry_reset(wifi_retry_state_t *st, int64_t now_us); + +/* Decide whether a STA connect attempt may start now. On WIFI_RETRY_ATTEMPT the + state is advanced (attempt counted, backoff restarted); the other outcomes + leave it untouched, so a suppressed retry stays overdue and fires as soon as + the AP client leaves. */ +wifi_retry_action_t wifi_retry_step(wifi_retry_state_t *st, bool ap_client_associated, int64_t now_us); + +#ifdef __cplusplus +} +#endif diff --git a/components/wifi_manager/wifi_manager.c b/components/wifi_manager/wifi_manager.c index fb76d8b4..25e5be07 100644 --- a/components/wifi_manager/wifi_manager.c +++ b/components/wifi_manager/wifi_manager.c @@ -1,11 +1,16 @@ #include "wifi_manager.h" +#include "wifi_retry_policy.h" +#include "app_config.h" #include "esp_wifi.h" #include "esp_event.h" #include "esp_netif.h" #include "esp_log.h" +#include "esp_timer.h" #include "nvs_flash.h" #include "nvs.h" #include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "freertos/queue.h" #include "freertos/event_groups.h" #include @@ -14,38 +19,97 @@ static const char *TAG = "wifi_mgr"; #define WIFI_CONNECTED_BIT BIT0 #define WIFI_FAIL_BIT BIT1 +/* The worker owns every mode switch and every esp_wifi_connect(). It is a plain + low-priority task that sleeps on its queue, so nothing here can delay the + firing or safety tasks, and it never touches the SSR. */ +#define WIFI_WORKER_TICK_MS 1000 +#define WIFI_WORKER_STACK 4096 +#define WIFI_WORKER_PRIO 1 +#define WIFI_WORKER_CORE 0 + +typedef enum { + WIFI_CMD_STA_CONNECT = 0, /* STA_START, or a paced retry */ + WIFI_CMD_ENTER_AP_FALLBACK, /* STA gave up; bring the provisioning AP up */ + WIFI_CMD_POLL, /* wake the worker so it re-evaluates now */ +} wifi_cmd_t; + static EventGroupHandle_t s_wifi_event_group; -static int s_retry_count = 0; -static int s_max_retries = 5; -static bool s_is_ap_mode = false; +static QueueHandle_t s_cmd_queue; + +/* Written by the event-loop task, read by the worker. Single writer each, and + every value is word-sized, so no lock is needed. */ +static volatile int s_ap_clients = 0; +static volatile bool s_sta_connected = false; +static volatile bool s_ap_active = false; + +static int s_retry_count = 0; /* fast pre-fallback attempts; event-loop task only */ + +/* Worker-task-owned. */ +static wifi_retry_state_t s_retry; + +static esp_netif_t *s_netif_sta; +static esp_netif_t *s_netif_ap; + static char s_ip_str[16] = "0.0.0.0"; +static bool s_sta_configured = false; static const char *s_ap_ssid; static const char *s_ap_pass; -static void start_ap(void); +static void post_cmd(wifi_cmd_t cmd) +{ + if (s_cmd_queue != NULL) { + /* Never block: this runs on the event-loop task. A full queue just means + the worker already has work pending, which is the same outcome. */ + (void)xQueueSend(s_cmd_queue, &cmd, 0); + } +} + +/* ── Event handler — bookkeeping only, no radio calls ──────────────────── */ +/* Everything that touches the radio is deferred to the worker. The old handler + called esp_wifi_stop() and esp_netif_create_default_wifi_ap() inline, which is + what made a second fallback abort on the duplicate netif (issue #78). */ static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_id, void *event_data) { if (event_base == WIFI_EVENT) { switch (event_id) { case WIFI_EVENT_STA_START: - esp_wifi_connect(); + post_cmd(WIFI_CMD_STA_CONNECT); break; case WIFI_EVENT_STA_DISCONNECTED: - if (s_retry_count < s_max_retries) { - s_retry_count++; - ESP_LOGI(TAG, "STA retry %d/%d", s_retry_count, s_max_retries); - esp_wifi_connect(); - } else { - ESP_LOGW(TAG, "STA connection failed, switching to AP mode"); - xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT); - start_ap(); + s_sta_connected = false; + if (!s_ap_active) { + /* The AP is not up yet: burn through the fast retries, then fall + back. Losing an established STA link re-enters this path, so a + router reboot still reaches the AP. */ + xEventGroupClearBits(s_wifi_event_group, WIFI_CONNECTED_BIT); + if (s_retry_count < APP_WIFI_MAX_RETRY) { + s_retry_count++; + ESP_LOGI(TAG, "STA retry %d/%d", s_retry_count, APP_WIFI_MAX_RETRY); + post_cmd(WIFI_CMD_STA_CONNECT); + } else { + ESP_LOGW(TAG, "STA connection failed, switching to AP mode"); + xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT); + post_cmd(WIFI_CMD_ENTER_AP_FALLBACK); + } } + /* While the AP is up the backoff policy paces retries; reacting to + the failure here would busy-loop the radio during a firing. */ break; case WIFI_EVENT_AP_STACONNECTED: { wifi_event_ap_staconnected_t *evt = (wifi_event_ap_staconnected_t *)event_data; - ESP_LOGI(TAG, "Station connected to AP, AID=%d", evt->aid); + s_ap_clients++; + ESP_LOGI(TAG, "Station connected to AP, AID=%d (%d associated)", evt->aid, s_ap_clients); + break; + } + case WIFI_EVENT_AP_STADISCONNECTED: { + if (s_ap_clients > 0) { + s_ap_clients--; + } + ESP_LOGI(TAG, "Station left AP (%d associated)", s_ap_clients); + /* A pending retry may have been suppressed by this client. */ + post_cmd(WIFI_CMD_POLL); break; } default: @@ -56,7 +120,9 @@ static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_ snprintf(s_ip_str, sizeof(s_ip_str), IPSTR, IP2STR(&evt->ip_info.ip)); ESP_LOGI(TAG, "STA connected, IP: %s", s_ip_str); s_retry_count = 0; + s_sta_connected = true; xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); + post_cmd(WIFI_CMD_POLL); } } @@ -83,15 +149,14 @@ static void copy_credential(uint8_t *dst, size_t dst_size, const char *src) memcpy(dst, src, strnlen(src, dst_size)); } -static void start_ap(void) -{ - /* Stop STA first */ - esp_wifi_stop(); +/* ── Worker task — the only place the Wi-Fi mode changes ───────────────── */ +static void apply_ap_config(void) +{ wifi_config_t ap_config = { .ap = { - .channel = 1, + .channel = APP_WIFI_AP_CHANNEL, .max_connection = 4, .authmode = WIFI_AUTH_WPA2_PSK, }, @@ -104,51 +169,184 @@ static void start_ap(void) ap_config.ap.authmode = WIFI_AUTH_OPEN; } - esp_netif_create_default_wifi_ap(); - ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_AP)); - ESP_ERROR_CHECK(esp_wifi_set_config(WIFI_IF_AP, &ap_config)); - ESP_ERROR_CHECK(esp_wifi_start()); + esp_err_t err = esp_wifi_set_config(WIFI_IF_AP, &ap_config); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_set_config(AP) failed: %s", esp_err_to_name(err)); + } +} - s_is_ap_mode = true; +/* Bring the provisioning AP up alongside (not instead of) the STA interface. + APSTA keeps the STA interface available for the retry loop, so recovery never + requires tearing the AP down. Idempotent: a second call is a no-op rather + than a duplicate-netif abort. */ +static void enter_ap_fallback(void) +{ + if (s_ap_active) { + return; + } + + wifi_mode_t mode = s_sta_configured ? WIFI_MODE_APSTA : WIFI_MODE_AP; + esp_err_t err = esp_wifi_set_mode(mode); + if (err != ESP_OK) { + /* Deliberately not ESP_ERROR_CHECK: aborting the controller over a + Wi-Fi hiccup would take a firing with it. Retry on the next tick. */ + ESP_LOGE(TAG, "esp_wifi_set_mode(%s) failed: %s", s_sta_configured ? "APSTA" : "AP", esp_err_to_name(err)); + return; + } + apply_ap_config(); + + s_ap_active = true; + wifi_retry_reset(&s_retry, esp_timer_get_time()); snprintf(s_ip_str, sizeof(s_ip_str), "192.168.4.1"); ESP_LOGI(TAG, "AP started: SSID=%s, IP=%s", s_ap_ssid, s_ip_str); xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); } +/* The configured network came back. Drop the AP and return to the plain STA + steady state — but only when nobody is associated, so a user who joined the + AP in the seconds since the successful retry is not cut off mid-form. */ +static void leave_ap_fallback(void) +{ + if (s_ap_clients > 0) { + return; + } + + esp_err_t err = esp_wifi_set_mode(WIFI_MODE_STA); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_set_mode(STA) failed: %s", esp_err_to_name(err)); + return; + } + + s_ap_active = false; + xEventGroupClearBits(s_wifi_event_group, WIFI_FAIL_BIT); + ESP_LOGI(TAG, "STA recovered, AP stopped; IP: %s", s_ip_str); +} + +static void try_sta_connect(void) +{ + esp_err_t err = esp_wifi_connect(); + if (err != ESP_OK && err != ESP_ERR_WIFI_CONN) { + ESP_LOGW(TAG, "esp_wifi_connect failed: %s", esp_err_to_name(err)); + } +} + +static void service_ap_fallback(void) +{ + if (!s_ap_active) { + return; + } + if (s_sta_connected) { + leave_ap_fallback(); + return; + } + if (!s_sta_configured) { + return; /* provisioning-only AP; there is nothing to reconnect to */ + } + + switch (wifi_retry_step(&s_retry, s_ap_clients > 0, esp_timer_get_time())) { + case WIFI_RETRY_ATTEMPT: + ESP_LOGI(TAG, "AP fallback: STA reconnect attempt %u (next in %u s)", (unsigned)s_retry.attempt_count, + (unsigned)(wifi_retry_backoff_ms(s_retry.attempt_count) / 1000)); + try_sta_connect(); + break; + case WIFI_RETRY_SUPPRESSED: + ESP_LOGD(TAG, "AP fallback: retry due but %d client(s) associated, holding", s_ap_clients); + break; + case WIFI_RETRY_NOT_DUE: + default: + break; + } +} + +static void wifi_worker_task(void *arg) +{ + for (;;) { + wifi_cmd_t cmd; + if (xQueueReceive(s_cmd_queue, &cmd, pdMS_TO_TICKS(WIFI_WORKER_TICK_MS)) == pdTRUE) { + switch (cmd) { + case WIFI_CMD_STA_CONNECT: + try_sta_connect(); + break; + case WIFI_CMD_ENTER_AP_FALLBACK: + enter_ap_fallback(); + break; + case WIFI_CMD_POLL: + default: + break; + } + } + service_ap_fallback(); + } +} + +/* ── Init ──────────────────────────────────────────────────────────────── */ + esp_err_t wifi_manager_init(const char *sta_ssid, const char *sta_pass, const char *ap_ssid, const char *ap_pass) { s_ap_ssid = ap_ssid; s_ap_pass = ap_pass; + s_sta_configured = (sta_ssid != NULL && sta_ssid[0] != '\0'); s_wifi_event_group = xEventGroupCreate(); + s_cmd_queue = xQueueCreate(8, sizeof(wifi_cmd_t)); + if (s_wifi_event_group == NULL || s_cmd_queue == NULL) { + return ESP_ERR_NO_MEM; + } ESP_ERROR_CHECK(esp_netif_init()); ESP_ERROR_CHECK(esp_event_loop_create_default()); + /* Both netifs are created exactly once, here, before the radio starts. + Creating them up front is what makes the AP fallback re-entrant — the old + code created the AP netif inside the fallback path, so a second fallback + aborted on the duplicate. The unused one costs a few hundred bytes. */ + if (s_netif_sta == NULL) { + s_netif_sta = esp_netif_create_default_wifi_sta(); + } + if (s_netif_ap == NULL) { + s_netif_ap = esp_netif_create_default_wifi_ap(); + } + if (s_netif_sta == NULL || s_netif_ap == NULL) { + return ESP_ERR_NO_MEM; + } + wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT(); ESP_ERROR_CHECK(esp_wifi_init(&cfg)); ESP_ERROR_CHECK(esp_event_handler_instance_register(WIFI_EVENT, ESP_EVENT_ANY_ID, &event_handler, NULL, NULL)); ESP_ERROR_CHECK(esp_event_handler_instance_register(IP_EVENT, IP_EVENT_STA_GOT_IP, &event_handler, NULL, NULL)); - /* If no STA SSID, go directly to AP mode */ - if (sta_ssid == NULL || sta_ssid[0] == '\0') { + if (s_sta_configured) { + wifi_config_t sta_config = {}; + copy_credential(sta_config.sta.ssid, sizeof(sta_config.sta.ssid), sta_ssid); + copy_credential(sta_config.sta.password, sizeof(sta_config.sta.password), sta_pass); + + ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_STA)); + ESP_ERROR_CHECK(esp_wifi_set_config(WIFI_IF_STA, &sta_config)); + } else { ESP_LOGI(TAG, "No STA SSID configured, starting AP mode"); - start_ap(); - return ESP_OK; + ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_AP)); + apply_ap_config(); } - /* Try STA mode */ - esp_netif_create_default_wifi_sta(); - - wifi_config_t sta_config = {}; - copy_credential(sta_config.sta.ssid, sizeof(sta_config.sta.ssid), sta_ssid); - copy_credential(sta_config.sta.password, sizeof(sta_config.sta.password), sta_pass); + BaseType_t rc = xTaskCreatePinnedToCore(wifi_worker_task, "wifi_worker", WIFI_WORKER_STACK, NULL, WIFI_WORKER_PRIO, + NULL, WIFI_WORKER_CORE); + if (rc != pdPASS) { + ESP_LOGE(TAG, "Failed to create wifi_worker task (rc=%d)", (int)rc); + return ESP_ERR_NO_MEM; + } - ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_STA)); - ESP_ERROR_CHECK(esp_wifi_set_config(WIFI_IF_STA, &sta_config)); + /* Started last, so the worker is already draining the queue when the first + STA_START / AP_START event lands. */ ESP_ERROR_CHECK(esp_wifi_start()); - ESP_LOGI(TAG, "STA mode started, connecting to %s", sta_ssid); + if (s_sta_configured) { + ESP_LOGI(TAG, "STA mode started, connecting to %s", sta_ssid); + } else { + s_ap_active = true; + snprintf(s_ip_str, sizeof(s_ip_str), "192.168.4.1"); + ESP_LOGI(TAG, "AP started: SSID=%s, IP=%s", s_ap_ssid, s_ip_str); + xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); + } return ESP_OK; } @@ -171,7 +369,10 @@ bool wifi_manager_is_connected(void) bool wifi_manager_is_ap_mode(void) { - return s_is_ap_mode; + /* During APSTA recovery the AP is briefly still up while the STA already + has an IP. Callers (status LED, boot banner, /api/wifi) mean "the AP is + the only way in", so a live STA link wins. */ + return s_ap_active && !s_sta_connected; } const char *wifi_manager_get_ip(void) diff --git a/components/wifi_manager/wifi_retry_policy.c b/components/wifi_manager/wifi_retry_policy.c new file mode 100644 index 00000000..b49dee95 --- /dev/null +++ b/components/wifi_manager/wifi_retry_policy.c @@ -0,0 +1,39 @@ +#include "wifi_retry_policy.h" + +uint32_t wifi_retry_backoff_ms(uint32_t attempt_count) +{ + uint32_t ms = WIFI_RETRY_BASE_MS; + for (uint32_t i = 0; i < attempt_count && ms < WIFI_RETRY_MAX_MS; i++) { + ms *= 2; + } + return (ms > WIFI_RETRY_MAX_MS) ? WIFI_RETRY_MAX_MS : ms; +} + +void wifi_retry_reset(wifi_retry_state_t *st, int64_t now_us) +{ + st->attempt_count = 0; + st->last_attempt_us = now_us; +} + +wifi_retry_action_t wifi_retry_step(wifi_retry_state_t *st, bool ap_client_associated, int64_t now_us) +{ + int64_t due_us = st->last_attempt_us + (int64_t)wifi_retry_backoff_ms(st->attempt_count) * 1000; + if (now_us < due_us) { + return WIFI_RETRY_NOT_DUE; + } + + /* Suppression deliberately leaves the state alone, so the retry stays + overdue rather than being consumed — it fires on the first poll after the + client disconnects instead of waiting out a fresh backoff. The same + property makes the overdue-by amount a free suppression clock, so + bounding suppression needs no extra field. */ + if (ap_client_associated && (now_us - due_us) < WIFI_RETRY_SUPPRESS_MAX_US) { + return WIFI_RETRY_SUPPRESSED; + } + + if (st->attempt_count < UINT32_MAX) { + st->attempt_count++; + } + st->last_attempt_us = now_us; + return WIFI_RETRY_ATTEMPT; +} diff --git a/tests/host/CMakeLists.txt b/tests/host/CMakeLists.txt index 306ae338..5170584e 100644 --- a/tests/host/CMakeLists.txt +++ b/tests/host/CMakeLists.txt @@ -109,6 +109,13 @@ add_host_test(test_firing_helpers add_host_test(test_safety_helpers SOURCES test_safety_helpers.c ${ROOT}/components/safety/safety_helpers.c) +# wifi_retry_policy — the STA-reconnect policy extracted from wifi_manager.c +# (issue #78). The rest of wifi_manager.c is esp_wifi/esp_event all the way +# down and is not host-buildable; this file is deliberately free of both. +add_host_test(test_wifi_retry_policy + SOURCES test_wifi_retry_policy.c ${ROOT}/components/wifi_manager/wifi_retry_policy.c) +target_include_directories(test_wifi_retry_policy PRIVATE ${ROOT}/components/wifi_manager/include) + # cone_table — profile generation for every cone × speed × {preheat, slow_cool}. add_host_test(test_cone_table SOURCES test_cone_table.c ${ROOT}/components/cone_table/cone_table.c) diff --git a/tests/host/test_wifi_retry_policy.c b/tests/host/test_wifi_retry_policy.c new file mode 100644 index 00000000..b7856dfd --- /dev/null +++ b/tests/host/test_wifi_retry_policy.c @@ -0,0 +1,184 @@ +#include "unity.h" +#include "wifi_retry_policy.h" + +void setUp(void) +{ +} +void tearDown(void) +{ +} + +#define MS_US(ms) ((int64_t)(ms) * 1000) +#define SEC_US(s) ((int64_t)(s) * 1000 * 1000) +#define MIN_US(m) SEC_US((m) * 60) + +/* Fallback is entered at a non-zero boot offset everywhere below, so a policy + that accidentally treats "never attempted" as epoch-zero shows up. */ +#define FALLBACK_US SEC_US(90) + +static wifi_retry_state_t armed_at(int64_t now_us) +{ + wifi_retry_state_t st; + wifi_retry_reset(&st, now_us); + return st; +} + +/* Run the policy from `from_us` to `to_us` in `step_us` slices, counting the + attempts it grants. Mirrors how the wifi worker task polls it. */ +static int drive(wifi_retry_state_t *st, int64_t from_us, int64_t to_us, int64_t step_us, bool ap_client) +{ + int attempts = 0; + for (int64_t t = from_us; t <= to_us; t += step_us) { + if (wifi_retry_step(st, ap_client, t) == WIFI_RETRY_ATTEMPT) { + attempts++; + } + } + return attempts; +} + +/* ── the bug (issue #78): AP fallback is a one-way door ─────────────────── */ + +/* The whole point: a kiln that fell back to its provisioning AP because the + router was rebooting must keep trying the configured network. Today it never + does, and only a power cycle recovers it — mid-firing, that means no remote + monitoring for the rest of the firing. */ +void test_retries_sta_after_ap_fallback(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, false, FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS))); +} + +/* And it must keep trying, not give up after a handful. */ +void test_retries_are_persistent_over_hours(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + + int attempts = drive(&st, FALLBACK_US, FALLBACK_US + MIN_US(180), SEC_US(1), false); + + /* 3 h at a 5 min ceiling: ~35 attempts. Assert only that it stays busy. */ + TEST_ASSERT_GREATER_THAN_INT(20, attempts); +} + +/* ── cadence: bounded, not a 5 s hammer ─────────────────────────────────── */ + +void test_first_retry_waits_the_base_backoff(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, wifi_retry_step(&st, false, FALLBACK_US)); + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, wifi_retry_step(&st, false, FALLBACK_US + SEC_US(5))); + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, wifi_retry_step(&st, false, FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS) - 1)); + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, false, FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS))); +} + +void test_backoff_doubles_then_saturates(void) +{ + TEST_ASSERT_EQUAL_UINT32(WIFI_RETRY_BASE_MS, wifi_retry_backoff_ms(0)); + TEST_ASSERT_EQUAL_UINT32(WIFI_RETRY_BASE_MS * 2, wifi_retry_backoff_ms(1)); + TEST_ASSERT_EQUAL_UINT32(WIFI_RETRY_BASE_MS * 4, wifi_retry_backoff_ms(2)); + + /* Monotonic non-decreasing, and never past the ceiling — including at the + saturating attempt_count a long-running kiln would reach. */ + uint32_t prev = 0; + for (uint32_t i = 0; i < 64; i++) { + uint32_t ms = wifi_retry_backoff_ms(i); + TEST_ASSERT_GREATER_OR_EQUAL_UINT32(prev, ms); + TEST_ASSERT_LESS_OR_EQUAL_UINT32(WIFI_RETRY_MAX_MS, ms); + prev = ms; + } + TEST_ASSERT_EQUAL_UINT32(WIFI_RETRY_MAX_MS, wifi_retry_backoff_ms(64)); + TEST_ASSERT_EQUAL_UINT32(WIFI_RETRY_MAX_MS, wifi_retry_backoff_ms(UINT32_MAX)); +} + +/* An hour of fallback must not add up to hundreds of radio events. */ +void test_cadence_is_not_a_hammer(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + + int attempts = drive(&st, FALLBACK_US, FALLBACK_US + MIN_US(60), SEC_US(1), false); + + /* 30/60/120/240/300... over an hour ≈ 12. Anything near "every 5 s" (720) + is radio noise during a firing. */ + TEST_ASSERT_LESS_OR_EQUAL_INT(20, attempts); +} + +/* Backoff restarts from the attempt, not from fallback entry — otherwise every + subsequent poll would re-fire immediately once the first retry came due. */ +void test_attempt_restarts_the_interval(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + int64_t t = FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS); + + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, false, t)); + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, wifi_retry_step(&st, false, t + SEC_US(1))); + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, wifi_retry_step(&st, false, t + MS_US(WIFI_RETRY_BASE_MS))); + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, false, t + MS_US(WIFI_RETRY_BASE_MS * 2))); +} + +/* ── AP-client suppression ──────────────────────────────────────────────── */ + +/* Someone is on the AP filling in the provisioning form. A STA connect makes + the shared radio scan off-channel; do not yank the interface out from under + them. */ +void test_associated_ap_client_suppresses_retry(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + + TEST_ASSERT_EQUAL(WIFI_RETRY_SUPPRESSED, wifi_retry_step(&st, true, FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS))); +} + +/* Suppression must not consume the retry: the moment the client leaves, the + overdue attempt fires rather than waiting out another backoff. */ +void test_suppressed_retry_fires_as_soon_as_client_leaves(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + int64_t due = FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS); + + TEST_ASSERT_EQUAL(WIFI_RETRY_SUPPRESSED, wifi_retry_step(&st, true, due)); + TEST_ASSERT_EQUAL(WIFI_RETRY_SUPPRESSED, wifi_retry_step(&st, true, due + SEC_US(10))); + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, false, due + SEC_US(11))); +} + +/* A phone that auto-joined "Bisque" and was pocketed must not strand the kiln + on its own AP forever. */ +void test_suppression_is_bounded(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + int64_t due = FALLBACK_US + MS_US(WIFI_RETRY_BASE_MS); + + TEST_ASSERT_EQUAL(WIFI_RETRY_SUPPRESSED, wifi_retry_step(&st, true, due + WIFI_RETRY_SUPPRESS_MAX_US - 1)); + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, true, due + WIFI_RETRY_SUPPRESS_MAX_US)); +} + +/* ── reset ──────────────────────────────────────────────────────────────── */ + +/* After a successful connect the next fallback starts from the short backoff + again, not from wherever the previous fallback's escalation left off. */ +void test_reset_rearms_the_short_backoff(void) +{ + wifi_retry_state_t st = armed_at(FALLBACK_US); + drive(&st, FALLBACK_US, FALLBACK_US + MIN_US(60), SEC_US(1), false); + + int64_t reconnected = FALLBACK_US + MIN_US(60); + wifi_retry_reset(&st, reconnected); + + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, wifi_retry_step(&st, false, reconnected + MS_US(WIFI_RETRY_BASE_MS) - 1)); + TEST_ASSERT_EQUAL(WIFI_RETRY_ATTEMPT, wifi_retry_step(&st, false, reconnected + MS_US(WIFI_RETRY_BASE_MS))); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_retries_sta_after_ap_fallback); + RUN_TEST(test_retries_are_persistent_over_hours); + RUN_TEST(test_first_retry_waits_the_base_backoff); + RUN_TEST(test_backoff_doubles_then_saturates); + RUN_TEST(test_cadence_is_not_a_hammer); + RUN_TEST(test_attempt_restarts_the_interval); + RUN_TEST(test_associated_ap_client_suppresses_retry); + RUN_TEST(test_suppressed_retry_fires_as_soon_as_client_leaves); + RUN_TEST(test_suppression_is_bounded); + RUN_TEST(test_reset_rearms_the_short_backoff); + return UNITY_END(); +} From 999e52e28fdf687b4b1697675e63d165ebe0a406 Mon Sep 17 00:00:00 2001 From: Ben Severson Date: Tue, 28 Jul 2026 10:39:27 -0700 Subject: [PATCH 2/3] Correct the single-writer note: s_ap_active flows worker to handler, not the reverse --- components/wifi_manager/wifi_manager.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/wifi_manager/wifi_manager.c b/components/wifi_manager/wifi_manager.c index 25e5be07..bb9d2b08 100644 --- a/components/wifi_manager/wifi_manager.c +++ b/components/wifi_manager/wifi_manager.c @@ -36,8 +36,13 @@ typedef enum { static EventGroupHandle_t s_wifi_event_group; static QueueHandle_t s_cmd_queue; -/* Written by the event-loop task, read by the worker. Single writer each, and - every value is word-sized, so no lock is needed. */ +/* Shared between the event-loop task and the worker. Each has exactly one + writer and every value is word-sized, so no lock is needed — but the + direction differs per flag, so check before adding a writer: + s_ap_clients, s_sta_connected — written by the event-loop task, read by + the worker. + s_ap_active — the reverse: written by the worker (and once by + wifi_manager_init, before the worker task exists), read by the handler. */ static volatile int s_ap_clients = 0; static volatile bool s_sta_connected = false; static volatile bool s_ap_active = false; From bb963871917bdd068ee7d584e9d90b1e45985634 Mon Sep 17 00:00:00 2001 From: Ben Severson Date: Wed, 29 Jul 2026 09:30:58 -0700 Subject: [PATCH 3/3] Make the AP fallback survive its own failures, and stop lying about readiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #238, all in the paths that only run when something already went wrong — which is why none of them showed up in normal use. Splits the transition logic out into wifi_fallback.c: it owns "should the provisioning AP be up, and is it?", takes every radio call as an injected op, and is driven by tests/host/test_wifi_fallback.c against a fake radio. The five fixes: 1. Intent is tracked separately from achieved state. A failed esp_wifi_set_mode() used to be terminal — the one queued command had been consumed and the service loop began at "is the AP up? no → nothing to do", so a transient error left the controller with neither STA nor AP until a reboot. ap_wanted now stands until the transition lands. 2. The AP is marked active only after esp_wifi_set_config() succeeds. Previously a failed config was logged and the AP advertised anyway, so callers were pointed at whatever SSID the interface happened to hold. 3. The backoff policy is re-armed on every tick the STA link is up. Only s_retry_count was reset on GOT_IP, so when an AP client held the fallback open across a recovery, a second outage fired a reconnect instantly — and being that overdue also walks through the bounded client suppression. 4. The reported address is derived from (ap_active && !sta_connected) rather than cached, so a LAN address can no longer outlive its link. It used to survive a disconnect that happened while the AP was up, leaving /api/v1/wifi advertising a dead address for a device only reachable at 192.168.4.1. 5. Readiness is signalled from the AP-up hook, not from STA retry exhaustion. The old WIFI_FAIL_BIT woke wait_connected() before either of its documented conditions held, so app_main could see is_ap_mode() as false and skip the setup-mode boot status. The bit had no other reader and is gone. --- components/wifi_manager/CMakeLists.txt | 2 +- .../wifi_manager/include/wifi_fallback.h | 89 +++++ components/wifi_manager/wifi_fallback.c | 102 ++++++ components/wifi_manager/wifi_manager.c | 196 ++++++----- tests/host/CMakeLists.txt | 10 + tests/host/test_wifi_fallback.c | 303 ++++++++++++++++++ 6 files changed, 614 insertions(+), 88 deletions(-) create mode 100644 components/wifi_manager/include/wifi_fallback.h create mode 100644 components/wifi_manager/wifi_fallback.c create mode 100644 tests/host/test_wifi_fallback.c diff --git a/components/wifi_manager/CMakeLists.txt b/components/wifi_manager/CMakeLists.txt index ec86fbe6..40bf767d 100644 --- a/components/wifi_manager/CMakeLists.txt +++ b/components/wifi_manager/CMakeLists.txt @@ -1,5 +1,5 @@ idf_component_register( - SRCS "wifi_manager.c" "wifi_retry_policy.c" + SRCS "wifi_manager.c" "wifi_fallback.c" "wifi_retry_policy.c" INCLUDE_DIRS "include" REQUIRES esp_wifi esp_event esp_netif esp_timer nvs_flash app_config ) diff --git a/components/wifi_manager/include/wifi_fallback.h b/components/wifi_manager/include/wifi_fallback.h new file mode 100644 index 00000000..758ed9f5 --- /dev/null +++ b/components/wifi_manager/include/wifi_fallback.h @@ -0,0 +1,89 @@ +#pragma once + +/* + * AP-fallback state machine for wifi_manager's worker task. + * + * Owns the "should the provisioning AP be up right now, and is it?" question, + * plus the paced STA reconnect that gets us back off it. Every radio call and + * every notification is injected as an op, so — like wifi_retry_policy.c — this + * file pulls in no esp_wifi/esp_event/FreeRTOS header and is driven directly by + * the host test harness (tests/host/test_wifi_fallback.c). wifi_manager.c + * supplies the ops and remains the only place that talks to the radio. + * + * The reason the transitions live here rather than inline in the worker is that + * every one of them can fail, and the failure paths are where the bugs were: + * a mode switch that failed used to drop the fallback on the floor forever, and + * an AP config that failed used to be advertised anyway. + */ + +#include +#include + +#include "wifi_retry_policy.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* The address esp_netif's default AP DHCP server always answers on. Reported to + callers whenever the AP is the only way in. */ +#define WIFI_FALLBACK_AP_IP "192.168.4.1" + +/* Radio and notification hooks. The int-returning ones use 0 for success so + this header stays free of esp_err_t; wifi_manager.c logs the real esp_err. + set_ap_mode/apply_ap_config must be safe to call again after a failure — the + whole transition is re-attempted from the top. */ +typedef struct { + int (*set_ap_mode)(void); /* AP, or APSTA when STA is configured */ + int (*apply_ap_config)(void); /* push the provisioning SSID/PSK */ + int (*set_sta_mode)(void); /* back to plain STA */ + void (*sta_connect)(void); /* start one STA connect attempt */ + void (*ap_up)(void); /* AP is genuinely up: publish it, signal readiness */ + void (*ap_down)(void); /* AP torn down, STA is the way in again */ + int64_t (*now_us)(void); +} wifi_fallback_ops_t; + +typedef struct { + const wifi_fallback_ops_t *ops; + bool sta_configured; + + /* Intent, deliberately separate from the achieved state below. The command + that asked for the fallback is consumed once; if the transition fails, + only a standing intent tells the next tick to try again. */ + bool ap_wanted; + + /* Achieved state. Written here (worker task), read by the event handler. */ + volatile bool ap_active; + + wifi_retry_state_t retry; +} wifi_fallback_t; + +void wifi_fallback_init(wifi_fallback_t *fb, const wifi_fallback_ops_t *ops, bool sta_configured); + +/* Ask for the provisioning AP. Idempotent; the transition itself happens in + wifi_fallback_service() and is retried there until it succeeds. */ +void wifi_fallback_request_ap(wifi_fallback_t *fb); + +/* One worker tick. Drives any pending AP transition, then either the return to + * plain STA (once nobody is on the AP) or the next paced reconnect attempt. + * + * Returns what the retry policy decided, so the caller can log it; + * WIFI_RETRY_NOT_DUE whenever no retry was evaluated at all. */ +wifi_retry_action_t wifi_fallback_service(wifi_fallback_t *fb, bool sta_connected, int ap_clients); + +/* Is the AP up at all? (The event handler uses this to decide whether losing + the STA link should start the fast pre-fallback retries.) */ +bool wifi_fallback_ap_active(const wifi_fallback_t *fb); + +/* Is the AP the *only* way in? During APSTA recovery the AP is briefly still up + while the STA already has an IP, and callers (status LED, boot banner, + /api/v1/wifi) mean this narrower question. */ +bool wifi_fallback_ap_only(const wifi_fallback_t *fb, bool sta_connected); + +/* The address callers should be told to use. Derived rather than cached, so a + STA address cannot outlive the link it belongs to. */ +const char *wifi_fallback_reported_ip(const wifi_fallback_t *fb, bool sta_connected, const char *sta_ip); + +#ifdef __cplusplus +} +#endif diff --git a/components/wifi_manager/wifi_fallback.c b/components/wifi_manager/wifi_fallback.c new file mode 100644 index 00000000..926e3350 --- /dev/null +++ b/components/wifi_manager/wifi_fallback.c @@ -0,0 +1,102 @@ +#include "wifi_fallback.h" + +void wifi_fallback_init(wifi_fallback_t *fb, const wifi_fallback_ops_t *ops, bool sta_configured) +{ + fb->ops = ops; + fb->sta_configured = sta_configured; + fb->ap_wanted = false; + fb->ap_active = false; + wifi_retry_reset(&fb->retry, 0); +} + +void wifi_fallback_request_ap(wifi_fallback_t *fb) +{ + fb->ap_wanted = true; +} + +/* Bring the provisioning AP up alongside (not instead of) the STA interface. + APSTA keeps the STA interface available for the retry loop, so recovery never + requires tearing the AP down. + + Both steps must land before the AP counts as active: telling callers an AP is + available when esp_wifi_set_config() failed points them at whatever SSID the + interface happened to be holding — on the no-credentials boot path there is + no STA link to fall back on either. Returns false to leave ap_wanted standing + so the next tick retries the whole sequence. */ +static bool enter_ap(wifi_fallback_t *fb) +{ + if (fb->ops->set_ap_mode() != 0) { + return false; + } + if (fb->ops->apply_ap_config() != 0) { + return false; + } + + fb->ap_active = true; + wifi_retry_reset(&fb->retry, fb->ops->now_us()); + fb->ops->ap_up(); + return true; +} + +/* The configured network came back. Drop the AP and return to the plain STA + steady state — but only when nobody is associated, so a user who joined the + AP in the seconds since the successful retry is not cut off mid-form. */ +static void leave_ap(wifi_fallback_t *fb, int ap_clients) +{ + if (ap_clients > 0) { + return; + } + if (fb->ops->set_sta_mode() != 0) { + return; + } + + fb->ap_wanted = false; + fb->ap_active = false; + fb->ops->ap_down(); +} + +wifi_retry_action_t wifi_fallback_service(wifi_fallback_t *fb, bool sta_connected, int ap_clients) +{ + if (fb->ap_wanted && !fb->ap_active && !enter_ap(fb)) { + return WIFI_RETRY_NOT_DUE; /* transition failed; try again next tick */ + } + if (!fb->ap_active) { + return WIFI_RETRY_NOT_DUE; + } + + if (sta_connected) { + /* Re-arm on every tick the link is up. An AP client keeps the fallback + open past recovery, and without this the policy would still be + carrying the pre-recovery timestamp and escalated attempt count — so + a second outage would fire a reconnect instantly, and being that + overdue also defeats the bounded AP-client suppression. */ + wifi_retry_reset(&fb->retry, fb->ops->now_us()); + leave_ap(fb, ap_clients); + return WIFI_RETRY_NOT_DUE; + } + + if (!fb->sta_configured) { + return WIFI_RETRY_NOT_DUE; /* provisioning-only AP; nothing to reconnect to */ + } + + wifi_retry_action_t action = wifi_retry_step(&fb->retry, ap_clients > 0, fb->ops->now_us()); + if (action == WIFI_RETRY_ATTEMPT) { + fb->ops->sta_connect(); + } + return action; +} + +bool wifi_fallback_ap_active(const wifi_fallback_t *fb) +{ + return fb->ap_active; +} + +bool wifi_fallback_ap_only(const wifi_fallback_t *fb, bool sta_connected) +{ + return fb->ap_active && !sta_connected; +} + +const char *wifi_fallback_reported_ip(const wifi_fallback_t *fb, bool sta_connected, const char *sta_ip) +{ + return wifi_fallback_ap_only(fb, sta_connected) ? WIFI_FALLBACK_AP_IP : sta_ip; +} diff --git a/components/wifi_manager/wifi_manager.c b/components/wifi_manager/wifi_manager.c index bb9d2b08..9292dcfe 100644 --- a/components/wifi_manager/wifi_manager.c +++ b/components/wifi_manager/wifi_manager.c @@ -1,4 +1,5 @@ #include "wifi_manager.h" +#include "wifi_fallback.h" #include "wifi_retry_policy.h" #include "app_config.h" #include "esp_wifi.h" @@ -16,8 +17,13 @@ static const char *TAG = "wifi_mgr"; +/* The one readiness bit. There is deliberately no separate "STA gave up" bit: + wait_connected()'s contract is "STA connected OR the AP is up", and a bit set + when the STA retries ran out woke it while the AP was still being brought up + — long enough for app_main to read is_ap_mode() as false and skip the + setup-mode boot banner. Readiness is now signalled from one place, the AP-up + hook below, once the transition has actually happened. */ #define WIFI_CONNECTED_BIT BIT0 -#define WIFI_FAIL_BIT BIT1 /* The worker owns every mode switch and every esp_wifi_connect(). It is a plain low-priority task that sleeps on its queue, so nothing here can delay the @@ -39,23 +45,23 @@ static QueueHandle_t s_cmd_queue; /* Shared between the event-loop task and the worker. Each has exactly one writer and every value is word-sized, so no lock is needed — but the direction differs per flag, so check before adding a writer: - s_ap_clients, s_sta_connected — written by the event-loop task, read by - the worker. - s_ap_active — the reverse: written by the worker (and once by - wifi_manager_init, before the worker task exists), read by the handler. */ + s_ap_clients, s_sta_connected, s_sta_ip — written by the event-loop task, + read by the worker. + s_fb.ap_active — the reverse: written by the worker, read by the handler. */ static volatile int s_ap_clients = 0; static volatile bool s_sta_connected = false; -static volatile bool s_ap_active = false; static int s_retry_count = 0; /* fast pre-fallback attempts; event-loop task only */ -/* Worker-task-owned. */ -static wifi_retry_state_t s_retry; +/* Worker-task-owned (except .ap_active, see above). */ +static wifi_fallback_t s_fb; static esp_netif_t *s_netif_sta; static esp_netif_t *s_netif_ap; -static char s_ip_str[16] = "0.0.0.0"; +/* The last address DHCP handed the STA interface. Only ever reported while the + STA link is actually up — wifi_manager_get_ip() derives what to show. */ +static char s_sta_ip[16] = "0.0.0.0"; static bool s_sta_configured = false; static const char *s_ap_ssid; @@ -84,7 +90,7 @@ static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_ break; case WIFI_EVENT_STA_DISCONNECTED: s_sta_connected = false; - if (!s_ap_active) { + if (!wifi_fallback_ap_active(&s_fb)) { /* The AP is not up yet: burn through the fast retries, then fall back. Losing an established STA link re-enters this path, so a router reboot still reaches the AP. */ @@ -95,12 +101,13 @@ static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_ post_cmd(WIFI_CMD_STA_CONNECT); } else { ESP_LOGW(TAG, "STA connection failed, switching to AP mode"); - xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT); post_cmd(WIFI_CMD_ENTER_AP_FALLBACK); } } /* While the AP is up the backoff policy paces retries; reacting to - the failure here would busy-loop the radio during a firing. */ + the failure here would busy-loop the radio during a firing. The + reported address follows s_sta_connected, so it reverts to the AP + without anything to reset here. */ break; case WIFI_EVENT_AP_STACONNECTED: { wifi_event_ap_staconnected_t *evt = (wifi_event_ap_staconnected_t *)event_data; @@ -122,11 +129,13 @@ static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_ } } else if (event_base == IP_EVENT && event_id == IP_EVENT_STA_GOT_IP) { ip_event_got_ip_t *evt = (ip_event_got_ip_t *)event_data; - snprintf(s_ip_str, sizeof(s_ip_str), IPSTR, IP2STR(&evt->ip_info.ip)); - ESP_LOGI(TAG, "STA connected, IP: %s", s_ip_str); + snprintf(s_sta_ip, sizeof(s_sta_ip), IPSTR, IP2STR(&evt->ip_info.ip)); + ESP_LOGI(TAG, "STA connected, IP: %s", s_sta_ip); s_retry_count = 0; s_sta_connected = true; xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); + /* The worker re-arms the backoff policy off s_sta_connected, so a later + drop starts from the short backoff rather than firing instantly. */ post_cmd(WIFI_CMD_POLL); } } @@ -156,7 +165,12 @@ static void copy_credential(uint8_t *dst, size_t dst_size, const char *src) /* ── Worker task — the only place the Wi-Fi mode changes ───────────────── */ -static void apply_ap_config(void) +/* The transition logic itself lives in wifi_fallback.c (host-tested); these are + the radio ops it drives. Each returns 0 for success and logs the real + esp_err_t here, and must be safe to call again — the fallback re-runs the + whole sequence until it lands. */ + +static esp_err_t apply_ap_config(void) { wifi_config_t ap_config = { .ap = @@ -178,91 +192,77 @@ static void apply_ap_config(void) if (err != ESP_OK) { ESP_LOGE(TAG, "esp_wifi_set_config(AP) failed: %s", esp_err_to_name(err)); } + return err; } -/* Bring the provisioning AP up alongside (not instead of) the STA interface. - APSTA keeps the STA interface available for the retry loop, so recovery never - requires tearing the AP down. Idempotent: a second call is a no-op rather - than a duplicate-netif abort. */ -static void enter_ap_fallback(void) +static void try_sta_connect(void) { - if (s_ap_active) { - return; + esp_err_t err = esp_wifi_connect(); + if (err != ESP_OK && err != ESP_ERR_WIFI_CONN) { + ESP_LOGW(TAG, "esp_wifi_connect failed: %s", esp_err_to_name(err)); } +} - wifi_mode_t mode = s_sta_configured ? WIFI_MODE_APSTA : WIFI_MODE_AP; - esp_err_t err = esp_wifi_set_mode(mode); +/* APSTA keeps the STA interface available for the retry loop, so recovery never + requires tearing the AP down. Not ESP_ERROR_CHECK: aborting the controller + over a Wi-Fi hiccup would take a firing with it. */ +static int op_set_ap_mode(void) +{ + const char *name = s_sta_configured ? "APSTA" : "AP"; + esp_err_t err = esp_wifi_set_mode(s_sta_configured ? WIFI_MODE_APSTA : WIFI_MODE_AP); if (err != ESP_OK) { - /* Deliberately not ESP_ERROR_CHECK: aborting the controller over a - Wi-Fi hiccup would take a firing with it. Retry on the next tick. */ - ESP_LOGE(TAG, "esp_wifi_set_mode(%s) failed: %s", s_sta_configured ? "APSTA" : "AP", esp_err_to_name(err)); - return; + ESP_LOGE(TAG, "esp_wifi_set_mode(%s) failed: %s", name, esp_err_to_name(err)); } - apply_ap_config(); - - s_ap_active = true; - wifi_retry_reset(&s_retry, esp_timer_get_time()); - snprintf(s_ip_str, sizeof(s_ip_str), "192.168.4.1"); - ESP_LOGI(TAG, "AP started: SSID=%s, IP=%s", s_ap_ssid, s_ip_str); - xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); + return err == ESP_OK ? 0 : -1; } -/* The configured network came back. Drop the AP and return to the plain STA - steady state — but only when nobody is associated, so a user who joined the - AP in the seconds since the successful retry is not cut off mid-form. */ -static void leave_ap_fallback(void) +static int op_apply_ap_config(void) { - if (s_ap_clients > 0) { - return; - } + return apply_ap_config() == ESP_OK ? 0 : -1; +} +static int op_set_sta_mode(void) +{ esp_err_t err = esp_wifi_set_mode(WIFI_MODE_STA); if (err != ESP_OK) { ESP_LOGE(TAG, "esp_wifi_set_mode(STA) failed: %s", esp_err_to_name(err)); - return; } + return err == ESP_OK ? 0 : -1; +} - s_ap_active = false; - xEventGroupClearBits(s_wifi_event_group, WIFI_FAIL_BIT); - ESP_LOGI(TAG, "STA recovered, AP stopped; IP: %s", s_ip_str); +static void op_sta_connect(void) +{ + try_sta_connect(); } -static void try_sta_connect(void) +static void op_ap_up(void) { - esp_err_t err = esp_wifi_connect(); - if (err != ESP_OK && err != ESP_ERR_WIFI_CONN) { - ESP_LOGW(TAG, "esp_wifi_connect failed: %s", esp_err_to_name(err)); - } + ESP_LOGI(TAG, "AP started: SSID=%s, IP=%s", s_ap_ssid, WIFI_FALLBACK_AP_IP); + /* Readiness is signalled here and nowhere else: this is the first moment at + which wait_connected()'s "STA connected OR the AP is up" actually holds. */ + xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); } -static void service_ap_fallback(void) +static void op_ap_down(void) { - if (!s_ap_active) { - return; - } - if (s_sta_connected) { - leave_ap_fallback(); - return; - } - if (!s_sta_configured) { - return; /* provisioning-only AP; there is nothing to reconnect to */ - } + ESP_LOGI(TAG, "STA recovered, AP stopped; IP: %s", s_sta_ip); +} - switch (wifi_retry_step(&s_retry, s_ap_clients > 0, esp_timer_get_time())) { - case WIFI_RETRY_ATTEMPT: - ESP_LOGI(TAG, "AP fallback: STA reconnect attempt %u (next in %u s)", (unsigned)s_retry.attempt_count, - (unsigned)(wifi_retry_backoff_ms(s_retry.attempt_count) / 1000)); - try_sta_connect(); - break; - case WIFI_RETRY_SUPPRESSED: - ESP_LOGD(TAG, "AP fallback: retry due but %d client(s) associated, holding", s_ap_clients); - break; - case WIFI_RETRY_NOT_DUE: - default: - break; - } +static int64_t op_now_us(void) +{ + return esp_timer_get_time(); } +static const wifi_fallback_ops_t s_fb_ops = { + .set_ap_mode = op_set_ap_mode, + .apply_ap_config = op_apply_ap_config, + .set_sta_mode = op_set_sta_mode, + .sta_connect = op_sta_connect, + .ap_up = op_ap_up, + .ap_down = op_ap_down, + .now_us = op_now_us, +}; + static void wifi_worker_task(void *arg) { for (;;) { @@ -273,14 +273,30 @@ static void wifi_worker_task(void *arg) try_sta_connect(); break; case WIFI_CMD_ENTER_AP_FALLBACK: - enter_ap_fallback(); + /* Records the intent only. A failed mode switch or AP config + used to strand the controller with neither STA nor AP, + because this command is consumed once and the retry loop + started at "is the AP already up?". */ + wifi_fallback_request_ap(&s_fb); break; case WIFI_CMD_POLL: default: break; } } - service_ap_fallback(); + + switch (wifi_fallback_service(&s_fb, s_sta_connected, s_ap_clients)) { + case WIFI_RETRY_ATTEMPT: + ESP_LOGI(TAG, "AP fallback: STA reconnect attempt %u (next in %u s)", (unsigned)s_fb.retry.attempt_count, + (unsigned)(wifi_retry_backoff_ms(s_fb.retry.attempt_count) / 1000)); + break; + case WIFI_RETRY_SUPPRESSED: + ESP_LOGD(TAG, "AP fallback: retry due but %d client(s) associated, holding", s_ap_clients); + break; + case WIFI_RETRY_NOT_DUE: + default: + break; + } } } @@ -291,6 +307,7 @@ esp_err_t wifi_manager_init(const char *sta_ssid, const char *sta_pass, const ch s_ap_ssid = ap_ssid; s_ap_pass = ap_pass; s_sta_configured = (sta_ssid != NULL && sta_ssid[0] != '\0'); + wifi_fallback_init(&s_fb, &s_fb_ops, s_sta_configured); s_wifi_event_group = xEventGroupCreate(); s_cmd_queue = xQueueCreate(8, sizeof(wifi_cmd_t)); if (s_wifi_event_group == NULL || s_cmd_queue == NULL) { @@ -328,9 +345,15 @@ esp_err_t wifi_manager_init(const char *sta_ssid, const char *sta_pass, const ch ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_STA)); ESP_ERROR_CHECK(esp_wifi_set_config(WIFI_IF_STA, &sta_config)); } else { + /* Configure the AP up front so esp_wifi_start() brings up the right + SSID rather than the interface default, then hand ownership to the + worker: it re-runs the same (idempotent) transition, and only it + marks the AP active, publishes the address and signals readiness — + retrying for as long as either step keeps failing. */ ESP_LOGI(TAG, "No STA SSID configured, starting AP mode"); ESP_ERROR_CHECK(esp_wifi_set_mode(WIFI_MODE_AP)); - apply_ap_config(); + (void)apply_ap_config(); + post_cmd(WIFI_CMD_ENTER_AP_FALLBACK); } BaseType_t rc = xTaskCreatePinnedToCore(wifi_worker_task, "wifi_worker", WIFI_WORKER_STACK, NULL, WIFI_WORKER_PRIO, @@ -346,19 +369,14 @@ esp_err_t wifi_manager_init(const char *sta_ssid, const char *sta_pass, const ch if (s_sta_configured) { ESP_LOGI(TAG, "STA mode started, connecting to %s", sta_ssid); - } else { - s_ap_active = true; - snprintf(s_ip_str, sizeof(s_ip_str), "192.168.4.1"); - ESP_LOGI(TAG, "AP started: SSID=%s, IP=%s", s_ap_ssid, s_ip_str); - xEventGroupSetBits(s_wifi_event_group, WIFI_CONNECTED_BIT); } return ESP_OK; } esp_err_t wifi_manager_wait_connected(uint32_t timeout_ms) { - EventBits_t bits = xEventGroupWaitBits(s_wifi_event_group, WIFI_CONNECTED_BIT | WIFI_FAIL_BIT, pdFALSE, pdFALSE, - pdMS_TO_TICKS(timeout_ms)); + EventBits_t bits = + xEventGroupWaitBits(s_wifi_event_group, WIFI_CONNECTED_BIT, pdFALSE, pdFALSE, pdMS_TO_TICKS(timeout_ms)); if (bits & WIFI_CONNECTED_BIT) { return ESP_OK; @@ -377,12 +395,16 @@ bool wifi_manager_is_ap_mode(void) /* During APSTA recovery the AP is briefly still up while the STA already has an IP. Callers (status LED, boot banner, /api/wifi) mean "the AP is the only way in", so a live STA link wins. */ - return s_ap_active && !s_sta_connected; + return wifi_fallback_ap_only(&s_fb, s_sta_connected); } const char *wifi_manager_get_ip(void) { - return s_ip_str; + /* Derived from the same condition as is_ap_mode(), so the two can never + disagree. Caching the address instead is what let a dead LAN address be + reported after the router dropped while an AP client held the fallback + open — the device was only reachable at 192.168.4.1. */ + return wifi_fallback_reported_ip(&s_fb, s_sta_connected, s_sta_ip); } /* ── NVS Credential Persistence ───────────────────── */ diff --git a/tests/host/CMakeLists.txt b/tests/host/CMakeLists.txt index 5170584e..e0275447 100644 --- a/tests/host/CMakeLists.txt +++ b/tests/host/CMakeLists.txt @@ -116,6 +116,16 @@ add_host_test(test_wifi_retry_policy SOURCES test_wifi_retry_policy.c ${ROOT}/components/wifi_manager/wifi_retry_policy.c) target_include_directories(test_wifi_retry_policy PRIVATE ${ROOT}/components/wifi_manager/include) +# wifi_fallback — the AP-fallback transitions, driven against a fake radio. +# Same split as above: wifi_manager.c keeps the esp_wifi calls, this file keeps +# the decisions, and the failure paths (a mode switch or AP config that errors) +# are the whole reason it is worth separating. +add_host_test(test_wifi_fallback + SOURCES test_wifi_fallback.c + ${ROOT}/components/wifi_manager/wifi_fallback.c + ${ROOT}/components/wifi_manager/wifi_retry_policy.c) +target_include_directories(test_wifi_fallback PRIVATE ${ROOT}/components/wifi_manager/include) + # cone_table — profile generation for every cone × speed × {preheat, slow_cool}. add_host_test(test_cone_table SOURCES test_cone_table.c ${ROOT}/components/cone_table/cone_table.c) diff --git a/tests/host/test_wifi_fallback.c b/tests/host/test_wifi_fallback.c new file mode 100644 index 00000000..a705730e --- /dev/null +++ b/tests/host/test_wifi_fallback.c @@ -0,0 +1,303 @@ +#include "unity.h" +#include "wifi_fallback.h" + +#include + +/* The AP-fallback transitions, driven against a fake radio. Every case below is + a failure mode of the real thing: a mode switch that errors, an AP config + that errors, a client that keeps the AP open past STA recovery. On hardware + those are transient and rare, which is exactly why they went unnoticed — + each one used to leave the controller in a state only a reboot cleared. */ + +#define MS_US(ms) ((int64_t)(ms) * 1000) +#define SEC_US(s) ((int64_t)(s) * 1000 * 1000) +#define MIN_US(m) SEC_US((m) * 60) + +/* Fallback is entered at a non-zero boot offset so a policy that treats "never + attempted" as epoch-zero shows up. */ +#define FALLBACK_US SEC_US(90) + +#define STA_IP "10.0.0.7" + +typedef struct { + int64_t now_us; + + /* Return codes the fake radio hands back (0 = success). */ + int set_ap_mode_rc; + int apply_ap_config_rc; + int set_sta_mode_rc; + + int set_ap_mode_calls; + int apply_ap_config_calls; + int set_sta_mode_calls; + int sta_connect_calls; + int ap_up_calls; + int ap_down_calls; +} fake_radio_t; + +static fake_radio_t s_radio; + +static int fake_set_ap_mode(void) +{ + s_radio.set_ap_mode_calls++; + return s_radio.set_ap_mode_rc; +} + +static int fake_apply_ap_config(void) +{ + s_radio.apply_ap_config_calls++; + return s_radio.apply_ap_config_rc; +} + +static int fake_set_sta_mode(void) +{ + s_radio.set_sta_mode_calls++; + return s_radio.set_sta_mode_rc; +} + +static void fake_sta_connect(void) +{ + s_radio.sta_connect_calls++; +} + +static void fake_ap_up(void) +{ + s_radio.ap_up_calls++; +} + +static void fake_ap_down(void) +{ + s_radio.ap_down_calls++; +} + +static int64_t fake_now_us(void) +{ + return s_radio.now_us; +} + +static const wifi_fallback_ops_t k_fake_ops = { + .set_ap_mode = fake_set_ap_mode, + .apply_ap_config = fake_apply_ap_config, + .set_sta_mode = fake_set_sta_mode, + .sta_connect = fake_sta_connect, + .ap_up = fake_ap_up, + .ap_down = fake_ap_down, + .now_us = fake_now_us, +}; + +static wifi_fallback_t s_fb; + +void setUp(void) +{ + memset(&s_radio, 0, sizeof(s_radio)); + s_radio.now_us = FALLBACK_US; + wifi_fallback_init(&s_fb, &k_fake_ops, true); +} + +void tearDown(void) +{ +} + +/* One worker tick. */ +static wifi_retry_action_t tick(bool sta_connected, int ap_clients) +{ + return wifi_fallback_service(&s_fb, sta_connected, ap_clients); +} + +/* Tick once a second for `duration_us`, the way the worker task polls. */ +static void run_for(int64_t duration_us, bool sta_connected, int ap_clients) +{ + for (int64_t elapsed = 0; elapsed < duration_us; elapsed += SEC_US(1)) { + s_radio.now_us += SEC_US(1); + tick(sta_connected, ap_clients); + } +} + +/* The happy path, as a baseline for everything below. */ +static void enter_fallback_cleanly(void) +{ + wifi_fallback_request_ap(&s_fb); + tick(false, 0); + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(1, s_radio.ap_up_calls); +} + +/* ── the AP transition must survive its own failures ───────────────────── */ + +/* A transient esp_wifi_set_mode() error used to be terminal: the single queued + "enter AP fallback" command had already been consumed, and the service loop + started with "is the AP up? no → nothing to do". The kiln was then left with + neither the configured network nor its own provisioning AP until a reboot. */ +void test_mode_switch_failure_is_retried_until_it_lands(void) +{ + s_radio.set_ap_mode_rc = -1; + wifi_fallback_request_ap(&s_fb); + + for (int i = 0; i < 5; i++) { + s_radio.now_us += SEC_US(1); + tick(false, 0); + } + TEST_ASSERT_FALSE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(0, s_radio.ap_up_calls); + TEST_ASSERT_GREATER_OR_EQUAL_INT(5, s_radio.set_ap_mode_calls); /* still trying */ + + s_radio.set_ap_mode_rc = 0; + s_radio.now_us += SEC_US(1); + tick(false, 0); + + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(1, s_radio.ap_up_calls); +} + +/* Same story one step later: the mode switch succeeded but pushing the SSID and + PSK did not. The AP was marked active and advertised anyway, so callers were + told to look for a network that was never configured — and on the + no-credentials boot path there is no STA link to fall back to. */ +void test_ap_is_not_advertised_until_its_config_lands(void) +{ + s_radio.apply_ap_config_rc = -1; + wifi_fallback_request_ap(&s_fb); + + tick(false, 0); + TEST_ASSERT_FALSE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_FALSE(wifi_fallback_ap_only(&s_fb, false)); + TEST_ASSERT_EQUAL_STRING(STA_IP, wifi_fallback_reported_ip(&s_fb, false, STA_IP)); + TEST_ASSERT_EQUAL_INT(0, s_radio.ap_up_calls); + + /* The whole sequence is re-attempted, not just the half that failed. */ + s_radio.apply_ap_config_rc = 0; + s_radio.now_us += SEC_US(1); + tick(false, 0); + + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(2, s_radio.set_ap_mode_calls); + TEST_ASSERT_EQUAL_STRING(WIFI_FALLBACK_AP_IP, wifi_fallback_reported_ip(&s_fb, false, STA_IP)); +} + +/* Readiness (the bit app_main waits on before printing the setup-mode banner) + must not be raised while the AP is still only an intention. */ +void test_readiness_is_signalled_once_and_only_after_the_ap_is_up(void) +{ + s_radio.set_ap_mode_rc = -1; + wifi_fallback_request_ap(&s_fb); + run_for(SEC_US(10), false, 0); + TEST_ASSERT_EQUAL_INT(0, s_radio.ap_up_calls); + + s_radio.set_ap_mode_rc = 0; + run_for(SEC_US(10), false, 0); + + /* Exactly one, however many ticks pass: an idempotent transition. */ + TEST_ASSERT_EQUAL_INT(1, s_radio.ap_up_calls); +} + +/* ── recovery ──────────────────────────────────────────────────────────── */ + +void test_ap_is_dropped_once_the_sta_link_returns(void) +{ + enter_fallback_cleanly(); + + s_radio.now_us += SEC_US(1); + tick(true, 0); + + TEST_ASSERT_FALSE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(1, s_radio.set_sta_mode_calls); + TEST_ASSERT_EQUAL_INT(1, s_radio.ap_down_calls); +} + +void test_ap_is_held_open_while_a_client_is_associated(void) +{ + enter_fallback_cleanly(); + + run_for(MIN_US(5), true, 1); + + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(0, s_radio.set_sta_mode_calls); + TEST_ASSERT_EQUAL_INT(0, s_radio.ap_down_calls); +} + +/* The backoff has to be re-armed while the link is up. It was not: the state + still held the pre-recovery timestamp and escalated attempt count, so a + second outage fired a reconnect on the very next tick — and being that + overdue also walks straight through the bounded AP-client suppression, which + is measured in how overdue the attempt is. */ +void test_backoff_is_rearmed_while_the_sta_link_is_up(void) +{ + enter_fallback_cleanly(); + + /* An hour of fruitless retries pushes the backoff to its 5 min ceiling. */ + run_for(MIN_US(60), false, 0); + TEST_ASSERT_GREATER_THAN_INT(0, s_radio.sta_connect_calls); + + /* The router comes back, but a phone is sitting on the AP, so the fallback + stays up for another twenty minutes. */ + run_for(MIN_US(20), true, 1); + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + + int attempts_before = s_radio.sta_connect_calls; + + /* The router drops again. The next tick must not reconnect: the policy + should be counting from the link loss, not from an hour ago. */ + s_radio.now_us += SEC_US(1); + TEST_ASSERT_EQUAL(WIFI_RETRY_NOT_DUE, tick(false, 1)); + TEST_ASSERT_EQUAL_INT(attempts_before, s_radio.sta_connect_calls); + + /* Still nothing most of the way to the base backoff... */ + run_for(MS_US(WIFI_RETRY_BASE_MS) - SEC_US(5), false, 1); + TEST_ASSERT_EQUAL_INT(attempts_before, s_radio.sta_connect_calls); + + /* ...and when it does come due, the associated client suppresses it, which + an hour-overdue attempt would have ignored. */ + s_radio.now_us += SEC_US(10); + TEST_ASSERT_EQUAL(WIFI_RETRY_SUPPRESSED, tick(false, 1)); + TEST_ASSERT_EQUAL_INT(attempts_before, s_radio.sta_connect_calls); +} + +/* ── the address we tell people to use ─────────────────────────────────── */ + +/* An AP client holds the fallback open across a STA recovery; then the router + drops again. The device is reachable only at 192.168.4.1, but the cached LAN + address outlived the link and /api/v1/wifi kept advertising it. */ +void test_reported_ip_reverts_to_the_ap_when_the_sta_link_drops(void) +{ + enter_fallback_cleanly(); + TEST_ASSERT_EQUAL_STRING(WIFI_FALLBACK_AP_IP, wifi_fallback_reported_ip(&s_fb, false, "0.0.0.0")); + + /* STA back, AP held open by a client: the LAN address is the useful one. */ + run_for(SEC_US(5), true, 1); + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_FALSE(wifi_fallback_ap_only(&s_fb, true)); + TEST_ASSERT_EQUAL_STRING(STA_IP, wifi_fallback_reported_ip(&s_fb, true, STA_IP)); + + /* Router drops. Only the AP is reachable now. */ + TEST_ASSERT_TRUE(wifi_fallback_ap_only(&s_fb, false)); + TEST_ASSERT_EQUAL_STRING(WIFI_FALLBACK_AP_IP, wifi_fallback_reported_ip(&s_fb, false, STA_IP)); +} + +/* ── provisioning-only AP ──────────────────────────────────────────────── */ + +/* No credentials were ever saved, so there is nothing to reconnect to; the AP + must simply stay up rather than churning the radio. */ +void test_provisioning_only_ap_never_retries_sta(void) +{ + wifi_fallback_init(&s_fb, &k_fake_ops, false); + enter_fallback_cleanly(); + + run_for(MIN_US(60), false, 0); + + TEST_ASSERT_TRUE(wifi_fallback_ap_active(&s_fb)); + TEST_ASSERT_EQUAL_INT(0, s_radio.sta_connect_calls); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_mode_switch_failure_is_retried_until_it_lands); + RUN_TEST(test_ap_is_not_advertised_until_its_config_lands); + RUN_TEST(test_readiness_is_signalled_once_and_only_after_the_ap_is_up); + RUN_TEST(test_ap_is_dropped_once_the_sta_link_returns); + RUN_TEST(test_ap_is_held_open_while_a_client_is_associated); + RUN_TEST(test_backoff_is_rearmed_while_the_sta_link_is_up); + RUN_TEST(test_reported_ip_reverts_to_the_ap_when_the_sta_link_drops); + RUN_TEST(test_provisioning_only_ap_never_retries_sta); + return UNITY_END(); +}