Creating a function

Using Arduino Programming Questions
1

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:

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:
</mark> <mark>[code]</mark> <mark>

</mark> <mark>[/code]</mark> <mark>
tags added.

2

The logic looks all wrong to me.

Don't you just want:

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;
 }
3 
if( millis() >= waitTime + batPrevMillis[batteryNumber]){

does not allow for millis() rollover. You need to use subtraction as in

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

4

Robin2:

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

does not allow for millis() rollover. You need to use subtraction as in

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:

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

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

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

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.

6

sonyhome:
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:

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

As far as I can tell, the only necessary change was to stop the minor timing drift.

7

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...

8

arduinodlb:
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

9

Robin2:
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

10

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.

11

aarg:
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.

12

arduinodlb:
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.

13

Delta_G:
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.

14

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.

15

vceklic:
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.

16

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?

17

vceklic:
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

18

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.

19

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[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));
}
}
}

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;
}

20

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:

if(youAddCodeToAPost){
    pleaseUseCodeTags();
}