Skip to content

Commit

Permalink
Issue project-chip#12520 Remove BDX timeout tracking from TransferSes…
Browse files Browse the repository at this point in the history
…sion
  • Loading branch information
isiu-apple committed Mar 28, 2022
1 parent 7c69523 commit b3528c5
Showing 4 changed files with 12 additions and 44 deletions.
6 changes: 2 additions & 4 deletions src/app/clusters/ota-requestor/BDXDownloader.cpp
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@
#include <lib/support/CodeUtils.h>
#include <platform/CHIPDeviceLayer.h>
#include <protocols/bdx/BdxMessages.h>
#include <system/SystemClock.h> /* TODO:(#12520) remove */
#include <system/SystemPacketBuffer.h>
#include <transport/raw/MessageHeader.h>

@@ -83,8 +82,7 @@ bool BDXDownloader::HasTransferTimedOut()
void BDXDownloader::OnMessageReceived(const chip::PayloadHeader & payloadHeader, chip::System::PacketBufferHandle msg)
{
VerifyOrReturn(mState == State::kInProgress, ChipLogError(BDX, "Can't accept messages, no transfer in progress"));
CHIP_ERROR err =
mBdxTransfer.HandleMessageReceived(payloadHeader, std::move(msg), /* TODO:(#12520) */ chip::System::Clock::Seconds16(0));
CHIP_ERROR err = mBdxTransfer.HandleMessageReceived(payloadHeader, std::move(msg));
if (err != CHIP_NO_ERROR)
{
ChipLogError(BDX, "unable to handle message: %" CHIP_ERROR_FORMAT, err.Format());
@@ -219,7 +217,7 @@ void BDXDownloader::PollTransferSession()
// allow that?
do
{
mBdxTransfer.PollOutput(outEvent, /* TODO:(#12520) */ chip::System::Clock::Seconds16(0));
mBdxTransfer.PollOutput(outEvent);
CHIP_ERROR err = HandleBdxEvent(outEvent);
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "HandleBDXEvent: %" CHIP_ERROR_FORMAT, err.Format()));
} while (outEvent.EventType != TransferSession::OutputEventType::kNone);
33 changes: 5 additions & 28 deletions src/protocols/bdx/BdxTransferSession.cpp
Original file line number Diff line number Diff line change
@@ -64,24 +64,10 @@ TransferSession::TransferSession()
mSuppportedXferOpts.ClearAll();
}

void TransferSession::PollOutput(OutputEvent & event, System::Clock::Timestamp curTime)
void TransferSession::PollOutput(OutputEvent & event)
{
event = OutputEvent(OutputEventType::kNone);

if (mShouldInitTimeoutStart)
{
mTimeoutStartTime = curTime;
mShouldInitTimeoutStart = false;
}

if (mAwaitingResponse && ((curTime - mTimeoutStartTime) >= mTimeout))
{
event = OutputEvent(OutputEventType::kTransferTimeout);
mState = TransferState::kErrorState;
mAwaitingResponse = false;
return;
}

switch (mPendingOutput)
{
case OutputEventType::kNone:
@@ -94,8 +80,7 @@ void TransferSession::PollOutput(OutputEvent & event, System::Clock::Timestamp c
event = OutputEvent::StatusReportEvent(OutputEventType::kStatusReceived, mStatusReportData);
break;
case OutputEventType::kMsgToSend:
event = OutputEvent::MsgToSendEvent(mMsgTypeData, std::move(mPendingMsgHandle));
mTimeoutStartTime = curTime;
event = OutputEvent::MsgToSendEvent(mMsgTypeData, std::move(mPendingMsgHandle));
break;
case OutputEventType::kInitReceived:
event = OutputEvent::TransferInitEvent(mTransferRequestData, std::move(mPendingMsgHandle));
@@ -138,8 +123,7 @@ CHIP_ERROR TransferSession::StartTransfer(TransferRole role, const TransferInitD
{
VerifyOrReturnError(mState == TransferState::kUnitialized, CHIP_ERROR_INCORRECT_STATE);

mRole = role;
mTimeout = timeout;
mRole = role;

// Set transfer parameters. They may be overridden later by an Accept message
mSuppportedXferOpts = initData.TransferCtlFlags;
@@ -183,7 +167,6 @@ CHIP_ERROR TransferSession::WaitForTransfer(TransferRole role, BitFlags<Transfer

// Used to determine compatibility with any future TransferInit parameters
mRole = role;
mTimeout = timeout;
mSuppportedXferOpts = xferControlOpts;
mMaxSupportedBlockSize = maxBlockSize;

@@ -422,22 +405,16 @@ void TransferSession::Reset()
mLastQueryNum = 0;
mNextQueryNum = 0;

mTimeout = System::Clock::kZero;
mTimeoutStartTime = System::Clock::kZero;
mShouldInitTimeoutStart = true;
mAwaitingResponse = false;
mAwaitingResponse = false;
}

CHIP_ERROR TransferSession::HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg,
System::Clock::Timestamp curTime)
CHIP_ERROR TransferSession::HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg)
{
VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);

if (payloadHeader.HasProtocol(Protocols::BDX::Id))
{
ReturnErrorOnFailure(HandleBdxMessage(payloadHeader, std::move(msg)));

mTimeoutStartTime = curTime;
}
else if (payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport))
{
12 changes: 3 additions & 9 deletions src/protocols/bdx/BdxTransferSession.h
Original file line number Diff line number Diff line change
@@ -160,9 +160,8 @@ class DLL_EXPORT TransferSession
* See OutputEventType for all possible output event types.
*
* @param event Reference to an OutputEvent struct that will be filled out with any pending output data
* @param curTime Current time
*/
void PollOutput(OutputEvent & event, System::Clock::Timestamp curTime);
void PollOutput(OutputEvent & event);

/**
* @brief
@@ -285,13 +284,11 @@ class DLL_EXPORT TransferSession
* @param payloadHeader A PayloadHeader containing the Protocol type and Message Type
* @param msg A PacketBufferHandle pointing to the message buffer to process. May be BDX or StatusReport protocol.
* Buffer is expected to start at data (not header).
* @param curTime Current time
*
* @return CHIP_ERROR Indicates any problems in decoding the message, or if the message is not of the BDX or StatusReport
* protocols.
*/
CHIP_ERROR HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg,
System::Clock::Timestamp curTime);
CHIP_ERROR HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg);

TransferControlFlags GetControlMode() const { return mControlMode; }
uint64_t GetStartOffset() const { return mStartOffset; }
@@ -382,10 +379,7 @@ class DLL_EXPORT TransferSession
uint32_t mLastQueryNum = 0;
uint32_t mNextQueryNum = 0;

System::Clock::Timeout mTimeout = System::Clock::kZero;
System::Clock::Timestamp mTimeoutStartTime = System::Clock::kZero;
bool mShouldInitTimeoutStart = true;
bool mAwaitingResponse = false;
bool mAwaitingResponse = false;
};

} // namespace bdx
5 changes: 2 additions & 3 deletions src/protocols/bdx/TransferFacilitator.cpp
Original file line number Diff line number Diff line change
@@ -42,8 +42,7 @@ CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeConte

ChipLogDetail(BDX, "%s: message " ChipLogFormatMessageType " protocol " ChipLogFormatProtocolId, __FUNCTION__,
payloadHeader.GetMessageType(), ChipLogValueProtocolId(payloadHeader.GetProtocolID()));
CHIP_ERROR err =
mTransfer.HandleMessageReceived(payloadHeader, std::move(payload), System::SystemClock().GetMonotonicTimestamp());
CHIP_ERROR err = mTransfer.HandleMessageReceived(payloadHeader, std::move(payload));
if (err != CHIP_NO_ERROR)
{
ChipLogError(BDX, "failed to handle message: %s", ErrorStr(err));
@@ -74,7 +73,7 @@ void TransferFacilitator::PollTimerHandler(chip::System::Layer * systemLayer, vo
void TransferFacilitator::PollForOutput()
{
TransferSession::OutputEvent outEvent;
mTransfer.PollOutput(outEvent, System::SystemClock().GetMonotonicTimestamp());
mTransfer.PollOutput(outEvent);
HandleTransferSessionOutput(outEvent);

VerifyOrReturn(mSystemLayer != nullptr, ChipLogError(BDX, "%s mSystemLayer is null", __FUNCTION__));

0 comments on commit b3528c5

Please sign in to comment.