Printing Garbage

Question

I'm writing some code to log the voltage measurements from multiple sensors through SPI. I'm saving all the data in a custom data structure, and then printing the data structure to the terminal when, either too many events have occurred or a user requests the data. The problem I'm finding is that the last data value in each event is completely wrong when I call the DataDump function. If I try to print the data immediately after recording it, its correct. I have put what I think are the important code spinets below, but here is a link to the entire program http://pastebin.com/Y7kcntu3 .

My only thought as to what this could be is when I allocated the memory for the 200 events, the scope of the structure doesn't extend to the dumpData function. But this doesn't really make sense, as all the other data is present and correct. My other thought is that there is anderror when converting from char->Binary->decimal, but again only the single value is wrong.

This first chunk of code is a data structure I created to handle the data from multiple sensors. The data is just 8bit SPI data from an ADC so I used unsigned chars to minimize the amount of memory used. I also allocated enough memory for 200 events.

typedef struct                //event structure, containts a timestamp element and an array of 18 data points
{
  unsigned long int timeStamp;
  unsigned char data[SENSORS];
} Event;

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Event eventLog[MAX_DATA];        //an array of structures representing 200 events, once the 200 events have been filled the data will be printed

In Main "loop()", when a pin is high there is a data gathering process. when an even occurs the first event data structure will record a time stamp, then being the process of starting analog to digital conversions and storing the data in the data array for the current event.

eventLog[i].timeStamp = micros();

    for (j=0; j<=SENSORS; ++j) {
      lockAndPop();                         //lock digital value and reset conversion
      selector = inputSelector(selector);      //increment the selector pin
      PORTA = selector;
      digitalWrite(RD, LOW);                //Start new conversion

      digitalWrite(CLK_INH, LOW);                //Start the data transfer
      eventLog[i].data[j] = SPI.transfer(0);     //read a single byte from the SPI line
      digitalWrite(CLK_INH, HIGH);              //Inhibit clock
      digitalWrite(LD, LOW);
      while(digitalRead(INT1)){}                //wait for previous conversion to end            
      } 
    i++;

The data dump function is called in two situations, the first when 200 events have occurred, the seconds is when an external button is de-pressed. This function will create a print buffer that the event data can be formatted and displayed in decimal value. The data values are converted from binary to Decimal not ASCII.

void dataDump (){
  char buf[100], *pos = buf;    //create a buffer of 100 charaters, anda pointer to the begining of that buffer
  char *base = buf;             //create a base address to reset the buffer
  unsigned char eventCount = i;               //how many events occured before dump command was called
  unsigned char localCount;      
  unsigned char localData;
  Serial.begin(115200);
  for (localCount = 0; localCount<=eventCount; ++localCount){
    pos += sprintf(pos, "%lu", eventLog[localCount].timeStamp);          //sprintf will append the data to the pointer "pos", and return the number of byte append.
    for (localData = 0; localData<=SENSORS; ++localData){
       pos += sprintf(pos, " %d", (unsigned int)(eventLog[localCount].data[localData]));   
    }
    Serial.println(buf);
    pos = base;
  }
  i=0;
  j=0;
  Serial.end();
  return;

The issue I'm facing is that when the data dump function is called, everything is printed fine except the last data value in the array. here is an example of what is printed

539580840 171 149 120 152
539581080 170 149 119 216
539581912 170 149 120 196
539582148 170 149 120 180
539582388 170 149 120 168
539582632 170 149 119 148
539582868 170 148 119 128
539583104 170 149 119 216
539583704 170 149 120 196
539583940 170 149 120 176
539584176 170 149 120 160
539584416 170 149 120 148
539584660 170 149 120 128
539584896 170 149 120 112
539585136 170 149 120 92
539585372 170 149 119 80
539585616 170 149 120 60
539585852 170 149 119 59

The last value should be constant like the three before it, as the ADC is reading a reference voltage. The data is perfectly valid when printing

Serial.println(eventLog[i].data[3])

from the main "loop()" code. I have tried running the same code from the data dump functions but I still get garbage. Also I monitored the SPI line using a logic analyzer and I'm seeing the data I should on the data line.

**My only thought as to what this could be is when I allocated the memory for the 200 events, the scope of the structure doesn't extend to the dumpData function. But this doesn't really make sense, as all the other data is present. **

Answer 1

I didn't realize it at first, but once you answered my questions it stuck out like a sore thumb.

If SENSORS is equal to three, then unsigned char data[SENSORS]; is an array of three elements. Not four as you are expecting.

Arrays are used with a zero based index, (0,1,2,3,...) however when you declare them, you need to put the actual count of elements needed.

To fix, I'd make SENSORS equal to four, then adjust your loops to go from zero, to less than SENSORS (not less than or equal to). The same goes for MAX_DATA. Your loops are using one more element than you actually own.

Unless you need an index, C++11 has a feature which will prevent overrunning your arrays. It is the ranged-for loop.

The loop you are using now:

for (localCount = 0; localCount<=eventCount; ++localCount){
  pos += sprintf(pos, "%lu", eventLog[localCount].timeStamp);          //sprintf will append the data to the pointer "pos", and return the number of byte append.
  for (localData = 0; localData<=SENSORS; ++localData){
     pos += sprintf(pos, " %d", (unsigned int)(eventLog[localCount].data[localData]));   
  }
  Serial.println(buf);
  pos = base;
}

Can become this:

for( auto &entry : eventLog ){
  pos += sprintf(pos, "%lu", entry.timeStamp); 
  for( char reading : entry.data ){
    pos += sprintf(pos, " %d", (unsigned int)reading);
  }
  Serial.println(buf);
  pos = base;
}

As there are no indices involved, you will always use the actual number of elements. Ranged loops can be quite confusing at first, but are very useful.

And fixing the use of SENSORS and MAX_DATA may not solve all the problems, however fix that, then we can look at the rest if your problem persists. If there are more problems, leave some info in the comments below and I'll update my answer.