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

[BUG] POW() #306

Closed
Candas1 opened this issue Sep 6, 2023 · 8 comments
Closed

[BUG] POW() #306

Candas1 opened this issue Sep 6, 2023 · 8 comments
Assignees
Labels
enhancement New feature or request possible bug
Milestone

Comments

@Candas1
Copy link
Collaborator

Candas1 commented Sep 6, 2023

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
Copy link
Member

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
Copy link
Collaborator Author

Candas1 commented Sep 7, 2023

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 askuric added the enhancement New feature or request label Sep 8, 2023
@askuric
Copy link
Member

askuric commented Sep 8, 2023

Very interesting!

@Candas1
Copy link
Collaborator Author

Candas1 commented Sep 9, 2023

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
Copy link
Member

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
Copy link
Collaborator Author

Candas1 commented Sep 11, 2023

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
Copy link
Member

pow stuff is part of PR #304

@runger1101001 runger1101001 added this to the 2.3.1_Release milestone Sep 11, 2023
@runger1101001 runger1101001 self-assigned this Sep 11, 2023
@Candas1
Copy link
Collaborator Author

Candas1 commented Sep 11, 2023

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

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

No branches or pull requests

3 participants