From c500f7d4cecbf242d20c1ca98fc10b053f792b7f Mon Sep 17 00:00:00 2001 From: Dave Behnke <916775+dbehnke@users.noreply.github.com> Date: Sun, 28 Dec 2025 15:32:13 -0500 Subject: [PATCH] Cleanup P25 debug logs and buffering, keep Header/Terminator fixes --- reflector/P25Protocol.cpp | 96 ++------------------------------------- 1 file changed, 3 insertions(+), 93 deletions(-) diff --git a/reflector/P25Protocol.cpp b/reflector/P25Protocol.cpp index dda8d44..e02b7d7 100644 --- a/reflector/P25Protocol.cpp +++ b/reflector/P25Protocol.cpp @@ -23,10 +23,7 @@ #include "P25Protocol.h" #include "Global.h" -#include -#include - -static std::map> g_P25Pending; +#include "Global.h" const uint8_t REC62[] = {0x62U, 0x02U, 0x02U, 0x0CU, 0x0BU, 0x12U, 0x64U, 0x00U, 0x00U, 0x80U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U,0x00U, 0x00U, 0x00U, 0x00U, 0x00U}; const uint8_t REC63[] = {0x63U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x02U}; @@ -98,10 +95,6 @@ void CP25Protocol::Task(void) // crack the packet if ( IsValidDvPacket(Ip, Buffer, Frame) ) { - if (Frame == nullptr) { - // Buffered packet, waiting for header - } - else { if( !m_uiStreamId && IsValidDvHeaderPacket(Ip, Buffer, Header) ) { // callsign muted? @@ -112,35 +105,16 @@ void CP25Protocol::Task(void) // Fix Header Orphan: If Header packet 0x66 was also parsed as a Frame with ID 0, // update its ID now that stream is open (m_uiStreamId is set). if (Frame && Frame->GetStreamId() == 0 && m_uiStreamId != 0) { - // CDvFramePacket doesn't have SetStreamId? It's often immutable or difficult. - // But CPacket has m_uiStreamId (protected). - // CDvFramePacket usually exposes it? - // Check Packet.h: CPacket has m_uiStreamId. - // We need a way to set it. - // CPacket::SetStreamId doesn't exist? - // Let's check CPacket in Packet.h. - // CPacket has `uint16_t m_uiStreamId;` protected. - // It doesn't have a setter? - // Wait, `SetStreamId` is needed. - // Or recreate the frame? - // Frame is unique_ptr. - // frame = std::unique_ptr(new CDvFramePacket(&(Buffer.data()[offset]), m_uiStreamId, last)); + // Recreate frame with correct ID // We know the offset for 0x66 is 5U. - // Let's recreate it. int offset = 5U; // For 0x66 - // But Frame is generic here. Buffer[0] might not be 0x66? - // ValidDvHeaderPacket checks Buffer[0] == 0x66. - // So yes, it is 0x66. bool last = false; - // Recreate frame with correct ID Frame = std::unique_ptr(new CDvFramePacket(&(Buffer.data()[offset]), m_uiStreamId, last)); - printf("P25_DEBUG: Fixed Orphaned Header Frame 0x%X ID 0 -> 0x%X\n", Buffer.data()[0U], m_uiStreamId); } } } // push the packet OnDvFramePacketIn(Frame, &Ip); - } } else if ( IsValidConnectPacket(Buffer, &Callsign) ) { @@ -344,19 +318,6 @@ bool CP25Protocol::IsValidDvPacket(const CIp &Ip, const CBuffer &Buffer, std::un { if ( (Buffer.size() >= 14) ) { - // P25/MMDVM sends 0x62..0x65 BEFORE 0x66 (Header/StreamID). - // We must buffer these until 0x66 arrives to avoid "Orphaned Frame" loss. - if (m_uiStreamId == 0 && Buffer.data()[0U] != 0x66U && Buffer.data()[0U] != 0x80U) { - // Buffer this packet - std::vector& list = g_P25Pending[Ip]; - if (list.size() < 20) { // Safety limit - list.push_back(Buffer); - printf("P25_DEBUG: Buffering pre-header frame 0x%02X (Count=%zu)\n", Buffer.data()[0U], list.size()); - } - frame = nullptr; - return true; // Claim valid to prevent "Unknown" log, but return no frame - } - int offset = 0; bool last = false; @@ -401,36 +362,21 @@ bool CP25Protocol::IsValidDvPacket(const CIp &Ip, const CBuffer &Buffer, std::un case 0x73U: offset = 4U; break; - case 0x80U: + case 0x80U: { - printf("P25_DEBUG: RX 0x80 Terminator. Resetting StreamID 0x%X -> 0\n", m_uiStreamId); last = true; uint32_t lastId = m_uiStreamId; // Capture ID before reset m_uiStreamId = 0; - g_P25Pending[Ip].clear(); // Clear any pending garbage // Override creation with lastId frame = std::unique_ptr(new CDvFramePacket(&(Buffer.data()[0U]), lastId, last)); - // Note: 0x80 usually has no payload or minimal. - // CDvFramePacket constructor expects data pointer. - // Buffer.data()[0U] is the 0x80 byte itself. - // IsValidDvPacket logic for 0x80 usually did "break". - // It fell through to `new CDvFramePacket(&(Buffer.data()[offset]), m_uiStreamId, last));` - // offset was 0 ! - // (Buffer.data()[0U]) is correct. return true; - // Return early to avoid the fallthrough creation with ID 0 } break; default: - printf("P25_DEBUG: Unknown P25 Byte0=0x%02X\n", Buffer.data()[0U]); break; } - if (m_uiStreamId == 0 && Buffer.data()[0U] != 0x66U) { - printf("P25_DEBUG: IsValidDvPacket ID=0 for Byte0=0x%02X (Should have been buffered)\n", Buffer.data()[0U]); - } - frame = std::unique_ptr(new CDvFramePacket(&(Buffer.data()[offset]), m_uiStreamId, last)); return true; } @@ -452,42 +398,6 @@ bool CP25Protocol::IsValidDvHeaderPacket(const CIp &Ip, const CBuffer &Buffer, s rpt1.SetCSModule(P25_MODULE_ID); rpt2.SetCSModule(' '); header = std::unique_ptr(new CDvHeaderPacket(csMY, CCallsign("CQCQCQ"), rpt1, rpt2, m_uiStreamId, false)); - - // Replay buffered packets now that we have a Stream ID - std::vector& list = g_P25Pending[Ip]; - if (!list.empty()) { - printf("P25_DEBUG: Replaying %zu buffered frames for StreamID 0x%X\n", list.size(), m_uiStreamId); - for (const auto& buf : list) { - std::unique_ptr delayedFrame; - // We call IsValidDvPacket recursively? No, infinite loop with header check? - // IsValidDvPacket logic needs to be manually invoked or bypassed - // Actually we can just perform the creation logic here since we know they are valid frames - int offset = 0; - switch (buf.data()[0U]) { - case 0x62U: offset = 10U; break; - case 0x63U: offset = 1U; break; - case 0x64U: offset = 5U; break; - case 0x65U: offset = 5U; break; - // ... assume standard offsets for buffered frames (they were filtered to be valid before?) - // No, IsValidDvPacket checks size >= 14. We should duplicate that minimal check or just trust our buffer. - // For simplicity, handle common 0x62-0x65 range which logic covers. - default: offset = 5U; break; // Best effort default - } - // For logic consistency we should probably duplicate the switch? - // Or call IsValidDvPacket but temporarily set m_uiStreamId != 0 (which it is now) - // The IsValidDvPacket logic will create the frame if m_uiStreamId != 0. - // But IsValidDvPacket signature is (..., unique_ptr&). - // Yes taking advantage of m_uiStreamId being set! - - if (IsValidDvPacket(Ip, buf, delayedFrame)) { - if (delayedFrame) { - // Push immediately - OnDvFramePacketIn(delayedFrame, &Ip); - } - } - } - list.clear(); - } } return true; }