From 1ddb9fe75d49a453f28b6191b2a750aff6b44a0b Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Mon, 18 Dec 2023 16:40:49 +0000 Subject: [PATCH 1/6] CommandSender and CommandHandler reserves space for Finalize to succeed --- src/app/CommandHandler.cpp | 7 ++ src/app/CommandSender.cpp | 9 ++ src/app/MessageDef/InvokeRequestMessage.cpp | 9 ++ src/app/MessageDef/InvokeRequestMessage.h | 7 ++ src/app/MessageDef/InvokeRequests.cpp | 6 ++ src/app/MessageDef/InvokeRequests.h | 7 ++ src/app/MessageDef/InvokeResponseIBs.cpp | 6 ++ src/app/MessageDef/InvokeResponseIBs.h | 7 ++ src/app/MessageDef/InvokeResponseMessage.cpp | 9 ++ src/app/MessageDef/InvokeResponseMessage.h | 7 ++ src/app/tests/TestMessageDef.cpp | 100 +++++++++++++++++++ 11 files changed, 174 insertions(+) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 1b4f8ea92998ba..b7c22d388e53a5 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -69,6 +69,10 @@ CHIP_ERROR CommandHandler::AllocateBuffer() mInvokeResponseBuilder.CreateInvokeResponses(); ReturnErrorOnFailure(mInvokeResponseBuilder.GetError()); + + auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); mBufferAllocated = true; } @@ -651,6 +655,9 @@ CHIP_ERROR CommandHandler::FinishCommand(bool aStartDataStruct) ReturnErrorOnFailure(commandData.Ref(mRefForResponse.Value())); } + auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); ReturnErrorOnFailure(commandData.EndOfCommandDataIB()); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().EndOfInvokeResponseIB()); MoveToState(State::AddedCommand); diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index a17022d80a1f77..0e7963637b4062 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -91,6 +91,10 @@ CHIP_ERROR CommandSender::AllocateBuffer() mInvokeRequestBuilder.CreateInvokeRequests(); ReturnErrorOnFailure(mInvokeRequestBuilder.GetError()); + auto * tlvWriter = mInvokeRequestBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeRequestBuilder.GetInvokeRequests().GetSizeToEndInvokeRequests())); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeRequestBuilder.GetSizeToEndInvokeRequestMessage())); + mBufferAllocated = true; } @@ -508,6 +512,11 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional & aTimedInvokeT CHIP_ERROR CommandSender::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + + auto * tlvWriter = mInvokeRequestBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeRequestBuilder.GetInvokeRequests().GetSizeToEndInvokeRequests())); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeRequestBuilder.GetSizeToEndInvokeRequestMessage())); + ReturnErrorOnFailure(mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests()); ReturnErrorOnFailure(mInvokeRequestBuilder.EndOfInvokeRequestMessage()); return mCommandMessageWriter.Finalize(&commandPacket); diff --git a/src/app/MessageDef/InvokeRequestMessage.cpp b/src/app/MessageDef/InvokeRequestMessage.cpp index 3cd3066f03e4ef..c6fc665fae1966 100644 --- a/src/app/MessageDef/InvokeRequestMessage.cpp +++ b/src/app/MessageDef/InvokeRequestMessage.cpp @@ -152,5 +152,14 @@ CHIP_ERROR InvokeRequestMessage::Builder::EndOfInvokeRequestMessage() } return GetError(); } + +uint32_t InvokeRequestMessage::Builder::GetSizeToEndInvokeRequestMessage() +{ + // This encodes uint8_t into Tag 0xFF. This means 1 bytes for tag, 1 byte for length, 1 byte for value + uint32_t kEncodeInteractionModelSize = 1 + 1 + 1; + uint32_t kEndOfContainerSize = 1; + + return kEncodeInteractionModelSize + kEndOfContainerSize; +} }; // namespace app }; // namespace chip diff --git a/src/app/MessageDef/InvokeRequestMessage.h b/src/app/MessageDef/InvokeRequestMessage.h index 0678e03ec10480..3c7be83be46012 100644 --- a/src/app/MessageDef/InvokeRequestMessage.h +++ b/src/app/MessageDef/InvokeRequestMessage.h @@ -106,6 +106,13 @@ class Builder : public MessageBuilder */ CHIP_ERROR EndOfInvokeRequestMessage(); + /** + * @brief Get number of bytes required to call EndOfInvokeRequestMessage() + * + * @return Expected number of bytes required to call EndOfInvokeRequestMessage() + */ + uint32_t GetSizeToEndInvokeRequestMessage(); + private: InvokeRequests::Builder mInvokeRequests; }; diff --git a/src/app/MessageDef/InvokeRequests.cpp b/src/app/MessageDef/InvokeRequests.cpp index 56456121de9b3d..4e46cf45799d07 100644 --- a/src/app/MessageDef/InvokeRequests.cpp +++ b/src/app/MessageDef/InvokeRequests.cpp @@ -84,5 +84,11 @@ CHIP_ERROR InvokeRequests::Builder::EndOfInvokeRequests() EndOfContainer(); return GetError(); } + +uint32_t InvokeRequests::Builder::GetSizeToEndInvokeRequests() +{ + uint32_t kEndOfContainerSize = 1; + return kEndOfContainerSize; +} } // namespace app } // namespace chip diff --git a/src/app/MessageDef/InvokeRequests.h b/src/app/MessageDef/InvokeRequests.h index 71f9f74c328725..06f4402d37c052 100644 --- a/src/app/MessageDef/InvokeRequests.h +++ b/src/app/MessageDef/InvokeRequests.h @@ -61,6 +61,13 @@ class Builder : public ArrayBuilder */ CHIP_ERROR EndOfInvokeRequests(); + /** + * @brief Get number of bytes required to call EndOfInvokeRequests() + * + * @return Expected number of bytes required to call EndOfInvokeRequests() + */ + uint32_t GetSizeToEndInvokeRequests(); + private: CommandDataIB::Builder mCommandData; }; diff --git a/src/app/MessageDef/InvokeResponseIBs.cpp b/src/app/MessageDef/InvokeResponseIBs.cpp index 536d18abb25687..e22e9e72a93ddf 100644 --- a/src/app/MessageDef/InvokeResponseIBs.cpp +++ b/src/app/MessageDef/InvokeResponseIBs.cpp @@ -84,5 +84,11 @@ CHIP_ERROR InvokeResponseIBs::Builder::EndOfInvokeResponses() EndOfContainer(); return GetError(); } + +uint32_t InvokeResponseIBs::Builder::GetSizeToEndInvokeResponses() +{ + uint32_t kEndOfContainerSize = 1; + return kEndOfContainerSize; +} } // namespace app } // namespace chip diff --git a/src/app/MessageDef/InvokeResponseIBs.h b/src/app/MessageDef/InvokeResponseIBs.h index 2531785fd6fa94..e739664664b5cd 100644 --- a/src/app/MessageDef/InvokeResponseIBs.h +++ b/src/app/MessageDef/InvokeResponseIBs.h @@ -61,6 +61,13 @@ class Builder : public ArrayBuilder */ CHIP_ERROR EndOfInvokeResponses(); + /** + * @brief Get number of bytes required to call EndOfInvokeResponses() + * + * @return Expected number of bytes required to call EndOfInvokeResponses() + */ + uint32_t GetSizeToEndInvokeResponses(); + private: InvokeResponseIB::Builder mInvokeResponse; }; diff --git a/src/app/MessageDef/InvokeResponseMessage.cpp b/src/app/MessageDef/InvokeResponseMessage.cpp index cb36aa15160633..a19bd4affab967 100644 --- a/src/app/MessageDef/InvokeResponseMessage.cpp +++ b/src/app/MessageDef/InvokeResponseMessage.cpp @@ -152,5 +152,14 @@ CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() } return GetError(); } + +uint32_t InvokeResponseMessage::Builder::GetSizeToEndInvokeResponseMessage() +{ + // This encodes uint8_t into Tag 0xFF. This means 1 bytes for tag, 1 byte for length, 1 byte for value + uint32_t kEncodeInteractionModelSize = 1 + 1 + 1; + uint32_t kEndOfContainerSize = 1; + + return kEncodeInteractionModelSize + kEndOfContainerSize; +} } // namespace app } // namespace chip diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index 34cc69aecfe976..365261f86c195e 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -111,6 +111,13 @@ class Builder : public MessageBuilder */ CHIP_ERROR EndOfInvokeResponseMessage(); + /** + * @brief Get number of bytes required to call EndOfInvokeResponseMessage() + * + * @return Expected number of bytes required to call EndOfInvokeResponseMessage() + */ + uint32_t GetSizeToEndInvokeResponseMessage(); + private: InvokeResponseIBs::Builder mInvokeResponses; }; diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index c78545926f59b2..578cd535b230ff 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -2057,6 +2057,54 @@ void InvokeInvokeRequestMessageTest(nlTestSuite * apSuite, void * apContext) ParseInvokeRequestMessage(apSuite, reader); } +void InvokeRequestMessageEndOfMessageReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeRequestMessage::Builder invokeRequestMessageBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeRequestMessageBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeRequestMessageBuilder.GetSizeToEndInvokeRequestMessage(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeRequestMessageBuilder.EndOfInvokeRequestMessage(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeRequestMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeRequestMessage); +} + +void InvokeRequestsEndOfRequestReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeRequests::Builder invokeRequestsBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeRequestsBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeRequestsBuilder.GetSizeToEndInvokeRequests(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeRequestsBuilder.EndOfInvokeRequests(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeRequests = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeRequests); +} + void InvokeInvokeResponseMessageTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -2074,6 +2122,54 @@ void InvokeInvokeResponseMessageTest(nlTestSuite * apSuite, void * apContext) ParseInvokeResponseMessage(apSuite, reader); } +void InvokeResponseMessageEndOfMessageReservationTest(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.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeResponseMessageBuilder.GetSizeToEndInvokeResponseMessage(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeResponseMessageBuilder.EndOfInvokeResponseMessage(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeResponseMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeResponseMessage); +} + +void InvokeResponsesEndOfResponseReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeResponseIBs::Builder invokeResponsesBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeResponsesBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeResponsesBuilder.GetSizeToEndInvokeResponses(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeResponsesBuilder.EndOfInvokeResponses(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeResponses = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeResponses); +} + void ReportDataMessageTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -2283,7 +2379,11 @@ const nlTest sTests[] = NL_TEST_DEF("InvokeRequestsTest", InvokeRequestsTest), NL_TEST_DEF("InvokeResponsesTest", InvokeResponsesTest), NL_TEST_DEF("InvokeInvokeRequestMessageTest", InvokeInvokeRequestMessageTest), + NL_TEST_DEF("InvokeRequestMessageEndOfMessageReservationTest", InvokeRequestMessageEndOfMessageReservationTest), + NL_TEST_DEF("InvokeRequestsEndOfRequestReservationTest", InvokeRequestsEndOfRequestReservationTest), NL_TEST_DEF("InvokeInvokeResponseMessageTest", InvokeInvokeResponseMessageTest), + NL_TEST_DEF("InvokeResponseMessageEndOfMessageReservationTest", InvokeResponseMessageEndOfMessageReservationTest), + NL_TEST_DEF("InvokeResponsesEndOfResponseReservationTest", InvokeResponsesEndOfResponseReservationTest), NL_TEST_DEF("ReportDataMessageTest", ReportDataMessageTest), NL_TEST_DEF("ReadRequestMessageTest", ReadRequestMessageTest), NL_TEST_DEF("WriteRequestMessageTest", WriteRequestMessageTest), From e2efda120181b6bb22513258ef6b9bea9f43bd66 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 2 Jan 2024 14:21:09 +0000 Subject: [PATCH 2/6] Address PR comments --- src/app/CommandHandler.cpp | 4 ++-- src/app/MessageDef/InvokeRequestMessage.cpp | 6 +++++- src/app/MessageDef/InvokeRequestMessage.h | 4 ++-- src/app/MessageDef/InvokeRequests.cpp | 3 +++ src/app/MessageDef/InvokeRequests.h | 4 ++-- src/app/MessageDef/InvokeResponseIBs.cpp | 3 +++ src/app/MessageDef/InvokeResponseIBs.h | 4 ++-- src/app/MessageDef/InvokeResponseMessage.cpp | 6 +++++- src/app/MessageDef/InvokeResponseMessage.h | 4 ++-- src/app/tests/TestMessageDef.cpp | 4 ++-- 10 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index b7c22d388e53a5..74d664b1e87508 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -656,8 +656,6 @@ CHIP_ERROR CommandHandler::FinishCommand(bool aStartDataStruct) } auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); - ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); - ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); ReturnErrorOnFailure(commandData.EndOfCommandDataIB()); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().EndOfInvokeResponseIB()); MoveToState(State::AddedCommand); @@ -763,6 +761,8 @@ CommandHandler::Handle::Handle(CommandHandler * handle) CHIP_ERROR CommandHandler::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().EndOfInvokeResponses()); ReturnErrorOnFailure(mInvokeResponseBuilder.EndOfInvokeResponseMessage()); return mCommandMessageWriter.Finalize(&commandPacket); diff --git a/src/app/MessageDef/InvokeRequestMessage.cpp b/src/app/MessageDef/InvokeRequestMessage.cpp index c6fc665fae1966..65f68b5dd2d27e 100644 --- a/src/app/MessageDef/InvokeRequestMessage.cpp +++ b/src/app/MessageDef/InvokeRequestMessage.cpp @@ -142,6 +142,9 @@ InvokeRequests::Builder & InvokeRequestMessage::Builder::CreateInvokeRequests() CHIP_ERROR InvokeRequestMessage::Builder::EndOfInvokeRequestMessage() { + // If any changes are made to how we end the invoke request message that involves how many + // bytes are needed, a corresponding change to GetSizeToEndInvokeRequestMessage indicating + // the new size that will be required. if (mError == CHIP_NO_ERROR) { mError = MessageBuilder::EncodeInteractionModelRevision(); @@ -155,7 +158,8 @@ CHIP_ERROR InvokeRequestMessage::Builder::EndOfInvokeRequestMessage() uint32_t InvokeRequestMessage::Builder::GetSizeToEndInvokeRequestMessage() { - // This encodes uint8_t into Tag 0xFF. This means 1 bytes for tag, 1 byte for length, 1 byte for value + // EncodeInteractionModelRevision() encodes a uint8_t with context tag 0xFF. This means 1 control byte, + // 1 byte for the tag, 1 byte for the value. uint32_t kEncodeInteractionModelSize = 1 + 1 + 1; uint32_t kEndOfContainerSize = 1; diff --git a/src/app/MessageDef/InvokeRequestMessage.h b/src/app/MessageDef/InvokeRequestMessage.h index 3c7be83be46012..3f2447892a37c6 100644 --- a/src/app/MessageDef/InvokeRequestMessage.h +++ b/src/app/MessageDef/InvokeRequestMessage.h @@ -107,9 +107,9 @@ class Builder : public MessageBuilder CHIP_ERROR EndOfInvokeRequestMessage(); /** - * @brief Get number of bytes required to call EndOfInvokeRequestMessage() + * @brief Get number of bytes required in the buffer by EndOfInvokeRequestMessage() * - * @return Expected number of bytes required to call EndOfInvokeRequestMessage() + * @return Expected number of bytes required in the buffer by EndOfInvokeRequestMessage() */ uint32_t GetSizeToEndInvokeRequestMessage(); diff --git a/src/app/MessageDef/InvokeRequests.cpp b/src/app/MessageDef/InvokeRequests.cpp index 4e46cf45799d07..9f7d09895baa07 100644 --- a/src/app/MessageDef/InvokeRequests.cpp +++ b/src/app/MessageDef/InvokeRequests.cpp @@ -81,6 +81,9 @@ CommandDataIB::Builder & InvokeRequests::Builder::CreateCommandData() CHIP_ERROR InvokeRequests::Builder::EndOfInvokeRequests() { + // If any changes are made to how we end the invoke requests that involves how many bytes are + // needed, a corresponding change to GetSizeToEndInvokeRequests indicating the new size that + // will be required. EndOfContainer(); return GetError(); } diff --git a/src/app/MessageDef/InvokeRequests.h b/src/app/MessageDef/InvokeRequests.h index 06f4402d37c052..b54795a3d7c2cc 100644 --- a/src/app/MessageDef/InvokeRequests.h +++ b/src/app/MessageDef/InvokeRequests.h @@ -62,9 +62,9 @@ class Builder : public ArrayBuilder CHIP_ERROR EndOfInvokeRequests(); /** - * @brief Get number of bytes required to call EndOfInvokeRequests() + * @brief Get number of bytes required in the buffer by EndOfInvokeRequests() * - * @return Expected number of bytes required to call EndOfInvokeRequests() + * @return Expected number of bytes required in the buffer by EndOfInvokeRequests() */ uint32_t GetSizeToEndInvokeRequests(); diff --git a/src/app/MessageDef/InvokeResponseIBs.cpp b/src/app/MessageDef/InvokeResponseIBs.cpp index e22e9e72a93ddf..f20426d5415d39 100644 --- a/src/app/MessageDef/InvokeResponseIBs.cpp +++ b/src/app/MessageDef/InvokeResponseIBs.cpp @@ -81,6 +81,9 @@ InvokeResponseIB::Builder & InvokeResponseIBs::Builder::CreateInvokeResponse() CHIP_ERROR InvokeResponseIBs::Builder::EndOfInvokeResponses() { + // If any changes are made to how we end the invoke responses that involves how many bytes are + // needed, a corresponding change to GetSizeToEndInvokeResponses indicating the new size that + // will be required. EndOfContainer(); return GetError(); } diff --git a/src/app/MessageDef/InvokeResponseIBs.h b/src/app/MessageDef/InvokeResponseIBs.h index e739664664b5cd..2dd4d2ce2061f1 100644 --- a/src/app/MessageDef/InvokeResponseIBs.h +++ b/src/app/MessageDef/InvokeResponseIBs.h @@ -62,9 +62,9 @@ class Builder : public ArrayBuilder CHIP_ERROR EndOfInvokeResponses(); /** - * @brief Get number of bytes required to call EndOfInvokeResponses() + * @brief Get number of bytes required in the buffer by EndOfInvokeResponses() * - * @return Expected number of bytes required to call EndOfInvokeResponses() + * @return Expected number of bytes required in the buffer by EndOfInvokeResponses() */ uint32_t GetSizeToEndInvokeResponses(); diff --git a/src/app/MessageDef/InvokeResponseMessage.cpp b/src/app/MessageDef/InvokeResponseMessage.cpp index a19bd4affab967..44484d91c234d7 100644 --- a/src/app/MessageDef/InvokeResponseMessage.cpp +++ b/src/app/MessageDef/InvokeResponseMessage.cpp @@ -142,6 +142,9 @@ InvokeResponseMessage::Builder & InvokeResponseMessage::Builder::MoreChunkedMess CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() { + // If any changes are made to how we end the invoke response message that involves how many + // bytes are needed, a corresponding change to GetSizeToEndInvokeResponseMessage indicating + // the new size that will be required. if (mError == CHIP_NO_ERROR) { mError = MessageBuilder::EncodeInteractionModelRevision(); @@ -155,7 +158,8 @@ CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() uint32_t InvokeResponseMessage::Builder::GetSizeToEndInvokeResponseMessage() { - // This encodes uint8_t into Tag 0xFF. This means 1 bytes for tag, 1 byte for length, 1 byte for value + // EncodeInteractionModelRevision() encodes a uint8_t with context tag 0xFF. This means 1 control byte, + // 1 byte for the tag, 1 byte for the value. uint32_t kEncodeInteractionModelSize = 1 + 1 + 1; uint32_t kEndOfContainerSize = 1; diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index 365261f86c195e..f06b0fb83e5a96 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -112,9 +112,9 @@ class Builder : public MessageBuilder CHIP_ERROR EndOfInvokeResponseMessage(); /** - * @brief Get number of bytes required to call EndOfInvokeResponseMessage() + * @brief Get number of bytes required in the buffer by EndOfInvokeResponseMessage() * - * @return Expected number of bytes required to call EndOfInvokeResponseMessage() + * @return Expected number of bytes required in the buffer by EndOfInvokeResponseMessage() */ uint32_t GetSizeToEndInvokeResponseMessage(); diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 578cd535b230ff..c548fcc1354402 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -2077,8 +2077,8 @@ void InvokeRequestMessageEndOfMessageReservationTest(nlTestSuite * apSuite, void err = invokeRequestMessageBuilder.EndOfInvokeRequestMessage(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterEndingInvokeRequestMessage = writer.GetRemainingFreeLength(); - NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeRequestMessage); + uint32_t remainingLengthAfterEndingInvokeRequestMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remainingLengthAfterEndingInvokeRequestMessage); } void InvokeRequestsEndOfRequestReservationTest(nlTestSuite * apSuite, void * apContext) From a37c7826a52ddfc0a45dc68ce89f2435a741acbc Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 2 Jan 2024 15:22:05 +0000 Subject: [PATCH 3/6] Fix CI --- src/app/CommandHandler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 74d664b1e87508..ec5bf1d83d9f74 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -655,7 +655,6 @@ CHIP_ERROR CommandHandler::FinishCommand(bool aStartDataStruct) ReturnErrorOnFailure(commandData.Ref(mRefForResponse.Value())); } - auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); ReturnErrorOnFailure(commandData.EndOfCommandDataIB()); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().EndOfInvokeResponseIB()); MoveToState(State::AddedCommand); @@ -761,6 +760,8 @@ CommandHandler::Handle::Handle(CommandHandler * handle) CHIP_ERROR CommandHandler::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + + auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().EndOfInvokeResponses()); From 3ba7555100657b76d13f7b32d393c41e51d674d1 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 3 Jan 2024 15:45:27 +0000 Subject: [PATCH 4/6] Address PR comments --- src/app/CommandHandler.cpp | 11 +--- src/app/CommandSender.cpp | 13 +--- src/app/MessageDef/InvokeRequestMessage.cpp | 25 +++++++- src/app/MessageDef/InvokeRequestMessage.h | 9 ++- src/app/MessageDef/InvokeRequests.cpp | 13 ++++ src/app/MessageDef/InvokeRequests.h | 7 +++ src/app/MessageDef/InvokeResponseIBs.cpp | 13 ++++ src/app/MessageDef/InvokeResponseIBs.h | 7 +++ src/app/MessageDef/InvokeResponseMessage.cpp | 25 +++++++- src/app/MessageDef/InvokeResponseMessage.h | 9 ++- src/app/tests/TestMessageDef.cpp | 62 ++++++++------------ 11 files changed, 132 insertions(+), 62 deletions(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index ec5bf1d83d9f74..3096982ff3df45 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -62,17 +62,14 @@ CHIP_ERROR CommandHandler::AllocateBuffer() VerifyOrReturnError(!commandPacket.IsNull(), CHIP_ERROR_NO_MEMORY); mCommandMessageWriter.Init(std::move(commandPacket)); - ReturnErrorOnFailure(mInvokeResponseBuilder.Init(&mCommandMessageWriter)); + ReturnErrorOnFailure(mInvokeResponseBuilder.InitWithEndBufferReserved(&mCommandMessageWriter)); mInvokeResponseBuilder.SuppressResponse(mSuppressResponse); ReturnErrorOnFailure(mInvokeResponseBuilder.GetError()); - mInvokeResponseBuilder.CreateInvokeResponses(); + mInvokeResponseBuilder.CreateInvokeResponses(/*aReserveEndBuffer =*/true); ReturnErrorOnFailure(mInvokeResponseBuilder.GetError()); - auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); - ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); - ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); mBufferAllocated = true; } @@ -760,10 +757,6 @@ CommandHandler::Handle::Handle(CommandHandler * handle) CHIP_ERROR CommandHandler::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); - - auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); - ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); - ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().EndOfInvokeResponses()); ReturnErrorOnFailure(mInvokeResponseBuilder.EndOfInvokeResponseMessage()); return mCommandMessageWriter.Finalize(&commandPacket); diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 0e7963637b4062..6b93b4ab9c67c9 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -83,18 +83,14 @@ CHIP_ERROR CommandSender::AllocateBuffer() VerifyOrReturnError(!commandPacket.IsNull(), CHIP_ERROR_NO_MEMORY); mCommandMessageWriter.Init(std::move(commandPacket)); - ReturnErrorOnFailure(mInvokeRequestBuilder.Init(&mCommandMessageWriter)); + ReturnErrorOnFailure(mInvokeRequestBuilder.InitWithEndBufferReserved(&mCommandMessageWriter)); mInvokeRequestBuilder.SuppressResponse(mSuppressResponse).TimedRequest(mTimedRequest); ReturnErrorOnFailure(mInvokeRequestBuilder.GetError()); - mInvokeRequestBuilder.CreateInvokeRequests(); + mInvokeRequestBuilder.CreateInvokeRequests(/* aReserveEndBuffer = */ true); ReturnErrorOnFailure(mInvokeRequestBuilder.GetError()); - auto * tlvWriter = mInvokeRequestBuilder.GetWriter(); - ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeRequestBuilder.GetInvokeRequests().GetSizeToEndInvokeRequests())); - ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeRequestBuilder.GetSizeToEndInvokeRequestMessage())); - mBufferAllocated = true; } @@ -512,11 +508,6 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional & aTimedInvokeT CHIP_ERROR CommandSender::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); - - auto * tlvWriter = mInvokeRequestBuilder.GetWriter(); - ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeRequestBuilder.GetInvokeRequests().GetSizeToEndInvokeRequests())); - ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeRequestBuilder.GetSizeToEndInvokeRequestMessage())); - ReturnErrorOnFailure(mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests()); ReturnErrorOnFailure(mInvokeRequestBuilder.EndOfInvokeRequestMessage()); return mCommandMessageWriter.Finalize(&commandPacket); diff --git a/src/app/MessageDef/InvokeRequestMessage.cpp b/src/app/MessageDef/InvokeRequestMessage.cpp index 65f68b5dd2d27e..50bd5cd45f70c0 100644 --- a/src/app/MessageDef/InvokeRequestMessage.cpp +++ b/src/app/MessageDef/InvokeRequestMessage.cpp @@ -113,6 +113,14 @@ CHIP_ERROR InvokeRequestMessage::Parser::GetInvokeRequests(InvokeRequests::Parse return apInvokeRequests->Init(reader); } +CHIP_ERROR InvokeRequestMessage::Builder::InitWithEndBufferReserved(TLV::TLVWriter * const apWriter) +{ + ReturnErrorOnFailure(Init(apWriter)); + ReturnErrorOnFailure(GetWriter()->ReserveBuffer(GetSizeToEndInvokeRequestMessage())); + mIsEndBufferReserved = true; + return CHIP_NO_ERROR; +} + InvokeRequestMessage::Builder & InvokeRequestMessage::Builder::SuppressResponse(const bool aSuppressResponse) { if (mError == CHIP_NO_ERROR) @@ -131,11 +139,18 @@ InvokeRequestMessage::Builder & InvokeRequestMessage::Builder::TimedRequest(cons return *this; } -InvokeRequests::Builder & InvokeRequestMessage::Builder::CreateInvokeRequests() +InvokeRequests::Builder & InvokeRequestMessage::Builder::CreateInvokeRequests(const bool aReserveEndBuffer) { if (mError == CHIP_NO_ERROR) { - mError = mInvokeRequests.Init(mpWriter, to_underlying(Tag::kInvokeRequests)); + if (aReserveEndBuffer) + { + mError = mInvokeRequests.InitWithEndBufferReserved(mpWriter, to_underlying(Tag::kInvokeRequests)); + } + else + { + mError = mInvokeRequests.Init(mpWriter, to_underlying(Tag::kInvokeRequests)); + } } return mInvokeRequests; } @@ -145,6 +160,12 @@ CHIP_ERROR InvokeRequestMessage::Builder::EndOfInvokeRequestMessage() // If any changes are made to how we end the invoke request message that involves how many // bytes are needed, a corresponding change to GetSizeToEndInvokeRequestMessage indicating // the new size that will be required. + ReturnErrorOnFailure(mError); + if (mIsEndBufferReserved) + { + ReturnErrorOnFailure(GetWriter()->UnreserveBuffer(GetSizeToEndInvokeRequestMessage())); + mIsEndBufferReserved = false; + } if (mError == CHIP_NO_ERROR) { mError = MessageBuilder::EncodeInteractionModelRevision(); diff --git a/src/app/MessageDef/InvokeRequestMessage.h b/src/app/MessageDef/InvokeRequestMessage.h index 3f2447892a37c6..600442e4752dba 100644 --- a/src/app/MessageDef/InvokeRequestMessage.h +++ b/src/app/MessageDef/InvokeRequestMessage.h @@ -75,6 +75,12 @@ class Parser : public MessageParser class Builder : public MessageBuilder { public: + /** + * @brief Performs underlying StructBuilder::Init, but reserves memory need in + * EndOfInvokeRequestMessage() with underlying TLVWriter. + */ + CHIP_ERROR InitWithEndBufferReserved(TLV::TLVWriter * const apWriter); + /** * @brief when sets to true, it means do not send a response to this action */ @@ -90,7 +96,7 @@ class Builder : public MessageBuilder * * @return A reference to InvokeRequests::Builder */ - InvokeRequests::Builder & CreateInvokeRequests(); + InvokeRequests::Builder & CreateInvokeRequests(const bool aReserveEndBuffer = false); /** * @brief Get reference to InvokeRequests::Builder @@ -115,6 +121,7 @@ class Builder : public MessageBuilder private: InvokeRequests::Builder mInvokeRequests; + bool mIsEndBufferReserved = false; }; } // namespace InvokeRequestMessage } // namespace app diff --git a/src/app/MessageDef/InvokeRequests.cpp b/src/app/MessageDef/InvokeRequests.cpp index 9f7d09895baa07..e01ad853e8b93c 100644 --- a/src/app/MessageDef/InvokeRequests.cpp +++ b/src/app/MessageDef/InvokeRequests.cpp @@ -70,6 +70,14 @@ CHIP_ERROR InvokeRequests::Parser::PrettyPrint() const } #endif // CHIP_CONFIG_IM_PRETTY_PRINT +CHIP_ERROR InvokeRequests::Builder::InitWithEndBufferReserved(TLV::TLVWriter * const apWriter, const uint8_t aContextTagToUse) +{ + ReturnErrorOnFailure(Init(apWriter, aContextTagToUse)); + ReturnErrorOnFailure(GetWriter()->ReserveBuffer(GetSizeToEndInvokeRequests())); + mIsEndBufferReserved = true; + return CHIP_NO_ERROR; +} + CommandDataIB::Builder & InvokeRequests::Builder::CreateCommandData() { if (mError == CHIP_NO_ERROR) @@ -84,6 +92,11 @@ CHIP_ERROR InvokeRequests::Builder::EndOfInvokeRequests() // If any changes are made to how we end the invoke requests that involves how many bytes are // needed, a corresponding change to GetSizeToEndInvokeRequests indicating the new size that // will be required. + if (mIsEndBufferReserved) + { + ReturnErrorOnFailure(GetWriter()->UnreserveBuffer(GetSizeToEndInvokeRequests())); + mIsEndBufferReserved = false; + } EndOfContainer(); return GetError(); } diff --git a/src/app/MessageDef/InvokeRequests.h b/src/app/MessageDef/InvokeRequests.h index b54795a3d7c2cc..a32d812fa3ef92 100644 --- a/src/app/MessageDef/InvokeRequests.h +++ b/src/app/MessageDef/InvokeRequests.h @@ -42,6 +42,12 @@ class Parser : public ArrayParser class Builder : public ArrayBuilder { public: + /** + * @brief Performs underlying StructBuilder::Init, but reserves memory need in + * EndOfInvokeRequests() with underlying TLVWriter. + */ + CHIP_ERROR InitWithEndBufferReserved(TLV::TLVWriter * const apWriter, const uint8_t aContextTagToUse); + /** * @brief Initialize a CommandDataIB::Builder for writing into the TLV stream * @@ -70,6 +76,7 @@ class Builder : public ArrayBuilder private: CommandDataIB::Builder mCommandData; + bool mIsEndBufferReserved = false; }; } // namespace InvokeRequests } // namespace app diff --git a/src/app/MessageDef/InvokeResponseIBs.cpp b/src/app/MessageDef/InvokeResponseIBs.cpp index f20426d5415d39..d7b8c48c231616 100644 --- a/src/app/MessageDef/InvokeResponseIBs.cpp +++ b/src/app/MessageDef/InvokeResponseIBs.cpp @@ -70,6 +70,14 @@ CHIP_ERROR InvokeResponseIBs::Parser::PrettyPrint() const } #endif // CHIP_CONFIG_IM_PRETTY_PRINT +CHIP_ERROR InvokeResponseIBs::Builder::InitWithEndBufferReserved(TLV::TLVWriter * const apWriter, const uint8_t aContextTagToUse) +{ + ReturnErrorOnFailure(Init(apWriter, aContextTagToUse)); + ReturnErrorOnFailure(GetWriter()->ReserveBuffer(GetSizeToEndInvokeResponses())); + mIsEndBufferReserved = true; + return CHIP_NO_ERROR; +} + InvokeResponseIB::Builder & InvokeResponseIBs::Builder::CreateInvokeResponse() { if (mError == CHIP_NO_ERROR) @@ -84,6 +92,11 @@ CHIP_ERROR InvokeResponseIBs::Builder::EndOfInvokeResponses() // If any changes are made to how we end the invoke responses that involves how many bytes are // needed, a corresponding change to GetSizeToEndInvokeResponses indicating the new size that // will be required. + if (mIsEndBufferReserved) + { + ReturnErrorOnFailure(GetWriter()->UnreserveBuffer(GetSizeToEndInvokeResponses())); + mIsEndBufferReserved = false; + } EndOfContainer(); return GetError(); } diff --git a/src/app/MessageDef/InvokeResponseIBs.h b/src/app/MessageDef/InvokeResponseIBs.h index 2dd4d2ce2061f1..415e6935c4ab85 100644 --- a/src/app/MessageDef/InvokeResponseIBs.h +++ b/src/app/MessageDef/InvokeResponseIBs.h @@ -42,6 +42,12 @@ class Parser : public ArrayParser class Builder : public ArrayBuilder { public: + /** + * @brief Performs underlying StructBuilder::Init, but reserves memory need in + * EndOfInvokeResponses() with underlying TLVWriter. + */ + CHIP_ERROR InitWithEndBufferReserved(TLV::TLVWriter * const apWriter, const uint8_t aContextTagToUse); + /** * @brief Initialize a InvokeResponseIB::Builder for writing into the TLV stream * @@ -70,6 +76,7 @@ class Builder : public ArrayBuilder private: InvokeResponseIB::Builder mInvokeResponse; + bool mIsEndBufferReserved = false; }; } // namespace InvokeResponseIBs } // namespace app diff --git a/src/app/MessageDef/InvokeResponseMessage.cpp b/src/app/MessageDef/InvokeResponseMessage.cpp index 44484d91c234d7..cc9b89d3b15f35 100644 --- a/src/app/MessageDef/InvokeResponseMessage.cpp +++ b/src/app/MessageDef/InvokeResponseMessage.cpp @@ -112,6 +112,14 @@ CHIP_ERROR InvokeResponseMessage::Parser::GetMoreChunkedMessages(bool * const ap return GetSimpleValue(to_underlying(Tag::kMoreChunkedMessages), TLV::kTLVType_Boolean, apMoreChunkedMessages); } +CHIP_ERROR InvokeResponseMessage::Builder::InitWithEndBufferReserved(TLV::TLVWriter * const apWriter) +{ + ReturnErrorOnFailure(Init(apWriter)); + ReturnErrorOnFailure(GetWriter()->ReserveBuffer(GetSizeToEndInvokeResponseMessage())); + mIsEndBufferReserved = true; + return CHIP_NO_ERROR; +} + InvokeResponseMessage::Builder & InvokeResponseMessage::Builder::SuppressResponse(const bool aSuppressResponse) { if (mError == CHIP_NO_ERROR) @@ -121,11 +129,18 @@ InvokeResponseMessage::Builder & InvokeResponseMessage::Builder::SuppressRespons return *this; } -InvokeResponseIBs::Builder & InvokeResponseMessage::Builder::CreateInvokeResponses() +InvokeResponseIBs::Builder & InvokeResponseMessage::Builder::CreateInvokeResponses(const bool aReserveEndBuffer) { if (mError == CHIP_NO_ERROR) { - mError = mInvokeResponses.Init(mpWriter, to_underlying(Tag::kInvokeResponses)); + if (aReserveEndBuffer) + { + mError = mInvokeResponses.InitWithEndBufferReserved(mpWriter, to_underlying(Tag::kInvokeResponses)); + } + else + { + mError = mInvokeResponses.Init(mpWriter, to_underlying(Tag::kInvokeResponses)); + } } return mInvokeResponses; } @@ -145,6 +160,12 @@ CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() // If any changes are made to how we end the invoke response message that involves how many // bytes are needed, a corresponding change to GetSizeToEndInvokeResponseMessage indicating // the new size that will be required. + ReturnErrorOnFailure(mError); + if (mIsEndBufferReserved) + { + ReturnErrorOnFailure(GetWriter()->UnreserveBuffer(GetSizeToEndInvokeResponseMessage())); + mIsEndBufferReserved = false; + } if (mError == CHIP_NO_ERROR) { mError = MessageBuilder::EncodeInteractionModelRevision(); diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index f06b0fb83e5a96..ff08ab1780cc02 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -77,6 +77,12 @@ class Parser : public MessageParser class Builder : public MessageBuilder { public: + /** + * @brief Performs underlying StructBuilder::Init, but reserves memory need in + * EndOfInvokeResponseMessage() with underlying TLVWriter. + */ + CHIP_ERROR InitWithEndBufferReserved(TLV::TLVWriter * const apWriter); + /** * @brief This is set to 'true' by the subscriber to indicate preservation of previous subscriptions. If omitted, it implies * 'false' as a value. @@ -88,7 +94,7 @@ class Builder : public MessageBuilder * * @return A reference to InvokeResponseIBs::Builder */ - InvokeResponseIBs::Builder & CreateInvokeResponses(); + InvokeResponseIBs::Builder & CreateInvokeResponses(const bool aReserveEndBuffer = false); /** * @brief Get reference to InvokeResponseIBs::Builder @@ -120,6 +126,7 @@ class Builder : public MessageBuilder private: InvokeResponseIBs::Builder mInvokeResponses; + bool mIsEndBufferReserved = false; }; } // namespace InvokeResponseMessage } // namespace app diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index c548fcc1354402..7df4f003c8b91b 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -2064,45 +2064,41 @@ void InvokeRequestMessageEndOfMessageReservationTest(nlTestSuite * apSuite, void InvokeRequestMessage::Builder invokeRequestMessageBuilder; const uint32_t kSmallBufferSize = 100; writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); - err = invokeRequestMessageBuilder.Init(&writer); + err = invokeRequestMessageBuilder.InitWithEndBufferReserved(&writer); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t sizeToReserve = invokeRequestMessageBuilder.GetSizeToEndInvokeRequestMessage(); - err = writer.ReserveBuffer(sizeToReserve); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + uint32_t remainingLengthAfterInitWithReservation = writer.GetRemainingFreeLength(); - err = writer.UnreserveBuffer(sizeToReserve); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); err = invokeRequestMessageBuilder.EndOfInvokeRequestMessage(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); uint32_t remainingLengthAfterEndingInvokeRequestMessage = writer.GetRemainingFreeLength(); - NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remainingLengthAfterEndingInvokeRequestMessage); + NL_TEST_ASSERT(apSuite, remainingLengthAfterInitWithReservation == remainingLengthAfterEndingInvokeRequestMessage); } void InvokeRequestsEndOfRequestReservationTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; chip::System::PacketBufferTLVWriter writer; - InvokeRequests::Builder invokeRequestsBuilder; + InvokeRequestMessage::Builder invokeRequestMessageBuilder; const uint32_t kSmallBufferSize = 100; writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); - err = invokeRequestsBuilder.Init(&writer); + err = invokeRequestMessageBuilder.InitWithEndBufferReserved(&writer); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t sizeToReserve = invokeRequestsBuilder.GetSizeToEndInvokeRequests(); - err = writer.ReserveBuffer(sizeToReserve); + invokeRequestMessageBuilder.CreateInvokeRequests(/* aReserveEndBuffer = */ true); + InvokeRequests::Builder & invokeRequestsBuilder = invokeRequestMessageBuilder.GetInvokeRequests(); + err = invokeRequestsBuilder.GetError(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); - err = writer.UnreserveBuffer(sizeToReserve); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + auto * invokeRequestsWriter = invokeRequestsBuilder.GetWriter(); + uint32_t remainingLengthAfterInitWithReservation = invokeRequestsWriter->GetRemainingFreeLength(); + err = invokeRequestsBuilder.EndOfInvokeRequests(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterEndingInvokeRequests = writer.GetRemainingFreeLength(); - NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeRequests); + uint32_t remainingLengthAfterEndingInvokeRequests = invokeRequestsWriter->GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remainingLengthAfterInitWithReservation == remainingLengthAfterEndingInvokeRequests); } void InvokeInvokeResponseMessageTest(nlTestSuite * apSuite, void * apContext) @@ -2129,45 +2125,39 @@ void InvokeResponseMessageEndOfMessageReservationTest(nlTestSuite * apSuite, voi InvokeResponseMessage::Builder invokeResponseMessageBuilder; const uint32_t kSmallBufferSize = 100; writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); - err = invokeResponseMessageBuilder.Init(&writer); + err = invokeResponseMessageBuilder.InitWithEndBufferReserved(&writer); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t sizeToReserve = invokeResponseMessageBuilder.GetSizeToEndInvokeResponseMessage(); - err = writer.ReserveBuffer(sizeToReserve); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); - - err = writer.UnreserveBuffer(sizeToReserve); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remainingLengthAfterInitWithReservation = writer.GetRemainingFreeLength(); err = invokeResponseMessageBuilder.EndOfInvokeResponseMessage(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterEndingInvokeResponseMessage = writer.GetRemainingFreeLength(); - NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeResponseMessage); + uint32_t remainingLengthAfterEndingInvokeResponseMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remainingLengthAfterInitWithReservation == remainingLengthAfterEndingInvokeResponseMessage); } void InvokeResponsesEndOfResponseReservationTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; chip::System::PacketBufferTLVWriter writer; - InvokeResponseIBs::Builder invokeResponsesBuilder; + InvokeResponseMessage::Builder invokeResponseMessageBuilder; const uint32_t kSmallBufferSize = 100; writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); - err = invokeResponsesBuilder.Init(&writer); + err = invokeResponseMessageBuilder.InitWithEndBufferReserved(&writer); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t sizeToReserve = invokeResponsesBuilder.GetSizeToEndInvokeResponses(); - err = writer.ReserveBuffer(sizeToReserve); + invokeResponseMessageBuilder.CreateInvokeResponses(/* aReserveEndBuffer = */ true); + InvokeResponseIBs::Builder & invokeResponsesBuilder = invokeResponseMessageBuilder.GetInvokeResponses(); + err = invokeResponsesBuilder.GetError(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); - err = writer.UnreserveBuffer(sizeToReserve); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + auto * invokeResponsesWriter = invokeResponsesBuilder.GetWriter(); + uint32_t remainingLengthAfterInitWithReservation = invokeResponsesWriter->GetRemainingFreeLength(); err = invokeResponsesBuilder.EndOfInvokeResponses(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - uint32_t remianingLengthAfterEndingInvokeResponses = writer.GetRemainingFreeLength(); - NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeResponses); + uint32_t remainingLengthAfterEndingInvokeResponses = invokeResponsesWriter->GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remainingLengthAfterInitWithReservation == remainingLengthAfterEndingInvokeResponses); } void ReportDataMessageTest(nlTestSuite * apSuite, void * apContext) From 3f152afe10f819d47ffb918b010ff49722b2f676 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 3 Jan 2024 15:48:05 +0000 Subject: [PATCH 5/6] Restyled by clang-format --- src/app/tests/TestMessageDef.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 7df4f003c8b91b..4d53ba04cc5446 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -2129,7 +2129,7 @@ void InvokeResponseMessageEndOfMessageReservationTest(nlTestSuite * apSuite, voi NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); uint32_t remainingLengthAfterInitWithReservation = writer.GetRemainingFreeLength(); - err = invokeResponseMessageBuilder.EndOfInvokeResponseMessage(); + err = invokeResponseMessageBuilder.EndOfInvokeResponseMessage(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); uint32_t remainingLengthAfterEndingInvokeResponseMessage = writer.GetRemainingFreeLength(); @@ -2153,7 +2153,7 @@ void InvokeResponsesEndOfResponseReservationTest(nlTestSuite * apSuite, void * a auto * invokeResponsesWriter = invokeResponsesBuilder.GetWriter(); uint32_t remainingLengthAfterInitWithReservation = invokeResponsesWriter->GetRemainingFreeLength(); - err = invokeResponsesBuilder.EndOfInvokeResponses(); + err = invokeResponsesBuilder.EndOfInvokeResponses(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); uint32_t remainingLengthAfterEndingInvokeResponses = invokeResponsesWriter->GetRemainingFreeLength(); From 12b00809b2b841a0982ec6e3cfe050d166e276b9 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 3 Jan 2024 13:23:05 -0500 Subject: [PATCH 6/6] Update src/app/CommandHandler.cpp Co-authored-by: Boris Zbarsky --- src/app/CommandHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 3096982ff3df45..a573fa807b0c39 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -67,7 +67,7 @@ CHIP_ERROR CommandHandler::AllocateBuffer() mInvokeResponseBuilder.SuppressResponse(mSuppressResponse); ReturnErrorOnFailure(mInvokeResponseBuilder.GetError()); - mInvokeResponseBuilder.CreateInvokeResponses(/*aReserveEndBuffer =*/true); + mInvokeResponseBuilder.CreateInvokeResponses(/* aReserveEndBuffer = */ true); ReturnErrorOnFailure(mInvokeResponseBuilder.GetError()); mBufferAllocated = true;