simplefoc / Arduino-FOC

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

[BUG] sin/cos integer overflow on 16-bit CPUs #415

Open dekutree64 opened 3 days ago

dekutree64 commented 3 days ago

Thanks to ChrisMicro on the forum for finding this bug in the fast sin/cos added last year https://github.com/simplefoc/Arduino-FOC/pull/285

Forum thread https://community.simplefoc.com/t/simplefoc-sin-calculation/5131

There are two potential solutions: Solution 1 is to change the lookup table to signed 16-bit and fudge the last entry to 32767 so it doesn't overflow, and add some casts to do part of the interpolation calculation in 32-bit: return (1.0f/32768.0f) (t1 + (int)((((int32_t)t2 - t1) frac) >> 8));

Solution 2 is to change t1 and t2 to int32_t. This will be a few cycles slower on 16-bit platforms, but retain the behavior of returning exactly 1.0 at pi/2 and -1.0 at 3pi/2.

I would be fine with either. I think 1 was my original intent, but after all the iterations testing the speed and accuracy of the calculations, I forgot to make the final edits.

EDIT: No measured speed difference when testing, so I went with solution 2. Pull request #416