Preserve healthy OTA access points and drain stopping uploaders safely

src/helpers/ESP32Board.cpp

@@ -67,8 +67,6 @@ bool ESP32Board::isUserGpioAvailable(uint8_t pin) const {
 #include <freertos/task.h>
 #include <strings.h>
 
-static bool lightweight_ota_started_ap;
-
 // Full Companion retains its established uploader URL on port 8080.
 // Infrastructure uses port 80; CLI startup hands WebConfig's WiFi to OTA.
 #if defined(COMPANION_RADIO_FULL)
@@ -214,6 +212,7 @@ class LightweightOTAServer {
       }
     } while (line[0]);
 
+    if (!running) return;
     if (get_page) sendPage(client);
     else if (get_log) sendLog(client);
     else if (post_update) receiveUpdate(client, content_length);
@@ -256,11 +255,17 @@ public:
     return true;
   }
 
-  void end() {
+  bool isRunning() const { return running; }
+
+  bool end() {
     running = false;
     server.stop();
     for (unsigned i = 0; task != nullptr && i < 100; i++) delay(10);
+    // A slow client can still be returning from a socket write. Keep its
+    // board/session alive until the task exits; a retry can finish cleanup.
+    if (task != nullptr) return false;
     board = nullptr;
+    return true;
   }
 };
 
@@ -268,6 +273,10 @@ static LightweightOTAServer lightweight_ota_server;
 
 bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) {
   (void)id;
+  if (ota_server != nullptr && !lightweight_ota_server.isRunning()) {
+    strcpy(reply, "ERR: OTA stopping; retry stop ota first");
+    return false;
+  }
 #if defined(COMPANION_RADIO_FULL)
   const esp_partition_t* running = esp_ota_get_running_partition();
   const esp_partition_t* next = esp_ota_get_next_update_partition(nullptr);
@@ -283,17 +292,19 @@ bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) {
   inhibit_sleep = true;
 
   IPAddress ip;
-  const bool use_ap = force_ap || WiFi.status() != WL_CONNECTED;
+  const bool use_ap = force_ap || WiFi.status() != WL_CONNECTED
+      || static_cast<uint32_t>(WiFi.localIP()) == 0;
   if (!use_ap) {
     ip = WiFi.localIP();
   } else {
-    if (!lightweight_ota_started_ap) {
+    if (!ota_started_ap || !(WiFi.getMode() & WIFI_AP)
+        || static_cast<uint32_t>(WiFi.softAPIP()) == 0) {
       const IPAddress ap_ip(192, 168, 4, 1);
       const IPAddress ap_mask(255, 255, 255, 0);
-      lightweight_ota_started_ap = WiFi.softAPConfig(ap_ip, ap_ip, ap_mask)
+      ota_started_ap = WiFi.softAPConfig(ap_ip, ap_ip, ap_mask)
           && WiFi.softAP("MeshCore-OTA", nullptr);
     }
-    if (!lightweight_ota_started_ap) {
+    if (!ota_started_ap) {
       inhibit_sleep = ota_server != nullptr;
       strcpy(reply, "ERR: OTA WiFi failed");
       return false;
@@ -303,8 +314,8 @@ bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) {
 
   if (ota_server == nullptr) {
     if (!lightweight_ota_server.begin(this)) {
-      if (lightweight_ota_started_ap) WiFi.softAPdisconnect(true);
-      lightweight_ota_started_ap = false;
+      if (ota_started_ap) WiFi.softAPdisconnect(true);
+      ota_started_ap = false;
       inhibit_sleep = false;
       strcpy(reply, "ERR: OTA server failed");
       return false;
@@ -329,10 +340,13 @@ bool ESP32Board::stopOTAUpdate(char reply[]) {
     return true;
   }
 
-  lightweight_ota_server.end();
+  if (!lightweight_ota_server.end()) {
+    strcpy(reply, "ERR: OTA stopping; retry stop ota");
+    return false;
+  }
   ota_server = nullptr;
-  if (lightweight_ota_started_ap) WiFi.softAPdisconnect(true);
-  lightweight_ota_started_ap = false;
+  if (ota_started_ap) WiFi.softAPdisconnect(true);
+  ota_started_ap = false;
   inhibit_sleep = false;
   strcpy(reply, "OK - OTA stopped");
   MESH_DEBUG_PRINTLN("stopOTAUpdate: %s", reply);
@@ -361,14 +375,19 @@ bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) {
   // reachable even when the joined network applies client isolation and the
   // station IP can't be reached.
   IPAddress ip;
-  const bool use_ap = force_ap || WiFi.status() != WL_CONNECTED;
+  const bool use_ap = force_ap || WiFi.status() != WL_CONNECTED
+      || static_cast<uint32_t>(WiFi.localIP()) == 0;
   if (!use_ap) {
     ip = WiFi.localIP();
   } else {
-    const IPAddress ap_ip(192, 168, 4, 1);
-    const IPAddress ap_mask(255, 255, 255, 0);
-    if (!WiFi.softAPConfig(ap_ip, ap_ip, ap_mask)
-        || !WiFi.softAP("MeshCore-OTA", NULL)) {
+    if (!ota_started_ap || !(WiFi.getMode() & WIFI_AP)
+        || static_cast<uint32_t>(WiFi.softAPIP()) == 0) {
+      const IPAddress ap_ip(192, 168, 4, 1);
+      const IPAddress ap_mask(255, 255, 255, 0);
+      ota_started_ap = WiFi.softAPConfig(ap_ip, ap_ip, ap_mask)
+          && WiFi.softAP("MeshCore-OTA", NULL);
+    }
+    if (!ota_started_ap) {
       inhibit_sleep = ota_server != nullptr;
       strcpy(reply, "ERR: OTA WiFi failed");
       return false;
@@ -385,9 +404,9 @@ bool ESP32Board::startOTAUpdate(const char* id, char reply[], bool force_ap) {
   if (ota_server != nullptr) return true;
 
   static char id_buf[60];
-  sprintf(id_buf, "%s (%s)", id, getManufacturerName());
+  snprintf(id_buf, sizeof(id_buf), "%s (%s)", id, getManufacturerName());
   static char home_buf[90];
-  sprintf(home_buf, "<H2>Hi! I am a MeshCore Repeater. ID: %s</H2>", id);
+  snprintf(home_buf, sizeof(home_buf), "<H2>Hi! I am a MeshCore Repeater. ID: %s</H2>", id);
 
   ota_server = new AsyncWebServer(80);
 
@@ -414,7 +433,8 @@ bool ESP32Board::stopOTAUpdate(char reply[]) {
   ota_server->end();
   delete ota_server;
   ota_server = nullptr;
-  WiFi.softAPdisconnect(true);
+  if (ota_started_ap) WiFi.softAPdisconnect(true);
+  ota_started_ap = false;
   inhibit_sleep = false;
 
   strcpy(reply, "OK - OTA stopped");

src/helpers/ESP32Board.h

@@ -39,6 +39,7 @@ class ESP32Board : public mesh::MainBoard {
 protected:
   uint8_t startup_reason;
   bool inhibit_sleep = false;
+  bool ota_started_ap = false;
 #if MESH_ESP32_USB_CONSOLE_COOPERATIVE
   mesh::UsbHostSleepPolicy usb_host_sleep_policy;
 #endif

test/fixtures/wifi_ota_start.cpp

@@ -10,6 +10,7 @@
 #define MESH_DEBUG_PRINTLN(...) ((void)0)
 constexpr int WL_CONNECTED = 3;
 constexpr int HTTP_GET = 0;
+constexpr int WIFI_AP = 2;
 struct IPAddress {
   std::string value;
   IPAddress() = default;
@@ -18,11 +19,13 @@ struct IPAddress {
         + std::to_string(c) + "." + std::to_string(d);
   }
   std::string toString() const { return value; }
+  explicit operator uint32_t() const { return value.empty() || value == "0.0.0.0" ? 0 : 1; }
 };
 struct WiFiFake {
   bool connected = true, ap = false, allow_ap = true;
   unsigned starts = 0;
   int status() const { return connected ? WL_CONNECTED : 0; }
+  int getMode() const { return ap ? WIFI_AP : 0; }
   IPAddress localIP() const { return IPAddress(10, 20, 30, 40); }
   IPAddress softAPIP() const { return ap ? IPAddress(192, 168, 4, 1) : IPAddress(0, 0, 0, 0); }
   bool softAPConfig(IPAddress ip, IPAddress gateway, IPAddress mask) {
@@ -54,6 +57,7 @@ struct Elegant {
 static int SPIFFS;
 struct ESP32Board {
   bool inhibit_sleep = false;
+  bool ota_started_ap = false;
 #ifdef LIGHTWEIGHT_WIFI_OTA
   void* ota_server = nullptr;
 #else
@@ -64,11 +68,11 @@ struct ESP32Board {
   bool stopOTAUpdate(char*);
 };
 struct LightweightServer {
-  bool allow = true;
-  bool begin(ESP32Board*) { ++server_starts; return allow; }
-  void end() {}
+  bool allow = true, running = false, allow_stop = true;
+  bool begin(ESP32Board*) { ++server_starts; running = allow; return allow; }
+  bool isRunning() const { return running; }
+  bool end() { running = false; return allow_stop; }
 } lightweight_ota_server;
-static bool lightweight_ota_started_ap;
 #ifdef COMPANION_RADIO_FULL
 static constexpr uint16_t LIGHTWEIGHT_OTA_PORT = 8080;
 #else
@@ -114,6 +118,20 @@ int main() {
   // In particular, an already-running LAN server must honor a later `ap`.
   start(true, "192.168.4.1", "Join WiFi MeshCore-OTA");
   assert(WiFi.ap && WiFi.connected && server_starts == 1);
+  const auto ap_starts = WiFi.starts;
+  start(true, "192.168.4.1", "Join WiFi MeshCore-OTA");
+  assert(WiFi.starts == ap_starts); // Healthy repeats retain associated clients.
+  WiFi.ap = false; // An external driver stop invalidates cached AP ownership.
+  start(true, "192.168.4.1", "Join WiFi MeshCore-OTA");
+  assert(WiFi.starts == ap_starts + 1);
+#ifdef LIGHTWEIGHT_WIFI_OTA
+  lightweight_ota_server.allow_stop = false;
+  assert(!board.stopOTAUpdate(reply.text));
+  assert(strstr(reply.text, "OTA stopping") && board.inhibit_sleep && board.ota_server);
+  assert(!board.startOTAUpdate("test node", reply.text, true));
+  assert(strstr(reply.text, "retry stop ota first"));
+  lightweight_ota_server.allow_stop = true;
+#endif
   assert(board.stopOTAUpdate(reply.text));
   assert(!board.inhibit_sleep && !board.ota_server && !WiFi.ap && WiFi.connected);
 

test/test_wifi_ota_lifecycle.py

@@ -1,9 +1,7 @@
 """Execute production WiFi ownership, ESP-NOW cleanup and ElegantOTA startup.
 
-Desired-behavior assertions are marked as known failures until firmware is fixed. Set
-MESHCORE_REQUIRE_WIFI_OTA_FIX=1 to make those regressions ordinary failures.
-Compilation, every fixture execution and observation parsing happen in
-setUpClass, outside expectedFailure, so infrastructure failures cannot pass.
+All lifecycle regressions are required to pass. Compilation, every fixture
+execution and observation parsing happen in setUpClass.
 OTA startup coverage deliberately selects ElegantOTA, not LightweightOTA.
 """
 from pathlib import Path
@@ -39,12 +37,14 @@ STUBS = r'''
 #define WITH_ESPNOW_BRIDGE 1
 #define MESH_DEBUG_PRINTLN(...) ((void)0)
 constexpr int WL_CONNECTED = 3, WL_DISCONNECTED = 6, HTTP_GET = 0;
+constexpr int WIFI_AP = 2;
 
 struct IPAddress {
   uint32_t value = 0;
   IPAddress() = default;
   IPAddress(uint8_t a, uint8_t b, uint8_t c, uint8_t d)
       : value((uint32_t(a) << 24) | (uint32_t(b) << 16) | (uint32_t(c) << 8) | d) {}
+  explicit operator uint32_t() const { return value; }
   std::string toString() const {
     char text[16];
     std::snprintf(text, sizeof(text), "%u.%u.%u.%u", (value >> 24) & 255,
@@ -62,6 +62,7 @@ struct MockWiFi {
   bool initialized_cache = false, started_cache = false;
   IPAddress station_ip, ap_ip, configured_ap_ip;
   int status() const { return station_status; }
+  int getMode() const { return sdk_ap_active ? WIFI_AP : 0; }
   IPAddress localIP() const { return station_ip; }
   IPAddress softAPIP() const { return ap_ip; }
   bool softAPConfig(IPAddress ip, IPAddress, IPAddress) {
@@ -108,6 +109,7 @@ struct MockSPIFFS {} SPIFFS;
 class ESP32Board {
 public:
   bool inhibit_sleep = false;
+  bool ota_started_ap = false;
   AsyncWebServer* ota_server = nullptr;
   ~ESP32Board() { delete ota_server; }
   const char* getManufacturerName() const { return "test board"; }
@@ -289,9 +291,8 @@ class WifiOtaLifecycleTests(unittest.TestCase):
     def test_espnow_alone_is_not_infrastructure_wifi(self):
         self.assertEqual(self.observations["owners_espnow"]["enabled"], 4)
 
-    @unittest.expectedFailure
     def test_http_ota_alone_owns_wifi(self):
-        """Known bug: enabled() omits board HTTP OTA ownership in both AP and STA modes."""
+        """HTTP OTA owns WiFi in both AP and STA modes."""
         self.assertEqual(tuple(self.observations[scenario]["enabled"] & 1
                                for scenario in ("owners_httpota", "owners_httpota_sta")), (1, 1))
 
@@ -321,9 +322,8 @@ class WifiOtaLifecycleTests(unittest.TestCase):
     def test_espnow_without_infrastructure_allows_driver_cleanup(self):
         self.assert_cleanup("cleanup_espnow", 1)
 
-    @unittest.expectedFailure
     def test_http_ota_owner_protects_wifi_from_bridge_cleanup(self):
-        """Known bug: raw cleanup stops/deinits the AP or STA owned only by HTTP OTA."""
+        """Bridge cleanup retains the AP or STA owned only by HTTP OTA."""
         self.assertEqual(tuple((self.observations[scenario]["stop_calls"],
                                 self.observations[scenario]["deinit_calls"],
                                 self.observations[scenario]["endpoint_usable"],
@@ -334,7 +334,8 @@ class WifiOtaLifecycleTests(unittest.TestCase):
     def assert_healthy_ota(self, scenario, url, ap_starts):
         observation = self.observations[scenario]
         self.assertTrue(observation["first_started"] and observation["started"])
-        self.assertEqual(observation["reply"], f"Started: http://{url}/update")
+        network = "Join WiFi MeshCore-OTA" if url == "192.168.4.1" else "Use same WiFi/LAN"
+        self.assertEqual(observation["reply"], f"Started: http://{url}/update - WiFi on; {network}")
         self.assertEqual(observation["server_count"], 1)
         self.assertEqual(observation["server_begin_calls"], 1)
         self.assertEqual(observation["ap_start_calls"], ap_starts)
@@ -351,9 +352,8 @@ class WifiOtaLifecycleTests(unittest.TestCase):
     def test_healthy_station_repeat_start_is_idempotent(self):
         self.assert_healthy_ota("ota_sta_repeat", "10.0.0.7", 0)
 
-    @unittest.expectedFailure
     def test_stale_server_cannot_claim_success_at_zero_ap_ip(self):
-        """Known bug: non-null ElegantOTA server bypasses dead-AP validation/recovery."""
+        """A non-null ElegantOTA server still validates/recovers a dead AP."""
         observation = self.observations["ota_stale_ap"]
         if observation["started"]:
             self.assertTrue(observation["endpoint_usable"])
@@ -371,12 +371,5 @@ class WifiOtaLifecycleTests(unittest.TestCase):
         self.assertFalse(observation["ota_running"] or observation["sleep_inhibited"])
 
 
-if os.environ.get("MESHCORE_REQUIRE_WIFI_OTA_FIX") == "1":
-    # A strict run must fail on current firmware, and passes only after fixes.
-    for test in vars(WifiOtaLifecycleTests).values():
-        if getattr(test, "__unittest_expecting_failure__", False):
-            test.__unittest_expecting_failure__ = False
-
-
 if __name__ == "__main__":
     unittest.main()

test/test_wifi_ota_start.py

@@ -216,6 +216,33 @@ int main() {
 '''
         self.compile_and_run(fixture.replace("@COMMAND@", command), "-DWITH_WEBCONFIG=1")
 
+    def test_lightweight_stop_keeps_slow_client_state_until_task_exits(self):
+        source = (ROOT / "src/helpers/ESP32Board.cpp").read_text()
+        implementation = source[source.index("class LightweightOTAServer {"):]
+        method = extract_braced(implementation, "bool end()")
+        fixture = r'''
+#include <cassert>
+#include <cstdint>
+static unsigned delays = 0;
+static void delay(unsigned value) { assert(value == 10); ++delays; }
+struct Server { void stop() {} };
+struct Updater {
+  bool running = true;
+  Server server;
+  void* task = reinterpret_cast<void*>(1);
+  void* board = reinterpret_cast<void*>(2);
+  @END@
+};
+int main() {
+  Updater updater;
+  assert(!updater.end());
+  assert(!updater.running && updater.task && updater.board && delays == 100);
+  updater.task = nullptr; // Slow network write has now returned and task exited.
+  assert(updater.end() && !updater.board && delays == 100);
+}
+'''
+        self.compile_and_run(fixture.replace("@END@", method))
+
 
 if __name__ == "__main__":
     unittest.main()