Skip to content

Commit

Permalink
[ntcore] Remove table multi-subscriber (wpilibsuite#4789)
Browse files Browse the repository at this point in the history
This wasn't well thought out, and leaks horribly in Java.

Reverts part of wpilibsuite#4640.
  • Loading branch information
PeterJohnson authored Dec 10, 2022
1 parent bde383f commit e6552d2
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public final class NetworkTable {
private final String m_path;
private final String m_pathWithSep;
private final NetworkTableInstance m_inst;
private final MultiSubscriber m_topicSub;

/**
* Gets the "base name" of a key. For example, "/foo/bar" becomes "bar". If the key has a trailing
Expand Down Expand Up @@ -115,8 +114,6 @@ public static List<String> getHierarchy(String key) {
m_path = path;
m_pathWithSep = path + PATH_SEPARATOR;
m_inst = inst;
m_topicSub =
new MultiSubscriber(inst, new String[] {m_pathWithSep}, PubSubOption.topicsOnly(true));
}

/**
Expand Down Expand Up @@ -533,7 +530,7 @@ public int addSubTableListener(SubTableListener listener) {
final NetworkTable parent = this;

return m_inst.addListener(
m_topicSub,
new String[] {m_pathWithSep},
EnumSet.of(NetworkTableEvent.Kind.kPublish, NetworkTableEvent.Kind.kImmediate),
new Consumer<NetworkTableEvent>() {
final Set<String> m_notifiedTables = new HashSet<>();
Expand Down Expand Up @@ -583,8 +580,4 @@ public boolean equals(Object other) {
public int hashCode() {
return Objects.hash(m_inst, m_path);
}

void close() {
m_topicSub.close();
}
}
13 changes: 4 additions & 9 deletions ntcore/src/main/native/cpp/networktables/NetworkTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,9 @@ std::vector<std::string> NetworkTable::GetHierarchy(std::string_view key) {

NetworkTable::NetworkTable(NT_Inst inst, std::string_view path,
const private_init&)
: m_inst(inst),
m_path(path),
m_topicSub{::nt::SubscribeMultiple(inst, {{fmt::format("{}/", path)}},
{{PubSubOption::TopicsOnly(true)}})} {}
: m_inst(inst), m_path(path) {}

NetworkTable::~NetworkTable() {
::nt::UnsubscribeMultiple(m_topicSub);
}
NetworkTable::~NetworkTable() = default;

NetworkTableInstance NetworkTable::GetInstance() const {
return NetworkTableInstance{m_inst};
Expand Down Expand Up @@ -405,8 +400,8 @@ NT_Listener NetworkTable::AddSubTableListener(SubTableListener listener) {
// a shared_ptr to it.
auto notified_tables = std::make_shared<wpi::StringMap<char>>();

return ::nt::AddListener(
m_topicSub, NT_EVENT_PUBLISH | NT_EVENT_IMMEDIATE,
return NetworkTableInstance{m_inst}.AddListener(
{{fmt::format("{}/", m_path)}}, NT_EVENT_PUBLISH | NT_EVENT_IMMEDIATE,
[this, cb = std::move(listener), notified_tables](const Event& event) {
auto topicInfo = event.GetTopicInfo();
if (!topicInfo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class NetworkTable final {
private:
NT_Inst m_inst;
std::string m_path;
NT_MultiSubscriber m_topicSub;
mutable wpi::mutex m_mutex;
mutable wpi::StringMap<NT_Entry> m_entries;

Expand Down

0 comments on commit e6552d2

Please sign in to comment.