Merge pull request #214 from mysteriumnetwork/reconnect-improvement

Reconnect improvement

android/app/src/main/java/network/mysterium/AppContainer.kt

@@ -78,7 +78,8 @@ class AppContainer {
 
         networkMonitor = NetworkMonitor(
                 connectivity = ctx.getSystemService(ConnectivityManager::class.java),
-                wifi = ctx.getSystemService(WifiManager::class.java)
+                wifiManager = ctx.getSystemService(WifiManager::class.java),
+                networkState = sharedViewModel.networkState
         )
     }
 

android/app/src/main/java/network/mysterium/MainActivity.kt

@@ -36,10 +36,10 @@ import androidx.navigation.ui.setupWithNavController
 import com.google.android.material.navigation.NavigationView
 import io.intercom.android.sdk.Intercom
 import kotlinx.coroutines.*
-import kotlinx.coroutines.flow.*
-import mysterium.ConnectRequest
 import network.mysterium.net.NetworkState
-import network.mysterium.service.core.*
+import network.mysterium.service.core.DeferredNode
+import network.mysterium.service.core.MysteriumAndroidCoreService
+import network.mysterium.service.core.MysteriumCoreService
 import network.mysterium.ui.Screen
 import network.mysterium.ui.navigateTo
 import network.mysterium.vpn.R
@@ -61,7 +61,6 @@ class MainActivity : AppCompatActivity() {
         }
     }
 
-    @ExperimentalCoroutinesApi
     override fun onCreate(savedInstanceState: Bundle?) {
         setTheme(R.style.AppTheme)
         super.onCreate(savedInstanceState)
@@ -90,18 +89,14 @@ class MainActivity : AppCompatActivity() {
         ensureVpnServicePermission()
         bindMysteriumService()
 
-        appContainer.networkMonitor.startWatching()
-
         // Start network connectivity checker and handle connection change event to
         // start mobile node and initial data when network is available.
         CoroutineScope(Dispatchers.Main).launch {
-            val core = deferredMysteriumCoreService.await()
-            appContainer.networkMonitor.state.observe(this@MainActivity, Observer {
+            deferredMysteriumCoreService.await()
+            appContainer.sharedViewModel.networkState.observe(this@MainActivity, Observer {
                 CoroutineScope(Dispatchers.Main).launch { handleConnChange(it) }
             })
-            appContainer.networkMonitor.state.observe(this@MainActivity, Observer {
-                CoroutineScope(Dispatchers.Main).launch { reconnect(core, it) }
-            })
+            appContainer.networkMonitor.start()
         }
 
         // Navigate to main vpn screen and check if terms are accepted or app
@@ -121,32 +116,9 @@ class MainActivity : AppCompatActivity() {
         }
     }
 
-    @ExperimentalCoroutinesApi
-    private suspend fun reconnect(core: MysteriumCoreService, state: NetworkState) {
-        if (!state.connected) {
-            return
-        }
-        val proposal = core.getActiveProposal() ?: return
-
-        val req = ConnectRequest().apply {
-            identityAddress = appContainer.nodeRepository.getIdentity().address
-            providerID = proposal.providerID
-            serviceType = proposal.serviceType.type
-            forceReconnect = true
-        }
-        flow<Nothing> {
-            Log.i(TAG, "Reconnecting identity ${req.identityAddress} to provider ${req.providerID} with service ${req.serviceType}")
-            appContainer.nodeRepository.reconnect(req)
-        }
-                .retry(3) { t -> t is ConnectInvalidProposalException }
-                .catch { e ->
-                    Log.e(TAG, "Failed to reconnect", e)
-                }
-                .collect {}
-    }
-
     override fun onDestroy() {
         unbindMysteriumService()
+        appContainer.networkMonitor.stop()
         super.onDestroy()
     }
 

android/app/src/main/java/network/mysterium/net/NetworkMonitor.kt

@@ -1,7 +1,10 @@
 package network.mysterium.net
 
-import android.net.*
+import android.net.ConnectivityManager
+import android.net.Network
 import android.net.NetworkCapabilities.*
+import android.net.NetworkInfo.DetailedState.CONNECTED
+import android.net.NetworkInfo.DetailedState.OBTAINING_IPADDR
 import android.net.wifi.WifiInfo
 import android.net.wifi.WifiManager
 import android.util.Log
@@ -10,79 +13,110 @@ import kotlinx.coroutines.*
 
 class NetworkMonitor(
         private val connectivity: ConnectivityManager,
-        private val wifi: WifiManager
+        private val wifiManager: WifiManager,
+        private val networkState: MutableLiveData<NetworkState>
+
 ) : ConnectivityManager.NetworkCallback() {
 
     companion object {
         private const val TAG = "NetworkMonitor"
     }
 
-    val state = MutableLiveData(NetworkState())
-
-    fun startWatching() {
-        connectivity.registerNetworkCallback(NetworkRequest.Builder()
-                .addTransportType(TRANSPORT_WIFI)
-                .addTransportType(TRANSPORT_CELLULAR)
-                .build(),
-                this
-        )
-    }
+    private var networkCallback: ConnectivityManager.NetworkCallback? = null
+    private var activeNetwork: Network? = null
+    private var delayedNetworkJoin: Job? = null
+    private var delayedNetworkLoss: Job? = null
+    private val networkStateChangeDelay = 7_000L
 
-    override fun onCapabilitiesChanged(network: Network?, networkCapabilities: NetworkCapabilities?) {
-        Log.d(TAG, "Network capabilities changed, refreshing state $networkCapabilities")
-        refreshNetworkState()
+    fun start() {
+        networkCallback = NetworkStatePublisher()
+        connectivity.activeNetwork?.let {
+            val capabilities = connectivity.getNetworkCapabilities(it)
+            val initialState = when {
+                capabilities.hasTransport(TRANSPORT_WIFI) -> NetworkState(wifiConnected = true, wifiNetworkId = getWifiNetworkId())
+                capabilities.hasTransport(TRANSPORT_CELLULAR) -> NetworkState(cellularConnected = true)
+                else -> NetworkState()
+            }
+            CoroutineScope(Dispatchers.Main).launch {
+                networkState.value = initialState
+            }
+        }
+        connectivity.registerDefaultNetworkCallback(networkCallback)
     }
 
-    override fun onLost(network: Network?) {
-        Log.d(TAG, "Lost a network, refreshing state")
-        refreshNetworkState()
+    fun stop() {
+        networkCallback?.let { connectivity.unregisterNetworkCallback(it) }
+        networkCallback = null
     }
 
-    private fun refreshNetworkState() {
-        val connectedNetworks = connectivity.allNetworks.filter { connectivity.getNetworkInfo(it)?.isConnected == true }
-        val wifiNetwork = connectedNetworks.find {
-            val cap = connectivity.getNetworkCapabilities(it) ?: return@find false
-            return@find cap.hasTransport(TRANSPORT_WIFI)
-                    && cap.hasCapability(NET_CAPABILITY_INTERNET)
-                    && cap.hasCapability(NET_CAPABILITY_VALIDATED)
-        }
-        val cellularNetwork = connectedNetworks.find {
-            val cap = connectivity.getNetworkCapabilities(it) ?: return@find false
-            return@find cap.hasTransport(TRANSPORT_CELLULAR)
-                    && cap.hasCapability(NET_CAPABILITY_INTERNET)
-                    && cap.hasCapability(NET_CAPABILITY_VALIDATED)
-        }
-        val newState = when {
-            wifiNetwork != null -> {
-                NetworkState(wifiConnected = true, wifiNetworkId = getWifiNetworkId(this.wifi))
+    inner class NetworkStatePublisher : ConnectivityManager.NetworkCallback() {
+        override fun onAvailable(network: Network?) {
+            if (network == null) {
+                return
             }
-            cellularNetwork != null -> {
-                NetworkState(cellularConnected = true)
+            val networkInfo = connectivity.getNetworkInfo(network) ?: return
+            val capabilities = connectivity.getNetworkCapabilities(network) ?: return
+
+            val newState = when {
+                capabilities.hasTransport(TRANSPORT_VPN) -> return
+                capabilities.hasTransport(TRANSPORT_WIFI) -> NetworkState(wifiConnected = true, wifiNetworkId = getWifiNetworkId())
+                capabilities.hasTransport(TRANSPORT_CELLULAR) -> NetworkState(cellularConnected = true)
+                else -> NetworkState()
             }
-            else -> {
-                NetworkState()
+
+            if (activeNetwork != network) {
+                delayedNetworkLoss?.let {
+                    Log.i(TAG, "Network connectivity restored, canceling network loss")
+                    it.cancel()
+                    delayedNetworkLoss = null
+                }
+                delayedNetworkJoin?.let {
+                    Log.i(TAG, "Connected to another network, canceling network join")
+                    it.cancel()
+                    delayedNetworkJoin = null
+                }
+                delayedNetworkJoin = CoroutineScope(Dispatchers.Main).launch {
+                    delay(if (newState.wifiConnected) 0 else networkStateChangeDelay)
+                    Log.i(TAG, "Network joined: ${transportType(network)} ${networkInfo.detailedState}")
+                    networkState.value = newState
+                    activeNetwork = network
+                    delayedNetworkJoin = null
+                }
             }
         }
 
-        if (newState != state.value) {
-            CoroutineScope(Dispatchers.Main).launch {
-                Log.i(TAG, "Network state changed: ${state.value} -> $newState")
-                state.postValue(newState)
+        override fun onLost(network: Network?) {
+            if (connectivity.activeNetworkInfo?.isConnected != true) {
+                delayedNetworkLoss = CoroutineScope(Dispatchers.Main).launch {
+                    delay(networkStateChangeDelay)
+                    Log.i(TAG, "Network lost: ${transportType(network)}")
+                    networkState.value = NetworkState()
+                    delayedNetworkLoss = null
+                }
             }
         }
     }
 
-}
+    private fun transportType(network: Network?): String {
+        val cap = network?.let { connectivity.getNetworkCapabilities(it) } ?: return "UNKNOWN"
+        return when {
+            cap.hasTransport(TRANSPORT_WIFI) -> "WIFI"
+            cap.hasTransport(TRANSPORT_CELLULAR) -> "CELLULAR"
+            else -> "UNKNOWN"
+        }
+    }
 
-fun getWifiNetworkId(manager: WifiManager): Int {
-    if (manager.isWifiEnabled) {
-        val wifiInfo = manager.connectionInfo
-        if (wifiInfo != null) {
-            val state = WifiInfo.getDetailedStateOf(wifiInfo.supplicantState)
-            if (state == NetworkInfo.DetailedState.CONNECTED || state == NetworkInfo.DetailedState.OBTAINING_IPADDR) {
-                return wifiInfo.networkId
+    private fun getWifiNetworkId(): Int {
+        if (wifiManager.isWifiEnabled) {
+            val wifiInfo = wifiManager.connectionInfo
+            if (wifiInfo != null) {
+                val state = WifiInfo.getDetailedStateOf(wifiInfo.supplicantState)
+                if (state in listOf(CONNECTED, OBTAINING_IPADDR)) {
+                    return wifiInfo.networkId
+                }
             }
         }
+        return 0
     }
-    return 0
-}
+
+}

android/app/src/main/java/network/mysterium/ui/MainVpnFragment.kt

@@ -192,6 +192,10 @@ class MainVpnFragment : Fragment() {
     }
 
     private fun updateConnStateLabel(state: ConnectionState) {
+        if (state != ConnectionState.CONNECTED && sharedViewModel.reconnecting) {
+            connStatusLabel.text = getString(R.string.conn_state_reconnecting)
+            return
+        }
         val connStateText = when (state) {
             ConnectionState.NOT_CONNECTED, ConnectionState.UNKNOWN -> getString(R.string.conn_state_not_connected)
             ConnectionState.CONNECTED, ConnectionState.IP_NOT_CHANGED -> getString(R.string.conn_state_connected)

android/app/src/main/java/network/mysterium/ui/SharedViewModel.kt

@@ -21,13 +21,13 @@ import android.content.Context
 import android.graphics.Bitmap
 import android.util.Log
 import androidx.lifecycle.MutableLiveData
+import androidx.lifecycle.Observer
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
-import kotlinx.coroutines.CompletableDeferred
-import kotlinx.coroutines.delay
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.*
 import mysterium.ConnectRequest
 import network.mysterium.AppNotificationManager
+import network.mysterium.net.NetworkState
 import network.mysterium.service.core.*
 import network.mysterium.vpn.R
 
@@ -75,15 +75,21 @@ class SharedViewModel(
         private val mysteriumCoreService: CompletableDeferred<MysteriumCoreService>,
         private val notificationManager: AppNotificationManager,
         private val walletViewModel: WalletViewModel
-) : ViewModel() {
+) : ViewModel(), Observer<NetworkState> {
 
     val selectedProposal = MutableLiveData<ProposalViewItem>()
     val connectionState = MutableLiveData<ConnectionState>()
+    var reconnecting = false
     val statistics = MutableLiveData<StatisticsModel>()
     val location = MutableLiveData<LocationModel>()
+    val networkState = MutableLiveData<NetworkState>(NetworkState(wifiConnected = true))
 
     private var isConnected = false
 
+    init {
+        networkState.observeForever(this)
+    }
+
     suspend fun load() {
         initListeners()
         loadActiveProposal()
@@ -105,6 +111,29 @@ class SharedViewModel(
         return state != null && (state == ConnectionState.CONNECTED || state == ConnectionState.IP_NOT_CHANGED)
     }
 
+    override fun onChanged(t: NetworkState) {
+        CoroutineScope(Dispatchers.Main).launch {
+            loadLocation()
+        }
+        if (!isConnected) {
+            Log.d(TAG, "No active connection, ignoring network state change")
+            return
+        }
+        if (t.connected) {
+            CoroutineScope(Dispatchers.Main).launch {
+                Log.i(TAG, "Network changed, reconnecting")
+                reconnect()
+            }
+        } else {
+            Log.i(TAG, "Network lost, disconnecting")
+            CoroutineScope(Dispatchers.Main).launch {
+                if (isConnected) {
+                    disconnect()
+                }
+            }
+        }
+    }
+
     suspend fun connect(identityAddress: String, providerID: String, serviceType: String) {
         try {
             connectionState.value = ConnectionState.CONNECTING
@@ -134,6 +163,41 @@ class SharedViewModel(
         }
     }
 
+    private suspend fun reconnect() {
+        if (!isConnected) {
+            return
+        }
+
+        val proposal = selectedProposal.value ?: return
+        val req = ConnectRequest().apply {
+            identityAddress = nodeRepository.getIdentity().address
+            providerID = proposal.providerID
+            serviceType = proposal.serviceType.type
+            forceReconnect = true
+        }
+
+        this.reconnecting = true
+        val tries = 3
+        for (i in 1..tries) {
+            try {
+                Log.i(TAG, "Reconnecting identity ${req.identityAddress} to provider ${req.providerID} with service ${req.serviceType}")
+                nodeRepository.reconnect(req)
+                Log.i(TAG, "Reconnected successfully")
+                break
+            } catch (e: Exception) {
+                if (!(e is ConnectInvalidProposalException)) {
+                    Log.e(TAG, "Failed to reconnect", e)
+                    break
+                }
+                Log.e(TAG, "Failed to reconnect (${i}/${tries})", e)
+                if (i < tries) {
+                    delay(500)
+                }
+            }
+        }
+        this.reconnecting = false
+    }
+
     suspend fun disconnect() {
         try {
             connectionState.value = ConnectionState.DISCONNECTING