From bf44b6876ab9eddd2dee6aa55a571b3d4bb8e621 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Wed, 23 Mar 2022 10:46:37 -0700
Subject: [PATCH 01/14] Issue #16462 [ota-requestor] Add a watchdog timer to
 catch OTA process getting stuck

---
 .../GenericOTARequestorDriver.cpp             | 125 ++++++++++++++++--
 .../ota-requestor/GenericOTARequestorDriver.h |  15 ++-
 .../clusters/ota-requestor/OTARequestor.cpp   | 123 ++++++++++++++---
 src/app/clusters/ota-requestor/OTARequestor.h |   5 -
 .../ota-requestor/OTARequestorDriver.h        |  13 ++
 5 files changed, 242 insertions(+), 39 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index cc2bdb109a82e3..ffbf12a4eeac5b 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -118,17 +118,44 @@ bool GenericOTARequestorDriver::ProviderLocationsEqual(const ProviderLocationTyp
 
 void GenericOTARequestorDriver::HandleError(UpdateFailureState state, CHIP_ERROR error) {}
 
+IdleStateReason GenericOTARequestorDriver::MapErrorToIdleStateReason(CHIP_ERROR error)
+{
+    if (error == CHIP_NO_ERROR)
+    {
+        return IdleStateReason::kIdle;
+    }
+    else if (error == CHIP_ERROR_CONNECTION_CLOSED_UNEXPECTEDLY)
+    {
+        return IdleStateReason::kInvalidSession;
+    }
+
+    return IdleStateReason::kUnknown;
+}
+
+void GenericOTARequestorDriver::HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
+{
+    ChipLogDetail(SoftwareUpdate, "//is: GenericOTARequestorDriver::HandleStateTransition");
+
+    if ((newState == OTAUpdateStateEnum::kIdle) && (currentUpdateState != OTAUpdateStateEnum::kIdle))
+    {
+        IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);
+
+        // Inform the driver that the OTARequestor has entered the Idle state
+        HandleIdleState(idleStateReason); 
+    }
+}
+
 void GenericOTARequestorDriver::HandleIdleState(IdleStateReason reason)
 {
     switch (reason)
     {
     case IdleStateReason::kUnknown:
         ChipLogProgress(SoftwareUpdate, "Unknown idle state reason so set the periodic timer for a next attempt");
-        StartDefaultProviderTimer();
+        StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
         break;
     case IdleStateReason::kIdle:
         // There is no current OTA update in progress so start the periodic query timer
-        StartDefaultProviderTimer();
+        StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
         break;
     case IdleStateReason::kInvalidSession:
         // An invalid session is detected which may be temporary so try to query the same provider again
@@ -201,7 +228,7 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst
     else
     {
         ChipLogProgress(SoftwareUpdate, "UpdateNotFound, not scheduling further retries");
-        StartDefaultProviderTimer();
+        StartPeriodicQueryTimer();
     }
 }
 
@@ -240,7 +267,7 @@ void GenericOTARequestorDriver::UpdateDiscontinued()
     UpdateCancelled();
 
     // Restart the periodic default provider timer
-    StartDefaultProviderTimer();
+    StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
 }
 
 // Cancel all OTA update timers
@@ -337,14 +364,14 @@ 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
-    StopDefaultProviderTimer();
+    StopSelectedTimer(SelectedTimer::kDefaultProviderTimer);
 
     mProviderRetryCount++;
 
     DeviceLayer::SystemLayer().ScheduleLambda([this] { mRequestor->TriggerImmediateQueryInternal(); });
 }
 
-void GenericOTARequestorDriver::DefaultProviderTimerHandler(System::Layer * systemLayer, void * appState)
+void GenericOTARequestorDriver::PeriodicQueryTimerHandler(System::Layer * systemLayer, void * appState)
 {
     ChipLogProgress(SoftwareUpdate, "Default Provider timer handler is invoked");
 
@@ -353,7 +380,7 @@ void GenericOTARequestorDriver::DefaultProviderTimerHandler(System::Layer * syst
     bool listExhausted = false;
     if (GetNextProviderLocation(providerLocation, listExhausted) != true)
     {
-        StartDefaultProviderTimer();
+        StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
         return;
     }
 
@@ -362,28 +389,104 @@ void GenericOTARequestorDriver::DefaultProviderTimerHandler(System::Layer * syst
     SendQueryImage();
 }
 
-void GenericOTARequestorDriver::StartDefaultProviderTimer()
+void GenericOTARequestorDriver::StartPeriodicQueryTimer()
 {
     ChipLogProgress(SoftwareUpdate, "Starting the Default Provider timer, timeout: %u seconds",
                     (unsigned int) mPeriodicQueryTimeInterval);
     ScheduleDelayedAction(
         System::Clock::Seconds32(mPeriodicQueryTimeInterval),
         [](System::Layer *, void * context) {
-            (static_cast<GenericOTARequestorDriver *>(context))->DefaultProviderTimerHandler(nullptr, context);
+            (static_cast<GenericOTARequestorDriver *>(context))->PeriodicQueryTimerHandler(nullptr, context);
         },
         this);
 }
 
-void GenericOTARequestorDriver::StopDefaultProviderTimer()
+void GenericOTARequestorDriver::StopPeriodicQueryTimer()
 {
     ChipLogProgress(SoftwareUpdate, "Stopping the Default Provider timer");
     CancelDelayedAction(
         [](System::Layer *, void * context) {
-            (static_cast<GenericOTARequestorDriver *>(context))->DefaultProviderTimerHandler(nullptr, context);
+            (static_cast<GenericOTARequestorDriver *>(context))->PeriodicQueryTimerHandler(nullptr, context);
+        },
+        this);
+}
+
+void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer, void * appState)
+{
+    ChipLogProgress(SoftwareUpdate, "Default watchdog timer handler is invoked");
+
+    OTAUpdateStateEnum currentState = mRequestor->GetCurrentUpdateState();
+    
+    if((currentState == OTAUpdateStateEnum::kIdle) || (currentState == OTAUpdateStateEnum::kUnknown))
+    {
+        // OTA Requestor is not stuck in non-idle state.  Restart watchdog timer.
+        StartWatchdogTimer();
+    }
+    else
+    {
+        // OTA Requestor has been in a non-idle state for too long.  Reset and start Provider timer for the next Query.
+        //is:
+        //mRequestor->Reset();
+        StartPeriodicQueryTimer();
+    }
+}
+
+void GenericOTARequestorDriver::StartWatchdogTimer()
+{
+    ChipLogProgress(SoftwareUpdate, "Starting the watchdog timer, timeout: %u seconds",
+                    (unsigned int) mWatchdogTimeInterval);
+    ScheduleDelayedAction(
+        System::Clock::Seconds32(mWatchdogTimeInterval),
+        [](System::Layer *, void * context) {
+            (static_cast<GenericOTARequestorDriver *>(context))->WatchdogTimerHandler(nullptr, context);
+        },
+        this);
+}
+
+void GenericOTARequestorDriver::StopWatchdogTimer()
+{
+    ChipLogProgress(SoftwareUpdate, "Stopping the watchdog timer");
+    CancelDelayedAction(
+        [](System::Layer *, void * context) {
+            (static_cast<GenericOTARequestorDriver *>(context))->WatchdogTimerHandler(nullptr, context);
         },
         this);
 }
 
+void GenericOTARequestorDriver::StartSelectedTimer(SelectedTimer timer)
+{
+    ChipLogProgress(SoftwareUpdate, "//is: StartSelectedTimer");
+
+    switch (timer)
+    {
+        case SelectedTimer::kDefaultProviderTimer:
+            StopWatchdogTimer();
+            StartPeriodicQueryTimer();
+            break;
+        case SelectedTimer::kWatchdogTimer:
+            StopPeriodicQueryTimer();
+            StartWatchdogTimer();
+            break;
+    }
+}
+
+void GenericOTARequestorDriver::StopSelectedTimer(SelectedTimer timer)
+{
+    ChipLogProgress(SoftwareUpdate, "//is: StopSelectedTimer");
+
+    switch (timer)
+    {
+        case SelectedTimer::kDefaultProviderTimer:
+            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
diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
index 5b2e51762ea0c3..d656e6021f3734 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
@@ -56,6 +56,7 @@ class GenericOTARequestorDriver : public OTARequestorDriver
     bool CanConsent() override;
     uint16_t GetMaxDownloadBlockSize() override;
     void HandleError(UpdateFailureState state, CHIP_ERROR error) override;
+    void HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error = CHIP_NO_ERROR) override;
     void HandleIdleState(IdleStateReason reason) override;
     void UpdateAvailable(const UpdateDescription & update, System::Clock::Seconds32 delay) override;
     void UpdateNotFound(UpdateNotFoundReason reason, System::Clock::Seconds32 delay) override;
@@ -71,17 +72,25 @@ class GenericOTARequestorDriver : public OTARequestorDriver
     bool GetNextProviderLocation(ProviderLocationType & providerLocation, bool & listExhausted) override;
 
 protected:
-    void StartDefaultProviderTimer();
-    void StopDefaultProviderTimer();
-    void DefaultProviderTimerHandler(System::Layer * systemLayer, void * appState);
+    void StartPeriodicQueryTimer();
+    void StopPeriodicQueryTimer();
+    void PeriodicQueryTimerHandler(System::Layer * systemLayer, void * appState);
+    void StartWatchdogTimer();
+    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);
+    // Map a CHIP_ERROR to an IdleStateReason enum type
+    IdleStateReason MapErrorToIdleStateReason(CHIP_ERROR error);
 
     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
+    uint32_t mWatchdogTimeInterval = (6 * 60 * 60); // Timeout (in seconds) for checking if Requestor has reverted back to idle mode
     // Maximum number of times to retry a BUSY OTA provider before moving to the next available one
     static constexpr uint8_t kMaxBusyProviderRetryCount = 3;
     uint8_t mProviderRetryCount; // Track retry count for the current provider
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index f6b19f5981922c..deec31b993650a 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -104,6 +104,8 @@ OTARequestorInterface * GetRequestorInstance()
 
 void OTARequestor::InitState(intptr_t context)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::InitState");
+
     OTARequestor * requestorCore = reinterpret_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
@@ -114,6 +116,8 @@ void OTARequestor::InitState(intptr_t context)
     //   resetting the states appropriately, including the current update state.
     OtaRequestorServerSetUpdateState(requestorCore->mCurrentUpdateState);
     OtaRequestorServerSetUpdateStateProgress(app::DataModel::NullNullable);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::InitState exit");
 }
 
 CHIP_ERROR OTARequestor::Init(Server & server, OTARequestorStorage & storage, OTARequestorDriver & driver,
@@ -138,6 +142,8 @@ CHIP_ERROR OTARequestor::Init(Server & server, OTARequestorStorage & storage, OT
 
 void OTARequestor::OnQueryImageResponse(void * context, const QueryImageResponse::DecodableType & response)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageResponse");
+
     LogQueryImageResponse(response);
 
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
@@ -212,10 +218,14 @@ void OTARequestor::OnQueryImageResponse(void * context, const QueryImageResponse
         requestorCore->RecordErrorUpdateState(UpdateFailureState::kQuerying, CHIP_ERROR_BAD_REQUEST);
         break;
     }
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageResponse Exit");
 }
 
 void OTARequestor::OnQueryImageFailure(void * context, CHIP_ERROR error)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageFailure");
+    
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
@@ -230,10 +240,14 @@ void OTARequestor::OnQueryImageFailure(void * context, CHIP_ERROR error)
     }
 
     requestorCore->RecordErrorUpdateState(UpdateFailureState::kQuerying, error);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageFailure Exit");
 }
 
 void OTARequestor::OnApplyUpdateResponse(void * context, const ApplyUpdateResponse::DecodableType & response)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateResponse");
+
     LogApplyUpdateResponse(response);
 
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
@@ -253,26 +267,39 @@ void OTARequestor::OnApplyUpdateResponse(void * context, const ApplyUpdateRespon
         requestorCore->mOtaRequestorDriver->UpdateDiscontinued();
         break;
     }
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateResponse Exit");
 }
 
 void OTARequestor::OnApplyUpdateFailure(void * context, CHIP_ERROR error)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateFailure");
+
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
     ChipLogDetail(SoftwareUpdate, "ApplyUpdate failure response %" CHIP_ERROR_FORMAT, error.Format());
     requestorCore->RecordErrorUpdateState(UpdateFailureState::kApplying, error);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateFailure Exit");
 }
 
-void OTARequestor::OnNotifyUpdateAppliedResponse(void * context, const app::DataModel::NullObjectType & response) {}
+void OTARequestor::OnNotifyUpdateAppliedResponse(void * context, const app::DataModel::NullObjectType & response) 
+{
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnNotifyUpdateAppliedResponse - Does nothing");
+}
 
 void OTARequestor::OnNotifyUpdateAppliedFailure(void * context, CHIP_ERROR error)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnNotifyUpdateAppliedFailure");
+
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
     ChipLogDetail(SoftwareUpdate, "NotifyUpdateApplied failure response %" CHIP_ERROR_FORMAT, error.Format());
     requestorCore->RecordErrorUpdateState(UpdateFailureState::kNotifying, error);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnNotifyUpdateAppliedFailure Exit");
 }
 
 void OTARequestor::Reset()
@@ -295,6 +322,8 @@ EmberAfStatus OTARequestor::HandleAnnounceOTAProvider(app::CommandHandler * comm
                                                       const app::ConcreteCommandPath & commandPath,
                                                       const AnnounceOtaProvider::DecodableType & commandData)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::HandleAnnounceOTAProvider");
+
     auto & announcementReason = commandData.announcementReason;
 
     ChipLogProgress(SoftwareUpdate, "OTA Requestor received AnnounceOTAProvider");
@@ -321,11 +350,15 @@ EmberAfStatus OTARequestor::HandleAnnounceOTAProvider(app::CommandHandler * comm
 
     mOtaRequestorDriver->ProcessAnnounceOTAProviders(providerLocation, announcementReason);
 
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::HandleAnnounceOTAProvider Exit");
+
     return EMBER_ZCL_STATUS_SUCCESS;
 }
 
 void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ConnectToProvider");
+    
     if (mServer == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Server not set");
@@ -363,10 +396,13 @@ void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction)
         RecordErrorUpdateState(UpdateFailureState::kUnknown, CHIP_ERROR_INCORRECT_STATE);
         return;
     }
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ConnectToProvider Exit");
 }
 
 void OTARequestor::DisconnectFromProvider()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DisconnectFromProvider");
+
     if (mServer == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Server not set");
@@ -390,17 +426,24 @@ void OTARequestor::DisconnectFromProvider()
     }
 
     mCASESessionManager->ReleaseSession(fabricInfo->GetPeerIdForNode(mProviderLocation.Value().providerNodeID));
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DisconnectFromProvider Exit");
 }
 
 // Requestor is directed to cancel image update in progress. All the Requestor state is
 // cleared, UpdateState is reset to Idle
 void OTARequestor::CancelImageUpdate()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::CancelImageUpdate");
+
     mBdxDownloader->EndDownload(CHIP_ERROR_CONNECTION_ABORTED);
 
     mOtaRequestorDriver->UpdateCancelled();
 
+    //is: replace with reset()
     RecordNewUpdateState(OTAUpdateStateEnum::kIdle, OTAChangeReasonEnum::kUnknown);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::CancelImageUpdate Exit");
 }
 
 CHIP_ERROR OTARequestor::GetUpdateStateProgressAttribute(EndpointId endpointId, app::DataModel::Nullable<uint8_t> & progress)
@@ -419,6 +462,8 @@ CHIP_ERROR OTARequestor::GetUpdateStateAttribute(EndpointId endpointId, OTAUpdat
 // Called whenever FindOrEstablishSession is successful
 void OTARequestor::OnConnected(void * context, OperationalDeviceProxy * deviceProxy)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnected");
+
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
     VerifyOrDie(deviceProxy != nullptr);
@@ -472,11 +517,15 @@ void OTARequestor::OnConnected(void * context, OperationalDeviceProxy * devicePr
     default:
         break;
     }
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnected Exit");
 }
 
 // Called whenever FindOrEstablishSession fails
 void OTARequestor::OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnectionFailure");
+
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
@@ -497,21 +546,29 @@ void OTARequestor::OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR
     default:
         break;
     }
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnectionFailure Exit");
 }
 
 // Sends the QueryImage command to the Provider currently set in the OTARequestor
 void OTARequestor::TriggerImmediateQueryInternal()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQueryInternal");
+
     // We are now connecting to a provider for the purpose of sending a QueryImage,
     // treat this as a move to the Querying state
     RecordNewUpdateState(OTAUpdateStateEnum::kQuerying, OTAChangeReasonEnum::kSuccess);
 
     ConnectToProvider(kQueryImage);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQueryInternal Exit");
 }
 
 // Sends the QueryImage command to the next available Provider
 OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQuery");
+
     ProviderLocationType providerLocation;
     bool listExhausted = false;
     if (mOtaRequestorDriver->GetNextProviderLocation(providerLocation, listExhausted) != true)
@@ -525,22 +582,30 @@ OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery()
     // Go through the driver as it has additional logic to execute
     mOtaRequestorDriver->SendQueryImage();
 
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQuery Exit");
+
     return kTriggerSuccessful;
 }
 
 void OTARequestor::DownloadUpdate()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DownloadUpdate");
+
     RecordNewUpdateState(OTAUpdateStateEnum::kDownloading, OTAChangeReasonEnum::kSuccess);
     ConnectToProvider(kDownload);
 }
 
 void OTARequestor::DownloadUpdateDelayedOnUserConsent()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DownloadUpdateDelayedOnUserConsent");
+
     RecordNewUpdateState(OTAUpdateStateEnum::kDelayedOnUserConsent, OTAChangeReasonEnum::kSuccess);
 }
 
 void OTARequestor::ApplyUpdate()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ApplyUpdate");
+
     RecordNewUpdateState(OTAUpdateStateEnum::kApplying, OTAChangeReasonEnum::kSuccess);
 
     // If image is successfully applied, the device will reboot so persist all relevant data
@@ -551,6 +616,8 @@ void OTARequestor::ApplyUpdate()
 
 void OTARequestor::NotifyUpdateApplied()
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::NotifyUpdateApplied");
+
     // Log the VersionApplied event
     uint16_t productId;
     if (DeviceLayer::ConfigurationMgr().GetProductId(productId) != CHIP_NO_ERROR)
@@ -567,6 +634,8 @@ void OTARequestor::NotifyUpdateApplied()
 
 CHIP_ERROR OTARequestor::ClearDefaultOtaProviderList(FabricIndex fabricIndex)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ClearDefaultOtaProviderList");
+
     CHIP_ERROR error = mDefaultOtaProviderList.Delete(fabricIndex);
 
     // Ignore the error if no entry for the associated fabric index has been found.
@@ -578,6 +647,8 @@ CHIP_ERROR OTARequestor::ClearDefaultOtaProviderList(FabricIndex fabricIndex)
 
 CHIP_ERROR OTARequestor::AddDefaultOtaProvider(const ProviderLocationType & providerLocation)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::AddDefaultOtaProvider");
+
     // Look for an entry with the same fabric index indicated
     auto iterator = mDefaultOtaProviderList.Begin();
     while (iterator.Next())
@@ -597,14 +668,18 @@ CHIP_ERROR OTARequestor::AddDefaultOtaProvider(const ProviderLocationType & prov
 
 void OTARequestor::OnDownloadStateChanged(OTADownloader::State state, OTAChangeReasonEnum reason)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnDownloadStateChanged");
+
     VerifyOrDie(mOtaRequestorDriver != nullptr);
 
     switch (state)
     {
     case OTADownloader::State::kComplete:
+        ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnDownloadStateChanged - OTADownloader::State::kComplete");
         mOtaRequestorDriver->UpdateDownloaded();
         break;
     case OTADownloader::State::kIdle:
+        ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnDownloadStateChanged - OTADownloader::State::kIdle");
         if (reason != OTAChangeReasonEnum::kSuccess)
         {
             // TODO: Should we call some driver API to give it a chance to reschedule?
@@ -622,22 +697,10 @@ void OTARequestor::OnUpdateProgressChanged(Nullable<uint8_t> percent)
     OtaRequestorServerSetUpdateStateProgress(percent);
 }
 
-IdleStateReason OTARequestor::MapErrorToIdleStateReason(CHIP_ERROR error)
-{
-    if (error == CHIP_NO_ERROR)
-    {
-        return IdleStateReason::kIdle;
-    }
-    else if (error == CHIP_ERROR_CONNECTION_CLOSED_UNEXPECTEDLY)
-    {
-        return IdleStateReason::kInvalidSession;
-    }
-
-    return IdleStateReason::kUnknown;
-}
-
 void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordNewUpdateState");
+
     // Set server UpdateState attribute
     OtaRequestorServerSetUpdateState(newState);
 
@@ -658,19 +721,25 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
     }
     OtaRequestorServerOnStateTransition(mCurrentUpdateState, newState, reason, targetSoftwareVersion);
 
-    if ((newState == OTAUpdateStateEnum::kIdle) && (mCurrentUpdateState != OTAUpdateStateEnum::kIdle))
-    {
-        IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);
+    //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);
-    }
+        //mOtaRequestorDriver->HandleIdleState(idleStateReason); 
+    //} 
+    mOtaRequestorDriver->HandleStateTransition(mCurrentUpdateState, newState, reason, error);
 
     mCurrentUpdateState = newState;
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordNewUpdateState Exit");
 }
 
 void OTARequestor::RecordErrorUpdateState(UpdateFailureState failureState, CHIP_ERROR error, OTAChangeReasonEnum reason)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordErrorUpdateState failureState %d reason %d", 
+            to_underlying(failureState), to_underlying(reason));
+
     // Inform driver of the error
     mOtaRequestorDriver->HandleError(failureState, error);
 
@@ -683,6 +752,8 @@ void OTARequestor::RecordErrorUpdateState(UpdateFailureState failureState, CHIP_
 
     // Whenever an error occurs, always reset to Idle state
     RecordNewUpdateState(OTAUpdateStateEnum::kIdle, reason, error);
+
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordErrorUpdateState Exit");
 }
 
 CHIP_ERROR OTARequestor::GenerateUpdateToken()
@@ -705,6 +776,8 @@ CHIP_ERROR OTARequestor::GenerateUpdateToken()
 
 CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & deviceProxy)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendQueryImageRequest");
+
     VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
 
     constexpr OTADownloadProtocol kProtocolsSupported[] = { OTADownloadProtocol::kBDXSynchronous };
@@ -742,6 +815,8 @@ CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & devicePr
     Controller::OtaSoftwareUpdateProviderCluster cluster;
     cluster.Associate(&deviceProxy, mProviderLocation.Value().endpoint);
 
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendQueryImageRequest Exit");
+
     return cluster.InvokeCommand(args, this, OnQueryImageResponse, OnQueryImageFailure);
 }
 
@@ -773,6 +848,8 @@ CHIP_ERROR OTARequestor::ExtractUpdateDescription(const QueryImageResponseDecoda
 
 CHIP_ERROR OTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::StartDownload");
+
     VerifyOrReturnError(mBdxDownloader != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
     // TODO: allow caller to provide their own OTADownloader instance and set BDX parameters
@@ -802,6 +879,8 @@ CHIP_ERROR OTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
 
 CHIP_ERROR OTARequestor::SendApplyUpdateRequest(OperationalDeviceProxy & deviceProxy)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendApplyUpdateRequest");
+
     VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
     ReturnErrorOnFailure(GenerateUpdateToken());
 
@@ -817,6 +896,8 @@ CHIP_ERROR OTARequestor::SendApplyUpdateRequest(OperationalDeviceProxy & deviceP
 
 CHIP_ERROR OTARequestor::SendNotifyUpdateAppliedRequest(OperationalDeviceProxy & deviceProxy)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendNotifyUpdateAppliedRequest");
+
     VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
     ReturnErrorOnFailure(GenerateUpdateToken());
 
@@ -912,6 +993,8 @@ void OTARequestor::LoadCurrentUpdateInfo()
 // Invoked when the device becomes commissioned
 void OTARequestor::OnCommissioningCompleteRequestor(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
 {
+    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnCommissioningCompleteRequestor");
+
     VerifyOrReturn(event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete);
 
     ChipLogProgress(SoftwareUpdate, "Device commissioned, schedule a default provider query");
diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h
index 664bf4c9b97947..74451fe4dce6c2 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.h
+++ b/src/app/clusters/ota-requestor/OTARequestor.h
@@ -188,11 +188,6 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
      */
     static void InitState(intptr_t context);
 
-    /**
-     * Map a CHIP_ERROR to an IdleStateReason enum type
-     */
-    IdleStateReason MapErrorToIdleStateReason(CHIP_ERROR error);
-
     /**
      * Record the new update state by updating the corresponding server attribute and logging a StateTransition event
      */
diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index fe2e52fabb3788..b454bf3278afff 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -69,11 +69,21 @@ enum class IdleStateReason
     kInvalidSession,
 };
 
+// The current selected OTA Requestor timer to be running
+enum class SelectedTimer
+{
+    kDefaultProviderTimer,
+    kWatchdogTimer,
+};
+
 // Interface class to abstract the OTA-related business logic. Each application
 // must implement this interface. All calls must be non-blocking unless stated otherwise
 class OTARequestorDriver
 {
 public:
+    using OTAUpdateStateEnum   = chip::app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum; //is:
+    using OTAChangeReasonEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum; //is:
+
     using ProviderLocationType = app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type;
 
     virtual ~OTARequestorDriver() = default;
@@ -87,6 +97,9 @@ 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 a state transition for which the driver may need to take various actions
+    virtual void HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error = CHIP_NO_ERROR) = 0;
+
     // Called when the OTA Requestor has entered the Idle state for which the driver may need to take various actions
     virtual void HandleIdleState(IdleStateReason reason) = 0;
 

From b264928932c4b0f24f97b1d5eb4745edd673d787 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Wed, 23 Mar 2022 10:48:43 -0700
Subject: [PATCH 02/14] Update watchdog handling.

---
 .../GenericOTARequestorDriver.cpp             | 50 ++++++++++++-------
 .../clusters/ota-requestor/OTARequestor.cpp   |  3 +-
 .../ota-requestor/OTARequestorDriver.h        |  2 +-
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index ffbf12a4eeac5b..922ae568000c0e 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -143,6 +143,12 @@ void GenericOTARequestorDriver::HandleStateTransition(OTAUpdateStateEnum current
         // Inform the driver that the OTARequestor has entered the Idle state
         HandleIdleState(idleStateReason); 
     }
+    else if(((currentUpdateState == OTAUpdateStateEnum::kIdle) || (currentUpdateState == OTAUpdateStateEnum::kUnknown)) &&
+            (newState != OTAUpdateStateEnum::kIdle))
+    {
+        // Start watchdog timer to monitor new Query Image session
+        StartSelectedTimer(SelectedTimer::kWatchdogTimer);
+    }
 }
 
 void GenericOTARequestorDriver::HandleIdleState(IdleStateReason reason)
@@ -151,11 +157,11 @@ void GenericOTARequestorDriver::HandleIdleState(IdleStateReason reason)
     {
     case IdleStateReason::kUnknown:
         ChipLogProgress(SoftwareUpdate, "Unknown idle state reason so set the periodic timer for a next attempt");
-        StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
+        StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
         break;
     case IdleStateReason::kIdle:
         // There is no current OTA update in progress so start the periodic query timer
-        StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
+        StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
         break;
     case IdleStateReason::kInvalidSession:
         // An invalid session is detected which may be temporary so try to query the same provider again
@@ -267,7 +273,7 @@ void GenericOTARequestorDriver::UpdateDiscontinued()
     UpdateCancelled();
 
     // Restart the periodic default provider timer
-    StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
+    StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
 }
 
 // Cancel all OTA update timers
@@ -364,7 +370,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::kDefaultProviderTimer);
+    StopSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
 
     mProviderRetryCount++;
 
@@ -380,7 +386,7 @@ void GenericOTARequestorDriver::PeriodicQueryTimerHandler(System::Layer * system
     bool listExhausted = false;
     if (GetNextProviderLocation(providerLocation, listExhausted) != true)
     {
-        StartSelectedTimer(SelectedTimer::kDefaultProviderTimer);
+        StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
         return;
     }
 
@@ -417,17 +423,27 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
 
     OTAUpdateStateEnum currentState = mRequestor->GetCurrentUpdateState();
     
-    if((currentState == OTAUpdateStateEnum::kIdle) || (currentState == OTAUpdateStateEnum::kUnknown))
-    {
-        // OTA Requestor is not stuck in non-idle state.  Restart watchdog timer.
-        StartWatchdogTimer();
-    }
-    else
+    switch(currentState)
     {
-        // OTA Requestor has been in a non-idle state for too long.  Reset and start Provider timer for the next Query.
-        //is:
-        //mRequestor->Reset();
-        StartPeriodicQueryTimer();
+        case OTAUpdateStateEnum::kIdle:
+        case OTAUpdateStateEnum::kUnknown:
+            // OTA Requestor is not stuck in non-idle state.  Restart watchdog timer.
+            StartWatchdogTimer();
+            break;
+        case OTAUpdateStateEnum::kQuerying:
+        case OTAUpdateStateEnum::kDelayedOnQuery:
+        case OTAUpdateStateEnum::kDownloading:
+        case OTAUpdateStateEnum::kApplying:
+        case OTAUpdateStateEnum::kDelayedOnApply:
+            UpdateCancelled();
+            mRequestor->Reset();
+            StartPeriodicQueryTimer();
+            break;
+        case OTAUpdateStateEnum::kRollingBack:
+        case OTAUpdateStateEnum::kDelayedOnUserConsent:
+            mRequestor->Reset();
+            StartPeriodicQueryTimer();
+            break;
     }
 }
 
@@ -459,7 +475,7 @@ void GenericOTARequestorDriver::StartSelectedTimer(SelectedTimer timer)
 
     switch (timer)
     {
-        case SelectedTimer::kDefaultProviderTimer:
+        case SelectedTimer::kPeriodicQueryTimer:
             StopWatchdogTimer();
             StartPeriodicQueryTimer();
             break;
@@ -476,7 +492,7 @@ void GenericOTARequestorDriver::StopSelectedTimer(SelectedTimer timer)
 
     switch (timer)
     {
-        case SelectedTimer::kDefaultProviderTimer:
+        case SelectedTimer::kPeriodicQueryTimer:
             StopPeriodicQueryTimer();
             StartWatchdogTimer();
             break;
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index deec31b993650a..739c70f405945a 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -440,8 +440,7 @@ void OTARequestor::CancelImageUpdate()
 
     mOtaRequestorDriver->UpdateCancelled();
 
-    //is: replace with reset()
-    RecordNewUpdateState(OTAUpdateStateEnum::kIdle, OTAChangeReasonEnum::kUnknown);
+    Reset();
 
     ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::CancelImageUpdate Exit");
 }
diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index b454bf3278afff..b2ff9fc8949fd0 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -72,7 +72,7 @@ enum class IdleStateReason
 // The current selected OTA Requestor timer to be running
 enum class SelectedTimer
 {
-    kDefaultProviderTimer,
+    kPeriodicQueryTimer,
     kWatchdogTimer,
 };
 

From 4ec995ff01f9c5c6ad55555269156470d0149c1e Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Tue, 22 Mar 2022 16:19:49 -0700
Subject: [PATCH 03/14] Remove debug code.

---
 .../GenericOTARequestorDriver.cpp             |  6 +-
 .../clusters/ota-requestor/OTARequestor.cpp   | 76 +------------------
 2 files changed, 2 insertions(+), 80 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index 922ae568000c0e..cc9e6f0f87fe70 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -409,7 +409,7 @@ void GenericOTARequestorDriver::StartPeriodicQueryTimer()
 
 void GenericOTARequestorDriver::StopPeriodicQueryTimer()
 {
-    ChipLogProgress(SoftwareUpdate, "Stopping the Default Provider timer");
+    ChipLogProgress(SoftwareUpdate, "Stopping the Periodic Query timer");
     CancelDelayedAction(
         [](System::Layer *, void * context) {
             (static_cast<GenericOTARequestorDriver *>(context))->PeriodicQueryTimerHandler(nullptr, context);
@@ -471,8 +471,6 @@ void GenericOTARequestorDriver::StopWatchdogTimer()
 
 void GenericOTARequestorDriver::StartSelectedTimer(SelectedTimer timer)
 {
-    ChipLogProgress(SoftwareUpdate, "//is: StartSelectedTimer");
-
     switch (timer)
     {
         case SelectedTimer::kPeriodicQueryTimer:
@@ -488,8 +486,6 @@ void GenericOTARequestorDriver::StartSelectedTimer(SelectedTimer timer)
 
 void GenericOTARequestorDriver::StopSelectedTimer(SelectedTimer timer)
 {
-    ChipLogProgress(SoftwareUpdate, "//is: StopSelectedTimer");
-
     switch (timer)
     {
         case SelectedTimer::kPeriodicQueryTimer:
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 739c70f405945a..49f41bb49a27af 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -104,8 +104,6 @@ OTARequestorInterface * GetRequestorInstance()
 
 void OTARequestor::InitState(intptr_t context)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::InitState");
-
     OTARequestor * requestorCore = reinterpret_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
@@ -116,8 +114,6 @@ void OTARequestor::InitState(intptr_t context)
     //   resetting the states appropriately, including the current update state.
     OtaRequestorServerSetUpdateState(requestorCore->mCurrentUpdateState);
     OtaRequestorServerSetUpdateStateProgress(app::DataModel::NullNullable);
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::InitState exit");
 }
 
 CHIP_ERROR OTARequestor::Init(Server & server, OTARequestorStorage & storage, OTARequestorDriver & driver,
@@ -142,8 +138,6 @@ CHIP_ERROR OTARequestor::Init(Server & server, OTARequestorStorage & storage, OT
 
 void OTARequestor::OnQueryImageResponse(void * context, const QueryImageResponse::DecodableType & response)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageResponse");
-
     LogQueryImageResponse(response);
 
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
@@ -218,14 +212,10 @@ void OTARequestor::OnQueryImageResponse(void * context, const QueryImageResponse
         requestorCore->RecordErrorUpdateState(UpdateFailureState::kQuerying, CHIP_ERROR_BAD_REQUEST);
         break;
     }
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageResponse Exit");
 }
 
 void OTARequestor::OnQueryImageFailure(void * context, CHIP_ERROR error)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageFailure");
-    
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
@@ -240,14 +230,10 @@ void OTARequestor::OnQueryImageFailure(void * context, CHIP_ERROR error)
     }
 
     requestorCore->RecordErrorUpdateState(UpdateFailureState::kQuerying, error);
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnQueryImageFailure Exit");
 }
 
 void OTARequestor::OnApplyUpdateResponse(void * context, const ApplyUpdateResponse::DecodableType & response)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateResponse");
-
     LogApplyUpdateResponse(response);
 
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
@@ -267,39 +253,26 @@ void OTARequestor::OnApplyUpdateResponse(void * context, const ApplyUpdateRespon
         requestorCore->mOtaRequestorDriver->UpdateDiscontinued();
         break;
     }
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateResponse Exit");
 }
 
 void OTARequestor::OnApplyUpdateFailure(void * context, CHIP_ERROR error)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateFailure");
-
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
     ChipLogDetail(SoftwareUpdate, "ApplyUpdate failure response %" CHIP_ERROR_FORMAT, error.Format());
     requestorCore->RecordErrorUpdateState(UpdateFailureState::kApplying, error);
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnApplyUpdateFailure Exit");
 }
 
-void OTARequestor::OnNotifyUpdateAppliedResponse(void * context, const app::DataModel::NullObjectType & response) 
-{
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnNotifyUpdateAppliedResponse - Does nothing");
-}
+void OTARequestor::OnNotifyUpdateAppliedResponse(void * context, const app::DataModel::NullObjectType & response) {}
 
 void OTARequestor::OnNotifyUpdateAppliedFailure(void * context, CHIP_ERROR error)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnNotifyUpdateAppliedFailure");
-
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
     ChipLogDetail(SoftwareUpdate, "NotifyUpdateApplied failure response %" CHIP_ERROR_FORMAT, error.Format());
     requestorCore->RecordErrorUpdateState(UpdateFailureState::kNotifying, error);
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnNotifyUpdateAppliedFailure Exit");
 }
 
 void OTARequestor::Reset()
@@ -322,8 +295,6 @@ EmberAfStatus OTARequestor::HandleAnnounceOTAProvider(app::CommandHandler * comm
                                                       const app::ConcreteCommandPath & commandPath,
                                                       const AnnounceOtaProvider::DecodableType & commandData)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::HandleAnnounceOTAProvider");
-
     auto & announcementReason = commandData.announcementReason;
 
     ChipLogProgress(SoftwareUpdate, "OTA Requestor received AnnounceOTAProvider");
@@ -350,15 +321,11 @@ EmberAfStatus OTARequestor::HandleAnnounceOTAProvider(app::CommandHandler * comm
 
     mOtaRequestorDriver->ProcessAnnounceOTAProviders(providerLocation, announcementReason);
 
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::HandleAnnounceOTAProvider Exit");
-
     return EMBER_ZCL_STATUS_SUCCESS;
 }
 
 void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ConnectToProvider");
-    
     if (mServer == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Server not set");
@@ -396,13 +363,10 @@ void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction)
         RecordErrorUpdateState(UpdateFailureState::kUnknown, CHIP_ERROR_INCORRECT_STATE);
         return;
     }
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ConnectToProvider Exit");
 }
 
 void OTARequestor::DisconnectFromProvider()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DisconnectFromProvider");
-
     if (mServer == nullptr)
     {
         ChipLogError(SoftwareUpdate, "Server not set");
@@ -426,23 +390,17 @@ void OTARequestor::DisconnectFromProvider()
     }
 
     mCASESessionManager->ReleaseSession(fabricInfo->GetPeerIdForNode(mProviderLocation.Value().providerNodeID));
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DisconnectFromProvider Exit");
 }
 
 // Requestor is directed to cancel image update in progress. All the Requestor state is
 // cleared, UpdateState is reset to Idle
 void OTARequestor::CancelImageUpdate()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::CancelImageUpdate");
-
     mBdxDownloader->EndDownload(CHIP_ERROR_CONNECTION_ABORTED);
 
     mOtaRequestorDriver->UpdateCancelled();
 
     Reset();
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::CancelImageUpdate Exit");
 }
 
 CHIP_ERROR OTARequestor::GetUpdateStateProgressAttribute(EndpointId endpointId, app::DataModel::Nullable<uint8_t> & progress)
@@ -461,8 +419,6 @@ CHIP_ERROR OTARequestor::GetUpdateStateAttribute(EndpointId endpointId, OTAUpdat
 // Called whenever FindOrEstablishSession is successful
 void OTARequestor::OnConnected(void * context, OperationalDeviceProxy * deviceProxy)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnected");
-
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
     VerifyOrDie(deviceProxy != nullptr);
@@ -516,15 +472,11 @@ void OTARequestor::OnConnected(void * context, OperationalDeviceProxy * devicePr
     default:
         break;
     }
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnected Exit");
 }
 
 // Called whenever FindOrEstablishSession fails
 void OTARequestor::OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnectionFailure");
-
     OTARequestor * requestorCore = static_cast<OTARequestor *>(context);
     VerifyOrDie(requestorCore != nullptr);
 
@@ -545,29 +497,21 @@ void OTARequestor::OnConnectionFailure(void * context, PeerId peerId, CHIP_ERROR
     default:
         break;
     }
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnConnectionFailure Exit");
 }
 
 // Sends the QueryImage command to the Provider currently set in the OTARequestor
 void OTARequestor::TriggerImmediateQueryInternal()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQueryInternal");
-
     // We are now connecting to a provider for the purpose of sending a QueryImage,
     // treat this as a move to the Querying state
     RecordNewUpdateState(OTAUpdateStateEnum::kQuerying, OTAChangeReasonEnum::kSuccess);
 
     ConnectToProvider(kQueryImage);
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQueryInternal Exit");
 }
 
 // Sends the QueryImage command to the next available Provider
 OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQuery");
-
     ProviderLocationType providerLocation;
     bool listExhausted = false;
     if (mOtaRequestorDriver->GetNextProviderLocation(providerLocation, listExhausted) != true)
@@ -581,30 +525,22 @@ OTARequestorInterface::OTATriggerResult OTARequestor::TriggerImmediateQuery()
     // Go through the driver as it has additional logic to execute
     mOtaRequestorDriver->SendQueryImage();
 
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::TriggerImmediateQuery Exit");
-
     return kTriggerSuccessful;
 }
 
 void OTARequestor::DownloadUpdate()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DownloadUpdate");
-
     RecordNewUpdateState(OTAUpdateStateEnum::kDownloading, OTAChangeReasonEnum::kSuccess);
     ConnectToProvider(kDownload);
 }
 
 void OTARequestor::DownloadUpdateDelayedOnUserConsent()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::DownloadUpdateDelayedOnUserConsent");
-
     RecordNewUpdateState(OTAUpdateStateEnum::kDelayedOnUserConsent, OTAChangeReasonEnum::kSuccess);
 }
 
 void OTARequestor::ApplyUpdate()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ApplyUpdate");
-
     RecordNewUpdateState(OTAUpdateStateEnum::kApplying, OTAChangeReasonEnum::kSuccess);
 
     // If image is successfully applied, the device will reboot so persist all relevant data
@@ -615,8 +551,6 @@ void OTARequestor::ApplyUpdate()
 
 void OTARequestor::NotifyUpdateApplied()
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::NotifyUpdateApplied");
-
     // Log the VersionApplied event
     uint16_t productId;
     if (DeviceLayer::ConfigurationMgr().GetProductId(productId) != CHIP_NO_ERROR)
@@ -633,8 +567,6 @@ void OTARequestor::NotifyUpdateApplied()
 
 CHIP_ERROR OTARequestor::ClearDefaultOtaProviderList(FabricIndex fabricIndex)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::ClearDefaultOtaProviderList");
-
     CHIP_ERROR error = mDefaultOtaProviderList.Delete(fabricIndex);
 
     // Ignore the error if no entry for the associated fabric index has been found.
@@ -646,8 +578,6 @@ CHIP_ERROR OTARequestor::ClearDefaultOtaProviderList(FabricIndex fabricIndex)
 
 CHIP_ERROR OTARequestor::AddDefaultOtaProvider(const ProviderLocationType & providerLocation)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::AddDefaultOtaProvider");
-
     // Look for an entry with the same fabric index indicated
     auto iterator = mDefaultOtaProviderList.Begin();
     while (iterator.Next())
@@ -667,18 +597,14 @@ CHIP_ERROR OTARequestor::AddDefaultOtaProvider(const ProviderLocationType & prov
 
 void OTARequestor::OnDownloadStateChanged(OTADownloader::State state, OTAChangeReasonEnum reason)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnDownloadStateChanged");
-
     VerifyOrDie(mOtaRequestorDriver != nullptr);
 
     switch (state)
     {
     case OTADownloader::State::kComplete:
-        ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnDownloadStateChanged - OTADownloader::State::kComplete");
         mOtaRequestorDriver->UpdateDownloaded();
         break;
     case OTADownloader::State::kIdle:
-        ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnDownloadStateChanged - OTADownloader::State::kIdle");
         if (reason != OTAChangeReasonEnum::kSuccess)
         {
             // TODO: Should we call some driver API to give it a chance to reschedule?

From b875e8ca3d83fcfd902b3b8f665619001257be63 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Tue, 22 Mar 2022 16:21:50 -0700
Subject: [PATCH 04/14] Remove debug comments.

---
 src/app/clusters/ota-requestor/OTARequestorDriver.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index b2ff9fc8949fd0..7ac8aee8f8a82e 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -81,8 +81,8 @@ enum class SelectedTimer
 class OTARequestorDriver
 {
 public:
-    using OTAUpdateStateEnum   = chip::app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum; //is:
-    using OTAChangeReasonEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum; //is:
+    using OTAUpdateStateEnum   = chip::app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum;
+    using OTAChangeReasonEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum;
 
     using ProviderLocationType = app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type;
 

From d30e9500d70f4bd55ce02c422ada15fec6f9dee3 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Tue, 22 Mar 2022 21:41:09 -0700
Subject: [PATCH 05/14] Minor code review changes.

---
 .../clusters/ota-requestor/GenericOTARequestorDriver.cpp | 9 ++++-----
 src/app/clusters/ota-requestor/OTARequestorDriver.h      | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index cc9e6f0f87fe70..37ceebb62120d9 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -134,8 +134,6 @@ IdleStateReason GenericOTARequestorDriver::MapErrorToIdleStateReason(CHIP_ERROR
 
 void GenericOTARequestorDriver::HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: GenericOTARequestorDriver::HandleStateTransition");
-
     if ((newState == OTAUpdateStateEnum::kIdle) && (currentUpdateState != OTAUpdateStateEnum::kIdle))
     {
         IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);
@@ -397,7 +395,7 @@ void GenericOTARequestorDriver::PeriodicQueryTimerHandler(System::Layer * system
 
 void GenericOTARequestorDriver::StartPeriodicQueryTimer()
 {
-    ChipLogProgress(SoftwareUpdate, "Starting the Default Provider timer, timeout: %u seconds",
+    ChipLogProgress(SoftwareUpdate, "Starting the periodic query timer, timeout: %u seconds",
                     (unsigned int) mPeriodicQueryTimeInterval);
     ScheduleDelayedAction(
         System::Clock::Seconds32(mPeriodicQueryTimeInterval),
@@ -419,7 +417,7 @@ void GenericOTARequestorDriver::StopPeriodicQueryTimer()
 
 void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer, void * appState)
 {
-    ChipLogProgress(SoftwareUpdate, "Default watchdog timer handler is invoked");
+    ChipLogProgress(SoftwareUpdate, "Watchdog timer handler is invoked");
 
     OTAUpdateStateEnum currentState = mRequestor->GetCurrentUpdateState();
     
@@ -435,7 +433,8 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
         case OTAUpdateStateEnum::kDownloading:
         case OTAUpdateStateEnum::kApplying:
         case OTAUpdateStateEnum::kDelayedOnApply:
-            UpdateCancelled();
+            UpdateDiscontinued();
+            mRequestor->CancelImageUpdate();
             mRequestor->Reset();
             StartPeriodicQueryTimer();
             break;
diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index 7ac8aee8f8a82e..2afe6d9dea9484 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -81,7 +81,7 @@ enum class SelectedTimer
 class OTARequestorDriver
 {
 public:
-    using OTAUpdateStateEnum   = chip::app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum;
+    using OTAUpdateStateEnum   = app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum;
     using OTAChangeReasonEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum;
 
     using ProviderLocationType = app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type;

From ac475f957bb89b2eb3606de679daca887a871c17 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Tue, 22 Mar 2022 22:09:58 -0700
Subject: [PATCH 06/14] Attempt to add back debug code to remove them again.

---
 src/app/clusters/ota-requestor/OTARequestor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 49f41bb49a27af..9511b349183110 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -656,7 +656,7 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
     mOtaRequestorDriver->HandleStateTransition(mCurrentUpdateState, newState, reason, error);
 
     mCurrentUpdateState = newState;
-
+    
     ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordNewUpdateState Exit");
 }
 

From bafb556325d3cbd6171971492a0e12b0cf4d75c2 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Tue, 22 Mar 2022 22:12:15 -0700
Subject: [PATCH 07/14] Remove debug code again.

---
 .../clusters/ota-requestor/OTARequestor.cpp   | 21 -------------------
 1 file changed, 21 deletions(-)

diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 9511b349183110..9184d1d4308790 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -624,8 +624,6 @@ void OTARequestor::OnUpdateProgressChanged(Nullable<uint8_t> percent)
 
 void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordNewUpdateState");
-
     // Set server UpdateState attribute
     OtaRequestorServerSetUpdateState(newState);
 
@@ -656,15 +654,10 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
     mOtaRequestorDriver->HandleStateTransition(mCurrentUpdateState, newState, reason, error);
 
     mCurrentUpdateState = newState;
-    
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordNewUpdateState Exit");
 }
 
 void OTARequestor::RecordErrorUpdateState(UpdateFailureState failureState, CHIP_ERROR error, OTAChangeReasonEnum reason)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordErrorUpdateState failureState %d reason %d", 
-            to_underlying(failureState), to_underlying(reason));
-
     // Inform driver of the error
     mOtaRequestorDriver->HandleError(failureState, error);
 
@@ -677,8 +670,6 @@ void OTARequestor::RecordErrorUpdateState(UpdateFailureState failureState, CHIP_
 
     // Whenever an error occurs, always reset to Idle state
     RecordNewUpdateState(OTAUpdateStateEnum::kIdle, reason, error);
-
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::RecordErrorUpdateState Exit");
 }
 
 CHIP_ERROR OTARequestor::GenerateUpdateToken()
@@ -701,8 +692,6 @@ CHIP_ERROR OTARequestor::GenerateUpdateToken()
 
 CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & deviceProxy)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendQueryImageRequest");
-
     VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
 
     constexpr OTADownloadProtocol kProtocolsSupported[] = { OTADownloadProtocol::kBDXSynchronous };
@@ -740,8 +729,6 @@ CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & devicePr
     Controller::OtaSoftwareUpdateProviderCluster cluster;
     cluster.Associate(&deviceProxy, mProviderLocation.Value().endpoint);
 
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendQueryImageRequest Exit");
-
     return cluster.InvokeCommand(args, this, OnQueryImageResponse, OnQueryImageFailure);
 }
 
@@ -773,8 +760,6 @@ CHIP_ERROR OTARequestor::ExtractUpdateDescription(const QueryImageResponseDecoda
 
 CHIP_ERROR OTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::StartDownload");
-
     VerifyOrReturnError(mBdxDownloader != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
     // TODO: allow caller to provide their own OTADownloader instance and set BDX parameters
@@ -804,8 +789,6 @@ CHIP_ERROR OTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
 
 CHIP_ERROR OTARequestor::SendApplyUpdateRequest(OperationalDeviceProxy & deviceProxy)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendApplyUpdateRequest");
-
     VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
     ReturnErrorOnFailure(GenerateUpdateToken());
 
@@ -821,8 +804,6 @@ CHIP_ERROR OTARequestor::SendApplyUpdateRequest(OperationalDeviceProxy & deviceP
 
 CHIP_ERROR OTARequestor::SendNotifyUpdateAppliedRequest(OperationalDeviceProxy & deviceProxy)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::SendNotifyUpdateAppliedRequest");
-
     VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
     ReturnErrorOnFailure(GenerateUpdateToken());
 
@@ -918,8 +899,6 @@ void OTARequestor::LoadCurrentUpdateInfo()
 // Invoked when the device becomes commissioned
 void OTARequestor::OnCommissioningCompleteRequestor(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
 {
-    ChipLogDetail(SoftwareUpdate, "//is: OTARequestor::OnCommissioningCompleteRequestor");
-
     VerifyOrReturn(event->Type == DeviceLayer::DeviceEventType::kCommissioningComplete);
 
     ChipLogProgress(SoftwareUpdate, "Device commissioned, schedule a default provider query");

From 19363b44e64c4980271d5f16d78d240c472a800c Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Wed, 23 Mar 2022 11:28:57 -0700
Subject: [PATCH 08/14] Resolve code review comments.

- Remove HandleStateTransition() logic and move it back out to OTARequestor.
- Added HandleIdleStateExit() in OTARequestorDriver to start Watchdog timer.
---
 .../GenericOTARequestorDriver.cpp             | 33 +++----------------
 .../ota-requestor/GenericOTARequestorDriver.h |  4 +--
 .../clusters/ota-requestor/OTARequestor.cpp   | 30 +++++++++++++----
 src/app/clusters/ota-requestor/OTARequestor.h |  5 +++
 .../ota-requestor/OTARequestorDriver.h        |  4 +--
 5 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index 37ceebb62120d9..cfa329170cc9a1 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -85,7 +85,7 @@ void GenericOTARequestorDriver::Init(OTARequestorInterface * requestor, OTAImage
     else
     {
         // Start the first periodic query timer
-        StartDefaultProviderTimer();
+        StartPeriodicQueryTimer();
     }
 }
 
@@ -118,35 +118,10 @@ bool GenericOTARequestorDriver::ProviderLocationsEqual(const ProviderLocationTyp
 
 void GenericOTARequestorDriver::HandleError(UpdateFailureState state, CHIP_ERROR error) {}
 
-IdleStateReason GenericOTARequestorDriver::MapErrorToIdleStateReason(CHIP_ERROR error)
-{
-    if (error == CHIP_NO_ERROR)
-    {
-        return IdleStateReason::kIdle;
-    }
-    else if (error == CHIP_ERROR_CONNECTION_CLOSED_UNEXPECTEDLY)
-    {
-        return IdleStateReason::kInvalidSession;
-    }
-
-    return IdleStateReason::kUnknown;
-}
-
-void GenericOTARequestorDriver::HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
+void GenericOTARequestorDriver::HandleIdleStateExit()
 {
-    if ((newState == OTAUpdateStateEnum::kIdle) && (currentUpdateState != OTAUpdateStateEnum::kIdle))
-    {
-        IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);
-
-        // Inform the driver that the OTARequestor has entered the Idle state
-        HandleIdleState(idleStateReason); 
-    }
-    else if(((currentUpdateState == OTAUpdateStateEnum::kIdle) || (currentUpdateState == OTAUpdateStateEnum::kUnknown)) &&
-            (newState != OTAUpdateStateEnum::kIdle))
-    {
-        // Start watchdog timer to monitor new Query Image session
-        StartSelectedTimer(SelectedTimer::kWatchdogTimer);
-    }
+    // Start watchdog timer to monitor new Query Image session
+    StartSelectedTimer(SelectedTimer::kWatchdogTimer);
 }
 
 void GenericOTARequestorDriver::HandleIdleState(IdleStateReason reason)
diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
index d656e6021f3734..18d98052b409cd 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
@@ -56,7 +56,7 @@ class GenericOTARequestorDriver : public OTARequestorDriver
     bool CanConsent() override;
     uint16_t GetMaxDownloadBlockSize() override;
     void HandleError(UpdateFailureState state, CHIP_ERROR error) override;
-    void HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error = CHIP_NO_ERROR) override;
+    void HandleIdleStateExit() override;
     void HandleIdleState(IdleStateReason reason) override;
     void UpdateAvailable(const UpdateDescription & update, System::Clock::Seconds32 delay) override;
     void UpdateNotFound(UpdateNotFoundReason reason, System::Clock::Seconds32 delay) override;
@@ -83,8 +83,6 @@ class GenericOTARequestorDriver : public OTARequestorDriver
     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);
-    // Map a CHIP_ERROR to an IdleStateReason enum type
-    IdleStateReason MapErrorToIdleStateReason(CHIP_ERROR error);
 
     OTARequestorInterface * mRequestor           = nullptr;
     OTAImageProcessorInterface * mImageProcessor = nullptr;
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 9184d1d4308790..845a9ca6539146 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -622,6 +622,20 @@ void OTARequestor::OnUpdateProgressChanged(Nullable<uint8_t> percent)
     OtaRequestorServerSetUpdateStateProgress(percent);
 }
 
+IdleStateReason OTARequestor::MapErrorToIdleStateReason(CHIP_ERROR error)
+{
+    if (error == CHIP_NO_ERROR)
+    {
+        return IdleStateReason::kIdle;
+    }
+    else if (error == CHIP_ERROR_CONNECTION_CLOSED_UNEXPECTEDLY)
+    {
+        return IdleStateReason::kInvalidSession;
+    }
+
+    return IdleStateReason::kUnknown;
+}
+
 void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
 {
     // Set server UpdateState attribute
@@ -644,14 +658,18 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
     }
     OtaRequestorServerOnStateTransition(mCurrentUpdateState, newState, reason, targetSoftwareVersion);
 
-    //if ((newState == OTAUpdateStateEnum::kIdle) && (mCurrentUpdateState != OTAUpdateStateEnum::kIdle))
-    //{
-    //    IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);
+    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); 
-    //} 
-    mOtaRequestorDriver->HandleStateTransition(mCurrentUpdateState, newState, reason, error);
+        mOtaRequestorDriver->HandleIdleState(idleStateReason); 
+    }
+    else if(((mCurrentUpdateState == OTAUpdateStateEnum::kIdle) || (mCurrentUpdateState == OTAUpdateStateEnum::kUnknown)) &&
+            (newState != OTAUpdateStateEnum::kIdle))
+    {
+        mOtaRequestorDriver->HandleIdleStateExit(); 
+    }
 
     mCurrentUpdateState = newState;
 }
diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h
index 74451fe4dce6c2..558f21ff5892e1 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.h
+++ b/src/app/clusters/ota-requestor/OTARequestor.h
@@ -187,6 +187,11 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
      * Callback to initialize states and server attributes in the CHIP context
      */
     static void InitState(intptr_t context);
+    
+    /**
+     * Map a CHIP_ERROR to an IdleStateReason enum type
+     */
+    IdleStateReason MapErrorToIdleStateReason(CHIP_ERROR error);
 
     /**
      * Record the new update state by updating the corresponding server attribute and logging a StateTransition event
diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index 2afe6d9dea9484..b13d43892c0a53 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -97,8 +97,8 @@ 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 a state transition for which the driver may need to take various actions
-    virtual void HandleStateTransition(OTAUpdateStateEnum currentUpdateState, OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error = CHIP_NO_ERROR) = 0;
+    /// Called when OTA Requestor has exitted 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
     virtual void HandleIdleState(IdleStateReason reason) = 0;

From ed10ed6303f93dbf54ccd49ac6f7be56c0659dec Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Wed, 23 Mar 2022 11:31:49 -0700
Subject: [PATCH 09/14] scripts/helpers/restyle-diff.sh

---
 .../GenericOTARequestorDriver.cpp             | 79 +++++++++----------
 .../clusters/ota-requestor/OTARequestor.cpp   |  8 +-
 src/app/clusters/ota-requestor/OTARequestor.h |  2 +-
 .../ota-requestor/OTARequestorDriver.h        |  2 +-
 4 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index cfa329170cc9a1..4790cc3bdfb7e4 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -395,36 +395,35 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
     ChipLogProgress(SoftwareUpdate, "Watchdog timer handler is invoked");
 
     OTAUpdateStateEnum currentState = mRequestor->GetCurrentUpdateState();
-    
-    switch(currentState)
+
+    switch (currentState)
     {
-        case OTAUpdateStateEnum::kIdle:
-        case OTAUpdateStateEnum::kUnknown:
-            // OTA Requestor is not stuck in non-idle state.  Restart watchdog timer.
-            StartWatchdogTimer();
-            break;
-        case OTAUpdateStateEnum::kQuerying:
-        case OTAUpdateStateEnum::kDelayedOnQuery:
-        case OTAUpdateStateEnum::kDownloading:
-        case OTAUpdateStateEnum::kApplying:
-        case OTAUpdateStateEnum::kDelayedOnApply:
-            UpdateDiscontinued();
-            mRequestor->CancelImageUpdate();
-            mRequestor->Reset();
-            StartPeriodicQueryTimer();
-            break;
-        case OTAUpdateStateEnum::kRollingBack:
-        case OTAUpdateStateEnum::kDelayedOnUserConsent:
-            mRequestor->Reset();
-            StartPeriodicQueryTimer();
-            break;
+    case OTAUpdateStateEnum::kIdle:
+    case OTAUpdateStateEnum::kUnknown:
+        // OTA Requestor is not stuck in non-idle state.  Restart watchdog timer.
+        StartWatchdogTimer();
+        break;
+    case OTAUpdateStateEnum::kQuerying:
+    case OTAUpdateStateEnum::kDelayedOnQuery:
+    case OTAUpdateStateEnum::kDownloading:
+    case OTAUpdateStateEnum::kApplying:
+    case OTAUpdateStateEnum::kDelayedOnApply:
+        UpdateDiscontinued();
+        mRequestor->CancelImageUpdate();
+        mRequestor->Reset();
+        StartPeriodicQueryTimer();
+        break;
+    case OTAUpdateStateEnum::kRollingBack:
+    case OTAUpdateStateEnum::kDelayedOnUserConsent:
+        mRequestor->Reset();
+        StartPeriodicQueryTimer();
+        break;
     }
 }
 
 void GenericOTARequestorDriver::StartWatchdogTimer()
 {
-    ChipLogProgress(SoftwareUpdate, "Starting the watchdog timer, timeout: %u seconds",
-                    (unsigned int) mWatchdogTimeInterval);
+    ChipLogProgress(SoftwareUpdate, "Starting the watchdog timer, timeout: %u seconds", (unsigned int) mWatchdogTimeInterval);
     ScheduleDelayedAction(
         System::Clock::Seconds32(mWatchdogTimeInterval),
         [](System::Layer *, void * context) {
@@ -447,14 +446,14 @@ void GenericOTARequestorDriver::StartSelectedTimer(SelectedTimer timer)
 {
     switch (timer)
     {
-        case SelectedTimer::kPeriodicQueryTimer:
-            StopWatchdogTimer();
-            StartPeriodicQueryTimer();
-            break;
-        case SelectedTimer::kWatchdogTimer:
-            StopPeriodicQueryTimer();
-            StartWatchdogTimer();
-            break;
+    case SelectedTimer::kPeriodicQueryTimer:
+        StopWatchdogTimer();
+        StartPeriodicQueryTimer();
+        break;
+    case SelectedTimer::kWatchdogTimer:
+        StopPeriodicQueryTimer();
+        StartWatchdogTimer();
+        break;
     }
 }
 
@@ -462,14 +461,14 @@ void GenericOTARequestorDriver::StopSelectedTimer(SelectedTimer timer)
 {
     switch (timer)
     {
-        case SelectedTimer::kPeriodicQueryTimer:
-            StopPeriodicQueryTimer();
-            StartWatchdogTimer();
-            break;
-        case SelectedTimer::kWatchdogTimer:
-            StopWatchdogTimer();
-            StartPeriodicQueryTimer();
-            break;
+    case SelectedTimer::kPeriodicQueryTimer:
+        StopPeriodicQueryTimer();
+        StartWatchdogTimer();
+        break;
+    case SelectedTimer::kWatchdogTimer:
+        StopWatchdogTimer();
+        StartPeriodicQueryTimer();
+        break;
     }
 }
 
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 845a9ca6539146..3516de66bdda9f 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -663,12 +663,12 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
         IdleStateReason idleStateReason = MapErrorToIdleStateReason(error);
 
         // Inform the driver that the OTARequestor has entered the Idle state
-        mOtaRequestorDriver->HandleIdleState(idleStateReason); 
+        mOtaRequestorDriver->HandleIdleState(idleStateReason);
     }
-    else if(((mCurrentUpdateState == OTAUpdateStateEnum::kIdle) || (mCurrentUpdateState == OTAUpdateStateEnum::kUnknown)) &&
-            (newState != OTAUpdateStateEnum::kIdle))
+    else if (((mCurrentUpdateState == OTAUpdateStateEnum::kIdle) || (mCurrentUpdateState == OTAUpdateStateEnum::kUnknown)) &&
+             (newState != OTAUpdateStateEnum::kIdle))
     {
-        mOtaRequestorDriver->HandleIdleStateExit(); 
+        mOtaRequestorDriver->HandleIdleStateExit();
     }
 
     mCurrentUpdateState = newState;
diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h
index 558f21ff5892e1..664bf4c9b97947 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.h
+++ b/src/app/clusters/ota-requestor/OTARequestor.h
@@ -187,7 +187,7 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
      * Callback to initialize states and server attributes in the CHIP context
      */
     static void InitState(intptr_t context);
-    
+
     /**
      * Map a CHIP_ERROR to an IdleStateReason enum type
      */
diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index b13d43892c0a53..475d6f15afe4c0 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -81,7 +81,7 @@ enum class SelectedTimer
 class OTARequestorDriver
 {
 public:
-    using OTAUpdateStateEnum   = app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum;
+    using OTAUpdateStateEnum  = app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum;
     using OTAChangeReasonEnum = app::Clusters::OtaSoftwareUpdateRequestor::OTAChangeReasonEnum;
 
     using ProviderLocationType = app::Clusters::OtaSoftwareUpdateRequestor::Structs::ProviderLocation::Type;

From b55d5126f71bb74d8303c2eea7e2c52243394a30 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Wed, 23 Mar 2022 15:33:48 -0700
Subject: [PATCH 10/14] Code review changes.

- Removed StopSelectedtimer()
- Removed extra Reset() call.
- Spelling errors.
---
 .../GenericOTARequestorDriver.cpp             | 25 ++++---------------
 .../ota-requestor/GenericOTARequestorDriver.h |  1 -
 .../clusters/ota-requestor/OTARequestor.cpp   |  4 +--
 .../ota-requestor/OTARequestorDriver.h        |  5 +---
 4 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index 4790cc3bdfb7e4..827156a65fb29f 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -85,7 +85,7 @@ void GenericOTARequestorDriver::Init(OTARequestorInterface * requestor, OTAImage
     else
     {
         // Start the first periodic query timer
-        StartPeriodicQueryTimer();
+        StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
     }
 }
 
@@ -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++;
 
@@ -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:
@@ -410,7 +411,6 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
     case OTAUpdateStateEnum::kDelayedOnApply:
         UpdateDiscontinued();
         mRequestor->CancelImageUpdate();
-        mRequestor->Reset();
         StartPeriodicQueryTimer();
         break;
     case OTAUpdateStateEnum::kRollingBack:
@@ -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
diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
index 18d98052b409cd..25bc2ba8a359cd 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.h
@@ -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);
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 3516de66bdda9f..561c29f5c7f582 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -658,6 +658,7 @@ 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);
@@ -665,8 +666,7 @@ void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeRe
         // 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();
     }
diff --git a/src/app/clusters/ota-requestor/OTARequestorDriver.h b/src/app/clusters/ota-requestor/OTARequestorDriver.h
index 475d6f15afe4c0..6ed7a74e0e208c 100644
--- a/src/app/clusters/ota-requestor/OTARequestorDriver.h
+++ b/src/app/clusters/ota-requestor/OTARequestorDriver.h
@@ -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;
@@ -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

From 03ae0435c0b4da22f34a57c3086fc059ac74ffb9 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Wed, 23 Mar 2022 23:33:45 -0700
Subject: [PATCH 11/14] Code review changes.

- Changed StartPeriodicQueryTimer() call to StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer).
---
 .../clusters/ota-requestor/GenericOTARequestorDriver.cpp    | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index 827156a65fb29f..3e8dc04fba7877 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -207,7 +207,7 @@ void GenericOTARequestorDriver::UpdateNotFound(UpdateNotFoundReason reason, Syst
     else
     {
         ChipLogProgress(SoftwareUpdate, "UpdateNotFound, not scheduling further retries");
-        StartPeriodicQueryTimer();
+        StartSelectedTimer(SelectedTimer::kPeriodicQueryTimer);
     }
 }
 
@@ -404,8 +404,6 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
         // Scheduling periodic timer here just in case.
         StartPeriodicQueryTimer();
         break;
-    case OTAUpdateStateEnum::kQuerying:
-    case OTAUpdateStateEnum::kDelayedOnQuery:
     case OTAUpdateStateEnum::kDownloading:
     case OTAUpdateStateEnum::kApplying:
     case OTAUpdateStateEnum::kDelayedOnApply:
@@ -413,6 +411,8 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
         mRequestor->CancelImageUpdate();
         StartPeriodicQueryTimer();
         break;
+    case OTAUpdateStateEnum::kQuerying:
+    case OTAUpdateStateEnum::kDelayedOnQuery:
     case OTAUpdateStateEnum::kRollingBack:
     case OTAUpdateStateEnum::kDelayedOnUserConsent:
         mRequestor->Reset();

From cc741ad3422a7661926993ed86e779e28109d891 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Thu, 24 Mar 2022 11:01:04 -0700
Subject: [PATCH 12/14] Code review changes.

- In WatchdogTimerHandler(), just cancel download and reset regardless of which state OTA-R is stuck in.
- In SendQueryImage(), remove duplicate call to StartSelectedTimer(SelectedTimer::kWatchdogTimer)
---
 .../GenericOTARequestorDriver.cpp             | 35 ++++++-------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index 3e8dc04fba7877..798053d150f014 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -342,8 +342,8 @@ void GenericOTARequestorDriver::SendQueryImage()
     UpdateCancelled();
 
     // 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
-    StartSelectedTimer(SelectedTimer::kWatchdogTimer);
+    // TriggerImmediateQueryInternal() will cause the state to change from kIdle, which will start
+    // the Watchdog timer.  (Clean this up with Issue#16151)
 
     mProviderRetryCount++;
 
@@ -396,29 +396,14 @@ void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer
 
     OTAUpdateStateEnum currentState = mRequestor->GetCurrentUpdateState();
 
-    switch (currentState)
-    {
-    case OTAUpdateStateEnum::kIdle:
-    case OTAUpdateStateEnum::kUnknown:
-        // 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::kDownloading:
-    case OTAUpdateStateEnum::kApplying:
-    case OTAUpdateStateEnum::kDelayedOnApply:
-        UpdateDiscontinued();
-        mRequestor->CancelImageUpdate();
-        StartPeriodicQueryTimer();
-        break;
-    case OTAUpdateStateEnum::kQuerying:
-    case OTAUpdateStateEnum::kDelayedOnQuery:
-    case OTAUpdateStateEnum::kRollingBack:
-    case OTAUpdateStateEnum::kDelayedOnUserConsent:
-        mRequestor->Reset();
-        StartPeriodicQueryTimer();
-        break;
-    }
+    ChipLogError(SoftwareUpdate, "Watchdog timer detects state stuck at %u. Cancelling download and resetting state.",
+                 to_underlying(currentState));
+
+    // Something went wrong and OTA requestor is stuck in a non-idle state for too long.
+    // Let's just cancel download, reset state, and re-start periodic query timer.
+    UpdateDiscontinued();
+    mRequestor->CancelImageUpdate();
+    StartPeriodicQueryTimer();
 }
 
 void GenericOTARequestorDriver::StartWatchdogTimer()

From 6102e1619a9e4b70175f6fd3f46f7142fccb683c Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Thu, 24 Mar 2022 13:03:09 -0700
Subject: [PATCH 13/14] - Resolve compilation error due to un-used variable. -
 Remove debug log.

---
 .../clusters/ota-requestor/GenericOTARequestorDriver.cpp    | 6 +-----
 src/app/clusters/ota-requestor/OTARequestor.cpp             | 3 +++
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
index 798053d150f014..a94a2aa2348fa2 100644
--- a/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
+++ b/src/app/clusters/ota-requestor/GenericOTARequestorDriver.cpp
@@ -392,12 +392,8 @@ void GenericOTARequestorDriver::StopPeriodicQueryTimer()
 
 void GenericOTARequestorDriver::WatchdogTimerHandler(System::Layer * systemLayer, void * appState)
 {
-    ChipLogProgress(SoftwareUpdate, "Watchdog timer handler is invoked");
-
-    OTAUpdateStateEnum currentState = mRequestor->GetCurrentUpdateState();
-
     ChipLogError(SoftwareUpdate, "Watchdog timer detects state stuck at %u. Cancelling download and resetting state.",
-                 to_underlying(currentState));
+                 to_underlying(mRequestor->GetCurrentUpdateState()));
 
     // Something went wrong and OTA requestor is stuck in a non-idle state for too long.
     // Let's just cancel download, reset state, and re-start periodic query timer.
diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index 561c29f5c7f582..d798aa4ce35be5 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -638,6 +638,9 @@ IdleStateReason OTARequestor::MapErrorToIdleStateReason(CHIP_ERROR error)
 
 void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
 {
+    ChipLogError(SoftwareUpdate, "//is: RecordNewUpdateState newState %d, reason %d", to_underlying(newState),
+                 to_underlying(reason));
+
     // Set server UpdateState attribute
     OtaRequestorServerSetUpdateState(newState);
 

From 16735151777e3edb6743d28274e47845da061920 Mon Sep 17 00:00:00 2001
From: "Irene Siu (Apple)" <98558756+isiu-apple@users.noreply.github.com>
Date: Thu, 24 Mar 2022 13:24:49 -0700
Subject: [PATCH 14/14] Remove remaining debug logs.

---
 src/app/clusters/ota-requestor/OTARequestor.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp
index d798aa4ce35be5..561c29f5c7f582 100644
--- a/src/app/clusters/ota-requestor/OTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/OTARequestor.cpp
@@ -638,9 +638,6 @@ IdleStateReason OTARequestor::MapErrorToIdleStateReason(CHIP_ERROR error)
 
 void OTARequestor::RecordNewUpdateState(OTAUpdateStateEnum newState, OTAChangeReasonEnum reason, CHIP_ERROR error)
 {
-    ChipLogError(SoftwareUpdate, "//is: RecordNewUpdateState newState %d, reason %d", to_underlying(newState),
-                 to_underlying(reason));
-
     // Set server UpdateState attribute
     OtaRequestorServerSetUpdateState(newState);