From 9e57264361fb62ac23936a63f9688f1732503958 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Sat, 26 Oct 2024 13:24:52 -0400 Subject: [PATCH 1/6] Fix NT tab generation Was relying on the main plugin's subscribe-all behavior --- .../first/shuffleboard/plugin/networktables/TabGenerator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java index 2a067de9a..d0eb3341b 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java @@ -102,12 +102,12 @@ public void start() { }); metadataSubscriber = - new MultiSubscriber(inst, new String[] {METADATA_TABLE_NAME + "/"}, PubSubOption.hidden(true)); + new MultiSubscriber(inst, new String[] {METADATA_TABLE_NAME + "/"}, PubSubOption.sendAll(true)); metadataListener = inst.addListener( metadataSubscriber, EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate), this::metadataChanged); - dataSubscriber = new MultiSubscriber(inst, new String[] {ROOT_TABLE_NAME + "/"}, PubSubOption.hidden(true)); + dataSubscriber = new MultiSubscriber(inst, new String[] {ROOT_TABLE_NAME + "/"}); dataListener = inst.addListener( dataSubscriber, EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate), From 1ea78d74e975ec17e755acfd3c9aaf13c4c63b98 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Sat, 26 Oct 2024 13:28:06 -0400 Subject: [PATCH 2/6] Prioritize .type NT metadata events Prevents data type issues where topic ordering is nondeterministic and type information could arrive after sources are set up --- .../first/shuffleboard/api/util/FxUtils.java | 22 +++++++++++++++++++ .../sources/NetworkTableSourceType.java | 17 +++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java b/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java index a76114cf4..8d08af427 100644 --- a/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java +++ b/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java @@ -4,9 +4,14 @@ import edu.wpi.first.shuffleboard.api.widget.ParametrizedController; import java.io.IOException; +import java.time.Duration; +import java.time.temporal.ChronoUnit; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.function.Function; import javafx.application.Platform; @@ -38,6 +43,14 @@ public final class FxUtils { private static final Object FX_CONTROLLER_KEY = new Object(); + private static final ScheduledExecutorService laterScheduler = + Executors.newSingleThreadScheduledExecutor((runnable) -> { + var thread = new Thread(runnable); + thread.setDaemon(true); + thread.setName("Shuffleboard Run Later Scheduler"); + return thread; + }); + private FxUtils() { throw new UnsupportedOperationException("This is a utility class!"); } @@ -68,6 +81,15 @@ public static CompletableFuture runOnFxThread(Runnable task) { return future; } + /** + * Runs a task on the platform thread at some point in the future. + * @param task the task to run + * @param delay how far in the future the task should run + */ + public static void runLater(Runnable task, Duration delay) { + laterScheduler.schedule(() -> runOnFxThread(task), delay.get(ChronoUnit.NANOS), TimeUnit.NANOSECONDS); + } + /** * Binds a property to the value of an entry in a map. * diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSourceType.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSourceType.java index 5bd9a5078..ea8fd1e7e 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSourceType.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSourceType.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.List; +import java.time.Duration; import java.util.Map; import javafx.application.Platform; import javafx.collections.FXCollections; @@ -81,7 +82,10 @@ public NetworkTableSourceType(NetworkTablesPlugin plugin) { topic.getInfo(), null, null, - null)); + null + ), + true + ); } } }); @@ -93,7 +97,7 @@ public NetworkTableSourceType(NetworkTablesPlugin plugin) { EnumSet.of( NetworkTableEvent.Kind.kImmediate, NetworkTableEvent.Kind.kTopic), - event -> AsyncUtils.runAsync(() -> handleEvent(event))); + event -> AsyncUtils.runAsync(() -> handleEvent(event, true))); } @Override @@ -191,9 +195,16 @@ public String toUri(String sourceName) { return super.toUri(NetworkTable.normalizeKey(sourceName)); } - private void handleEvent(NetworkTableEvent event) { + private void handleEvent(NetworkTableEvent event, boolean allowDeferral) { final boolean delete = event.is(NetworkTableEvent.Kind.kUnpublish); final TopicInfo topicInfo = event.topicInfo; + + if (!topicInfo.name.endsWith("/.type") && allowDeferral) { + // Defer later to avoid race conditions where type metadata is not received first + FxUtils.runLater(() -> handleEvent(event, false), Duration.ofMillis(50)); + return; + } + if (topicInfo.name.endsWith("/.type") && !delete) { // Got type metadata for composite data // Remove trailing "/.type" From b636afc170e4b1b39e6c69419168f6be117fc230 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Sat, 26 Oct 2024 13:37:08 -0400 Subject: [PATCH 3/6] Fit NT type lookup Was based on NT table structures, which weren't returning correct data --- .../sources/NetworkTableSource.java | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java index 8a006278f..ce7c0c4c8 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java @@ -16,6 +16,7 @@ import edu.wpi.first.networktables.NetworkTableInstance; import edu.wpi.first.networktables.PubSubOption; +import java.util.Comparator; import java.util.EnumSet; import java.util.Map; import java.util.Optional; @@ -198,24 +199,19 @@ public static void removeAllCachedSources() { public static DataSource forKey(String fullTableKey) { String key = NetworkTable.normalizeKey(fullTableKey, false); final String uri = NetworkTableSourceType.getInstance().toUri(key); - if (NetworkTableUtils.rootTable.containsKey(key)) { - // Key-value pair - return sources.computeIfAbsent(uri, __ -> - new SingleKeyNetworkTableSource<>(NetworkTableUtils.rootTable, key, - NetworkTableUtils.dataTypeForEntry(key))); - } - if (NetworkTableUtils.rootTable.containsSubTable(key) || key.isEmpty()) { - // Composite - return sources.computeIfAbsent(uri, __ -> { - DataType lookup = NetworkTableUtils.dataTypeForEntry(key); - if (lookup == DataTypes.Unknown) { - // No known data type, fall back to generic map data - lookup = DataTypes.Map; - } + return sources.computeIfAbsent(uri, __ -> { + DataType lookup = NetworkTableUtils.dataTypeForEntry(key); + if (lookup == DataTypes.Unknown) { + // No known data type, fall back to generic map data + lookup = DataTypes.Map; + } + if (lookup.isComplex()) { return new CompositeNetworkTableSource<>(key, (ComplexDataType) lookup); - }); - } - return DataSource.none(); + } else { + return new SingleKeyNetworkTableSource<>(NetworkTableUtils.rootTable, key, + NetworkTableUtils.dataTypeForEntry(key)); + } + }); } /** From de08b0deed83de09e1bf10af702f2770c9dd6387 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Sat, 26 Oct 2024 15:21:12 -0400 Subject: [PATCH 4/6] Handle non-normalized topic names Shuffleboard historically relied on the assumption that a leading slash was optional for paths in networktables. NT4 changed that by making topic names exact (so two topic names only differing by the presence or absence of a leading slash would be two unique topics), which broke NT data sources for topics without leading slashes --- .../sources/CompositeNetworkTableSource.java | 2 +- .../sources/NetworkTableSource.java | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/CompositeNetworkTableSource.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/CompositeNetworkTableSource.java index 1276908d0..135757a4d 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/CompositeNetworkTableSource.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/CompositeNetworkTableSource.java @@ -47,7 +47,7 @@ public CompositeNetworkTableSource(String tableName, ComplexDataType dataType setData(dataType.getDefaultValue()); setTableListener((key, event) -> { - String relativeKey = NetworkTable.normalizeKey(key.substring(path.length() + 1), false); + String relativeKey = NetworkTable.basenameKey(key); if (event.is(NetworkTableEvent.Kind.kUnpublish)) { backingMap.remove(relativeKey); } else if (event.valueData != null) { diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java index ce7c0c4c8..c4560be46 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java @@ -16,6 +16,7 @@ import edu.wpi.first.networktables.NetworkTableInstance; import edu.wpi.first.networktables.PubSubOption; +import java.util.Arrays; import java.util.Comparator; import java.util.EnumSet; import java.util.Map; @@ -73,7 +74,19 @@ protected final void setTableListener(TableListener listener) { } setConnected(true); if (isSingular()) { - singleSub = inst.getTopic(fullTableKey).genericSubscribe(PubSubOption.hidden(false), PubSubOption.sendAll(true)); + // Handle leading slashes. Topic names are exact and do no normalization + String topicName; + if (Arrays.stream(inst.getTopicInfo()).anyMatch(t -> t.name.equals(fullTableKey))) { + topicName = fullTableKey; + } else { + if (fullTableKey.startsWith("/")) { + topicName = NetworkTable.normalizeKey(fullTableKey, false); + } else { + topicName = NetworkTable.normalizeKey(fullTableKey, true); + } + } + + singleSub = inst.getTopic(topicName).genericSubscribe(PubSubOption.hidden(false), PubSubOption.sendAll(true)); listenerUid = inst.addListener( singleSub, EnumSet.of( @@ -197,7 +210,7 @@ public static void removeAllCachedSources() { */ @SuppressWarnings("unchecked") public static DataSource forKey(String fullTableKey) { - String key = NetworkTable.normalizeKey(fullTableKey, false); + String key = fullTableKey; final String uri = NetworkTableSourceType.getInstance().toUri(key); return sources.computeIfAbsent(uri, __ -> { DataType lookup = NetworkTableUtils.dataTypeForEntry(key); From dcea59f92eecd0cc27b909b6b85b2cc023cfb44e Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Sat, 26 Oct 2024 15:25:31 -0400 Subject: [PATCH 5/6] Linting --- .../main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java | 2 +- .../plugin/networktables/sources/NetworkTableSource.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java b/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java index 8d08af427..0f6e129cc 100644 --- a/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java +++ b/api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java @@ -44,7 +44,7 @@ public final class FxUtils { private static final Object FX_CONTROLLER_KEY = new Object(); private static final ScheduledExecutorService laterScheduler = - Executors.newSingleThreadScheduledExecutor((runnable) -> { + Executors.newSingleThreadScheduledExecutor(runnable -> { var thread = new Thread(runnable); thread.setDaemon(true); thread.setName("Shuffleboard Run Later Scheduler"); diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java index c4560be46..c55d979f5 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/sources/NetworkTableSource.java @@ -17,7 +17,6 @@ import edu.wpi.first.networktables.PubSubOption; import java.util.Arrays; -import java.util.Comparator; import java.util.EnumSet; import java.util.Map; import java.util.Optional; From 5b4b3b9a2db6cc904ce6df6ae0715a8ff3e480ae Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Fri, 1 Nov 2024 19:30:58 -0400 Subject: [PATCH 6/6] Remove unnecessary `sendAll` from NT tab generator code We don't write to the metadata topics, it's all sent over from the server --- .../first/shuffleboard/plugin/networktables/TabGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java index d0eb3341b..1c8f25a8a 100644 --- a/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java +++ b/plugins/networktables/src/main/java/edu/wpi/first/shuffleboard/plugin/networktables/TabGenerator.java @@ -102,7 +102,7 @@ public void start() { }); metadataSubscriber = - new MultiSubscriber(inst, new String[] {METADATA_TABLE_NAME + "/"}, PubSubOption.sendAll(true)); + new MultiSubscriber(inst, new String[] {METADATA_TABLE_NAME + "/"}); metadataListener = inst.addListener( metadataSubscriber, EnumSet.of(NetworkTableEvent.Kind.kValueAll, NetworkTableEvent.Kind.kImmediate),