From beba1b4882391f841fa18495a8e3db1ead279745 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Thu, 22 May 2025 20:33:46 -0500 Subject: [PATCH] Added map report precision bounds (#6862) * Added map report precision bounds * Log warning * Precision range should be 12-15 * Missed commit * Update tests * That method was renamed * Removed now-defunct test call * Remove defunct test --- src/mqtt/MQTT.cpp | 29 +++++++++++++++-------------- test/test_mqtt/MQTT.cpp | 29 +++-------------------------- 2 files changed, 18 insertions(+), 40 deletions(-) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 713077272..dca8a3b44 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -773,15 +773,20 @@ void MQTT::perhapsReportToMap() !(moduleConfig.mqtt.proxy_to_client_enabled || isConnectedDirectly())) return; + // Coerce the map position precision to be within the valid range + // This removes obtusely large radius and privacy problematic ones from the map + if (map_position_precision < 12 || map_position_precision > 15) { + LOG_WARN("MQTT Map report position precision %u is out of range, using default %u", map_position_precision, + default_map_position_precision); + map_position_precision = default_map_position_precision; + } + if (Throttle::isWithinTimespanMs(last_report_to_map, map_publish_interval_msecs)) return; - if (map_position_precision == 0 || (localPosition.latitude_i == 0 && localPosition.longitude_i == 0)) { + if (localPosition.latitude_i == 0 && localPosition.longitude_i == 0) { last_report_to_map = millis(); - if (map_position_precision == 0) - LOG_WARN("MQTT Map report enabled, but precision is 0"); - if (localPosition.latitude_i == 0 && localPosition.longitude_i == 0) - LOG_WARN("MQTT Map report enabled, but no position available"); + LOG_WARN("MQTT Map report enabled, but no position available"); return; } @@ -805,15 +810,11 @@ void MQTT::perhapsReportToMap() mapReport.has_opted_report_location = true; // Set position with precision (same as in PositionModule) - if (map_position_precision < 32 && map_position_precision > 0) { - mapReport.latitude_i = localPosition.latitude_i & (UINT32_MAX << (32 - map_position_precision)); - mapReport.longitude_i = localPosition.longitude_i & (UINT32_MAX << (32 - map_position_precision)); - mapReport.latitude_i += (1 << (31 - map_position_precision)); - mapReport.longitude_i += (1 << (31 - map_position_precision)); - } else { - mapReport.latitude_i = localPosition.latitude_i; - mapReport.longitude_i = localPosition.longitude_i; - } + mapReport.latitude_i = localPosition.latitude_i & (UINT32_MAX << (32 - map_position_precision)); + mapReport.longitude_i = localPosition.longitude_i & (UINT32_MAX << (32 - map_position_precision)); + mapReport.latitude_i += (1 << (31 - map_position_precision)); + mapReport.longitude_i += (1 << (31 - map_position_precision)); + mapReport.altitude = localPosition.altitude; mapReport.position_precision = map_position_precision; diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index c1f5da358..8047079ba 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -708,42 +708,21 @@ void test_reportToMapDefaultImprecise(void) TEST_ASSERT_EQUAL(1, pubsub->published_.size()); const auto &[topic, payload] = pubsub->published_.front(); TEST_ASSERT_EQUAL_STRING("msh/2/map/", topic.c_str()); - verifyLatLong(std::get(payload), 70123520, 30015488); -} - -// Precise location is reported when configured. -void test_reportToMapPrecise(void) -{ - unitTest->reportToMap(/*precision=*/32); - - TEST_ASSERT_EQUAL(1, pubsub->published_.size()); - const auto &[topic, payload] = pubsub->published_.front(); - TEST_ASSERT_EQUAL_STRING("msh/2/map/", topic.c_str()); - verifyLatLong(std::get(payload), localPosition.latitude_i, localPosition.longitude_i); } // Location is sent over the phone proxy. -void test_reportToMapPreciseProxied(void) +void test_reportToMapImpreciseProxied(void) { moduleConfig.mqtt.proxy_to_client_enabled = true; MQTTUnitTest::restart(); - unitTest->reportToMap(/*precision=*/32); + unitTest->reportToMap(/*precision=*/14); TEST_ASSERT_EQUAL(1, mockMeshService->messages_.size()); const meshtastic_MqttClientProxyMessage &message = mockMeshService->messages_.front(); TEST_ASSERT_EQUAL_STRING("msh/2/map/", message.topic); TEST_ASSERT_EQUAL(meshtastic_MqttClientProxyMessage_data_tag, message.which_payload_variant); const DecodedServiceEnvelope env(message.payload_variant.data.bytes, message.payload_variant.data.size); - verifyLatLong(env, localPosition.latitude_i, localPosition.longitude_i); -} - -// No location is reported when the precision is invalid. -void test_reportToMapInvalidPrecision(void) -{ - unitTest->reportToMap(/*precision=*/0); - - TEST_ASSERT_TRUE(pubsub->published_.empty()); } // isUsingDefaultServer returns true when using the default server. @@ -920,9 +899,7 @@ void setup() RUN_TEST(test_publishTextMessageDirect); RUN_TEST(test_publishTextMessageWithProxy); RUN_TEST(test_reportToMapDefaultImprecise); - RUN_TEST(test_reportToMapPrecise); - RUN_TEST(test_reportToMapPreciseProxied); - RUN_TEST(test_reportToMapInvalidPrecision); + RUN_TEST(test_reportToMapImpreciseProxied); RUN_TEST(test_usingDefaultServer); RUN_TEST(test_usingDefaultServerWithPort); RUN_TEST(test_usingDefaultServerWithInvalidPort);