[ntcore] Fix publish-triggered announcements being blocked by subscription announcements #8515
+10
−2
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.
Description
Fixes #7680
This PR fixes a bug where publish-triggered announcements (with
pubuid) were being blocked if a subscription-triggered announcement (withoutpubuid) was already sent to the same client for the same topic.Problem
According to the NetworkTables 4.1 specification, the server shall respond to a
publishmessage with anannouncemessage containing thepubuidprovided in the publish request. However, the current implementation had a bug where publish-triggered announcements were blocked in several scenarios:Prefix subscription scenario: If a client had a prefix subscription (e.g.,
"/"or"/MyTable") and published a topic matching that prefix:CreateTopiccall would triggerm_sendAnnounce, which broadcasts to subscribed clientspubuid), settingm_announceSent[topic] = trueClientPublishthen calledSendAnnounce(topic, pubuid), it was blocked becausem_announceSent[topic]was alreadytrueRetained topics after reconnection: When a client reconnects and republishes a retained topic:
Publishing without subscription: When a client publishes a topic without subscribing, the announcement should still be sent with the
pubuid.This violated the spec requirement that the server must respond to publish messages with an announcement containing the pubuid.
Solution
Modified
SendAnnouncein bothServerClient4andServerClientLocalto allow publish-triggered announcements (withpubuid) even if a subscription-triggered announcement was already sent. The logic now:pubuid)pubuid), ensuring spec complianceTesting
Tested locally using an example robot application (Java/WPILib) and client application (TypeScript) running on localhost:
Retained topic with prefix subscription scenario:
/MyTable/Pose(protobuf topic)/MyTable/and''for all topics)/MyTable/AutoModeas a retained topicannouncemessage withpubuidimmediately after callingpublish()on the retained topic, even though it already received subscription-triggered announcements for the topic due to the prefix subscriptionsRetained topic reconnection scenario:
/MyTable/AutoModeas retainedpubuidwithout timing outGeneral behavior verification:
announceresponse withpubuidThe fix ensures that publish-triggered announcements (with
pubuid) are always sent, even when subscription-triggered announcements have already been sent for the same topic.Related Issues
Closes #7680