bitdump
a bug with calc_next_comm_timing_fast
calc_next_comm_timing_fast: ; Calculate new commutation time mov Temp3, Comm_Period4x_L ; Comm_Period4x(-l-h) holds the time of 4 commutations mov Temp4, Comm_Period4x_H mov A, Temp4 ; Divide by 2 4 times swap A mov Temp7, A mov A, Temp3 swap A anl A, #0Fh orl A, Temp7 mov Temp5, A
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; Comm_Period4x/16 is stored in temp5 with above code ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
clr C
mov A, Temp3 ; Subtract a fraction
subb A, Temp5
mov Temp3, A
mov A, Temp4
subb A, #0
mov Temp4, A
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; 15*Comm_Period4x/16 is stored in temp5 with above code ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
clr C
mov A, Temp1
rrc A ; Divide by 2 2 times
clr C
rrc A
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; Tcomm/4 is object that the code want to calculate,and newest commutation period is stored in temp1 and temp2. In the calc_next_comm_timing_fast label, high rpm flag is set,and Temp2 is less than 2,but it's not zero. So in above code,the result with Temp1 right shifted 2 is not Tcomm / 4. There is a bug! ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
mov Temp1, A
mov A, Temp3 ; Add the divided new time
add A, Temp1
mov Temp3, A
mov A, Temp4
addc A, #0
mov Temp4, A
mov Comm_Period4x_L, Temp3 ; Store Comm_Period4x_X
mov Comm_Period4x_H, Temp4
clr C
mov A, Temp4 ; If erpm below 156k - go to normal case
subb A, #2
jc ($+4)
clr Flags1.HIGH_RPM ; Clear high rpm bit
; Set timing reduction
mov Temp1, #2
mov A, Temp4 ; Divide by 2 4 times
swap A
mov Temp7, A
mov Temp4, #0
mov A, Temp3
swap A
anl A, #0Fh
orl A, Temp7
mov Temp3, A
clr C
mov A, Temp3
subb A, Temp1
mov Temp3, A
jc load_min_time_fast ; Check that result is still positive
clr C
subb A, #1
jnc calc_new_wait_times_fast_done ; Check that result is still above minumum
Good catch, I think you are right. Although I believe some of your comments are not accurate.
Wouldn't this fix it:?
clr C
mov A, Temp3 ; Subtract a fraction
subb A, Temp5
mov Temp3, A
mov A, Temp4
subb A, #0
mov Temp4, A
; Incorrect, need to consider LSB of Temp2
;clr C
; Corrected, storing LSB of Temp2 in C
mov A, Temp2
rrc A
mov A, Temp1
rrc A ; Divide by 2 2 times
clr C
rrc A
mov Temp1, A
mov A, Temp3 ; Add the divided new time
add A, Temp1
mov Temp3, A
mov A, Temp4
addc A, #0
mov Temp4, A
mov Comm_Period4x_L, Temp3 ; Store Comm_Period4x_X
mov Comm_Period4x_H, Temp4
Good catch, I think you are right. Although I believe some of your comments are not accurate.
Wouldn't this fix it:?
clr C mov A, Temp3 ; Subtract a fraction subb A, Temp5 mov Temp3, A mov A, Temp4 subb A, #0 mov Temp4, A ; Incorrect, need to consider LSB of Temp2 ;clr C ; Corrected, storing LSB of Temp2 in C mov A, Temp2 rrc A mov A, Temp1 rrc A ; Divide by 2 2 times clr C rrc A mov Temp1, A mov A, Temp3 ; Add the divided new time add A, Temp1 mov Temp3, A mov A, Temp4 addc A, #0 mov Temp4, A mov Comm_Period4x_L, Temp3 ; Store Comm_Period4x_X mov Comm_Period4x_H, Temp4
Good,your code is right. what's wrong of my comment ?
Not: "15Comm_Period4x/16 is stored in temp5 with above code". Should be "15Comm_Period4x/16 is stored in Temp4/3 with above code"
The question now, is what do we do about this? I feel it is not sufficient for generating a new revision (16.8), What about just generating an update to the current rev16.7? Not a clean way of doing revs - traceability will be lost - but maybe ok in this case?
can calc_next_comm_normal,calc_next_comm_startup,and calc_next_comm_timing_fast has same next called function calc_new_wait_times_setup?
These three label all has same operation as followed: temp3/4 = Comm_Period4x >>4 if temp3/4 > 2 temp3/4 = temp3/4 - 2; end
if temp3/4 < 1 temp3/4 = 1 end
calc_new_wait_per_demag_done:
; Set timing reduction
mov Temp7, #2
; Load current commutation timing
mov A, Comm_Period4x_H ; Divide 4 times
swap A
anl A, #00Fh
mov Temp2, A
mov A, Comm_Period4x_H
swap A
anl A, #0F0h
mov Temp1, A
mov A, Comm_Period4x_L
swap A
anl A, #00Fh
add A, Temp1
mov Temp1, A
> clr C
> mov A, Temp1
> subb A, Temp7
> mov Temp3, A
> mov A, Temp2
> subb A, #0
> mov Temp4, A
> jc load_min_time ; Check that result is still positive
>
> clr C
> mov A, Temp3
> subb A, #1
> mov A, Temp4
> subb A, #0
> jnc calc_new_wait_times_exit ; Check that result is still above minumum
>
> load_min_time:
> mov Temp3, #1
> clr A
> mov Temp4, A
; Fast calculation (Comm_Period4x_H less than 2)
calc_next_comm_timing_fast:
; Calculate new commutation time
mov Temp3, Comm_Period4x_L ; Comm_Period4x(-l-h) holds the time of 4 commutations
mov Temp4, Comm_Period4x_H
mov A, Temp4 ; Divide by 2 4 times
swap A
mov Temp7, A
mov A, Temp3
swap A
anl A, #0Fh
orl A, Temp7
mov Temp5, A
clr C
mov A, Temp3 ; Subtract a fraction
subb A, Temp5
mov Temp3, A
mov A, Temp4
subb A, #0
mov Temp4, A
clr C
mov A, Temp1
rrc A ; Divide by 2 2 times
clr C
rrc A
mov Temp1, A
mov A, Temp3 ; Add the divided new time
add A, Temp1
mov Temp3, A
mov A, Temp4
addc A, #0
mov Temp4, A
mov Comm_Period4x_L, Temp3 ; Store Comm_Period4x_X
mov Comm_Period4x_H, Temp4
clr C
mov A, Temp4 ; If erpm below 156k - go to normal case
subb A, #2
jc ($+4)
clr Flags1.HIGH_RPM ; Clear high rpm bit
> ; Set timing reduction
> mov Temp1, #2
> mov A, Temp4 ; Divide by 2 4 times
> swap A
> mov Temp7, A
> mov Temp4, #0
> mov A, Temp3
> swap A
> anl A, #0Fh
> orl A, Temp7
> mov Temp3, A
> clr C
> mov A, Temp3
> subb A, Temp1
> mov Temp3, A
> jc load_min_time_fast ; Check that result is still positive
>
> clr C
> subb A, #1
> jnc calc_new_wait_times_fast_done ; Check that result is still above minumum
>
> load_min_time_fast:
> mov Temp3, #1
There are some confusion about timer3 interrupt process function. Timer3 reload register TMR3RLL/TMR3RLH is reset value with 0xfffa. It means that when timer3's interrupt is triggered,it's reload register is set to 0xfffa. But in label of wait_advance_timing,setup_comm_wait and wait_for_comm_wait,the realod register of timer3 is set. Does it work?
;**** **** **** **** **** **** **** **** **** **** **** **** ****
;
; Timer 3 interrupt routine
;
; No assumptions
; Requirements: Temp variables can NOT be used since PSW.x is not set
; ACC can not be used, as it is not pushed to stack
;
;**** **** **** **** **** **** **** **** **** **** **** **** ****
t3_int: ; Used for commutation timing
clr IE_EA ; Disable all interrupts
anl EIE1, #7Fh ; Disable timer 3 interrupts
mov TMR3RLL, #0FAh ; Set a short delay before next interrupt
mov TMR3RLH, #0FFh
clr Flags0.T3_PENDING ; Flag that timer has wrapped
anl TMR3CN0, #07Fh ; Timer 3 interrupt flag cleared
setb IE_EA ; Enable all interrupts
reti
;**** **** **** **** **** **** **** **** **** **** **** **** ****
;
; Wait advance timing routine
;
; No assumptions
; NOTE: Be VERY careful if using temp registers. They are passed over this routine
;
; Waits for the advance timing to elapse and sets up the next zero cross wait
;
;**** **** **** **** **** **** **** **** **** **** **** **** ****
wait_advance_timing:
jnb Flags0.T3_PENDING, ($+5)
ajmp wait_advance_timing
; Setup next wait time
mov TMR3RLL, Wt_ZC_Tout_Start_L
mov TMR3RLH, Wt_ZC_Tout_Start_H
setb Flags0.T3_PENDING
orl EIE1, #80h ; Enable timer 3 interrupts
;**** **** **** **** **** **** **** **** **** **** **** **** ****
;
; Setup commutation timing routine
;
; No assumptions
;
; Sets up and starts wait from commutation to zero cross
;
;**** **** **** **** **** **** **** **** **** **** **** **** ****
setup_comm_wait:
clr IE_EA
anl EIE1, #7Fh ; Disable timer 3 interrupts
mov TMR3CN0, #00h ; Timer 3 disabled and interrupt flag cleared
mov TMR3L, Wt_Comm_Start_L
mov TMR3H, Wt_Comm_Start_H
mov TMR3CN0, #04h ; Timer 3 enabled and interrupt flag cleared95`
; Setup next wait time
mov TMR3RLL, Wt_Adv_Start_L
mov TMR3RLH, Wt_Adv_Start_H
setb Flags0.T3_PENDING
orl EIE1, #80h ; Enable timer 3 interrupts
setb IE_EA ; Enable interrupts again
wait_for_comm_wait:
jnb Flags0.T3_PENDING, ($+5)
ajmp wait_for_comm_wait
; Setup next wait time
mov TMR3RLL, Wt_Zc_Scan_Start_L
mov TMR3RLH, Wt_Zc_Scan_Start_H
setb Flags0.T3_PENDING
orl EIE1, #80h ; Enable timer 3 interrupts
ret
I'm not sure I understand what you point to above. But note that there is no longer any development on BLHeli_S. For new developments/optimizations please check out the Bluejay repo that is a further development of BLHeli_S.
@garlinplus Thanks for tracking this kind of bugs. Has you been able to test the fix? Do you think it could also affect Bluejay?
