search

CrossTheRoadElec / Phoenix-Releases

Release repository for Phoenix-Framework and Phoenix-Tuner
79 stars 6 forks source link

[Phoenix 6] CANCoder and CoreCanCoder don't inherit from frc::Encoder #64

Closed spacey-sooty closed 9 months ago

spacey-sooty commented 10 months ago

It would be very useful if one of these classes inherited from frc::Encoder so they could be switched with other encoders easier. They seem mostly api compatible but there would be some work required to make it inherit.

mjhealy commented 10 months ago

(speaking from the outside) Unfortunately, the Encoder class is currently geared (no pun intended) towards a particular kind of connection, which isn't really compatible with CAN. And the WPILib team seems to be actively moving away from providing reusable interfaces (or base classes in C++) that can be used to unite things. (Look at what happened with Gyro this year, for example.)

I wound up implementing some sample C++/Java code for the kids on my team that provides a wrapper type that provides access to standard encoder functionality (getting position, resetting, etc.), and some functions to create wrapped forms of a few types of encoders that they commonly work with. Gets the job done, and removes dependencies on various vendors (WPI or commercial) to "do the reasonable thing".

spacey-sooty commented 10 months ago

That's fair. That is what we are currently doing but I do like to decrease the amount of in house code we must rely on. If we have to continue using an in team wrapper I am ok with that but if their is a way for that to not be on us I would also appreciate that. At the end of the day its a design choice that isn't up to me and if required I'll just continue using wrappers

nuttle commented 10 months ago

It would be nice if there was at least a method that could return an object of the appropriate/standard class, even if there wasn't inheritance. It would be fine if this object's lifetime were tied to the vendor-specific object's lifetime (return a reference and maybe disable copy/assign).

bhall-ctre commented 10 months ago

It is not possible to inherit from WPILib's Encoder class, as it is not an interface, nor is it meant for extension by vendors. It is also not possible to provide an API that returns a WPILib Encoder, as it does not work with a CAN sensor.