Skip to content

feat(wifi/lan): WiFi/LAN transport + tinfl inflate; pin pioarduino 55.03.39#124

Merged
jonasniesner merged 5 commits into
OpenDisplay:mainfrom
davelee98:feat/tinfl5
Jul 25, 2026
Merged

feat(wifi/lan): WiFi/LAN transport + tinfl inflate; pin pioarduino 55.03.39#124
jonasniesner merged 5 commits into
OpenDisplay:mainfrom
davelee98:feat/tinfl5

Conversation

@davelee98

@davelee98 davelee98 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Ports the WiFi/LAN transport and the ROM-tinfl inflate path onto current main, plus the build and cleanup work that came out of getting it green.

Five commits, each self-contained — happy to split the build/refactor ones into separate PRs if you'd rather review the feature alone.

2f38224 — WiFi/LAN transport + tinfl inflate

Squashed replay of a 31-commit branch. A commit-by-commit rebase wasn't viable: #120 (Seeed GFX → FastEPD) and #121 (writeSerialod_log_*) touch the same files, so every intermediate commit conflicted on the same renames and none would have compiled.

  • LAN transport — mDNS identity TXT, port/mode selection, TLS-PSK listener, command-origin routing with CCM bypass on TLS, per-transport session ownership, Nagle off, drain-to-dry RX with single compaction.
  • WiFi — strongest-AP selection with RTC-cached BSSID, roaming at −75 dBm, all work moved out of the event callbacks (fixes a panic — handlers ran String concatenation on a 4 KB stack), lazy RSSI-low handler registration.
  • TLS — both mbedTLS record buffers pre-reserved at boot while internal DRAM is still contiguous. ssl_setup was failing in the field at 51 KB free / 31.7 KB largest block, because it needs two ~16.7 KB contiguous internal-DRAM blocks and cycles 33 KB per connection.
  • Inflate — compressed transfers routed through ROM tinfl, LZ dict sized to the actual DEFLATE window.

Conflict resolutions kept your od_log_* form while preserving this branch's content: transport tag and pre-enqueue [Q:n] depth on the TX line, backlog-only queue logging, compression mode/ratio on DW start/complete, and SSID/password redaction in the config dump. setConfigLoggingQuiet()/cfgInfo() were dropped — they existed to skip the config dump in the deep-sleep wake window, which the compile-time OD_LOG_LEVEL default (INFO) now does by eliding every od_log_debug.

Note: the vendored include/opendisplay_protocol.h resolved to main's copy, which is newer than the canonical one in opendisplay-protocol (upstream added the 0x43 version-response docs). That repo needs a --pull, or the next --push reverts them.

2645c43 — pin pioarduino 55.03.39, make the C6 mempool script non-fatal

Moves the pin from 55.03.32 forward to 55.03.39 (Arduino 3.3.9 / IDF 5.5.4). This restores the platform's own merged-image post-build action — firmware.factory.bin next to firmware.bin, flashable at offset 0. That action doesn't exist in 55.03.32, so local merged images disappeared when the floating stable URL was pinned back in 0d95b37.

Deliberately not 55.03.311, which is what stable currently serves. IDF 5.5.5 drops the r_ prefix on the C6 BLE controller's mempool exports and moves them from libble_app.a to libbt.a (r_os_mempool_initos_mempool_init, likewise os_memblock_get/_put). NimBLE-Arduino 2.5.0 — latest release, 2026-04-02 — still calls the r_ names, so esp32-c6-N4 fails to link there. No library-side fix exists; it has to land upstream in NimBLE-Arduino. Only C6 is affected: it alone ships a precompiled controller blob carrying its own NimBLE porting layer (CONFIG_BT_LE_CONTROLLER_NPL_OS_PORTING_SUPPORT=y). Full analysis in docs/FINDINGS_C6_NIMBLE_IDF555_MEMPOOL_ABI_2026-07-25.md, and the constraint is recorded in platformio.ini and CLAUDE.md so the next bump doesn't walk into it.

esp32c6_nimble_mempool_link.py now warns and skips on every failure path instead of sys.exit(1). On 55.03.311 the hard exit aborted with no entry os_mempool.c.o in archive, hiding the real cause behind a missing-member error; skipping lets the link proceed and report the genuine undefined reference to r_os_mempool_init.

Measured across three platform versions, all eleven envs:

Platform C6 without script C6 with script Other 10
55.03.32 links links (no-op, +28 B) OK
55.03.39 links links (no-op, +148 B) OK
55.03.311 link fails link fails OK

47432e0esp32-s3-N16R8-extuart-debug env

OD_LOG_LEVEL defaults to INFO, which compiles away every od_log_debug() — and that's where the config dump, BLE RX/TX hex with [BLE][Q:n] depth, and DIRECT_WRITE throughput live. Adds a variant env via extends (same pattern as esp32-s3-E1004) rather than a flag on the shipping one. Costs 12,392 bytes flash, no extra RAM. CI and release.yml enumerate envs explicitly, so neither picks it up.

799f324 — delete the WiFi power-save scaffolding

The transfer-scoped WIFI_PS_NONE experiment was already gutted after hardware measurement showed it collapses throughput under BLE coex. What remained was a subsystem that couldn't do anything: lanPsSuspended was never assigned true anywhere, so lanPowerSaveSuspend() was an empty body, lanPowerSaveRestore() early-returned before touching the radio, and lanPowerSaveSuspended() was a constant false — yet eight call sites and an 18-line loop() safety net with its own timer and warning log still guarded it, and the header advertised the original semantics with no hint the functions were inert.

Also drops the two esp_wifi_set_ps(WIFI_PS_MIN_MODEM) calls, so the firmware never sets power-save mode at all. MIN_MODEM is the driver default and exactly what those calls set, so runtime behaviour should be unchanged — flagging it explicitly since that's an assumption about the IDF default rather than something measured. A note in wifi_service.h records why nothing calls esp_wifi_set_ps(), so "force PS_NONE during transfers" doesn't read like an untried optimisation.

Separately: the two DW complete: … KB/s lines move from od_log_debug to od_log_info, so upload throughput is visible in a normal build.

d785ac0transferActive()

Three flags mark an in-flight transfer — directWriteActive (global in main.h), pipeState.active and partialCtx.active (file-static in display_service.cpp). Callers meaning "a transfer is in flight" spelled the disjunction out themselves, and drifted: the log predicates had all three (longhand, twice), the roam gate had direct+pipe, the touch poll had direct only.

Two behaviour changes fall out, both widening an existing skip:

  • Roam gate — a BLE-origin PARTIAL write with no LAN client attached could be interrupted mid-stream by a full-channel scan. Neither the LAN-client check nor the direct+pipe check fired, though the surrounding comment already claimed BLE transfers were protected.
  • Touch poll — GT911 I²C reads are now skipped during PIPE and PARTIAL too. The original rationale (bus and loop-task contention) applies identically to all three.

Clear-timing was checked before unifying: all three flags are cleared before their panel teardown, so the predicate drops at the same relative point and gates open no earlier than before. Left deliberately specific: the direct-write watchdog and teardown in main.cpp, and the (directWriteActive || partialCtx.active) 0x0072 session-ownership check that excludes PIPE.

Also removes two dead symbols: handleButtonPress(uint8_t) (declared 2026-02-12, never defined or called — processButtonEvents() does the job) and partialWriteActive() (only caller was the deleted power-save net).

Verification

Hardware: the WiFi/LAN transport and TLS-PSK encryption have been flashed and exercised end-to-end on an E1003 — the full chain from Home Assistant through py-opendisplay to the firmware, not just a local loopback. That covers the substance of 2f38224: association, mDNS discovery, the TLS-PSK listener, command-origin routing with the CCM bypass, and image delivery over LAN.

Build: all eleven environments compile on 55.03.39nrf52840custom, all five S3 variants, esp32-s3-E1004, both C3, esp32-c6-N4, esp32-N4.

Not separately exercised on hardware, and worth a look during review:

  • C6 BLE on the newer IDF. The pin bump changes the controller blob under NimBLE-Arduino; C6 links and builds clean, but a C6 board should advertise, connect and complete a transfer before this is trusted.
  • The two gate changes in d785ac0 — roaming now deferred during PARTIAL writes, touch polling suppressed during PIPE/PARTIAL. Both only widen an existing skip, so the worst case should be touch being unresponsive slightly longer during an upload; a GT911 device running a long partial-write stream is the case to exercise.
  • The esp_wifi_set_ps() removal rests on WIFI_PS_MIN_MODEM being the driver default, which is reasoning from the IDF default rather than a measurement.

One environment note: 55.03.311 requires PlatformIO Core ≥ 6.1.19 (it silently uninstalls itself on older Core). The pinned .39 has no such requirement.

Squashed replay of feat/tinfl4 (31 commits) onto the post-2.24.4 main. A
commit-by-commit rebase was not viable: upstream landed two sweeping
refactors in the same files -- writeSerial() -> od_log_* (OpenDisplay#121) and Seeed
GFX -> FastEPD (OpenDisplay#120) -- so every intermediate commit would have conflicted
on the same renames and none would have compiled.

Carried over from feat/tinfl4:
  - LAN transport: mDNS identity TXT, port/mode selection, TLS-PSK listener,
    command-origin routing with CCM bypass on TLS, per-transport session
    ownership, Nagle off, drain-to-dry RX with single compaction.
  - WiFi: strongest-AP selection with RTC-cached BSSID, roaming at -75 dBm,
    all work moved out of the event callbacks (panic fix), lazy RSSI-low
    handler registration, no WIFI_PS_NONE mid-transfer.
  - TLS: both mbedTLS record buffers pre-reserved at boot, while internal
    DRAM is still contiguous; graceful close is not a read error.
  - Inflate: compressed transfers routed through ROM tinfl, LZ dict sized to
    the actual DEFLATE window.

Resolutions worth noting:
  - Log call sites took upstream's od_log_* form, keeping this branch's
    content: transport tag + pre-enqueue [Q:n] depth on the TX line,
    backlog-only queue logging, compression mode/ratio on DW start/complete,
    and SSID/password redaction in the config dump.
  - The vendored protocol header resolved to main's copy, which is newer
    than both this branch's and the canonical repo's (upstream added the
    0x43 version-response docs). ../opendisplay-protocol needs a --pull.
  - setConfigLoggingQuiet()/cfgInfo() dropped: they existed to skip the
    config dump in the deep-sleep wake window, which upstream's compile-time
    OD_LOG_LEVEL (default INFO) now does by eliding every od_log_debug.
  - communication.cpp's static send_wifi_lan_frame() dropped in favour of
    this branch's opendisplay_lan_send_frame() in wifi_service.cpp.
  - wifi_service.cpp keeps its String-built log lines behind one file-local
    lanLog() adapter that routes to od_log_* by ERROR:/WARNING: prefix,
    rather than reflowing 60-odd call sites into format strings.

Builds: nrf52840custom, esp32-s3-N16R8, esp32-c3-N16, esp32-c6-N4.
Not flashed -- no hardware verification of the LAN/TLS paths on this port.
…on-fatal

Moves the ESP pin from 55.03.32 forward to 55.03.39 (Arduino 3.3.9 / IDF
5.5.4), which restores the platform's own merged-image post-build action --
`firmware.factory.bin` next to firmware.bin, flashable at offset 0. That
action does not exist in 55.03.32, which is why local merged images
disappeared when the floating `stable` URL was pinned back.

Deliberately NOT 55.03.311, the release `stable` currently serves. IDF 5.5.5
drops the r_ prefix on the C6 BLE controller's mempool exports and moves them
from libble_app.a to libbt.a (r_os_mempool_init -> os_mempool_init, likewise
os_memblock_get/_put). NimBLE-Arduino 2.5.0 -- the latest release, 2026-04-02
-- still calls the r_ names, so esp32-c6-N4 fails to link there with a wall of
`undefined reference to r_os_mempool_init`. No library-side fix exists; it has
to land upstream in NimBLE-Arduino. Only C6 is affected: it alone ships a
precompiled controller blob carrying its own NimBLE porting layer
(CONFIG_BT_LE_CONTROLLER_NPL_OS_PORTING_SUPPORT=y). The other ten envs build
clean on 5.5.5.

esp32c6_nimble_mempool_link.py now warns and skips on every failure path
instead of sys.exit(1). Its job is to force linkage the ordinary archive scan
usually manages anyway, so when its assumptions break the right outcome is to
let the link proceed and report what it finds. The hard exit did the opposite:
on 55.03.311 it aborted with "no entry os_mempool.c.o in archive", hiding the
rename behind a missing-member error. It now surfaces the genuine undefined
reference, which names the actual problem. `ar x` failure was previously an
uncaught CalledProcessError traceback; it is now a skip like the rest.

Verified locally, all eleven environments:
  - 55.03.39  : all 11 build; C6 links with AND without the script
  - 55.03.311 : 10 build; C6 fails to link either way
  - factory.bin restored (S3 1.56 MB, C6 1.82 MB)
A --defsym alias workaround for 5.5.5 does link but is NOT applied -- it
aliases BLE mempool allocation to symbols assumed ABI-identical by name, which
would corrupt the BLE heap at runtime rather than fail at build time. Recorded
in the findings doc for whoever does the eventual bump.

Also corrects CLAUDE.md: CI builds all eleven envs, not three.
OD_LOG_LEVEL defaults to INFO, which compiles away every od_log_debug() call
-- and that is where the useful diagnostics live: the config parse dump and
summary, the BLE RX/TX hex lines with [BLE][Q:n] queue depth, and the
DIRECT_WRITE progress plus the "DW complete: ... KB/s" throughput line. None of
it appears in a normal build.

Adds a variant env rather than a flag on the shipping one, following the
esp32-s3-E1004 `extends` precedent. CI (.github/workflows/main.yaml) and
release.yml both enumerate envs explicitly, so neither picks this up and no
published artifact can accidentally ship debug logging.

Costs 12,392 bytes of flash and no extra RAM (1,520,466 vs 1,508,074; RAM
135,676 both). Logs over the inherited CH343P UART on GPIO43/44 at 115200,
not USB CDC.
…put at INFO

The transfer-scoped WIFI_PS_NONE experiment (48f4ecf) was gutted by a5b7143
after hardware measurement showed it collapses throughput: WiFi and BLE share
one radio with software coex compiled in, and the coex arbiter needs WiFi's
modem-sleep windows to time-share the antenna with the always-on BLE
advertiser. WIFI_PS_NONE tells the AP "I never sleep", which is a lie under
coex -- downlink lands while the device is off-channel and TCP retransmit +
backoff does the rest. The DTIM ack-ladder stall it was chasing measured
negligible.

What was left behind was a complete subsystem that could not do anything:
lanPsSuspended was never assigned true anywhere, so lanPowerSaveSuspend() was
an empty body, lanPowerSaveRestore() early-returned before touching the radio,
and lanPowerSaveSuspended() was a constant false. Eight call sites and an
18-line loop() safety net -- with its own static timer, 5-second re-arm window
and warning log -- all guarded a condition that could never be true. The
header still advertised the original semantics with no hint the functions were
inert, so anyone reading it would believe power save was being managed.

Removes all of it: the flag, the three functions, both restore call sites in
wifi_service.cpp, the two suspend guards and two restore calls in
display_service.cpp, the loop() safety net, and the header declarations.

Also drops the two esp_wifi_set_ps(WIFI_PS_MIN_MODEM) calls, so the firmware
now never sets power-save mode at all. MIN_MODEM is the driver default and is
exactly what those calls set, so runtime behaviour is unchanged. A note in
wifi_service.h records why nothing calls esp_wifi_set_ps() -- without it,
"force PS_NONE during transfers" reads like an untried optimisation and will
be re-added.

Separately, the two "DW complete: ... KB/s" lines move from od_log_debug to
od_log_info so upload throughput is visible in a normal build rather than only
in a DEBUG one.

Builds: nrf52840custom, esp32-s3-N16R8, esp32-c3-N16, esp32-c6-N4.
Not flashed -- no hardware verification.
…and log gates

Three flags mark an in-flight transfer -- directWriteActive (a global in main.h),
plus pipeState.active and partialCtx.active (both file-static in
display_service.cpp). Every caller that just meant "a transfer is in flight" had
to spell the disjunction out itself, and they drifted:

  - imageWriteLogQuietCmd/Ack had all three, written longhand, twice
  - the WiFi roam gate had direct+pipe, missing PARTIAL
  - the GT911 touch poll had direct only, missing PIPE and PARTIAL

Adds bool transferActive() next to pipeWriteActive(), in the one TU that can see
all three flags, and routes those four call sites through it. Adding a fourth
transfer type is now a one-line change here instead of a hunt across three files.

Two behaviour changes fall out, both widening an existing skip:

  - Roam gate: a BLE-origin PARTIAL write with no LAN client attached could be
    interrupted mid-stream by a full-channel scan -- neither the LAN-client check
    nor the direct+pipe check fired. The surrounding comment already claimed BLE
    transfers were protected; now they are.
  - Touch poll: GT911 I2C reads are skipped during PIPE and PARTIAL streams too.
    The original rationale (touch reads contend with the transfer for the bus and
    the loop task) applies identically to all three.

Clear-timing was checked before unifying: all three flags are cleared before
their panel teardown, so the unified predicate drops at the same relative point
as each individual flag did and the gates open no earlier than before.

Left deliberately specific: the direct-write watchdog and teardown in main.cpp,
and the (directWriteActive || partialCtx.active) session-ownership check for
0x0072, which excludes PIPE on purpose.

Also removes two dead symbols found while surveying:
  - handleButtonPress(uint8_t) -- declared in main.h by f378685 (2026-02-12) and
    never defined or called; processButtonEvents() does the job.
  - partialWriteActive() -- its only caller was the power-save safety net deleted
    in 799f324. transferActive() now covers the need it existed for.

touch_input.cpp no longer externs directWriteActive, so it stops reaching into
another module's global.

Builds: nrf52840custom, esp32-s3-N16R8, esp32-c3-N16, esp32-c6-N4.
Not flashed -- neither gate change is hardware-verified.
@davelee98
davelee98 requested a review from jonasniesner as a code owner July 25, 2026 16:52
@jonasniesner
jonasniesner merged commit 2e2131b into OpenDisplay:main Jul 25, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants