Fix tethering stability issues

mobile/src/main/java/be/mygod/vpnhotspot/App.kt

@@ -2,6 +2,7 @@ package be.mygod.vpnhotspot
 
 import android.annotation.TargetApi
 import android.app.Application
+import android.app.Notification
 import android.app.NotificationChannel
 import android.app.NotificationManager
 import android.content.SharedPreferences
@@ -33,13 +34,18 @@ class App : Application() {
     private fun updateNotificationChannels() {
         if (Build.VERSION.SDK_INT >= 26) @TargetApi(26) {
             val nm = getSystemService(NotificationManager::class.java)
-            nm.createNotificationChannel(NotificationChannel(RepeaterService.CHANNEL,
+            val tethering = NotificationChannel(TetheringService.CHANNEL,
-                    getText(R.string.notification_channel_repeater), NotificationManager.IMPORTANCE_LOW))
+                    getText(R.string.notification_channel_tethering), NotificationManager.IMPORTANCE_LOW)
+            tethering.lockscreenVisibility = Notification.VISIBILITY_PUBLIC
+            nm.createNotificationChannels(listOf(
+                    NotificationChannel(RepeaterService.CHANNEL, getText(R.string.notification_channel_repeater),
+                            NotificationManager.IMPORTANCE_LOW), tethering
+            ))
             nm.deleteNotificationChannel("hotspot") // remove old service channel
         }
     }
 
-    private val handler = Handler()
+    val handler = Handler()
     val pref: SharedPreferences by lazy { PreferenceManager.getDefaultSharedPreferences(this) }
     val dns: String get() = app.pref.getString("service.dns", "8.8.8.8:53")
 

mobile/src/main/java/be/mygod/vpnhotspot/RepeaterFragment.kt

@@ -123,7 +123,7 @@ class RepeaterFragment : Fragment(), ServiceConnection, Toolbar.OnMenuItemClickL
     private lateinit var binding: FragmentRepeaterBinding
     private val data = Data()
     private val adapter = ClientAdapter()
-    private var binder: RepeaterService.HotspotBinder? = null
+    private var binder: RepeaterService.RepeaterBinder? = null
     private var p2pInterface: String? = null
     private var tetheredInterfaces = emptySet<String>()
     private val receiver = broadcastReceiver { _, intent ->
@@ -167,7 +167,7 @@ class RepeaterFragment : Fragment(), ServiceConnection, Toolbar.OnMenuItemClickL
     }
 
     override fun onServiceConnected(name: ComponentName?, service: IBinder?) {
-        val binder = service as RepeaterService.HotspotBinder
+        val binder = service as RepeaterService.RepeaterBinder
         binder.data = data
         this.binder = binder
         data.onStatusChanged()

mobile/src/main/java/be/mygod/vpnhotspot/RepeaterService.kt

@@ -27,6 +27,7 @@ import java.util.regex.Pattern
 class RepeaterService : Service(), WifiP2pManager.ChannelListener, VpnMonitor.Callback {
     companion object {
         const val CHANNEL = "repeater"
+        const val CHANNEL_ID = 1
         const val ACTION_STATUS_CHANGED = "be.mygod.vpnhotspot.RepeaterService.STATUS_CHANGED"
         const val KEY_NET_ID = "netId"
         private const val TAG = "RepeaterService"
@@ -81,7 +82,7 @@ class RepeaterService : Service(), WifiP2pManager.ChannelListener, VpnMonitor.Ca
         IDLE, STARTING, ACTIVE
     }
 
-    inner class HotspotBinder : Binder() {
+    inner class RepeaterBinder : Binder() {
         val service get() = this@RepeaterService
         var data: RepeaterFragment.Data? = null
         val active get() = status == Status.ACTIVE
@@ -130,7 +131,7 @@ class RepeaterService : Service(), WifiP2pManager.ChannelListener, VpnMonitor.Ca
             field = value
             if (value != null) app.pref.edit().putInt(KEY_NET_ID, value.netId).apply()
         }
-    private val binder = HotspotBinder()
+    private val binder = RepeaterBinder()
     private var receiverRegistered = false
     private val receiver = broadcastReceiver { _, intent ->
         when (intent.action) {
@@ -160,8 +161,7 @@ class RepeaterService : Service(), WifiP2pManager.ChannelListener, VpnMonitor.Ca
     val password get() = if (status == Status.ACTIVE) group?.passphrase else null
 
     private var upstream: String? = null
-    var routing: Routing? = null
+    private var routing: Routing? = null
-        private set
 
     var status = Status.IDLE
         private set(value) {
@@ -312,9 +312,10 @@ class RepeaterService : Service(), WifiP2pManager.ChannelListener, VpnMonitor.Ca
                 .setSmallIcon(R.drawable.ic_device_wifi_tethering)
                 .setContentIntent(PendingIntent.getActivity(this, 0,
                         Intent(this, MainActivity::class.java), PendingIntent.FLAG_UPDATE_CURRENT))
-        if (group != null) builder.setContentText(resources.getQuantityString(R.plurals.notification_connected_devices,
+        val size = group?.clientList?.size ?: 0
-                group.clientList.size, group.clientList.size))
+        if (size != 0) builder.setContentText(resources.getQuantityString(R.plurals.notification_connected_devices,
-        startForeground(1, builder.build())
+                size, size, group!!.`interface`))
+        startForeground(CHANNEL_ID, builder.build())
     }
 
     private fun removeGroup() {

mobile/src/main/java/be/mygod/vpnhotspot/TetheringFragment.kt

@@ -2,12 +2,16 @@ package be.mygod.vpnhotspot
 
 import android.animation.Animator
 import android.animation.AnimatorListenerAdapter
+import android.content.ComponentName
+import android.content.Context
 import android.content.Intent
+import android.content.ServiceConnection
 import android.databinding.BaseObservable
 import android.databinding.DataBindingUtil
 import android.os.Bundle
+import android.os.IBinder
 import android.support.v4.app.Fragment
-import android.support.v4.content.LocalBroadcastManager
+import android.support.v4.content.ContextCompat
 import android.support.v7.util.SortedList
 import android.support.v7.widget.DefaultItemAnimator
 import android.support.v7.widget.LinearLayoutManager
@@ -22,7 +26,7 @@ import be.mygod.vpnhotspot.databinding.ListitemInterfaceBinding
 import be.mygod.vpnhotspot.net.NetUtils
 import be.mygod.vpnhotspot.net.TetherType
 
-class TetheringFragment : Fragment(), Toolbar.OnMenuItemClickListener {
+class TetheringFragment : Fragment(), ServiceConnection, Toolbar.OnMenuItemClickListener {
     private abstract class BaseSorter<T> : SortedList.Callback<T>() {
         override fun onInserted(position: Int, count: Int) { }
         override fun areContentsTheSame(oldItem: T?, newItem: T?): Boolean = oldItem == newItem
@@ -39,9 +43,9 @@ class TetheringFragment : Fragment(), Toolbar.OnMenuItemClickListener {
     }
     private object StringSorter : DefaultSorter<String>()
 
-    class Data(val iface: String) : BaseObservable() {
+    inner class Data(val iface: String) : BaseObservable() {
         val icon: Int get() = TetherType.ofInterface(iface).icon
-        var active = TetheringService.active.contains(iface)
+        var active = binder?.active?.contains(iface) == true
     }
 
     class InterfaceViewHolder(val binding: ListitemInterfaceBinding) : RecyclerView.ViewHolder(binding.root),
@@ -53,8 +57,10 @@ class TetheringFragment : Fragment(), Toolbar.OnMenuItemClickListener {
         override fun onClick(view: View) {
             val context = itemView.context
             val data = binding.data!!
-            context.startService(Intent(context, TetheringService::class.java).putExtra(if (data.active)
+            if (data.active) context.startService(Intent(context, TetheringService::class.java)
-                TetheringService.EXTRA_REMOVE_INTERFACE else TetheringService.EXTRA_ADD_INTERFACE, data.iface))
+                    .putExtra(TetheringService.EXTRA_REMOVE_INTERFACE, data.iface))
+            else ContextCompat.startForegroundService(context, Intent(context, TetheringService::class.java)
+                    .putExtra(TetheringService.EXTRA_ADD_INTERFACE, data.iface))
             data.active = !data.active
         }
     }
@@ -80,12 +86,10 @@ class TetheringFragment : Fragment(), Toolbar.OnMenuItemClickListener {
     }
 
     private lateinit var binding: FragmentTetheringBinding
-    private val adapter = InterfaceAdapter()
+    private var binder: TetheringService.TetheringBinder? = null
+    val adapter = InterfaceAdapter()
     private val receiver = broadcastReceiver { _, intent ->
-        when (intent.action) {
+        adapter.update(NetUtils.getTetheredIfaces(intent.extras).toSet())
-            TetheringService.ACTION_ACTIVE_INTERFACES_CHANGED -> adapter.notifyDataSetChanged()
-            NetUtils.ACTION_TETHER_STATE_CHANGED -> adapter.update(NetUtils.getTetheredIfaces(intent.extras).toSet())
-        }
     }
 
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
@@ -104,17 +108,27 @@ class TetheringFragment : Fragment(), Toolbar.OnMenuItemClickListener {
         super.onStart()
         val context = context!!
         context.registerReceiver(receiver, intentFilter(NetUtils.ACTION_TETHER_STATE_CHANGED))
-        LocalBroadcastManager.getInstance(context)
+        context.bindService(Intent(context, TetheringService::class.java), this, Context.BIND_AUTO_CREATE)
-                .registerReceiver(receiver, intentFilter(TetheringService.ACTION_ACTIVE_INTERFACES_CHANGED))
     }
 
     override fun onStop() {
         val context = context!!
+        context.unbindService(this)
         context.unregisterReceiver(receiver)
-        LocalBroadcastManager.getInstance(context).unregisterReceiver(receiver)
         super.onStop()
     }
 
+    override fun onServiceConnected(name: ComponentName?, service: IBinder?) {
+        val binder = service as TetheringService.TetheringBinder
+        this.binder = binder
+        binder.fragment = this
+    }
+
+    override fun onServiceDisconnected(name: ComponentName?) {
+        binder?.fragment = null
+        binder = null
+    }
+
     override fun onMenuItemClick(item: MenuItem) = when (item.itemId) {
         R.id.systemTethering -> {
             startActivity(Intent().setClassName("com.android.settings",

mobile/src/main/java/be/mygod/vpnhotspot/TetheringService.kt

@@ -1,34 +1,33 @@
 package be.mygod.vpnhotspot
 
+import android.app.Notification
+import android.app.PendingIntent
 import android.app.Service
 import android.content.Intent
+import android.os.Binder
+import android.support.v4.app.NotificationCompat
+import android.support.v4.content.ContextCompat
 import android.support.v4.content.LocalBroadcastManager
 import android.widget.Toast
 import be.mygod.vpnhotspot.App.Companion.app
-import be.mygod.vpnhotspot.net.NetUtils
+import be.mygod.vpnhotspot.net.*
-import be.mygod.vpnhotspot.net.Routing
-import be.mygod.vpnhotspot.net.VpnMonitor
 
-class TetheringService : Service(), VpnMonitor.Callback {
+class TetheringService : Service(), VpnMonitor.Callback, IpNeighbourMonitor.Callback {
     companion object {
-        const val ACTION_ACTIVE_INTERFACES_CHANGED = "be.mygod.vpnhotspot.TetheringService.ACTIVE_INTERFACES_CHANGED"
+        const val CHANNEL = "tethering"
+        const val CHANNEL_ID = 2
         const val EXTRA_ADD_INTERFACE = "interface.add"
         const val EXTRA_REMOVE_INTERFACE = "interface.remove"
-        private const val KEY_ACTIVE = "persist.service.tether.active"
-
-        private var alive = false
-        var active: Set<String>
-            get() = if (alive) app.pref.getStringSet(KEY_ACTIVE, null) ?: emptySet() else {
-                app.pref.edit().remove(KEY_ACTIVE).apply()
-                emptySet()
-            }
-            private set(value) {
-                app.pref.edit().putStringSet(KEY_ACTIVE, value).apply()
-                LocalBroadcastManager.getInstance(app).sendBroadcast(Intent(ACTION_ACTIVE_INTERFACES_CHANGED))
-            }
     }
 
+    inner class TetheringBinder : Binder() {
+        val active get() = routings.keys
+        var fragment: TetheringFragment? = null
+    }
+
+    private val binder = TetheringBinder()
     private val routings = HashMap<String, Routing?>()
+    private var neighbours = emptyList<IpNeighbour>()
     private var upstream: String? = null
     private var receiverRegistered = false
     private val receiver = broadcastReceiver { _, intent ->
@@ -47,6 +46,7 @@ class TetheringService : Service(), VpnMonitor.Callback {
     private fun updateRoutings() {
         if (routings.isEmpty()) {
             unregisterReceiver()
+            stopForeground(true)
             stopSelf()
         } else {
             val upstream = upstream
@@ -65,29 +65,24 @@ class TetheringService : Service(), VpnMonitor.Callback {
                 registerReceiver(receiver, intentFilter(NetUtils.ACTION_TETHER_STATE_CHANGED))
                 LocalBroadcastManager.getInstance(this)
                         .registerReceiver(receiver, intentFilter(App.ACTION_CLEAN_ROUTINGS))
+                IpNeighbourMonitor.registerCallback(this)
                 VpnMonitor.registerCallback(this)
                 receiverRegistered = true
             }
         }
-        active = routings.keys
+        postIpNeighbourAvailable()
+        app.handler.post { binder.fragment?.adapter?.notifyDataSetChanged() }
     }
 
-    override fun onCreate() {
+    override fun onBind(intent: Intent?) = binder
-        super.onCreate()
-        alive = true
-    }
 
-    override fun onBind(intent: Intent?) = null
+    override fun onStartCommand(intent: Intent, flags: Int, startId: Int): Int {
-
+        val iface = intent.getStringExtra(EXTRA_ADD_INTERFACE)
-    override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
+        if (iface != null) routings[iface] = null
-        if (intent != null) {   // otw service is recreated after being killed
+        if (routings.remove(intent.getStringExtra(EXTRA_REMOVE_INTERFACE))?.stop() == false)
-            val iface = intent.getStringExtra(EXTRA_ADD_INTERFACE)
+            Toast.makeText(this, getText(R.string.noisy_su_failure), Toast.LENGTH_SHORT).show()
-            if (iface != null) routings.put(iface, null)
-            if (routings.remove(intent.getStringExtra(EXTRA_REMOVE_INTERFACE))?.stop() == false)
-                Toast.makeText(this, getText(R.string.noisy_su_failure), Toast.LENGTH_SHORT).show()
-        } else active.forEach { routings.put(it, null) }
         updateRoutings()
-        return START_STICKY
+        return START_NOT_STICKY
     }
 
     override fun onAvailable(ifname: String) {
@@ -107,9 +102,31 @@ class TetheringService : Service(), VpnMonitor.Callback {
         if (failed) Toast.makeText(this, getText(R.string.noisy_su_failure), Toast.LENGTH_SHORT).show()
     }
 
+    override fun onIpNeighbourAvailable(neighbours: Map<String, IpNeighbour>) {
+        this.neighbours = neighbours.values.toList()
+    }
+    override fun postIpNeighbourAvailable() {
+        val builder = NotificationCompat.Builder(this, CHANNEL)
+                .setWhen(0)
+                .setColor(ContextCompat.getColor(this, R.color.colorPrimary))
+                .setContentTitle("VPN tethering active")
+                .setSmallIcon(R.drawable.ic_device_wifi_tethering)
+                .setContentIntent(PendingIntent.getActivity(this, 0,
+                        Intent(this, MainActivity::class.java), PendingIntent.FLAG_UPDATE_CURRENT))
+                .setVisibility(Notification.VISIBILITY_PUBLIC)
+        val content = neighbours.groupBy { it.dev }
+                .filter { (dev, _) -> routings.contains(dev) }
+                .mapValues { (_, neighbours) -> neighbours.size }
+                .toList()
+                .joinToString(", ") { (dev, size) ->
+                    resources.getQuantityString(R.plurals.notification_connected_devices, size, size, dev)
+                }
+        if (content.isNotEmpty()) builder.setContentText(content)
+        startForeground(CHANNEL_ID, builder.build())
+    }
+
     override fun onDestroy() {
         unregisterReceiver()
-        alive = false
         super.onDestroy()
     }
 

mobile/src/main/java/be/mygod/vpnhotspot/net/IpNeighbourMonitor.kt

@@ -20,7 +20,10 @@ class IpNeighbourMonitor private constructor() {
                 monitor = IpNeighbourMonitor()
                 instance = monitor
                 monitor.flush()
-            } else synchronized(monitor.neighbours) { callback.onIpNeighbourAvailable(monitor.neighbours) }
+            } else {
+                synchronized(monitor.neighbours) { callback.onIpNeighbourAvailable(monitor.neighbours) }
+                callback.postIpNeighbourAvailable()
+            }
         }
         fun unregisterCallback(callback: Callback) {
             if (!callbacks.remove(callback) || callbacks.isNotEmpty()) return
@@ -43,7 +46,7 @@ class IpNeighbourMonitor private constructor() {
 
     interface Callback {
         fun onIpNeighbourAvailable(neighbours: Map<String, IpNeighbour>)
-        fun postIpNeighbourAvailable() { }
+        fun postIpNeighbourAvailable()
     }
 
     private val handler = Handler()

mobile/src/main/res/values/strings.xml

@@ -52,9 +52,10 @@
     <string name="settings_misc_donate_summary">I love money</string>
 
     <string name="notification_channel_repeater">Repeater Service</string>
+    <string name="notification_channel_tethering">VPN Tethering Service</string>
     <plurals name="notification_connected_devices">
-        <item quantity="one">1 connected device</item>
+        <item quantity="one">%d device connected to %s</item>
-        <item quantity="other">%d connected devices</item>
+        <item quantity="other">%d devices connected to %s</item>
     </plurals>
 
     <string name="exception_interface_not_found">Fatal: Downstream interface not found</string>