CrossTheRoadElec / Phoenix-api

CTRE Phoenix language API (targets FIRST Robotics Competition, Linux, RaspPi, Windows)
Other
39 stars 28 forks source link

TalonFXControlMode unnecessary #66

Closed Redrield closed 4 years ago

Redrield commented 4 years ago

When skimming the new API for the 2020 season, I'm struggling to figure out why TalonFXControlMode and TalonFXFeedbackDevice are necessary. Since TalonFX is a subclass of BaseMotorController, it still functions with the old enums, and internally the new enums are simply converted and then delegate to the functions in BaseMotorController anyways.

ozrien commented 4 years ago

TalonFXFeedbackDevice is for Talon-FX. It contains only the enums supported by the product. It self-documents what the capabilities that hardware can do. Same with TalonSRXFeedbackDevice. Notice the FX enum does not have analog, but does have integrated-sensor.

However if you need to write software that works with all Talon* classes, you can use the general FeedBackDevice enum class, which allows you to do this, BUT does not provide compile-time checking to ensure you don't select something unsupported in your hardware device.

The same is true with control modes. ControlMode will hold all possible values, including values that may not be supported in all hardware products. TalonFXControlMode values are guaranteed to work on that particular product. It just appears strange now because they happen to be the same. I suspect this will change in the future, at which point we may add a TalonSRXControlMode (which doesn't back-break anything since ControlMode is still a valid choice).

As the variety of motor controller implementations grows, I see this as necessary design to cover two scenarios

Makes sense?