RDK-61444 : Network Manager Plugin to support Scan Specific SSID#306
Open
jincysam87 wants to merge 55 commits into
Open
RDK-61444 : Network Manager Plugin to support Scan Specific SSID#306jincysam87 wants to merge 55 commits into
jincysam87 wants to merge 55 commits into
Conversation
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the GNOME NetworkManager WiFi scan path to support scanning with multiple SSID filters (instead of a single optional SSID string), aligning the plugin with the requirement to “scan multiple SSIDs”.
Changes:
- Updated
wifiScanRequestAPI to accept astd::vector<std::string>of SSIDs to filter. - Built a
GVariantaaySSID list fornm_device_wifi_request_scan_options_async()when filters are provided; otherwise falls back to a normal scan. - Updated the GNOME proxy to pass the SSID filter list through to the WiFi manager.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| plugin/gnome/NetworkManagerGnomeWIFI.h | Changes wifiScanRequest signature to take a vector of SSIDs. |
| plugin/gnome/NetworkManagerGnomeWIFI.cpp | Implements building a multi-SSID scan options payload and triggers filtered vs unfiltered scan. |
| plugin/gnome/NetworkManagerGnomeProxy.cpp | Passes the SSID filter list to the updated scan request API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool wifiConnectedSSIDInfo(Exchange::INetworkManager::WiFiSSIDInfo &ssidinfo); | ||
| bool wifiConnect(const Exchange::INetworkManager::WiFiConnectTo &ssidInfo); | ||
| bool wifiScanRequest(std::string ssidReq = ""); | ||
| bool wifiScanRequest(std::vector<std::string> ssidsToFilter = {}); |
| if(!ssidsToFilter.empty()) | ||
| { | ||
| NMLOG_INFO("starting wifi scanning .. %s", ssidReq.c_str()); | ||
| NMLOG_INFO("Starting wifi scanning for %d SSIDs:", ssidsToFilter.size()); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
| bool wifiConnectedSSIDInfo(Exchange::INetworkManager::WiFiSSIDInfo &ssidinfo); | ||
| bool wifiConnect(const Exchange::INetworkManager::WiFiConnectTo &ssidInfo); | ||
| bool wifiScanRequest(std::string ssidReq = ""); | ||
| bool wifiScanRequest(std::vector<std::string> ssidsToFilter = {}); |
| if(!ssidsToFilter.empty()) | ||
| { | ||
| NMLOG_INFO("starting wifi scanning .. %s", ssidReq.c_str()); | ||
| NMLOG_INFO("Starting wifi scanning for %d SSIDs:", ssidsToFilter.size()); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
Contributor
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
plugin/NetworkManagerJsonRpc.cpp:678
- Once
frequenciesis allocated here, later early returns in SSID parsing/iterator creation exit without releasing it. Add cleanup before those returns or manage the iterator with RAII so invalid SSID input or allocation failure does not leak the frequency iterator.
if (!frequencyList.empty()) {
using FrequencyIterator = RPC::IteratorType<Exchange::INetworkManager::IWIFIFrequencyIterator>;
frequencies = Core::Service<FrequencyIterator>::Create<Exchange::INetworkManager::IWIFIFrequencyIterator>(frequencyList);
if (frequencies == nullptr) {
returnJson(rc);
legacy/LegacyWiFiManagerAPIs.cpp:671
- After allocating
frequencieshere, the laterssids == nullptrearly return exits without releasing it. Add cleanup before returning on subsequent errors or use RAII for the iterator to avoid leaking the frequency iterator.
if (!frequencyList.empty()) {
using FrequencyIterator = RPC::IteratorType<Exchange::INetworkManager::IWIFIFrequencyIterator>;
frequencies = Core::Service<FrequencyIterator>::Create<Exchange::INetworkManager::IWIFIFrequencyIterator>(frequencyList);
if (frequencies == nullptr) {
returnJson(rc);
| || freq > static_cast<int>(Exchange::INetworkManager::WIFI_FREQUENCY_6_GHZ)) | ||
| { | ||
| NMLOG_ERROR("Invalid frequency value in array"); | ||
| return Core::ERROR_BAD_REQUEST; |
| || freq > static_cast<int>(Exchange::INetworkManager::WIFI_FREQUENCY_6_GHZ)) | ||
| { | ||
| NMLOG_ERROR("Invalid frequency value in array"); | ||
| return Core::ERROR_BAD_REQUEST; |
Comment on lines
+644
to
+665
| JsonArray array = parameters["frequency"].Array(); | ||
| JsonArray::Iterator index(array.Elements()); | ||
| while (index.Next() == true) | ||
| { | ||
| if (Core::JSON::Variant::type::NUMBER == index.Current().Content()) | ||
| { | ||
| const int freq = index.Current().Number(); | ||
| if (freq < static_cast<int>(Exchange::INetworkManager::WIFI_FREQUENCY_NONE) | ||
| || freq > static_cast<int>(Exchange::INetworkManager::WIFI_FREQUENCY_6_GHZ)) | ||
| { | ||
| NMLOG_ERROR("Invalid frequency value in array"); | ||
| return Core::ERROR_BAD_REQUEST; | ||
| returnJson(rc); | ||
| } | ||
| frequencyList.push_back(static_cast<Exchange::INetworkManager::WIFIFrequency>(freq)); | ||
| } | ||
| else | ||
| { | ||
| NMLOG_ERROR("Unexpected variant type in frequency array."); | ||
| returnJson(rc); | ||
| } | ||
| } |
Contributor
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
plugin/NetworkManagerJsonRpc.cpp:676
- After this iterator is created, the later
ssidsparsing paths can still return early on an invalid SSID entry or iterator allocation failure before reaching the cleanup block, which leaksfrequencies. Route those errors through a common cleanup path (or parse/validate all inputs before creating the iterator) so malformed requests do not leak RPC iterator objects.
frequencies = Core::Service<FrequencyIterator>::Create<Exchange::INetworkManager::IWIFIFrequencyIterator>(frequencyList);
legacy/LegacyWiFiManagerAPIs.cpp:669
- After this iterator is created, the subsequent
ssiditerator creation can return early on failure without releasingfrequencies. Use a common cleanup path or delay iterator creation until all input parsing/allocation has succeeded to avoid leaking the frequency iterator on error.
frequencies = Core::Service<FrequencyIterator>::Create<Exchange::INetworkManager::IWIFIFrequencyIterator>(frequencyList);
| } | ||
| else | ||
| { | ||
| NMLOG_ERROR("Unexpected variant type in frequency array."); |
Contributor
Contributor
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
plugin/NetworkManagerJsonRpc.cpp:670
- The non-numeric frequency path also returns with
rcleft asCore::ERROR_GENERAL. This should be treated as a bad request, matching the out-of-range validation path, before returning the JSON-RPC response.
NMLOG_ERROR("Unexpected variant type in frequency array.");
returnJson(rc);
| } | ||
|
|
||
| uint32_t NetworkManagerImplementation::StartWiFiScan(const string& frequency /* @in */, IStringIterator* const ssids/* @in */) | ||
| uint32_t NetworkManagerImplementation::StartWiFiScan(IWIFIFrequencyIterator* const frequencies /* @in */, IStringIterator* const ssids/* @in */) |
Comment on lines
+662
to
+663
| NMLOG_ERROR("Invalid frequency value in array"); | ||
| returnJson(rc); |
Comment on lines
+667
to
+670
| if (!frequencyList.empty()) { | ||
| using FrequencyIterator = RPC::IteratorType<Exchange::INetworkManager::IWIFIFrequencyIterator>; | ||
| frequencies = Core::Service<FrequencyIterator>::Create<Exchange::INetworkManager::IWIFIFrequencyIterator>(frequencyList); | ||
| if (frequencies == nullptr) { |
| } | ||
| else | ||
| { | ||
| NMLOG_ERROR("Unexpected variant type in frequency array."); |
Comment on lines
+1706
to
+1709
| for (const auto& ssid : ssidsToFilter) { | ||
| g_variant_builder_add(&array_builder, "@ay", | ||
| g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, (const guint8 *) ssid.c_str(), ssid.length(), 1) | ||
| ); |
Contributor
Contributor
Comment on lines
+988
to
+990
| if (frequencies && frequencies->Count() > 0) | ||
| { | ||
| m_filterfrequency = frequency; | ||
| NMLOG_DEBUG("Scan SSIDs of frequency %s", m_filterfrequency.c_str()); | ||
| NMLOG_DEBUG("Scan SSIDs of frequency not supported"); |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Support to scan multiple SSIDs
Test Procedure: Test wifi scan API with multiple SSIDs
Risks: Low
Signed-off-by: jincysaramma_sam@comcast.com