canSerialize() validated payload_len and the destination size but not whether the
path encoding is one writePath() will actually emit. writePath() self-guards
against overrunning the path array, but it does so by writing nothing and
returning 0 — a correctness problem, not a safety one, because getRawLength()
still counts the path. An over-long or reserved encoding therefore passed the
size check and then serialized to a truncated frame that was published as the
packet.
Worst case is path_len 0xFF with no payload: 63 hops of 4 bytes counts as 254
bytes, inside the 255-byte buffer, while writeTo() emits just the 2-byte header.
The `raw` field would carry 4 hex chars presented as the frame. Reserved 4-byte
hash encodings passed too, producing frames Packet::readFrom() rejects.
Now gated on Packet::isValidPathLen(), which rejects the reserved 4-byte hash
size and any count * size above MAX_PATH_SIZE in one predicate. It is the same
check readFrom() applies to every received packet, and TX packets are built via
setPathHashSizeAndCount() with real hash sizes, so no decodable packet is turned
away.
Two tests added for the cases a destination-size check cannot reach. The existing
truncation test passed for the wrong reason -- its payload_len of 4 pushed
getRawLength() to 258 and tripped the size check, masking the hole -- so it is
split into the >0xFF truncation case and the counted-length-fits case, with the
254/2-byte asymmetry asserted explicitly so it cannot be masked again.
274/274 native tests; both observer envs and an nRF52 repeater build clean.
The post-NTP-correction refresh looped to _max_active_slots, which is a count of
activation positions and never an index bound. The indices holding those
positions are not contiguous: a slot passed over by isSlotReady() -- or, since
the demand-driven work, by a failed setup -- leaves a higher index activated. On
a two-position board that meant slots 1 and 2 could be live while only indices 0
and 1 were scanned, so slot 3 kept a JWT issued against the pre-correction clock
until its own expiry or a reconnect regenerated it.
Now bounded by RUNTIME_MQTT_SLOTS. The existing guard already skips disabled,
non-JWT, and clientless slots, so widening the range cannot touch a slot that
was never set up.
Pre-existing (the loop predates the demand-driven work) and kept as its own
commit so it can be picked separately. Audited the other _max_active_slots uses:
all are count comparisons or log arguments, so this was the only misuse.
last_reconnect_attempt starts at zero and teardownSlot() re-zeroes it, so the
retry gate in maintainSlotConnections() reduced to "uptime >=
SLOT_SETUP_RETRY_INTERVAL". Past 60 s of uptime a failed setup was therefore
retried on the next maintenance pass rather than 60 s later -- in the same task
iteration for a live reconfigure, since reconfigure processing runs before
maintenance in the loop.
setupSlot() now stamps last_reconnect_attempt on each failure that represents a
real attempt (client allocation, and both JWT token paths), so all three callers
get the interval measured from the failure. The bounds check and the !enabled
early return are not attempts and stay unstamped. The reconnect ladder reads
this field only for slots with initial_connect_done set, which a failed setup
never sets, so reconnect timing is unaffected.
The retry path's own pre-call stamp is kept as a backstop for any future
false-returning path that does not stamp itself.
reconnect_attempted_this_cycle is computed once before the maintenance loop and
only maintainSlotConnection() was setting it. The deferred-setup retry armed the
15 s cross-slot guard via _last_slot_reconnect_ms but left the local flag false,
so a disconnected slot later in the same pass still saw "no reconnect yet" and
started a second TLS handshake. Two concurrent ~40 KB sessions is exactly the
contention the guard prevents, and it landed in the one situation where internal
heap is already known to be short -- a failed allocation is why the retry runs.
Set on success only: setupSlot() returns true only after client->connect(), so
success means a handshake was launched. A failed retry launches nothing and
continues to spend only setup_retry_this_cycle, which rate-limits the allocation
attempts without consuming the handshake allowance.
[P1] A failed setup no longer strands the slot. setupSlot() returns bool and the
startup loops count only successful activations, so a slot that fails on a client
allocation neither consumes an active-slot position (starving a later healthy
broker on capped hardware) nor sits dead forever: maintainSlotConnections()
previously skipped clientless slots and the reconnect ladder is gated on
initial_connect_done, so nothing retried it. It now retries an enabled but
unactivated slot on a 60 s timer, one per cycle, gated on the same
_slots_setup_done ordering so the NTP-deferred setup sequence is preserved.
[P2] JWT setup no longer proceeds without a usable token. Both the preset and
custom-audience paths returned after ignoring createSlotAuthToken()'s result,
then called connect() and latched initial_connect_done -- so the token-allocation
failure introduced by the previous commit produced an unauthenticated attempt
exactly when memory was exhausted. They now return false and let the retry path
handle it.
[P2] ensureSlotAuthToken() no longer clears an existing token. It cleared
unconditionally, so every renewal wiped the current token before JWTHelper ran;
a renewal that then failed left an empty password where the inline buffer used to
preserve working credentials (JWTHelper writes only on success). Only freshly
allocated buffers are initialised now.
[P2] Raw publications reuse the shared document. buildRawJSON() reached
MQTTPayloadBuilder::buildRawMessage(), which constructed its own default
JsonDocument and therefore malloc'd and freed an internal-heap variant pool per
message -- on the highest-rate topic. The document is threaded through both
builders and the bridge passes _json_scratch_doc.
[P3] The writeTo() guard validates the source fields, not just the destination.
A corrupt payload_len of MAX_PACKET_PAYLOAD + 1 still leaves getRawLength()
inside MAX_TRANS_UNIT, so writeTo() read past packet->payload. Sizing and
validation moved to a pure MQTTWireScratch header with host tests covering the
accept/reject edges, matching the MQTTPacketFilter/MQTTConnectionPolicy pattern.
Two findings fell out: MAX_PATH_SIZE one-byte hops is not encodable (the hop
count is 6 bits, so 64 & 63 == 0; 32 two-byte hops is the widest real path), and
a zero-payload packet serializes but does not survive readFrom() -- pinned as a
test because it constrains any future wire-only queue.
[P3] Corrected the pool-size comment: these targets are 32-bit, so
ARDUINOJSON_SLOT_ID_SIZE is 2 and a pool block is 128 slots / 1024 bytes, not
4096. The 4096 figure came from a pre-existing comment near
NEIGHBORS_DOC_POOL_BUDGET, which is left alone -- its byte measurements are
empirical and still stand, only the block-size attribution is wrong.
Activation is now centralized in activatedSlotCount()/canActivateSlot(), used by
both startup loops, the retry path, and applySlotPreset(). That closes the
pre-existing divergence where a live preset change called setupSlot() without
consulting _max_active_slots, letting a non-PSRAM board reach three concurrent
TLS sessions against a cap of two. BEHAVIOUR CHANGE: a reconfigure that would
exceed the cap now logs and leaves the slot inactive instead of connecting.
Reconfiguring an already-active slot still works, because teardownSlot() releases
its position first.
272/272 native tests pass (5 new); both observer envs and an nRF52 repeater build
clean. Flash 1593249 B non-PSRAM, 1555625 B PSRAM.
ensureSlotAuthToken()/releaseSlotAuthToken() now use the bridge's existing
psram_malloc/psram_free rather than malloc/free, so a JWT slot's 768-byte token
comes out of PSRAM instead of internal DRAM. On PSRAM boards this is the larger
half of the auth_token work: the previous commit reclaims 768 B per slot that
never creates a token, while this reclaims 768 B per slot that does -- up to
3840 B of internal DRAM on a fully configured five-slot board, which is where
the mbedTLS working set is competing for space.
No effect on non-PSRAM boards: psram_malloc falls back to internal DRAM, so the
buffer stays exactly where the previous commit left it. Same for a
BOARD_HAS_PSRAM board whose PSRAM fails to initialise.
Safe to move because every access is a CPU copy on the bridge task, never DMA,
an ISR, or a cache-disabled window: JWTHelper memcpy's the token into this
buffer, and esp-mqtt copies it out of _mqtt_cfg into its own internal-DRAM
storage when connect() applies the config. This is unlike the PSRAM-backed MQTT
task stack that was tried and reverted for resetting Heltec V4 boards, where
the fault was PSRAM execution context rather than a plain buffer read.
Split from the previous commit so it can be reverted alone if hardware soak
shows any PSRAM-related instability on the JWT path.
flash non-PSRAM 1592801 -> 1592865 B (+64)
PSRAM 1555213 -> 1555237 B (+24)
267/267 native tests pass; both observer envs and an nRF52 repeater build clean.
MQTTSlot carried an inline char auth_token[768] -- 768 of its 1192 bytes --
for every runtime slot, whether the slot was JWT, username/password, disabled,
or above the active-slot cap. Now a char* allocated by ensureSlotAuthToken()
from createSlotAuthToken(), the sole writer, which runs only once a slot is
confirmed to have a JWT audience.
Because _slots[] is a fixed member array, this shrinks the bridge object
unconditionally at construction and re-adds only what is used:
MQTTSlot 1192 -> 428 B (-764)
MQTTBridge non-PSRAM 12032 -> 9740 B (-2292)
PSRAM 10080 -> 5496 B (-4584)
The object is allocated as one contiguous block at boot, so a smaller boot-time
chunk is easier to satisfy and leaves a larger contiguous remainder; JWT slots
then take 768 B each as separate blocks. Net steady-state saving is 768 B per
slot that never creates a token -- 768 B on a fully configured board, up to
3840 B on a PSRAM board with one JWT slot. 22 of the 29 presets are
MQTT_AUTH_JWT, so this is mostly reclaiming unconfigured slots rather than
non-JWT ones.
Lifetime rules, which are the whole risk here:
- setCredentials() stores this pointer in _mqtt_cfg rather than copying, and
esp-mqtt re-reads it whenever a later connect() re-applies a dirtied config.
So the buffer is freed only alongside the client, in destroySlotClients(),
after the delete. MQTTSlot::broker_uri already carries an "avoids dangling
pointer" comment from this same hazard with setServer().
- teardownSlot() still clears the token to an empty string but keeps the
buffer: the client survives teardown and does not clear cfg->password (only
setupSlot()'s reconfigure branch does). Freeing there would dangle.
- Never freed per reconnect, preserving the churn-avoidance the inline buffer
was there for.
Allocation goes through MQTTRuntimeBufferLifecycle (already host-tested) using
plain malloc, so the buffer stays in internal DRAM exactly where it was when
inline. Failure propagates through createSlotAuthToken()'s existing bool.
The two call sites that create a token and then test it (preset JWT in
setupSlot, and the custom-slot audience path) now null-check first; the five
other readers are all inside if (createSlotAuthToken(...)) success branches and
need no change. destroySlotClients() releases the token unconditionally rather
than after its client null-check, so a token can never outlive its slot.
flash non-PSRAM 1592697 -> 1592801 B (+104)
PSRAM 1555109 -> 1555213 B (+104)
267/267 native tests pass; both observer envs build clean.
initSlotClients() created a PsychicMqttClient for every one of
RUNTIME_MQTT_SLOTS at begin(), without consulting slot.enabled or the active
cap -- even though presets are applied and _max_active_slots is computed
earlier in the same function. RUNTIME_MQTT_SLOTS is deliberately cap+1
(6/5 with PSRAM, 3/2 without), so at least one client was always unusable,
and a lightly-configured board wasted several.
Replaced with ensureSlotClient(index), called from setupSlot() -- i.e. only
for a slot that is enabled, inside the cap, and ready to connect. A client
that never reaches setupSlot() is completely inert: the reconnect ladder is
gated on initial_connect_done, which only setupSlot() sets. Retained for the
bridge lifetime as before, so reconfigure/reconnect still reuse one mbedTLS
context; that context is built by connect(), not by the constructor, so
deferring costs nothing but the 1284-byte object.
Internal DRAM saved, by configured slot count:
non-PSRAM (3 runtime / 2 cap) 1 configured 2568 B
2-3 1284 B
PSRAM (6 runtime / 5 cap) 1 configured 6420 B
2 5136 B
5-6 1284 B
A BOARD_HAS_PSRAM board whose PSRAM fails to init gets cap 2 against 6
runtime slots, so it saves at least 5136 B -- on the board that just lost its
PSRAM.
Two gates used "client != nullptr" as a proxy for "configured". Under eager
allocation that conjunct was always true and therefore harmless; with lazy
allocation it would have made shouldQueuePacketType() drop every packet
received before the post-NTP-sync slot setup, which is precisely the window
the queue exists to cover. Both now key on slot.enabled, matching the
documented intent above eligiblePacketSlots() that a configured-but-
disconnected broker is still a target.
formatSlotDiagReply() gains the !isSlotReady() -> "wait" branch that
get mqtt.status and getSlotStatusSnapshot() already have; that state
previously fell through to "disc" and read as a network fault when it is
really a missing token/IATA/credential. "no client" now means only what its
name says: the slot is ready but the client could not be allocated. That
state is newly reachable, so the allocation uses new (std::nothrow) -- this
framework enables C++ exceptions, and a throwing new on exhaustion would
panic the node instead of degrading one slot.
flash non-PSRAM 1592573 -> 1592697 B (+124)
PSRAM 1555001 -> 1555109 B (+108)
267/267 native tests pass; both observer envs build clean.
Five independent, behavior-preserving reductions. Measured on
Heltec_v3_repeater_observer_mqtt (non-PSRAM) and
ThinkNode_M7_repeater_observer_mqtt (PSRAM), 267/267 native tests green.
Drop the unused static-task bookkeeping. StaticTask_t _mqtt_task_tcb (344 B)
and StackType_t* _mqtt_task_stack were never used: there is no
xTaskCreateStatic call, the pointer was assigned nullptr immediately before
xTaskCreatePinnedToCore, and the two psram_free() calls on it were dead.
Size the wire-format scratch buffers from the protocol maximum. raw_hex[1024]
at three sites becomes 2*MAX_TRANS_UNIT+1, and raw_buf/reconstructed[512]
become MAX_TRANS_UNIT. Both writeTo() sites now check getRawLength() first:
writeTo() does not bounds-check and returns uint8_t, so the old 512-byte
buffers were the only thing absorbing a malformed payload_len, and the
post-hoc "raw_len > sizeof(buf)" test ran after the overrun.
Pass the already-known serialized length into publishToSlot() instead of
re-running strlen() per destination slot (up to 2 KB per packet per slot, and
NEIGHBORS_JSON_BUFFER_SIZE per neighbor snapshot). Same for the direct
publish in publishStatusToSlot().
Share one JSON buffer and one document across packet, raw, and status. All
publish paths serialize on the bridge task, so the separate status buffer and
document were never concurrent. Status keeps STATUS_JSON_BUFFER_SIZE as its
serialization ceiling, so which oversized status documents get dropped is
unchanged.
Route the document's pools through a PSRAM-preferring allocator. Under
ArduinoJson 7 StaticJsonDocument<N> is a deprecated empty subclass of
JsonDocument whose template argument only feeds capacity(); the object is
64 B and each pool block (4096 B here) came from plain malloc(), i.e. the
internal DRAM the mbedTLS working set needs. Mirrors NeighborsDocAllocator.
The old comment claiming an inline pool has been corrected.
Measured:
sizeof(MQTTBridge) non-PSRAM 13208 -> 12032 B (-1176, internal heap)
PSRAM 10492 -> 10080 B (-412, plus one fewer
768 B PSRAM allocation)
stack, non-PSRAM buildPacketJSON[FromRaw] 1264 -> 736 B
buildRawJSON 1136 -> 608 B
publishPacket 688 -> ~432 B
packetToHex 560 -> ~304 B
deepest publish chain ~2.6 -> ~1.8 KB of 8 KB
flash 1592513 -> 1592573 B (+60)
static RAM 74656 B unchanged -- MQTTBridge is heap-allocated, so
these savings are internal heap, not the linker figure
Deferred from the review: the QueuedPacket wire-only redesign (reward is
1.56 KB on non-PSRAM only, and Packet::readFrom() rejects payload_len == 0),
demand-driven slot clients/JWT tokens, and pool retention across publishes --
JsonDocument::to<T>() always calls clear(), which destroys pools, so
"retain pools by clearing the root object" needs a string-pool lifetime
analysis first.
13 commits, 6 files, zero conflicts. Records the byte-identical smoke-build
baseline, the LR1110 RX-timeout fix and the global nRF52 CC310 crypto switch,
and the fact that the ESP32-only smoke pair could not see either.
Three fork-introduced dependencies had leaked into non-ESP32 builds, breaking
every nRF52 and RP2040 target since 7e4f75c9 (2026-04-10):
- The `memory` CLI command called ESP.getFreeHeap()/heap_caps_* unguarded from
shared CommonCLI.cpp. ESP32 output is unchanged; other platforms now report
newlib arena stats, omitting min-ever-free and largest-free-block rather than
substituting numbers that mean something different.
- The vendored PsychicMqttClient (ESP-IDF esp-mqtt) was pulled in by the LDF on
nRF52; nrf52_base now lib_ignores it, matching how RP2040 variants ignore BLE.
- JWTHelper.cpp and MQTTMessageBuilder.cpp are excluded in arduino_base, but 22
variants re-glob helpers/*.cpp after inheriting it, which undoes the exclusion.
Guarding the file contents on WITH_MQTT_BRIDGE is robust against any variant's
filter, and matches helpers/esp32/WebConfigServer.cpp.
Verified: RAK4631 repeater + room server, Heltec T114, T1000-E, Wio WM1110,
Xiao nRF52 and ThinkNode M1 all build. ESP32 observer builds are byte-identical
to before (RAM and flash), and the native suite stays at 267/267.
RP2040 still fails separately: four boards declare the pre-force_ap
startOTAUpdate signature. Not addressed here.
The starvation needs ArduinoJson v7's fixed 4096-byte pool blocks plus dev's
custom budget-capped allocator. Production is still on v6 (MQTTMessageBuilder
uses createNestedArray, removed in 7.0), where DynamicJsonDocument(10240) is a
real compact slot pool needing ~3.8 KB for 50 entries, so prod is unaffected
and needs no hand-port.
Also notes that prod pins no ArduinoJson version at all.
Enables neighbors publication on the ESP32-S3 non-PSRAM observer envs via a
per-variant MQTT_NEIGHBORS_WITHOUT_PSRAM opt-in, and fixes a pre-existing JSON
pool-budget bug that silently dropped the publish on PSRAM boards with roughly
40+ neighbours.
Bench-verified 2026-08-03 at the 2-wss-slot non-PSRAM maximum.
Bench run 2026-08-03 on a non-PSRAM ESP32-S3 observer at the 2-wss-slot
non-PSRAM maximum: periodic publish succeeded, min-ever free internal heap
53776 B, payload 1252 B of the 4096 B buffer.
Also records the pre-existing PSRAM pool-budget bug found while verifying,
and the two scenarios still untested on hardware: the >20-neighbour
truncation path, and a publish coinciding with a slot reconnect.
Neighbors publication was gated on BOARD_HAS_PSRAM. Removing that gate alone
was not enough: the feature built but was inert without PSRAM, because three
allocation sites asked for MALLOC_CAP_SPIRAM (which returns null with no
PSRAM), the bridge's persistent buffer was allocated inside a BOARD_HAS_PSRAM
block, and neighborDiscoverReady() rejected every pass at runtime on
psramFound(). The entry table also did not fit: finishNeighborDiscover put
pubkey_hex[50][65] plus entries[50] on the stack, a 4752-byte frame against
the mesh loop task's 8 KB.
- Gate on MAX_NEIGHBOURS plus PSRAM or an explicit per-variant
MQTT_NEIGHBORS_WITHOUT_PSRAM opt-in.
- Move the entry table and its hex strings into one heap block sized to the
pass; the frame drops from 4752 to 304 bytes.
- Prefer PSRAM and fall back to internal DRAM in the mesh-side allocations
and the ArduinoJson pool; hoist the bridge's persistent buffer out of the
BOARD_HAS_PSRAM block (psram_malloc already falls back).
- Keep the runtime psramFound() check only where the buffers are sized for
PSRAM, so a board whose PSRAM failed to init still refuses.
- Size for internal DRAM without PSRAM: 4 KB text buffer and 20 entries per
publish, keeping the pool to a single block and the peak near 13 KB rather
than ~35 KB. Oversized tables truncate and report total_neighbors as before.
Enabled on the ESP32-S3 observer envs (Heltec V3/WSL3, RAK3112, Heltec
Tracker v1.1/v2). Left off for the classic ESP32 T-LoRa V2.1-1.6, which is
already limited to one active TLS slot.
Costs ~7.4 KB static DRAM on repeaters and ~9.6 KB on room servers. The
prefs layout is unchanged, so this is neutral for existing devices.
Josh Clarke
Adam Gessaman
mikecarper
Kevin Le
Hacuchino-hash
ripplebiz
taco
ViezeVingertjes
Liam Cottle
fdlamotte