Address code review comments

- Add missing idle updates
- Fix logic for setting UpdateStateProgress
- Remove OtaRequestorServer class and define the setters as global functions

src/app/clusters/ota-requestor/OTARequestor.cpp

@@ -20,7 +20,6 @@
  * OTA Requestor logic is contained in this class.
  */
 
-#include <app-common/zap-generated/attributes/Accessors.h>
 #include <lib/core/CHIPEncoding.h>
 #include <platform/CHIPDeviceLayer.h>
 #include <platform/OTAImageProcessor.h>
@@ -90,12 +89,12 @@ static void LogApplyUpdateResponse(const ApplyUpdateResponse::DecodableType & re
 
 static void SetUpdateStateAttribute(OTAUpdateStateEnum state)
 {
-    OtaRequestorServer::GetInstance().SetUpdateState(state);
+    OtaRequestorServerSetUpdateState(state);
 
     // The UpdateStateProgress attribute only applies to the querying state
     if (state != OTAUpdateStateEnum::kQuerying)
     {
-        OtaRequestorServer::GetInstance().SetUpdateStateProgress(0);
+        OtaRequestorServerSetUpdateStateProgress(0);
     }
 }
 
@@ -302,6 +301,7 @@ void OTARequestor::OnConnected(void * context, OperationalDeviceProxy * devicePr
         {
             ChipLogError(SoftwareUpdate, "Failed to start download: %" CHIP_ERROR_FORMAT, err.Format());
             requestorCore->mOtaRequestorDriver->HandleError(OTAUpdateStateEnum::kDownloading, err);
+            SetUpdateStateAttribute(OTAUpdateStateEnum::kIdle);
             return;
         }
 
@@ -315,6 +315,7 @@ void OTARequestor::OnConnected(void * context, OperationalDeviceProxy * devicePr
         {
             ChipLogError(SoftwareUpdate, "Failed to send ApplyUpdate command: %" CHIP_ERROR_FORMAT, err.Format());
             requestorCore->mOtaRequestorDriver->HandleError(OTAUpdateStateEnum::kApplying, err);
+            SetUpdateStateAttribute(OTAUpdateStateEnum::kIdle);
             return;
         }
 
@@ -394,7 +395,7 @@ void OTARequestor::OnDownloadStateChanged(OTADownloader::State state)
 
 void OTARequestor::OnUpdateProgressChanged(uint8_t percent)
 {
-    OtaRequestorServer::GetInstance().SetUpdateStateProgress(percent);
+    OtaRequestorServerSetUpdateStateProgress(percent);
 }
 
 CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & deviceProxy)

src/app/clusters/ota-requestor/OTARequestor.h

@@ -80,8 +80,8 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
         mOtaRequestorDriver = driver;
         mBdxDownloader      = downloader;
 
-        OtaRequestorServer::GetInstance().SetUpdateState(app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum::kIdle);
-        OtaRequestorServer::GetInstance().SetUpdateStateProgress(0);
+        OtaRequestorServerSetUpdateState(app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum::kIdle);
+        OtaRequestorServerSetUpdateStateProgress(0);
     }
 
     /**

src/app/clusters/ota-requestor/ota-requestor-server.cpp

@@ -20,10 +20,8 @@
  * to the OTA Requestor object that handles them
  */
 
-#include <app-common/zap-generated/attributes/Accessors.h>
 #include <app/AttributeAccessInterface.h>
 #include <app/clusters/ota-requestor/ota-requestor-server.h>
-#include <app/util/af.h>
 #include <app/util/attribute-storage.h>
 #include <platform/OTARequestorInterface.h>
 
@@ -81,19 +79,9 @@ CHIP_ERROR OtaSoftwareUpdateRequestorAttrAccess::Write(const ConcreteDataAttribu
 
 } // namespace
 
-namespace chip {
-
 // -----------------------------------------------------------------------------
-// OtaRequestorServer implementation
-
-static OtaRequestorServer sInstance;
-
-OtaRequestorServer & OtaRequestorServer::GetInstance(void)
-{
-    return sInstance;
-}
-
-EmberAfStatus OtaRequestorServer::SetUpdateState(OTAUpdateStateEnum value)
+// Global functions
+EmberAfStatus OtaRequestorServerSetUpdateState(OTAUpdateStateEnum value)
 {
     EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
 
@@ -114,7 +102,7 @@ EmberAfStatus OtaRequestorServer::SetUpdateState(OTAUpdateStateEnum value)
     return status;
 }
 
-EmberAfStatus OtaRequestorServer::SetUpdateStateProgress(uint8_t value)
+EmberAfStatus OtaRequestorServerSetUpdateStateProgress(uint8_t value)
 {
     EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
 
@@ -124,21 +112,16 @@ EmberAfStatus OtaRequestorServer::SetUpdateStateProgress(uint8_t value)
         app::DataModel::Nullable<uint8_t> currentValue;
         status = Attributes::UpdateStateProgress::Get(endpoint, currentValue);
         VerifyOrDie(EMBER_ZCL_STATUS_SUCCESS == status);
-        if (!currentValue.IsNull())
+        if (currentValue.IsNull() || currentValue.Value() != value)
         {
-            if (currentValue.Value() != value)
-            {
-                status = Attributes::UpdateStateProgress::Set(endpoint, value);
-                VerifyOrDie(EMBER_ZCL_STATUS_SUCCESS == status);
-            }
+            status = Attributes::UpdateStateProgress::Set(endpoint, value);
+            VerifyOrDie(EMBER_ZCL_STATUS_SUCCESS == status);
         }
     }
 
     return status;
 }
 
-} // namespace chip
-
 // -----------------------------------------------------------------------------
 // Callbacks implementation
 

src/app/clusters/ota-requestor/ota-requestor-server.h

@@ -16,15 +16,9 @@
  *    limitations under the License.
  */
 
-namespace chip {
+#pragma once
 
-class OtaRequestorServer
-{
-public:
-    static OtaRequestorServer & GetInstance(void);
+#include <app-common/zap-generated/attributes/Accessors.h>
 
-    EmberAfStatus SetUpdateState(chip::app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum value);
-    EmberAfStatus SetUpdateStateProgress(uint8_t value);
-};
-
-} // namespace chip
+EmberAfStatus OtaRequestorServerSetUpdateState(chip::app::Clusters::OtaSoftwareUpdateRequestor::OTAUpdateStateEnum value);
+EmberAfStatus OtaRequestorServerSetUpdateStateProgress(uint8_t value);