Resets while reading ADC?

So I am working on this project to track the sun using a single solar cell and no other sensors. Currently, there is a stepper motor (to which a solar cell is attached) connected to the Arduino through a darlington array, and an LCD screen to display battery voltage and solar cell voltage. The program sweeps the cell through a specified angle, taking a specified number of samples at each step (75 in this case) and taking an average. It then finds the maximum value read from the cell, and the number of times that maximum appears. It moves the cell to approximately the middle of the "plateau" where the max values are.

Here's the trouble: Everything works beautifully when the sweep angle is 90 degrees. It works fine when it's 135 degrees. And 140. And 145. But somewhere above that it stops working. At 180 degrees it will turn half that angle to begin the sweep (as it should) and then freeze, or reset. I've been trying to troubleshoot this for a while but I can't find the problem. I assumed it was some kind of overflow, but pretty much every variable has been set to a long int.

I'll post the code shortly.

This code is broken up into three tabs in the IDE.

First tab (Nokia_LCD) displays stuff to the screen, and dictates the sweep parameters (angle, number of samples/step). As you can tell I just used the demo Nokia sketch and built on it:

/*
  Nokia LCD Demo - A demo using the NokiaLCD library for a Nokia LCD with the epson driver.
 Created by Thomas Jespersen, July 2009 (Originally Arduino Sketch by Gravitech.us)
 Released into the public domain.
 */

// Stepper Driver Info
// Step Clock Pin: 7
// Half/Full Step Pin: 8
// Direction: 9
// Enable: 10
// Reset: 11

#include <NokiaLCD.h>
#include <Stepper.h>



// Nokia LCD Setup settings
#define RED			0x1C
#define GREEN			0xE0
#define BLUE			0x03
#define YELLOW			0xFC
#define MAGENTA			0x1F
#define CYAN			0xE3
#define BLACK			0x00
#define WHITE			0xFF
NokiaLCD nokiaLcd;


const int cellPin = A0;
const int battPin = A1;
const int s1 = 12;      // Switch 1 on Nokia LCD board
const int s2 = 13;      // Switch 2 on Nokia LCD board
const int step1 = 8;
const int step2 = 9;
const int step3 = 10;
const int step4 = 11;
int sensorValue = 0;  // variable to store the value coming from the sensor
int switch1 = 0;
int switch2 = 0;
char text[50];
char pinA0[10];
char pinA1[10];
float fBattVoltage = 0;
int iBattVoltage = 0;
long int runAverage = 0;
long int cellVoltage = 0;
int move;
int midpoint;
long int totalSteps;
int multipleV;
boolean centered;

Stepper myStepper(400, 8,9,10,11,1);


void setup() 
{ 
  Serial.begin(57600);  
  DDRD |= B01111100;   // Set SPI pins as output 
  PORTD |= B01111100;  // Set SPI pins HIGH
  
  nokiaLcd.lcd_init();
  delay(100);
  pinMode(s1, INPUT);
  pinMode(s2, INPUT);  
  pinMode(7, OUTPUT);

  
  analogReference(INTERNAL);  // Uses internal 1.1V reference
   
 } 

void loop() 
{
  /* This code reads analog input A0 to determine the solar cell voltage */
  
  //float averageVoltage = 0.0;
  runAverage = runningAVG(cellPin,5);
  //averageVoltage = (((float)runAverage/1024.0f)*1.1f)*1000.0f; // Convert reading to Voltage
  //cellVoltage =(int)averageVoltage;
  cellVoltage = (runAverage*1100) / 1024; // Convert reading to Voltage
  
 
  /* This code reads analog input A1 to determine the battery voltage */
  
  runAverage = runningAVG(battPin,5);
  fBattVoltage = (((float)runAverage/1024.0f)*1.1f)/(0.1232f)*1000.0f;
  iBattVoltage = (int)fBattVoltage;
  
  /* This will sound an alarm if the battery voltage drops below the cutoff. Be sure to set correct cut-off for battery! */
  // 2-cell Lipo cutoff: 6.8 V     
//  if (iBattVoltage > 6800) digitalWrite(
  if (iBattVoltage < 6800)
  {
    digitalWrite(7,HIGH);
    delay(500);
    digitalWrite(7,LOW);
    delay(500);
  }
   char battVoltageStr[50];
  
  /* This code updates the screen (Nokia LCD) with the solar cell voltage, and the battery voltage. */
  
  strcpy(text,"Cell Voltage");
  nokiaLcd.lcd_draw_text(BLUE, WHITE, 10, 20, text);
  sprintf(pinA0,"%i mV    ", cellVoltage);
  nokiaLcd.lcd_draw_text(RED, WHITE, 10, 37, pinA0); 
  strcpy(text,"Battery Voltage");
  nokiaLcd.lcd_draw_text(BLUE, WHITE, 10, 60, text);
  sprintf(battVoltageStr,"%d",iBattVoltage);
  sprintf(pinA1,"%c.%c%c V   ",battVoltageStr[0],battVoltageStr[1],battVoltageStr[2]);
  nokiaLcd.lcd_draw_text(RED, WHITE, 10, 75, pinA1); 
 
  
  switch1 = digitalRead(s1);
  switch2 = digitalRead(s2);
  char sweepVals;

  if (switch1 == LOW) sweep(180,400,50,0); // sweep(angle, steps per revolution, number of times to sample each step, which ADC pin to analyze)
    //sweepVals will return [move, totalSteps, midpoint]
  if (switch2 == LOW) 
  {
    int center = 0;
    if (centered)
    {
      Serial.println("Already centered, no move.");
    }
    else if (move < midpoint)
    {
      center = totalSteps - move - midpoint;
      myStepper.step(center);
      centered = true;
        Serial.print("Distance to center (Loop1): ");
        Serial.println(center);
    }
    else if (move > midpoint)
    {
      center = move + midpoint - totalSteps;
      myStepper.step(-center);
      centered = true;
      Serial.print("Distance to center (Loop3): ");
      Serial.println(-center);
    }
  }
  
    /* This turns off all of the pins controlling the stepper motor. These are initially off, but 
       the stepper library leaves certain pins on depending on which step it just took. Waste of power
       unless you need some holding capability, but I doubt they intended it to work that way. */
    digitalWrite(8, LOW);
    digitalWrite(9, LOW);
    digitalWrite(10, LOW);
    digitalWrite(11, LOW);

  
}

The next tab is runningAVG, it takes an average of however many samples:

int runningAVG(const int sensorPin, const int numReadings)
{

// ADC Smoothing parameters
long int readings[numReadings];      // the readings from the analog input
long int total = 0;                  // the running total
long int average = 0;                // the average

for (int thisReading = 0; thisReading < numReadings; thisReading++) readings[thisReading] = 0; // Resets all contents of "readings" to zero
    
 /* This code performs a running average on the selected pin with number of samples defined by "numReadings"*/
  for (int index = 0; index < numReadings; index++)
  {
    total = total - readings[index];   // subtract the last reading for this index:  
    Serial.print("Begin ADC.");    
    readings[index] = analogRead(sensorPin);    // read from the sensor: 
    Serial.print("Finish ADC.");    
    total = total + readings[index];     // add the reading to the total:   
    if (index >= numReadings) index = 0;        // if we're at the end of the array wrap around to the beginning:        
  }
  average = total / numReadings;     // calculate the average:
  return average;

}

The last tab is "sweep," it controls the stepper motor, and moves it to the correct position after performing a scan:

/*This will sweep a stepper motor through the designated angle while reading ADC values. 
 
 During the sweep the voltage of the solar cell will be read and stored in an array. At the conclusion of the
 sweep, the maximum value will be found and the number of steps needed to return to the corresponding position
 will be returned.
 
 Name: Sergey Feingold
 Date: 9-16-11
 
 --Inputs--
 angle: This is the angle in degrees through which the stepper will move. The starting position is assumed to 
 bisect the angle, therefore an input of 180 will sweep 90° each direction before returning to center.
 stepsPerRev: The is the number of steps per revolution of the stepper motor. This is required to ensure correct
 amount of movement. 
 numAverages: This is the number of times that the ADC value will be averaged at each step. More averages will result in a slower sweep.
 pinADC: This is the ADC pin that will be be analyzed
 */

void sweep(long int angle, long int stepsPerRev, long int numAverages, int pinADC) 
{
  // Stepper Parameters
  myStepper.setSpeed(25); 
  long int stepsPerRevolution = stepsPerRev;


  totalSteps = (((long)angle) * 400 / 360);
  long int halfSweep = totalSteps/2;
  int CW = 1;
  int CCW = -1;
  long int halfSweepCW = halfSweep * CW;
  long int halfSweepCCW = halfSweep * CCW;
  myStepper.step(halfSweepCW);  

  //float averageVoltage = 0.0;
  long int cellVoltage = 0;
  long int cellVoltages[totalSteps];
  long int runAverage = 0;

  /* Runs as many iterations as required, calling the runningAVG function and computing an average at each step */
  for (int i = 0; i < totalSteps; i++)
  {
    runAverage = runningAVG(pinADC,numAverages);
    cellVoltage = ((long)runAverage * 1100) / 1024; // Convert reading to Voltage
    cellVoltages[i] = cellVoltage;
    //cellVoltages[i] = cellVoltage;
    myStepper.step(-1);
  }

  /* This finds the maximum value in the array */

/* This part finds the maximum by going through the cellVoltages array and comparing the value at each index to the current value of maxValue.
   If it is greater, the value is written to maxValue as the new maximum */

  int maxValue = 0;
  multipleV = 0;
  int currentStep = 0;
  int stepCount = 0;
  move = 0;
  midpoint = totalSteps / 2;
  for (int i = 0; i < totalSteps; i++)
  {
    if (cellVoltages[i] > maxValue) 
    {
      stepCount = i;
      maxValue = cellVoltages[i];
    }
  }
  
  /* This goes through the cellVoltages array again and checks to see how many values are equal to maxValue. When one is found, 
     multipleV is incremented by one. This is used to correct for the offset that would otherwise be produced (i.e. 50 max values are
     roughly centered about the "true" maximum, but the program is only aware of the most recent maximum value at the edge of this plateau,
     and therefore will not move to the correct position */
  
  for (int j = 0; j < totalSteps; j++)
  {
    if (cellVoltages[j] == maxValue) multipleV++;
  }

  // This moves the stepper motor to the midpoint of the region of maximum values

  move = totalSteps - (stepCount + (multipleV / 2));
  myStepper.step(move);
  centered = false;

}

I should also mention that according to the serial printouts I inserted in various places, everything would work up until the ADC was read in the "runningAVG" function. So a serial printout would show if placed right before the line that reads the ADC, and the serial printout after that line would not show up.

I don't mean to flood you with code, but I'm open to the idea that some little thing in any of those sections could be causing the problem, I just can't find what it is. Any help would be greatly appreciated! I'm ready to conclude this project but I just can't move on until I figure out why this isn't working. Thanks!

    if (index >= numReadings) index = 0;        // if we're at the end of the array wrap around to the beginning:

This will never happen, with the for loop written the way it is. If the for loop is changed, then this will cause the whole for loop to be executed again, causing an infinite loop. Get rid of it, and make sure that the for loop is always correct, instead of trying to band-aid a bad situation.

  //float averageVoltage = 0.0;
  runAverage = runningAVG(cellPin,5);
  //averageVoltage = (((float)runAverage/1024.0f)*1.1f)*1000.0f; // Convert reading to Voltage
  //cellVoltage =(int)averageVoltage;

The commented out code clearly has nothing to do with your problem. Why did you post it? If you don't need it, delete it. If you later need it again, get it from a backup copy of the sketch.

  fBattVoltage = (((float)runAverage/1024.0f)*1.1f)/(0.1232f)*1000.0f;

Got to love magic numbers.

Everything works beautifully when the sweep angle is 90 degrees. It works fine when it's 135 degrees. And 140. And 145. But somewhere above that it stops working.

Here's a clue:

  long int cellVoltages[totalSteps];

The more you want to sweep, the larger this array gets.

    cellVoltage = ((long)runAverage * 1100) / 1024; // Convert reading to Voltage

The runAverage variable is a long int. Throughout your code, you are using longs where ints are sufficient, thereby doubling your memory requirements. runAverage should be an int, as the largest value it can ever hold is 1023, which is like storing a flea in a refrigerator box.

cellVoltage, being only slightly larger (max = 1098) would also fit in an int, halving the memory used by this array.

You are seeing the issues you are because you are running out of memory, because you are wasteful.

Thanks for the reply, you are right that this is wasteful coding, but that wasn't the biggest priority for this project. Unless of course it's so wasteful that it's causing the program to lock up, which seems to be the case.

PaulS:
This will never happen, with the for loop written the way it is. If the for loop is changed, then this will cause the whole for loop to be executed again, causing an infinite loop. Get rid of it, and make sure that the for loop is always correct, instead of trying to band-aid a bad situation.

I noticed the same thing, as I didn't write this code but got it from one of the forum posts somewhere. I couldn't figure out what it was supposed to do but left it in anyway assuming the original coder knew better than I did.

The commented out code clearly has nothing to do with your problem. Why did you post it? If you don't need it, delete it. If you later need it again, get it from a backup copy of the sketch.

Must have missed that...

  fBattVoltage = (((float)runAverage/1024.0f)*1.1f)/(0.1232f)*1000.0f;

Got to love magic numbers.

What do you mean? I suppose I was trying extra hard to make sure that all of that equation was processed as a float, and I had read that you should add the (float) if you want an initially non-FP number to be processed as a FP. I was also told that if you set a FP number to equal the result of some operation with an integer, that the equation will be first processed as integer math unless you use the (float) conversion. Is that true?

Here's a clue:

  long int cellVoltages[totalSteps];

The more you want to sweep, the larger this array gets.

    cellVoltage = ((long)runAverage * 1100) / 1024; // Convert reading to Voltage

The runAverage variable is a long int. Throughout your code, you are using longs where ints are sufficient, thereby doubling your memory requirements. runAverage should be an int, as the largest value it can ever hold is 1023, which is like storing a flea in a refrigerator box.

cellVoltage, being only slightly larger (max = 1098) would also fit in an int, halving the memory used by this array.

You are seeing the issues you are because you are running out of memory, because you are wasteful.

I did go a little crazy setting everything to long ints...I was feeling paranoid about overflows somewhere and I still don't have a good grasp on how operations are actually carried out. For instance, if you have an integer "A" set to 100, and a long integer "B," the operation "B = A * 100000000" will work correctly, yes? That's what I initially thought but after reading about the issue with floating point math mentioned above, I got a little crazy with the long int declarations...

Thanks for the help!

Here's a prime example of my confusion:

int cellVoltage = 0;
int runAverage = 1023;

cellVoltage = (runAverage * 1100) / 1024

They are both ints, with maximum values of 32,767. But the intermediate step (runAverage * 1100) can potentially be as high as 1,125,300. So, how does C handle this? Are intermediate results all stored in some working memory with plenty of room, or can you get an error or some unexpected behavior?

What do you mean? I suppose I was trying extra hard to make sure that all of that equation was processed as a float, and I had read that you should add the (float) if you want an initially non-FP number to be processed as a FP. I was also told that if you set a FP number to equal the result of some operation with an integer, that the equation will be first processed as integer math unless you use the (float) conversion. Is that true?

True, but irrelevant. The numbers in that equation might mean something to you today. Will they mean the same thing in 2 weeks? To someone else?

#define REFVOLT 1.1
#define MAXREAD 1024.0
#define STRANGE 0.1232
#define WEIRD 1000.0
  fBattVoltage = (((float)runAverage/MAXREAD)*REFVOLT)/(STRANGE)*WEIRD;

Now, the equation contains no magic numbers, and the names (should) tell you something about the value (at least enough to go "Oh, yeah, that's what that number means").

For instance, if you have an integer "A" set to 100, and a long integer "B," the operation "B = A * 100000000" will work correctly, yes?

Probably not. Literals are interpreted as ints, absent any indicator to the contrary. 100000000 is not an int. The operation "B = A * 100000000UL" will work correctly, yes? Yes.

But, again, irrelevant. You have no cases where you are doing that. At each step of the way to developing code for an Arduino, you must chose appropriately sized and typed variables, based on the maximum and minimum values to be stored in the variable, and the type of the value. If the minimum is 0, using signed types is a waste. If the maximum value is 1000, using a long is a waste. Use an int. If the maximum value is 100, even an int is a waste. Use a byte.

Often, you don't know exact maximum values. In these cases, you must look at the functions providing the values. For instance, digitalRead() returns an int, but it could (and should) return a byte, instead, since the maximum value it can return is 1, and the minimum is 0. The analofRead function returns an int, with a range of values from 0 to 1023.

I got a little crazy with the long int declarations...

Yep.

Here's a prime example of my confusion:

cellVoltage = (runAverage * 1100) / 1024

They are both ints, with maximum values of 32,767. But the intermediate step (runAverage * 1100) can potentially be as high as 1,125,300. So, how does C handle this? Are intermediate results all stored in some working memory with plenty of room, or can you get an error or some unexpected behavior?

And a really good example, too. No, intermediate results are not stored in a some kind of memory with lots of room. They are stored in an register of the appropriate size and type. What is appropriate? That is determined by the type of the operands. Since runAverage is an int, and 1100 is an int, the result will be stored in an int register. Oops.

To prevent this from happening, one needs to be aware that this will happen, and takes steps to prevent the problem. One or more of the operands needs to be promoted to a larger type (temporarily, usually). This is done either will a cast or an explicit typing of a literal:
cellVoltage = ((long)runAverage * 1100) / 1024
or
cellVoltage = (runAverage * 1100UL) / 1024

In either case, the intermediate result will be a long. The long/int result will fit in an int, regardless of the value that is stored in runAverage, as runAverage gets valued today.

If that changes, the type of cellVoltage may need to change, too.

Ahhh, I see. That makes things much clearer. Also thanks for pointing out that I need to make my code more readable. :slight_smile:

I did not know that literals were interpreted as ints by default. So in the case of a floating point number, is there a difference between 1023.0 and 1023f? Or - as I wrote - 1023.0f?

Re: your last post:

Ok, so in your example,
cellVoltage = (runAverage * 1000)/1024UL
would either return an error or not run correctly as you are dividing an int by a long?

Say you had cellVoltage as a long int and then used the original equation. Would the intermediate result also be stored in a long int register, or will the operations to the right of the = be carried out first (as integers, stored in an int register) and then the (incorrect) result written to the long int cellVoltage? But if runAverage is a long int, there is no such ambiguity as you explained.

It's nice to be able to get good and direct advice without having to spend hours puzzling through it on your own. Of course that's a good way to learn too...
Sorry for asking so many questions, I really appreciate the help!

EDIT: After just trying it out instead of asking lots of questions, I do indeed get incorrect results with
cellVoltage = runAverage1000/1024; (even with cellVoltage set as a long int)
and
cellVoltage = runAverage
1000/1024UL;

However (long)runAverage1100 and runAverage1100UL both work. Not that this will come as a surprise to you. :slight_smile: It's nice to see things working though...

I did not know that literals were interpreted as ints by default. So in the case of a floating point number, is there a difference between 1023.0 and 1023f? Or - as I wrote - 1023.0f?

My comment about literals only applies to literals that do not contain decimal points. So, 1023.0 will be interpreted as a float (most compilers would interpret it as a double, which is usually twice as big as a float (not on the Arduino, however). 1023f would be interpreted as a float, since the f is appended. 1023.0f is also a float, not a double (not that it matters on the Arduino, but it does on other platforms).

Ok, so in your example,
cellVoltage = (runAverage * 1000)/1024UL
would either return an error or not run correctly as you are dividing an int by a long?

That was not my example. My example had the 1000 (actually 1100) as the long and 1024 as the int.

Dividing an int by a long is no problem, as the int will be implicitly promoted to a long for the division operation.

Say you had cellVoltage as a long int and then used the original equation. Would the intermediate result also be stored in a long int register, or will the operations to the right of the = be carried out first (as integers, stored in an int register) and then the (incorrect) result written to the long int cellVoltage?

The type that the final result will be stored in is not considered when sizing intermediate storage, so the intermediate result of int times int would not be a long. It would be an int. Explicit casting of one (or both) or the operands IS required.

PaulS:

Ok, so in your example,
cellVoltage = (runAverage * 1000)/1024UL
would either return an error or not run correctly as you are dividing an int by a long?

That was not my example. My example had the 1000 (actually 1100) as the long and 1024 as the int.

I know, I should have said "In your example (that I changed in the next line)."

Dividing an int by a long is no problem, as the int will be implicitly promoted to a long for the division operation.

Funny you should say that...I actually just tried running all these different scenarios on the Arduino (reading the ADC and converting the result to a voltage).
It works when either runAverage or 1100 is set to be interpreted as a long int (such as writing 1100UL), but if it is changed to:
cellVoltage = (runAverage * 1100) / 1024UL;
I get garbage again. I.e. incorrect results, random negative numbers, etc...

but if it is changed to:
cellVoltage = (runAverage * 1100) / 1024UL;
I get garbage again. I.e. incorrect results, random negative numbers, etc...

Well, sure, but not because you are dividing a long by an int. You get bad results because the int you are dividing is not capable of holding the int * int value. When the calculation is not int * int, the long/int or long/long division works because the dividend is valid, as well as the divisor.