From 7fc9b450d093fbcc591885ab1d708646c61f3b5c Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Wed, 2 Aug 2023 22:36:01 +0530 Subject: [PATCH] Few more review comments --- src/protocols/secure_channel/CASEServer.cpp | 8 +++----- src/protocols/secure_channel/CASEServer.h | 3 ++- src/protocols/secure_channel/StatusReport.cpp | 6 +++--- src/protocols/secure_channel/StatusReport.h | 3 ++- .../secure_channel/tests/TestStatusReport.cpp | 10 +++++----- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 8674fa8d375e4c..f8d580fdd9957b 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -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()); @@ -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)); diff --git a/src/protocols/secure_channel/CASEServer.h b/src/protocols/secure_channel/CASEServer.h index 90b35e4f4c7d71..bb47460e26e972 100644 --- a/src/protocols/secure_channel/CASEServer.h +++ b/src/protocols/secure_channel/CASEServer.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace chip { @@ -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 diff --git a/src/protocols/secure_channel/StatusReport.cpp b/src/protocols/secure_channel/StatusReport.cpp index 0871c702f53283..129759d19c16f9 100644 --- a/src/protocols/secure_channel/StatusReport.cpp +++ b/src/protocols/secure_channel/StatusReport.cpp @@ -95,10 +95,10 @@ 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, @@ -106,7 +106,7 @@ System::PacketBufferHandle StatusReport::MakeBusyStatusReportMessage(uint16_t mi // 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")); diff --git a/src/protocols/secure_channel/StatusReport.h b/src/protocols/secure_channel/StatusReport.h index d10ffa6a0b2291..e25f2c6dc1a20b 100644 --- a/src/protocols/secure_channel/StatusReport.h +++ b/src/protocols/secure_channel/StatusReport.h @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace chip { @@ -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; diff --git a/src/protocols/secure_channel/tests/TestStatusReport.cpp b/src/protocols/secure_channel/tests/TestStatusReport.cpp index f75355f32d759b..093453029b156b 100644 --- a/src/protocols/secure_channel/tests/TestStatusReport.cpp +++ b/src/protocols/secure_channel/tests/TestStatusReport.cpp @@ -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()); @@ -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