[Silabs] Refactor WiFi scan result handling to use NetworkCommissioning::WiFiScanResponse (project-chip#43591)

* Refactor WiFi BSSID handling in DiagnosticDataProviderImpl to use constant for BSSID length.

* Refactor WiFi scan result handling to use NetworkCommissioning::WiFiScanResponse

Updated the DiagnosticDataProviderImpl and WiFi driver implementations to replace the usage of wfx_wifi_scan_result_t with the new NetworkCommissioning::WiFiScanResponse structure. This change improves consistency and aligns with the recent refactoring of WiFi structures. Adjustments were made to method signatures and internal logic to accommodate the new data structure across multiple files, enhancing the overall clarity and maintainability of the codebase.

* Restyled by clang-format

* Refactor DiagnosticDataProviderImpl to streamline namespace usage

* Remove additional comment

* Revert include reordering

* Remove unnecessary type verification in BackgroundScanCallback for requested SSID pointer

* Restyled by clang-format

---------

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

Author: rosahay-silabs

src/platform/silabs/DiagnosticDataProviderImpl.cpp

@@ -24,6 +24,7 @@
 #include <platform/internal/CHIPDeviceLayerInternal.h>
 
 #include <platform/DiagnosticDataProvider.h>
+#include <platform/NetworkCommissioning.h>
 #include <platform/silabs/DiagnosticDataProviderImpl.h>
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD
 #include <platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h>
@@ -35,6 +36,8 @@
 #include <sl_cmsis_os2_common.h>
 
 using namespace ::chip::app::Clusters::GeneralDiagnostics;
+using namespace ::chip::DeviceLayer::NetworkCommissioning;
+using namespace ::chip::DeviceLayer::Internal;
 
 namespace chip {
 namespace DeviceLayer {
@@ -335,15 +338,14 @@ void DiagnosticDataProviderImpl::ReleaseNetworkInterfaces(NetworkInterface * net
 #if defined(SL_WIFI) && SL_WIFI
 CHIP_ERROR DiagnosticDataProviderImpl::GetWiFiBssId(MutableByteSpan & BssId)
 {
-    constexpr size_t bssIdSize = 6;
-    wfx_wifi_scan_result_t ap  = { 0 };
+    WiFiScanResponse ap = {};
 
-    VerifyOrReturnError(BssId.size() >= bssIdSize, CHIP_ERROR_BUFFER_TOO_SMALL);
+    VerifyOrReturnError(BssId.size() >= kWiFiBSSIDLength, CHIP_ERROR_BUFFER_TOO_SMALL);
 
     if (Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap) == CHIP_NO_ERROR)
     {
-        memcpy(BssId.data(), ap.bssid, bssIdSize);
-        BssId.reduce_size(bssIdSize);
+        memcpy(BssId.data(), ap.bssid, kWiFiBSSIDLength);
+        BssId.reduce_size(kWiFiBSSIDLength);
         return CHIP_NO_ERROR;
     }
 
@@ -355,8 +357,8 @@ CHIP_ERROR DiagnosticDataProviderImpl::GetWiFiSecurityType(app::Clusters::WiFiNe
     using app::Clusters::NetworkCommissioning::WiFiSecurityBitmap;
     using app::Clusters::WiFiNetworkDiagnostics::SecurityTypeEnum;
 
-    wfx_wifi_scan_result_t ap = { 0 };
-    CHIP_ERROR error          = Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap);
+    WiFiScanResponse ap = {};
+    CHIP_ERROR error    = Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap);
     VerifyOrReturnError(error == CHIP_NO_ERROR, error);
 
     // Map Matter WiFiSecurityBitmap to WiFiNetworkDiagnostics SecurityTypeEnum (prefer highest)
@@ -395,23 +397,23 @@ CHIP_ERROR DiagnosticDataProviderImpl::GetWiFiVersion(app::Clusters::WiFiNetwork
 
 CHIP_ERROR DiagnosticDataProviderImpl::GetWiFiChannelNumber(uint16_t & channelNumber)
 {
-    wfx_wifi_scan_result_t ap = { 0 };
-    CHIP_ERROR error          = Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap);
+    WiFiScanResponse ap = {};
+    CHIP_ERROR error    = Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap);
     if (error == CHIP_NO_ERROR)
     {
-        channelNumber = ap.chan;
+        channelNumber = ap.channel;
         return CHIP_NO_ERROR;
     }
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
 }
 
 CHIP_ERROR DiagnosticDataProviderImpl::GetWiFiRssi(int8_t & rssi)
 {
-    wfx_wifi_scan_result_t ap = { 0 };
-    CHIP_ERROR error          = Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap);
+    WiFiScanResponse ap = {};
+    CHIP_ERROR error    = Silabs::WifiInterface::GetInstance().GetAccessPointInfo(ap);
     if (error == CHIP_NO_ERROR)
     {
-        rssi = ap.rssi;
+        rssi = ap.signal.strength;
         return CHIP_NO_ERROR;
     }
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;

src/platform/silabs/NetworkCommissioningWiFiDriver.cpp

@@ -249,7 +249,7 @@ bool SlWiFiDriver::StartScanWiFiNetworks(ByteSpan ssid)
     return true;
 }
 
-void SlWiFiDriver::OnScanWiFiNetworkDone(wfx_wifi_scan_result_t * aScanResult)
+void SlWiFiDriver::OnScanWiFiNetworkDone(NetworkCommissioning::WiFiScanResponse * aScanResult)
 {
     SlWiFiDriver * nwDriver = NetworkCommissioning::SlWiFiDriver::GetInstance();
     // Cannot use the driver if the instance is not initialized.
@@ -278,18 +278,7 @@ void SlWiFiDriver::OnScanWiFiNetworkDone(wfx_wifi_scan_result_t * aScanResult)
     }
     else
     {
-        NetworkCommissioning::WiFiScanResponse scanResponse = {};
-
-        scanResponse.security        = aScanResult->security;
-        scanResponse.channel         = aScanResult->chan;
-        scanResponse.signal.type     = NetworkCommissioning::WirelessSignalType::kdBm;
-        scanResponse.signal.strength = aScanResult->rssi;
-        scanResponse.ssidLen         = aScanResult->ssid_length;
-        memcpy(scanResponse.ssid, aScanResult->ssid, scanResponse.ssidLen);
-        memcpy(scanResponse.bssid, aScanResult->bssid, sizeof(scanResponse.bssid));
-        scanResponse.wiFiBand = aScanResult->wiFiBand;
-
-        mScanResponseIter.Add(&scanResponse);
+        mScanResponseIter.Add(aScanResult);
     }
 }
 

src/platform/silabs/NetworkCommissioningWiFiDriver.h

@@ -123,7 +123,7 @@ class SlWiFiDriver final : public WiFiDriver
 private:
     bool NetworkMatch(const Silabs::WifiInterface::WiFiCredentials & network, ByteSpan networkId);
     bool StartScanWiFiNetworks(ByteSpan ssid);
-    static void OnScanWiFiNetworkDone(wfx_wifi_scan_result_t * aScanResult);
+    static void OnScanWiFiNetworkDone(NetworkCommissioning::WiFiScanResponse * aScanResult);
 
     static SlWiFiDriver * mDriver;
     Silabs::WifiInterface::WiFiCredentials mSavedNetwork   = {};

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

@@ -22,12 +22,15 @@
 #include "ble_config.h"
 #include "sl_status.h"
 #include "sl_wifi_device.h"
+
+#include <algorithm>
 #include <app/icd/server/ICDServerConfig.h>
 #include <cmsis_os2.h>
 #include <inet/IPAddress.h>
 #include <lib/support/CHIPMem.h>
 #include <lib/support/CHIPMemString.h>
 #include <lib/support/logging/CHIPLogging.h>
+#include <platform/NetworkCommissioning.h>
 #include <platform/silabs/wifi/SiWx/WifiInterfaceImpl.h>
 #include <sl_cmsis_os2_common.h>
 
@@ -271,60 +274,58 @@ sl_status_t BackgroundScanCallback(sl_wifi_event_t event, sl_wifi_scan_result_t
     VerifyOrReturnError(result != nullptr, SL_STATUS_NULL_POINTER);
     VerifyOrReturnError(wfx_rsi.scan_cb != nullptr, SL_STATUS_INVALID_HANDLE);
 
-    sl_wifi_ssid_t * requestedSsidPtr = nullptr;
-    chip::ByteSpan requestedSsidSpan;
-    // arg is the requested SSID pointer passed during sl_wifi_set_scan_callback
+    chip::ByteSpan requestedSsidSpan = {};
+
+    // arg is set to requested SSID if provided in sl_wifi_set_scan_callback
     if (arg != nullptr)
     {
-        requestedSsidPtr  = reinterpret_cast<sl_wifi_ssid_t *>(arg);
-        requestedSsidSpan = chip::ByteSpan(requestedSsidPtr->value, requestedSsidPtr->length);
+        sl_wifi_ssid_t * requestedSsidPtr = static_cast<sl_wifi_ssid_t *>(arg);
+        requestedSsidSpan                 = chip::ByteSpan(requestedSsidPtr->value, requestedSsidPtr->length);
     }
 
     uint32_t nbreResults = result->scan_count;
     for (uint32_t i = 0; i < nbreResults; i++)
     {
-        wfx_wifi_scan_result_t currentScanResult = { 0 };
-
         // Length excludes null-character
-        size_t scannedSsidLength = strnlen(reinterpret_cast<char *>(result->scan_info[i].ssid), kMaxWiFiSSIDLength);
-        chip::ByteSpan scannedSsidSpan(result->scan_info[i].ssid, scannedSsidLength);
-
-        // Copy the scanned SSID to the current scan ssid buffer that will be forwarded to the callback
-        chip::MutableByteSpan currentScanSsid(currentScanResult.ssid, kMaxWiFiSSIDLength);
-        ReturnValueOnFailure(chip::CopySpanToMutableSpan(scannedSsidSpan, currentScanSsid),
-                             SL_STATUS_SI91X_MEMORY_IS_NOT_SUFFICIENT);
-        currentScanResult.ssid_length = currentScanSsid.size();
-
-        chip::ByteSpan inBssid(result->scan_info[i].bssid, kWiFiBSSIDLength);
-        chip::MutableByteSpan outBssid(currentScanResult.bssid, kWiFiBSSIDLength);
-        ReturnValueOnFailure(chip::CopySpanToMutableSpan(inBssid, outBssid), SL_STATUS_SI91X_MEMORY_IS_NOT_SUFFICIENT);
-
-        currentScanResult.security =
-            ConvertSlWifiSecurityToBitmap(static_cast<sl_wifi_security_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;
-        // TODO: change this when SDK provides values
-        currentScanResult.wiFiBand = WiFiBandEnum::k2g4;
-
-        // if user has provided ssid, check if the current scan result ssid matches the user provided ssid
-        // NOTE: background scan does not filter by ssid, so we need to do it here
-        if (!requestedSsidSpan.empty())
-        {
-            if (requestedSsidSpan.data_equal(currentScanSsid))
-            {
-                wfx_rsi.scan_cb(&currentScanResult);
-            }
-        }
-        else // No ssid was provide - forward all results
+        size_t ssidLen = strnlen(reinterpret_cast<char *>(result->scan_info[i].ssid), kMaxWiFiSSIDLength);
+        chip::ByteSpan ssidSpan(result->scan_info[i].ssid, ssidLen);
+
+        if (requestedSsidSpan.empty() || requestedSsidSpan.data_equal(ssidSpan))
         {
+
+            // Create a new scan response for the current scan result
+            chip::DeviceLayer::NetworkCommissioning::WiFiScanResponse currentScanResult = {};
+
+            // Copy the scanned SSID to the scan response
+            chip::MutableByteSpan responseSsidSpan(currentScanResult.ssid, kMaxWiFiSSIDLength);
+            VerifyOrReturnError(chip::CopySpanToMutableSpan(ssidSpan, responseSsidSpan) == CHIP_NO_ERROR,
+                                SL_STATUS_SI91X_MEMORY_IS_NOT_SUFFICIENT);
+            currentScanResult.ssidLen = static_cast<uint8_t>(ssidLen);
+
+            // Copy the BSSID to the scan response
+            chip::ByteSpan bssidSpan(result->scan_info[i].bssid, kWiFiBSSIDLength);
+            chip::MutableByteSpan responseBssidSpan(currentScanResult.bssid, kWiFiBSSIDLength);
+            VerifyOrReturnError(chip::CopySpanToMutableSpan(bssidSpan, responseBssidSpan) == CHIP_NO_ERROR,
+                                SL_STATUS_SI91X_MEMORY_IS_NOT_SUFFICIENT);
+
+            // Convert the RSSI to a int8_t value
+            int16_t rssi = std::clamp(((-1) * result->scan_info[i].rssi_val), INT8_MIN, INT8_MAX);
+
+            currentScanResult.signal.strength = static_cast<int8_t>(rssi);
+            currentScanResult.signal.type     = chip::DeviceLayer::NetworkCommissioning::WirelessSignalType::kdBm;
+
+            currentScanResult.channel  = static_cast<uint16_t>(result->scan_info[i].rf_channel);
+            currentScanResult.wiFiBand = WiFiBandEnum::k2g4;
+
+            currentScanResult.security =
+                ConvertSlWifiSecurityToBitmap(static_cast<sl_wifi_security_t>(result->scan_info[i].security_mode));
+
             wfx_rsi.scan_cb(&currentScanResult);
         }
     }
-
+    // null callback to indicate that the scan is complete
     wfx_rsi.scan_cb(nullptr);
-    // cleanup and return
     wfx_rsi.scan_cb = nullptr;
-
     wfx_rsi.dev_state.Clear(WifiInterface::WifiState::kScanStarted);
     osSemaphoreRelease(sScanCompleteSemaphore);
 
@@ -807,7 +808,7 @@ sl_status_t WifiInterfaceImpl::JoinCallback(sl_wifi_event_t event, char * result
     return status;
 }
 
-CHIP_ERROR WifiInterfaceImpl::GetAccessPointInfo(wfx_wifi_scan_result_t & info)
+CHIP_ERROR WifiInterfaceImpl::GetAccessPointInfo(NetworkCommissioning::WiFiScanResponse & info)
 {
     // TODO: Convert this to a int8
     int32_t rssi = 0;
@@ -836,19 +837,21 @@ CHIP_ERROR WifiInterfaceImpl::GetAccessPointInfo(wfx_wifi_scan_result_t & info)
     }
 
     info.security = wfx_rsi.credentials.security;
-    info.chan     = wfx_rsi.ap_chan;
+    info.channel  = wfx_rsi.ap_chan;
+    info.wiFiBand = WiFiBandEnum::k2g4;
 
     chip::MutableByteSpan output(info.ssid, kMaxWiFiSSIDLength);
     chip::ByteSpan ssid(wfx_rsi.credentials.ssid, wfx_rsi.credentials.ssidLen);
     ReturnErrorOnFailure(chip::CopySpanToMutableSpan(ssid, output));
-    info.ssid_length = output.size();
+    info.ssidLen = static_cast<uint8_t>(output.size());
     chip::ByteSpan apBssidSpan(wfx_rsi.ap_bssid.data(), wfx_rsi.ap_bssid.size());
     chip::MutableByteSpan bssidSpan(info.bssid, kWiFiBSSIDLength);
     ReturnErrorOnFailure(chip::CopySpanToMutableSpan(apBssidSpan, bssidSpan));
 
     // TODO: add error processing
     sl_wifi_get_signal_strength(SL_WIFI_CLIENT_INTERFACE, &(rssi));
-    info.rssi = rssi;
+    info.signal.strength = static_cast<int8_t>(std::clamp(rssi, static_cast<int32_t>(INT8_MIN), static_cast<int32_t>(INT8_MAX)));
+    info.signal.type     = NetworkCommissioning::WirelessSignalType::kdBm;
     return CHIP_NO_ERROR;
 }
 

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

@@ -67,7 +67,7 @@ class WifiInterfaceImpl final : public WifiInterface
     void CancelScanNetworks() override;
     bool IsWifiProvisioned() override;
     CHIP_ERROR InitWiFiStack(void) override;
-    CHIP_ERROR GetAccessPointInfo(wfx_wifi_scan_result_t & info) override;
+    CHIP_ERROR GetAccessPointInfo(chip::DeviceLayer::NetworkCommissioning::WiFiScanResponse & info) override;
     CHIP_ERROR GetAccessPointExtendedInfo(wfx_wifi_scan_ext_t & info) override;
     CHIP_ERROR ResetCounters() override;
 

src/platform/silabs/wifi/WifiInterface.h

@@ -43,17 +43,7 @@ constexpr size_t kWiFiMacAddressLength = 6;
 /* Defines to update */
 #define MAX_JOIN_RETRIES_COUNT (5)
 
-typedef struct wfx_wifi_scan_result
-{
-    uint8_t ssid[chip::DeviceLayer::Internal::kMaxWiFiSSIDLength]; // excludes null-character
-    size_t ssid_length;
-    chip::BitFlags<chip::app::Clusters::NetworkCommissioning::WiFiSecurityBitmap> security;
-    uint8_t bssid[chip::DeviceLayer::Internal::kWiFiBSSIDLength];
-    uint8_t chan;
-    int16_t rssi; /* I suspect this is in dBm - so signed */
-    chip::app::Clusters::NetworkCommissioning::WiFiBandEnum wiFiBand;
-} wfx_wifi_scan_result_t;
-using ScanCallback = void (*)(wfx_wifi_scan_result_t *);
+using ScanCallback = void (*)(chip::DeviceLayer::NetworkCommissioning::WiFiScanResponse *);
 
 typedef struct wfx_wifi_scan_ext
 {
@@ -244,14 +234,14 @@ class WifiInterface : public WifiStateProvider, public PowerSaveInterface
 
     /**
      * @brief Gets the connected access point information.
-     *        See @wfx_wifi_scan_result_t for the information that is returned by the function.
+     *        See @NetworkCommissioning::WiFiScanResponse for the information that is returned by the function.
      *
      * @param[out] info AP information
      *
      * @return CHIP_ERROR CHIP_NO_ERROR, device has succesfully pulled all the AP information
      *                    CHIP_ERROR_INTERNAL, otherwise. If the function returns an error, the data in ap cannot be used.
      */
-    virtual CHIP_ERROR GetAccessPointInfo(wfx_wifi_scan_result_t & info) = 0;
+    virtual CHIP_ERROR GetAccessPointInfo(chip::DeviceLayer::NetworkCommissioning::WiFiScanResponse & info) = 0;
 
     /**
      * @brief Gets the connected access point extended information.