From 95159568c3cccc1ff7e76ead3ec739b0d7d03b8a Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Mon, 13 Dec 2021 21:21:50 +0530 Subject: [PATCH] [bdx] Added BlockQueryWithSkip message and BlockCounter field to BlockData (#12767) * BDX: Added BlockCount field to BlockData * BDX: Added BlockQueryWithSkip message * Remove unnecessary doc-comments --- src/protocols/bdx/BdxMessages.cpp | 45 +++++++++++ src/protocols/bdx/BdxMessages.h | 54 ++++++------- src/protocols/bdx/BdxTransferSession.cpp | 80 +++++++++++++++++-- src/protocols/bdx/BdxTransferSession.h | 28 ++++++- src/protocols/bdx/tests/TestBdxMessages.cpp | 11 +++ .../bdx/tests/TestBdxTransferSession.cpp | 13 +-- 6 files changed, 188 insertions(+), 43 deletions(-) diff --git a/src/protocols/bdx/BdxMessages.cpp b/src/protocols/bdx/BdxMessages.cpp index 32e5db4b4534d0..3ac30d50912998 100644 --- a/src/protocols/bdx/BdxMessages.cpp +++ b/src/protocols/bdx/BdxMessages.cpp @@ -575,3 +575,48 @@ bool DataBlock::operator==(const DataBlock & another) const return ((BlockCounter == another.BlockCounter) && dataMatches); } + +// WARNING: this function should never return early, since MessageSize() relies on it to calculate +// the size of the message (even if the message is incomplete or filled out incorrectly). +Encoding::LittleEndian::BufferWriter & BlockQueryWithSkip::WriteToBuffer(Encoding::LittleEndian::BufferWriter & aBuffer) const +{ + aBuffer.Put32(BlockCounter); + aBuffer.Put64(BytesToSkip); + return aBuffer; +} + +CHIP_ERROR BlockQueryWithSkip::Parse(System::PacketBufferHandle aBuffer) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + uint8_t * bufStart = aBuffer->Start(); + Reader bufReader(bufStart, aBuffer->DataLength()); + SuccessOrExit(bufReader.Read32(&BlockCounter).StatusCode()); + SuccessOrExit(bufReader.Read64(&BytesToSkip).StatusCode()); + +exit: + if (bufReader.StatusCode() != CHIP_NO_ERROR) + { + err = bufReader.StatusCode(); + } + return err; +} + +size_t BlockQueryWithSkip::MessageSize() const +{ + BufferWriter emptyBuf(nullptr, 0); + return WriteToBuffer(emptyBuf).Needed(); +} + +bool BlockQueryWithSkip::operator==(const BlockQueryWithSkip & another) const +{ + return (BlockCounter == another.BlockCounter && BytesToSkip == another.BytesToSkip); +} + +#if CHIP_AUTOMATION_LOGGING +void BlockQueryWithSkip::LogMessage(bdx::MessageType messageType) const +{ + ChipLogAutomation("BlockQueryWithSkip"); + ChipLogAutomation(" Block Counter: %" PRIu32, BlockCounter); + ChipLogAutomation(" Bytes To Skip: %" PRIu64, BytesToSkip); +} +#endif // CHIP_AUTOMATION_LOGGING diff --git a/src/protocols/bdx/BdxMessages.h b/src/protocols/bdx/BdxMessages.h index 880008b6956f24..9ac493d6ab5711 100644 --- a/src/protocols/bdx/BdxMessages.h +++ b/src/protocols/bdx/BdxMessages.h @@ -34,15 +34,16 @@ namespace bdx { enum class MessageType : uint8_t { - SendInit = 0x01, - SendAccept = 0x02, - ReceiveInit = 0x04, - ReceiveAccept = 0x05, - BlockQuery = 0x10, - Block = 0x11, - BlockEOF = 0x12, - BlockAck = 0x13, - BlockAckEOF = 0x14, + SendInit = 0x01, + SendAccept = 0x02, + ReceiveInit = 0x04, + ReceiveAccept = 0x05, + BlockQuery = 0x10, + Block = 0x11, + BlockEOF = 0x12, + BlockAck = 0x13, + BlockAckEOF = 0x14, + BlockQueryWithSkip = 0x15, }; enum class StatusCode : uint16_t @@ -137,10 +138,6 @@ struct BdxMessage */ struct TransferInit : public BdxMessage { - /** - * @brief - * Equality check method. - */ bool operator==(const TransferInit &) const; // Proposed Transfer Control (required) @@ -182,10 +179,6 @@ using ReceiveInit = TransferInit; */ struct SendAccept : public BdxMessage { - /** - * @brief - * Equality check method. - */ bool operator==(const SendAccept &) const; // Transfer Control (required, only one should be set) @@ -216,10 +209,6 @@ struct SendAccept : public BdxMessage */ struct ReceiveAccept : public BdxMessage { - /** - * @brief - * Equality check method. - */ bool operator==(const ReceiveAccept &) const; // Transfer Control (required, only one should be set) @@ -257,10 +246,6 @@ struct ReceiveAccept : public BdxMessage */ struct CounterMessage : public BdxMessage { - /** - * @brief - * Equality check method. - */ bool operator==(const CounterMessage &) const; uint32_t BlockCounter = 0; @@ -282,10 +267,6 @@ using BlockAckEOF = CounterMessage; */ struct DataBlock : public BdxMessage { - /** - * @brief - * Equality check method. - */ bool operator==(const DataBlock &) const; uint32_t BlockCounter = 0; @@ -309,6 +290,21 @@ struct DataBlock : public BdxMessage using Block = DataBlock; using BlockEOF = DataBlock; +struct BlockQueryWithSkip : public BdxMessage +{ + bool operator==(const BlockQueryWithSkip &) const; + + uint32_t BlockCounter = 0; + uint64_t BytesToSkip = 0; + + CHIP_ERROR Parse(System::PacketBufferHandle aBuffer) override; + Encoding::LittleEndian::BufferWriter & WriteToBuffer(Encoding::LittleEndian::BufferWriter & aBuffer) const override; + size_t MessageSize() const override; +#if CHIP_AUTOMATION_LOGGING + void LogMessage(bdx::MessageType messageType) const override; +#endif // CHIP_AUTOMATION_LOGGING +}; + } // namespace bdx namespace Protocols { diff --git a/src/protocols/bdx/BdxTransferSession.cpp b/src/protocols/bdx/BdxTransferSession.cpp index 96aa2d205b71d9..33665bb9a740c0 100644 --- a/src/protocols/bdx/BdxTransferSession.cpp +++ b/src/protocols/bdx/BdxTransferSession.cpp @@ -106,6 +106,9 @@ void TransferSession::PollOutput(OutputEvent & event, System::Clock::Timestamp c case OutputEventType::kQueryReceived: event = OutputEvent(OutputEventType::kQueryReceived); break; + case OutputEventType::kQueryWithSkipReceived: + event = OutputEvent::QueryWithSkipEvent(mBytesToSkip); + break; case OutputEventType::kBlockReceived: event = OutputEvent::BlockDataEvent(mBlockEventData, std::move(mPendingMsgHandle)); break; @@ -285,6 +288,34 @@ CHIP_ERROR TransferSession::PrepareBlockQuery() return CHIP_NO_ERROR; } +CHIP_ERROR TransferSession::PrepareBlockQueryWithSkip(const uint64_t & bytesToSkip) +{ + const MessageType msgType = MessageType::BlockQueryWithSkip; + + VerifyOrReturnError(mState == TransferState::kTransferInProgress, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mRole == TransferRole::kReceiver, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mPendingOutput == OutputEventType::kNone, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); + + BlockQueryWithSkip queryMsg; + queryMsg.BlockCounter = mNextQueryNum; + queryMsg.BytesToSkip = bytesToSkip; + + ReturnErrorOnFailure(WriteToPacketBuffer(queryMsg, mPendingMsgHandle)); + +#if CHIP_AUTOMATION_LOGGING + ChipLogAutomation("Sending BDX Message"); + queryMsg.LogMessage(msgType); +#endif // CHIP_AUTOMATION_LOGGING + + mAwaitingResponse = true; + mLastQueryNum = mNextQueryNum++; + + PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); + + return CHIP_NO_ERROR; +} + CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) { VerifyOrReturnError(mState == TransferState::kTransferInProgress, CHIP_ERROR_INCORRECT_STATE); @@ -447,6 +478,9 @@ CHIP_ERROR TransferSession::HandleBdxMessage(const PayloadHeader & header, Syste case MessageType::BlockQuery: HandleBlockQuery(std::move(msg)); break; + case MessageType::BlockQueryWithSkip: + HandleBlockQueryWithSkip(std::move(msg)); + break; case MessageType::Block: HandleBlock(std::move(msg)); break; @@ -628,6 +662,29 @@ void TransferSession::HandleBlockQuery(System::PacketBufferHandle msgData) #endif // CHIP_AUTOMATION_LOGGING } +void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgData) +{ + VerifyOrReturn(mRole == TransferRole::kSender, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); + + BlockQueryWithSkip query; + const CHIP_ERROR err = query.Parse(std::move(msgData)); + VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); + + VerifyOrReturn(query.BlockCounter == mNextBlockNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); + + mPendingOutput = OutputEventType::kQueryWithSkipReceived; + + mAwaitingResponse = false; + mLastQueryNum = query.BlockCounter; + mBytesToSkip.BytesToSkip = query.BytesToSkip; + +#if CHIP_AUTOMATION_LOGGING + query.LogMessage(MessageType::BlockQueryWithSkip); +#endif // CHIP_AUTOMATION_LOGGING +} + void TransferSession::HandleBlock(System::PacketBufferHandle msgData) { VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); @@ -648,9 +705,10 @@ void TransferSession::HandleBlock(System::PacketBufferHandle msgData) PrepareStatusReport(StatusCode::kLengthMismatch)); } - mBlockEventData.Data = blockMsg.Data; - mBlockEventData.Length = blockMsg.DataLength; - mBlockEventData.IsEof = false; + mBlockEventData.Data = blockMsg.Data; + mBlockEventData.Length = blockMsg.DataLength; + mBlockEventData.IsEof = false; + mBlockEventData.BlockCounter = blockMsg.BlockCounter; mPendingMsgHandle = std::move(msgData); mPendingOutput = OutputEventType::kBlockReceived; @@ -678,9 +736,10 @@ void TransferSession::HandleBlockEOF(System::PacketBufferHandle msgData) VerifyOrReturn(blockEOFMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); VerifyOrReturn(blockEOFMsg.DataLength <= mTransferMaxBlockSize, PrepareStatusReport(StatusCode::kBadMessageContents)); - mBlockEventData.Data = blockEOFMsg.Data; - mBlockEventData.Length = blockEOFMsg.DataLength; - mBlockEventData.IsEof = true; + mBlockEventData.Data = blockEOFMsg.Data; + mBlockEventData.Length = blockEOFMsg.DataLength; + mBlockEventData.IsEof = true; + mBlockEventData.BlockCounter = blockEOFMsg.BlockCounter; mPendingMsgHandle = std::move(msgData); mPendingOutput = OutputEventType::kBlockReceived; @@ -856,6 +915,8 @@ const char * TransferSession::OutputEvent::ToString(OutputEventType outputEventT return "BlockReceived"; case OutputEventType::kQueryReceived: return "QueryReceived"; + case OutputEventType::kQueryWithSkipReceived: + return "QueryWithSkipReceived"; case OutputEventType::kAckReceived: return "AckReceived"; case OutputEventType::kAckEOFReceived: @@ -928,5 +989,12 @@ TransferSession::OutputEvent TransferSession::OutputEvent::MsgToSendEvent(Messag return event; } +TransferSession::OutputEvent TransferSession::OutputEvent::QueryWithSkipEvent(TransferSkipData bytesToSkip) +{ + OutputEvent event(OutputEventType::kQueryWithSkipReceived); + event.bytesToSkip = bytesToSkip; + return event; +} + } // namespace bdx } // namespace chip diff --git a/src/protocols/bdx/BdxTransferSession.h b/src/protocols/bdx/BdxTransferSession.h index 35882b05a15fe9..de94c67fbd6a12 100644 --- a/src/protocols/bdx/BdxTransferSession.h +++ b/src/protocols/bdx/BdxTransferSession.h @@ -34,6 +34,7 @@ class DLL_EXPORT TransferSession kAcceptReceived, kBlockReceived, kQueryReceived, + kQueryWithSkipReceived, kAckReceived, kAckEOFReceived, kStatusReceived, @@ -77,9 +78,10 @@ class DLL_EXPORT TransferSession struct BlockData { - const uint8_t * Data = nullptr; - size_t Length = 0; - bool IsEof = false; + const uint8_t * Data = nullptr; + size_t Length = 0; + bool IsEof = false; + uint32_t BlockCounter = 0; }; struct MessageTypeData @@ -98,6 +100,11 @@ class DLL_EXPORT TransferSession } }; + struct TransferSkipData + { + uint64_t BytesToSkip = 0; + }; + /** * @brief * All output data processed by the TransferSession object will be passed to the caller using this struct via PollOutput(). @@ -120,6 +127,7 @@ class DLL_EXPORT TransferSession BlockData blockdata; StatusReportData statusData; MessageTypeData msgTypeData; + TransferSkipData bytesToSkip; }; OutputEvent() : EventType(OutputEventType::kNone) { statusData = { StatusCode::kNone }; } @@ -133,6 +141,7 @@ class DLL_EXPORT TransferSession static OutputEvent BlockDataEvent(BlockData data, System::PacketBufferHandle msg); static OutputEvent StatusReportEvent(OutputEventType type, StatusReportData data); static OutputEvent MsgToSendEvent(MessageTypeData typeData, System::PacketBufferHandle msg); + static OutputEvent QueryWithSkipEvent(TransferSkipData bytesToSkip); }; /** @@ -220,6 +229,17 @@ class DLL_EXPORT TransferSession */ CHIP_ERROR PrepareBlockQuery(); + /** + * @brief + * Prepare a BlockQueryWithSkip message. The Block counter will be populated automatically. + * + * @param bytesToSkip Number of bytes to seek skip + * + * @return CHIP_ERROR The result of the preparation of a BlockQueryWithSkip message. May also indicate if the TransferSession + * object is unable to handle this request. + */ + CHIP_ERROR PrepareBlockQueryWithSkip(const uint64_t & bytesToSkip); + /** * @brief * Prepare a Block message. The Block counter will be populated automatically. @@ -302,6 +322,7 @@ class DLL_EXPORT TransferSession void HandleReceiveAccept(System::PacketBufferHandle msgData); void HandleSendAccept(System::PacketBufferHandle msgData); void HandleBlockQuery(System::PacketBufferHandle msgData); + void HandleBlockQueryWithSkip(System::PacketBufferHandle msgData); void HandleBlock(System::PacketBufferHandle msgData); void HandleBlockEOF(System::PacketBufferHandle msgData); void HandleBlockAck(System::PacketBufferHandle msgData); @@ -345,6 +366,7 @@ class DLL_EXPORT TransferSession TransferAcceptData mTransferAcceptData; BlockData mBlockEventData; MessageTypeData mMsgTypeData; + TransferSkipData mBytesToSkip; size_t mNumBytesProcessed = 0; diff --git a/src/protocols/bdx/tests/TestBdxMessages.cpp b/src/protocols/bdx/tests/TestBdxMessages.cpp index 1ae246c8b5c2db..1a6d2c4643fb2e 100644 --- a/src/protocols/bdx/tests/TestBdxMessages.cpp +++ b/src/protocols/bdx/tests/TestBdxMessages.cpp @@ -119,6 +119,16 @@ void TestDataBlockMessage(nlTestSuite * inSuite, void * inContext) TestHelperWrittenAndParsedMatch(inSuite, inContext, testMsg); } +void TestBlockQueryWithSkipMessage(nlTestSuite * inSuite, void * inContext) +{ + BlockQueryWithSkip testMsg; + + testMsg.BlockCounter = 5; + testMsg.BytesToSkip = 16; + + TestHelperWrittenAndParsedMatch(inSuite, inContext, testMsg); +} + // Test Suite /** @@ -132,6 +142,7 @@ static const nlTest sTests[] = NL_TEST_DEF("TestReceiveAcceptMessage", TestReceiveAcceptMessage), NL_TEST_DEF("TestCounterMessage", TestCounterMessage), NL_TEST_DEF("TestDataBlockMessage", TestDataBlockMessage), + NL_TEST_DEF("TestBlockQueryWithSkipMessage", TestBlockQueryWithSkipMessage), NL_TEST_SENTINEL() }; diff --git a/src/protocols/bdx/tests/TestBdxTransferSession.cpp b/src/protocols/bdx/tests/TestBdxTransferSession.cpp index 165b9e0a9c45df..f18487e2373af2 100644 --- a/src/protocols/bdx/tests/TestBdxTransferSession.cpp +++ b/src/protocols/bdx/tests/TestBdxTransferSession.cpp @@ -278,7 +278,7 @@ void SendAndVerifyQuery(nlTestSuite * inSuite, void * inContext, TransferSession // Helper method for preparing a sending a Block message between two TransferSession objects. The sender refers to the node that is // sending Blocks. Uses a static counter incremented with each call. Also verifies that block data received matches what was sent. void SendAndVerifyArbitraryBlock(nlTestSuite * inSuite, void * inContext, TransferSession & sender, TransferSession & receiver, - TransferSession::OutputEvent & outEvent, bool isEof) + TransferSession::OutputEvent & outEvent, bool isEof, uint32_t inBlockCounter) { CHIP_ERROR err = CHIP_NO_ERROR; static uint8_t dataCount = 0; @@ -319,6 +319,7 @@ void SendAndVerifyArbitraryBlock(nlTestSuite * inSuite, void * inContext, Transf if (outEvent.EventType == TransferSession::OutputEventType::kBlockReceived && outEvent.blockdata.Data != nullptr) { NL_TEST_ASSERT(inSuite, !memcmp(fakeBlockData, outEvent.blockdata.Data, outEvent.blockdata.Length)); + NL_TEST_ASSERT(inSuite, outEvent.blockdata.BlockCounter == inBlockCounter); } VerifyNoMoreOutput(inSuite, inContext, receiver); } @@ -414,7 +415,7 @@ void TestInitiatingReceiverReceiverDrive(nlTestSuite * inSuite, void * inContext // Test BlockQuery -> Block -> BlockAck SendAndVerifyQuery(inSuite, inContext, respondingSender, initiatingReceiver, outEvent); - SendAndVerifyArbitraryBlock(inSuite, inContext, respondingSender, initiatingReceiver, outEvent, false); + SendAndVerifyArbitraryBlock(inSuite, inContext, respondingSender, initiatingReceiver, outEvent, false, numBlocksSent); numBlocksSent++; // Test only one block can be prepared at a time, without receiving a response to the first @@ -441,7 +442,7 @@ void TestInitiatingReceiverReceiverDrive(nlTestSuite * inSuite, void * inContext bool isEof = (numBlocksSent == numBlockSends - 1); SendAndVerifyQuery(inSuite, inContext, respondingSender, initiatingReceiver, outEvent); - SendAndVerifyArbitraryBlock(inSuite, inContext, respondingSender, initiatingReceiver, outEvent, isEof); + SendAndVerifyArbitraryBlock(inSuite, inContext, respondingSender, initiatingReceiver, outEvent, isEof, numBlocksSent); numBlocksSent++; } @@ -509,14 +510,16 @@ void TestInitiatingSenderSenderDrive(nlTestSuite * inSuite, void * inContext) SendAndVerifyAcceptMsg(inSuite, inContext, outEvent, respondingReceiver, TransferRole::kReceiver, acceptData, initiatingSender, initOptions); + uint32_t numBlocksSent = 0; // Test multiple Block -> BlockAck -> Block for (int i = 0; i < 3; i++) { - SendAndVerifyArbitraryBlock(inSuite, inContext, initiatingSender, respondingReceiver, outEvent, false); + SendAndVerifyArbitraryBlock(inSuite, inContext, initiatingSender, respondingReceiver, outEvent, false, numBlocksSent); SendAndVerifyBlockAck(inSuite, inContext, initiatingSender, respondingReceiver, outEvent, false); + numBlocksSent++; } - SendAndVerifyArbitraryBlock(inSuite, inContext, initiatingSender, respondingReceiver, outEvent, true); + SendAndVerifyArbitraryBlock(inSuite, inContext, initiatingSender, respondingReceiver, outEvent, true, numBlocksSent); SendAndVerifyBlockAck(inSuite, inContext, initiatingSender, respondingReceiver, outEvent, true); }