Skip to content

Commit

Permalink
[ntcore] Use full handle when subscribing (#5013)
Browse files Browse the repository at this point in the history
Just using the index is insufficient because of Subscriber overlap with
MultiSubscriber.
  • Loading branch information
PeterJohnson authored Jan 27, 2023
1 parent 7cd30cf commit 1a47cc2
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
5 changes: 2 additions & 3 deletions ntcore/src/main/native/cpp/net/WireEncoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ bool nt::net::WireEncodeText(wpi::raw_ostream& os, const ClientMessage& msg) {
} else if (auto m = std::get_if<SetPropertiesMsg>(&msg.contents)) {
WireEncodeSetProperties(os, m->name, m->update);
} else if (auto m = std::get_if<SubscribeMsg>(&msg.contents)) {
WireEncodeSubscribe(os, Handle{m->subHandle}.GetIndex(), m->topicNames,
m->options);
WireEncodeSubscribe(os, m->subHandle, m->topicNames, m->options);
} else if (auto m = std::get_if<UnsubscribeMsg>(&msg.contents)) {
WireEncodeUnsubscribe(os, Handle{m->subHandle}.GetIndex());
WireEncodeUnsubscribe(os, m->subHandle);
} else {
return false;
}
Expand Down
5 changes: 3 additions & 2 deletions ntcore/src/test/native/cpp/net/WireEncoderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,15 @@ TEST_F(WireEncoderTextTest, MessageSubscribe) {
ASSERT_TRUE(net::WireEncodeText(os, msg));
ASSERT_EQ(os.str(),
"{\"method\":\"subscribe\",\"params\":{"
"\"options\":{},\"topics\":[\"a\",\"b\"],\"subuid\":5}}");
"\"options\":{},\"topics\":[\"a\",\"b\"],\"subuid\":402653189}}");
}

TEST_F(WireEncoderTextTest, MessageUnsubscribe) {
net::ClientMessage msg{
net::UnsubscribeMsg{Handle{0, 5, Handle::kSubscriber}}};
ASSERT_TRUE(net::WireEncodeText(os, msg));
ASSERT_EQ(os.str(), "{\"method\":\"unsubscribe\",\"params\":{\"subuid\":5}}");
ASSERT_EQ(os.str(),
"{\"method\":\"unsubscribe\",\"params\":{\"subuid\":402653189}}");
}

TEST_F(WireEncoderTextTest, MessageAnnounce) {
Expand Down

0 comments on commit 1a47cc2

Please sign in to comment.