Skip to content

Commit

Permalink
[OTA] Set only one timer when querying a non-existent provider (#16197)
Browse files Browse the repository at this point in the history
  • Loading branch information
carol-apple authored Mar 14, 2022
1 parent dd784cb commit b18b03a
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 30 deletions.
26 changes: 11 additions & 15 deletions src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
11 changes: 4 additions & 7 deletions src/app/clusters/ota-requestor/OTARequestor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions src/app/clusters/ota-requestor/OTARequestorDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit b18b03a

Please sign in to comment.