From 9dcfb59ee0d183681e3718cfbb7d0e98b9e9cab7 Mon Sep 17 00:00:00 2001
From: geeksville <kevinh@geeksville.com>
Date: Fri, 14 Feb 2020 04:41:20 -0800
Subject: [PATCH] clean up user setting

---
 TODO.md                                       |   6 +
 app/src/main/AndroidManifest.xml              |   3 -
 .../com/geeksville/mesh/IMeshService.aidl     |   1 +
 .../java/com/geeksville/mesh/MainActivity.kt  | 411 ++++++++----------
 .../geeksville/mesh/service/MeshService.kt    |  11 +-
 app/src/main/proto/mesh.proto                 |   2 +-
 6 files changed, 205 insertions(+), 229 deletions(-)

diff --git a/TODO.md b/TODO.md
index 819316594..810ea59c1 100644
--- a/TODO.md
+++ b/TODO.md
@@ -1,6 +1,11 @@
 # High priority
 MVP features required for first public alpha 
 
+* use google signin to get user name (later give other options)
+* always set a _unique_ owner id (if changed)
+* stop scan when we start the service
+* set the radio by using the service
+* startforegroundservice only if we have a valid radio
 * if no radio is selected, launch app on the radio select screen
 * when we select a new radio, restart the service
 * show bt scan progress centered and towards the bottom of the screen
@@ -47,6 +52,7 @@ Do this "Signal app compatible" release relatively soon after the alpha release
 # Medium priority
 Things for the betaish period.
 
+* let user pick/specify a name through ways other than google signin (for the privacy concerned, or devices without Play API)
 * make my android app show mesh state
 * show qr code for each channel https://medium.com/@aanandshekharroy/generate-barcode-in-android-app-using-zxing-64c076a5d83a
 * register app link for our URLs https://developer.android.com/studio/write/app-link-indexing.html
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 327d85170..1531466cf 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -12,9 +12,6 @@
 
     <uses-permission android:name="android.permission.WAKE_LOCK" /> <!-- for job intent service -->
 
-    <uses-permission android:name="android.permission.READ_CONTACTS " /> <!-- to get the owner's name so we can set it in the radio -->
-    <uses-permission android:name="android.permission.GET_ACCOUNTS" />
-
     <!--
     This permission is required to allow the application to send
     events and properties to Mixpanel.
diff --git a/app/src/main/aidl/com/geeksville/mesh/IMeshService.aidl b/app/src/main/aidl/com/geeksville/mesh/IMeshService.aidl
index d90384321..5912acb17 100644
--- a/app/src/main/aidl/com/geeksville/mesh/IMeshService.aidl
+++ b/app/src/main/aidl/com/geeksville/mesh/IMeshService.aidl
@@ -17,6 +17,7 @@ interface IMeshService {
     /**
     * Set the ID info for this node
 
+    If myId is null, then the existing unique node ID is preserved, only the human visible longName/shortName is changed
     */
     void setOwner(String myId, String longName, String shortName);
 
diff --git a/app/src/main/java/com/geeksville/mesh/MainActivity.kt b/app/src/main/java/com/geeksville/mesh/MainActivity.kt
index 864bcad87..78d678524 100644
--- a/app/src/main/java/com/geeksville/mesh/MainActivity.kt
+++ b/app/src/main/java/com/geeksville/mesh/MainActivity.kt
@@ -1,15 +1,12 @@
 package com.geeksville.mesh
 
 import android.Manifest
-import android.accounts.AccountManager
 import android.bluetooth.BluetoothAdapter
 import android.bluetooth.BluetoothManager
 import android.content.*
 import android.content.pm.PackageManager
 import android.os.Bundle
 import android.os.IBinder
-import android.provider.ContactsContract
-import android.provider.ContactsContract.CommonDataKinds.Phone
 import android.view.Menu
 import android.view.MenuItem
 import android.widget.Toast
@@ -53,9 +50,7 @@ class MainActivity : AppCompatActivity(), Logging,
             Manifest.permission.BLUETOOTH,
             Manifest.permission.BLUETOOTH_ADMIN,
             Manifest.permission.WAKE_LOCK,
-            Manifest.permission.WRITE_EXTERNAL_STORAGE,
-            Manifest.permission.READ_CONTACTS,
-            Manifest.permission.GET_ACCOUNTS
+            Manifest.permission.WRITE_EXTERNAL_STORAGE
         )
 
         val missingPerms = perms.filter {
@@ -99,237 +94,213 @@ class MainActivity : AppCompatActivity(), Logging,
 
     private fun setOwner() {
         try {
-            if (false) {
-                val SELF_PROJECTION =
-                    arrayOf(Phone._ID, Phone.DISPLAY_NAME, Phone.PHOTO_THUMBNAIL_URI)
 
-                val cursor = contentResolver.query(
-                    ContactsContract.Profile.CONTENT_URI,
-                    SELF_PROJECTION,
-                    null,
-                    null,
-                    null
+
+            // Note: we are careful to not set a new unique ID
+            meshService!!.setOwner(null, "Kevin Xter", "kx")
+        }
+
+        private fun sendTestPackets() {
+            exceptionReporter {
+                val m = meshService!!
+
+                // Do some test operations
+                val testPayload = "hello world".toByteArray()
+                m.sendData(
+                    "+16508675310",
+                    testPayload,
+                    MeshProtos.Data.Type.SIGNAL_OPAQUE_VALUE
+                )
+                m.sendData(
+                    "+16508675310",
+                    testPayload,
+                    MeshProtos.Data.Type.CLEAR_TEXT_VALUE
                 )
-
-                if (cursor == null || !cursor.moveToFirst())
-                    error("Can't get owner contact")
-                else {
-                    info("me: ${cursor.getString(1)}/${cursor.getString(2)}")
-                }
-            }
-            val am = AccountManager.get(this) // "this" references the current Context
-
-            val accounts = am.getAccountsByType("com.google")
-            accounts.forEach {
-                info("acct ${it.name} ${it.type}")
-
-            }
-        } catch (e: Throwable) {
-            error("getting owner failed: $e")
-        }
-
-        meshService!!.setOwner("+16508675309", "Kevin Xter", "kx")
-    }
-
-    private fun sendTestPackets() {
-        exceptionReporter {
-            val m = meshService!!
-
-            // Do some test operations
-            val testPayload = "hello world".toByteArray()
-            m.sendData(
-                "+16508675310",
-                testPayload,
-                MeshProtos.Data.Type.SIGNAL_OPAQUE_VALUE
-            )
-            m.sendData(
-                "+16508675310",
-                testPayload,
-                MeshProtos.Data.Type.CLEAR_TEXT_VALUE
-            )
-        }
-    }
-
-
-    override fun onCreate(savedInstanceState: Bundle?) {
-        super.onCreate(savedInstanceState)
-
-        setContent {
-            MeshApp()
-        }
-
-        // Ensures Bluetooth is available on the device and it is enabled. If not,
-        // displays a dialog requesting user permission to enable Bluetooth.
-        if (bluetoothAdapter != null) {
-            bluetoothAdapter!!.takeIf { !it.isEnabled }?.apply {
-                val enableBtIntent = Intent(BluetoothAdapter.ACTION_REQUEST_ENABLE)
-                startActivityForResult(enableBtIntent, REQUEST_ENABLE_BT)
-            }
-        } else {
-            Toast.makeText(this, "Error - this app requires bluetooth", Toast.LENGTH_LONG).show()
-        }
-        
-        requestPermission()
-    }
-
-    override fun onDestroy() {
-        unregisterMeshReceiver()
-        super.onDestroy()
-    }
-
-    private var receiverRegistered = false
-
-    private fun registerMeshReceiver() {
-        logAssert(!receiverRegistered)
-        val filter = IntentFilter()
-        filter.addAction(MeshService.ACTION_MESH_CONNECTED)
-        filter.addAction(MeshService.ACTION_NODE_CHANGE)
-        filter.addAction(MeshService.ACTION_RECEIVED_DATA)
-        registerReceiver(meshServiceReceiver, filter)
-
-    }
-
-    private fun unregisterMeshReceiver() {
-        if (receiverRegistered) {
-            receiverRegistered = false
-            unregisterReceiver(meshServiceReceiver)
-        }
-    }
-
-    /// Called when we gain/lose a connection to our mesh radio
-    private fun onMeshConnectionChanged(connected: Boolean) {
-        UIState.isConnected.value = connected
-        debug("connchange ${UIState.isConnected.value}")
-        if (connected) {
-            // everytime the radio reconnects, we slam in our current owner data
-            setOwner()
-        }
-    }
-
-    private val meshServiceReceiver = object : BroadcastReceiver() {
-
-        override fun onReceive(context: Context, intent: Intent) = exceptionReporter {
-            debug("Received from mesh service $intent")
-
-            when (intent.action) {
-                MeshService.ACTION_NODE_CHANGE -> {
-                    val info: NodeInfo = intent.getParcelableExtra(EXTRA_NODEINFO)!!
-                    debug("UI nodechange $info")
-
-                    // We only care about nodes that have user info
-                    info.user?.id?.let {
-                        val newnodes = UIState.nodes.value.toMutableMap()
-                        newnodes[it] = info
-                        UIState.nodes.value = newnodes
-                    }
-                }
-
-                MeshService.ACTION_RECEIVED_DATA -> {
-                    debug("TODO rxdata")
-                    val sender = intent.getStringExtra(EXTRA_SENDER)!!
-                    val payload = intent.getByteArrayExtra(EXTRA_PAYLOAD)!!
-                    val typ = intent.getIntExtra(EXTRA_TYP, -1)
-
-                    when (typ) {
-                        MeshProtos.Data.Type.CLEAR_TEXT_VALUE -> {
-                            // FIXME - use the real time from the packet
-                            val modded = UIState.messages.value.toMutableList()
-                            modded.add(TextMessage(Date(), sender, payload.toString(utf8)))
-                            UIState.messages.value = modded
-                        }
-                        else -> TODO()
-                    }
-                }
-                MeshService.ACTION_MESH_CONNECTED -> {
-                    val connected = intent.getBooleanExtra(EXTRA_CONNECTED, false)
-                    onMeshConnectionChanged(connected)
-                }
-                else -> TODO()
             }
         }
-    }
 
-    private var meshService: IMeshService? = null
-    private var isBound = false
 
-    private var serviceConnection = object : ServiceConnection {
-        override fun onServiceConnected(name: ComponentName, service: IBinder) = exceptionReporter {
-            val m = IMeshService.Stub.asInterface(service)
-            meshService = m
+        override fun onCreate(savedInstanceState: Bundle?) {
+            super.onCreate(savedInstanceState)
 
-            // We don't start listening for packets until after we are connected to the service
-            registerMeshReceiver()
+            setContent {
+                MeshApp()
+            }
 
-            // We won't receive a notify for the initial state of connection, so we force an update here
-            onMeshConnectionChanged(m.isConnected)
+            // Ensures Bluetooth is available on the device and it is enabled. If not,
+            // displays a dialog requesting user permission to enable Bluetooth.
+            if (bluetoothAdapter != null) {
+                bluetoothAdapter!!.takeIf { !it.isEnabled }?.apply {
+                    val enableBtIntent = Intent(BluetoothAdapter.ACTION_REQUEST_ENABLE)
+                    startActivityForResult(enableBtIntent, REQUEST_ENABLE_BT)
+                }
+            } else {
+                Toast.makeText(this, "Error - this app requires bluetooth", Toast.LENGTH_LONG)
+                    .show()
+            }
 
-            debug("connected to mesh service, isConnected=${UIState.isConnected.value}")
-
-            // make some placeholder nodeinfos
-            UIState.nodes.value =
-                m.nodes.toList().map {
-                    it.user?.id!! to it
-                }.toMap()
+            requestPermission()
         }
 
-        override fun onServiceDisconnected(name: ComponentName) {
-            warn("The mesh service has disconnected")
+        override fun onDestroy() {
             unregisterMeshReceiver()
+            super.onDestroy()
+        }
+
+        private var receiverRegistered = false
+
+        private fun registerMeshReceiver() {
+            logAssert(!receiverRegistered)
+            val filter = IntentFilter()
+            filter.addAction(MeshService.ACTION_MESH_CONNECTED)
+            filter.addAction(MeshService.ACTION_NODE_CHANGE)
+            filter.addAction(MeshService.ACTION_RECEIVED_DATA)
+            registerReceiver(meshServiceReceiver, filter)
+
+        }
+
+        private fun unregisterMeshReceiver() {
+            if (receiverRegistered) {
+                receiverRegistered = false
+                unregisterReceiver(meshServiceReceiver)
+            }
+        }
+
+        /// Called when we gain/lose a connection to our mesh radio
+        private fun onMeshConnectionChanged(connected: Boolean) {
+            UIState.isConnected.value = connected
+            debug("connchange ${UIState.isConnected.value}")
+            if (connected) {
+                // everytime the radio reconnects, we slam in our current owner data
+                setOwner()
+            }
+        }
+
+        private val meshServiceReceiver = object : BroadcastReceiver() {
+
+            override fun onReceive(context: Context, intent: Intent) = exceptionReporter {
+                debug("Received from mesh service $intent")
+
+                when (intent.action) {
+                    MeshService.ACTION_NODE_CHANGE -> {
+                        val info: NodeInfo = intent.getParcelableExtra(EXTRA_NODEINFO)!!
+                        debug("UI nodechange $info")
+
+                        // We only care about nodes that have user info
+                        info.user?.id?.let {
+                            val newnodes = UIState.nodes.value.toMutableMap()
+                            newnodes[it] = info
+                            UIState.nodes.value = newnodes
+                        }
+                    }
+
+                    MeshService.ACTION_RECEIVED_DATA -> {
+                        debug("TODO rxdata")
+                        val sender = intent.getStringExtra(EXTRA_SENDER)!!
+                        val payload = intent.getByteArrayExtra(EXTRA_PAYLOAD)!!
+                        val typ = intent.getIntExtra(EXTRA_TYP, -1)
+
+                        when (typ) {
+                            MeshProtos.Data.Type.CLEAR_TEXT_VALUE -> {
+                                // FIXME - use the real time from the packet
+                                val modded = UIState.messages.value.toMutableList()
+                                modded.add(TextMessage(Date(), sender, payload.toString(utf8)))
+                                UIState.messages.value = modded
+                            }
+                            else -> TODO()
+                        }
+                    }
+                    MeshService.ACTION_MESH_CONNECTED -> {
+                        val connected = intent.getBooleanExtra(EXTRA_CONNECTED, false)
+                        onMeshConnectionChanged(connected)
+                    }
+                    else -> TODO()
+                }
+            }
+        }
+
+        private var meshService: IMeshService? = null
+        private var isBound = false
+
+        private var serviceConnection = object : ServiceConnection {
+            override fun onServiceConnected(name: ComponentName, service: IBinder) =
+                exceptionReporter {
+                    val m = IMeshService.Stub.asInterface(service)
+                    meshService = m
+
+                    // We don't start listening for packets until after we are connected to the service
+                    registerMeshReceiver()
+
+                    // We won't receive a notify for the initial state of connection, so we force an update here
+                    onMeshConnectionChanged(m.isConnected)
+
+                    debug("connected to mesh service, isConnected=${UIState.isConnected.value}")
+
+                    // make some placeholder nodeinfos
+                    UIState.nodes.value =
+                        m.nodes.toList().map {
+                            it.user?.id!! to it
+                        }.toMap()
+                }
+
+            override fun onServiceDisconnected(name: ComponentName) {
+                warn("The mesh service has disconnected")
+                unregisterMeshReceiver()
+                meshService = null
+            }
+        }
+
+        private fun bindMeshService() {
+            debug("Binding to mesh service!")
+            // we bind using the well known name, to make sure 3rd party apps could also
+            logAssert(meshService == null)
+
+            val intent = MeshService.startService(this)
+            if (intent != null) {
+                // ALSO bind so we can use the api
+                logAssert(bindService(intent, serviceConnection, Context.BIND_AUTO_CREATE))
+                isBound = true;
+            }
+        }
+
+        private fun unbindMeshService() {
+            // If we have received the service, and hence registered with
+            // it, then now is the time to unregister.
+            // if we never connected, do nothing
+            debug("Unbinding from mesh service!")
+            if (isBound)
+                unbindService(serviceConnection)
             meshService = null
         }
-    }
 
-    private fun bindMeshService() {
-        debug("Binding to mesh service!")
-        // we bind using the well known name, to make sure 3rd party apps could also
-        logAssert(meshService == null)
+        override fun onPause() {
+            unregisterMeshReceiver() // No point in receiving updates while the GUI is gone, we'll get them when the user launches the activity
+            unbindMeshService()
 
-        val intent = MeshService.startService(this)
-        if (intent != null) {
-            // ALSO bind so we can use the api
-            logAssert(bindService(intent, serviceConnection, Context.BIND_AUTO_CREATE))
-            isBound = true;
+            super.onPause()
+        }
+
+        override fun onResume() {
+            super.onResume()
+
+            bindMeshService()
+        }
+
+        override fun onCreateOptionsMenu(menu: Menu): Boolean {
+            // Inflate the menu; this adds items to the action bar if it is present.
+            menuInflater.inflate(R.menu.menu_main, menu)
+            return true
+        }
+
+        override fun onOptionsItemSelected(item: MenuItem): Boolean {
+            // Handle action bar item clicks here. The action bar will
+            // automatically handle clicks on the Home/Up button, so long
+            // as you specify a parent activity in AndroidManifest.xml.
+            return when (item.itemId) {
+                R.id.action_settings -> true
+                else -> super.onOptionsItemSelected(item)
+            }
         }
     }
 
-    private fun unbindMeshService() {
-        // If we have received the service, and hence registered with
-        // it, then now is the time to unregister.
-        // if we never connected, do nothing
-        debug("Unbinding from mesh service!")
-        if (isBound)
-            unbindService(serviceConnection)
-        meshService = null
-    }
-
-    override fun onPause() {
-        unregisterMeshReceiver() // No point in receiving updates while the GUI is gone, we'll get them when the user launches the activity
-        unbindMeshService()
-
-        super.onPause()
-    }
-
-    override fun onResume() {
-        super.onResume()
-
-        bindMeshService()
-    }
-
-    override fun onCreateOptionsMenu(menu: Menu): Boolean {
-        // Inflate the menu; this adds items to the action bar if it is present.
-        menuInflater.inflate(R.menu.menu_main, menu)
-        return true
-    }
-
-    override fun onOptionsItemSelected(item: MenuItem): Boolean {
-        // Handle action bar item clicks here. The action bar will
-        // automatically handle clicks on the Home/Up button, so long
-        // as you specify a parent activity in AndroidManifest.xml.
-        return when (item.itemId) {
-            R.id.action_settings -> true
-            else -> super.onOptionsItemSelected(item)
-        }
-    }
-}
-
 
diff --git a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt
index 32bd6cd0b..f5263fc7a 100644
--- a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt
+++ b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt
@@ -314,8 +314,8 @@ class MeshService : Service(), Logging {
         updatefn(info)
 
         // This might have been the first time we know an ID for this node, so also update the by ID map
-        val userId = info.user?.id
-        if (userId != null)
+        val userId = info.user?.id.orEmpty()
+        if (userId.isNotEmpty())
             nodeDBbyID[userId] = info
 
         // parcelable is busted
@@ -535,12 +535,13 @@ class MeshService : Service(), Logging {
                 clientPackages[receiverName] = packageName
             }
 
-        override fun setOwner(myId: String, longName: String, shortName: String) =
+        override fun setOwner(myId: String?, longName: String, shortName: String) =
             toRemoteExceptions {
-                debug("TetOwner $myId : $longName : $shortName")
+                debug("SetOwner $myId : $longName : $shortName")
 
                 val user = MeshProtos.User.newBuilder().also {
-                    it.id = myId
+                    if (myId != null)  // Only set the id if it was provided
+                        it.id = myId
                     it.longName = longName
                     it.shortName = shortName
                 }.build()
diff --git a/app/src/main/proto/mesh.proto b/app/src/main/proto/mesh.proto
index 6aba9ffeb..b63e20065 100644
--- a/app/src/main/proto/mesh.proto
+++ b/app/src/main/proto/mesh.proto
@@ -105,7 +105,7 @@ A few nodenums are reserved and will never be requested:
 
 */
 message User {
-    string id = 1; // a globally unique ID string for this user.  In the case of Signal that would mean +16504442323
+    string id = 1; // a globally unique ID string for this user.  In the case of Signal that would mean +16504442323, for the default macaddr derived id it would be !<6 hexidecimal bytes>
     string long_name = 2; // A full name for this user, i.e. "Kevin Hester"
     string short_name = 3; // A VERY short name, ideally two characters.  Suitable for a tiny OLED screen
     bytes macaddr = 4; // This is the addr of the radio.  Not populated by the phone, but added by the esp32 when broadcasting