Rework on calc_next_comm_period #71
Conversation
Fixed calc_next_comm_period_fast. Optimized calc_next_comm_period. Fixed a bug Fixed a bug
|
I set your other branch as base, so we only have the fix here to review ;-) |
|
Interesting. Does this change low RPM performance in any way or has it any other implications for #39? Or is it purely cosmetic and readability? Also I liked the analysis you did in the related issue, you think you could add this in the code too? Minus the divisor of 8 part obviously. |
| subb A, #0 | ||
| mov Temp4, A | ||
|
|
||
| ; Note: Temp2 is assumed to be zero (approx. Comm_Period4x_H / 4) |
There was a problem hiding this comment.
@stylesuxx This assumption was not correct. New commutation time H can be different to zero. In the reported bug on Blheli_S calculations were not correct, but in Bluejay this assumption was also not correct.
I added this to the restructuring branch |
This pr may improve that situation because it uses a single weighted mean instead of 3, depending on the situation, but until benchmarks are done I am not sure. |
Alright, so this is something we could maybe ask @ChrisRosser to benchmark for us on his setup, no? |
|
Superseeded by #73 - closed. |
calc_next_comm_period routine has been improved.
Now, when motor is started it uses the same code path for all speeds (and the same 1/16 weighted average for all) making commutation timing more stable. Performance is just the same, because optimized code paths were executed after many if then if checks. Also code is way simpler to understand, and to maintain, because it uses two main paths: one for when motor is starting (1/4 weighted average) and a second one for when motor is started (1/16 weighted average).