Handle ROUTING_APP error response in onResponseTraceRoute#930
Open
NoctisSentinel wants to merge 3 commits into
Open
Handle ROUTING_APP error response in onResponseTraceRoute#930NoctisSentinel wants to merge 3 commits into
NoctisSentinel wants to merge 3 commits into
Conversation
Previously, receiving a ROUTING_APP packet in response to a traceroute would cause the function to attempt parsing it as a RouteDiscovery payload, resulting in a crash or silent failure followed by a timeout. This mirrors the error handling already present in onResponseTelemetry and onResponseWaypoint, but displays the specific errorReason instead of a hardcoded firmware version message, since traceroute is a diagnostic tool where the exact failure reason is more valuable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Meshtastic’s Python interface traceroute response handling to properly deal with ROUTING_APP error responses, avoiding mis-parsing routing errors as RouteDiscovery payloads and preventing misleading timeout behavior.
Changes:
- Added an early check in
onResponseTraceRouteforROUTING_APPpackets to detect traceroute failures. - Prints the specific
routing.errorReasonon traceroute failure and terminates the traceroute wait cleanly by settingreceivedTraceRoute.
Comment on lines
+690
to
+694
| if p["decoded"]["portnum"] == "ROUTING_APP": | ||
| error = p["decoded"]["routing"]["errorReason"] | ||
| print(f"Traceroute failed: {error}") | ||
| self._acknowledgment.receivedTraceRoute = True | ||
| return |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Thanks for the review! Added a unit test in the latest commit that feeds a |
ROUTING_APP with errorReason NONE is an ACK (success), not an error. Only print failure message for non-NONE error reasons. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
onResponseTraceRoutedid not handleROUTING_APPerror packets, causing it to attempt parsing them asRouteDiscoverypayloads, resulting in a crash or silent failure followed by a spurious timeout messageonResponseTraceRoutethat prints the specificerrorReasonand exits the wait loop cleanlyonResponseTelemetryandonResponseWaypointwhich show a hardcoded firmware version message, traceroute shows the actual error reason since it is a diagnostic tool where knowing the specific failure is more valuableTest plan
Traceroute failed: MAX_RETRANSMIT(or relevant error) without a spuriousTimed out waiting for traceroutemessage🤖 Generated with Claude Code