Merge pull request #2 from swwilshub/claude/fix-build-failure-YVLEc

Claude/fix build failure yvl ec

platformio.ini

@@ -28,6 +28,25 @@ build_flags =
   -std=c++2a
 # Enable UTF-8 long file names in SdFat
   -DUSE_UTF8_LONG_NAMES=1
+# LWIP TCP/IP stack optimizations for WiFi file transfer performance
+# These settings optimize buffer sizes and TCP parameters for maximum throughput
+  -DCONFIG_LWIP_MAX_SOCKETS=10
+  -DCONFIG_LWIP_TCP_MSS=1436
+  -DCONFIG_LWIP_TCP_SND_BUF_DEFAULT=5744
+  -DCONFIG_LWIP_TCP_WND_DEFAULT=5744
+  -DCONFIG_LWIP_TCP_RECVMBOX_SIZE=12
+  -DCONFIG_LWIP_UDP_RECVMBOX_SIZE=12
+  -DCONFIG_LWIP_TCPIP_RECVMBOX_SIZE=32
+  -DCONFIG_LWIP_TCP_RTO_TIME=3000
+# WiFi performance optimizations
+  -DCONFIG_ESP32_WIFI_STATIC_RX_BUFFER_NUM=16
+  -DCONFIG_ESP32_WIFI_DYNAMIC_RX_BUFFER_NUM=32
+  -DCONFIG_ESP32_WIFI_TX_BUFFER_TYPE=1
+  -DCONFIG_ESP32_WIFI_DYNAMIC_TX_BUFFER_NUM=32
+# TCP optimizations for file uploads
+  -DCONFIG_LWIP_TCP_OVERSIZE=1
+  -DCONFIG_LWIP_WND_SCALE=1
+  -DCONFIG_LWIP_TCP_RCV_SCALE=2
 
 ; Board configuration
 board_build.flash_mode = dio

src/activities/network/CrossPointWebServerActivity.cpp

@@ -48,7 +48,7 @@ void CrossPointWebServerActivity::onEnter() {
   updateRequired = true;
 
   xTaskCreate(&CrossPointWebServerActivity::taskTrampoline, "WebServerActivityTask",
-              2048,               // Stack size
+              6144,               // Stack size (increased from 2KB to 6KB for stability)
               this,               // Parameters
               1,                  // Priority
               &displayTaskHandle  // Task handle
@@ -147,6 +147,11 @@ void CrossPointWebServerActivity::onNetworkModeSelected(const NetworkMode mode)
     // AP mode - start access point
     state = WebServerActivityState::AP_STARTING;
     updateRequired = true;
+
+    // WiFi performance optimizations for AP mode
+    WiFi.setSleep(false);                 // Disable WiFi sleep
+    WiFi.setTxPower(WIFI_POWER_19_5dBm);  // Maximum TX power for ESP32-C3
+
     startAccessPoint();
   }
 }
@@ -187,6 +192,12 @@ void CrossPointWebServerActivity::startAccessPoint() {
   WiFi.mode(WIFI_AP);
   delay(100);
 
+  // WiFi performance optimizations for maximum throughput
+  WiFi.setSleep(false);                 // Disable WiFi sleep
+  WiFi.setTxPower(WIFI_POWER_19_5dBm);  // Maximum TX power for ESP32-C3
+
+  Serial.printf("[%lu] [WEBACT] WiFi optimizations applied (sleep disabled, max TX power)\n", millis());
+
   // Start soft AP
   bool apStarted;
   if (AP_PASSWORD && strlen(AP_PASSWORD) >= 8) {
@@ -300,6 +311,15 @@ void CrossPointWebServerActivity::loop() {
       constexpr int HANDLE_CLIENT_ITERATIONS = 10;
       for (int i = 0; i < HANDLE_CLIENT_ITERATIONS && webServer->isRunning(); i++) {
         webServer->handleClient();
+
+        // CRITICAL: Yield to WiFi stack and other tasks between iterations
+        // This prevents WiFi stack starvation in STA mode and improves stability
+        yield();
+
+        // Add small delay every few iterations to reduce CPU pressure
+        if (i % 3 == 2) {
+          delay(1);  // 1ms delay every 3 iterations
+        }
       }
       lastHandleClientTime = millis();
     }

src/network/CrossPointWebServer.cpp

@@ -26,7 +26,7 @@ CrossPointWebServer::CrossPointWebServer() {}
 CrossPointWebServer::~CrossPointWebServer() { stop(); }
 
 void CrossPointWebServer::begin() {
-  if (running) {
+  if (running.load(std::memory_order_acquire)) {
     Serial.printf("[%lu] [WEB] Web server already running\n", millis());
     return;
   }
@@ -51,13 +51,13 @@ void CrossPointWebServer::begin() {
   Serial.printf("[%lu] [WEB] Creating web server on port %d...\n", millis(), port);
   server.reset(new WebServer(port));
 
-  // Disable WiFi sleep to improve responsiveness and prevent 'unreachable' errors.
-  // This is critical for reliable web server operation on ESP32.
-  WiFi.setSleep(false);
+  // WiFi performance optimizations for maximum throughput
+  WiFi.setSleep(false);  // Disable WiFi sleep to improve responsiveness
 
-  // Note: WebServer class doesn't have setNoDelay() in the standard ESP32 library.
-  // We rely on disabling WiFi sleep for responsiveness.
+  // Set WiFi TX power to maximum for best signal and throughput
+  WiFi.setTxPower(WIFI_POWER_19_5dBm);  // Maximum power for ESP32-C3
 
+  Serial.printf("[%lu] [WEB] WiFi optimizations applied (sleep disabled, max TX power)\n", millis());
   Serial.printf("[%lu] [WEB] [MEM] Free heap after WebServer allocation: %d bytes\n", millis(), ESP.getFreeHeap());
 
   if (!server) {
@@ -86,7 +86,7 @@ void CrossPointWebServer::begin() {
   Serial.printf("[%lu] [WEB] [MEM] Free heap after route setup: %d bytes\n", millis(), ESP.getFreeHeap());
 
   server->begin();
-  running = true;
+  running.store(true, std::memory_order_release);
 
   Serial.printf("[%lu] [WEB] Web server started on port %d\n", millis(), port);
   // Show the correct IP based on network mode
@@ -96,14 +96,15 @@ void CrossPointWebServer::begin() {
 }
 
 void CrossPointWebServer::stop() {
-  if (!running || !server) {
-    Serial.printf("[%lu] [WEB] stop() called but already stopped (running=%d, server=%p)\n", millis(), running,
+  const bool wasRunning = running.load(std::memory_order_acquire);
+  if (!wasRunning || !server) {
+    Serial.printf("[%lu] [WEB] stop() called but already stopped (running=%d, server=%p)\n", millis(), wasRunning,
                   server.get());
     return;
   }
 
   Serial.printf("[%lu] [WEB] STOP INITIATED - setting running=false first\n", millis());
-  running = false;  // Set this FIRST to prevent handleClient from using server
+  running.store(false, std::memory_order_release);
 
   Serial.printf("[%lu] [WEB] [MEM] Free heap before stop: %d bytes\n", millis(), ESP.getFreeHeap());
 
@@ -111,33 +112,39 @@ void CrossPointWebServer::stop() {
   delay(100);
   Serial.printf("[%lu] [WEB] Waited 100ms for handleClient to finish\n", millis());
 
-  server->stop();
-  Serial.printf("[%lu] [WEB] [MEM] Free heap after server->stop(): %d bytes\n", millis(), ESP.getFreeHeap());
+  // Lock mutex to ensure no handleClient() is currently accessing server
+  std::lock_guard<std::mutex> lock(serverMutex);
 
-  // Add another delay before deletion to ensure server->stop() completes
-  delay(50);
-  Serial.printf("[%lu] [WEB] Waited 50ms before deleting server\n", millis());
+  if (server) {
+    server->stop();
+    Serial.printf("[%lu] [WEB] [MEM] Free heap after server->stop(): %d bytes\n", millis(), ESP.getFreeHeap());
 
-  server.reset();
-  Serial.printf("[%lu] [WEB] Web server stopped and deleted\n", millis());
-  Serial.printf("[%lu] [WEB] [MEM] Free heap after delete server: %d bytes\n", millis(), ESP.getFreeHeap());
+    // Add another delay before deletion to ensure server->stop() completes
+    delay(50);
+    Serial.printf("[%lu] [WEB] Waited 50ms before deleting server\n", millis());
 
-  // Note: Static upload variables (uploadFileName, uploadPath, uploadError) are declared
-  // later in the file and will be cleared when they go out of scope or on next upload
+    server.reset();
+    Serial.printf("[%lu] [WEB] Web server stopped and deleted\n", millis());
+    Serial.printf("[%lu] [WEB] [MEM] Free heap after delete server: %d bytes\n", millis(), ESP.getFreeHeap());
+  }
+
+  // Upload state is now instance variables and will be cleaned up automatically
   Serial.printf("[%lu] [WEB] [MEM] Free heap final: %d bytes\n", millis(), ESP.getFreeHeap());
 }
 
 void CrossPointWebServer::handleClient() const {
   static unsigned long lastDebugPrint = 0;
 
-  // Check running flag FIRST before accessing server
-  if (!running) {
+  // Check running flag FIRST before accessing server (atomic read)
+  if (!running.load(std::memory_order_acquire)) {
     return;
   }
 
-  // Double-check server pointer is valid
+  // Lock mutex to safely access server pointer
+  std::lock_guard<std::mutex> lock(serverMutex);
+
+  // Double-check server pointer is valid while holding mutex
   if (!server) {
-    Serial.printf("[%lu] [WEB] WARNING: handleClient called with null server!\n", millis());
     return;
   }
 
@@ -261,22 +268,27 @@ void CrossPointWebServer::handleFileListData() const {
   server->setContentLength(CONTENT_LENGTH_UNKNOWN);
   server->send(200, "application/json", "");
   server->sendContent("[");
-  char output[512];
-  constexpr size_t outputSize = sizeof(output);
   bool seenFirst = false;
   JsonDocument doc;
 
-  scanFiles(currentPath.c_str(), [this, &output, &doc, seenFirst](const FileInfo& info) mutable {
+  scanFiles(currentPath.c_str(), [this, &doc, seenFirst](const FileInfo& info) mutable {
     doc.clear();
     doc["name"] = info.name;
     doc["size"] = info.size;
     doc["isDirectory"] = info.isDirectory;
     doc["isEpub"] = info.isEpub;
 
-    const size_t written = serializeJson(doc, output, outputSize);
-    if (written >= outputSize) {
-      // JSON output truncated; skip this entry to avoid sending malformed JSON
-      Serial.printf("[%lu] [WEB] Skipping file entry with oversized JSON for name: %s\n", millis(), info.name.c_str());
+    // Calculate required size for JSON output
+    const size_t requiredSize = measureJson(doc) + 1;  // +1 for null terminator
+
+    // Dynamically allocate exact size needed (handles 500-char filenames safely)
+    std::unique_ptr<char[]> output(new char[requiredSize]);
+
+    const size_t written = serializeJson(doc, output.get(), requiredSize);
+    if (written >= requiredSize) {
+      // This should never happen with measureJson, but handle it anyway
+      Serial.printf("[%lu] [WEB] ERROR: JSON serialization failed for: %s (required: %d)\n", millis(),
+                    info.name.c_str(), requiredSize);
       return;
     }
 
@@ -285,7 +297,7 @@ void CrossPointWebServer::handleFileListData() const {
     } else {
       seenFirst = true;
     }
-    server->sendContent(output);
+    server->sendContent(output.get());
   });
   server->sendContent("]");
   // End of streamed response, empty chunk to signal client
@@ -293,21 +305,15 @@ void CrossPointWebServer::handleFileListData() const {
   Serial.printf("[%lu] [WEB] Served file listing page for path: %s\n", millis(), currentPath.c_str());
 }
 
-// Static variables for upload handling
-static FsFile uploadFile;
-static String uploadFileName;
-static String uploadPath = "/";
-static size_t uploadSize = 0;
-static bool uploadSuccess = false;
-static String uploadError = "";
+// Upload state is now instance variables in the class (see header)
+// with mutex protection for thread safety
 
 void CrossPointWebServer::handleUpload() const {
-  static unsigned long lastWriteTime = 0;
-  static unsigned long uploadStartTime = 0;
-  static size_t lastLoggedSize = 0;
+  // Lock upload mutex for thread-safe access to upload state
+  std::lock_guard<std::mutex> lock(uploadMutex);
 
   // Safety check: ensure server is still valid
-  if (!running || !server) {
+  if (!running.load(std::memory_order_acquire) || !server) {
     Serial.printf("[%lu] [WEB] [UPLOAD] ERROR: handleUpload called but server not running!\n", millis());
     return;
   }
@@ -315,10 +321,28 @@ void CrossPointWebServer::handleUpload() const {
   const HTTPUpload& upload = server->upload();
 
   if (upload.status == UPLOAD_FILE_START) {
+    // Check heap before starting upload
+    const size_t freeHeap = ESP.getFreeHeap();
+    if (freeHeap < 50000) {  // Less than 50KB free
+      uploadError = "Insufficient memory for upload";
+      Serial.printf("[%lu] [WEB] [UPLOAD] REJECTED - low memory: %d bytes\n", millis(), freeHeap);
+      return;
+    }
+
+    // Pre-allocate String capacities to avoid reallocations during upload
+    uploadFileName.clear();
+    uploadFileName.reserve(upload.filename.length() + 16);
     uploadFileName = upload.filename;
+
+    uploadPath.clear();
+    uploadPath.reserve(256);  // Typical path length
+
     uploadSize = 0;
     uploadSuccess = false;
-    uploadError = "";
+
+    uploadError.clear();
+    uploadError.reserve(128);  // Pre-allocate error string capacity
+
     uploadStartTime = millis();
     lastWriteTime = millis();
     lastLoggedSize = 0;
@@ -341,10 +365,12 @@ void CrossPointWebServer::handleUpload() const {
     }
 
     Serial.printf("[%lu] [WEB] [UPLOAD] START: %s to path: %s\n", millis(), uploadFileName.c_str(), uploadPath.c_str());
-    Serial.printf("[%lu] [WEB] [UPLOAD] Free heap: %d bytes\n", millis(), ESP.getFreeHeap());
+    Serial.printf("[%lu] [WEB] [UPLOAD] Free heap: %d bytes\n", millis(), freeHeap);
 
-    // Create file path
-    String filePath = uploadPath;
+    // Build file path efficiently with pre-allocation
+    String filePath;
+    filePath.reserve(uploadPath.length() + uploadFileName.length() + 2);
+    filePath = uploadPath;
     if (!filePath.endsWith("/")) filePath += "/";
     filePath += uploadFileName;
 

src/network/CrossPointWebServer.h

@@ -1,7 +1,10 @@
 #pragma once
 
+#include <SdFat.h>
 #include <WebServer.h>
 
+#include <atomic>
+#include <mutex>
 #include <vector>
 
 // Structure to hold file information
@@ -27,17 +30,30 @@ class CrossPointWebServer {
   void handleClient() const;
 
   // Check if server is running
-  bool isRunning() const { return running; }
+  bool isRunning() const { return running.load(std::memory_order_acquire); }
 
   // Get the port number
   uint16_t getPort() const { return port; }
 
  private:
   std::unique_ptr<WebServer> server = nullptr;
-  bool running = false;
-  bool apMode = false;  // true when running in AP mode, false for STA mode
+  std::atomic<bool> running{false};
+  mutable std::mutex serverMutex;  // Protects server pointer access
+  bool apMode = false;             // true when running in AP mode, false for STA mode
   uint16_t port = 80;
 
+  // Upload state (instance variables with mutex protection)
+  mutable std::mutex uploadMutex;
+  mutable FsFile uploadFile;
+  mutable String uploadFileName;
+  mutable String uploadPath;
+  mutable size_t uploadSize;
+  mutable bool uploadSuccess;
+  mutable String uploadError;
+  mutable unsigned long lastWriteTime;
+  mutable unsigned long uploadStartTime;
+  mutable size_t lastLoggedSize;
+
   // File scanning
   void scanFiles(const char* path, const std::function<void(FileInfo)>& callback) const;
   String formatFileSize(size_t bytes) const;