Writing a timing function

Posted by: @barrie

I am trying to avoid delay() and writing a timer function but as usual I am bogged down with syntax.

currentTime and lastTime were declared as unsigned long. interval declared as long.

What have I missed in this function. The error was:-

"expected constructor, destructor, or type conversion before '(' token" ...... referencing the first line.

elapsedTime (interval){
if (currentTime - lastTime >= interval){
return true;
  interval = 0;
  else
  lastTime = millis();
  return false;
}
}

The compiler is politely  asking you what elapsedTime is. Its not given a data type, so it can't be a function or subroutine. It doesn't have a data type so it can't be a variable ... hmmmm

Also, note that the first thing you do in the bracketed area where you test against elapsed tome is return, so none of the rest of that code will  be executed.

EDIT: I would suggest that you include lastTime as one of the input arguments and then this module will work for all your tests, not just the specific one where this particular lastTime appears.

Hi @barrie,

   Sorry, I am not completely clear what you wanted your code to do.

This compiles, but needs further work to do what you want .. hopefully it demonstrates a few points to get started.

void setup() {
  // put your setup code here, to run once:

}


bool elapsedTime (unsigned long interval)
{
  unsigned long currentTime = millis();
  unsigned lastTime = 0;
  if (currentTime - lastTime >= interval)
  {
    return true;
    interval = 0;
  }
  else
  {
    lastTime = millis();
    return false;
  }
}


void loop() {
  // put your main code here, to run repeatedly:

}

Errors:

• function elapsedTime returned a false/true, but the bool type was not declared. With C++, all functions must have a return type, even if there is not returned value, in which case use "void". (This was the error your compiler was highlighting.)
• the function argument interval also needs a type
• currentTime and lastTime were not declared .... perhaps they were declared in code you didn't copy, probably not in the function, but global variables. In general, consider minimising the number of global variables, as with larger programs it becomes more difficult to track what is using them.
• Your {} brackets were incomplete. Please note I have shown the way I prefer to do it with { on a newline so that it lines up with its closing partner ... I know lots of people prefer to put { at the end of the previous line, but new lines are cheap...

Best wishes, Dave

Posted by: @barrie

I am trying to avoid delay() and writing a timer function but as usual I am bogged down with syntax.

currentTime and lastTime were declared as unsigned long. interval declared as long. lastTime was set millis() before the loop.

What have I missed in this function. The error was:-

"expected constructor, destructor, or type conversion before '(' token" ...... referencing the first line.

elapsedTime (interval){
if (currentTime - lastTime >= interval){
return true;
  interval = 0;
  else
  lastTime = millis();
  return false;
}
}

@barrie

I like your enthusiasm and tenacity but I think you are trying to run before you can walk!

There are several issues with your posted code

1. The function should declared as bool, ie bool elapsedTime(...), as you are returning a boolean value - true or false
2. The function parameter must be a 32 bit unsigned variable and declared as follows bool elapsedTime(uint32_t interval), as the millisecond function returns a 32 bit unsigned value
3. Similarly, all of your other timer variables will need to be declared as 32 bit unsigned 
4. The conditional statement is all askew and you need to rework the logic here.

Have a look an old posting I put on the Arduino Project Hub some time ago, this shows you how you can work with unlimited timers without using delay - unlimited timers.

Regards

Ron B

Posted by: @barrie

I am trying to avoid delay() and writing a timer function but as usual I am bogged down with syntax.

currentTime and lastTime were declared as unsigned long. interval declared as long. lastTime was set millis() before the loop.

What have I missed in this function. The error was:-

"expected constructor, destructor, or type conversion before '(' token" ...... referencing the first line.

elapsedTime (interval){
if (currentTime - lastTime >= interval){
return true;
  interval = 0;
  else
  lastTime = millis();
  return false;
}
}

@barrie - This will work...

void setup() {
  // put your setup code here, to run once:

}

#define DELAY 1000

void loop() 
{
  static u32 last = millis();
  u32 now = millis();
  if (now - last >= DELAY)
  {
    last = now;

    // Do whatever you want on that 1 second interval.
  }
}

Posted by: @barrie

I am trying to avoid delay() and writing a timer function but as usual I am bogged down with syntax.

currentTime and lastTime were declared as unsigned long. interval declared as long. lastTime was set millis() before the loop.

What have I missed in this function. The error was:-

"expected constructor, destructor, or type conversion before '(' token" ...... referencing the first line.

elapsedTime (interval){
if (currentTime - lastTime >= interval){
return true;
  interval = 0;
  else
  lastTime = millis();
  return false;
}
}

As far as the error message, its the same problem you had last time.

TWO RULES ABOUT FUNCTIONS

1. They MUST have a return variable type
2. They MUST have variable types in the parameters.

BTW - This function will only work with one timed procedure.  For instance if you wanted two timed procedures... something timed on 1 second intervals and another on 3.5 second intervals this will not work.

bool elapsedTime(u32 interval)
{
  static u32 lastTime = millis();
  u32 currentTime = millis();
  if (currentTime - lastTime >= interval)
  {
    lastTime = currentTime;
    return true;
  }
  return false;
}

@will @davee @ronbentley1 @inq

Thank you all for your advice, help with syntax, examples and above all, your time. The timing function is an important feature of my project. After a couple of days trying to sort it out, thanks to you all, I can confidently move on to the next challenge.

@barrie 

Can you please tell us exactly what you want this piece of code to do ?

It's logic is jumbled and you're changing lastTime inside (which means that it must be a global variable defined elsewhere) and you're also trying to change the value of interval which you can not do because it's an input parameter and its value is copied before calling this routine (and the original value is NOT updated on the return of this module).

@will It is equivalent to delay() but does not block the program flow. It only blocks one leg of the code. In my case I want to stop the sonar for a short time while the steppers keep rotating. 

@davee 's code worked first time with no tweeks. All done with local variables. Thanks Davee

@davee 

Posted by: @davee

bool elapsedTime (unsigned long interval)
 {
  unsigned long currentTime = millis();
  unsigned lastTime = 0;

  if (currentTime - lastTime >= interval)
   {
    return true;
    interval = 0;
   }
  else
  {
    lastTime = millis();
    return false;
   }
 }

Hi Dave, I acknowledge that you said this compiles and needs further work, but there are a couple of important factors here that need to be addressed, as to avoid further difficulty and confusion for the OP.

Firstly, 'interval' highlighted in red above will never be executed after a return statement, but even if you put it above the return statement, it doesn't have any effect at all to the overall outcome anyway - interval will always be the same value passed in as an argument to the function - The statement should be removed as it is not required at all.

Secondly, there is no static variable to store the lastTime value, so every time the function executes and completes its scope, it's variables go out of scope too; and a reset again at the next call; thus, lastTime will always be zero, and therefore the function will always return true.

Cheers

@barrie, et, al...

Posted by: @barrie

@will It is equivalent to delay() but does not block the program flow. It only blocks one leg of the code. In my case I want to stop the sonar for a short time while the steppers keep rotating. 

@davee 's code worked first time with no tweeks. All done with local variables. Thanks Davee

 

Um... are you sure you didn't mean that @inq code worked first time for you?

Please see my previous post on Dave's code, as I just can't see how it worked for you without any tweeks?

Cheers

Hi @frogandtoad,

   Sorry,  you are of course correct ...unfortunately the fragment of code had several compile-time errors and did not include the contextual code, such as variable declarations (that are required to compile the function without error messages), but were probably outside of the function, or the remainder of the functionality of the overall program flow.

Please note, I started typing my reply before you published yours, so I didn't see your reply until after publishing mine ... as has been noted elsewhere, some of us (e.g. me) type a lot slower than others ... it was not meant as a 'correction' to yours.

I confess and apologise for not noticing the return and assignment statements had that perverse order ... they were actually part of @barrie 's original code, which also had {}'s missing , making the entire functionality obscure. Had I noticed it, I would have taken some action, probably reversing the order of the the two statements and commenting accordingly. However, I was 'concentrating' on showing how the brackets needed to match up on @barrie's code, which I hope was a useful lesson.

-------

I considered different places to put the two undeclared variables ... @barrie said they were declared elsewhere .. and I said similarly in my comments ... I apologise for any confusion.

------

My aim was to address the original question that @barrie was stuck on ... Why does it not compile? I was just trying to make something that compiled, and explain the reasons why it didn't compile.

I assumed @barrie would look at the code functionality and integrate it into his program - and I hoped it would enable @barrie to move forward - I was not aiming to provide a complete solution or even a function that did precisely right thing ... as it said it still needs some work.

I hope my limited aim of addressing compile-time errors was a reasonable one. I obviously did not set out to confuse anyone. I admit I was surprised to see @barrie say the code worked 'without tweaks' but I did not feel responsible for auditing the resulting (unseen) program.

Best wishes and take care my friends, Dave

@davee

 Posted by: @davee

Hi @frogandtoad,

   Sorry,  you are of course correct ...unfortunately the fragment of code had several compile-time errors and did not include the contextual code, such as variable declarations (that are required to compile the function without error messages), but were probably outside of the function, or the remainder of the functionality of the overall program flow.

Please note, I started typing my reply before you published yours, so I didn't see your reply until after publishing mine ... as has been noted elsewhere, some of us (e.g. me) type a lot slower than others ... it was not meant as a 'correction' to yours.

I confess and apologise for not noticing the return and assignment statements had that perverse order ... they were actually part of @barrie 's original code, which also had {}'s missing , making the entire functionality obscure. Had I noticed it, I would have taken some action, probably reversing the order of the the two statements and commenting accordingly. However, I was 'concentrating' on showing how the brackets needed to match up on @barrie's code, which I hope was a useful lesson.

-------

I considered different places to put the two undeclared variables ... @barrie said they were declared elsewhere .. and I said similarly in my comments ... I apologise for any confusion.

------

My aim was to address the original question that @barrie was stuck on ... Why does it not compile? I was just trying to make something that compiled, and explain the reasons why it didn't compile.

I assumed @barrie would look at the code functionality and integrate it into his program - and I hoped it would enable @barrie to move forward - I was not aiming to provide a complete solution or even a function that did precisely right thing ... as it said it still needs some work.

I hope my limited aim of addressing compile-time errors was a reasonable one. I obviously did not set out to confuse anyone. I admit I was surprised to see @barrie say the code worked 'without tweaks' but I did not feel responsible for auditing the resulting (unseen) program.

Best wishes and take care my friends, Dave

Hey Dave, no problem at all mate, you offered your help, and that's all that matters, you tried to help, and that is fantastic! - Sometimes we are are not so good at representing our arguments (me included), but we move on, and investigate alternative views and proposals put forward to us.

Cheers

@frogandtoad I believe I was correct, @davee 's code did work as is with no tweeks and no globals. Here it is pasted directly from my script. I only tested it with an LED that turned on after interval set at 10000. I have yet to use it extensively and thanks to your analysis, I should be able to fix the "interval" and "lastTime" issues. Maybe I should set them as global variables with "return false" above "lastTime". Does that make sense?

@barrie Your code is a mess. If you always set lastTime to 0 then currentTime will always be > interval. Second, you still have code after the return.

BTW, you appear to be setting lastTime to alpha lowercase o as in Ok! Learn how to use auto format under tools, and how to copy paste. That is in the help.