Position and velocity PIDControllers using same encoder

[Oblarg]

Is there any supported way to do this in wpilib? It seems you have to set an encoder to be either kRate or kPosition - what if you want it to be one for one controller, and another for a different one? I suppose you could write a wrapper class that implements PIDSource and contains the encoder, but that seems ugly.

[nickbrickmaster]

I suppose you could write a wrapper class that implements PIDSource and contains the encoder, but that seems ugly.

Looking at the code, I think this is your only option. If I had a dime for every time I had to implement an ugly solution, I'd have like, three dollars.

Alternately, use Python, and pass in the function you want as the PIDSource.

[Ether]

Are you permitted to wire the encoder to two separate pairs of inputs... one to decode rate and one to decode position?

[Oblarg]

Are you permitted to wire the encoder to two separate pairs of inputs... one to decode rate and one to decode position?


Can't see why not, unless this somehow counts as a custom circuit (I doubt it).

Not sure whether this or the in-code wrapper is clunkier. Perhaps this is something wpilib could support in the future.

[Ether]

Not sure whether this or the in-code wrapper is clunkier.

Do you want the FPGA to compute your instantaneous rate and position? Does the wrapper approach accomplish that?

[Oblarg]

Do you want the FPGA to compute your instantaneous rate and position? Does the wrapper approach accomplish that?


Wrapper should accomplish this, as encoder does have both a getPosition and getRate method - it's just that you can only set it to have PIDGet() call one of them or the other.

[kylelanman]

In your code where you set a set point on your velocity PID Controller couldn't you set the encoder to kRate mode and then in the code where you set a set point on your position PID Controller couldn't you set the encoder to kDistance mode? Unless I'm misunderstanding the use case you aren't going to be running both the position and the velocity PID Controllers at the same time so this should work.

[nickbrickmaster]

I haven't tested this, and haven't read the implementation, but you may also be able to create two encoder objects, one for position and one for rate.

Clarify: with the same port numbers.

Second edit: out of curiosity, what's your use case for this?

[Oblarg]

I haven't tested this, and haven't read the implementation, but you may also be able to create two encoder objects, one for position and one for rate.

Clarify: with the same port numbers.

Second edit: out of curiosity, what's your use case for this?


I considered constructing two encoders on the same pins, but I strongly suspect this will throw an error. Will have to check.

The use case is controlling a turret - we're wondering if it's possible to do a cascading loop using the same encoder for both the external (position) loop and the internal (velocity) loop. It's probably totally unnecessary to do this (simple fixes for dealing with friction and whatnot work fine), but it's an interesting thought.

[Thad House]

Creating 2 encoder objects on the same pins technically would be allowed, as long as you used the DigitalSource constructors for the encoder, rather then the int constructor, and passed in preconstructed DigitalInput objects. However, this is overkill, as you can pass the same instance of an encoder to 2 separate PIDController objects. This would be my recommended way, as constructing 2 separate encoder objects actually takes away an encoder you could use for another purpose, whereas 2 PIDController objects with the same encoder object wouldn't take any extra resources.

[Oblarg]

Creating 2 encoder objects on the same pins technically would be allowed, as long as you used the DigitalSource constructors for the encoder, rather then the int constructor, and passed in preconstructed DigitalInput objects. However, this is overkill, as you can pass the same instance of an encoder to 2 separate PIDController objects. This would be my recommended way, as constructing 2 separate encoder objects actually takes away an encoder you could use for another purpose, whereas 2 PIDController objects with the same encoder object wouldn't take any extra resources.

Yeah, but you can't send the same encoder object to two separate PIDController objects and have it be a kRate source for one of them and kPosition source for another. Hence the wrapper idea, which is ugly but would probably work.

[Thad House]

Yeah, but you can't send the same encoder object to two separate PIDController objects and have it be a kRate source for one of them and kPosition source for another. Hence the wrapper idea, which is ugly but would probably work.

Oh I guess thats true. A small wrapper class would allow that however, and that class should be pretty thin.

[calcmogul]

The use case is controlling a turret - we're wondering if it's possible to do a cascading loop using the same encoder for both the external (position) loop and the internal (velocity) loop.

If I understand your mechanism correctly, you are using both PID controllers to drive the same plant. One handles the position state while the second handles the velocity state. Velocity and acceleration feedforwards may help here. If you know what control signal you need to assert to produce a given velocity and acceleration, you could pass that information via feedforwards to a single position PID controller and remove the velocity PID controller. Otherwise, you could pass them to the inner velocity PID controller, and the velocity PID controller would, in effect, be accounting for uncertainties in your model that generates the velocity feedforward.

A custom controller that accounts for all three states (position, velocity, and acceleration) in the setpoint would be cleaner in my opinion, and the appropriate modifications to the PIDController class shouldn't be too difficult. By the way, state space controllers do this as well, but they require a reasonably accurate system model while PID does not.

[Oblarg]

Otherwise, you could pass them to the inner velocity PID controller, and the velocity PID controller would, in effect, be accounting for uncertainties in your model that generates the velocity feedforward.

This is the idea. As I said, it's probably completely unnecessary - but I didn't see any reason why it couldn't be done, and thought it might be worth playing with.

[MichaelBick]

This is the idea. As I said, it's probably completely unnecessary - but I didn't see any reason why it couldn't be done, and thought it might be worth playing with.

1836 did this for a couple different mechanisms in 2016. On the drivetrain, it helped keep the left and right sides of the drive in sync while also overcoming many of the issue with inertia during turns. We also used it on our catapult (cam fired) which made our zeroing process more consistent.

[GeeTwo]

I considered constructing two encoders on the same pins, but I strongly suspect this will throw an error. Will have to check.


Again, it's a matter of weighing the ugly, but perhaps you could free the encoder you aren't using (if I understand correctly, setting the variable(s) referencing it to null and possibly running garbage collection will accomplish this), then initiate the new one.

[Oblarg]

Again, it's a matter of weighing the ugly, but perhaps you could free the encoder you aren't using (if I understand correctly, setting the variable(s) referencing it to null and possibly running garbage collection will accomplish this), then initiate the new one.

If I were never going to use them at the same time, this would probably work - but, as mentioned earlier in the thread, the notion is to run a cascading control loop on a turret where the position controller outputs to a velocity controller using the same encoder.

[calcmogul]

Again, it's a matter of weighing the ugly, but perhaps you could free the encoder you aren't using (if I understand correctly, setting the variable(s) referencing it to null and possibly running garbage collection will accomplish this), then initiate the new one.

Doing this between two PIDController Notifiers, which are real-time processes, isn't a good idea since the garbage collector introduces a lot of non-determinism and delays. If they are running at low enough sample rates, it might work, but there's no guarantee. Forcing the garbage collector to run would also collect any other garbage lying around the Notifiers don't care about.

I thought of another option that avoids needing to do any trickery with multiple Encoder objects. You could inherit from the PIDController class with two classes called PositionPIDController and VelocityPIDController. They would both override calculate(), call m_pidInput.setPIDSourceType() with the PIDSourceType they each want to use, then call the base class calculate().

As a side note, you may need to synchronize access to the Encoder instance in calculate() since the PIDController instances are sharing it.

[Joe Ross]

Yeah, but you can't send the same encoder object to two separate PIDController objects and have it be a kRate source for one of them and kPosition source for another. Hence the wrapper idea, which is ugly but would probably work.

Here's a wrapper that 2485 wrote that does this.

https://github.com/team2485/frc-2016-command-based/blob/master/src/org/usfirst/frc/team2485/util/EncoderWrapperRateAndDistance.java