Data logging error message

Hi,

I’m logging a data sample from my ultrasonic sensor, along with it’s time stamp. Mostly ok, but a few problems. The first is an error msg I don’t understand.

error: expected '}' before ';' token
     millis_hold = millis(); //holds initial time stamp

Here is my code:

/***** LIBRARIES AND HEADERS *****/
#include <NewPing.h>  // sonar library


/***** GLOBAL VARIABLES *****/
// Sonar set up
#define TRIGGER_PIN  5  // Arduino trigger pin on sonar
#define ECHO_PIN     6  // Arduino echo pin on sonar
#define MAX_DISTANCE 200 /* Maximum distance we want to ping for (in centimeters). 
                            Maximum sensor distance is rated at 400-500cm.
                         */



bool time_flag = 1;  // default set to true, 1
int i = 0;           // array element position
int millis_hold;     // stores snapshot of Arduino timer



/***** STRUCTURES *****/
struct sensors                 // struct tag name
{
  const int data_count = 3;              // number of data to collect, should be placed in arrays but because different data types, doesn't appear to work?
  
  int sonar[3];                 // sonar sensor
  unsigned long time_stamp[3];  // time stamp
} sensors_vars;                          // sesnor's varabile

/***** FUNCTION PROTOTYPES *****/
NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE); // NewPing setup of pins and maximum distance

void data_log(struct sensors_vars, int);


void setup() 
{
  Serial.begin(9600);
}

void loop() 
{

  bool (time_flag)
  {
    millis_hold = millis(); //holds initial time stamp
    time_flag =! time_flag;  // toggles bool to false
  }
  
  // create event to log data every 3s
  if ((millis_hold + 3000)) < millis() & (i < 4)) // program should end after 3 data samples have been logged
    {
      // call data log function
        {
          delay(30);    // Wait between pings, 29ms should be min delay between pings
          data_log(&sensors_vars, i);
          time_flag =! time_flag;  // toggles bool to true
        }
    }
  

  Serial.flush();
  exit(0);

}

/***** FUNCTIONS *****/
void data_log(struct sensors *sensors_vars, int i)
{
    sensors_vars->sonar[i] = sonar.ping_cm();
    sensors_vars->time_stamp[i]  = millis();
    i++;  // increase element position in each array
      
    // test if data collection has ended so it can print array values logged
    if (i>1)  // since count began at 0, data samples in array positions are 0, 1, 2
      {
        int hold = i; // holds highest value of loop count for data sampled, which are the number of data sampled
                
        for (int i=0; i < hold + 1; i++)  // using 'hold + 1' since hold is at 2, but need to exceed this by 1 to get all 3 data samples
          {
             // sonar, units: cm
             Serial.print("sonar distance = ");
             Serial.println(sensors_vars->sonar[i]);

            // time stamp, units: ms
            Serial.print("time = ");
            Serial.println(sensors_vars->time_stamp[i]);

            Serial.println(); // skip a space             
          }
      }
 
   return 0;  //return nothing
}

What is this supposed to do?

  bool (time_flag)
  {
    millis_hold = millis(); //holds initial time stamp
    time_flag =! time_flag;  // toggles bool to false
  }

Why are you trying to exit?

  exit(0);
   return 0;  //return nothing

That doesn't return nothing it returns zero, which is not nothing. The function is declared as void so you must not have a return at the end, remove it.

ToddL1962:
What is this supposed to do?

  bool (time_flag)

{
    millis_hold = millis(); //holds initial time stamp
    time_flag =! time_flag;  // toggles bool to false
  }




Its purpose is to hold current clock value, and then ask in the following if structure if the running clock has exceed the held clock value + 3s. This allows data to be sampled every 3s.


Note: I fixed the other 2 comments, thank you.

project_science:
Note: I fixed the other 2 comments, thank you.

Is it working?

No, it has the same error, I just removed "extra stuff"

project_science:
No, it has the same error, I just removed "extra stuff"

OK. Post the latest code you have and we can go from there.

Here it is:

/***** LIBRARIES AND HEADERS *****/
#include <NewPing.h>  // sonar library


/***** GLOBAL VARIABLES *****/
// Sonar set up
#define TRIGGER_PIN  5  // Arduino trigger pin on sonar
#define ECHO_PIN     6  // Arduino echo pin on sonar
#define MAX_DISTANCE 200 /* Maximum distance we want to ping for (in centimeters). 
                            Maximum sensor distance is rated at 400-500cm.
                         */



bool time_flag = 1;  // default set to true, 1
int i = 0;           // array element position
int millis_hold;     // stores snapshot of Arduino timer



/***** STRUCTURES *****/
struct sensors                 // struct tag name
{
  const int data_count = 3;              // number of data to collect, should be placed in arrays but because different data types, doesn't appear to work?
  
  int sonar[3];                 // sonar sensor
  unsigned long time_stamp[3];  // time stamp
} sensors_vars;                          // sesnor's varabile

/***** FUNCTION PROTOTYPES *****/
NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE); // NewPing setup of pins and maximum distance

void data_log(struct sensors_vars, int);


void setup() 
{
  Serial.begin(9600);
}

void loop() 
{

  bool (time_flag)
  {
    millis_hold = millis(); //holds initial time stamp
    time_flag =! time_flag;  // toggles bool to false
  }
  
  // create event to log data every 3s
  if ((millis_hold + 3000)) < millis() & (i < 4)) // program should end after 3 data samples have been logged
    {
      // call data log function
        {
          delay(30);    // Wait between pings, 29ms should be min delay between pings
          data_log(&sensors_vars, i);
          time_flag =! time_flag;  // toggles bool to true
        }
    }
  
/*
  Serial.flush();
  exit(0);
*/
}

/***** FUNCTIONS *****/
void data_log(struct sensors *sensors_vars, int i)
{
    sensors_vars->sonar[i] = sonar.ping_cm();
    sensors_vars->time_stamp[i]  = millis();
    i++;  // increase element position in each array
      
    // test if data collection has ended so it can print array values logged
    if (i>1)  // since count began at 0, data samples in array positions are 0, 1, 2
      {
        int hold = i; // holds highest value of loop count for data sampled, which are the number of data sampled
                
        for (int i=0; i < hold + 1; i++)  // using 'hold + 1' since hold is at 2, but need to exceed this by 1 to get all 3 data samples
          {
             // sonar, units: cm
             Serial.print("sonar distance = ");
             Serial.println(sensors_vars->sonar[i]);

            // time stamp, units: ms
            Serial.print("time = ");
            Serial.println(sensors_vars->time_stamp[i]);

            Serial.println(); // skip a space             
          }
      }
 }
  1. Instead of 1 I would recommend using true
bool time_flag = 1;  // default set to true, 1
  1. I would recommend changing the name of the sonar array to something else (maybe distCm?) since it may be confusing to have the same name of the sonar object. Also I would change the type of the array to unsigned long since the method sonar.ping_cm() returns an unsigned long.
struct sensors                 // struct tag name
{
  const int data_count = 3;              // number of data to collect, should be placed in arrays but because different data types, doesn't appear to work?

  int sonar[3];                 // sonar sensor
  unsigned long time_stamp[3];  // time stamp
} sensors_vars;                          // sesnor's varabile
  1. This is not valid C syntax. I assume you want an if statement here.
  bool (time_flag)
  {
    millis_hold = millis(); //holds initial time stamp
    time_flag = ! time_flag; // toggles bool to false
  }
  1. The following is wrong with this if:
  if ((millis_hold + 3000)) < millis() & (i < 4)) // program should end after 3 data samples have been logged
  • There are too many parens
  • ‘&’ is an arithemtic AND. You want a logical AND which is ‘&&’.
  • I woud reccommend replacing (millis_hold + 3000) < millis() with millis() - millis_hold > 3000
  1. Is there some reason you have this as a separate code block?
    {
      delay(30);    // Wait between pings, 29ms should be min delay between pings
      data_log(&sensors_vars, i);
      time_flag = ! time_flag; // toggles bool to true
    }
  1. I presume you think you are incrementing the global variable i here but you are not since i is passed by value. Therefore, every time you call data_log you are always passing in the value of 0.
void data_log(struct sensors *sensors_vars, int i)
{
  sensors_vars->sonar[i] = sonar.ping_cm();
  sensors_vars->time_stamp[i]  = millis();
  i++;  // increase element position in each array

There is much more than can be fixed but these are the errors that would cause you the most problems. You should take time to review a C/C++ tutorial as most of these are fundamental mistakes.

ok, thank you for your help! I’ve made several corrections, and I see I’m struggling a bit with passing references via functions, which is where I think my last major hurdle is.

Per ToddL1962’s recommendations, I have fixed comments:

#1,3,4,5

For #2

  1. I would recommend changing the name of the sonar array to something else (maybe distCm?) since it may be confusing to have the same name of the sonar object. Also I would change the type of the array to unsigned long since the method sonar.ping_cm() returns an unsigned long.

I changed the array name, but kept the data type on ‘int’. I don’t see where sonar.ping_cm() is returning an unsigned long.

For #6:

  1. I presume you think you are incrementing the global variable i here but you are not since i is passed by value. Therefore, every time you call data_log you are always passing in the value of 0.

I believe in the code below, I am now passing the reference of ‘i’ to the function void data_log. However, in the code below, I’m getting this error:

In function ‘void loop()’: could not convert ‘& sensors_vars’ from ‘sensors*’ to ‘sensors’
data_log(&sensors_vars, &i);

In thought about changing the function call to:

data_log(sensors, &sensors_vars, &i)

But that didn’t work either, so I’m stuck.

here is the changed code:

/***** LIBRARIES AND HEADERS *****/
#include <NewPing.h>  // sonar library


/***** GLOBAL VARIABLES *****/
// Sonar set up
#define TRIGGER_PIN  5  // Arduino trigger pin on sonar
#define ECHO_PIN     6  // Arduino echo pin on sonar
#define MAX_DISTANCE 200 /* Maximum distance we want to ping for (in centimeters). 
                            Maximum sensor distance is rated at 400-500cm.
                         */


bool time_flag = true;  // default
int millis_hold;     // stores snapshot of Arduino timer

int i = 0;           // array element position
//int *ptr_i = NULL;  // will be dereferenced, and equated to l-value of 'i', so when referenced in print re-using *, it will print r-value of 'i'


/***** STRUCTURES *****/
struct sensors                 // struct tag name
{
  const int data_count = 3;              // number of data to collect, should be placed in arrays but because different data types, doesn't appear to work?
  
  int sonar_distance[3];                 // sonar sensor
  unsigned long time_stamp[3];  // time stamp
} sensors_vars;                          // sesnor's varabile

/***** FUNCTION PROTOTYPES *****/
NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE); // NewPing setup of pins and maximum distance

void data_log(struct sensors, int *sensors_vars, int *i);


void setup() 
{
  Serial.begin(9600);
}

void loop() 
{

  if (time_flag == true)
  {
    millis_hold = millis(); //holds initial time stamp
    time_flag =! time_flag;  // toggles bool to false
  }
  
  // create event to log data every 3s
  if ((millis() - millis_hold > 3000) & (i < 4)) // program should end after 3 data samples have been logged
    {
      delay(30);    // Wait between pings, 29ms should be min delay between pings
      data_log(&sensors_vars, &i);
      time_flag =! time_flag;  // toggles bool to true
    }
  
/*
  Serial.flush();
  exit(0);
*/
}

/***** FUNCTIONS *****/
void data_log(struct sensors, int *sensors_vars, int *i)
{
    sensors_vars->sonar_distance[i] = sonar.ping_cm();
    sensors_vars->time_stamp[i] = millis();
    i++;  // increase element position in each array
      
    // test if data collection has ended so it can print array values logged
    if (i>1)  // since count began at 0, data samples in array positions are 0, 1, 2
      {
        int hold = i; // holds highest value of loop count for data sampled, which are the number of data sampled
                
        for (int i=0; i < hold + 1; i++)  // using 'hold + 1' since hold is at 2, but need to exceed this by 1 to get all 3 data samples
          {
             // sonar, units: cm
             Serial.print("sonar distance = ");
             Serial.println(sensors_vars->sonar_distance[i]);

            // time stamp, units: ms
            Serial.print("time = ");
            Serial.println(sensors_vars->time_stamp[i]);

            Serial.println(); // skip a space             
          }
      }
 }

project_science:
For #2
I changed the array name, but kept the data type on 'int'. I don't see where sonar.ping_cm() is returning an unsigned long.

For #6:
I believe in the code below, I am now passing the reference of 'i' to the function void data_log. However, in the code below, I'm getting this error:

You would never see sonar.ping_cm() is returning an unsigned long unless you looked in the library code. I did look in it. I wish the authors of these libraries would document the return types but they generally don't. Return types are very important!

You are passing the reference of i but the declaration/definition of your void data_log function is messed up. I would recommend defining void data_log this way:

void data_log(sensors* sensors_vars, int& i)

Then you would call it like this:

      data_log(&sensors_vars, i);

You lack a fundamental understanding of aggregate data types, pointers, and references. You should study those some more.