I really appreciate the work put into WPILib and in general I find it easy to use and pretty straightforward. However, at least 2-3 time a season (if not more often), I find myself blocked because a class was programmed with various parameters as “final”. I can’t even subclass them and add my own setter/getter. I either throw up my hands or copy the code (which is sad).
Example from today: SingleJointedArmSim - the arm length is fixed. OK, so yes it is much easier to not have to compute certain parameters each loop, but there are a ton of extending arms out there.
Another examples I can dredge up:
ArmFeedForward (and presumably all the others): the K’s are all final. For an extending arm, it would not be hard for the user to have a linear model for Kg and maybe the others, but right now, they have no way to update the values and have to ignore the class.
I remember there were other examples we hit last year. I think they were related to the Trapezoidal subsystem. If I remember we could not update any of the constants after the constructor, and could not even do that with our own subclass.
Beyond rote style rules, is there any reason just about every parameter is set as “final”?
These classes are designed to model a particular system (an arm with fixed length). Properly modeling an arm with varying length is not as simple as just adding a setter to change a value, as it’s a coupled system. Maybe it might work in some instances as an approximation, but to do it right (eg work in all instances, which is what we want our classes to do) requires a completely different model. We don’t currently have such a model worked out, if we did, it would be a separate class. The reason we don’t have one is that extending arms were relatively rare in FRC before this year. It’s something we can look at adding in the future (contributions happily accepted).
Private variables without a setter should be marked final, as there’s no way to change them from outside the class anyway.
I hear what you are saying, but that style also means that users can’t even write a subclass, or wrapper to do the updating.
I guess my prime example is ArmFeedForward. The 4 K-variables are public final. The math is really easy within the class. Why can’t these be written with setters/getters and allow the end user to apply a (external) model? Sure, you could copy the whole class, and clean out all the “locking”, but why not allow that from the beginning?
(And just a small note: on a simple FRC scale, an extending arm is not really a coupled system. To the accuracy that the model is known, it would be fine to move the center of mass out by X and recompute the MoI. After all, SingleJointedArmSim already has “estimateMOI” which assumes a linear weight distribute, which is probably not right for any real arm.)
We don’t intend for users to override the class. If we wanted to be really opinionated about it, we could have even marked the class as final. Changing the dynamics of the single-jointed arm feedforward makes it not a single-jointed arm anymore.
It is actually a coupled system. When the arm is rotating, a force is applied to the arm radially outward (F = mv^2/r) . This is more noticeable for heavier arms with weaker motors to hold them in place.
We prefer to implement higher fidelity solutions because it’s easier to wrap reasonable defaults around those than it is for a team to implement the higher fidelity solution themselves if/when they need it.
This is actually an example of what I just described. We offer a default for users that don’t need or want a more accurate value, but users can plug in a more accurate value from CAD or system ID if they need to. They don’t need to go implement a custom thing to accurately model their system.
To give you a sense of how I think about feedforwards, they’re duals of our physics sim classes; physics sim computes state evolution from inputs, and feedforward computes inputs from state evolution. We strive for maximal fidelity of the physics sims when we can, and the feedforwards benefit directly from that work.
If the goal is to simulate a telescoping arm, a dedicated class should be made for that with the appropriate dynamics. Then, a feedforward can be derived more generally from what it does. We do the hard math (because we can and it’s fun) so teams don’t have to.
Pardon the divergence from the topic to the sub example. I am curious if our interpetation of the ArmFeedForward for a telescoping arm was correct, It worked very well when we finally did it but that doesn’t mean it was correct . We used sysId to determine the feed forward constants for our arm fully retracted and another set of constants for fully extened. We created 2 instances of ArmFeedForward, one for each set of constants. Then when we were moving the arm we did ran the max extension ff calculate and the min extension ff calculate and did a linear interpolation between the two based on our current extension.
What you did is not strictly kinematically correct but is a good enough approximation if the coupling terms for your mechanism are small, which they were for most robots this year.
Feedforward is always a balancing act between model complexity and fidelity.
Re: public final fields, these allow us to avoid clunky accessor patterns in places where there isn’t any additional internal complexity to hide or any need for flexibility to change the data structure in the future. There’s nothing wrong with copying the implementation if you want a similar class that works differently, but WPILib itself is not trying to provide customizable/extendable feedforward classes.
. We used sysId to determine the feed forward constants for our arm fully retracted and another set of constants for fully extened. We created 2 instances of ArmFeedForward, one for each set of constants. Then when we were moving the arm we did ran the max extension ff calculate and the min extension ff calculate and did a linear interpolation between the two based on our current extension.