fix(mqtt): coarsen public position uplinks#10290
Open
Komzpa wants to merge 4 commits into
Open
Conversation
701f04b to
8ac2d6d
Compare
8ac2d6d to
8633c41
Compare
8633c41 to
6106339
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Coarsens public-MQTT POSITION_APP uplinks to address the public broker's filtering of precise positions. When the configured MQTT server is the default public one, decoded position packets are dual-published: first a generated copy with coordinates bucketed to ≤15 precision bits (and a fresh packet id), then the original packet unchanged. JSON output mirrors the same ordering. Encrypted protobuf uplinks are intentionally left as-is. When the offline queue has only one slot left, only the imprecise copy is queued so the public-usable variant survives eviction.
Changes:
- New
getPublicMqttPositionPrecision()andmakePublicMqttPositionPacket()helpers insrc/mqtt/MQTT.cppthat build a coarsened POSITION_APP copy. MQTT::onSendrefactored around apublishPacketlambda that handles a single (proto, json) publish; it's now invoked twice for public-server position packets, with a queue-pressure carve-out.- New
test_mqttcases covering imprecise-then-original ordering for protobuf and JSON, coarser-source preservation, no-coordinates skipping, encrypted-stays-encrypted, and offline-queue-near-full behavior; the mock MQTT server's command parser was rewritten to handle multi-packet writes and varint remaining-length.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/mqtt/MQTT.cpp |
Adds the public-position coarsening helpers and reworks onSend to dual-publish (coarse + original) for default-public-broker decoded position packets, with queue-pressure handling. |
test/test_mqtt/MQTT.cpp |
Adds position helpers, drainQueue helper, multi-packet/varint-length parsing in the mock MQTT server, JSON-topic capture, and new test cases for the dual-publish behavior. |
Comment on lines
+848
to
+856
| const meshtastic_MeshPacket *p; | ||
| if (moduleConfig.mqtt.encryption_enabled) { | ||
| p = &mp_encrypted; | ||
| LOG_DEBUG("encrypted message"); | ||
| } else if (mp_decoded.which_payload_variant == meshtastic_MeshPacket_decoded_tag) { | ||
| p = &mp_decoded; | ||
| if (isConfiguredForDefaultServer && !isMqttServerAddressPrivate && | ||
| makePublicMqttPositionPacket(mqttPositionPacket, mp_decoded)) | ||
| publicMqttPositionPacket = &mqttPositionPacket; |
| position.precision_bits = precision; | ||
|
|
||
| mqttPacket = sourcePacket; | ||
| mqttPacket.id = generatePacketId(); |
Comment on lines
+64
to
+67
| // Public MQTT map reports currently accept precision bits up to 15. | ||
| // Forwarded LoRa positions use that most precise public value as a cap so | ||
| // an already-coarser source packet keeps its original privacy boundary. | ||
| return 15; |
Comment on lines
+62
to
+98
| uint32_t getPublicMqttPositionPrecision() | ||
| { | ||
| // Public MQTT map reports currently accept precision bits up to 15. | ||
| // Forwarded LoRa positions use that most precise public value as a cap so | ||
| // an already-coarser source packet keeps its original privacy boundary. | ||
| return 15; | ||
| } | ||
|
|
||
| bool makePublicMqttPositionPacket(meshtastic_MeshPacket &mqttPacket, const meshtastic_MeshPacket &sourcePacket) | ||
| { | ||
| if (sourcePacket.which_payload_variant != meshtastic_MeshPacket_decoded_tag || | ||
| sourcePacket.decoded.portnum != meshtastic_PortNum_POSITION_APP) | ||
| return false; | ||
|
|
||
| meshtastic_Position position = meshtastic_Position_init_default; | ||
| if (!pb_decode_from_bytes(sourcePacket.decoded.payload.bytes, sourcePacket.decoded.payload.size, &meshtastic_Position_msg, | ||
| &position)) | ||
| return false; | ||
| if (!position.has_latitude_i || !position.has_longitude_i) | ||
| return false; | ||
|
|
||
| uint32_t precision = getPublicMqttPositionPrecision(); | ||
| if (position.precision_bits > 0 && position.precision_bits < precision) | ||
| precision = position.precision_bits; | ||
| position.latitude_i = position.latitude_i & (UINT32_MAX << (32 - precision)); | ||
| position.longitude_i = position.longitude_i & (UINT32_MAX << (32 - precision)); | ||
| position.latitude_i += (1 << (31 - precision)); | ||
| position.longitude_i += (1 << (31 - precision)); | ||
| position.precision_bits = precision; | ||
|
|
||
| mqttPacket = sourcePacket; | ||
| mqttPacket.id = generatePacketId(); | ||
| mqttPacket.decoded.payload.size = | ||
| pb_encode_to_bytes(mqttPacket.decoded.payload.bytes, sizeof(mqttPacket.decoded.payload.bytes), &meshtastic_Position_msg, | ||
| &position); | ||
| return mqttPacket.decoded.payload.size > 0; | ||
| } |
Comment on lines
+94
to
+97
| mqttPacket.decoded.payload.size = | ||
| pb_encode_to_bytes(mqttPacket.decoded.payload.bytes, sizeof(mqttPacket.decoded.payload.bytes), &meshtastic_Position_msg, | ||
| &position); | ||
| return mqttPacket.decoded.payload.size > 0; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
precision_bits = 15, then publish the original packet unchangedWhy
The public MQTT broker currently filters precise position packets. Publishing the coarse packet first lets current public consumers receive a location update, while publishing the original second preserves current behavior for private/custom brokers and for any future public-broker policy that accepts more precise positions.
Notes
POSITION_APPpackets sent to the default public MQTT server are duplicated/coarsened.Tests
test_mqttcoverage for protobuf dual publish, JSON dual publish, and encrypted-position behavior.heltec-v3successfully with PlatformIO.