Possibly two off-by-one errors in motor controller?
Opened this issue · 6 comments
I've been studying the motor controller code, and I found two instances where the code is off-by-one from what I would have expected. This could mean that either I'm misunderstanding something, or there are indeed off-by-one errors in there.
The first one is in the svm_table. I've written code to generate that I believe should have been in the table, and I noticed that the current values are offset by one index, as can be seen from the following graph:
The code used to generate the table & plot the results can be found here.
("Calculated" is the output of the python code, "Current" is what's currently in motor.c. Not related to electrical current ;))
Again, it's also possible the error is in my python code, and I'm open to feedback on the way I try to generate this table.
I suspect an error was made where the table was generated using indices ranging from 1 to 256, instead of 0 to 255 (as the code is using it).
The second one is in motor.c on line 610: The comments state it's rotating by 90 degrees (in the 360-degree total rotation world), which should coincide with 64 'binary'-degrees (in the 256 total rotation world), but instead 63 is subtracted.
I suspect that there might've been an error in calculating this value, where 255 was used instead of 256. e.g. (90/360)*255=63.75, rounded down to 63, instead of t(90/360)*256=64...
Would someone be able to verify these findings ?
Forgot a quick note:
These errors don't cancel each other out, as they're both in the same direction. If both these findings are correct, this means that the code is currently calculating (62/256)*360 = 87.1875 degrees ahead instead of the desired 90 degrees.
For the first point maybe you are right. As there is an offset I found experimentally, I guess it solves that shift by 1 unit:
// This value should be near 0.
// You can try to tune with the whell on the air, full throttle and look at batttery current: adjust for lower battery current
#define MOTOR_ROTOR_OFFSET_ANGLE 10
For 2, you are probably right and I do some errors on calculations :-)
I suggest you do a pull request. Thank you.
Thanks for the response :)
Since I branched off from 0.19.0, my experimentation branch & mainline have already quite diverged, but I'll definitely see if I can cherry-pick the motor related changes into a pull-request once I'm done experimenting.
@Frans-Willem I'd love to use your fix also!
@geeksville All motor related stuff I've done so far is in this branch:
https://github.com/Frans-Willem/TSDZ2-Clean-EBike/tree/frans-willem/motor-code-revision
So far I've converted the hard-coded SVPWM table to one generated by a python script, fixed the off-by-ones described in this issue, and bumped the PWM up to 9 bits. I've checked the assembly output, and believe the generated code will be faster than the 18.0 implementation, but I have not yet been able to verify this with a scope.
Note however that I have not had the time to re-check MOTOR_ROTOR_ANGLE_OFFSET as described by @casainho, and I won't be able to until early September.
Really nice that piece of code for 9 bits PWM!! I am always learning, so good opportunities.