Remove NT reconnect loop entirely#2398
Merged
mcm001 merged 3 commits intoPhotonVision:mainfrom Mar 16, 2026
Merged
Conversation
crschardt
reviewed
Mar 16, 2026
...-core/src/main/java/org/photonvision/common/dataflow/networktables/NetworkTablesManager.java
Outdated
Show resolved
Hide resolved
crschardt
approved these changes
Mar 16, 2026
Contributor
crschardt
left a comment
There was a problem hiding this comment.
Simply removes the reconnect loop. Based on my testing, this should eliminate the NT disconnect and failure to connect problems.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Back in #467 (comment) and https://discord.com/channels/725836368059826228/725846784131203222/974498049609056266 we added code to poke our NT client every 5 seconds to "clicking the save button in the settings window makes Photon show up again over networktables". Total hack, but it seemed to work. We didn't at the time dig any deeper in Wireshark or debug-level NT logs.
Now, it's 2026. 4 years on from the OG bug. And this code seems linked to these issues
Craig collected these log files as well:
craig-nt-never-connects.zip
The code path that handles TCP re-connection was also changed entirely since we first added this workaround. Regardless this hack was not removed as part of the NT3 to NT4 upgrade:
TCPConnector::connect_parallelwhich delegates toTCPConnector. This uses raw socket APIsParallelTcpConnector. This uses libuv exclusively@crschardt did some really great debugging with a rio and radio in the loop with a luma p1. In this test setup, removing this code improves stability markedly. I'd still like to get this more on robot time, as well as try to understand from Peter why we might have needed this code in the first place.
Changes
NetworkTablesInstance::isConnectedreturns false.Meta
Merge checklist: