Error: invalid use of void expression

Projects Programming
1

Man, having real trouble trying to troubleshoot whats going on here.

this is supposed to wait x minutes then run the function blinks. but there is an error:

PAS___turn_on_after_2.cpp: In function 'void loop()':
PAS___turn_on_after_2:75: error: invalid use of void expression

and I've tried all sorts of different things with using the blinks funtion but can't figure it out.

I am assuming its the line: MsTimer2::set(5000, blinks(20,50)); //
as when I comment it out it compiles.

any help from someone with more brains then me will be much appreciated :slight_smile:

Tim

#include <MsTimer2.h>

// constants won't change. They're used here to 
// set pin numbers:
const int buttonPin = 12;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin


#define fortyfive 480000  //milliseconds in eight minutes - will later be 45 mins
#define hour 3600000  //milliseconds in an hour


// variables will change:
int buttonState = 0;         // variable for reading the pushbutton status


void blinks(int time, int amount){
  int i=0;
  
  if (!amount) {
    amount =50 ;
  }
   
  if (!time) {
    time=20;
  }
  for(i=0;i<amount;i++){
    digitalWrite(ledPin, HIGH); 
    delay (time);
    digitalWrite(ledPin, LOW);
    delay (time);
  }

}


void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);      
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);    
 

}

void loop(){
  // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);
  int t=300;
  int reset=0;
  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  
  
  if (buttonState == HIGH) {  
 
 
    blinks(20, 5);
     MsTimer2::set(5000, blinks(20,50)); // 
    MsTimer2::start();
    reset=1;
  }  
  
  
  blinks(50, 50);
  if (reset == 1 and buttonState == HIGH) {
   //digitalWrite(ledPin, LOW); 
  }
  //else {
    // turn LED off:
  //  digitalWrite(ledPin, LOW); 
 // }
}
2

You have MsTimer2 in your Arduino:Libraries folder?

480,000 and 3,600,000 are bigger than an int - might want to declare these as unsigned long,
or 480000UL, 3600000UL should work also
altho you don't seem to be using them yet.

3

MsTimer2::set(5000, blinks(20,50));

Uhh yeah, you cannot do this. You can't give parameters to blinks() here. MsTimer2::set is expecting a function that takes no parameters.

4

Ahh, thanks,

so any ideas what I can do to pass variables to the function? I guess I can use global variables and change them before running the function, though it would be nice to know if I can pass stuff to it as a normal function..

Cheers for your help..

Tim

5

o any ideas what I can do to pass variables to the function?

you have to rewrite/overload the MsTimer2::set(); function definition, but using global vars is much easier - optionally combine them in one struct to make a logical unit.

6

Maybe something like the following:

int blinksX;
int blinksY;

void blinksXY(void)
{
    blinks(blinksX, blinksY);
}
void blinks(int time, int amount)
{
// Your blinks code here
}


void loop()
{
.
.
.
        blinksX = 20;
        blinksY = 50;
        MsTimer2::set(5000, blinksXY);
 .
.
.

That way, you don't have to modify the library, and your blinks() code could be used in its "normal" way in other parts of your program.

Using globals in the body of the blinksXY() function is frowned on from a software engineering point of view, but lots of things we do in embedded systems like Arduino wouldn't stand up to a design review from my boss. (She would insist on rewriting MsTimer2 as a template class and using a functor. Stuff like that.)

Oh, well...

Regards,

Dave

7

davekw7x:
Maybe something like the following:

Yep, that is the way to do it. They're not quite 'global' variables, you can treat them as module statics. That is, don't go use them from other modules.

davekw7x:
She would insist on rewriting MsTimer2 as a template class and using a functor. Stuff like that.

I'm with your boss. An intermediate solution would be to rewrite MsTimer2 so that it takes a data pointer, and then the function it expects also takes that data pointer back. e.g.

void setup(void)
{
...
        blinks_data.X = 20;
        blinks_data.Y = 50;
        MsTimer2::set(5000, blinks_data,blinks_fn);
}

void blinks_fn(const blinks_data_t& data)
{
    blinks(data.X, data.Y);
}
8

The problem here is you need to pass the function name, not a value (that is, the function name IS the value required here).

Instead of this

MsTimer2::set(5000, blinks(20,50));

Do this

MsTimer2::set(5000, blinks);

And it should compile

9

[quote author=David Pankhurst link=topic=66119.msg485716#msg485716 date=1310397684]
The problem here is you need to pass the function name, not a value (that is, the function name IS the value required here).[/quote]I'm with you so far. A function name by itself (no parentheses) is treated by the compiler as a pointer whose value is the address wherever the function is loaded. ("Normal" programmers don't need to know the actual value of the pointer; just use the name of the function where it is needed.)

No, it won't. Did you try it? (See Footnote.)

The second parameter of MsTimer2::set() is a pointer to a function that has no parameters, and the compiler won't let you give it a pointer to a function that takes two parameters. Period. Full stop.

Regards,

Dave

Footnote:
Suppose you could somehow (by hook or crook) get the compiler to accept the function name "blinks" as the second argument to the MsTimer2::set() function? How the heck would it know what values to use for the time and amount arguments when invoking the function? I mean, really!

Bottom line: One approach might be to build a working solution patterned after the suggestion in reply number 5 of this thread. There are other ways...

10

davekw7x:
Maybe something like the following:

int blinksX;

int blinksY;

void blinksXY(void)
{
   blinks(blinksX, blinksY);
}
void blinks(int time, int amount)
{
// Your blinks code here
}

void loop()
{
.
.
.
       blinksX = 20;
       blinksY = 50;
       MsTimer2::set(5000, blinksXY);
.
.
.

Thanks Dave, this looks like it makes sense, although I don't actually know if I understand why it needs to then call upon the blinks function again with the real x/y values. I mean I think I get that the MsTimer2 can only receive a 'pointer' to a function, is it like that pointer is a thing that when sending it to MsTimer2 it becomes solidified as such? Hence you have to call again a function from within that which can have the non static variables, x & y. I'd change these several times in a program before running MsTimer on each instance I change them.

Sorry I don't have the Arduino on this computer to test it otherwise I could just tell you the answer hehe. But I think I'm really trying to learn about how the inner workings of c (or is it c#) work. I am a Perl programmer and can get away with just about anything to do just about anything else. Although I don't think I can do the same thing in Perl as in my first post now that I think about it, it's like trying to send a reference of a sub routine and then expect it to recode itself with new information once it's already been sent sortta thing...

Cheers for your help :slight_smile:

11

Another option I guess is to simply have 3 or however many different functions which have the different values I need hardcoded in the function, as it's so simple that would be fine. (but I'm really trying to learn now rather than mash up a solution)

The entire concept of the program is very simple..
Turn on...
Wait for a button to be pressed... (one of 2 buttons, the second isn't coded yet)
Blink a couple times to say the button was pressed...
Wait 45 mins (5 seconds in this case)...
Blink many times....
'reset' and wait for a button to be pressed again...

I will then have this hooked up to 2 different buttons, one for 45 mins and one for 35 (it's for an aeroplane flashing alarm to warn the pilot to switch fuel lines)

12

Oops Tim - I didn't look over the function enough - as it stands, the code will compile if you change the function to blinks(void), but with the parameters it won't. If the values never change, you can just include them in the function itself, but likely that won't be too flexible - you'd need a new function every time you wish to time differently.

Another option is to dispense with the timer and poll the items directly - I just wrote some code for another post that explains polling - it sounds like it could be useful for you:

You just set up a time for an even, and turn it on/off as needed. For multiple events, you just turn on a second timer when you turn off the first, etc.

Hope this helps.

13

timb0:
...why it needs to then call upon the blinks function again

When you execute <sTimer2::set(5000, blinksXY), it sets up Timer 2 to generate an overflow interrupt every 5000 milliseconds. The function whose name (pointer) you give it will be executed every time Timer 2 overflows. The way that the Timer2::set() function is defined, the function that you give it can not have any arguments. Therefore the blinksXY() function calls the real worker function, (your original blinks()), with arguments that you have previously set up. After that function completes its task, it returns to blinksXY(), which eventually returns from the interrupt.

Now, getting the stuff to compile is one thing; getting it to "work" is another.

Since the blinksXY() function is part of an interrupt service routine, all interrupts are automatically disabled by the time it is entered (when Timer2 overflows). That means that anything that uses millis() or delay() won't work, since they depend on Timer 0 interrupts.

Now, if all that your program is going to do is blink the lights and it doesn't matter whether everything else is frozen out until blinksXY() (and blinks()) have completed their tasks, then you can make a simple change by enabling the interrupts when you first enter blnksXY(). It may be satisfactory for demonstration purposes, but "real" applications may require some re-organization. Most programmers will recommend that interrupt service routines should not do anything that requires any significant delays (including print statements). Generally speaking, there is a good reason for this.

However...

A simple-minded demo:

#include <MsTimer2.h>

const int ledPin =  13;

// Global variables that will be parameters for
// blinksXY() so that it can call blinks() from
// a Timer 2 interrupt service routine
int blinksX;
int blinksY;

// blinksXY is called from the Timer2 overflow Interrupt
// Serivce Routine
void blinksXY(void)
{
    // Enable interrupts so that Timer 0 can generate timing
    // for delay() and millis()
    sei();
    Serial.print("blinksXY: ");Serial.println(millis());
    blinks(blinksX, blinksY);
}

void blinks(unsigned long time, int amount)
{
    Serial.print("blinks(");Serial.print(time);
    Serial.print(",");Serial.print(amount);
    Serial.println(")");
    for(int i = 0; i < amount; i++) {
        digitalWrite(ledPin, HIGH); 
        Serial.print("1:");Serial.println(millis());
        delay(time);
        Serial.print("2:");Serial.println(millis());
        digitalWrite(ledPin, LOW);
        delay(time);
        Serial.print("3:");Serial.println(millis());
        Serial.println();
    }

}


void setup()
{
    Serial.begin(115200);
    pinMode(ledPin, OUTPUT);      
    blinksX = 200;
    blinksY = 3;
    MsTimer2::set(5000, blinksXY); // 
    Serial.print("Starting MsTimer2 at ");
    Serial.println(millis());
    Serial.println();
    MsTimer2::start();
}

void loop() {
   // loop does nothing.  All of the action
   // is in the Timer 2 interrupt service
   // routine.
}

Output

Starting MsTimer2 at 1

blinksXY: 5001
blinks(200,3)
1:5003
2:5203
3:5404

1:5405
2:5606
3:5807

1:5808
2:6008
3:6209

blinksXY: 10001
blinks(200,3)
1:10003
2:10204
3:10405

1:10406
2:10607
3:10808

1:10809
2:11010
3:11210
.
.
.

Regards,

Dave

Footnote:
The MsTimer2 library functions are good certain things, but they hide some important information. Namely that you are now dealing with interrupts, and things in the "callback" function that you give to Timer2::set() can affect lots of other things in meaningful ways that we might not have known about when we started.

Bottom line: Libraries like MsTimer2 are tools that have been crafted and shared so that some of us won't have to deal with the intricacies of timer registers and other somewhat arcane details of the processor. The downside is that, by protecting us from having to know this to get started, it also may lead us into unknown territory through which we will, somehow, have to learn to navigate. In particular, it has hidden from us the fact that the function that we give it will be executed as part of the timer's overflow interrupt service routine.

That's pretty much a valid description for the entire Arduino Way: Easy to get started without having to learn a lot of the details that people are usually exposed to in programming classes.

It's kind of like learning to ride a bike with training wheels.

But...

After leaving the simple examples and simple applications behind, we may want to have more fun than is possible with the kid's bike. The training wheels eventually may have to come off and we may actually need to learn more than we thought we did.

By that time, of course, we are hooked. There is no turning back once our creative juices have started flowing and we can actually see some fun uses for this stuff.

14

Wow thanks Dave, your explanation is astounding given you're doing this from your own goodness as a human being :wink: I'm really getting my head around this now (yet to actually test it though argh!) but reading through it, I think it's making real sense and opening my mind to how micro controllers work (and their limitations). I really hope other people can read and understand this too. I will update in the next few days with what I end up using. I am thinking of just using millis() and capturing start times of button presses etc then running through until a specific time is met to blink the light without using delay() although I can obviously compensate if I know how much it's being delayed for before doing the final blink so to speak. It's not exactly hard what I'm trying to do but a very good exercise for me to get done correctly.

Thanks for your time, energy and knowledge given in helping me out!

Tim