Go Down

Topic: error: expected ';', ',' or ')' before '&' token (Read 9322 times) previous topic - next topic

capsid

Hi there!  I'm getting a strange error when I try to pass a struct to a function by reference.
Code: [Select]
error: expected ';', ',' or ')' before '&' token

It errors on the first line of this function definition in file "channel.c":
Code: [Select]
void initChannel(struct Channel &chan){
 chan.state = OFF;
 chan.dirty = 1;
 initPWM(chan.pwm);
}


The compile-results box shows the same error for all of the similar function prototypes in my header file "custom_types.h".   This led me to believe that I couldn't pass structures by reference, but I made another sketch in which I passed structures (and structures in structures) by reference and it compiled with no problems.  

Here's an excerpt from "custom_types.h", so you can see if I'm doing something obviously wrong:
Code: [Select]
struct PWM{
 int duty;  
 int state;
 long lastUpdate;
 int updateInterval;
 int onInterval;
 int offInterval;
};

void initPWM(struct PWM &pwm);           //errors on this line
boolean updatePWM(struct PWM &pwm);   //and this one


struct Channel{
 int pin;
 int color;
 int state;
 
 boolean dirty;
 
 struct PWM pwm;  
};

void initChannel(struct Channel &chan);   //error here, too
void updateChannel(struct Channel &chan);  //and again...


I'd be very grateful for some hints on what to try next, or what other info I can provide.   For the record, I do include "custom_types.h" in several files, including the main sketch file.  I also include "WProgram.h" in "custom_types.h" so I can have access to certain constants like HIGH and LOW.

Thanks for your help:)

AlphaBeta

How about:
Code: [Select]
typedef struct {
 int pin;
 int color;
 int state;
 
 boolean dirty;
 
 struct PWM pwm;  
} Channel;

void initChannel(Channel &chan);   //error here, too
void updateChannel(Channel &chan);  //and again...


capsid

Unfortunately, that yields an identical set of errors.  

That's similar to how I did it at first.  I prefixed the type declarations with 'struct' since I was having errors and hoped to be less ambiguous to the compiler.  

I'm not terribly well grounded in C, which is why I feel like I'm missing something obvious.  I could push the project (and its many tabs) to my github if that would make it easier to find the problem.

Thanks, though, for any and all suggestions.

Kristian

Hi,

I simply copied your code, but it compiles. I'm using version 00018.
So I'm not sure what is wrong...

Quote

struct PWM{
  int duty;  
  int state;
  long lastUpdate;
  int updateInterval;
  int onInterval;
  int offInterval;
};

void initPWM(struct PWM &pwm);           //errors on this line
boolean updatePWM(struct PWM &pwm);   //and this one


typedef struct {
  int pin;
  int color;
  int state;

  boolean dirty;

  struct PWM pwm;
} Channel;

void initChannel(Channel &chan);   //error here, too
void updateChannel(Channel &chan);  //and again...

void setup()
{
  
}

void loop()
{
  
}



halley

Probably missing a semicolon WAY before this token.  Such as at the end of a struct or class declaration, just after the curly brace closes.  Easy semicolon to forget.

capsid

Hi again.  I'm also using Arduino 0018.

I checked for missing semicolons at the end of my three struct declarations, but they were there.  Also triple checked my matching braces, but they appear to be evenly paired.

Here's a zip of my project folder.  Perhaps you could see if you get the same error when compiling the entire thing? :
http://github.com/jordanapplewhite/digi/zipball/e08a6dbb139c3e2475108375f31bcf82389dbcc8.

Thanks again for the help!!! I really appreciate the reminders to check the obvious stuff.  I'm starting to worry that this is a more subtle bug related to the automatic code generation, or something being built out of place.  

PaulS

In custom_types.h, in the Channel struct, you have a variable of type PWM. I do believe that the type of that variable should be "struct PWM".

capsid

Quote
In custom_types.h, in the Channel struct, you have a variable of type PWM. I do believe that the type of that variable should be "struct PWM".


That's true, but changing it doesn't remove the previously mentioned errors.  Thanks for taking a look.

I didn't mention this before since I don't think it's important, but there's two errors in the middle of the list (you'll see if you open and compile the project) that are different:
Code: [Select]
/custom_types.h:49: error: variably modified 'chan' at file scope

/custom_types.h:49: error: array type has incomplete element type


Does this offer any hint?  

Goodness gracious, I wonder if I'm just missing a bracket somewhere?   Usually the compiler helps me out quite a bit with a mundane mistake like that uneven brackets....


Mitch_CA

Where you have this...
Code: [Select]
void initChannel(Channel &chan);


I think you might mean this...
Code: [Select]
void initChannel(struct Channel*);

You'll run into custom_types.h:49 not allowing a variable in the struct definition.  That's where I stopped pressing ctrl-R.

PaulS

I was able, eventually, to get all of your code to compile. I changed all the .c files to .cpp. Whether that was needed, or not, I'm not sure. It looked like it was needed when I did it.

Every single file needed to be changed. The most significant change was to change all the & arguments to * arguments.

Here's the custom_types.h file that I ended up with:
Code: [Select]
#ifndef CUSTOM_TYPES_H
#define CUSTOM_TYPES_H

#include "WProgram.h"

//Matrix
const int pinMap[] = {13,12,6,8,11,7,5,10,4,9,2};
#define numChannels 11

//PWM
const int frequency = 512;
const int minDuty = 0;
const int maxDuty = 256;
const int period = 2;

const int ON = HIGH;
const int OFF = LOW;

struct _PWM
{
 int duty;  
 int state;
 long lastUpdate;
 int updateInterval;
 int onInterval;
 int offInterval;
};
typedef struct _PWM PWM;

void initPWM(PWM *pwm);
boolean updatePWM(PWM *pwm);

struct _Channel
{
 int pin;
 int color;
 int state;
 
 boolean dirty;
 
 PWM *pwm;  
};
typedef struct _Channel Channel;

void initChannel(Channel *chan);
void updateChannel(Channel *chan);

struct _Matrix
{
 int width;
 Channel *chan[numChannels];
};
typedef struct _Matrix Matrix;

void initMatrix(Matrix *matrix);
void updateMatrix(Matrix *matrix);
void runMode(Matrix *matrix);
void changeMode();
void render(Matrix *matrix);
void Tide(Channel *matrix[]);
void Knight(Channel *matrix[]);
#endif


The sketch:
Code: [Select]
#include "custom_types.h"
#include <EEPROM.h>

Matrix matrix;

void setup()
{
 //Visit the Matrix tab to
 //set the pin mapping.
 initMatrix(&matrix);
 
 changeMode();
}

void loop()
{
 //Visit the mode tab for
 //instructions on programming modes.
 runMode(&matrix);
 
 updateMatrix(&matrix);
 
 render(&matrix);
}


The PWM.cpp file:
Code: [Select]
#include "custom_types.h"

void initPWM(PWM *pwm)
{
 pwm->duty = maxDuty;
 pwm->state = OFF;
 pwm->lastUpdate = 0;
 pwm->updateInterval = 0;
}


boolean updatePWM(PWM *pwm)
{
 long now = millis();
 
 if (now - pwm->lastUpdate > pwm->updateInterval){

   pwm->lastUpdate = now;    
   pwm->onInterval = pwm->duty * period;
   pwm->offInterval = frequency - pwm->onInterval;
   
   if (pwm->state == ON)
   {
     pwm->updateInterval = pwm->offInterval;
     return true;
   }
   else if (pwm->state == OFF)
   {
     pwm->updateInterval = pwm->onInterval;
     return true;
   }
   else
     return false;
 }
}


The channel.cpp file:
Code: [Select]
#include "custom_types.h"

void initChannel(Channel *chan)
{
 chan->state = OFF;
 chan->dirty = 1;
 initPWM(chan->pwm);
}

void updateChannel(Channel *chan)
{
 if (chan->state == ON)
 {
   //Set dirty flag to signal a need for digitalWrite() :)
   PWM *pwm = chan->pwm;
   boolean stat = updatePWM(pwm);
   chan->dirty = stat;
 }
}


The matrix.cpp file:
Code: [Select]
#include "custom_types.h"

void initMatrix(Matrix *matrix)
{
/*
This function should be called in setup()
*/
 matrix->width = numChannels;  
 
 for (int i=0; i<matrix->width; i++)
 {
   matrix->chan[i]->pin = pinMap[i];
   pinMode(matrix->chan[i]->pin, OUTPUT);
 }
}

void updateMatrix(Matrix *matrix)
{
 for (int i=0; i < matrix->width; i++)
 {
   updateChannel(matrix->chan[i]);
 }
}


The modes.cpp file:
Code: [Select]
#include "custom_types.h"

const int TIDE = 0;
const int KNIGHT = 1;
const int PULSE = 2;
const int NUM_MODES = 3;

int mode;
int forward = true;

void runMode(Matrix *matrix)
{  
 switch (mode)
 {
   case TIDE:
     Tide(matrix->chan);
     break;
   case KNIGHT:
     Knight(matrix->chan);
     break;
 }
}

#include <EEPROM.h>

void changeMode()
{
 int address = 0;
 mode = EEPROM.read(address);
 EEPROM.write(address, (byte)((mode+1) % NUM_MODES) );
}


//Modes ===========================================================


//Tide() turns the leds on in sequence.  They turn on one by one until every
//LED is on.  then they turn off in the same order.
void Tide(Channel *matrix[])
{
 int ledCount = 3;
 if (forward)
 {
   for (int i=0; i<ledCount; i++)
   {
     if (matrix[i]->state == LOW)
     {
       matrix[i]->state = HIGH;
       if (i+1 == ledCount) {forward = false;}
       return;
     }
   }
 }
 else
 { //go backwards
   for (int i = ledCount-1; i > -1; i--)
   {
     if (matrix[i]->state == HIGH)
     {
       matrix[i]->state = LOW;
       if(i == 0) {forward = true;}
       return;
     }
   }
 }
}



//Almost exactly like Tide(), except only one LED is on at a time.
//It looks like the car KITT from KNIGHT RIDER.
void Knight(Channel *matrix[])
{
 int ledCount = 3;
 if (forward)
 {
   for (int i=0; i<ledCount; i++)
   {
     if (matrix[i]->state == LOW)
     {
       matrix[i]->state = HIGH;
       if (i>0) { matrix[i-1]->state = LOW; }
       if (i+1 == ledCount){ forward = false; }
       return;
     }
   }
 }
 else
 { //go backwards
   for (int i = ledCount-1 ; i > -1; i--)
   {
     if (matrix[i]->state == HIGH)
     {
       matrix[i]->state = LOW;
       if (i < ledCount-1) {matrix[i+1]->state = LOW; }
       if(i == 0){ forward = true; }
       return;
     }
   }
 }
}


The ledCount variable was not defined anywhere. I stuck something in there to get this function to compile. Probably should be defined in custom_types.h (as const).

Finally, render.cpp:
Code: [Select]
#include "custom_types.h"

void render(Matrix *matrix)
{
/*
Similar to how a video game separates logic and rendering,
We'll calculate the matrix state and apply PWM effects
before pushing out whether the pins are high or low.  
We even track which chans are 'dirty.'

Passing by reference for speed, not sure if that really
needs to be done, but I figured it was a couple hundred bytes
per cycle that don't need to be copied :p
*/
 for (int i=0; i < matrix->width; i++)
 {
   if (matrix->chan[i]->dirty)
   {
     digitalWrite(matrix->chan[i]->pin, matrix->chan[i]->state);
   }
 }
}


I don't have your hardware, so I have no idea if this executes properly, but it does compile and link.

capsid

Thank you guys so much for the extensive help.  PaulS, that was really nice of you to go so far.

It seems C doesn't quite like passing structs around by reference, but I know it's perfectly sensible in C++.    I wonder: If a function accepts a struct where the argument should be a struct pointer, does this mean that structs are actually just pointers (like arrays?), and member access is just shorthand for pointer arithmetic?  

I don't have the hardware either, it's still in the mail.   But getting it compiled takes a huge worry off my shoulders.   Thanks so much.

Groove

Quote
C doesn't quite like passing structs around by reference

C doesn't have references, only pointers.
Per Arduino ad Astra

capsid

Quote
C doesn't have references, only pointers.


Well, that explains it :)  Thanks Groove.

capsid

Now that I've gotten it to compile, I wanted to reply with some interesting things I found out.  Basically, all I had to do was change the file extension from .c to .cpp, and the compiler no longer complained about passing structs by reference.  

There were a host of other errors to track down, mainly caused by using non-constant extern globals.  Which is just as well, because I shouldn't be using non-constant globals anyway:)  Global vars declared const worked fine.  Just wanted to warn you that thar be dragons that way.

Thanks again.

gravelbar

And maybe a help to others; I got this error when I put a block of code outside the loop()  {} brackets and before function code; took a bit to track down.

Go Up