diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index 0a342921..1e138dc4 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -23,6 +23,7 @@ #endif #ifdef WITH_MQTT_BRIDGE #include "bridges/MQTTBridge.h" +#include "MQTTConnectionPolicy.h" // classifySlotActivation() — "will this slot connect here?" #include "MQTTDefaults.h" #endif @@ -432,14 +433,32 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { sprintf(reply, "OK - slot %d preset: %s", slot + 1, preset_name); } - // Slots beyond RUNTIME_MQTT_SLOTS are still stored (so they take effect - // if the prefs move to a PSRAM board) but won't connect on this - // hardware — say so, or the operator waits for a connection that never - // comes (A15). - if (slot >= RUNTIME_MQTT_SLOTS) { + // Warn when this slot won't actually connect on this hardware. The set + // is never blocked — prefs persist so the config carries over if the + // device is moved to a board with more slots — but flag it, or the + // operator waits for a connection that never comes (A15). Two failure + // modes, keyed off the same rule the bridge's setup loop uses + // (classifySlotActivation): slots past the runtime array are never + // tried; slots within it are skipped once more than getMaxActiveSlots() + // are enabled (each WSS/TLS link costs ~40 KB heap). + if (strcmp(preset_name, MQTT_PRESET_NONE) != 0) { + bool slot_enabled[MAX_MQTT_SLOTS]; + for (int s = 0; s < MAX_MQTT_SLOTS; s++) { + slot_enabled[s] = _mqtt_prefs.mqtt_slot_preset[s][0] != '\0' && + strcmp(_mqtt_prefs.mqtt_slot_preset[s], MQTT_PRESET_NONE) != 0; + } + const int max_active = MQTTBridge::getMaxActiveSlots(); + const MQTTConnectionPolicy::SlotActivation act = + MQTTConnectionPolicy::classifySlotActivation(slot, slot_enabled, + RUNTIME_MQTT_SLOTS, max_active); size_t used = strlen(reply); if (used < 158) { - snprintf(reply + used, 160 - used, " (slot inactive on this hardware)"); + if (act == MQTTConnectionPolicy::SlotActivation::BeyondArray) { + snprintf(reply + used, 160 - used, " (slot inactive on this hardware)"); + } else if (act == MQTTConnectionPolicy::SlotActivation::OverActiveCap) { + snprintf(reply + used, 160 - used, + " (won't connect: %d-slot limit on this hardware)", max_active); + } } } } diff --git a/src/helpers/MQTTConnectionPolicy.h b/src/helpers/MQTTConnectionPolicy.h index 72dfae64..99f3d69d 100644 --- a/src/helpers/MQTTConnectionPolicy.h +++ b/src/helpers/MQTTConnectionPolicy.h @@ -161,4 +161,35 @@ static inline bool jwtClockAvailable(bool ntp_synced, uint32_t current_time) { return ntp_synced || current_time >= kJwtClockThreshold; } +// How a given MQTT slot fares at bridge setup on this hardware. MQTTBridge's +// setup loop iterates runtime slots in index order and connects the first +// `max_active` *enabled* slots, skipping the rest (each WSS/TLS link needs +// ~40 KB internal heap, so non-PSRAM caps at 2 concurrent, PSRAM at 5). Slots at +// or beyond the runtime array size (`slot_count`, e.g. 3 on non-PSRAM) are never +// iterated at all. Extracted so the CLI can tell the operator, at +// `set mqttN.preset` time, whether a slot will actually come up — and so the +// exact rule is host-tested rather than hand-reasoned (it is easy to conflate +// slot_count with max_active). +enum class SlotActivation : uint8_t { + Connects, // enabled and within the concurrent-connection budget + Disabled, // slot has no preset ("none") — not attempted + BeyondArray, // index >= slot_count: outside the runtime slot array here + OverActiveCap, // enabled, but lower-numbered slots already fill the budget +}; + +// `enabled` must have at least `slot_count` entries; `slot` is 0-based. Mirrors +// the bridge's first-come-by-index activation order exactly. +static inline SlotActivation classifySlotActivation(int slot, const bool* enabled, + int slot_count, int max_active) { + if (slot < 0) return SlotActivation::Disabled; + if (slot >= slot_count) return SlotActivation::BeyondArray; + if (enabled == nullptr || !enabled[slot]) return SlotActivation::Disabled; + int rank = 0; // this slot's position among enabled slots, counting by index + for (int i = 0; i <= slot; i++) { + if (enabled[i]) rank++; + } + return (rank <= max_active) ? SlotActivation::Connects + : SlotActivation::OverActiveCap; +} + } // namespace MQTTConnectionPolicy 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 d09f5e30..496a1c43 100644 --- a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp +++ b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp @@ -195,6 +195,53 @@ TEST(MQTTConnectionPolicy, WifiReconnectDueSurvivesMillisRollover) { EXPECT_FALSE(Policy::wifiReconnectDue(down_since + 14999U, down_since, last_attempt, attempt)); } +// --- classifySlotActivation: the "will this slot connect on this hardware" rule +// the CLI uses to warn at `set mqttN.preset` time. Non-PSRAM Heltec V3 is the +// motivating case: RUNTIME_MQTT_SLOTS=3 but only 2 concurrent connections. --- + +using Policy::SlotActivation; + +TEST(SlotActivation, NonPsramV3FirstTwoConnectThirdIsOverCap) { + // slot_count = 3 (runtime array), max_active = 2 (concurrent cap). + const bool enabled[6] = {true, true, true, false, false, false}; + EXPECT_EQ(SlotActivation::Connects, Policy::classifySlotActivation(0, enabled, 3, 2)); + EXPECT_EQ(SlotActivation::Connects, Policy::classifySlotActivation(1, enabled, 3, 2)); + // slot 3 (index 2) is the case in the field log: iterated, but skipped. + EXPECT_EQ(SlotActivation::OverActiveCap, Policy::classifySlotActivation(2, enabled, 3, 2)); +} + +TEST(SlotActivation, NonPsramSlotsBeyondRuntimeArrayAreNeverIterated) { + const bool enabled[6] = {true, true, true, true, false, false}; + // mqtt4/5/6 (index 3-5) are outside the 3-slot runtime array on non-PSRAM. + EXPECT_EQ(SlotActivation::BeyondArray, Policy::classifySlotActivation(3, enabled, 3, 2)); + EXPECT_EQ(SlotActivation::BeyondArray, Policy::classifySlotActivation(5, enabled, 3, 2)); +} + +TEST(SlotActivation, RankCountsOnlyEnabledLowerSlots) { + // Only slot index 2 enabled (1 and 2 disabled): it is the first enabled slot, + // so it connects even though its index equals the cap. + const bool enabled[6] = {false, false, true, false, false, false}; + EXPECT_EQ(SlotActivation::Connects, Policy::classifySlotActivation(2, enabled, 3, 2)); + + // Gap in the middle: slots 0 and 2 enabled, slot 1 off. Slot 2 is rank 2 <= 2. + const bool enabled2[6] = {true, false, true, false, false, false}; + EXPECT_EQ(SlotActivation::Connects, Policy::classifySlotActivation(2, enabled2, 3, 2)); +} + +TEST(SlotActivation, PsramFiveOfSixConnect) { + // PSRAM: slot_count = 6, max_active = 5. Sixth enabled slot is over the cap. + const bool enabled[6] = {true, true, true, true, true, true}; + EXPECT_EQ(SlotActivation::Connects, Policy::classifySlotActivation(4, enabled, 6, 5)); + EXPECT_EQ(SlotActivation::OverActiveCap, Policy::classifySlotActivation(5, enabled, 6, 5)); +} + +TEST(SlotActivation, DisabledAndOutOfRangeSlots) { + const bool enabled[6] = {true, false, true, false, false, false}; + EXPECT_EQ(SlotActivation::Disabled, Policy::classifySlotActivation(1, enabled, 3, 2)); + EXPECT_EQ(SlotActivation::Disabled, Policy::classifySlotActivation(-1, enabled, 3, 2)); + EXPECT_EQ(SlotActivation::Disabled, Policy::classifySlotActivation(0, nullptr, 3, 2)); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();