Errors with motor angle (insufficient precision of float)

Since Antun mentioned, that this change needs a lot of testing for all types of sensors and drivers, which he doesn’t have time for right now, and since I have only the simpleFOCshield and some bldc motors without encoders, I am not able to test the new code for all sensors and drivers.
If Antun or another administrator finds time for testing all that, please send me a note and I will continue the work, as I don’t need the changes for my project at this point.
But be aware, if you have problems with the motor angle or if your motor stops like my little gimbal motor, to check if the problem could be the small precision of the motor angle representation.

Maybe I’m thinking the wrong thing but couldn’t this be solved by using encoder counts instead of angles on the calculations? They’re whole numbers and you can fit 2 billion of them in 32 bits. From my (small) experience in industrial motion control that’s the standard way to do them.

You are absolutly right, of course there are many opportunities to solve this problem. Using the encoder counts is one of them. Still, depending on the sensor type, some encoders have many thousend counts per one rotation, so even the encoder counts with 31 bit (one bit for sign) can overflow within a few hours.
Using a 32 bit long (again this is 31 bit precision, 1 bit for sign : two’s compliment) for the rotations will last much longer before overflow; when using a 5000 RPM motor, this should work for almost a year (298 days) before overflow :wink: .

Furthermore, using the rotations and the modulo angle will be easier to use in practice than the encoder count, as one has to use some information (how many counts per rotation, cpr) before calculating rotations or distances. But of couse, this is just a personal opinion.

@Juergen_Abel fix is for openloop velocity mode. I think for closed loop some of the sensor implementations might also suffer from this float overflow
e.g. magneticsensorspi,cpp has

  // if overflow happened track it as full rotation
  if(abs(d_angle) > (0.8*cpr) ) full_rotation_offset += d_angle > 0 ? -_2PI : _2PI; 
 return natural_direction * (full_rotation_offset + ( angle_data / (float)cpr) * _2PI);

The full_rotation_offset is a float and is in radians.

I’m trying to imagine what the behaviour would be if I was in closed loop velocity mode. I suspect it would appear as a momentary (massive) spike in velocity error.

Absolutly, that’s why I stated this post.
The problem not only affects open loop operation, but also closed loop operation.

The problem starts at the sensor and encoder implementation:

  • virtual float Sensor::getAngle ()
  • float Encoder::getAngle ()
  • float HallSensor::getAngle()
  • float MagneticSensorAnalog::getAngle()
  • float MagneticSensorI2C::getAngle()
  • float MagneticSensorSPI::getAngle()

But also almost all motor files are affected too:

  • int BLDCMotor::alignSensor()
  • void BLDCMotor::loopFOC()
  • void BLDCMotor::move(float new_target)
  • void BLDCMotor::setPhaseVoltage(float Uq, float Ud, float angle_el)
  • void BLDCMotor::velocityOpenloop(float target_velocity)
  • void BLDCMotor::angleOpenloop(float target_angle)
  • int StepperMotor::alignSensor()
  • int StepperMotor::absoluteZeroAlign()
  • void StepperMotor::move(float new_target)
  • void StepperMotor::setPhaseVoltage(float Uq, float Ud, float angle_el)
  • void StepperMotor::velocityOpenloop(float target_velocity)
  • void StepperMotor::angleOpenloop(float target_angle)

In all these functions (and many others), the angle is implemented as a 32 bit float value.

Since so many core files are affected, it would be a major change, to correct all that.

In the open loop implementation, this is critical, as shaft_angle stays at the same value all the time, once the overflow occured.
In most other cases, the effect depends on your program: in some programs, the motor might just shake a little when reaching the overflow, but if your program uses the angle value for other calculations, like distances or volume calculation of a pump, the effect might raise a huge error or disaster.

Please note, that depending on the kind of encoder (cpr) and motor speed, the error might occur much later (or never if you turn off the program before) and your operation might not be affected.
In any case, the low precision of the float data type should lead to an appropriate revision in a later version.

Hi Everybody,
From my point of view and the simplest, I think you have to create a “getElectricalAngle()” from 0 to 2PI for all the sensors and to directly use this one in loopFOC() function.
Then, to keep the shaft_Angle variable (currently “getAngle()” that is mechanical angle) and to do a reset at modulo 2PI before the overflow of this variable. We can replace this last one by long type (counter of position) but the issue will be the same, we will also have to do a count position reset before the overflow.

I mean also use only shaft_angle for the position.

Your thoughts ?

1 Like

As stated before, I would prefer to have a modulo angle (ElectricalAngle) as 32 bit float and motor rotations as 32 bit long.
The modulo angle is already reset after 2 * pi, the motor rotations could be reset after reaching the maximum/minimum value, but the reset would cause the same problems than mentioned above, if one calculates distance or volume by this value.
One of the advanages would be, that in most applications, the overflow will not occure, as it takes to long before it happens. For applications, which run a very long time, the programmer has to take care by itself (which shouldn’t be to difficult, once you are aware of the problem).
For calculations of the voltages and currents, the modulo angle should be sufficient, as everything is repeating after one rotation (2 * pi in Radiant).

Please note that it makes a difference, using “modulo angle” and “rotations” in comparison to “modulo angle” and “motor total angle”, as “rotations” has 31 bit precision whereas the “motor total angle” has only 23 bits precision (fraction part of 32 bit float).

In my experience most servo applications don’t require you to rotate the motor for thousands of revolutions. Even with a 23bit encoder(like most professional servo motors) 32 bits is enough to contain all the movements the servo needs to do in counts. And if you need to drive the motor for a long time(like a wheel) I’d think you want to use an encoder with maybe 100 steps or less, giving you a plenty of resolution and time for your needs. Or use a mix of rotation counts and counts inside a rotation or something. Maybe were looking at this for different applications.

But I’m really not experienced enough to give authoritative answers.

To this I would say that people seem to be using SimpleFOC for all kinds of things… certainly the guys using it for their Racecars would be going thousands of rotations in the same direction… race-cars don’t usually reverse… :wink:
So I would recommend not to assume any one use-case, and fix the code to support years of rotation if possible…

I would also keep the values as angles, and not switch to encoder counts. Many of the sensors supported are absolute rotation sensors anyway, and don’t work in an incremental counting way. And having the external interface as angles is certainly much nicer.

will setting all the ‘float’ 32 bits to ‘double’ 64 bits will solve this and maybe give us more time before overflowing?

Yes, using doubles would give you (a lot!) more time. Its a big code change though, it will make the code slower, and it won’t work on ATMega MCU and other MCUs that don’t have 64 bit doubles.

I have been working on the fix for this bug, and the current state can be found here.

If anyone feels like testing it, it would be much appreciated, since its the kind of change that affects many different setups.

Hi Runger,
I tried to replace all of ::getAngle () functions to return double, but the problem still persists. It doesn’t solve it.
I tried to download your library manually, and replacing all the files with the original library files.
trying to compile, I get these errors:

c:/users/rotem/appdata/local/arduino15/packages/stm32/tools/xpack-arm-none-eabi-gcc/9.2.1-1.1/bin/…/lib/gcc/arm-none-eabi/9.2.1/…/…/…/…/arm-none-eabi/bin/ld.exe: sketch\angle_control_WORKING_446RE.ino.cpp.o: in function relative(char*)': angle_control_WORKING_446RE.ino.cpp:(.text._Z8relativePc+0x4c): undefined reference to Sensor::getAngle()’
c:/users/rotem/appdata/local/arduino15/packages/stm32/tools/xpack-arm-none-eabi-gcc/9.2.1-1.1/bin/…/lib/gcc/arm-none-eabi/9.2.1/…/…/…/…/arm-none-eabi/bin/ld.exe: angle_control_WORKING_446RE.ino.cpp:(.text._Z8relativePc+0x6c): undefined reference to Sensor::getAngle()' c:/users/rotem/appdata/local/arduino15/packages/stm32/tools/xpack-arm-none-eabi-gcc/9.2.1-1.1/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld.exe: sketch\angle_control_WORKING_446RE.ino.cpp.o: in function script(char*)’:
angle_control_WORKING_446RE.ino.cpp:(.text._Z6scriptPc+0xbe): undefined reference to Sensor::getAngle()' c:/users/rotem/appdata/local/arduino15/packages/stm32/tools/xpack-arm-none-eabi-gcc/9.2.1-1.1/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld.exe: sketch\angle_control_WORKING_446RE.ino.cpp.o: in function loop’:
angle_control_WORKING_446RE.ino.cpp:(.text.loop+0x11a): undefined reference to Sensor::getAngle()' c:/users/rotem/appdata/local/arduino15/packages/stm32/tools/xpack-arm-none-eabi-gcc/9.2.1-1.1/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld.exe: angle_control_WORKING_446RE.ino.cpp:(.text.loop+0x1a0): undefined reference to Sensor::getAngle()’
c:/users/rotem/appdata/local/arduino15/packages/stm32/tools/xpack-arm-none-eabi-gcc/9.2.1-1.1/bin/…/lib/gcc/arm-none-eabi/9.2.1/…/…/…/…/arm-none-eabi/bin/ld.exe: sketch\angle_control_WORKING_446RE.ino.cpp.o:angle_control_WORKING_446RE.ino.cpp:(.text.loop+0x1d4): more undefined references to `Sensor::getAngle()’ follow
collect2.exe: error: ld returned 1 exit status
exit status 1
Error compiling for board Nucleo-64.

I’m not sure what is going on there…

The branch I referred to compiles for me for STM32 without problems… I’m using platformIO, and when I use a cloned version of a library I put it in my project’s “lib” folder. So my PlatformIO looks something like this:

In Arduino IDE it is a bit more difficult - you have to put the libraries in the Ardunio/libraries folder, replacing the existing libraries if you already have them…

Does the branch you refer to also supports MA730 encoder and fixes the overflow problem?

This branch solves the precision problem in velocity mode. It does not solve all of the precision problems we have in the code, but the other problems are less likely to occur in practice, so this patch should fix the precision problems that are affecting most users.

In terms of the MA730, it is supported the same way as before - via the generic “MagneticSensorSPI” class.

If using the branch, please remember it is a trade-off: early access to the bug fixes vs. less well tested code.
I’m suggesting this as a way forward (possibly!) so you don’t have to wait for the next release version, but please forgive me if it does not solve your problems, or even introduces new ones!

Dear @Juergen_Abel

Don’t know if you’re still interested, but we have implemented a precision fix for the first part of the problems (the most common problem case) where velocity was not updating after a certain number of turns.
Although I only came across this thread later, it is actually implemented in the way you suggested: with a 32bit float angle (0-2PI) for the shaft, plus a 32bit integer number of rotations count. In terms of the sensor, I think this is now numerically precise.
In terms of the remaining code, there is still some work to do to get rid of the precision bugs - I have attempted to identify them with comments in the code, but not yet solved them. I view this as less important, since the remaining bugs are less likely to occur in practice - things like rotating the motor 10000 rotations and then using position mode to change the position by 0.001°. Of course we should support this, but so far users have not complained about it.

The fixes are currently in this branch, and we’re working on getting them merged to the main repo, and into the next release.

I downloaded the library from the branch, put it in the libs folder, and it compiles and runs. (by the way in your screenshot I can see also the Arduino-FOC-drivers which I didn’t copy).
When running the code, on the motor.init() function, the motor draws high current and can’t find the correct electrical angle.
In my code I also update the BLDCmotor.cpp:
for (int i = 500; i >=0; i-- ) {
float angle = _3PI_2 + _2PI * i / 500.0 ;
setPhaseVoltage(voltage_sensor_align, 0, angle);
if (i%50==0) {
because I have a 1 pole pair motor and without this, the standard SimpleFOC folder doesn’t succeed in init().
Sadly your update doesn’t work for me…

Just to update, I change the getAngle float to double and now I don’t have this precision overflow problem. I spanned the motor for 20 minutes at 1400 rad/sec with no problem.

The only problem now is the monitoring function at these speeds needs to be disabled or use downsampling of 2000 and above, or else the motor starts jittering because of too much calculations and printing to the serial port. Even with high downsampling I still get tiny hickups here and there. By the way this problem also occurs with the default library without any changes.

All of this is with 180MHz processor! (STM32F446RE)

I suggested, in my latest MR, that we use this abtract base class

The idea is to keep counts in integers until the very last moment. this should both improve performance and solve your issue. Using doubles will just push the issue further in time, while slowing down the code.

I think it would make most sense to replace all usages of float angle with a class such as:

struct FocAngle {
  int rotations;
  float angle;

With the appropriate overloads (i.e. adding conversions from/to float, operator+/operator-) we may be able to prevent breaking user code. This should fix all classes of precision errors, including rotating from position 999999 to 999999.01, which current code can’t do since 999999 == 999999.01. This solution seems cleaner than adding a whole array of new functions to the sensor base class.