NetworkTables needs to change.

[virtuald]

I agree that NetworkTables has been a problem for awhile, and that the code quality isn't what it should have been -- I should know, I've spent a ton of time in the code, and have found/fixed a number of deadlocks and race conditions in it. However, the ease of use it provides is great, and the tools that have been created around it are quite useful.

I haven't seen the rewritten code yet, but I believe the one doing it is my Python co-collaborator and I have great faith that it'll get done right this time.

I strongly disagree that it should be moved to UDP. There's a reason TCP exists -- namely, it takes care of retransmission/etc for you, so you can depend (ish) that the data you sent will eventually get there (with lots of caveats on this, of course). Coupled with an transmit-on-update-only scheme like NetworkTables uses, this works out great, particularly for large tables or for variables that don't get updated often (which for our usage, is over half of them).

In UDP, if a packet gets dropped, then you have to either implement the retransmission yourself, or you need to ensure that the data gets transmitted often enough that it doesn't matter that you lose the packets.

Off the top of my head, there are two ways you can deal with this in UDP. Send all the data in every packet (which for large tables, would be bad) or ensure that the data gets sent often (which is still bad, doesn't adequately deal with the retransmit problem, and for things that don't get updated often like boolean flags, is annoying to implement). Neither of these sound like attractive options to me.

The internal 100ms (I set it to 50ms on python) wait of NetworkTables is not necessarily a *performance* problem, it's a latency problem. And honestly, that amount of latency doesn't matter 99% of the time if you design your code appropriately. In the cases that it does matter (like the DS packets), then yes, this scheme doesn't work. It would be useful to have a low-latency option built in (maybe by using TCP_NODELAY properly), but I suspect it would be overused by teams.

One other reason that the poll loop exists is that it's very important to decouple network latency from your robot's main loop. Transmitting a packet on the network (even in UDP, particularly on the cRio, not so much on the RoboRIO) can take a non-trivial amount of time compared to the rest of things that you do in your robot's main loop -- particularly if your wireless connection has interference in it -- so sending the packets directly from your robot's main loop is a bad idea. They chose to decouple this with a separate transmit thread and a queue, but it could certainly be implemented more efficiently with an epoll-like event-based mechanism if desired. In fact, many of the performance/deadlock problems NT had initially (and, are still there to some degree) are related to improper locking that led to the main loop waiting for network packets to transmit.

[calcmogul]

Originally Posted by virtuald

I haven't seen the rewritten code yet, but I believe the one doing it is my Python co-collaborator and I have great faith that it'll get done right this time.

The repository for the new NetworkTables code is located at https://github.com/PeterJohnson/ntcore. It's included in WPILib as a Git submodule.

[JesseK]

Originally Posted by mathmogul

The repository for the new NetworkTables code is located at https://github.com/PeterJohnson/ntcore. It's included in WPILib as a Git submodule.

Do you know if there is a min / max supported Java version with this? i.e. Java 8 only?

[calcmogul]

Originally Posted by JesseK

Do you know if there is a min / max supported Java version with this? i.e. Java 8 only?

I'm not sure. WPILib uses Java 8 exclusively so that's all we've bothered to test. Peter could give a definitive answer.

[Peter Johnson]

Originally Posted by mathmogul

I'm not sure. WPILib uses Java 8 exclusively so that's all we've bothered to test. Peter could give a definitive answer.

I'll try to respond to some of the other messages in this thread later this evening, but first, a couple of quick notes.

I know of only one definite Java 8 dependency in the new NT, and I just committed it a couple days ago: I added more advanced notification features but I didn't want to break backwards compatibility with existing Java code, so I added a "default" implementation of the new interface function to ITableListener. Having a "default" like this is unfortunately a Java 8 feature, but this should be easy to work around for previous versions if you don't care about backwards compatibility. Is there real interest out there for older Java versions?

That being said, I've not actually tested it with older Java versions, so it's possible there's another dependency, but the new Java code is a thin wrapper around a JNI interface to the C++ library, so there's only 5 or 6 Java files (and only one real "implementation" class).

Java and C++ will both use the new C++ implementation, but it's still up in the air whether LabView will use the C++ library or continue using a LabView-native implementation (with the same protocol improvements).

Please note there are a few API changes in the library that required updates to wpilib code, which means it won't work "out of the box" with last year's wpilib (the C++ version I know has API breakage; I need to double-check, but the Java API should have minimal to no breakage).

It's worth noting the new C++ implementation is interoperable with old clients and servers, with the only downside being you won't have access to the new features from the older clients/servers (of course).

[JesseK]

I don't have an interest in older Java versions - just wanted to know what to look for when profiling the JNI stuff.

[Thad House]

Originally Posted by Peter Johnson

Java and C++ will both use the new C++ implementation, but it's still up in the air whether LabView will use the C++ library or continue using a LabView-native implementation (with the same protocol improvements).

I would hope that LabVIEW chooses to go with the new library. Having only 1 version of the communication protocol makes debugging MUCH easier. It's also not that hard to port, and would probably only take a day or 2 if somebody works on it full time.

1 other great change this allows is adding new languages easily. Since all the comms code is in the native library, as long as your language can somehow interop into the C library, all you need is a very thin wrapper to gain full functionality. When I ported the old NetworkTables to C#, it was not easy, especially dealing with the endian issues. Porting the new library was much much easier, took less then a day, and you only had to write interop code and not communication code.

[Tom Line]

Originally Posted by Thad House

I would hope that LabVIEW chooses to go with the new library. Having only 1 version of the communication protocol makes debugging MUCH easier. It's also not that hard to port, and would probably only take a day or 2 if somebody works on it full time.

1 other great change this allows is adding new languages easily. Since all the comms code is in the native library, as long as your language can somehow interop into the C library, all you need is a very thin wrapper to gain full functionality. When I ported the old NetworkTables to C#, it was not easy, especially dealing with the endian issues. Porting the new library was much much easier, took less then a day, and you only had to write interop code and not communication code.

I'm going to second this. We had a real headache with network tables this year. The driver station received data just fine, but trying to send data to the robot was hit-and-miss.

[fovea1959]

Originally Posted by virtuald

I haven't seen the rewritten code yet, but I believe the one doing it is my Python co-collaborator and I have great faith that it'll get done right this time.

Is there a strong possibility that pynetworktables2js will have appropriate changes made to keep working? I'm thinking about having the team move to that for dashboards....

[virtuald]

Originally Posted by fovea1959

Is there a strong possibility that pynetworktables2js will have appropriate changes made to keep working? I'm thinking about having the team move to that for dashboards....

I've been told that the new version of networktables will be backwards compatible with the old, so even if I did zero work, it would work.

However, I'm sure it'll get changes to take advantage of any new features.

[fovea1959]

Originally Posted by virtuald

I've been told that the new version of networktables will be backwards compatible with the old, so even if I did zero work, it would work.

However, I'm sure it'll get changes to take advantage of any new features.

from what I've seen so far, I'm not at all surprised at that.

any ideas of what's changing (what additional functionality we'll see)?

[Peter Johnson]

Originally Posted by fovea1959

from what I've seen so far, I'm not at all surprised at that.

any ideas of what's changing (what additional functionality we'll see)?

FYI, I'm the person rewriting/updating C++/Java NetworkTables for 2016. The short list of new features include: value persistence (basically improved preferences; network tables will save/load changed flagged values automatically to an .ini file on the server), server and client self-identification (so it's possible for the server to more easily figure out what clients are connected in a dynamic IP environment), entry deletion, remote procedure calls, server reboot detection, and "raw" values (distinct from strings). There have also been minor improvements made to the underlying wire protocol, such as changing the way string lengths are encoded. A full list can be found at https://github.com/PeterJohnson/ntco...rom-2-0-to-3-0. A couple of these features (namely RPC) have not yet been fully exposed to the Java/C++ "NetworkTable" class even though they're implemented in the core library, but I expect to finish adding them this fall.

One change that did not make it into the new protocol version was a broader protocol-level approach to improve synchronization on reconnect (namely the issue where you need to shut down every client and server to start "afresh"). We were discussing a number of ideas for this but ran out of time to make a change this year. I'm still mulling over whether there's a good approach to use server reboot detection to make a smarter decision here without doing something fancier at the protocol level.