Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3452: only alter received packet if port number matches #3474

Merged
merged 2 commits into from
Mar 23, 2024

Conversation

GUVWAF
Copy link
Member

@GUVWAF GUVWAF commented Mar 23, 2024

Fixes #3452.

If enabled, the NeighborInfo module was altering a packet even if it wasn’t a NeighborInfo packet. This was introduced when wantPacket() was changed in PR #3321, because with hopStart we can also determine who are neighbors for any other packet.

To avoid this, I let the NeighborInfo module (and also Traceroute) now use alterReceivedProtobuf() instead of calling the methods in the FloodingRouter. To make sure alterReceived() of the Protobuf module does not call this with decoded set to NULL, I moved that into the clause which checks if the port number matches.
Tested that this fixes the decoding issues and it still works correctly for both the NeighborInfo module and Traceroute (also with hop in between).

Also removed some extensive logging from the NeighborInfo module since it would print your full NodeDB when sending a packet. Now it only prints your neighbors.

GUVWAF added 2 commits March 23, 2024 10:13
`alterReceived()` should never return NULL
Traceroute should be promiscuous
@thebentern
Copy link
Contributor

Very nice! It cleans up the code quite a bit to use that new alter method.

@thebentern thebentern merged commit d30d6bd into meshtastic:master Mar 23, 2024
69 checks passed
mverch67 pushed a commit that referenced this pull request Mar 24, 2024
* Use `alterReceivedProtobuf()` for NeighborInfo and Traceroute
`alterReceived()` should never return NULL
Traceroute should be promiscuous

* Remove extensive logging from NeighborInfo module
@GUVWAF GUVWAF deleted the decodingFix branch September 28, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Messages often come through garbled as "�떗" since recent update
2 participants