Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Frans-Willem opened this issue Jul 17, 2019 · 6 comments
Open

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

Frans-Willem opened this issue Jul 17, 2019 · 6 comments

Comments

@Frans-Willem
Copy link

Frans-Willem commented Jul 17, 2019

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
Copy link
Author

Frans-Willem commented Jul 17, 2019

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
Copy link
Contributor

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
Copy link
Author

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
Copy link

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

@Frans-Willem
Copy link
Author

@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
Copy link
Contributor

casainho commented Aug 9, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants