[BUG] MagneticSensorI2C: incorrect mask calculation

[greymfm]

Hello, The I2C mask calculations are wrong: https://github.com/simplefoc/Arduino-FOC/blob/dev/src/sensors/MagneticSensorI2C.cpp

   lsb_mask = (uint8_t)( (2 << lsb_used) - 1 );
  msb_mask = (uint8_t)( (2 << _bits_used_msb) - 1 );

The correct (and tested) calculation looks like this:

   lsb_mask = (uint8_t)( (1 << lsb_used) - 1 );
  msb_mask = (uint8_t)( (1 << _bits_used_msb) - 1 );

:-)

[runger1101001]

It's a good observation, thank you!

We have to ask @askuric if it was deliberate. The sensors, esp. the AS5600 which is what this driver is mostly used for, are often noisy in the last bit. Maybe the idea was to cut it out?

If so I am not sure it effective, since I think it will only suppress 1-bit noise 50% of the time, and 50% of the time it will amplify it to be 2-bit noise.

I think a hysteresis approach could be quite effective on the other hand, if we're willing to trade accuracy for stability on the bit level.

[greymfm]

Well, the existing code does not cut but instead uses one additional (not existing) bit:

   lsb_mask = (uint8_t)( (2 << lsb_used) - 1 );
  msb_mask = (uint8_t)( (2 << _bits_used_msb) - 1 );

E.g. (lsb_used=8 and bits_used_msb=4) gives (2 << 8)-1 = 512-1 for the low mask and (2 << 4)-1 = 32-1 for the high mask ... that is much more than the sensor registers can output (correct would be 255 for the low mask and 15 for the high mask for this specific example :-)

Too much shifting again... :-)

[askuric]

Hey guys, Huh, this code has been refactored several times, I've written one version for as5600 and as5048 at the begining but it was @owennewo who refactored the code and since we did not touch it. I'll look into it this week and report back, thanks for reporting it @greymfm.

[askuric]

Hey @greymfm, You're absolutely right. I've gone through the code and the datasheets and tested it with my as5600, and I guess that this shifting error did not make any issues so far, but that was probably just luck.

This is an old bug coming from the first i2c sensor implementation so thanks for taking the time to go trough the code so deeply :D

Do you want to do a PR or should I update the code directly. If you do a PR, please do it with respect to the dev branch.

[greymfm]

You can update the code directly (I have no PR for this...). Cheers!