MagneticSensorAnalog: Fix angle calculation

[austin-bowen]

Description

MagneticSensorAnalog::getSensorAngle has a slightly incorrect implementation for converting raw_count to degrees:

return ( (float) (raw_count) / (float)cpr) * _2PI;

It should first subtract min_raw_count from raw_count so it is properly in the range [0, cpr] before dividing by cpr.

Also, the cpr calculation is slightly incorrect; it should be:
cpr = _min_raw_count - _max_raw_count + 1;
Reasoning in comments below.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
  ^ Not sure which of the two it is. I'd expect it to only make things better due to more accurate position calculations, but hard to be sure somebody's current code doesn't somehow depend on the slightly incorrect original implementation.

How Has This Been Tested?

Ran velocity control, which was working before my change, and worked after as well.

Test Configuration/Setup:

• Hardware:
  • SimpleFOC Mini
  • 2804 gimbal motor
  • AS5600 magnetic encoder (using analog OUT)
  • All from this Amazon listing
• IDE: Arduino
• MCU package version: XIAO RP2040

[askuric]

Oh wow, nice catch!
We did not see this for years, we did not use the min max values at all....

But I think that this is still not correct right?
We should not divide by CPR but with the range.

Would you be able to test this?

EDIT: I read it too quickly, we already divide by the range.
Thanks for the PR and for spotting this!
We are gonna merge it for the next release :)

[austin-bowen]

@askuric I realized another thing: cpr needs 1 added to it. Right now, if raw_count is max_raw_count, the equation becomes:

(max_raw_count - min_raw-count) / cpr * 2pi
= cpr / cpr * 2pi
= 1 * 2pi
= 360°
= 0°

But that's not accurate -- the angle represented by max_raw_count isn't 0°, it's one tick before the wraparound to 0°. Adding 1 to cpr fixes this:

(max_raw_count - min_raw-count) / (cpr + 1) * 2pi
= cpr / (cpr + 1) * 2pi
= Just under 360°

If you agree with this, I can update my PR to add 1 to cpr in the constructor.

Lmk what you think

[austin-bowen]

I went ahead and added the cpr calculation change to the PR