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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
