search

OpenSourceEBike / TSDZ2-Smart-EBike

Flexible OpenSource firmware for TongSheng TSDZ2 mid drive ebike motor
GNU General Public License v3.0
255 stars 131 forks source link

Possibly two off-by-one errors in motor controller? #109

Open Frans-Willem opened 5 years ago

Frans-Willem commented 5 years ago

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: svm_compared 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 ?

Frans-Willem commented 5 years ago

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.

casainho commented 5 years ago

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.

Frans-Willem commented 5 years ago

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.

geeksville commented 5 years ago

@Frans-Willem I'd love to use your fix also!

Frans-Willem commented 5 years ago

@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.

casainho commented 5 years ago

Really nice that piece of code for 9 bits PWM!! I am always learning, so good opportunities.