From 532182149050ad233a63213760699671e15f0c65 Mon Sep 17 00:00:00 2001 From: Bryan Biedenkapp Date: Wed, 5 Aug 2026 15:16:14 -0400 Subject: [PATCH] fix incorrect assumptions by some HostControl tests; correct HostControl class name collisions that would cause strange link time errors; correct intermittent CRC-6 test failure; --- src/host/p25/Control.cpp | 6 +- tests/dmr/HostControl_Tests.cpp | 34 ++++++----- tests/edac/CRC_6_Test.cpp | 17 +++--- tests/nxdn/HostControl_Tests.cpp | 28 +++++---- tests/p25/HostControl_Tests.cpp | 102 +++++++++++++++++++++++++------ 5 files changed, 131 insertions(+), 56 deletions(-) diff --git a/src/host/p25/Control.cpp b/src/host/p25/Control.cpp index 1cb99372..879ccef9 100644 --- a/src/host/p25/Control.cpp +++ b/src/host/p25/Control.cpp @@ -1021,10 +1021,10 @@ void Control::clock() m_rfCallTermSrcId = 0U; } - if (m_netState == RS_NET_AUDIO || m_netState == RS_NET_DATA) { + if (m_networkWatchdog.isRunning()) { m_networkWatchdog.clock(ms); - if (m_networkWatchdog.isRunning() && m_networkWatchdog.hasExpired()) { + if (m_networkWatchdog.hasExpired()) { if (m_netState == RS_NET_AUDIO) { if (m_voice->m_netFrames > 0.0F) { ::ActivityLog("P25", false, "network watchdog has expired, %.1f seconds, %u%% packet loss", @@ -1476,6 +1476,8 @@ void Control::processNetwork() return; } + m_networkWatchdog.start(); + if (duid == DUID::PDU) { frameLength = length; data = std::unique_ptr(new uint8_t[length]); diff --git a/tests/dmr/HostControl_Tests.cpp b/tests/dmr/HostControl_Tests.cpp index 4ab94b0a..58f02e79 100644 --- a/tests/dmr/HostControl_Tests.cpp +++ b/tests/dmr/HostControl_Tests.cpp @@ -166,12 +166,12 @@ void buildDMRVoiceSyncPayload(uint8_t* payload) /** * @brief Dummy implementation of a modem port for testing purposes. */ -class TestModemPort final : public modem::port::IModemPort { +class DMRTestModemPort final : public modem::port::IModemPort { public: /** - * @brief Finalizes the instance of the TestModemPort class. + * @brief Finalizes the instance of the DMRTestModemPort class. */ - ~TestModemPort() override = default; + ~DMRTestModemPort() override = default; /** * @brief Opens the modem port. @@ -213,18 +213,22 @@ public: /** * @brief Lightweight network test double that records DMR reset calls. */ -class TestNetwork final : public network::Network { +class DMRTestNetwork final : public network::Network { public: /** - * @brief Initializes a new instance of the TestNetwork class. + * @brief Initializes a new instance of the DMRTestNetwork class. * @param localPort The local port number. * @param peerId The peer ID. */ - TestNetwork(uint16_t localPort = 0U, uint32_t peerId = 1U) : + DMRTestNetwork(uint16_t localPort = 0U, uint32_t peerId = 1U) : network::Network("127.0.0.1", 1U, localPort, peerId, "test", true, true, true, false, false, false, true, true, false, false, false, false), m_resetDMRCount(0U) { - /* stub */ + // keep protocol gates deterministic for this P25-focused harness + m_dmrEnabled = true; + m_p25Enabled = false; + m_nxdnEnabled = false; + m_analogEnabled = false; } /** @@ -388,14 +392,14 @@ public: */ explicit DMRHostHarness(bool authoritative = true, bool withNetwork = false, uint16_t networkLocalPort = 0U, uint32_t networkPeerId = 1U) : m_rpc("127.0.0.1", 1U, 0U, "test", false), - m_modem(new TestModemPort(), false, false, false, false, false, false, + m_modem(new DMRTestModemPort(), false, false, false, false, false, false, 0U, 0U, 0U, 4096U, 4096U, 1024U, true, true, false, false, false, false), m_chLookup(), m_ridLookup("", 0U, false, false), m_tidLookup("", 0U, false, false), m_idenLookup("", 0U), m_rssiMapper(), - m_network(withNetwork ? new TestNetwork(networkLocalPort, networkPeerId) : nullptr), + m_network(withNetwork ? new DMRTestNetwork(networkLocalPort, networkPeerId) : nullptr), m_control(nullptr) { g_RPC = &m_rpc; @@ -415,7 +419,7 @@ public: g_RPC = nullptr; } - TestNetwork* network() const + DMRTestNetwork* network() const { return m_network; } @@ -428,7 +432,7 @@ public: lookups::TalkgroupRulesLookup m_tidLookup; lookups::IdenTableLookup m_idenLookup; lookups::RSSIInterpolator m_rssiMapper; - TestNetwork* m_network; + DMRTestNetwork* m_network; dmr::Control* m_control; }; @@ -444,7 +448,7 @@ TEST_CASE("DMR host e2e loopback handles missed frames without dropping active c const uint32_t streamId = 0x610001U; DMRHostHarness harness(true, true, hostPort, hostPeerId); - TestNetwork sender(senderPort, 7002U); + DMRTestNetwork sender(senderPort, 7002U); REQUIRE(harness.network() != nullptr); REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort)); @@ -493,7 +497,7 @@ TEST_CASE("DMR host e2e loopback handles dropped call terminator and returns idl const uint32_t streamId = 0x610002U; DMRHostHarness harness(true, true, hostPort, hostPeerId); - TestNetwork sender(senderPort, 7004U); + DMRTestNetwork sender(senderPort, 7004U); REQUIRE(harness.network() != nullptr); REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort)); @@ -540,7 +544,7 @@ TEST_CASE("DMR host e2e loopback times out stale call and resets stream state", const uint32_t streamId = 0x610003U; DMRHostHarness harness(true, true, hostPort, hostPeerId); - TestNetwork sender(senderPort, 7006U); + DMRTestNetwork sender(senderPort, 7006U); REQUIRE(harness.network() != nullptr); REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort)); @@ -582,7 +586,7 @@ TEST_CASE("DMR host e2e loopback enforces stream lock until active stream termin const uint32_t streamB = 0x610102U; DMRHostHarness harness(true, true, hostPort, hostPeerId); - TestNetwork sender(senderPort, 7008U); + DMRTestNetwork sender(senderPort, 7008U); REQUIRE(harness.network() != nullptr); REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort)); diff --git a/tests/edac/CRC_6_Test.cpp b/tests/edac/CRC_6_Test.cpp index a9ef60df..a07808c2 100644 --- a/tests/edac/CRC_6_Test.cpp +++ b/tests/edac/CRC_6_Test.cpp @@ -25,20 +25,21 @@ TEST_CASE("CRC 6-bit Test", "[crc][6bit]") { srand((unsigned int)time(NULL)); - const uint32_t len = 32U; - const uint32_t lenBits = len * 8U; - uint8_t* random = (uint8_t*)malloc(len); + const uint32_t payloadLen = 32U; + const uint32_t lenBits = payloadLen * 8U; + const uint32_t totalLen = payloadLen + 1U; // spare byte stores 6-bit CRC at bit offset lenBits + uint8_t* random = (uint8_t*)calloc(totalLen, sizeof(uint8_t)); - for (size_t i = 0; i < len - 1U; i++) { + for (size_t i = 0; i < payloadLen; i++) { random[i] = rand(); } CRC::addCRC6(random, lenBits); - uint32_t inCrc = (random[len - 1U] << 0); + uint32_t inCrc = (random[totalLen - 1U] << 0); ::LogInfoEx("T", "CRC::checkCRC6(), crc = $%02X", inCrc); - Utils::dump(2U, "6_Sanity_Test CRC", random, len); + Utils::dump(2U, "6_Sanity_Test CRC", random, totalLen); bool ret = CRC::checkCRC6(random, lenBits); if (!ret) { @@ -47,8 +48,8 @@ TEST_CASE("CRC 6-bit Test", "[crc][6bit]") { goto cleanup; } - random[10U] >>= 8; - random[11U] >>= 8; + random[10U] ^= 0x01U; + random[11U] ^= 0x01U; ret = CRC::checkCRC6(random, lenBits); if (ret) { diff --git a/tests/nxdn/HostControl_Tests.cpp b/tests/nxdn/HostControl_Tests.cpp index 4415f907..9db8ac3a 100644 --- a/tests/nxdn/HostControl_Tests.cpp +++ b/tests/nxdn/HostControl_Tests.cpp @@ -107,12 +107,12 @@ uint16_t reserveLoopbackPort() /** * @brief Dummy implementation of a modem port for testing purposes. */ -class TestModemPort final : public modem::port::IModemPort { +class NXDNTestModemPort final : public modem::port::IModemPort { public: /** - * @brief Finalizes the instance of the TestModemPort class. + * @brief Finalizes the instance of the NXDNTestModemPort class. */ - ~TestModemPort() override = default; + ~NXDNTestModemPort() override = default; /** * @brief Opens the modem port. @@ -154,13 +154,17 @@ public: /** * @brief Lightweight network test double for NXDN ingress stream-lock tests. */ -class TestNetwork final : public network::Network { +class NXDNTestNetwork final : public network::Network { public: - TestNetwork(uint16_t localPort = 0U, uint32_t peerId = 1U) : - network::Network("127.0.0.1", 1U, localPort, peerId, "test", false, true, false, false, true, false, true, true, false, false, false, false), + NXDNTestNetwork(uint16_t localPort = 0U, uint32_t peerId = 1U) : + network::Network("127.0.0.1", 1U, localPort, peerId, "test", true, true, false, false, true, false, true, true, false, false, false, false), m_resetNXDNCount(0U) { - /* stub */ + // keep protocol gates deterministic for this P25-focused harness + m_dmrEnabled = false; + m_p25Enabled = false; + m_nxdnEnabled = true; + m_analogEnabled = false; } bool activateLoopback(const std::string& remoteAddress, uint16_t remotePort) @@ -262,14 +266,14 @@ public: */ explicit NXDNHostHarness(bool authoritative = true, bool withNetwork = false, uint16_t networkLocalPort = 0U, uint32_t networkPeerId = 1U) : m_rpc("127.0.0.1", 1U, 0U, "test", false), - m_modem(new TestModemPort(), false, false, false, false, false, false, + m_modem(new NXDNTestModemPort(), false, false, false, false, false, false, 0U, 0U, 0U, 1024U, 4096U, 1024U, true, true, false, false, false, false), m_chLookup(), m_ridLookup("", 0U, false, false), m_tidLookup("", 0U, false, false), m_idenLookup("", 0U), m_rssiMapper(), - m_network(withNetwork ? new TestNetwork(networkLocalPort, networkPeerId) : nullptr), + m_network(withNetwork ? new NXDNTestNetwork(networkLocalPort, networkPeerId) : nullptr), m_control(nullptr) { g_RPC = &m_rpc; @@ -288,7 +292,7 @@ public: g_RPC = nullptr; } - TestNetwork* network() const + NXDNTestNetwork* network() const { return m_network; } @@ -318,7 +322,7 @@ public: lookups::TalkgroupRulesLookup m_tidLookup; lookups::IdenTableLookup m_idenLookup; lookups::RSSIInterpolator m_rssiMapper; - TestNetwork* m_network; + NXDNTestNetwork* m_network; nxdn::Control* m_control; }; @@ -382,7 +386,7 @@ TEST_CASE("NXDN host e2e loopback enforces stream lock until active stream termi const uint32_t streamB = 0x620102U; NXDNHostHarness harness(true, true, hostPort, hostPeerId); - TestNetwork sender(senderPort, 8008U); + NXDNTestNetwork sender(senderPort, 8008U); REQUIRE(harness.network() != nullptr); REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort)); diff --git a/tests/p25/HostControl_Tests.cpp b/tests/p25/HostControl_Tests.cpp index 2fcc5f9f..425e6709 100644 --- a/tests/p25/HostControl_Tests.cpp +++ b/tests/p25/HostControl_Tests.cpp @@ -130,8 +130,6 @@ uint16_t reserveLoopbackPort() return ntohs(address.sin_port); } -} - /** * @brief Builds a P25 RF frame. * @param payload The payload data to include in the frame. @@ -144,6 +142,8 @@ void buildP25RFFrame(const uint8_t* payload, uint8_t* frame) ::memcpy(frame + 2U, payload, p25::defines::P25_LDU_FRAME_LENGTH_BYTES); } +} + // --------------------------------------------------------------------------- // Class Declaration // --------------------------------------------------------------------------- @@ -151,12 +151,12 @@ void buildP25RFFrame(const uint8_t* payload, uint8_t* frame) /** * @brief Dummy implementation of a modem port for testing purposes. */ -class TestModemPort final : public modem::port::IModemPort { +class P25TestModemPort final : public modem::port::IModemPort { public: /** - * @brief Finalizes the instance of the TestModemPort class. + * @brief Finalizes the instance of the P25TestModemPort class. */ - ~TestModemPort() override = default; + ~P25TestModemPort() override = default; /** * @brief Opens the modem port. @@ -202,18 +202,22 @@ public: /** * @brief Lightweight network test double that records P25 reset calls. */ -class TestNetwork final : public network::Network { +class P25TestNetwork final : public network::Network { public: /** - * @brief Initializes a new instance of the TestNetwork class. + * @brief Initializes a new instance of the P25TestNetwork class. * @param localPort The local port number. * @param peerId The peer ID. */ - TestNetwork(uint16_t localPort = 0U, uint32_t peerId = 1U) : - network::Network("127.0.0.1", 1U, localPort, peerId, "test", false, true, false, true, false, false, true, true, false, false, false, false), + P25TestNetwork(uint16_t localPort = 0U, uint32_t peerId = 1U) : + network::Network("127.0.0.1", 1U, localPort, peerId, "test", true, true, false, true, false, false, true, true, false, false, false, false), m_resetP25Count(0U) { - /* stub */ + // keep protocol gates deterministic for this P25-focused harness + m_dmrEnabled = false; + m_p25Enabled = true; + m_nxdnEnabled = false; + m_analogEnabled = false; } /** @@ -370,14 +374,14 @@ public: */ explicit P25HostHarness(bool authoritative = true, bool withNetwork = false, uint16_t networkLocalPort = 0U, uint32_t networkPeerId = 1U) : m_rpc("127.0.0.1", 1U, 0U, "test", false), - m_modem(new TestModemPort(), false, false, false, false, false, false, + m_modem(new P25TestModemPort(), false, false, false, false, false, false, 0U, 0U, 0U, 1024U, 4096U, 1024U, true, true, false, false, false, false), m_chLookup(), m_ridLookup("", 0U, false, false), m_tidLookup("", 0U, false, false), m_idenLookup("", 0U), m_rssiMapper(), - m_network(withNetwork ? new TestNetwork(networkLocalPort, networkPeerId) : nullptr), + m_network(withNetwork ? new P25TestNetwork(networkLocalPort, networkPeerId) : nullptr), m_control(nullptr) { g_RPC = &m_rpc; @@ -397,7 +401,7 @@ public: g_RPC = nullptr; } - TestNetwork* network() const + P25TestNetwork* network() const { return m_network; } @@ -428,7 +432,7 @@ public: lookups::TalkgroupRulesLookup m_tidLookup; lookups::IdenTableLookup m_idenLookup; lookups::RSSIInterpolator m_rssiMapper; - TestNetwork* m_network; + P25TestNetwork* m_network; p25::Control* m_control; }; @@ -468,7 +472,7 @@ TEST_CASE("P25 watchdog expiry resets network stream state", "[p25][host][contro harness.m_control->clock(); REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_IDLE); - REQUIRE(harness.network()->resetP25Count() == 0U); + REQUIRE(harness.network()->resetP25Count() == 1U); } TEST_CASE("P25 host net hang expiry clears active network voice state", "[p25][host][control]") @@ -498,7 +502,7 @@ TEST_CASE("P25 net hang expiry resets network stream state", "[p25][host][contro harness.m_control->clock(); REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_IDLE); - REQUIRE(harness.network()->resetP25Count() == 0U); + REQUIRE(harness.network()->resetP25Count() == 1U); } TEST_CASE("P25 recovers inconsistent net state via network reset", "[p25][host][control][net][stream]") @@ -520,7 +524,7 @@ TEST_CASE("P25 recovers inconsistent net state via network reset", "[p25][host][ (void)HostTestHooks::p25TerminateNetCall(*harness.m_control, control, p25::defines::DUID::TDU); REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_IDLE); - REQUIRE(harness.network()->resetP25Count() == 0U); + REQUIRE(harness.network()->resetP25Count() == 1U); } TEST_CASE("P25 host e2e loopback handles missed frames without dropping active call", "[p25][host][control][net][e2e]") @@ -589,7 +593,67 @@ TEST_CASE("P25 host e2e loopback times out stale call and resets stream state", harness.m_control->clock(); REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_IDLE); - REQUIRE(harness.network()->resetP25Count() == 0U); + REQUIRE(harness.network()->resetP25Count() == 1U); +} + +TEST_CASE("P25 host e2e loopback times out a stream before call state starts", "[p25][host][control][net][e2e]") +{ + const uint16_t hostPort = reserveLoopbackPort(); + const uint16_t senderPort = reserveLoopbackPort(); + REQUIRE(hostPort != 0U); + REQUIRE(senderPort != 0U); + REQUIRE(hostPort != senderPort); + + const uint32_t hostPeerId = 6007U; + const uint32_t staleStream = 0x500101U; + const uint32_t nextStream = 0x500102U; + + P25HostHarness harness(true, true, hostPort, hostPeerId); + P25TestNetwork sender(senderPort, 6008U); + + REQUIRE(harness.network() != nullptr); + REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort)); + REQUIRE(sender.activateLoopback("127.0.0.1", hostPort)); + + REQUIRE(sender.sendP25LDU1Frame(hostPeerId, staleStream, 400U, 1301U, 2301U)); + + for (uint32_t i = 0U; i < 40U; i++) { + sender.clock(1U); + harness.network()->clock(1U); + harness.m_control->clock(); + if (harness.network()->rxP25StreamId() == staleStream) { + break; + } + + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + + REQUIRE(harness.network()->rxP25StreamId() == staleStream); + REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_IDLE); + REQUIRE(HostTestHooks::p25NetworkWatchdog(*harness.m_control).isRunning()); + + HostTestHooks::p25NetworkWatchdog(*harness.m_control).clock(expireTimerTicks(HostTestHooks::p25NetworkWatchdog(*harness.m_control))); + harness.m_control->clock(); + + REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_IDLE); + REQUIRE(harness.network()->rxP25StreamId() == 0U); + + REQUIRE(sender.sendP25LDU1Frame(hostPeerId, nextStream, 500U, 1301U, 2301U)); + REQUIRE(sender.sendP25LDU2Frame(hostPeerId, nextStream, 501U, 1301U, 2301U)); + + for (uint32_t i = 0U; i < 40U; i++) { + sender.clock(1U); + harness.network()->clock(1U); + harness.m_control->clock(); + if (HostTestHooks::p25NetState(*harness.m_control) == RS_NET_AUDIO) { + break; + } + + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + + REQUIRE(HostTestHooks::p25NetState(*harness.m_control) == RS_NET_AUDIO); + REQUIRE(harness.network()->rxP25StreamId() == nextStream); } TEST_CASE("P25 host e2e loopback enforces stream lock until active stream terminates", "[p25][host][control][net][e2e]") @@ -605,7 +669,7 @@ TEST_CASE("P25 host e2e loopback enforces stream lock until active stream termin const uint32_t streamB = 0x500102U; P25HostHarness harness(true, true, hostPort, hostPeerId); - TestNetwork sender(senderPort, 6008U); + P25TestNetwork sender(senderPort, 6008U); REQUIRE(harness.network() != nullptr); REQUIRE(harness.network()->activateLoopback("127.0.0.1", senderPort));