Skip to content

Commit

Permalink
Code review changes.
Browse files Browse the repository at this point in the history
- Removed StopSelectedtimer()
- Removed extra Reset() call.
- Spelling errors.
  • Loading branch information
isiu-apple committed Mar 23, 2022
1 parent 77c6b12 commit af0aa7f
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 27 deletions.
25 changes: 5 additions & 20 deletions src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void GenericOTARequestorDriver::Init(OTARequestorInterface * requestor, OTAImage
else
{
// Start the first periodic query timer
StartPeriodicQueryTimer();
StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
}
}

Expand Down Expand Up @@ -343,7 +343,7 @@ void GenericOTARequestorDriver::SendQueryImage()

// Default provider timer only runs when there is no ongoing query/update; must stop it now.
// TriggerImmediateQueryInternal() will cause the state to change from kIdle
StopSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
StartSelectedTimer(SelectedTimer::kWatchdogTimer);

mProviderRetryCount++;

Expand Down Expand Up @@ -400,8 +400,9 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
{
case OTAUpdateStateEnum::kIdle:
case OTAUpdateStateEnum::kUnknown:
// OTA Requestor is not stuck in non-idle state. Restart watchdog timer.
StartWatchdogTimer();
// Watchdog timer should not expire in Idle state and periodic timer should already be scheduled.
// Scheduling periodic timer here just in case.
StartPeriodicQueryTimer();
break;
case OTAUpdateStateEnum::kQuerying:
case OTAUpdateStateEnum::kDelayedOnQuery:
Expand All @@ -410,7 +411,6 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
case OTAUpdateStateEnum::kDelayedOnApply:
UpdateDiscontinued();
mRequestor->CancelImageUpdate();
mRequestor->Reset();
StartPeriodicQueryTimer();
break;
case OTAUpdateStateEnum::kRollingBack:
Expand Down Expand Up @@ -457,21 +457,6 @@ void GenericOTARequestorDriver::StartSelectedTimer(SelectedTimer timer)
}
}

void GenericOTARequestorDriver::StopSelectedTimer(SelectedTimer timer)
{
switch (timer)
{
case SelectedTimer::kPeriodicQueryTimer:
StopPeriodicQueryTimer();
StartWatchdogTimer();
break;
case SelectedTimer::kWatchdogTimer:
StopWatchdogTimer();
StartPeriodicQueryTimer();
break;
}
}

/**
* 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
Expand Down
1 change: 0 additions & 1 deletion src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ class GenericOTARequestorDriver : public OTARequestorDriver
void StopWatchdogTimer();
void WatchdogTimerHandler(System::Layer * systemLayer, void * appState);
void StartSelectedTimer(SelectedTimer timer);
void StopSelectedTimer(SelectedTimer timer);
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);
Expand Down
4 changes: 2 additions & 2 deletions src/app/clusters/ota-requestor/OTARequestor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -658,15 +658,15 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
}
OtaRequestorServerOnStateTransition(mCurrentUpdateState, newState, reason, targetSoftwareVersion);

// Issue#16151 tracks re-factoring error and state transitioning handling.
if ((newState == OTAUpdateStateEnum::kIdle) && (mCurrentUpdateState != OTAUpdateStateEnum::kIdle))
{
IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);

// Inform the driver that the OTARequestor has entered the Idle state
mOtaRequestorDriver->HandleIdleState(idleStateReason);
}
else if (((mCurrentUpdateState == OTAUpdateStateEnum::kIdle) || (mCurrentUpdateState == OTAUpdateStateEnum::kUnknown)) &&
(newState != OTAUpdateStateEnum::kIdle))
else if ((mCurrentUpdateState == OTAUpdateStateEnum::kIdle) && (newState != OTAUpdateStateEnum::kIdle))
{
mOtaRequestorDriver->HandleIdleStateExit();
}
Expand Down
5 changes: 1 addition & 4 deletions src/app/clusters/ota-requestor/OTARequestorDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ enum class SelectedTimer
class OTARequestorDriver
{
public:
using OTAUpdateStateEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum;
using OTAChangeReasonEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum;

using ProviderLocationType = app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type;

virtual ~OTARequestorDriver() = default;
Expand All @@ -97,7 +94,7 @@ class OTARequestorDriver
/// Called when an error occurs at any OTA requestor operation
virtual void HandleError(UpdateFailureState state, CHIP_ERROR error) = 0;

/// Called when OTA Requestor has exitted the Idle state for which the driver may need to take various actions
/// Called when OTA Requestor has exited the Idle state for which the driver may need to take various actions
virtual void HandleIdleStateExit() = 0;

// Called when the OTA Requestor has entered the Idle state for which the driver may need to take various actions
Expand Down

0 comments on commit af0aa7f

Please sign in to comment.