7 Replies Latest reply on Jun 20, 2015 9:38 AM by AlexT_Intel

    millis() appears to have been implemented incorrectly

    wolfsong

      Hi,

        Was trying to do some long benchmarks to eliminate overhead & jitter and was using millis() to log elapse time. Noticed when I ran tests which went more than 71 minutes, times came out funny.

       

      I took the arduino millis() example and added several displays plus grabbing the micros() value and displaying it as well. Sure enough when 71 minutes (1 hour & 11 minutes) came around both micros and millis rolled over together!

       

      From this I am assuming that the millis counter is just a reduced copy of the micros counter rather than counting in millis so that it can rollover in 49 days as advertised.

       

      If I am right, is this going to be fixed any time soon? I would like to run timings longer that 71 minutes. I attached a copy of the code I used running under arduino on the galileo.

       

      If I goofed, please let me know. Thanks,

      =Gerry=

        • 1. Re: millis() appears to have been implemented incorrectly
          AlexT_Intel

          As far as I can see in arduino-1.5.3\hardware\arduino\x86\cores\arduino\UtilTime.cpp, millis() is simply

           

          return micros() / 1000;
          

           

          So your guess is correct.

           

          And micros() is essentially:

           

          struct timespec t;
            t.tv_sec = t.tv_nsec = 0;
          clock_gettime(CLOCK_REALTIME, &t);
            return (unsigned long)(t.tv_sec)*1000000L + t.tv_nsec / 1000L ;
          

           

          As you can see, unsigned long type is used as micros() return type. Which means it will roll over every 4294967295 (max unsigned long value for 32-bit system) /1000000 (microseconds in a second) /60 (seconds in a minute) = 71.58 minutes, exactly what you've seen and exactly how it's advertised on the Arduino website.

           

          But millis() looks like off indeed, due to that reuse of micros().

          Probably the simplest way to fix that would be to replace the millis() code with the following, you can do it yourself in the abovementioned file, it will get compiled and included into the sketch the next time you compile it.

           

          struct timespec t;
            t.tv_sec = t.tv_nsec = 0;
          clock_gettime(CLOCK_REALTIME, &t);
            return (unsigned long)(t.tv_sec)*1000L + t.tv_nsec / 1000000L
          

           

          This will roll over in 4294967295/1000/3600/24=49.7 days, which is almost exactly those 50 days mentioned on the Arduino site.

           

          If anyone from the Galileo team is reading this - please feel free to use the code above as a fix if you think it's worth it. Or tell me if there's any place I could submit this as a bug record.

          • 2. Re: millis() appears to have been implemented incorrectly
            Intel_Jesus

            Hi AlexT_Intel/wolfsong,

             

            I will escalate the bug to the appropriate team within Intel.

            Thanks for spoting this bug.

             

            Regards,

            Intel_JEspinoza

            • 3. Re: millis() appears to have been implemented incorrectly
              AlexT_Intel

              Thanks! If you could look into the thread regarding ISRBlink example, there's a similar case with a bug and proposed fix, appreciate if you could escalate that one too.

              • 4. Re: millis() appears to have been implemented incorrectly
                sfigueroa

                Hi Alex you really helped me with your post. I was having prombles to comunicate Galileo Board with Scratch for Arduino (S4A), and I used your implementation of millis and it inmediatly worked. I found that an easy way to make my code portable when I change computer was to "override" the function millis inside my code doing this:

                 

                 

                #define millis millis2

                unsigned long millis2( void )

                {

                struct timespec t; 

                  t.tv_sec = t.tv_nsec = 0; 

                clock_gettime(CLOCK_REALTIME, &t); 

                  return (unsigned long)(t.tv_sec)*1000L + t.tv_nsec / 1000000L;

                }

                 

                I think this could be helpfull to someone.

                • 5. Re: millis() appears to have been implemented incorrectly
                  AlexT_Intel

                  Glad it helped It's interesting though that you had to do something like that - I'd have hoped it would be fixed in the Arduino IDE by now. Which version are you using?

                  • 6. Re: millis() appears to have been implemented incorrectly
                    sfigueroa

                    Hi AlexT_Intel, I am using 1.6.0 Version. I think it is the last version.  So It seems that it still hasn`t been fixed.

                    • 7. Re: millis() appears to have been implemented incorrectly
                      AlexT_Intel

                      I've looked into this and actually it's strange that you have this problem in the first place. I can see that in 1.6.0 they indeed fixed the thing originally reported in this thread, which is millis() result rolling over each ~70 minutes (like the micros() one) instead of something ~50 days as it should.

                       

                      Looking at the 1.6.0 sources I see that they are now using a slightly different method of getting the time (from TSC instead of RTC), but the most important thing is that they're using 64-bit numbers for calculation and only then converting that to 32-bit, so this rollover we saw initially shouldn't happen earlier than ~50 days, as intended.

                       

                      All in all I believe it *is* fixed right now, so whatever problem you saw it should've been something else. If you want to dig into this, please post your original code you had troubles with and the error you faced and we'll go from there.