Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert PR 16743 #17427

Merged
merged 2 commits into from
Apr 18, 2022
Merged

Revert PR 16743 #17427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3541d4a
Revert "Issue #12520 Remove BDX timeout tracking from TransferSession"
isiu-apple Apr 15, 2022
3a9868e
Revert "- Removed timeout from TransferSession::StartTransfer() and T…
isiu-apple Apr 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/app/clusters/ota-requestor/BDXDownloader.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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