From 7c28c4091fda3330029069af0155169fe7f29e8e Mon Sep 17 00:00:00 2001 From: Andre K Date: Mon, 2 Jan 2023 22:23:23 -0300 Subject: [PATCH] distinguish between implicit and real ACKs (#552) --- .../mesh/database/PacketRepository.kt | 4 ++ .../geeksville/mesh/database/dao/PacketDao.kt | 8 +++ .../geeksville/mesh/service/MeshService.kt | 51 +++++-------------- .../geeksville/mesh/ui/MessagesFragment.kt | 9 +++- .../res/drawable/ic_twotone_how_to_reg_24.xml | 20 ++++++++ 5 files changed, 54 insertions(+), 38 deletions(-) create mode 100644 app/src/main/res/drawable/ic_twotone_how_to_reg_24.xml diff --git a/app/src/main/java/com/geeksville/mesh/database/PacketRepository.kt b/app/src/main/java/com/geeksville/mesh/database/PacketRepository.kt index 56eea3ee8..abf6a72df 100644 --- a/app/src/main/java/com/geeksville/mesh/database/PacketRepository.kt +++ b/app/src/main/java/com/geeksville/mesh/database/PacketRepository.kt @@ -30,6 +30,10 @@ class PacketRepository @Inject constructor(private val packetDaoLazy: dagger.Laz packetDao.updateMessageStatus(d, m) } + suspend fun getDataPacketById(requestId: Int) = withContext(Dispatchers.IO) { + packetDao.getDataPacketById(requestId) + } + suspend fun deleteAllMessages() = withContext(Dispatchers.IO) { packetDao.deleteAllMessages() } diff --git a/app/src/main/java/com/geeksville/mesh/database/dao/PacketDao.kt b/app/src/main/java/com/geeksville/mesh/database/dao/PacketDao.kt index 82f4fbab3..e98cc3bbe 100644 --- a/app/src/main/java/com/geeksville/mesh/database/dao/PacketDao.kt +++ b/app/src/main/java/com/geeksville/mesh/database/dao/PacketDao.kt @@ -47,4 +47,12 @@ interface PacketDao { val new = data.copy(status = m) findDataPacket(data)?.let { update(it.copy(data = new)) } } + + @Query("Select data from packet") + fun getDataPackets(): List + + @Transaction + fun getDataPacketById(requestId: Int): DataPacket? { + return getDataPackets().firstOrNull { it.id == requestId } + } } 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 11579d2f3..7be997ca8 100644 --- a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt +++ b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt @@ -660,10 +660,8 @@ class MeshService : Service(), Logging { shouldBroadcast = true // We always send acks to other apps, because they might care about the messages they sent val u = MeshProtos.Routing.parseFrom(data.payload) - if (u.errorReasonValue == MeshProtos.Routing.Error.NONE_VALUE) - handleAckNak(true, data.requestId) - else - handleAckNak(false, data.requestId) + val isAck = u.errorReasonValue == MeshProtos.Routing.Error.NONE_VALUE + handleAckNak(isAck, fromId, data.requestId) } Portnums.PortNum.ADMIN_APP_VALUE -> { @@ -778,9 +776,6 @@ class MeshService : Service(), Logging { /// If apps try to send packets when our radio is sleeping, we queue them here instead private val offlineSentPackets = mutableListOf() - /** Keep a record of recently sent packets, so we can properly handle ack/nak */ - private val sentPackets = mutableMapOf() - /// Update our model and resend as needed for a MeshPacket we just received from the radio private fun handleReceivedMeshPacket(packet: MeshPacket) { if (haveNodeDB) { @@ -824,6 +819,7 @@ class MeshService : Service(), Logging { * Change the status on a data packet and update watchers */ private fun changeStatus(p: DataPacket, m: MessageStatus) { + if (p.status == m) return serviceScope.handledLaunch { packetRepository.get().updateMessageStatus(p, m) } @@ -833,9 +829,17 @@ class MeshService : Service(), Logging { /** * Handle an ack/nak packet by updating sent message status */ - private fun handleAckNak(isAck: Boolean, id: Int) { - sentPackets.remove(id)?.let { p -> - changeStatus(p, if (isAck) MessageStatus.DELIVERED else MessageStatus.ERROR) + private fun handleAckNak(isAck: Boolean, fromId: String?, requestId: Int) { + serviceScope.handledLaunch { + val p = packetRepository.get().getDataPacketById(requestId) + if (p != null && p.status != MessageStatus.RECEIVED) { + val m = when { + isAck && fromId == p.to -> MessageStatus.RECEIVED + isAck -> MessageStatus.DELIVERED + else -> MessageStatus.ERROR + } + changeStatus(p, m) + } } } @@ -1546,28 +1550,6 @@ class MeshService : Service(), Logging { } } - /** - * Remove any sent packets that have been sitting around too long - * - * Note: we give each message what the timeout the device code is using, though in the normal - * case the device will fail after 3 retries much sooner than that (and it will provide a nak to us) - */ - private fun deleteOldPackets() { - myNodeInfo?.apply { - val now = System.currentTimeMillis() - - val old = sentPackets.values.filter { p -> - (p.status == MessageStatus.ENROUTE && p.time + messageTimeoutMsec < now) - } - - // Do this using a separate list to prevent concurrent modification exceptions - old.forEach { p -> - handleAckNak(false, p.id) - } - } - } - - private fun enqueueForSending(p: DataPacket) { p.status = MessageStatus.QUEUED offlineSentPackets.add(p) @@ -1642,11 +1624,6 @@ class MeshService : Service(), Logging { // Keep a record of datapackets, so GUIs can show proper chat history rememberDataPacket(p) - if (p.id != 0) { // If we have an ID we can wait for an ack or nak - deleteOldPackets() - sentPackets[p.id] = p - } - GeeksvilleApplication.analytics.track( "data_send", DataPair("num_bytes", p.bytes.size), diff --git a/app/src/main/java/com/geeksville/mesh/ui/MessagesFragment.kt b/app/src/main/java/com/geeksville/mesh/ui/MessagesFragment.kt index 1bd6b92a3..8ee7cd8d5 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/MessagesFragment.kt +++ b/app/src/main/java/com/geeksville/mesh/ui/MessagesFragment.kt @@ -167,6 +167,7 @@ class MessagesFragment : Fragment(), Logging { holder.messageTime.text = getShortDateTime(Date(msg.time)) val icon = when (msg.status) { + MessageStatus.RECEIVED -> R.drawable.ic_twotone_how_to_reg_24 MessageStatus.QUEUED -> R.drawable.ic_twotone_cloud_upload_24 MessageStatus.DELIVERED -> R.drawable.cloud_on MessageStatus.ENROUTE -> R.drawable.ic_twotone_cloud_24 @@ -174,12 +175,18 @@ class MessagesFragment : Fragment(), Logging { else -> null } - if (icon != null) { + if (icon != null && isLocal) { holder.messageStatusIcon.setImageResource(icon) holder.messageStatusIcon.visibility = View.VISIBLE } else holder.messageStatusIcon.visibility = View.GONE + holder.messageStatusIcon.setOnClickListener { + if (isAdded) { + Toast.makeText(context, "${msg.status}", Toast.LENGTH_SHORT).show() + } + } + holder.itemView.setOnLongClickListener { clickItem(holder) if (actionMode == null) { diff --git a/app/src/main/res/drawable/ic_twotone_how_to_reg_24.xml b/app/src/main/res/drawable/ic_twotone_how_to_reg_24.xml new file mode 100644 index 000000000..578905e5b --- /dev/null +++ b/app/src/main/res/drawable/ic_twotone_how_to_reg_24.xml @@ -0,0 +1,20 @@ + + + + +