simplefoc / Arduino-FOC

Arduino FOC for BLDC and Stepper motors - Arduino Based Field Oriented Control Algorithm Library
https://docs.simplefoc.com
MIT License
2.03k stars 521 forks source link

Kv rating fix #347

Closed nmscode closed 10 months ago

nmscode commented 10 months ago

fixes: #344

Need to verify that this is the correct math/logic. It assumes sinusoidal bemf. Perhaps can add an option for more trapezoidal bemf motors.

runger1101001 commented 10 months ago

Thank you very much for contributing this!

I'm going to hold off merging it until after the next release, which I hope will be quite soon. That will give us some time to then test the effects on the dev branch.

Introducing this change will correct the calculations, but it will have the side effect of changing the behaviour of existing setups. Many people will have "tuned" their settings, picking a KV value for themselves which differs from the actual one but works well in practice. People will have to change these values after we introduce the fix. So its something we need to do carefully.

runger1101001 commented 10 months ago

I have another question: why not pre-multiply the KV_rating value by the factor of _SQRT3/_RPM_TO_RADS in the init method, and then not have to do this multiplication/division each time through the loop?

nmscode commented 10 months ago

That is also a possibility. I was not sure if the KV_rating variable might be used for something else and would be needed without the multiplier. However, I can make the change so that it is pre-multiplied if that is preferable.

Candas1 commented 10 months ago

I also have the feeling this will be confusing if KV_rating is used somewhere else. And someone changing kv_rating after the constructor should know that as well. It could be this is optimized by the compiler anyway.

runger1101001 commented 10 months ago

You're right - it would be confusing to have the parameter so named, but the value represent a different quantity.

So keep as is, but perhaps remove the unneeded brackets from the calculation - not sure if the compiler would respect the order implied by the parenthesis or optimize it away, but in this case we'd prefer the brackets around the two constants to make it clear to the compiler?

askuric commented 10 months ago

This looks good to me. I've added sqrt(2) in order for the people to avoid setting too high KV values, to the KV values that people set relatively similar to the ones given by the manufacturers. Honestly, I've found sqrt(2) to be empirically a good number. But I did not go any further with it.

This solution is much better both in terms of the code that is easy to understand(sqrt(3) instead sqrt(2)), and keeping the KV_rating the same as the one user has set will make it much easier to set for the user.

runger1101001 commented 10 months ago

So it's merged! :-) Thank you very much for the contribution and the discussion!

Candas1 commented 2 months ago

Hi Guys,

Is the statement in the docs about increasing the kv by 10-20% still valid now ? https://docs.simplefoc.com/voltage_torque_mode#voltage-control-with-current-estimation-and-back-emf-compensation

image