RDKEMW-15309 : Improvements in Wifi Connection Processing in Network Manager plugin by cmuhammedrafi · Pull Request #300 · rdkcentral/networkmanager

cmuhammedrafi · 2026-04-09T13:51:23Z

No description provided.

…#283) * spike Initial code change

Reason for change: Integrated breakpad support in networkmanager plugin Test Procedure: Check whether minidumps are generated properly Priority:P1 Risks: Medium Signed-off-by: Gururaaja ESR<Gururaja_ErodeSriranganRamlingham@comcast.com>

bug fix

This reverts commit 63073e7.

rdkcmf-jenkins · 2026-04-09T13:55:08Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

Protex Server Path: /home/blackduck/github/networkmanager/300/rdkcentral/networkmanager
Commit: 47c331e

Report detail: gist'

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for connecting to a saved WiFi connection by SSID and extends WiFi connection handling with optional BSSID + frequency selection, along with corresponding JSON-RPC, interface, events, docs, and tests updates.

Changes:

Introduce ConnectToKnownSSID across interface, implementations (RDK + GNOME), and JSON-RPC registration.
Extend WiFiConnect inputs to accept optional bssid and frequency, and include BSSID in scan results events.
Update/expand L2 tests and improve object-path/memory handling in the GNOME WiFi implementation.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
tests/mocks/INetworkManagerMock.h	Adds mock for new `ConnectToKnownSSID` API.
tests/l2Test/libnm/l2_test_libnmproxyWifi.cpp	Adds coverage for wrong BSSID, known-SSID connect, and extra AP property mocking.
tests/l2Test/libnm/l2_test_libnmproxyEvent.cpp	Adjusts scan/event test setup to include device list and BSSID mocking.
plugin/rdk/NetworkManagerRDKProxy.cpp	Implements `ConnectToKnownSSID` as unavailable for RDK backend.
plugin/gnome/NetworkManagerGnomeWIFI.h	Adds GNOME `connectToKnownSSID` and changes `m_objectPath` ownership type.
plugin/gnome/NetworkManagerGnomeWIFI.cpp	Implements known-SSID connect; adds BSSID/frequency handling and various lifecycle fixes.
plugin/gnome/NetworkManagerGnomeUtils.h	Declares BSSID validator helper.
plugin/gnome/NetworkManagerGnomeUtils.cpp	Implements BSSID format validation; adjusts nmcli logging domains.
plugin/gnome/NetworkManagerGnomeProxy.cpp	Wires `ConnectToKnownSSID` into GNOME implementation; changes WiFiConnect behavior/validation.
plugin/gnome/NetworkManagerGnomeEvents.cpp	Adds BSSID to scan results and strengthens scan callback checks.
plugin/NetworkManagerJsonRpc.cpp	Registers `ConnectToKnownSSID`; parses `bssid`/`frequency` for `WiFiConnect`.
plugin/NetworkManagerImplementation.h	Changes process monitor interval constant and adds interface method.
plugin/NetworkManager.h	Exposes JSON-RPC handler declaration for `ConnectToKnownSSID`.
plugin/CMakeLists.txt	Whitespace-only formatting changes.
interface/INetworkManager.h	Adds `WIFIFrequency`, `bssid/frequency` to connect struct, and new interface method.
docs/NetworkManagerPlugin.md	Documents new RPC + new parameters and adds BSSID to scan results event docs.
definition/NetworkManager.json	Adds schema for `ConnectToKnownSSID`, makes WiFiConnect params optional, and adds BSSID to event schema.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T14:00:18Z

+            const char* specificObjPath = "/";
+            NMConnection *knownConnection = NULL;
+            bool ret = false;
+
+            if(!createClientNewConnection())
+                return ret;
+
+            m_wifidevice = getWifiDevice();
+            if(m_wifidevice == NULL)
+            {
+                deleteClientConnection();
+                return false;
+            }


nm_client_activate_connection_async expects specific_object to be either a valid NetworkManager object path (e.g., an AP path) or NULL. Passing "/" is typically not a valid NM object path and can cause activation to fail. Use NULL for “no specific object”, or pass a real AP object path when available.

bpunnuru · 2026-04-09T15:09:16Z

+            if (!std::regex_match(bssid, bssidRegex))
+            {
+                NMLOG_ERROR("Invalid BSSID format: %s. Expected format is XX:XX:XX:XX:XX:XX where X is a hexadecimal digit.", bssid.c_str());
+                return false;


exposing BSSID is valid? Can u check if it violates data privacy ?

bpunnuru · 2026-04-09T15:12:50Z

            // Connection settings with autoconnect enabled
            NMSettingConnection *sConnection = (NMSettingConnection *)nm_setting_connection_new();
            std::string connId = "Wired connection 1";
+            char *uuidRawEth = nm_utils_uuid_generate();


pls use std::string

bpunnuru · 2026-04-09T15:14:27Z

+                    }
+
+                    std::string bssidStr = bssid;
+                    if(strcasecmp(ssidInfo.bssid.c_str(), bssidStr.c_str()) == 0)


why are we not using = operator in string?

mac address return as upper-case AA:BB:CC:DD:EE:FF" Ap info return as lower case

mhughesacn · 2026-04-09T16:13:59Z

                        NM_SETTING_CONNECTION_AUTOCONNECT, TRUE,  // Enable autoconnect
                        NULL);
+            g_free(uuidRawEth);
            nm_connection_add_setting(connection, NM_SETTING(sConnection));


Hi @cmuhammedrafi : Lines 629-660 are coming up with a code match (to the GNOME Network Manager) which may be due to a periodic upgrade of the Blackduck code database. Please will you confirm that the code in this range is standard use of NM APIs and looks similar because this is the sequence of APIs that you have to call.

bpunnuru · 2026-04-09T20:19:12Z

                free(ssidStr);
             }
             ssidObj["ssid"] = ssidString;
+             bssid = nm_access_point_get_bssid(ap);


nm_access_point_get_bssid returns const char*; we must copy the value locally to reuse its contents.

bpunnuru · 2026-04-09T20:32:47Z

            gboolean m_createNewConnection;
            GMainContext *m_nmContext = nullptr;
-            const char* m_objectPath;
+            char* m_objectPath = nullptr;


can we use std::string?

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

plugin/gnome/NetworkManagerGnomeWIFI.cpp:1162

If AccessPoint is null (SSID not found) and persist is false, the function continues and later calls APIs like nm_access_point_connection_valid(AccessPoint, ...) / nm_object_get_path(NM_OBJECT(AccessPoint)) with a null AccessPoint, which is undefined behavior. Return failure immediately when the SSID isn’t found and you’re not going to add/persist it, or add explicit null-guards before any AccessPoint use.

            AccessPoint = findMatchingSSID(ApList, ssidInfo);
            if(AccessPoint == NULL)
            {
                NMLOG_WARNING("SSID '%s' not found !", ssidInfo.ssid.c_str());
                // if(_instance != nullptr)
                //     _instance->ReportWiFiStateChange(Exchange::INetworkManager::WIFI_STATE_SSID_NOT_FOUND);
                /* ssid not found in scan list so add to known ssid it will do a scanning and connect */
                if(ssidInfo.persist)
                {
                    if(addToKnownSSIDs(ssidInfo))
                    {
                        NMLOG_DEBUG("Adding to known ssid '%s' ", ssidInfo.ssid.c_str());
                        deleteClientConnection();
                        return activateKnownConnection(nmUtils::wlanIface(), ssidInfo.ssid);
                    }
                    else
                    {
                        deleteClientConnection();
                        return false;
                    }
                }
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            const char* specificObjPath = "/";
+            NMConnection *knownConnection = NULL;
+            bool ret = false;
+
+            if(!createClientNewConnection())
+                return ret;
+
+            m_wifidevice = getWifiDevice();
+            if(m_wifidevice == NULL)
+            {
+                deleteClientConnection();
+                return false;
+            }
+
+            allnmConn = nm_client_get_connections(m_client);
+            if(allnmConn == NULL || allnmConn->len == 0)
+            {
+                NMLOG_ERROR("No connections found !");
+                deleteClientConnection();
+                return ret;
+            }
+
+            for (guint i = 0; i < allnmConn->len; i++)
+            {
+                NMConnection *conn = static_cast<NMConnection*>(g_ptr_array_index(allnmConn, i));
+                if(conn == NULL)
+                    continue;
+
+                const char *connId = nm_connection_get_id(NM_CONNECTION(conn));
+                if (connId == NULL) {
+                    NMLOG_WARNING("connection id is NULL");
+                    continue;
+                }
+
+                const char *connTyp = nm_connection_get_connection_type(NM_CONNECTION(conn));
+                if (connTyp == NULL) {
+                    NMLOG_WARNING("connection type is NULL");
+                    continue;
+                }
+
+                std::string connTypStr = connTyp;
+                if(connTypStr != "802-11-wireless")
+                {
+                    NMLOG_DEBUG("%s not a wifi connection", connId);
+                    continue;
+                }
+
+                if(ssid == connId)
+                {
+                    knownConnection = g_object_ref(conn);
+                    NMLOG_DEBUG("connection '%s' exists !", ssid.c_str());
+                    break;
+                }
+            }
+
+            if(knownConnection != NULL)
+            {
+                NMLOG_INFO("activating known wifi '%s' connection", ssid.c_str());
+                m_isSuccess = false;
+                m_createNewConnection = false; // no need to create new connection
+                nm_client_activate_connection_async(m_client, NM_CONNECTION(knownConnection), m_wifidevice, specificObjPath, m_cancellable, wifiConnectCb, this);
+                wait(m_loop);


+                }
+
+                ssidstr = ssidUtf8;
+                free(ssidUtf8);


        {
            /* set networkmanager daemon log level based on current plugin log level */
-            const char* command = "nmcli general logging level TRACE domains ALL";
+            const char* command = "nmcli general logging level TRACE domains PLATFORM,DEVICE,WIFI,ETHER,DNS,DHCP4,DHCP6,DEVICE,SETTINGS,DISPATCH";


+| params?.passphrase | string | <sup>*(optional)*</sup> The access point password |
+| params?.security | integer | <sup>*(optional)*</sup> Optional override to explicitly select the security mode; if omitted, the mode is decided based on the highest security mode provided by the SSID. See `GetSupportedSecurityModes` for valid values |
+| params?.bssid | string | <sup>*(optional)*</sup> BSSID to connect to. If specified, restricts connection to this BSSID only. Defaults to the best available BSSID for the SSID |
+| params?.frequency | integer | <sup>*(optional)*</sup> Frequency band: `1` - 2.4GHz, `2` - 5GHz. If specified, connects only to SSIDs on the given frequency band. Defaults to best available |


+                        "summary": "Frequency band: `1` - 2.4GHz, `2` - 5GHz. If specified, connects only to SSIDs on the given frequency band. Defaults to best available.",
+                        "type": "integer",


-| params?.security | integer | <sup>*(optional)*</sup> Optional override to explicitly select the security mode; if omitted, the mode is decided based on the highest security mode provided by the SSID. See `GetSupportedSecurityModes` for valid values. |
+| params?.ssid | string | <sup>*(optional)*</sup> The WiFi SSID Name |
+| params?.passphrase | string | <sup>*(optional)*</sup> The access point password |
+| params?.security | integer | <sup>*(optional)*</sup> Optional override to explicitly select the security mode; if omitted, the mode is decided based on the highest security mode provided by the SSID. See `GetSupportedSecurityModes` for valid values |


@@ -776,6 +782,16 @@ namespace WPEFramework
                free(ssidStr);


+            if(ssid.ssid.size() > 32)
            {
-                NMLOG_WARNING("ssid is empty activating last connectd ssid !");
-                if(wifi->activateKnownConnection(nmUtils::wlanIface(), _instance->m_lastConnectedSSID))
-                    rc = Core::ERROR_NONE;
+                NMLOG_WARNING("SSID is invalid");
+                return rc;
            }
-            else if(ssid.ssid.size() <= 32)
+
+            if(!ssid.bssid.empty() && !nmUtils::isValidBSSID(ssid.bssid))
            {
-                if(wifi->wifiConnect(ssid))
-                    rc = Core::ERROR_NONE;
+                return Core::ERROR_GENERAL;
            }


cmuhammedrafi and others added 30 commits March 3, 2026 15:15

RDK-60453 [NW-Plugin]BSSID Improvements in Wifi Connection Processing (…

ad4928e

…#283) * spike Initial code change

InetworkmanagerMock fix

ecf6c0a

test case updated

07efcc9

api name changed

84b7e34

code cleanup

d2230b3

improved test coverage

4a6d69d

already connected check updated

4eb7fe6

error fix

8610a71

unit test case updated

eea308b

update the review comments

a332956

Copilot review comments

47b7a8d

review comment updated

8e9bd41

Merge branch 'develop' into topic/RDK-60454-Dev

4e92b32

doc updated

f9427da

update log line

f342812

specific object path null check added

4bbda5a

Merge branch 'develop' into topic/RDK-60454-Dev

8394e2e

copilot review comment updated

638ea20

loglevel updated

4fc89f6

address sanitizer added

c729144

add process monitor memmeory

2557fcd

networkmanager asan linking

9b18c62

wait debugg added

4218b0e

wificonnect scanning removed

44fa377

removed address sanitizer

7c4fd3f

AP object path fix

e11eb01

bug fix

gamilLoop getContext removed

8ca99d9

unref m_connection

e83673f

more memmory leak fix

6b6e049

Revert "RDKEMW-15980: Adding breakpad support in networkmanager plugin"

47c331e

This reverts commit 63073e7.

cmuhammedrafi requested a review from a team as a code owner April 9, 2026 13:51

Copilot AI review requested due to automatic review settings April 9, 2026 13:51

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Copilot started reviewing on behalf of cmuhammedrafi April 9, 2026 14:08 View session