From 616fe96d1ef7392c6af5203f5c1b4b99c100cec2 Mon Sep 17 00:00:00 2001 From: agessaman Date: Wed, 29 Jul 2026 15:51:28 -0700 Subject: [PATCH] feat(mqtt): publish all message types except raw to MeshRank MeshRank slots previously took packets only. The broker tolerates status and neighbors (and its maintainer intends to look at using the neighbors data), so those now publish under meshrank/uplink/{token}/{device}/{type}, using the same type suffixes as the MeshCore layout. Raw stays excluded: 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. All MeshRank gating funnels through the topic router, so relaxing that single guard was sufficient - publishStatusToSlot, publishStatus, publishRaw, publishNeighbors, and eligiblePacketSlots already skip slots that cannot form a topic. A per-slot token is still required. Because eligiblePacketSlots resolves topic support before serialising, raw JSON is never built for a MeshRank slot rather than built and discarded. observer-firmware sends raw to MeshRank; this supersedes that decision, so raw must stay excluded when the branches merge. MeshRankTakesEveryTypeExceptRaw fails if a merge re-enables it. --- MQTT_IMPLEMENTATION.md | 4 +-- MQTT_INTERNALS.md | 5 +-- src/helpers/MQTTPresets.h | 2 +- src/helpers/MQTTTopicRouter.h | 15 +++++---- src/helpers/bridges/MQTTBridge.cpp | 17 +++++----- test/README.md | 2 +- .../test_mqtt_topic_router.cpp | 31 ++++++++++--------- 7 files changed, 41 insertions(+), 35 deletions(-) diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 2c656ca5..c7181f69 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 35809219..a56f94b0 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 2a8d4bdd..9b5a0948 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 9dfbf17a..4dd4b1b8 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 0b90f312..0da28d58 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 cd258ee6..711e6484 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 1156fbd6..2a453663 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) {