Code review changes.

- Moved mTimeoutSec to a constant define kTimeoutSec. - Moved resetting of timeout timer and prevPercentageComplete to a Reset() function.
project-chip · Mar 3, 2022 · e771a84 · e771a84
1 parent ec68777
commit e771a84
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 14 deletions.
diff --git a/src/app/clusters/ota-requestor/BDXDownloader.cpp b/src/app/clusters/ota-requestor/BDXDownloader.cpp
@@ -58,6 +58,12 @@ System::Clock::Timeout BDXDownloader::GetTimeout()
     return mTimeout;
 }
 
+void BDXDownloader::Reset()
+{
+    prevPercentageComplete = 0;
+    DeviceLayer::SystemLayer().StartTimer(mTimeout, TransferTimeoutCheckHandler, this);
+}
+
 bool BDXDownloader::CheckTransferTimeout()
 {
     uint8_t curPercentageComplete =
@@ -136,6 +142,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);
     }
@@ -154,8 +161,7 @@ CHIP_ERROR BDXDownloader::FetchNextData()
 
 void BDXDownloader::OnDownloadTimeout()
 {
-    prevPercentageComplete = 0;
-    DeviceLayer::SystemLayer().CancelTimer(TransferTimeoutCheckHandler, this);
+    Reset();
 
     if (mState == State::kInProgress)
     {
@@ -175,8 +181,7 @@ void BDXDownloader::OnDownloadTimeout()
 
 void BDXDownloader::EndDownload(CHIP_ERROR reason)
 {
-    prevPercentageComplete = 0;
-    DeviceLayer::SystemLayer().CancelTimer(TransferTimeoutCheckHandler, this);
+    Reset();
 
     if (mState == State::kInProgress)
     {
@@ -238,8 +243,7 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu
         ReturnErrorOnFailure(mMsgDelegate->SendMessage(outEvent));
         if (outEvent.msgTypeData.HasMessageType(chip::bdx::MessageType::BlockAckEOF))
         {
-            prevPercentageComplete = 0;
-            DeviceLayer::SystemLayer().CancelTimer(TransferTimeoutCheckHandler, this);
+            Reset();
 
             // BDX transfer is not complete until BlockAckEOF has been sent
             SetState(State::kComplete, OTAChangeReasonEnum::kSuccess);
@@ -267,15 +271,13 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu
         break;
     case TransferSession::OutputEventType::kInternalError:
         ChipLogError(BDX, "TransferSession error");
-        prevPercentageComplete = 0;
-        DeviceLayer::SystemLayer().CancelTimer(TransferTimeoutCheckHandler, this);
+        Reset();
         mBdxTransfer.Reset();
         ReturnErrorOnFailure(mImageProcessor->Abort());
         break;
     case TransferSession::OutputEventType::kTransferTimeout:
         ChipLogError(BDX, "Transfer timed out");
-        prevPercentageComplete = 0;
-        DeviceLayer::SystemLayer().CancelTimer(TransferTimeoutCheckHandler, this);
+        Reset();
         mBdxTransfer.Reset();
         ReturnErrorOnFailure(mImageProcessor->Abort());
         break;

diff --git a/src/app/clusters/ota-requestor/BDXDownloader.h b/src/app/clusters/ota-requestor/BDXDownloader.h
@@ -84,6 +84,7 @@ class BDXDownloader : public chip::OTADownloader
     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;

diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/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(300);
+
 static void LogQueryImageResponse(const QueryImageResponse::DecodableType & response)
 {
     ChipLogDetail(SoftwareUpdate, "QueryImageResponse:");
@@ -744,7 +747,7 @@ CHIP_ERROR OTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
     mBdxDownloader->SetMessageDelegate(&mBdxMessenger);
     mBdxDownloader->SetStateDelegate(this);
 
-    ReturnErrorOnFailure(mBdxDownloader->SetBDXParams(initOptions, mTimeoutSec));
+    ReturnErrorOnFailure(mBdxDownloader->SetBDXParams(initOptions, kDownloadTimeoutSec));
     return mBdxDownloader->BeginPrepareDownload();
 }
 

diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h
@@ -301,9 +301,7 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
     char mFileDesignatorBuffer[bdx::kMaxFileDesignatorLen];
     CharSpan mFileDesignator;
     OTAUpdateStateEnum mCurrentUpdateState = OTAUpdateStateEnum::kIdle;
-    System::Clock::Timeout mTimeoutSec =
-        chip::System::Clock::Seconds32(300); // Abort the QueryImage download request if there's been no progress for 5 minutes
-    Server * mServer = nullptr;
+    Server * mServer                       = nullptr;
     chip::Optional<bool> mRequestorCanConsent;
     ProviderLocationList mDefaultOtaProviderList;
     Optional<ProviderLocationType> mProviderLocation; // Provider location used for the current update in progress