Skip to content

Fix NodeInfo suppression logic to ensure suppression only applies to external requests#9947

Merged
thebentern merged 2 commits into
masterfrom
fix-nodeinfo-broadcast
Mar 19, 2026
Merged

Fix NodeInfo suppression logic to ensure suppression only applies to external requests#9947
thebentern merged 2 commits into
masterfrom
fix-nodeinfo-broadcast

Conversation

@thebentern

Copy link
Copy Markdown
Contributor

Based on some logs, it seems that the suppression was influencing our own periodic NodeInfo broadcasts

@thebentern thebentern requested review from Xaositek and Copilot March 19, 2026 11:46
@github-actions github-actions Bot added the bugfix Pull request that fixes bugs label Mar 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts NodeInfo reply-suppression behavior so that suppression doesn’t accidentally block the device’s own periodic NodeInfo broadcasts, aligning suppression strictly with “replying to external requests”.

Changes:

  • Adds clarifying comment around the 12-hour suppression window.
  • Gates suppression in allocReply() so it only applies when replying to a non-local (external) request.
Comments suppressed due to low confidence (1)

src/modules/NodeInfoModule.cpp:45

  • The suppression flag is set for any promiscuously-sniffed packet with want_response, even when the packet is not addressed to us (in which case MeshModule will never call sendResponse() for this module). That makes suppressReplyForCurrentRequest represent state that might never be consumed and can leak into later sends. Consider only computing/updating suppression when the request is actually eligible for a reply (same toUs condition used in MeshModule::callModules: broadcast or isToUs(&mp)).
    if (mp.decoded.want_response) {
        const NodeNum sender = getFrom(&mp);
        const uint32_t now = mp.rx_time ? mp.rx_time : getTime();
        auto it = lastNodeInfoSeen.find(sender);
        if (it != lastNodeInfoSeen.end()) {
            uint32_t sinceLast = now >= it->second ? now - it->second : 0;
            if (sinceLast < NodeInfoReplySuppressSeconds) {
                suppressReplyForCurrentRequest = true;
            }
        }
        lastNodeInfoSeen[sender] = now;
        pruneLastNodeInfoCache();

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/modules/NodeInfoModule.cpp Outdated
@thebentern thebentern requested a review from Copilot March 19, 2026 12:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts NodeInfoModule suppression so it only suppresses replies to external NodeInfo requests, preventing suppression from affecting the device’s own periodic NodeInfo broadcasts.

Changes:

  • Avoids marking suppression based on packets originating from the local node (e.g., local/loopback sources).
  • Applies suppression only when allocReply() is servicing an external NodeInfo request (not periodic broadcasts).

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/modules/NodeInfoModule.cpp
@thebentern thebentern merged commit e4c5bfd into master Mar 19, 2026
68 of 74 checks passed
@thebentern thebentern deleted the fix-nodeinfo-broadcast branch March 26, 2026 11:35
jeek pushed a commit to jeek/Meshtastic-Exploiteers-Hacker-Pager that referenced this pull request Jun 30, 2026
…external requests (meshtastic#9947)

* Fix NodeInfo suppression logic to ensure suppression only applies to external requests

* Ensure NodeInfo reply suppression logic to only apply for external requests which are actually nodeinfo packets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants