openinverter forum

I have been using the Encoder class in inc_encoder.h/cpp as the basis of a resolver encoder for my Tesla M3 project. I've run into a couple of bugs in the way the class works that probably need looking at.

1) The first problem is an off by one error in the calculation of Encoder::GetFullTurns(). The poleCounter value that is in Encoder::UpdateRotorAngle() is updated by 1 more than the number of resolver pole pairs. So respolepairs = 1 it goes 0 then 1. For respolepairs = 3 it goes 0, 1, 2, 3. The number of turns is only incremented when poleCounter == 0. poleCounter should be reset with Param::GetInt(Param::respolepairs) - 1; The impact appears minimal as GetFullTurns() appears to be used for only for information purposes.

2) The turnsSinceLastSample variable is racily updated by both the main control loop through Encoder::UpdateRotorAngle() as well as the 100ms interrupt calling UpdateRotorFrequency(). I believe this will result in occasional glitches in the frequency calculation. Given the frequency calculation is fairly simple would it not be better to do this all in UpdateRotorAngle()? A counter could use the pwmFrq to calculate at the desired frequency. This would ensure that all accesses from outside of PWM motor control context are reads.

I'd offer to fix the code but I haven't got an STM32 inverter and motor HW so it would be pretty difficult to test.

1) Oh...
2) indeed, it is reset to 0 in UpdateRotorFrequency() and incremented in UpdateRotorAngle()
It isn't a practical problem as 32 bit accesses are atomic. But I still agree it isn't the best style. The only reason that UpdateRotorFrequency() is called from the 10ms task is to have sufficient time pass by between updates. Of course the same could be achieved by calling the function every Nth cycle of the PWM interrupt

Reads and writes are atomic but the increment in UpdateTurns(): turnsSinceLastSample += signedDiff; is Read-Modify-Write. I don't think you're running into problems because of the interrupt priority ensures that the PWM interrupt will never be interrupted by the reset in UpdateRotorFrequency(). Seems less than idea to depend on this but I can't see it ever breaking.

There are other racy comparisons/calculations in UpdateRotorFrequency() but they seem benign if they operate with inconsistent data.

Yeah that's right, that "scheduler" timer has the lowest possible priority

Agreed you'd loose the reset to 0 if the read-mod-write was interrupted after the read stage.