Skip to content

Commit 65117c0

Browse files
committed
Address maintainer feedback: reclassify RX paths + restore log detail
Review feedback from @GUVWAF and @Copilot on PR #10252: 1. CRC mismatch (GUVWAF): keep as LOG_INFO (not DEBUG), restore the full parameter set (id, from, to, flags, SNR, RSSI, nextHop, relay) — it is valuable info for operators even if it is not ERROR severity. 2. 'from == 0' path (GUVWAF): this is a correctly-received OTA packet that we refuse to process by policy, not a bad RX. Remove the rxBad++ / airTime logging I had added; rxGood++ already counts the OTA reception above. 3. payloadLen < 0 path (GUVWAF): same reasoning — not a bad RX class. Remove the rxBad++ / airTime additions; retain only the LOG_WARN. 4. Packet pool exhaustion (Copilot): the allocator (Allocator:: allocZeroed) already emits a WARN on failure. Drop the duplicate LOG_ERROR I had added to avoid log flooding under sustained pressure. Behaviour now preserved: - all 4 original bugs still fixed (CRC log volume, randomBytes off-by-one, from==0 no-deref, pool-exhaustion null-check) - counter semantics restored to match existing philosophy - log signal-to-noise improved without dropping useful parameters
1 parent 879a5f9 commit 65117c0

1 file changed

Lines changed: 13 additions & 14 deletions

File tree

src/mesh/RadioLibInterface.cpp

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,15 @@ void RadioLibInterface::handleReceiveInterrupt()
483483
}
484484
#endif
485485
if (state != RADIOLIB_ERR_NONE) {
486-
// CRC errors are normal noise in RF-congested areas — drop to DEBUG so we don't flood logs.
486+
// Log PacketHeader similar to RadioInterface::printPacket so we can try to match RX errors to other packets in the logs.
487+
// CRC mismatches are routine in RF-congested areas (noise mis-demodulating to a syncword hit) — not an error class worth
488+
// ERROR severity, but still valuable info for operators looking at the log. Everything else stays at LOG_ERROR.
487489
if (state == RADIOLIB_ERR_CRC_MISMATCH) {
488-
LOG_DEBUG("RX CRC mismatch (noise?) fr=0x%08x rxSNR=%g rxRSSI=%i",
489-
radioBuffer.header.from, iface->getSNR(), lround(iface->getRSSI()));
490+
LOG_INFO("Ignore received packet due to CRC mismatch (maybe id=0x%08x fr=0x%08x to=0x%08x flags=0x%02x rxSNR=%g "
491+
"rxRSSI=%i nextHop=0x%x relay=0x%x)",
492+
radioBuffer.header.id, radioBuffer.header.from, radioBuffer.header.to, radioBuffer.header.flags,
493+
iface->getSNR(), lround(iface->getRSSI()), radioBuffer.header.next_hop, radioBuffer.header.relay_node);
490494
} else {
491-
// Log PacketHeader similar to RadioInterface::printPacket so we can try to match RX errors to other packets in the logs.
492495
LOG_ERROR("Ignore received packet due to error=%d (maybe id=0x%08x fr=0x%08x to=0x%08x flags=0x%02x rxSNR=%g rxRSSI=%i "
493496
"nextHop=0x%x relay=0x%x)",
494497
state, radioBuffer.header.id, radioBuffer.header.from, radioBuffer.header.to, radioBuffer.header.flags,
@@ -505,28 +508,24 @@ void RadioLibInterface::handleReceiveInterrupt()
505508
// check for short packets
506509
if (payloadLen < 0) {
507510
LOG_WARN("Ignore received packet too short");
508-
rxBad++;
509-
airTime->logAirtime(RX_ALL_LOG, rxMsec);
510511
} else {
511-
// altered packet with "from == 0" can do Remote Node Administration without permission
512+
rxGood++;
513+
// altered packet with "from == 0" can do Remote Node Administration without permission.
514+
// Still counted as a "good" OTA reception above — we just refuse to process it.
512515
if (radioBuffer.header.from == 0) {
513516
LOG_WARN("Ignore received packet without sender");
514-
rxBad++;
515-
airTime->logAirtime(RX_ALL_LOG, rxMsec);
516517
return;
517518
}
518519

519-
rxGood++;
520-
521520
// Note: we deliver _all_ packets to our router (i.e. our interface is intentionally promiscuous).
522521
// This allows the router and other apps on our node to sniff packets (usually routing) between other
523522
// nodes.
524523
meshtastic_MeshPacket *mp = packetPool.allocZeroed();
525524

526-
// Packet pool exhaustion: drop this packet on the floor rather than deref NULL.
527-
// Happens under sustained heap pressure (e.g., config dump mid-RX, dense mesh bursts).
525+
// Packet pool exhaustion: drop this packet rather than deref NULL. Can happen under
526+
// sustained heap pressure (e.g. config dump mid-RX, dense mesh bursts). The allocator
527+
// already emits a WARN on failure, so don't duplicate it here.
528528
if (!mp) {
529-
rxBad++;
530529
airTime->logAirtime(RX_ALL_LOG, rxMsec);
531530
return;
532531
}

0 commit comments

Comments
 (0)