RDKEMW-516 : GetPublicIP Method is not taking ipversion & interface c…

…orrectly (rdkcentral#69) Reason for change: GetPublicIP Method is not taking ipversion & interface correctly Test Procedure: As per RDKEMW-516 Risks: Low Signed-off-by: kamirt573_comcast <[email protected]>
parvathika · Jan 3, 2025 · 3749f8f · 3749f8f
1 parent a196415
commit 3749f8f
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,11 @@ All notable changes to this RDK Service will be documented in this file.
 
 * Changes in CHANGELOG should be updated when commits are added to the main or release branches. There should be one CHANGELOG entry per JIRA Ticket. This is not enforced on sprint branches since there could be multiple changes for the same JIRA ticket during development. 
 
+## [0.8.0] - 2025-01-03
+### Changed
+- Updated the definition of GetPublicIP method to take interface as additional input
+- Updated the Legacy Plugin to use newly defined API
+
 ## [0.7.0] - 2024-12-30
 ### Added
 - Gnome WPS custom design implemented

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -5,7 +5,7 @@ find_package(WPEFramework)
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake")
 set(VERSION_MAJOR 0)
-set(VERSION_MINOR 7)
+set(VERSION_MINOR 8)
 set(VERSION_PATCH 0)
 
 add_compile_definitions(NETWORKMANAGER_MAJOR_VERSION=${VERSION_MAJOR})

diff --git a/INetworkManager.h b/INetworkManager.h
@@ -243,7 +243,7 @@ namespace WPEFramework
             virtual uint32_t StopConnectivityMonitoring(void) const = 0;
 
             /* @brief Get the Public IP used for external world communication */
-            virtual uint32_t GetPublicIP (string &ipversion /* @inout */,  string& ipaddress /* @out */) = 0;
+            virtual uint32_t GetPublicIP (string& interface /* @inout */, string &ipversion /* @inout */,  string& ipaddress /* @out */) = 0;
 
             /* @brief Request for ping and get the response in as event. The GUID used in the request will be returned in the event. */
             virtual uint32_t Ping (const string ipversion /* @in */,  const string endpoint /* @in */, const uint32_t count /* @in */, const uint16_t timeout /* @in */, const string guid /* @in */, string& response /* @out */) = 0;

diff --git a/LegacyPlugin_NetworkAPIs.cpp b/LegacyPlugin_NetworkAPIs.cpp
@@ -712,13 +712,20 @@ const string CIDR_PREFIXES[CIDR_NETMASK_IP_LEN+1] = {
             uint32_t rc = Core::ERROR_GENERAL;
             string ipAddress{};
             string ipversion = "IPv4";
-            if (parameters.HasLabel("ipversion"))
-                ipversion = parameters["ipversion"].String();
+            if (parameters.HasLabel("ipv6") && parameters["ipv6"].Boolean())
+                ipversion = "IPv6";
+
+            string interface = "";
+            if (parameters.HasLabel("iface"))
+            {
+                string givenInterface = parameters["iface"].String();
+                interface = getInterfaceTypeToName(givenInterface);
+            }
 
             auto _nwmgr = m_service->QueryInterfaceByCallsign<Exchange::INetworkManager>(NETWORK_MANAGER_CALLSIGN);
             if (_nwmgr)
             {
-                rc = _nwmgr->GetPublicIP(ipversion, ipAddress);
+                rc = _nwmgr->GetPublicIP(interface, ipversion, ipAddress);
                 _nwmgr->Release();
             }
             else

diff --git a/NetworkManager.json b/NetworkManager.json
@@ -9,7 +9,7 @@
         "status": "development",
         "description": "A Unified `NetworkManager` plugin that allows you to manage Ethernet and Wifi interfaces on the device.",
         "sourcelocation": "https://github.com/rdkcentral/networkmanager/blob/main/NetworkManager.json",
-        "version": "0.7.0"
+        "version": "0.8.0"
     },
     "definitions": {
         "success": {
@@ -423,7 +423,7 @@
                     "gateway",
                     "primarydns",
                     "secondarydns",
-                    "success"  
+                    "success"
                 ]
             }
         },
@@ -714,6 +714,9 @@
                 "type":"object",
                 "summary":"it allows empty parameter too",
                 "properties": {
+                    "interface":{
+                        "$ref": "#/definitions/interface"
+                    },
                     "ipversion": {
                         "$ref": "#/definitions/ipversion"
                     }
@@ -724,19 +727,23 @@
             "result": {
                 "type": "object",
                 "properties": {
-                    "ipaddress": {
-                        "$ref": "#/definitions/ipaddress"
+                    "interface":{
+                        "$ref": "#/definitions/interface"
                     },
                     "ipversion": {
                         "$ref": "#/definitions/ipversion"
                     },
+                    "ipaddress": {
+                        "$ref": "#/definitions/ipaddress"
+                    },
                     "success": {
                         "$ref": "#/definitions/success"
                     }
                 },
                 "required": [
-                    "ipaddress",
+                    "interface",
                     "ipversion",
+                    "ipaddress",
                     "success"
                 ]
             }
@@ -846,7 +853,7 @@
                 "type":"object",
                 "properties": {
                     "endpoint":{
-                        "$ref": "#/definitions/endpoint"                       
+                        "$ref": "#/definitions/endpoint"
                     },
                     "ipversion": {
                         "$ref": "#/definitions/ipversion"
@@ -867,7 +874,7 @@
                 "type": "object",
                 "properties": {
                     "endpoint":{
-                        "$ref": "#/definitions/endpoint"                       
+                        "$ref": "#/definitions/endpoint"
                     },
                    "results": {
                         "summary": "The response of traceroute",
@@ -1333,7 +1340,7 @@
                                 "type": "integer",
                                 "example": 12
                             },
-	                   "NET_WIFI_SECURITY_WPA3_PSK_AES": {
+                            "NET_WIFI_SECURITY_WPA3_PSK_AES": {
                                 "type": "integer",
                                 "example": 13
                             },
@@ -1387,7 +1394,7 @@
                 "type": "object",
                 "properties": {
                     "interface":{
-                        "$ref": "#/definitions/interface" 
+                        "$ref": "#/definitions/interface"
                     },
                     "state":{
                         "summary": "Current state of the interface",
@@ -1546,7 +1553,7 @@
                     "state":{
                         "summary": "WiFi State",
                         "type": "integer",
-                        "example": "5"	
+                        "example": "5"
                     },
                     "status": {
                         "summary": "WiFi status",
@@ -1568,17 +1575,17 @@
                     "ssid":{
                         "summary": "Signal Strength changed SSID",
                         "type": "string",
-                        "example": "home-new_123"	
+                        "example": "home-new_123"
                     },
                     "strength":{
                         "summary": "Signal Strength",
                         "type": "string",
-                        "example": "-27.000000"	
+                        "example": "-27.000000"
                     },
                     "quality":{
                         "summary": "Signal quality",
                         "type": "string",
-                        "example": "Excellent"	
+                        "example": "Excellent"
                     }
                 },
                 "required": [

diff --git a/NetworkManagerImplementation.cpp b/NetworkManagerImplementation.cpp
@@ -301,20 +301,30 @@ namespace WPEFramework
         }
 
         /* @brief Get the Public IP used for external world communication */
-        uint32_t NetworkManagerImplementation::GetPublicIP (string &ipversion /* @inout */,  string& ipaddress /* @out */)
+        uint32_t NetworkManagerImplementation::GetPublicIP (string& interface /* @inout */, string &ipversion /* @inout */,  string& ipaddress /* @out */)
         {
             LOG_ENTRY_FUNCTION();
             stun::bind_result result;
             bool isIPv6 = (ipversion == "IPv6");
 
+            // Either Interface must be connected to get the public IP
+            if (!(m_ethConnected | m_wlanConnected))
+            {
+                NMLOG_WARNING("No interface Connected");
+                return Core::ERROR_GENERAL;
+            }
+
             stun::protocol  proto (isIPv6 ? stun::protocol::af_inet6  : stun::protocol::af_inet);
-            if(stunClient.bind(m_stunEndpoint, m_stunPort, m_defaultInterface, proto, m_stunBindTimeout, m_stunCacheTimeout, result))
+            if(stunClient.bind(m_stunEndpoint, m_stunPort, interface, proto, m_stunBindTimeout, m_stunCacheTimeout, result))
             {
                 if (isIPv6)
                     ipversion = "IPv6";
                 else
                     ipversion = "IPv4";
 
+                if (interface.empty())
+                    interface = m_defaultInterface;
+
                 ipaddress = result.public_ip;
                 return Core::ERROR_NONE;
             }
@@ -630,6 +640,9 @@ namespace WPEFramework
             else if (currentActiveinterface == "wlan0")
                 m_wlanConnected = true;
 
+            // FIXME : This could be the place to define `m_defaultInterface` to incoming `currentActiveinterface`.
+            // m_defaultInterface = currentActiveinterface;
+
             for (const auto callback : _notificationCallbacks) {
                 callback->onActiveInterfaceChange(prevActiveInterface, currentActiveinterface);
             }
@@ -646,6 +659,12 @@ namespace WPEFramework
                 else if(interface == "wlan0")
                     m_wlanConnected = true;
 
+                // FIXME : Availability of ip address for a given interface does not mean that its the default interface. This hardcoding will work for RDKProxy but not for Gnome.
+                if (m_ethConnected && m_wlanConnected)
+                    m_defaultInterface = "eth0";
+                else
+                    m_defaultInterface = interface;
+
                 // Start the connectivity monitor with 'true' to indicate the interface is up.
                 // The monitor will conntinoue even after no internet retry completed, Exit when fully connectd.
                 connectivityMonitor.startConnectivityMonitor();

diff --git a/NetworkManagerImplementation.h b/NetworkManagerImplementation.h
@@ -230,7 +230,7 @@ namespace WPEFramework
                 uint32_t StopConnectivityMonitoring(void) const override;
 
                 /* @brief Get the Public IP used for external world communication */
-                uint32_t GetPublicIP (string &ipversion /* @inout */,  string& ipaddress /* @out */) override;
+                uint32_t GetPublicIP (string& interface /* @inout */, string &ipversion /* @inout */,  string& ipaddress /* @out */) override;
 
                 /* @brief Request for ping and get the response in as event. The GUID used in the request will be returned in the event. */
                 uint32_t Ping (const string ipversion /* @in */,  const string endpoint /* @in */, const uint32_t noOfRequest /* @in */, const uint16_t timeOutInSeconds /* @in */, const string guid /* @in */, string& response /* @out */) override;

diff --git a/NetworkManagerJsonRpc.cpp b/NetworkManagerJsonRpc.cpp
@@ -535,30 +535,27 @@ namespace WPEFramework
         {
             LOG_INPARAM();
             uint32_t rc = Core::ERROR_GENERAL;
-            string ipAddress{};
+            string interface{};
+            string ipaddress{};
             string ipversion = "IPv4";
             if (parameters.HasLabel("ipversion"))
                 ipversion = parameters["ipversion"].String();
 
-            if ((!m_publicIPAddress.empty()) && (m_publicIPAddressType == ipversion))
-            {
-                rc = Core::ERROR_NONE;
-                ipAddress = m_publicIPAddress;
-            }
+            if (parameters.HasLabel("interface"))
+                interface = parameters["interface"].String();
+
+            if (_networkManager)
+                rc = _networkManager->GetPublicIP(interface, ipversion, ipaddress);
             else
-            {
-                if (_networkManager)
-                    rc = _networkManager->GetPublicIP(ipversion, ipAddress);
-                else
-                    rc = Core::ERROR_UNAVAILABLE;
-            }
+                rc = Core::ERROR_UNAVAILABLE;
 
             if (Core::ERROR_NONE == rc)
             {
-                response["ipaddress"] = ipAddress;
+                response["interface"] = interface;
+                response["ipaddress"] = ipaddress;
                 response["ipversion"] = ipversion;
 
-                m_publicIPAddress = ipAddress;
+                m_publicIPAddress = ipaddress;
                 m_publicIPAddressType = ipversion;
                 PublishToThunderAboutInternet();
             }
@@ -567,15 +564,8 @@ namespace WPEFramework
 
         void NetworkManager::PublishToThunderAboutInternet()
         {
-            if (m_publicIPAddress.empty())
-            {
-                JsonObject input, output;
-                GetPublicIP(input, output);
-            }
-
             if (!m_publicIPAddress.empty())
             {
-                NMLOG_DEBUG("No public IP persisted yet; Update the data");
                 PluginHost::ISubSystem* subSystem = _service->SubSystems();
 
                 if (subSystem != nullptr)

diff --git a/docs/NetworkManagerPlugin.md b/docs/NetworkManagerPlugin.md
@@ -2,7 +2,7 @@
 <a name="head.NetworkManager_Plugin"></a>
 # NetworkManager Plugin
 
-**Version: 0.7.0**
+**Version: 0.8.0**
 
 **Status: :white_circle::white_circle::white_circle:**
 
@@ -23,7 +23,7 @@ org.rdk.NetworkManager interface for Thunder framework.
 <a name="head.Scope"></a>
 ## Scope
 
-This document describes purpose and functionality of the org.rdk.NetworkManager interface (version 0.7.0). It includes detailed specification about its methods provided and notifications sent.
+This document describes purpose and functionality of the org.rdk.NetworkManager interface (version 0.8.0). It includes detailed specification about its methods provided and notifications sent.
 
 <a name="head.Case_Sensitivity"></a>
 ## Case Sensitivity
@@ -960,15 +960,17 @@ Gets the internet/public IP Address of the device.
 | Name | Type | Description |
 | :-------- | :-------- | :-------- |
 | params | object | It allows empty parameter too |
+| params?.interface | string | <sup>*(optional)*</sup> An interface, such as `eth0` or `wlan0`, depending upon availability of the given interface |
 | params?.ipversion | string | <sup>*(optional)*</sup> Either IPv4 or IPv6 |
 
 ### Result
 
 | Name | Type | Description |
 | :-------- | :-------- | :-------- |
 | result | object |  |
-| result.ipaddress | string | The IP address |
+| result.interface | string | An interface, such as `eth0` or `wlan0`, depending upon availability of the given interface |
 | result.ipversion | string | Either IPv4 or IPv6 |
+| result.ipaddress | string | The IP address |
 | result.success | boolean | Whether the request succeeded |
 
 ### Example
@@ -981,6 +983,7 @@ Gets the internet/public IP Address of the device.
   "id": 42,
   "method": "org.rdk.NetworkManager.1.GetPublicIP",
   "params": {
+    "interface": "wlan0",
     "ipversion": "IPv4"
   }
 }
@@ -993,8 +996,9 @@ Gets the internet/public IP Address of the device.
   "jsonrpc": "2.0",
   "id": 42,
   "result": {
-    "ipaddress": "192.168.1.101",
+    "interface": "wlan0",
     "ipversion": "IPv4",
+    "ipaddress": "192.168.1.101",
     "success": true
   }
 }