Improve NT behavior

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

Hide file tree

Showing 9 changed files with 110 additions and 58 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;
@@ -33,10 +34,14 @@ public final class CameraServerSourceType extends SourceType {
   private final ObservableList<String> availableUris = FXCollections.observableArrayList();
   private final ObservableMap<String, Object> availableSources = FXCollections.observableHashMap();
 
+  private final MultiSubscriber subscriber;
+
   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));
+    inst.addListener(
+        subscriber,
         EnumSet.of(
           NetworkTableEvent.Kind.kUnpublish,
           NetworkTableEvent.Kind.kValueAll,

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,7 @@ 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);
+    startStopControlSub = ntInstance.getBooleanTopic(startStopKey).subscribe(false, PubSubOption.hidden(true));
     fileNameFormatSub =
-        ntInstance.getStringTopic(fileNameFormatKey).subscribe(Recorder.DEFAULT_RECORDING_FILE_NAME_FORMAT);
+        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,7 @@ 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 +86,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 +100,14 @@ 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 +120,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/.../edu/wpi/first/shuffleboard/plugin/networktables/sources/CompositeNetworkTableSource.java b/.../edu/wpi/first/shuffleboard/plugin/networktables/sources/CompositeNetworkTableSource.java
@@ -2,6 +2,7 @@
 
 import edu.wpi.first.shuffleboard.api.data.ComplexData;
 import edu.wpi.first.shuffleboard.api.data.ComplexDataType;
+import edu.wpi.first.shuffleboard.api.data.DataTypes;
 import edu.wpi.first.shuffleboard.api.data.IncompleteDataException;
 import edu.wpi.first.shuffleboard.api.sources.Sources;
 import edu.wpi.first.shuffleboard.plugin.networktables.util.NetworkTableUtils;

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;
@@ -33,6 +32,7 @@ public final class NetworkTableSourceType extends SourceType {
   private final ObservableList<String> availableSourceIds = FXCollections.observableArrayList();
   private final ObservableMap<String, Object> availableSources = FXCollections.observableHashMap();
   private final NetworkTablesPlugin plugin;
+  private final MultiSubscriber subscriber;
 
   @SuppressWarnings("JavadocMethod")
   public NetworkTableSourceType(NetworkTablesPlugin plugin) {
@@ -43,30 +43,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);
-            }
-          }
-        });
-      }
-    });
+    subscriber = new MultiSubscriber(inst, new String[] {""}, PubSubOption.hidden(true));
     inst.addListener(
-        new String[] {""},
+        subscriber,
         EnumSet.of(
           NetworkTableEvent.Kind.kImmediate,
           NetworkTableEvent.Kind.kTopic,

diff --git a/...src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/util/NetworkTableUtils.java b/...src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/util/NetworkTableUtils.java
@@ -39,6 +39,26 @@ public static String topicNameForEvent(NetworkTableEvent event) {
     }
   }
 
+  public static DataType dataTypeForTypeString(String typeString) {
+    if ("boolean".equals(typeString)) {
+      return DataTypes.Boolean;
+    } else if ("double".equals(typeString) || "int".equals(typeString) || "float".equals(typeString)) {
+      return DataTypes.Number;
+    } else if ("string".equals(typeString) || "json".equals(typeString)) {
+      return DataTypes.String;
+    } else if ("raw".equals(typeString)) {
+      return DataTypes.ByteArray;
+    } else if ("boolean[]".equals(typeString)) {
+      return DataTypes.BooleanArray;
+    } else if ("double[]".equals(typeString) || "int[]".equals(typeString) || "float[]".equals(typeString)) {
+      return DataTypes.NumberArray;
+    } else if ("string[]".equals(typeString)) {
+      return DataTypes.StringArray;
+    } else {
+      return DataTypes.Unknown;
+    }
+  }
+
   /**
    * Gets the data type most closely associated with the value of the given network table key.
    *
@@ -52,16 +72,18 @@ public static DataType dataTypeForEntry(String key) {
       return DataTypes.Map;
     }
     if (rootTable.containsKey(normalKey)) {
-      var ntValue = rootTable.getEntry(normalKey).getValue();
-      if (ntValue.isValid()) {
-        return DataTypes.getDefault().forJavaType(ntValue.getValue().getClass()).get();
-      } else {
-        return null;
-      }
+      return dataTypeForTypeString(rootTable.getTopic(normalKey).getTypeString());
     }
     if (rootTable.containsSubTable(normalKey)) {
       NetworkTable table = rootTable.getSubTable(normalKey);
-      String type = table.getEntry("~TYPE~").getString(table.getEntry(".type").getString(null));
+      String type;
+      if (table.containsKey("~TYPE~")) {
+        type = table.getEntry("~TYPE~").getString(null);
+      } else if (table.containsKey(".type")) {
+        type = table.getEntry(".type").getString(null);
+      } else {
+        return DataTypes.Map;
+      }
       if (type == null) {
         return DataTypes.Map;
       } else {