Skip to content

[wpilibj] Add Preferences.getNetworkTable() #7962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 17, 2025

Conversation

kcooney
Copy link
Contributor

@kcooney kcooney commented May 8, 2025

Use case: Our team has a shared library that includes wrapper APIs for the
Java Preferences API. The wrapper API adds a topic (with a topic name starting
with ".") where it stores non-persistent data that is used for validation.

The code for our wrapper API is here.

The unit tests for those classes use Preferences.setNetworkTableInstance()
to ensure that the tests are isolated. Since there is no current API to get
the current preference table, our wrapper APIs either have to take in a
NetworkTableInstance (which makes the classes less simple to use) or our code
to add these internal topics needs to use the default network table instance (making the
tests not completely isolated).

Providing an API to get the NetworkTable used by Preferences would allow our
API to remain simple and our tests to be truly isolated. It doesn't appear
to be something that would be hard to maintain.

I reviewed the C++ and Python APIs, and they appear to not have an equivalent
to setNetworkTableInstance() so this PR only updates the Java API.

@kcooney kcooney requested a review from a team as a code owner May 8, 2025 19:07
@github-actions github-actions bot added the component: wpilibj WPILib Java label May 8, 2025
@kcooney kcooney changed the title Add NetworkTable.getNetworkTable() Add Preferences.getNetworkTable() May 8, 2025
@kcooney kcooney force-pushed the preferences_getNetworkTable branch from 6ab315c to 8308294 Compare May 8, 2025 19:09
@kcooney
Copy link
Contributor Author

kcooney commented May 21, 2025

@PeterJohnson would you be able to review this?

Thanks!

@kcooney kcooney changed the title Add Preferences.getNetworkTable() [wpilibj] Add Preferences.getNetworkTable() Jun 8, 2025
@kcooney
Copy link
Contributor Author

kcooney commented Jun 9, 2025

@PeterJohnson friendly ping

@PeterJohnson
Copy link
Member

Can you add this to C++ as well?

@kcooney
Copy link
Contributor Author

kcooney commented Jun 14, 2025

Can you add this to C++ as well?

@PeterJohnson I am not familiar with the C++ code. Is there a C++ equivalent of Preferences.setNetworkTableInstance()? If so, could you point to where it is defined?

I couldn't find one, so I didn't see the same need for a C++ equivalent of the API I am proposing (in the C++ code, preferences appear to be always stored and read relative to the root of the network table default instance).

Be happy to add Preferences::GetNetworkTable() if I am misunderstanding something.

@kcooney kcooney force-pushed the preferences_getNetworkTable branch from 8308294 to 4d2cf14 Compare June 16, 2025 16:30
@PeterJohnson PeterJohnson merged commit c655b7a into wpilibsuite:main Jun 17, 2025
44 checks passed
@kcooney kcooney deleted the preferences_getNetworkTable branch June 17, 2025 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants