[Silabs] Refactor network scan wifi abstraction function (project-chip#36802)

* Refactor scan operation for the 917 SoC

* Refactor scan operation for the wf200

* Add instant scan configuration

* fix rs911x scan function

* restyle

* Finish mutable span use

* finish clean up

* restyle

* fix lenght change for wf200

* remove scan define since the feature always need to be present

* Restyled by clang-format

* address review comments

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <commits@restyled.io>

Author: mkardous-silabs

src/platform/silabs/NetworkCommissioningWiFiDriver.cpp

@@ -265,26 +265,24 @@ chip::BitFlags<WiFiSecurity> SlWiFiDriver::ConvertSecuritytype(wfx_sec_t securit
 
 bool SlWiFiDriver::StartScanWiFiNetworks(ByteSpan ssid)
 {
-    bool scanStarted = false;
     ChipLogProgress(DeviceLayer, "Start Scan WiFi Networks");
-    if (!ssid.empty()) // ssid is given, only scan this network
-    {
-        char cSsid[DeviceLayer::Internal::kMaxWiFiSSIDLength] = {};
-        memcpy(cSsid, ssid.data(), ssid.size());
-        scanStarted = wfx_start_scan(cSsid, OnScanWiFiNetworkDone);
-    }
-    else // scan all networks
+    CHIP_ERROR err = StartNetworkScan(ssid, OnScanWiFiNetworkDone);
+
+    if (err != CHIP_NO_ERROR)
     {
-        scanStarted = wfx_start_scan(nullptr, OnScanWiFiNetworkDone);
+        ChipLogError(DeviceLayer, "StartNetworkScan failed: %s", chip::ErrorStr(err));
+        return false;
     }
-    return scanStarted;
+
+    return true;
 }
 
 void SlWiFiDriver::OnScanWiFiNetworkDone(wfx_wifi_scan_result_t * aScanResult)
 {
-    ChipLogProgress(DeviceLayer, "OnScanWiFiNetworkDone");
     if (!aScanResult)
     {
+        ChipLogProgress(DeviceLayer, "OnScanWiFiNetworkDone: Receive all scanned networks information.");
+
         if (GetInstance().mpScanCallback != nullptr)
         {
             if (mScanResponseIter.Count() == 0)
@@ -309,7 +307,7 @@ void SlWiFiDriver::OnScanWiFiNetworkDone(wfx_wifi_scan_result_t * aScanResult)
         scanResponse.security.Set(GetInstance().ConvertSecuritytype(aScanResult->security));
         scanResponse.channel = aScanResult->chan;
         scanResponse.rssi    = aScanResult->rssi;
-        scanResponse.ssidLen = strnlen(aScanResult->ssid, DeviceLayer::Internal::kMaxWiFiSSIDLength);
+        scanResponse.ssidLen = aScanResult->ssid_length;
         memcpy(scanResponse.ssid, aScanResult->ssid, scanResponse.ssidLen);
         memcpy(scanResponse.bssid, aScanResult->bssid, sizeof(scanResponse.bssid));
 

src/platform/silabs/wifi/BUILD.gn

@@ -21,7 +21,6 @@ import("${chip_root}/third_party/silabs/silabs_board.gni")
 declare_args() {
   # Wifi related stuff - they are overridden by gn -args="use_wf200=true"
   sl_wfx_config_softap = false
-  sl_wfx_config_scan = true
 
   # Argument to force enable WPA3 security on rs91x
   rs91x_wpa3_transition = true
@@ -85,10 +84,6 @@ config("wifi-platform-config") {
     defines += [ "SL_WFX_CONFIG_SOFTAP" ]
   }
 
-  if (sl_wfx_config_scan) {
-    defines += [ "SL_WFX_CONFIG_SCAN" ]
-  }
-
   if (chip_enable_wifi_ipv4) {
     defines += [ "CHIP_DEVICE_CONFIG_ENABLE_IPV4" ]
   }

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

@@ -187,13 +187,80 @@ constexpr uint8_t kAdvActiveScanDuration     = 15;
 constexpr uint8_t kAdvPassiveScanDuration    = 20;
 constexpr uint8_t kAdvMultiProbe             = 1;
 constexpr uint8_t kAdvScanPeriodicity        = 10;
+constexpr uint8_t kAdvEnableInstantbgScan    = 1;
 
 // TODO: Confirm that this value works for size and timing
 constexpr uint8_t kWfxQueueSize = 10;
 
 // TODO: Figure out why we actually need this, we are already handling failure and retries somewhere else.
 constexpr uint16_t kWifiScanTimeoutTicks = 10000;
 
+/**
+ * @brief Network Scan callback when the device receive a scan operation from the controller.
+ *        This callback is used whe the Network Commission Driver send a ScanNetworks command.
+ *
+ *        If the scan network was requested for a specific SSID - wfx_rsi.scan_ssid had a valid value,
+ *        the callback will only forward that specific networks information.
+ *        If no ssid is provided, wfx_rsi.scan_ssid is a nullptr, we return the information of all scanned networks.
+ */
+sl_status_t BackgroundScanCallback(sl_wifi_event_t event, sl_wifi_scan_result_t * result, uint32_t result_length, void * arg)
+{
+    VerifyOrReturnError(result != nullptr, SL_STATUS_NULL_POINTER);
+    VerifyOrReturnError(wfx_rsi.scan_cb != nullptr, SL_STATUS_INVALID_HANDLE);
+
+    uint32_t nbreResults = result->scan_count;
+    chip::ByteSpan requestedSsidSpan(wfx_rsi.scan_ssid, wfx_rsi.scan_ssid_length);
+
+    for (uint32_t i = 0; i < nbreResults; i++)
+    {
+        wfx_wifi_scan_result_t currentScanResult = { 0 };
+
+        // Lenght excludes null-character
+        size_t scannedSsidLenght = strnlen(reinterpret_cast<char *>(result->scan_info[i].ssid), WFX_MAX_SSID_LENGTH);
+        chip::ByteSpan scannedSsidSpan(result->scan_info[i].ssid, scannedSsidLenght);
+
+        // Copy the scanned SSID to the current scan ssid buffer that will be forwarded to the callback
+        chip::MutableByteSpan currentScanSsid(currentScanResult.ssid, WFX_MAX_SSID_LENGTH);
+        chip::CopySpanToMutableSpan(scannedSsidSpan, currentScanSsid);
+        currentScanResult.ssid_length = currentScanSsid.size();
+
+        chip::ByteSpan inBssid(result->scan_info[i].bssid, kWifiMacAddressLength);
+        chip::MutableByteSpan outBssid(currentScanResult.bssid, kWifiMacAddressLength);
+        chip::CopySpanToMutableSpan(inBssid, outBssid);
+
+        // TODO: We should revisit this to make sure we are setting the correct values
+        currentScanResult.security = static_cast<wfx_sec_t>(result->scan_info[i].security_mode);
+        currentScanResult.rssi     = (-1) * result->scan_info[i].rssi_val; // The returned value is positive - we need to flip it
+        currentScanResult.chan     = result->scan_info[i].rf_channel;
+
+        // if user has provided ssid, check if the current scan result ssid matches the user provided ssid
+        if (!requestedSsidSpan.empty())
+        {
+            if (requestedSsidSpan.data_equal(currentScanSsid))
+            {
+                wfx_rsi.scan_cb(&currentScanResult);
+            }
+        }
+        else // No ssid was provide - forward all results
+        {
+            wfx_rsi.scan_cb(&currentScanResult);
+        }
+    }
+
+    // cleanup and return
+    wfx_rsi.dev_state.Clear(WifiState::kScanStarted);
+    wfx_rsi.scan_cb(nullptr);
+    wfx_rsi.scan_cb = nullptr;
+    if (wfx_rsi.scan_ssid)
+    {
+        chip::Platform::MemoryFree(wfx_rsi.scan_ssid);
+        wfx_rsi.scan_ssid = nullptr;
+    }
+    osSemaphoreRelease(sScanCompleteSemaphore);
+
+    return SL_STATUS_OK;
+}
+
 void DHCPTimerEventHandler(void * arg)
 {
     WifiPlatformEvent event = WifiPlatformEvent::kStationDhcpPoll;
@@ -474,16 +541,26 @@ sl_status_t sl_matter_wifi_platform_init(void)
  *********************************************************************/
 int32_t wfx_rsi_get_ap_info(wfx_wifi_scan_result_t * ap)
 {
-    sl_status_t status = SL_STATUS_OK;
-    int32_t rssi       = 0;
-    ap->ssid_length    = wfx_rsi.sec.ssid_length;
-    ap->security       = wfx_rsi.sec.security;
-    ap->chan           = wfx_rsi.ap_chan;
-    chip::Platform::CopyString(ap->ssid, ap->ssid_length, wfx_rsi.sec.ssid);
-    memcpy(&ap->bssid[0], wfx_rsi.ap_mac.data(), kWifiMacAddressLength);
-    sl_wifi_get_signal_strength(SL_WIFI_CLIENT_INTERFACE, &rssi);
+    // TODO: Convert this to a int8
+    int32_t rssi = 0;
+    ap->security = wfx_rsi.sec.security;
+    ap->chan     = wfx_rsi.ap_chan;
+
+    chip::MutableByteSpan output(ap->ssid, WFX_MAX_SSID_LENGTH);
+    // Cast is a workaround until the wfx_rsi structure is refactored
+    chip::ByteSpan ssid(reinterpret_cast<uint8_t *>(wfx_rsi.sec.ssid), wfx_rsi.sec.ssid_length);
+    chip::CopySpanToMutableSpan(ssid, output);
+    ap->ssid_length = output.size();
+
+    chip::ByteSpan apMacSpan(wfx_rsi.ap_mac.data(), wfx_rsi.ap_mac.size());
+    chip::MutableByteSpan bssidSpan(ap->bssid, kWifiMacAddressLength);
+    chip::CopySpanToMutableSpan(apMacSpan, bssidSpan);
+
+    // TODO: add error processing
+    sl_wifi_get_signal_strength(SL_WIFI_CLIENT_INTERFACE, &(rssi));
     ap->rssi = rssi;
-    return status;
+
+    return SL_STATUS_OK;
 }
 
 /******************************************************************
@@ -547,59 +624,6 @@ int32_t sl_wifi_platform_disconnect(void)
     return sl_net_down((sl_net_interface_t) SL_NET_WIFI_CLIENT_INTERFACE);
 }
 
-sl_status_t show_scan_results(sl_wifi_scan_result_t * scan_result)
-{
-    SL_WIFI_ARGS_CHECK_NULL_POINTER(scan_result);
-    VerifyOrReturnError(wfx_rsi.scan_cb != nullptr, SL_STATUS_INVALID_HANDLE);
-
-    wfx_wifi_scan_result_t cur_scan_result;
-    for (int idx = 0; idx < (int) scan_result->scan_count; idx++)
-    {
-        memset(&cur_scan_result, 0, sizeof(cur_scan_result));
-
-        cur_scan_result.ssid_length = strnlen((char *) scan_result->scan_info[idx].ssid,
-                                              std::min<size_t>(sizeof(scan_result->scan_info[idx].ssid), WFX_MAX_SSID_LENGTH));
-        chip::Platform::CopyString(cur_scan_result.ssid, cur_scan_result.ssid_length, (char *) scan_result->scan_info[idx].ssid);
-
-        // if user has provided ssid, then check if the current scan result ssid matches the user provided ssid
-        if (wfx_rsi.scan_ssid != nullptr &&
-            (strncmp(wfx_rsi.scan_ssid, cur_scan_result.ssid, std::min(strlen(wfx_rsi.scan_ssid), strlen(cur_scan_result.ssid))) ==
-             0))
-        {
-            continue;
-        }
-        cur_scan_result.security = static_cast<wfx_sec_t>(scan_result->scan_info[idx].security_mode);
-        cur_scan_result.rssi     = (-1) * scan_result->scan_info[idx].rssi_val;
-        memcpy(cur_scan_result.bssid, scan_result->scan_info[idx].bssid, kWifiMacAddressLength);
-        wfx_rsi.scan_cb(&cur_scan_result);
-
-        // if user has not provided the ssid, then call the callback for each scan result
-        if (wfx_rsi.scan_ssid == nullptr)
-        {
-            continue;
-        }
-        break;
-    }
-
-    // cleanup and return
-    wfx_rsi.dev_state.Clear(WifiState::kScanStarted);
-    wfx_rsi.scan_cb((wfx_wifi_scan_result_t *) 0);
-    wfx_rsi.scan_cb = nullptr;
-    if (wfx_rsi.scan_ssid)
-    {
-        chip::Platform::MemoryFree(wfx_rsi.scan_ssid);
-        wfx_rsi.scan_ssid = nullptr;
-    }
-    return SL_STATUS_OK;
-}
-
-sl_status_t bg_scan_callback_handler(sl_wifi_event_t event, sl_wifi_scan_result_t * result, uint32_t result_length, void * arg)
-{
-    show_scan_results(result); // To do Check error
-    osSemaphoreRelease(sScanCompleteSemaphore);
-    return SL_STATUS_OK;
-}
-
 /// NotifyConnectivity
 /// @brief Notify the application about the connectivity status if it has not been notified yet.
 ///        Helper function for HandleDHCPPolling.
@@ -711,53 +735,61 @@ void ProcessEvent(WifiPlatformEvent event)
 
     case WifiPlatformEvent::kScan:
         ChipLogDetail(DeviceLayer, "WifiPlatformEvent::kScan");
-
-#ifdef SL_WFX_CONFIG_SCAN
         if (!(wfx_rsi.dev_state.Has(WifiState::kScanStarted)))
         {
             ChipLogDetail(DeviceLayer, "WifiPlatformEvent::kScan");
-            sl_wifi_scan_configuration_t wifi_scan_configuration;
-            memset(&wifi_scan_configuration, 0, sizeof(wifi_scan_configuration));
-
-            // TODO: Add scan logic
-            sl_wifi_advanced_scan_configuration_t advanced_scan_configuration = { 0 };
-            int32_t status;
-            advanced_scan_configuration.active_channel_time  = kAdvActiveScanDuration;
-            advanced_scan_configuration.passive_channel_time = kAdvPassiveScanDuration;
-            advanced_scan_configuration.trigger_level        = kAdvScanThreshold;
-            advanced_scan_configuration.trigger_level_change = kAdvRssiToleranceThreshold;
-            advanced_scan_configuration.enable_multi_probe   = kAdvMultiProbe;
-            status = sl_wifi_set_advanced_scan_configuration(&advanced_scan_configuration);
-            if (SL_STATUS_OK != status)
-            {
-                // TODO: Seems like Chipdie should be called here, the device should be initialized here
-                ChipLogError(DeviceLayer, "sl_wifi_set_advanced_scan_configuration failed: 0x%lx", static_cast<uint32_t>(status));
-                return;
-            }
+            sl_status_t status = SL_STATUS_OK;
 
+            sl_wifi_scan_configuration_t wifi_scan_configuration = default_wifi_scan_configuration;
             if (wfx_rsi.dev_state.Has(WifiState::kStationConnected))
             {
                 /* Terminate with end of scan which is no ap sent back */
                 wifi_scan_configuration.type                   = SL_WIFI_SCAN_TYPE_ADV_SCAN;
                 wifi_scan_configuration.periodic_scan_interval = kAdvScanPeriodicity;
             }
-            else
+
+            sl_wifi_advanced_scan_configuration_t advanced_scan_configuration = {
+                .trigger_level        = kAdvScanThreshold,
+                .trigger_level_change = kAdvRssiToleranceThreshold,
+                .active_channel_time  = kAdvActiveScanDuration,
+                .passive_channel_time = kAdvPassiveScanDuration,
+                .enable_instant_scan  = kAdvEnableInstantbgScan,
+                .enable_multi_probe   = kAdvMultiProbe,
+            };
+
+            status = sl_wifi_set_advanced_scan_configuration(&advanced_scan_configuration);
+
+            // TODO: Seems like Chipdie should be called here, the device should be initialized here
+            VerifyOrReturn(
+                status == SL_STATUS_OK,
+                ChipLogError(DeviceLayer, "sl_wifi_set_advanced_scan_configuration failed: 0x%lx", static_cast<uint32_t>(status)));
+
+            sl_wifi_set_scan_callback(BackgroundScanCallback, nullptr);
+            wfx_rsi.dev_state.Set(WifiState::kScanStarted);
+
+            // If an ssid was not provided, we need to call the scan API with nullptr to scan all Wi-Fi networks
+            sl_wifi_ssid_t ssid      = { 0 };
+            sl_wifi_ssid_t * ssidPtr = nullptr;
+
+            if (wfx_rsi.scan_ssid != nullptr)
             {
-                wifi_scan_configuration = default_wifi_scan_configuration;
+                chip::ByteSpan requestedSsid(wfx_rsi.scan_ssid, wfx_rsi.scan_ssid_length);
+                chip::MutableByteSpan ouputSsid(ssid.value, sizeof(ssid.value));
+                chip::CopySpanToMutableSpan(requestedSsid, ouputSsid);
+
+                ssid.length = ouputSsid.size();
+                ssidPtr     = &ssid;
             }
-            sl_wifi_set_scan_callback(bg_scan_callback_handler, nullptr);
-            wfx_rsi.dev_state.Set(WifiState::kScanStarted);
 
             osSemaphoreAcquire(sScanInProgressSemaphore, osWaitForever);
-            status = sl_wifi_start_scan(SL_WIFI_CLIENT_2_4GHZ_INTERFACE, nullptr, &wifi_scan_configuration);
+            status = sl_wifi_start_scan(SL_WIFI_CLIENT_2_4GHZ_INTERFACE, ssidPtr, &wifi_scan_configuration);
             if (SL_STATUS_IN_PROGRESS == status)
             {
                 osSemaphoreAcquire(sScanCompleteSemaphore, kWifiScanTimeoutTicks);
             }
 
             osSemaphoreRelease(sScanInProgressSemaphore);
         }
-#endif /* SL_WFX_CONFIG_SCAN */
         break;
 
     case WifiPlatformEvent::kStationStartJoin:

src/platform/silabs/wifi/WifiInterfaceAbstraction.h

@@ -131,13 +131,14 @@ typedef enum
 
 typedef struct wfx_wifi_scan_result
 {
-    char ssid[WFX_MAX_SSID_LENGTH + 1];
+    uint8_t ssid[WFX_MAX_SSID_LENGTH]; // excludes null-character
     size_t ssid_length;
     wfx_sec_t security;
     uint8_t bssid[kWifiMacAddressLength];
     uint8_t chan;
     int16_t rssi; /* I suspect this is in dBm - so signed */
 } wfx_wifi_scan_result_t;
+using ScanCallback = void (*)(wfx_wifi_scan_result_t *);
 
 typedef struct wfx_wifi_scan_ext
 {
@@ -167,11 +168,9 @@ typedef struct wfx_rsi_s
     chip::BitFlags<WifiState> dev_state;
     uint16_t ap_chan; /* The chan our STA is using	*/
     wfx_wifi_provision_t sec;
-#ifdef SL_WFX_CONFIG_SCAN
-    void (*scan_cb)(wfx_wifi_scan_result_t *);
-    char * scan_ssid; /* Which one are we scanning for */
+    ScanCallback scan_cb;
+    uint8_t * scan_ssid; /* Which one are we scanning for */
     size_t scan_ssid_length;
-#endif
 #ifdef SL_WFX_CONFIG_SOFTAP
     MacAddress softap_mac;
 #endif
@@ -234,6 +233,22 @@ void NotifyConnection(const MacAddress & ap);
  */
 CHIP_ERROR GetMacAddress(sl_wfx_interface_t interface, chip::MutableByteSpan & addr);
 
+/**
+ * @brief Triggers a network scan
+ *        The function is asynchronous and the result is provided via the callback.
+ *
+ * @param ssid The SSID to scan for. If empty, all networks are scanned
+ * @param callback The callback to be called when the scan is complete. Cannot be nullptr.
+ *                 The callback is called asynchrounously.
+ *
+ * @return CHIP_ERROR CHIP_NO_ERROR if the network scan was successfully started
+ *                    CHIP_INVALID_ARGUMENT if the callback is nullptr
+ *                    CHIP_ERROR_IN_PROGRESS, if there is already a network scan in progress
+ *                    CHIP_ERROR_INVALID_STRING_LENGTH, if there SSID length exceeds handled limit
+ *                    other, if there is a platform error when starting the scan
+ */
+CHIP_ERROR StartNetworkScan(chip::ByteSpan ssid, ScanCallback callback);
+
 /* Function to update */
 
 sl_status_t wfx_wifi_start(void);
@@ -256,7 +271,6 @@ bool wfx_have_ipv4_addr(sl_wfx_interface_t);
 
 bool wfx_have_ipv6_addr(sl_wfx_interface_t);
 wifi_mode_t wfx_get_wifi_mode(void);
-bool wfx_start_scan(char * ssid, void (*scan_cb)(wfx_wifi_scan_result_t *)); /* true returned if successfully started */
 void wfx_cancel_scan(void);
 
 /*