From b18b03abec3be3707cd81bc8089a0452f9de1754 Mon Sep 17 00:00:00 2001 From: Carol Yang Date: Mon, 14 Mar 2022 15:46:25 -0700 Subject: [PATCH] [OTA] Set only one timer when querying a non-existent provider (#16197) --- .../GenericOTARequestorDriver.cpp | 26 ++++++++----------- .../ota-requestor/GenericOTARequestorDriver.h | 4 +-- .../clusters/ota-requestor/OTARequestor.cpp | 11 +++----- .../ota-requestor/OTARequestorDriver.h | 9 +++---- 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp index 566eca59387e1c..f278dfb1ab40e2 100644 --- 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 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 lastUsedProvider; mRequestor->GetProviderLocation(lastUsedProvider); diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h index a10f991b528bdf..5fe392fd4e6516 100644 --- 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 index 0c817043d3abae..29b8efc03a3234 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.cpp +++ b/src/app/clusters/ota-requestor/OTARequestor.cpp @@ -170,7 +170,7 @@ 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))); } @@ -178,12 +178,12 @@ void OTARequestor::OnQueryImageResponse(void * context, const QueryImageResponse } 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 index cb6bdb6994410f..40c8af5a37d399 100644 --- 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