Arduino Forum

Using Arduino => Programming Questions => Topic started by: vceklic on Jun 10, 2015, 07:22 am

Title: Creating a function
Post by: vceklic on Jun 10, 2015, 07:22 am
Hi all,

I am trying to create a function that would be used in my program as a generic timer. I have this code so far:

Code: [Select]
bool idleTime(int batteryNumber, unsigned long waitTime){
   bool retVal = false;
   long unsigned timePast = millis() - batPrevMillis[batteryNumber]; //keeping prev millis in array
   
   if( (long)( millis() - waitTime ) >= 0){ //check if time defined has passed
     retVal = true;
     batWaitTime[batteryNumber] = 0UL;
     batPrevMillis[batteryNumber] = 0;
   }
   else
   {        
     if(timePast > 0){
        batWaitTime[batteryNumber] += timePast;  
     }
     else{
        batWaitTime[batteryNumber] += millis();
     }
     batPrevMillis[batteryNumber] = millis();      
   }
   return retVal;
 }


What I am trying to achieve here is get true/false from function if time passed into function has finished. Looks like I am not resetting something  as numbers are off and increase in every loop pass. Any suggestions?

Moderator edit: [code] [/code] tags added.
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 08:26 am
The logic looks all wrong to me.

Don't you just want:

Code: [Select]

bool idleTime(int batteryNumber, unsigned long waitTime){
   if( millis() >= waitTime + batPrevMillis[batteryNumber]){ //check if time defined has passed
     batPrevMillis[batteryNumber] = millis();
     return true;
   }
   return false;
 }
Title: Re: Creating a function
Post by: Robin2 on Jun 10, 2015, 09:39 am
Code: [Select]
if( millis() >= waitTime + batPrevMillis[batteryNumber]){
does not allow for millis() rollover. You need to use subtraction as in
Code: [Select]
bool idleTime(int batteryNumber, unsigned long waitTime){
   if( millis()- batPrevMillis[batteryNumber] >= waitTime){ //check if time defined has passed
     batPrevMillis[batteryNumber] += waitTime; // prevents timing errors accumulating
     return true;
   }
   return false;
 }


...R
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 09:48 am
Code: [Select]
if( millis() >= waitTime + batPrevMillis[batteryNumber]){
does not allow for millis() rollover. You need to use subtraction as in
Code: [Select]

bool idleTime(int batteryNumber, unsigned long waitTime){
   if( millis()- batPrevMillis[batteryNumber] >= waitTime){ //check if time defined has passed
     batPrevMillis[batteryNumber] += waitTime; // prevents timing errors accumulating
     return true;
   }
   return false;
 }

I agree with this line for more accurate timing:
Code: [Select]

     batPrevMillis[batteryNumber] += waitTime; // prevents timing errors accumulating


but why do you say you need you need to use subtraction for this line?:

Code: [Select]

     if( millis() >= waitTime + batPrevMillis[batteryNumber])
Title: Re: Creating a function
Post by: sonyhome on Jun 10, 2015, 11:07 am
when you implement integer logic, you need to account for a value to overflow aka roll over.

millis will jump from maxint to zero suddenly all that while using integers.
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 11:14 am
when you implement integer logic, you need to account for a value to overflow aka roll over.

millis will jump from maxint to zero suddenly all that while using integers.
yes, but that doesn't explain why you need to use subtraction instead of addition.

I'm asking why he changed this line to a subtraction, since this handles the rollover as well:

Code: [Select]

if( millis() >= waitTime + batPrevMillis[batteryNumber])

As far as I can tell, the only necessary change was to stop the minor timing drift.
Title: Re: Creating a function
Post by: sonyhome on Jun 10, 2015, 11:19 am
you use unsigned, hence you generate 0-n > w, not n > w-0,  that is negative but cant be stored. compiler might flag the warning...
Title: Re: Creating a function
Post by: Robin2 on Jun 10, 2015, 11:28 am
but why do you say you need you need to use subtraction for this line?:
Because it still gives the correct answer if the interval spans the moment when the value of millis() changes from 232-1 back to 0 . It is one of the features of integer maths.

Try it manually with a byte that rollsover from (say) 253 to 007

...R
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 11:46 am
Because it still gives the correct answer if the interval spans the moment when the value of millis() changes from 232-1 back to 0 . It is one of the features of integer maths.

Try it manually with a byte that rollsover from (say) 253 to 007

...R
Mathematically, it also gives the correct answer with addition. It does not need to use subtraction. Try using addition, it's fine, no?

I do agree that the change to add the waitTime to the last stored time makes timing more accurate, but your claim that subtraction is necessary is inaccurate, so I'm trying to clear that up for everyone.

if millis is now rolled over to say 8,

if 8 > (2^32 - 4) + 6 is true, same as if 8 - (2^32 - 4) > 6
Title: Re: Creating a function
Post by: aarg on Jun 10, 2015, 02:13 pm
You're wrong. Subtraction is necessary. This subject comes up from time to time, and the conclusion (after arduous explanations) is always the same. Your theory look great on paper, but you need to test it, then you will see that it will fail on a roll over.
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 02:17 pm
You're wrong. Subtraction is necessary. This subject comes up from time to time, and the conclusion (after arduous explanations) is always the same. Your theory look great on paper, but you need to test it, then you will see that it will fail on a roll over.
Well, I can't test it as my arduino is not here, so please explain why it is wrong.
Title: Re: Creating a function
Post by: Delta_G on Jun 10, 2015, 02:28 pm
Well, I can't test it as my arduino is not here, so please explain why it is wrong.
You don't need your arduino.  You need a piece of paper.

What happens when the batPrevMillis number is 4,294,967,285 and waitTime is 20 and millis is at 4,294,967,286. 

4,294,967,285 + 20 is going to be 9.  So now you have if 4,294,967,286 > 9.  And yes, it is definitely greater.  But the whole interval hasn't passed yet has it?  You've only gone 1 millisecond.
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 02:58 pm
You don't need your arduino.  You need a piece of paper.

What happens when the batPrevMillis number is 4,294,967,285 and waitTime is 20 and millis is at 4,294,967,286. 

4,294,967,285 + 20 is going to be 9.  So now you have if 4,294,967,286 > 9.  And yes, it is definitely greater.  But the whole interval hasn't passed yet has it?  You've only gone 1 millisecond.

Ahhh. OK. That makes sense now. Thanks for explaining it.
Title: Re: Creating a function
Post by: vceklic on Jun 10, 2015, 06:14 pm
Thank you all. I understand part with subtraction, but just to clarify, waitTime is time in milliseconds that I pass into function, why do I need to add it to pastBatteryTime array value? Thanks for your help.
Title: Re: Creating a function
Post by: arduinodlb on Jun 10, 2015, 07:02 pm
Thank you all. I understand part with subtraction, but just to clarify, waitTime is time in milliseconds that I pass into function, why do I need to add it to pastBatteryTime array value? Thanks for your help.
It only gets added when you have passed the previous waitTime, so it forms the basis of the new starting time to check against.
Title: Re: Creating a function
Post by: vceklic on Jun 10, 2015, 08:10 pm
I have not given you enough explanation on what I am trying to do. I have 10 batteries that I am trying to work with hence batPrevMillis[batteryNumber] array to hold previous time for particular battery. I am just trying to pass time to function and do nothing until that time has passed. Example that you show does that but, when time passed I need to reset timer so that when I do some other operations I would call the function with same or different waitTime variable as previous time, so

batPrevMillis[batteryNumber] += waitTime;

does not do the trick.

Problem is that it is not always the same starting point. Any ideas?
Title: Re: Creating a function
Post by: Robin2 on Jun 10, 2015, 08:37 pm
Example that you show does that but, when time passed I need to reset timer so that when I do some other operations I would call the function with same or different waitTime variable as previous time, so

batPrevMillis[batteryNumber] += waitTime;

does not do the trick.

Problem is that it is not always the same starting point. Any ideas?
I don't understand and I am not sure what "example" you refer to. Please post the latest version of the code that is causing the problem.

The code quoted above assumes that the wait period is the same for each battery. But it should keep the times for each battery separate - in your Array.

If that is not what you want please explain clearly what is required.

It may be that your assumption that a "generic timer" is needed is not correct. Please explain what your project is trying to do.

...R
Title: Re: Creating a function
Post by: vceklic on Jun 10, 2015, 09:28 pm
OK, I can use array to store values for times but it is generated at runtime, so that is why I pass it into the function.

I will try to explain:

In void loop() if condition is met, I call function waitTime
then in each pass I check if given time is passed and just return false if time not passed, else return true and reset timer for that battery to 0.

Next pass into void loop() I do noting with this function since condition is not met.

Again, when condition (that might be after long time few hours or days) is met, I repeat the process, but this time waitTime might not be the same as last run. I store battery settings in array, so if needed I can add more parameters to it.

Hopefully this explains it a bit more.
Thanks and best regards.

 
Title: Re: Creating a function
Post by: vceklic on Jun 10, 2015, 10:15 pm
Thank you for helping. I have created small isolated program to simulate and I think I got it. I just hope that it would survive millis rollover. Here is the code:

long unsigned prevTime[] = {0UL, 0UL, 0UL}; //previous times for batteries
long unsigned waitTime[] = {5000UL, 8000UL, 22500UL};  //simulated times for batteries
int wt[] = {0,0,0}; //flag for battery timer status {0 = timer not running, 1 = timer running}

void setup() {
  // put your setup code here, to run once:
Serial.begin(9600);
}

void loop() {
  for(int x = 0; x <3; x++){
    if(wt
      prevTime
      Serial.println("Curent millis: " + String(prevTime
      wt
    }
    if(idleTime(x, waitTime
      Serial.println("Time has passed for battery " + String(x));     
    } 
  }
}


bool idleTime(int y, unsigned long waitTime){
   if( millis()- prevTime[y] >= waitTime){ //check if time defined has passed
     wt[y] = 0; //reset timer flag to not running
     return true;
   } 
   return false;
 }
Title: Re: Creating a function
Post by: Delta_G on Jun 10, 2015, 11:51 pm
Please use code tags.  The code in your last post got mangled with a bunch of little box symbols so I can't read it.  When you post code use the little icon above that looks like </> or manually add them so your code goes in a code box.  

Like this:
Code: [Select]


if(youAddCodeToAPost){
    pleaseUseCodeTags();
}

Title: Re: Creating a function
Post by: vceklic on Jun 11, 2015, 12:16 am
Sorry about that. Here is code:

Code: [Select]

long unsigned prevTime[] = {0UL, 0UL, 0UL, 0UL}; //previous times for batteries
long unsigned waitTime[] = {5000UL, 8000UL, 22500UL, 32000UL};  //simulated times for batteries
int wt[] = {0,0,0,0}; //flag for battery timer status {0 = timer not running, 1 = timer running}

void setup() {
  // put your setup code here, to run once:
Serial.begin(9600);
}

void loop() {
 
  
  
  for(int x = 0; x <3; x++){
    if(wt[x] == 0){ //if timer is not runnig
      prevTime[x] = millis(); //get current value of millis
      Serial.println("Curent millis: " + String(prevTime[x])); //debuging
      wt[x]= 1; //set timer to running
    }
    if(idleTime(x, waitTime[x])){
      Serial.println("Time has passed for battery " + String(x));     
    }  
  } 
  
    if(wt[3] == 0){
        prevTime[3] = millis(); //get current value of millis
        Serial.println("Curent millis: " + String(prevTime[3])); //debuging
        wt[3]= 1; //set timer to running
    }
    if(idleTime(waitTime[3])){
        Serial.println("Time has passed for random item");     
      }
  
}


bool idleTime(int y, unsigned long waitTime){
   if( millis()- prevTime[y] >= waitTime){ //check if time defined has passed
     wt[y] = 0; //reset timer flag to not running
     return true;
   }  
   return false;
 }
 
 bool idleTime(unsigned long waitTime){
   if( millis()- prevTime[3] >= waitTime){ //check if time defined has passed
     wt[3] = 0; //reset timer flag to not running
     return true;
   }  
   return false;
 }

Title: Re: Creating a function
Post by: Robin2 on Jun 11, 2015, 11:26 am
I think the code in Reply #20 should work.

I now have a better idea of how you want to use the function.

It is probably just personal preference but I think I would have created a batteryCheck() function that includes the timing question. Something like


Code: [Select]
void loop() {
   for (byte batt = 0; batt < 3; batt ++) {
      batteryCheck(batt);
   }
}
void batteryCheck(byte battNum) {
    if( millis()- prevTime[battNum] >= waitTime[battNum]) {
       prevTime[battNum] += waitTime[battNum];
       wt[battNum] = 0;
}


...R