simplefoc / Arduino-FOC

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

[BUG] POW() #306

Closed Candas1 closed 9 months ago

Candas1 commented 10 months ago

Hi,

A user came across something weird when testing on a gd32 board. A change in one of my last commit is making the main loop much slower, impacting motor control.

It seems the reason for the performance drop is the usage of pow() in the init of the voltage sense, although it's not even running in the main loop.... I still need to figure out why this happens, the gd32 is using a 4 years old gcc.

On stm32, I haven't noticed the performance drop, but I see another impact, which is the flash usage. This is without the voltage sense:

RAM:   [=         ]   9.2% (used 4520 bytes from 49152 bytes)
Flash: [===       ]  27.3% (used 71484 bytes from 262144 bytes)

This is with volage sense:

RAM:   [=         ]   9.3% (used 4572 bytes from 49152 bytes)
Flash: [===       ]  29.5% (used 77332 bytes from 262144 bytes)

Now, as we are using the pow function only for powers of 2 and integers, I use this macro #define pwrtwo(x) (1 << (x)):

RAM:   [=         ]   9.3% (used 4572 bytes from 49152 bytes)
Flash: [===       ]  28.0% (used 73420 bytes from 262144 bytes)

So 1.5% less on a big chip (256Kb).

It seems pow() is used for the same purpose in MagneticSensorI2C and in MagneticSensorSPI

runger1101001 commented 10 months ago

Thank you for reporting this! We’ll try to fix it for next release, since using pow() seems quite gratuitous for raising to powers of 2

Candas1 commented 10 months ago

Please let me know if a PR helps I can add #define _powtwo(x) (1 << (x)) here, update magnetic sensors init, and update arduino-foc-drivers when the first PR is accepted

askuric commented 10 months ago

Very interesting!

Candas1 commented 10 months ago

I also checked, the svpwm without atan2 I mentioned in the community also reduces flash by 0.5%.

You guys seem very busy, please let me know how I can make your life easier with this.

Do you prefer an issue on github for any improvement to better track. I can even create a PR.

runger1101001 commented 10 months ago

Thanks so much @Candas1 ! I'm sorry we're so busy, but don't think all your work has gone unnoticed! We're really very grateful for your support and careful testing.

I will work in the pow() changes right now.

For the SVPWM a PR would be greatly appreciated, and also it would credit you for the change on github, which seems more than fair!! Note that we take PRs against the dev branch only.

Candas1 commented 10 months ago

Don't get me wrong, it's not a complain.

For the SVPWM change I have some questions but we can discuss it separately.

runger1101001 commented 10 months ago

pow stuff is part of PR #304

Candas1 commented 10 months ago

Thanks. It works, I see the flash reducing, and the motor control works as usual. I cannot test the magnetic encoder part.