From f25492c7fd9f3c0ad63ba98c07fcc85558b56964 Mon Sep 17 00:00:00 2001 From: agessaman Date: Mon, 20 Jul 2026 16:00:24 -0700 Subject: [PATCH] feat(mqtt): implement slot activation classification for MQTT connections Add a new classification mechanism for MQTT slot activation to inform users whether a slot will connect on the current hardware. This includes detailed handling of enabled slots and their limits, improving the CLI's ability to provide accurate feedback during configuration. Additionally, unit tests are introduced to validate the new functionality across various scenarios, enhancing overall robustness. --- src/helpers/CommonCLI_Observer.cpp | 31 +++++++++--- src/helpers/MQTTConnectionPolicy.h | 31 ++++++++++++ .../test_mqtt_connection_policy.cpp | 47 +++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-) 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();