Refactor changes to simplify and remove req on exchange context consu…

…mmer

src/app/clusters/ota-requestor/DefaultOTARequestor.h

@@ -165,7 +165,6 @@ class DefaultOTARequestor : public OTARequestorInterface, public BDXDownloader::
             if (!payloadHeader.HasMessageType(chip::Protocols::SecureChannel::MsgType::StatusReport))
             {
                 ec->WillSendMessage();
-                ec->SetPeerActiveStateHint(true);
             }
 
             return CHIP_NO_ERROR;

src/messaging/ExchangeContext.cpp

@@ -514,14 +514,6 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
     // layer has completed its work on the ExchangeContext.
     ExchangeHandle ref(*this);
 
-    // If we receive a message while we are expecting a response for our previous message,
-    // we assume that the peer is likely active, since it has not sent us that response yet and hence
-    // still has an active exchange corresponding to this one.  If this message is in fact the response,
-    // the flag will be reset to false when we process the message later in the function.
-    // If we receive a message while we are not expecting a response,
-    // we reset the flag to false because we don't know whether the peer should still be active or not.
-    SetPeerActiveStateHint(IsResponseExpected());
-
     bool isStandaloneAck = payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StandaloneAck);
     bool isDuplicate     = msgFlags.Has(MessageFlagValues::kDuplicateMessage);
 
@@ -597,6 +589,9 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
     app::ICDNotifier::GetInstance().BroadcastNetworkActivityNotification();
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 
+    // Set kFlagReceivedAtLeastOneMessage to true since we have received at least one new application level message
+    SetHasReceivedAtLeastOneMessage(true);
+
     if (IsResponseExpected())
     {
         // Since we got the response, cancel the response timer.
@@ -605,9 +600,6 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
         // If the context was expecting a response to a previously sent message, this message
         // is implicitly that response.
         SetResponseExpected(false);
-
-        // If we received the expected response, we don't if the peer is still active after having sent the response
-        SetPeerActiveStateHint(false);
     }
 
     // Don't send messages on to our delegate if our dispatch does not allow
@@ -669,21 +661,5 @@ void ExchangeContext::ExchangeSessionHolder::GrabExpiredSession(const SessionHan
     GrabUnchecked(session);
 }
 
-bool ExchangeContext::IsPeerLikelyActiveHint()
-{
-    // If we are not the initiator, it means the peer sent the first message.
-    // This means our peer is active if we are sending a message to them.
-    if (!IsInitiator())
-        return true;
-
-    // If we create an Ephemeral Exchange, it was just to generate a StandaloneAck.
-    // Since we are sending an ack, we know the peer is active.
-    if (IsEphemeralExchange())
-        return true;
-
-    // Return previously stored state if we have no absolute answer.
-    return mFlags.Has(Flags::kPeerShouldBeActive);
-}
-
 } // namespace Messaging
 } // namespace chip

src/messaging/ExchangeContext.h

@@ -215,45 +215,12 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
     bool IsSendExpected() const { return mFlags.Has(Flags::kFlagWillSendMessage); }
 
     /**
-     * Notifies the exchange that our consummer is going to send a message where it knows the peer (receiver) should be active
+     * Tracks if we have received at least one application level message
+     * during the life-time of this exchange
      *
-     * This API is used to set the IsPeerLikelyActiveHint flag.
-     * The IsPeerLikelyActiveHint flag allows the consummer of the exchange to notify the exchange that the peer (receiver)
-     * will be active for the next message the consummer will send.
-     * This allows the consummer to notify the exchange on the state of the peer with the context of the messages being sent.
-     *
-     * The IsPeerLikelyActiveHint flag is only used when calculating the next retransmission time in the ReliableMessageMgr.
-     * If the IsPeerLikelyActiveHint flag is true, we will used the Active Retransmission timeout when calculating the next
-     * retransmission time. If the IsPeerLikelyActiveHint flag is false, we will rely on other information to determine which
-     * retransmission timeout should be used. See the IsPeerLikelyActiveHint API for the alternative means to determine if the Peer
-     * (receiver) is active.
-     *
-     * The first message of the exchange set as the initiator should never have the IsPeerLikelyActiveHint flag to true.
-     * The API should only be called after receiving a message from the peer and after calling the WillSendMessage API.
-     * The next message sent will then use the active retransmission time out.
-     * The flag is reset to false each time we receive the response from our peer (receiver).
-     * As such, the API needs to be called each time we send another message over the exchange.
-     *
-     * The API call is not mandatory for the communication to be successful.
-     * If the call is not done, we will rely on information within the exchange context.
-     *
-     * @param IsPeerLikelyActiveHint true if the peer should be active, false if we don't know or it should not be
+     * @return Returns 'true' if we have received at least one message, else 'false'
      */
-    void SetPeerActiveStateHint(bool IsPeerLikelyActiveHint) { mFlags.Set(Flags::kPeerShouldBeActive, IsPeerLikelyActiveHint); }
-
-    /**
-     * Function checks with information available in the ExchangeContext if the peer should be active or not
-     * API should only be used when we are determining if we need to use the Active or Idle retransmission timeout
-     *
-     * If we are not initiator                          -> peer is active
-     * If ephemeral context                             -> peer is active
-     * else we default to information provided by the consummer
-     *
-     * @return true Based on available information, Peer should be active
-     * @return false We weren't able to determine if the peer is active.
-     *               We don't know in which state the peer is.
-     */
-    bool IsPeerLikelyActiveHint();
+    inline bool HasReceivedAtLeastOneMessage() { return mFlags.Has(Flags::kFlagReceivedAtLeastOneMessage); }
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
     SessionHolder & GetSessionHolder() { return mSession; }
@@ -337,19 +304,15 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
 
     // If SetAutoReleaseSession() is called, this exchange must be using a SecureSession, and should
     // evict it when the exchange is done with all its work (including any MRP traffic).
-    inline void SetIgnoreSessionRelease(bool ignore);
-    inline bool ShouldIgnoreSessionRelease();
-};
+    inline void SetIgnoreSessionRelease(bool ignore) { mFlags.Set(Flags::kFlagIgnoreSessionRelease, ignore); }
 
-inline void ExchangeContext::SetIgnoreSessionRelease(bool ignore)
-{
-    mFlags.Set(Flags::kFlagIgnoreSessionRelease, ignore);
-}
+    inline bool ShouldIgnoreSessionRelease() { return mFlags.Has(Flags::kFlagIgnoreSessionRelease); }
 
-inline bool ExchangeContext::ShouldIgnoreSessionRelease()
-{
-    return mFlags.Has(Flags::kFlagIgnoreSessionRelease);
-}
+    inline void SetHasReceivedAtLeastOneMessage(bool hasReceivedMessage)
+    {
+        mFlags.Set(Flags::kFlagReceivedAtLeastOneMessage, hasReceivedMessage);
+    }
+};
 
 } // namespace Messaging
 } // namespace chip

src/messaging/ReliableMessageContext.h

@@ -172,9 +172,11 @@ class ReliableMessageContext
         /// When set, ignore session being released, because we are releasing it ourselves.
         kFlagIgnoreSessionRelease = (1u << 9),
 
-        // When set, sender knows that the peer (receiver) should currently be active
-        // When bit is not set, we don't know if the peer (receiver) is active or not
-        kPeerShouldBeActive = (1u << 10),
+        // This flag is used to determine if the peer (receiver) should be active or not
+        // When set, sender knows it has received at least one application-level message
+        // from the peer and can assume the peer (receiver) is active
+        // If the flag is not set, we don't know if the peer (receiver) is active or not
+        kFlagReceivedAtLeastOneMessage = (1u << 10),
 
         /// When set:
         ///

src/messaging/ReliableMessageMgr.cpp

@@ -454,14 +454,15 @@ void ReliableMessageMgr::CalculateNextRetransTime(RetransTableEntry & entry)
 {
     System::Clock::Timestamp baseTimeout = System::Clock::Milliseconds64(0);
 
-    if (entry.ec->IsPeerLikelyActiveHint())
+    // Check if we have received at least one application-level message
+    if (entry.ec->HasReceivedAtLeastOneMessage())
     {
-        // If we know the peer is active with the exchangeContext, use the Active Retrans timeout
+        // If we have received at least one message, assume peer is active and use ActiveRetransTimeout
         baseTimeout = entry.ec->GetSessionHandle()->GetRemoteMRPConfig().mActiveRetransTimeout;
     }
     else
     {
-        // If the exchange context doesn't know if the peer is active or not.
+        // If we haven't received at least one message
         // Choose active/idle timeout from PeerActiveMode of session per 4.11.2.1. Retransmissions.
         baseTimeout = entry.ec->GetSessionHandle()->GetMRPBaseTimeout();
     }

src/messaging/tests/TestReliableMessageProtocol.cpp

@@ -1620,7 +1620,7 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     mockReceiver.mRetainExchange = true;
     mockSender.mRetainExchange   = true;
 
-    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
+    NL_TEST_ASSERT(inSuite, !exchange->HasReceivedAtLeastOneMessage());
 
     err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendFlags(SendMessageFlags::kExpectResponse));
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
@@ -1635,7 +1635,7 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= 1; });
     ctx.DrainAndServiceIO();
 
-    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
+    NL_TEST_ASSERT(inSuite, !exchange->HasReceivedAtLeastOneMessage());
 
     // // Make sure nothing happened
     NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1);
@@ -1645,15 +1645,15 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     ctx.GetIOContext().DriveIOUntil(2000_ms32, [&] { return loopback.mSentMessageCount >= 2; });
     ctx.DrainAndServiceIO();
 
-    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
+    NL_TEST_ASSERT(inSuite, !exchange->HasReceivedAtLeastOneMessage());
 
     // // Make sure nothing happened
     NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 2);
     NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled);
 
     // // Verify that the receiver considers the sender is active
-    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
-    NL_TEST_ASSERT(inSuite, mockReceiver.mExchange->IsPeerLikelyActiveHint());
+    NL_TEST_ASSERT(inSuite, !exchange->HasReceivedAtLeastOneMessage());
+    NL_TEST_ASSERT(inSuite, mockReceiver.mExchange->HasReceivedAtLeastOneMessage());
 
     mockReceiver.mExchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({
         1000_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL
@@ -2182,7 +2182,8 @@ const nlTest sTests[] = {
                 TestReliableMessageProtocol::CheckLostStandaloneAck),
     NL_TEST_DEF("Test Is Peer Active Retry logic", TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator),
     NL_TEST_DEF("Test MRP backoff algorithm", TestReliableMessageProtocol::CheckGetBackoff),
-    NL_TEST_DEF("Test an application response that comes after MRP retransmits run out", TestReliableMessageProtocol::CheckApplicationResponseDelayed),
+    NL_TEST_DEF("Test an application response that comes after MRP retransmits run out",
+                TestReliableMessageProtocol::CheckApplicationResponseDelayed),
     NL_TEST_DEF("Test an application response that never comes, so MRP retransmits run out and then exchange times out",
                 TestReliableMessageProtocol::CheckApplicationResponseNeverComes),
     NL_TEST_SENTINEL(),

src/protocols/bdx/TransferFacilitator.cpp

@@ -54,7 +54,6 @@ CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeConte
     // For this reason, it is left up to the application logic to call ExchangeContext::Close() when it has determined that the
     // transfer is finished.
     mExchangeCtx->WillSendMessage();
-    mExchangeCtx->SetPeerActiveStateHint(true);
 
     return err;
 }

src/protocols/secure_channel/CASESession.cpp

@@ -1410,8 +1410,6 @@ CHIP_ERROR CASESession::SendSigma3a()
             SuccessOrExit(err = helper->ScheduleWork());
             mSendSigma3Helper = helper;
             mExchangeCtxt->WillSendMessage();
-            mExchangeCtxt->SetPeerActiveStateHint(
-                true); // When we send sigma3, peer has processed sigma 1 and sent sigma2 which means it is likely active
             mState = State::kSendSigma3Pending;
         }
         else
@@ -1705,8 +1703,6 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg)
         SuccessOrExit(err = helper->ScheduleWork());
         mHandleSigma3Helper = helper;
         mExchangeCtxt->WillSendMessage();
-        mExchangeCtxt->SetPeerActiveStateHint(
-            true); // When we receive sigma3, peer has received and processed sigma2 which means it is active
         mState = State::kHandleSigma3Pending;
     }
 

src/protocols/secure_channel/PASESession.cpp

@@ -825,9 +825,6 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl
     MsgType msgType = static_cast<MsgType>(payloadHeader.GetMessageType());
     SuccessOrExit(err);
 
-    // Once we receive a message for the PASE protocol, we know the Peer is active
-    mExchangeCtxt->SetPeerActiveStateHint(true);
-
 #if CHIP_CONFIG_SLOW_CRYPTO
     if (msgType == MsgType::PBKDFParamRequest || msgType == MsgType::PBKDFParamResponse || msgType == MsgType::PASE_Pake1 ||
         msgType == MsgType::PASE_Pake2 || msgType == MsgType::PASE_Pake3)