From 9662385249f0c6ae64a54d892e9b892d54349629 Mon Sep 17 00:00:00 2001 From: Bryan Biedenkapp Date: Thu, 24 Sep 2026 12:43:58 -0400 Subject: [PATCH] correct various P25P2 issues; correct bad P25P1 bit additions from network TSDUs; --- src/common/p25/P25Defines.h | 2 +- src/common/p25/lc/LC.cpp | 11 +++++--- src/host/p25/packet/ControlSignaling.cpp | 6 ++--- src/host/p25/phase2/Control.cpp | 4 +-- src/host/p25/phase2/Control.h | 3 ++- src/host/p25/phase2/Slot.cpp | 16 +++++++++++- src/host/p25/phase2/Slot.h | 5 +++- tests/p25/P2_Control_Tests.cpp | 32 +++++++++++++++++++++++- tests/p25/P2_VCH_MACPDU_Test.cpp | 20 +++++++++++++++ 9 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/common/p25/P25Defines.h b/src/common/p25/P25Defines.h index 54b484d8..7f8f34b0 100644 --- a/src/common/p25/P25Defines.h +++ b/src/common/p25/P25Defines.h @@ -931,7 +931,7 @@ namespace p25 PRIVATE = 0x02U, //!< UU VCH USER - Unit-to-Unit Voice Channel User TEL_INT_VCH_USER = 0x03U, //!< TEL INT VCH USER - Telephone Interconnect Voice Channel User - MAC_RELEASE = 0x61U, //!< MAC RELEASE - MAC Release + MAC_RELEASE = 0x31U, //!< MAC RELEASE - MAC Release /* Any abbreviated or explicit partition opcodes are essentially just TSBKO's. */ }; diff --git a/src/common/p25/lc/LC.cpp b/src/common/p25/lc/LC.cpp index 3a061336..aebc8b96 100644 --- a/src/common/p25/lc/LC.cpp +++ b/src/common/p25/lc/LC.cpp @@ -1365,8 +1365,8 @@ bool LC::decodeMACPDU(const uint8_t* raw, uint32_t macLength) ** bryanb: likely will need extra work here -- IDLE,ACTIVE,HANGTIME PDUs can contain multiple ** MCOs; for now we're only gonna be decoding the first one... */ - m_macPartition = raw[1U] >> 5U; // MAC Partition - m_lco = raw[1U] & 0x1FU; // MCO + m_macPartition = raw[1U] & 0xC0U; // MAC Partition + m_lco = raw[1U] & 0x3FU; // MCO if (m_macPartition == P2_MAC_MCO_PARTITION::UNIQUE) { switch (m_lco) { @@ -1468,8 +1468,8 @@ void LC::encodeMACPDU(uint8_t* raw, uint32_t macLength) ** bryanb: likely will need extra work here -- IDLE,ACTIVE,HANGTIME PDUs can contain multiple ** MCOs; for now we're only gonna be decoding the first one... */ - raw[1U] = ((m_macPartition & 0x07U) << 5U) + // MAC Partition - (m_lco & 0x1FU); // MCO + raw[1U] = (m_macPartition & 0xC0U) + // MAC Partition + (m_lco & 0x3FU); // MCO if (m_macPartition == P2_MAC_MCO_PARTITION::UNIQUE) { switch (m_lco) { @@ -1875,6 +1875,9 @@ void LC::applyP2Scrambler(uint8_t* data, bool inbound, bool sync) // initialize the scrambler state based on the network ID, system ID, color code, and direction uint64_t state = p25P2InitialScramblerState(s_siteData.netId(), s_siteData.sysId(), m_colorCode, inbound); state = p25P2AdvanceScrambler(state, (uint64_t)m_p2ScrambleOffset); + // Scrambler-sequence bits corresponding to leading sync/DUID positions are + // discarded even though those burst bits are not XORed. + state = p25P2AdvanceScrambler(state, cursor); // apply the scrambler to each field, advancing the scrambler state as necessary for (uint8_t field = 0U; field < fieldCount; field++) { diff --git a/src/host/p25/packet/ControlSignaling.cpp b/src/host/p25/packet/ControlSignaling.cpp index 4c7b810e..97662099 100644 --- a/src/host/p25/packet/ControlSignaling.cpp +++ b/src/host/p25/packet/ControlSignaling.cpp @@ -1561,8 +1561,8 @@ void ControlSignaling::writeNet_TSDU(lc::TSBK* tsbk) tsbk->encode(buffer + 2U); // add status bits - P25Utils::addStatusBits(buffer + 2U, P25_TSDU_FRAME_LENGTH_BYTES, false, true); - P25Utils::addIdleStatusBits(buffer + 2U, P25_TSDU_FRAME_LENGTH_BYTES); + P25Utils::addStatusBits(buffer + 2U, P25_TSDU_FRAME_LENGTH_BITS, false, true); + P25Utils::addIdleStatusBits(buffer + 2U, P25_TSDU_FRAME_LENGTH_BITS); P25Utils::setStatusBitsStartIdle(buffer + 2U); m_p25->addFrame(buffer, P25_TSDU_FRAME_LENGTH_BYTES + 2U, true); @@ -3174,7 +3174,7 @@ void ControlSignaling::writeNet_TSDU_From_RF(lc::TSBK* tsbk, uint8_t* data) tsbk->encode(data); // add status bits - P25Utils::addStatusBits(data, P25_TSDU_FRAME_LENGTH_BYTES, false, false); + P25Utils::addStatusBits(data, P25_TSDU_FRAME_LENGTH_BITS, false, false); P25Utils::setStatusBitsStartIdle(data); } diff --git a/src/host/p25/phase2/Control.cpp b/src/host/p25/phase2/Control.cpp index 68cb28a8..fff63ab0 100644 --- a/src/host/p25/phase2/Control.cpp +++ b/src/host/p25/phase2/Control.cpp @@ -24,7 +24,7 @@ using namespace p25::phase2; Control::Control(bool authoritative, uint32_t callHang, uint32_t timeout, uint32_t tgHang, modem::Modem* modem, network::Network* network, lookups::P25AffiliationLookup* affiliations, ::lookups::RadioIdLookup* ridLookup, ::lookups::TalkgroupRulesLookup* tidLookup, - uint32_t queueSize, bool debug, bool verbose) : + uint32_t queueSize, bool debug, bool verbose, uint16_t colorCode) : m_modem(modem), m_network(network), m_slot1(nullptr), @@ -38,7 +38,7 @@ Control::Control(bool authoritative, uint32_t callHang, uint32_t timeout, uint32 m_debug(debug), m_verbose(verbose) { - Slot::init(this, authoritative, callHang, modem, network, affiliations, ridLookup, tidLookup); + Slot::init(this, authoritative, callHang, modem, network, affiliations, ridLookup, tidLookup, colorCode); m_slot1 = new Slot(0U, timeout, tgHang, queueSize, debug, verbose); m_slot2 = new Slot(1U, timeout, tgHang, queueSize, debug, verbose); } diff --git a/src/host/p25/phase2/Control.h b/src/host/p25/phase2/Control.h index 0cc4dd30..28999bee 100644 --- a/src/host/p25/phase2/Control.h +++ b/src/host/p25/phase2/Control.h @@ -66,11 +66,12 @@ namespace p25 * @param queueSize Modem frame buffer queue size (bytes). * @param debug Flag indicating whether P25 debug is enabled. * @param verbose Flag indicating whether P25 verbose logging is enabled. + * @param colorCode P25 Phase 2 color code expected on inbound unsrambled MAC PDUs. */ Control(bool authoritative, uint32_t callHang, uint32_t timeout, uint32_t tgHang, modem::Modem* modem, network::Network* network, lookups::P25AffiliationLookup* affiliations, ::lookups::RadioIdLookup* ridLookup, ::lookups::TalkgroupRulesLookup* tidLookup, - uint32_t queueSize, bool debug, bool verbose); + uint32_t queueSize, bool debug, bool verbose, uint16_t colorCode = 0U); /** * @brief Finalizes a P25 Phase 2 controller. */ diff --git a/src/host/p25/phase2/Slot.cpp b/src/host/p25/phase2/Slot.cpp index 1f4db395..4f1cdf66 100644 --- a/src/host/p25/phase2/Slot.cpp +++ b/src/host/p25/phase2/Slot.cpp @@ -52,6 +52,7 @@ using namespace p25::defines; bool Slot::s_authoritative = true; uint32_t Slot::s_callHang = 0U; +uint16_t Slot::s_colorCode = 0U; modem::Modem* Slot::s_modem = nullptr; network::Network* Slot::s_network = nullptr; @@ -304,6 +305,17 @@ bool Slot::processFrame(uint8_t* data, uint32_t length) break; case defines::P2_MAC_HEADER_OPCODE::END_PTT: if (isFACCHDUID(duid)) { + const bool colorCodeMatches = m_control.getColorCode() == s_colorCode; + const bool addressMatches = m_rfLastDstId == 0U || m_control.getDstId() == m_rfLastDstId; + if (!colorCodeMatches || !addressMatches) { + if (m_debug) { + LogDebugEx(LOG_RF, "Slot::processFrame()", + "P25 Phase 2 Slot %u, ignoring END_PTT for another call, colorCode = $%03X, dstId = %u", + m_slotNo + 1U, m_control.getColorCode(), m_control.getDstId()); + } + return true; + } + m_rfEndPTTCount++; if (m_rfEndPTTCount == 2U) { ::ActivityLog("P25P2", true, "Slot %u RF voice call ended from %u to %u, %.1f seconds, BER: %.1f%%", @@ -704,13 +716,15 @@ void Slot::clock(uint32_t ms) /* Helper to initialize shared P25 Phase 2 slot configuration. */ void Slot::init(Control* control, bool authoritative, uint32_t callHang, modem::Modem* modem, network::Network* network, - lookups::P25AffiliationLookup* affiliations, ::lookups::RadioIdLookup* ridLookup, ::lookups::TalkgroupRulesLookup* tidLookup) + lookups::P25AffiliationLookup* affiliations, ::lookups::RadioIdLookup* ridLookup, + ::lookups::TalkgroupRulesLookup* tidLookup, uint16_t colorCode) { s_control = control; s_authoritative = authoritative; s_callHang = callHang; + s_colorCode = colorCode & 0x0FFFU; s_modem = modem; s_network = network; diff --git a/src/host/p25/phase2/Slot.h b/src/host/p25/phase2/Slot.h index 9118674a..5968c510 100644 --- a/src/host/p25/phase2/Slot.h +++ b/src/host/p25/phase2/Slot.h @@ -175,9 +175,11 @@ namespace p25 * @param affiliations Instance of the P25AffiliationLookup class. * @param ridLookup Instance of the RadioIdLookup class. * @param tidLookup Instance of the TalkgroupRulesLookup class. + * @param colorCode P25 Phase 2 color code expected on inbound unsrambled MAC PDUs. */ static void init(Control* control, bool authoritative, uint32_t callHang, modem::Modem* modem, network::Network* network, - lookups::P25AffiliationLookup* affiliations, ::lookups::RadioIdLookup* ridLookup, ::lookups::TalkgroupRulesLookup* tidLookup); + lookups::P25AffiliationLookup* affiliations, ::lookups::RadioIdLookup* ridLookup, + ::lookups::TalkgroupRulesLookup* tidLookup, uint16_t colorCode = 0U); private: #if defined(CATCH2_TEST_COMPILATION) @@ -272,6 +274,7 @@ namespace p25 static bool s_authoritative; static uint32_t s_callHang; + static uint16_t s_colorCode; static modem::Modem* s_modem; static network::Network* s_network; diff --git a/tests/p25/P2_Control_Tests.cpp b/tests/p25/P2_Control_Tests.cpp index 1bba471f..f269d012 100644 --- a/tests/p25/P2_Control_Tests.cpp +++ b/tests/p25/P2_Control_Tests.cpp @@ -41,13 +41,14 @@ namespace { */ std::array makeInboundMAC( P25DEF::P2_DUID::E duid, uint8_t opcode, uint32_t srcId = 0x123456U, - uint32_t dstId = 0x2345U) + uint32_t dstId = 0x2345U, uint16_t colorCode = 0U) { p25::lc::LC control; control.setGroup(true); control.setLCO(P25DEF::P2_MAC_MCO::GROUP); control.setSrcId(srcId); control.setDstId(dstId); + control.setColorCode(colorCode); control.setP2DUID(static_cast(duid)); control.setMACPDUOpcode(opcode); @@ -58,6 +59,35 @@ std::array makeInboundMAC( return frame; } +TEST_CASE("P25 Phase 2 ignores END_PTT for another call", "[p25][p2][end-ptt]") +{ + p25::phase2::Control control(true, 1U, 20U, 20U, nullptr, nullptr, nullptr, nullptr, + nullptr, 4096U, false, false); + p25::phase2::Slot& slot = HostTestHooks::p25P2Slot(control, 0U); + + auto ptt = makeInboundMAC(P25DEF::P2_DUID::FACCH_UNSCRAMBLED, + P25DEF::P2_MAC_HEADER_OPCODE::PTT); + REQUIRE(control.processFrame(0U, ptt.data(), ptt.size())); + + auto wrongColor = makeInboundMAC(P25DEF::P2_DUID::FACCH_UNSCRAMBLED, + P25DEF::P2_MAC_HEADER_OPCODE::END_PTT, 0xFFFFFFU, 0x2345U, 1U); + REQUIRE(control.processFrame(0U, wrongColor.data(), wrongColor.size())); + REQUIRE(control.processFrame(0U, wrongColor.data(), wrongColor.size())); + REQUIRE(HostTestHooks::p25P2RFVCHState(slot) == p25::phase2::Slot::VCH_STATE::PTT); + + auto wrongAddress = makeInboundMAC(P25DEF::P2_DUID::FACCH_UNSCRAMBLED, + P25DEF::P2_MAC_HEADER_OPCODE::END_PTT, 0xFFFFFFU, 0x3456U); + REQUIRE(control.processFrame(0U, wrongAddress.data(), wrongAddress.size())); + REQUIRE(control.processFrame(0U, wrongAddress.data(), wrongAddress.size())); + REQUIRE(HostTestHooks::p25P2RFVCHState(slot) == p25::phase2::Slot::VCH_STATE::PTT); + + auto matching = makeInboundMAC(P25DEF::P2_DUID::FACCH_UNSCRAMBLED, + P25DEF::P2_MAC_HEADER_OPCODE::END_PTT, 0xFFFFFFU); + REQUIRE(control.processFrame(0U, matching.data(), matching.size())); + REQUIRE(control.processFrame(0U, matching.data(), matching.size())); + REQUIRE(HostTestHooks::p25P2RFVCHState(slot) == p25::phase2::Slot::VCH_STATE::HANGTIME); +} + /** * @brief Creates a P25 Phase 2 voice burst frame for testing purposes. * @param duid The DUID of the voice burst. diff --git a/tests/p25/P2_VCH_MACPDU_Test.cpp b/tests/p25/P2_VCH_MACPDU_Test.cpp index 4578dbd3..05a4ee44 100644 --- a/tests/p25/P2_VCH_MACPDU_Test.cpp +++ b/tests/p25/P2_VCH_MACPDU_Test.cpp @@ -11,6 +11,7 @@ #include "common/edac/RS634717.h" #include "common/p25/P25Defines.h" #include "common/p25/lc/LC.h" +#include "common/p25/lc/mac/MACFactory.h" #include "common/p25/Sync.h" #include "common/Log.h" #include "common/Utils.h" @@ -386,6 +387,25 @@ TEST_CASE("P25 Phase 2 VCH MAC PDU Scrambled I-OEMI Round-Trip Test", "[p25][p2_ REQUIRE(failed == false); } +TEST_CASE("P25 Phase 2 MAC Release preserves six-bit MCO on wire", "[p25][p2][mac-release]") { + LC lc; + lc.setLCO(P2_MAC_MCO::MAC_RELEASE); + lc.setSrcId(0x123456U); + lc.setP2DUID(P2_DUID::SACCH_UNSCRAMBLED); + lc.setMACPDUOpcode(P2_MAC_HEADER_OPCODE::ACTIVE); + lc.setMACPartition(P2_MAC_MCO_PARTITION::UNIQUE); + + uint8_t encodedData[P25_P2_FRAME_LENGTH_BYTES] = { 0U }; + lc.encodeVCH_MACPDU(encodedData, false); + + LC decoded; + REQUIRE(decoded.decodeVCH_MACPDU_OEMI(encodedData, false)); + REQUIRE(decoded.getMACPartition() == P2_MAC_MCO_PARTITION::UNIQUE); + REQUIRE(decoded.getLCO() == P2_MAC_MCO::MAC_RELEASE); + REQUIRE(decoded.getSrcId() == 0x123456U); + REQUIRE(p25::lc::mac::MACFactory::createMACPDU(decoded) != nullptr); +} + TEST_CASE("P25 Phase 2 VCH MAC PDU Scrambled S-OEMI Round-Trip Test", "[p25][p2_vch_macpdu_scrambled_soemi][!mayfail]") { bool failed = false;