address review comments

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

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

src/messaging/ExchangeContext.cpp

@@ -537,9 +537,12 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
     // layer has completed its work on the ExchangeContext.
     ExchangeHandle ref(*this);
 
-    // If we received a message from the Peer and we are still expecting a response, we know the peer is active
-    // If we receive a message from the Peer but we are not expecting a response, we don't know if the peer is still active
-    SetShouldPeerBeActive(IsResponseExpected());
+    // If we receive a message while we are expecting a response for our previous message,
+    // we assume that the peer is likely active. If the message was 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 if the peer should still be active or not.
+    SetPeerActiveStateHint(IsResponseExpected());
 
     bool isStandaloneAck = payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StandaloneAck);
     bool isDuplicate     = msgFlags.Has(MessageFlagValues::kDuplicateMessage);
@@ -633,7 +636,7 @@ CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const Payload
         SetResponseExpected(false);
 
         // If we received the expected response, we don't if the peer is still active after having sent the response
-        SetShouldPeerBeActive(false);
+        SetPeerActiveStateHint(false);
     }
 
     // Don't send messages on to our delegate if our dispatch does not allow
@@ -695,7 +698,7 @@ void ExchangeContext::ExchangeSessionHolder::GrabExpiredSession(const SessionHan
     GrabUnchecked(session);
 }
 
-bool ExchangeContext::ShouldPeerBeActive()
+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.

src/messaging/ExchangeContext.h

@@ -217,29 +217,29 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
     /**
      * Notifies the exchange that our consummer is going to send a message where it knows the peer (receiver) should be active
      *
-     * This API is used to set the ShouldPeerBeActive flag.
-     * The ShouldPeerBeActive flag allows the consummer of the exchange to notify the exchange that the peer (receiver)
+     * 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 ShouldPeerBeActive flag is only used when calculating the next retransmission time in the ReliableMessageMgr.
-     * If the ShouldPeerBeActive flag is true, we will used the Active Retransmission timeout when calculating the next
-     * retransmission time. If the ShouldPeerBeActive flag is false, we will rely on other information to determine which
-     * retransmission timeout should be used. See the ShouldPeerBeActive API for the alternative means to determine if the Peer
+     * 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 ShouldPeerBeActive flag to true.
-     * The API should only called after receiving a message from the peer and after calling the WillSendMessage API.
+     * 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 shouldPeerBeActive true if the peer should be active, false if we don't know or it should not be
+     * @param IsPeerLikelyActiveHint true if the peer should be active, false if we don't know or it should not be
      */
-    void SetShouldPeerBeActive(bool shouldPeerBeActive) { mFlags.Set(Flags::kPeerShouldBeActive, shouldPeerBeActive); }
+    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
@@ -253,7 +253,7 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
      * @return false We weren't able to determine if the peer is active.
      *               We don't know in which state the peer is.
      */
-    bool ShouldPeerBeActive();
+    bool IsPeerLikelyActiveHint();
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
     SessionHolder & GetSessionHolder() { return mSession; }

src/messaging/ReliableMessageContext.h

@@ -180,13 +180,13 @@ class ReliableMessageContext
      *
      * If we are not initiator                          -> peer is active
      * If ephemeral context                             -> peer is active
-     * else we default to information provided by the consummer
+     * else we default to information provided by the client
      *
      * @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 ShouldPeerBeActive();
+    bool IsPeerLikelyActiveHint();
 
 private:
     void HandleRcvdAck(uint32_t ackMessageCounter);

src/messaging/ReliableMessageMgr.cpp

@@ -465,11 +465,11 @@ CHIP_ERROR ReliableMessageMgr::MapSendError(CHIP_ERROR error, uint16_t exchangeI
     return error;
 }
 
-void ReliableMessageMgr::CalculateNextRetransTime(RetransTableEntry * entry)
+void ReliableMessageMgr::CalculateNextRetransTime(RetransTableEntry *& entry)
 {
     System::Clock::Timestamp baseTimeout = System::Clock::Milliseconds64(0);
 
-    if (entry->ec->ShouldPeerBeActive())
+    if (entry->ec->IsPeerLikelyActiveHint())
     {
         // If we know the peer is active with the exchangeContext, use the Active Retrans timeout
         baseTimeout = entry->ec->GetSessionHandle()->GetRemoteMRPConfig().mActiveRetransTimeout;

src/messaging/ReliableMessageMgr.h

@@ -210,9 +210,9 @@ class ReliableMessageMgr
      * Calculates the next retransmission time for the entry
      * Function sets the nextRetransTime of the entry
      *
-     * @param[in/out] entry RetransTableEntry for which we need to calculate the nextRetransTime
+     * @param[in,out] entry RetransTableEntry for which we need to calculate the nextRetransTime
      */
-    void CalculateNextRetransTime(RetransTableEntry * entry);
+    void CalculateNextRetransTime(RetransTableEntry *& entry);
 
     ObjectPool<ExchangeContext, CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS> & mContextPool;
     chip::System::Layer * mSystemLayer;

src/messaging/tests/TestReliableMessageProtocol.cpp

@@ -1572,7 +1572,7 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     /**
      * This tests the following scenario:
      * 1) A reliable message expecting a response is sent from the initiator to responder which is losts
-     * 2) Initiator resends the message a the IdleRetrans interval
+     * 2) Initiator resends the message at the IdleRetrans interval
      * 3) Responder receives the message and sends a standalone ack
      * 4) Responder sends a response and fails
      * 5) Responder retries at the ActiveRestrans interval
@@ -1617,7 +1617,7 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     mockReceiver.mRetainExchange = true;
     mockSender.mRetainExchange   = true;
 
-    NL_TEST_ASSERT(inSuite, !exchange->ShouldPeerBeActive());
+    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
 
     err = exchange->SendMessage(Echo::MsgType::EchoRequest, std::move(buffer), SendFlags(SendMessageFlags::kExpectResponse));
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
@@ -1632,7 +1632,7 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return loopback.mSentMessageCount >= 1; });
     ctx.DrainAndServiceIO();
 
-    NL_TEST_ASSERT(inSuite, !exchange->ShouldPeerBeActive());
+    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
 
     // // Make sure nothing happened
     NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount == 1);
@@ -1642,15 +1642,15 @@ void TestReliableMessageProtocol::CheckIsPeerActiveNotInitiator(nlTestSuite * in
     ctx.GetIOContext().DriveIOUntil(2000_ms32, [&] { return loopback.mSentMessageCount >= 2; });
     ctx.DrainAndServiceIO();
 
-    NL_TEST_ASSERT(inSuite, !exchange->ShouldPeerBeActive());
+    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
 
     // // 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->ShouldPeerBeActive());
-    NL_TEST_ASSERT(inSuite, mockReceiver.mExchange->ShouldPeerBeActive());
+    NL_TEST_ASSERT(inSuite, !exchange->IsPeerLikelyActiveHint());
+    NL_TEST_ASSERT(inSuite, mockReceiver.mExchange->IsPeerLikelyActiveHint());
 
     mockReceiver.mExchange->GetSessionHandle()->AsSecureSession()->SetRemoteMRPConfig({
         1000_ms32, // CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL

src/protocols/bdx/TransferFacilitator.cpp

@@ -54,7 +54,7 @@ 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->SetShouldPeerBeActive(true);
+    mExchangeCtx->SetPeerActiveStateHint(true);
 
     return err;
 }

src/protocols/secure_channel/CASESession.cpp

@@ -1410,8 +1410,8 @@ CHIP_ERROR CASESession::SendSigma3a()
             SuccessOrExit(err = helper->ScheduleWork());
             mSendSigma3Helper = helper;
             mExchangeCtxt->WillSendMessage();
-            mExchangeCtxt->SetShouldPeerBeActive(
-                true); // When we send sigma3, peer has processed sigma 1 and sent sigma2 which means it is active
+            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,7 +1705,7 @@ CHIP_ERROR CASESession::HandleSigma3a(System::PacketBufferHandle && msg)
         SuccessOrExit(err = helper->ScheduleWork());
         mHandleSigma3Helper = helper;
         mExchangeCtxt->WillSendMessage();
-        mExchangeCtxt->SetShouldPeerBeActive(
+        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

@@ -826,7 +826,7 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl
     SuccessOrExit(err);
 
     // Once we receive a message for the PASE protocol, we know the Peer is active
-    mExchangeCtxt->SetShouldPeerBeActive(true);
+    mExchangeCtxt->SetPeerActiveStateHint(true);
 
 #if CHIP_CONFIG_SLOW_CRYPTO
     if (msgType == MsgType::PBKDFParamRequest || msgType == MsgType::PBKDFParamResponse || msgType == MsgType::PASE_Pake1 ||