[OTA] Set only one timer when querying a non-existent provider (#16197)

project-chip · Dec 7, 2023 · ad610a2 · ad610a2
1 parent 1e7f6fa
commit ad610a2
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 30 deletions.
diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -116,12 +116,8 @@ void GenericOTARequestorDriver::HandleIdleState(IdleStateReason reason)
         StartDefaultProviderTimer();
         break;
     case IdleStateReason::kInvalidSession:
-        // An invalid session is detected which may be temporary so try to query again
-        ProviderLocationType providerLocation;
-        if (DetermineProviderLocation(providerLocation) == true)
-        {
-            DeviceLayer::SystemLayer().ScheduleLambda([this] { mRequestor->TriggerImmediateQueryInternal(); });
-        }
+        // An invalid session is detected which may be temporary so try to query the same provider again
+        DeviceLayer::SystemLayer().ScheduleLambda([this] { mRequestor->TriggerImmediateQueryInternal(); });
         break;
     }
 }
@@ -145,19 +141,18 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst
 
     switch (reason)
     {
-    case UpdateNotFoundReason::UpToDate:
+    case UpdateNotFoundReason::kUpToDate:
         willTryAnotherQuery = false;
         break;
-    case UpdateNotFoundReason::Busy:
+    case UpdateNotFoundReason::kBusy:
         willTryAnotherQuery = true;
         break;
-    case UpdateNotFoundReason::ConnectionFailed:
-    case UpdateNotFoundReason::NotAvailable: {
+    case UpdateNotFoundReason::kNotAvailable: {
         // IMPLEMENTATION CHOICE:
         // This implementation schedules a query only if a different provider is available
         Optional<ProviderLocationType> lastUsedProvider;
         mRequestor->GetProviderLocation(lastUsedProvider);
-        if ((DetermineProviderLocation(providerLocation) != true) ||
+        if ((GetNextProviderLocation(providerLocation) != true) ||
             (lastUsedProvider.HasValue() && ProviderLocationsEqual(providerLocation, lastUsedProvider.Value())))
         {
             willTryAnotherQuery = false;
@@ -318,7 +313,7 @@ void GenericOTARequestorDriver::DefaultProviderTimerHandler(System::Layer * syst
 
     // Determine which provider to query next
     ProviderLocationType providerLocation;
-    if (DetermineProviderLocation(providerLocation) != true)
+    if (GetNextProviderLocation(providerLocation) != true)
     {
         StartDefaultProviderTimer();
         return;
@@ -353,10 +348,11 @@ void GenericOTARequestorDriver::StopDefaultProviderTimer()
 }
 
 /**
- * Returns the next available Provider location. The algorithm is to simply loop through the list of DefaultOtaProviders and return
- * the next value (based on the last used provider). If no suitable candidate is found, FALSE is returned.
+ * Returns the next available Provider location. The algorithm is to simply loop through the list of DefaultOtaProviders as a
+ * circular list and return the next value (based on the last used provider). If the list of DefaultOtaProviders is empty, FALSE is
+ * returned.
  */
-bool GenericOTARequestorDriver::DetermineProviderLocation(ProviderLocationType & providerLocation)
+bool GenericOTARequestorDriver::GetNextProviderLocation(ProviderLocationType & providerLocation)
 {
     Optional<ProviderLocationType> lastUsedProvider;
     mRequestor->GetProviderLocation(lastUsedProvider);

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
@@ -68,9 +68,7 @@ class GenericOTARequestorDriver : public OTARequestorDriver
     void ProcessAnnounceOTAProviders(const ProviderLocationType & providerLocation,
                                      app::Clusters::OtaSoftwareUpdateRequestor::OTAAnnouncementReason announcementReason) override;
     void SendQueryImage() override;
-
-    // Returns the next available Provider location
-    bool DetermineProviderLocation(ProviderLocationType & providerLocation) override;
+    bool GetNextProviderLocation(ProviderLocationType & providerLocation) override;
 
 protected:
     void StartDefaultProviderTimer();

diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -170,20 +170,20 @@ void OTARequestor::OnQueryImageResponse(void * context, const QueryImageResponse
                           update.softwareVersion, requestorCore->mCurrentVersion);
 
             requestorCore->RecordNewUpdateState(OTAUpdateStateEnum::kIdle, OTAChangeReasonEnum::kSuccess);
-            requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::UpToDate,
+            requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::kUpToDate,
                                                                System::Clock::Seconds32(response.delayedActionTime.ValueOr(0)));
         }
 
         break;
     }
     case OTAQueryStatus::kBusy:
         requestorCore->RecordNewUpdateState(OTAUpdateStateEnum::kDelayedOnQuery, OTAChangeReasonEnum::kDelayByProvider);
-        requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::Busy,
+        requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::kBusy,
                                                            System::Clock::Seconds32(response.delayedActionTime.ValueOr(0)));
         break;
     case OTAQueryStatus::kNotAvailable:
         requestorCore->RecordNewUpdateState(OTAUpdateStateEnum::kIdle, OTAChangeReasonEnum::kSuccess);
-        requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::NotAvailable,
+        requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::kNotAvailable,
                                                            System::Clock::Seconds32(response.delayedActionTime.ValueOr(0)));
         break;
     default:
@@ -459,9 +459,6 @@ void OTARequestor::OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR
     default:
         break;
     }
-
-    // Give driver a chance to schedule another query
-    requestorCore->mOtaRequestorDriver->UpdateNotFound(UpdateNotFoundReason::ConnectionFailed, chip::System::Clock::Seconds32(0));
 }
 
 // Sends the QueryImage command to the Provider currently set in the OTARequestor
@@ -478,7 +475,7 @@ void OTARequestor::TriggerImmediateQueryInternal()
 OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery()
 {
     ProviderLocationType providerLocation;
-    if (mOtaRequestorDriver->DetermineProviderLocation(providerLocation) != true)
+    if (mOtaRequestorDriver->GetNextProviderLocation(providerLocation) != true)
     {
         ChipLogError(SoftwareUpdate, "No OTA Providers available");
         return kNoProviderKnown;

diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -56,10 +56,9 @@ enum class UpdateFailureState
 
 enum class UpdateNotFoundReason
 {
-    Busy,
-    NotAvailable,
-    UpToDate,
-    ConnectionFailed,
+    kBusy,
+    kNotAvailable,
+    kUpToDate,
 };
 
 // The reasons for why the OTA Requestor has entered idle state
@@ -129,7 +128,7 @@ class OTARequestorDriver
     // Driver picks the OTA Provider that should be used for the next query and update. The Provider is picked according to
     // the driver's internal logic such as, for example, traversing the default providers list.
     // Returns true if there is a Provider available for the next query, returns false otherwise.
-    virtual bool DetermineProviderLocation(ProviderLocationType & providerLocation) = 0;
+    virtual bool GetNextProviderLocation(ProviderLocationType & providerLocation) = 0;
 };
 
 } // namespace chip