From a102200923057ecaee3027ed9d344a83425d0b1c Mon Sep 17 00:00:00 2001 From: Nivedita Sarkar Date: Wed, 1 Nov 2023 14:22:02 -0700 Subject: [PATCH] Revert changes to BDXTransferSession --- .../CHIP/MTRDiagnosticLogsTransferHandler.h | 4 -- .../CHIP/MTRDiagnosticLogsTransferHandler.mm | 23 ++++----- src/protocols/bdx/BdxTransferSession.cpp | 48 ++++++------------- 3 files changed, 23 insertions(+), 52 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h index 6fc3366da3c503..d6c2f0ebe76806 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h @@ -86,8 +86,4 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder { std::function mCallback; bool mInitialized = false; - - bool isBlockEOFSent = false; - - uint64_t downloadedBytes = 0; }; diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm index c3292b45c89b98..e2f8629073ceca 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm @@ -38,7 +38,7 @@ ReturnErrorOnFailure(ConfigureState(fabricIndex, nodeId)); - BitFlags flags(bdx::TransferControlFlags::kReceiverDrive); + BitFlags flags(bdx::TransferControlFlags::kSenderDrive); return Responder::PrepareForTransfer(layer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout); } @@ -85,16 +85,16 @@ CHIP_ERROR MTRDiagnosticLogsTransferHandler::OnTransferSessionEnd(TransferSession::OutputEvent & event) { assertChipStackLockedByCurrentThread(); - VerifyOrReturnError(mFabricIndex.HasValue(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mNodeId.HasValue(), CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR error = CHIP_NO_ERROR; + if (event.EventType == TransferSession::OutputEventType::kTransferTimeout) { error = CHIP_ERROR_TIMEOUT; - } else if (!isBlockEOFSent) { + } else if (event.EventType != TransferSession::OutputEventType::kMsgToSend || !event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { error = CHIP_ERROR_INTERNAL; } + // Notify the MTRDevice via the callback that the BDX transfer has completed with error or success. if (mCallback) { mCallback(error != CHIP_NO_ERROR ? NO : YES); @@ -170,7 +170,8 @@ void MTRDiagnosticLogsTransferHandler::HandleTransferSessionOutput(TransferSession::OutputEvent & event) { assertChipStackLockedByCurrentThread(); - ChipLogError(BDX, "HandleTransferSessionOutput: Got an event %s", event.ToString(event.EventType)); + ChipLogError(BDX, "Got an event %s", event.ToString(event.EventType)); + CHIP_ERROR err = CHIP_NO_ERROR; switch (event.EventType) { case TransferSession::OutputEventType::kInitReceived: @@ -181,7 +182,7 @@ } break; case TransferSession::OutputEventType::kStatusReceived: - ChipLogError(BDX, "HandleTransferSessionOutput: Got StatusReport %x", static_cast(event.statusData.statusCode)); + ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); [[fallthrough]]; case TransferSession::OutputEventType::kInternalError: case TransferSession::OutputEventType::kTransferTimeout: @@ -195,21 +196,14 @@ } break; case TransferSession::OutputEventType::kAckEOFReceived: - // Need to call OnTransferSessionEnd(event). Need to fix this and remove isBlockEOFSent. break; case TransferSession::OutputEventType::kMsgToSend: err = OnMessageToSend(event); if (event.msgTypeData.HasMessageType(MessageType::BlockAckEOF)) { - // TODO: This is a hack for determinin that the Ack EOF is sent before cleaning up. - // Need to fix this. - isBlockEOFSent = true; + err = OnTransferSessionEnd(event); } break; case TransferSession::OutputEventType::kNone: - if (isBlockEOFSent) { - OnTransferSessionEnd(event); - } - break; case TransferSession::OutputEventType::kQueryWithSkipReceived: case TransferSession::OutputEventType::kQueryReceived: case TransferSession::OutputEventType::kAckReceived: @@ -225,6 +219,7 @@ void MTRDiagnosticLogsTransferHandler::AbortTransfer(chip::bdx::StatusCode reason) { + assertChipStackLockedByCurrentThread(); mTransfer.AbortTransfer(reason); } diff --git a/src/protocols/bdx/BdxTransferSession.cpp b/src/protocols/bdx/BdxTransferSession.cpp index b506048e28c21d..a098f915643855 100644 --- a/src/protocols/bdx/BdxTransferSession.cpp +++ b/src/protocols/bdx/BdxTransferSession.cpp @@ -48,6 +48,7 @@ void PrepareOutgoingMessageEvent(MessageType messageType, chip::bdx::TransferSes chip::bdx::TransferSession::MessageTypeData & outputMsgType) { static_assert(std::is_same, uint8_t>::value, "Cast is not safe"); + pendingOutput = chip::bdx::TransferSession::OutputEventType::kMsgToSend; outputMsgType.ProtocolId = chip::Protocols::MessageTypeTraits::ProtocolId(); outputMsgType.MessageType = static_cast(messageType); @@ -320,12 +321,7 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) VerifyOrReturnError(mState == TransferState::kTransferInProgress, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mRole == TransferRole::kSender, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mPendingOutput == OutputEventType::kNone, CHIP_ERROR_INCORRECT_STATE); - bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || - (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); - if (checkAwaitingResponse) - { - VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); - } + VerifyOrReturnError(!mAwaitingResponse, CHIP_ERROR_INCORRECT_STATE); // Verify non-zero data is provided and is no longer than MaxBlockSize (BlockEOF may contain 0 length data) VerifyOrReturnError((inData.Data != nullptr) && (inData.Length <= mTransferMaxBlockSize), CHIP_ERROR_INVALID_ARGUMENT); @@ -350,8 +346,7 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData) } mAwaitingResponse = true; - - mLastBlockNum = mNextBlockNum++; + mLastBlockNum = mNextBlockNum++; PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); @@ -391,7 +386,7 @@ CHIP_ERROR TransferSession::PrepareBlockAck() mState = TransferState::kTransferDone; mAwaitingResponse = false; } - ChipLogError(BDX, "sending block ack %hhu", msgType); + PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData); return CHIP_NO_ERROR; @@ -532,7 +527,6 @@ CHIP_ERROR TransferSession::HandleStatusReportMessage(const PayloadHeader & head void TransferSession::HandleTransferInit(MessageType msgType, System::PacketBufferHandle msgData) { - VerifyOrReturn(mState == TransferState::kAwaitingInitMsg, PrepareStatusReport(StatusCode::kUnexpectedMessage)); if (mRole == TransferRole::kSender) @@ -570,6 +564,7 @@ void TransferSession::HandleTransferInit(MessageType msgType, System::PacketBuff mPendingOutput = OutputEventType::kInitReceived; mState = TransferState::kNegotiateTransferParams; + #if CHIP_AUTOMATION_LOGGING transferInit.LogMessage(msgType); #endif // CHIP_AUTOMATION_LOGGING @@ -692,26 +687,18 @@ void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgDat void TransferSession::HandleBlock(System::PacketBufferHandle msgData) { - VerifyOrReturn(mRole == TransferRole::kReceiver || mRole == TransferRole::kSender, - PrepareStatusReport(StatusCode::kUnexpectedMessage)); + VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || - (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); - if (checkAwaitingResponse) - { - VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - } + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); Block blockMsg; const CHIP_ERROR err = blockMsg.Parse(msgData.Retain()); VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); - if (mControlMode == TransferControlFlags::kReceiverDrive) - { - VerifyOrReturn(blockMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); - } + VerifyOrReturn(blockMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); VerifyOrReturn((blockMsg.DataLength > 0) && (blockMsg.DataLength <= mTransferMaxBlockSize), PrepareStatusReport(StatusCode::kBadMessageContents)); + if (IsTransferLengthDefinite()) { VerifyOrReturn(mNumBytesProcessed + blockMsg.DataLength <= mTransferLength, @@ -740,24 +727,16 @@ void TransferSession::HandleBlockEOF(System::PacketBufferHandle msgData) { VerifyOrReturn(mRole == TransferRole::kReceiver, PrepareStatusReport(StatusCode::kUnexpectedMessage)); VerifyOrReturn(mState == TransferState::kTransferInProgress, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - - bool checkAwaitingResponse = (mRole == TransferRole::kReceiver && mControlMode == TransferControlFlags::kSenderDrive) || - (mRole == TransferRole::kSender && mControlMode == TransferControlFlags::kReceiverDrive); - if (checkAwaitingResponse) - { - VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); - } + VerifyOrReturn(mAwaitingResponse, PrepareStatusReport(StatusCode::kUnexpectedMessage)); BlockEOF blockEOFMsg; const CHIP_ERROR err = blockEOFMsg.Parse(msgData.Retain()); VerifyOrReturn(err == CHIP_NO_ERROR, PrepareStatusReport(StatusCode::kBadMessageContents)); - if (mControlMode == TransferControlFlags::kReceiverDrive) - { - VerifyOrReturn(blockEOFMsg.BlockCounter == mLastQueryNum, PrepareStatusReport(StatusCode::kBadBlockCounter)); - } + 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.BlockCounter = blockEOFMsg.BlockCounter; @@ -891,6 +870,7 @@ CHIP_ERROR TransferSession::VerifyProposedMode(const BitFlags