diff --git a/src/helpers/AlertReporter.cpp b/src/helpers/AlertReporter.cpp index 4bce1fd0..b55ac90c 100644 --- a/src/helpers/AlertReporter.cpp +++ b/src/helpers/AlertReporter.cpp @@ -4,6 +4,9 @@ #include #include #include +#ifdef WITH_MQTT_BRIDGE +#include "AlertFaultPolicy.h" +#endif // Header layout for PAYLOAD_TYPE_GRP_TXT before encryption: // [0..3] timestamp (uint32_t LE) — also helps make packet_hash unique @@ -129,11 +132,9 @@ bool AlertReporter::resolveChannel(mesh::GroupChannel& out) const { void AlertReporter::onConfigChanged() { // Reset transient state so a config change re-arms the edge detector. #ifdef WITH_MQTT_BRIDGE - _wifi.state = OK; - _wifi.fired_at_ms = 0; + AlertFaultPolicy::reset(_wifi); for (size_t i = 0; i < sizeof(_mqtt) / sizeof(_mqtt[0]); i++) { - _mqtt[i].state = OK; - _mqtt[i].fired_at_ms = 0; + AlertFaultPolicy::reset(_mqtt[i]); } #endif } @@ -194,124 +195,75 @@ bool AlertReporter::sendText(const char* text) { return sendChannel(text); } -void AlertReporter::formatAge(unsigned long age_ms, char* out, size_t out_size) const { - unsigned long secs = age_ms / 1000UL; - unsigned long h = secs / 3600UL; - unsigned long m = (secs % 3600UL) / 60UL; - if (h > 0) { - snprintf(out, out_size, "%luh%lum", h, m); - } else { - snprintf(out, out_size, "%lum", m); - } -} - void AlertReporter::onLoop(unsigned long now_ms) { if (!_prefs || !_obs || !_obs->alert_enabled) return; if (!_mesh) return; - // Throttle: ~5 s cadence. The thresholds are minutes-scale so this is fine. - if ((long)(now_ms - _next_check_ms) < 0) return; - _next_check_ms = now_ms + 5000UL; + const uint32_t now = (uint32_t)now_ms; + if (!AlertFaultPolicy::checkDue(now, (uint32_t)_next_check_ms)) return; + _next_check_ms = AlertFaultPolicy::nextCheckMs(now); #ifdef WITH_MQTT_BRIDGE - // Clamp to a 60-minute floor regardless of what's in NodePrefs. The CLI - // already enforces this on set, but a stale prefs file or future field - // tweak shouldn't be able to drag the floor below 1 hour and let a - // flapping link spam the mesh. - // - // The rate limiter only applies between two real sends: fired_at_ms == 0 - // means "never fired since boot/config change", and treating it as a send - // at millis()==0 would suppress every first alert until uptime reaches - // min_interval (observed as a 30-minute alert.mqtt threshold not reporting - // until 60 minutes after a reboot). - uint16_t cfg_min = _obs->alert_min_interval_min; - if (cfg_min < 60) cfg_min = 60; - unsigned long min_interval_ms = (unsigned long)cfg_min * 60000UL; + const uint32_t min_interval_ms = + AlertFaultPolicy::minIntervalMs(_obs->alert_min_interval_min); // -------- WiFi fault -------- if (_obs->alert_wifi_minutes > 0) { - unsigned long wifi_disc_ms = MQTTBridge::getLastWifiDisconnectTime(); - unsigned long wifi_conn_ms = MQTTBridge::getWifiConnectedAtMillis(); - bool wifi_down = (wifi_disc_ms != 0 && wifi_conn_ms == 0); - unsigned long down_ms = wifi_down ? (now_ms - wifi_disc_ms) : 0; - unsigned long thresh_ms = (unsigned long)_obs->alert_wifi_minutes * 60000UL; - - if (_wifi.state == OK) { - if (wifi_down && down_ms >= thresh_ms && - (_wifi.fired_at_ms == 0 || (now_ms - _wifi.fired_at_ms) >= min_interval_ms)) { - char age[16]; - formatAge(down_ms, age, sizeof(age)); - uint8_t reason = MQTTBridge::getLastWifiDisconnectReason(); + if (_bridge != nullptr) { + const AlertFaultPolicy::OutageSnapshot snap = _bridge->getWifiOutageSnapshot(); + AlertFaultPolicy::TickResult r = AlertFaultPolicy::tick( + _wifi, now, snap, + AlertFaultPolicy::thresholdMs(_obs->alert_wifi_minutes), + min_interval_ms); + if (r.action == AlertFaultPolicy::Action::FireDown) { char text[80]; - if (reason != 0) { - snprintf(text, sizeof(text), "WiFi down %s (reason %u)", age, (unsigned)reason); - } else { - snprintf(text, sizeof(text), "WiFi down %s", age); - } + AlertFaultPolicy::formatWifiAlert(text, sizeof(text), r, snap); if (sendChannel(text)) { - _wifi.state = FIRING; - _wifi.fired_at_ms = now_ms; - _wifi.last_outage_started_ms = wifi_disc_ms; + AlertFaultPolicy::commitDown(_wifi, now, snap.started_ms); } - } - } else { // FIRING - if (!wifi_down) { - unsigned long total = (wifi_conn_ms != 0 && _wifi.last_outage_started_ms != 0) - ? (wifi_conn_ms - _wifi.last_outage_started_ms) : 0; - char age[16]; - formatAge(total, age, sizeof(age)); + } else if (r.action == AlertFaultPolicy::Action::FireRecovered) { char text[80]; - snprintf(text, sizeof(text), "WiFi recovered after %s", age); + AlertFaultPolicy::formatWifiAlert(text, sizeof(text), r, snap); sendChannel(text); - _wifi.state = OK; + AlertFaultPolicy::commitRecovered(_wifi); } } - } else if (_wifi.state == FIRING) { - _wifi.state = OK; // threshold disabled mid-fault: silently re-arm + } else { + AlertFaultPolicy::rearmIfDisabled(_wifi); } // -------- MQTT slot faults -------- if (_obs->alert_mqtt_minutes > 0 && _bridge != nullptr) { int n = MQTTBridge::getRuntimeSlotCount(); if (n > (int)(sizeof(_mqtt) / sizeof(_mqtt[0]))) n = (int)(sizeof(_mqtt) / sizeof(_mqtt[0])); - unsigned long thresh_ms = (unsigned long)_obs->alert_mqtt_minutes * 60000UL; + const uint32_t thresh_ms = AlertFaultPolicy::thresholdMs(_obs->alert_mqtt_minutes); for (int i = 0; i < n; i++) { - Fault& f = _mqtt[i]; + AlertFaultPolicy::Fault& f = _mqtt[i]; if (!_bridge->isSlotEnabledAndAttempted(i)) { - if (f.state == FIRING) f.state = OK; // slot disabled mid-fault + AlertFaultPolicy::rearmIfDisabled(f); continue; } - unsigned long outage_start = _bridge->getSlotCurrentOutageStartMs(i); - bool down = (outage_start != 0); - unsigned long down_ms = down ? (now_ms - outage_start) : 0; - - if (f.state == OK) { - if (down && down_ms >= thresh_ms && - (f.fired_at_ms == 0 || (now_ms - f.fired_at_ms) >= min_interval_ms)) { - char age[16]; - formatAge(down_ms, age, sizeof(age)); - char text[100]; - snprintf(text, sizeof(text), "MQTT slot %d (%s) down %s", - i + 1, _bridge->getSlotPresetName(i), age); - if (sendChannel(text)) { - f.state = FIRING; - f.fired_at_ms = now_ms; - f.last_outage_started_ms = outage_start; - } - } - } else { // FIRING - if (!down) { - unsigned long total = (f.last_outage_started_ms != 0) - ? (now_ms - f.last_outage_started_ms) : 0; - char age[16]; - formatAge(total, age, sizeof(age)); - char text[100]; - snprintf(text, sizeof(text), "MQTT slot %d (%s) recovered after %s", - i + 1, _bridge->getSlotPresetName(i), age); - sendChannel(text); - f.state = OK; + const uint32_t outage_start = (uint32_t)_bridge->getSlotCurrentOutageStartMs(i); + const AlertFaultPolicy::OutageSnapshot snap = + AlertFaultPolicy::fromStartMs(outage_start); + AlertFaultPolicy::TickResult r = AlertFaultPolicy::tick( + f, now, snap, thresh_ms, min_interval_ms); + if (r.action == AlertFaultPolicy::Action::FireDown) { + char text[100]; + AlertFaultPolicy::formatMqttDown(text, sizeof(text), i + 1, + _bridge->getSlotPresetName(i), + r.duration_ms); + if (sendChannel(text)) { + AlertFaultPolicy::commitDown(f, now, outage_start); } + } else if (r.action == AlertFaultPolicy::Action::FireRecovered) { + char text[100]; + AlertFaultPolicy::formatMqttRecovered(text, sizeof(text), i + 1, + _bridge->getSlotPresetName(i), + r.duration_ms); + sendChannel(text); + AlertFaultPolicy::commitRecovered(f); } } } diff --git a/src/helpers/AlertReporter.h b/src/helpers/AlertReporter.h index 390262b6..6f52b571 100644 --- a/src/helpers/AlertReporter.h +++ b/src/helpers/AlertReporter.h @@ -6,6 +6,7 @@ #ifdef WITH_MQTT_BRIDGE #include "bridges/MQTTBridge.h" +#include "AlertFaultPolicy.h" #endif /** @@ -89,14 +90,6 @@ public: private: bool resolveChannel(mesh::GroupChannel& out) const; bool sendChannel(const char* text); - void formatAge(unsigned long age_ms, char* out, size_t out_size) const; - - enum FaultState { OK, FIRING }; - struct Fault { - FaultState state; - unsigned long fired_at_ms; // millis() when we last sent a "down" alert - unsigned long last_outage_started_ms; // remembered so the recovered msg can quote duration - }; NodePrefs* _prefs; MQTTPrefs* _obs; @@ -104,8 +97,8 @@ private: CommonCLICallbacks* _callbacks; #ifdef WITH_MQTT_BRIDGE MQTTBridge* _bridge; - Fault _wifi; - Fault _mqtt[RUNTIME_MQTT_SLOTS]; + AlertFaultPolicy::Fault _wifi; + AlertFaultPolicy::Fault _mqtt[RUNTIME_MQTT_SLOTS]; #endif unsigned long _next_check_ms; }; diff --git a/src/helpers/MQTTConnectionPolicy.h b/src/helpers/MQTTConnectionPolicy.h index d7ff2f5e..cf459459 100644 --- a/src/helpers/MQTTConnectionPolicy.h +++ b/src/helpers/MQTTConnectionPolicy.h @@ -120,6 +120,24 @@ static inline uint8_t nextWifiBackoffAttempt(uint8_t attempt) { return attempt < 5 ? static_cast(attempt + 1) : attempt; } +// Current WiFi outage start (millis), or 0 while associated. handleWiFiConnection() +// applies this on each STA status observation. Reconnect attempts that fire +// while already down must pass connected=false and last_connected=false so the +// start is preserved — WiFi.disconnect() in the backoff ladder is not a new +// outage, and must not become the clock AlertReporter quotes as downtime. +static inline uint32_t wifiCurrentOutageStartMs(uint32_t now, bool connected, + bool last_connected, + uint32_t current_start, + bool initialized) { + if (!initialized) { + return connected ? 0U : now; + } + if (connected) { + return last_connected ? current_start : 0U; + } + return last_connected ? now : current_start; +} + // Each later slot expires up to five percent of the base lifetime earlier, // capped at five minutes per slot. Runtime slot indexes are bounded by the // persisted MQTT slot count; the final clamp also prevents underflow if this diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 947722a3..39c8f05b 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -662,7 +662,7 @@ MQTTBridge::MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mg _snmp_agent(nullptr), #endif _last_wifi_check(0), _last_wifi_status(WL_DISCONNECTED), _wifi_status_initialized(false), - _wifi_disconnected_time(0), _last_wifi_reconnect_attempt(0), _wifi_reconnect_backoff_attempt(0), + _wifi_outage_bits{0}, _last_wifi_reconnect_attempt(0), _wifi_reconnect_backoff_attempt(0), _last_slot_reconnect_ms(0) #ifdef ESP_PLATFORM , _packet_queue_handle(nullptr), _mqtt_task_handle(nullptr), @@ -1257,16 +1257,23 @@ void MQTTBridge::initializeWiFiInTask() { switch(event) { case ARDUINO_EVENT_WIFI_STA_GOT_IP: MQTT_DEBUG_PRINTLN("WiFi connected: %s", IPAddress(info.got_ip.ip_info.ip.addr).toString().c_str()); + setWifiOutage(AlertFaultPolicy::applyWifiGotIp(wifiOutage())); + _wifi_reconnect_backoff_attempt = 0; // Set flag to trigger NTP sync from loop() instead of doing it here if (!_ntp_synced && !_ntp_sync_pending) { _ntp_sync_pending = true; } break; - case ARDUINO_EVENT_WIFI_STA_DISCONNECTED: - s_wifi_disconnect_reason = info.wifi_sta_disconnected.reason; - s_wifi_disconnect_time = millis(); + case ARDUINO_EVENT_WIFI_STA_DISCONNECTED: { + const uint8_t reason = info.wifi_sta_disconnected.reason; + const unsigned long t = millis(); + s_wifi_disconnect_reason = reason; + s_wifi_disconnect_time = t; + setWifiOutage(AlertFaultPolicy::applyWifiDisconnectEvent( + (uint32_t)t, reason, wifiOutage())); MQTT_DEBUG_PRINTLN("WiFi disconnected: reason %d", s_wifi_disconnect_reason); break; + } default: break; } @@ -2795,11 +2802,19 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) { if (!_wifi_status_initialized) { _last_wifi_status = current_wifi_status; _wifi_status_initialized = true; - if (current_wifi_status != WL_CONNECTED) { - _wifi_disconnected_time = now; - } + setWifiOutage(AlertFaultPolicy::applyWifiStatus( + (uint32_t)now, current_wifi_status == WL_CONNECTED, wifiOutage(), false)); } if (now - _last_wifi_check <= 10000) { + // Events own the snapshot between 10 s polls. If STA is associated again + // and GOT_IP was missed, still close the outage so a flap contained + // between polls does not look like one continuous downtime. + if (current_wifi_status == WL_CONNECTED) { + AlertFaultPolicy::OutageSnapshot snap = wifiOutage(); + if (snap.down) { + setWifiOutage(AlertFaultPolicy::applyWifiGotIp(snap)); + } + } return false; } _last_wifi_check = now; @@ -2807,7 +2822,8 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) { if (current_wifi_status == WL_CONNECTED) { if (_last_wifi_status != WL_CONNECTED) { transitioned_to_connected = true; - _wifi_disconnected_time = 0; + setWifiOutage(AlertFaultPolicy::applyWifiStatus( + (uint32_t)now, true, wifiOutage(), true)); s_wifi_connected_at = now; _wifi_reconnect_backoff_attempt = 0; #ifdef ESP_PLATFORM @@ -2833,8 +2849,11 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) { } _last_wifi_status = WL_CONNECTED; } else { - if (_last_wifi_status == WL_CONNECTED) { - _wifi_disconnected_time = now; + const bool last_connected = (_last_wifi_status == WL_CONNECTED); + AlertFaultPolicy::OutageSnapshot snap = AlertFaultPolicy::applyWifiStatus( + (uint32_t)now, false, wifiOutage(), true); + setWifiOutage(snap); + if (last_connected) { s_wifi_connected_at = 0; // Disconnect all slot clients when WiFi drops for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { @@ -2842,13 +2861,13 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) { _slots[i].client->disconnect(); } } - } else if (_wifi_disconnected_time > 0) { + } else if (snap.down) { // Backoff ladder + wrap-safe timing live in MQTTConnectionPolicy (Phase 6), // exercised by host tests. Behavior is unchanged: both the link-down // duration and the since-last-attempt interval must clear the current rung // (elapsedMs is the wrap-safe form of the old ULONG_MAX branch). if (MQTTConnectionPolicy::wifiReconnectDue( - (uint32_t)now, (uint32_t)_wifi_disconnected_time, + (uint32_t)now, snap.started_ms, (uint32_t)_last_wifi_reconnect_attempt, _wifi_reconnect_backoff_attempt)) { _last_wifi_reconnect_attempt = now; diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index e94a5033..e2d27253 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -11,6 +11,7 @@ #include "helpers/MQTTPacketFilter.h" #include "helpers/MQTTPresets.h" #include "helpers/MQTTLifecycle.h" +#include "helpers/AlertFaultPolicy.h" #include #ifdef WITH_SNMP @@ -409,11 +410,21 @@ private: unsigned long _last_wifi_check; wl_status_t _last_wifi_status; bool _wifi_status_initialized; - unsigned long _wifi_disconnected_time; // 0 when connected + // Packed OutageSnapshot; Core 0 (event + MQTT task) stores, Core 1 loads. + std::atomic _wifi_outage_bits; unsigned long _last_wifi_reconnect_attempt; uint8_t _wifi_reconnect_backoff_attempt; // 0..5 → 15s, 30s, 60s, 120s, 300s; reset on connect unsigned long _last_slot_reconnect_ms; // guards against concurrent TLS handshakes (15 s inter-slot gap) + AlertFaultPolicy::OutageSnapshot wifiOutage() const { + return AlertFaultPolicy::unpackOutageSnapshot( + _wifi_outage_bits.load(std::memory_order_acquire)); + } + void setWifiOutage(AlertFaultPolicy::OutageSnapshot snap) { + _wifi_outage_bits.store(AlertFaultPolicy::packOutageSnapshot(snap), + std::memory_order_release); + } + // Optional pointers for collecting stats internally (set by mesh if available) mesh::Dispatcher* _dispatcher; // For air times and errors mesh::Radio* _radio; // For noise floor @@ -643,6 +654,15 @@ public: static unsigned long getWifiConnectedAtMillis(); + /** + * Current WiFi outage snapshot for AlertReporter: down, started_ms, and the + * initiating disconnect reason. Distinct from getLastWifiDisconnectTime() / + * getLastWifiDisconnectReason(), which follow the most recent ESP-IDF + * DISCONNECTED event and are overwritten by STA-backoff WiFi.disconnect() + * (reason 8 / ASSOC_LEAVE). + */ + AlertFaultPolicy::OutageSnapshot getWifiOutageSnapshot() const { return wifiOutage(); } + /** * Per-slot outage accessors used by AlertReporter to detect prolonged * MQTT broker outages. Indices are 0..RUNTIME_MQTT_SLOTS-1. diff --git a/test/README.md b/test/README.md index 80182510..6fde2d7f 100644 --- a/test/README.md +++ b/test/README.md @@ -28,7 +28,8 @@ does not reflect the GoogleTest count — run the built binary directly | `test_webconfig_keys` | `src/helpers/WebConfigKeys.h` | POST-key allowlist, secret detection, admin-password classification/validation, slot-index bounds, and the short-key out-of-bounds guard (attacker-supplied keys) | | `test_topic_template` | `src/helpers/MQTTTopicTemplate.h` | `{iata}/{device}/{token}/{type}` expansion, overflow/NUL-termination, and a buffer-size fuzz | | `test_mqtt_topic_router` | `src/helpers/MQTTTopicRouter.h` | complete preset/custom topic-routing contract; MeshRank all types except raw; required identifiers; invalid inputs/slots; exact buffer boundaries | -| `test_mqtt_connection_policy` | `src/helpers/MQTTConnectionPolicy.h` | reconnect guard/backoff/stagger and breaker transitions; stable reset; JWT lifetime/renewal policy; exact timing boundaries and 32-bit `millis()` rollover | +| `test_mqtt_connection_policy` | `src/helpers/MQTTConnectionPolicy.h` | reconnect guard/backoff/stagger and breaker transitions; stable reset; JWT lifetime/renewal policy; exact timing boundaries and 32-bit `millis()` rollover; WiFi current-outage start sticky across STA reconnect attempts | +| `test_alert_fault_policy` | `src/helpers/AlertFaultPolicy.h` | WiFi/MQTT fault edge detector; `OutageSnapshot` (down / started_ms / initiating reason) fed to tick and `formatWifiAlert`; reason-8 reconnects change neither duration nor initiating reason; flap between status polls; down at `millis()==0`; packed 64-bit cross-task word; rate-limit floor and first-fire; 5 s poll cadence and `millis()` rollover | | `test_mqtt_packet_queue_policy` | `src/helpers/MQTTPacketQueuePolicy.h` | queue-full eviction; stale-disconnect flush; adaptive drain limits; bounded QoS0 retries; exact timing boundaries and 32-bit `millis()` rollover | | `test_mqtt_packet_filter` | `src/helpers/MQTTPacketFilter.h` | per-slot 0-15 allowlist parsing/formatting, numeric and named spellings; exact bounds; membership; candidate/eligible split and retry-completion policy; pre-queue union gate; default-mask detection | | `test_mqtt_runtime_buffer_lifecycle` | `src/helpers/MQTTRuntimeBufferLifecycle.h` | idempotent allocation/release; partial-allocation degradation; retry of only missing buffers | diff --git a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp index f92e5b89..e5fef6d0 100644 --- a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp +++ b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp @@ -230,6 +230,21 @@ TEST(MQTTConnectionPolicy, WifiReconnectRequiresBothDownAndSinceAttemptToClearRu EXPECT_TRUE(Policy::wifiReconnectDue(1000U + 15000U, down_since, last_attempt, attempt)); } +TEST(MQTTConnectionPolicy, WifiOutageStartStickyAcrossReconnectAttempts) { + // First observe-down (or connected→down) records `now`. Further down samples + // — including STA backoff WiFi.disconnect() — must keep that start so + // AlertReporter quotes the outage, not the last attempt / boot event. + EXPECT_EQ(0U, Policy::wifiCurrentOutageStartMs(5000, true, false, 0, false)); + EXPECT_EQ(5000U, Policy::wifiCurrentOutageStartMs(5000, false, false, 0, false)); + + const uint32_t start = 9000U; + EXPECT_EQ(start, Policy::wifiCurrentOutageStartMs(start, false, true, 0, true)); + EXPECT_EQ(start, Policy::wifiCurrentOutageStartMs(start + 15000U, false, false, start, true)); + EXPECT_EQ(start, Policy::wifiCurrentOutageStartMs(start + 2U * 3600U * 1000U, + false, false, start, true)); + EXPECT_EQ(0U, Policy::wifiCurrentOutageStartMs(start + 1000U, true, false, start, true)); +} + TEST(MQTTConnectionPolicy, WifiReconnectDueSurvivesMillisRollover) { const uint32_t down_since = std::numeric_limits::max() - 100U; const uint32_t last_attempt = down_since;