Skip to content
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

Add an option to run without NetworkTables #7

Merged
merged 14 commits into from
Sep 3, 2024

Conversation

spacey-sooty
Copy link
Contributor

depends on #6
closes #5

src/main/java/org/littletonrobotics/urcl/URCL.java Outdated Show resolved Hide resolved
src/main/native/cpp/URCL.cpp Outdated Show resolved Hide resolved
URCL.json Outdated Show resolved Hide resolved
publish.gradle Outdated Show resolved Hide resolved
src/main/java/org/littletonrobotics/urcl/URCL.java Outdated Show resolved Hide resolved
Copy link
Member

@jwbonner jwbonner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the new API. The only change to make is switching the persistent and alias fields to use update rather than append (for both Java and C++). Since they don't change often there's no point filling up the log unnecessarily (normally it would rely on NT to do change checking).

@spacey-sooty
Copy link
Contributor Author

spacey-sooty commented Aug 20, 2024

I like the new API. The only change to make is switching the persistent and alias fields to use update rather than append (for both Java and C++). Since they don't change often there's no point filling up the log unnecessarily (normally it would rely on NT to do change checking).

Update wont exist until we are on 2025 versions of WPILib I believe

Do we want to hold off on merging this till 2025 beta is released or merge this and change it later?

@jwbonner
Copy link
Member

jwbonner commented Aug 20, 2024

I believe we're building against the WPILib development builds, so update should exist already. We won't do a public release until 2025 beta anyway.

@spacey-sooty
Copy link
Contributor Author

I believe we're building against the WPILib development builds, so update should exist already. We won't do a public release until 2025 beta anyway.

We are building against WPILibs 2024 artifacts which are no longer development builds

@jwbonner
Copy link
Member

Right, yes. I think we should hold off merging this on the 2024 builds, but I also don't see any reason not to update URCL to the 2025 artifacts at this point. I can do that soon, or feel free to just add it to this PR if you prefer.

Copy link
Member

@jwbonner jwbonner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating to 2025. Java looks good, I just noticed a couple of odd things on the C++ version

src/main/native/cpp/URCL.cpp Outdated Show resolved Hide resolved
src/main/native/cpp/URCL.cpp Outdated Show resolved Hide resolved
src/main/native/cpp/URCL.cpp Outdated Show resolved Hide resolved
src/main/native/cpp/URCL.cpp Outdated Show resolved Hide resolved
@jwbonner jwbonner merged commit ce62274 into Mechanical-Advantage:main Sep 3, 2024
5 checks passed
@spacey-sooty spacey-sooty deleted the nont branch September 4, 2024 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to run without network tables
4 participants