[nrfconnect] Fix OTA initialization order (#16609)

Recent OTA changes impose an assumption that OTA Requestor
must be initialized prior to the OTA Requestor driver.

Fix the order and extract OTA requestor initialization to
a common function to reduce code duplication in the future.

.github/workflows/examples-nrfconnect.yaml

@@ -111,7 +111,7 @@ jobs:
                     /tmp/bloat_reports/
             - name: Build example nRF Connect SDK Lighting App on nRF52840 DK with RPC
               if: github.event_name == 'push' || steps.changed_paths.outputs.nrfconnect == 'true'
-              timeout-minutes: 10
+              timeout-minutes: 20
               run: |
                   scripts/examples/nrfconnect_example.sh lighting-app nrf52840dk_nrf52840 -DOVERLAY_CONFIG=rpc.overlay
                   .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \

examples/all-clusters-app/nrfconnect/CMakeLists.txt

@@ -73,8 +73,11 @@ target_sources(app PRIVATE
                ${GEN_DIR}/all-clusters-app/zap-generated/IMClusterCommandHandler.cpp
                ${NRFCONNECT_COMMON}/util/LEDWidget.cpp)
 
-
 chip_configure_data_model(app
     INCLUDE_SERVER
     ZAP_FILE ${ALL_CLUSTERS_COMMON_DIR}/all-clusters-app.zap
 )
+
+if(CONFIG_CHIP_OTA_REQUESTOR)
+    target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/OTAUtil.cpp)
+endif()

examples/all-clusters-app/nrfconnect/main/AppTask.cpp

@@ -31,6 +31,10 @@
 #include <credentials/DeviceAttestationCredsProvider.h>
 #include <credentials/examples/DeviceAttestationCredsExample.h>
 
+#if CONFIG_CHIP_OTA_REQUESTOR
+#include "OTAUtil.h"
+#endif
+
 #include <dk_buttons_and_leds.h>
 #include <logging/log.h>
 #include <zephyr.h>
@@ -127,9 +131,11 @@ CHIP_ERROR AppTask::Init()
     // Initialize CHIP server
     SetDeviceAttestationCredentialsProvider(Examples::GetExampleDACProvider());
     ReturnErrorOnFailure(chip::Server::GetInstance().Init());
+#if CONFIG_CHIP_OTA_REQUESTOR
+    InitBasicOTARequestor();
+#endif
     ConfigurationMgr().LogDeviceConfig();
     PrintOnboardingCodes(chip::RendezvousInformationFlag(chip::RendezvousInformationFlag::kBLE));
-    InitOTARequestor();
 
     // Add CHIP event handler and start CHIP thread.
     // Note that all the initialization code should happen prior to this point to avoid data races
@@ -144,18 +150,6 @@ CHIP_ERROR AppTask::Init()
     return err;
 }
 
-void AppTask::InitOTARequestor()
-{
-#if CONFIG_CHIP_OTA_REQUESTOR
-    OTAImageProcessorNrf::Get().SetOTADownloader(&mBDXDownloader);
-    mBDXDownloader.SetImageProcessorDelegate(&OTAImageProcessorNrf::Get());
-    mOTARequestorDriver.Init(&mOTARequestor, &OTAImageProcessorNrf::Get());
-    mOTARequestorStorage.Init(chip::Server::GetInstance().GetPersistentStorage());
-    mOTARequestor.Init(chip::Server::GetInstance(), mOTARequestorStorage, mOTARequestorDriver, mBDXDownloader);
-    chip::SetRequestorInstance(&mOTARequestor);
-#endif
-}
-
 CHIP_ERROR AppTask::StartApp()
 {
     ReturnErrorOnFailure(Init());

examples/all-clusters-app/nrfconnect/main/include/AppTask.h

@@ -19,14 +19,6 @@
 
 #include <platform/CHIPDeviceLayer.h>
 
-#if CONFIG_CHIP_OTA_REQUESTOR
-#include "OTAUtil.h"
-#include <app/clusters/ota-requestor/BDXDownloader.h>
-#include <app/clusters/ota-requestor/DefaultOTARequestorStorage.h>
-#include <app/clusters/ota-requestor/GenericOTARequestorDriver.h>
-#include <app/clusters/ota-requestor/OTARequestor.h>
-#endif
-
 struct k_timer;
 class AppEvent;
 class LEDWidget;
@@ -51,7 +43,6 @@ class AppTask
 
     CHIP_ERROR Init();
     void DispatchEvent(AppEvent * aEvent);
-    void InitOTARequestor();
 
     // statics needed to interact with zephyr C API
     static void CancelTimer(void);
@@ -72,11 +63,4 @@ class AppTask
     bool mIsThreadProvisioned{ false };
     bool mIsThreadEnabled{ false };
     bool mHaveBLEConnections{ false };
-
-#if CONFIG_CHIP_OTA_REQUESTOR
-    chip::DefaultOTARequestorStorage mOTARequestorStorage;
-    chip::DeviceLayer::GenericOTARequestorDriver mOTARequestorDriver;
-    chip::BDXDownloader mBDXDownloader;
-    chip::OTARequestor mOTARequestor;
-#endif
 };

examples/lighting-app/nrfconnect/CMakeLists.txt

@@ -100,6 +100,10 @@ chip_configure_data_model(app
     GEN_DIR ${GEN_DIR}/lighting-app/zap-generated
 )
 
+if(CONFIG_CHIP_OTA_REQUESTOR)
+    target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/OTAUtil.cpp)
+endif()
+
 if(BUILD_WITH_DFU STREQUAL "BLE")
     target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/DFUOverSMP.cpp)
 endif()

examples/lighting-app/nrfconnect/main/AppTask.cpp

@@ -41,10 +41,6 @@
 
 #if CONFIG_CHIP_OTA_REQUESTOR
 #include "OTAUtil.h"
-#include <app/clusters/ota-requestor/BDXDownloader.h>
-#include <app/clusters/ota-requestor/DefaultOTARequestorStorage.h>
-#include <app/clusters/ota-requestor/GenericOTARequestorDriver.h>
-#include <app/clusters/ota-requestor/OTARequestor.h>
 #endif
 
 #include <dk_buttons_and_leds.h>
@@ -85,13 +81,6 @@ bool sIsThreadProvisioned = false;
 bool sIsThreadEnabled     = false;
 bool sHaveBLEConnections  = false;
 
-#if CONFIG_CHIP_OTA_REQUESTOR
-DefaultOTARequestorStorage sRequestorStorage;
-GenericOTARequestorDriver sOTARequestorDriver;
-chip::BDXDownloader sBDXDownloader;
-chip::OTARequestor sOTARequestor;
-#endif
-
 } // namespace
 
 AppTask AppTask::sAppTask;
@@ -178,9 +167,11 @@ CHIP_ERROR AppTask::Init()
 
     // Initialize CHIP server
     SetDeviceAttestationCredentialsProvider(Examples::GetExampleDACProvider());
-    InitOTARequestor();
     chip::app::DnssdServer::Instance().SetExtendedDiscoveryTimeoutSecs(kExtDiscoveryTimeoutSecs);
     ReturnErrorOnFailure(chip::Server::GetInstance().Init());
+#if CONFIG_CHIP_OTA_REQUESTOR
+    InitBasicOTARequestor();
+#endif
     ConfigurationMgr().LogDeviceConfig();
     PrintOnboardingCodes(chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE));
 
@@ -198,18 +189,6 @@ CHIP_ERROR AppTask::Init()
     return err;
 }
 
-void AppTask::InitOTARequestor()
-{
-#if CONFIG_CHIP_OTA_REQUESTOR
-    OTAImageProcessorNrf::Get().SetOTADownloader(&sBDXDownloader);
-    sBDXDownloader.SetImageProcessorDelegate(&OTAImageProcessorNrf::Get());
-    sOTARequestorDriver.Init(&sOTARequestor, &OTAImageProcessorNrf::Get());
-    sRequestorStorage.Init(Server::GetInstance().GetPersistentStorage());
-    sOTARequestor.Init(Server::GetInstance(), sRequestorStorage, sOTARequestorDriver, sBDXDownloader);
-    chip::SetRequestorInstance(&sOTARequestor);
-#endif
-}
-
 CHIP_ERROR AppTask::StartApp()
 {
     ReturnErrorOnFailure(Init());

examples/lighting-app/nrfconnect/main/include/AppTask.h

@@ -56,7 +56,6 @@ class AppTask
 
     friend AppTask & GetAppTask(void);
     CHIP_ERROR Init();
-    void InitOTARequestor();
 
     static void ActionInitiated(LightingManager::Action_t aAction, int32_t aActor);
     static void ActionCompleted(LightingManager::Action_t aAction, int32_t aActor);

examples/lock-app/nrfconnect/CMakeLists.txt

@@ -91,6 +91,10 @@ chip_configure_data_model(app
     ZAP_FILE ${CMAKE_CURRENT_SOURCE_DIR}/../lock-common/lock-app.zap
 )
 
+if(CONFIG_CHIP_OTA_REQUESTOR)
+    target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/OTAUtil.cpp)
+endif()
+
 if(BUILD_WITH_DFU STREQUAL "BLE")
     target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/DFUOverSMP.cpp)
 endif()

examples/lock-app/nrfconnect/main/AppTask.cpp

@@ -37,10 +37,6 @@
 
 #if CONFIG_CHIP_OTA_REQUESTOR
 #include "OTAUtil.h"
-#include <app/clusters/ota-requestor/BDXDownloader.h>
-#include <app/clusters/ota-requestor/DefaultOTARequestorStorage.h>
-#include <app/clusters/ota-requestor/GenericOTARequestorDriver.h>
-#include <app/clusters/ota-requestor/OTARequestor.h>
 #endif
 
 #include <dk_buttons_and_leds.h>
@@ -74,13 +70,6 @@ bool sIsThreadProvisioned = false;
 bool sIsThreadEnabled     = false;
 bool sHaveBLEConnections  = false;
 
-#if CONFIG_CHIP_OTA_REQUESTOR
-DefaultOTARequestorStorage sRequestorStorage;
-GenericOTARequestorDriver sOTARequestorDriver;
-chip::BDXDownloader sBDXDownloader;
-chip::OTARequestor sOTARequestor;
-#endif
-
 } // namespace
 
 AppTask AppTask::sAppTask;
@@ -160,8 +149,10 @@ CHIP_ERROR AppTask::Init()
 
     // Initialize CHIP server
     SetDeviceAttestationCredentialsProvider(Examples::GetExampleDACProvider());
-    InitOTARequestor();
     ReturnErrorOnFailure(chip::Server::GetInstance().Init());
+#if CONFIG_CHIP_OTA_REQUESTOR
+    InitBasicOTARequestor();
+#endif
     ConfigurationMgr().LogDeviceConfig();
     PrintOnboardingCodes(chip::RendezvousInformationFlags(chip::RendezvousInformationFlag::kBLE));
 
@@ -179,18 +170,6 @@ CHIP_ERROR AppTask::Init()
     return err;
 }
 
-void AppTask::InitOTARequestor()
-{
-#if CONFIG_CHIP_OTA_REQUESTOR
-    OTAImageProcessorNrf::Get().SetOTADownloader(&sBDXDownloader);
-    sBDXDownloader.SetImageProcessorDelegate(&OTAImageProcessorNrf::Get());
-    sOTARequestorDriver.Init(&sOTARequestor, &OTAImageProcessorNrf::Get());
-    sRequestorStorage.Init(Server::GetInstance().GetPersistentStorage());
-    sOTARequestor.Init(Server::GetInstance(), sRequestorStorage, sOTARequestorDriver, sBDXDownloader);
-    chip::SetRequestorInstance(&sOTARequestor);
-#endif
-}
-
 CHIP_ERROR AppTask::StartApp()
 {
     ReturnErrorOnFailure(Init());

examples/lock-app/nrfconnect/main/include/AppTask.h

@@ -44,7 +44,6 @@ class AppTask
     friend AppTask & GetAppTask(void);
 
     CHIP_ERROR Init();
-    void InitOTARequestor();
 
     static void ActionInitiated(BoltLockManager::Action_t aAction, int32_t aActor);
     static void ActionCompleted(BoltLockManager::Action_t aAction, int32_t aActor);

examples/platform/nrfconnect/util/OTAUtil.cpp

@@ -0,0 +1,58 @@
+/*
+ *    Copyright (c) 2022 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#include <app/clusters/ota-requestor/BDXDownloader.h>
+#include <app/clusters/ota-requestor/DefaultOTARequestorStorage.h>
+#include <app/clusters/ota-requestor/GenericOTARequestorDriver.h>
+#include <app/clusters/ota-requestor/OTARequestor.h>
+#include <app/server/Server.h>
+#include <platform/nrfconnect/OTAImageProcessorImpl.h>
+
+using namespace chip;
+using namespace chip::DeviceLayer;
+
+namespace {
+
+DefaultOTARequestorStorage sOTARequestorStorage;
+GenericOTARequestorDriver sOTARequestorDriver;
+chip::BDXDownloader sBDXDownloader;
+chip::OTARequestor sOTARequestor;
+
+} // namespace
+
+// compile-time factory method
+OTAImageProcessorImpl & GetOTAImageProcessor()
+{
+#if CONFIG_PM_DEVICE && CONFIG_NORDIC_QSPI_NOR
+    static ExtFlashHandler sQSPIHandler;
+    static OTAImageProcessorImplPMDevice sOTAImageProcessor{ sQSPIHandler };
+#else
+    static OTAImageProcessorImpl sOTAImageProcessor;
+#endif
+    return sOTAImageProcessor;
+}
+
+void InitBasicOTARequestor()
+{
+    OTAImageProcessorImpl & imageProcessor = GetOTAImageProcessor();
+    imageProcessor.SetOTADownloader(&sBDXDownloader);
+    sBDXDownloader.SetImageProcessorDelegate(&imageProcessor);
+    sOTARequestorStorage.Init(Server::GetInstance().GetPersistentStorage());
+    sOTARequestor.Init(Server::GetInstance(), sOTARequestorStorage, sOTARequestorDriver, sBDXDownloader);
+    sOTARequestorDriver.Init(&sOTARequestor, &imageProcessor);
+    chip::SetRequestorInstance(&sOTARequestor);
+}

examples/platform/nrfconnect/util/include/OTAUtil.h

@@ -17,21 +17,26 @@
 
 #pragma once
 
-#include <platform/nrfconnect/OTAImageProcessorImpl.h>
+namespace chip {
+namespace DeviceLayer {
+class OTAImageProcessorImpl;
+} // namespace DeviceLayer
+} // namespace chip
 
-#ifdef CONFIG_CHIP_OTA_REQUESTOR
-namespace OTAImageProcessorNrf {
-// compile-time factory method
-inline chip::DeviceLayer::OTAImageProcessorImpl & Get()
-{
-#if CONFIG_PM_DEVICE && CONFIG_NORDIC_QSPI_NOR
-    static chip::DeviceLayer::ExtFlashHandler sQSPIHandler;
-    static chip::DeviceLayer::OTAImageProcessorImplPMDevice sOTAImageProcessor{ sQSPIHandler };
-#else
-    static chip::DeviceLayer::OTAImageProcessorImpl sOTAImageProcessor;
-#endif
-    return sOTAImageProcessor;
-}
-} // namespace OTAImageProcessorNrf
+/**
+ * Select recommended OTA image processor implementation.
+ *
+ * If the application uses QSPI external flash and enables API for controlling
+ * power states of peripherals, select the implementation that automatically
+ * powers off the external flash when no longer needed. Otherwise, select the
+ * most basic implementation.
+ */
+chip::DeviceLayer::OTAImageProcessorImpl & GetOTAImageProcessor();
 
-#endif
+/** Initialize basic OTA requestor.
+ *
+ * Initialize all necessary components and start the OTA requestor state machine.
+ * Assume that the device is not able to ask a user for consent before applying
+ * an update so the confirmation must be done on the OTA provider side.
+ */
+void InitBasicOTARequestor();

examples/pump-app/nrfconnect/CMakeLists.txt

@@ -91,6 +91,10 @@ chip_configure_data_model(app
     ZAP_FILE ${CMAKE_CURRENT_SOURCE_DIR}/../pump-common/pump-app.zap
 )
 
+if(CONFIG_CHIP_OTA_REQUESTOR)
+    target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/OTAUtil.cpp)
+endif()
+
 if(BUILD_WITH_DFU STREQUAL "BLE")
     target_sources(app PRIVATE ${NRFCONNECT_COMMON}/util/DFUOverSMP.cpp)
 endif()