diff --git a/.github/workflows/build-observer-firmwares-beta.yml b/.github/workflows/build-observer-firmwares-beta.yml index a81e348442..34a474db22 100644 --- a/.github/workflows/build-observer-firmwares-beta.yml +++ b/.github/workflows/build-observer-firmwares-beta.yml @@ -93,6 +93,12 @@ env: jobs: + verify: + uses: ./.github/workflows/run-unit-tests.yml + + preset-parity: + uses: ./.github/workflows/check-mqtt-preset-parity.yml + enumerate: runs-on: ubuntu-latest outputs: @@ -191,7 +197,7 @@ jobs: if-no-files-found: error release: - needs: [enumerate, build] + needs: [enumerate, build, verify, preset-parity] runs-on: ubuntu-latest steps: - name: Clone Repo diff --git a/.github/workflows/build-observer-firmwares.yml b/.github/workflows/build-observer-firmwares.yml index 81312ea2ac..b6d9b315ed 100644 --- a/.github/workflows/build-observer-firmwares.yml +++ b/.github/workflows/build-observer-firmwares.yml @@ -57,6 +57,12 @@ env: jobs: + verify: + uses: ./.github/workflows/run-unit-tests.yml + + preset-parity: + uses: ./.github/workflows/check-mqtt-preset-parity.yml + # Discover the *_observer_mqtt envs and split them into SHARDS groups so the # build fans out across runners (pio builds envs sequentially within a job). enumerate: @@ -170,7 +176,7 @@ jobs: # Collect all shard outputs, publish the rolling release, then point the # flasher at the new build by bumping the hash + notes in its config.json. release: - needs: [enumerate, build] + needs: [enumerate, build, verify, preset-parity] runs-on: ubuntu-latest steps: - name: Clone Repo diff --git a/.github/workflows/check-mqtt-preset-parity.yml b/.github/workflows/check-mqtt-preset-parity.yml index 7b7d663de4..319a2dc929 100644 --- a/.github/workflows/check-mqtt-preset-parity.yml +++ b/.github/workflows/check-mqtt-preset-parity.yml @@ -12,6 +12,7 @@ permissions: on: workflow_dispatch: + workflow_call: pull_request: branches: - observer-firmware @@ -47,6 +48,8 @@ jobs: EVENT_NAME: ${{ github.event_name }} PR_BASE: ${{ github.event.pull_request.base.ref }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + CANDIDATE_SHA: ${{ github.sha }} + CANDIDATE_BRANCH: ${{ github.ref_name }} run: | set -euo pipefail PRESET_PATH=src/helpers/MQTTPresets.h @@ -72,9 +75,22 @@ jobs: ;; esac else - # push / workflow_dispatch: compare current channel tips. - git show "origin/observer-firmware:${PRESET_PATH}" > /tmp/preset-parity/prod.h - git show "origin/observer-firmware-dev:${PRESET_PATH}" > /tmp/preset-parity/dev.h + # Pin the channel being verified to this run's candidate. Its + # branch may have advanced while the release waited in the queue. + case "$CANDIDATE_BRANCH" in + observer-firmware) + git show "${CANDIDATE_SHA}:${PRESET_PATH}" > /tmp/preset-parity/prod.h + git show "origin/observer-firmware-dev:${PRESET_PATH}" > /tmp/preset-parity/dev.h + ;; + observer-firmware-dev) + git show "origin/observer-firmware:${PRESET_PATH}" > /tmp/preset-parity/prod.h + git show "${CANDIDATE_SHA}:${PRESET_PATH}" > /tmp/preset-parity/dev.h + ;; + *) + echo "::error::preset parity requires an observer channel branch" >&2 + exit 1 + ;; + esac fi echo "=== production (observer-firmware) head preview ===" diff --git a/.github/workflows/pr-build-check.yml b/.github/workflows/pr-build-check.yml index 788390c57a..9ce4f0ac87 100644 --- a/.github/workflows/pr-build-check.yml +++ b/.github/workflows/pr-build-check.yml @@ -2,7 +2,7 @@ name: PR Build Check on: pull_request: - branches: [main, dev] + branches: [main, dev, observer-firmware, observer-firmware-dev] paths: - 'src/**' - 'include/**' @@ -14,10 +14,12 @@ on: - 'scripts/**' - 'ssl_certs/**' - 'webui/**' + - 'test/**' - 'default_8MB.csv' - 'platformio.ini' - '.github/actions/setup-build-environment/**' - '.github/workflows/pr-build-check.yml' + - '.github/workflows/run-unit-tests.yml' push: branches: [main, dev] paths: @@ -49,6 +51,8 @@ jobs: - Heltec_v3_room_server # MQTT observer smoke builds: internal RAM and PSRAM coverage. - Heltec_v3_repeater_observer_mqtt + - Heltec_v3_room_server_observer_mqtt + - ThinkNode_M7_repeater_observer_mqtt - T_Beam_S3_Supreme_SX1262_repeater_observer_mqtt # nRF52 - RAK_4631_companion_radio_ble diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 826c4cb37c..40965d31c2 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -7,6 +7,7 @@ on: - master pull_request: workflow_dispatch: + workflow_call: jobs: test: @@ -24,8 +25,14 @@ jobs: python3 -B scripts/check_arduinojson_pin.py --self-test python3 -B scripts/check_arduinojson_pin.py + - name: Verify observer tooling + run: | + python3 -m pip install PyYAML==6.0.2 + python3 -B scripts/check_mqtt_preset_parity.py --self-test + python3 -B scripts/check_observer_ci.py + - name: Run Unit Tests - run: pio test -e native -e native_kiss_modem -vv + run: pio test -e native -e native_kiss_modem -e native_sanitized -vv - name: Upload Test Results # Upload test results even if the test step failed. @@ -33,4 +40,7 @@ jobs: uses: actions/upload-artifact@v4 with: name: test-results - path: .pio/build/native/ + path: | + .pio/build/native/ + .pio/build/native_kiss_modem/ + .pio/build/native_sanitized/ diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 72fdbfc028..7922f93f4a 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -529,7 +529,8 @@ These settings apply across all MQTT slots: - `get mqtt.neighbors` - Get periodic neighbors publishing setting (on/off; neighbors-enabled builds) - `get mqtt.neighbors.interval` - Get neighbors publish interval in hours (neighbors-enabled builds) - `get mqtt.ntp` - Get effective NTP server hostname -- `get mqtt.ntp.diag` - Probe every configured NTP server for connectivity (does not change the clock; serial console shows each server's reported time, or why a probe was rejected — `DNS failed`, `unsolicited reply`, `server unsynced`, ... — and LoRa shows a compact ` ok|fail` list) +- `get mqtt.runtime` - Read worker sample age, configuration revisions, and SDK event overflow count. +- `get mqtt.ntp.diag` - Queue a probe and return immediately; repeat for its ID/age/result (cached for 30 seconds). Probe every configured NTP server for connectivity (does not change the clock; serial console shows each server's reported time, or why a probe was rejected — `DNS failed`, `unsolicited reply`, `server unsynced`, ... — and LoRa shows a compact ` ok|fail` list) - `get mqtt.owner` - Get owner public key (serial console only) - `get mqtt.email` - Get owner email address (serial console only) diff --git a/MQTT_OWNERSHIP.md b/MQTT_OWNERSHIP.md index 7929890530..c0fa998af5 100644 --- a/MQTT_OWNERSHIP.md +++ b/MQTT_OWNERSHIP.md @@ -1,185 +1,66 @@ -# MQTT Bridge Cross-Core Ownership Model - -This document is the Phase 4 deliverable from `STABILITY_TESTABILITY_HANDOFF.md`: -it records **one owner for each mutable runtime domain**, maps every place a -non-owner reads owned state across cores today, and states the target primitive -for each. It is paired with the fork-owned lifecycle test seam -(`src/helpers/MQTTLifecycle.h`, `test/test_mqtt_lifecycle/`). - -**Status:** ownership model documented and the lifecycle/teardown test seam -landed (Phase 4). **Phase 5 (branch `phase5/cooperative-mqtt-shutdown`) has now -implemented the cooperative shutdown and the OTA barrier — hazard §4 below.** -Still **deferred** (carried to Phase 5b / Phase 6): publishing a plain-data -snapshot and repointing the §1/§2 consumers, and replacing the §3 `volatile` -handshakes with a command channel. This document is the plan; §1–§3 still -describe current behavior, while §4 is now resolved (see its note). Line -references are against the tree at the time of writing and should be re-verified -before editing. - -## Execution contexts - -- **MQTT task — Core 0** (`"MQTTBridge"`, `xTaskCreatePinnedToCore(..., MQTT_TASK_CORE=0)`, - entry `mqttTask` → `mqttTaskLoop`, `MQTTBridge.cpp:923`, `:991`). Owns all - WiFi / MQTT / NTP I/O and every mutation of `_slots[]`, connection state, and - NTP state. -- **Loop task — Core 1** (`MyMesh::loop()`). Runs the CLI, WebConfig `tick()`, - and `AlertReporter::onLoop()`. `examples/simple_repeater/MyMesh.cpp:1529` - notes the bridge loop is *not* called here — it lives on Core 0. -- **Producer / radio context.** Stages raw radio bytes before queue handoff - (`storeRawRadioData` → `_staged_*`, consumed by `queuePacket`; both Core 1, in - guaranteed sequence, `MQTTBridge.h:232-239`). -- **Async TCP context.** WebConfig request parsing and immutable response - handoff only. - -## Ownership model (one owner per mutable domain) - -| Domain | Owner | Notes | -|--------|-------|-------| -| MQTT clients, slot connection state, publish counters, packet drain, NTP I/O | **MQTT task (Core 0)** | `_slots[]`, `PsychicMqttClient`s, `_ntp_client`, `_last_raw_*` | -| CLI execution, preference persistence, WebConfig batch draining, bridge lifecycle requests | **Loop task (Core 1)** | issues start/stop/reconfigure, drains the WebConfig batch | -| Packet staging before queue handoff | **Producer / radio (Core 1)** | `_staged_*`, no lock needed (sequential) | -| Request parse + immutable response | **Async TCP** | must not touch mutable bridge state | - -The rule that follows: **loop/WebConfig/CLI/AlertReporter code must not directly -inspect mutable MQTT slot objects or client counters.** The MQTT task must -publish a plain-data snapshot they can read instead. - -## Current cross-core hazards (to resolve in Phase 5) - -All of the following run on **Core 1** and read state mutated by **Core 0** -without a lock or a published snapshot. - -### 1. Diagnostic reads of live `_slots[]` via the singleton - -Four `static` accessors reach the live object through the file-scope -`s_mqtt_bridge_instance` (`MQTTBridge.cpp:201`, set at `begin()` end `:849`, -nulled first in `end()` `:858` — a plain, non-atomic pointer): - -- `getSlotStatusSnapshot` (`:299`) — despite its name, built on demand from - live `_slots[slot_index]`. **Refined premise:** the returned `name`/`state` - `const char*`s point at static rodata (string literals / `MQTT_PRESETS[]`), - so they are *not* dangling. The real hazards are (a) reading the - `slot.preset` **pointer value**, which Core 0 can null/reassign mid-read - (`applySlotPreset`, `teardownSlot`), and (b) `slot.client->getPublishOk()`, - a live client pointer Core 0 can `delete` during teardown (UAF window). -- `formatMqttStatusReply` (`:207`), `formatMqttStatsReply` (`:263`), - `formatSlotDiagReply` (`:397`) — same singleton + live `_slots[]` reads. - -Consumers: CLI (`CommonCLI_Observer.cpp:726, :728, :791`) and the app-layer -stats JSON (`examples/simple_repeater/MyMesh.cpp:1423`, mirrored in -`examples/simple_room_server/MyMesh.cpp:1050`). **Refined premise:** -`WebConfigServer.cpp` itself reads only a compile-time constant -(`getMaxActiveSlots`, `:479`); the live-bridge web reads are in `buildStatsJson` -in the `MyMesh.cpp` app layer. - -### 2. Instance reads that survive `end()` - -`AlertReporter` (Core 1, `onLoop`, `AlertReporter.cpp:208`) reads live `_slots[]` -via instance methods it reaches through its own `_bridge` pointer -(`AlertReporter.cpp:281` `isSlotEnabledAndAttempted`, `:285` -`getSlotCurrentOutageStartMs`, `:296`/`:311` `getSlotPresetName`). The stats -JSON also calls `bridge->getQueueSize()` (`MyMesh.cpp:1419`). - -**Refined premise (teardown hazard):** `end()` nulls only -`s_mqtt_bridge_instance`. The app's `bridge` pointer and `AlertReporter::_bridge` -are **not** cleared by `end()`, so these instance reads can touch a torn-down -bridge. (`getConnectedBrokers()` at `MQTTBridge.cpp:3676` is defined but has -zero consumers.) - -### 3. `volatile` cross-core handshakes - -Plain `volatile` flags (no atomics/barriers), Core 1 sets the request, Core 0 -clears/processes and writes a "done" flag last, Core 1 spins: - -- `_slot_reconfigure_pending[]` — set `MQTTBridge.cpp:2057` (Core 1), read/clear - `:1118` (Core 0). -- `_ntp_force_{requested,done,result}` — request `:3298`, process `:1063-1068`, - spin-wait `:3308-3315`. -- `_ntp_diag_{requested,done}` (+ non-volatile `_ntp_diag_results[]`) — request - `:3344`, process `:1073-1076`, spin-wait `:3347`. - -The two blocking waiters (`requestForcedNtpSync` `:3298`, `ntpDiag` `:3344`) spin -on Core 1 while `end()` could tear down the singleton/task concurrently. - -### 4. Abrupt teardown, no start guard — RESOLVED in Phase 5 - -Original hazard (retained for context): `end()` used -`vTaskDelete(_mqtt_task_handle)` to kill the task wherever it was (possibly -mid-`_slots[]` mutation or inside mbedTLS), then ran slot/client cleanup *after* -deletion on the caller's context — the OTA teardown heap-panic path. `begin()` -had no double-call guard, and lifecycle state was a single `_initialized` bool. - -**Phase 5 resolution** (branch `phase5/cooperative-mqtt-shutdown`): - -- `end()` now requests a cooperative stop through `MQTTLifecycle::Coordinator`. - The MQTT task (Core 0) tears down its own clients where the mbedTLS contexts - live, acknowledges via `_stop_acked`, and self-terminates; `end()` waits for - the ack before freeing the queue/buffers. The blind `vTaskDelete` survives - only as the bounded-timeout fallback, which sets a dirty latch that withholds - OTA flashing (`canFlashAfterStop()`). -- `begin()` has an idempotent double-call guard and drives the Coordinator to - `Running`; the lifecycle state now lives in the tested state machine, not a - bare bool. -- The OTA barrier gates `simple_repeater`'s deferred flash on a clean stop. - -Not hardware-validated yet (Phase 7); `MQTT_STOP_TIMEOUT_MS` is a Phase-0 -placeholder. Note the residual §1/§2 instance-pointer reads remain deferred, so -consumers can still (as before) touch a torn-down bridge — that is unchanged by -Phase 5 and tracked above. - -## Target primitives (Phase 5) - -- **Task notifications or a command queue** for one-way lifecycle / reconfigure - / NTP requests — replacing every `volatile` flag in §3. -- **Immutable published snapshots** for WebConfig, CLI diagnostics, and alerting - — the MQTT task publishes a plain-data `SlotStatusSnapshot` (owned char - buffers + scalars, no live pointers) that §1/§2 consumers read. This also - removes the "instance pointer survives `end()`" hazard because consumers stop - dereferencing the live bridge. -- **Atomics** only for truly independent scalar state. -- **A mutex** only where ownership transfer or snapshot publication cannot - express the operation cleanly. - -## Lifecycle contract (the test seam) - -`src/helpers/MQTTLifecycle.h` encodes the cooperative lifecycle Phase 5 must -implement, as a pure state machine plus a narrow injected `Ops` seam -(clock / task control / resource owner / OTA barrier). The invariants proven by -`test/test_mqtt_lifecycle/` — and that Phase 5's production wiring must preserve: - -- `Stopped → Starting → Running → StopRequested → Stopping → Stopped`. -- Idempotent start and stop; safe restart only from `Stopped` (`mayRestart`). -- New connects/publishes/retries/reconfigurations cease once a stop is requested - (`acceptsNewWork`). -- Resources are released **only** after a stop acknowledgment (or the reviewed - timeout fallback) — never mid-run. -- A late/stale callback consults `mayTouchOwnedState()` and is a no-op once the - owner has released resources. -- Bounded stop timeout → reviewed fallback (models replacing the abrupt - `vTaskDelete`). -- **OTA barrier:** `mayBeginFlash()` is true only after a **clean** stop - acknowledgment; a timed-out stop leaves flashing blocked so OTA aborts rather - than writing under uncertain ownership. - -### Phase 0 / hardware-pending items - -Per the "derive from code + flag" discipline, these are **not** encoded as -constants and must be characterized on hardware before Phase 5 ships: - -- The concrete stop timeout (injected as `Coordinator`'s `stop_timeout_ms`; - must be measured against mbedTLS teardown over `wss` with a down broker). -- Exact callback timing/ordering under a real TLS disconnect. -- Heap / largest-block / task-stack high-water and task/client counts across - start/stop/restart (Phase 7 gates). - -## Deferred to Phase 5 (not done here) - -- Publishing the plain-data snapshot and repointing the §1/§2 consumers at it. -- Replacing the §3 `volatile` handshakes with a command queue / task - notifications. -- The cooperative shutdown state machine in `MQTTBridge` (`end()` rewrite, the - `begin()` double-call guard) and the OTA teardown barrier. - -`MQTTBridge.cpp` was intentionally left untouched in Phase 4 to keep the -merge-sensitive file (≈3.7k lines) free of churn until the Phase 5 change lands -as a single reviewable unit. +# Observer task ownership + +Current implementation: `feat/observer-reliability`, based on `5c0da89117cdddcafbe1c9c5803fa92082ba7e2f`, September 2026. The framework remains Arduino-ESP32 2.0.17 / ESP-IDF 4.4.7 on PlatformIO espressif32 6.11.0. + +## Owners and publication boundaries + +| Domain | Owner | Handoff | +|---|---|---| +| SDK handles, slot transitions, config application, MQTT publication and NTP probes | MQTT bridge worker | SDK callbacks enqueue copied `MqttSlotEvent` records; the worker applies them | +| SDK connection/stop acknowledgment flags | SDK MQTT task and wrapper caller | `std::atomic`; lifecycle effects never run inside a callback | +| Durable preferences, CLI, portal batch execution, lifecycle coordinator | Arduino loop task | Candidate edit, verified save, then durable RAM commit; revisioned config mailbox to worker | +| RF packet staging and score, radio/board telemetry | Arduino loop task | Value-copy FreeRTOS packet queue; one-second telemetry snapshot | +| Diagnostic slot state and counters | MQTT bridge worker | Pointer-free runtime snapshot; readers copy under a short lock, then format | +| Portal configuration responses | AsyncTCP task | Reads a portal-owned view published by `tick()` between loop commands; mutex also protects batch state | +| SNMP OID storage | MQTT bridge worker | Mesh task publishes one radio/name/version record; worker copies it before servicing a request | +| Network event observations | Wi-Fi event task | Atomic outage/status publication; credentials remain with network owner | +| OTA manifest check | Dedicated task | Cached asynchronous result with request ID and completion time | + +Core affinity is not ownership: the SDK MQTT task and bridge worker may both run on Core 0 and still require synchronization. The queue and snapshot publishers above run in task context, not from an ISR. + +`ObserverMailbox`, `ObserverConfigMailbox`, `MqttEventChannel` and `ObserverAsyncJob` share `ObserverLock`: a statically allocated FreeRTOS mutex on ESP32 and `std::mutex` in host tests. Every holder is a task, never an ISR, so the lock does not disable interrupts; priority inheritance covers a preempted holder. No allocation, formatting, flash write, SDK call or network operation runs under it. The worker's runtime snapshot publishes every pass, but its heap fields come from a once-per-second sample so the heap walk is not paid per iteration. `MQTTPrefs` is 2,880 bytes in the current layout; the bridge retains two copies (pending and applied). The portal retains another copy while its object exists. These costs need to be included in non-PSRAM heap measurements. + +## Event and configuration ordering + +Callbacks capture an incarnation when a client is allocated and copy their error fields before the SDK event expires. They never mutate slots or retain SDK event pointers. Retired-incarnation and stopped/quarantined-client events are ignored. A start attempt enters `Starting` before calling the driver. A failed call restores the prior state without claiming the credential was applied. + +The event channel holds 32 records. Overflow records a count and affected-slot mask. The worker clears the affected slot's connected state and stops its client before another publication cycle. A proven stop can be retried normally; an unproven stop retains its resources. Queue loss never means an assumed connected session. + +Configuration and the accumulated slot-reconfigure mask travel together. The worker consumes one complete revision at a safe point. Other runtime settings and radio metadata are sampled between loop commands, at most every 100 ms. Failed and indeterminate saves never publish a candidate to the running worker; indeterminate flash outcomes still require reboot/recovery to determine which image is durable on storage. + +`get mqtt.runtime` reports runtime sample sequence/age, desired/applied configuration revisions and event-overflow count. Revision equality means the worker consumed the configuration, not that a broker accepted it. The whole report is a sampled value: if the worker is blocked in the SDK, its age grows and a newly queued revision may not yet appear. + +The packet capture task reads loop-owned durable settings and published slot admission state. It never reads a worker-owned client or mutable slot. QoS0 publish acceptance retains its existing meaning; no delivery guarantee or public broker schema has changed. + +## Stop and restart + +The loop task requests stop through `MQTTLifecycle::Coordinator`. The worker tears down clients and acknowledges only after their stops are proven. The task trampoline publishes its final acknowledgment immediately before self-deletion. Resource cleanup follows that acknowledgment. + +A timeout enters `StopUnproven`. There is **no forced worker deletion** and no freeing of reachable clients, tokens, packet buffers or task state. OTA flashing is withheld. A late acknowledgment permits cleanup and a later restart; an unexpected failed SDK stop quarantines the client for the boot. Do not turn a timeout into permission to free resources. + +Client handles can be reused across compatible configuration changes, but TLS session buffers are allocated and released as connections open and close. Retaining a wrapper does not guarantee persistent mbedTLS session allocations. The existing OTA settle delay is retained; native/build results do not establish idle-task reclamation or device heap behavior. + +## Diagnostics + +`get mqtt.ntp.diag` queues a probe and returns immediately. Repeat it to retrieve the completed result, identified by request ID and age. Results are cached for 30 seconds; after expiry, the next call starts a new probe. The probe never sets the clock. Forced primary NTP synchronization also returns immediately, and shares the completion-based retry scheduler with ordinary synchronization. + +`ota check` follows the same request/result pattern, caching results for 60 seconds. A queued or running check cannot authorize flashing. If `ota update` has to start a check, the app loop arms `OtaUpdateFollowUp`, polls `MainBoard::otaCheckInProgress()`, and schedules the ordinary deferred flash itself once the cached dry run reports an applicable build; a refusal or a check that does not settle within two minutes is reported on the console and alert channel instead. Actual flashing still uses the existing deferred stop/barrier path and re-fetches the manifest with certificate verification. The check remains advisory. + +The portal terminal uses the same commands and replies; repeat a queued diagnostic command to poll its job. A completed portal command batch means its CLI commands returned, not that an asynchronous probe finished. + +## Validation and release gate + +Host tests exercise the production snapshot/config/event/job helpers, transition effects, checked client initialization, NTP scheduling, and existing persistence/lifecycle policies. ASan/UBSan run in `native_sanitized`; those sanitizers do not detect all data races or validate the precompiled SDK. Concurrent host tests check coherent copies and ordering, while firmware builds check the FreeRTOS adapter and framework integration. + +Observer PR smoke coverage includes repeater, room server, Ethernet, PSRAM and a non-observer target. Both observer release workflows require the reusable host verification workflow and preset parity before publication. Parity compares the candidate SHA with the sibling channel. These local workflow checks are not evidence of a live GitHub run. + +Before fleet deployment, bench-test portal polling during reconfiguration, fast callbacks, broken brokers/DNS/NTP, queue saturation, stop during connect/probe, repeated start/stop, and OTA abort/resume on non-PSRAM, PSRAM and Ethernet boards. Record loop gaps, heap/largest block, stack high-water marks and task/client counts. Run power-cut migration tests separately. + +## History and upstream seams + +- [Earlier ownership phase plan](docs/observer-ownership-history.md): historical hazards and proposed phases. +- [Stability/testability handoff](STABILITY_TESTABILITY_HANDOFF.md): historical device evidence; use its recorded board/date/revision, not as proof of this branch. +- [Observer implementation](MQTT_IMPLEMENTATION.md): operating/configuration behavior. +- [Upstream restoration history](RESTORE_UPSTREAM_NOTES.md): earlier restoration notes, with current-source correction. + +Keep observer preference layouts and shared helpers outside upstream `NodePrefs`. CommonCLI's serializer/candidate hooks remain narrow; application integration retains role-specific mesh behavior. Do not replace the framework, remove stop guards, or expand the private WebSocket ABI shim without revalidating the pinned SDK. diff --git a/RESTORE_UPSTREAM_NOTES.md b/RESTORE_UPSTREAM_NOTES.md index 6195ecda46..bdf24a0271 100644 --- a/RESTORE_UPSTREAM_NOTES.md +++ b/RESTORE_UPSTREAM_NOTES.md @@ -69,9 +69,11 @@ observer's purpose. Mitigated on observer builds by `RxReservePacketManager` reserve (own responses/ACKs stay queueable) plus 30 s expiry of stale queued outbound. See MQTT_INTERNALS.md "Capture vs. duty-cycle throttling". -## Phase 2 — CAD and FEM RX gain (NOT done; needs care + device testing) +## Historical Phase 2 — CAD and FEM RX gain -Still missing at HEAD, also dropped by `22eb9b87`, still present upstream: +**Current-source correction (2026-09-20):** the missing-feature list below describes the old restoration checkpoint. Current `CommonCLI.h`, `CommonRadioPrefs`, role `getCADEnabled()` hooks, and radio wrappers contain CAD/FEM settings and integration. Per-board hardware behavior still requires validation. Do not use this historical list as instructions to restore those fields again. + +At the historical checkpoint these were missing, also dropped by `22eb9b87`, still present upstream: - **`cad_enabled`** — hardware Channel Activity Detection (listen-before-talk) before TX. The `Dispatcher`/`Radio` interface (`setCADEnabled`/`getCADEnabled`) is restored by diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 5001f69960..fb36a0260b 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -1279,12 +1279,14 @@ region save **Usage:** - `get mqtt.ntp.diag` -**Description:** Probes every configured NTP server (the custom primary, if set, plus the built-in fallbacks) and reports whether each responds. This is a pure connectivity diagnostic — it does **not** change the system clock. +**Description:** Queues a probe of every configured NTP server and returns immediately. Repeat the command to poll. Completed results include the request ID and age and are cached for 30 seconds; a later call queues a new probe. This diagnostic does **not** change the system clock. - **Serial console:** prints a detailed table with each server's reported UTC time (or `FAIL`). - **Over LoRa:** returns a compact ` ok|fail` list, one per line. -Requires WiFi connected and the MQTT bridge running. +Requires a connected network and the MQTT bridge running. + +`get mqtt.runtime` reports the worker sample sequence and age, desired/applied configuration revisions, and lost SDK event count. Applied configuration does not imply a successful broker connection. --- @@ -1374,3 +1376,7 @@ Ethernet support is available on RAK4631 boards with a RAK13800 (W5100S) Etherne - Connect with any TCP client (e.g. `nc`, PuTTY) to access the same CLI available over serial. --- + +### Observer OTA checks + +`ota check` queues a manifest check and returns immediately. Repeat it for the result, which is cached for 60 seconds and includes an ID and age. `ota update` with no fresh check queues one and replies `OTA check #N queued; update starts automatically if a newer build applies`: the node waits for that check (up to two minutes) and then schedules the flash itself, or reports `OTA not started: ` on the serial console and the alert channel. A pending check never starts an update. Actual updates retain the deferred bridge-stop barrier and revalidate the manifest before flashing. diff --git a/docs/observer-ownership-history.md b/docs/observer-ownership-history.md new file mode 100644 index 0000000000..00b168f1a7 --- /dev/null +++ b/docs/observer-ownership-history.md @@ -0,0 +1,189 @@ +# Historical MQTT ownership plan + +Archived on 2026-09-20. This is the earlier phase plan, not the current ownership contract. Its line references, deferred items and forced-delete description are historical. See [current contract](../MQTT_OWNERSHIP.md). + +# MQTT Bridge Cross-Core Ownership Model + +This document is the Phase 4 deliverable from `STABILITY_TESTABILITY_HANDOFF.md`: +it records **one owner for each mutable runtime domain**, maps every place a +non-owner reads owned state across cores today, and states the target primitive +for each. It is paired with the fork-owned lifecycle test seam +(`src/helpers/MQTTLifecycle.h`, `test/test_mqtt_lifecycle/`). + +**Status:** ownership model documented and the lifecycle/teardown test seam +landed (Phase 4). **Phase 5 (branch `phase5/cooperative-mqtt-shutdown`) has now +implemented the cooperative shutdown and the OTA barrier — hazard §4 below.** +Still **deferred** (carried to Phase 5b / Phase 6): publishing a plain-data +snapshot and repointing the §1/§2 consumers, and replacing the §3 `volatile` +handshakes with a command channel. This document is the plan; §1–§3 still +describe current behavior, while §4 is now resolved (see its note). Line +references are against the tree at the time of writing and should be re-verified +before editing. + +## Execution contexts + +- **MQTT task — Core 0** (`"MQTTBridge"`, `xTaskCreatePinnedToCore(..., MQTT_TASK_CORE=0)`, + entry `mqttTask` → `mqttTaskLoop`, `MQTTBridge.cpp:923`, `:991`). Owns all + WiFi / MQTT / NTP I/O and every mutation of `_slots[]`, connection state, and + NTP state. +- **Loop task — Core 1** (`MyMesh::loop()`). Runs the CLI, WebConfig `tick()`, + and `AlertReporter::onLoop()`. `examples/simple_repeater/MyMesh.cpp:1529` + notes the bridge loop is *not* called here — it lives on Core 0. +- **Producer / radio context.** Stages raw radio bytes before queue handoff + (`storeRawRadioData` → `_staged_*`, consumed by `queuePacket`; both Core 1, in + guaranteed sequence, `MQTTBridge.h:232-239`). +- **Async TCP context.** WebConfig request parsing and immutable response + handoff only. + +## Ownership model (one owner per mutable domain) + +| Domain | Owner | Notes | +|--------|-------|-------| +| MQTT clients, slot connection state, publish counters, packet drain, NTP I/O | **MQTT task (Core 0)** | `_slots[]`, `PsychicMqttClient`s, `_ntp_client`, `_last_raw_*` | +| CLI execution, preference persistence, WebConfig batch draining, bridge lifecycle requests | **Loop task (Core 1)** | issues start/stop/reconfigure, drains the WebConfig batch | +| Packet staging before queue handoff | **Producer / radio (Core 1)** | `_staged_*`, no lock needed (sequential) | +| Request parse + immutable response | **Async TCP** | must not touch mutable bridge state | + +The rule that follows: **loop/WebConfig/CLI/AlertReporter code must not directly +inspect mutable MQTT slot objects or client counters.** The MQTT task must +publish a plain-data snapshot they can read instead. + +## Current cross-core hazards (to resolve in Phase 5) + +All of the following run on **Core 1** and read state mutated by **Core 0** +without a lock or a published snapshot. + +### 1. Diagnostic reads of live `_slots[]` via the singleton + +Four `static` accessors reach the live object through the file-scope +`s_mqtt_bridge_instance` (`MQTTBridge.cpp:201`, set at `begin()` end `:849`, +nulled first in `end()` `:858` — a plain, non-atomic pointer): + +- `getSlotStatusSnapshot` (`:299`) — despite its name, built on demand from + live `_slots[slot_index]`. **Refined premise:** the returned `name`/`state` + `const char*`s point at static rodata (string literals / `MQTT_PRESETS[]`), + so they are *not* dangling. The real hazards are (a) reading the + `slot.preset` **pointer value**, which Core 0 can null/reassign mid-read + (`applySlotPreset`, `teardownSlot`), and (b) `slot.client->getPublishOk()`, + a live client pointer Core 0 can `delete` during teardown (UAF window). +- `formatMqttStatusReply` (`:207`), `formatMqttStatsReply` (`:263`), + `formatSlotDiagReply` (`:397`) — same singleton + live `_slots[]` reads. + +Consumers: CLI (`CommonCLI_Observer.cpp:726, :728, :791`) and the app-layer +stats JSON (`examples/simple_repeater/MyMesh.cpp:1423`, mirrored in +`examples/simple_room_server/MyMesh.cpp:1050`). **Refined premise:** +`WebConfigServer.cpp` itself reads only a compile-time constant +(`getMaxActiveSlots`, `:479`); the live-bridge web reads are in `buildStatsJson` +in the `MyMesh.cpp` app layer. + +### 2. Instance reads that survive `end()` + +`AlertReporter` (Core 1, `onLoop`, `AlertReporter.cpp:208`) reads live `_slots[]` +via instance methods it reaches through its own `_bridge` pointer +(`AlertReporter.cpp:281` `isSlotEnabledAndAttempted`, `:285` +`getSlotCurrentOutageStartMs`, `:296`/`:311` `getSlotPresetName`). The stats +JSON also calls `bridge->getQueueSize()` (`MyMesh.cpp:1419`). + +**Refined premise (teardown hazard):** `end()` nulls only +`s_mqtt_bridge_instance`. The app's `bridge` pointer and `AlertReporter::_bridge` +are **not** cleared by `end()`, so these instance reads can touch a torn-down +bridge. (`getConnectedBrokers()` at `MQTTBridge.cpp:3676` is defined but has +zero consumers.) + +### 3. `volatile` cross-core handshakes + +Plain `volatile` flags (no atomics/barriers), Core 1 sets the request, Core 0 +clears/processes and writes a "done" flag last, Core 1 spins: + +- `_slot_reconfigure_pending[]` — set `MQTTBridge.cpp:2057` (Core 1), read/clear + `:1118` (Core 0). +- `_ntp_force_{requested,done,result}` — request `:3298`, process `:1063-1068`, + spin-wait `:3308-3315`. +- `_ntp_diag_{requested,done}` (+ non-volatile `_ntp_diag_results[]`) — request + `:3344`, process `:1073-1076`, spin-wait `:3347`. + +The two blocking waiters (`requestForcedNtpSync` `:3298`, `ntpDiag` `:3344`) spin +on Core 1 while `end()` could tear down the singleton/task concurrently. + +### 4. Abrupt teardown, no start guard — RESOLVED in Phase 5 + +Original hazard (retained for context): `end()` used +`vTaskDelete(_mqtt_task_handle)` to kill the task wherever it was (possibly +mid-`_slots[]` mutation or inside mbedTLS), then ran slot/client cleanup *after* +deletion on the caller's context — the OTA teardown heap-panic path. `begin()` +had no double-call guard, and lifecycle state was a single `_initialized` bool. + +**Phase 5 resolution** (branch `phase5/cooperative-mqtt-shutdown`): + +- `end()` now requests a cooperative stop through `MQTTLifecycle::Coordinator`. + The MQTT task (Core 0) tears down its own clients where the mbedTLS contexts + live, acknowledges via `_stop_acked`, and self-terminates; `end()` waits for + the ack before freeing the queue/buffers. The blind `vTaskDelete` survives + only as the bounded-timeout fallback, which sets a dirty latch that withholds + OTA flashing (`canFlashAfterStop()`). +- `begin()` has an idempotent double-call guard and drives the Coordinator to + `Running`; the lifecycle state now lives in the tested state machine, not a + bare bool. +- The OTA barrier gates `simple_repeater`'s deferred flash on a clean stop. + +Not hardware-validated yet (Phase 7); `MQTT_STOP_TIMEOUT_MS` is a Phase-0 +placeholder. Note the residual §1/§2 instance-pointer reads remain deferred, so +consumers can still (as before) touch a torn-down bridge — that is unchanged by +Phase 5 and tracked above. + +## Target primitives (Phase 5) + +- **Task notifications or a command queue** for one-way lifecycle / reconfigure + / NTP requests — replacing every `volatile` flag in §3. +- **Immutable published snapshots** for WebConfig, CLI diagnostics, and alerting + — the MQTT task publishes a plain-data `SlotStatusSnapshot` (owned char + buffers + scalars, no live pointers) that §1/§2 consumers read. This also + removes the "instance pointer survives `end()`" hazard because consumers stop + dereferencing the live bridge. +- **Atomics** only for truly independent scalar state. +- **A mutex** only where ownership transfer or snapshot publication cannot + express the operation cleanly. + +## Lifecycle contract (the test seam) + +`src/helpers/MQTTLifecycle.h` encodes the cooperative lifecycle Phase 5 must +implement, as a pure state machine plus a narrow injected `Ops` seam +(clock / task control / resource owner / OTA barrier). The invariants proven by +`test/test_mqtt_lifecycle/` — and that Phase 5's production wiring must preserve: + +- `Stopped → Starting → Running → StopRequested → Stopping → Stopped`. +- Idempotent start and stop; safe restart only from `Stopped` (`mayRestart`). +- New connects/publishes/retries/reconfigurations cease once a stop is requested + (`acceptsNewWork`). +- Resources are released **only** after a stop acknowledgment (or the reviewed + timeout fallback) — never mid-run. +- A late/stale callback consults `mayTouchOwnedState()` and is a no-op once the + owner has released resources. +- Bounded stop timeout → reviewed fallback (models replacing the abrupt + `vTaskDelete`). +- **OTA barrier:** `mayBeginFlash()` is true only after a **clean** stop + acknowledgment; a timed-out stop leaves flashing blocked so OTA aborts rather + than writing under uncertain ownership. + +### Phase 0 / hardware-pending items + +Per the "derive from code + flag" discipline, these are **not** encoded as +constants and must be characterized on hardware before Phase 5 ships: + +- The concrete stop timeout (injected as `Coordinator`'s `stop_timeout_ms`; + must be measured against mbedTLS teardown over `wss` with a down broker). +- Exact callback timing/ordering under a real TLS disconnect. +- Heap / largest-block / task-stack high-water and task/client counts across + start/stop/restart (Phase 7 gates). + +## Deferred to Phase 5 (not done here) + +- Publishing the plain-data snapshot and repointing the §1/§2 consumers at it. +- Replacing the §3 `volatile` handshakes with a command queue / task + notifications. +- The cooperative shutdown state machine in `MQTTBridge` (`end()` rewrite, the + `begin()` double-call guard) and the OTA teardown barrier. + +`MQTTBridge.cpp` was intentionally left untouched in Phase 4 to keep the +merge-sensitive file (≈3.7k lines) free of churn until the Phase 5 change lands +as a single reviewable unit. diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index dfd407b66d..d0d671ad84 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -1601,27 +1601,7 @@ void MyMesh::buildStatsJson(char* buf, size_t buf_size) { (unsigned long)getNumRecvFlood(), (unsigned long)getNumRecvDirect(), (int)_mgr->getOutboundCount(0xFFFFFFFF), wifi_rssi, network_medium, ip, bridge ? bridge->getQueueSize() : 0); - if (pos < 0 || pos >= (int)buf_size - 3) return; // truncated; snprintf terminated it - bool first = true; - for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - MQTTBridge::SlotStatusSnapshot s; - if (!MQTTBridge::getSlotStatusSnapshot(i, &s)) continue; - // "filt" is omitted for the all-types default, so the portal only has to - // render the exception and the JSON stays inside the stats buffer. - char filt[24]; - filt[0] = '\0'; - if (s.filter_mask != MQTTPacketFilter::kAllPacketTypes) { - snprintf(filt, sizeof(filt), ",\"filt\":%u", (unsigned)s.filter_mask); - } - int n = snprintf(buf + pos, buf_size - pos, - "%s{\"n\":%d,\"name\":\"%s\",\"state\":\"%s\",\"ok\":%lu,\"err\":%lu%s}", - first ? "" : ",", i + 1, s.name, s.state, s.publish_ok, s.publish_err, - filt); - if (n < 0 || n >= (int)(buf_size - pos)) break; - pos += n; - first = false; - } - snprintf(buf + pos, buf_size - pos, "]}"); + MQTTBridge::appendSlotStatsJson(buf, buf_size, pos); } #endif @@ -1748,10 +1728,11 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, char *command, char *reply void MyMesh::loop() { // Check radio FIRST to ensure we don't miss incoming packets - // MQTT processing runs in a separate FreeRTOS task on Core 0, so we don't call bridge.loop() here + // MQTT I/O runs in its worker; bridge.loop() publishes loop-owned inputs only. mesh::Mesh::loop(); #ifdef WITH_MQTT_BRIDGE + if (bridge) bridge->loop(); // A timed-out stop keeps the bridge down until the MQTT task acknowledges it. // Release the withheld resources whenever that late ack lands, and restart // only a bridge that is still meant to be running. @@ -1795,6 +1776,7 @@ void MyMesh::loop() { } #if defined(WITH_MQTT_BRIDGE) && defined(OTA_MANIFEST_BASE) + pollDeferredOtaCheck(); if (_ota_update_at && millisHasNowPassed(_ota_update_at)) { // deferred `ota update` _ota_update_at = 0; // clear timer // The "Beginning update..." reply has now gone out. Free the bridge for heap diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index 344d493288..536b548f90 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -29,6 +29,7 @@ #include "helpers/bridges/MQTTBridge.h" #define WITH_BRIDGE #include "helpers/esp32/WebConfigServer.h" // defines WITH_WEBCONFIG on ESP32 +#include "helpers/OtaUpdateFollowUp.h" #endif #ifdef WITH_SNMP @@ -126,6 +127,9 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks CayenneLPP telemetry; unsigned long set_radio_at, revert_radio_at; unsigned long _ota_update_at = 0; // deferred `ota update` fire time (0 = none scheduled) +#ifdef WITH_MQTT_BRIDGE + OtaUpdateFollowUp _ota_follow_up; // `ota update` waiting on a queued manifest check +#endif float pending_freq; float pending_bw; uint8_t pending_sf; @@ -489,6 +493,44 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks return true; } +#if defined(WITH_MQTT_BRIDGE) + bool beginDeferredOtaUpdateAfterCheck() override { + _ota_follow_up.arm(millis()); + return true; + } + + // Poll the queued manifest check that an `ota update` is waiting on. Runs on + // the loop task; the board's check task owns the HTTP work. + void pollDeferredOtaCheck() { + if (!_ota_follow_up.armed()) return; + char ota_reply[160] = ""; + mesh::MainBoard* board = _cli.getBoard(); + const OtaUpdateFollowUp::Action action = _ota_follow_up.poll(millis(), board->otaCheckInProgress(), [&] { + if (board->otaFromManifest(getFirmwareVer(), true, ota_reply)) return OtaUpdateFollowUp::Check::Applicable; + return board->otaCheckInProgress() ? OtaUpdateFollowUp::Check::Requeued + : OtaUpdateFollowUp::Check::NotApplicable; + }); + char msg[160]; + switch (action) { + case OtaUpdateFollowUp::Action::Start: + Serial.print("OTA: check complete - "); Serial.println(ota_reply); + beginDeferredOtaUpdate(); + break; + case OtaUpdateFollowUp::Action::Refuse: + Serial.print("OTA: not started - "); Serial.println(ota_reply); + snprintf(msg, sizeof(msg), "OTA not started: %s", ota_reply); + otaAlert(msg); + break; + case OtaUpdateFollowUp::Action::Timeout: + Serial.println("OTA: manifest check did not complete; update cancelled"); + otaAlert("OTA not started: manifest check did not complete"); + break; + default: + break; + } + } +#endif + int getQueueSize() override { return bridge ? bridge->getQueueSize() : 0; } diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 1eba3892fd..ea6572b351 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -1403,27 +1403,7 @@ void MyMesh::buildStatsJson(char* buf, size_t buf_size) { (unsigned long)getNumRecvFlood(), (unsigned long)getNumRecvDirect(), (int)_mgr->getOutboundCount(0xFFFFFFFF), wifi_rssi, network_medium, ip, bridge ? bridge->getQueueSize() : 0); - if (pos < 0 || pos >= (int)buf_size - 3) return; // truncated; snprintf terminated it - bool first = true; - for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - MQTTBridge::SlotStatusSnapshot s; - if (!MQTTBridge::getSlotStatusSnapshot(i, &s)) continue; - // "filt" is omitted for the all-types default, so the portal only has to - // render the exception and the JSON stays inside the stats buffer. - char filt[24]; - filt[0] = '\0'; - if (s.filter_mask != MQTTPacketFilter::kAllPacketTypes) { - snprintf(filt, sizeof(filt), ",\"filt\":%u", (unsigned)s.filter_mask); - } - int n = snprintf(buf + pos, buf_size - pos, - "%s{\"n\":%d,\"name\":\"%s\",\"state\":\"%s\",\"ok\":%lu,\"err\":%lu%s}", - first ? "" : ",", i + 1, s.name, s.state, s.publish_ok, s.publish_err, - filt); - if (n < 0 || n >= (int)(buf_size - pos)) break; - pos += n; - first = false; - } - snprintf(buf + pos, buf_size - pos, "]}"); + MQTTBridge::appendSlotStatsJson(buf, buf_size, pos); } #endif @@ -1568,6 +1548,7 @@ void MyMesh::loop() { mesh::Mesh::loop(); #ifdef WITH_MQTT_BRIDGE + if (bridge) bridge->loop(); // A timed-out stop keeps the bridge down until the MQTT task acknowledges it. // Release the withheld resources whenever that late ack lands, and restart // only a bridge that is still meant to be running. @@ -1663,6 +1644,7 @@ void MyMesh::loop() { #endif #if defined(WITH_MQTT_BRIDGE) && defined(OTA_MANIFEST_BASE) + pollDeferredOtaCheck(); if (_ota_update_at && millisHasNowPassed(_ota_update_at)) { // deferred `ota update` _ota_update_at = 0; // clear timer // The "Beginning update..." reply has now been queued. Flush it before OTA diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index 1870df79bb..162d9ec016 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -34,6 +34,7 @@ #include "helpers/bridges/MQTTBridge.h" #define WITH_BRIDGE #include "helpers/esp32/WebConfigServer.h" // defines WITH_WEBCONFIG on ESP32 +#include "helpers/OtaUpdateFollowUp.h" #endif #ifdef WITH_SNMP @@ -143,6 +144,9 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks TransportKey default_scope; unsigned long set_radio_at, revert_radio_at; unsigned long _ota_update_at = 0; // deferred `ota update` fire time (0 = none scheduled) +#ifdef WITH_MQTT_BRIDGE + OtaUpdateFollowUp _ota_follow_up; // `ota update` waiting on a queued manifest check +#endif float pending_freq; float pending_bw; uint8_t pending_sf; @@ -479,6 +483,46 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks return true; } +#if defined(WITH_MQTT_BRIDGE) + bool beginDeferredOtaUpdateAfterCheck() override { + _ota_follow_up.arm(millis()); + return true; + } + + // Poll the queued manifest check that an `ota update` is waiting on. Runs on + // the loop task; the board's check task owns the HTTP work. + void pollDeferredOtaCheck() { + if (!_ota_follow_up.armed()) return; + char ota_reply[160] = ""; + mesh::MainBoard* board = _cli.getBoard(); + const OtaUpdateFollowUp::Action action = _ota_follow_up.poll(millis(), board->otaCheckInProgress(), [&] { + if (board->otaFromManifest(getFirmwareVer(), true, ota_reply)) return OtaUpdateFollowUp::Check::Applicable; + return board->otaCheckInProgress() ? OtaUpdateFollowUp::Check::Requeued + : OtaUpdateFollowUp::Check::NotApplicable; + }); + char msg[160]; + switch (action) { + case OtaUpdateFollowUp::Action::Start: + Serial.print("OTA: check complete - "); Serial.println(ota_reply); + beginDeferredOtaUpdate(); + break; + case OtaUpdateFollowUp::Action::Refuse: + Serial.print("OTA: not started - "); Serial.println(ota_reply); + snprintf(msg, sizeof(msg), "OTA not started: %s", ota_reply); + if (_cli.getObserverPrefs() && _cli.getObserverPrefs()->alert_enabled) _alerter.sendText(msg); + break; + case OtaUpdateFollowUp::Action::Timeout: + Serial.println("OTA: manifest check did not complete; update cancelled"); + if (_cli.getObserverPrefs() && _cli.getObserverPrefs()->alert_enabled) { + _alerter.sendText("OTA not started: manifest check did not complete"); + } + break; + default: + break; + } + } +#endif + int getQueueSize() override { return bridge ? bridge->getQueueSize() : 0; } diff --git a/lib/PsychicMqttClient/src/MqttClientInit.h b/lib/PsychicMqttClient/src/MqttClientInit.h new file mode 100644 index 0000000000..148b8e22cd --- /dev/null +++ b/lib/PsychicMqttClient/src/MqttClientInit.h @@ -0,0 +1,17 @@ +#pragma once + +// Initialization is incomplete until lifecycle events have a registered sink. +// The candidate has never started, so registration failure can destroy it. +template +int initializeMqttClient(Handle& target, Init init, Register register_events, + Destroy destroy, int no_memory) { + Handle candidate = init(); + if (!candidate) return no_memory; + const int result = register_events(candidate); + if (result != 0) { + destroy(candidate); + return result; + } + target = candidate; + return 0; +} diff --git a/lib/PsychicMqttClient/src/PsychicMqttClient.cpp b/lib/PsychicMqttClient/src/PsychicMqttClient.cpp index 01efb194be..b4567071d7 100644 --- a/lib/PsychicMqttClient/src/PsychicMqttClient.cpp +++ b/lib/PsychicMqttClient/src/PsychicMqttClient.cpp @@ -1,3 +1,4 @@ +#include "MqttClientInit.h" #include "PsychicMqttClient.h" #include @@ -386,17 +387,17 @@ esp_err_t PsychicMqttClient::applyConfig() if (_client == nullptr) { - // esp_mqtt_client_init() takes the whole configuration, including the - // buffer sizes it allocates once and never resizes. - _client = esp_mqtt_client_init(&_mqtt_cfg); - if (_client == nullptr) - { - ESP_LOGE(TAG, "esp_mqtt_client_init failed"); - return ESP_ERR_NO_MEM; + const esp_err_t result = initializeMqttClient(_client, + [&]() { return esp_mqtt_client_init(&_mqtt_cfg); }, + [&](esp_mqtt_client_handle_t client) { + return esp_mqtt_client_register_event(client, MQTT_EVENT_ANY, _onMqttEventStatic, this); + }, + [](esp_mqtt_client_handle_t client) { esp_mqtt_client_destroy(client); }, + ESP_ERR_NO_MEM); + if (result != ESP_OK) { + ESP_LOGE(TAG, "MQTT initialization failed: %s", esp_err_to_name(result)); + return result; } - // Register event handler only once when client is first created - // to avoid memory leak from repeated registrations - esp_mqtt_client_register_event(_client, MQTT_EVENT_ANY, _onMqttEventStatic, this); _config_dirty = false; return ESP_OK; } diff --git a/lib/PsychicMqttClient/src/PsychicMqttClient.h b/lib/PsychicMqttClient/src/PsychicMqttClient.h index 93b85a4378..99f34833fd 100644 --- a/lib/PsychicMqttClient/src/PsychicMqttClient.h +++ b/lib/PsychicMqttClient/src/PsychicMqttClient.h @@ -1,4 +1,5 @@ #pragma once +#include /** * PsychicMqttClient * @@ -545,8 +546,8 @@ class PsychicMqttClient esp_mqtt_client_handle_t _client = nullptr; esp_mqtt_client_config_t _mqtt_cfg; esp_mqtt_error_codes_t _lastError; - bool _connected = false; - bool _stopMqttClient = false; + std::atomic _connected{false}; + std::atomic _stopMqttClient{false}; bool _config_dirty = true; bool _started = false; diff --git a/platformio.ini b/platformio.ini index 2c090ae5f9..84f54931e7 100644 --- a/platformio.ini +++ b/platformio.ini @@ -193,6 +193,13 @@ lib_deps = google/googletest @ 1.17.0 bblanchon/ArduinoJson @ 7.4.3 +[env:native_sanitized] +extends = env:native +build_flags = ${env:native.build_flags} + -fsanitize=address,undefined + -fno-omit-frame-pointer + -g + [env:native_kiss_modem] platform = native test_framework = googletest diff --git a/scripts/check_observer_ci.py b/scripts/check_observer_ci.py new file mode 100644 index 0000000000..da9b0c0067 --- /dev/null +++ b/scripts/check_observer_ci.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 +"""Check the observer release verification graph without publishing a build.""" + +from pathlib import Path +import copy +import unittest +import yaml + + +ROOT = Path(__file__).resolve().parents[1] +CHANNELS = {"observer-firmware", "observer-firmware-dev"} +SMOKE_ENVS = { + "Heltec_v3_repeater_observer_mqtt", + "Heltec_v3_room_server_observer_mqtt", + "T_Beam_S3_Supreme_SX1262_repeater_observer_mqtt", + "ThinkNode_M7_repeater_observer_mqtt", + "Heltec_v3_repeater", +} + + +def read_workflow(name): + # BaseLoader keeps GitHub's `on` key as text under YAML 1.1 parsers too. + return yaml.load((ROOT / ".github/workflows" / name).read_text(), Loader=yaml.BaseLoader) + + +def check_release(workflow): + jobs = workflow["jobs"] + release = jobs["release"] + required = { + "verify": "./.github/workflows/run-unit-tests.yml", + "preset-parity": "./.github/workflows/check-mqtt-preset-parity.yml", + } + for name, target in required.items(): + assert name in release["needs"], f"release must need {name}" + assert jobs[name]["uses"] == target, f"{name} must call {target}" + assert "if" not in jobs[name], f"{name} must not be optional" + assert "continue-on-error" not in jobs[name], f"{name} must fail the run" + # GitHub's default success() condition blocks publication if a dependency + # fails or is skipped. An explicit condition must be separately reviewed. + assert "if" not in release, "release must retain its default success gate" + + +class ReleaseGateTests(unittest.TestCase): + def setUp(self): + self.workflow = read_workflow("build-observer-firmwares.yml") + + def test_release_requires_both_checks(self): + check_release(self.workflow) + + def test_removed_dependency_is_rejected(self): + for name in ("verify", "preset-parity"): + broken = copy.deepcopy(self.workflow) + broken["jobs"]["release"]["needs"].remove(name) + with self.assertRaises(AssertionError): + check_release(broken) + + def test_failed_checks_cannot_be_bypassed(self): + broken = copy.deepcopy(self.workflow) + broken["jobs"]["release"]["if"] = "always()" + with self.assertRaises(AssertionError): + check_release(broken) + + +def main(): + for name in ("build-observer-firmwares.yml", "build-observer-firmwares-beta.yml"): + check_release(read_workflow(name)) + for name in ("run-unit-tests.yml", "check-mqtt-preset-parity.yml"): + assert "workflow_call" in read_workflow(name)["on"], f"{name} must be reusable" + smoke = read_workflow("pr-build-check.yml") + assert CHANNELS <= set(smoke["on"]["pull_request"]["branches"]) + assert SMOKE_ENVS <= set(smoke["jobs"]["build"]["strategy"]["matrix"]["environment"]) + native = read_workflow("run-unit-tests.yml") + commands = "\n".join(step.get("run", "") for step in native["jobs"]["test"]["steps"]) + assert "-e native_sanitized" in commands + suite = unittest.defaultTestLoader.loadTestsFromTestCase(ReleaseGateTests) + if not unittest.TextTestRunner().run(suite).wasSuccessful(): + raise SystemExit(1) + print("Observer release gates and smoke coverage passed.") + + +if __name__ == "__main__": + main() diff --git a/src/MeshCore.h b/src/MeshCore.h index 9ce7b1026a..503bdb5d7c 100644 --- a/src/MeshCore.h +++ b/src/MeshCore.h @@ -76,6 +76,9 @@ class MainBoard { // current_ver is the running firmware version string (used to skip if already up to date); when // dry_run is true the build is only reported, not flashed. Observer (ESP32+WiFi) builds only. virtual bool otaFromManifest(const char* current_ver, bool dry_run, char reply[]) { return false; } + // True while a queued or running manifest check owns the dry-run result, so a + // caller that wants the outcome knows to poll rather than treat "false" as "no update". + virtual bool otaCheckInProgress() const { return false; } // Power management interface (boards with power management override these) virtual bool isPwrMgtInitialised() const { return false; } diff --git a/src/helpers/AlertReporter.cpp b/src/helpers/AlertReporter.cpp index 4a58ad612f..bc198553df 100644 --- a/src/helpers/AlertReporter.cpp +++ b/src/helpers/AlertReporter.cpp @@ -247,11 +247,12 @@ void AlertReporter::onLoop(unsigned long now_ms) { for (int i = 0; i < n; i++) { AlertFaultPolicy::Fault& f = _mqtt[i]; - if (!_bridge->isSlotEnabledAndAttempted(i)) { + const MQTTBridge::SlotOutageSnapshot slot = _bridge->getSlotOutageSnapshot(i); + if (!slot.monitored) { AlertFaultPolicy::rearmIfDisabled(f); continue; } - const uint32_t outage_start = (uint32_t)_bridge->getSlotCurrentOutageStartMs(i); + const uint32_t outage_start = slot.started_ms; const AlertFaultPolicy::OutageSnapshot snap = AlertFaultPolicy::fromStartMs(outage_start); AlertFaultPolicy::TickResult r = AlertFaultPolicy::tick( @@ -259,7 +260,7 @@ void AlertReporter::onLoop(unsigned long now_ms) { if (r.action == AlertFaultPolicy::Action::FireDown) { char text[100]; AlertFaultPolicy::formatMqttDown(text, sizeof(text), i + 1, - _bridge->getSlotPresetName(i), + slot.name, r.duration_ms); if (sendChannel(text)) { AlertFaultPolicy::commitDown(f, now, outage_start); @@ -267,7 +268,7 @@ void AlertReporter::onLoop(unsigned long now_ms) { } else if (r.action == AlertFaultPolicy::Action::FireRecovered) { char text[100]; AlertFaultPolicy::formatMqttRecovered(text, sizeof(text), i + 1, - _bridge->getSlotPresetName(i), + slot.name, r.duration_ms); sendChannel(text); AlertFaultPolicy::commitRecovered(f); diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 657157d1d8..459815dcf1 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -1117,7 +1117,7 @@ bool CommonCLI::saveMQTTPrefs(FILESYSTEM* fs) { return false; } setMQTTPrefsDefaults(repair_defaults); - MQTTPrefsSerializer serializer(&_mqtt_prefs, repair_defaults); + MQTTPrefsSerializer serializer(_observer_candidate ? _observer_candidate : &_mqtt_prefs, repair_defaults); // The serializer hierarchy copies the default values it needs. Release this // large temporary before file I/O and the independent verification scratch. delete repair_defaults; @@ -1678,32 +1678,32 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep } } else if (memcmp(config, "bridge.source ", 14) == 0) { #ifdef WITH_MQTT_BRIDGE - MQTTPrefs* observer_rollback = new (std::nothrow) MQTTPrefs; - if (observer_rollback == nullptr) { + MQTTPrefs* observer_candidate = new (std::nothrow) MQTTPrefs; + if (observer_candidate == nullptr) { strcpy(reply, "Error: insufficient memory to update observer setting"); return; } - memcpy(observer_rollback, &_mqtt_prefs, sizeof(*observer_rollback)); + memcpy(observer_candidate, &_mqtt_prefs, sizeof(*observer_candidate)); const uint8_t old_bridge_pkt_src = _prefs->bridge_pkt_src; #endif _prefs->bridge_pkt_src = memcmp(&config[14], "rx", 2) == 0; #ifdef WITH_MQTT_BRIDGE if (_prefs->bridge_pkt_src == 1) { - _mqtt_prefs.mqtt_rx_enabled = 1; - _mqtt_prefs.mqtt_tx_enabled = 0; + observer_candidate->mqtt_rx_enabled = 1; + observer_candidate->mqtt_tx_enabled = 0; } else { - _mqtt_prefs.mqtt_rx_enabled = 0; - _mqtt_prefs.mqtt_tx_enabled = 1; + observer_candidate->mqtt_rx_enabled = 0; + observer_candidate->mqtt_tx_enabled = 1; } - _observer_prefs_rollback = observer_rollback; + _observer_candidate = observer_candidate; if (!persistObserverPrefs(reply)) { _prefs->bridge_pkt_src = old_bridge_pkt_src; - _observer_prefs_rollback = nullptr; - delete observer_rollback; + _observer_candidate = nullptr; + delete observer_candidate; return; } - _observer_prefs_rollback = nullptr; - delete observer_rollback; + _observer_candidate = nullptr; + delete observer_candidate; #endif savePrefs(); strcpy(reply, "OK"); diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index 34912525fc..0dcc4cfd7e 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -340,6 +340,13 @@ class CommonCLICallbacks { return false; }; + // `ota update` arrived while the manifest check was still queued. The app loop + // waits for that check and then arms beginDeferredOtaUpdate() itself if a + // newer build applies. Returns true if the follow-up was armed. + virtual bool beginDeferredOtaUpdateAfterCheck() { + return false; + }; + virtual int getQueueSize() { return 0; // no op by default }; @@ -416,9 +423,8 @@ class CommonCLI { char tmp[PRV_KEY_SIZE*2 + 4]; #ifdef WITH_MQTT_BRIDGE MQTTPrefs _mqtt_prefs; - // Points at a per-command snapshot only while an observer setter is running. - // persistObserverPrefs() uses it to undo RAM mutations when flash commit fails. - const MQTTPrefs* _observer_prefs_rollback = nullptr; + // Per-command candidate; durable RAM is replaced only after a verified save. + MQTTPrefs* _observer_candidate = nullptr; LegacyObserverTail _legacy_tail; // /mqtt.json is newer, corrupt, or temporarily unreadable. The in-memory prefs // run on defaults and saveMQTTPrefs() must not overwrite the source file. diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index 2696b8363d..2ffb64c653 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -12,6 +12,7 @@ #include #include "CommonCLI.h" +#include "ObserverPreferenceCommit.h" #include "TxtDataHelpers.h" #include "AlertReporter.h" // for alertReporterBannedChannelMatch[Hex]() #include "MQTTObserverValidation.h" // pure input validators (host-testable) @@ -123,35 +124,18 @@ static bool isObserverPrefsSetCommand(const char* config) { strncmp(config, "display.", 8) == 0; } -// Keep observer setters atomic from the caller's perspective. The live object -// is restored if its JSON transaction fails, and the snapshot pointer is valid -// only for this synchronous command dispatch. -class ObserverPrefsRollbackScope { +// Stage observer edits away from the loop-owned durable image. The serializer +// sees the candidate; consumers see it only after a verified commit. +class ObserverPrefsCandidateScope { public: - ObserverPrefsRollbackScope(const MQTTPrefs** active_snapshot, - const MQTTPrefs& live, - bool capture) - : _active_snapshot(active_snapshot) { - *_active_snapshot = nullptr; - if (capture) { - _snapshot = new (std::nothrow) MQTTPrefs; - if (_snapshot != nullptr) { - memcpy(_snapshot, &live, sizeof(*_snapshot)); - *_active_snapshot = _snapshot; - } - } + ObserverPrefsCandidateScope(MQTTPrefs** active, const MQTTPrefs& durable, bool capture) + : _active(active) { + *_active = capture ? new (std::nothrow) MQTTPrefs(durable) : nullptr; } - - ~ObserverPrefsRollbackScope() { - *_active_snapshot = nullptr; - delete _snapshot; - } - - bool available() const { return _snapshot != nullptr; } - + ~ObserverPrefsCandidateScope() { delete *_active; *_active = nullptr; } + bool available() const { return *_active != nullptr; } private: - const MQTTPrefs** _active_snapshot; - MQTTPrefs* _snapshot = nullptr; + MQTTPrefs** _active; }; static const char* getMQTTPresetNameByIndex(int index) { @@ -208,10 +192,8 @@ static void formatMQTTPresetListReply(char* reply, size_t reply_size, int start) bool CommonCLI::persistObserverPrefs(char* reply) { #ifdef WITH_MQTT_BRIDGE - if (_callbacks->saveObserverPrefs()) return true; - if (_observer_prefs_rollback != nullptr) { - memcpy(&_mqtt_prefs, _observer_prefs_rollback, sizeof(_mqtt_prefs)); - } + if (commitObserverPreferenceCandidate(_mqtt_prefs, _observer_candidate, + [&]() { return _callbacks->saveObserverPrefs(); })) return true; // The running node is back on the old value either way, but only claim the // change is gone when the write really was undone on flash. if (_observer_save_indeterminate) { @@ -229,12 +211,13 @@ bool CommonCLI::persistObserverPrefs(char* reply) { bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* config, char* reply) { #ifdef WITH_MQTT_BRIDGE const bool needs_snapshot = isObserverPrefsSetCommand(config); - ObserverPrefsRollbackScope rollback_scope( - &_observer_prefs_rollback, _mqtt_prefs, needs_snapshot); - if (needs_snapshot && !rollback_scope.available()) { + ObserverPrefsCandidateScope candidate_scope( + &_observer_candidate, this->_mqtt_prefs, needs_snapshot); + if (needs_snapshot && !candidate_scope.available()) { strcpy(reply, "Error: insufficient memory to update observer setting"); return true; } + MQTTPrefs& _mqtt_prefs = _observer_candidate ? *_observer_candidate : this->_mqtt_prefs; bool handled = true; if (memcmp(config, "snmp.community ", 15) == 0) { if (valueTooLong(&config[15], sizeof(_mqtt_prefs.snmp_community), reply, "snmp.community")) return true; @@ -946,6 +929,8 @@ bool CommonCLI::handleObserverGetCmd(uint32_t sender_timestamp, const char* conf memcmp(config, "mqtt.neighbors", 14) == 0) { strcpy(reply, "Err - neighbors not enabled in this build"); #endif + } else if (strcmp(config, "mqtt.runtime") == 0) { + MQTTBridge::formatRuntimeReply(reply, 160); } else if (memcmp(config, "mqtt.ntp.diag", 13) == 0 && (config[13] == '\0' || config[13] == ' ')) { #ifdef ESP_PLATFORM // Connectivity probe across all configured NTP servers; never updates the clock. @@ -1218,10 +1203,9 @@ bool CommonCLI::handleObserverCommand(uint32_t sender_timestamp, char* command, snprintf(reply, 160, "ERR: %s not connected", activeNetworkLink().mediumName()); } else if (memcmp(command, "ota check", 9) == 0) { - // Check is synchronous so its result lands in this reply, and runs with the - // MQTT bridge UP: the slim per-variant manifest is tiny, so the fetch only - // costs a single TLS handshake (no large JSON doc) — which fits alongside - // the live MQTT sessions even on no-PSRAM boards. No bridge bounce needed. + // Queues a Core-0 HTTP fetch and returns immediately. Repeat the command + // for the cached result. The MQTT bridge stays up: the slim per-variant + // manifest is tiny enough to fetch alongside live sessions. _board->otaFromManifest(_callbacks->getFirmwareVer(), true, reply); } else { // `ota update`: cheap pre-check first (plain HTTP, bridge stays up). Only @@ -1259,6 +1243,18 @@ bool CommonCLI::handleObserverCommand(uint32_t sender_timestamp, char* command, } else { strcpy(reply, "ERR: online OTA not available"); } + } else if (_board->otaCheckInProgress()) { + // reply holds "OTA check #N queued|running; repeat ...". Keep the ID and + // let the app loop pick the result up instead of asking the operator to. + char head[48]; + const char* tail = strchr(reply, ';'); + const size_t len = tail ? (size_t)(tail - reply) : strlen(reply); + snprintf(head, sizeof(head), "%.*s", (int)(len < sizeof(head) - 1 ? len : sizeof(head) - 1), reply); + if (_callbacks->beginDeferredOtaUpdateAfterCheck()) { + snprintf(reply, 160, "%s; update starts automatically if a newer build applies", head); + } else { + snprintf(reply, 160, "%s; repeat 'ota update' for result", head); + } } } #else diff --git a/src/helpers/ESP32Board.cpp b/src/helpers/ESP32Board.cpp index 19c3e5cc9f..bb2ca64928 100644 --- a/src/helpers/ESP32Board.cpp +++ b/src/helpers/ESP32Board.cpp @@ -266,25 +266,79 @@ static void ota_partitionSignature(char* out, size_t out_sz) { } } -// Parameters handed to the worker task; lives on otaFromManifest()'s stack, +#include "ObserverAsyncJob.h" +#include + +struct OtaCheckResult { + bool applicable = false; + char reply[160]{}; +}; +static ObserverAsyncJob s_ota_check; +static ESP32Board* s_ota_check_board = nullptr; +static char s_ota_check_version[64]{}; + +static void ota_check_entry(void*) { + const uint32_t id = s_ota_check.begin(); + OtaCheckResult result; + NetworkLink& network = activeNetworkLink(); + network.lockSwitching(); + result.applicable = s_ota_check_board->otaFromManifestImpl(s_ota_check_version, true, result.reply); + network.unlockSwitching(); + s_ota_check.complete(id, millis(), result); + vTaskDelete(nullptr); +} + +// Parameters handed to the flash task; lives on otaFromManifest()'s stack, // which stays valid because that function blocks until the worker signals done. struct OtaTaskArgs { ESP32Board* self; const char* current_ver; bool dry_run; char* reply; - volatile bool result; - volatile bool done; + bool result; + std::atomic done{false}; }; static void ota_task_entry(void* param) { OtaTaskArgs* a = static_cast(param); a->result = a->self->otaFromManifestImpl(a->current_ver, a->dry_run, a->reply); - a->done = true; // on a successful `ota update` we reboot before reaching here + a->done.store(true, std::memory_order_release); vTaskDelete(nullptr); } +bool ESP32Board::otaCheckInProgress() const { + using Check = ObserverAsyncJob; + const auto check = s_ota_check.read(); + return check.state == Check::State::Queued || check.state == Check::State::Running; +} + bool ESP32Board::otaFromManifest(const char* current_ver, bool dry_run, char reply[]) { + using Check = ObserverAsyncJob; + const auto check = s_ota_check.read(); + if (check.state == Check::State::Queued || check.state == Check::State::Running) { + snprintf(reply, 160, "OTA check #%lu running; repeat the command for result", (unsigned long)check.id); + return false; + } + if (dry_run) { + if (check.state == Check::State::Complete && + (uint32_t)(millis() - check.finished_ms) < 60000 && + strcmp(s_ota_check_version, current_ver) == 0) { + // Preserve the manifest reply prefix used by the deferred update parser. + snprintf(reply, 160, "%s [check #%lu age:%lus]", check.result.reply, + (unsigned long)check.id, (unsigned long)((uint32_t)(millis() - check.finished_ms) / 1000)); + return check.result.applicable; + } + s_ota_check_board = this; + snprintf(s_ota_check_version, sizeof(s_ota_check_version), "%s", current_ver); + s_ota_check.request(millis()); + if (xTaskCreatePinnedToCore(ota_check_entry, "ota-check", 24576, nullptr, 1, nullptr, 0) != pdPASS) { + s_ota_check.cancel(millis()); + strcpy(reply, "ERR: OTA check task spawn failed"); + return false; + } + snprintf(reply, 160, "OTA check #%lu queued; repeat the command for result", (unsigned long)s_ota_check.read().id); + return false; + } // The TLS handshake (cert-bundle verify) + JSON parse / HTTPUpdate use far more // stack than the ~8 KB loop task offers — especially when reached via the deep // mesh-receive call chain (it overflows the loopTask canary). Run the work in a @@ -292,7 +346,12 @@ bool ESP32Board::otaFromManifest(const char* current_ver, bool dry_run, char rep // freed when the task exits; on a successful update the chip reboots inside it. NetworkLink& network = activeNetworkLink(); network.lockSwitching(); - OtaTaskArgs args = { this, current_ver, dry_run, reply, false, false }; + OtaTaskArgs args; + args.self = this; + args.current_ver = current_ver; + args.dry_run = dry_run; + args.reply = reply; + args.result = false; TaskHandle_t handle = nullptr; BaseType_t ok = xTaskCreatePinnedToCore(ota_task_entry, "ota", 24576, &args, 5, &handle, 1); if (ok != pdPASS) { @@ -300,7 +359,7 @@ bool ESP32Board::otaFromManifest(const char* current_ver, bool dry_run, char rep strcpy(reply, "ERR: OTA task spawn failed"); return false; } - while (!args.done) { + while (!args.done.load(std::memory_order_acquire)) { delay(50); // Arduino delay() yields to other tasks } network.unlockSwitching(); @@ -534,6 +593,7 @@ bool ESP32Board::otaFromManifest(const char* current_ver, bool dry_run, char rep strcpy(reply, "ERR: not supported"); return false; } +bool ESP32Board::otaCheckInProgress() const { return false; } #endif // WITH_MQTT_BRIDGE void ESP32Board::powerOff() { diff --git a/src/helpers/ESP32Board.h b/src/helpers/ESP32Board.h index 02d0ac4bfd..f601bc8cda 100644 --- a/src/helpers/ESP32Board.h +++ b/src/helpers/ESP32Board.h @@ -162,6 +162,7 @@ class ESP32Board : public mesh::MainBoard { bool isOTAUpdateInProgress() const override; void maintainOTAUpdate(uint32_t now_ms) override; bool otaFromManifest(const char* current_ver, bool dry_run, char reply[]) override; + bool otaCheckInProgress() const override; // Heavy body (TLS + JSON / HTTPUpdate). Runs in a dedicated large-stack task // spawned by otaFromManifest() — public only so that task entry point can call // it; not meant to be invoked directly. diff --git a/src/helpers/ESP32WsTransportFix.cpp b/src/helpers/ESP32WsTransportFix.cpp index a69e0b0885..2febce0680 100644 --- a/src/helpers/ESP32WsTransportFix.cpp +++ b/src/helpers/ESP32WsTransportFix.cpp @@ -62,7 +62,12 @@ #if ESP_IDF_VERSION_MAJOR == 4 +#if ESP_IDF_VERSION_MINOR != 4 || ESP_IDF_VERSION_PATCH != 7 +#error "Revalidate the private WebSocket layout before using another IDF 4 SDK" +#endif + #include +#include "RequiredTransportBuffer.h" #include "sdkconfig.h" #include "esp_transport.h" #include "esp_transport_ws.h" @@ -97,22 +102,15 @@ extern "C" { esp_transport_handle_t __real_esp_transport_ws_init(esp_transport_handle_t parent_handle); esp_transport_handle_t __wrap_esp_transport_ws_init(esp_transport_handle_t parent_handle) { - esp_transport_handle_t t = __real_esp_transport_ws_init(parent_handle); - if (t != nullptr) { - transport_ws_t* ws = (transport_ws_t*)esp_transport_get_context_data(t); - if (ws != nullptr && ws->buffer != nullptr) { - // The buffer is untouched at this point (allocated moments ago inside - // __real_esp_transport_ws_init), so a swap is safe. - char* padded = (char*)malloc(CONFIG_WS_BUFFER_SIZE + 1); - if (padded != nullptr) { + return initWithRequiredTransportBuffer( + CONFIG_WS_BUFFER_SIZE + 1, malloc, + [&]() { return __real_esp_transport_ws_init(parent_handle); }, + [](esp_transport_handle_t transport, void* buffer) { + // Successful init in the pinned SDK guarantees this context and buffer. + transport_ws_t* ws = (transport_ws_t*)esp_transport_get_context_data(transport); free(ws->buffer); - ws->buffer = padded; - } - // On alloc failure keep the original buffer: same behavior as before - // this fix, which is still strictly better than failing init here. - } - } - return t; + ws->buffer = static_cast(buffer); + }, free); } } // extern "C" diff --git a/src/helpers/MQTTEventChannel.h b/src/helpers/MQTTEventChannel.h new file mode 100644 index 0000000000..b06aae1285 --- /dev/null +++ b/src/helpers/MQTTEventChannel.h @@ -0,0 +1,70 @@ +#pragma once +#include +#include +#include "ObserverMailbox.h" + +struct MqttSlotEvent { + enum class Kind : uint8_t { Connected, Disconnected, Error }; + Kind kind = Kind::Disconnected; + uint8_t slot = 0; + uint32_t incarnation = 0; + uint32_t at_ms = 0; + int32_t tls = 0; + int32_t tls_stack = 0; + int32_t socket = 0; + uint8_t connack = 0; +}; + +// SDK tasks publish values; only the bridge worker consumes them. An overflow +// latches the slot even if the lost event was DISCONNECTED. +template +class MqttEventChannel { + public: + bool push(const MqttSlotEvent& event) { + ObserverGuard guard(_lock); + if (_size == Capacity) { + _overflow_slots |= uint32_t(1) << event.slot; + ++_overflows; + return false; + } + _events[(_head + _size) % Capacity] = event; + ++_size; + return true; + } + bool pop(MqttSlotEvent& event) { + ObserverGuard guard(_lock); + if (!_size) return false; + event = _events[_head]; + _head = (_head + 1) % Capacity; + --_size; + return true; + } + uint32_t takeOverflowSlots() { + ObserverGuard guard(_lock); + uint32_t slots = _overflow_slots; + _overflow_slots = 0; + return slots; + } + uint32_t overflows() const { + ObserverGuard guard(_lock); + return _overflows; + } + // Only after the SDK task has joined or acknowledged its disconnect. + void discardSlot(uint8_t slot) { + ObserverGuard guard(_lock); + size_t kept = 0; + for (size_t i = 0; i < _size; ++i) { + const MqttSlotEvent event = _events[(_head + i) % Capacity]; + if (event.slot != slot) _events[(_head + kept++) % Capacity] = event; + } + _size = kept; + _overflow_slots &= ~(uint32_t(1) << slot); + } + private: + mutable ObserverLock _lock; + MqttSlotEvent _events[Capacity]{}; + size_t _head = 0; + size_t _size = 0; + uint32_t _overflow_slots = 0; + uint32_t _overflows = 0; +}; diff --git a/src/helpers/MQTTSlotEvents.h b/src/helpers/MQTTSlotEvents.h new file mode 100644 index 0000000000..4948c93831 --- /dev/null +++ b/src/helpers/MQTTSlotEvents.h @@ -0,0 +1,62 @@ +#pragma once +#include "MQTTClientState.h" +#include "MQTTEventChannel.h" + +// The production transition, independent of SDK handles. Called only by the +// worker; callbacks supply records through MqttEventChannel. +template +bool applyMqttSlotEvent(Slot& slot, const MqttSlotEvent& event, + bool& force_jwt_mint, bool& publish_status) { + if (event.incarnation != slot.incarnation || !mqttClientStateIsLive(slot.client_state)) return false; + switch (event.kind) { + case MqttSlotEvent::Kind::Connected: + if (!slot.enabled || (slot.client_state != MqttClientState::Starting && + slot.client_state != MqttClientState::Disconnected)) return false; + slot.client_state = MqttClientState::Connected; + slot.connected = true; + slot.connected_at_ms = event.at_ms; + // A CONNACK does not reset the backoff: maintenance waits for stability. + slot.circuit_breaker_tripped = false; + slot.last_tls_err = 0; + slot.last_tls_stack_err = 0; + slot.last_sock_errno = 0; + slot.last_connack_code = 0; + slot.last_error_time = 0; + slot.current_outage_started_ms = 0; + force_jwt_mint = false; + publish_status = true; + break; + case MqttSlotEvent::Kind::Disconnected: + slot.client_state = MqttClientState::Disconnected; + ++slot.disconnect_count; + if (!slot.first_disconnect_time) slot.first_disconnect_time = event.at_ms; + if (!slot.current_outage_started_ms) slot.current_outage_started_ms = event.at_ms; + slot.connected = false; + slot.connected_at_ms = 0; + publish_status = false; + break; + case MqttSlotEvent::Kind::Error: + slot.last_tls_err = event.tls; + slot.last_tls_stack_err = event.tls_stack; + slot.last_sock_errno = event.socket; + slot.last_connack_code = event.connack; + slot.last_error_time = event.at_ms; + if (event.connack) force_jwt_mint = true; + break; + } + return true; +} + +template +int startMqttSlotAttempt(Slot& slot, Start start) { + const MqttClientState previous = slot.client_state; + slot.client_state = MqttClientState::Starting; + const int result = start(); + if (result == 0) { + ++slot.generation; + slot.applied_token_expires_at = slot.token_expires_at; + } else { + slot.client_state = previous; + } + return result; +} diff --git a/src/helpers/NetworkLink.cpp b/src/helpers/NetworkLink.cpp index ba87b35a50..3b4be87c6d 100644 --- a/src/helpers/NetworkLink.cpp +++ b/src/helpers/NetworkLink.cpp @@ -156,7 +156,7 @@ class WiFiNetworkLink final : public NetworkLinkBase { char _ssid[33] = {}; char _password[65] = {}; unsigned long _last_reconnect_attempt = 0; - uint8_t _reconnect_backoff_attempt = 0; + std::atomic _reconnect_backoff_attempt{0}; // One mapping for startup, reconnect and CLI (see WifiPowerSavePolicy). The // stored default is `none`; `min` means MIN_MODEM here exactly as the CLI @@ -520,6 +520,7 @@ class AutomaticNetworkLink final : public NetworkLink { std::atomic _ethernet_started{false}; std::atomic _wifi_started{false}; char _wifi_ssid[33] = {}; + std::atomic _wifi_configured{false}; char _wifi_password[65] = {}; std::atomic _ethernet_stable_since{0}; std::atomic _selected_down_since{0}; @@ -547,13 +548,14 @@ class AutomaticNetworkLink final : public NetworkLink { : static_cast(_wifi); } - bool wifiConfigured() const { return _wifi_ssid[0] != '\0'; } + bool wifiConfigured() const { return _wifi_configured.load(std::memory_order_acquire); } void rememberWifi(const char* ssid, const char* password) { strncpy(_wifi_ssid, ssid ? ssid : "", sizeof(_wifi_ssid) - 1); _wifi_ssid[sizeof(_wifi_ssid) - 1] = '\0'; strncpy(_wifi_password, password ? password : "", sizeof(_wifi_password) - 1); _wifi_password[sizeof(_wifi_password) - 1] = '\0'; + _wifi_configured.store(_wifi_ssid[0] != '\0', std::memory_order_release); } void startWifiFallback() { diff --git a/src/helpers/NtpSchedule.h b/src/helpers/NtpSchedule.h new file mode 100644 index 0000000000..699ed528c4 --- /dev/null +++ b/src/helpers/NtpSchedule.h @@ -0,0 +1,66 @@ +#pragma once + +#include + +// Owned by the network worker. Completion, rather than a pre-probe loop sample, +// arms the next deadline so slow or failed probes cannot trigger a refresh storm. +class NtpSchedule { +public: + enum class Result : uint8_t { NetworkTime, Holdover, Failed }; + + bool due(uint32_t now) { + if (!_active && (!_scheduled || (int32_t)(now - _next_attempt_ms) >= 0)) { + _ready = true; + } + return !_active && _ready; + } + + bool begin(uint32_t now, bool force = false) { + if (_active || (!force && !due(now))) return false; + _active = true; + _ready = false; + _last_attempt_ms = now; + return true; + } + + void complete(uint32_t now, Result result) { + if (!_active) return; + _active = false; + _scheduled = true; + _result = result; + uint32_t interval; + if (result == Result::NetworkTime) { + _has_network_time = true; + _last_success_ms = now; + _failures = 0; + interval = 3600000; + } else { + // A usable RTC keeps MQTT available, but is not a successful NTP probe. + if (_failures < 5) ++_failures; + interval = result == Result::Holdover ? 300000 : retryDelay(_failures); + } + _next_attempt_ms = now + interval; + } + + bool active() const { return _active; } + bool hasNetworkTime() const { return _has_network_time; } + uint32_t lastAttemptMs() const { return _last_attempt_ms; } + uint32_t lastSuccessMs() const { return _last_success_ms; } + Result result() const { return _result; } + +private: + static uint32_t retryDelay(uint8_t failures) { + const uint32_t delays[] = {30000, 60000, 120000, 240000, 300000}; + return delays[failures ? failures - 1 : 0]; + } + + bool _active = false; + bool _ready = true; + bool _scheduled = false; + bool _has_network_time = false; + uint8_t _failures = 0; + uint32_t _last_attempt_ms = 0; + uint32_t _last_success_ms = 0; + uint32_t _next_attempt_ms = 0; + Result _result = Result::Failed; +}; diff --git a/src/helpers/ObserverAsyncJob.h b/src/helpers/ObserverAsyncJob.h new file mode 100644 index 0000000000..0b66cd604b --- /dev/null +++ b/src/helpers/ObserverAsyncJob.h @@ -0,0 +1,50 @@ +#pragma once +#include +#include "ObserverMailbox.h" + +template +class ObserverAsyncJob { + public: + enum class State : uint8_t { Idle, Queued, Running, Complete, Cancelled }; + struct Snapshot { + State state = State::Idle; + uint32_t id = 0, started_ms = 0, finished_ms = 0; + Result result{}; + }; + bool request(uint32_t now) { + ObserverGuard guard(_lock); + if (_value.state == State::Queued || _value.state == State::Running) return false; + if (++_value.id == 0) ++_value.id; + _value.started_ms = now; + _value.finished_ms = 0; + _value.result = Result{}; + _value.state = State::Queued; + return true; + } + uint32_t begin() { + ObserverGuard guard(_lock); + if (_value.state != State::Queued) return 0; + _value.state = State::Running; + return _value.id; + } + void complete(uint32_t id, uint32_t now, const Result& result) { + ObserverGuard guard(_lock); + if (_value.id != id || _value.state != State::Running) return; + _value.result = result; + _value.finished_ms = now; + _value.state = State::Complete; + } + void cancel(uint32_t now) { + ObserverGuard guard(_lock); + if (_value.state != State::Queued && _value.state != State::Running) return; + _value.finished_ms = now; + _value.state = State::Cancelled; + } + Snapshot read() const { + ObserverGuard guard(_lock); + return _value; + } + private: + mutable ObserverLock _lock; + Snapshot _value{}; +}; diff --git a/src/helpers/ObserverConfigMailbox.h b/src/helpers/ObserverConfigMailbox.h new file mode 100644 index 0000000000..dd4ba5f832 --- /dev/null +++ b/src/helpers/ObserverConfigMailbox.h @@ -0,0 +1,39 @@ +#pragma once +#include +#include +#include "ObserverMailbox.h" + +// One loop-task publisher, one worker consumer. Reconfigure requests coalesce +// with the complete configuration that caused them, never in separate flags. +template +class ObserverConfigMailbox { + public: + void publish(const Prefs& prefs, const Metadata& metadata, uint32_t reconfigure = 0) { + ObserverGuard guard(_lock); + if (_revision && !reconfigure && memcmp(&_prefs, &prefs, sizeof(prefs)) == 0 && + memcmp(&_metadata, &metadata, sizeof(metadata)) == 0) return; + _prefs = prefs; + _metadata = metadata; + _reconfigure |= reconfigure; + if (++_revision == 0) ++_revision; + } + bool consume(Prefs& prefs, Metadata& metadata, uint32_t& revision, uint32_t& reconfigure) { + ObserverGuard guard(_lock); + if (!_revision || revision == _revision) return false; + prefs = _prefs; + metadata = _metadata; + revision = _revision; + reconfigure = _reconfigure; + _reconfigure = 0; + return true; + } + uint32_t revision() const { + ObserverGuard guard(_lock); + return _revision; + } + private: + mutable ObserverLock _lock; + Prefs _prefs{}; + Metadata _metadata{}; + uint32_t _revision = 0, _reconfigure = 0; +}; diff --git a/src/helpers/ObserverMailbox.h b/src/helpers/ObserverMailbox.h new file mode 100644 index 0000000000..b64f60826f --- /dev/null +++ b/src/helpers/ObserverMailbox.h @@ -0,0 +1,74 @@ +#pragma once + +#include +#ifdef ESP_PLATFORM +#include +#include +#else +#include +#endif + +// Task-context lock for bounded value copies. Every holder is a FreeRTOS task +// (loop, bridge worker, SDK event task, AsyncTCP), never an ISR, so a mutex is +// the right primitive: a critical section would disable interrupts on the radio +// core for the whole copy, and some of these records are kilobytes. Priority +// inheritance covers a preempted holder. Never hold this across allocation, +// I/O, flash writes or SDK calls. +class ObserverLock { + public: + ObserverLock() { +#ifdef ESP_PLATFORM + _mux = xSemaphoreCreateMutexStatic(&_storage); +#endif + } + ObserverLock(const ObserverLock&) = delete; + ObserverLock& operator=(const ObserverLock&) = delete; + void lock() { +#ifdef ESP_PLATFORM + xSemaphoreTake(_mux, portMAX_DELAY); +#else + _mux.lock(); +#endif + } + void unlock() { +#ifdef ESP_PLATFORM + xSemaphoreGive(_mux); +#else + _mux.unlock(); +#endif + } + private: +#ifdef ESP_PLATFORM + StaticSemaphore_t _storage; + SemaphoreHandle_t _mux; +#else + std::mutex _mux; +#endif +}; + +class ObserverGuard { + public: + explicit ObserverGuard(ObserverLock& lock) : _lock(lock) { _lock.lock(); } + ~ObserverGuard() { _lock.unlock(); } + ObserverGuard(const ObserverGuard&) = delete; + ObserverGuard& operator=(const ObserverGuard&) = delete; + private: + ObserverLock& _lock; +}; + +template +class ObserverMailbox { + static_assert(std::is_trivially_copyable::value, "mailbox requires value records"); + public: + void publish(const T& value) { + ObserverGuard guard(_lock); + _value = value; + } + T read() const { + ObserverGuard guard(_lock); + return _value; + } + private: + mutable ObserverLock _lock; + T _value{}; +}; diff --git a/src/helpers/ObserverPreferenceCommit.h b/src/helpers/ObserverPreferenceCommit.h new file mode 100644 index 0000000000..adf13db14c --- /dev/null +++ b/src/helpers/ObserverPreferenceCommit.h @@ -0,0 +1,10 @@ +#pragma once + +// save() serializes the candidate. Publishing it to loop-owned durable RAM is +// the last step; a failed or indeterminate save leaves that image untouched. +template +bool commitObserverPreferenceCandidate(Prefs& durable, const Prefs* candidate, Save save) { + if (!save()) return false; + if (candidate) durable = *candidate; + return true; +} diff --git a/src/helpers/ObserverSlotStatsJson.h b/src/helpers/ObserverSlotStatsJson.h new file mode 100644 index 0000000000..9b3bf054c5 --- /dev/null +++ b/src/helpers/ObserverSlotStatsJson.h @@ -0,0 +1,36 @@ +#pragma once +#include +#include +#include +#include "MQTTPacketFilter.h" + +// Append complete slot objects to an existing JSON prefix ending in '['. +// Names/states come from firmware tables, never unescaped user strings. +template +void appendObserverSlotStatsJson(char* buf, size_t size, int pos, const Slot* slots, size_t count) { + if (!buf || !size) return; + if (pos < 0 || static_cast(pos) + 3 > size) { + snprintf(buf, size, "{}"); + return; + } + bool first = true; + for (size_t i = 0; i < count; ++i) { + const Slot& slot = slots[i]; + if (!slot.configured) continue; + char filter[24] = ""; + if (slot.filter_mask != MQTTPacketFilter::kAllPacketTypes) { + snprintf(filter, sizeof(filter), ",\"filt\":%u", (unsigned)slot.filter_mask); + } + char entry[192]; + const int n = snprintf(entry, sizeof(entry), + "%s{\"n\":%u,\"name\":\"%s\",\"state\":\"%s\",\"ok\":%lu,\"err\":%lu%s}", + first ? "" : ",", (unsigned)i + 1, slot.name, slot.state, + slot.publish_ok, slot.publish_err, filter); + if (n < 0 || static_cast(n) >= sizeof(entry) || + static_cast(pos) + static_cast(n) + 3 > size) break; + memcpy(buf + pos, entry, n); + pos += n; + first = false; + } + snprintf(buf + pos, size - pos, "]}"); +} diff --git a/src/helpers/OtaUpdateFollowUp.h b/src/helpers/OtaUpdateFollowUp.h new file mode 100644 index 0000000000..f4d4ae137e --- /dev/null +++ b/src/helpers/OtaUpdateFollowUp.h @@ -0,0 +1,42 @@ +#pragma once +#include + +// `ota update` was issued while the manifest check was still queued or running. +// The app loop polls the board until that check settles, then either arms the +// ordinary deferred flash or reports why it will not run. Bounded so a check +// that never completes cannot leave an update armed for the rest of the boot. +class OtaUpdateFollowUp { + public: + enum class Action : uint8_t { None, Start, Refuse, Timeout }; + enum class Check : uint8_t { Applicable, NotApplicable, Requeued }; + static constexpr uint32_t kTimeoutMs = 120000; + + bool armed() const { return _armed; } + void arm(uint32_t now) { + _armed = true; + _deadline = now + kTimeoutMs; + } + void clear() { _armed = false; } + + // dry_run runs only once the board reports no check in progress. Requeued + // means the cached result had already expired and the board started another + // check; stay armed and wait for that one. + template + Action poll(uint32_t now, bool check_in_progress, DryRun dry_run) { + if (!_armed) return Action::None; + if ((int32_t)(now - _deadline) >= 0) { + _armed = false; + return Action::Timeout; + } + if (check_in_progress) return Action::None; + switch (dry_run()) { + case Check::Applicable: _armed = false; return Action::Start; + case Check::Requeued: return Action::None; + default: _armed = false; return Action::Refuse; + } + } + + private: + bool _armed = false; + uint32_t _deadline = 0; +}; diff --git a/src/helpers/RequiredTransportBuffer.h b/src/helpers/RequiredTransportBuffer.h new file mode 100644 index 0000000000..dba5df7e51 --- /dev/null +++ b/src/helpers/RequiredTransportBuffer.h @@ -0,0 +1,18 @@ +#pragma once +#include + +// Allocate the protection buffer first. A successful init must install it; +// neither allocation failure is allowed to return an unprotected transport. +template +Handle initWithRequiredTransportBuffer(size_t size, Allocate allocate, Init init, + Install install, Free release) { + void* buffer = allocate(size); + if (!buffer) return nullptr; + Handle transport = init(); + if (!transport) { + release(buffer); + return nullptr; + } + install(transport, buffer); + return transport; +} diff --git a/src/helpers/SNMPAgent.cpp b/src/helpers/SNMPAgent.cpp index eb29bd688b..5a052d58c1 100644 --- a/src/helpers/SNMPAgent.cpp +++ b/src/helpers/SNMPAgent.cpp @@ -69,6 +69,22 @@ void MeshSNMPAgent::begin(const char* community) { void MeshSNMPAgent::loop() { if (!_running) return; + const RadioSnapshot snapshot = _radio_snapshot.read(); + _packets_recv = snapshot.packets_recv; + _packets_sent = snapshot.packets_sent; + _recv_errors = snapshot.recv_errors; + _noise_floor = snapshot.noise_floor; + _last_rssi = snapshot.last_rssi; + _last_snr = snapshot.last_snr; + _sent_flood = snapshot.sent_flood; + _sent_direct = snapshot.sent_direct; + _recv_flood = snapshot.recv_flood; + _recv_direct = snapshot.recv_direct; + _total_air_time_secs = snapshot.total_air_time_secs; + _uptime_secs = snapshot.uptime_secs; + memcpy(_node_name, snapshot.node_name, sizeof(_node_name)); + memcpy(_firmware_version, snapshot.firmware_version, sizeof(_firmware_version)); + // Update memory and selected-network stats locally on Core 0. _free_heap = (int)ESP.getFreeHeap(); _max_alloc = (int)ESP.getMaxAllocHeap(); @@ -91,18 +107,19 @@ void MeshSNMPAgent::updateRadioStats( uint32_t sent_flood, uint32_t sent_direct, uint32_t recv_flood, uint32_t recv_direct, uint32_t total_air_time_secs, uint32_t uptime_secs) { - _packets_recv = (int)packets_recv; - _packets_sent = (int)packets_sent; - _recv_errors = (int)recv_errors; - _noise_floor = (int)noise_floor; - _last_rssi = (int)last_rssi; - _last_snr = (int)last_snr; - _sent_flood = (int)sent_flood; - _sent_direct = (int)sent_direct; - _recv_flood = (int)recv_flood; - _recv_direct = (int)recv_direct; - _total_air_time_secs = (int)total_air_time_secs; - _uptime_secs = (int)uptime_secs; + _radio_pending.packets_recv = (int)packets_recv; + _radio_pending.packets_sent = (int)packets_sent; + _radio_pending.recv_errors = (int)recv_errors; + _radio_pending.noise_floor = (int)noise_floor; + _radio_pending.last_rssi = (int)last_rssi; + _radio_pending.last_snr = (int)last_snr; + _radio_pending.sent_flood = (int)sent_flood; + _radio_pending.sent_direct = (int)sent_direct; + _radio_pending.recv_flood = (int)recv_flood; + _radio_pending.recv_direct = (int)recv_direct; + _radio_pending.total_air_time_secs = (int)total_air_time_secs; + _radio_pending.uptime_secs = (int)uptime_secs; + _radio_snapshot.publish(_radio_pending); } void MeshSNMPAgent::updateMQTTStats(int connected_slots, int queue_depth, int skipped_publishes) { @@ -112,13 +129,15 @@ void MeshSNMPAgent::updateMQTTStats(int connected_slots, int queue_depth, int sk } void MeshSNMPAgent::setNodeName(const char* name) { - strncpy(_node_name, name, sizeof(_node_name) - 1); - _node_name[sizeof(_node_name) - 1] = '\0'; + strncpy(_radio_pending.node_name, name, sizeof(_radio_pending.node_name) - 1); + _radio_pending.node_name[sizeof(_radio_pending.node_name) - 1] = '\0'; + _radio_snapshot.publish(_radio_pending); } void MeshSNMPAgent::setFirmwareVersion(const char* version) { - strncpy(_firmware_version, version, sizeof(_firmware_version) - 1); - _firmware_version[sizeof(_firmware_version) - 1] = '\0'; + strncpy(_radio_pending.firmware_version, version, sizeof(_radio_pending.firmware_version) - 1); + _radio_pending.firmware_version[sizeof(_radio_pending.firmware_version) - 1] = '\0'; + _radio_snapshot.publish(_radio_pending); } #endif // WITH_SNMP diff --git a/src/helpers/SNMPAgent.h b/src/helpers/SNMPAgent.h index fe94b10fe1..9465ba57fb 100644 --- a/src/helpers/SNMPAgent.h +++ b/src/helpers/SNMPAgent.h @@ -3,6 +3,7 @@ #ifdef WITH_SNMP #include +#include "ObserverMailbox.h" #include // Temporary private enterprise OID base — replace with registered PEN when available. @@ -22,8 +23,7 @@ class MeshSNMPAgent { void begin(const char* community); void loop(); - // Called from the mesh task (Core 1) to push fresh stats into SNMP-visible variables. - // Copies are atomic for 32-bit aligned ints on ESP32, so no mutex needed. + // Loop-task producers publish a coherent record; the SNMP worker owns OID storage. void updateRadioStats(uint32_t packets_recv, uint32_t packets_sent, uint32_t recv_errors, int16_t noise_floor, int16_t last_rssi, int16_t last_snr, uint32_t sent_flood, uint32_t sent_direct, @@ -38,6 +38,23 @@ class MeshSNMPAgent { bool isRunning() const { return _running; } private: + struct RadioSnapshot { + int packets_recv = 0; + int packets_sent = 0; + int recv_errors = 0; + int noise_floor = 0; + int last_rssi = 0; + int last_snr = 0; + int sent_flood = 0; + int sent_direct = 0; + int recv_flood = 0; + int recv_direct = 0; + int total_air_time_secs = 0; + int uptime_secs = 0; + char node_name[32]{}; + char firmware_version[32]{}; + } _radio_pending; + ObserverMailbox _radio_snapshot; WiFiUDP _udp; SNMPAgent _snmp; bool _running; diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 29edc89e75..68af108a7c 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -1,4 +1,6 @@ #include "MQTTBridge.h" +#include "helpers/MQTTSlotEvents.h" +#include "helpers/ObserverSlotStatsJson.h" #include "MQTTErrorLabels.h" #include "../WifiPowerSavePolicy.h" #include "../MQTTConnectionPolicy.h" @@ -34,10 +36,10 @@ #endif // Effective MQTT origin: empty mqtt_origin follows node_name; otherwise mqtt_origin override (quotes stripped). -static void applyEffectiveOrigin(const NodePrefs* np, const MQTTPrefs* obs, char* dest, size_t dest_size) { - if (!np || !obs || !dest || dest_size == 0) return; +static void applyEffectiveOrigin(const char* node_name, const MQTTPrefs* obs, char* dest, size_t dest_size) { + if (!node_name || !obs || !dest || dest_size == 0) return; if (obs->mqtt_origin[0] == '\0') { - strncpy(dest, np->node_name, dest_size - 1); + strncpy(dest, node_name, dest_size - 1); } else { strncpy(dest, obs->mqtt_origin, dest_size - 1); } @@ -94,8 +96,7 @@ const char* MQTTBridge::effectiveNtpPrimary(const MQTTPrefs* obs) { } void MQTTBridge::refreshOriginFromPrefs() { - if (!_prefs) return; - applyEffectiveOrigin(_prefs, _obs, _origin, sizeof(_origin)); + applyEffectiveOrigin(_radio_metadata.node_name, _obs, _origin, sizeof(_origin)); } void MQTTBridge::getEffectiveMqttOrigin(const NodePrefs* np, const MQTTPrefs* obs, char* buf, size_t buf_size) { @@ -104,7 +105,7 @@ void MQTTBridge::getEffectiveMqttOrigin(const NodePrefs* np, const MQTTPrefs* ob buf[0] = '\0'; return; } - applyEffectiveOrigin(np, obs, buf, buf_size); + applyEffectiveOrigin(np->node_name, obs, buf, buf_size); } static bool isNetworkConfigValid(const MQTTPrefs* obs) { @@ -209,10 +210,7 @@ void* MQTTBridge::JsonScratchAllocator::reallocate(void* ptr, size_t new_size) { return psram_realloc(ptr, new_size); } -// Latched by end() when a stop passes its deadline unacknowledged. end() clears -// the diagnostic singleton, so without this `get mqtt.status` could only say -// "not running" and an operator would have no way to learn that the bridge is -// down for the rest of the boot and why. +// Loop-task latch explaining why a stopped bridge cannot yet restart. static bool s_stop_unproven = false; #ifdef MQTT_MEMORY_DEBUG @@ -230,8 +228,65 @@ static void agentLogHeap(const char* location, const char* message, const char* // #endregion #endif -// Singleton for formatMqttStatusReply (set in begin(), cleared in end()) -static MQTTBridge* s_mqtt_bridge_instance = nullptr; +ObserverMailbox MQTTBridge::_published; + +void MQTTBridge::publishRuntimeSnapshot() { + RuntimeSnapshot snapshot; + snapshot.sampled_ms = millis(); + snapshot.sequence = ++_snapshot_sequence; + snapshot.desired_revision = _configuration.revision(); + snapshot.applied_revision = _applied_revision; + snapshot.running = !_stop_requested.load(std::memory_order_acquire); + snapshot.queue = getQueueSize(); + snapshot.skipped = _skipped_publishes; + snapshot.filtered = _filtered_packets.load(std::memory_order_relaxed); + snapshot.event_overflows = _slot_events.overflows(); + if (!_heap_sampled || (uint32_t)(snapshot.sampled_ms - _heap_sample_ms) >= 1000) { + _heap_sampled = true; + _heap_sample_ms = snapshot.sampled_ms; + _heap_sample_free = ESP.getFreeHeap(); + _heap_sample_max = ESP.getMaxAllocHeap(); + } + snapshot.free_heap = _heap_sample_free; + snapshot.max_heap = _heap_sample_max; +#if defined(WITH_MQTT_NEIGHBORS) + snapshot.neighbors_phase = _neighbors_phase.load(std::memory_order_relaxed); + snapshot.neighbors_result = _neighbors_last_result.load(std::memory_order_relaxed); + snapshot.neighbors_seconds = _neighbors_secs_until_next.load(std::memory_order_relaxed); +#endif + for (int i = 0; i < RUNTIME_MQTT_SLOTS; ++i) { + const MQTTSlot& slot = _slots[i]; + SlotRuntimeSnapshot& out = snapshot.slots[i]; + out.configured = slot.enabled || slot.preset; + out.enabled = slot.enabled; + out.client = slot.client != nullptr; + out.connected = slot.connected; + out.attempted = slot.initial_connect_done; + out.ready = isSlotReady(i); + out.circuit_breaker_tripped = slot.circuit_breaker_tripped; + out.client_state = slot.client_state; + snprintf(out.name, sizeof(out.name), "%s", slot.preset ? slot.preset->name : slot.enabled ? "custom" : "none"); + snprintf(out.state, sizeof(out.state), "%s", !slot.enabled ? "inactive" : !out.ready ? "wait" : + slot.connected ? "ok" : slot.circuit_breaker_tripped ? "fail" : "disc"); + out.filter_mask = _obs->mqtt_slot_packet_filter[i]; + out.disconnect_count = slot.disconnect_count; + out.first_disconnect_time = slot.first_disconnect_time; + out.current_outage_started_ms = slot.current_outage_started_ms; + out.last_error_time = slot.last_error_time; + out.last_tls_err = slot.last_tls_err; + out.last_tls_stack_err = slot.last_tls_stack_err; + out.last_sock_errno = slot.last_sock_errno; + out.last_connack_code = slot.last_connack_code; + if (slot.enabled && slot.connected) ++snapshot.connected; + if (slot.client && slot.client_state != ClientState::Quarantined) { + out.publish_ok = slot.client->getPublishOk(); + out.publish_err = slot.client->getPublishErr(); + snapshot.outbox += slot.client->getOutboxSize(); + snapshot.outbox_drops += slot.client->getOutboxDrops(); + } + } + _published.publish(snapshot); +} unsigned long MQTTBridge::getWifiConnectedAtMillis() { return activeNetworkLink().connectedAtMillis(); @@ -256,52 +311,25 @@ static void formatDuration(char* buf, size_t len, uint32_t secs) { void MQTTBridge::formatMqttStatusReply(char* buf, size_t bufsize, const MQTTPrefs* obs) { if (buf == nullptr || bufsize == 0) return; const char* msgs = (obs && obs->mqtt_status_enabled) ? "on" : "off"; - if (s_mqtt_bridge_instance == nullptr || !s_mqtt_bridge_instance->_initialized) { + const RuntimeSnapshot snapshot = _published.read(); + if (!snapshot.running) { snprintf(buf, bufsize, "> msgs: %s (bridge %s)", msgs, s_stop_unproven ? "stopped: waiting for the previous stop to finish; reboot if it persists" : "not running"); return; } - MQTTBridge* b = s_mqtt_bridge_instance; - // Build per-slot status strings (compact format to fit 160-byte reply buffer) - // Only show configured slots, skip "none" slots - int q = 0; -#ifdef ESP_PLATFORM - if (b->_packet_queue_handle != nullptr) { - q = (int)uxQueueMessagesWaiting(b->_packet_queue_handle); - } -#else - q = b->_queue_count; -#endif + const int q = snapshot.queue; // replyAppendf clamps pos into the buffer on every call, so no per-append // guard or trailing clamp is needed (see MQTTReplyFormat.h / A1). int pos = 0; replyAppendf(buf, bufsize, &pos, "> msgs: %s", msgs); for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - const MQTTSlot& slot = b->_slots[i]; - const char* name = nullptr; - const char* state = nullptr; - - if (!slot.enabled && slot.preset) { - name = slot.preset->name; - state = "inactive"; - } else if (!slot.enabled) { - continue; // Skip unconfigured slots - } else if (!b->isSlotReady(i)) { - name = slot.preset ? slot.preset->name : "custom"; - state = "wait"; - } else if (slot.connected) { - name = slot.preset ? slot.preset->name : "custom"; - state = "ok"; - } else if (slot.circuit_breaker_tripped) { - name = slot.preset ? slot.preset->name : "custom"; - state = "fail"; - } else { - name = slot.preset ? slot.preset->name : "custom"; - state = "disc"; - } + const SlotRuntimeSnapshot& slot = snapshot.slots[i]; + if (!slot.configured) continue; + const char* name = slot.name; + const char* state = slot.state; replyAppendf(buf, bufsize, &pos, ", %d: %s (%s)", i + 1, name, state); } replyAppendf(buf, bufsize, &pos, ", q:%d", q); @@ -310,16 +338,16 @@ void MQTTBridge::formatMqttStatusReply(char* buf, size_t bufsize, const MQTTPref // Periodic neighbors: time to next publish + how the last one went. if (obs && obs->mqtt_neighbors_enabled) { char when[16]; - switch (b->_neighbors_phase.load(std::memory_order_relaxed)) { + switch (snapshot.neighbors_phase) { case NBR_ACTIVE: strcpy(when, "active"); break; case NBR_DUE: strcpy(when, "due"); break; default: formatDuration(when, sizeof(when), - b->_neighbors_secs_until_next.load(std::memory_order_relaxed)); + snapshot.neighbors_seconds); break; } const char* last; - switch (b->_neighbors_last_result.load(std::memory_order_relaxed)) { + switch (snapshot.neighbors_result) { case NBR_RESULT_OK: last = "ok"; break; case NBR_RESULT_FAIL: last = "failed"; break; default: last = "none"; break; @@ -337,48 +365,33 @@ void MQTTBridge::formatMqttStatusReply(char* buf, size_t bufsize, const MQTTPref // broker whose uplink is dropping writes. void MQTTBridge::formatMqttStatsReply(char* buf, size_t bufsize) { if (buf == nullptr || bufsize == 0) return; - if (s_mqtt_bridge_instance == nullptr || !s_mqtt_bridge_instance->_initialized) { + const RuntimeSnapshot snapshot = _published.read(); + if (!snapshot.running) { snprintf(buf, bufsize, "> (bridge not running)"); return; } - MQTTBridge* b = s_mqtt_bridge_instance; - - int q = 0; -#ifdef ESP_PLATFORM - if (b->_packet_queue_handle != nullptr) { - q = (int)uxQueueMessagesWaiting(b->_packet_queue_handle); - } -#else - q = b->_queue_count; -#endif - - size_t outbox_total = 0; - unsigned long outbox_drops = 0; - for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - if (b->_slots[i].client) { - outbox_total += b->_slots[i].client->getOutboxSize(); - outbox_drops += b->_slots[i].client->getOutboxDrops(); - } - } + const int q = snapshot.queue; + const unsigned long outbox_total = snapshot.outbox; + const unsigned long outbox_drops = snapshot.outbox_drops; // drops=/: outbox-cap drops vs. memory-pressure skips. int pos = 0; replyAppendf(buf, bufsize, &pos, "> Free=%d Max=%d q:%d/%d Outbox=%u drops=%lu/%d", - (int)ESP.getFreeHeap(), (int)ESP.getMaxAllocHeap(), + (int)snapshot.free_heap, (int)snapshot.max_heap, q, MAX_QUEUE_SIZE, (unsigned)outbox_total, - outbox_drops, b->_skipped_publishes); + outbox_drops, snapshot.skipped); // filt=: packets the per-slot type filters rejected before the queue. // Omitted while zero so an unfiltered node's reply keeps its former length — // the per-slot list below is what usually gets clamped away first. - if (b->_filtered_packets > 0) { - replyAppendf(buf, bufsize, &pos, " filt=%lu", b->_filtered_packets); + if (snapshot.filtered > 0) { + replyAppendf(buf, bufsize, &pos, " filt=%lu", snapshot.filtered); } replyAppendf(buf, bufsize, &pos, " |"); for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - if (!b->_slots[i].enabled || !b->_slots[i].client) continue; + if (!snapshot.slots[i].enabled || !snapshot.slots[i].client) continue; replyAppendf(buf, bufsize, &pos, " s%d=%lu/%lu", i + 1, - b->_slots[i].client->getPublishOk(), - b->_slots[i].client->getPublishErr()); + snapshot.slots[i].publish_ok, + snapshot.slots[i].publish_err); } } @@ -387,35 +400,22 @@ void MQTTBridge::formatMqttStatsReply(char* buf, size_t bufsize) { // unconfigured, or bridge-not-running slots. bool MQTTBridge::getSlotStatusSnapshot(int slot_index, SlotStatusSnapshot* out) { if (out == nullptr || slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return false; - if (s_mqtt_bridge_instance == nullptr || !s_mqtt_bridge_instance->_initialized) return false; - MQTTBridge* b = s_mqtt_bridge_instance; - const MQTTSlot& slot = b->_slots[slot_index]; - - if (!slot.enabled && slot.preset) { - out->name = slot.preset->name; - out->state = "inactive"; - } else if (!slot.enabled) { - return false; // unconfigured slot - } else { - out->name = slot.preset ? slot.preset->name : "custom"; - if (!b->isSlotReady(slot_index)) { - out->state = "wait"; - } else if (slot.connected) { - out->state = "ok"; - } else if (slot.circuit_breaker_tripped) { - out->state = "fail"; - } else { - out->state = "disc"; - } - } - out->publish_ok = slot.client ? slot.client->getPublishOk() : 0; - out->publish_err = slot.client ? slot.client->getPublishErr() : 0; - // Lets the portal show why a healthy slot is quiet. - out->filter_mask = b->_obs ? b->_obs->mqtt_slot_packet_filter[slot_index] - : MQTTPacketFilter::kAllPacketTypes; + const RuntimeSnapshot snapshot = _published.read(); + const SlotRuntimeSnapshot& slot = snapshot.slots[slot_index]; + if (!snapshot.running || !slot.configured) return false; + memcpy(out->name, slot.name, sizeof(out->name)); + memcpy(out->state, slot.state, sizeof(out->state)); + out->publish_ok = slot.publish_ok; + out->publish_err = slot.publish_err; + out->filter_mask = slot.filter_mask; return true; } +void MQTTBridge::appendSlotStatsJson(char* buf, size_t size, int pos) { + const RuntimeSnapshot snapshot = _published.read(); + appendObserverSlotStatsJson(buf, size, pos, snapshot.slots, snapshot.running ? RUNTIME_MQTT_SLOTS : 0); +} + int MQTTBridge::getMaxActiveSlots() { // Each WSS/TLS connection needs ~40KB for mbedTLS buffers. Without PSRAM even // 3 concurrent connections would exhaust internal heap, so cap at 2; with @@ -444,21 +444,43 @@ unsigned long MQTTBridge::getLastWifiDisconnectTime() { return activeNetworkLink().lastDisconnectTime(); } +MQTTBridge::SlotOutageSnapshot MQTTBridge::getSlotOutageSnapshot(int slot_index) const { + SlotOutageSnapshot out; + if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return out; + const RuntimeSnapshot snapshot = _published.read(); + const SlotRuntimeSnapshot& slot = snapshot.slots[slot_index]; + out.monitored = snapshot.running && slot.enabled && slot.attempted; + out.started_ms = slot.current_outage_started_ms; + memcpy(out.name, slot.name, sizeof(out.name)); + return out; +} + +void MQTTBridge::formatRuntimeReply(char* buf, size_t bufsize) { + if (!buf || !bufsize) return; + const RuntimeSnapshot s = _published.read(); + snprintf(buf, bufsize, "> %s seq:%lu age:%lums cfg:%lu/%lu events_lost:%lu", + s.running ? "running" : s_stop_unproven ? "stop unproven" : "stopped", + (unsigned long)s.sequence, (unsigned long)((uint32_t)(millis() - s.sampled_ms)), + (unsigned long)s.desired_revision, (unsigned long)s.applied_revision, + (unsigned long)s.event_overflows); +} + unsigned long MQTTBridge::getSlotCurrentOutageStartMs(int slot_index) const { if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return 0; - return _slots[slot_index].current_outage_started_ms; + return _published.read().slots[slot_index].current_outage_started_ms; } bool MQTTBridge::isSlotEnabledAndAttempted(int slot_index) const { if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return false; - const MQTTSlot& s = _slots[slot_index]; - return s.enabled && s.initial_connect_done; + const SlotRuntimeSnapshot s = _published.read().slots[slot_index]; + return s.enabled && s.attempted; } const char* MQTTBridge::getSlotPresetName(int slot_index) const { if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return "?"; - const MQTTSlot& s = _slots[slot_index]; - if (s.preset && s.preset->name) return s.preset->name; + const SlotRuntimeSnapshot s = _published.read().slots[slot_index]; + const MQTTPresetDef* preset = findMQTTPreset(s.name); + if (preset) return preset->name; if (!s.enabled) return MQTT_PRESET_NONE; return MQTT_PRESET_CUSTOM; } @@ -513,7 +535,8 @@ const char* MQTTBridge::tlsErrorStr(int32_t err) { void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) { if (!buf || bufsize == 0) return; - if (!s_mqtt_bridge_instance || !s_mqtt_bridge_instance->_initialized) { + const RuntimeSnapshot snapshot = _published.read(); + if (!snapshot.running) { snprintf(buf, bufsize, "> mqtt%d: bridge not running", slot_index + 1); return; } @@ -522,17 +545,16 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) return; } - MQTTBridge* b = s_mqtt_bridge_instance; - const MQTTSlot& slot = b->_slots[slot_index]; + const SlotRuntimeSnapshot& slot = snapshot.slots[slot_index]; // Determine state string const char* state; - if (!slot.enabled && !slot.preset && slot.host[0] == '\0') { + if (!slot.configured) { snprintf(buf, bufsize, "> mqtt%d: not configured", slot_index + 1); return; } else if (!slot.enabled) { state = "inactive"; - } else if (!b->isSlotReady(slot_index)) { + } else if (!slot.ready) { // Same classification as `get mqtt.status` and getSlotStatusSnapshot(): the slot // is configured but missing a token/IATA/credential, so it was never set up and // has no client yet. Previously reported "disc", which read as a network fault. @@ -560,8 +582,7 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) // the 160-byte reply, and an error tail (TLS + mbedTLS + errno + age) can // already reach ~117 chars, so putting the filter first would push the // operator's diagnostic detail off the end of a failing slot's line. - const uint16_t filter_mask = b->_obs ? b->_obs->mqtt_slot_packet_filter[slot_index] - : MQTTPacketFilter::kAllPacketTypes; + const uint16_t filter_mask = slot.filter_mask; char filter_text[MQTTPacketFilter::kFilterTextSize]; const bool show_filter = filter_mask != MQTTPacketFilter::kAllPacketTypes && MQTTPacketFilter::format(filter_mask, filter_text, sizeof(filter_text)); @@ -674,13 +695,11 @@ static inline uint32_t mqttStopTimeoutForSlots(int slots) { // --------------------------------------------------------------------------- MQTTBridge::MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mgr, mesh::RTCClock *rtc, mesh::LocalIdentity *identity) : BridgeBase(prefs, mgr, rtc), - _obs(obs), + _source_obs(obs), _network(&activeNetworkLink()), _queue_count(0), _last_status_publish(0), _last_status_retry(0), _status_interval(300000), - _last_ntp_sync(0), _ntp_synced(false), _ntp_sync_pending(false), _slots_setup_done(false), _max_active_slots(RUNTIME_MQTT_SLOTS), - _ntp_force_requested(false), _ntp_force_done(false), _ntp_force_result(false), - _ntp_diag_requested(false), _ntp_diag_done(false), _ntp_diag_count(0), + _ntp_synced(false), _slots_setup_done(false), _max_active_slots(RUNTIME_MQTT_SLOTS), // Default to UTC; setRules() will be called from syncTimeWithNTP when a // non-UTC timezone string is configured. Timezone has no default ctor, // so we must pass rules here. @@ -749,20 +768,14 @@ MQTTBridge::MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mg _slots[i].generation = 0; _slots[i].applied_config = MqttEffectiveConfig(); _slots[i].allocated_buffer_size = 0; - _slot_reconfigure_pending[i] = false; _slot_force_jwt_mint[i] = false; _status_publish_pending[i] = false; } // Reset CLI-requested forced NTP sync handshake (bridge object is reused across restarts) _ntp_force_requested = false; - _ntp_force_done = false; - _ntp_force_result = false; // Reset CLI-requested NTP diagnostic handshake - _ntp_diag_requested = false; - _ntp_diag_done = false; - _ntp_diag_count = 0; #if defined(WITH_MQTT_NEIGHBORS) // Neighbors publish handoff (buffer allocated in begin() after PSRAM probe). @@ -876,6 +889,11 @@ void MQTTBridge::begin() { return; } + _ntp_schedule = NtpSchedule(); + _heap_sampled = false; + _slots_setup_done = false; + _ntp_force_requested.store(false); + // PSRAM diagnostic - helps debug memory fragmentation on boards with external RAM #ifdef BOARD_HAS_PSRAM { @@ -913,6 +931,10 @@ void MQTTBridge::begin() { MQTT_DEBUG_PRINTLN("Max active slots: %d", _max_active_slots); // Ethernet needs no credentials; Wi-Fi preserves the existing SSID gate. + publishConfiguration(); + uint32_t initial_reconfigure = 0; + _applied_revision = 0; + _configuration.consume(_runtime_prefs, _radio_metadata, _applied_revision, initial_reconfigure); if (!isNetworkConfigValid(_obs)) { MQTT_DEBUG_PRINTLN("MQTT Bridge initialization skipped - %s is not configured", _network->mediumName()); @@ -1076,6 +1098,7 @@ void MQTTBridge::begin() { _stop_requested.store(false, std::memory_order_relaxed); _stop_acked.store(false, std::memory_order_relaxed); _teardown_complete.store(false, std::memory_order_relaxed); + publishRuntimeSnapshot(); BaseType_t create_result = xTaskCreatePinnedToCore( mqttTask, "MQTTBridge", @@ -1088,6 +1111,7 @@ void MQTTBridge::begin() { if (create_result != pdPASS) _mqtt_task_handle = nullptr; if (_mqtt_task_handle == nullptr) { MQTT_DEBUG_PRINTLN("Failed to create MQTT task!"); + _published.publish(RuntimeSnapshot{}); vQueueDelete(_packet_queue_handle); _packet_queue_handle = nullptr; #if defined(BOARD_HAS_PSRAM) @@ -1108,9 +1132,8 @@ void MQTTBridge::begin() { // MQTT client objects are NOT allocated here. setupSlot() creates one on a slot's // first setup, so unconfigured and capped-off slots never cost their ~1.3 KB of - // internal DRAM. Once created a client lives for the bridge's lifetime, so the - // reconfigure/reconnect paths still reuse the same mbedTLS context instead of - // churning ~40 KB of internal heap per cycle. + // internal DRAM. Reconnects reuse the wrapper and SDK task where possible; + // closing a TLS transport still releases its session buffers. // Sync the lifecycle Coordinator to Running now that all resources exist and // the task is created. Driven only on the success path: the failure rollbacks @@ -1121,7 +1144,6 @@ void MQTTBridge::begin() { _lifecycle.onTaskStarted(); // Starting -> Running _initialized = true; - s_mqtt_bridge_instance = this; MQTT_DEBUG_PRINTLN("MQTT Bridge initialized"); } @@ -1130,6 +1152,7 @@ void MQTTBridge::begin() { // --------------------------------------------------------------------------- void MQTTBridge::end() { MQTT_DEBUG_PRINTLN("Stopping MQTT Bridge..."); + _ntp_diag_job.cancel(millis()); // Idempotent stop: nothing to tear down if we never started (or already stopped). if (!_initialized) { @@ -1137,8 +1160,10 @@ void MQTTBridge::end() { return; } - // Stop new diagnostic reads through the singleton before teardown begins. - s_mqtt_bridge_instance = nullptr; + // Readers see a stopped snapshot while teardown proceeds. + RuntimeSnapshot stopped = _published.read(); + stopped.running = false; + _published.publish(stopped); // Size the stop timeout to the work about to happen: each enabled slot's // mbedTLS/wss client takes ~5-6 s to disconnect + destroy, sequentially (Phase @@ -1192,7 +1217,10 @@ void MQTTBridge::end() { // above). begin() refuses until the task acknowledges, so _initialized == false // cannot be turned into a second task over the same state. _initialized = false; - _slots_setup_done = false; // Reset so deferred setup runs again on next begin() + RuntimeSnapshot stopped_snapshot = _published.read(); + stopped_snapshot.running = false; + stopped_snapshot.connected = 0; + _published.publish(stopped_snapshot); s_stop_unproven = _lifecycle.isStopUnproven(); if (_lifecycle.isStopUnproven()) { MQTT_DEBUG_PRINTLN("MQTT Bridge stop UNPROVEN after %lu ms: task did not acknowledge. " @@ -1322,8 +1350,7 @@ void MQTTBridge::initializeNetworkInTask() { // not race the first DNS lookup after a bridge restart. if (!_network->begin(_obs->wifi_ssid, _obs->wifi_password)) { MQTT_DEBUG_PRINTLN("%s network initialization failed", _network->mediumName()); - } else if (_network->isConnected() && !_ntp_synced && !_ntp_sync_pending) { - _ntp_sync_pending = true; + } else if (_network->isConnected() && !_ntp_synced) { } // NOTE: Slot setup is deferred until after NTP sync in mqttTaskLoop(). @@ -1346,8 +1373,6 @@ void MQTTBridge::mqttTaskLoop() { // Treat the first post-settle sample as an edge. A fast Wi-Fi association // may complete during the delay above, before the task has sampled state; // initializing from the live value would then defer NTP until its retry. - bool network_was_connected = false; - unsigned long last_ntp_attempt = 0; // Main task loop #ifdef MQTT_MEMORY_DEBUG @@ -1366,6 +1391,7 @@ void MQTTBridge::mqttTaskLoop() { teardownSlot(i); } destroySlotClients(); + publishRuntimeSnapshot(); // The acknowledgement means "every client is destroyed, nothing I own is // still running". One client whose esp_mqtt_client_stop() never completed @@ -1428,48 +1454,18 @@ void MQTTBridge::mqttTaskLoop() { } #endif + processSlotEvents(); + applyConfiguration(); handleNetworkConnection(now); - // Schedule once per link-up edge. Failed syncs are owned by the 30-second - // retry below; re-arming here on every loop would make a blocked NTP path - // run the multi-second sync sequence back-to-back forever. - const bool network_connected = _network->isConnected(); - _ntp_sync_pending = NetworkPolicy::ntpPendingAfterConnectivitySample( - _ntp_synced, _ntp_sync_pending, network_was_connected, - network_connected); - network_was_connected = network_connected; - if (_ntp_sync_pending && network_connected) { - _ntp_sync_pending = false; - last_ntp_attempt = now; - syncTimeWithNTP(); - } - - // Retry NTP every 30s if initial sync failed (slots can't start without valid time) - if (NetworkPolicy::ntpRetryDue( - _ntp_synced, network_connected, (uint32_t)now, - (uint32_t)last_ntp_attempt)) { - last_ntp_attempt = now; - syncTimeWithNTP(); - } - - // Process a CLI-requested forced NTP sync (queued from Core 1). Running it here - // keeps all NTP I/O on Core 0; requestForcedNtpSync() blocks the CLI thread until - // we publish the outcome below. - if (_ntp_force_requested) { - _ntp_force_requested = false; - // primary_only: validate just the server that was set, so a typo fails fast. - bool ok = syncTimeWithNTP(true, /*primary_only=*/true); - _ntp_force_result = ok; - _ntp_force_done = true; // set last so the waiter sees a consistent result - } - - // Process a CLI-requested NTP connectivity diagnostic (queued from Core 1). - // Probe-only — never touches the system clock. - if (_ntp_diag_requested) { - _ntp_diag_requested = false; - runNtpDiagProbe(); - _ntp_diag_done = true; // set last so the waiter sees populated results + const bool ntp_due = _ntp_schedule.due((uint32_t)millis()); + if (_ntp_force_requested.exchange(false, std::memory_order_acq_rel)) { + syncTimeWithNTP(true, /*primary_only=*/true); + } else if (ntp_due && _network->isConnected()) { + syncTimeWithNTP(false, false, _ntp_synced ? 1 : 2); } + const uint32_t ntp_probe = _ntp_diag_job.begin(); + if (ntp_probe) runNtpDiagProbe(ntp_probe); // Deferred slot setup: wait until NTP is synced so JWT tokens get valid timestamps. // This avoids wasted TLS handshakes that get rejected due to bad token times. @@ -1509,14 +1505,7 @@ void MQTTBridge::mqttTaskLoop() { } } - // Process pending slot reconfigures (queued from CLI on Core 1) - for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - if (_slot_reconfigure_pending[i]) { - _slot_reconfigure_pending[i] = false; - MQTT_DEBUG_PRINTLN("Applying deferred reconfigure for MQTT%d (preset: %s)", i + 1, _obs->mqtt_slot_preset[i]); - applySlotPreset(i, _obs->mqtt_slot_preset[i]); - } - } + processSlotEvents(); // Publish on-connect status for slots whose onConnect callback fired since // the last loop. Raised on the esp-mqtt event task, consumed here on the @@ -1578,13 +1567,6 @@ void MQTTBridge::mqttTaskLoop() { // Periodic configuration check (throttled to avoid spam) checkConfigurationMismatch(); - // Periodic NTP refresh (every hour): a validated probe, one attempt per - // server. Blocks this task for about one probe timeout per server that does - // not answer (see refreshNTP()). - if (_network->isConnected() && now - _last_ntp_sync > 3600000) { - refreshNTP(); - } - // Publish status updates (handle millis() overflow correctly). // Read the toggle live from prefs (like mqtt.packets/rx/tx below) so a // CLI/web `set mqtt.status` change applies without a bridge restart. @@ -1650,6 +1632,8 @@ void MQTTBridge::mqttTaskLoop() { last_slot_status_update = now; } + publishRuntimeSnapshot(); + // Adaptive delay: 5 ms when packets are queued, 50 ms when idle. // The previous "status approaching" check (widening to 5 ms for 10 s before each status // publish) caused 2 000 unnecessary wakeups per interval; the 50 ms idle tick catches @@ -1670,11 +1654,8 @@ void MQTTBridge::mqttTaskLoop() { // at all until setupSlot() runs (the reconnect ladder is gated on // initial_connect_done), so slots that are unconfigured or capped off never get one. // -// Once created the object lives until destroySlotClients(): reconfiguring a slot -// (preset change, JWT renewal, reconnect) reuses it, so the mbedTLS context and its -// ~40 KB of internal-heap buffers are allocated once instead of every reconfigure. -// That context is created by connect(), not by this constructor, so deferring the -// allocation to first use costs nothing beyond the object itself. +// Compatible reconnects reuse the wrapper. Transport/trust changes can recreate +// it after a proven stop. TLS session buffers still follow connection lifetime. bool MQTTBridge::ensureSlotClient(int index) { if (index < 0 || index >= RUNTIME_MQTT_SLOTS) return false; MQTTSlot& slot = _slots[index]; @@ -1696,98 +1677,63 @@ bool MQTTBridge::ensureSlotClient(int index) { slot.allocated_buffer_size = kMqttClientBufferSize; slot.client->setAutoReconnect(false); // we handle reconnect with our own backoff - slot.client->onConnect([this, index](bool sessionPresent) { - // A CONNECT started before this slot was disabled or reconfigured can still - // complete afterwards. Accepting it marked a slot connected that the - // operator had switched off, scheduled its status publish, and published - // through the old session (F04). The bridge task owns client_state, so it - // is the authority on whether this event was asked for. - const ClientState st = _slots[index].client_state; - if (!_slots[index].enabled || !(st == ClientState::Starting || st == ClientState::Disconnected)) { - MQTT_DEBUG_PRINTLN("MQTT%d ignoring late CONNECTED (state=%s, gen=%lu, enabled=%d)", - index + 1, clientStateName(st), - (unsigned long)_slots[index].generation, (int)_slots[index].enabled); - return; - } - MQTT_DEBUG_PRINTLN("MQTT%d connected", index + 1); - _slots[index].client_state = ClientState::Connected; - _slots[index].connected = true; - _slot_force_jwt_mint[index] = false; - // NOTE: reconnect_backoff / max_backoff_failures are NOT reset here. - // A CONNACK alone doesn't prove the link is healthy — a broker that - // accepts and then drops within seconds would reset the ladder every - // cycle and retry at the 10 s rung forever, and each retry is a full - // TLS session alloc/free (~40 KB of internal-heap churn, a known - // fragmentation driver). The ladder is instead cleared by - // maintainSlotConnection() once the connection has stayed up for - // BACKOFF_STABLE_RESET_MS, so flapping endpoints keep their earned - // backoff level. The breaker itself does clear now: while connected - // the diag/status must not claim the slot gave up, and the next - // disconnect should be governed by the (still-elevated) ladder. - _slots[index].connected_at_ms = millis(); - _slots[index].circuit_breaker_tripped = false; - _slots[index].last_tls_err = 0; - _slots[index].last_tls_stack_err = 0; - _slots[index].last_sock_errno = 0; - _slots[index].last_connack_code = 0; - _slots[index].last_error_time = 0; - _slots[index].current_outage_started_ms = 0; // clear current-outage timer for AlertReporter - updateCachedConnectionStatus(); // bool store — safe from this (esp-mqtt) task - // This callback runs on the client's esp-mqtt event task, not the bridge - // task. Do NOT build/publish status here: publishStatusToSlot() writes the - // shared _json_scratch_doc/_json_scratch_buffer/_origin that the periodic - // publishStatus() uses on the bridge task, and two slots' callbacks could - // race each other over them. Marshal the publish onto the bridge task via a - // per-slot flag (see mqttTaskLoop consumer / A2). - _status_publish_pending[index] = true; + const uint32_t incarnation = ++slot.incarnation; + slot.client->onConnect([this, index, incarnation](bool) { + MqttSlotEvent event; + event.kind = MqttSlotEvent::Kind::Connected; + event.slot = index; + event.incarnation = incarnation; + event.at_ms = millis(); + _slot_events.push(event); }); - slot.client->onDisconnect([this, index](bool sessionPresent) { - MQTT_DEBUG_PRINTLN("MQTT%d disconnected", index + 1); - // Only a live client's disconnect is news. One arriving for a client we - // already stopped (or quarantined) must not resurrect its state. - if (clientStateIsLive(_slots[index].client_state)) { - _slots[index].client_state = ClientState::Disconnected; - } - _slots[index].disconnect_count++; - if (_slots[index].first_disconnect_time == 0) { - _slots[index].first_disconnect_time = millis(); - } - if (_slots[index].current_outage_started_ms == 0) { - _slots[index].current_outage_started_ms = millis(); - } - _slots[index].connected = false; - _slots[index].connected_at_ms = 0; // stability clock only runs while connected - updateCachedConnectionStatus(); + slot.client->onDisconnect([this, index, incarnation](bool) { + MqttSlotEvent event; + event.kind = MqttSlotEvent::Kind::Disconnected; + event.slot = index; + event.incarnation = incarnation; + event.at_ms = millis(); + _slot_events.push(event); }); - slot.client->onError([this, index](esp_mqtt_error_codes error) { - _slots[index].last_tls_err = error.esp_tls_last_esp_err; - _slots[index].last_tls_stack_err = error.esp_tls_stack_err; - _slots[index].last_sock_errno = error.esp_transport_sock_errno; - _slots[index].last_error_time = millis(); - // Cleared on any other error type so the diag describes the latest failure - // rather than pairing a fresh transport error with an old refusal. - _slots[index].last_connack_code = - (error.error_type == MQTT_ERROR_TYPE_CONNECTION_REFUSED) - ? (uint8_t)error.connect_return_code : 0; - if (error.error_type == MQTT_ERROR_TYPE_CONNECTION_REFUSED) { - _slot_force_jwt_mint[index] = true; - // Broker rejected the MQTT CONNECT itself — not a transport failure. - // return code: 1=protocol, 2=client-id rejected, 3=server unavailable, - // 4=bad username/password, 5=not authorized. Codes 3/4/5 point at a - // server-side lockout or auth problem rather than the network. - MQTT_DEBUG_PRINTLN("MQTT%d connection refused by broker (return code=%d)", - index + 1, (int)error.connect_return_code); - } else if (error.esp_tls_last_esp_err != 0 || error.esp_tls_stack_err != 0 || error.esp_transport_sock_errno != 0) { - MQTT_DEBUG_PRINTLN("MQTT%d error: tls=%d, tls_stack=%d, sock=%d, type=%d", - index + 1, error.esp_tls_last_esp_err, error.esp_tls_stack_err, - error.esp_transport_sock_errno, error.error_type); - } else { - MQTT_DEBUG_PRINTLN("MQTT%d error: type=%d", index + 1, error.error_type); - } + slot.client->onError([this, index, incarnation](esp_mqtt_error_codes error) { + MqttSlotEvent event; + event.kind = MqttSlotEvent::Kind::Error; + event.slot = index; + event.incarnation = incarnation; + event.at_ms = millis(); + event.tls = error.esp_tls_last_esp_err; + event.tls_stack = error.esp_tls_stack_err; + event.socket = error.esp_transport_sock_errno; + event.connack = error.error_type == MQTT_ERROR_TYPE_CONNECTION_REFUSED + ? static_cast(error.connect_return_code) : 0; + _slot_events.push(event); }); return true; } +void MQTTBridge::processSlotEvents() { + const uint32_t overflow = _slot_events.takeOverflowSlots(); + for (int i = 0; i < RUNTIME_MQTT_SLOTS; ++i) { + if (!(overflow & (uint32_t(1) << i))) continue; + // Losing lifecycle events makes connectivity unproven. Stop before allowing + // another publish, then let ordinary maintenance restart the joined client. + _slots[i].connected = false; + _status_publish_pending[i] = false; + if (_slots[i].client && clientStateIsLive(_slots[i].client_state)) stopSlotClient(i); + _slot_events.discardSlot(i); + } + MqttSlotEvent event; + for (unsigned n = 0; n < 32 && _slot_events.pop(event); ++n) { + const int index = event.slot; + if (index >= RUNTIME_MQTT_SLOTS) continue; + if (applyMqttSlotEvent(_slots[index], event, _slot_force_jwt_mint[index], _status_publish_pending[index])) { + MQTT_DEBUG_PRINTLN("MQTT%d event=%u state=%s incarnation=%lu connack=%u tls=%ld sock=%ld", + index + 1, (unsigned)event.kind, clientStateName(_slots[index].client_state), + (unsigned long)event.incarnation, (unsigned)event.connack, (long)event.tls, (long)event.socket); + } + } + updateCachedConnectionStatus(); +} + // Allocate this slot's JWT token buffer. Called only from createSlotAuthToken(), the // sole writer, so a slot on a non-JWT preset (or no preset at all) never allocates. // @@ -2292,10 +2238,6 @@ bool MQTTBridge::recreateSlotClient(int index) { return ensureSlotClient(index); } -// Disconnect the slot's MQTT client and clear per-connection state, but leave -// the client object alive so a subsequent setupSlot() can reuse its mbedTLS -// context. This is called both on reconfigure (preset change) and at shutdown; -// destruction of the underlying client happens once in destroySlotClients(). // Close a live client for a reconfigure: cheap where that is safe, a real stop // where it is not. // @@ -2326,7 +2268,13 @@ void MQTTBridge::closeLiveClientForReconfigure(int index) { MQTT_DEBUG_PRINTLN("MQTT%d reconfigure: transport did not close cleanly (%s)", index + 1, esp_err_to_name(r)); } + if (r != ESP_OK) { + stopSlotClient(index); + return; + } + _slot_events.discardSlot(index); slot.client_state = ClientState::Disconnected; + slot.connected = false; } // The old route is already gone, so a link transition waits far less for the @@ -2413,6 +2361,8 @@ void MQTTBridge::stopSlotClient(int index) { MQTT_DEBUG_PRINTLN("MQTT%d stop: client was not started (nothing to join)", index + 1); } slot.client_state = ClientState::Stopped; + slot.connected = false; + _slot_events.discardSlot(index); return; } MQTT_DEBUG_PRINTLN("MQTT%d stop returned %s - client quarantined for this boot", @@ -2479,21 +2429,9 @@ esp_err_t MQTTBridge::reconnectSlotClient(int index) { return ESP_ERR_INVALID_STATE; } - esp_err_t r; - if (!slot.client->isStarted()) { - MQTT_DEBUG_PRINTLN("MQTT%d start (client was stopped)", index + 1); - r = slot.client->connect(); - } else { - r = slot.client->reconnect(); - } - if (r == ESP_OK) { - slot.client_state = ClientState::Starting; - slot.generation++; - // The attempt carries whatever credential is configured right now, so this - // is the point at which a freshly minted token becomes the one in use. - slot.applied_token_expires_at = slot.token_expires_at; - } - return r; + return startMqttSlotAttempt(slot, [&]() { + return slot.client->isStarted() ? slot.client->reconnect() : slot.client->connect(); + }); } @@ -2703,8 +2641,8 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns MQTT_DEBUG_PRINTLN("MQTT%d int_heap=%d at token renewal reconnect", index + 1, (int)heap_caps_get_free_size(MALLOC_CAP_INTERNAL)); MQTT_DEBUG_PRINTLN(" radio_state=%d, last_rx=%lums ago", - _radio ? _radio->getRadioState() : -1, - (_radio && _radio->getLastRecvMillis() > 0) ? (_ms->getMillis() - _radio->getLastRecvMillis()) : 0); + _radio_stats.read().state, + _radio_stats.read().last_rx_age_ms); } else { // Token renewed but old one still valid — just update credentials for next reconnect slot.client->setCredentials(_jwt_username, slot.auth_token); @@ -2721,11 +2659,6 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns } } - // Phase 4 (MQTT memory-defrag): the MIN_TLS_HEAP preflight was a workaround - // for the fragmentation caused by per-reconnect mbedTLS allocations. With - // persistent clients (Phase 1), the mbedTLS context is allocated once at - // startup and the preflight is no longer necessary. - const auto prepareJwtReconnect = [&](bool force_mint, int backoff_level) { const bool has_token = slot.auth_token && slot.auth_token[0] != '\0'; const unsigned long expires_at = slot.token_expires_at; @@ -2800,8 +2733,8 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns MQTT_DEBUG_PRINTLN("MQTT%d circuit breaker probe (attempting single reconnect after %lu ms, int_heap=%d)", index + 1, probe_elapsed, (int)heap_caps_get_free_size(MALLOC_CAP_INTERNAL)); MQTT_DEBUG_PRINTLN(" radio_state=%d, last_rx=%lums ago", - _radio ? _radio->getRadioState() : -1, - (_radio && _radio->getLastRecvMillis() > 0) ? (_ms->getMillis() - _radio->getLastRecvMillis()) : 0); + _radio_stats.read().state, + _radio_stats.read().last_rx_age_ms); if (slot_uses_jwt) { prepareJwtReconnect(true, -1); } @@ -2835,8 +2768,8 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns MQTT_DEBUG_PRINTLN("MQTT%d reconnecting (backoff level %d, failures at max: %d, int_heap=%d)", index + 1, slot.reconnect_backoff, slot.max_backoff_failures, (int)heap_caps_get_free_size(MALLOC_CAP_INTERNAL)); MQTT_DEBUG_PRINTLN(" radio_state=%d, last_rx=%lums ago", - _radio ? _radio->getRadioState() : -1, - (_radio && _radio->getLastRecvMillis() > 0) ? (_ms->getMillis() - _radio->getLastRecvMillis()) : 0); + _radio_stats.read().state, + _radio_stats.read().last_rx_age_ms); reconnect_attempted = true; _last_slot_reconnect_ms = now_millis; if (slot_uses_jwt) { @@ -3092,7 +3025,7 @@ void MQTTBridge::publishStatusToSlot(int index) { MQTTMessageBuilder::formatIsoTimestampForMqtt(now_tv.tv_sec, now_tv.tv_usec, _timezone, timestamp, sizeof(timestamp)); snprintf(radio_info, sizeof(radio_info), "%.6f,%.1f,%d,%d", - _prefs->freq, _prefs->bw, _prefs->sf, _prefs->cr); + _radio_metadata.freq, _radio_metadata.bw, _radio_metadata.sf, _radio_metadata.cr); strncpy(origin_id, _device_id, sizeof(origin_id) - 1); origin_id[sizeof(origin_id) - 1] = '\0'; @@ -3100,30 +3033,16 @@ void MQTTBridge::publishStatusToSlot(int index) { char client_version[64]; getClientVersion(client_version, sizeof(client_version)); - // Collect stats on-demand if sources are available - int battery_mv = -1; - int uptime_secs = -1; - int errors = -1; - int noise_floor = -999; - int tx_air_secs = -1; - int rx_air_secs = -1; - int recv_errors = -1; - int packets_sent = -1; - int packets_received = -1; - - if (_board) battery_mv = _board->getBattMilliVolts(); - if (_ms) uptime_secs = _ms->getMillis() / 1000; - if (_dispatcher) { - errors = _dispatcher->getErrFlags(); - tx_air_secs = _dispatcher->getTotalAirTime() / 1000; - rx_air_secs = _dispatcher->getReceiveAirTime() / 1000; - packets_sent = (int)(_dispatcher->getNumSentFlood() + _dispatcher->getNumSentDirect()); - packets_received = (int)(_dispatcher->getNumRecvFlood() + _dispatcher->getNumRecvDirect()); - } - if (_radio) { - noise_floor = (int16_t)_radio->getNoiseFloor(); - recv_errors = (int)_radio->getPacketsRecvErrors(); - } + const RadioStatsSnapshot stats = _radio_stats.read(); + const int battery_mv = stats.battery_mv; + const int uptime_secs = stats.uptime_secs; + const int errors = stats.errors; + const int noise_floor = stats.noise_floor; + const int tx_air_secs = stats.tx_air_secs; + const int rx_air_secs = stats.rx_air_secs; + const int recv_errors = stats.recv_errors; + const int packets_sent = stats.packets_sent; + const int packets_received = stats.packets_received; // Internal heap free (for diagnosing repeater hangs from internal heap exhaustion) int internal_heap_free = (int)heap_caps_get_free_size(MALLOC_CAP_INTERNAL); @@ -3135,7 +3054,7 @@ void MQTTBridge::publishStatusToSlot(int index) { battery_mv, uptime_secs, errors, _queue_count, noise_floor, tx_air_secs, rx_air_secs, recv_errors, internal_heap_free, packets_sent, packets_received, - _prefs->disable_fwd ? "off" : "on" + _radio_metadata.disable_fwd ? "off" : "on" ); if (len > 0) { @@ -3171,21 +3090,55 @@ bool MQTTBridge::isAnySlotConnected() { return false; } -void MQTTBridge::setSlotPreset(int slot_index, const char* preset_name) { - if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return; +void MQTTBridge::publishRadioStats() { + RadioStatsSnapshot s; + if (_board) s.battery_mv = _board->getBattMilliVolts(); + if (_ms) s.uptime_secs = _ms->getMillis() / 1000; + if (_dispatcher) { + s.errors = _dispatcher->getErrFlags(); + s.tx_air_secs = _dispatcher->getTotalAirTime() / 1000; + s.rx_air_secs = _dispatcher->getReceiveAirTime() / 1000; + s.packets_sent = _dispatcher->getNumSentFlood() + _dispatcher->getNumSentDirect(); + s.packets_received = _dispatcher->getNumRecvFlood() + _dispatcher->getNumRecvDirect(); + } + if (_radio) { + s.noise_floor = _radio->getNoiseFloor(); + s.recv_errors = _radio->getPacketsRecvErrors(); + s.state = _radio->getRadioState(); + if (_ms && _radio->getLastRecvMillis()) s.last_rx_age_ms = _ms->getMillis() - _radio->getLastRecvMillis(); + } + _radio_stats.publish(s); +} - // On ESP32, teardown/setup involves TLS and must run on the MQTT task (Core 0). - // Set a flag so the MQTT task picks it up on its next loop iteration. - #ifdef ESP_PLATFORM - if (_mqtt_task_handle != nullptr) { - _slot_reconfigure_pending[slot_index] = true; - MQTT_DEBUG_PRINTLN("MQTT%d reconfigure queued (preset: %s)", slot_index + 1, preset_name); - return; +void MQTTBridge::publishConfiguration(uint32_t reconfigure) { + RadioMetadata metadata{}; + memcpy(metadata.node_name, _prefs->node_name, sizeof(metadata.node_name)); + metadata.freq = _prefs->freq; + metadata.bw = _prefs->bw; + metadata.sf = _prefs->sf; + metadata.cr = _prefs->cr; + metadata.disable_fwd = _prefs->disable_fwd; + _configuration.publish(*_source_obs, metadata, reconfigure); +} + +void MQTTBridge::applyConfiguration() { + uint32_t reconfigure = 0; + if (!_configuration.consume(_runtime_prefs, _radio_metadata, _applied_revision, reconfigure)) return; + refreshOriginFromPrefs(); + snprintf(_iata, sizeof(_iata), "%s", _obs->mqtt_iata); + _status_interval = _obs->mqtt_status_interval >= 1000 && _obs->mqtt_status_interval <= 3600000 + ? _obs->mqtt_status_interval : 300000; + for (int i = 0; i < RUNTIME_MQTT_SLOTS; ++i) { + if (reconfigure & (uint32_t(1) << i)) applySlotPreset(i, _obs->mqtt_slot_preset[i]); } - #endif +} - // Non-ESP32 or bridge not yet started: apply directly - applySlotPreset(slot_index, preset_name); +void MQTTBridge::setSlotPreset(int slot_index, const char* preset_name) { + if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return; + // The durable image already contains preset_name. Publish it and the request + // together so the worker cannot apply the request to another revision. + (void)preset_name; + publishConfiguration(uint32_t(1) << slot_index); } void MQTTBridge::applySlotPreset(int slot_index, const char* preset_name) { @@ -3224,7 +3177,7 @@ void MQTTBridge::applySlotPreset(int slot_index, const char* preset_name) { strncpy(slot.audience, _obs->mqtt_slot_audience[slot_index], sizeof(slot.audience) - 1); slot.audience[sizeof(slot.audience) - 1] = '\0'; slot.enabled = (slot.host[0] != '\0'); - if (_initialized && slot.enabled && customEndpointComplete(slot.host, slot.port)) { + if (!_stop_requested.load() && slot.enabled && customEndpointComplete(slot.host, slot.port)) { // Same cap startup applies. teardownSlot() above already released this slot's own // position, so reconfiguring a live slot still passes. if (!canActivateSlot(slot_index)) { @@ -3247,7 +3200,7 @@ void MQTTBridge::applySlotPreset(int slot_index, const char* preset_name) { strncpy(slot.password, _obs->mqtt_slot_password[slot_index], sizeof(slot.password) - 1); slot.password[sizeof(slot.password) - 1] = '\0'; } - if (_initialized) { + if (!_stop_requested.load()) { char reason[80]; if (!isSlotReady(slot_index, reason, sizeof(reason))) { MQTT_DEBUG_PRINTLN("MQTT%d (%s) not ready - run '%s' to connect", slot_index + 1, preset_name, reason); @@ -3265,8 +3218,15 @@ void MQTTBridge::applySlotPreset(int slot_index, const char* preset_name) { } } +bool MQTTBridge::mayEditStartupMetadata() { + if (_initialized) return false; + pollLateStopAck(); + return !_lifecycle.isStopUnproven(); +} + void MQTTBridge::setSlotCustomBroker(int slot_index, const char* host, uint16_t port, const char* username, const char* password) { + if (!mayEditStartupMetadata()) return; if (slot_index < 0 || slot_index >= RUNTIME_MQTT_SLOTS) return; MQTTSlot& slot = _slots[slot_index]; @@ -3349,7 +3309,7 @@ bool MQTTBridge::handleNetworkConnection(unsigned long now) { } bool MQTTBridge::isReady() const { - return _initialized && isNetworkConfigValid(_obs); + return _initialized && isNetworkConfigValid(_source_obs); } bool MQTTBridge::isIATAValid() const { @@ -3405,16 +3365,22 @@ void MQTTBridge::loop() { if (!_initialized) return; #ifdef ESP_PLATFORM - // On ESP32, loop() is a no-op - all processing happens in the FreeRTOS task + // Publish only between loop-owned commands, after save/rollback has finished. + const uint32_t now = millis(); + if ((uint32_t)(now - _last_radio_sample_ms) >= 1000) { + _last_radio_sample_ms = now; + publishRadioStats(); + } + if ((uint32_t)(now - _last_config_sample_ms) >= 100) { + _last_config_sample_ms = now; + publishConfiguration(); + } return; #else unsigned long now = millis(); - if (handleNetworkConnection(now) && !_ntp_synced) { - syncTimeWithNTP(); - } - if (_ntp_sync_pending && _network->isConnected()) { - _ntp_sync_pending = false; - syncTimeWithNTP(); + handleNetworkConnection(now); + if (_ntp_schedule.due((uint32_t)millis()) && _network->isConnected()) { + syncTimeWithNTP(false, false, _ntp_synced ? 1 : 2); } // Deferred slot setup after NTP sync (non-ESP32 path) @@ -3434,13 +3400,8 @@ void MQTTBridge::loop() { } } - // Process pending slot reconfigures - for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - if (_slot_reconfigure_pending[i]) { - _slot_reconfigure_pending[i] = false; - applySlotPreset(i, _obs->mqtt_slot_preset[i]); - } - } + applyConfiguration(); + processSlotEvents(); // Maintain slot connections (token renewal, reconnect with backoff) maintainSlotConnections(); @@ -3451,11 +3412,6 @@ void MQTTBridge::loop() { // Periodic configuration check (throttled to avoid spam) checkConfigurationMismatch(); - // Periodic NTP refresh (every hour) — lightweight, non-blocking. - if (_network->isConnected() && millis() - _last_ntp_sync > 3600000) { - refreshNTP(); - } - // Publish status updates (handle millis() overflow correctly). // Read the toggle live from prefs so a CLI/web `set mqtt.status` change // applies without a bridge restart. @@ -3523,7 +3479,7 @@ void MQTTBridge::loop() { // --------------------------------------------------------------------------- void MQTTBridge::onPacketReceived(mesh::Packet *packet) { - if (!_initialized || !_obs->mqtt_packets_enabled || !_obs->mqtt_rx_enabled) return; + if (!_initialized || !_source_obs->mqtt_packets_enabled || !_source_obs->mqtt_rx_enabled) return; // Drop before the queue copy when no configured slot allows this payload // type. A QueuedPacket carries the packet plus up to 256 bytes of raw radio @@ -3541,8 +3497,8 @@ void MQTTBridge::onPacketReceived(mesh::Packet *packet) { } void MQTTBridge::sendPacket(mesh::Packet *packet) { - uint8_t tx_mode = _obs->mqtt_tx_enabled; // Read live from prefs (no restart needed) - if (!_initialized || !_obs->mqtt_packets_enabled || tx_mode == 0) return; + uint8_t tx_mode = _source_obs->mqtt_tx_enabled; // Read live from prefs (no restart needed) + if (!_initialized || !_source_obs->mqtt_packets_enabled || tx_mode == 0) return; // Advert mode: only queue self-originated advert packets if (tx_mode == 2) { @@ -3648,6 +3604,7 @@ void MQTTBridge::processPacketQueue() { _last_snr = queued.snr; _last_rssi = queued.rssi; _last_raw_timestamp = millis(); + _last_packet_score = queued.score; } bool packet_eligible = false; @@ -3655,7 +3612,7 @@ void MQTTBridge::processPacketQueue() { packet_eligible, queued.has_raw_data ? queued.raw_data : nullptr, queued.has_raw_data ? queued.raw_len : 0, - queued.snr, queued.rssi); + queued.snr, queued.rssi, queued.score); taskYIELD(); // allow higher-priority tasks to run between packet publishes // Publish raw if enabled (live from prefs so `set mqtt.raw` applies without @@ -3783,6 +3740,7 @@ void MQTTBridge::processPacketQueue() { _last_snr = queued.snr; _last_rssi = queued.rssi; _last_raw_timestamp = millis(); + _last_packet_score = queued.score; } bool packet_eligible = false; @@ -3790,7 +3748,7 @@ void MQTTBridge::processPacketQueue() { packet_eligible, queued.has_raw_data ? queued.raw_data : nullptr, queued.has_raw_data ? queued.raw_len : 0, - queued.snr, queued.rssi); + queued.snr, queued.rssi, queued.score); // No taskYIELD() on non-ESP32 platforms (non-FreeRTOS, cooperative scheduling not needed) // Live from prefs so `set mqtt.raw` applies without a bridge restart. @@ -3900,17 +3858,18 @@ uint8_t MQTTBridge::eligiblePacketSlots(uint8_t packet_type, MQTTMessageType typ // from "no broker configured at all" (the pre-existing silent drop). bool MQTTBridge::shouldQueuePacketType(uint8_t packet_type, bool& filtered) { filtered = false; - if (!_obs) return false; + if (!_source_obs) return false; + const RuntimeSnapshot snapshot = _published.read(); uint16_t masks[RUNTIME_MQTT_SLOTS]; bool enabled[RUNTIME_MQTT_SLOTS]; bool any_enabled = false; for (int i = 0; i < RUNTIME_MQTT_SLOTS; ++i) { - masks[i] = _obs->mqtt_slot_packet_filter[i]; + masks[i] = _source_obs->mqtt_slot_packet_filter[i]; // Configured, not allocated — see eligiblePacketSlots(). Gating on the client // here would silently drop every packet received before the post-NTP-sync slot // setup, which is exactly the window the queue exists to cover. - enabled[i] = _slots[i].enabled; + enabled[i] = snapshot.slots[i].enabled; any_enabled = any_enabled || enabled[i]; } if (!any_enabled) return false; @@ -3950,7 +3909,7 @@ bool MQTTBridge::publishStatus() { MQTTMessageBuilder::formatIsoTimestampForMqtt(now_tv.tv_sec, now_tv.tv_usec, _timezone, timestamp, sizeof(timestamp)); snprintf(radio_info, sizeof(radio_info), "%.6f,%.1f,%d,%d", - _prefs->freq, _prefs->bw, _prefs->sf, _prefs->cr); + _radio_metadata.freq, _radio_metadata.bw, _radio_metadata.sf, _radio_metadata.cr); strncpy(origin_id, _device_id, sizeof(origin_id) - 1); origin_id[sizeof(origin_id) - 1] = '\0'; @@ -3958,30 +3917,16 @@ bool MQTTBridge::publishStatus() { char client_version[64]; getClientVersion(client_version, sizeof(client_version)); - // Collect stats on-demand if sources are available - int battery_mv = -1; - int uptime_secs = -1; - int errors = -1; - int noise_floor = -999; - int tx_air_secs = -1; - int rx_air_secs = -1; - int recv_errors = -1; - int packets_sent = -1; - int packets_received = -1; - - if (_board) battery_mv = _board->getBattMilliVolts(); - if (_ms) uptime_secs = _ms->getMillis() / 1000; - if (_dispatcher) { - errors = _dispatcher->getErrFlags(); - tx_air_secs = _dispatcher->getTotalAirTime() / 1000; - rx_air_secs = _dispatcher->getReceiveAirTime() / 1000; - packets_sent = (int)(_dispatcher->getNumSentFlood() + _dispatcher->getNumSentDirect()); - packets_received = (int)(_dispatcher->getNumRecvFlood() + _dispatcher->getNumRecvDirect()); - } - if (_radio) { - noise_floor = (int16_t)_radio->getNoiseFloor(); - recv_errors = (int)_radio->getPacketsRecvErrors(); - } + const RadioStatsSnapshot stats = _radio_stats.read(); + const int battery_mv = stats.battery_mv; + const int uptime_secs = stats.uptime_secs; + const int errors = stats.errors; + const int noise_floor = stats.noise_floor; + const int tx_air_secs = stats.tx_air_secs; + const int rx_air_secs = stats.rx_air_secs; + const int recv_errors = stats.recv_errors; + const int packets_sent = stats.packets_sent; + const int packets_received = stats.packets_received; // Internal heap free (for diagnosing repeater hangs from internal heap exhaustion) int internal_heap_free = (int)heap_caps_get_free_size(MALLOC_CAP_INTERNAL); @@ -3993,7 +3938,7 @@ bool MQTTBridge::publishStatus() { battery_mv, uptime_secs, errors, _queue_count, noise_floor, tx_air_secs, rx_air_secs, recv_errors, internal_heap_free, packets_sent, packets_received, - _prefs->disable_fwd ? "off" : "on" + _radio_metadata.disable_fwd ? "off" : "on" ); if (len > 0) { @@ -4025,7 +3970,7 @@ bool MQTTBridge::publishStatus() { bool MQTTBridge::publishPacket(mesh::Packet* packet, bool is_tx, bool& has_eligible_target, const uint8_t* raw_data, int raw_len, - float snr, float rssi) { + float snr, float rssi, float captured_score) { has_eligible_target = false; if (!packet) return false; @@ -4040,11 +3985,9 @@ bool MQTTBridge::publishPacket(mesh::Packet* packet, bool is_tx, // Memory pressure check: Skip publishes when there's not enough contiguous // heap for the publish itself (JSON buffer + esp-mqtt outbox frame + WiFi TX - // path). Headroom only — NOT an mbedTLS preflight: persistent clients keep - // their TLS contexts allocated for the bridge lifetime, so the old ~52 KB - // "reserve space for reconnect" guard is obsolete post Phase 1. Publish - // payload is capped at PUBLISH_JSON_BUFFER_SIZE (2 KB); 8 KB is a safe - // ceiling including esp-mqtt frame overhead and transient TCP buffers. + // path). This is publish headroom, not a guarantee that a later TLS handshake + // can allocate its record buffers. Payload is capped at 2 KB; the remaining + // headroom covers MQTT frame overhead and transient TCP buffers. #ifdef ESP32 #if defined(BOARD_HAS_PSRAM) static const size_t PUBLISH_SKIP_MAX_ALLOC_THRESHOLD = 16000; @@ -4096,21 +4039,21 @@ bool MQTTBridge::publishPacket(mesh::Packet* packet, bool is_tx, origin_id[sizeof(origin_id) - 1] = '\0'; // Firmware rebroadcast "score" for this packet — only meaningful for RX packets - // (depends on receive SNR). Recomputed here exactly as Dispatcher::checkRecv() does, - // via the radio's packetScore(snr, len); NaN signals "omit" (tx, or no radio). + // (depends on receive SNR). Captured on the mesh task using packetScore(snr, len); + // NaN signals "omit" (tx, or no radio). // buildPacketMessage scales it x1000 to match the integer in the serial RX log. // Build packet message using raw radio data if provided int len; if (raw_data && raw_len > 0) { - float score = (_radio && !is_tx) ? _radio->packetScore(snr, raw_len) : NAN; + float score = captured_score; len = MQTTMessageBuilder::buildPacketJSONFromRaw( _json_scratch_doc, raw_data, raw_len, packet, is_tx, _origin, origin_id, snr, rssi, score, _timezone, active_buffer, active_buffer_size ); } else if (!is_tx && _last_raw_data && _last_raw_len > 0 && (millis() - _last_raw_timestamp) < 1000) { - float score = _radio ? _radio->packetScore(_last_snr, _last_raw_len) : NAN; + float score = _last_packet_score; len = MQTTMessageBuilder::buildPacketJSONFromRaw( _json_scratch_doc, _last_raw_data, _last_raw_len, packet, is_tx, _origin, origin_id, @@ -4127,7 +4070,7 @@ bool MQTTBridge::publishPacket(mesh::Packet* packet, bool is_tx, rlen = packet->writeTo(reconstructed); } if (rlen > 0) { - float score = (_radio && !is_tx) ? _radio->packetScore(snr, rlen) : NAN; + float score = captured_score; len = MQTTMessageBuilder::buildPacketJSONFromRaw( _json_scratch_doc, reconstructed, rlen, packet, is_tx, _origin, origin_id, @@ -4326,6 +4269,15 @@ void MQTTBridge::queuePacket(mesh::Packet* packet, bool is_tx) { } } + queued.score = NAN; + if (_radio && !is_tx) { + int score_len = queued.has_raw_data ? queued.raw_len : 0; + if (!score_len && MQTTMessageBuilder::canSerializePacket(packet, sizeof(queued.raw_data))) { + score_len = packet->writeTo(queued.raw_data); + } + if (score_len > 0) queued.score = _radio->packetScore(queued.snr, score_len); + } + // Try to send to queue (non-blocking) if (xQueueSend(_packet_queue_handle, &queued, 0) != pdTRUE) { const MQTTPacketQueuePolicy::EnqueueAction action = @@ -4350,8 +4302,7 @@ void MQTTBridge::queuePacket(mesh::Packet* packet, bool is_tx) { } } - UBaseType_t queue_messages = uxQueueMessagesWaiting(_packet_queue_handle); - _queue_count = queue_messages; + // Queue depth is sampled by the worker; the capture task does not write its cache. #else // Non-ESP32: Use circular buffer const MQTTPacketQueuePolicy::EnqueueAction action = @@ -4391,6 +4342,15 @@ void MQTTBridge::queuePacket(mesh::Packet* packet, bool is_tx) { } } + queued.score = NAN; + if (_radio && !is_tx) { + int score_len = queued.has_raw_data ? queued.raw_len : 0; + if (!score_len && MQTTMessageBuilder::canSerializePacket(packet, sizeof(queued.raw_data))) { + score_len = packet->writeTo(queued.raw_data); + } + if (score_len > 0) queued.score = _radio->packetScore(queued.snr, score_len); + } + _queue_tail = (_queue_tail + 1) % MAX_QUEUE_SIZE; _queue_count++; #endif @@ -4440,10 +4400,6 @@ void MQTTBridge::storeRawRadioData(const uint8_t* raw_data, int len, float snr, // as every other sync and touches nothing until a reply passes every check. // One attempt per server bounds the blocking walk on a network that drops // UDP/123 to about one probe timeout per server, instead of three. -void MQTTBridge::refreshNTP() { - syncTimeWithNTP(/*force=*/true, /*primary_only=*/false, /*attempts_per_server=*/1); -} - // One validated NTP exchange with one named server, on a fresh ephemeral socket. // // Replaces NTPClient, which accepted any datagram that arrived on its fixed @@ -4551,16 +4507,7 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only, int attempts_per return false; } - unsigned long now = millis(); - if (!force && _ntp_synced && (now - _last_ntp_sync) < 5000) { - return false; - } - - static bool sync_in_progress = false; - if (sync_in_progress) { - return false; - } - sync_in_progress = true; + if (!_ntp_schedule.begin((uint32_t)millis(), force)) return false; MQTT_DEBUG_PRINTLN("Syncing time with NTP..."); @@ -4674,8 +4621,8 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only, int attempts_per bool was_ntp_synced = _ntp_synced; _ntp_synced = true; - _last_ntp_sync = millis(); - sync_in_progress = false; + _ntp_schedule.complete((uint32_t)millis(), ntp_server_used + ? NtpSchedule::Result::NetworkTime : NtpSchedule::Result::Holdover); MQTT_DEBUG_PRINTLN("Time synced: %lu (via %s)", epochTime, ntp_server_used ? ntp_server_used : "existing clock"); @@ -4758,41 +4705,23 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only, int attempts_per } MQTT_DEBUG_PRINTLN("NTP sync failed"); - sync_in_progress = false; + _ntp_schedule.complete((uint32_t)millis(), NtpSchedule::Result::Failed); return false; } bool MQTTBridge::requestForcedNtpSync(uint32_t timeout_ms) { + (void)timeout_ms; // Retained for source compatibility; requests never block. if (!isRunning()) return false; - - // Publish the request to the MQTT task. Clear the completion flags before - // raising _ntp_force_requested so the task can't observe a stale result. - _ntp_force_done = false; - _ntp_force_result = false; - _ntp_force_requested = true; - - // Fire-and-forget: callers on the Arduino loop task (web config batch, and - // the CLI which shares that task) must not block up to 30 s polling the MQTT - // task — that stalls mesh/radio forwarding, portal DNS, and reboot timers. - // The task still performs the sync; the result is observable via - // `get mqtt.ntp.diag`. Blocking callers pass a non-zero timeout. - if (timeout_ms == 0) return true; - - unsigned long start = millis(); - while (!_ntp_force_done) { - if (millis() - start >= timeout_ms) { - MQTT_DEBUG_PRINTLN("Forced NTP sync timed out waiting for MQTT task"); - return false; // task still running; result is ignored when it eventually completes - } - vTaskDelay(pdMS_TO_TICKS(50)); - } - return _ntp_force_result; + publishConfiguration(); + _ntp_force_requested.store(true, std::memory_order_release); + return true; } // Runs on the MQTT task (Core 0). Probes every configured NTP server for connectivity // and records the time each reports. Deliberately does NOT call configTime() or update // the RTC — this is a read-only diagnostic and must leave the system clock untouched. -void MQTTBridge::runNtpDiagProbe() { +void MQTTBridge::runNtpDiagProbe(uint32_t request_id) { + NtpDiagReport report; const char* servers[kMaxNtpServers]; int count = 0; fillNtpServerList(_obs, servers, count); @@ -4803,43 +4732,46 @@ void MQTTBridge::runNtpDiagProbe() { uint32_t epoch = 0; const char* why = nullptr; bool ok = probeNtpServer(servers[i], kNtpMinValidEpoch, &epoch, &why); - NtpDiagResult& r = _ntp_diag_results[i]; + if (_stop_requested.load(std::memory_order_acquire)) { + _ntp_diag_job.cancel(millis()); + return; + } + NtpDiagResult& r = report.servers[i]; strncpy(r.server, servers[i], sizeof(r.server) - 1); r.server[sizeof(r.server) - 1] = '\0'; r.ok = ok; r.epoch = ok ? epoch : 0; - r.why = why; // static literal from NtpValidation/probeNtpServer + snprintf(r.why, sizeof(r.why), "%s", why ? why : "no reply"); } - _ntp_diag_count = count; + report.count = count; + _ntp_diag_job.complete(request_id, millis(), report); } bool MQTTBridge::ntpDiag(char* reply, size_t reply_size, bool verbose) { if (!isRunning() || reply == nullptr || reply_size == 0) return false; - // Marshal the probe onto the MQTT task (Core 0); clear the completion flag first. - _ntp_diag_done = false; - _ntp_diag_requested = true; - - unsigned long start = millis(); - while (!_ntp_diag_done) { - if (millis() - start >= 30000) { - snprintf(reply, reply_size, "Error: NTP diag timed out"); - return true; - } - vTaskDelay(pdMS_TO_TICKS(50)); + using Job = ObserverAsyncJob; + auto snapshot = _ntp_diag_job.read(); + if (snapshot.state != Job::State::Complete || (uint32_t)(millis() - snapshot.finished_ms) >= 30000) { + publishConfiguration(); + _ntp_diag_job.request(millis()); + snapshot = _ntp_diag_job.read(); + snprintf(reply, reply_size, "> NTP probe #%lu %s; repeat get mqtt.ntp.diag for result", + (unsigned long)snapshot.id, snapshot.state == Job::State::Running ? "running" : "queued"); + return true; } - + const NtpDiagReport& report = snapshot.result; int ok_count = 0; - for (int i = 0; i < _ntp_diag_count; i++) { - if (_ntp_diag_results[i].ok) ok_count++; + for (int i = 0; i < report.count; i++) { + if (report.servers[i].ok) ok_count++; } if (verbose) { // Detailed table to the serial console; reply carries a short summary (the operator // sees the table above the "-> " line). Mirrors the dumpLogFile() convention. - Serial.printf("NTP diag - %d server(s):\r\n", _ntp_diag_count); - for (int i = 0; i < _ntp_diag_count; i++) { - const NtpDiagResult& r = _ntp_diag_results[i]; + Serial.printf("NTP diag - %d server(s):\r\n", report.count); + for (int i = 0; i < report.count; i++) { + const NtpDiagResult& r = report.servers[i]; if (r.ok) { time_t t = (time_t)r.epoch; struct tm tmv; @@ -4848,16 +4780,20 @@ bool MQTTBridge::ntpDiag(char* reply, size_t reply_size, bool verbose) { r.server, tmv.tm_year + 1900, tmv.tm_mon + 1, tmv.tm_mday, tmv.tm_hour, tmv.tm_min, tmv.tm_sec); } else { - Serial.printf(" %-20s FAIL %s\r\n", r.server, r.why ? r.why : "no reply"); + Serial.printf(" %-20s FAIL %s\r\n", r.server, r.why); } } - snprintf(reply, reply_size, "> NTP diag: %d/%d OK (see console)", ok_count, _ntp_diag_count); + snprintf(reply, reply_size, "> NTP #%lu age:%lus %d/%d OK (see console)", + (unsigned long)snapshot.id, (unsigned long)((uint32_t)(millis() - snapshot.finished_ms) / 1000), + ok_count, report.count); } else { // Compact " ok|fail" list for LoRa, bounded to reply_size. - size_t used = 0; - reply[0] = '\0'; - for (int i = 0; i < _ntp_diag_count; i++) { - const NtpDiagResult& r = _ntp_diag_results[i]; + int header = 0; + replyAppendf(reply, reply_size, &header, "> NTP #%lu age:%lus\n", + (unsigned long)snapshot.id, (unsigned long)((uint32_t)(millis() - snapshot.finished_ms) / 1000)); + size_t used = header; + for (int i = 0; i < report.count; i++) { + const NtpDiagResult& r = report.servers[i]; int n = snprintf(reply + used, reply_size - used, "%s%s %s", used ? "\n" : "", r.server, r.ok ? "ok" : "fail"); if (n < 0 || (size_t)n >= reply_size - used) { @@ -5120,11 +5056,13 @@ void MQTTBridge::logMemoryStatus() { // --------------------------------------------------------------------------- void MQTTBridge::setOrigin(const char* origin) { + if (!mayEditStartupMetadata()) return; strncpy(_origin, origin, sizeof(_origin) - 1); _origin[sizeof(_origin) - 1] = '\0'; } void MQTTBridge::setIATA(const char* iata) { + if (!mayEditStartupMetadata()) return; strncpy(_iata, iata, sizeof(_iata) - 1); _iata[sizeof(_iata) - 1] = '\0'; for (int i = 0; _iata[i]; i++) { @@ -5133,40 +5071,39 @@ void MQTTBridge::setIATA(const char* iata) { } void MQTTBridge::setDeviceID(const char* device_id) { + if (!mayEditStartupMetadata()) return; strncpy(_device_id, device_id, sizeof(_device_id) - 1); _device_id[sizeof(_device_id) - 1] = '\0'; MQTT_DEBUG_PRINTLN("Device ID set to: %s", _device_id); } void MQTTBridge::setFirmwareVersion(const char* firmware_version) { + if (!mayEditStartupMetadata()) return; strncpy(_firmware_version, firmware_version, sizeof(_firmware_version) - 1); _firmware_version[sizeof(_firmware_version) - 1] = '\0'; } void MQTTBridge::setBoardModel(const char* board_model) { + if (!mayEditStartupMetadata()) return; strncpy(_board_model, board_model, sizeof(_board_model) - 1); _board_model[sizeof(_board_model) - 1] = '\0'; } void MQTTBridge::setBuildDate(const char* build_date) { + if (!mayEditStartupMetadata()) return; strncpy(_build_date, build_date, sizeof(_build_date) - 1); _build_date[sizeof(_build_date) - 1] = '\0'; } void MQTTBridge::setMessageTypes(bool status, bool packets, bool raw) { + if (!mayEditStartupMetadata()) return; _status_enabled = status; _packets_enabled = packets; _raw_enabled = raw; } int MQTTBridge::getConnectedBrokers() const { - int count = 0; - for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { - if (_slots[i].enabled && _slots[i].connected) { - count++; - } - } - return count; + return _published.read().connected; } int MQTTBridge::getQueueSize() const { @@ -5182,10 +5119,12 @@ int MQTTBridge::getQueueSize() const { void MQTTBridge::setStatsSources(mesh::Dispatcher* dispatcher, mesh::Radio* radio, mesh::MainBoard* board, mesh::MillisecondClock* ms) { + if (!mayEditStartupMetadata()) return; _dispatcher = dispatcher; _radio = radio; _board = board; _ms = ms; + publishRadioStats(); } #endif diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index 1126da0bcb..ca8a610322 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -15,6 +15,10 @@ #include "helpers/AlertFaultPolicy.h" #include "helpers/MQTTEffectiveConfig.h" #include "helpers/MQTTClientState.h" +#include "helpers/NtpSchedule.h" +#include "helpers/MQTTEventChannel.h" +#include "helpers/ObserverConfigMailbox.h" +#include "helpers/ObserverAsyncJob.h" #include #ifdef WITH_SNMP @@ -129,9 +133,10 @@ class MQTTBridge : public BridgeBase { // accept/reject decision for a late callback is made on client_state, which // the bridge task owns. uint32_t generation; + uint32_t incarnation; const MQTTPresetDef* preset; // Points to MQTT_PRESETS[] entry, nullptr for custom/none bool enabled; // true when preset is not "none" - bool connected; // Updated in callbacks + bool connected; // Updated by the worker from event records bool initial_connect_done; // True after first connect() call // JWT auth state (used by preset JWT slots and custom slots with audience set). @@ -192,10 +197,62 @@ class MQTTBridge : public BridgeBase { }; MQTTSlot _slots[RUNTIME_MQTT_SLOTS]; + MqttEventChannel<32> _slot_events; + void processSlotEvents(); + struct SlotRuntimeSnapshot { + char name[32]{}; + char state[16]{}; + bool configured = false; + bool enabled = false; + bool client = false; + bool connected = false; + bool attempted = false; + bool ready = false; + bool circuit_breaker_tripped = false; + ClientState client_state = ClientState::Absent; + uint16_t filter_mask = 0; + unsigned long publish_ok = 0, publish_err = 0; + uint32_t disconnect_count = 0; + unsigned long first_disconnect_time = 0, current_outage_started_ms = 0; + unsigned long last_error_time = 0; + int32_t last_tls_err = 0, last_tls_stack_err = 0; + int last_sock_errno = 0; + uint8_t last_connack_code = 0; + }; + struct RuntimeSnapshot { + uint32_t sampled_ms = 0, sequence = 0; + uint32_t desired_revision = 0, applied_revision = 0; + bool running = false; + int queue = 0, connected = 0, skipped = 0; + uint32_t free_heap = 0, max_heap = 0, event_overflows = 0; + unsigned long outbox = 0, outbox_drops = 0, filtered = 0; + uint8_t neighbors_phase = 0, neighbors_result = 0; + uint32_t neighbors_seconds = 0; + SlotRuntimeSnapshot slots[RUNTIME_MQTT_SLOTS]; + }; + static ObserverMailbox _published; + uint32_t _snapshot_sequence = 0; + // Snapshots publish every worker pass; the heap walk behind getMaxAllocHeap() + // is sampled at most once per second and reused between samples. + uint32_t _heap_sample_ms = 0; + uint32_t _heap_sample_free = 0, _heap_sample_max = 0; + bool _heap_sampled = false; + void publishRuntimeSnapshot(); + bool mayEditStartupMetadata(); // JWT username shared across all JWT-auth slots (same device identity) char _jwt_username[70]; // Format: v1_{UPPERCASE_PUBLIC_KEY} + struct RadioStatsSnapshot { + int battery_mv = -1, uptime_secs = -1, errors = -1, noise_floor = -999; + int tx_air_secs = -1, rx_air_secs = -1, recv_errors = -1; + int packets_sent = -1, packets_received = -1, state = -1; + uint32_t last_rx_age_ms = 0; + }; + ObserverMailbox _radio_stats; + uint32_t _last_radio_sample_ms = 0; + void publishRadioStats(); // loop task + // Message configuration char _origin[32]; char _iata[8]; @@ -223,6 +280,7 @@ class MQTTBridge : public BridgeBase { bool is_tx; float snr; float rssi; + float score; // Raw radio bytes embedded at enqueue time (Core 1), never shared across cores. // On non-PSRAM boards the queue is smaller (6 items) to offset the per-item cost. uint8_t raw_data[256]; @@ -260,55 +318,27 @@ class MQTTBridge : public BridgeBase { // NTP time sync. The socket is opened per probe and closed again (see // probeNtpServer); nothing listens between syncs. WiFiUDP _ntp_udp; - unsigned long _last_ntp_sync; + NtpSchedule _ntp_schedule; bool _ntp_synced; - bool _ntp_sync_pending; // Flag to trigger NTP sync from loop() instead of event handler bool _slots_setup_done; // Deferred: slots set up after NTP sync int _max_active_slots; // Runtime limit: 5 with PSRAM, 2 without - // Pending slot reconfigure: set from CLI (Core 1), processed by MQTT task (Core 0) - volatile bool _slot_reconfigure_pending[RUNTIME_MQTT_SLOTS]; - - // A broker refusal can invalidate an otherwise clock-valid JWT. The esp-mqtt - // callback sets this and the bridge loop consumes it; byte access is atomic. - volatile bool _slot_force_jwt_mint[RUNTIME_MQTT_SLOTS]; - - // Pending on-connect status publish: set from the onConnect callback (which - // runs on the esp-mqtt event task, NOT this bridge task), consumed by the MQTT - // task (Core 0). publishStatusToSlot() touches the shared status doc/buffer/ - // origin that publishStatus() also uses, so it must run only on the bridge - // task — the callback just raises this flag. Same idiom as - // _slot_reconfigure_pending; a single-byte volatile store/load is atomic. - volatile bool _status_publish_pending[RUNTIME_MQTT_SLOTS]; - - // CLI-requested forced NTP sync, marshalled onto the MQTT task (Core 0). - // All NTP I/O must run on Core 0; the CLI thread - // (Core 1) sets _ntp_force_requested and blocks in requestForcedNtpSync() - // until the task publishes the outcome via _ntp_force_result/_ntp_force_done. - // Single-requester assumption: CLI commands are serialized, so at most one - // forced sync is outstanding at a time. - volatile bool _ntp_force_requested; - volatile bool _ntp_force_done; - volatile bool _ntp_force_result; - - // CLI-requested NTP connectivity diagnostic, marshalled onto the MQTT task (Core 0) - // with the same handshake as the forced sync. Probe-only: it queries each server and - // records the reported time but never sets the system clock or the RTC, so the - // system clock is left untouched. Results are written by the task and read by the CLI - // thread once _ntp_diag_done is set. - volatile bool _ntp_diag_requested; - volatile bool _ntp_diag_done; + // Worker-owned effects requested by applied SDK event records. + bool _slot_force_jwt_mint[RUNTIME_MQTT_SLOTS]; + bool _status_publish_pending[RUNTIME_MQTT_SLOTS]; + + std::atomic _ntp_force_requested{false}; struct NtpDiagResult { - char server[64]; - bool ok; - uint32_t epoch; // server-reported UTC epoch when ok - // Why a probe failed, as a static literal ("DNS failed", "unsolicited - // reply", ...). Previously every failure looked alike, and a name that never - // resolved could be credited with another server's reply. - const char* why; + char server[64]{}; + bool ok = false; + uint32_t epoch = 0; + char why[48]{}; + }; + struct NtpDiagReport { + NtpDiagResult servers[kMaxNtpServers]; + int count = 0; }; - NtpDiagResult _ntp_diag_results[kMaxNtpServers]; - int _ntp_diag_count; + ObserverAsyncJob _ntp_diag_job; // Cooperative-shutdown handshake. The loop task (Core 1) raises // _stop_requested through the lifecycle Coordinator; the MQTT task (Core 0) @@ -341,6 +371,7 @@ class MQTTBridge : public BridgeBase { uint8_t _staged_raw[LAST_RAW_DATA_SIZE]; int _staged_raw_len = 0; float _staged_snr = 0.0f; + float _last_packet_score = NAN; float _staged_rssi = 0.0f; bool _staged_raw_valid = false; @@ -424,10 +455,8 @@ class MQTTBridge : public BridgeBase { unsigned long _last_memory_check; bool _memory_pressure = false; // Cached max-alloc verdict; re-sampled at most once per interval in publishPacket() so the heap walk isn't paid per-packet under pressure int _skipped_publishes; // Exposed via SNMP; count of publishes skipped when max_alloc is too low - // Packets rejected by the per-slot filters before reaching the queue. Written - // on Core 1 (radio callbacks), read on Core 0 for `mqtt.stats`; a torn read of - // a diagnostic counter is harmless, so no atomic is warranted. - unsigned long _filtered_packets = 0; + // Capture task increments; worker samples for diagnostics. + std::atomic _filtered_packets{0}; // Status publish retry tracking unsigned long _last_status_retry; // Track last retry attempt (separate from successful publish) @@ -436,8 +465,8 @@ class MQTTBridge : public BridgeBase { // Device identity for JWT token creation mesh::LocalIdentity *_identity; - // Cached connection status (updated in callbacks to avoid redundant checks) - bool _cached_has_connected_slots; + // Worker publishes a cheap connection hint to the capture task. + std::atomic _cached_has_connected_slots; // Queue staleness tracking unsigned long _queue_disconnected_since; // 0 = has connected slots @@ -480,10 +509,8 @@ class MQTTBridge : public BridgeBase { // - destroySlotClients() disconnects and deletes each client. Runs once in end(). // - setupSlot() ensures the client exists, then configures it (server, // credentials, CA) and calls connect(). Safe to call again to reconfigure. - // - teardownSlot() only disconnects — it never deletes the client. Leaves - // the mbedTLS/transport state ready for a subsequent setupSlot(). - // This avoids delete/new cycles that shed ~40 KB of mbedTLS buffers per - // reconfigure and fragment the internal heap on non-PSRAM boards. + // - teardownSlot() disconnects without deleting the wrapper. TLS session + // buffers follow the transport connection lifetime, not wrapper lifetime. bool ensureSlotClient(int index); // Allocate this slot's persistent client + callbacks on first use // Stop, destroy and re-allocate this slot's client. Only for configuration // changes that cannot be applied to a live client, and only after the stop is @@ -555,8 +582,8 @@ class MQTTBridge : public BridgeBase { void initializeNetworkInTask(); // Selected-link initialization moved to task #endif bool publishPacket(mesh::Packet* packet, bool is_tx, bool& has_eligible_target, - const uint8_t* raw_data = nullptr, int raw_len = 0, - float snr = 0.0f, float rssi = 0.0f); + const uint8_t* raw_data, int raw_len, + float snr, float rssi, float captured_score); bool publishRaw(mesh::Packet* packet, bool& has_eligible_target); #if defined(WITH_MQTT_NEIGHBORS) // Publishes the pending _neighbors_json_buffer to every connected slot's @@ -566,8 +593,7 @@ class MQTTBridge : public BridgeBase { void queuePacket(mesh::Packet* packet, bool is_tx); void dequeuePacket(); bool isAnySlotConnected(); - void refreshNTP(); // Lightweight periodic NTP refresh (non-blocking) - void runNtpDiagProbe(); // Probe every server for connectivity; never sets the clock. Core 0 only. + void runNtpDiagProbe(uint32_t request_id); // Probe every server for connectivity; never sets the clock. Core 0 only. // One validated NTP exchange with one server on a fresh ephemeral socket. // Core 0 only; never touches the clock. *why receives a static reason literal. bool probeNtpServer(const char* server, uint32_t min_epoch, @@ -613,7 +639,20 @@ class MQTTBridge : public BridgeBase { // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt.json. // _prefs (held by BridgeBase) still provides upstream fields (freq/sf/node_name…). - MQTTPrefs* _obs = nullptr; + struct RadioMetadata { + char node_name[32]{}; + float freq = 0, bw = 0; + uint8_t sf = 0, cr = 0, disable_fwd = 0; + }; + MQTTPrefs _runtime_prefs{}; + RadioMetadata _radio_metadata{}; + ObserverConfigMailbox _configuration; + uint32_t _applied_revision = 0; + uint32_t _last_config_sample_ms = 0; + const MQTTPrefs* _source_obs = nullptr; // loop-task owned + MQTTPrefs* _obs = &_runtime_prefs; // worker owned + void publishConfiguration(uint32_t reconfigure = 0); // loop task + void applyConfiguration(); // worker NetworkLink* _network = nullptr; public: @@ -729,8 +768,7 @@ class MQTTBridge : public BridgeBase { // Honours a stop acknowledgement that arrived after the deadline: releases the // withheld resources and makes the bridge restartable. Loop task only. void pollLateStopAck(); - // Survives end() clearing the diagnostic singleton, so `get mqtt.status` can - // still explain why a stopped bridge will not come back without a reboot. + // Retains the reason for a stopped bridge until late acknowledgment or restart. static bool stopUnprovenLatched(); static unsigned long getWifiConnectedAtMillis(); @@ -767,6 +805,12 @@ class MQTTBridge : public BridgeBase { unsigned long getSlotCurrentOutageStartMs(int slot_index) const; bool isSlotEnabledAndAttempted(int slot_index) const; const char* getSlotPresetName(int slot_index) const; + struct SlotOutageSnapshot { + bool monitored = false; + uint32_t started_ms = 0; + char name[32]{}; + }; + SlotOutageSnapshot getSlotOutageSnapshot(int slot_index) const; static int getRuntimeSlotCount() { return RUNTIME_MQTT_SLOTS; } /** Max slots that can be connected at once: 5 with PSRAM, 2 without (each * WSS/TLS connection needs ~40KB for mbedTLS buffers). This is the number of @@ -776,35 +820,26 @@ class MQTTBridge : public BridgeBase { /** Resolved origin for MQTT JSON: node_name when mqtt_origin is empty, else mqtt_origin (with quote stripping). */ static void getEffectiveMqttOrigin(const NodePrefs* np, const MQTTPrefs* obs, char* buf, size_t buf_size); static const char* effectiveNtpPrimary(const MQTTPrefs* obs); - /** Sync system clock via NTP. force=true bypasses the 5s post-sync rate limit. - * primary_only=true tests just the effective primary server (no fallback walk) so a - * mistyped hostname fails fast instead of blocking through the whole fallback list. - * Performs blocking NTP I/O and must only be called from the MQTT task (Core 0). - * Other tasks (e.g. the CLI on Core 1) must use requestForcedNtpSync() instead. */ + // Worker only: blocking validated NTP I/O; force bypasses the retry deadline. bool syncTimeWithNTP(bool force = false, bool primary_only = false, int attempts_per_server = 2); - /** Request a forced NTP sync from another task (e.g. CLI on Core 1). Marshals the - * work onto the MQTT task so all NTP I/O stays on Core 0, then blocks up to - * timeout_ms for the result. Returns true if the sync succeeded, false on failure, - * timeout, or if the bridge is not running. */ - bool requestForcedNtpSync(uint32_t timeout_ms = 30000); - /** Probe every configured NTP server (custom primary + built-in fallbacks) for - * connectivity and report each server's reported time WITHOUT touching the system - * clock. Marshals the probe onto the MQTT task (Core 0), then formats on the caller's - * thread. verbose=true prints a detailed table to the serial console and leaves a short - * summary in reply; verbose=false fills reply with a compact " ok|fail" list - * (for LoRa). Returns false if the bridge is not running. */ + // Loop task: queues a primary-server sync and returns immediately. The legacy + // timeout argument is ignored; no production caller may block the mesh loop. + bool requestForcedNtpSync(uint32_t timeout_ms = 0); + // First call queues a probe; repeat to read its ID/age/results. Completed + // results are cached for 30 seconds. Probing never changes the clock. bool ntpDiag(char* reply, size_t reply_size, bool verbose); static void formatMqttStatusReply(char* buf, size_t bufsize, const MQTTPrefs* obs); /** On-demand publish-health + heap snapshot for `get mqtt.stats` (per-slot ok/err, * outbox size, free/max heap, queue depth). */ static void formatMqttStatsReply(char* buf, size_t bufsize); + static void formatRuntimeReply(char* buf, size_t bufsize); /** Structured per-slot snapshot for the webconfig stats endpoint. Same state - * logic as formatMqttStatusReply, same cross-task read semantics. Returns + * logic as formatMqttStatusReply, copied from the published worker snapshot. Returns * false when the bridge is not running, the index is out of range, or the * slot is unconfigured (skip it). */ struct SlotStatusSnapshot { - const char* name; // preset name or "custom" - const char* state; // "inactive" | "wait" | "ok" | "fail" | "disc" + char name[32]; // preset name or "custom" + char state[16]; // "inactive" | "wait" | "ok" | "fail" | "disc" unsigned long publish_ok; unsigned long publish_err; // Raw packet-type allowlist. Kept as the mask rather than canonical text so @@ -812,6 +847,7 @@ class MQTTBridge : public BridgeBase { uint16_t filter_mask; }; static bool getSlotStatusSnapshot(int slot_index, SlotStatusSnapshot* out); + static void appendSlotStatsJson(char* buf, size_t size, int pos); /** True when the selected network is configured and at least one MQTT slot can run. */ static bool isConfigValid(const MQTTPrefs* obs); static void formatSlotDiagReply(char* buf, size_t bufsize, int slot_index); @@ -824,7 +860,7 @@ class MQTTBridge : public BridgeBase { mesh::MainBoard* board, mesh::MillisecondClock* ms); #ifdef WITH_SNMP - void setSNMPAgent(MeshSNMPAgent* agent) { _snmp_agent = agent; } + void setSNMPAgent(MeshSNMPAgent* agent) { if (mayEditStartupMetadata()) _snmp_agent = agent; } #endif }; diff --git a/src/helpers/esp32/WebConfigServer.cpp b/src/helpers/esp32/WebConfigServer.cpp index a898872bea..58eab59dc3 100644 --- a/src/helpers/esp32/WebConfigServer.cpp +++ b/src/helpers/esp32/WebConfigServer.cpp @@ -221,6 +221,35 @@ WebConfigServer::WebConfigServer(NodePrefs* prefs, MQTTPrefs* obs, Callbacks* ca : _prefs(prefs), _obs(obs), _cb(callbacks), _pub_key(pub_key), _fw_ver(fw_ver), _build_date(build_date), _role(role), _board_name(board_name) { _mux = xSemaphoreCreateMutex(); + if (_mux) publishPrefs(); +} + +void WebConfigServer::publishPrefs() { + WCLock lock(_mux); + memcpy(&_view_prefs.advert_interval, &_prefs->advert_interval, sizeof(_view_prefs.advert_interval)); + memcpy(&_view_prefs.airtime_factor, &_prefs->airtime_factor, sizeof(_view_prefs.airtime_factor)); + memcpy(&_view_prefs.bw, &_prefs->bw, sizeof(_view_prefs.bw)); + memcpy(&_view_prefs.cad_enabled, &_prefs->cad_enabled, sizeof(_view_prefs.cad_enabled)); + memcpy(&_view_prefs.cr, &_prefs->cr, sizeof(_view_prefs.cr)); + memcpy(&_view_prefs.disable_fwd, &_prefs->disable_fwd, sizeof(_view_prefs.disable_fwd)); + memcpy(&_view_prefs.flood_advert_interval, &_prefs->flood_advert_interval, sizeof(_view_prefs.flood_advert_interval)); + memcpy(&_view_prefs.flood_max, &_prefs->flood_max, sizeof(_view_prefs.flood_max)); + memcpy(&_view_prefs.flood_max_advert, &_prefs->flood_max_advert, sizeof(_view_prefs.flood_max_advert)); + memcpy(&_view_prefs.flood_max_unscoped, &_prefs->flood_max_unscoped, sizeof(_view_prefs.flood_max_unscoped)); + memcpy(&_view_prefs.freq, &_prefs->freq, sizeof(_view_prefs.freq)); + memcpy(&_view_prefs.loop_detect, &_prefs->loop_detect, sizeof(_view_prefs.loop_detect)); + memcpy(&_view_prefs.node_lat, &_prefs->node_lat, sizeof(_view_prefs.node_lat)); + memcpy(&_view_prefs.node_lon, &_prefs->node_lon, sizeof(_view_prefs.node_lon)); + memcpy(&_view_prefs.node_name, &_prefs->node_name, sizeof(_view_prefs.node_name)); + memcpy(&_view_prefs.password, &_prefs->password, sizeof(_view_prefs.password)); + memcpy(&_view_prefs.radio_fem_rxgain, &_prefs->radio_fem_rxgain, sizeof(_view_prefs.radio_fem_rxgain)); + memcpy(&_view_prefs.radio_fem_txgain, &_prefs->radio_fem_txgain, sizeof(_view_prefs.radio_fem_txgain)); + memcpy(&_view_prefs.rx_boosted_gain, &_prefs->rx_boosted_gain, sizeof(_view_prefs.rx_boosted_gain)); + memcpy(&_view_prefs.rx_delay_base, &_prefs->rx_delay_base, sizeof(_view_prefs.rx_delay_base)); + memcpy(&_view_prefs.sf, &_prefs->sf, sizeof(_view_prefs.sf)); + memcpy(&_view_prefs.tx_delay_factor, &_prefs->tx_delay_factor, sizeof(_view_prefs.tx_delay_factor)); + memcpy(&_view_prefs.tx_power_dbm, &_prefs->tx_power_dbm, sizeof(_view_prefs.tx_power_dbm)); + _view_obs = *_obs; } WebConfigServer::~WebConfigServer() { @@ -264,10 +293,15 @@ bool WebConfigServer::isLanSetup() { // --------------------------------------------------------------------------- bool WebConfigServer::startSetupMode(char reply[]) { + if (_mux == nullptr) { + strcpy(reply, "Err: webconfig mutex unavailable"); + return false; + } if (_mode != MODE_OFF || _stopping) { strcpy(reply, "Err: webconfig busy"); return false; } + publishPrefs(); if (!HttpPort80Lease::acquire(HttpPort80Lease::Owner::WebConfig)) { snprintf(reply, 160, "Err: port 80 is in use by %s", HttpPort80Lease::ownerName()); @@ -318,10 +352,15 @@ bool WebConfigServer::startSetupMode(char reply[]) { } bool WebConfigServer::startLanMode(bool initial_setup, char reply[]) { + if (_mux == nullptr) { + strcpy(reply, "Err: webconfig mutex unavailable"); + return false; + } if (_mode != MODE_OFF || _stopping) { strcpy(reply, "Err: webconfig busy"); return false; } + publishPrefs(); if (!HttpPort80Lease::acquire(HttpPort80Lease::Owner::WebConfig)) { snprintf(reply, 160, "Err: port 80 is in use by %s", HttpPort80Lease::ownerName()); @@ -514,6 +553,7 @@ void WebConfigServer::tick(uint32_t now) { } if (_mode == MODE_OFF) return; + publishPrefs(); if (!_board_cmds_probed) probeBoardCommands(); if (_mode == MODE_LAN && _initial_setup && _setup_reminder_at != 0 && @@ -788,8 +828,11 @@ void WebConfigServer::handleStatus(AsyncWebServerRequest* req) { DynamicJsonDocument doc(640); doc["mode"] = (_mode == MODE_SETUP) ? "setup" : "lan"; doc["auth"] = authed; - doc["needs_setup"] = !mqttNetworkSetupComplete(_obs); - doc["name"] = (const char*)_prefs->node_name; + { + WCLock lock(_mux); + doc["needs_setup"] = !mqttNetworkSetupComplete(&_view_obs); + doc["name"] = _view_prefs.node_name; + } char node_id[17]; for (int i = 0; i < 8; i++) sprintf(&node_id[i * 2], "%02x", _pub_key[i]); doc["node_id"] = node_id; @@ -859,9 +902,14 @@ void WebConfigServer::handleLogin(AsyncWebServerRequest* req) { return; } const char* pwd = doc["password"] | ""; - const char* expected = _initial_setup ? _setup_code : _prefs->password; + char password[sizeof(_view_prefs.password)]; + { + WCLock lock(_mux); + memcpy(password, _view_prefs.password, sizeof(password)); + } + const char* expected = _initial_setup ? _setup_code : password; const size_t expected_size = _initial_setup ? sizeof(_setup_code) - : sizeof(_prefs->password); + : sizeof(password); if (!fixedTimeEquals(pwd, expected, expected_size)) { if (++_login_fails >= 5) { _login_lock_until = now + 30000; @@ -924,70 +972,70 @@ void WebConfigServer::handleConfigGet(AsyncWebServerRequest* req) { JsonObject radio = doc.createNestedObject("radio"); // round via double so float error doesn't leak into the JSON // (910.525f would otherwise serialize as 910.5250244) - radio["freq"] = (double)roundf(_prefs->freq * 1000.0f) / 1000.0; - radio["bw"] = (double)roundf(_prefs->bw * 100.0f) / 100.0; - radio["sf"] = _prefs->sf; - radio["cr"] = _prefs->cr; - radio["tx"] = _prefs->tx_power_dbm; - radio["af"] = _prefs->airtime_factor; - radio["rxdelay"] = _prefs->rx_delay_base; - radio["txdelay"] = _prefs->tx_delay_factor; - radio["cad"] = (bool)_prefs->cad_enabled; - radio["rxgain"] = (bool)_prefs->rx_boosted_gain; + radio["freq"] = (double)roundf(_view_prefs.freq * 1000.0f) / 1000.0; + radio["bw"] = (double)roundf(_view_prefs.bw * 100.0f) / 100.0; + radio["sf"] = _view_prefs.sf; + radio["cr"] = _view_prefs.cr; + radio["tx"] = _view_prefs.tx_power_dbm; + radio["af"] = _view_prefs.airtime_factor; + radio["rxdelay"] = _view_prefs.rx_delay_base; + radio["txdelay"] = _view_prefs.tx_delay_factor; + radio["cad"] = (bool)_view_prefs.cad_enabled; + radio["rxgain"] = (bool)_view_prefs.rx_boosted_gain; // FEM gain is driven by Board::handleCommand(), not CommonCLI, so these are // the stored intent; a board with no front-end module rejects the `set`. - radio["fem_rxgain"] = (bool)_prefs->radio_fem_rxgain; - radio["fem_txgain"] = (bool)_prefs->radio_fem_txgain; - radio["repeat"] = !(bool)_prefs->disable_fwd; // CLI `repeat on` == disable_fwd 0 - radio["flood_max"] = _prefs->flood_max; - radio["flood_max_advert"] = _prefs->flood_max_advert; - radio["flood_max_unscoped"] = _prefs->flood_max_unscoped; + radio["fem_rxgain"] = (bool)_view_prefs.radio_fem_rxgain; + radio["fem_txgain"] = (bool)_view_prefs.radio_fem_txgain; + radio["repeat"] = !(bool)_view_prefs.disable_fwd; // CLI `repeat on` == disable_fwd 0 + radio["flood_max"] = _view_prefs.flood_max; + radio["flood_max_advert"] = _view_prefs.flood_max_advert; + radio["flood_max_unscoped"] = _view_prefs.flood_max_unscoped; static const char* const LOOP_MODES[] = { "off", "minimal", "moderate", "strict" }; - radio["loop_detect"] = LOOP_MODES[_prefs->loop_detect <= LOOP_DETECT_STRICT ? _prefs->loop_detect : 0]; - radio["name"] = (const char*)_prefs->node_name; - radio["lat"] = _prefs->node_lat; - radio["lon"] = _prefs->node_lon; - radio["advert_interval"] = _prefs->advert_interval * 2; // stored as mins/2 - radio["flood_advert_interval"] = _prefs->flood_advert_interval; // hours + radio["loop_detect"] = LOOP_MODES[_view_prefs.loop_detect <= LOOP_DETECT_STRICT ? _view_prefs.loop_detect : 0]; + radio["name"] = _view_prefs.node_name; + radio["lat"] = _view_prefs.node_lat; + radio["lon"] = _view_prefs.node_lon; + radio["advert_interval"] = _view_prefs.advert_interval * 2; // stored as mins/2 + radio["flood_advert_interval"] = _view_prefs.flood_advert_interval; // hours JsonObject wifi = doc.createNestedObject("wifi"); - wifi["ssid"] = (const char*)_obs->wifi_ssid; - wifi["pwd"] = _obs->wifi_password[0] ? SECRET_SENTINEL : ""; - wifi["powersave"] = WifiPowerSavePolicy::nameFor(_obs->wifi_power_save); + wifi["ssid"] = _view_obs.wifi_ssid; + wifi["pwd"] = _view_obs.wifi_password[0] ? SECRET_SENTINEL : ""; + wifi["powersave"] = WifiPowerSavePolicy::nameFor(_view_obs.wifi_power_save); JsonObject mqtt = doc.createNestedObject("mqtt"); - mqtt["origin"] = (const char*)_obs->mqtt_origin; - mqtt["iata"] = (const char*)_obs->mqtt_iata; - mqtt["status"] = (bool)_obs->mqtt_status_enabled; - mqtt["packets"] = (bool)_obs->mqtt_packets_enabled; - mqtt["raw"] = (bool)_obs->mqtt_raw_enabled; - mqtt["tx"] = _obs->mqtt_tx_enabled == 2 ? "advert" - : _obs->mqtt_tx_enabled == 1 ? "on" : "off"; - mqtt["rx"] = (bool)_obs->mqtt_rx_enabled; - mqtt["interval"] = _obs->mqtt_status_interval / 60000; // CLI takes minutes - mqtt["neighbors"] = (bool)_obs->mqtt_neighbors_enabled; - mqtt["neighbors_interval"] = _obs->mqtt_neighbors_interval / 3600000UL; // CLI takes hours - mqtt["timezone"] = (const char*)_obs->timezone_string; - mqtt["timezone_offset"] = _obs->timezone_offset; - mqtt["ntp"] = (const char*)_obs->mqtt_ntp_server; - mqtt["owner"] = (const char*)_obs->mqtt_owner_public_key; - mqtt["email"] = (const char*)_obs->mqtt_email; - mqtt["snmp"] = (bool)_obs->snmp_enabled; - mqtt["snmp_community"] = (const char*)_obs->snmp_community; + mqtt["origin"] = _view_obs.mqtt_origin; + mqtt["iata"] = _view_obs.mqtt_iata; + mqtt["status"] = (bool)_view_obs.mqtt_status_enabled; + mqtt["packets"] = (bool)_view_obs.mqtt_packets_enabled; + mqtt["raw"] = (bool)_view_obs.mqtt_raw_enabled; + mqtt["tx"] = _view_obs.mqtt_tx_enabled == 2 ? "advert" + : _view_obs.mqtt_tx_enabled == 1 ? "on" : "off"; + mqtt["rx"] = (bool)_view_obs.mqtt_rx_enabled; + mqtt["interval"] = _view_obs.mqtt_status_interval / 60000; // CLI takes minutes + mqtt["neighbors"] = (bool)_view_obs.mqtt_neighbors_enabled; + mqtt["neighbors_interval"] = _view_obs.mqtt_neighbors_interval / 3600000UL; // CLI takes hours + mqtt["timezone"] = _view_obs.timezone_string; + mqtt["timezone_offset"] = _view_obs.timezone_offset; + mqtt["ntp"] = _view_obs.mqtt_ntp_server; + mqtt["owner"] = _view_obs.mqtt_owner_public_key; + mqtt["email"] = _view_obs.mqtt_email; + mqtt["snmp"] = (bool)_view_obs.snmp_enabled; + mqtt["snmp_community"] = _view_obs.snmp_community; JsonArray slots = mqtt.createNestedArray("slots"); for (int i = 0; i < MAX_MQTT_SLOTS; i++) { JsonObject s = slots.createNestedObject(); - s["preset"] = (const char*)_obs->mqtt_slot_preset[i]; - s["server"] = (const char*)_obs->mqtt_slot_host[i]; - s["port"] = _obs->mqtt_slot_port[i]; - s["username"] = (const char*)_obs->mqtt_slot_username[i]; - s["password"] = _obs->mqtt_slot_password[i][0] ? SECRET_SENTINEL : ""; - s["token"] = _obs->mqtt_slot_token[i][0] ? SECRET_SENTINEL : ""; - s["topic"] = (const char*)_obs->mqtt_slot_topic[i]; - s["audience"] = (const char*)_obs->mqtt_slot_audience[i]; + s["preset"] = _view_obs.mqtt_slot_preset[i]; + s["server"] = _view_obs.mqtt_slot_host[i]; + s["port"] = _view_obs.mqtt_slot_port[i]; + s["username"] = _view_obs.mqtt_slot_username[i]; + s["password"] = _view_obs.mqtt_slot_password[i][0] ? SECRET_SENTINEL : ""; + s["token"] = _view_obs.mqtt_slot_token[i][0] ? SECRET_SENTINEL : ""; + s["topic"] = _view_obs.mqtt_slot_topic[i]; + s["audience"] = _view_obs.mqtt_slot_audience[i]; char filter_text[MQTTPacketFilter::kFilterTextSize]; - if (MQTTPacketFilter::format(_obs->mqtt_slot_packet_filter[i], + if (MQTTPacketFilter::format(_view_obs.mqtt_slot_packet_filter[i], filter_text, sizeof(filter_text))) { // Mutable char input is copied into the ArduinoJson document; the // stack buffer is reused on the next slot. diff --git a/src/helpers/esp32/WebConfigServer.h b/src/helpers/esp32/WebConfigServer.h index bdd3e27252..1f60fed51a 100644 --- a/src/helpers/esp32/WebConfigServer.h +++ b/src/helpers/esp32/WebConfigServer.h @@ -28,6 +28,7 @@ #define WITH_WEBCONFIG 1 #include +#include "../CommonCLI.h" #include #include #include @@ -125,6 +126,33 @@ class WebConfigServer { char reply[160]; // CLI reply budget, same 160 bytes the serial console gets }; + struct PrefsSnapshot { + decltype(NodePrefs::advert_interval) advert_interval{}; + decltype(NodePrefs::airtime_factor) airtime_factor{}; + decltype(NodePrefs::bw) bw{}; + decltype(NodePrefs::cad_enabled) cad_enabled{}; + decltype(NodePrefs::cr) cr{}; + decltype(NodePrefs::disable_fwd) disable_fwd{}; + decltype(NodePrefs::flood_advert_interval) flood_advert_interval{}; + decltype(NodePrefs::flood_max) flood_max{}; + decltype(NodePrefs::flood_max_advert) flood_max_advert{}; + decltype(NodePrefs::flood_max_unscoped) flood_max_unscoped{}; + decltype(NodePrefs::freq) freq{}; + decltype(NodePrefs::loop_detect) loop_detect{}; + decltype(NodePrefs::node_lat) node_lat{}; + decltype(NodePrefs::node_lon) node_lon{}; + decltype(NodePrefs::node_name) node_name{}; + decltype(NodePrefs::password) password{}; + decltype(NodePrefs::radio_fem_rxgain) radio_fem_rxgain{}; + decltype(NodePrefs::radio_fem_txgain) radio_fem_txgain{}; + decltype(NodePrefs::rx_boosted_gain) rx_boosted_gain{}; + decltype(NodePrefs::rx_delay_base) rx_delay_base{}; + decltype(NodePrefs::sf) sf{}; + decltype(NodePrefs::tx_delay_factor) tx_delay_factor{}; + decltype(NodePrefs::tx_power_dbm) tx_power_dbm{}; + } _view_prefs; + MQTTPrefs _view_obs{}; + void publishPrefs(); // loop task, between commands NodePrefs* _prefs; MQTTPrefs* _obs; Callbacks* _cb; diff --git a/test/README.md b/test/README.md index ee33b4c5b5..78fb62b195 100644 --- a/test/README.md +++ b/test/README.md @@ -12,12 +12,26 @@ separately; see "Local testing without hardware" in `MQTT_IMPLEMENTATION.md`. ```sh pio test -e native # all suites pio test -e native -f test_webconfig_keys # a single suite +pio test -e native -e native_sanitized -e native_kiss_modem # CI verification ``` A green `[PASSED]` per suite means GoogleTest returned 0 (all assertions -passed). PlatformIO's "0 test cases" line is just its Unity-style counter and -does not reflect the GoogleTest count — run the built binary directly -(`.pio/build/native/program`) to see the per-assertion breakdown. +passed). The environments use PlatformIO's GoogleTest runner, which reports +individual test cases. `native_sanitized` adds AddressSanitizer and UBSan; +it does not validate ESP32 task scheduling or the precompiled SDK. + +The observer reliability suites call helpers used by the firmware: + +- `test_observer_handoffs`: concurrent snapshot/configuration copies, coalesced + reconfigure requests, callback ordering and overflow, retired clients, + asynchronous job cancellation, durable preference commits, and stats JSON. +- `test_observer_init`: failed client allocation/event registration and required + transport-buffer allocation cleanup. +- `test_ntp_schedule`: completion-based refresh/retry deadlines, forced requests, + clock holdover, and millisecond rollover. + +These cover the extracted production operations, not the complete bridge or +AsyncTCP routes. Device validation remains required; see `MQTT_OWNERSHIP.md`. ## Suites diff --git a/test/test_ntp_schedule/test_ntp_schedule.cpp b/test/test_ntp_schedule/test_ntp_schedule.cpp new file mode 100644 index 0000000000..4e2e800e85 --- /dev/null +++ b/test/test_ntp_schedule/test_ntp_schedule.cpp @@ -0,0 +1,78 @@ +#include "helpers/NtpSchedule.h" +#include + +TEST(NtpSchedule, CompletionDoesNotRefreshAgainstThePreProbeSample) { + NtpSchedule s; + ASSERT_TRUE(s.begin(1000)); + s.complete(2200, NtpSchedule::Result::NetworkTime); + EXPECT_FALSE(s.due(1000)); + EXPECT_FALSE(s.due(2200)); + EXPECT_FALSE(s.due(3602199)); + EXPECT_TRUE(s.due(3602200)); +} + +TEST(NtpSchedule, NoClockFailureAfterAnHourStillBacksOff) { + NtpSchedule s; + uint32_t now = 3600001; + const uint32_t delays[] = {30000, 60000, 120000, 240000, 300000, 300000}; + for (uint32_t delay : delays) { + ASSERT_TRUE(s.begin(now)); + now += 6000; + s.complete(now, NtpSchedule::Result::Failed); + EXPECT_FALSE(s.due(now + delay - 1)); + now += delay; + EXPECT_TRUE(s.due(now)); + } + EXPECT_FALSE(s.hasNetworkTime()); +} + +TEST(NtpSchedule, ForcedRequestSharesDeadlineAndCannotOverlap) { + NtpSchedule s; + ASSERT_TRUE(s.begin(0)); + EXPECT_FALSE(s.begin(1, true)); + s.complete(100, NtpSchedule::Result::NetworkTime); + EXPECT_FALSE(s.begin(200)); + ASSERT_TRUE(s.begin(200, true)); + s.complete(500, NtpSchedule::Result::Failed); + EXPECT_FALSE(s.due(501)); + EXPECT_TRUE(s.due(30500)); + EXPECT_EQ(100u, s.lastSuccessMs()); +} + +TEST(NtpSchedule, HoldoverDoesNotClaimANetworkSuccess) { + NtpSchedule s; + ASSERT_TRUE(s.begin(0)); + s.complete(0, NtpSchedule::Result::Holdover); + EXPECT_FALSE(s.hasNetworkTime()); + EXPECT_EQ(NtpSchedule::Result::Holdover, s.result()); + EXPECT_FALSE(s.due(299999)); + EXPECT_TRUE(s.due(300000)); +} + +TEST(NtpSchedule, DueAttemptRemainsDueDuringALongNetworkOutage) { + NtpSchedule s; + ASSERT_TRUE(s.begin(0)); + s.complete(0, NtpSchedule::Result::Failed); + EXPECT_TRUE(s.due(30000)); + EXPECT_TRUE(s.due(0x80010000u)); + EXPECT_TRUE(s.begin(0x80010000u)); +} + +TEST(NtpSchedule, NetworkSuccessAtZeroAndWrappedDeadlineAreValid) { + NtpSchedule s; + ASSERT_TRUE(s.begin(0xfffffff0u)); + s.complete(0, NtpSchedule::Result::NetworkTime); + EXPECT_TRUE(s.hasNetworkTime()); + EXPECT_EQ(0u, s.lastSuccessMs()); + EXPECT_FALSE(s.due(1)); + ASSERT_TRUE(s.begin(0xfffffff0u, true)); + s.complete(0xfffffff0u, NtpSchedule::Result::Failed); + EXPECT_FALSE(s.due(0)); + EXPECT_FALSE(s.due(29983)); + EXPECT_TRUE(s.due(29984)); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/test_observer_handoffs/test_observer_handoffs.cpp b/test/test_observer_handoffs/test_observer_handoffs.cpp new file mode 100644 index 0000000000..431e8a663a --- /dev/null +++ b/test/test_observer_handoffs/test_observer_handoffs.cpp @@ -0,0 +1,361 @@ +#include +#include +#include +#include "helpers/ObserverMailbox.h" +#include "helpers/ObserverConfigMailbox.h" +#include "helpers/ObserverAsyncJob.h" +#include "helpers/MQTTEventChannel.h" +#include "helpers/MQTTSlotEvents.h" +#include "helpers/ObserverPreferenceCommit.h" +#include "helpers/ObserverSlotStatsJson.h" +#include "helpers/OtaUpdateFollowUp.h" + + +struct Image { uint32_t sequence = 0; uint32_t fields[64]{}; }; + +TEST(ObserverHandoffs, snapshot_reads_never_mix_publications) { + ObserverMailbox mailbox; + std::atomic done{false}, consistent{true}; + std::thread writer([&] { + for (uint32_t n = 1; n <= 10000; ++n) { + Image image; + image.sequence = n; + for (auto& field : image.fields) field = n; + mailbox.publish(image); + } + done = true; + }); + do { + Image image = mailbox.read(); + for (auto field : image.fields) if (field != image.sequence) consistent = false; + } while (!done.load()); + writer.join(); + ASSERT_TRUE(consistent.load()); + EXPECT_EQ(10000, mailbox.read().sequence); +} + +TEST(ObserverHandoffs, config_requests_coalesce_with_latest_complete_image) { + ObserverConfigMailbox mailbox; + Image first, second, applied; + first.sequence = 10; + second.sequence = 11; + mailbox.publish(first, 100, 1); + mailbox.publish(second, 200, 4); + uint32_t revision = 0, metadata = 0, mask = 0; + ASSERT_TRUE(mailbox.consume(applied, metadata, revision, mask)); + EXPECT_EQ(11, applied.sequence); + EXPECT_EQ(200, metadata); + EXPECT_EQ(5, mask); + EXPECT_EQ(2, revision); + ASSERT_FALSE(mailbox.consume(applied, metadata, revision, mask)); + mailbox.publish(second, 200); + EXPECT_EQ(2, mailbox.revision()); + mailbox.publish(second, 200, 2); + ASSERT_TRUE(mailbox.consume(applied, metadata, revision, mask)); + EXPECT_EQ(2, mask); +} + +TEST(ObserverHandoffs, config_copy_is_coherent_during_concurrent_replacement) { + ObserverConfigMailbox mailbox; + std::atomic done{false}, consistent{true}; + std::thread writer([&] { + for (uint32_t n = 1; n <= 10000; ++n) { + Image image; + image.sequence = n; + for (auto& field : image.fields) field = n; + mailbox.publish(image, n); + } + done = true; + }); + Image image; + uint32_t revision = 0, metadata = 0, mask = 0; + do { + if (mailbox.consume(image, metadata, revision, mask)) { + if (metadata != image.sequence) consistent = false; + for (auto field : image.fields) if (field != metadata) consistent = false; + } + } while (!done.load()); + writer.join(); + ASSERT_TRUE(consistent.load()); +} + +TEST(ObserverHandoffs, overflow_identifies_every_affected_slot) { + MqttEventChannel<2> events; + MqttSlotEvent event; + event.slot = 0; + event.incarnation = 9; + ASSERT_TRUE(events.push(event)); + event.kind = MqttSlotEvent::Kind::Connected; + ASSERT_TRUE(events.push(event)); + event.slot = 1; + ASSERT_FALSE(events.push(event)); + event.slot = 4; + ASSERT_FALSE(events.push(event)); + EXPECT_EQ(18, events.takeOverflowSlots()); + EXPECT_EQ(0, events.takeOverflowSlots()); + EXPECT_EQ(2, events.overflows()); + ASSERT_TRUE(events.pop(event)); + ASSERT_TRUE(event.kind == MqttSlotEvent::Kind::Disconnected); + EXPECT_EQ(9, event.incarnation); + ASSERT_TRUE(events.pop(event)); + ASSERT_TRUE(event.kind == MqttSlotEvent::Kind::Connected); + ASSERT_FALSE(events.pop(event)); +} + +TEST(ObserverHandoffs, retired_slot_is_discarded_without_losing_other_slots) { + MqttEventChannel<3> events; + MqttSlotEvent event; + event.slot = 1; + events.push(event); + events.pop(event); // move the ring head off zero + event.slot = 0; event.incarnation = 5; events.push(event); + event.slot = 1; event.incarnation = 6; events.push(event); + event.slot = 0; event.incarnation = 7; events.push(event); + events.discardSlot(0); + ASSERT_TRUE(events.pop(event)); + EXPECT_EQ(1, event.slot); + EXPECT_EQ(6, event.incarnation); + ASSERT_FALSE(events.pop(event)); +} + +TEST(ObserverHandoffs, concurrent_callbacks_preserve_each_producer_order) { + MqttEventChannel<32> events; + std::atomic done{0}; + auto producer = [&](uint8_t slot) { + for (uint32_t n = 1; n <= 2000; ++n) { + MqttSlotEvent event; + event.slot = slot; event.at_ms = n; + while (!events.push(event)) std::this_thread::yield(); + } + ++done; + }; + std::thread first(producer, 0), second(producer, 1); + uint32_t last[2]{}; + bool ordered = true; + MqttSlotEvent event; + while (done.load() != 2 || last[0] != 2000 || last[1] != 2000) { + if (events.pop(event)) { + if (event.at_ms != last[event.slot] + 1) ordered = false; + last[event.slot] = event.at_ms; + } + } + first.join(); second.join(); + ASSERT_TRUE(ordered); +} + +TEST(ObserverHandoffs, job_polling_does_not_restart_or_misattribute_a_result) { + ObserverAsyncJob job; + ASSERT_TRUE(job.request(10)); + ASSERT_FALSE(job.request(11)); + const uint32_t id = job.begin(); + EXPECT_EQ(1, id); + ASSERT_FALSE(job.request(12)); + EXPECT_EQ(0, job.begin()); + job.complete(id, 20, 42); + auto result = job.read(); + ASSERT_TRUE(result.state == ObserverAsyncJob::State::Complete); + EXPECT_EQ(42, result.result); + EXPECT_EQ(20, result.finished_ms); + ASSERT_TRUE(job.request(30)); + const uint32_t second = job.begin(); + job.complete(id, 40, 99); + EXPECT_EQ(second, job.read().id); + ASSERT_TRUE(job.read().state == ObserverAsyncJob::State::Running); + job.complete(second, 45, 123); + EXPECT_EQ(123, job.read().result); +} + +TEST(ObserverHandoffs, stopped_job_rejects_late_completion_and_can_restart) { + ObserverAsyncJob job; + job.request(0xfffffff0); + const uint32_t id = job.begin(); + job.cancel(5); + job.complete(id, 6, 7); + ASSERT_TRUE(job.read().state == ObserverAsyncJob::State::Cancelled); + ASSERT_TRUE(job.request(8)); + EXPECT_NE(id, job.begin()); +} + +struct TestSlot { + MqttClientState client_state = MqttClientState::Configured; + uint32_t incarnation = 3, generation = 0; + bool enabled = true, connected = false, circuit_breaker_tripped = true; + uint32_t token_expires_at = 123, applied_token_expires_at = 0; + uint32_t connected_at_ms = 0, last_error_time = 0, current_outage_started_ms = 0; + uint32_t disconnect_count = 0, first_disconnect_time = 0; + int32_t last_tls_err = 0, last_tls_stack_err = 0, last_sock_errno = 0; + uint8_t last_connack_code = 0; + uint8_t reconnect_backoff = 4; +}; + +TEST(ObserverHandoffs, ConnectedBeforeStartReturnsIsAppliedByTheOwner) { + TestSlot slot; + MqttEventChannel<2> events; + bool mint = true, status = false; + const int result = startMqttSlotAttempt(slot, [&] { + EXPECT_TRUE(slot.client_state == MqttClientState::Starting); + MqttSlotEvent event; + event.kind = MqttSlotEvent::Kind::Connected; + event.incarnation = slot.incarnation; + event.at_ms = 10; + events.push(event); // SDK callback before start returns + EXPECT_FALSE(slot.connected); + return 0; + }); + EXPECT_EQ(0, result); + MqttSlotEvent event; + ASSERT_TRUE(events.pop(event)); + ASSERT_TRUE(applyMqttSlotEvent(slot, event, mint, status)); + EXPECT_TRUE(slot.connected); + EXPECT_TRUE(status); + EXPECT_FALSE(mint); + EXPECT_EQ(4, slot.reconnect_backoff); + EXPECT_EQ(slot.token_expires_at, slot.applied_token_expires_at); +} + +TEST(ObserverHandoffs, FailedStartRestoresStateWithoutApplyingCredential) { + TestSlot slot; + EXPECT_EQ(-1, startMqttSlotAttempt(slot, [] { return -1; })); + EXPECT_TRUE(slot.client_state == MqttClientState::Configured); + EXPECT_EQ(0u, slot.generation); + EXPECT_EQ(0u, slot.applied_token_expires_at); +} + +TEST(ObserverHandoffs, RetiredDisabledAndStoppedClientsCannotResurrect) { + TestSlot slot; + slot.client_state = MqttClientState::Starting; + bool mint = false, status = false; + MqttSlotEvent event; + event.kind = MqttSlotEvent::Kind::Connected; + event.incarnation = 2; + EXPECT_FALSE(applyMqttSlotEvent(slot, event, mint, status)); + event.incarnation = 3; + slot.enabled = false; + EXPECT_FALSE(applyMqttSlotEvent(slot, event, mint, status)); + slot.enabled = true; + for (auto state : {MqttClientState::Stopped, MqttClientState::Quarantined}) { + slot.client_state = state; + for (auto kind : {MqttSlotEvent::Kind::Connected, MqttSlotEvent::Kind::Disconnected, MqttSlotEvent::Kind::Error}) { + event.kind = kind; + EXPECT_FALSE(applyMqttSlotEvent(slot, event, mint, status)); + } + EXPECT_EQ(state, slot.client_state); + } + EXPECT_FALSE(slot.connected); + EXPECT_FALSE(status); +} + +TEST(ObserverHandoffs, ErrorThenConnectClearsFailureAndDisconnectCancelsStatus) { + TestSlot slot; + slot.client_state = MqttClientState::Starting; + bool mint = false, status = false; + MqttSlotEvent event; + event.incarnation = 3; + event.kind = MqttSlotEvent::Kind::Error; + event.connack = 5; + event.at_ms = 100; + ASSERT_TRUE(applyMqttSlotEvent(slot, event, mint, status)); + EXPECT_TRUE(mint); + EXPECT_EQ(5, slot.last_connack_code); + event.kind = MqttSlotEvent::Kind::Connected; + event.at_ms = 200; + ASSERT_TRUE(applyMqttSlotEvent(slot, event, mint, status)); + EXPECT_EQ(0, slot.last_connack_code); + EXPECT_TRUE(status); + event.kind = MqttSlotEvent::Kind::Disconnected; + event.at_ms = 300; + ASSERT_TRUE(applyMqttSlotEvent(slot, event, mint, status)); + EXPECT_FALSE(status); + EXPECT_FALSE(slot.connected); + EXPECT_EQ(300u, slot.current_outage_started_ms); + EXPECT_EQ(1u, slot.disconnect_count); +} + +TEST(ObserverHandoffs, RejectedAndIndeterminateCandidatesNeverBecomeDurable) { + uint32_t durable = 1, candidate = 2; + for (bool indeterminate : {false, true}) { + EXPECT_FALSE(commitObserverPreferenceCandidate(durable, &candidate, [&] { + EXPECT_EQ(1u, durable); // A reader during flash I/O still sees the prior image. + EXPECT_EQ(2u, candidate); + (void)indeterminate; // Both outcomes report false; CLI retains distinct text. + return false; + })); + EXPECT_EQ(1u, durable); + } + EXPECT_TRUE(commitObserverPreferenceCandidate(durable, &candidate, [&] { + EXPECT_EQ(1u, durable); + candidate = 3; // Serializer normalization is also private until commit. + return true; + })); + EXPECT_EQ(3u, durable); +} + +struct JsonSlot { + bool configured = true; + const char* name = "custom"; + const char* state = "ok"; + unsigned long publish_ok = 1, publish_err = 2; + uint16_t filter_mask = MQTTPacketFilter::kAllPacketTypes; +}; + +TEST(ObserverHandoffs, SharedStatsPreserveSchemaAndOmitDefaultFilter) { + char buf[256] = "{\"slots\":["; + JsonSlot slots[2]; + slots[1].filter_mask = 8; + appendObserverSlotStatsJson(buf, sizeof(buf), strlen(buf), slots, 2); + EXPECT_STREQ("{\"slots\":[{\"n\":1,\"name\":\"custom\",\"state\":\"ok\",\"ok\":1,\"err\":2},{\"n\":2,\"name\":\"custom\",\"state\":\"ok\",\"ok\":1,\"err\":2,\"filt\":8}]}", buf); +} + +TEST(ObserverHandoffs, StatsReserveClosingBracketsAtEveryBufferBoundary) { + JsonSlot slot; + char full[256] = "{\"slots\":["; + const size_t prefix = strlen(full); + appendObserverSlotStatsJson(full, sizeof(full), prefix, &slot, 1); + for (size_t size = prefix + 3; size < strlen(full) + 1; ++size) { + char buf[256]; + memset(buf, 'x', sizeof(buf)); + memcpy(buf, "{\"slots\":[", prefix + 1); + appendObserverSlotStatsJson(buf, size, prefix, &slot, 1); + EXPECT_STREQ("{\"slots\":[]}", buf); + EXPECT_EQ('x', buf[size]); + } +} + +TEST(ObserverHandoffs, UpdateFollowUpWaitsForTheCheckThenStartsOnce) { + using F = OtaUpdateFollowUp; + F follow; + int dry_runs = 0; + auto applicable = [&] { ++dry_runs; return F::Check::Applicable; }; + EXPECT_EQ(F::Action::None, follow.poll(0, false, applicable)); // never armed + follow.arm(1000); + EXPECT_EQ(F::Action::None, follow.poll(1500, true, applicable)); // still checking + EXPECT_EQ(0, dry_runs); + EXPECT_EQ(F::Action::Start, follow.poll(2000, false, applicable)); + EXPECT_EQ(1, dry_runs); + EXPECT_FALSE(follow.armed()); + EXPECT_EQ(F::Action::None, follow.poll(2001, false, applicable)); // consumed + EXPECT_EQ(1, dry_runs); +} + +TEST(ObserverHandoffs, UpdateFollowUpRefusesRequeuesAndTimesOut) { + using F = OtaUpdateFollowUp; + F follow; + follow.arm(0); + EXPECT_EQ(F::Action::Refuse, follow.poll(10, false, [] { return F::Check::NotApplicable; })); + EXPECT_FALSE(follow.armed()); + + follow.arm(0); + EXPECT_EQ(F::Action::None, follow.poll(10, false, [] { return F::Check::Requeued; })); + EXPECT_TRUE(follow.armed()); // the cache had expired; wait for the new check + EXPECT_EQ(F::Action::Start, follow.poll(20, false, [] { return F::Check::Applicable; })); + + follow.arm(0xffffff00); // deadline wraps past zero + EXPECT_EQ(F::Action::None, follow.poll(0xffffff10, true, [] { return F::Check::Applicable; })); + EXPECT_EQ(F::Action::Timeout, follow.poll(0xffffff00 + F::kTimeoutMs, true, [] { return F::Check::Applicable; })); + EXPECT_FALSE(follow.armed()); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/test_observer_init/test_observer_init.cpp b/test/test_observer_init/test_observer_init.cpp new file mode 100644 index 0000000000..75f0b5f3d9 --- /dev/null +++ b/test/test_observer_init/test_observer_init.cpp @@ -0,0 +1,73 @@ +#include +#include "helpers/RequiredTransportBuffer.h" +#include "../../lib/PsychicMqttClient/src/MqttClientInit.h" + +TEST(ObserverInit, MissingPaddingPreventsSdkInitialization) { + bool initialized = false, installed = false, freed = false; + int transport; + int* result = initWithRequiredTransportBuffer(1025, + [](size_t) -> void* { return nullptr; }, + [&] { initialized = true; return &transport; }, + [&](int*, void*) { installed = true; }, + [&](void*) { freed = true; }); + EXPECT_EQ(nullptr, result); + EXPECT_FALSE(initialized); + EXPECT_FALSE(installed); + EXPECT_FALSE(freed); +} + +TEST(ObserverInit, FailedSdkInitReleasesReservedPadding) { + char buffer[1025]; + bool freed = false, installed = false; + int* result = initWithRequiredTransportBuffer(sizeof(buffer), + [&](size_t n) -> void* { EXPECT_EQ(sizeof(buffer), n); return buffer; }, + []() -> int* { return nullptr; }, + [&](int*, void*) { installed = true; }, + [&](void* p) { EXPECT_EQ(buffer, p); freed = true; }); + EXPECT_EQ(nullptr, result); + EXPECT_TRUE(freed); + EXPECT_FALSE(installed); +} + +TEST(ObserverInit, SuccessfulTransportOwnsProtectionBuffer) { + char buffer[1025]; + int transport; + bool installed = false, freed = false; + int* result = initWithRequiredTransportBuffer(sizeof(buffer), + [&](size_t) -> void* { return buffer; }, + [&] { return &transport; }, + [&](int* t, void* p) { EXPECT_EQ(&transport, t); EXPECT_EQ(buffer, p); installed = true; }, + [&](void*) { freed = true; }); + EXPECT_EQ(&transport, result); + EXPECT_TRUE(installed); + EXPECT_FALSE(freed); +} + +TEST(ObserverInit, FailedRegistrationDestroysCandidateAndCanRetry) { + int candidate; + int* client = nullptr; + int registrations = 0, destroys = 0; + auto create = [&] { return &candidate; }; + auto register_events = [&](int* p) { EXPECT_EQ(&candidate, p); return ++registrations == 1 ? -7 : 0; }; + auto destroy = [&](int* p) { EXPECT_EQ(&candidate, p); ++destroys; }; + EXPECT_EQ(-7, initializeMqttClient(client, create, register_events, destroy, -2)); + EXPECT_EQ(nullptr, client); + EXPECT_EQ(1, destroys); + EXPECT_EQ(0, initializeMqttClient(client, create, register_events, destroy, -2)); + EXPECT_EQ(&candidate, client); + EXPECT_EQ(1, destroys); +} + +TEST(ObserverInit, FailedClientAllocationDoesNotRegisterOrDestroy) { + int* client = nullptr; + int effects = 0; + EXPECT_EQ(-2, initializeMqttClient(client, []() -> int* { return nullptr; }, + [&](int*) { ++effects; return 0; }, [&](int*) { ++effects; }, -2)); + EXPECT_EQ(nullptr, client); + EXPECT_EQ(0, effects); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/webui/index.html b/webui/index.html index daac4f52a8..3ece1c1554 100644 --- a/webui/index.html +++ b/webui/index.html @@ -1506,8 +1506,8 @@

Rebooting…

["powersaving off","Disable power saving"], ["password ","Change the admin password {new-password}"], ["alert test","Send a test alert on the configured channel"], - ["ota check","Check for a newer build (does not flash)"], - ["ota update","Download and flash the newer build, then reboot"], + ["ota check","Queue update check; repeat for result (does not flash)"], + ["ota update","Check, then download and flash a newer build and reboot"], // `start ota` is absent: it binds port 80, which this portal is already using. ["start webconfig","Start this portal on the LAN"], ["start webconfig ap","Start this portal on its own setup AP"], @@ -1577,7 +1577,7 @@

Rebooting…

["mqtt.owner","Owner public key {64-hex-char-key}",0], ["mqtt.email","Owner email address",0], ["mqtt.ntp","NTP server (none clears it)",0], - ["mqtt.ntp.diag","Last NTP sync result",1], + ["mqtt.ntp.diag","Queue NTP probe; repeat for result and age",1], ["timezone","POSIX timezone string",0], ["timezone.offset","UTC offset in hours {-12 to 14}",0], ["snmp","SNMP agent (restart required)",0,"on|off"],