From 9687fb7370807512cdb7ec02bf667cc16192fa56 Mon Sep 17 00:00:00 2001 From: Mike Cumings Date: Fri, 22 Apr 2022 17:39:48 -0700 Subject: [PATCH 1/3] `RadioInterfaceService` is no longer an Android `Service` Removes the AIDL and associated support for the `IRadioInterfaceService`. This should give some performance benefit since radio data no longer has to round-trip through the platform. --- app/src/main/AndroidManifest.xml | 6 - .../mesh/IRadioInterfaceService.aidl | 19 --- .../repository/radio/BluetoothInterface.kt | 12 +- .../mesh/repository/radio/InterfaceFactory.kt | 2 +- .../mesh/repository/radio/MockInterface.kt | 1 + .../mesh/repository/radio/NopInterface.kt | 2 + .../repository/radio/RadioInterfaceService.kt | 147 +++++++--------- .../radio/RadioServiceConnectionState.kt | 6 + .../mesh/repository/radio/SerialInterface.kt | 1 + .../mesh/repository/radio/TCPInterface.kt | 2 + .../geeksville/mesh/service/MeshService.kt | 157 ++++++------------ 11 files changed, 134 insertions(+), 221 deletions(-) delete mode 100644 app/src/main/aidl/com/geeksville/mesh/IRadioInterfaceService.aidl create mode 100644 app/src/main/java/com/geeksville/mesh/repository/radio/RadioServiceConnectionState.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index f9b82f0b..272a2894 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -115,12 +115,6 @@ - - - () + val receivedData: SharedFlow = _receivedData + + init { + processLifecycle.coroutineScope.launch { + bluetoothRepository.state.collect { state -> + if (state.enabled) { + startInterface() + } else { + stopInterface() + } + } + } } - @Inject - lateinit var bluetoothRepository: BluetoothRepository - - @Inject - lateinit var usbRepository: UsbRepository - companion object : Logging { /** * The RECEIVED_FROMRADIO @@ -133,9 +143,6 @@ class RadioInterfaceService : Service(), Logging { } return address } - - /// If our service is currently running, this pointer can be used to reach it (in case setBondedDeviceAddress is called) - private var runningService: RadioInterfaceService? = null } private val logSends = false @@ -161,10 +168,12 @@ class RadioInterfaceService : Service(), Logging { private fun broadcastConnectionChanged(isConnected: Boolean, isPermanent: Boolean) { debug("Broadcasting connection=$isConnected") - val intent = Intent(RADIO_CONNECTED_ACTION) - intent.putExtra(EXTRA_CONNECTED, isConnected) - intent.putExtra(EXTRA_PERMANENT, isPermanent) - sendBroadcast(intent) + + processLifecycle.coroutineScope.launch(dispatchers.default) { + _connectionState.emit( + RadioServiceConnectionState(isConnected, isPermanent) + ) + } } /// Send a packet/command out the radio link, this routine can block if it needs to @@ -181,10 +190,9 @@ class RadioInterfaceService : Service(), Logging { // ignoreException { debug("FromRadio: ${MeshProtos.FromRadio.parseFrom(p)}") } - broadcastReceivedFromRadio( - this, - p - ) + processLifecycle.coroutineScope.launch(dispatchers.io) { + _receivedData.emit(p) + } } fun onConnect() { @@ -201,48 +209,12 @@ class RadioInterfaceService : Service(), Logging { } } - - override fun onCreate() { - runningService = this - lifecycleDispatcher.onServicePreSuperOnCreate() - super.onCreate() - - lifecycleOwner.lifecycle.coroutineScope.launch { - bluetoothRepository.state.collect { state -> - if (state.enabled) { - startInterface() - } else { - stopInterface() - } - } - } - } - - override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { - lifecycleDispatcher.onServicePreSuperOnStart() - return super.onStartCommand(intent, flags, startId) - } - - override fun onDestroy() { - stopInterface() - serviceScope.cancel("Destroying RadioInterface") - runningService = null - lifecycleDispatcher.onServicePreSuperOnDestroy() - super.onDestroy() - } - - override fun onBind(intent: Intent?): IBinder? { - lifecycleDispatcher.onServicePreSuperOnBind() - return binder - } - - /** Start our configured interface (if it isn't already running) */ private fun startInterface() { if (radioIf !is NopInterface) warn("Can't start interface - $radioIf is already running") else { - val address = getBondedDeviceAddress(this, usbRepository) + val address = getBondedDeviceAddress(context, usbRepository) if (address == null) warn("No bonded mesh radio, can't start interface") else { @@ -250,14 +222,14 @@ class RadioInterfaceService : Service(), Logging { isStarted = true if (logSends) - sentPacketsLog = BinaryLogFile(this, "sent_log.pb") + sentPacketsLog = BinaryLogFile(context, "sent_log.pb") if (logReceives) - receivedPacketsLog = BinaryLogFile(this, "receive_log.pb") + receivedPacketsLog = BinaryLogFile(context, "receive_log.pb") val c = address[0] val rest = address.substring(1) radioIf = - InterfaceFactory.getFactory(c)?.createInterface(this, usbRepository, rest) ?: NopInterface() + InterfaceFactory.getFactory(c)?.createInterface(context, this, usbRepository, rest) ?: NopInterface() } } } @@ -291,7 +263,7 @@ class RadioInterfaceService : Service(), Logging { */ @SuppressLint("NewApi") private fun setBondedDeviceAddress(address: String?): Boolean { - return if (getBondedDeviceAddress(this, usbRepository) == address && isStarted) { + return if (getBondedDeviceAddress(context, usbRepository) == address && isStarted) { warn("Ignoring setBondedDevice ${address.anonymize}, because we are already using that device") false } else { @@ -309,7 +281,7 @@ class RadioInterfaceService : Service(), Logging { debug("Setting bonded device to ${address.anonymize}") - getPrefs(this).edit(commit = true) { + getPrefs(context).edit(commit = true) { if (address == null) this.remove(DEVADDR_KEY) else @@ -322,23 +294,20 @@ class RadioInterfaceService : Service(), Logging { } } - private val binder = object : IRadioInterfaceService.Stub() { + fun setDeviceAddress(deviceAddr: String?): Boolean = toRemoteExceptions { + setBondedDeviceAddress(deviceAddr) + } - override fun setDeviceAddress(deviceAddr: String?): Boolean = toRemoteExceptions { - setBondedDeviceAddress(deviceAddr) - } + /** If the service is not currently connected to the radio, try to connect now. At boot the radio interface service will + * not connect to a radio until this call is received. */ + fun connect() = toRemoteExceptions { + // We don't start actually talking to our device until MeshService binds to us - this prevents + // broadcasting connection events before MeshService is ready to receive them + startInterface() + } - /** If the service is not currently connected to the radio, try to connect now. At boot the radio interface service will - * not connect to a radio until this call is received. */ - override fun connect() = toRemoteExceptions { - // We don't start actually talking to our device until MeshService binds to us - this prevents - // broadcasting connection events before MeshService is ready to receive them - startInterface() - } - - override fun sendToRadio(a: ByteArray) { - // Do this in the IO thread because it might take a while (and we don't care about the result code) - serviceScope.handledLaunch { handleSendToRadio(a) } - } + fun sendToRadio(a: ByteArray) { + // Do this in the IO thread because it might take a while (and we don't care about the result code) + serviceScope.handledLaunch { handleSendToRadio(a) } } } diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/RadioServiceConnectionState.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/RadioServiceConnectionState.kt new file mode 100644 index 00000000..1bf3f723 --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/RadioServiceConnectionState.kt @@ -0,0 +1,6 @@ +package com.geeksville.mesh.repository.radio + +data class RadioServiceConnectionState( + val isConnected: Boolean = false, + val isPermanent: Boolean = false +) diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/SerialInterface.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/SerialInterface.kt index 0c58a954..4e97f58c 100644 --- a/app/src/main/java/com/geeksville/mesh/repository/radio/SerialInterface.kt +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/SerialInterface.kt @@ -19,6 +19,7 @@ class SerialInterface( StreamInterface(service), Logging { companion object : Logging, InterfaceFactory('s') { override fun createInterface( + context: Context, service: RadioInterfaceService, usbRepository: UsbRepository, rest: String diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/TCPInterface.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/TCPInterface.kt index e9b1cb71..173b95e8 100644 --- a/app/src/main/java/com/geeksville/mesh/repository/radio/TCPInterface.kt +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/TCPInterface.kt @@ -1,5 +1,6 @@ package com.geeksville.mesh.repository.radio +import android.content.Context import com.geeksville.android.Logging import com.geeksville.mesh.repository.usb.UsbRepository import com.geeksville.util.Exceptions @@ -18,6 +19,7 @@ class TCPInterface(service: RadioInterfaceService, private val address: String) companion object : Logging, InterfaceFactory('t') { override fun createInterface( + context: Context, service: RadioInterfaceService, usbRepository: UsbRepository, // Temporary until dependency injection transition is completed rest: String 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 54de09d6..184aafbb 100644 --- a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt +++ b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt @@ -2,10 +2,8 @@ package com.geeksville.mesh.service import android.annotation.SuppressLint import android.app.Service -import android.content.BroadcastReceiver import android.content.Context import android.content.Intent -import android.content.IntentFilter import android.os.IBinder import android.os.Looper import android.os.RemoteException @@ -15,7 +13,6 @@ import androidx.core.content.edit import com.geeksville.analytics.DataPair import com.geeksville.android.GeeksvilleApplication import com.geeksville.android.Logging -import com.geeksville.android.ServiceClient import com.geeksville.android.isGooglePlayAvailable import com.geeksville.concurrent.handledLaunch import com.geeksville.mesh.* @@ -27,6 +24,7 @@ import com.geeksville.mesh.database.entity.Packet import com.geeksville.mesh.model.DeviceVersion import com.geeksville.mesh.repository.radio.BluetoothInterface import com.geeksville.mesh.repository.radio.RadioInterfaceService +import com.geeksville.mesh.repository.radio.RadioServiceConnectionState import com.geeksville.mesh.repository.usb.UsbRepository import com.geeksville.mesh.service.SoftwareUpdateService.Companion.ProgressNotStarted import com.geeksville.util.* @@ -41,6 +39,7 @@ import com.google.protobuf.InvalidProtocolBufferException import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.* +import kotlinx.coroutines.flow.onEach import kotlinx.serialization.json.Json import java.util.* import javax.inject.Inject @@ -56,9 +55,15 @@ import kotlin.math.max */ @AndroidEntryPoint class MeshService : Service(), Logging { + @Inject + lateinit var dispatchers: CoroutineDispatchers + @Inject lateinit var packetRepository: Lazy + @Inject + lateinit var radioInterfaceService: RadioInterfaceService + @Inject lateinit var usbRepository: Lazy @@ -135,13 +140,6 @@ class MeshService : Service(), Logging { // If we've ever read a valid region code from our device it will be here var curRegionValue = RadioConfigProtos.RegionCode.Unset_VALUE - val radio = ServiceClient { - IRadioInterfaceService.Stub.asInterface(it).apply { - // Now that we are connected to the radio service, tell it to connect to the radio - connect() - } - } - private val locationCallback = MeshServiceLocationCallback( ::sendPositionScoped, onSendPositionFailed = { onConnectionChanged(ConnectionState.DEVICE_SLEEP) }, @@ -269,16 +267,11 @@ class MeshService : Service(), Logging { } } - /// Safely access the radio service, if not connected an exception will be thrown - private val connectedRadio: IRadioInterfaceService - get() = (if (connectionState == ConnectionState.CONNECTED) radio.serviceP else null) - ?: throw RadioNotConnectedException() - /** Send a command/packet to our radio. But cope with the possiblity that we might start up before we are fully bound to the RadioInterfaceService @param requireConnected set to false if you are okay with using a partially connected device (i.e. during startup) */ - private fun sendToRadio(p: ToRadio.Builder, requireConnected: Boolean = true) { + private fun sendToRadio(p: ToRadio.Builder) { val built = p.build() debug("Sending to radio ${built.toPIIString()}") val b = built.toByteArray() @@ -286,21 +279,16 @@ class MeshService : Service(), Logging { if (SoftwareUpdateService.isUpdating) throw IsUpdatingException() - if (requireConnected) - connectedRadio.sendToRadio(b) - else { - val s = radio.serviceP ?: throw RadioNotConnectedException() - s.sendToRadio(b) - } + radioInterfaceService.sendToRadio(b) } /** * Send a mesh packet to the radio, if the radio is not currently connected this function will throw NotConnectedException */ - private fun sendToRadio(packet: MeshPacket, requireConnected: Boolean = true) { + private fun sendToRadio(packet: MeshPacket) { sendToRadio(ToRadio.newBuilder().apply { this.packet = packet - }, requireConnected) + }) } private fun updateMessageNotification(message: DataPacket) = @@ -338,17 +326,11 @@ class MeshService : Service(), Logging { serviceScope.handledLaunch { loadSettings() // Load our last known node DB - // we listen for messages from the radio receiver _before_ trying to create the service - val filter = IntentFilter().apply { - addAction(RadioInterfaceService.RECEIVE_FROMRADIO_ACTION) - addAction(RadioInterfaceService.RADIO_CONNECTED_ACTION) - } - registerReceiver(radioInterfaceReceiver, filter) + radioInterfaceService.receivedData.onEach(::onReceiveFromRadio) + radioInterfaceService.connectionState.onEach(::onRadioConnectionState) // We in turn need to use the radiointerface service - val intent = Intent(this@MeshService, RadioInterfaceService::class.java) - // intent.action = IMeshService::class.java.name - radio.connect(this@MeshService, intent, Context.BIND_AUTO_CREATE) + radioInterfaceService.connect() // the rest of our init will happen once we are in radioConnection.onServiceConnected } @@ -375,12 +357,6 @@ class MeshService : Service(), Logging { override fun onDestroy() { info("Destroying mesh service") - // This might fail if we get destroyed before the handledLaunch completes - ignoreException(silent = true) { - unregisterReceiver(radioInterfaceReceiver) - } - - radio.close() saveSettings() stopForeground(true) // Make sure we aren't using the notification first @@ -1217,67 +1193,44 @@ class MeshService : Service(), Logging { } } - /** - * Receives messages from our BT radio service and processes them to update our model - * and send to clients as needed. - */ - private val radioInterfaceReceiver = object : BroadcastReceiver() { - - // Important to never throw exceptions out of onReceive - override fun onReceive(context: Context, intent: Intent) = exceptionReporter { - // NOTE: Do not call handledLaunch here, because it can cause out of order message processing - because each routine is scheduled independently - // serviceScope.handledLaunch { - debug("Received broadcast ${intent.action}") - when (intent.action) { - RadioInterfaceService.RADIO_CONNECTED_ACTION -> { - try { - // sleep now disabled by default on ESP32, permanent is true unless isPowerSaving enabled - val lsEnabled = radioConfig?.preferences?.isPowerSaving ?: false - val connected = intent.getBooleanExtra(EXTRA_CONNECTED, false) - val permanent = intent.getBooleanExtra(EXTRA_PERMANENT, false) || !lsEnabled - onConnectionChanged( - when { - connected -> ConnectionState.CONNECTED - permanent -> ConnectionState.DISCONNECTED - else -> ConnectionState.DEVICE_SLEEP - } - ) - } catch (ex: RemoteException) { - // This can happen sometimes (especially if the device is slowly dying due to killing power, don't report to crashlytics - warn("Abandoning reconnect attempt, due to errors during init: ${ex.message}") - } - } - - RadioInterfaceService.RECEIVE_FROMRADIO_ACTION -> { - val bytes = intent.getByteArrayExtra(EXTRA_PAYLOAD)!! - try { - val proto = - MeshProtos.FromRadio.parseFrom(bytes) - // info("Received from radio service: ${proto.toOneLineString()}") - when (proto.payloadVariantCase.number) { - MeshProtos.FromRadio.PACKET_FIELD_NUMBER -> handleReceivedMeshPacket( - proto.packet - ) - - MeshProtos.FromRadio.CONFIG_COMPLETE_ID_FIELD_NUMBER -> handleConfigComplete( - proto.configCompleteId - ) - - MeshProtos.FromRadio.MY_INFO_FIELD_NUMBER -> handleMyInfo(proto.myInfo) - - MeshProtos.FromRadio.NODE_INFO_FIELD_NUMBER -> handleNodeInfo(proto.nodeInfo) - - // MeshProtos.FromRadio.RADIO_FIELD_NUMBER -> handleRadioConfig(proto.radio) - - else -> errormsg("Unexpected FromRadio variant") - } - } catch (ex: InvalidProtocolBufferException) { - errormsg("Invalid Protobuf from radio, len=${bytes.size}", ex) - } - } - - else -> errormsg("Unexpected radio interface broadcast") + private fun onRadioConnectionState(state: RadioServiceConnectionState) { + // sleep now disabled by default on ESP32, permanent is true unless isPowerSaving enabled + val lsEnabled = radioConfig?.preferences?.isPowerSaving ?: false + val connected = state.isConnected + val permanent = state.isPermanent || !lsEnabled + onConnectionChanged( + when { + connected -> ConnectionState.CONNECTED + permanent -> ConnectionState.DISCONNECTED + else -> ConnectionState.DEVICE_SLEEP } + ) + } + + private fun onReceiveFromRadio(bytes: ByteArray) { + try { + val proto = + MeshProtos.FromRadio.parseFrom(bytes) + // info("Received from radio service: ${proto.toOneLineString()}") + when (proto.payloadVariantCase.number) { + MeshProtos.FromRadio.PACKET_FIELD_NUMBER -> handleReceivedMeshPacket( + proto.packet + ) + + MeshProtos.FromRadio.CONFIG_COMPLETE_ID_FIELD_NUMBER -> handleConfigComplete( + proto.configCompleteId + ) + + MeshProtos.FromRadio.MY_INFO_FIELD_NUMBER -> handleMyInfo(proto.myInfo) + + MeshProtos.FromRadio.NODE_INFO_FIELD_NUMBER -> handleNodeInfo(proto.nodeInfo) + + // MeshProtos.FromRadio.RADIO_FIELD_NUMBER -> handleRadioConfig(proto.radio) + + else -> errormsg("Unexpected FromRadio variant") + } + } catch (ex: InvalidProtocolBufferException) { + errormsg("Invalid Protobuf from radio, len=${bytes.size}", ex) } } @@ -1533,13 +1486,13 @@ class MeshService : Service(), Logging { private fun requestRadioConfig() { sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket(wantResponse = true) { getRadioRequest = true - }, requireConnected = false) + }) } private fun requestChannel(channelIndex: Int) { sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket(wantResponse = true) { getChannelRequest = channelIndex + 1 - }, requireConnected = false) + }) } private fun setChannel(channel: ChannelProtos.Channel) { @@ -1759,7 +1712,7 @@ class MeshService : Service(), Logging { override fun setDeviceAddress(deviceAddr: String?) = toRemoteExceptions { debug("Passing through device change to radio service: ${deviceAddr.anonymize}") - val res = radio.service.setDeviceAddress(deviceAddr) + val res = radioInterfaceService.setDeviceAddress(deviceAddr) if (res) { discardNodeDB() } From 678a0358a575f4219e756ecc684d68aaec6e35d6 Mon Sep 17 00:00:00 2001 From: Mike Cumings Date: Fri, 6 May 2022 11:18:24 -0700 Subject: [PATCH 2/3] Fix local props being uninitialized prior to constructor execution --- .../repository/radio/RadioInterfaceService.kt | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/geeksville/mesh/repository/radio/RadioInterfaceService.kt b/app/src/main/java/com/geeksville/mesh/repository/radio/RadioInterfaceService.kt index ce14c321..f8ae8fe8 100644 --- a/app/src/main/java/com/geeksville/mesh/repository/radio/RadioInterfaceService.kt +++ b/app/src/main/java/com/geeksville/mesh/repository/radio/RadioInterfaceService.kt @@ -51,6 +51,27 @@ class RadioInterfaceService @Inject constructor( private val _receivedData = MutableSharedFlow() val receivedData: SharedFlow = _receivedData + private val logSends = false + private val logReceives = false + private lateinit var sentPacketsLog: BinaryLogFile // inited in onCreate + private lateinit var receivedPacketsLog: BinaryLogFile + + /** + * We recreate this scope each time we stop an interface + */ + var serviceScope = CoroutineScope(Dispatchers.IO + Job()) + + private var radioIf: IRadioInterface = NopInterface() + + /** true if we have started our interface + * + * Note: an interface may be started without necessarily yet having a connection + */ + private var isStarted = false + + /// true if our interface is currently connected to a device + private var isConnected = false + init { processLifecycle.coroutineScope.launch { bluetoothRepository.state.collect { state -> @@ -145,27 +166,6 @@ class RadioInterfaceService @Inject constructor( } } - private val logSends = false - private val logReceives = false - private lateinit var sentPacketsLog: BinaryLogFile // inited in onCreate - private lateinit var receivedPacketsLog: BinaryLogFile - - /** - * We recreate this scope each time we stop an interface - */ - var serviceScope = CoroutineScope(Dispatchers.IO + Job()) - - private var radioIf: IRadioInterface = NopInterface() - - /** true if we have started our interface - * - * Note: an interface may be started without necessarily yet having a connection - */ - private var isStarted = false - - /// true if our interface is currently connected to a device - private var isConnected = false - private fun broadcastConnectionChanged(isConnected: Boolean, isPermanent: Boolean) { debug("Broadcasting connection=$isConnected") From 7cd1e7fc96ec645813969005bcbd2876d453258e Mon Sep 17 00:00:00 2001 From: Mike Cumings Date: Fri, 6 May 2022 12:17:17 -0700 Subject: [PATCH 3/3] Mesh service needs to collect on Flow instances --- .../com/geeksville/mesh/service/MeshService.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 184aafbb..2341788e 100644 --- a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt +++ b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt @@ -39,7 +39,6 @@ import com.google.protobuf.InvalidProtocolBufferException import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.* -import kotlinx.coroutines.flow.onEach import kotlinx.serialization.json.Json import java.util.* import javax.inject.Inject @@ -326,14 +325,17 @@ class MeshService : Service(), Logging { serviceScope.handledLaunch { loadSettings() // Load our last known node DB - radioInterfaceService.receivedData.onEach(::onReceiveFromRadio) - radioInterfaceService.connectionState.onEach(::onRadioConnectionState) - // We in turn need to use the radiointerface service radioInterfaceService.connect() - - // the rest of our init will happen once we are in radioConnection.onServiceConnected } + serviceScope.handledLaunch { + radioInterfaceService.connectionState.collect(::onRadioConnectionState) + } + serviceScope.handledLaunch { + radioInterfaceService.receivedData.collect(::onReceiveFromRadio) + } + + // the rest of our init will happen once we are in radioConnection.onServiceConnected } /**