You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/05/13 08:39:30 UTC

[GitHub] [cordova-plugin-network-information] ZumelzuR opened a new pull request #130: 5g feature

ZumelzuR opened a new pull request #130:
URL: https://github.com/apache/cordova-plugin-network-information/pull/130


   ### Platforms affected
   
   
   
   ### Motivation and Context
         Solve 5g detections for iOS and Android (the current state of the plugin classify as UKNOWN the 5g network)
        open issue -> https://github.com/apache/cordova-plugin-network-information/issues/125
   
   ### Description
        We added a a new listener for check if NR is available and using that, if is the case, detect the 5g.
   
   
   
   ### Testing
           We tested on android simulator and a physical android 10 and iOS simulator
   
   ### Checklist
   
   - [ x ] I've run the tests to see all new and existing tests pass
   - [ x] I added automated test coverage as appropriate for this change
   - [x ] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
   - [ x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
   - [x ] I've updated the documentation if necessary
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-network-information] almothafar commented on pull request #130: 5g feature

Posted by GitBox <gi...@apache.org>.
almothafar commented on pull request #130:
URL: https://github.com/apache/cordova-plugin-network-information/pull/130#issuecomment-920054976


   Any news about when to release this fix 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-network-information] ZumelzuR commented on pull request #130: 5g feature

Posted by GitBox <gi...@apache.org>.
ZumelzuR commented on pull request #130:
URL: https://github.com/apache/cordova-plugin-network-information/pull/130#issuecomment-952110519


   Thank, these days are a little problematic for me but I will check and resolve all as soon as possible


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-network-information] breautek commented on a change in pull request #130: 5g feature

Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #130:
URL: https://github.com/apache/cordova-plugin-network-information/pull/130#discussion_r728992603



##########
File path: src/android/NetworkManager.java
##########
@@ -222,16 +244,20 @@ private void updateConnectionInfo(NetworkInfo info) {
      * @param info the current active network info
      * @return type the type of network
      */
-    private String getTypeOfNetworkFallbackToTypeNoneIfNotConnected(NetworkInfo info) {
+    @RequiresApi(api = Build.VERSION_CODES.N)

Review comment:
       I don't think we can use `@RequiresAPI`... cordova-android 10.x supports Android API 22 - 30.
   
   Android N is Android 7.0, or Android API 24. If this method requires something only accessible on API >= 24 devices, then we must encapsulate that part of the code in a condition rather thank saying the entire method requires API 24. Especially since this method appears to include important logic for all android devices.

##########
File path: src/android/NetworkManager.java
##########
@@ -240,6 +266,11 @@ private String getTypeOfNetworkFallbackToTypeNoneIfNotConnected(NetworkInfo info
         LOG.d(LOG_TAG, "Connection Type: " + type);
         return type;
     }
+    @RequiresApi(api = Build.VERSION_CODES.N)
+    private String getTypeOfNetworkFallbackToTypeNoneIfNotConnected(NetworkInfo info) {

Review comment:
       I don't think we can use `@RequiresAPI`... cordova-android 10.x supports Android API 22 - 30.
   
   Android N is Android 7.0, or Android API 24. If this method requires something only accessible on API >= 24 devices, then we must encapsulate that part of the code in a condition rather thank saying the entire method requires API 24. Especially since this method appears to include important logic for all android devices.

##########
File path: src/android/NetworkManager.java
##########
@@ -78,29 +92,37 @@ Licensed to the Apache Software Foundation (ASF) under one
     public static final String TYPE_2G = "2g";
     public static final String TYPE_3G = "3g";
     public static final String TYPE_4G = "4g";
+    public static final String TYPE_5G = "5g";
     public static final String TYPE_NONE = "none";
 
+    public static final int NETWORK_TYPE_NR = 20;
+    public static final int NETWORK_TYPE_LTE_CA = 19;
+
     private static final String LOG_TAG = "NetworkManager";
 
     private CallbackContext connectionCallbackContext;
 
     ConnectivityManager sockMan;
     BroadcastReceiver receiver;
-    private String lastTypeOfNetwork;
+    private String lastTypeOfNetwork = TYPE_UNKNOWN;
+    TelephonyManager telMan;
+    private static boolean isNrAvailable;
 
     /**
-     * Sets the context of the Command. This can then be used to do things like
-     * get file paths associated with the Activity.
-     *
-     * @param cordova The context of the main Activity.
-     * @param webView The CordovaWebView Cordova is running in.
-     */
+   * Sets the context of the Command. This can then be used to do things like
+   * get file paths associated with the Activity.
+   *
+   * @param cordova The context of the main Activity.
+   * @param webView The CordovaWebView Cordova is running in.
+   */
     public void initialize(CordovaInterface cordova, CordovaWebView webView) {

Review comment:
       You've removed the indentation of this method, please revert to maintain code styling.

##########
File path: src/android/NetworkManager.java
##########
@@ -25,19 +25,28 @@ Licensed to the Apache Software Foundation (ASF) under one
 import org.apache.cordova.PluginResult;
 import org.apache.cordova.CordovaWebView;
 import org.json.JSONArray;
-import org.json.JSONException;
-import org.json.JSONObject;
 
+import android.Manifest;
+import android.annotation.SuppressLint;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
+import android.content.pm.PackageManager;
 import android.net.ConnectivityManager;
 import android.net.NetworkInfo;
 import android.os.Build;
+import android.telephony.PhoneStateListener;
+import android.telephony.TelephonyManager;
+import android.telephony.ServiceState;
+
+import androidx.annotation.RequiresApi;

Review comment:
       Given the notes below, I suspect we can remove `RequiresApi` import.

##########
File path: package.json
##########
@@ -1,6 +1,6 @@
 {
   "name": "cordova-plugin-network-information",
-  "version": "3.0.0-dev",
+  "version": "3.0.1-dev",

Review comment:
       The PR shouldn't be touching version numbers. Please revert this change.

##########
File path: plugin.xml
##########
@@ -21,7 +21,7 @@
 <plugin xmlns="http://apache.org/cordova/ns/plugins/1.0"
 xmlns:android="http://schemas.android.com/apk/res/android"
            id="cordova-plugin-network-information"
-      version="3.0.0-dev">
+      version="3.0.1-dev">

Review comment:
       The PR shouldn't be touching version numbers. Please revert this change.

##########
File path: src/android/NetworkManager.java
##########
@@ -270,28 +301,104 @@ private String getType(NetworkInfo info) {
         } else if (type.toLowerCase().equals(TYPE_ETHERNET) || type.toLowerCase().startsWith(TYPE_ETHERNET_SHORT)) {
             return TYPE_ETHERNET;
         } else if (type.equals(MOBILE) || type.equals(CELLULAR)) {
-            type = info.getSubtypeName().toLowerCase(Locale.US);
-            if (type.equals(GSM) ||
-                    type.equals(GPRS) ||
-                    type.equals(EDGE) ||
-                    type.equals(TWO_G)) {
-                return TYPE_2G;
-            } else if (type.startsWith(CDMA) ||
-                    type.equals(UMTS) ||
-                    type.equals(ONEXRTT) ||
-                    type.equals(EHRPD) ||
-                    type.equals(HSUPA) ||
-                    type.equals(HSDPA) ||
-                    type.equals(HSPA) ||
-                    type.equals(THREE_G)) {
-                return TYPE_3G;
-            } else if (type.equals(LTE) ||
-                    type.equals(UMB) ||
-                    type.equals(HSPA_PLUS) ||
-                    type.equals(FOUR_G)) {
-                return TYPE_4G;
+            return getMobileType(info);
+        }
+        return TYPE_UNKNOWN;
+    }
+
+    /**
+     * Determine the subtype of mobile connection
+     *
+     * @param info the network info so we can determine connection type.
+     * @return the type of mobile network we are on
+     */
+    private String getMobileType(NetworkInfo info){
+        int subTypeId = info.getSubtype();
+        String subTypeName = info.getSubtypeName().toLowerCase(Locale.US);
+        if(is2G(subTypeId, subTypeName)){
+            return TYPE_2G;
+        } else if(is3G(subTypeId, subTypeName)) {
+            return TYPE_3G;
+        } else if(is4G(subTypeId, subTypeName)) {
+            if(isNrAvailable){ // if is LTE network could be 5g if NR is available
+                return TYPE_5G;
             }
+            return TYPE_4G;
+        } else if(is5G(subTypeId, subTypeName)) {
+            return TYPE_5G;
         }
         return TYPE_UNKNOWN;
     }
+
+    private boolean is2G(int type, String name){
+        return  type == TelephonyManager.NETWORK_TYPE_GPRS ||
+                type == TelephonyManager.NETWORK_TYPE_EDGE ||
+                type == TelephonyManager.NETWORK_TYPE_CDMA ||
+                type == TelephonyManager.NETWORK_TYPE_1xRTT ||
+                type == TelephonyManager.NETWORK_TYPE_IDEN ||    // api< 8: replace by 11
+                type == TelephonyManager.NETWORK_TYPE_GSM ||     // api<25: replace by 16
+                name.equals(GSM) ||
+                name.equals(GPRS) ||
+                name.equals(EDGE) ||
+                name.equals(TWO_G);
+    }
+
+    private boolean is3G(int type, String name){
+        return  type ==  TelephonyManager.NETWORK_TYPE_UMTS ||
+                type ==  TelephonyManager.NETWORK_TYPE_EVDO_0 ||
+                type ==  TelephonyManager.NETWORK_TYPE_EVDO_A ||
+                type ==  TelephonyManager.NETWORK_TYPE_HSDPA ||
+                type ==  TelephonyManager.NETWORK_TYPE_HSUPA ||
+                type ==  TelephonyManager.NETWORK_TYPE_HSPA ||
+                type ==  TelephonyManager.NETWORK_TYPE_EVDO_B ||   // api< 9: replace by 12
+                type ==  TelephonyManager.NETWORK_TYPE_EHRPD ||    // api<11: replace by 14
+                type ==  TelephonyManager.NETWORK_TYPE_HSPAP ||    // api<13: replace by 15
+                type ==  TelephonyManager.NETWORK_TYPE_TD_SCDMA || // api<25: replace by 17
+                name.startsWith(CDMA) ||
+                name.equals(UMTS) ||
+                name.equals(ONEXRTT) ||
+                name.equals(EHRPD) ||
+                name.equals(HSUPA) ||
+                name.equals(HSDPA) ||
+                name.equals(HSPA) ||
+                name.equals(THREE_G);
+    }
+
+    private boolean is4G(int type, String name){
+
+        return  type == TelephonyManager.NETWORK_TYPE_LTE ||     // api<11: replace by 13
+                type == TelephonyManager.NETWORK_TYPE_IWLAN ||  // api<25: replace by 18
+                type == NETWORK_TYPE_LTE_CA || // LTE_CA
+                name.equals(LTE) ||
+                name.equals(UMB) ||
+                name.equals(HSPA_PLUS) ||
+                name.equals(FOUR_G);
+    }
+
+    private boolean is5G(int type, String name){
+
+        return  type == TelephonyManager.NETWORK_TYPE_LTE ||     // api<11: replace by 13
+                type == NETWORK_TYPE_NR ||  // api<25: replace by 18
+                name.equals(FIVE_G) ||
+                name.equals(NR);

Review comment:
       Can you explain why `TelephonyManager.NETWORK_TYPE_LTE` is tested in both 5G and 4G?
   
   It seems like this could incorrectly state that a connection is 4g or 5g. I would think if `NETWORK_TYPE_LTE` is used in both 4g and 5g connections, then we need an `&&` condition to against the `name` to determine whether the connection is 4g or 5g.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-network-information] dev-jcb commented on pull request #130: 5g feature

Posted by GitBox <gi...@apache.org>.
dev-jcb commented on pull request #130:
URL: https://github.com/apache/cordova-plugin-network-information/pull/130#issuecomment-875246066


   How do we get pull requests to get merged in master branch? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org