Address review comments

Author: mkardous-silabs

examples/platform/silabs/MatterConfig.cpp

@@ -203,6 +203,8 @@ void SilabsMatterConfig::AppInit()
 
 CHIP_ERROR SilabsMatterConfig::InitMatter(const char * appName)
 {
+    using namespace chip::DeviceLayer::Silabs;
+
     CHIP_ERROR err;
 #ifdef SL_WIFI
     // Because OpenThread needs to use memory allocation during its Key operations, we initialize the memory management for thread
@@ -234,10 +236,9 @@ CHIP_ERROR SilabsMatterConfig::InitMatter(const char * appName)
     ReturnErrorOnFailure(InitWiFi());
 
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
-    ReturnErrorOnFailure(DeviceLayer::Silabs::WifiSleepManager::GetInstance().Init(
-        &DeviceLayer::Silabs::WifiInterface::GetInstance(), &DeviceLayer::Silabs::WifiInterface::GetInstance()));
+    ReturnErrorOnFailure(WifiSleepManager::GetInstance().Init(&WifiInterface::GetInstance(), &WifiInterface::GetInstance()));
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
-#endif
+#endif // SL_WIFI
 
     ReturnErrorOnFailure(PlatformMgr().InitChipStack());
 

src/platform/silabs/ConfigurationManagerImpl.cpp

@@ -29,6 +29,7 @@
 #include <platform/internal/GenericConfigurationManagerImpl.ipp>
 #include <platform/silabs/SilabsConfig.h>
 #include <platform/silabs/platformAbstraction/SilabsPlatform.h>
+#include <platform/silabs/wifi/icd/WifiSleepManager.h>
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION
 #include <platform/silabs/wifi/WifiInterface.h>

src/platform/silabs/SiWx917/OTAImageProcessorImpl.cpp

@@ -162,7 +162,7 @@ void OTAImageProcessorImpl::HandlePrepareDownload(intptr_t context)
 
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
     // Setting the device in high performance - no-sleep mode during OTA tranfer
-    DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformance();
+    DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition();
 #endif /* CHIP_CONFIG_ENABLE_ICD_SERVER*/
 
     imageProcessor->mDownloader->OnPreparedForDownload(CHIP_NO_ERROR);
@@ -220,7 +220,7 @@ void OTAImageProcessorImpl::HandleApply(intptr_t context)
 
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
     // Setting the device is in high performace - no-sleepy mode before soft reset as soft reset is not happening in sleep mode
-    DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformance();
+    DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition();
 #endif /* CHIP_CONFIG_ENABLE_ICD_SERVER*/
 
     if (mReset)

src/platform/silabs/tests/args.gni

@@ -12,7 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import("//build/chip/tests.gni")
+import("//build_overrides/chip.gni")
+import("${chip_root}/build/chip/tests.gni")
 
 declare_args() {
   # Build argument to enable Silabs specific unit tests

src/platform/silabs/wifi/BUILD.gn

@@ -15,6 +15,7 @@
 import("//build_overrides/chip.gni")
 import("//build_overrides/lwip.gni")
 import("${chip_root}/src/platform/device.gni")
+import("${chip_root}/src/platform/silabs/tests/args.gni")
 import("${chip_root}/third_party/silabs/efr32_sdk.gni")
 import("${chip_root}/third_party/silabs/silabs_board.gni")
 
@@ -35,7 +36,8 @@ declare_args() {
   sl_enable_wifi_debug = false
 }
 
-if (chip_enable_wifi && !wifi_soc) {
+# Workaround to prevent the assert when we are building the unit tests
+if (chip_enable_wifi && !wifi_soc && !sl_build_unit_tests) {
   assert(use_rs9116 || use_wf200 || use_SiWx917)
   import("${chip_root}/src/platform/silabs/wifi/args.gni")
 
@@ -100,7 +102,7 @@ config("wifi-platform-config") {
   }
 }
 
-source_set("wifi-state-provider") {
+source_set("state-provider") {
   sources = [ "${silabs_platform_dir}/wifi/WifiStateProvider.h" ]
 
   public_configs = [ ":wifi-platform-config" ]
@@ -116,7 +118,7 @@ source_set("wifi-platform") {
   public_configs = [ ":wifi-platform-config" ]
 
   public_deps = [
-    ":wifi-state-provider",
+    ":state-provider",
     "${chip_root}/src/app/icd/server:icd-server-config",
     "${chip_root}/src/inet",
     "${chip_root}/src/lib/support",

src/platform/silabs/wifi/SiWx/WifiInterfaceImpl.cpp

@@ -489,6 +489,9 @@ void WiseconnectWifiInterface::MatterWifiTask(void * arg)
     VerifyOrReturn(status == SL_STATUS_OK,
                    ChipLogError(DeviceLayer, "MatterWifiTask: SiWxPlatformInit failed: 0x%lx", static_cast<uint32_t>(status)));
 
+    // Remove High performance request after the device is initialized
+    // chip::DeviceLayer::Silabs::WifiSleepManager::GetInstance().RemoveHighPerformanceRequest();
+
     WifiInterfaceImpl::GetInstance().NotifyWifiTaskInitialized();
 
     ChipLogDetail(DeviceLayer, "MatterWifiTask: starting event loop");
@@ -509,6 +512,9 @@ CHIP_ERROR WifiInterfaceImpl::InitWiFiStack(void)
 {
     sl_status_t status = SL_STATUS_OK;
 
+    // Force the device to high performance mode during the init sequence.
+    chip::DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformanceWithoutTransition();
+
     status = sl_net_init(SL_NET_WIFI_CLIENT_INTERFACE, &config, &wifi_client_context, nullptr);
     VerifyOrReturnError(status == SL_STATUS_OK, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "sl_net_init failed: %lx", status));
 
@@ -705,7 +711,7 @@ sl_status_t WifiInterfaceImpl::JoinWifiNetwork(void)
 // To avoid IOP issues, it is recommended to enable high-performance mode before joining the network.
 // TODO: Remove this once the IOP issue related to power save mode switching is fixed in the Wi-Fi SDK.
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
-    chip::DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformance();
+    chip::DeviceLayer::Silabs::WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition();
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 
     status = sl_net_up(SL_NET_WIFI_CLIENT_INTERFACE, SL_NET_DEFAULT_WIFI_CLIENT_PROFILE_ID);

src/platform/silabs/wifi/icd/BUILD.gn

@@ -25,17 +25,12 @@ source_set("wifi-sleep-manager") {
   sources = [
     "${chip_root}/src/platform/silabs/wifi/icd/WifiSleepManager.cpp",
     "${chip_root}/src/platform/silabs/wifi/icd/WifiSleepManager.h",
-
-    # TODO: Remove duplicated source files - This is a temporary workaround until the efr32_sdk.gni assert is refactored
-    #       This source is necessary because we can't use anything in the src/platform/silabs/wifi/BUILD.gn due to the efr32_sdk.gni import
-    #       The import causes an assert when building unit-tests due the silabs_board not being defined
-    #
-    "${chip_root}/src/platform/silabs/wifi/WifiStateProvider.h",
   ]
 
   public_deps = [
     ":power-save-interface",
     "${chip_root}/src/app/icd/server:configuration-data",
     "${chip_root}/src/lib/core",
+    "${chip_root}/src/platform/silabs/wifi:state-provider",
   ]
 }

src/platform/silabs/wifi/icd/WifiSleepManager.cpp

@@ -37,15 +37,18 @@ CHIP_ERROR WifiSleepManager::Init(PowerSaveInterface * platformInterface, WifiSt
     return VerifyAndTransitionToLowPowerMode(PowerEvent::kGenericEvent);
 }
 
-CHIP_ERROR WifiSleepManager::RequestHighPerformance()
+CHIP_ERROR WifiSleepManager::RequestHighPerformance(bool triggerTransition)
 {
     VerifyOrReturnError(mHighPerformanceRequestCounter < std::numeric_limits<uint8_t>::max(), CHIP_ERROR_INTERNAL,
                         ChipLogError(DeviceLayer, "High performance request counter overflow"));
 
     mHighPerformanceRequestCounter++;
 
-    // We don't do the mHighPerformanceRequestCounter check here; the check is in the VerifyAndTransitionToLowPowerMode function
-    ReturnErrorOnFailure(VerifyAndTransitionToLowPowerMode(PowerEvent::kGenericEvent));
+    if (triggerTransition)
+    {
+        // We don't do the mHighPerformanceRequestCounter check here; the check is in the VerifyAndTransitionToLowPowerMode function
+        ReturnErrorOnFailure(VerifyAndTransitionToLowPowerMode(PowerEvent::kGenericEvent));
+    }
 
     return CHIP_NO_ERROR;
 }
@@ -90,8 +93,8 @@ CHIP_ERROR WifiSleepManager::HandlePowerEvent(PowerEvent event)
 
 CHIP_ERROR WifiSleepManager::VerifyAndTransitionToLowPowerMode(PowerEvent event)
 {
-    VerifyOrDieWithMsg(mWifiStateProvider != nullptr, DeviceLayer, "WifiSleepManager is not initialized");
-    VerifyOrDieWithMsg(mPowerSaveInterface != nullptr, DeviceLayer, "WifiSleepManager is not initialized");
+    VerifyOrDieWithMsg(mWifiStateProvider != nullptr, DeviceLayer, "WifiStateProvider is not initialized");
+    VerifyOrDieWithMsg(mPowerSaveInterface != nullptr, DeviceLayer, "PowerSaveInterface is not initialized");
 
     ReturnErrorOnFailure(HandlePowerEvent(event));
 
@@ -117,6 +120,8 @@ CHIP_ERROR WifiSleepManager::VerifyAndTransitionToLowPowerMode(PowerEvent event)
 
 CHIP_ERROR WifiSleepManager::ConfigureDTIMBasedSleep()
 {
+    VerifyOrDieWithMsg(mPowerSaveInterface != nullptr, DeviceLayer, "PowerSaveInterface is not initialized");
+
     ReturnLogErrorOnFailure(mPowerSaveInterface->ConfigureBroadcastFilter(false));
 
     // Allowing the device to go to sleep must be the last actions to avoid configuration failures.
@@ -128,12 +133,16 @@ CHIP_ERROR WifiSleepManager::ConfigureDTIMBasedSleep()
 
 CHIP_ERROR WifiSleepManager::ConfigureDeepSleep()
 {
+    VerifyOrDieWithMsg(mPowerSaveInterface != nullptr, DeviceLayer, "PowerSaveInterface is not initialized");
+
     ReturnLogErrorOnFailure(mPowerSaveInterface->ConfigurePowerSave(PowerSaveInterface::PowerSaveConfiguration::kDeepSleep, 0));
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR WifiSleepManager::ConfigureHighPerformance()
 {
+    VerifyOrDieWithMsg(mPowerSaveInterface != nullptr, DeviceLayer, "PowerSaveInterface is not initialized");
+
     ReturnLogErrorOnFailure(
         mPowerSaveInterface->ConfigurePowerSave(PowerSaveInterface::PowerSaveConfiguration::kHighPerformance, 0));
     return CHIP_NO_ERROR;

src/platform/silabs/wifi/icd/WifiSleepManager.h

@@ -61,18 +61,26 @@ class WifiSleepManager
 
     inline void HandleCommissioningSessionStarted()
     {
-        mIsCommissioningInProgress = true;
-
-        // TODO: Remove High Performance Req during commissioning when sleep issues are resolved
-        WifiSleepManager::GetInstance().RequestHighPerformance();
+        bool wasCommissioningInProgress = mIsCommissioningInProgress;
+        mIsCommissioningInProgress      = true;
+
+        if (!wasCommissioningInProgress)
+        {
+            // TODO: Remove High Performance Req during commissioning when sleep issues are resolved
+            WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition();
+        }
     }
 
     inline void HandleCommissioningSessionStopped()
     {
-        mIsCommissioningInProgress = false;
-
-        // TODO: Remove High Performance Req during commissioning when sleep issues are resolved
-        WifiSleepManager::GetInstance().RemoveHighPerformanceRequest();
+        bool wasCommissioningInProgress = mIsCommissioningInProgress;
+        mIsCommissioningInProgress      = false;
+
+        if (wasCommissioningInProgress)
+        {
+            // TODO: Remove High Performance Req during commissioning when sleep issues are resolved
+            WifiSleepManager::GetInstance().RemoveHighPerformanceRequest();
+        }
     }
 
     /**
@@ -86,7 +94,22 @@ class WifiSleepManager
      * @return CHIP_ERROR CHIP_NO_ERROR if the chip was set to high performance or already in high performance
      *                    CHIP_ERROR_INTERNAL, if the high performance configuration failed
      */
-    CHIP_ERROR RequestHighPerformance();
+    CHIP_ERROR RequestHighPerformanceWithTransition() { return RequestHighPerformance(true); }
+
+    /**
+     * @brief Public API to increase the HighPerformance request counter without transitioning the Wi-Fi chip to High Performance.
+     *        The transition to a different power mode will be done the next the VerifyAndTransitionToLowPowerMode is called.
+     *
+     *        This API does not call the VerifyAndTransitionToLowPowerMode function.
+     *        To trigger the update directly adding a high performance request, call RequestHighPerformanceWithTransition.
+     *
+     *        This API can be called before the Init function. By doing so, the device will transition to High Performance during
+     *        the Init sequence
+     *
+     * @return CHIP_ERROR CHIP_NO_ERROR if the chip was set to high performance or already in high performance
+     *                    CHIP_ERROR_INTERNAL, if the high performance configuration failed
+     */
+    CHIP_ERROR RequestHighPerformanceWithoutTransition() { return RequestHighPerformance(false); }
 
     /**
      * @brief Public API to remove request to keep the Wi-Fi chip in High Performance.
@@ -161,6 +184,17 @@ class WifiSleepManager
      */
     CHIP_ERROR ConfigureDTIMBasedSleep();
 
+    /**
+     * @brief Increments the HighPerformance request counter and triggers the transition to High Performance if requested.
+     *
+     * @param triggerTransition true, triggers the transition to High Performance
+     *                          false, only increments the HighPerformance request counter
+     *
+     * @return CHIP_ERROR CHIP_NO_ERROR if the req removal and sleep transition succeed
+     *                    CHIP_ERROR_INTERNAL, if the req removal or the transition to sleep failed
+     */
+    CHIP_ERROR RequestHighPerformance(bool triggerTransition);
+
     static WifiSleepManager mInstance;
 
     PowerSaveInterface * mPowerSaveInterface = nullptr;

src/platform/silabs/wifi/icd/tests/TestWifiSleepManager.cpp

@@ -129,13 +129,13 @@ TEST_F(TestWifiSleepManager, TestRequestHighPerformanceMaxCounter)
     // Set the counter to its maximum value
     for (uint8_t i = 0; i < std::numeric_limits<uint8_t>::max(); i++)
     {
-        EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformance(), CHIP_NO_ERROR);
+        EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition(), CHIP_NO_ERROR);
         EXPECT_EQ(mMock.GetLastPowerSaveConfiguration(), PowerSaveInterface::PowerSaveConfiguration::kHighPerformance);
         EXPECT_TRUE(mMock.WasConfigurePowerSaveCalled());
     }
 
     // The next request should fail
-    EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformance(), CHIP_ERROR_INTERNAL);
+    EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition(), CHIP_ERROR_INTERNAL);
     EXPECT_FALSE(mMock.WasConfigurePowerSaveCalled());
 
     // Reset the counter & validate reset
@@ -148,7 +148,7 @@ TEST_F(TestWifiSleepManager, TestRequestHighPerformanceMaxCounter)
 
 TEST_F(TestWifiSleepManager, TestRequestRemoveHighPerformance)
 {
-    EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformance(), CHIP_NO_ERROR);
+    EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformanceWithTransition(), CHIP_NO_ERROR);
     EXPECT_EQ(mMock.GetLastPowerSaveConfiguration(), PowerSaveInterface::PowerSaveConfiguration::kHighPerformance);
     EXPECT_TRUE(mMock.WasConfigurePowerSaveCalled());
 
@@ -199,3 +199,16 @@ TEST_F(TestWifiSleepManager, TestVerifyOrTransitionStandardOperation)
     EXPECT_TRUE(mMock.WasConfigureBroadcastFilterCalled());
     EXPECT_FALSE(mMock.WasBroadcastFilterEnabled());
 }
+
+TEST_F(TestWifiSleepManager, TestRequestHighPerformanceWithoutProvisioning)
+{
+    EXPECT_EQ(WifiSleepManager::GetInstance().VerifyAndTransitionToLowPowerMode(WifiSleepManager::PowerEvent::kGenericEvent),
+              CHIP_NO_ERROR);
+
+    PowerSaveInterface::PowerSaveConfiguration config = mMock.GetLastPowerSaveConfiguration();
+    EXPECT_EQ(PowerSaveInterface::PowerSaveConfiguration::kDeepSleep, config);
+
+    // The configuration should not change
+    EXPECT_EQ(WifiSleepManager::GetInstance().RequestHighPerformanceWithoutTransition(), CHIP_NO_ERROR);
+    EXPECT_EQ(mMock.GetLastPowerSaveConfiguration(), config);
+}

third_party/silabs/silabs_board.gni

@@ -12,6 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import("//build_overrides/chip.gni")
+import("${chip_root}/src/platform/silabs/tests/args.gni")
+
 declare_args() {
   # Silabs wireless starter kit plug-in boards featuring specific mcu family and mcu model.
   # Find more information at https://www.silabs.com/development-tools/wireless.
@@ -77,6 +80,16 @@ if (silabs_board == "") {
   silabs_board = getenv("SILABS_BOARD")
 }
 
+# Workaround to enable unit-tests when depending on files that import this gni
+if (silabs_board == "" && sl_build_unit_tests) {
+  print("Building the unit test configuration since silabs_board is not set.")
+  print("If you are building the efr32 test_driver, a board is required.")
+
+  silabs_board = "unit-test"
+  silabs_family = "unit-test"
+  silabs_mcu = "unit-test"
+}
+
 assert(silabs_board != "", "silabs_board must be specified")
 
 # Si917 WIFI board ----------
@@ -190,6 +203,8 @@ if (silabs_board == "BRD4338A" || silabs_board == "BRD2605A" ||
   print("Using custom board configuration")
   print("silabs_family:", silabs_family)
   print("silabs_mcu:", silabs_mcu)
+} else if (silabs_board == "unit-test") {
+  # Do nothing
 } else {
   assert(
       false,