diff --git a/src/app/clusters/ota-requestor/ExtendedOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/ExtendedOTARequestorDriver.cpp index cce1b2f7bbda10..ca5acb389a67a1 100644 --- a/src/app/clusters/ota-requestor/ExtendedOTARequestorDriver.cpp +++ b/src/app/clusters/ota-requestor/ExtendedOTARequestorDriver.cpp @@ -68,15 +68,16 @@ void ExtendedOTARequestorDriver::PollUserConsentState() CHIP_ERROR ExtendedOTARequestorDriver::GetUserConsentSubject(chip::ota::UserConsentSubject & subject, const UpdateDescription & update) { - if (mLastUsedProvider.HasValue()) + Optional lastUsedProvider; + mRequestor->GetProviderLocation(lastUsedProvider); + if (lastUsedProvider.HasValue()) { - // mLastUsedProvider has the provider fabric index and endpoint id - subject.fabricIndex = mLastUsedProvider.Value().fabricIndex; - subject.providerEndpointId = mLastUsedProvider.Value().endpoint; + subject.fabricIndex = lastUsedProvider.Value().fabricIndex; + subject.providerEndpointId = lastUsedProvider.Value().endpoint; } else { - ChipLogError(SoftwareUpdate, "mLastProvider is empty"); + ChipLogError(SoftwareUpdate, "Last used provider is empty"); return CHIP_ERROR_INTERNAL; } diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp index df842d80ee4678..ebf0f1890b090d 100644 --- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp +++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp @@ -69,7 +69,7 @@ void StartDelayTimerHandler(System::Layer * systemLayer, void * appState) static_cast(appState)->SendQueryImage(); } -bool ProviderLocationsEqual(const ProviderLocation::Type & a, const ProviderLocation::Type & b) +bool GenericOTARequestorDriver::ProviderLocationsEqual(const ProviderLocationType & a, const ProviderLocationType & b) { if ((a.fabricIndex == b.fabricIndex) && (a.providerNodeID == b.providerNodeID) && (a.endpoint == b.endpoint)) { @@ -104,7 +104,7 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst { VerifyOrDie(mRequestor != nullptr); - ProviderLocation::Type providerLocation; + ProviderLocationType providerLocation; bool willTryAnotherQuery = false; switch (reason) @@ -112,28 +112,27 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst case UpdateNotFoundReason::UpToDate: willTryAnotherQuery = false; break; - case UpdateNotFoundReason::Busy: willTryAnotherQuery = true; break; - case UpdateNotFoundReason::ConnectionFailed: - case UpdateNotFoundReason::NotAvailable: + case UpdateNotFoundReason::NotAvailable: { // IMPLEMENTATION CHOICE: // This implementation schedules a query only if a different provider is available + Optional lastUsedProvider; + mRequestor->GetProviderLocation(lastUsedProvider); if ((DetermineProviderLocation(providerLocation) != true) || - (mLastUsedProvider.HasValue() && ProviderLocationsEqual(providerLocation, mLastUsedProvider.Value()))) + (lastUsedProvider.HasValue() && ProviderLocationsEqual(providerLocation, lastUsedProvider.Value()))) { willTryAnotherQuery = false; } else { willTryAnotherQuery = true; + mRequestor->SetCurrentProviderLocation(providerLocation); } - mRequestor->SetCurrentProviderLocation(providerLocation); - mLastUsedProvider.SetValue(providerLocation); break; - + } default: willTryAnotherQuery = false; break; @@ -143,6 +142,7 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst { delay = kDefaultDelayedActionTime; } + if (willTryAnotherQuery == true) { ChipLogProgress(SoftwareUpdate, "UpdateNotFound, scheduling a retry"); @@ -151,7 +151,6 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst else { ChipLogProgress(SoftwareUpdate, "UpdateNotFound, not scheduling further retries"); - mRequestor->ClearCurrentProviderLocation(); } } @@ -259,7 +258,6 @@ void GenericOTARequestorDriver::ProcessAnnounceOTAProviders( // Point the OTARequestor to the announced provider mRequestor->SetCurrentProviderLocation(providerLocation); - mLastUsedProvider.SetValue(providerLocation); ScheduleDelayedAction(System::Clock::Seconds32(secToStart), StartDelayTimerHandler, this); } @@ -283,7 +281,7 @@ void GenericOTARequestorDriver::DefaultProviderTimerHandler(System::Layer * syst ChipLogProgress(SoftwareUpdate, "Default Provider timer handler is invoked"); // Determine which provider to query next - ProviderLocation::Type providerLocation; + ProviderLocationType providerLocation; if (DetermineProviderLocation(providerLocation) != true) { StartDefaultProviderTimer(); @@ -291,7 +289,6 @@ void GenericOTARequestorDriver::DefaultProviderTimerHandler(System::Layer * syst } mRequestor->SetCurrentProviderLocation(providerLocation); - mLastUsedProvider.SetValue(providerLocation); SendQueryImage(); } @@ -301,8 +298,6 @@ void GenericOTARequestorDriver::StartDefaultProviderTimer() ChipLogProgress(SoftwareUpdate, "Starting the Default Provider timer, timeout: %u seconds", (unsigned int) mPeriodicQueryTimeInterval); - DeviceLayer::SystemLayer().ScheduleLambda([this] { mRequestor->ClearCurrentProviderLocation(); }); - ScheduleDelayedAction( System::Clock::Seconds32(mPeriodicQueryTimeInterval), [](System::Layer *, void * context) { @@ -325,14 +320,16 @@ 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. */ - -bool GenericOTARequestorDriver::DetermineProviderLocation(ProviderLocation::Type & providerLocation) +bool GenericOTARequestorDriver::DetermineProviderLocation(ProviderLocationType & providerLocation) { + Optional lastUsedProvider; + mRequestor->GetProviderLocation(lastUsedProvider); + // Iterate through the default providers list and find the last used provider. If found, return the provider after it auto iterator = mRequestor->GetDefaultOTAProviderListIterator(); - while (mLastUsedProvider.HasValue() && iterator.Next()) + while (lastUsedProvider.HasValue() && iterator.Next()) { - if (ProviderLocationsEqual(iterator.GetValue(), mLastUsedProvider.Value())) + if (ProviderLocationsEqual(iterator.GetValue(), lastUsedProvider.Value())) { if (iterator.Next()) { diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h index 3601ec9f2c1a6f..68c3fe39010506 100644 --- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h +++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h @@ -79,13 +79,12 @@ class GenericOTARequestorDriver : public OTARequestorDriver void DefaultProviderTimerHandler(System::Layer * systemLayer, void * appState); void ScheduleDelayedAction(System::Clock::Seconds32 delay, System::TimerCompleteCallback action, void * aAppState); void CancelDelayedAction(System::TimerCompleteCallback action, void * aAppState); + bool ProviderLocationsEqual(const ProviderLocationType & a, const ProviderLocationType & b); OTARequestorInterface * mRequestor = nullptr; OTAImageProcessorInterface * mImageProcessor = nullptr; uint32_t mOtaStartDelaySec = 0; uint32_t mPeriodicQueryTimeInterval = (24 * 60 * 60); // Timeout for querying providers on the default OTA provider list - - Optional mLastUsedProvider; // Provider location used for the last query or update }; } // namespace DeviceLayer diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp index b1b91ff82932ba..4fbc75c0d58742 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.cpp +++ b/src/app/clusters/ota-requestor/OTARequestor.cpp @@ -264,9 +264,9 @@ EmberAfStatus OTARequestor::HandleAnnounceOTAProvider(app::CommandHandler * comm return EMBER_ZCL_STATUS_FAILURE; } - ProviderLocation::Type providerLocation = { .providerNodeID = commandData.providerNodeId, - .endpoint = commandData.endpoint, - .fabricIndex = commandObj->GetAccessingFabricIndex() }; + ProviderLocationType providerLocation = { .providerNodeID = commandData.providerNodeId, + .endpoint = commandData.endpoint, + .fabricIndex = commandObj->GetAccessingFabricIndex() }; ChipLogDetail(SoftwareUpdate, " FabricIndex: %u", providerLocation.fabricIndex); ChipLogDetail(SoftwareUpdate, " ProviderNodeID: 0x" ChipLogFormatX64, ChipLogValueX64(providerLocation.providerNodeID)); @@ -474,7 +474,7 @@ void OTARequestor::TriggerImmediateQueryInternal() // Sends the QueryImage command to the next available Provider OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery() { - ProviderLocation::Type providerLocation; + ProviderLocationType providerLocation; if (mOtaRequestorDriver->DetermineProviderLocation(providerLocation) != true) { ChipLogError(SoftwareUpdate, "No OTA Providers available"); @@ -539,13 +539,13 @@ CHIP_ERROR OTARequestor::ClearDefaultOtaProviderList(FabricIndex fabricIndex) return mStorage->StoreDefaultProviders(mDefaultOtaProviderList); } -CHIP_ERROR OTARequestor::AddDefaultOtaProvider(const ProviderLocation::Type & providerLocation) +CHIP_ERROR OTARequestor::AddDefaultOtaProvider(const ProviderLocationType & providerLocation) { // Look for an entry with the same fabric index indicated auto iterator = mDefaultOtaProviderList.Begin(); while (iterator.Next()) { - ProviderLocation::Type pl = iterator.GetValue(); + ProviderLocationType pl = iterator.GetValue(); if (pl.GetFabricIndex() == providerLocation.GetFabricIndex()) { ChipLogError(SoftwareUpdate, "Default OTA provider entry with fabric %d already exists", pl.GetFabricIndex()); diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h index 537e51d1d7aa76..4c195732c39586 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.h +++ b/src/app/clusters/ota-requestor/OTARequestor.h @@ -84,11 +84,10 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe mProviderLocation.SetValue(providerLocation); } - void ClearCurrentProviderLocation() override { mProviderLocation.ClearValue(); } + void GetProviderLocation(Optional & providerLocation) override { providerLocation = mProviderLocation; } // Add a default OTA provider to the cached list - CHIP_ERROR AddDefaultOtaProvider( - const app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type & providerLocation) override; + CHIP_ERROR AddDefaultOtaProvider(const ProviderLocationType & providerLocation) override; // Retrieve an iterator to the cached default OTA provider list ProviderLocationList::Iterator GetDefaultOTAProviderListIterator(void) override { return mDefaultOtaProviderList.Begin(); } @@ -313,7 +312,7 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe OTAUpdateStateEnum mCurrentUpdateState = OTAUpdateStateEnum::kUnknown; Server * mServer = nullptr; ProviderLocationList mDefaultOtaProviderList; - Optional mProviderLocation; // Provider location used for the current update in progress + Optional mProviderLocation; // Provider location used for the current/last update in progress }; } // namespace chip diff --git a/src/app/clusters/ota-requestor/OTARequestorInterface.h b/src/app/clusters/ota-requestor/OTARequestorInterface.h index 7a946bd38d87c5..6d1bade0494442 100644 --- a/src/app/clusters/ota-requestor/OTARequestorInterface.h +++ b/src/app/clusters/ota-requestor/OTARequestorInterface.h @@ -199,12 +199,12 @@ class OTARequestorInterface // Set the provider location to be used in the next query and OTA update process virtual void SetCurrentProviderLocation(ProviderLocationType providerLocation) = 0; - // Clear the provider location to indicate that no OTA update may be in progress - virtual void ClearCurrentProviderLocation() = 0; + // If there is an OTA update in progress, returns the provider location for the current OTA update, otherwise, returns the + // provider location that was last used + virtual void GetProviderLocation(Optional & providerLocation) = 0; // Add a default OTA provider to the cached list - virtual CHIP_ERROR - AddDefaultOtaProvider(const app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type & providerLocation) = 0; + virtual CHIP_ERROR AddDefaultOtaProvider(const ProviderLocationType & providerLocation) = 0; // Retrieve an iterator to the cached default OTA provider list virtual ProviderLocationList::Iterator GetDefaultOTAProviderListIterator(void) = 0;