Skip to content

Commit

Permalink
Various NetworkTables fixes (#815)
Browse files Browse the repository at this point in the history
* Fix NT tab generation

Was relying on the main plugin's subscribe-all behavior

* Prioritize .type NT metadata events

Prevents data type issues where topic ordering is nondeterministic and type information could arrive after sources are set up

* Fit NT type lookup

Was based on NT table structures, which weren't returning correct data

* 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

* Linting

* Remove unnecessary `sendAll` from NT tab generator code

We don't write to the metadata topics, it's all sent over from the server
  • Loading branch information
SamCarlberg authored Nov 13, 2024
1 parent cdd5e7a commit 60e3ba1
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 25 deletions.
22 changes: 22 additions & 0 deletions api/src/main/java/edu/wpi/first/shuffleboard/api/util/FxUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!");
}
Expand Down Expand Up @@ -68,6 +81,15 @@ public static CompletableFuture<Boolean> 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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 + "/"});
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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public CompositeNetworkTableSource(String tableName, ComplexDataType<D> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import edu.wpi.first.networktables.NetworkTableInstance;
import edu.wpi.first.networktables.PubSubOption;

import java.util.Arrays;
import java.util.EnumSet;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -72,7 +73,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(
Expand Down Expand Up @@ -196,26 +209,21 @@ 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);
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));
}
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -81,7 +82,10 @@ public NetworkTableSourceType(NetworkTablesPlugin plugin) {
topic.getInfo(),
null,
null,
null));
null
),
true
);
}
}
});
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 60e3ba1

Please sign in to comment.