Improve NT behavior (#796)

- Don't create individual subscriptions for every topic. - Use type string to determine data type.
wpilibsuite · Feb 17, 2024 · be4f85f · be4f85f
1 parent 54d89c6
commit be4f85f
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 62 deletions.
diff --git a/...in/java/edu/wpi/first/shuffleboard/plugin/cameraserver/source/CameraServerSourceType.java b/...in/java/edu/wpi/first/shuffleboard/plugin/cameraserver/source/CameraServerSourceType.java
@@ -10,10 +10,11 @@
 import edu.wpi.first.shuffleboard.plugin.cameraserver.data.CameraServerData;
 import edu.wpi.first.shuffleboard.plugin.cameraserver.data.type.CameraServerDataType;
 import edu.wpi.first.shuffleboard.plugin.networktables.util.NetworkTableUtils;
-
+import edu.wpi.first.networktables.MultiSubscriber;
 import edu.wpi.first.networktables.NetworkTable;
 import edu.wpi.first.networktables.NetworkTableEvent;
 import edu.wpi.first.networktables.NetworkTableInstance;
+import edu.wpi.first.networktables.PubSubOption;
 
 import java.util.EnumSet;
 import java.util.HashMap;
@@ -25,18 +26,23 @@
 import javafx.collections.ObservableMap;
 
 @UiHints(showConnectionIndicator = false) // same host as NetworkTables plugin, so no need to duplicate that information
-public final class CameraServerSourceType extends SourceType {
+public final class CameraServerSourceType extends SourceType implements AutoCloseable {
 
   public static final CameraServerSourceType INSTANCE = new CameraServerSourceType();
 
   private final Map<String, CameraServerSource> sources = new HashMap<>();
   private final ObservableList<String> availableUris = FXCollections.observableArrayList();
   private final ObservableMap<String, Object> availableSources = FXCollections.observableHashMap();
 
+  private final MultiSubscriber subscriber;
+  private final int listener;
+
   private CameraServerSourceType() {
     super("CameraServer", true, "camera_server://", CameraServerSourceType::forName);
-    NetworkTableInstance.getDefault().addListener(
-        new String[] {"/CameraPublisher"},
+    NetworkTableInstance inst = NetworkTableInstance.getDefault();
+    subscriber = new MultiSubscriber(inst, new String[] {"/CameraPublisher"}, PubSubOption.hidden(true));
+    listener = inst.addListener(
+        subscriber,
         EnumSet.of(
           NetworkTableEvent.Kind.kUnpublish,
           NetworkTableEvent.Kind.kValueAll,
@@ -61,6 +67,12 @@ private CameraServerSourceType() {
           }));
   }
 
+  @Override
+  public void close() {
+    subscriber.close();
+    NetworkTableInstance.getDefault().removeListener(listener);
+  }
+
   public static CameraServerSource forName(String name) {
     return INSTANCE.sources.computeIfAbsent(name, CameraServerSource::new);
   }

diff --git a/...src/main/java/edu/wpi/first/shuffleboard/plugin/cameraserver/source/StreamDiscoverer.java b/...src/main/java/edu/wpi/first/shuffleboard/plugin/cameraserver/source/StreamDiscoverer.java
@@ -7,6 +7,7 @@
 
 import edu.wpi.first.networktables.NetworkTable;
 import edu.wpi.first.networktables.NetworkTableEvent;
+import edu.wpi.first.networktables.PubSubOption;
 import edu.wpi.first.networktables.StringArraySubscriber;
 
 import java.util.EnumSet;
@@ -33,7 +34,8 @@ public final class StreamDiscoverer implements AutoCloseable {
    * @param cameraName     the name of the camera to discover streams for
    */
   public StreamDiscoverer(NetworkTable publisherTable, String cameraName) {
-    streamsSub = publisherTable.getSubTable(cameraName).getStringArrayTopic(STREAMS_KEY).subscribe(emptyStringArray);
+    streamsSub = publisherTable.getSubTable(cameraName).getStringArrayTopic(STREAMS_KEY)
+        .subscribe(emptyStringArray, PubSubOption.hidden(true));
     streamsListener = publisherTable.getInstance().addListener(
         streamsSub,
         EnumSet.of(

diff --git a/...tables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/MarkerGenerator.java b/...tables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/MarkerGenerator.java
@@ -2,10 +2,11 @@
 
 import edu.wpi.first.shuffleboard.api.sources.recording.MarkerImportance;
 import edu.wpi.first.shuffleboard.api.sources.recording.Recorder;
-
+import edu.wpi.first.networktables.MultiSubscriber;
 import edu.wpi.first.networktables.NetworkTable;
 import edu.wpi.first.networktables.NetworkTableEvent;
 import edu.wpi.first.networktables.NetworkTableInstance;
+import edu.wpi.first.networktables.PubSubOption;
 
 import java.util.Arrays;
 import java.util.EnumSet;
@@ -42,6 +43,7 @@ final class MarkerGenerator {
   private final NetworkTableInstance inst;
   private final Recorder recorder;
 
+  private MultiSubscriber subscriber;
   private int listenerHandle = 0;
 
   MarkerGenerator(NetworkTableInstance inst, Recorder recorder) {
@@ -50,12 +52,14 @@ final class MarkerGenerator {
   }
 
   public void start() {
-    listenerHandle = inst.addListener(new String[] {EVENT_TABLE_NAME},
+    subscriber = new MultiSubscriber(inst, new String[] {EVENT_TABLE_NAME}, PubSubOption.hidden(true));
+    listenerHandle = inst.addListener(subscriber,
         EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate), this::handleMarkerEvent);
   }
 
   public void stop() {
     inst.removeListener(listenerHandle);
+    subscriber.close();
   }
 
   private void handleMarkerEvent(NetworkTableEvent event) {

diff --git a/...les/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/RecorderController.java b/...les/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/RecorderController.java
@@ -8,6 +8,7 @@
 import edu.wpi.first.networktables.BooleanSubscriber;
 import edu.wpi.first.networktables.NetworkTableEvent;
 import edu.wpi.first.networktables.NetworkTableInstance;
+import edu.wpi.first.networktables.PubSubOption;
 import edu.wpi.first.networktables.StringSubscriber;
 
 /**
@@ -55,9 +56,9 @@ public RecorderController(NetworkTableInstance ntInstance,
                             String startStopKey,
                             String fileNameFormatKey,
                             Recorder recorder) {
-    startStopControlSub = ntInstance.getBooleanTopic(startStopKey).subscribe(false);
-    fileNameFormatSub =
-        ntInstance.getStringTopic(fileNameFormatKey).subscribe(Recorder.DEFAULT_RECORDING_FILE_NAME_FORMAT);
+    startStopControlSub = ntInstance.getBooleanTopic(startStopKey).subscribe(false, PubSubOption.hidden(true));
+    fileNameFormatSub = ntInstance.getStringTopic(fileNameFormatKey)
+        .subscribe(Recorder.DEFAULT_RECORDING_FILE_NAME_FORMAT, PubSubOption.hidden(true));
     this.recorder = recorder;
     this.markerGenerator = new MarkerGenerator(ntInstance, recorder);
   }

diff --git a/...orktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java b/...orktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java
@@ -13,7 +13,7 @@
 import edu.wpi.first.shuffleboard.api.widget.Components;
 import edu.wpi.first.shuffleboard.api.widget.TileSize;
 import edu.wpi.first.shuffleboard.plugin.networktables.sources.NetworkTableSource;
-
+import edu.wpi.first.networktables.MultiSubscriber;
 import edu.wpi.first.networktables.NetworkTable;
 import edu.wpi.first.networktables.NetworkTableEvent;
 import edu.wpi.first.networktables.NetworkTableInstance;
@@ -59,7 +59,9 @@ final class TabGenerator {
   private int tabsListener;
   private StringSubscriber tabSelectionSubscriber;
   private int tabSelectionListener;
+  private MultiSubscriber metadataSubscriber;
   private int metadataListener;
+  private MultiSubscriber dataSubscriber;
   private int dataListener;
   private final Components componentRegistry;
 
@@ -74,7 +76,8 @@ final class TabGenerator {
   public void start() {
     // Make sure all tabs exist if they're defined, even if they're empty
     NetworkTable rootMetaTable = inst.getTable(METADATA_TABLE_NAME);
-    tabsSubscriber = rootMetaTable.getStringArrayTopic(TABS_ENTRY_KEY).subscribe(new String[] {});
+    tabsSubscriber = rootMetaTable.getStringArrayTopic(TABS_ENTRY_KEY)
+        .subscribe(new String[] {}, PubSubOption.hidden(true));
     tabsListener = inst.addListener(tabsSubscriber,
       EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate), event -> {
         for (String tabName : event.valueData.value.getStringArray()) {
@@ -84,7 +87,7 @@ public void start() {
       });
 
     tabSelectionSubscriber = rootMetaTable.getStringTopic(SELECTED_ENTRY_NAME)
-            .subscribe("", PubSubOption.keepDuplicates(true));
+            .subscribe("", PubSubOption.keepDuplicates(true), PubSubOption.hidden(true));
     tabSelectionListener = inst.addListener(
         tabSelectionSubscriber,
         EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate),
@@ -98,12 +101,15 @@ public void start() {
             }
         });
 
+    metadataSubscriber =
+        new MultiSubscriber(inst, new String[] {METADATA_TABLE_NAME + "/"}, PubSubOption.hidden(true));
     metadataListener = inst.addListener(
-        new String[] {METADATA_TABLE_NAME + "/"},
+        metadataSubscriber,
         EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate),
         this::metadataChanged);
+    dataSubscriber = new MultiSubscriber(inst, new String[] {ROOT_TABLE_NAME + "/"}, PubSubOption.hidden(true));
     dataListener = inst.addListener(
-        new String[] {ROOT_TABLE_NAME + "/"},
+        dataSubscriber,
         EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate),
         this::dataChanged);
   }
@@ -116,7 +122,9 @@ public void stop() {
     inst.removeListener(tabsListener);
     tabSelectionSubscriber.close();
     inst.removeListener(tabSelectionListener);
+    metadataSubscriber.close();
     inst.removeListener(metadataListener);
+    dataSubscriber.close();
     inst.removeListener(dataListener);
   }
 

diff --git a/...main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java b/...main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java
@@ -8,10 +8,12 @@
 import edu.wpi.first.shuffleboard.api.sources.Sources;
 import edu.wpi.first.shuffleboard.api.util.AsyncUtils;
 import edu.wpi.first.shuffleboard.plugin.networktables.util.NetworkTableUtils;
-
+import edu.wpi.first.networktables.GenericSubscriber;
+import edu.wpi.first.networktables.MultiSubscriber;
 import edu.wpi.first.networktables.NetworkTable;
 import edu.wpi.first.networktables.NetworkTableEvent;
 import edu.wpi.first.networktables.NetworkTableInstance;
+import edu.wpi.first.networktables.PubSubOption;
 
 import java.util.EnumSet;
 import java.util.Map;
@@ -31,6 +33,8 @@ public abstract class NetworkTableSource<T> extends AbstractDataSource<T> {
   private static final Map<String, NetworkTableSource> sources = new ConcurrentHashMap<>();
 
   protected final String fullTableKey;
+  private MultiSubscriber multiSub;
+  private GenericSubscriber singleSub;
   private int listenerUid = -1;
   private volatile boolean ntUpdate = false;
 
@@ -56,24 +60,46 @@ protected NetworkTableSource(String fullTableKey, DataType<T> dataType) {
    */
   protected final void setTableListener(TableListener listener) {
     NetworkTableInstance inst = NetworkTableInstance.getDefault();
-    inst.removeListener(listenerUid);
+    if (listenerUid != -1) {
+      inst.removeListener(listenerUid);
+    }
+    if (multiSub != null) {
+      multiSub.close();
+    }
+    if (singleSub != null) {
+      singleSub.close();
+    }
     setConnected(true);
-    listenerUid = inst.addListener(
-        new String[] {fullTableKey},
+    if (isSingular()) {
+      singleSub = inst.getTopic(fullTableKey).genericSubscribe(PubSubOption.hidden(true));
+      listenerUid = inst.addListener(
+        singleSub,
         EnumSet.of(
           NetworkTableEvent.Kind.kImmediate,
           NetworkTableEvent.Kind.kTopic,
           NetworkTableEvent.Kind.kValueAll),
         event -> {
-          String name = NetworkTableUtils.topicNameForEvent(event);
-          if (isSingular() && !name.equals(fullTableKey)) {
-            // Since NetworkTableInstance.addEntryListener() will fire on anything that starts with the key,
-            // a singular source will be notified for an unrelated entry.
-            // For example, a singular source for the entry "/S" will also be fired for any changing entry that
-            // starts with "/S", such as "/SmartDashboard/<anything>" or "/SomeUnrelatedEntry".
-            // This check prevents the source from being erroneously updated for an unrelated entry.
-            return;
+          if (isConnected()) {
+            AsyncUtils.runAsync(() -> {
+              try {
+                ntUpdate = true;
+                listener.onChange(fullTableKey, event);
+              } finally {
+                ntUpdate = false;
+              }
+            });
           }
+        });
+    } else {
+      multiSub = new MultiSubscriber(inst, new String[] {fullTableKey}, PubSubOption.hidden(true));
+      listenerUid = inst.addListener(
+        multiSub,
+        EnumSet.of(
+          NetworkTableEvent.Kind.kImmediate,
+          NetworkTableEvent.Kind.kTopic,
+          NetworkTableEvent.Kind.kValueAll),
+        event -> {
+          String name = NetworkTableUtils.topicNameForEvent(event);
           if (isConnected()) {
             AsyncUtils.runAsync(() -> {
               try {
@@ -85,6 +111,7 @@ protected final void setTableListener(TableListener listener) {
             });
           }
         });
+    }
   }
 
   /**
@@ -117,6 +144,12 @@ public void close() {
     setActive(false);
     setConnected(false);
     NetworkTableInstance.getDefault().removeListener(listenerUid);
+    if (multiSub != null) {
+      multiSub.close();
+    }
+    if (singleSub != null) {
+      singleSub.close();
+    }
     Sources.getDefault().unregister(this);
     sources.remove(getId());
   }

diff --git a/.../java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSourceType.java b/.../java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSourceType.java
@@ -8,15 +8,14 @@
 import edu.wpi.first.shuffleboard.api.sources.Sources;
 import edu.wpi.first.shuffleboard.api.sources.recording.TimestampedData;
 import edu.wpi.first.shuffleboard.api.util.AsyncUtils;
-import edu.wpi.first.shuffleboard.api.util.FxUtils;
 import edu.wpi.first.shuffleboard.plugin.networktables.NetworkTablesPlugin;
 import edu.wpi.first.shuffleboard.plugin.networktables.util.NetworkTableUtils;
-
+import edu.wpi.first.networktables.MultiSubscriber;
 import edu.wpi.first.networktables.NetworkTable;
 import edu.wpi.first.networktables.NetworkTableEntry;
 import edu.wpi.first.networktables.NetworkTableEvent;
 import edu.wpi.first.networktables.NetworkTableInstance;
-import edu.wpi.first.networktables.Topic;
+import edu.wpi.first.networktables.PubSubOption;
 
 import java.util.EnumSet;
 import java.util.List;
@@ -26,13 +25,15 @@
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableMap;
 
-public final class NetworkTableSourceType extends SourceType {
+public final class NetworkTableSourceType extends SourceType implements AutoCloseable {
 
   private static NetworkTableSourceType instance;
 
   private final ObservableList<String> availableSourceIds = FXCollections.observableArrayList();
   private final ObservableMap<String, Object> availableSources = FXCollections.observableHashMap();
   private final NetworkTablesPlugin plugin;
+  private final MultiSubscriber subscriber;
+  private final int listener;
 
   @SuppressWarnings("JavadocMethod")
   public NetworkTableSourceType(NetworkTablesPlugin plugin) {
@@ -43,30 +44,9 @@ public NetworkTableSourceType(NetworkTablesPlugin plugin) {
     plugin.serverIdProperty().addListener((__, old, serverId) -> setConnectionStatus(serverId, false));
     inst.addConnectionListener(true,
         event -> setConnectionStatus(plugin.getServerId(), event.is(NetworkTableEvent.Kind.kConnected)));
-    inst.addConnectionListener(false, event -> {
-      if (event.is(NetworkTableEvent.Kind.kDisconnected)) {
-        FxUtils.runOnFxThread(() -> {
-          availableSources.clear();
-          availableSourceIds.clear();
-          NetworkTableSource.removeAllCachedSources();
-          Sources.getDefault().forType(NetworkTableSourceType.instance).forEach(Sources.getDefault()::unregister);
-        });
-      } else if (event.is(NetworkTableEvent.Kind.kConnected)) {
-        FxUtils.runOnFxThread(() -> {
-          for (Topic topic : event.getInstance().getTopics()) {
-            String uri = toUri(topic.getName());
-            if (!availableSources.containsKey(uri)) {
-              availableSources.put(uri, topic.genericSubscribe().get().getValue());
-            }
-            if (!availableSourceIds.contains(uri)) {
-              availableSourceIds.add(uri);
-            }
-          }
-        });
-      }
-    });
-    inst.addListener(
-        new String[] {""},
+    subscriber = new MultiSubscriber(inst, new String[] {""}, PubSubOption.hidden(true));
+    listener = inst.addListener(
+        subscriber,
         EnumSet.of(
           NetworkTableEvent.Kind.kImmediate,
           NetworkTableEvent.Kind.kTopic,
@@ -98,6 +78,12 @@ public NetworkTableSourceType(NetworkTablesPlugin plugin) {
         });
   }
 
+  @Override
+  public void close() {
+    subscriber.close();
+    NetworkTableInstance.getDefault().removeListener(listener);
+  }
+
   private void setConnectionStatus(String serverId, boolean connected) {
     Platform.runLater(() -> {
       String host;