From 3e4f654f5836dd7b89a2565eaa79fa0deb3ae052 Mon Sep 17 00:00:00 2001 From: HarukiToreda <116696711+HarukiToreda@users.noreply.github.com> Date: Sun, 28 Sep 2025 00:58:48 -0400 Subject: [PATCH] Ack message cleanup --- src/MessageStore.cpp | 12 ++-- src/MessageStore.h | 11 ++-- src/graphics/draw/MessageRenderer.cpp | 40 +++++++++++- src/modules/CannedMessageModule.cpp | 92 +++++++++++++++++---------- src/modules/CannedMessageModule.h | 13 ++-- 5 files changed, 113 insertions(+), 55 deletions(-) diff --git a/src/MessageStore.cpp b/src/MessageStore.cpp index ec3fff416..ff346f224 100644 --- a/src/MessageStore.cpp +++ b/src/MessageStore.cpp @@ -60,8 +60,8 @@ const StoredMessage &MessageStore::addFromPacket(const meshtastic_MeshPacket &pa sm.type = MessageType::DM_TO_US; } - // Outgoing messages start as UNKNOWN until ACK/NACK arrives - sm.ackStatus = AckStatus::UNKNOWN; + // Outgoing messages start as NONE until ACK/NACK arrives + sm.ackStatus = AckStatus::NONE; } else { // Normal incoming sm.sender = packet.from; @@ -109,8 +109,8 @@ void MessageStore::addFromString(uint32_t sender, uint8_t channelIndex, const st sm.dest = NODENUM_BROADCAST; sm.type = MessageType::BROADCAST; - // Manual/outgoing messages start as UNKNOWN until ACK/NACK arrives - sm.ackStatus = AckStatus::UNKNOWN; + // Outgoing messages start as NONE until ACK/NACK arrives + sm.ackStatus = AckStatus::NONE; addLiveMessage(sm); } @@ -208,10 +208,10 @@ void MessageStore::loadFromFlash() if (f.readBytes((char *)&statusByte, 1) == 1) { m.ackStatus = static_cast(statusByte); } else { - m.ackStatus = AckStatus::UNKNOWN; // fallback + m.ackStatus = AckStatus::NONE; } } else { - m.ackStatus = AckStatus::UNKNOWN; // legacy files + m.ackStatus = AckStatus::NONE; } // Recompute type from dest diff --git a/src/MessageStore.h b/src/MessageStore.h index d1edc067a..9ef0c4153 100644 --- a/src/MessageStore.h +++ b/src/MessageStore.h @@ -13,10 +13,11 @@ enum class MessageType : uint8_t { BROADCAST = 0, DM_TO_US = 1 }; // Delivery status for messages we sent enum class AckStatus : uint8_t { - UNKNOWN = 0, // default (not yet resolved) - ACKED = 1, // got a valid ACK + NONE = 0, // just sent, waiting (no symbol shown) + ACKED = 1, // got a valid ACK from destination NACKED = 2, // explicitly failed - TIMEOUT = 3 // no ACK after retry window + TIMEOUT = 3, // no ACK after retry window + RELAYED = 4 // got an ACK from relay, not destination }; struct StoredMessage { @@ -43,7 +44,7 @@ struct StoredMessage { // Default constructor to initialize all fields safely StoredMessage() : timestamp(0), sender(0), channelIndex(0), text(""), dest(0xffffffff), type(MessageType::BROADCAST), - isBootRelative(false), ackStatus(AckStatus::UNKNOWN) + isBootRelative(false), ackStatus(AckStatus::NONE) // start as NONE (waiting, no symbol) { } }; @@ -92,4 +93,4 @@ class MessageStore }; // Global instance (defined in MessageStore.cpp) -extern MessageStore messageStore; \ No newline at end of file +extern MessageStore messageStore; diff --git a/src/graphics/draw/MessageRenderer.cpp b/src/graphics/draw/MessageRenderer.cpp index 4d804f195..113883b67 100644 --- a/src/graphics/draw/MessageRenderer.cpp +++ b/src/graphics/draw/MessageRenderer.cpp @@ -233,7 +233,7 @@ const std::vector &getSeenPeers() return seenPeers; } -// Helpers for drawing status marks +// Helpers for drawing status marks (thickened strokes) void drawCheckMark(OLEDDisplay *display, int x, int y, int size = 8) { int h = size; @@ -243,8 +243,15 @@ void drawCheckMark(OLEDDisplay *display, int x, int y, int size = 8) int midY = y + (FONT_HEIGHT_SMALL / 2); int topY = midY - (h / 2); + display->setColor(WHITE); // ensure we use current fg + + // Draw thicker checkmark by overdrawing lines with 1px offset + // arm 1 display->drawLine(x, topY + h / 2, x + w / 3, topY + h); + display->drawLine(x, topY + h / 2 + 1, x + w / 3, topY + h + 1); + // arm 2 display->drawLine(x + w / 3, topY + h, x + w, topY); + display->drawLine(x + w / 3, topY + h + 1, x + w, topY + 1); } void drawXMark(OLEDDisplay *display, int x, int y, int size = 8) @@ -256,8 +263,31 @@ void drawXMark(OLEDDisplay *display, int x, int y, int size = 8) int midY = y + (FONT_HEIGHT_SMALL / 2); int topY = midY - (h / 2); + display->setColor(WHITE); + + // Draw thicker X with 1px offset display->drawLine(x, topY, x + w, topY + h); + display->drawLine(x, topY + 1, x + w, topY + h + 1); display->drawLine(x + w, topY, x, topY + h); + display->drawLine(x + w, topY + 1, x, topY + h + 1); +} + +void drawRelayMark(OLEDDisplay *display, int x, int y, int size = 8) +{ + int r = size / 2; + int midY = y + (FONT_HEIGHT_SMALL / 2); + int centerY = midY; + int centerX = x + r; + + display->setColor(WHITE); + + // Draw circle outline (relay = uncertain status) + display->drawCircle(centerX, centerY, r); + + // Draw "?" inside (approx, 3px wide) + display->drawLine(centerX, centerY - 2, centerX, centerY); // stem + display->setPixel(centerX, centerY + 2); // dot + display->drawLine(centerX - 1, centerY - 4, centerX + 1, centerY - 4); } void drawTextMessageFrame(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y) @@ -453,7 +483,7 @@ void drawTextMessageFrame(OLEDDisplay *display, OLEDDisplayUiState *state, int16 allLines.push_back(ln); isMine.push_back(mine); isHeader.push_back(false); - ackForLine.push_back(AckStatus::UNKNOWN); + ackForLine.push_back(AckStatus::NONE); } } @@ -519,10 +549,16 @@ void drawTextMessageFrame(OLEDDisplay *display, OLEDDisplayUiState *state, int16 int markX = headerX - 10; int markY = lineY; if (ackForLine[i] == AckStatus::ACKED) { + // Destination ACK drawCheckMark(display, markX, markY, 8); } else if (ackForLine[i] == AckStatus::NACKED || ackForLine[i] == AckStatus::TIMEOUT) { + // Failure or timeout drawXMark(display, markX, markY, 8); + } else if (ackForLine[i] == AckStatus::RELAYED) { + // Relay ACK + drawRelayMark(display, markX, markY, 8); } + // AckStatus::NONE → show nothing } // Draw underline just under header text diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index de6732f3f..e3e4638a6 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -200,13 +200,13 @@ void CannedMessageModule::drawHeader(OLEDDisplay *display, int16_t x, int16_t y, { if (graphics::isHighResolution) { if (this->dest == NODENUM_BROADCAST) { - display->drawStringf(x, y, buffer, "To: Broadcast#%s", channels.getName(this->channel)); + display->drawStringf(x, y, buffer, "To: #%s", channels.getName(this->channel)); } else { display->drawStringf(x, y, buffer, "To: %s", getNodeName(this->dest)); } } else { if (this->dest == NODENUM_BROADCAST) { - display->drawStringf(x, y, buffer, "To: Broadc#%.5s", channels.getName(this->channel)); + display->drawStringf(x, y, buffer, "To: #%.5s", channels.getName(this->channel)); } else { display->drawStringf(x, y, buffer, "To: %s", getNodeName(this->dest)); } @@ -961,6 +961,9 @@ void CannedMessageModule::sendText(NodeNum dest, ChannelIndex channel, const cha this->lastSentNode = dest; this->incoming = dest; + // Track this packet’s request ID for matching ACKs + this->lastRequestId = p->id; + // Copy payload p->decoded.payload.size = strlen(message); memcpy(p->decoded.payload.bytes, message, p->decoded.payload.size); @@ -1008,7 +1011,7 @@ void CannedMessageModule::sendText(NodeNum dest, ChannelIndex channel, const cha sm.dest = dest; sm.type = MessageType::DM_TO_US; } - sm.ackStatus = AckStatus::UNKNOWN; + sm.ackStatus = AckStatus::NONE; messageStore.addLiveMessage(sm); @@ -2158,72 +2161,93 @@ void CannedMessageModule::drawFrame(OLEDDisplay *display, OLEDDisplayUiState *st ProcessMessage CannedMessageModule::handleReceived(const meshtastic_MeshPacket &mp) { - if (mp.decoded.portnum == meshtastic_PortNum_ROUTING_APP && waitingForAck) { + // Only process routing ACK/NACK packets that are responses to our own outbound + if (mp.decoded.portnum == meshtastic_PortNum_ROUTING_APP && waitingForAck && mp.to == nodeDB->getNodeNum() && + mp.decoded.request_id == this->lastRequestId) // only ACKs for our last sent packet + { if (mp.decoded.request_id != 0) { // Decode the routing response meshtastic_Routing decoded = meshtastic_Routing_init_default; pb_decode_from_bytes(mp.decoded.payload.bytes, mp.decoded.payload.size, meshtastic_Routing_fields, &decoded); - // Track hop metadata - this->lastAckWasRelayed = (mp.hop_limit != mp.hop_start); - this->lastAckHopStart = mp.hop_start; - this->lastAckHopLimit = mp.hop_limit; - - // Determine ACK status + // Determine ACK/NACK status bool isAck = (decoded.error_reason == meshtastic_Routing_Error_NONE); bool isFromDest = (mp.from == this->lastSentNode); bool wasBroadcast = (this->lastSentNode == NODENUM_BROADCAST); // Identify the responding node if (wasBroadcast && mp.from != nodeDB->getNodeNum()) { - this->incoming = mp.from; // Relayed by another node + this->incoming = mp.from; // relayed by another node } else { - this->incoming = this->lastSentNode; // Direct reply + this->incoming = this->lastSentNode; // direct reply } - // Final ACK confirmation logic - this->ack = isAck && (wasBroadcast || isFromDest); + // Final ACK/NACK logic + if (wasBroadcast) { + // Any ACK counts for broadcast + this->ack = isAck; + waitingForAck = false; + } else if (isFromDest) { + // Only ACK from destination counts as final + this->ack = isAck; + waitingForAck = false; + } else if (isAck) { + // Relay ACK → mark as RELAYED, still no final ACK + this->ack = false; + waitingForAck = false; // don’t wait for more + } else { + // Explicit failure + this->ack = false; + waitingForAck = false; + } - waitingForAck = false; - - // Update last sent StoredMessage with ACK/NACK result + // Update last sent StoredMessage with ACK/NACK/RELAYED result if (!messageStore.getMessages().empty()) { StoredMessage &last = const_cast(messageStore.getMessages().back()); if (last.sender == nodeDB->getNodeNum()) { // only update our own messages - if (this->ack) { + if (wasBroadcast && isAck) { last.ackStatus = AckStatus::ACKED; + } else if (isFromDest && isAck) { + last.ackStatus = AckStatus::ACKED; + } else if (!isFromDest && isAck) { + last.ackStatus = AckStatus::RELAYED; } else { - // If error_reason was explicit, you can map to NACKED; otherwise TIMEOUT - last.ackStatus = - (decoded.error_reason == meshtastic_Routing_Error_NONE) ? AckStatus::ACKED : AckStatus::NACKED; + last.ackStatus = AckStatus::NACKED; } } } - // Capture radio metrics from this ACK/NACK packet + // Capture radio metrics this->lastRxRssi = mp.rx_rssi; this->lastRxSnr = mp.rx_snr; - // Show ACK/NACK as overlay banner + // Show overlay banner if (screen) { graphics::BannerOverlayOptions opts; - static char buf[96]; + static char buf[128]; + + const char *channelName = channels.getName(this->channel); + const char *nodeName = getNodeName(this->incoming); if (this->ack) { if (this->lastSentNode == NODENUM_BROADCAST) { - snprintf(buf, sizeof(buf), "Broadcast sent to \n%s\n\nSNR: %.1f dB RSSI: %d dBm", - channels.getName(this->channel), this->lastRxSnr, this->lastRxRssi); - } else if (this->lastAckHopLimit > this->lastAckHopStart) { - snprintf(buf, sizeof(buf), "Delivered (%d hops) to \n%s\n\nSNR: %.1f dB RSSI: %d dBm", - this->lastAckHopLimit - this->lastAckHopStart, getNodeName(this->incoming), this->lastRxSnr, - this->lastRxRssi); + snprintf(buf, sizeof(buf), "Message sent to\n#%s\n\nSNR: %.1f dB RSSI: %d dBm", + (channelName && channelName[0]) ? channelName : "unknown", this->lastRxSnr, this->lastRxRssi); } else { - snprintf(buf, sizeof(buf), "Delivered to %s\n\nSNR: \n%.1f dB RSSI: %d dBm", getNodeName(this->incoming), - this->lastRxSnr, this->lastRxRssi); + snprintf(buf, sizeof(buf), "DM sent to\n@%s\n\nSNR: %.1f dB RSSI: %d dBm", + (nodeName && nodeName[0]) ? nodeName : "unknown", this->lastRxSnr, this->lastRxRssi); } + } else if (isAck && !isFromDest) { + // Relay ACK banner + snprintf(buf, sizeof(buf), "DM Relayed\n(Status Unknown)%s\n\nSNR: %.1f dB RSSI: %d dBm", + (nodeName && nodeName[0]) ? nodeName : "unknown", this->lastRxSnr, this->lastRxRssi); } else { - snprintf(buf, sizeof(buf), "Delivery failed to \n%s", getNodeName(this->incoming), this->lastRxSnr, - this->lastRxRssi); + if (this->lastSentNode == NODENUM_BROADCAST) { + snprintf(buf, sizeof(buf), "Message failed to\n#%s", + (channelName && channelName[0]) ? channelName : "unknown"); + } else { + snprintf(buf, sizeof(buf), "DM failed to\n@%s", (nodeName && nodeName[0]) ? nodeName : "unknown"); + } } opts.message = buf; diff --git a/src/modules/CannedMessageModule.h b/src/modules/CannedMessageModule.h index 890dda4b2..f32047f51 100644 --- a/src/modules/CannedMessageModule.h +++ b/src/modules/CannedMessageModule.h @@ -167,14 +167,11 @@ class CannedMessageModule : public SinglePortModule, public Observable