frontend/android: refactor network related code

Our MainActivity code is quite complicated and mixes a lot of different
things. This change moves the code related to network management (e.g. to
display alerts when the network is not available) to a dedicated
helper class and declutters the main activity a bit.

It also fixes a bug that caused the app to misdetect mobile connection
or missing connectivity when switching between WIFI and cellular
connection. The bug was caused by the fact that the "onLost" call is not
guaranteed to be executed after the network is actually lost. This can
bring both to false positive and false negatives results, depending on
race conditions. This caused to occasionally not display the "connection
lost" banner when needed, or to e.g. display both the "connection lost" and
the "mobile connection" banner at the same time.

Adding a 250ms delay in the execution of the "onLost" handler is not
nice, but seems to reliably fix the issue on all the tested Android
versions.

This also drops the `netWorkStateReceiver`, as it was registered as a
broadcast receiver for the `CONNECTIVITY_ACTION`, which is now
deprecated.
See https://developer.android.com/reference/android/net/ConnectivityManager#CONNECTIVITY_ACTION

The `usingMobileDataChanged` call, which is used to force the mobile
server to fetch again the network source (wifi/mobile) and optionally
display the "mobile connection" banner, is moved inside the
`checkNetworkConnectivity` call, so that it's executed every time we are
reassessing the connectivity status.

Author: Beerosagos

CHANGELOG.md

@@ -3,6 +3,8 @@
 ## Unreleased
 - Add feedback link to guide and about settings
 - Move active currencies to top of currency dropdown
+- Android: fix connectivity misdetection when switching between WIFI and cellular network.
+- Android: dropped support for Android versions lower than 7.
 
 ## v4.49.0
 - Bundle BitBox02 Nova firmware version v9.24.0

frontends/android/BitBoxApp/app/src/main/java/ch/shiftcrypto/bitboxapp/GoViewModel.java

@@ -9,7 +9,6 @@
 import android.hardware.usb.UsbInterface;
 import android.hardware.usb.UsbManager;
 import android.net.ConnectivityManager;
-import android.net.NetworkCapabilities;
 import android.os.Handler;
 
 import androidx.lifecycle.AndroidViewModel;
@@ -170,14 +169,7 @@ public void bluetoothConnect(String identifier) {
 
         @Override
         public boolean usingMobileData() {
-            // Adapted from https://stackoverflow.com/a/53243938
-            ConnectivityManager cm = (ConnectivityManager) getApplication().getApplicationContext().getSystemService(Context.CONNECTIVITY_SERVICE);
-            if (cm == null) {
-                return false;
-            }
-            NetworkCapabilities capabilities = cm.getNetworkCapabilities(cm.getActiveNetwork());
-            return capabilities != null && capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR);
-
+            return networkHelper != null && networkHelper.usingMobileData();
         }
 
         @Override
@@ -207,11 +199,18 @@ public boolean detectDarkTheme() {
     private final MutableLiveData<Boolean> authSetting = new MutableLiveData<>(false);
     private final GoEnvironment goEnvironment;
     private final GoAPI goAPI;
+    private NetworkHelper networkHelper;
+
+    public NetworkHelper getNetworkHelper() {
+        return networkHelper;
+    }
 
     public GoViewModel(Application app) {
         super(app);
         this.goEnvironment = new GoEnvironment();
         this.goAPI = new GoAPI();
+        this.networkHelper = new NetworkHelper((ConnectivityManager) app.getSystemService(Context.CONNECTIVITY_SERVICE));
+
     }
 
     public MutableLiveData<Boolean> getIsDarkTheme() {

frontends/android/BitBoxApp/app/src/main/java/ch/shiftcrypto/bitboxapp/MainActivity.java

@@ -14,10 +14,6 @@
 import android.content.res.Configuration;
 import android.hardware.usb.UsbDevice;
 import android.hardware.usb.UsbManager;
-import android.net.ConnectivityManager;
-import android.net.Network;
-import android.net.NetworkCapabilities;
-import android.net.NetworkRequest;
 import android.net.Uri;
 import android.os.Bundle;
 import android.os.Handler;
@@ -62,47 +58,6 @@ public class MainActivity extends AppCompatActivity {
 
     private BitBoxWebChromeClient webChrome;
 
-    private ConnectivityManager connectivityManager;
-    private ConnectivityManager.NetworkCallback networkCallback;
-
-    private boolean hasInternetConnectivity(NetworkCapabilities capabilities) {
-        // To avoid false positives, if we can't obtain connectivity info,
-        // we return true.
-        // Note: this should never happen per Android documentation, as:
-        // - these can not be null it come from the onCapabilitiesChanged callback.
-        // - when obtained with getNetworkCapabilities(network), they can only be null if the
-        //   network is null or unknown, but we guard against both in the caller.
-        if (capabilities == null) {
-            Util.log("Got null capabilities when we shouldn't have. Assuming we are online.");
-            return true;
-        }
-
-
-        boolean hasInternet = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
-
-        // We need to check for both internet and validated, since validated reports that the system
-        // found connectivity the last time it checked. But if this callback triggers when going offline
-        // (e.g. airplane mode), this bit would still be true when we execute this method.
-        boolean isValidated = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED);
-        return hasInternet && isValidated;
-
-        // Fallback for older devices
-    }
-
-    private void checkConnectivity() {
-        Network activeNetwork = connectivityManager.getActiveNetwork();
-
-        // If there is no active network (e.g. airplane mode), there is no check to perform.
-        if (activeNetwork == null) {
-            Mobileserver.setOnline(false);
-            return;
-        }
-
-        NetworkCapabilities capabilities = connectivityManager.getNetworkCapabilities(activeNetwork);
-
-        Mobileserver.setOnline(hasInternetConnectivity(capabilities));
-    }
-
     // Connection to bind with GoService
     private final ServiceConnection connection = new ServiceConnection() {
 
@@ -129,14 +84,6 @@ public void onReceive(Context context, Intent intent) {
         }
     };
 
-    private final BroadcastReceiver networkStateReceiver = new BroadcastReceiver() {
-        @Override
-        public void onReceive(Context context, Intent intent) {
-            Mobileserver.usingMobileDataChanged();
-        }
-    };
-
-
     @Override
     public void onConfigurationChanged(Configuration newConfig) {
         int currentNightMode = newConfig.uiMode & Configuration.UI_MODE_NIGHT_MASK;
@@ -248,21 +195,6 @@ protected void onCreate(Bundle savedInstanceState) {
         // In that case, handleIntent() is not called with ACTION_USB_DEVICE_ATTACHED.
         this.updateDevice();
 
-        connectivityManager = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE);
-        networkCallback = new ConnectivityManager.NetworkCallback() {
-            @Override
-            public void onCapabilitiesChanged(@NonNull android.net.Network network, @NonNull android.net.NetworkCapabilities capabilities) {
-                super.onCapabilitiesChanged(network, capabilities);
-                Mobileserver.setOnline(hasInternetConnectivity(capabilities));
-            }
-            // When we lose the network, onCapabilitiesChanged does not trigger, so we need to override onLost.
-            @Override
-            public void onLost(@NonNull Network network) {
-                super.onLost(network);
-                Mobileserver.setOnline(false);
-            }
-        };
-
         getOnBackPressedDispatcher().addCallback(this, new OnBackPressedCallback(true) {
             @Override
             public void handleOnBackPressed() {
@@ -336,8 +268,8 @@ private void startServer() {
         final GoViewModel gVM = ViewModelProviders.of(this).get(GoViewModel.class);
         goService.startServer(getApplicationContext().getFilesDir().getAbsolutePath(), gVM.getGoEnvironment(), gVM.getGoAPI());
 
-        // Trigger connectivity check (as the network may already be unavailable when the app starts).
-        checkConnectivity();
+        // Trigger connectivity and mobile connection check (as the network may already be unavailable when the app starts).
+        gVM.getNetworkHelper().checkConnectivity();
     }
 
     @Override
@@ -355,15 +287,7 @@ protected void onStart() {
         Util.log("lifecycle: onStart");
         final GoViewModel goViewModel = ViewModelProviders.of(this).get(GoViewModel.class);
         goViewModel.getIsDarkTheme().observe(this, this::setDarkTheme);
-
-
-        NetworkRequest request = new NetworkRequest.Builder()
-            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
-            .addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
-            .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
-            .build();
-        // Register the network callback to listen for changes in network capabilities.
-        connectivityManager.registerNetworkCallback(request, networkCallback);
+        goViewModel.getNetworkHelper().registerNetworkCallback();
     }
 
     @Override
@@ -389,11 +313,9 @@ protected void onResume() {
                 ContextCompat.RECEIVER_NOT_EXPORTED
         );
 
-        // Listen on changes in the network connection. We are interested in if the user is connected to a mobile data connection.
-        registerReceiver(this.networkStateReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
-
         // Trigger connectivity check (as the network may already be unavailable when the app starts).
-        checkConnectivity();
+        final GoViewModel goViewModel = ViewModelProviders.of(this).get(GoViewModel.class);
+        goViewModel.getNetworkHelper().checkConnectivity();
 
         Intent intent = getIntent();
         handleIntent(intent);
@@ -404,7 +326,6 @@ protected void onPause() {
         super.onPause();
         Util.log("lifecycle: onPause");
         unregisterReceiver(this.usbStateReceiver);
-        unregisterReceiver(this.networkStateReceiver);
     }
 
     private void handleIntent(Intent intent) {
@@ -472,9 +393,8 @@ private void updateDevice() {
     @Override
     protected void onStop() {
         super.onStop();
-        if (connectivityManager != null && networkCallback != null) {
-            connectivityManager.unregisterNetworkCallback(networkCallback);
-        }
+        final GoViewModel goViewModel = ViewModelProviders.of(this).get(GoViewModel.class);
+        goViewModel.getNetworkHelper().unregisterNetworkCallback();
         Util.log("lifecycle: onStop");
     }
 

frontends/android/BitBoxApp/app/src/main/java/ch/shiftcrypto/bitboxapp/NetworkHelper.java

@@ -0,0 +1,116 @@
+package ch.shiftcrypto.bitboxapp;
+
+import android.net.ConnectivityManager;
+import android.net.Network;
+import android.net.NetworkCapabilities;
+import android.os.Handler;
+import android.os.Looper;
+
+import androidx.annotation.NonNull;
+
+import mobileserver.Mobileserver;
+
+public class NetworkHelper {
+    private final ConnectivityManager connectivityManager;
+    private final ConnectivityManager.NetworkCallback networkCallback;
+
+    public NetworkHelper(ConnectivityManager connectivityManager) {
+        this.connectivityManager = connectivityManager;
+        networkCallback = new ConnectivityManager.NetworkCallback() {
+            @Override
+            public void onCapabilitiesChanged(@NonNull android.net.Network network, @NonNull android.net.NetworkCapabilities capabilities) {
+                Util.log("onCapabilitiesChanged");
+                super.onCapabilitiesChanged(network, capabilities);
+                checkNetworkConnectivity(network);
+            }
+
+            @Override
+            public void onLost(@NonNull Network network) {
+                Util.log("onLost");
+                super.onLost(network);
+                // Workaround: onLost could trigger while the network is still winding down.
+                // Checking immediately for connection and mobile usage could lead to wrong results.
+                // see https://developer.android.com/reference/android/net/ConnectivityManager.NetworkCallback#onLost(android.net.Network)
+                // Since onCapabilitiesChanged doesn't seem to be enough, this is the only solution I
+                // found to make this reliable. It's not beautiful, but seems to fix the issue.
+                new Handler(Looper.getMainLooper()).postDelayed(() -> {
+                    checkConnectivity();
+                    Mobileserver.usingMobileDataChanged();
+                }, 250);
+            }
+        };
+    }
+
+    public void registerNetworkCallback() {
+        if (connectivityManager == null) {
+            return;
+        }
+
+        // Register the network callback to listen for changes in network capabilities.
+        // It needs to be unregistered when the app is in background to avoid resource consumption.
+        // See https://developer.android.com/reference/android/net/ConnectivityManager#registerNetworkCallback(android.net.NetworkRequest,%20android.net.ConnectivityManager.NetworkCallback)
+        connectivityManager.registerDefaultNetworkCallback(networkCallback);
+    }
+
+    public void unregisterNetworkCallback() {
+        if (connectivityManager != null && networkCallback != null) {
+            connectivityManager.unregisterNetworkCallback(networkCallback);
+        }
+    }
+
+    // Fetches the active network and verifies if that provides internet access.
+    public void checkConnectivity() {
+        if (connectivityManager == null) {
+            Mobileserver.setOnline(true);
+            return;
+        }
+        Network activeNetwork = connectivityManager.getActiveNetwork();
+        checkNetworkConnectivity(activeNetwork);
+    }
+
+    private void checkNetworkConnectivity(Network network) {
+        // We force the server to fetch the mobile data status and possibly update the
+        // related banner.
+        Mobileserver.usingMobileDataChanged();
+
+        if (network == null) {
+            Util.log("checkConnectivity: network null");
+            Mobileserver.setOnline(false);
+            return;
+        }
+
+        NetworkCapabilities capabilities = connectivityManager.getNetworkCapabilities(network);
+        Mobileserver.setOnline(hasInternetConnectivity(capabilities));
+    }
+
+    private boolean hasInternetConnectivity(NetworkCapabilities capabilities) {
+        Util.log("hasInternetConnectivity");
+        if (capabilities == null) {
+            Util.log("hasInternetConnectivity: null capabilities");
+            return false;
+        }
+
+        // NET_CAPABILITY_INTERNET means that the network should be able to provide internet access,
+        // NET_CAPABILITY_VALIDATED means that the network connectivity was successfully detected.
+        // see https://developer.android.com/reference/android/net/NetworkCapabilities#NET_CAPABILITY_VALIDATED
+        // Checking only VALIDATED would be probably enough, but checking for both
+        // is probably better for reliability.
+        boolean hasInternet = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
+        boolean isValidated = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED);
+        Util.log("Has internet connectivity: " + (hasInternet && isValidated));
+        return hasInternet && isValidated;
+    }
+
+    // if usingMobileData returns true, a banner will be displayed in the app to warn about
+    // possible network data consumption.
+    public boolean usingMobileData() {
+        if (connectivityManager == null) {
+            return false;
+        }
+
+        NetworkCapabilities capabilities = connectivityManager.getNetworkCapabilities(connectivityManager.getActiveNetwork());
+        boolean mobileData = capabilities != null && capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR);
+        Util.log("Using mobile data: " + mobileData);
+        return mobileData;
+    }
+}