Depends what you mean by telemetry.
For updating the (Smart) Dashboard, I currently favour using initSendable to attach properties to suppliers. It doesn’t really change the amount of work that gets done, but it pulls it out of periodic.
1 Like
internal measurements used for control
I’m sorry. Could you explain a little more? Is this something exemplified in the code you posted above?
The new code just takes another approach. One that separates periodics from the CommandScheduler for independent timings. You cannot run a control loop in periodic at a separate frequency without bypassing the CommandScheduler or providing more plumbing. Unless you put all things in the command scheduler at faster frequency.
The commands just set which setpoint is active and which control mode to use. The periodic method handles the rest. I know this goes against your thoughts above on putting the loops in a RunCommand, but i don’t see any choice if one wants the possiblity of faster loop timings. Unless you use a NotifierCommand which adds a separate thread.
The only command that is a RunCommand is the manual control. I always prefer to have a manual control for tuning and emergencies (need to shake game element out of robot).
The RobotContainer is gone. The subsystem doesn’t extend SubsystemBase. It just implements Subsystem and Sendable. This allows the periodic method to be passed into addPeriodic and handled on its own separate frequency and not interfered with by the CommandScheduler. That way I’m not wasting the periodic function, but I’ve given it some more freedom. From what I can tell this doesn’t violate any of the best approach ideas.
1 Like
Well, every rule has exceptions. For a lot of control applications, 50Hz is enough. If it isn’t, I would hope that using the PID controller on the motor controller would pick up the slack, even if we’re blending it with a profile and a feedforward.
A possible downside to your approach is that we may not have a predictable delay between the command setting the goal and your periodic method servicing the controller. This variable control lag might offset the benefit of servicing the controller more often.
Another is that I’m not sure I would recommend using addPeriodic to increase the frequency above 50Hz. Methods registered with addPeriodic run in between the calls to IterativeRobotBase.loopFunc. Unless you can get your main event loop to take only a small fraction of the 20ms budget, then a periodic nominally running at, say, 100Hz might not end up running above 50Hz in practice.
You’re doing this so that the periodic method is not called by the CommandScheduler. I think it would be less confusing simply to call your periodic method something else and leave periodic unimplemented. That way you’re free to use SubsystemBase.
In my article and example, I assumed the use of RobotContainer because it is the current convention, but I don’t have strong feelings about retaining it.
I just got rid of it to make addPeriodic easier to acess
That’s fair. Just didn’t want something to go to waste.
Sure. I’m probably going to give up the ghost on running simple mechanisms on their own loop. Really only the odometry should need it and I can use a Notifier class for that.
I find that thread-safe programming is one of the hardest concepts to communicate to students. The WPILIB team made the decision to protect FRC programmers from it by default. In the few cases where we do use threads, I see many bugs related to that. When I give FRC programming advice, I try to strike a balance between the way I would write something and what I can reasonably get high school students to implement.
3 Likes
Professionals, too!
I advise sticking strictly to fairly rigid message-passing architectures if you try multithreading at all, but also it is rigorously pointless on the RIO in Java. Only native threads give you anything worthwhile in this environment.
2 Likes
I agree with this. The only thing I’ve noticed recently is the CTRE Swerve Code is using a separate thread for the odometry and if we do anything with multithreading that’s to me where it makes the most sense.
Most of our mutli-threading relates to cameras and image processing, so is often neither roboRIO nor Java.
1 Like
I found the source code and I now understand what you were referring to as “telemetry”. It appears that everything there happens under a ReentrantReadWriteLock, using copy-on-write and consumer patterns. So it’s thread-safe, but I’m unclear how large the gain is. By default the update frequency is 250Hz on CANivore or 100Hz on the regular CAN bus.
Regardless, I feel that pose estimation does not fit neatly into these best practices.
That’s understandable. Do you have a resource on threaded programming. I always feel like I can know more even from the basics
I don’t. It’s slightly different for every language and environment. I don’t even have good recommendations for WPILIB programming beyond “don’t do it”. If you must, then synchronized and Atomic* are good starting points. Make sure your synchronized region is as small as possible, but no smaller. Think really hard about all points where different threads have to interact. Make sure anything not thread-safe is private.
A common trick is for the writer to prepare a new datastructure on the side and then make it available to readers by changing a single reference. That still needs to be protected, e.g. with AtomicReference.
This is way off-topic. 
1 Like
So another hang up for students, especially new ones, is wrapping their heads around lambda functions. The syntax of such nested in another set of parentheses doesn’t help. The :: operator helps immensely but of course would require a private helper method. However because run, runOnce, etc. require runnables it begs the question of having overloads for these that take consumers and suppliers. I’m finding several times that my command factories are taking in a supplier and that supplier is being used to feed a consumer which is then being passed as lambda runnable to the run command creation method.
Something like this
public Command createVelocityControl(DoubleSupplier velocityMPS) {
return runOnce(this::resetVelocityProfile)
.andThen(() -> velocityControlLoop(velocityMPS.getAsDouble()));
}
I agree that lambda functions are a sticking point for getting students into using this aspect of WPILIB. I know I’ve mentioned this before, but I wrote an article on lambda functions to try to get students over this hump, and I recently rewrote and expanded it.
There’s also a WPILib article on lambdas (Treating Functions as Data — FIRST Robotics Competition documentation). Contributions welcome to further enhance it!
2 Likes
I’m not entirely sure I understand what this would look like or help fix. The example you gave can’t really be simplified more by the command library.
Are you suggesting adding (Consumer<T>, Supplier<T>) overloads? I think that would bloat the scope of the library and decrease readability.
That was my thought. I thought about making a PR draft, but it sound like I should not.
That’s really nice. I linked to it.
The book I used when I taught concurrent programming was “Java Concurrency in Practice” by Brian Goetz. Published in 2006 (i.e. Java 6 was then current) it is somewhat dated now but it is still useful in that it lays out the ways that threads interact with the memory model to set traps for the unwary.
If you look on Amazon, beware. The listing shows “Effective Java” as a newer edition of this book. It isn’t.