Fix: Traceroute through MQTT misses uplink node if MQTT is encrypted by domusonline · Pull Request #9798 · meshtastic/firmware

domusonline · 2026-03-03T12:26:01Z

This PR fixes #9713

As described in the bug, a traceroute that goes through MQTT server will not show the gateway node.
This is caused by the code in Router.cpp which saves the original image of the packet (before TraceRoute module changes it) and uses that image to send to the MQTT server.

The solution proposed works, but I don't find it the best due to these reasons:

It re-encrypts the packet which the code is already doing somewhere to send through RF. But I have no idea how to obtain a reference to that image back in Router.cpp
The code releases the original encrypted packet from packetPool. Then it allocates a copy from the non-encrypted packet (changed by the TraceRoute module) and tries to encrypt it. If something fails the reference to the packet is not valid.

The second issues caused a lot of "repeated ELSE" structures which look weird. But I couldn't find a better way to avoid calling mqtt::onSend with an invalid reference.

In favor of the fix it only applies to the specific situation where the packet is a traceroute. So the impact is rather low (just one or two IFs) for all the other cases.

I didn't try to avoid the release at the end of the method, because it is done even if the pointer is "nullptr" if I understood the code correctly.

I've tested this in the following way:

Compiled a firmware based on current develop branch with this change applied
I forced a trace though the node and verified that the uploaded MQTT message contained the node itself (issue fixed in practice)
I left the node running for a cumulative period of around two days to make sure there were no memory leaks or other unexpected failures. I had 100% uptime on that node, and being connected to the local MQTT server it processed and rebroadcastd thousands of packets.

With this in mind, I just feel there would be more elegant ways to solve this, but I'm pretty confident it works and will only change the behavior when a node connected to MQTT receives and processes a traceroute. I'd be happy if anyone else can provide a better solution, but if that takes time I think this is a viable solution.

Appreciated the support and feedback I had on Discord and local mesh community.

🤝 Attestations

I have tested that my proposed changes behave as described.
I have tested that my proposed changes do not cause any obvious regressions on the following devices:
- Heltec (Lora32) V3
- LilyGo T-Deck
- LilyGo T-Beam
- RAK WisBlock 4631
- Seeed Studio T-1000E tracker card
- Other (please specify below)
  Heltec WSL V3

github-actions · 2026-03-03T12:26:12Z

@domusonline, Welcome to Meshtastic!

Thanks for opening your first pull request. We really appreciate it.

We discuss work as a team in discord, please join us in the #firmware channel.
There's a big backlog of patches at the moment. If you have time,
please help us with some code review and testing of other PRs!

Welcome to the team 😄

CLAassistant · 2026-03-03T12:26:12Z

All committers have signed the CLA.

Copilot

Pull request overview

Fixes Meshtastic traceroute packets published via encrypted MQTT so the gateway node appears in the route (by ensuring the MQTT-encrypted payload reflects traceroute module in-place modifications).

Changes:

Rebuilds the encrypted MQTT copy for TRACEROUTE_APP packets after modules run, then re-encodes/encrypts it before publishing to MQTT.
Skips MQTT publish for traceroute packets if re-encryption fails.

domusonline · 2026-03-03T15:36:57Z

I agree with the co-pilot suggested changes. Some were considered but I tried to follow what has been done previously in other parts of the code.
This being my first PR it's not really clear to me if the next steps are on my side, or if this needs to wait for a "human" review which I think would be good as some of my concerns are beyond the AI observations.
Thanks for your attention.

GUVWAF · 2026-03-03T17:59:07Z

Thanks for your PR! Indeed it looks a bit cumbersome, but I think it's OK. We could also always re-encrypt it, but I think checking for the traceroute app is a nice optimization as encrypting can take a while.

I responded to the AI's comments and I think indeed two of them are worth implementing.

…shtastic-firmware into issues/issue-9713

domusonline · 2026-03-05T18:58:35Z

Thanks for your PR! Indeed it looks a bit cumbersome, but I think it's OK. We could also always re-encrypt it, but I think checking for the traceroute app is a nice optimization as encrypting can take a while.

I responded to the AI's comments and I think indeed two of them are worth implementing.

I updated the Router.cpp file with the following changes:
1- Added the condition to check that MQTT configuration has "encrypted" turned on. This will reduce the overload on cases where the MQTT server is not using encrypted messages
2- Checked the allocation of the new packet before releasing the original
3- Checked the encryption of the new packet before releasing the original
4- The code fallsback to sending the original if either 2) or 3) fails

I think it's more efficient, more resilient and looks a bit "cleaner" now.
I had some issues with git/commit (my fault for sure) which forced me to redo a new commit. But the differences in the file look good.
I tested the changes and left the node running for several hours to make sure it didn't show any memory issues.
Hope this is ok now. If not, please point the issues.
Thanks.

GUVWAF

Thanks for the effort!
Looks good to me.

fifieldt · 2026-03-05T20:45:26Z

@domusonline , are you familiar with the trunk code linter? We use it to keep all our formatting consistent.

domusonline · 2026-03-05T23:03:39Z

@domusonline , are you familiar with the trunk code linter? We use it to keep all our formatting consistent.

I installed it, tried to run it, but had some issues. Given the small ammount of lines changed I thought it followed the recommend formatting, but I noticed some automated test/validation apparently failed. I'm motivated to try more contributions and in case I do I'll make sure I clear the mentioned issues...

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…9798) * Attempt to fix issue 9713 * Code formatting issue. * Remade the fix to follow Copilot observations on PR * Rebuild after AI and GUVWAF * Update src/mesh/Router.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/mesh/Router.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * trunk fmt --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: GUVWAF <thijs@havinga.eu> Co-authored-by: GUVWAF <78759985+GUVWAF@users.noreply.github.com>

…eshtastic#9798) * Attempt to fix issue 9713 * Code formatting issue. * Remade the fix to follow Copilot observations on PR * Rebuild after AI and GUVWAF * Update src/mesh/Router.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/mesh/Router.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * trunk fmt --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: GUVWAF <thijs@havinga.eu> Co-authored-by: GUVWAF <78759985+GUVWAF@users.noreply.github.com>

domusonline added 3 commits March 2, 2026 11:31

Attempt to fix issue 9713

794170f

Code formatting issue.

5e6bf64

Merge branch 'meshtastic:develop' into issues/issue-9713

9e4fd7d

github-actions Bot added first-contribution needs-review Needs human review bugfix Pull request that fixes bugs labels Mar 3, 2026

thebentern requested a review from Copilot March 3, 2026 12:40

Copilot started reviewing on behalf of thebentern March 3, 2026 12:40 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Comment thread src/mesh/Router.cpp Outdated

Comment thread src/mesh/Router.cpp Outdated

Comment thread src/mesh/Router.cpp Outdated

thebentern requested a review from GUVWAF March 3, 2026 12:48

domusonline added 4 commits March 5, 2026 18:34

Remade the fix to follow Copilot observations on PR

2a0c579

Merge branch 'meshtastic:develop' into issues/issue-9713

b6c87f0

Merge branch 'issues/issue-9713' of https://github.com/domusonline/me…

935972c

…shtastic-firmware into issues/issue-9713

Rebuild after AI and GUVWAF

2cf8608

GUVWAF approved these changes Mar 5, 2026

View reviewed changes

fifieldt changed the title ~~Attempt to fix issue 9713~~ Fix: Traceroute through MQTT misses uplink node if MQTT is encrypted Mar 5, 2026

fifieldt approved these changes Mar 5, 2026

View reviewed changes

fifieldt removed the needs-review Needs human review label Mar 5, 2026

Merge branch 'develop' into issues/issue-9713

71bff04

thebentern requested a review from Copilot March 6, 2026 14:14

Copilot started reviewing on behalf of thebentern March 6, 2026 14:15 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Comment thread src/mesh/Router.cpp Outdated

Comment thread src/mesh/Router.cpp Outdated

Update src/mesh/Router.cpp

53db5d6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

domusonline and others added 4 commits March 6, 2026 15:17

Update src/mesh/Router.cpp

ebbbd37

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'develop' into issues/issue-9713

7db24d6

trunk fmt

f7937d9

Merge branch 'develop' into issues/issue-9713

21290df

fifieldt merged commit ac7a58c into meshtastic:develop Mar 16, 2026
77 checks passed

Uh oh!

Uh oh!

Conversation

domusonline commented Mar 3, 2026

🤝 Attestations

Uh oh!

github-actions Bot commented Mar 3, 2026

@domusonline, Welcome to Meshtastic!

Uh oh!

CLAassistant commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domusonline commented Mar 3, 2026

Uh oh!

GUVWAF commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

domusonline commented Mar 5, 2026

Uh oh!

GUVWAF left a comment

Choose a reason for hiding this comment

Uh oh!

fifieldt commented Mar 5, 2026

Uh oh!

domusonline commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Mar 3, 2026 •

edited

Loading

GUVWAF commented Mar 3, 2026 •

edited

Loading