Account for the retry delay booster in unit test (#27256)

* Fix compilation warnings when RMP_TICKLESS_DEBUG is defined * Account for the retry delay booster in unit test
project-chip · Jan 19, 2024 · 1d29a69 · 1d29a69
1 parent 24f1225
commit 1d29a69
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 34 deletions.
diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp
@@ -77,10 +77,10 @@ void ReliableMessageMgr::Shutdown()
     mSystemLayer = nullptr;
 }
 
-#if defined(RMP_TICKLESS_DEBUG)
 void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log)
 {
-    ChipLogDetail(ExchangeManager, log);
+#if defined(RMP_TICKLESS_DEBUG)
+    ChipLogDetail(ExchangeManager, "%s", log);
 
     mRetransTable.ForEachActiveObject([&](auto * entry) {
         ChipLogDetail(ExchangeManager,
@@ -89,17 +89,15 @@ void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log)
                       entry->nextRetransTime.count());
         return Loop::Continue;
     });
+#endif
 }
-#else
-void ReliableMessageMgr::TicklessDebugDumpRetransTable(const char * log) {}
-#endif // RMP_TICKLESS_DEBUG
 
 void ReliableMessageMgr::ExecuteActions()
 {
     System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp();
 
 #if defined(RMP_TICKLESS_DEBUG)
-    ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExecuteActions at % " PRIu64 "ms", now.count());
+    ChipLogDetail(ExchangeManager, "ReliableMessageMgr::ExecuteActions at %" PRIu64 "ms", now.count());
 #endif
 
     ExecuteForAllContext([&](ReliableMessageContext * rc) {
@@ -180,7 +178,7 @@ void ReliableMessageMgr::Timeout(System::Layer * aSystemLayer, void * aAppState)
     VerifyOrDie((aSystemLayer != nullptr) && (manager != nullptr));
 
 #if defined(RMP_TICKLESS_DEBUG)
-    ChipLogDetail(ExchangeManager, "ReliableMessageMgr::Timeout\n");
+    ChipLogDetail(ExchangeManager, "ReliableMessageMgr::Timeout");
 #endif
 
     // Execute any actions that are due this tick
@@ -392,24 +390,24 @@ void ReliableMessageMgr::StartTimer()
         return Loop::Continue;
     });
 
+    StopTimer();
+
     if (nextWakeTime != System::Clock::Timestamp::max())
     {
-#if defined(RMP_TICKLESS_DEBUG)
-        ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer wake at %" PRIu64 "ms", nextWakeTime);
-#endif
-
-        StopTimer();
-
         const System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp();
         const auto nextWakeDelay           = (nextWakeTime > now) ? nextWakeTime - now : 0_ms;
+
+#if defined(RMP_TICKLESS_DEBUG)
+        ChipLogDetail(ExchangeManager, "ReliableMessageMgr::StartTimer at %" PRIu64 "ms wake at %" PRIu64 "ms (in %" PRIu64 "ms)",
+                      now.count(), nextWakeTime.count(), nextWakeDelay.count());
+#endif
         VerifyOrDie(mSystemLayer->StartTimer(nextWakeDelay, Timeout, this) == CHIP_NO_ERROR);
     }
     else
     {
 #if defined(RMP_TICKLESS_DEBUG)
         ChipLogDetail(ExchangeManager, "ReliableMessageMgr skipped timer");
 #endif
-        StopTimer();
     }
 
     TicklessDebugDumpRetransTable("ReliableMessageMgr::StartTimer Dumping mRetransTable entries after setting wakeup times");

diff --git a/src/messaging/tests/TestAbortExchangesForFabric.cpp b/src/messaging/tests/TestAbortExchangesForFabric.cpp
@@ -26,6 +26,7 @@
 #include <lib/support/UnitTestUtils.h>
 #include <messaging/ExchangeContext.h>
 #include <messaging/ExchangeMgr.h>
+#include <messaging/ReliableMessageProtocolConfig.h>
 #include <messaging/tests/MessagingContext.h>
 #include <protocols/echo/Echo.h>
 #include <system/SystemPacketBuffer.h>
@@ -36,6 +37,7 @@ namespace {
 using namespace chip;
 using namespace chip::Messaging;
 using namespace chip::System;
+using namespace chip::System::Clock::Literals;
 using namespace chip::Protocols;
 
 using TestContext = Test::LoopbackMessagingContext;
@@ -203,7 +205,13 @@ void CommonCheckAbortAllButOneExchange(nlTestSuite * inSuite, TestContext & ctx,
         // We've set the session into responsive mode by altering the MRP intervals, so we should be able to
         // trigger a MRP failure due to timing out waiting for an ACK.
         //
-        ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1000), [&]() { return false; });
+        auto waitTimeout = System::Clock::Milliseconds32(1000);
+#ifdef CHIP_CONFIG_MRP_RETRY_INTERVAL_SENDER_BOOST
+        // Account for the retry delay booster, so that we do not timeout our IO processing before the
+        // retransmission failure is triggered.
+        waitTimeout += CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS * CHIP_CONFIG_MRP_RETRY_INTERVAL_SENDER_BOOST;
+#endif
+        ctx.GetIOContext().DriveIOUntil(waitTimeout, [&]() { return false; });
     }
     else
     {
@@ -235,32 +243,20 @@ void CheckAbortAllButOneExchangeResponseTimeout(nlTestSuite * inSuite, void * in
     CommonCheckAbortAllButOneExchange(inSuite, ctx, true);
 }
 
-// Test Suite
-
-/**
- *  Test Suite that lists all the test functions.
- */
-// clang-format off
-const nlTest sTests[] =
-{
-    NL_TEST_DEF("Test aborting all but one exchange",                       CheckAbortAllButOneExchange),
-    NL_TEST_DEF("Test aborting all but one exchange + response timeout",    CheckAbortAllButOneExchangeResponseTimeout),
-
-    NL_TEST_SENTINEL()
+const nlTest sTests[] = {
+    NL_TEST_DEF("Test aborting all but one exchange", CheckAbortAllButOneExchange),
+    NL_TEST_DEF("Test aborting all but one exchange + response timeout", CheckAbortAllButOneExchangeResponseTimeout),
+    NL_TEST_SENTINEL(),
 };
-// clang-format on
 
-// clang-format off
-nlTestSuite sSuite =
-{
+nlTestSuite sSuite = {
     "Test-AbortExchangesForFabric",
     &sTests[0],
     TestContext::Initialize,
-    TestContext::Finalize
+    TestContext::Finalize,
 };
-// clang-format on
 
-} // anonymous namespace
+} // namespace
 
 /**
  *  Main