Skip to content

Commit

Permalink
Reduce logging in BDXTransferSession by removing logging for BlockQue… (
Browse files Browse the repository at this point in the history
project-chip#37412)

* Reduce logging in BDXTransferSession by removing logging for BlockQuery, Block and BlockAck messages as they generate a lot of logs

- The Exchange Manager alreadys logs the BlockQuery, Block and BlockAck messages and that can be used for debugging purposes

- Do not log messages for OutputEvent when the event type is None

- Also reduce additional logging in the darwin code.

* Restyle

* Address review comments

* Fix whitespace restyled issue.

---------

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
nivi-apple and bzbarsky-apple authored Feb 7, 2025
1 parent 93114c7 commit cbe17ed
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ - (instancetype)initWithMTROTAImageTransferHandler:(MTROTAImageTransferHandler *

TransferSession::OutputEventType eventType = event.EventType;

ChipLogError(BDX, "OutputEvent type: %s", event.ToString(eventType));
ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(eventType));

CHIP_ERROR err = CHIP_NO_ERROR;
switch (event.EventType) {
Expand Down
5 changes: 2 additions & 3 deletions src/protocols/bdx/AsyncTransferFacilitator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,6 @@ CHIP_ERROR AsyncResponder::Init(System::Layer * layer, Messaging::ExchangeContex

void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType eventType, CHIP_ERROR status)
{
ChipLogDetail(BDX, "NotifyEventHandled : Event %s Error %" CHIP_ERROR_FORMAT,
TransferSession::OutputEvent::TypeToString(eventType), status.Format());

// If this is the end of the transfer (whether a clean end, or some sort of error condition), ensure
// that we destroy ourselves after unwinding the processing loop in the ProcessOutputEvents API.
// We can ignore the status for these output events because none of them are supposed to result in
Expand All @@ -194,6 +191,8 @@ void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType e
eventType == TransferSession::OutputEventType::kTransferTimeout ||
eventType == TransferSession::OutputEventType::kStatusReceived)
{
ChipLogProgress(BDX, "NotifyEventHandled : Event %s Error %" CHIP_ERROR_FORMAT,
TransferSession::OutputEvent::TypeToString(eventType), status.Format());
mDestroySelfAfterProcessingEvents = true;
}
else if (status != CHIP_NO_ERROR)
Expand Down
57 changes: 17 additions & 40 deletions src/protocols/bdx/BdxTransferSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,6 @@ CHIP_ERROR TransferSession::PrepareBlockQuery()

ReturnErrorOnFailure(WriteToPacketBuffer(queryMsg, mPendingMsgHandle));

#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Sending BDX Message");
queryMsg.LogMessage(msgType);
#endif // CHIP_AUTOMATION_LOGGING

mAwaitingResponse = true;
mLastQueryNum = mNextQueryNum++;

Expand All @@ -319,11 +314,6 @@ CHIP_ERROR TransferSession::PrepareBlockQueryWithSkip(const uint64_t & bytesToSk

ReturnErrorOnFailure(WriteToPacketBuffer(queryMsg, mPendingMsgHandle));

#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Sending BDX Message");
queryMsg.LogMessage(msgType);
#endif // CHIP_AUTOMATION_LOGGING

mAwaitingResponse = true;
mLastQueryNum = mNextQueryNum++;

Expand Down Expand Up @@ -351,13 +341,12 @@ CHIP_ERROR TransferSession::PrepareBlock(const BlockData & inData)

const MessageType msgType = inData.IsEof ? MessageType::BlockEOF : MessageType::Block;

#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Sending BDX Message");
blockMsg.LogMessage(msgType);
#endif // CHIP_AUTOMATION_LOGGING

if (msgType == MessageType::BlockEOF)
{
#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Sending BDX Message");
blockMsg.LogMessage(msgType);
#endif // CHIP_AUTOMATION_LOGGING
mState = TransferState::kAwaitingEOFAck;
}

Expand All @@ -382,11 +371,6 @@ CHIP_ERROR TransferSession::PrepareBlockAck()

ReturnErrorOnFailure(WriteToPacketBuffer(ackMsg, mPendingMsgHandle));

#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Sending BDX Message");
ackMsg.LogMessage(msgType);
#endif // CHIP_AUTOMATION_LOGGING

if (mState == TransferState::kTransferInProgress)
{
if (mControlMode == TransferControlFlags::kSenderDrive)
Expand All @@ -399,6 +383,10 @@ CHIP_ERROR TransferSession::PrepareBlockAck()
}
else if (mState == TransferState::kReceivedEOF)
{
#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Sending BDX Message");
ackMsg.LogMessage(msgType);
#endif // CHIP_AUTOMATION_LOGGING
mState = TransferState::kTransferDone;
mAwaitingResponse = false;
}
Expand Down Expand Up @@ -475,20 +463,25 @@ CHIP_ERROR TransferSession::HandleBdxMessage(const PayloadHeader & header, Syste

const MessageType msgType = static_cast<MessageType>(header.GetMessageType());

#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Handling received BDX Message");
#endif // CHIP_AUTOMATION_LOGGING

switch (msgType)
{
case MessageType::SendInit:
case MessageType::ReceiveInit:
#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Handling received BDX Message");
#endif // CHIP_AUTOMATION_LOGGING
HandleTransferInit(msgType, std::move(msg));
break;
case MessageType::SendAccept:
#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Handling received BDX Message");
#endif // CHIP_AUTOMATION_LOGGING
HandleSendAccept(std::move(msg));
break;
case MessageType::ReceiveAccept:
#if CHIP_AUTOMATION_LOGGING
ChipLogAutomation("Handling received BDX Message");
#endif // CHIP_AUTOMATION_LOGGING
HandleReceiveAccept(std::move(msg));
break;
case MessageType::BlockQuery:
Expand Down Expand Up @@ -672,10 +665,6 @@ void TransferSession::HandleBlockQuery(System::PacketBufferHandle msgData)

mAwaitingResponse = false;
mLastQueryNum = query.BlockCounter;

#if CHIP_AUTOMATION_LOGGING
query.LogMessage(MessageType::BlockQuery);
#endif // CHIP_AUTOMATION_LOGGING
}

void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgData)
Expand All @@ -695,10 +684,6 @@ void TransferSession::HandleBlockQueryWithSkip(System::PacketBufferHandle msgDat
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)
Expand Down Expand Up @@ -733,10 +718,6 @@ void TransferSession::HandleBlock(System::PacketBufferHandle msgData)
mLastBlockNum = blockMsg.BlockCounter;

mAwaitingResponse = false;

#if CHIP_AUTOMATION_LOGGING
blockMsg.LogMessage(MessageType::Block);
#endif // CHIP_AUTOMATION_LOGGING
}

void TransferSession::HandleBlockEOF(System::PacketBufferHandle msgData)
Expand Down Expand Up @@ -787,10 +768,6 @@ void TransferSession::HandleBlockAck(System::PacketBufferHandle msgData)
// In Receiver Drive, the Receiver can send a BlockAck to indicate receipt of the message and reset the timeout.
// In this case, the Sender should wait to receive a BlockQuery next.
mAwaitingResponse = (mControlMode == TransferControlFlags::kReceiverDrive);

#if CHIP_AUTOMATION_LOGGING
ackMsg.LogMessage(MessageType::BlockAck);
#endif // CHIP_AUTOMATION_LOGGING
}

void TransferSession::HandleBlockAckEOF(System::PacketBufferHandle msgData)
Expand Down

0 comments on commit cbe17ed

Please sign in to comment.