diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 2c656ca5d..c7181f69f 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -360,7 +360,7 @@ set mqtt3.preset meshrank set mqtt3.token FE1B34242C5938C39225310081FD6718 ``` -The token is generated on the MeshRank website and is tied to your account. MeshRank only receives packet data (no status or raw messages). +The token is generated on the MeshRank website and is tied to your account. MeshRank receives status, packets, and neighbors under `meshrank/uplink/{token}/{device}/`, using the same type suffixes as the MeshCore layout. Raw is **not** sent to MeshRank — it is the highest-volume topic and the broker does not consume it — so `set mqtt.raw on` has no effect on a MeshRank slot. #### Example: Configure MeshMapper on Slot 3 ```bash @@ -656,7 +656,7 @@ Minimal raw packet data for map integration. Periodic snapshot of this node's zero-hop neighbor table plus each neighbor's region scopes (PSRAM boards only; disabled by default). Published non-retained at QoS 1 on the interval set by `mqtt.neighbors.interval` (12–336 h, default 24 h). -Like status/raw, this topic is **not** sent to MeshRank slots (packets-only contract). +MeshRank slots receive this topic too, as `meshrank/uplink/{token}/{device}/neighbors`. **Note**: `{DEVICE_PUBLIC_KEY}` is the device's public key in hexadecimal format (64 characters). diff --git a/MQTT_INTERNALS.md b/MQTT_INTERNALS.md index 358092199..a56f94b0f 100644 --- a/MQTT_INTERNALS.md +++ b/MQTT_INTERNALS.md @@ -78,8 +78,9 @@ defined in `MQTTBridge.h`). It spans two subsystems and two cores: `_neighbors_publish_pending` with a release store; the MQTT task (`mqttTaskLoop`, Core 0) consumes it with an acquire load, calls `publishNeighbors()`, and clears the flag. A second snapshot is dropped while one is in flight. `publishNeighbors()` sends QoS 1, - retain = `preset->allow_retain` (custom slots non-retained). MeshRank slots are skipped - (the topic router rejects non-packets for MeshRank). + retain = `preset->allow_retain` (custom slots non-retained). MeshRank slots are included, + publishing to `meshrank/uplink/{token}/{device}/neighbors` (non-retained, since the + preset sets `allow_retain = false`). - **Status reporting**: `MyMesh` reports the schedule each loop via `setNeighborsSchedule(phase, secs)`; `formatMqttStatusReply` renders it as the trailing `nbr: /` field in `get mqtt.status` while the feature is enabled. diff --git a/src/helpers/MQTTPresets.h b/src/helpers/MQTTPresets.h index 2a8d4bdde..9b5a09486 100644 --- a/src/helpers/MQTTPresets.h +++ b/src/helpers/MQTTPresets.h @@ -26,7 +26,7 @@ enum MQTTAuthType : uint8_t { enum MQTTTopicStyle : uint8_t { MQTT_TOPIC_MESHCORE, // meshcore/{iata}/{device_id}/{status|packets|raw} - MQTT_TOPIC_MESHRANK, // meshrank/uplink/{token}/{device_id}/packets (packets only) + MQTT_TOPIC_MESHRANK, // meshrank/uplink/{token}/{device_id}/{type} (no raw) }; struct MQTTPresetDef { diff --git a/src/helpers/MQTTTopicRouter.h b/src/helpers/MQTTTopicRouter.h index 9dfbf17a1..4dd4b1b8f 100644 --- a/src/helpers/MQTTTopicRouter.h +++ b/src/helpers/MQTTTopicRouter.h @@ -46,11 +46,12 @@ static inline bool mqttWriteTopic(char* buf, size_t buf_size, const char* format return written > 0 && (size_t)written < buf_size; } -// Build the complete topic for one publication. MeshRank is deliberately -// packets-only; status, raw, and neighbors are unsupported by the current -// broker contract (the type != PACKETS guard below rejects them all). -// MeshCore routes require a configured IATA and device id. Custom templates may -// omit either placeholder, so their individual values are allowed to be empty. +// Build the complete topic for one publication. MeshRank takes status, packets, +// and neighbors under meshrank/uplink/{token}/{device}/, using the same type +// suffixes as the MeshCore layout, and requires a per-slot token rather than an +// IATA. MeshCore routes require a configured IATA and device id. Custom +// templates may omit either placeholder, so their individual values are allowed +// to be empty. static inline bool mqttBuildPublicationTopic(MQTTTopicRouteStyle style, int type, const char* custom_template, const char* iata, const char* device, @@ -70,7 +71,9 @@ static inline bool mqttBuildPublicationTopic(MQTTTopicRouteStyle style, int type return mqttWriteTopic(buf, buf_size, "meshcore/%s/%s/%s", iata, device, type_name); case MQTT_ROUTE_MESHRANK: - if (type != MQTT_PUBLICATION_PACKETS || !token || token[0] == '\0' || + // Raw is deliberately withheld: highest-volume topic, and the broker does + // not consume it. observer-firmware still sends it — keep this on merge. + if (type == MQTT_PUBLICATION_RAW || !token || token[0] == '\0' || !device || device[0] == '\0') { return false; } diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 0b90f3126..0da28d581 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -2269,7 +2269,7 @@ void MQTTBridge::publishStatusToSlot(int index) { // Build per-slot topic (handles IATA check for meshcore, token check for meshrank) char status_topic[128]; if (!buildTopicForSlot(index, MSG_STATUS, status_topic, sizeof(status_topic))) { - return; // Slot doesn't support status (e.g., meshrank) or missing required config + return; // Slot is missing required topic configuration } // Reuse pre-allocated buffer to avoid heap alloc/free churn under memory pressure. @@ -2878,7 +2878,7 @@ void MQTTBridge::processPacketQueue() { } // Decide intentional completion once across the entire queue item. An - // ineligible raw path (for example, MeshRank's packets-only topic style) + // ineligible raw path (for example, MeshRank, which does not take raw) // must not hide a failed eligible structured publish. const bool queue_complete = MQTTPacketFilter::publishComplete( packet_eligible || raw_eligible, @@ -3012,7 +3012,7 @@ void MQTTBridge::processPacketQueue() { } // Decide intentional completion once across the entire queue item. An - // ineligible raw path (for example, MeshRank's packets-only topic style) + // ineligible raw path (for example, MeshRank, which does not take raw) // must not hide a failed eligible structured publish. const bool queue_complete = MQTTPacketFilter::publishComplete( packet_eligible || raw_eligible, @@ -3216,8 +3216,8 @@ bool MQTTBridge::publishStatus() { } } } - // If no connected slot accepts status topics (e.g. meshrank is packets-only), - // treat as success to avoid infinite retry loops + // If no connected slot accepts status topics, treat as success to avoid + // infinite retry loops if (published || !any_slot_wants_status) { if (published) MQTT_DEBUG_PRINTLN("Status published"); return true; @@ -3374,8 +3374,8 @@ bool MQTTBridge::publishRaw(mesh::Packet* packet, bool& has_eligible_target) { const uint8_t packet_type = packet->getPayloadType(); const uint8_t eligible_slots = eligiblePacketSlots(packet_type, MSG_RAW); has_eligible_target = eligible_slots != 0; - // Filtered out everywhere, or no slot has a raw topic (MeshRank is - // packets-only): intentionally complete, and no JSON is built. + // Filtered out everywhere, or no slot has a raw topic (MeshRank does not take + // raw): intentionally complete, and no JSON is built. if (!has_eligible_target) return false; refreshOriginFromPrefs(); @@ -3456,8 +3456,7 @@ bool MQTTBridge::publishNeighbors() { char topic[128]; for (int i = 0; i < RUNTIME_MQTT_SLOTS; i++) { if (_slots[i].enabled && _slots[i].client && _slots[i].connected) { - // MeshRank slots reject non-packets by contract, so buildTopicForSlot - // returns false for them here and the slot is skipped. + // Slots that cannot form a neighbors topic are skipped here. if (buildTopicForSlot(i, MSG_NEIGHBORS, topic, sizeof(topic))) { // Neighbor snapshots are periodically refreshed. Publish synchronously // at QoS 0 to avoid the QoS 1 outbox, retaining where the broker allows. diff --git a/test/README.md b/test/README.md index cd258ee67..711e64840 100644 --- a/test/README.md +++ b/test/README.md @@ -27,7 +27,7 @@ does not reflect the GoogleTest count — run the built binary directly | `test_observer_validation` | `src/helpers/MQTTObserverValidation.h` | IATA (exactly 3 alphanumerics), owner key (64 hex), NTP hostname, and the buffer-fit check behind the #17 length validation — including boundaries and nulls | | `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 packets-only behavior; required identifiers; invalid inputs/slots; exact buffer boundaries | +| `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_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 | diff --git a/test/test_mqtt_topic_router/test_mqtt_topic_router.cpp b/test/test_mqtt_topic_router/test_mqtt_topic_router.cpp index 1156fbd63..2a453663a 100644 --- a/test/test_mqtt_topic_router/test_mqtt_topic_router.cpp +++ b/test/test_mqtt_topic_router/test_mqtt_topic_router.cpp @@ -43,24 +43,27 @@ TEST(MQTTTopicRouter, EveryMeshCorePresetSupportsEveryPublicationType) { } } -TEST(MQTTTopicRouter, MeshRankContractIsPacketsOnly) { +// Guards the raw exclusion: a merge from observer-firmware must not re-enable it. +TEST(MQTTTopicRouter, MeshRankTakesEveryTypeExceptRaw) { const MQTTPresetDef* preset = findMQTTPreset("meshrank"); ASSERT_NE(nullptr, preset); ASSERT_EQ(MQTT_TOPIC_MESHRANK, preset->topic_style); - char topic[128]; - EXPECT_FALSE(mqttBuildPublicationTopic(MQTT_ROUTE_MESHRANK, MQTT_PUBLICATION_STATUS, - nullptr, IATA, DEVICE, TOKEN, topic, sizeof(topic))); - EXPECT_STREQ("", topic); - ASSERT_TRUE(mqttBuildPublicationTopic(MQTT_ROUTE_MESHRANK, MQTT_PUBLICATION_PACKETS, - nullptr, IATA, DEVICE, TOKEN, topic, sizeof(topic))); - EXPECT_STREQ("meshrank/uplink/account-token/0123456789ABCDEF/packets", topic); - EXPECT_FALSE(mqttBuildPublicationTopic(MQTT_ROUTE_MESHRANK, MQTT_PUBLICATION_RAW, - nullptr, IATA, DEVICE, TOKEN, topic, sizeof(topic))); - EXPECT_STREQ("", topic); - EXPECT_FALSE(mqttBuildPublicationTopic(MQTT_ROUTE_MESHRANK, MQTT_PUBLICATION_NEIGHBORS, - nullptr, IATA, DEVICE, TOKEN, topic, sizeof(topic))); - EXPECT_STREQ("", topic); + for (const TypeCase& type : kTypes) { + char topic[128]; + if (type.type == MQTT_PUBLICATION_RAW) { + EXPECT_FALSE(mqttBuildPublicationTopic(MQTT_ROUTE_MESHRANK, type.type, nullptr, + IATA, DEVICE, TOKEN, topic, sizeof(topic))); + EXPECT_STREQ("", topic); + continue; + } + ASSERT_TRUE(mqttBuildPublicationTopic(MQTT_ROUTE_MESHRANK, type.type, nullptr, + IATA, DEVICE, TOKEN, topic, sizeof(topic))) + << type.name; + EXPECT_EQ(std::string("meshrank/uplink/account-token/0123456789ABCDEF/") + type.name, + topic) + << type.name; + } } TEST(MQTTTopicRouter, MeshCoreRequiresUsableIataAndDevice) {