diff --git a/src/helpers/MQTTPayloadBuilder.h b/src/helpers/MQTTPayloadBuilder.h index 42dd3ff86..7f0cd85b0 100644 --- a/src/helpers/MQTTPayloadBuilder.h +++ b/src/helpers/MQTTPayloadBuilder.h @@ -17,6 +17,8 @@ // heap alone does not — an 85 KB-free board sat unable to connect for hours. // A low value with all slots up is normal, not a fault: established sessions // already hold their buffers. It means only that the next slot to ask will fail. +// Sampled at status-publish time, not at the attempt, so it is a trend, not a +// per-attempt reading. struct MQTTConnHealth { int slots_up = -1; // -1 = not supplied, field omitted int slots_total = -1; // enabled slots, whether or not they connect diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 134b12e7c..2bee5bb25 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -584,17 +584,18 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) replyAppendf(buf, bufsize, &pos, "> mqtt%d: %s", slot_index + 1, state); if (slot.disconnect_count > 0) { replyAppendf(buf, bufsize, &pos, ", dc:%lu", (unsigned long)slot.disconnect_count); - // cf: attempts that never reached onConnect. A slot retrying into a heap that - // cannot satisfy the TLS allocation moves only this counter — dc rises with it, - // but the publish counters cannot move at all while the slot is down. - if (slot.connect_failures > 0) { - replyAppendf(buf, bufsize, &pos, ", cf:%lu", (unsigned long)slot.connect_failures); - } if (slot.first_disconnect_time > 0) { unsigned long first_disc_age_sec = (millis() - slot.first_disconnect_time) / 1000; replyAppendf(buf, bufsize, &pos, ", first_disc:%lus", first_disc_age_sec); } } + // cf: attempts that never reached onConnect, plus starts that failed outright. A + // slot retrying into a heap that cannot satisfy the TLS allocation moves only this; + // the publish counters cannot move at all while the slot is down. + const uint32_t cf = slot.connect_failures + slot.start_failures; + if (cf > 0) { + replyAppendf(buf, bufsize, &pos, ", cf:%lu", (unsigned long)cf); + } // Connected with no errors: nothing more to say about the connection. if (slot.connected && slot.last_error_time == 0) { @@ -1728,6 +1729,7 @@ bool MQTTBridge::ensureSlotClient(int index) { 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); + _slot_attempt_pending[index] = false; // still the end of that attempt return; } MQTT_DEBUG_PRINTLN("MQTT%d connected", index + 1); @@ -1766,8 +1768,9 @@ bool MQTTBridge::ensureSlotClient(int index) { slot.client->onDisconnect([this, index](bool sessionPresent) { MQTT_DEBUG_PRINTLN("MQTT%d disconnected", index + 1); // Ended an attempt that never reached onConnect. A deliberate stop clears the flag - // first, so cancelling an attempt is not counted as a failure. - if (_slot_attempt_pending[index]) { + // first; a bounced live session is still marked connected until this handler runs, + // so its late DISCONNECTED after a timed-out softDisconnect() is not counted either. + if (_slot_attempt_pending[index] && !_slots[index].connected) { _slot_attempt_pending[index] = false; _slots[index].connect_failures++; } @@ -2524,6 +2527,7 @@ esp_err_t MQTTBridge::reconnectSlotClient(int index) { slot.applied_token_expires_at = slot.token_expires_at; } else { _slot_attempt_pending[index] = false; + slot.start_failures++; } return r; } @@ -3199,7 +3203,7 @@ MQTTConnHealth MQTTBridge::collectConnHealth() const { const MQTTSlot& slot = _slots[i]; if (!slot.enabled) continue; // "none" slots are not an outage, they are unconfigured health.slots_total++; - connect_failures += slot.connect_failures; + connect_failures += slot.connect_failures + slot.start_failures; if (slot.circuit_breaker_tripped) breaker_slots++; if (slot.connected) { health.slots_up++; diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index 87b75b050..87e585b10 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -187,6 +187,10 @@ private: // disconnect_count (which also counts dropped live sessions) and from the // publish error counter, which cannot move at all while a slot is down. uint32_t connect_failures; + // connect()/reconnect() calls that returned an error, so no attempt started and + // no event will arrive. Written only by the bridge task; reported summed with + // connect_failures. + uint32_t start_failures; unsigned long first_disconnect_time; // millis() of first disconnect after boot // Current-outage timer (used by AlertReporter to fire faults after a sustained @@ -288,7 +292,7 @@ private: // Set by the bridge task before connect()/reconnect(), so a failure the event task // delivers before that call returns is still seen; cleared by the attempt's outcome - // or a deliberate stop. A disconnect that finds it set is a connect failure. + // (including an ignored late CONNECTED) or a deliberate stop. volatile bool _slot_attempt_pending[RUNTIME_MQTT_SLOTS]; // CLI-requested forced NTP sync, marshalled onto the MQTT task (Core 0). @@ -381,8 +385,9 @@ private: // _device_id[65], _board_model[64], _firmware_version[64] and a 63-byte client version): // measured 762/768 before the connection-health fields existed. Overflow is silent — // serializeComplete() returns 0 and the bridge publishes only len>0, so status stops - // entirely. Raised to 1024, which costs no extra RAM here because status serializes into - // the shared scratch buffer. See the worst-case test in test_mqtt_payload_builder. + // entirely. Raised to 1024: no extra RAM while status serializes into the shared scratch + // buffer, but +256 B of bridge-task stack in the fallback used if the PSRAM allocation + // failed. See the worst-case test in test_mqtt_payload_builder. static const size_t STATUS_JSON_BUFFER_SIZE = 1024; static_assert(STATUS_JSON_BUFFER_SIZE <= PUBLISH_JSON_BUFFER_SIZE, "status payloads serialize into the shared publish buffer"); diff --git a/test/test_mqtt_payload_builder/test_mqtt_payload_builder.cpp b/test/test_mqtt_payload_builder/test_mqtt_payload_builder.cpp index e8ae51c87..eedf69795 100644 --- a/test/test_mqtt_payload_builder/test_mqtt_payload_builder.cpp +++ b/test/test_mqtt_payload_builder/test_mqtt_payload_builder.cpp @@ -222,11 +222,11 @@ TEST(MQTTPayloadBuilder, WorstCaseStatusWithConnectionHealthFitsProductionBuffer char buffer[kStatusJsonBufferSize]; MQTTConnHealth health; health.slots_up = 0; - health.slots_total = 5; // PSRAM slot cap, the widest configuration + health.slots_total = 6; // RUNTIME_MQTT_SLOTS on PSRAM, the widest health.worst_outage_secs = 4294967; // millis() rollover / 1000, the widest value health.heap_largest = 8388608; health.connect_failures = 2147483647; - health.slots_breaker = 5; + health.slots_breaker = 6; int len = MQTTPayloadBuilder::buildStatusMessage( scratch, origin.c_str(), origin_id.c_str(), model.c_str(), firmware.c_str(),