Addressed reviews and added the unit tests

src/messaging/ExchangeContext.cpp

@@ -45,7 +45,6 @@
 #include <platform/LockTracker.h>
 #include <protocols/Protocols.h>
 #include <protocols/secure_channel/Constants.h>
-#include <protocols/secure_channel/StatusReport.h>
 
 #if CONFIG_DEVICE_LAYER
 #include <platform/CHIPDeviceLayer.h>
@@ -267,30 +266,6 @@ CHIP_ERROR ExchangeContext::SendMessage(Protocols::Id protocolId, uint8_t msgTyp
     }
 }
 
-CHIP_ERROR ExchangeContext::SendStatusReport(const Protocols::SecureChannel::StatusReport & statusReport)
-{
-    ChipLogProgress(ExchangeManager, "Sending status report. Protocol code %u, exchange " ChipLogFormatExchange,
-                    statusReport.GetProtocolCode(), ChipLogValueExchange(this));
-
-    auto handle = System::PacketBufferHandle::New(statusReport.Size());
-    VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY,
-                        ChipLogError(SecureChannel, "Failed to allocate status report message"));
-    Encoding::LittleEndian::PacketBufferWriter bbuf(std::move(handle));
-
-    statusReport.WriteToBuffer(bbuf);
-
-    handle = bbuf.Finalize();
-    VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_BUFFER_TOO_SMALL,
-                        ChipLogError(SecureChannel, "Failed to finalize status report message"));
-
-    CHIP_ERROR err = SendMessage(Protocols::SecureChannel::MsgType::StatusReport, std::move(handle));
-    if (err != CHIP_NO_ERROR)
-    {
-        ChipLogError(SecureChannel, "Failed to send status report message: %" CHIP_ERROR_FORMAT, err.Format());
-    }
-    return err;
-}
-
 void ExchangeContext::DoClose(bool clearRetransTable)
 {
     if (mFlags.Has(Flags::kFlagClosed))

src/messaging/ExchangeContext.h

@@ -32,7 +32,6 @@
 #include <messaging/Flags.h>
 #include <messaging/ReliableMessageContext.h>
 #include <protocols/Protocols.h>
-#include <protocols/secure_channel/StatusReport.h>
 #include <transport/SessionManager.h>
 
 namespace chip {
@@ -131,15 +130,6 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
      */
     void WillSendMessage() { mFlags.Set(Flags::kFlagWillSendMessage); }
 
-    /**
-     *  Helper API to send a protocol status report
-     *
-     *  @param[in]  statusReport A secure channel status report
-     *
-     *  @retval CHIP_NO_ERROR on success, appropriate error code otherwise
-     */
-    CHIP_ERROR SendStatusReport(const Protocols::SecureChannel::StatusReport & statusReport);
-
     /**
      *  Handle a received CHIP message on this exchange.
      *

src/protocols/secure_channel/CASEServer.cpp

@@ -85,8 +85,16 @@ CHIP_ERROR CASEServer::OnMessageReceived(Messaging::ExchangeContext * ec, const
         if (!watchdogFired)
         {
             // Handshake wasn't stuck, send the busy status report and let the existing handshake continue.
-            SendBusyStatusReport(ec);
-            return CHIP_NO_ERROR;
+
+            // A successful CASE handshake can take several seconds and some may time out (30 seconds or more).
+            // TODO: Come up with better estimate: https://github.com/project-chip/connectedhomeip/issues/28288
+            // For now, setting minimum wait time to 5000 milliseconds.
+            CHIP_ERROR err = SendBusyStatusReport(ec, 5 * 1000);
+            if (err != CHIP_NO_ERROR)
+            {
+                ChipLogError(Inet, "Failed to send the busy status report, err:%" CHIP_ERROR_FORMAT, err.Format());
+            }
+            return err;
         }
     }
 
@@ -181,28 +189,16 @@ void CASEServer::OnSessionEstablished(const SessionHandle & session)
     PrepareForSessionEstablishment(session->GetPeer());
 }
 
-void CASEServer::SendBusyStatusReport(Messaging::ExchangeContext * ec)
+CHIP_ERROR CASEServer::SendBusyStatusReport(Messaging::ExchangeContext * ec, uint16_t minimumWaitTime)
 {
-    using namespace Protocols::SecureChannel;
-
-    ChipLogProgress(SecureChannel, "Already in the middle of CASE handshake, sending busy status report");
-
-    // A successful CASE handshake can take several seconds and some may time out (30 seconds or more).
-    // TODO: Come up with better estimate: https://github.com/project-chip/connectedhomeip/issues/28288
-    // For now, setting minimum wait time to 5 seconds.
-    uint16_t minimumWaitTime                            = 5 * 1000;                // milliseconds
-    constexpr uint8_t kBusyStatusReportProtocolDataSize = sizeof(minimumWaitTime); // 16-bits
-
-    auto handle = System::PacketBufferHandle::New(kBusyStatusReportProtocolDataSize, 0);
-    VerifyOrReturn(!handle.IsNull(), ChipLogError(SecureChannel, "Failed to allocate protocol data for busy status report"));
+    ChipLogProgress(Inet, "Already in the middle of CASE handshake, sending busy status report");
+    VerifyOrReturnError(ec != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Inet, "Exchange context cannot be NULL"));
 
-    Encoding::LittleEndian::PacketBufferWriter protocolDataBufferWriter(std::move(handle));
-    protocolDataBufferWriter.Put16(minimumWaitTime);
-    handle = protocolDataBufferWriter.Finalize();
-    VerifyOrReturn(!handle.IsNull(), ChipLogError(SecureChannel, "Failed to finalize protocol data for busy status report"));
+    System::PacketBufferHandle handle = Protocols::SecureChannel::StatusReport::MakeBusyStatusReportMessage(minimumWaitTime);
+    VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY, ChipLogError(SecureChannel, "Failed to build a busy status report"));
 
-    StatusReport statusReport(GeneralStatusCode::kBusy, Id, kProtocolCodeBusy, std::move(handle));
-    ec->SendStatusReport(statusReport);
+    ChipLogProgress(Inet, "Sending status report, exchange " ChipLogFormatExchange, ChipLogValueExchange(ec));
+    return ec->SendMessage(Protocols::SecureChannel::MsgType::StatusReport, std::move(handle));
 }
 
 } // namespace chip

src/protocols/secure_channel/CASEServer.h

@@ -109,7 +109,11 @@ class CASEServer : public SessionEstablishmentDelegate,
     void PrepareForSessionEstablishment(const ScopedNodeId & previouslyEstablishedPeer = ScopedNodeId());
 
     // If we are in the middle of handshake and receive a Sigma1 then respond with Busy status code.
-    void SendBusyStatusReport(Messaging::ExchangeContext * ec);
+    // @param[in] ec              Exchange Context
+    // @param[in] minimumWaitTime Minimum wait time before client resends sigma1
+    //
+    // @return CHIP_NO_ERROR on success, error code otherwise
+    CHIP_ERROR SendBusyStatusReport(Messaging::ExchangeContext * ec, uint16_t minimumWaitTime);
 };
 
 } // namespace chip

src/protocols/secure_channel/PairingSession.h

@@ -132,20 +132,31 @@ class DLL_EXPORT PairingSession : public SessionDelegate
         return CHIP_ERROR_INTERNAL;
     }
 
-    /*
-     * Helper function to send a status report. This function sends a status report without any protocol data.
-     */
     void SendStatusReport(Messaging::ExchangeContext * exchangeCtxt, uint16_t protocolCode)
     {
-        VerifyOrReturn(exchangeCtxt);
-
         Protocols::SecureChannel::GeneralStatusCode generalCode = (protocolCode == Protocols::SecureChannel::kProtocolCodeSuccess)
             ? Protocols::SecureChannel::GeneralStatusCode::kSuccess
             : Protocols::SecureChannel::GeneralStatusCode::kFailure;
 
+        ChipLogDetail(SecureChannel, "Sending status report. Protocol code %d, exchange %d", protocolCode,
+                      exchangeCtxt->GetExchangeId());
+
         Protocols::SecureChannel::StatusReport statusReport(generalCode, Protocols::SecureChannel::Id, protocolCode);
 
-        exchangeCtxt->SendStatusReport(statusReport);
+        auto handle = System::PacketBufferHandle::New(statusReport.Size());
+        VerifyOrReturn(!handle.IsNull(), ChipLogError(SecureChannel, "Failed to allocate status report message"));
+        Encoding::LittleEndian::PacketBufferWriter bbuf(std::move(handle));
+
+        statusReport.WriteToBuffer(bbuf);
+
+        System::PacketBufferHandle msg = bbuf.Finalize();
+        VerifyOrReturn(!msg.IsNull(), ChipLogError(SecureChannel, "Failed to allocate status report message"));
+
+        CHIP_ERROR err = exchangeCtxt->SendMessage(Protocols::SecureChannel::MsgType::StatusReport, std::move(msg));
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(SecureChannel, "Failed to send status report message: %" CHIP_ERROR_FORMAT, err.Format());
+        }
     }
 
     CHIP_ERROR HandleStatusReport(System::PacketBufferHandle && msg, bool successExpected)
@@ -172,7 +183,7 @@ class DLL_EXPORT PairingSession : public SessionDelegate
                     Encoding::LittleEndian::Reader reader(report.GetProtocolData()->Start(),
                                                           report.GetProtocolData()->DataLength());
 
-                    // TODO: https://github.com/project-chip/connectedhomeip/issues/28290
+                    // TODO: CASE: Notify minimum wait time to clients on receiving busy status report #28290
                     uint16_t minimumWaitTime = 0;
                     err                      = reader.Read16(&minimumWaitTime).StatusCode();
                     if (err != CHIP_NO_ERROR)

src/protocols/secure_channel/StatusReport.cpp

@@ -95,6 +95,32 @@ size_t StatusReport::Size() const
     return WriteToBuffer(emptyBuf).Needed();
 }
 
+System::PacketBufferHandle StatusReport::MakeBusyStatusReportMessage(uint16_t minimumWaitTime)
+{
+    using namespace Protocols::SecureChannel;
+    constexpr uint8_t kBusyStatusReportProtocolDataSize = sizeof(minimumWaitTime); // 16-bits
+
+    auto handle = System::PacketBufferHandle::New(kBusyStatusReportProtocolDataSize, 0);
+    VerifyOrReturnValue(!handle.IsNull(), handle, ChipLogError(SecureChannel, "Failed to allocate protocol data for busy status report"));
+
+    // Build the protocol data with minimum wait time
+    Encoding::LittleEndian::PacketBufferWriter protocolDataBufferWriter(std::move(handle));
+    protocolDataBufferWriter.Put16(minimumWaitTime);
+    handle = protocolDataBufferWriter.Finalize();
+    VerifyOrReturnValue(!handle.IsNull(), handle, ChipLogError(SecureChannel, "Failed to finalize protocol data for busy status report"));
+
+    // Build a busy status report
+    StatusReport statusReport(GeneralStatusCode::kBusy, Id, kProtocolCodeBusy, std::move(handle));
+
+    // Build the status report message
+    handle = System::PacketBufferHandle::New(statusReport.Size());
+    VerifyOrReturnValue(!handle.IsNull(), handle, ChipLogError(SecureChannel, "Failed to allocate status report message"));
+    Encoding::LittleEndian::PacketBufferWriter bbuf(std::move(handle));
+
+    statusReport.WriteToBuffer(bbuf);
+    return bbuf.Finalize();
+}
+
 } // namespace SecureChannel
 } // namespace Protocols
 } // namespace chip

src/protocols/secure_channel/StatusReport.h

@@ -94,6 +94,15 @@ class DLL_EXPORT StatusReport
     uint16_t GetProtocolCode() const { return mProtocolCode; }
     const System::PacketBufferHandle & GetProtocolData() const { return mProtocolData; }
 
+    /**
+     * Builds a busy status report with protocol data containing the minimum wait time.
+     *
+     * @param[in] minimumWaitTime Time in milliseconds before initiator retries the request
+     *
+     * @return Packet buffer handle which can be passed to SendMessage.
+     */
+    static System::PacketBufferHandle MakeBusyStatusReportMessage(uint16_t minimumWaitTime);
+
 private:
     GeneralStatusCode mGeneralCode;
     Protocols::Id mProtocolId;

src/protocols/secure_channel/tests/TestStatusReport.cpp

@@ -16,6 +16,7 @@
  *    limitations under the License.
  */
 
+#include <lib/support/BufferReader.h>
 #include <lib/support/BufferWriter.h>
 #include <lib/support/CHIPMem.h>
 #include <lib/support/UnitTestRegistration.h>
@@ -104,6 +105,33 @@ void TestBadStatusReport(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR);
 }
 
+void TestMakeBusyStatusReport(nlTestSuite * inSuite, void * inContext)
+{
+    GeneralStatusCode generalCode = GeneralStatusCode::kBusy;
+    auto protocolId               = SecureChannel::Id;
+    uint16_t protocolCode         = kProtocolCodeBusy;
+    uint16_t minimumWaitTime      = 5 * 1000;
+
+    System::PacketBufferHandle handle = StatusReport::MakeBusyStatusReportMessage(minimumWaitTime);
+    NL_TEST_ASSERT(inSuite, !handle.IsNull());
+
+    StatusReport reportToParse;
+    CHIP_ERROR err = reportToParse.Parse(std::move(handle));
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == err);
+    NL_TEST_ASSERT(inSuite, reportToParse.GetGeneralCode() == generalCode);
+    NL_TEST_ASSERT(inSuite, reportToParse.GetProtocolId() == protocolId);
+    NL_TEST_ASSERT(inSuite, reportToParse.GetProtocolCode() == protocolCode);
+
+    const System::PacketBufferHandle & rcvData = reportToParse.GetProtocolData();
+    NL_TEST_ASSERT(inSuite, !rcvData.IsNull());
+    NL_TEST_ASSERT(inSuite, rcvData->DataLength() == sizeof(minimumWaitTime));
+
+    uint16_t readMinimumWaitTime = 0;
+    Encoding::LittleEndian::Reader reader(rcvData->Start(), rcvData->DataLength());
+    NL_TEST_ASSERT(inSuite, CHIP_NO_ERROR == reader.Read16(&readMinimumWaitTime).StatusCode());
+    NL_TEST_ASSERT(inSuite, readMinimumWaitTime == minimumWaitTime);
+}
+
 // Test Suite
 
 /**
@@ -115,6 +143,7 @@ static const nlTest sTests[] =
     NL_TEST_DEF("TestStatusReport_NoData", TestStatusReport_NoData),
     NL_TEST_DEF("TestStatusReport_WithData", TestStatusReport_WithData),
     NL_TEST_DEF("TestBadStatusReport", TestBadStatusReport),
+    NL_TEST_DEF("TestMakeBusyStatusReport", TestMakeBusyStatusReport),
 
     NL_TEST_SENTINEL()
 };