Skip to content

Commit

Permalink
Revert PR 16743 (#17427)
Browse files Browse the repository at this point in the history 
* Revert "Issue #12520 Remove BDX timeout tracking from TransferSession"

This reverts commit b3528c5.

* Revert "- Removed timeout from TransferSession::StartTransfer() and TransferSession::WaitForTransfer()"

This reverts commit 7a4061a.
  • Loading branch information
@isiu-apple @pull
isiu-apple authored and pull[bot] committed Sep 28, 2023
1 parent 4ec43e3 commit 1546580
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 39 deletions.
9 changes: 6 additions & 3 deletions src/app/clusters/ota-requestor/BDXDownloader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#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>

Expand Down Expand Up @@ -80,7 +81,8 @@ 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));
CHIP_ERROR err =
mBdxTransfer.HandleMessageReceived(payloadHeader, std::move(msg), /* TODO:(#12520) */ chip::System::Clock::Seconds16(0));
if (err != CHIP_NO_ERROR)
{
ChipLogError(BDX, "unable to handle message: %" CHIP_ERROR_FORMAT, err.Format());
Expand All @@ -102,7 +104,8 @@ CHIP_ERROR BDXDownloader::SetBDXParams(const chip::bdx::TransferSession::Transfe

// Must call StartTransfer() here to store the the pointer data contained in bdxInitData in the TransferSession object.
// Otherwise it could be freed before we can use it.
ReturnErrorOnFailure(mBdxTransfer.StartTransfer(chip::bdx::TransferRole::kReceiver, bdxInitData));
ReturnErrorOnFailure(mBdxTransfer.StartTransfer(chip::bdx::TransferRole::kReceiver, bdxInitData,
/* TODO:(#12520) */ chip::System::Clock::Seconds16(30)));

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -217,7 +220,7 @@ void BDXDownloader::PollTransferSession()
// allow that?
do
{
mBdxTransfer.PollOutput(outEvent);
mBdxTransfer.PollOutput(outEvent, /* TODO:(#12520) */ chip::System::Clock::Seconds16(0));
CHIP_ERROR err = HandleBdxEvent(outEvent);
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "HandleBDXEvent: %" CHIP_ERROR_FORMAT, err.Format()));
} while (outEvent.EventType != TransferSession::OutputEventType::kNone);
Expand Down
37 changes: 30 additions & 7 deletions src/protocols/bdx/BdxTransferSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,24 @@ TransferSession::TransferSession()
mSuppportedXferOpts.ClearAll();
}

void TransferSession::PollOutput(OutputEvent & event)
void TransferSession::PollOutput(OutputEvent & event, System::Clock::Timestamp curTime)
{
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:
Expand All @@ -80,7 +94,8 @@ void TransferSession::PollOutput(OutputEvent & event)
event = OutputEvent::StatusReportEvent(OutputEventType::kStatusReceived, mStatusReportData);
break;
case OutputEventType::kMsgToSend:
event = OutputEvent::MsgToSendEvent(mMsgTypeData, std::move(mPendingMsgHandle));
event = OutputEvent::MsgToSendEvent(mMsgTypeData, std::move(mPendingMsgHandle));
mTimeoutStartTime = curTime;
break;
case OutputEventType::kInitReceived:
event = OutputEvent::TransferInitEvent(mTransferRequestData, std::move(mPendingMsgHandle));
Expand Down Expand Up @@ -119,11 +134,12 @@ void TransferSession::PollOutput(OutputEvent & event)
mPendingOutput = OutputEventType::kNone;
}

CHIP_ERROR TransferSession::StartTransfer(TransferRole role, const TransferInitData & initData)
CHIP_ERROR TransferSession::StartTransfer(TransferRole role, const TransferInitData & initData, System::Clock::Timeout timeout)
{
VerifyOrReturnError(mState == TransferState::kUnitialized, CHIP_ERROR_INCORRECT_STATE);

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

// Set transfer parameters. They may be overridden later by an Accept message
mSuppportedXferOpts = initData.TransferCtlFlags;
Expand Down Expand Up @@ -161,12 +177,13 @@ CHIP_ERROR TransferSession::StartTransfer(TransferRole role, const TransferInitD
}

CHIP_ERROR TransferSession::WaitForTransfer(TransferRole role, BitFlags<TransferControlFlags> xferControlOpts,
uint16_t maxBlockSize)
uint16_t maxBlockSize, System::Clock::Timeout timeout)
{
VerifyOrReturnError(mState == TransferState::kUnitialized, CHIP_ERROR_INCORRECT_STATE);

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

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

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

CHIP_ERROR TransferSession::HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg)
CHIP_ERROR TransferSession::HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg,
System::Clock::Timestamp curTime)
{
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))
{
Expand Down
20 changes: 15 additions & 5 deletions src/protocols/bdx/BdxTransferSession.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,9 @@ 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);
void PollOutput(OutputEvent & event, System::Clock::Timestamp curTime);

/**
* @brief
Expand All @@ -172,11 +173,13 @@ class DLL_EXPORT TransferSession
* @param role Inidcates whether this object will be sending or receiving data
* @param initData Data for initializing this object and for populating a TransferInit message
* The role parameter will determine whether to populate a ReceiveInit or SendInit
* @param timeout The amount of time to wait for a response before considering the transfer failed
* @param curTime The current time since epoch. Needed to set a start time for the transfer timeout.
*
* @return CHIP_ERROR Result of initialization and preparation of a TransferInit message. May also indicate if the
* TransferSession object is unable to handle this request.
*/
CHIP_ERROR StartTransfer(TransferRole role, const TransferInitData & initData);
CHIP_ERROR StartTransfer(TransferRole role, const TransferInitData & initData, System::Clock::Timeout timeout);

/**
* @brief
Expand All @@ -187,11 +190,13 @@ class DLL_EXPORT TransferSession
* @param role Inidcates whether this object will be sending or receiving data
* @param xferControlOpts Indicates all supported control modes. Used to respond to a TransferInit message
* @param maxBlockSize The max Block size that this object supports.
* @param timeout The amount of time to wait for a response before considering the transfer failed
*
* @return CHIP_ERROR Result of initialization. May also indicate if the TransferSession object is unable to handle this
* request.
*/
CHIP_ERROR WaitForTransfer(TransferRole role, BitFlags<TransferControlFlags> xferControlOpts, uint16_t maxBlockSize);
CHIP_ERROR WaitForTransfer(TransferRole role, BitFlags<TransferControlFlags> xferControlOpts, uint16_t maxBlockSize,
System::Clock::Timeout timeout);

/**
* @brief
Expand Down Expand Up @@ -280,11 +285,13 @@ 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);
CHIP_ERROR HandleMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg,
System::Clock::Timestamp curTime);

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

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

} // namespace bdx
Expand Down
9 changes: 5 additions & 4 deletions src/protocols/bdx/TransferFacilitator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ 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));
CHIP_ERROR err =
mTransfer.HandleMessageReceived(payloadHeader, std::move(payload), System::SystemClock().GetMonotonicTimestamp());
if (err != CHIP_NO_ERROR)
{
ChipLogError(BDX, "failed to handle message: %s", ErrorStr(err));
Expand Down Expand Up @@ -73,7 +74,7 @@ void TransferFacilitator::PollTimerHandler(chip::System::Layer * systemLayer, vo
void TransferFacilitator::PollForOutput()
{
TransferSession::OutputEvent outEvent;
mTransfer.PollOutput(outEvent);
mTransfer.PollOutput(outEvent, System::SystemClock().GetMonotonicTimestamp());
HandleTransferSessionOutput(outEvent);

VerifyOrReturn(mSystemLayer != nullptr, ChipLogError(BDX, "%s mSystemLayer is null", __FUNCTION__));
Expand All @@ -94,7 +95,7 @@ CHIP_ERROR Responder::PrepareForTransfer(System::Layer * layer, TransferRole rol
mPollFreq = pollFreq;
mSystemLayer = layer;

ReturnErrorOnFailure(mTransfer.WaitForTransfer(role, xferControlOpts, maxBlockSize));
ReturnErrorOnFailure(mTransfer.WaitForTransfer(role, xferControlOpts, maxBlockSize, timeout));

mSystemLayer->StartTimer(mPollFreq, PollTimerHandler, this);
return CHIP_NO_ERROR;
Expand All @@ -108,7 +109,7 @@ CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role,
mPollFreq = pollFreq;
mSystemLayer = layer;

ReturnErrorOnFailure(mTransfer.StartTransfer(role, initData));
ReturnErrorOnFailure(mTransfer.StartTransfer(role, initData, timeout));

mSystemLayer->StartTimer(mPollFreq, PollTimerHandler, this);
return CHIP_NO_ERROR;
Expand Down
Loading

0 comments on commit 1546580

Please sign in to comment.