Skip to content

Commit 0d89643

Browse files
authored
Correct longstanding timing bug in pulseIn()
Prior to seeing the pulse start, the timeout would tick by at 16/11ths of the speed it should because the first two loops - the first two loops aren't also incrementing width, which requires 5 cycles, so of course they'll run faster! This change adds 5 cycles of nops to each loop (3 instructions - 2 rjmp+0, 1 nop). This cost 6 instruction words of flash - however, the compiler missed a chance to share the set of pop instructions between the two paths to return; doing this saved 6 instruction words, getting us back the flash the timing fix took away.
1 parent fc5bd7f commit 0d89643

File tree

1 file changed

+20
-9
lines changed

1 file changed

+20
-9
lines changed

cores/arduino/wiring_pulse.S

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
wiring_pulse.s - pulseInASM() function in different flavours
33
Part of Arduino - http://www.arduino.cc/
44

5-
Copyright (c) 2014 Martino Facchin
5+
Copyright (c) 2014 Martino Facchin, 2020 Spence Konde
66

77
This library is free software; you can redistribute it and/or
88
modify it under the terms of the GNU Lesser General Public
@@ -47,6 +47,14 @@
4747
* }
4848
*
4949
* some compiler outputs were removed but the rest of the code is untouched
50+
*
51+
* Spence feb 2020: not untouched anymore! The first two loops ran in 11 cycles instead
52+
* of 16, so if no pulse was detected, the timeout would be reached when
53+
* 11/16ths of the requested timeout had elapsed. This was fixed by the addition
54+
* of 2 rjmps to the next line (a 2 cycle nop that uses only 1 word) and 1 nop
55+
* to each of these loops before they decrement maxloops.
56+
* Additionally, removed duplication of return sequence to save 12b flash
57+
* which is conveniently exactly how much the other fix cost.
5058
*/
5159

5260
#include <avr/io.h>
@@ -80,6 +88,11 @@ countPulseASM:
8088
.L4:
8189
/* if (--maxloops == 0) */
8290
.LM2:
91+
rjmp .LM2A ; waste an extra 5 cycles
92+
.LM2A:
93+
rjmp .LM2B ;
94+
.LM2B:
95+
nop ;
8396
subi r16,1 ; maxloops, ; 17 addsi3/2 [length = 4]
8497
sbc r17, r1 ; maxloops
8598
sbc r18, r1 ; maxloops
@@ -101,6 +114,11 @@ countPulseASM:
101114
*** if (--maxloops == 0)
102115
*/
103116
.LM4:
117+
rjmp .LM4A ; waste an extra 5 cycles
118+
.LM4A:
119+
rjmp .LM4B ;
120+
.LM4B:
121+
nop ;
104122
subi r16,1 ; maxloops, ; 31 addsi3/2 [length = 4]
105123
sbc r17, r1 ; maxloops
106124
sbc r18, r1 ; maxloops
@@ -152,15 +170,8 @@ countPulseASM:
152170
mov r23,r13 ; D.1553, width ; 109 movqi_insn/1 [length = 1]
153171
mov r24,r14 ; D.1553, width ; 110 movqi_insn/1 [length = 1]
154172
mov r25,r15 ; D.1553, width ; 111 movqi_insn/1 [length = 1]
173+
rjmp .LM11 ;
155174
/* epilogue start */
156-
.LM9:
157-
pop r17 ; ; 171 popqi [length = 1]
158-
pop r16 ; ; 172 popqi [length = 1]
159-
pop r15 ; ; 173 popqi [length = 1]
160-
pop r14 ; ; 174 popqi [length = 1]
161-
pop r13 ; ; 175 popqi [length = 1]
162-
pop r12 ; ; 176 popqi [length = 1]
163-
ret ; 177 return_from_epilogue [length = 1]
164175
.L13:
165176
.LM10:
166177
ldi r22,0 ; D.1553 ; 120 movqi_insn/1 [length = 1]

0 commit comments

Comments
 (0)