fix(wifi): stabilize hosted softap bringup#175
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| bool HalEsp32::wifi_init() | ||
| { | ||
| static bool s_wifi_task_started = false; | ||
| auto& state = wifi_state(); | ||
| portMUX_TYPE& spinlock = wifi_state_spinlock(); | ||
|
|
||
| mclog::tagInfo(TAG, "wifi init"); | ||
| portENTER_CRITICAL(&spinlock); | ||
| bool already_started = state.started; | ||
| bool previously_failed = state.failed; | ||
| state.attempted = true; | ||
| portEXIT_CRITICAL(&spinlock); | ||
|
|
||
| esp_err_t ret = nvs_flash_init(); | ||
| if (ret == ESP_ERR_NVS_NO_FREE_PAGES || ret == ESP_ERR_NVS_NEW_VERSION_FOUND) { | ||
| ESP_ERROR_CHECK(nvs_flash_erase()); | ||
| ret = nvs_flash_init(); | ||
| if (already_started) | ||
| { | ||
| return true; | ||
| } | ||
| if (previously_failed) | ||
| { | ||
| ESP_LOGW(TAG, "Skipping Wi-Fi init after previous failure"); |
There was a problem hiding this comment.
[P1] Guard against concurrent wifi_init calls
The new runtime state tracks only started and failed, so two tasks calling startWifiAp() at the same time will both see state.started == false and enter the body of wifi_init() despite the spinlock. This results in two simultaneous calls to bsp_set_wifi_power_enable, esp_hosted_slave_reset, and start_softap, which can double‑reset the hosted coprocessor and race esp‑netif initialisation. The previous implementation offloaded the work to a single task so duplicate invocations were effectively idempotent, but the synchronous refactor makes the race far more likely and can leave the Wi‑Fi stack in an undefined state. Consider marking an “in‑progress” flag (e.g. reuse attempted) before releasing the lock so that concurrent callers bail out until the first attempt completes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Guarded
wifi_init()with an in-progress check so concurrent callers bail out instead of double-triggering SoftAP bring-up, avoiding duplicate resets of the hosted coprocessor. platforms/tab5/main/hal/components/hal_wifi.cppL211-L239
Testing
⚠️ idf.py build(fails: command not found in Codex workspace)
Summary
esp_hosted_slave_reset()before enabling the hosted SoftAPstartWifiAp()so callers can see that provisioning is unavailableTesting
https://chatgpt.com/codex/tasks/task_e_68d03a4e550483248162aad6d5b44af9