I recently had to debug a problem on our robot where Preferences weren’t loading reliably at startup. It turned out that we had a lot of subsystems loading as static constructors in the Robot class. When I moved them all into robotInit(), the problems went away.
But I’m still leery.
Q1: Are there any guarantees in the framework that Preferences and NetworkTables will be fully loaded and stable before robotInit is called?
Q2: If Preferences and NetworkTables are unsafe to use before robotInit, why are there no protections built into them so they can report this type of unsafe usage? e.g., why don’t they throw during getInstance() if the service is not ready. The current behavior of silent failure is actually much worse than dealing with an exception.
Yes. The RobotBase constructor’s call to startServer() loads the persistent file. The RobotBase constructor is called prior to your robot class constructor (as it’s a base class) as well as before robotInit() is called.
The problem you ran into is due to the fact that the RobotBase constructor runs after your static initializers run. Arguably most of the code in the RobotBase constructor should be moved to a static initializer instead to avoid this issue.
The reason an exception is not thrown is that the NetworkTables getInstance() doesn’t know if you’re a client or a server until you call startServer() or startClient(). Only servers have preferences files.
My concern with moving more stuff to static initializers is that order of initialization of statics is not guaranteed by the language, so you would be causing more randomness, not less. I think I’d prefer the deterministic behavior we have now.
Is there any way to log any kind of warning if getInstance is called before either startServer or startClient? Clearly, you have to call it at least once in order to call startServer… But you could still log a message along the lines of “warning: NetworkTables not fully initialized yet.” I may be grasping at straws, but it seems like this may be a common error by teams and anything we can do to prevent or at least detect the error will be helpful.
Unlike C++, in Java, static initialization order is well defined by the language. For example, base class static initializers always run before derived class static initializers. We can’t fix C++, but we can make Java work either with static initializers or by moving the NetworkTables initialization from the RobotBase constructor to the program’s main function.
Printing a warning if getInstance() is called will cause a warning to print out every time the robot program is started, and that will basically guarantee the warning gets ignored.
I’d be quite hesitant to have different behavior between C++ and Java. Teams making the switch or trying to base their code on samples from the other language could get tripped up.
I know this may be starting to sound silly, but how many times is getInstance called before startServer? If only just that once, then it would be easy enough to test in the singleton code and flag extraneous calls with a warning. But that is starting to sound kind of hacky.
On another tack, is it always required for either startServer or startClient to be called? Is it ever reasonable to get or set values into the table before these calls? if not, then there’s an obvious point to validate operation.
In any case, any restriction like this should be well documented in order to reduce the kind of problem I experienced. Is such a notice in the docs today and I just missed it? Or does it need to be added?
They’re needed if you want it to talk to anything else. But it’s perfectly valid to switch an instance from client to server, for example. And you can use NetworkTables without anything else connected (e.g. purely locally).
I’m thinking the correct solution is to create a WPILib NetworkTableServer singleton wrapper class whose constructor does the default instance + server setup that is currently in the RobotBase constructor, and then use that NetworkTableServer class everwhere else, including in the Preferences class. That would solve the static initialization order issue for both C++ and Java, as long as that class was used instead of calling NetworkTableInstance.getDefault() directly.
Can you describe the issue in more detail? putString() in robotInit should definitely work. The original issue being described was trying to get persistent values (e.g. preferences) before the RobotBase constructor was called.
If you include static data members in a class and initialize them on the same line where they’re defined, then you’ve got static initializers. Those data members are initialized just before the first instance of the class is constructed. If your class extends some other class, the static members are initialized after the static members of the base class, but still before any non-static members of the base class.
This is the root of the problem that inspired this thread. The static members of my Robot class were getting initialized before the saved Preferences were getting loaded from their file by the RobotBase constructor.
Example:
class Robot extends RobotBase {
static double val1 = Preferences.getInstance().getDouble("important_value"); // happens first, gets wrong value
double val2;
Robot() {
// the class constructor
val2 = Preferences.getInstance().getDouble("important_value"); // happens second, gets correct value
}
}
Then I’m hijacking the thread. Link to my question is Here
The issue persists to today. It’s been so long that I forgot to reply: I found out that the value exists in network tables but shuffleboard has problems finding it. We have to restart shuffleboard.