[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()