From 62c9b8d01ad5dcb29bf20d234fa20ba20a6fcad1 Mon Sep 17 00:00:00 2001 From: Kevin Cooney Date: Sun, 7 Sep 2025 09:41:01 -0700 Subject: [PATCH 1/2] Store PersistedConfiguration record registry outside of /Preferences This change moves the NetworkTables keys which store which PersistedConfiguration names map to which record classes outside of /Preferences. As of the 2025 WPILib code, Preferences has a listener that updates all new topics to be persistent (for backwards compatibility of old dashboards; see https://github.com/wpilibsuite/allwpilib/commit/87fc49c66). Due to this, we cannot store data that should not be persistent under /Preferences without risking a race condition where the topic is later marked as persistent. --- .../lib2813/preferences/PersistedConfiguration.java | 8 +++++--- .../lib2813/preferences/PersistedConfigurationTest.java | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java b/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java index 0bbac58b..4db7a40d 100644 --- a/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java +++ b/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java @@ -95,6 +95,8 @@ * @since 2.0.0 */ public final class PersistedConfiguration { + static final String REGISTERED_CLASSES_NETWORK_TABLE_KEY = "PersistedConfiguration/registry"; + // The below package-scope fields are for the self-tests. static boolean throwExceptions = false; static Consumer errorReporter = DataLogManager::log; @@ -191,10 +193,11 @@ private static void verifyNotRegisteredToAnotherClass( recordName = recordClass.getName(); } - NetworkTable preferencesTable = ntInstance.getTable("Preferences"); - NetworkTableEntry entry = preferencesTable.getEntry(name + "/.registeredTo"); + NetworkTable registeredClassesTable = ntInstance.getTable(REGISTERED_CLASSES_NETWORK_TABLE_KEY); + NetworkTableEntry entry = registeredClassesTable.getEntry(name); if (!entry.exists()) { entry.setString(recordName); + entry.clearPersistent(); } else { String registeredTo = entry.getString(""); if (!recordName.equals(registeredTo)) { @@ -203,7 +206,6 @@ private static void verifyNotRegisteredToAnotherClass( "Preference with name '%s' already registered to %s", name, registeredTo)); } } - entry.clearPersistent(); } private static T createFromPreferences( diff --git a/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java b/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java index 91053644..a6f46f2e 100644 --- a/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java +++ b/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java @@ -2,6 +2,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.team2813.lib2813.preferences.PersistedConfiguration.REGISTERED_CLASSES_NETWORK_TABLE_KEY; import static java.util.stream.Collectors.toMap; import static org.junit.Assert.assertThrows; @@ -155,10 +156,10 @@ public void preferenceNameMapsToOnlyOneRecordType() { .hasMessageThat() .containsMatch("Preference with name '" + preferenceName + "' already registered"); - // Assert: .registeredTo topic added, and is not persistent + // Assert: topic added under "/PersistedConfiguration", and is not persistent NetworkTable table = - NetworkTableInstance.getDefault().getTable("Preferences").getSubTable(preferenceName); - NetworkTableEntry entry = table.getEntry(".registeredTo"); + NetworkTableInstance.getDefault().getTable(REGISTERED_CLASSES_NETWORK_TABLE_KEY); + NetworkTableEntry entry = table.getEntry(preferenceName); assertThat(entry.exists()).isTrue(); assertThat(entry.isPersistent()).isFalse(); assertThat(entry.getType()).isEqualTo(NetworkTableType.kString); From 031d4441d0ded0cf60a2846a997789e46400443a Mon Sep 17 00:00:00 2001 From: Kevin Cooney Date: Sun, 14 Sep 2025 17:55:04 -0700 Subject: [PATCH 2/2] Delete ".registeredTo" topics that were persisted under /Preferences --- .../preferences/PersistedConfiguration.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java b/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java index 4db7a40d..68491c3a 100644 --- a/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java +++ b/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java @@ -96,6 +96,7 @@ */ public final class PersistedConfiguration { static final String REGISTERED_CLASSES_NETWORK_TABLE_KEY = "PersistedConfiguration/registry"; + private static boolean deletedLegacyKeys = false; // The below package-scope fields are for the self-tests. static boolean throwExceptions = false; @@ -160,6 +161,7 @@ public static T fromPreferences(String preferenceName, Class< private static T fromPreferences( String preferenceName, Class recordClass, T configWithDefaults) { + deleteLegacyKeys(); NetworkTableInstance ntInstance = NetworkTableInstance.getDefault(); validatePreferenceName(preferenceName); verifyNotRegisteredToAnotherClass(ntInstance, preferenceName, recordClass); @@ -456,6 +458,20 @@ private static Supplier supplierFactory( return () -> factory.create(component, key, null, false); } + private static void deleteLegacyKeys() { + if (!deletedLegacyKeys) { + // Preferences installs a listener that makes all new topics persistent. The ".registeredTo" + // topics used to be placed under /Preferences, so could have been persisted. They are now + // written under a different top-level table. Delete the topics created by the previous code. + for (var key : Preferences.getKeys()) { + if (key.endsWith(".registeredTo")) { + Preferences.remove(key); + } + } + deletedLegacyKeys = true; + } + } + private static void warn(String format, Object... args) { String message = String.format("WARNING: " + format, args); errorReporter.accept(message);