Skip to content

Commit

Permalink
Few more review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
shubhamdp committed Aug 2, 2023
1 parent 95752ac commit 7fc9b45
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 15 deletions.
8 changes: 3 additions & 5 deletions src/protocols/secure_channel/CASEServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ CHIP_ERROR CASEServer::OnMessageReceived(Messaging::ExchangeContext * ec, const
// 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);
CHIP_ERROR err = SendBusyStatusReport(ec, System::Clock::Milliseconds16(5000));
if (err != CHIP_NO_ERROR)
{
ChipLogError(Inet, "Failed to send the busy status report, err:%" CHIP_ERROR_FORMAT, err.Format());
Expand Down Expand Up @@ -189,14 +189,12 @@ void CASEServer::OnSessionEstablished(const SessionHandle & session)
PrepareForSessionEstablishment(session->GetPeer());
}

CHIP_ERROR CASEServer::SendBusyStatusReport(Messaging::ExchangeContext * ec, uint16_t minimumWaitTime)
CHIP_ERROR CASEServer::SendBusyStatusReport(Messaging::ExchangeContext * ec, System::Clock::Milliseconds16 minimumWaitTime)
{
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"));

System::PacketBufferHandle handle = Protocols::SecureChannel::StatusReport::MakeBusyStatusReportMessage(minimumWaitTime);
VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY,
ChipLogError(SecureChannel, "Failed to build a busy status report"));
VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY);

ChipLogProgress(Inet, "Sending status report, exchange " ChipLogFormatExchange, ChipLogValueExchange(ec));
return ec->SendMessage(Protocols::SecureChannel::MsgType::StatusReport, std::move(handle));
Expand Down
3 changes: 2 additions & 1 deletion src/protocols/secure_channel/CASEServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <messaging/ExchangeDelegate.h>
#include <messaging/ExchangeMgr.h>
#include <protocols/secure_channel/CASESession.h>
#include <system/SystemClock.h>

namespace chip {

Expand Down Expand Up @@ -113,7 +114,7 @@ class CASEServer : public SessionEstablishmentDelegate,
// @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);
CHIP_ERROR SendBusyStatusReport(Messaging::ExchangeContext * ec, System::Clock::Milliseconds16 minimumWaitTime);
};

} // namespace chip
6 changes: 3 additions & 3 deletions src/protocols/secure_channel/StatusReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,18 @@ size_t StatusReport::Size() const
return WriteToBuffer(emptyBuf).Needed();
}

System::PacketBufferHandle StatusReport::MakeBusyStatusReportMessage(uint16_t minimumWaitTime)
System::PacketBufferHandle StatusReport::MakeBusyStatusReportMessage(System::Clock::Milliseconds16 minimumWaitTime)
{
using namespace Protocols::SecureChannel;
constexpr uint8_t kBusyStatusReportProtocolDataSize = sizeof(minimumWaitTime); // 16-bits
constexpr uint8_t kBusyStatusReportProtocolDataSize = sizeof(minimumWaitTime.count()); // 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);
protocolDataBufferWriter.Put16(minimumWaitTime.count());
handle = protocolDataBufferWriter.Finalize();
VerifyOrReturnValue(!handle.IsNull(), handle,
ChipLogError(SecureChannel, "Failed to finalize protocol data for busy status report"));
Expand Down
3 changes: 2 additions & 1 deletion src/protocols/secure_channel/StatusReport.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <lib/support/BufferWriter.h>
#include <protocols/Protocols.h>
#include <protocols/secure_channel/Constants.h>
#include <system/SystemClock.h>
#include <system/SystemPacketBuffer.h>

namespace chip {
Expand Down Expand Up @@ -101,7 +102,7 @@ class DLL_EXPORT StatusReport
*
* @return Packet buffer handle which can be passed to SendMessage.
*/
static System::PacketBufferHandle MakeBusyStatusReportMessage(uint16_t minimumWaitTime);
static System::PacketBufferHandle MakeBusyStatusReportMessage(System::Clock::Milliseconds16 minimumWaitTime);

private:
GeneralStatusCode mGeneralCode;
Expand Down
10 changes: 5 additions & 5 deletions src/protocols/secure_channel/tests/TestStatusReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ void TestBadStatusReport(nlTestSuite * inSuite, void * inContext)

void TestMakeBusyStatusReport(nlTestSuite * inSuite, void * inContext)
{
GeneralStatusCode generalCode = GeneralStatusCode::kBusy;
auto protocolId = SecureChannel::Id;
uint16_t protocolCode = kProtocolCodeBusy;
uint16_t minimumWaitTime = 5 * 1000;
GeneralStatusCode generalCode = GeneralStatusCode::kBusy;
auto protocolId = SecureChannel::Id;
uint16_t protocolCode = kProtocolCodeBusy;
System::Clock::Milliseconds16 minimumWaitTime = System::Clock::Milliseconds16(5000);

System::PacketBufferHandle handle = StatusReport::MakeBusyStatusReportMessage(minimumWaitTime);
NL_TEST_ASSERT(inSuite, !handle.IsNull());
Expand All @@ -129,7 +129,7 @@ void TestMakeBusyStatusReport(nlTestSuite * inSuite, void * inContext)
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);
NL_TEST_ASSERT(inSuite, System::Clock::Milliseconds16(readMinimumWaitTime) == minimumWaitTime);
}

// Test Suite
Expand Down

0 comments on commit 7fc9b45

Please sign in to comment.