Quick note on the magneticsensorPWM.cpp file - needs a modification to prevent NaN and other borkedness

I just want to drop a quick note, I discovered an error that prevents PWM sensors from operating properly (the hard way :().

There appears to be at least 2 errors in the file.

one is on line 48, I forget what it used to say , I think it divided the pulse length by the counts per rotation. However the problem is that the pulse length is never exactly right, thus the maximal rotational angle can easily exceed the counts per rotation, even when you use the sensor pulse length min/max checking example.

This leads to errors later on as the getrawAngle thing returns a rotation of more than 2pi rotations pre revolution sometimes.

This leads to a NaN when the sensor calibration is used, in particular, which propagates and makes everything else break…

I was able to correct it by going

    if (pulse_length_us>(cpr-1)){
        pulse_length_us = (cpr-1);
    }

cpr is for counts per rotation, and is the number of microseconds a full length pulse is supposed to be. I made it cpr-1 just in case of rounding issues when things get converted to a float or whatever.

The second error appears to be on line 61, it said

if (!digitalRead(pinPWM)) pulse_length_us = now_us - last_call_us;

But I’m thinking it should say

if (!digitalRead(pinPWM)) pulse_length_us = (now_us - last_call_us)-min_raw_count;

The sensor doesn’t give a pulse length from zero to full, it has a minimal pulse length that corresponds to zero angle. Thus, what we want is surely the difference between the measured pulse and the minimum, I think looking at the datasheet of the AS5600 this is right yes?

Yes, you’re right, this change is already present in the dev branch of SimpleFOC… except it is solved in the getSensorAngle() method…

Yeah, I really recommend against using the PWM sensor…

but you are correct that we should add a check to make sure it does not return values >2PI… the other sensor classes just never do this, I guess. I will look into adding such a check for the next release.

If you want to go 3000RPM, that’s 1rad in 0.003s if I calculated that right. That’s only 3ms for one rad if of angle. You need to update the sensor several times per revolution to ensure the full rotations are counted correctly, every 15ms at least if I calculated that right.

1 Like

Hey @Anthony_Douglas,

So both of these changes are now on the dev branch if you wanted to try it - the limit to <=2PI and the subtraction of the min_value (which was there already).

1 Like