We’ve already made a head start.
Split the class Encoder into several smaller classes that each have a clear easy-to-understand purpose.
We had a slightly different way in mind. Namely, using a parent encoder class with additional inherited classes for specific encoder types. This allows an axis’ controller to interact with the encoder regardless of type and we won’t have to define similar variables like shadow_count across multiple encoder types.
Decouple the hardware-specific encoder drivers (quadrature encoder, hall-effect encoder, various SPI encoders, …) from estimation algorithms (e.g. the PLL and interpolation algorithms).
so that for instance the SPI protocol code is disentangled from the SPI have a clearly defined purpose and to disentangle hardware-dependent code (like “get the quadrature encoder count”)
As mentioned above, we intend to keep general encoder entities into the parent class while moving encoder type specifics to their specific inherited classes.
Though we currently have limited ourselves to splitting them into incremental and absolute encoders.
Decouple the encoders from the axes: Apart from the label on the PCB, the links “M0<=>ENC0” and “M1<=>ENC1” in the code are purely artificial.
Have support for dynamically defining the number and types of encoders that the user wants to use. For instance there can be only two quadrature encoders (because they depend on MCU timers) but there can be many more SPI encoders (each with its own nCS line).
We’re trying to add an encoder_manager class which has the purpose of properly regulating the switching between encoders for each axis. So that the axis requires no knowledge of how many or what type of encoders are present but can just select one of the encoders available. (The encoder_manager would handle all safety issues like checking when axis wants to use encoder5 if it exists and whether it is ready.)
The last point is a bit tricky to get right so as an intermediate step I would propose to have a predefined collection of encoders (quad_enc0, quad_enc1, spi_enc0, spi_enc1, hall_enc0, hall_enc1) as toplevel objects in class ODrive. In addition there could be separate objects for the estimator components (the estimator stuff could be further split into separate components but that can happen at a later point).
We had a similar idea but then divided in 2 incremental encoders and up to 4 absolute encoders.
As you might have noticed, our user case is oriented to using an incremental and absolute encoder together on a single axis. We’ve no particular interest in encoder fusion as we use the incremental encoder for velocity estimates of the motor and the absolute encoder for position estimates of the mechanical joint which the motor is a component of. With the exception of current control, we run our control separately on a SBC.