Global variable to store Millis() or multiple calls of Millis() in each fn?.

I want to know if I should have a global variable to store Millis and having my functions check that for timing. Or should I call multiple instances of Millis as CurrentMillis within each function? Is there anything I should be concern about having global Millis? (Using Arudino DUE)

Basically this,

unsigned long g_currentTimer;
unsigned long g_previousMillisFn1;
unsigned long g_previousMillisFn2;

void setup(){

// setup stuff

}

void loop(){

function1();
function2();

}

void function1(){

 if ((unsigned long)(currentMillis - g_previousMillisFn1) >= INTERVAL) {

// do something
g_previousMillisFn1;= currentMillis;
}

void function2(){

 if ((unsigned long)(currentMillis - g_previousMillisFn2) >= INTERVAL2) {

// do something
g_previousMillisFn2;= currentMillis;



}

OR

unsigned long g_previousMillisFn1;
unsigned long g_previousMillisFn2;

void setup(){

// setup stuff

}

void loop(){

function1();
function2();

}

void function1(){

unsigned long g_currentTimer;


 if ((unsigned long)(currentMillis - g_previousMillisFn1) >= INTERVAL) {

// do something
g_previousMillisFn1;= currentMillis;
}

void function2(){

unsigned long g_currentTimer;

 if ((unsigned long)(currentMillis - g_previousMillisFn2) >= INTERVAL2) {

// do something
g_previousMillisFn2;= currentMillis;


}

Only reason I would make it a global is if the timing calls for it. It millis got one-uped between the call of function1() and function2(), can thinks go wrong? In other words, is it necessary for millis to be the same in function1() and function2() each loop? Most of the time it's not so just skip the global then. I would drop the globals altogether...

void setup(){

  // setup stuff

}

void loop(){
  function1();
  function2();
}

void function1(){
  static unsigned long previousMillis;

  if(millis() - previousMillis >= INTERVAL) {
    previousMillis = millis();
    
    // do something
}

void function2(){
  static unsigned long previousMillis;

  if(millis() - previousMillis >= INTERVAL) {
    previousMillis = millis();
    
    // do something
}

I think most programmers prefer to encapsulate data as much as possible. That is, hide the data from the outside world and use it only where you need to use it. Indeed, I would move these definitions:

unsigned long g_previousMillisFn1; unsigned long g_previousMillisFn2;

inside of the functions where they are used and change them to:

void function1(){

   static unsigned long g_previousMillisFn1;

   if ((unsigned long)(currentMillis - g_previousMillisFn1) >= INTERVAL) {

Now the variable g_previousMillisFn1 is hidden in the function where it's used, but the static storage class modifier allows it to preserve it's old values between calls, which I think you may need to do.

I see nothing wrong with global variables. And I like the idea of using a single value of currentMillis for all calculations in a single iteration of loop().

...R

Thanks for reply guys,

I didn't know about the "static" storage class modifier, I'll put those into use.

How about in terms of memory? Having just one global declaration of current Millis() that occupies a static memory is better than having multiple local variables of currentMillis() in each function?

I'll probably use one global declartion of currentMillis(). Seems easier to understand and less copy pasting declaration of currentmillis() in each function that needs it. I'm not too worry about "outside world" since this is an embedded project I'm doing.

I use global variables in all my coding, and let any code act on it. If you look at my piano code here, see reply #4, you can see I use one call of currentTime = micros(); at the top of loop() and make 13 decisions about notes to transition from high to low, or low to high, based on that, to have 13 output tones. (used a '1284P chip, so 13 keys, 13 notes. 328P would be less, 2560 could be more, have to check responsiveness to keys being pressed tho) http://forum.arduino.cc/index.php?topic=179761.0

Here's the 1st note:

void loop(){
  //Serial.print (" ");
  // if key is pressed, determine time for next transition
  // if key is not pressed, no transition, use AC coupling into summing op-amp for no output
  // idea is to allow multiple notes to be created simultaneoulsy. Can the reading & if's all be done fast enough?
  portDkeys = PIND; // Read the input keys - using direct port manipulation for speed
  portBkeys = PINB; 
  currentTime = micros();

  // Didn't make this part of a larger loop/array lookup because that adds 12-15uS for each pass thru the loop

  /* Check if key for note C4 is pressed, key0  */
  
  if ((portDkeys & 0x04) == 0){ // key is pressed D2 -> A2
     if (keyActive[0]==0){
       keyActive[0] = 1;
       changeTime[0] = currentTime;
     }
    // see if time to change hi to lo, or lo to hi
    if ( (currentTime - changeTime[0])>=noteArray[48]){ // time for a period toggle?
      changeTime[0] = changeTime[0] + noteArray[48];    // setup time for the next toggle
      PINA=0x04;  // write a 1 to PINx toggles an output bit - NickGammon to the rescue again!
    }
  }
  else{keyActive[0] = 0;}
// next key ...

PDK91: How about in terms of memory? Having just one global declaration of current Millis() that occupies a static memory is better than having multiple local variables of currentMillis() in each function?

Making a currentMillis (no (), it's not a function) will take up more space. Because you don't need to make a local variable unless 1ms error is not allowed (but in a lot of uses I can't be bothered). You just use millis() where you need it:

if(millis() - previousMillis >= INTERVAL) {
  previousMillis = millis();
}

Maybe you are off by 1ms because between the two calls to millis() may have been one upped. But for things like blinking an LED it's no biggy.

If you do make it a local variable in every function it doesn't matter. You don't make it a static so the variable is cleaned after the function ends.

And I think it's better to make the previousMillis a static inside the function. If you only use the global variable in one function, just make it a static in that function. Then you group the variable where it is needed. And you don't end up with a long list of previousMillis1, previousMillis2 etc. You end up with more readable code.

I see nothing wrong with global variables.

There's nothing wrong with global variables. However, if they are used in only one function, why make them global? The whole idea behind encapsulation is to hide the data so it can't be contaminated by outside agents. If a variable is global, but used in multiple functions, it's more difficult to find out where the variable is being contaminated. If the variable has non-global scope, at least you have some idea where to start looking for what's causing it to assume a bogus value.

One point relative to the compiler.
It is better to create a local/global variable for millis() (assuming you need several instances of the same time) because the compiler has to call millis() each time since it has to assume that the value of millis() might change with each call.

A variable allows the compiler to optimise to minimize the number of references to the variable, or even common subexpressions such as current_time - old_time, which might only be calculated once.

That is true. But yeay, the overhead isn't that much. Otherwise we wouldn't create functions at all... But making or not making a current var is a choice. If you make it, there is no difference in making it global of local in each function. The local variable is only created when the function is called and destroyed when the function call ends.

Global variables are evil(tm) and often (usually?) unnecessary***.

Forget the compiler optimization and memory use. We don't write code for machines we write code for the humans reading it. (If you don't think that's true, please start coding in byte code immediately; and no I don't mean assembly, I mean byte code).

In non time-critical applications (and lets face it - if you're using Arduino or millis(), it's not time critical ;) ), an explicit call to millis() is usually much more readable and less prone to errors. The problem with global variables is maintaining state, avoid multi-access issues/problems and clutter/readability.

If all the depended functions you're calling from the loop rely on the fact that they all need to be called with exactly the same timestamp, you can pass the timestamp in as a function parameter as well. That would also be cleaner than a global variable since it will be obvious why/how the parameter is needed.


*** Yes, I am aware - exceptions prove the rule and in any given program a global variable will sneak in somewhere eventually.