declare timer0_fract non static and volatile to make it available for linking

[kfessel]

using timer0_fract and Timer0s count register it's possible to write more
precise millis function that include all milliseconds or return some extra
accuracy (eg 1/4 on 1/8 millisecond) with less overhead than micros.

unsigned long millisx()
{
    //this includes all milliseconds (e.g. 42)
    extern unsigned long timer0_millis;
    extern uint8_t timer0_fract;
    unsigned long m;
    uint8_t oldSREG = SREG;
    uint8_t t;

    #if defined(TCNT0)
        t = TCNT0;
    #elif defined(TCNT0L)
        t = TCNT0L;
    #else
        #error TIMER 0 not defined
    #endif
    // disable interrupts while we read timer0_millis or we might get an
    // inconsistent value (e.g. in the middle of a write to timer0_millis)


    cli();
    m = timer0_millis;
    SREG = oldSREG;

    if( (timer0_fract >> 1) + (t >> 2) > 63) ++m;
    return m;
}

[matthijskooijman]

Commit looks good to me. It's not part of the official API, but it makes sense to at least allow access if people need it. Also, the other variables are not static, so this makes them more consistent. @fesselk, did you compare the generated .hex files before and after this change to see if the change influences the generated code? I would expect not, but perhaps this removes some optimization opportunities, so it would be good to check.

However, I wonder if your suggested millisx() function shouldn't just replace millis(). I was actually under the impression that millis() already consulted TCNT0 for an accurate return value, but it seems that in reality there is some jitter (with millis only advancing every 1024us, and then every 42ms or so, it advances by 2 at the same time, missing a beat).

@fesselk, any idea how much slower your implementation is compared to the original?

[kfessel]

I did a test before i did the commit and pull request and it showed no difference for the volatile vs the static. There was no optimizations that was possible due to it being static. ( the target platforms for my tests are the mega2560 (megaADK) and mega328 (uno) )

I just did a retest and had a look at the elf s and Assembler they are identical.

The millisx function that i suggested in my first comment is only for 16MHz (which both my testboard are) may be that is a problem for a replacing millis.

millis is 0x18 in size (24 byte ~ 12 cycles)
millisx is 0x40 in size (64 byte ~ 32 cycles)

since both have no loops the size roughly point to their execution time (AVR does about 2 bytes per cycle)

[darrylp1per]

surely a better way would be.....

#if defined(TCNT0)
  #define TIMER_0_COUNT TCNT0
#elif defined(TCNT0L)
  #define TIMER_0_COUNT TCNT0L
#else
  #error TIMER 0 not defined
#endif
#if defined(TIFR0)
  #define TIFRx TIFR0
#else
  #define TIFRx TIFR
#endif

and then inside the millis ( millisx) use this instead
if ((TIFRx & _BV(TOV0)) && (TIMER_0_COUNT < 255)) ms++;

then it works on diffent clock speeds, no longer being hard coded to the 16MHz values.

[matthijskooijman]

@darrylp1per, it seems there's two different problems being solved here? The original solved the problem where the timer0_fract, together with the current timer value, adds up to one more millisecond. Your suggestion only handles the fact where the timer overflowed, but was not processed yet.

Also, you should use triple backticks (```) for code blocks, or indent them by at least four spaces (I edited your comment for you).

[facchinm]

Closing here, if someone wants to adopt this PR please use this link to reopen it in the right repo, thanks.