From 2c57a17124840245d8be85f08d41e7ff46148e08 Mon Sep 17 00:00:00 2001 From: Tom <116762865+NomDeTom@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:32:18 +0100 Subject: [PATCH] Phantom node fix perhaps (#11271) * trying to fix phantom nodes * fix comment spam * oops - missed one --- protobufs | 2 +- src/mesh/LR11x0Interface.cpp | 1 + src/mesh/LR20x0Interface.cpp | 1 + src/mesh/MeshService.cpp | 4 ++- src/mesh/NodeDB.cpp | 31 ++++++++++++++-- src/mesh/NodeDB.h | 13 ++++++- src/mesh/PhoneAPI.cpp | 35 ++++++++++++++----- src/mesh/RF95Interface.cpp | 1 + src/mesh/RadioInterface.cpp | 2 +- src/mesh/SX126xInterface.cpp | 1 + src/mesh/SX128xInterface.cpp | 1 + src/mesh/TypeConversions.cpp | 3 ++ src/mesh/generated/meshtastic/deviceonly.pb.h | 7 ++-- src/mesh/generated/meshtastic/mesh.pb.h | 20 ++++++++--- src/mesh/udp/UdpMulticastHandler.h | 6 +++- src/modules/StoreForwardModule.cpp | 1 + src/modules/TraceRouteModule.cpp | 6 +++- src/serialization/MeshPacketSerializer.cpp | 8 +++-- test/test_fuzz_packets/test_main.cpp | 2 ++ .../test_meshpacket_serializer/test_helpers.h | 1 + 20 files changed, 120 insertions(+), 26 deletions(-) diff --git a/protobufs b/protobufs index bfd718fa1..94cdec45a 160000 --- a/protobufs +++ b/protobufs @@ -1 +1 @@ -Subproject commit bfd718fa1dcb019ed11b7b7185f37318abebdafc +Subproject commit 94cdec45ae729650d5a1b0cea356cad12747983b diff --git a/src/mesh/LR11x0Interface.cpp b/src/mesh/LR11x0Interface.cpp index b051bc114..bb87ca7ae 100644 --- a/src/mesh/LR11x0Interface.cpp +++ b/src/mesh/LR11x0Interface.cpp @@ -247,6 +247,7 @@ template void LR11x0Interface::addReceiveMetadata(meshtastic_Mes // LOG_DEBUG("PacketStatus %x", lora.getPacketStatus()); mp->rx_snr = lora.getSNR(); mp->rx_rssi = lround(lora.getRSSI()); + mp->has_rx_rssi = true; // rx_rssi has explicit presence - a genuine reading must be marked present to survive encoding LOG_DEBUG("Corrected frequency offset: %f", lora.getFrequencyError()); } diff --git a/src/mesh/LR20x0Interface.cpp b/src/mesh/LR20x0Interface.cpp index 95dad82e9..3e5ec9250 100644 --- a/src/mesh/LR20x0Interface.cpp +++ b/src/mesh/LR20x0Interface.cpp @@ -253,6 +253,7 @@ template void LR20x0Interface::addReceiveMetadata(meshtastic_Mes // LOG_DEBUG("PacketStatus %x", lora.getPacketStatus()); mp->rx_snr = lora.getSNR(); mp->rx_rssi = lround(lora.getRSSI()); + mp->has_rx_rssi = true; // rx_rssi has explicit presence - a genuine reading must be marked present to survive encoding // LOG_DEBUG("Corrected frequency offset: %f", lora.getFrequencyError()); // not implemented for LR20x0, but noop for LR11x0 // too(!) } diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 0efd23c81..782b0dc73 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -216,8 +216,10 @@ void MeshService::injectAsReceived(meshtastic_MeshPacket &p) return; if (mp->rx_snr == 0) // plausible synthetic link metadata unless the caller set it mp->rx_snr = 8; - if (mp->rx_rssi == 0) + if (!mp->has_rx_rssi) { // rx_rssi has explicit presence; only fabricate if the caller didn't supply a real one mp->rx_rssi = -40; + mp->has_rx_rssi = true; + } mp->rx_time = getValidTime(RTCQualityFromNet); LOG_INFO("inject: RX from=0x%08x to=0x%08x id=0x%08x ch=%d %s", mp->from, mp->to, mp->id, mp->channel, mp->which_payload_variant == meshtastic_MeshPacket_encrypted_tag ? "encrypted" : "decoded"); diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 9ee69636d..c7d139aaa 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -202,7 +202,9 @@ bool meshtastic_NodeDatabase_callback(pb_istream_t *istream, pb_ostream_t *ostre if (ostream) { const auto *vec = static_cast *>(iter->pData); for (auto item : *vec) { - item.snr_q4 = (int32_t)(item.snr * 4.0f); + // Round rather than truncate: truncation wiped any |SNR| < 0.25 dB to exactly + // 0, which collided with the "never stored" sentinel below. + item.snr_q4 = (int32_t)lroundf(item.snr * 4.0f); item.snr = 0.0f; if (!pb_encode_tag_for_field(ostream, iter)) return false; @@ -214,8 +216,14 @@ bool meshtastic_NodeDatabase_callback(pb_istream_t *istream, pb_ostream_t *ostre meshtastic_NodeInfoLite node = meshtastic_NodeInfoLite_init_zero; auto *vec = static_cast *>(iter->pData); if (pb_decode(istream, meshtastic_NodeInfoLite_fields, &node)) { - if (node.snr_q4) + // snr_q4 = 0 is byte-identical to "field never written" but 0 dB is valid. + // NODEINFO_BITFIELD_HAS_SNR_MASK disambiguates going forward; legacy + // records (bit clear) treat this as unknown. + if (nodeInfoLiteHasSnr(&node)) { node.snr = node.snr_q4 / 4.0f; + } else if (node.snr_q4) { + node.snr = node.snr_q4 / 4.0f; + } node.snr_q4 = 0; vec->push_back(node); } @@ -3624,8 +3632,20 @@ void NodeDB::updateFrom(const meshtastic_MeshPacket &mp) if (mp.rx_time) // if the packet has a valid timestamp use it to update our last_heard info->last_heard = mp.rx_time; - if (mp.rx_snr) + // Gate on the packet actually having been received over our own radio, not on rx_snr being + // truthy, because 0 dB is valid. TRANSPORT_LORA is set only on the real over-the-air RX path + // (RadioInterface.cpp); it excludes TRANSPORT_INTERNAL and TRANSPORT_MQTT, while still accepting + // an MQTT-origin packet that a gateway rebroadcasts onto LoRa - we genuinely measured that one + // ourselves. Mirrors hop histogram below. + // Belt-and-braces: also require has_rx_rssi, which every genuine RF-reception site sets + // unconditionally alongside rx_snr - unlike PhoneAPI's replay packets, which set TRANSPORT_LORA + // too (so the client treats restored history as if heard over the air) but never has_rx_rssi. + // Replay packets don't reach updateFrom() today; this check guards against a future change that + // routes them back through this path silently recording a replayed rx_snr as a fresh measurement. + if (mp.transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA && mp.has_rx_rssi) { info->snr = mp.rx_snr; // keep the most recent SNR we received for this node. + nodeInfoLiteSetBit(info, NODEINFO_BITFIELD_HAS_SNR_MASK, true); + } nodeInfoLiteSetBit(info, NODEINFO_BITFIELD_VIA_MQTT_MASK, mp.via_mqtt); // Store if we received this packet via MQTT @@ -3637,6 +3657,11 @@ void NodeDB::updateFrom(const meshtastic_MeshPacket &mp) // inflate the local mesh-size estimate with non-RF nodes (and they usually carry // hop_start==0, landing in the hop-0 bucket that pulls the recommendation lowest), so // exclude via_mqtt too. + // + // The std::max clamp below is deliberate, not a bug: Counting an unproven-but-real neighbor as 0 hops is the + // conservative direction. This intentionally does not agree with the `hopsAway >= 0` + // gate below, which rejects the same -1 rather than storing it as a fabricated 0 - + // the histogram and the stored hops_away serve different purposes. if (mp.transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA && !mp.via_mqtt && hopScalingModule) { uint8_t hopCount = std::max(int8_t(0), getHopsAway(mp)); diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index cb93754a6..5320c571b 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -740,7 +740,12 @@ extern uint32_t error_address; #define NODEINFO_BITFIELD_HAS_IS_UNMESSAGABLE_MASK (1u << NODEINFO_BITFIELD_HAS_IS_UNMESSAGABLE_SHIFT) #define NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_SHIFT 9 #define NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK (1u << NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_SHIFT) -// Bits 10..31 reserved for future single-bit flags. +// snr_q4 (persisted, sint32) is proto3 singular, so 0 == "never written", but 0 dB is valid. +// This bit disambiguates: whenever snr_q4 is written from a genuine RF measurement. +// Use this instead of `if (snr_q4)`. Legacy records (bit clear) are unambiguously "unknown". +#define NODEINFO_BITFIELD_HAS_SNR_SHIFT 10 +#define NODEINFO_BITFIELD_HAS_SNR_MASK (1u << NODEINFO_BITFIELD_HAS_SNR_SHIFT) +// Bits 11..31 reserved for future single-bit flags. // Convenience accessors so call sites read like the old struct fields. inline bool nodeInfoLiteHasUser(const meshtastic_NodeInfoLite *n) @@ -783,6 +788,12 @@ inline bool nodeInfoLiteHasXeddsaSigned(const meshtastic_NodeInfoLite *n) { return n && (n->bitfield & NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK); } +/// True if this node's snr_q4 was written from a genuine RF measurement (including a real +/// 0 dB reading). False means "never measured" - do not treat 0 as data. +inline bool nodeInfoLiteHasSnr(const meshtastic_NodeInfoLite *n) +{ + return n && (n->bitfield & NODEINFO_BITFIELD_HAS_SNR_MASK); +} /// A node that the eviction/migration paths must not drop: a favourite, an /// ignored (blocked) node, or a manually-verified key. inline bool nodeInfoLiteIsProtected(const meshtastic_NodeInfoLite *n) diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 28d047f13..75385f03f 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -1222,17 +1222,29 @@ uint32_t makeReplayPacketId(NodeNum num, uint32_t timestamp, uint32_t kind) return h ? h : 1; // some clients treat id 0 as "unset" } -/// Populate hop_start/hop_limit from the node's real last-known hop count (if any) so a -/// replayed packet doesn't read as "heard directly" when it wasn't. +/// Populate hop_start/hop_limit from the node's last-known hop count - never fabricate one. +/// hop_start == 0 with no decoded bitfield means unknown, not a direct neighbor; clients must +/// treat it that way too (see hop_start in mesh.proto). void setReplayHopFields(meshtastic_MeshPacket &pkt, const meshtastic_NodeInfoLite *header) { + if (!header || !header->has_hops_away) { + pkt.hop_start = 0; // unknown - do not fabricate a direct-neighbor reading + pkt.hop_limit = 0; + return; + } uint8_t hopLimit = Default::getConfiguredOrDefaultHopLimit(config.lora.hop_limit); - uint8_t hopsAway = (header && header->has_hops_away) ? header->hops_away : 0; + uint8_t hopsAway = header->hops_away; pkt.hop_start = hopLimit; pkt.hop_limit = hopsAway < hopLimit ? (uint8_t)(hopLimit - hopsAway) : 0; } + } // namespace +// Replayed packets deliberately leave rx_rssi absent. NodeInfoLite stores no RSSI, and +// rx_rssi has explicit presence on the wire, indicating "unknown". +// Previously these packets carried a bare 0, which a client renders as a real reading. +// Note the asymmetry with rx_snr below: that field is still proto3 singular, so "unknown" and +// "0 dB" remain indistinguishable there. meshtastic_MeshPacket PhoneAPI::makeReplayPositionPacket(NodeNum num, const meshtastic_PositionLite &pos) { // Shape this exactly like a fresh live broadcast Position from the peer so the @@ -1242,12 +1254,16 @@ meshtastic_MeshPacket PhoneAPI::makeReplayPositionPacket(NodeNum num, const mesh const meshtastic_NodeInfoLite *header = nodeDB->getMeshNode(num); pkt.from = num; pkt.to = NODENUM_BROADCAST; - pkt.rx_time = pos.time; + // rx_time means "when *we* received this" - use last_heard, not the position's own GPS + // fix time (which is often 0 and, when present, already round-trips inside the payload + // via ConvertToPosition). + pkt.rx_time = header ? header->last_heard : 0; // Stable per-node/per-fix id: replaying the same unchanged history on every // reconnect must not look like a brand new packet to the phone's history/dedup. pkt.id = makeReplayPacketId(num, pkt.rx_time, meshtastic_PortNum_POSITION_APP); - pkt.channel = 0; + pkt.channel = header ? header->channel : 0; pkt.rx_snr = header ? header->snr : 0; + pkt.via_mqtt = nodeInfoLiteViaMqtt(header); setReplayHopFields(pkt, header); pkt.priority = meshtastic_MeshPacket_Priority_BACKGROUND; // Mark as if heard over the air, not internally generated @@ -1270,8 +1286,9 @@ meshtastic_MeshPacket PhoneAPI::makeReplayTelemetryPacket(NodeNum num, const mes const meshtastic_NodeInfoLite *header = nodeDB->getMeshNode(num); pkt.rx_time = header ? header->last_heard : 0; pkt.id = makeReplayPacketId(num, pkt.rx_time, meshtastic_Telemetry_device_metrics_tag); - pkt.channel = 0; + pkt.channel = header ? header->channel : 0; pkt.rx_snr = header ? header->snr : 0; + pkt.via_mqtt = nodeInfoLiteViaMqtt(header); setReplayHopFields(pkt, header); pkt.priority = meshtastic_MeshPacket_Priority_BACKGROUND; // Mark as if heard over the air, not internally generated - iOS client filters @@ -1375,8 +1392,9 @@ meshtastic_MeshPacket PhoneAPI::makeReplayEnvironmentPacket(uint32_t num, const const meshtastic_NodeInfoLite *header = nodeDB->getMeshNode(num); pkt.rx_time = header ? header->last_heard : 0; pkt.id = makeReplayPacketId(num, pkt.rx_time, meshtastic_Telemetry_environment_metrics_tag); - pkt.channel = 0; + pkt.channel = header ? header->channel : 0; pkt.rx_snr = header ? header->snr : 0; + pkt.via_mqtt = nodeInfoLiteViaMqtt(header); setReplayHopFields(pkt, header); pkt.priority = meshtastic_MeshPacket_Priority_BACKGROUND; // Mark as if heard over the air, not internally generated - iOS client filters @@ -1439,8 +1457,9 @@ meshtastic_MeshPacket PhoneAPI::makeReplayStatusPacket(uint32_t num, const mesht const meshtastic_NodeInfoLite *header = nodeDB->getMeshNode(num); pkt.rx_time = header ? header->last_heard : 0; pkt.id = makeReplayPacketId(num, pkt.rx_time, meshtastic_PortNum_NODE_STATUS_APP); - pkt.channel = 0; + pkt.channel = header ? header->channel : 0; pkt.rx_snr = header ? header->snr : 0; + pkt.via_mqtt = nodeInfoLiteViaMqtt(header); setReplayHopFields(pkt, header); pkt.priority = meshtastic_MeshPacket_Priority_BACKGROUND; // Mark as if heard over the air, not internally generated - client filters diff --git a/src/mesh/RF95Interface.cpp b/src/mesh/RF95Interface.cpp index cbd792878..26a765ea8 100644 --- a/src/mesh/RF95Interface.cpp +++ b/src/mesh/RF95Interface.cpp @@ -267,6 +267,7 @@ void RF95Interface::addReceiveMetadata(meshtastic_MeshPacket *mp) { mp->rx_snr = lora->getSNR(); mp->rx_rssi = lround(lora->getRSSI()); + mp->has_rx_rssi = true; // rx_rssi has explicit presence - a genuine reading must be marked present to survive encoding LOG_DEBUG("Corrected frequency offset: %f", lora->getFrequencyError()); } diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 043bd59ef..ec1b9c9cb 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -881,7 +881,7 @@ void printPacket(const char *prefix, const meshtastic_MeshPacket *p) out += DEBUG_PORT.mt_sprintf(" rxtime=%u", p->rx_time); if (p->rx_snr != 0.0) out += DEBUG_PORT.mt_sprintf(" rxSNR=%g", p->rx_snr); - if (p->rx_rssi != 0) + if (p->has_rx_rssi) // rx_rssi has explicit presence; a != 0 check would hide a genuine 0 dBm reading out += DEBUG_PORT.mt_sprintf(" rxRSSI=%i", p->rx_rssi); if (p->via_mqtt != 0) out += DEBUG_PORT.mt_sprintf(" via MQTT"); diff --git a/src/mesh/SX126xInterface.cpp b/src/mesh/SX126xInterface.cpp index 817134b58..7c46bee71 100644 --- a/src/mesh/SX126xInterface.cpp +++ b/src/mesh/SX126xInterface.cpp @@ -335,6 +335,7 @@ template void SX126xInterface::addReceiveMetadata(meshtastic_Mes // LOG_DEBUG("PacketStatus %x", lora.getPacketStatus()); mp->rx_snr = lora.getSNR(); mp->rx_rssi = lround(lora.getRSSI()); + mp->has_rx_rssi = true; // rx_rssi has explicit presence - a genuine reading must be marked present to survive encoding LOG_DEBUG("Corrected frequency offset: %f", lora.getFrequencyError()); } diff --git a/src/mesh/SX128xInterface.cpp b/src/mesh/SX128xInterface.cpp index 608cd37e0..7848d51db 100644 --- a/src/mesh/SX128xInterface.cpp +++ b/src/mesh/SX128xInterface.cpp @@ -205,6 +205,7 @@ template void SX128xInterface::addReceiveMetadata(meshtastic_Mes // LOG_DEBUG("PacketStatus %x", lora.getPacketStatus()); mp->rx_snr = lora.getSNR(); mp->rx_rssi = lround(lora.getRSSI()); + mp->has_rx_rssi = true; // rx_rssi has explicit presence - a genuine reading must be marked present to survive encoding LOG_DEBUG("Corrected frequency offset: %f", lora.getFrequencyError()); } diff --git a/src/mesh/TypeConversions.cpp b/src/mesh/TypeConversions.cpp index 0b33578de..9fc80fd86 100644 --- a/src/mesh/TypeConversions.cpp +++ b/src/mesh/TypeConversions.cpp @@ -10,6 +10,9 @@ meshtastic_NodeInfo TypeConversions::ConvertToNodeInfo(const meshtastic_NodeInfo meshtastic_NodeInfo info = meshtastic_NodeInfo_init_default; info.num = lite->num; + // NodeInfo.snr (wire) is still proto3 singular float - unlike NodeInfoLite.snr_q4, it has no + // presence bit and cannot distinguish a genuine 0 dB from "unknown". nodeInfoLiteHasSnr(lite) + // is available if a future NodeInfo revision needs to carry that distinction to clients. info.snr = lite->snr; info.last_heard = lite->last_heard; info.channel = lite->channel; diff --git a/src/mesh/generated/meshtastic/deviceonly.pb.h b/src/mesh/generated/meshtastic/deviceonly.pb.h index 059398cf0..669792ddd 100644 --- a/src/mesh/generated/meshtastic/deviceonly.pb.h +++ b/src/mesh/generated/meshtastic/deviceonly.pb.h @@ -95,8 +95,11 @@ typedef struct _meshtastic_NodeInfoLite { /* The public key of the user's device, for PKI-based encrypted DMs. */ meshtastic_NodeInfoLite_public_key_t public_key; /* Q4-encoded SNR: dB × 4, sint32 zigzag. Matches RouteDiscovery convention. - Encode: snr_q4 = (int32_t)(snr * 4.0f). Decode: snr = snr_q4 / 4.0f. - float snr is always zeroed on disk; this field carries all persisted SNR. */ + Encode: snr_q4 = (int32_t)lroundf(snr * 4.0f). Decode: snr = snr_q4 / 4.0f. + float snr is always zeroed on disk; this field carries all persisted SNR. + A stored 0 does not by itself mean "unknown" here - see NODEINFO_BITFIELD_HAS_SNR in + src/mesh/NodeDB.h for the presence bit that disambiguates a genuine 0 dB reading from + "never measured". */ int32_t snr_q4; } meshtastic_NodeInfoLite; diff --git a/src/mesh/generated/meshtastic/mesh.pb.h b/src/mesh/generated/meshtastic/mesh.pb.h index 7ce8115ba..637aae3ab 100644 --- a/src/mesh/generated/meshtastic/mesh.pb.h +++ b/src/mesh/generated/meshtastic/mesh.pb.h @@ -1103,14 +1103,24 @@ typedef struct _meshtastic_MeshPacket { /* The priority of this message for sending. See MeshPacket.Priority description for more details. */ meshtastic_MeshPacket_Priority priority; - /* rssi of received packet. Only sent to phone for dispay purposes. */ + /* rssi of received packet. Only sent to phone for dispay purposes. + Explicit presence: rssi 0 is a legitimate reading on some radios (SX126x can report exactly + 0 dBm; SX127x's formula can even go positive), so implicit-presence proto3 made an unset + value indistinguishable from a measured one. has_rx_rssi disambiguates; a replayed packet + built from history the device never restored an RSSI for should leave this field absent + rather than emitting 0. */ + bool has_rx_rssi; int32_t rx_rssi; /* Describe if this message is delayed */ meshtastic_MeshPacket_Delayed delayed; /* Describes whether this packet passed via MQTT somewhere along the path it currently took. */ bool via_mqtt; /* Hop limit with which the original packet started. Sent via LoRa using three bits in the unencrypted header. - When receiving a packet, the difference between hop_start and hop_limit gives how many hops it traveled. */ + When receiving a packet, the difference between hop_start and hop_limit gives how many hops it traveled. + hop_start == 0 does not necessarily mean a direct (0-hop) neighbor: firmware prior to 2.3.0 + never populated this field, so a receiver can only trust hop_start == 0 as genuine once it has + decoded the packet and confirmed the sender's bitfield is present (added in 2.5.0). Until then, + or for a sender that never sets that bitfield, treat hop_start == 0 as unknown, not direct. */ uint8_t hop_start; /* Records the public key the packet was encrypted with, if applicable. */ meshtastic_MeshPacket_public_key_t public_key; @@ -1730,7 +1740,7 @@ extern "C" { #define meshtastic_Waypoint_init_default {0, false, 0, false, 0, 0, 0, "", "", 0, 0, false, meshtastic_BoundingBox_init_default, 0, 0, 0} #define meshtastic_StatusMessage_init_default {""} #define meshtastic_MqttClientProxyMessage_init_default {"", 0, {{0, {0}}}, 0} -#define meshtastic_MeshPacket_init_default {0, 0, 0, 0, {meshtastic_Data_init_default}, 0, 0, 0, 0, 0, _meshtastic_MeshPacket_Priority_MIN, 0, _meshtastic_MeshPacket_Delayed_MIN, 0, 0, {0, {0}}, 0, 0, 0, 0, _meshtastic_MeshPacket_TransportMechanism_MIN, 0} +#define meshtastic_MeshPacket_init_default {0, 0, 0, 0, {meshtastic_Data_init_default}, 0, 0, 0, 0, 0, _meshtastic_MeshPacket_Priority_MIN, false, 0, _meshtastic_MeshPacket_Delayed_MIN, 0, 0, {0, {0}}, 0, 0, 0, 0, _meshtastic_MeshPacket_TransportMechanism_MIN, 0} #define meshtastic_NodeInfo_init_default {0, false, meshtastic_User_init_default, false, meshtastic_Position_init_default, 0, 0, false, meshtastic_DeviceMetrics_init_default, 0, 0, false, 0, 0, 0, 0, 0, 0} #define meshtastic_MyNodeInfo_init_default {0, 0, 0, {0, {0}}, "", _meshtastic_FirmwareEdition_MIN, 0} #define meshtastic_LogRecord_init_default {"", 0, "", _meshtastic_LogRecord_Level_MIN} @@ -1769,7 +1779,7 @@ extern "C" { #define meshtastic_Waypoint_init_zero {0, false, 0, false, 0, 0, 0, "", "", 0, 0, false, meshtastic_BoundingBox_init_zero, 0, 0, 0} #define meshtastic_StatusMessage_init_zero {""} #define meshtastic_MqttClientProxyMessage_init_zero {"", 0, {{0, {0}}}, 0} -#define meshtastic_MeshPacket_init_zero {0, 0, 0, 0, {meshtastic_Data_init_zero}, 0, 0, 0, 0, 0, _meshtastic_MeshPacket_Priority_MIN, 0, _meshtastic_MeshPacket_Delayed_MIN, 0, 0, {0, {0}}, 0, 0, 0, 0, _meshtastic_MeshPacket_TransportMechanism_MIN, 0} +#define meshtastic_MeshPacket_init_zero {0, 0, 0, 0, {meshtastic_Data_init_zero}, 0, 0, 0, 0, 0, _meshtastic_MeshPacket_Priority_MIN, false, 0, _meshtastic_MeshPacket_Delayed_MIN, 0, 0, {0, {0}}, 0, 0, 0, 0, _meshtastic_MeshPacket_TransportMechanism_MIN, 0} #define meshtastic_NodeInfo_init_zero {0, false, meshtastic_User_init_zero, false, meshtastic_Position_init_zero, 0, 0, false, meshtastic_DeviceMetrics_init_zero, 0, 0, false, 0, 0, 0, 0, 0, 0} #define meshtastic_MyNodeInfo_init_zero {0, 0, 0, {0, {0}}, "", _meshtastic_FirmwareEdition_MIN, 0} #define meshtastic_LogRecord_init_zero {"", 0, "", _meshtastic_LogRecord_Level_MIN} @@ -2194,7 +2204,7 @@ X(a, STATIC, SINGULAR, FLOAT, rx_snr, 8) \ X(a, STATIC, SINGULAR, UINT32, hop_limit, 9) \ X(a, STATIC, SINGULAR, BOOL, want_ack, 10) \ X(a, STATIC, SINGULAR, UENUM, priority, 11) \ -X(a, STATIC, SINGULAR, INT32, rx_rssi, 12) \ +X(a, STATIC, OPTIONAL, INT32, rx_rssi, 12) \ X(a, STATIC, SINGULAR, UENUM, delayed, 13) \ X(a, STATIC, SINGULAR, BOOL, via_mqtt, 14) \ X(a, STATIC, SINGULAR, UINT32, hop_start, 15) \ diff --git a/src/mesh/udp/UdpMulticastHandler.h b/src/mesh/udp/UdpMulticastHandler.h index 37ab44524..60967ccef 100644 --- a/src/mesh/udp/UdpMulticastHandler.h +++ b/src/mesh/udp/UdpMulticastHandler.h @@ -86,9 +86,13 @@ class UdpMulticastHandler final UniquePacketPoolPacket p = packetPool.allocUniqueCopy(mp); if (!p) return; - // Unset received SNR/RSSI + // Unset received SNR/RSSI - no local RF measurement exists for a UDP arrival. rx_rssi + // has explicit presence, so also clear has_rx_rssi: `mp` may have arrived already + // carrying a real measurement from whichever node forwarded it onto UDP, and leaving + // the presence bit set would misrepresent that stale value as "0 dBm over UDP". p->rx_snr = 0; p->rx_rssi = 0; + p->has_rx_rssi = false; router->enqueueReceivedMessage(p.release()); } } diff --git a/src/modules/StoreForwardModule.cpp b/src/modules/StoreForwardModule.cpp index de1d6865c..3c6d2de7d 100644 --- a/src/modules/StoreForwardModule.cpp +++ b/src/modules/StoreForwardModule.cpp @@ -259,6 +259,7 @@ meshtastic_MeshPacket *StoreForwardModule::preparePayload(NodeNum dest, uint32_t p->rx_time = this->packetHistory[i].time; p->decoded.emoji = (uint32_t)this->packetHistory[i].emoji; p->rx_rssi = this->packetHistory[i].rx_rssi; + p->has_rx_rssi = true; // rx_rssi has explicit presence; the stored value was a genuine measurement p->rx_snr = this->packetHistory[i].rx_snr; p->hop_start = this->packetHistory[i].hop_start; p->hop_limit = this->packetHistory[i].hop_limit; diff --git a/src/modules/TraceRouteModule.cpp b/src/modules/TraceRouteModule.cpp index c0cac7123..4d3819f12 100644 --- a/src/modules/TraceRouteModule.cpp +++ b/src/modules/TraceRouteModule.cpp @@ -422,7 +422,11 @@ void TraceRouteModule::appendMyIDandSNR(meshtastic_RouteDiscovery *updated, floa } if (*snr_count < ROUTE_SIZE) { - snr_list[*snr_count] = (int8_t)(snr * 4); // Convert SNR to 1 byte + // Clamp before the cast: q4-scaled SNR at or below the demodulation floor can reach + // -128 (=-32dB), which is bit-identical to the INT8_MIN "unknown SNR" sentinel used + // throughout this file. Reserve -128 for the sentinel; clamp real readings to -127. + int32_t q4 = clamp(lroundf(snr * 4.0f), -127, 127); + snr_list[*snr_count] = (int8_t)q4; *snr_count += 1; } if (SNRonly) diff --git a/src/serialization/MeshPacketSerializer.cpp b/src/serialization/MeshPacketSerializer.cpp index 7dd728284..88d134496 100644 --- a/src/serialization/MeshPacketSerializer.cpp +++ b/src/serialization/MeshPacketSerializer.cpp @@ -426,7 +426,9 @@ std::string MeshPacketSerializer::JsonSerialize(const meshtastic_MeshPacket *mp, jsonObj["channel"] = (Json::UInt)mp->channel; jsonObj["type"] = msgType; jsonObj["sender"] = nodeDB->getNodeId(); - if (mp->rx_rssi != 0) + // rx_rssi has explicit presence on the wire (unlike rx_snr): trust has_rx_rssi rather than a + // != 0 heuristic, since 0 dBm is a legitimate reading on some radios. + if (mp->has_rx_rssi) jsonObj["rssi"] = (int)mp->rx_rssi; if (mp->rx_snr != 0) jsonObj["snr"] = (float)mp->rx_snr; @@ -456,7 +458,9 @@ std::string MeshPacketSerializer::JsonSerializeEncrypted(const meshtastic_MeshPa jsonObj["channel"] = (Json::UInt)mp->channel; jsonObj["want_ack"] = mp->want_ack; - if (mp->rx_rssi != 0) + // rx_rssi has explicit presence on the wire (unlike rx_snr): trust has_rx_rssi rather than a + // != 0 heuristic, since 0 dBm is a legitimate reading on some radios. + if (mp->has_rx_rssi) jsonObj["rssi"] = (int)mp->rx_rssi; if (mp->rx_snr != 0) jsonObj["snr"] = (float)mp->rx_snr; diff --git a/test/test_fuzz_packets/test_main.cpp b/test/test_fuzz_packets/test_main.cpp index c9edcabb1..2bccf1d40 100644 --- a/test/test_fuzz_packets/test_main.cpp +++ b/test/test_fuzz_packets/test_main.cpp @@ -616,6 +616,7 @@ void test_E7_nodedb_update_fuzz(void) mp.id = rngNext(); mp.rx_snr = (float)((int)rngRange(60) - 30); mp.rx_rssi = (int32_t)rngRange(256) - 128; + mp.has_rx_rssi = (rngRange(2) != 0); // exercise both the presence and absence paths mp.hop_start = (uint8_t)rngRange(8); mp.hop_limit = (uint8_t)rngRange(8); mp.which_payload_variant = meshtastic_MeshPacket_decoded_tag; @@ -666,6 +667,7 @@ static void fuzzRxHeader(meshtastic_MeshPacket &mp, meshtastic_PortNum portnum) mp.channel = (uint8_t)rngRange(channels.getNumChannels()); mp.rx_snr = (float)((int)rngRange(40) - 20); mp.rx_rssi = -(int)rngRange(130); + mp.has_rx_rssi = (rngRange(2) != 0); // exercise both the presence and absence paths mp.hop_start = (uint8_t)rngRange(8); // 0..7, wire-bounded mp.hop_limit = (uint8_t)rngRange(8); mp.want_ack = (rngRange(2) == 0); diff --git a/test/test_meshpacket_serializer/test_helpers.h b/test/test_meshpacket_serializer/test_helpers.h index 5f5efd2cf..da8c89fff 100644 --- a/test/test_meshpacket_serializer/test_helpers.h +++ b/test/test_meshpacket_serializer/test_helpers.h @@ -40,6 +40,7 @@ static meshtastic_MeshPacket create_test_packet(meshtastic_PortNum port, const u packet.rx_snr = 10.5f; packet.hop_start = 3; packet.rx_rssi = -85; + packet.has_rx_rssi = true; // rx_rssi has explicit presence; mark this synthetic reading as measured packet.delayed = meshtastic_MeshPacket_Delayed_NO_DELAY; // Set decoded variant