Skip to content

Commit

Permalink
Revert changes to BDXTransferSession
Browse files Browse the repository at this point in the history
  • Loading branch information
@nivi-apple
nivi-apple committed Nov 1, 2023
1 parent a5ec507 commit a102200
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 52 deletions.
4 changes: 0 additions & 4 deletions src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,4 @@ class MTRDiagnosticLogsTransferHandler : public chip::bdx::Responder {
std::function<void(bool)> mCallback;

bool mInitialized = false;

bool isBlockEOFSent = false;

uint64_t downloadedBytes = 0;
};
23 changes: 9 additions & 14 deletions src/darwin/Framework/CHIP/MTRDiagnosticLogsTransferHandler.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

ReturnErrorOnFailure(ConfigureState(fabricIndex, nodeId));

BitFlags<bdx::TransferControlFlags> flags(bdx::TransferControlFlags::kReceiverDrive);
BitFlags<bdx::TransferControlFlags> flags(bdx::TransferControlFlags::kSenderDrive);

return Responder::PrepareForTransfer(layer, kBdxRole, flags, kMaxBdxBlockSize, kBdxTimeout);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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:
Expand All @@ -181,7 +182,7 @@
}
break;
case TransferSession::OutputEventType::kStatusReceived:
ChipLogError(BDX, "HandleTransferSessionOutput: Got StatusReport %x", static_cast<uint16_t>(event.statusData.statusCode));
ChipLogError(BDX, "Got StatusReport %x", static_cast<uint16_t>(event.statusData.statusCode));
[[fallthrough]];
case TransferSession::OutputEventType::kInternalError:
case TransferSession::OutputEventType::kTransferTimeout:
Expand All @@ -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:
Expand All @@ -225,6 +219,7 @@

void MTRDiagnosticLogsTransferHandler::AbortTransfer(chip::bdx::StatusCode reason)
{
assertChipStackLockedByCurrentThread();
mTransfer.AbortTransfer(reason);
}

Expand Down
48 changes: 14 additions & 34 deletions src/protocols/bdx/BdxTransferSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void PrepareOutgoingMessageEvent(MessageType messageType, chip::bdx::TransferSes
chip::bdx::TransferSession::MessageTypeData & outputMsgType)
{
static_assert(std::is_same<std::underlying_type_t<decltype(messageType)>, uint8_t>::value, "Cast is not safe");

pendingOutput = chip::bdx::TransferSession::OutputEventType::kMsgToSend;
outputMsgType.ProtocolId = chip::Protocols::MessageTypeTraits<MessageType>::ProtocolId();
outputMsgType.MessageType = static_cast<uint8_t>(messageType);
Expand Down Expand Up @@ -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);
Expand All @@ -350,8 +346,7 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData)
}

mAwaitingResponse = true;

mLastBlockNum = mNextBlockNum++;
mLastBlockNum = mNextBlockNum++;

PrepareOutgoingMessageEvent(msgType, mPendingOutput, mMsgTypeData);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -891,6 +870,7 @@ CHIP_ERROR TransferSession::VerifyProposedMode(const BitFlags<TransferControlFla
void TransferSession::PrepareStatusReport(StatusCode code)
{
mStatusReportData.statusCode = code;

Protocols::SecureChannel::StatusReport report(Protocols::SecureChannel::GeneralStatusCode::kFailure, Protocols::BDX::Id,
to_underlying(code));
size_t msgSize = report.Size();
Expand Down Expand Up @@ -1017,4 +997,4 @@ TransferSession::OutputEvent TransferSession::OutputEvent::QueryWithSkipEvent(Tr
}

} // namespace bdx
} // namespace chip
} // namespace chip

0 comments on commit a102200

Please sign in to comment.