Issue #11760 - [ota-requestor-app] Abort query image download on time…

…out (#15768)

* Issue #11760 - [ota-provider-app] Support for error/timeout midway in transfer

* restyle-diff.sh changes

* Code review changes.

- Moved mTimeout to BDXDownloader class.
- Moved prevPercentageComplete to BDXDownloader class.
- Renamed StartTimeoutTimerHandler to TransferTimeoutCheckHandler.
- Added sanity checks.

* Code review changes.

- Moved mTimeoutSec to a constant define kTimeoutSec.
- Moved resetting of timeout timer and prevPercentageComplete to a Reset() function.

* Code review changes.

- Renamed prevPercentageComplete to mPrevPercentageComplete

* Code review changes.

- Renmed CheckTransferTimeout() to HasTransferTimedOut().
- Calculate timeout using block counter instead of percentage complete.
- Use 5*60 instead of 300 for timeout value for readability.

* Fix merge issues.

src/app/clusters/ota-requestor/BDXDownloader.cpp

@@ -23,6 +23,7 @@
 #include <lib/support/BufferReader.h>
 #include <lib/support/BytesToHex.h>
 #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>
@@ -35,6 +36,50 @@ using chip::bdx::TransferSession;
 
 namespace chip {
 
+void TransferTimeoutCheckHandler(System::Layer * systemLayer, void * appState)
+{
+    VerifyOrReturn(appState != nullptr);
+    BDXDownloader * bdxDownloader = static_cast<BDXDownloader *>(appState);
+
+    if (bdxDownloader->HasTransferTimedOut())
+    {
+        // End download if transfer timeout has been detected
+        bdxDownloader->OnDownloadTimeout();
+    }
+    else
+    {
+        // Else restart the timer
+        systemLayer->StartTimer(bdxDownloader->GetTimeout(), TransferTimeoutCheckHandler, appState);
+    }
+}
+
+System::Clock::Timeout BDXDownloader::GetTimeout()
+{
+    return mTimeout;
+}
+
+void BDXDownloader::Reset()
+{
+    mPrevBlockCounter = 0;
+    DeviceLayer::SystemLayer().StartTimer(mTimeout, TransferTimeoutCheckHandler, this);
+}
+
+bool BDXDownloader::HasTransferTimedOut()
+{
+    uint32_t curBlockCounter = mBdxTransfer.GetNextBlockNum();
+
+    if (curBlockCounter > mPrevBlockCounter)
+    {
+        mPrevBlockCounter = curBlockCounter;
+        return false;
+    }
+    else
+    {
+        ChipLogError(BDX, "BDX transfer timeout");
+        return true;
+    }
+}
+
 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"));
@@ -50,9 +95,11 @@ void BDXDownloader::OnMessageReceived(const chip::PayloadHeader & payloadHeader,
     PollTransferSession();
 }
 
-CHIP_ERROR BDXDownloader::SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData)
+CHIP_ERROR BDXDownloader::SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData,
+                                       System::Clock::Timeout timeout)
 {
-    mState = State::kIdle;
+    mTimeout = timeout;
+    mState   = State::kIdle;
     mBdxTransfer.Reset();
 
     VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE);
@@ -69,6 +116,10 @@ CHIP_ERROR BDXDownloader::BeginPrepareDownload()
 {
     VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE);
+
+    mPrevBlockCounter = 0;
+    DeviceLayer::SystemLayer().StartTimer(mTimeout, TransferTimeoutCheckHandler, this);
+
     ReturnErrorOnFailure(mImageProcessor->PrepareDownload());
 
     SetState(State::kPreparing, OTAChangeReasonEnum::kSuccess);
@@ -90,6 +141,7 @@ CHIP_ERROR BDXDownloader::OnPreparedForDownload(CHIP_ERROR status)
     else
     {
         ChipLogError(BDX, "failed to prepare download: %" CHIP_ERROR_FORMAT, status.Format());
+        Reset();
         mBdxTransfer.Reset();
         SetState(State::kIdle, OTAChangeReasonEnum::kFailure);
     }
@@ -108,6 +160,8 @@ CHIP_ERROR BDXDownloader::FetchNextData()
 
 void BDXDownloader::OnDownloadTimeout()
 {
+    Reset();
+
     if (mState == State::kInProgress)
     {
         ChipLogDetail(BDX, "aborting due to timeout");
@@ -126,6 +180,8 @@ void BDXDownloader::OnDownloadTimeout()
 
 void BDXDownloader::EndDownload(CHIP_ERROR reason)
 {
+    Reset();
+
     if (mState == State::kInProgress)
     {
         bdx::StatusCode status = bdx::StatusCode::kUnknown;
@@ -186,6 +242,8 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu
         ReturnErrorOnFailure(mMsgDelegate->SendMessage(outEvent));
         if (outEvent.msgTypeData.HasMessageType(chip::bdx::MessageType::BlockAckEOF))
         {
+            Reset();
+
             // BDX transfer is not complete until BlockAckEOF has been sent
             SetState(State::kComplete, OTAChangeReasonEnum::kSuccess);
         }
@@ -212,11 +270,13 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu
         break;
     case TransferSession::OutputEventType::kInternalError:
         ChipLogError(BDX, "TransferSession error");
+        Reset();
         mBdxTransfer.Reset();
         ReturnErrorOnFailure(mImageProcessor->Abort());
         break;
     case TransferSession::OutputEventType::kTransferTimeout:
         ChipLogError(BDX, "Transfer timed out");
+        Reset();
         mBdxTransfer.Reset();
         ReturnErrorOnFailure(mImageProcessor->Abort());
         break;

src/app/clusters/ota-requestor/BDXDownloader.h

@@ -63,7 +63,7 @@ class BDXDownloader : public chip::OTADownloader
     void SetStateDelegate(StateDelegate * delegate) { mStateDelegate = delegate; }
 
     // Initialize a BDX transfer session but will not proceed until OnPreparedForDownload() is called.
-    CHIP_ERROR SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData);
+    CHIP_ERROR SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData, System::Clock::Timeout timeout);
 
     // OTADownloader Overrides
     CHIP_ERROR BeginPrepareDownload() override;
@@ -75,14 +75,24 @@ class BDXDownloader : public chip::OTADownloader
     CHIP_ERROR FetchNextData() override;
     // TODO: override SkipData
 
+    System::Clock::Timeout GetTimeout();
+    // If True, there's been a timeout in the transfer as measured by no download progress after 'mTimeout' seconds.
+    // If False, there's been progress in the transfer.
+    bool HasTransferTimedOut();
+
 private:
     void PollTransferSession();
     CHIP_ERROR HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent);
     void SetState(State state, app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum reason);
+    void Reset();
 
     chip::bdx::TransferSession mBdxTransfer;
     MessagingDelegate * mMsgDelegate = nullptr;
     StateDelegate * mStateDelegate   = nullptr;
+    // Timeout value in seconds to abort the download if there's no progress in the transfer session.
+    System::Clock::Timeout mTimeout = System::Clock::kZero;
+    // Tracks the last block counter used during the transfer session as of the previous check.
+    uint32_t mPrevBlockCounter = 0;
 };
 
 } // namespace chip

src/app/clusters/ota-requestor/OTARequestor.cpp

@@ -44,6 +44,9 @@ using bdx::TransferSession;
 // Global instance of the OTARequestorInterface.
 OTARequestorInterface * globalOTARequestorInstance = nullptr;
 
+// Abort the QueryImage download request if there's been no progress for 5 minutes
+static constexpr System::Clock::Timeout kDownloadTimeoutSec = chip::System::Clock::Seconds32(5 * 60);
+
 static void LogQueryImageResponse(const QueryImageResponse::DecodableType & response)
 {
     ChipLogDetail(SoftwareUpdate, "QueryImageResponse:");
@@ -701,7 +704,7 @@ CHIP_ERROR OTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
     mBdxDownloader->SetMessageDelegate(&mBdxMessenger);
     mBdxDownloader->SetStateDelegate(this);
 
-    ReturnErrorOnFailure(mBdxDownloader->SetBDXParams(initOptions));
+    ReturnErrorOnFailure(mBdxDownloader->SetBDXParams(initOptions, kDownloadTimeoutSec));
     return mBdxDownloader->BeginPrepareDownload();
 }
 

src/protocols/bdx/BdxTransferSession.h

@@ -297,6 +297,7 @@ class DLL_EXPORT TransferSession
     uint64_t GetStartOffset() const { return mStartOffset; }
     uint64_t GetTransferLength() const { return mTransferLength; }
     uint16_t GetTransferBlockSize() const { return mTransferMaxBlockSize; }
+    uint32_t GetNextBlockNum() { return mNextBlockNum; }
     size_t GetNumBytesProcessed() const { return mNumBytesProcessed; }
     const uint8_t * GetFileDesignator(uint16_t & fileDesignatorLen) const
     {