From 0cf7b93f286975efe2289a9df82f4679b4e3ac79 Mon Sep 17 00:00:00 2001 From: Phil Oliver <3497406+poliver@users.noreply.github.com> Date: Thu, 25 Sep 2025 17:22:14 -0400 Subject: [PATCH] Less state held by `MeshService` (#3205) --- .../geeksville/mesh/service/MeshService.kt | 161 +++++------------- .../mesh/service/MeshServiceBroadcasts.kt | 26 ++- .../MeshServiceConnectionStateHolder.kt | 32 ++++ .../geeksville/mesh/service/PacketHandler.kt | 22 ++- 4 files changed, 111 insertions(+), 130 deletions(-) create mode 100644 app/src/main/java/com/geeksville/mesh/service/MeshServiceConnectionStateHolder.kt 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 760ec8cec..a9bca546f 100644 --- a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt +++ b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt @@ -159,6 +159,12 @@ class MeshService : @Inject lateinit var uiPrefs: UiPrefs + @Inject lateinit var connectionStateHolder: MeshServiceConnectionStateHolder + + @Inject lateinit var packetHandler: PacketHandler + + @Inject lateinit var serviceBroadcasts: MeshServiceBroadcasts + private val tracerouteStartTimes = ConcurrentHashMap() companion object : Logging { @@ -217,23 +223,8 @@ class MeshService : private var previousSummary: String? = null private var previousStats: LocalStats? = null - // A mapping of receiver class name to package name - used for explicit broadcasts - private val clientPackages = mutableMapOf() - private val serviceBroadcasts = - MeshServiceBroadcasts(this, clientPackages) { - connectionState.also { serviceRepository.setConnectionState(it) } - } - private val packetHandler: PacketHandler by lazy { - PacketHandler( - packetRepository = packetRepository, - serviceBroadcasts = serviceBroadcasts, - radioInterfaceService = radioInterfaceService, - meshLogRepository = meshLogRepository, - ) - } private val serviceJob = Job() private val serviceScope = CoroutineScope(Dispatchers.IO + serviceJob) - private var connectionState = ConnectionState.DISCONNECTED private var locationFlow: Job? = null private var mqttMessageFlow: Job? = null @@ -252,7 +243,7 @@ class MeshService : private val notificationSummary get() = - when (connectionState) { + when (connectionStateHolder.getState()) { ConnectionState.CONNECTED -> getString(R.string.connected_count).format(numOnlineNodes) ConnectionState.DISCONNECTED -> getString(R.string.disconnected) @@ -1135,7 +1126,7 @@ class MeshService : val packet = toMeshPacket(p) p.time = System.currentTimeMillis() // update time to the actual time we started sending // debug("Sending to radio: ${packet.toPIIString()}") - packetHandler.sendToRadio(packet) { connectionState } + packetHandler.sendToRadio(packet) } private fun processQueuedPackets() { @@ -1284,7 +1275,7 @@ class MeshService : // Called when we gain/lose connection to our radio private fun onConnectionChanged(c: ConnectionState) { - debug("onConnectionChanged: $connectionState -> $c") + debug("onConnectionChanged: ${connectionStateHolder.getState()} -> $c") // Perform all the steps needed once we start waiting for device sleep to complete fun startDeviceSleep() { @@ -1355,7 +1346,7 @@ class MeshService : // state we don't want // to // claim we have a valid connection still - connectionState = ConnectionState.DEVICE_SLEEP + connectionStateHolder.setState(ConnectionState.DEVICE_SLEEP) startDeviceSleep() throw ex // Important to rethrow so that we don't tell the app all is well } @@ -1367,7 +1358,7 @@ class MeshService : sleepTimeout = null } - connectionState = c + connectionStateHolder.setState(c) when (c) { ConnectionState.CONNECTED -> startConnect() ConnectionState.DEVICE_SLEEP -> startDeviceSleep() @@ -1794,9 +1785,7 @@ class MeshService : } private fun onHasSettings() { - packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { setTimeOnly = currentSecond() }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { setTimeOnly = currentSecond() }) processQueuedPackets() // send any packets that were queued up startMqttClientProxy() serviceBroadcasts.broadcastConnection() @@ -1893,9 +1882,7 @@ class MeshService : payload = position.toByteString() this.wantResponse = wantResponse }, - ) { - connectionState - } + ) } } catch (ex: BLEException) { warn("Ignoring disconnected radio during gps location update") @@ -1922,11 +1909,7 @@ class MeshService : handleReceivedUser(dest.num, user) // encapsulate our payload in the proper protobuf and fire it off - packetHandler.sendToRadio( - newMeshPacketTo(dest.num).buildAdminPacket(id = packetId) { setOwner = user }, - ) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(dest.num).buildAdminPacket(id = packetId) { setOwner = user }) } } @@ -1965,18 +1948,14 @@ class MeshService : } private fun importContact(contact: AdminProtos.SharedContact) { - packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { addContact = contact }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { addContact = contact }) handleReceivedUser(contact.nodeNum, contact.user) } private fun getDeviceMetadata(destNum: Int) = toRemoteExceptions { packetHandler.sendToRadio( newMeshPacketTo(destNum).buildAdminPacket(wantResponse = true) { getDeviceMetadataRequest = true }, - ) { - connectionState - } + ) } private fun favoriteNode(node: Node) = toRemoteExceptions { @@ -1990,9 +1969,7 @@ class MeshService : setFavoriteNode = node.num } }, - ) { - connectionState - } + ) updateNodeInfo(node.num) { it.isFavorite = !node.isFavorite } } @@ -2007,9 +1984,7 @@ class MeshService : setIgnoredNode = node.num } }, - ) { - connectionState - } + ) updateNodeInfo(node.num) { it.isIgnored = !node.isIgnored } } @@ -2025,7 +2000,7 @@ class MeshService : portnumValue = Portnums.PortNum.TEXT_MESSAGE_APP_VALUE payload = ByteString.copyFrom(reaction.emoji.encodeToByteArray()) } - packetHandler.sendToRadio(packet) { connectionState } + packetHandler.sendToRadio(packet) rememberReaction(packet.copy { from = myNodeNum }) } @@ -2088,7 +2063,7 @@ class MeshService : // wrapper // per https://blog.classycode.com/dealing-with-exceptions-in-aidl-9ba904c6d63 override fun subscribeReceiver(packageName: String, receiverName: String) = toRemoteExceptions { - clientPackages[receiverName] = packageName + serviceBroadcasts.subscribeReceiver(receiverName, packageName) } override fun getUpdateStatus(): Int = -4 // ProgressNotStarted @@ -2123,9 +2098,7 @@ class MeshService : override fun getRemoteOwner(id: Int, destNum: Int) = toRemoteExceptions { packetHandler.sendToRadio( newMeshPacketTo(destNum).buildAdminPacket(id = id, wantResponse = true) { getOwnerRequest = true }, - ) { - connectionState - } + ) } override fun send(p: DataPacket) { @@ -2135,7 +2108,7 @@ class MeshService : val bytes = p.bytes!! info( "sendData dest=${p.to}, id=${p.id} <- ${bytes.size} bytes" + - " (connectionState=$connectionState)", + " (connectionState=${connectionStateHolder.getState()})", ) if (p.dataType == 0) { @@ -2149,7 +2122,7 @@ class MeshService : p.status = MessageStatus.QUEUED } - if (connectionState == ConnectionState.CONNECTED) { + if (connectionStateHolder.getState() == ConnectionState.CONNECTED) { try { sendNow(p) } catch (ex: Exception) { @@ -2186,9 +2159,7 @@ class MeshService : override fun setRemoteConfig(id: Int, num: Int, payload: ByteArray) = toRemoteExceptions { debug("Setting new radio config!") val config = ConfigProtos.Config.parseFrom(payload) - packetHandler.sendToRadio(newMeshPacketTo(num).buildAdminPacket(id = id) { setConfig = config }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(num).buildAdminPacket(id = id) { setConfig = config }) if (num == myNodeNum) setLocalConfig(config) // Update our local copy } @@ -2201,18 +2172,14 @@ class MeshService : getConfigRequestValue = config } }, - ) { - connectionState - } + ) } /** Send our current module config to the device */ override fun setModuleConfig(id: Int, num: Int, payload: ByteArray) = toRemoteExceptions { debug("Setting new module config!") val config = ModuleConfigProtos.ModuleConfig.parseFrom(payload) - packetHandler.sendToRadio(newMeshPacketTo(num).buildAdminPacket(id = id) { setModuleConfig = config }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(num).buildAdminPacket(id = id) { setModuleConfig = config }) if (num == myNodeNum) setLocalModuleConfig(config) // Update our local copy } @@ -2221,15 +2188,11 @@ class MeshService : newMeshPacketTo(destNum).buildAdminPacket(id = id, wantResponse = true) { getModuleConfigRequestValue = config }, - ) { - connectionState - } + ) } override fun setRingtone(destNum: Int, ringtone: String) = toRemoteExceptions { - packetHandler.sendToRadio(newMeshPacketTo(destNum).buildAdminPacket { setRingtoneMessage = ringtone }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(destNum).buildAdminPacket { setRingtoneMessage = ringtone }) } override fun getRingtone(id: Int, destNum: Int) = toRemoteExceptions { @@ -2237,17 +2200,13 @@ class MeshService : newMeshPacketTo(destNum).buildAdminPacket(id = id, wantResponse = true) { getRingtoneRequest = true }, - ) { - connectionState - } + ) } override fun setCannedMessages(destNum: Int, messages: String) = toRemoteExceptions { packetHandler.sendToRadio( newMeshPacketTo(destNum).buildAdminPacket { setCannedMessageModuleMessages = messages }, - ) { - connectionState - } + ) } override fun getCannedMessages(id: Int, destNum: Int) = toRemoteExceptions { @@ -2255,9 +2214,7 @@ class MeshService : newMeshPacketTo(destNum).buildAdminPacket(id = id, wantResponse = true) { getCannedMessageModuleMessagesRequest = true }, - ) { - connectionState - } + ) } override fun setChannel(payload: ByteArray?) = toRemoteExceptions { @@ -2266,9 +2223,7 @@ class MeshService : override fun setRemoteChannel(id: Int, num: Int, payload: ByteArray?) = toRemoteExceptions { val channel = ChannelProtos.Channel.parseFrom(payload) - packetHandler.sendToRadio(newMeshPacketTo(num).buildAdminPacket(id = id) { setChannel = channel }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(num).buildAdminPacket(id = id) { setChannel = channel }) } override fun getRemoteChannel(id: Int, destNum: Int, index: Int) = toRemoteExceptions { @@ -2276,21 +2231,15 @@ class MeshService : newMeshPacketTo(destNum).buildAdminPacket(id = id, wantResponse = true) { getChannelRequest = index + 1 }, - ) { - connectionState - } + ) } override fun beginEditSettings() = toRemoteExceptions { - packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { beginEditSettings = true }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { beginEditSettings = true }) } override fun commitEditSettings() = toRemoteExceptions { - packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { commitEditSettings = true }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { commitEditSettings = true }) } override fun getChannelSet(): ByteArray = toRemoteExceptions { this@MeshService.channelSet.toByteArray() } @@ -2303,7 +2252,7 @@ class MeshService : } override fun connectionState(): String = toRemoteExceptions { - val r = this@MeshService.connectionState + val r = connectionStateHolder.getState() info("in connectionState=$r") r.toString() } @@ -2314,9 +2263,7 @@ class MeshService : override fun removeByNodenum(requestId: Int, nodeNum: Int) = toRemoteExceptions { nodeDBbyNodeNum.remove(nodeNum) - packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { removeByNodenum = nodeNum }) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(myNodeNum).buildAdminPacket { removeByNodenum = nodeNum }) } override fun requestUserInfo(destNum: Int) = toRemoteExceptions { @@ -2327,9 +2274,7 @@ class MeshService : wantResponse = true payload = nodeDBbyNodeNum[myNodeNum]!!.user.toByteString() }, - ) { - connectionState - } + ) } } @@ -2369,9 +2314,7 @@ class MeshService : payload = meshPosition.toByteString() wantResponse = true }, - ) { - connectionState - } + ) } } @@ -2389,9 +2332,7 @@ class MeshService : removeFixedPosition = true } }, - ) { - connectionState - } + ) updateNodeInfo(destNum) { it.setPosition(pos, currentSecond()) } } @@ -2406,41 +2347,29 @@ class MeshService : portnumValue = Portnums.PortNum.TRACEROUTE_APP_VALUE wantResponse = true }, - ) { - connectionState - } + ) } override fun requestShutdown(requestId: Int, destNum: Int) = toRemoteExceptions { packetHandler.sendToRadio( newMeshPacketTo(destNum).buildAdminPacket(id = requestId) { shutdownSeconds = 5 }, - ) { - connectionState - } + ) } override fun requestReboot(requestId: Int, destNum: Int) = toRemoteExceptions { packetHandler.sendToRadio( newMeshPacketTo(destNum).buildAdminPacket(id = requestId) { rebootSeconds = 5 }, - ) { - connectionState - } + ) } override fun requestFactoryReset(requestId: Int, destNum: Int) = toRemoteExceptions { packetHandler.sendToRadio( newMeshPacketTo(destNum).buildAdminPacket(id = requestId) { factoryResetDevice = 1 }, - ) { - connectionState - } + ) } override fun requestNodedbReset(requestId: Int, destNum: Int) = toRemoteExceptions { - packetHandler.sendToRadio( - newMeshPacketTo(destNum).buildAdminPacket(id = requestId) { nodedbReset = 1 }, - ) { - connectionState - } + packetHandler.sendToRadio(newMeshPacketTo(destNum).buildAdminPacket(id = requestId) { nodedbReset = 1 }) } } } diff --git a/app/src/main/java/com/geeksville/mesh/service/MeshServiceBroadcasts.kt b/app/src/main/java/com/geeksville/mesh/service/MeshServiceBroadcasts.kt index de60aebbe..d36a6f9ce 100644 --- a/app/src/main/java/com/geeksville/mesh/service/MeshServiceBroadcasts.kt +++ b/app/src/main/java/com/geeksville/mesh/service/MeshServiceBroadcasts.kt @@ -20,15 +20,28 @@ package com.geeksville.mesh.service import android.content.Context import android.content.Intent import android.os.Parcelable +import dagger.hilt.android.qualifiers.ApplicationContext import org.meshtastic.core.model.DataPacket import org.meshtastic.core.model.MessageStatus import org.meshtastic.core.model.NodeInfo +import javax.inject.Inject +import javax.inject.Singleton -class MeshServiceBroadcasts( - private val context: Context, - private val clientPackages: MutableMap, - private val getConnectionState: () -> ConnectionState, +@Singleton +class MeshServiceBroadcasts +@Inject +constructor( + @ApplicationContext private val context: Context, + private val connectionStateHolder: MeshServiceConnectionStateHolder, + private val serviceRepository: ServiceRepository, ) { + // A mapping of receiver class name to package name - used for explicit broadcasts + private val clientPackages = mutableMapOf() + + fun subscribeReceiver(receiverName: String, packageName: String) { + clientPackages[receiverName] = packageName + } + /** Broadcast some received data Payload will be a DataPacket */ fun broadcastReceivedData(payload: DataPacket) { explicitBroadcast(Intent(MeshService.actionReceived(payload.dataType)).putExtra(EXTRA_PAYLOAD, payload)) @@ -59,8 +72,9 @@ class MeshServiceBroadcasts( /** Broadcast our current connection status */ fun broadcastConnection() { - val intent = - Intent(MeshService.ACTION_MESH_CONNECTED).putExtra(EXTRA_CONNECTED, getConnectionState().toString()) + val connectionState = connectionStateHolder.getState() + val intent = Intent(MeshService.ACTION_MESH_CONNECTED).putExtra(EXTRA_CONNECTED, connectionState.toString()) + serviceRepository.setConnectionState(connectionState) explicitBroadcast(intent) } diff --git a/app/src/main/java/com/geeksville/mesh/service/MeshServiceConnectionStateHolder.kt b/app/src/main/java/com/geeksville/mesh/service/MeshServiceConnectionStateHolder.kt new file mode 100644 index 000000000..40c6f99de --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/service/MeshServiceConnectionStateHolder.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2025 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.geeksville.mesh.service + +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class MeshServiceConnectionStateHolder @Inject constructor() { + private var connectionState = ConnectionState.DISCONNECTED + + fun setState(state: ConnectionState) { + connectionState = state + } + + fun getState() = connectionState +} diff --git a/app/src/main/java/com/geeksville/mesh/service/PacketHandler.kt b/app/src/main/java/com/geeksville/mesh/service/PacketHandler.kt index bc9506bc2..094f48ba7 100644 --- a/app/src/main/java/com/geeksville/mesh/service/PacketHandler.kt +++ b/app/src/main/java/com/geeksville/mesh/service/PacketHandler.kt @@ -44,12 +44,18 @@ import java.util.UUID import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException +import javax.inject.Inject +import javax.inject.Singleton -class PacketHandler( +@Singleton +class PacketHandler +@Inject +constructor( private val packetRepository: Lazy, private val serviceBroadcasts: MeshServiceBroadcasts, private val radioInterfaceService: RadioInterfaceService, private val meshLogRepository: Lazy, + private val connectionStateHolder: MeshServiceConnectionStateHolder, ) { private var queueJob: Job? = null @@ -89,9 +95,9 @@ class PacketHandler( * Send a mesh packet to the radio, if the radio is not currently connected this function will throw * NotConnectedException */ - fun sendToRadio(packet: MeshPacket, getConnectionState: () -> ConnectionState) { + fun sendToRadio(packet: MeshPacket) { queuedPackets.add(packet) - startPacketQueue(getConnectionState) + startPacketQueue() } fun stopPacketQueue() { @@ -121,17 +127,17 @@ class PacketHandler( } @Suppress("TooGenericExceptionCaught", "SwallowedException") - private fun startPacketQueue(getConnectionState: () -> ConnectionState) { + private fun startPacketQueue() { if (queueJob?.isActive == true) return queueJob = scope.handledLaunch { debug("packet queueJob started") - while (getConnectionState() == ConnectionState.CONNECTED) { + while (connectionStateHolder.getState() == ConnectionState.CONNECTED) { // take the first packet from the queue head val packet = queuedPackets.poll() ?: break try { // send packet to the radio and wait for response - val response = sendPacket(packet, getConnectionState) + val response = sendPacket(packet) debug("queueJob packet id=${packet.id.toUInt()} waiting") val success = response.get(2, TimeUnit.MINUTES) debug("queueJob packet id=${packet.id.toUInt()} success $success") @@ -166,13 +172,13 @@ class PacketHandler( } @Suppress("TooGenericExceptionCaught") - private fun sendPacket(packet: MeshPacket, getConnectionState: () -> ConnectionState): CompletableFuture { + private fun sendPacket(packet: MeshPacket): CompletableFuture { // send the packet to the radio and return a CompletableFuture that will be completed with // the result val future = CompletableFuture() queueResponse[packet.id] = future try { - if (getConnectionState() != ConnectionState.CONNECTED) throw RadioNotConnectedException() + if (connectionStateHolder.getState() != ConnectionState.CONNECTED) throw RadioNotConnectedException() sendToRadio(ToRadio.newBuilder().apply { this.packet = packet }) } catch (ex: Exception) { errormsg("sendToRadio error:", ex)