Feat stepper current sensing

[askuric]

So this is the initial PR that extends the StepperMotor class to support the current sensing
The PR brings several changes:

• StepperMotor class is extended to support the foc_current torque control
• CurrentSense class is extended to support Stepper motors (and drivers)
  • InlineCurrentSense and LowsideCurrentSense are extended as well
  • Current sense align is moved to the CurrentSense class and separated for BLDC and Stepper drivers
• New common class is introduced called FOCDriver and the BLDCDriver and StepperDriver extend it

I've tested the code and it seems to be working on the SimpleFOC StepShield using inline and low-side current sensing.
More testing is needed to make it more robust, but its a good start I'd say.

[Candas1]

Hi @askuric

Just one idea.
One of the reason why I refactored some of the FOC math is that we can run each piece separately.
So instead of running current = current_sense->getFOCCurrents(electrical_angle); in loopfoc, we can run this in BLDCMotor:

// read current phase currents
motor.phase_current = getPhaseCurrents();

// calculate clarke transform
motor.ABcurrent = getABCurrents(phase_current);
    
// calculate park transform
motor.current = getDQCurrents(ABcurrent,angle_el);

and this in StepperMotor:

// read current phase currents
motor.phase_current = getPhaseCurrents();

 // calculate clarke transform
ABCurrent_s ABcurrent;
ABcurrent.alpha = phase_current.a;
ABcurrent.beta = phase_current.b;
    
 // calculate park transform
 motor.current = getDQCurrents(ABcurrent,angle_el);

So you don't have to check if it's a BLDC or stepper motor:

 if (driver_type == DriverType::Stepper){
        ABCurrent_s return_ABcurrent;
        return_ABcurrent.alpha = current.a;
        return_ABcurrent.beta = current.b;
        return return_ABcurrent;
    }

And we can keep phase currents and AB currents as members to be reused by other algorithms like sensorless or displayed in the main loop.

[askuric]

Makes perfect sense!
I really don't like this if statement, but I was a bit hesitant to create a new current sensing class for stepper motors as they use 99% of the same logic, so I opted to merging them.
The consequence, at least in this code attempt, is this if-statement which is executed in real-time.

Your idea makes perfect sense and we could definitely go in this direction.

The only issue that I see is that the getFOCCurrents and getABCurrents will no longer calculate the right values for steppers.
We could discourage people from using them maybe.

Maybe we could create a special set of functions for steppers to avoid this if statement, something like getStepperFOCCurrents and getStepperABCurrents.
I don.t know. Maybe at the end it's just easier to create separate classes. :D

[Candas1]

Maybe we could create a special set of functions for steppers to avoid this if statement, something like getStepperFOCCurrents and getStepperABCurrents.
I don.t know. Maybe at the end it's just easier to create separate classes. :D

getStepperFOCCurrents could return ABcurrents directly so no need for getStepperABCurrents ?

[askuric]

Maybe we could create a special set of functions for steppers to avoid this if statement, something like getStepperFOCCurrents and getStepperABCurrents.
I don.t know. Maybe at the end it's just easier to create separate classes. :D

getStepperFOCCurrents could return ABcurrents directly so no need for getStepperABCurrents ?

True, but then the getABCurrents for steppers would not be correct right?

Let me be clear, I think that this is very edgy use-case. Most of the users will never call these functions anyway. I am just concerned a bit with the simplicity/clarity of it. If we have some functions that are available but do not return good results I am a bit uncomfortable about it.

I think that your approach is great and it resolves this issue almost completely. Especially since all the current values would be available in the motor class, so no need to calculate them outside. But still if someone really wants to calculate only the Alpha/beta currents outside the motor for steppers and if they call getABCurrents then the result would not be the correct one.
That is my only concern really.

[Candas1]

I think there is some confusion.
My point is that for Steppers, phase currents are the AB currents, so one of the functions is redundant.
getStepperFOCCurrents could return alpha and beta directly.

[askuric]

My point is that for Steppers, phase currents are the AB currents, so one of the functions is redundant.
getStepperFOCCurrents could return alpha and beta directly.

Do you mean getPhaseCurrents?

[Candas1]

Yeah sorry I am getting confused with all the currents :)

[askuric]

So as this change would change the API a bit (with the currents being available to the end users) I'd rather include the stepper current sensing code as a minimal working example, and then we can make these optimizations later, another PR. And cover all the edge cases.

Let me know what you think guys (@Candas1, @runger1101001).