Minor miscalculation in delayMicroseconds() #130
Comments
|
Thanks for reporting. I don't have time to look into this right now, but do you want to propose a fix, so that timing is accurate (and perhaps test it on actual hardware)? |
|
Need some tests compilation and disassembling to view the effect of volatile. I think, the solution is: us <<= 1; us -= ?; |
|
Voliatile variable was not a good idea. Compiler generate too complex and big code. But there is another problem even not using voliatile variable. If delayMicroseconds() called with constants only compiler generate function call free code. This example is from a complex code using timing in Onewire library. Need more tricks. Thinking...
compiled to:
|
|
Here are some compiled and disassembled codes. Link time optimization (LTO) must be disabled. |
|
This comment was edited, because I realized the code was not as precise as I thought at first. Now this code really precise. (If your MCU is a Chinese LGT8Fx, you have to replace 4000000L to 3000000L ) |
|
The code above has an error. Higher values of requested microseconds cause compiler 32 bit overflow. |
|
I created a new version of delayMicroseconds() function. I would be happy if someone check it: https://github.com/LaZsolt/delayMicroseconds/tree/master/for_MCUdude/function |
|
Sorry for staying quiet for so long. I'll see if I can look into it tonight. But thanks for your work! Improvements are always welcome 🙂 |
|
so, what would you recommend using? #define delayMicroseconds(us) \
(__extension__({ \
__asm__ __volatile__ ( \
"usL_%=:" "sbiw %0,1" "\n\t" \
"brne usL_%=" \
: /* no outputs */ \
:"w" ( (uint16_t) (( (F_CPU/1000) * (uint32_t) us ) / 4000L) ) \
); \
}))is of course way shorter, but I'm so inline assembly guy, so I can't really tell what's going on. Will the function above work for all F_CPU values? How about the code provided at https://github.com/LaZsolt/delayMicroseconds/tree/master/for_MCUdude/function? Speaking of the code here: https://github.com/LaZsolt/delayMicroseconds/tree/master/for_MCUdude/function |
|
Yes, this simple inline assembly above will work all F_CPU values. ArduinoCore-samd delay.h defines almost the same code for delayMicroseconds().
I tested it in my IOT web temperature sensor on 16 MHz, with DS18B20 sensors and ENC28J60 ehernet controller. Calculated cyles and delays in microseconds:
|
|
For MiniCore, and all the other Arduino cores for ATmega's (Mighty/Mega/MajorCore), I think it's best to keep compatibility and use the function you provided instead of the macro. I bet there is a lot of code out there that depends on delay times that are not known at compile time. The tables you posted above, is this data from the macro or from the function? |
|
I connected my trusty signal generator to the clock input of the AVR development board I'm using to get a very accurate clock. I made a simple sketch that toggles a pin by witing to PINB. I tested your delayMicroseconds code with all clocks MiniCore supports, and it looks very good. I'm going to merge this code into MCUdude_corefiles. This way the new delayMicroseconds function will be available on MightyCore, MiniCore, MegaCore, and MajorCore. Thanks for your contribution! |
Thanks to @LaZsolt for the contribution. Discussion here: MCUdude/MiniCore#130
|
The updated delayMicroseconds function will be available in the next MiniCore release! |
…ffdd bfadeffdd Remove tab 88cc4bfb4 Improve delayMicroseconds accuracy Thanks to @LaZsolt for the contribution. Discussion here: MCUdude/MiniCore#130 d81bb03ef Improve set-bit/clear-bit logic Usually, the sbo and cbi macros didn't compile down to a single instruction. The new implementation increases speed and reduces the compiled size 6977bd002 Improve formatting By adding names to parameters, it's easier to determine what each parameter is for in certain editors like VScode without having to look it up git-subtree-dir: avr/cores/MCUdude_corefiles git-subtree-split: bfadeffdd4b2e445c5bccfb97dd11d5c57ccc58b
…ffdd bfadeffdd Remove tab 88cc4bfb4 Improve delayMicroseconds accuracy Thanks to @LaZsolt for the contribution. Discussion here: #130 d81bb03ef Improve set-bit/clear-bit logic Usually, the sbo and cbi macros didn't compile down to a single instruction. The new implementation increases speed and reduces the compiled size 6977bd002 Improve formatting By adding names to parameters, it's easier to determine what each parameter is for in certain editors like VScode without having to look it up f18d41698 Add PROGMEM1..3 Makes it much easier to place and access stuff in far memory git-subtree-dir: avr/cores/MCUdude_corefiles git-subtree-split: bfadeffdd4b2e445c5bccfb97dd11d5c57ccc58b
bfadeffd Remove tab 88cc4bfb Improve delayMicroseconds accuracy Thanks to @LaZsolt for the contribution. Discussion here: MCUdude/MiniCore#130 d81bb03e Improve set-bit/clear-bit logic Usually, the sbo and cbi macros didn't compile down to a single instruction. The new implementation increases speed and reduces the compiled size 6977bd00 Improve formatting By adding names to parameters, it's easier to determine what each parameter is for in certain editors like VScode without having to look it up f18d4169 Add PROGMEM1..3 Makes it much easier to place and access stuff in far memory git-subtree-dir: avr/cores/MCUdude_corefiles git-subtree-split: bfadeffdd4b2e445c5bccfb97dd11d5c57ccc58b
Hi!
I found a minor miscalculation in wiring.c and delayMicroseconds()
In case of 32 MHz clock speed, at line 201 microseconds multiplied by 8 takes 15 clock cycles.
us <<= 3; // x8 us, = 6 cycles not trueThe assembly code:
63 e0 ldi r22, 0x03 ; 1 clock cycle88 0f add r24, r24 ; 3 x 1 clock cycles99 1f adc r25, r25 ; 3 x 16a 95 dec r22 ; 3 x 1e1 f7 brne .-8 ; 2 x 2 + 1; Sum 15 Cycles
The text was updated successfully, but these errors were encountered: