Falcon Implementation

[theVerySharpFlat]

This was my implementation for the falcon 500s that we never used. Since we're doing swerve with falcons, this is going to be necessary

[gcjurgiel]

Everything looks good! We can merge it now, or we may want to wait until we implement the other features we'll need. I'll repeat the checklist below for clarity.

• [x] Current Limits
• [x] Open Loop Ramp Limits
• [x] Open Loop Min/Max Output Limits
• [x] Closed Loop Ramp Limits (Separate from profiling/smartMotion/MotionMagic)
• [x] Remote Encoders (CANCoder)

The only difficult one should be the remote encoders. You'll have to set it up with ConfigureRemoteFeedbackFilter then select it for use with ConfigureFeedbackDevice, but I'm not sure about specifics.

[theVerySharpFlat]

Everything looks good! We can merge it now, or we may want to wait until we implement the other features we'll need. I'll repeat the checklist below for clarity.

• [ ] Current Limits
• [ ] Open Loop Ramp Limits
• [ ] Open Loop Min/Max Output Limits
• [ ] Closed Loop Ramp Limits (Separate from profiling/smartMotion/MotionMagic)
• [ ] Remote Encoders (CANCoder)

The only difficult one should be the remote encoders. You'll have to set it up with ConfigureRemoteFeedbackFilter then select it for use with ConfigureFeedbackDevice, but I'm not sure about specifics.

Yeah, I think we should implement and test this before merging

[theVerySharpFlat]

@gcjurgiel I implemented all of the features mentioned above except the cancoder stuff. I was a little distracted while doing it, but I think I got everything right. Should there be ramp rate limits for position controllers as well? This is something I didn't do.

[theVerySharpFlat]

@gcjurgiel I think I implemented the CANCoders correctly, but we shall see how they turn out

[gcjurgiel]

First, I'd add the ramp limits to the position controllers as well just for completeness (I think I'm adding a open loop override for them as well just in case we ever need it).

Second, it looks like you did everything correct to me, though it doesn't quite line up with how I did it for the SparkMaxes, and it looks like there may be duplicate information in the config. Its mostly personal preference, but it may be worth reorganizing the config struts for both types so that they're more consistent

[theVerySharpFlat]

15d7788 adds ramp rates

[theVerySharpFlat]

Second, it looks like you did everything correct to me, though it doesn't quite line up with how I did it for the SparkMaxes, and it looks like there may be duplicate information in the config. Its mostly personal preference, but it may be worth reorganizing the config struts for both types so that they're more consistent

Would you say that a common config structs for velocity/position controllers would be beneficial? Perhaps in the constructors for the individual controllers we could pass in both the common and device-specific configs as seperate structs.

[gcjurgiel]

Not necessarily. My main point was that with the new way you've set up the constructor we may consider reorganizing the config structs. Since you're the one that's going to be using them next year, I'd leave it up to you.

[gcjurgiel]

The motor would't spin because it had the same id as the PDP. Zero is always reserved for the PDP so don't use it.

[theVerySharpFlat]

The motor would't spin because it had the same id as the PDP. Zero is always reserved for the PDP so don't use it.

Dang. I would have never caught this. Thanks

[theVerySharpFlat]

@gcjurgiel 98c0ae3 and 2df18ba fix incorrect unit conversions which were giving me problems today. This error made the setpoints extremely low.

[theVerySharpFlat]

83afa40 fixes a major bug in the code where I assume that feedback from the CANCoder needs to be multiplied/divided by the gear ratio between the axle on the motor and the axle on the module. However, the cancoder measures the speed/position of the module, so this multiplication/division of the gear ratio led to the motors spinning about 12.8x more than what they were supposed to