Fix MQTT client issues with JWT renewal, memory allocation, and NTP handling - #46
Merged
Merged
Conversation
The scheduled JWT bounce called PsychicMqttClient::disconnect(), which ends with esp_mqtt_client_stop(). That ends the client task and returns its 6 KiB stack to the heap at the moment the TLS teardown vacates two 16 KiB mbedTLS record buffers, so the stack lands in that hole and the next handshake cannot reuse it. On non-PSRAM boards the largest free block then ratchets down 16 KiB at a time while total free heap stays flat. Soak evidence from a Heltec V3 on 8d1a0eb: 43 of 60 disconnects had no preceding transport error, i.e. they were this proactive bounce rather than a broker FIN, and two of the three max_alloc steps landed within 5 s of one. Losing a whole TLS session later returned exactly 16,384 bytes of contiguity. softDisconnect() closes the transport without the stop, so the task and its stack stay put across the handshake. The bounce uses it plus reconnect(), and falls back to connect() when the client really is stopped, since reconnect() is a silent no-op in that state. Also corrects a comment claiming the mbedTLS context survives a transport close: only the esp-mqtt client object does. (cherry picked from commit 10cf5cf48fb009e751e25b37fcc1f3d1256ddbbc)
…idge start allocateRuntimeBuffers() took NEIGHBORS_JSON_BUFFER_SIZE unconditionally on every board built WITH_MQTT_NEIGHBORS, whether or not mqtt.neighbors was ever turned on. On a non-PSRAM board that is 4 KB of internal DRAM held for the bridge's lifetime by a node that may never publish a neighbours snapshot. Gating the existing allocation on the pref would not work: mqtt.neighbors is read live by the mesh loop with no bridge restart, so enabling it at runtime would find no buffer and silently publish nothing. Allocate on first use instead, in requestPublishNeighbors(), which is reached only when something actually wants to publish — periodic or a manual discovery. Publishing the pointer across cores is safe with the existing handshake: the allocation precedes the release store on _neighbors_publish_pending, and the task loop reads the pointer only after its matching acquire load, so the pointer cannot be observed half-published. A failed allocation drops that one snapshot and retries on the next, rather than disabling neighbours for the bridge's lifetime as the eager path did. (cherry picked from commit e6da052a93f8765824d0fb4bd0c704ca3ed3d294)
…and verified
The reduced-TLS work was validated on hardware but only reachable through
PLATFORMIO_BUILD_FLAGS pointing at an absolute path in a developer's home
directory, so nothing outside that machine could reproduce it.
Distribute the archives as a release asset instead of committing them: ~6 MB
per architecture, and they must be rebuilt for every espressif32 bump, so
committing would grow history permanently and go stale without any signal.
scripts/mbedtls_4k_manifest.txt per-arch sha256 of each archive
scripts/fetch_mbedtls_4k.sh fetch into .mbedtls-4k/<arch>/, verify
scripts/mbedtls_4k.py pre-build wiring and post-link proof
Off by default. The script is attached to esp32_base but returns immediately
unless MESHCORE_REDUCED_TLS=1, so ordinary builds need no artifact and are
byte-for-byte unaffected — confirmed by building with it absent.
Both ways this can fail silently produce a firmware that looks fine and lacks
the change, so the opt-in path refuses to guess:
- a -L at a missing or partial directory: the linker ignores an unusable
search path and resolves mbedTLS from the framework. Now a hard error.
- archives left over from an earlier platform version: now a sha256
mismatch against the manifest, naming both hashes.
- a -L that is present but outranked, leaving the flag inert: after the
link, firmware.map must resolve every libmbed*.a into .mbedtls-4k/, or
the build fails and prints the offending paths.
That last check earned its place immediately — it caught its own first
implementation comparing a relative map path against an absolute one, and an
earlier build flag in this investigation was accepted by the compiler while
no source read it. A flag reaching the compiler proves nothing about the link.
Verified all four paths on Heltec_v3_repeater_observer_mqtt: default build
unaffected; opted in with archives present links all four from .mbedtls-4k/
and says so; archives absent fails with a fetch hint; a single appended byte
fails on sha256.
Also removes platformio.local.ini.hold, which held the superseded approach of
pointing platform_packages at a whole custom framework. That installs over the
shared framework package and changes mbedTLS for every other ESP32 project on
the machine; the -L path keeps the change scoped to one env.
Note the inbound record buffer stays at 16 KiB, so this lowers per-connection
footprint by ~12 KiB but does not move the contiguous allocation a handshake
needs. It buys headroom, not a lower floor.
(cherry picked from commit a87faff6ff170c328fdd0550f4b4dd9089aa2ea0)
…en map check
Three defects found reviewing the preceding commits.
1. reconnectSlotClient() stranded a STOPPED client, reintroducing the very bug
this branch fixes. It only rebuilt when isStarted() was true and otherwise
fell through to reconnect(), which is a documented no-op on a stopped client
— so nothing restarted it, at any rung, including the breaker probe. The
WiFi-transition teardown reaches exactly this state: it calls the hard
disconnect(), clearing _started while initial_connect_done stays set, so
after WiFi returned the slot could never come back. Now a stopped client is
started with connect() before the rebuild/reuse decision is considered.
The post-NTP credential refresh had the same exposure — it called
reconnect() directly — so it now goes through the helper too, still reusing
the transport since its fault is stale credentials, not the transport.
2. Allocating the neighbors buffer on first use let a stopped bridge allocate.
A neighbour discovery started before a stop can complete after it, and
neither caller rechecks bridge state, so requestPublishNeighbors() would
allocate 4 KB after releaseRuntimeBuffers() had already run and strand
_neighbors_publish_pending with no task to consume it. end() then returns
early on !_initialized, retaining the buffer until a later begin/end or a
reboot. Guarded on isRunning(), the same flag end() checks.
The release/acquire handoff itself was confirmed sound: the allocation and
copy precede the release store, and the task loop reads the pointer only
after its acquire load, so a half-published pointer is not observable.
3. The post-link map check failed open, contradicting the fail-closed claim in
its own commit message. A missing map, an unrecognised map format, or a
partial archive list each warned and passed; and it hardcoded firmware.map
while the post-action target used ${PROGNAME}, so a renamed program could
inspect a stale or absent file and still succeed. All four now fail the
build, and it requires every one of the four archives to appear rather than
at least one.
Rebuilt Heltec_v3_repeater_observer_mqtt, Heltec_v3_repeater and
heltec_v4_repeater_observer_mqtt; the opt-in path still reports all 4 archives
linked from .mbedtls-4k/.
(cherry picked from commit 5b5f076e5e165997e8050f2be061c7c67340fcf7)
waev's operator confirmed on 2026-08-11 that their servers do not disconnect a client when its JWT passes exp — a 60-minute token can hold a session open for hours. The renewal path assumed the opposite, in as many words: the comment at the bounce called the renewal buffer "the ONLY margin between 'device re-authenticates' and 'broker enforces exp and FIN-closes the session mid-stream' — observed on the waev preset". That premise made waev expensive, because waev is the only preset with a short token_lifetime (3300 s; every other is 0, meaning the 24 h default). It was therefore the only slot bouncing often: measured every ~47 minutes, about 30 times a day per device. And the bounce's re-handshake is where contiguous internal DRAM goes — one renewal traced on hardware took the largest free block from 27,124 to 16,372 B, below the 16,384 B mbedTLS inbound record buffer, after which that slot could not re-handshake at all. The teardown and the credential update cost nothing; the handshake costs everything. So for a broker that leaves live sessions alone, refresh the credentials in place and let the next genuine reconnect use them. That path already existed for the "token renewed but old one still valid" case; this just stops treating imminent expiry as a reason to tear down a healthy connection. mqttPresetEnforcesTokenExp() defaults to true and is keyed by preset name rather than a new struct field: adding a field would mean re-ordering a dozen positional initialisers, where a mistake is silent, and the wrong default costs an outage rather than a re-handshake. Custom and audience-only slots have no preset and are treated as enforcing. Our own logs already argued against the premise and we had not noticed: across 14 multi-device outages (10 hitting all four devices) the drops landed within ~3 s of each other, on devices whose independent boot times gave them independent token issue times. Independent expiries cannot align that tightly, so exp enforcement was never a good explanation for them. Unverified on hardware yet — the operator's statement is second-hand. Next: apply to one board only and confirm the session survives past exp, that a later reconnect still authenticates, and that the ~47-minute 27,124<->16,372 oscillation stops. (cherry picked from commit 27bd05a17b9303b158feec7dab60af2fe128f5ce)
The comment claimed the (WS_BUFFER_SIZE + 1) padding makes an oversized
upgrade response "fail cleanly (Upgrade header not found)". Reading
release/v4.4 transport_ws.c against release/v5.3 shows it does not.
v4.4's response loop is
} while (NULL == strstr(ws->buffer, "\r\n\r\n") && header_len < WS_BUFFER_SIZE);
so it also exits when the buffer fills without the terminator, and the code
then looks for "Sec-WebSocket-Accept:" and returns 0 if it is present. That
header appears early in a response, so an oversized header block yields a
BOGUS SUCCESS rather than a clean failure: the unread remainder stays queued
on the socket and is delivered as the first post-upgrade read, where the
deframer parses HTTP bytes as a WebSocket frame header.
Observed on hardware 2026-08-12 on a Heltec V4: a Cloudflare Page Shield CSP
report-uri header pushed the 101 response past the buffer, and the tail of
that header ("csp-reporting.cloudflare.com/cdn-cgi/script_monitor/report?")
reached the MQTT layer as payload, surfacing as
"Invalid MSG_TYPE response: 3" (0x35 = '5', high nibble 3).
The padding's real and only value is preventing the one-byte overflow of the
heap canary, which is still worth having. Narrow the comment to that claim and
record where the parser fix has to come from: IDF 5.2+ requires the "\r\n\r\n"
delimiter, memmoves the bytes following it, and fails cleanly when the buffer
fills. It cannot be patched here, since transport_ws.c ships precompiled in
libtcp_transport.a on Arduino 2.x.
No functional change.
(cherry picked from commit 9894e65e8ad961704d430a12a065baffda353f50)
Every ordinary backoff reconnect and every circuit-breaker probe minted a
fresh JWT and re-applied credentials, with no check of whether the existing
token was still valid. setCredentials() always dirties the esp-mqtt config, so
reconnect() then called esp_mqtt_set_config() as well. On a flapping broker
that is a signing plus a configuration-copy cycle on every retry, and these
observers see ~38 genuine reconnects/day per slot.
The no-bounce renewal change (27bd05a1) only stopped the proactive renewal
from tearing down a live session; it left this retry path untouched, which is
why a soak shows renewals neither firing nor failing for hours while drops
continue — each reconnect silently re-mints and pushes the expiry out.
Reuse the credentials when their validity is provable and refresh them
otherwise. canReuseJwtForReconnect() lives with the other policy predicates so
it is host-testable, and it establishes current_time < token_expires_at before
subtracting: token_expires_at is unsigned, so an already-expired token would
otherwise wrap to ~4e9 seconds and read as valid for decades. The
>= kMinimumValidEpoch term also rejects the 0 that a failed renewal writes.
Minting stays the default for every uncertain case — unsynced clock, missing or
insane expiry, empty token, or an expiry inside kJwtReconnectSafetyMarginSecs
(60 s), which covers the handshake itself.
Two paths still always mint, deliberately:
- The circuit-breaker probe. It is the recovery of last resort for a slot
that has already failed repeatedly, quite possibly on auth, and it runs
once per 30 minutes — so a fresh token there costs nothing worth counting
against keeping that path guaranteed-clean.
- Any slot whose last error was a broker refusal. Before this change, minting
on every retry accidentally recovered from server-side credential
invalidation: key rotation, revocation, broker clock skew, or an audience
change after a reconfigure. Reuse would have retried a rejected credential
until it neared expiry — up to 24 h for every preset that leaves
token_lifetime at the default. onError already detects
MQTT_ERROR_TYPE_CONNECTION_REFUSED and only logged it; it now also sets a
per-slot force-mint flag, cleared on a successful connect and wherever the
credentials it referred to are blanked. The flag is volatile because the
esp-mqtt callback sets it and the bridge loop consumes it.
The reconnect log line reports the decision and its outcome — REUSE, MINT with
a reason, and OK/FAILED for the mint — because a silently failed mint is the
case most likely to end in an auth refusal. It never prints the token.
Host tests cover the reuse boundary: exact margin, already-expired, expiry 0,
sub-epoch expiry, empty token, unsynced clock, and the force-mint override.
reconnectSlotClient() checks isStarted() and calls connect() instead of reconnect() when the client was stopped, but only the post-NTP stale-token path used it. The ordinary backoff ladder and the circuit-breaker probe called slot.client->reconnect() directly, and esp_mqtt_client_reconnect() is a no-op on a client that is not started. Two ways in. connect() sets _started only when esp_mqtt_client_start() returns ESP_OK while setupSlot() sets initial_connect_done unconditionally, so a start failure under heap pressure stranded the slot. More routinely, the WiFi-drop handler calls disconnect() on every connected slot, which clears _started — after that the ladder issued no-ops forever and the slot never came back. Not caught by the soaks: the log line the guard prints can only come from the NTP path, so a stranded slot and a slot that never entered the state produce identical logs. Observed reconnects were broker-side drops with WiFi up, which leave the client started. The renewal-bounce path keeps its own isStarted() branch — it needs softDisconnect(), which the helper does not do.
connect() logged "MQTT client started." unconditionally, so a failing esp_mqtt_client_start() looked identical to a successful one. That is the one state a later reconnect() cannot recover from, which made it the worst possible line to be wrong.
… live client esp_mqtt_client_reconnect() is honoured only from MQTT_STATE_WAIT_RECONNECT, so the post-correction path minted a fresh token, staged it, and then asked a connected client to reconnect — a request esp-mqtt refuses. The slot kept running on the token the clock correction had just proven stale, and recovery became broker-driven rather than the clean reconnect this code intends. Split the three states the path can find: a stopped or waiting client goes through reconnectSlotClient() as before, and a live one has its transport closed first. Only where the broker enforces exp, though — waev leaves live sessions alone past expiry, so bouncing it would spend the 16 KiB contiguous handshake that the rest of this branch exists to avoid. Not a regression: the base branch called client->reconnect() directly at the same site. On ESP32 the block is reachable from the WiFi-reconnect resync and the CLI forced sync; the hourly refresh uses refreshNTP(), which does not carry it.
…built against The manifest said "rebuild these for every espressif32 platform bump" and nothing enforced it. mbedtls_4k.py verified the staged archives against the manifest's own hashes, which proves the pair agrees with itself and nothing more: bump the platform without rebuilding and every check still passes while the link takes mbedTLS built against a different IDF. That fails at runtime on struct-layout drift, not at the link, which is the failure the mechanism claimed to prevent. Fingerprint the framework's own mbedTLS archives — the ones ours displace — as stock: lines in the manifest and check them before the build. If the framework moves, the staged pair is stale by construction and the build stops with the replacement hashes printed ready to paste. Stronger than comparing a version string: framework-arduinoespressif32 versions independently of the platform, and its archives are what actually has to match. The lib directory is resolved by trying the layouts espressif32 has used rather than hardcoding one, and failing closed if none holds all four archives. The fetch script ignores the new lines; its known-arches hint skips them so they cannot be reported as architectures.
…erver The fallback configured a server, waited 500 ms, and accepted any plausible system clock as proof that server had answered. It usually has not answered that fast — and the device usually already holds valid time, from an earlier sync or the RTC — so the first server in the list was credited unconditionally, the walk stopped there, _last_ntp_sync was refreshed, and an unreachable host was logged as the source. On the `set mqtt.ntp` validation path, where the single-server walk exists so a typo fails fast, that reported a bad server as OK. Poll sntp_get_sync_status() for SNTP_SYNC_STATUS_COMPLETED instead, which is the layer's own statement that a packet arrived. The status is one-shot — reading COMPLETED clears it — so a result left by an earlier sync would latch on the first poll; clear it before the loop. An implausible epoch after a completed sync now moves to the next server rather than spinning out the remaining attempts against a server that has answered.
syncTimeWithNTP() read an epoch over UDP, called configTime(), set _ntp_synced, and then had the stale-token test and createSlotAuthToken() read time(nullptr) — without anything having put the accepted epoch there. configTime() restarts SNTP and returns; the clock lands whenever a packet does. _rtc->setCurrentTime() looks like it covers this and does not. AutoDiscoverRTCClock::setCurrentTime() writes a detected DS3231/RV3028/PCF8563/ RX8130CE *instead of* delegating to its fallback, and only that fallback (ESP32RTCClock) calls settimeofday(). So on every board carrying an RTC chip — T-Beam Supreme and Station G3 both compile this bridge and both instantiate AutoDiscoverRTCClock — libc kept the pre-correction time, and the correction path tested staleness and minted iat claims against exactly the clock it had just proven wrong. Boards without a chip take the fallback and were unaffected, which is why the soak rig (Heltec V3/V4, no RTC) never showed it. settimeofday() with the accepted epoch first, so the invariant downstream code already assumes actually holds: once _ntp_synced is true, time(nullptr) returns the epoch we accepted. configTime() still follows, to keep future syncs running.
The corrected-clock path reconnected a disconnected slot whether or not createSlotAuthToken() had produced anything, which re-presented the credentials the correction had just invalidated. Minting fails for recoverable reasons — allocation pressure is treated as recoverable elsewhere in this file — so the path is reachable, and the reconnect it spends is one that cannot succeed. Move the decision into MQTTConnectionPolicy as classifyStaleToken(), where the four outcomes are named and host-tested rather than spelled out in nested conditions: Defer on a failed mint, Reconnect a client that is down, Bounce a live session whose broker enforces exp, KeepAlive one whose broker does not. Deferring leaves the slot to the backoff ladder, which mints again on its next attempt. Covers the reviewer's first four cases. The other two — that a completed SNTP sync is required, and that time(nullptr) reflects the accepted epoch before _ntp_synced flips — are inside MQTTBridge.cpp, which the native env does not compile; locking those down needs a seam around the IDF calls that does not exist yet.
Requiring a real SNTP completion took away something the plausible-clock test was doing by accident. _ntp_synced gates slot setup outright (:1386, :2894), so a device that cannot reach NTP now brings up no slots at all — and a network that blocks UDP/123 while allowing 443 is an ordinary firewall configuration, not a corner case. An RTC-backed observer there used to stay synced and keep minting JWTs against a perfectly good clock. Accept the existing clock explicitly when every server has failed, logged as what it is rather than as a claim about a server that never replied. Excluded from the `set mqtt.ntp` validation path, where the question is whether that server works and the clock cannot answer it. configTime() is now called only when a server did answer, since otherwise there is nothing new to point SNTP at.
The reset was on the wrong side of configTime(). configTime() configures the server, calls sntp_init(), and returns — the new request is live before it comes back — so a fast reply could set SNTP_SYNC_STATUS_COMPLETED inside that call, and the reset immediately after would erase it. The following ten seconds of polling would then see nothing and reject a server that had in fact answered. On the `set mqtt.ntp` path that surfaces as a good server failing validation. Stop any running session first, discard its status, then start the new one, so the only completion observable is the one being waited for.
The usable-clock fallback asked libc only, which does not answer for the case it was written to cover. On a cold boot ESP32RTCClock::begin() stamps libc with a 2024 placeholder on power-on; AutoDiscoverRTCClock::begin() probes the chip but never copies its time across, and getCurrentTime() reads the chip directly. So a Station G3 or T-Beam Supreme that knows exactly what time it is, on a network with UDP/123 blocked, still failed the plausibility test, left _ntp_synced false, and brought up no slots — precisely the deployment the fallback exists for. Ask the RTC when libc is below the floor. libc still wins when it is usable: a clock SNTP set recently outranks a chip that may have drifted. Accepting the RTC value then flows through the same block, so settimeofday() repairs libc and the epoch is written back to the chip. The choice is chooseFallbackClock() in MQTTConnectionPolicy, host-tested across the four states including the power-on placeholder and the exact floor. Also corrects the previous commit's claim that configTime() is called only when a server replied — the fallback necessarily points it at each server before knowing that; it is the post-acceptance call that is now conditional.
Found on hardware while validating the SNTP fix. `set mqtt.ntp bogus.invalid`
reported SUCCESS with a correct epoch, in 4 s, with no retry and without ever
reaching the SNTP fallback:
[E] hostByName(): DNS Failed for bogus.invalid
[E] beginPacket(): could not get host from dns: 11
MQTT: Time synced: 1786764354 (via bogus.invalid)
Three pieces compose it. WiFiUDP::beginPacket(const char*, port) returns 0 on a
DNS failure and leaves remote_ip/remote_port at their previous values.
NTPClient::sendNTPPacket() discards that return and calls endPacket() regardless.
endPacket() sends to whatever remote_ip still holds. So the request went to the
pool address resolved at boot, that server answered with a genuine timestamp, and
the loop recorded ntp_server_used as the name that had never been contacted.
This sits one layer above the fallback that b1ceaf0 made honest — control never
reaches it — so `set mqtt.ntp <typo>`, whose whole purpose is to fail fast, still
reported OK and the fleet kept a server name it had never spoken to.
The DNS pre-check was already here and only logged a warning. Make it decide:
skip a name that does not resolve rather than attempt a send that cannot go where
it claims. IP literals are unaffected — hostByName() returns them via
fromString() without a lookup — and the lookup already ran, so no latency is
added. Moved setPoolServerName() below it so the client is never pointed at a
server being skipped.
Residual, narrower window: our lookup succeeds and NTPClient's own
gethostbyname() then fails, which needs the entry to leave the lwIP cache between
two calls microseconds apart. Closing it properly needs the resolved IP handed to
NTPClient, and this version exposes no setPoolServerIP(); the constructor is the
only way in.
Not host-testable — NTPClient and WiFiUDP both. Verified by inspection of both
library sources plus the captured hardware trace above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #45. This includes all of the reconnect / JWT / reduced-TLS work from that PR plus the correctness fixes found during subsequent review and hardware validation.
Summary
This hardens the observer MQTT path around the failure modes that matter most on constrained ESP32 targets:
The original motivation was reconnect/renewal stability on non-PSRAM observers. Review of that work exposed several pre-existing clock and reconnect edge cases in the same lifecycle, so this PR carries those fixes as well rather than landing an intermediate state and immediately following it with another PR.
Reconnect and JWT renewal
Keep the esp-mqtt client alive when possible
JWT renewal previously stopped and restarted the client to install new credentials. That destroys the TLS session and forces another handshake every renewal.
This PR updates credentials in place and decides separately whether the transport actually needs to reconnect:
expreconnect after renewalwaev, whose broker permits an already-established session to outlive token expiry, keeps the live connectionThe reuse predicate rejects unsynced clocks, empty tokens, expired tokens, forced-mint cases, and tokens inside the reconnect safety margin. The expiry comparison occurs before unsigned subtraction so an already-expired token cannot wrap into an apparently enormous remaining lifetime.
Recover stopped clients through the correct API
esp_mqtt_client_reconnect()only works on a started client in the reconnect state. Several bridge paths could instead encounter a client that had been stopped, or one whose initialesp_mqtt_client_start()failed.Reconnect ladders now route stopped clients through
connect()rather than issuing a reconnect that can never succeed.The underlying PsychicMqttClient wrapper also no longer logs
"MQTT client started"unconditionally: a failedesp_mqtt_client_start()is surfaced as an error.Handle clock-correction reconnects correctly
A time correction can prove an already-issued JWT stale.
That path previously minted and staged a replacement token, then called
reconnect()on a still-connected client. esp-mqtt refuses that request, so the slot could continue using the token the correction had just proven stale.The post-correction policy now distinguishes:
exp→ close the live transport and reconnectThis fixes a pre-existing recovery bug rather than introducing a new reconnect mechanism.
NTP, RTC, and JWT clock correctness
The review also found that the bridge's time-sync contract was weaker than the JWT code assumed.
JWT creation reads
time(nullptr), so_ntp_syncedmust not become true until libc itself reflects the epoch the bridge accepted.Make the accepted epoch authoritative
After a successful NTP/SNTP result, the bridge now explicitly calls
settimeofday()before_ntp_syncedis set or any JWT work runs.This matters on boards using
AutoDiscoverRTCClock: when an external DS3231/RV3028/PCF8563/RX8130CE is present,setCurrentTime()writes the hardware RTC instead of the ESP32 fallback clock. Without the explicit libc update, the RTC could contain the new time whiletime(nullptr)still returned the old one.The accepted epoch is then written back through the RTC abstraction as before.
Require an actual SNTP completion
The old SNTP fallback treated a plausible
time(nullptr)as evidence that the server being tested had answered. On a device that already had valid time from a previous sync or RTC, that could credit the first configured server without receiving a packet from it.The fallback now waits for SNTP itself to report
SNTP_SYNC_STATUS_COMPLETED.Before each request it:
configTime()The stop/reset ordering is deliberate:
configTime()starts SNTP before returning, so clearing status afterwards creates a race where a fast valid reply can be erased.Preserve operation when UDP/123 is blocked
Requiring a real server response exposed behavior the previous plausible-clock check had provided accidentally: a node with a perfectly usable local clock could operate on a network that allows MQTT/TLS but blocks NTP.
The fallback is now explicit:
System time wins when valid because it may have been synchronized more recently than the RTC.
The RTC path is important on cold power-on. ESP32's fallback clock is initialized to a fixed 2024 placeholder, while an external RTC may already contain valid current time. Accepting the RTC then flows through the same success block, repairing libc with
settimeofday()and writing the accepted epoch back through the clock abstraction.This local-clock fallback is not allowed during
set mqtt.ntpvalidation. That command asks whether the configured server works; an existing clock cannot satisfy that test.Do not credit an unresolvable NTP hostname
Hardware validation found another false-success path:
WiFiUDP::beginPacket(hostname, ...)leaves its previous remote address in place when DNS fails, while this NTPClient version ignores the failedbeginPacket()result and still callsendPacket().The result was a real NTP request sent to the previously resolved server, whose genuine response was then incorrectly credited to the invalid hostname.
The existing DNS preflight is now authoritative: if the hostname does not resolve, that candidate is skipped and is never handed to NTPClient.
There remains a much narrower theoretical window where the explicit lookup succeeds and NTPClient's immediately-following internal lookup fails after the cache entry disappears. This NTPClient version exposes no setter for the already-resolved IP, so eliminating that would require a larger library/API change. It has not been observed.
Reduced-TLS build for constrained observers
MESHCORE_REDUCED_TLS=1remains opt-in.It substitutes mbedTLS archives built with:
CONFIG_MBEDTLS_SSL_OUT_CONTENT_LEN=4096reducing the outbound record allocation and recovering roughly 12 KiB of internal DRAM per TLS connection on the non-PSRAM observer configuration.
Inbound remains 16,384 bytes. This is therefore a headroom improvement, not a fix for the fundamental contiguous-allocation floor during a TLS handshake.
The replacement libraries are now bound to the framework they were built from:
espressif32platform version is recorded in the manifestA framework/platform change therefore cannot silently reuse replacement archives built for another version.
The fetch script and manifest make the reduced archives reproducible/installable without committing binary libraries to the repository.
Memory-pressure cleanup
A few related allocations are moved out of the constrained reconnect path:
Hardware evidence
The original reconnect/JWT work was soaked on non-PSRAM Heltec V3 observers using multiple WSS MQTT slots.
d1 — 60 hours
expis enforcedtls_stack=32512d3 — 98 hours
With reconnect token reuse enabled:
Additional on-target validation of the NTP work reproduced the invalid-hostname false-success described above and drove the DNS-validation fix.
The reconnect safety-margin mint and forced mint after a broker credential refusal remain difficult to induce reliably on the soak hardware; those policy decisions are host-tested.
Verification
pio test -e native: 307/307 passedThe RTC cold-boot tests use the actual ESP32 power-on placeholder epoch (
1715770351) rather than an artificial zero value.Scope / non-goals
This PR reduces unnecessary TLS handshakes and fixes recovery/timekeeping cases that could strand or mis-authenticate an MQTT slot.
It does not eliminate the underlying ESP32/mbedTLS contiguous-memory constraint. The inbound TLS record remains 16 KiB, so a non-PSRAM observer can still approach the same handshake allocation floor under multi-slot WSS load.
It also does not address the ESP-IDF 4.4 WebSocket parser issue that can produce:
Invalid MSG_TYPE response: 8That transport code is supplied precompiled by the current framework and is separate from the reconnect/JWT/NTP work here.
Why this supersedes #45
#45 contains the original reconnect, renewal, and reduced-TLS work. Review and hardware validation of that branch then exposed several correctness gaps in adjacent code:
Rather than merge #45 in that intermediate state and immediately follow it with another corrective PR, this PR presents the complete reviewed and validated behavior as one merge unit.