diff --git a/src/display_service.cpp b/src/display_service.cpp index 8253dc0..57e259f 100644 --- a/src/display_service.cpp +++ b/src/display_service.cpp @@ -33,8 +33,9 @@ // region (0x76), and PIPE_WRITE (0x80-0x82). PIPE_WRITE is BLE-only, so BLE transfers // decode through tinfl here too. The WiFi keying of OPENDISPLAY_USE_TINFL selects // which builds opt in (the LAN wire is what makes software inflate the bottleneck and -// justifies tinfl's ~11 KB of DRAM tables); it does NOT restrict the engine to LAN -// traffic. See od_inflate_tinfl.h for the full rationale and RAM cost. +// justifies tinfl's ~11 KB of DRAM tables, and that flag is now set only on PSRAM +// envs, so a part without the DRAM budget never opts in); it does NOT restrict the +// engine to LAN traffic. See od_inflate_tinfl.h for the full rationale and RAM cost. #include "od_inflate_tinfl.h" #if OPENDISPLAY_USE_TINFL #define od_zlib_stream_reset od_inflate_tinfl_reset diff --git a/src/main.cpp b/src/main.cpp index b6c0548..4fc0f0a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -178,6 +178,12 @@ void setup() { // observed failing with -0x7f00 at 51 KB free / 31.7 KB largest block. No-op when // encryption is disabled. od_tls_reserve_records(); + // Strictly after the TLS slots: this prefers PSRAM and normally costs no internal + // DRAM at all, but on a board whose PSRAM is absent or dead it falls back to 16 KB + // of internal -- which must not land in the middle of the contiguous region the + // reservation above depends on. One call site serves both the normal-boot and the + // deep-sleep-wake path, which share this stretch of setup(). + odLanReserveRxBuffer(); #endif if (is_deep_sleep_wake) { od_log_info("[wake] << full_config_init >> initio"); od_log_flush(); } #if defined(TARGET_NRF) && defined(OPENDISPLAY_BOOT_DIAG) diff --git a/src/main.h b/src/main.h index d9f93e9..430b3c1 100644 --- a/src/main.h +++ b/src/main.h @@ -143,16 +143,19 @@ uint16_t wifiServerPort = 2446; // bool wifiServerConfigured = false; // dead -- nothing reads it (config_parser.cpp) #endif #ifdef OPENDISPLAY_HAS_WIFI -// Heavy WiFi-transport surface: the TCP server/client objects and the 16 KB RX -// reassembly buffer exist ONLY when the WiFi transport is compiled in -- every -// S3 env (E1004 inherits the flag from esp32-s3-N32R8-extuart), C6, and both C3 -// envs. Only the classic esp32-N4 reclaims this RAM. -// 16 KB = four max wire frames (OD_LAN_MAX_FRAME 4096): headroom for the -// streaming client to keep whole frames queued ahead of the parser. +// Heavy WiFi-transport surface: the TCP server/client objects and the RX +// reassembly buffer exist ONLY when the WiFi transport is compiled in -- the S3 +// envs (E1004 inherits the flag from esp32-s3-N32R8-extuart). The classic and +// no-PSRAM parts reclaim this RAM; see src/wifi_service.h for the env list. +// +// tcpReceiveBuffer is the DEFINITION of the pointer declared in wifi_service.h +// (included above); its storage is reserved at boot by odLanReserveRxBuffer(), +// in PSRAM where there is any. Size lives with the declaration as +// OD_LAN_RX_BUFFER_SIZE -- do not reintroduce a literal here. WiFiServer wifiServer; WiFiClient wifiClient; bool wifiServerConnected = false; -uint8_t tcpReceiveBuffer[16384]; +uint8_t* tcpReceiveBuffer = nullptr; uint32_t tcpReceiveBufferPos = 0; #endif diff --git a/src/od_inflate_tinfl.h b/src/od_inflate_tinfl.h index 8cd8385..e8721dc 100644 --- a/src/od_inflate_tinfl.h +++ b/src/od_inflate_tinfl.h @@ -16,6 +16,12 @@ * The gate below keys off OPENDISPLAY_ENABLE_WIFI only as a proxy for "a build that * cares about inflate throughput and can spare the RAM" — it does NOT mean the * engine is limited to WiFi traffic. Do not read the gate as a transport filter. + * The "can spare the RAM" half is now load-bearing rather than incidental: that flag + * is set only on envs with -DBOARD_HAS_PSRAM (see src/wifi_service.h), so the parts + * without PSRAM — esp32-c3-N4, esp32-c6-N4, esp32-c3-N16, plus the classic esp32-N4 + * and esp32-wrover-e-N4R8 — fall back to uzlib and reclaim the ~15 KB of tables + * below. They have no PSRAM to relocate anything into, and internal DRAM there is + * the scarcest resource on the part. * * WHY: the uzlib engine is a bit-serial, byte-at-a-time resumable state machine — * tolerable for BLE (wire << inflate), but the LAN wire is ~10-100x faster so diff --git a/src/wifi_service.cpp b/src/wifi_service.cpp index eb3062d..35236cb 100644 --- a/src/wifi_service.cpp +++ b/src/wifi_service.cpp @@ -38,8 +38,8 @@ extern uint16_t wifiServerPort; extern WiFiServer wifiServer; extern WiFiClient wifiClient; extern bool wifiServerConnected; -extern uint8_t tcpReceiveBuffer[16384]; -extern uint32_t tcpReceiveBufferPos; +// tcpReceiveBuffer / tcpReceiveBufferPos are declared in wifi_service.h (included +// above) so the pointer type is checked against its definition in main.h. extern uint8_t msd_payload[16]; // This file builds its log lines with Arduino String concatenation, while the rest @@ -250,6 +250,33 @@ void od_tls_reserve_records(void) { } } +void odLanReserveRxBuffer(void) { + if (tcpReceiveBuffer != nullptr) return; // idempotent, per od_tls_reserve_records + // calloc, not malloc: this was a .bss array and callers may read ahead of the write + // cursor on a partial frame. PSRAM first -- it is 16 KB of internal DRAM otherwise, + // on a part where mbedTLS alone needs 34 KB contiguous internal. + tcpReceiveBuffer = (uint8_t*)heap_caps_calloc(1, OD_LAN_RX_BUFFER_SIZE, + MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT); + const bool inPsram = (tcpReceiveBuffer != nullptr); + if (!inPsram) { + tcpReceiveBuffer = (uint8_t*)heap_caps_calloc(1, OD_LAN_RX_BUFFER_SIZE, + MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); + } + if (tcpReceiveBuffer == nullptr) { + lanLog("ERROR: LAN RX buffer reservation failed -- LAN transport will not start"); + return; + } + lanLog("LAN: reserved RX buffer " + String((unsigned)OD_LAN_RX_BUFFER_SIZE) + " B in " + + String(inPsram ? "PSRAM" : "DRAM") + + ", internal free=" + String((unsigned)heap_caps_get_free_size(MALLOC_CAP_INTERNAL)) + + ", largest block=" + String((unsigned)heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL))); + if (!inPsram) { + // The only signal that this board's PSRAM is absent or dead: + // CONFIG_SPIRAM_IGNORE_NOTFOUND=1 lets it boot silently. No reclaim here. + lanLog("WARNING: LAN RX buffer fell back to internal DRAM -- no PSRAM on this board?"); + } +} + // Build the shared server config once (RNG + PSK + one ECDHE-PSK ciphersuite). static bool tlsEnsureConfig(void) { // Late fallback: encryption can be turned on by a runtime config write, long after @@ -439,6 +466,13 @@ static void restartLanService(void) { } static void startLanServer(void) { + // No RX buffer, no listener. Refusing here is the whole degrade path: it covers + // every caller, and it is better than accepting a socket the parser cannot serve. + // BLE and the display path are unaffected. + if (tcpReceiveBuffer == nullptr) { + lanLog("ERROR: LAN RX buffer unavailable -- LAN transport disabled"); + return; + } tlsMode = isEncryptionEnabled(); uint16_t port = lanActivePort(); wifiServer.begin(port); @@ -864,7 +898,13 @@ static int s_lastLanReadErr = 0; // mbedTLS ret for the last OD_LAN_READ_ERROR // of bytes appended (>=0), OD_LAN_READ_CLOSED on a graceful peer close, or // OD_LAN_READ_ERROR on a fatal channel error (caller drops in both cases). static int lanReadIntoBuffer(void) { - int space = (int)sizeof(tcpReceiveBuffer) - (int)tcpReceiveBufferPos; + // Belt-and-braces: startLanServer() refuses to listen without the buffer, so no + // socket should reach here, but never index a null on the read path. + if (tcpReceiveBuffer == nullptr) return OD_LAN_READ_ERROR; + // OD_LAN_RX_BUFFER_SIZE, not sizeof(): tcpReceiveBuffer is a pointer now, and + // sizeof would silently yield 4/8 -- reads would collapse to a few bytes per tick + // and read as a network fault rather than a code bug. + int space = (int)OD_LAN_RX_BUFFER_SIZE - (int)tcpReceiveBufferPos; if (space <= 0) { lanLog("LAN RX buffer full, dropping connection"); return -1; @@ -1126,7 +1166,7 @@ void handleWiFiServer() { if (!wifiServerConnected || !wifiClient.connected()) { return; } - } while (got > 0 && drainedBytes < sizeof(tcpReceiveBuffer)); + } while (got > 0 && drainedBytes < OD_LAN_RX_BUFFER_SIZE); } void restartWiFiLanAfterReconnect() { diff --git a/src/wifi_service.h b/src/wifi_service.h index c6ad953..e5fa910 100644 --- a/src/wifi_service.h +++ b/src/wifi_service.h @@ -6,13 +6,26 @@ // OPENDISPLAY_HAS_WIFI gates the entire WiFi/LAN transport surface (mDNS, TCP // server, TLS-PSK listener, RX reassembly buffer, LAN response framing). It is // defined only on ESP32 targets built with -DOPENDISPLAY_ENABLE_WIFI, which is -// applied to every S3, C6, and C3 platformio env (esp32-s3-E1004 sets no flag of -// its own but inherits it from esp32-s3-N32R8-extuart). TWO classic-ESP32 envs lack -// it -- esp32-N4 and esp32-wrover-e-N4R8 -- so neither compiles the WiFi surface, -// and both reclaim the 16 KB RX buffer + WiFiServer/WiFiClient RAM. Note -// esp32-wrover-e-N4R8 is NOT in platformio.ini's default_envs, so a bare `pio run` -// skips it: it ships via .github/firmware-targets.json, and it is the target most -// likely to catch a broken #ifndef OPENDISPLAY_HAS_WIFI path. Build it explicitly. Call sites in +// applied to every S3 platformio env (esp32-s3-E1004 sets no flag of its own but +// inherits it from esp32-s3-N32R8-extuart). +// +// SET THAT FLAG ONLY ON ENVS THAT ALSO SET -DBOARD_HAS_PSRAM. It is the single +// control point for two DRAM-expensive subsystems, because it is also what gates +// OPENDISPLAY_USE_TINFL (src/od_inflate_tinfl.h) -- together they cost roughly +// 50 KB of internal DRAM: 34,816 B of mbedTLS record slots (od_tls_reserve_records +// below), tinfl's 15,120 B of tables, and a 16 KB RX buffer that PSRAM builds +// relocate off the internal heap (odLanReserveRxBuffer below). A part without +// PSRAM has nowhere to put any of it -- see the DRAM-exhaustion panics that +// motivated this. Nothing in code enforces the pairing; this comment and the +// per-env notes in platformio.ini are the guard. +// +// FIVE envs therefore lack it and compile no WiFi surface, reclaiming the RX +// buffer + WiFiServer/WiFiClient RAM and falling back to uzlib for inflate: +// esp32-N4, esp32-wrover-e-N4R8 (classic ESP32) and esp32-c3-N4, esp32-c6-N4, +// esp32-c3-N16 (no PSRAM). Note esp32-wrover-e-N4R8 is NOT in platformio.ini's +// default_envs, so a bare `pio run` skips it: it ships via +// .github/firmware-targets.json, and it is the target most likely to catch a +// broken #ifndef OPENDISPLAY_HAS_WIFI path. Build it explicitly. Call sites in // main.cpp / communication.cpp / display_service.cpp / device_control.cpp / // config_parser.cpp are #ifdef-guarded on this macro. #if defined(TARGET_ESP32) && defined(OPENDISPLAY_ENABLE_WIFI) @@ -29,6 +42,29 @@ // free, because the two buffers need contiguous internal DRAM. void od_tls_reserve_records(void); +// LAN RX reassembly buffer. 16 KB = four max wire frames (OD_LAN_MAX_FRAME 4096): +// headroom for a streaming client to keep whole frames queued ahead of the parser. +// +// Declared HERE rather than in main.h so that the definition (main.h, which includes +// this header) is type-checked against it. It used to be a 16 KB array defined in +// main.h and re-declared `extern uint8_t[16384]` in wifi_service.cpp, with the size +// literal duplicated: because C++ does not mangle namespace-scope variable names, +// converting one side to a pointer and not the other would have LINKED CLEANLY and +// then read the first bytes of the buffer as a pointer. +#define OD_LAN_RX_BUFFER_SIZE 16384u +extern uint8_t* tcpReceiveBuffer; +extern uint32_t tcpReceiveBufferPos; + +// Reserve the RX buffer above, preferring PSRAM so it costs no internal DRAM -- 16 KB +// that is idle during every BLE transfer anyway, since the link-owner rule makes LAN +// and BLE mutually exclusive. Falls back to internal DRAM (i.e. exactly the old static +// cost) if PSRAM is absent or dead, which CONFIG_SPIRAM_IGNORE_NOTFOUND=1 makes silent. +// Call from setup() AFTER od_tls_reserve_records(): on the fallback path this takes +// internal DRAM and must not fragment the contiguous blocks mbedTLS needs. Idempotent; +// never freed. On failure startLanServer() refuses to listen rather than accepting a +// socket the parser cannot serve. +void odLanReserveRxBuffer(void); + void initWiFi(bool waitForConnection = true); void disconnectWiFiServer(); /**