Ack message cleanup

This commit is contained in:
HarukiToreda 2025-09-28 00:58:48 -04:00
parent 28502c93c9
commit 3e4f654f58
5 changed files with 113 additions and 55 deletions

View File

@ -60,8 +60,8 @@ const StoredMessage &MessageStore::addFromPacket(const meshtastic_MeshPacket &pa
sm.type = MessageType::DM_TO_US; sm.type = MessageType::DM_TO_US;
} }
// Outgoing messages start as UNKNOWN until ACK/NACK arrives // Outgoing messages start as NONE until ACK/NACK arrives
sm.ackStatus = AckStatus::UNKNOWN; sm.ackStatus = AckStatus::NONE;
} else { } else {
// Normal incoming // Normal incoming
sm.sender = packet.from; sm.sender = packet.from;
@ -109,8 +109,8 @@ void MessageStore::addFromString(uint32_t sender, uint8_t channelIndex, const st
sm.dest = NODENUM_BROADCAST; sm.dest = NODENUM_BROADCAST;
sm.type = MessageType::BROADCAST; sm.type = MessageType::BROADCAST;
// Manual/outgoing messages start as UNKNOWN until ACK/NACK arrives // Outgoing messages start as NONE until ACK/NACK arrives
sm.ackStatus = AckStatus::UNKNOWN; sm.ackStatus = AckStatus::NONE;
addLiveMessage(sm); addLiveMessage(sm);
} }
@ -208,10 +208,10 @@ void MessageStore::loadFromFlash()
if (f.readBytes((char *)&statusByte, 1) == 1) { if (f.readBytes((char *)&statusByte, 1) == 1) {
m.ackStatus = static_cast<AckStatus>(statusByte); m.ackStatus = static_cast<AckStatus>(statusByte);
} else { } else {
m.ackStatus = AckStatus::UNKNOWN; // fallback m.ackStatus = AckStatus::NONE;
} }
} else { } else {
m.ackStatus = AckStatus::UNKNOWN; // legacy files m.ackStatus = AckStatus::NONE;
} }
// Recompute type from dest // Recompute type from dest

View File

@ -13,10 +13,11 @@ enum class MessageType : uint8_t { BROADCAST = 0, DM_TO_US = 1 };
// Delivery status for messages we sent // Delivery status for messages we sent
enum class AckStatus : uint8_t { enum class AckStatus : uint8_t {
UNKNOWN = 0, // default (not yet resolved) NONE = 0, // just sent, waiting (no symbol shown)
ACKED = 1, // got a valid ACK ACKED = 1, // got a valid ACK from destination
NACKED = 2, // explicitly failed 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 { struct StoredMessage {
@ -43,7 +44,7 @@ struct StoredMessage {
// Default constructor to initialize all fields safely // Default constructor to initialize all fields safely
StoredMessage() StoredMessage()
: timestamp(0), sender(0), channelIndex(0), text(""), dest(0xffffffff), type(MessageType::BROADCAST), : 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)
{ {
} }
}; };

View File

@ -233,7 +233,7 @@ const std::vector<uint32_t> &getSeenPeers()
return seenPeers; 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) void drawCheckMark(OLEDDisplay *display, int x, int y, int size = 8)
{ {
int h = size; 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 midY = y + (FONT_HEIGHT_SMALL / 2);
int topY = midY - (h / 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, 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, 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) 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 midY = y + (FONT_HEIGHT_SMALL / 2);
int topY = midY - (h / 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, 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, 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) 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); allLines.push_back(ln);
isMine.push_back(mine); isMine.push_back(mine);
isHeader.push_back(false); 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 markX = headerX - 10;
int markY = lineY; int markY = lineY;
if (ackForLine[i] == AckStatus::ACKED) { if (ackForLine[i] == AckStatus::ACKED) {
// Destination ACK
drawCheckMark(display, markX, markY, 8); drawCheckMark(display, markX, markY, 8);
} else if (ackForLine[i] == AckStatus::NACKED || ackForLine[i] == AckStatus::TIMEOUT) { } else if (ackForLine[i] == AckStatus::NACKED || ackForLine[i] == AckStatus::TIMEOUT) {
// Failure or timeout
drawXMark(display, markX, markY, 8); 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 // Draw underline just under header text

View File

@ -200,13 +200,13 @@ void CannedMessageModule::drawHeader(OLEDDisplay *display, int16_t x, int16_t y,
{ {
if (graphics::isHighResolution) { if (graphics::isHighResolution) {
if (this->dest == NODENUM_BROADCAST) { 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 { } else {
display->drawStringf(x, y, buffer, "To: %s", getNodeName(this->dest)); display->drawStringf(x, y, buffer, "To: %s", getNodeName(this->dest));
} }
} else { } else {
if (this->dest == NODENUM_BROADCAST) { 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 { } else {
display->drawStringf(x, y, buffer, "To: %s", getNodeName(this->dest)); 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->lastSentNode = dest;
this->incoming = dest; this->incoming = dest;
// Track this packets request ID for matching ACKs
this->lastRequestId = p->id;
// Copy payload // Copy payload
p->decoded.payload.size = strlen(message); p->decoded.payload.size = strlen(message);
memcpy(p->decoded.payload.bytes, message, p->decoded.payload.size); 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.dest = dest;
sm.type = MessageType::DM_TO_US; sm.type = MessageType::DM_TO_US;
} }
sm.ackStatus = AckStatus::UNKNOWN; sm.ackStatus = AckStatus::NONE;
messageStore.addLiveMessage(sm); messageStore.addLiveMessage(sm);
@ -2158,72 +2161,93 @@ void CannedMessageModule::drawFrame(OLEDDisplay *display, OLEDDisplayUiState *st
ProcessMessage CannedMessageModule::handleReceived(const meshtastic_MeshPacket &mp) 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) { if (mp.decoded.request_id != 0) {
// Decode the routing response // Decode the routing response
meshtastic_Routing decoded = meshtastic_Routing_init_default; meshtastic_Routing decoded = meshtastic_Routing_init_default;
pb_decode_from_bytes(mp.decoded.payload.bytes, mp.decoded.payload.size, meshtastic_Routing_fields, &decoded); pb_decode_from_bytes(mp.decoded.payload.bytes, mp.decoded.payload.size, meshtastic_Routing_fields, &decoded);
// Track hop metadata // Determine ACK/NACK status
this->lastAckWasRelayed = (mp.hop_limit != mp.hop_start);
this->lastAckHopStart = mp.hop_start;
this->lastAckHopLimit = mp.hop_limit;
// Determine ACK status
bool isAck = (decoded.error_reason == meshtastic_Routing_Error_NONE); bool isAck = (decoded.error_reason == meshtastic_Routing_Error_NONE);
bool isFromDest = (mp.from == this->lastSentNode); bool isFromDest = (mp.from == this->lastSentNode);
bool wasBroadcast = (this->lastSentNode == NODENUM_BROADCAST); bool wasBroadcast = (this->lastSentNode == NODENUM_BROADCAST);
// Identify the responding node // Identify the responding node
if (wasBroadcast && mp.from != nodeDB->getNodeNum()) { if (wasBroadcast && mp.from != nodeDB->getNodeNum()) {
this->incoming = mp.from; // Relayed by another node this->incoming = mp.from; // relayed by another node
} else { } else {
this->incoming = this->lastSentNode; // Direct reply this->incoming = this->lastSentNode; // direct reply
} }
// Final ACK confirmation logic // Final ACK/NACK logic
this->ack = isAck && (wasBroadcast || isFromDest); if (wasBroadcast) {
// Any ACK counts for broadcast
this->ack = isAck;
waitingForAck = false; 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; // dont wait for more
} else {
// Explicit failure
this->ack = false;
waitingForAck = false;
}
// Update last sent StoredMessage with ACK/NACK result // Update last sent StoredMessage with ACK/NACK/RELAYED result
if (!messageStore.getMessages().empty()) { if (!messageStore.getMessages().empty()) {
StoredMessage &last = const_cast<StoredMessage &>(messageStore.getMessages().back()); StoredMessage &last = const_cast<StoredMessage &>(messageStore.getMessages().back());
if (last.sender == nodeDB->getNodeNum()) { // only update our own messages if (last.sender == nodeDB->getNodeNum()) { // only update our own messages
if (this->ack) { if (wasBroadcast && isAck) {
last.ackStatus = AckStatus::ACKED; last.ackStatus = AckStatus::ACKED;
} else if (isFromDest && isAck) {
last.ackStatus = AckStatus::ACKED;
} else if (!isFromDest && isAck) {
last.ackStatus = AckStatus::RELAYED;
} else { } else {
// If error_reason was explicit, you can map to NACKED; otherwise TIMEOUT last.ackStatus = AckStatus::NACKED;
last.ackStatus =
(decoded.error_reason == meshtastic_Routing_Error_NONE) ? AckStatus::ACKED : AckStatus::NACKED;
} }
} }
} }
// Capture radio metrics from this ACK/NACK packet // Capture radio metrics
this->lastRxRssi = mp.rx_rssi; this->lastRxRssi = mp.rx_rssi;
this->lastRxSnr = mp.rx_snr; this->lastRxSnr = mp.rx_snr;
// Show ACK/NACK as overlay banner // Show overlay banner
if (screen) { if (screen) {
graphics::BannerOverlayOptions opts; 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->ack) {
if (this->lastSentNode == NODENUM_BROADCAST) { if (this->lastSentNode == NODENUM_BROADCAST) {
snprintf(buf, sizeof(buf), "Broadcast sent to \n%s\n\nSNR: %.1f dB RSSI: %d dBm", snprintf(buf, sizeof(buf), "Message sent to\n#%s\n\nSNR: %.1f dB RSSI: %d dBm",
channels.getName(this->channel), this->lastRxSnr, this->lastRxRssi); (channelName && channelName[0]) ? channelName : "unknown", 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);
} else { } else {
snprintf(buf, sizeof(buf), "Delivered to %s\n\nSNR: \n%.1f dB RSSI: %d dBm", getNodeName(this->incoming), snprintf(buf, sizeof(buf), "DM sent to\n@%s\n\nSNR: %.1f dB RSSI: %d dBm",
this->lastRxSnr, this->lastRxRssi); (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 { } else {
snprintf(buf, sizeof(buf), "Delivery failed to \n%s", getNodeName(this->incoming), this->lastRxSnr, if (this->lastSentNode == NODENUM_BROADCAST) {
this->lastRxRssi); 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; opts.message = buf;

View File

@ -169,12 +169,9 @@ class CannedMessageModule : public SinglePortModule, public Observable<const UIF
bool ack = false; // True = ACK received, False = NACK or failed bool ack = false; // True = ACK received, False = NACK or failed
bool waitingForAck = false; // True if we're expecting an ACK and should monitor routing packets bool waitingForAck = false; // True if we're expecting an ACK and should monitor routing packets
bool lastAckWasRelayed = false; // True if the ACK was relayed through intermediate nodes
uint8_t lastAckHopStart = 0; // Hop start value from the received ACK packet
uint8_t lastAckHopLimit = 0; // Hop limit value from the received ACK packet
float lastRxSnr = 0; // SNR from last received ACK (used for diagnostics/UI) float lastRxSnr = 0; // SNR from last received ACK (used for diagnostics/UI)
int32_t lastRxRssi = 0; // RSSI from last received ACK (used for diagnostics/UI) int32_t lastRxRssi = 0; // RSSI from last received ACK (used for diagnostics/UI)
uint32_t lastRequestId = 0; // tracks the request_id of our last sent packet
// === State Tracking === // === State Tracking ===
cannedMessageModuleRunState runState = CANNED_MESSAGE_RUN_STATE_INACTIVE; cannedMessageModuleRunState runState = CANNED_MESSAGE_RUN_STATE_INACTIVE;