From e8244ecd55e21e5d51c285250b191217f29abd13 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Mon, 8 Jan 2024 19:04:57 +0000 Subject: [PATCH 1/3] Allow reserving space for MoreChunkedMessages --- src/app/MessageDef/InvokeResponseMessage.cpp | 31 ++++++++++++++++++-- src/app/MessageDef/InvokeResponseMessage.h | 15 ++++++++++ src/app/tests/TestMessageDef.cpp | 24 +++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/app/MessageDef/InvokeResponseMessage.cpp b/src/app/MessageDef/InvokeResponseMessage.cpp index cc9b89d3b15f35..f9887530c15e15 100644 --- a/src/app/MessageDef/InvokeResponseMessage.cpp +++ b/src/app/MessageDef/InvokeResponseMessage.cpp @@ -147,14 +147,32 @@ InvokeResponseIBs::Builder & InvokeResponseMessage::Builder::CreateInvokeRespons InvokeResponseMessage::Builder & InvokeResponseMessage::Builder::MoreChunkedMessages(const bool aMoreChunkedMessages) { + // If any changes are made to how we encoded MoreChunkedMessage that involves how many + // bytes are needed, a corresponding change to GetSizeForMoreChunkResponses indicating + // the new size that will be required. + // skip if error has already been set - if (mError == CHIP_NO_ERROR) + SuccessOrExit(mError); + + if (mIsMoreChunkMessageBufferReserved) { - mError = mpWriter->PutBoolean(TLV::ContextTag(Tag::kMoreChunkedMessages), aMoreChunkedMessages); + mError = GetWriter()->UnreserveBuffer(GetSizeForMoreChunkResponses()); + SuccessOrExit(mError); + mIsMoreChunkMessageBufferReserved = false; } + + mError = mpWriter->PutBoolean(TLV::ContextTag(Tag::kMoreChunkedMessages), aMoreChunkedMessages); +exit: return *this; } +CHIP_ERROR InvokeResponseMessage::Builder::ReserveSpaceForMoreChunkedMessages() +{ + ReturnErrorOnFailure(GetWriter()->ReserveBuffer(GetSizeForMoreChunkResponses())); + mIsMoreChunkMessageBufferReserved = true; + return CHIP_NO_ERROR; +} + CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() { // If any changes are made to how we end the invoke response message that involves how many @@ -177,6 +195,15 @@ CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() return GetError(); } +uint32_t InvokeResponseMessage::Builder::GetSizeForMoreChunkResponses() +{ + // MoreChunkedMessages() encodes a uint8_t with context tag 0x02. This means 1 control byte, + // 1 byte for the tag. For booleans the value is encoded in control byte. + uint32_t kEncodeMoreChunkedMessages = 1 + 1; + + return kEncodeMoreChunkedMessages; +} + uint32_t InvokeResponseMessage::Builder::GetSizeToEndInvokeResponseMessage() { // EncodeInteractionModelRevision() encodes a uint8_t with context tag 0xFF. This means 1 control byte, diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index ff08ab1780cc02..8d50c31aff8d5b 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -110,6 +110,13 @@ class Builder : public MessageBuilder */ InvokeResponseMessage::Builder & MoreChunkedMessages(const bool aMoreChunkedMessages); + /** + * @brief Set True if the set of InvokeResponseIB have to be sent across multiple packets in a single transaction + * @param [in] aMoreChunkedMessages true if more chunked messages are needed + * @return A reference to *this + */ + CHIP_ERROR ReserveSpaceForMoreChunkedMessages(); + /** * @brief Mark the end of this InvokeResponseMessage * @@ -117,6 +124,13 @@ class Builder : public MessageBuilder */ CHIP_ERROR EndOfInvokeResponseMessage(); + /** + * @brief Get number of bytes required in the buffer by MoreChunkedMessages + * + * @return Expected number of bytes required in the buffer by MoreChunkedMessages() + */ + uint32_t GetSizeForMoreChunkResponses(); + /** * @brief Get number of bytes required in the buffer by EndOfInvokeResponseMessage() * @@ -127,6 +141,7 @@ class Builder : public MessageBuilder private: InvokeResponseIBs::Builder mInvokeResponses; bool mIsEndBufferReserved = false; + bool mIsMoreChunkMessageBufferReserved = false; }; } // namespace InvokeResponseMessage } // namespace app diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 4d53ba04cc5446..10417fa2025522 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -2136,6 +2136,29 @@ void InvokeResponseMessageEndOfMessageReservationTest(nlTestSuite * apSuite, voi NL_TEST_ASSERT(apSuite, remainingLengthAfterInitWithReservation == remainingLengthAfterEndingInvokeResponseMessage); } +void InvokeResponseMessageReservationForEndandMoreChunkTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeResponseMessage::Builder invokeResponseMessageBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeResponseMessageBuilder.InitWithEndBufferReserved(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeResponseMessageBuilder.ReserveSpaceForMoreChunkedMessages(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remainingLengthAllReservations = writer.GetRemainingFreeLength(); + + invokeResponseMessageBuilder.MoreChunkedMessages(/* aMoreChunkedMessages = */ true); + NL_TEST_ASSERT(apSuite, invokeResponseMessageBuilder.GetError() == CHIP_NO_ERROR); + err = invokeResponseMessageBuilder.EndOfInvokeResponseMessage(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remainingLengthAfterEndingInvokeResponseMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remainingLengthAllReservations == remainingLengthAfterEndingInvokeResponseMessage); +} + void InvokeResponsesEndOfResponseReservationTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -2373,6 +2396,7 @@ const nlTest sTests[] = NL_TEST_DEF("InvokeRequestsEndOfRequestReservationTest", InvokeRequestsEndOfRequestReservationTest), NL_TEST_DEF("InvokeInvokeResponseMessageTest", InvokeInvokeResponseMessageTest), NL_TEST_DEF("InvokeResponseMessageEndOfMessageReservationTest", InvokeResponseMessageEndOfMessageReservationTest), + NL_TEST_DEF("InvokeResponseMessageReservationForEndandMoreChunkTest", InvokeResponseMessageReservationForEndandMoreChunkTest), NL_TEST_DEF("InvokeResponsesEndOfResponseReservationTest", InvokeResponsesEndOfResponseReservationTest), NL_TEST_DEF("ReportDataMessageTest", ReportDataMessageTest), NL_TEST_DEF("ReadRequestMessageTest", ReadRequestMessageTest), From e2cdbbf6b08f1c97d57455dc2dc0c5ed8e765078 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 8 Jan 2024 19:08:53 +0000 Subject: [PATCH 2/3] Restyled by clang-format --- src/app/MessageDef/InvokeResponseMessage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index 8d50c31aff8d5b..ce552ce523e8c3 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -140,7 +140,7 @@ class Builder : public MessageBuilder private: InvokeResponseIBs::Builder mInvokeResponses; - bool mIsEndBufferReserved = false; + bool mIsEndBufferReserved = false; bool mIsMoreChunkMessageBufferReserved = false; }; } // namespace InvokeResponseMessage From 69f716c49e904b1b50f34f810ce5c78feb794dc1 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 9 Jan 2024 09:22:07 -0500 Subject: [PATCH 3/3] Fix CI --- src/app/MessageDef/InvokeResponseMessage.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index ce552ce523e8c3..020a049a8eae9e 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -111,9 +111,9 @@ class Builder : public MessageBuilder InvokeResponseMessage::Builder & MoreChunkedMessages(const bool aMoreChunkedMessages); /** - * @brief Set True if the set of InvokeResponseIB have to be sent across multiple packets in a single transaction - * @param [in] aMoreChunkedMessages true if more chunked messages are needed - * @return A reference to *this + * @brief Reserved space in TLVWriter for MoreChunkedMessages + * @return CHIP_NO_ERROR upon successfully reserving space for MoreChunkedMessages + * @return other CHIP error see TLVWriter::ReserveBuffer for more details. */ CHIP_ERROR ReserveSpaceForMoreChunkedMessages();