From 7b68371a7f36985af94682a3aa34c99e8b61c9c8 Mon Sep 17 00:00:00 2001 From: agessaman Date: Mon, 20 Jul 2026 19:21:29 -0700 Subject: [PATCH] feat(alerts): enhance alert test command response for clarity Improve the response message for the `alert test` command to clarify that a successful test send does not guarantee automatic alerts are enabled. This change helps prevent confusion for operators regarding the status of the alert system after testing. --- ALERTS.md | 2 +- examples/simple_repeater/MyMesh.cpp | 10 ++++++++++ examples/simple_repeater/MyMesh.h | 20 ++++++++++++++++++++ src/helpers/CommonCLI_Observer.cpp | 11 ++++++++++- 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/ALERTS.md b/ALERTS.md index 047eef0a..383abf31 100644 --- a/ALERTS.md +++ b/ALERTS.md @@ -77,7 +77,7 @@ Set: - `set alert.interval ` (60-10080; 60-minute floor to protect mesh airtime) Action: -- `alert test` - send a one-off `[test] alert channel ok` immediately on the configured channel; ignores `alert on/off` so operators can verify the channel before enabling fault firing. Returns an error if no channel is configured. +- `alert test` - send a one-off `[test] alert channel ok` immediately on the configured channel; ignores `alert on/off` so operators can verify the channel before enabling fault firing. Returns an error if no channel is configured. If the send succeeds but the `alert` master switch is still `off`, the reply says so (`OK - test sent, but automatic alerts are OFF`) - a working test alone does **not** mean automatic WiFi/MQTT/OTA alerts will fire. - `alert test ` - send a custom test message: `[test] `. ## Example: dedicated hashtag channel (recommended for operator groups) diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 57eb7a8c..2358581c 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -60,6 +60,12 @@ #define CLI_REPLY_DELAY_MILLIS 600 +// Max time to flush the outbound queue (START alert + CLI reply) before the OTA +// teardown blocks the loop until reboot. Best-effort: exits early once the queue +// drains (immediate on a healthy node), caps the wait on a jammed/duty-limited +// channel so an update is never stalled indefinitely. +#define OTA_TX_DRAIN_TIMEOUT_MS 5000 + #define LAZY_CONTACTS_WRITE_DELAY 5000 void MyMesh::putNeighbour(const mesh::Identity &id, uint32_t timestamp, float snr) { @@ -1652,6 +1658,10 @@ void MyMesh::loop() { // (so this never returns); on any abort (already up to date, partition change, // download error) it returns and we resume the bridge. Serial.println("OTA: starting update"); + // Flush the START alert (and CLI reply) out the radio BEFORE teardown blocks + // the loop until reboot — otherwise a packet still queued here (busy / + // duty-limited channel) is lost when the flash spins the loop and reboots. + drainOutbound(OTA_TX_DRAIN_TIMEOUT_MS); setBridgeState(false); char ota_reply[160]; // OTA teardown barrier (Phase 5): only flash after a CLEAN MQTT shutdown. diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index de4dfc80..562ba93b 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -394,6 +394,26 @@ public: auto* obs = _cli.getObserverPrefs(); if (obs && obs->alert_enabled) _alerter.sendText(msg); } + + // Best-effort flush of the outbound packet queue before an OTA teardown that + // blocks the loop until reboot. The START alert (otaAlert) and the CLI reply + // are queued fire-and-forget (delay 0 / CLI_REPLY_DELAY_MILLIS); once + // setBridgeState(false) + otaFromManifest() run they spin the loop task until + // the chip reboots, so anything still in the send queue at that point is + // silently lost — the observed "OTA update starting never arrives" case on a + // busy / duty-limited channel where the packet can't win a TX slot inside the + // 2.5 s window. Pump the mesh loop so already-queued packets get their airtime, + // bounded by timeout_ms so a jammed or budget-exhausted channel can't stall the + // update. Respects duty cycle / CAD: it only drains what is queued, it does not + // force a transmit. Returns instantly on a healthy node (queue already empty). + void drainOutbound(uint32_t timeout_ms) { + unsigned long start = millis(); + while (hasOutbound() || _mgr->getOutboundCount(millis()) > 0) { + if (millis() - start >= timeout_ms) break; + mesh::Mesh::loop(); // base dispatcher only — drives RX + checkSend()/TX + delay(1); // yield to the radio ISR / other FreeRTOS tasks + } + } #endif // Schedule the pull-OTA flash to run from loop() in ~2.5 s, leaving time for the diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index 1e138dc4..4d8d137d 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -1111,7 +1111,16 @@ bool CommonCLI::handleObserverCommand(uint32_t sender_timestamp, char* command, strcpy(reply, "Error: alert channel not configured (set alert.psk or set alert.hashtag)"); } else { bool ok = _callbacks->sendAlertText(text); - strcpy(reply, ok ? "OK - alert sent" : "Error: alert send failed (bad PSK or PUBLIC key refused?)"); + if (!ok) { + strcpy(reply, "Error: alert send failed (bad PSK or PUBLIC key refused?)"); + } else if (!_mqtt_prefs.alert_enabled) { + // `alert test` deliberately bypasses the master switch, so a successful + // send here does NOT mean automatic WiFi/MQTT/OTA alerts will fire — those + // gate on `alert on`. Flag it so a working test can't give false confidence. + strcpy(reply, "OK - test sent, but automatic alerts are OFF (run 'set alert on')"); + } else { + strcpy(reply, "OK - alert sent"); + } } return true; }