Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial 8pwm #260

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

Initial 8pwm #260

wants to merge 13 commits into from

Conversation

Juanduino
Copy link

Will you cross examine?

@Juanduino
Copy link
Author

Yes, the StepperDriver8PWM::setPwm(float Ualpha, float Ubeta) function is intended to be called by the setPhaseVoltage function in order to set the appropriate PWM duty cycles based on the calculated alpha and beta voltages. The setPhaseVoltage function performs the necessary calculations to transform the input voltages Uq and Ud to the alpha and beta voltages required for the 8 PWM control. These alpha and beta voltages are then passed as arguments to the setPwm function, which sets the appropriate duty cycles on the PWM pins. So, the setPhaseVoltage and StepperDriver8PWM::setPwm functions are intended to work together as part of a larger system for driving a 2-phase stepper motor using 8 PWM pins.

@runger1101001
Copy link
Member

Hey,

Looks like a good start, but:

  • looks like a bunch of other commits from the 2.3.0 release have got tangled with this one... please apply your changes to a clean copy of the dev branch
  • please separate the CORDIC stuff out, we won't put that in the main repo. Hardware specific code like that should not go in the main classes in that way. One option could be to put it in the drivers repo, as a StepperMotorSTM32CORDIC which subclasses the StepperMotor class. Another option can be just to replace the _sin() and _cos() implementations, I think I'll be making that possible in one of my next commits...
  • Looking at the code it doesn't look 100% finished - or I missed some stuff. Which cases can be handled? 8 different timers? 4 pairs of pins with each pair on the same timer? 4 pairs with each pair complementary? I think these things have to be reflected in the scoring function which selects the pin combination used...

@runger1101001
Copy link
Member

Hey, also it doesn't pass the checks for any architectures, not even STM32...
I think adding 8-PWM support would be a cool addition, but the PR needs a bit of work.
If you want to just get it working in your repo, and then have me help you make a clean PR out of that I'm also happy to do it that way :-)

@runger1101001 runger1101001 added the enhancement New feature or request label Mar 21, 2023
@Juanduino
Copy link
Author

Hey, also it doesn't pass the checks for any architectures, not even STM32... I think adding 8-PWM support would be a cool addition, but the PR needs a bit of work. If you want to just get it working in your repo, and then have me help you make a clean PR out of that I'm also happy to do it that way :-)

Yes, it still need some work, but im glad you like the general idea. Will organize it a bit and get a clean dev branch. Maybe I should define a HAS_CORDIC ? And list the MCU´s which currently does have that. And a USE_CORDIC?

@Juanduino
Copy link
Author

@Juanduino
Copy link
Author

Is that branch ok to use ?

@Juanduino
Copy link
Author

Juanduino commented Mar 22, 2023

@runger1101001 if you can find the time. Plz compose a draft of how to integrate the 8pwm driver. There are some of the code that would work e.g. the setpwm function:

// Set voltage to the pwm pin for 8PWM control
void StepperDriver8PWM::setPwm(float Ualpha, float Ubeta) {
float duty_cycle1A_h1(0.0f),duty_cycle1A_h2(0.0f),duty_cycle1B_h1(0.0f),duty_cycle1B_h2(0.0f);
float duty_cycle2A_h1(0.0f),duty_cycle2A_h2(0.0f),duty_cycle2B_h1(0.0f),duty_cycle2B_h2(0.0f);

// limit the voltage in driver
Ualpha = _constrain(Ualpha, -voltage_limit, voltage_limit);
Ubeta = _constrain(Ubeta, -voltage_limit, voltage_limit);

// hardware specific writing
if( Ualpha > 0 ) {
duty_cycle1B_h1 = _constrain(abs(Ualpha)/voltage_power_supply,0.0f,1.0f);
duty_cycle1B_h2 = 0.0f; // set second half-bridge duty cycle to 0
}
else {
duty_cycle1A_h1 = _constrain(abs(Ualpha)/voltage_power_supply,0.0f,1.0f);
duty_cycle1A_h2 = 0.0f; // set second half-bridge duty cycle to 0
}

if( Ubeta > 0 ) {
duty_cycle2B_h1 = _constrain(abs(Ubeta)/voltage_power_supply,0.0f,1.0f);
duty_cycle2B_h2 = 0.0f; // set second half-bridge duty cycle to 0
}
else {
duty_cycle2A_h1 = _constrain(abs(Ubeta)/voltage_power_supply,0.0f,1.0f);
duty_cycle2A_h2 = 0.0f; // set second half-bridge duty cycle to 0
}

// write to hardware
_writeDutyCycle8PWM(duty_cycle1A_h1, duty_cycle1A_h2, duty_cycle1B_h1, duty_cycle1B_h2,
duty_cycle2A_h1, duty_cycle2A_h2, duty_cycle2B_h1, duty_cycle2B_h2, params);
}

@Juanduino
Copy link
Author

Just a brief; Current 4PWM implementation can more or less be copied over to 8PWM, since setting duty-cycle to 4, already setup complimentary PWM outputs, is in a way similar to 4 single pin PWM, just a bit different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants