# analogRead problem

I've setup my arduino duemilanove to output to 3 digit 7-segment leds. I can print out any 3 digit combination manually. Now I want to use analog input from a temperature sensor to print out the numbers but I'm having problems with the analogRead function.

If I use the analogRead function, the LEDs print out gibberish. If I comment out the analogRead function and manually set the digits, the LEDs output correctly.

This is my code for reading the temperature sensor:

``````getLocalTemperature(0);

.
.
.

byte* getLocalTemperature(int localTemperatureSensorPin)
{
int tempReading = 0;
float tempRatio = 0;
float temp2Kelvin = 0;
float temperature = 0;
byte temperatureArray[3];

tempReading = analogRead(localTemperatureSensorPin);

tempRatio = (tempReading/1023)*5;
temp2Kelvin = tempRatio/(0.0001);
temperature = ((temp2Kelvin - 273) * 1.8) + 32;

temperatureArray[0] = dataArray[5];
temperatureArray[1] = dataArray[0];
temperatureArray[2] = dataArray[5];

return temperatureArray;
}
``````

The temperatureArray data will be used for the 3 7-segment LEDs. Right now I have manually entered data for it, but it's showing gibberish if the analogRead() function is uncommented.

Analog input 0 is being used. The temperature sensor is outputting 2.94v. What could be the problem?

``````    tempRatio = (tempReading/1023)*5;
``````

The tempReading variable is an integer, with a value of 0 to 1023. Dividing that value by 1023 will give either a 0 or a 1 (almost always 0). 0 * 5 is 0.

``````    temp2Kelvin = tempRatio/(0.0001);
``````

Why is the 0.0001 in parentheses? Division is much slower that multiplication, so multiplication is generally preferred. Not that it really matters, since 0 divided by anything is still 0.

``````    temperatureArray[0] = dataArray[5];
temperatureArray[1] = dataArray[0];
temperatureArray[2] = dataArray[5];
``````

What the heck is this doing, and what does it have to do with the analog reading that you are mangling?

``````    return temperatureArray;
``````

You're returning garbage.

I'm outputting to an 8-bit serial to parallel shift register which then outputs to a 3 digit 7-segment display. I'm using dataArray to store the correct signals for numbers 0-9;

``````   // 0 is ON, 1 is OFF
dataArray[0] = 0xC0; //11000000
dataArray[1] = 0xF9; //11111001
dataArray[2] = 0xA4; //10100100
dataArray[3] = 0xB0; //10110000
dataArray[4] = 0x99; //10011001
dataArray[5] = 0x92; //10010010
dataArray[6] = 0x82; //10000010
dataArray[7] = 0xF8; //11111000
dataArray[8] = 0x80; //10000000
dataArray[9] = 0x90; //10010000
``````

Let me simplify my code:

``````byte* getLocalTemperature(int localTemperatureSensorPin)
{
int tempReading = 0;
byte temperatureArray[3];

tempReading = analogRead(localTemperatureSensorPin);

temperatureArray[0] = dataArray[5]; //displays 5 on LED 1
temperatureArray[1] = dataArray[0]; //displays 0 on LED 2
temperatureArray[2] = dataArray[5]; //displays 5 on LED 3

return temperatureArray;
}
``````

If I run this code the output from the 3 digit 7-segment display is gibberish.

If I comment out:

`````` tempReading = analogRead(localTemperatureSensorPin);
``````

The output reads: 505 //which is correct

Right now I'm not using the analog input (tempReading). So why does the LED outputs change when I use the function analogRead?

You first post did not include the dataArray declaration/definition code.

Without seeing what you have connected to what pins on the Arduino, it's hard to say what the problem is. Have you declared the correct pin to be an INPUT pin?

Sorry. Here’s the entire code:

``````////-- PINOUTS

//OUTPUTS
int latchPin = 5; //ST_CP pin 12
int clockPin = 4; //SH_CP pin 11
int dataPin = 3; //DS pin 14
int ledSelectPin = 2; //DS pin 14 other SR

//INPUT
int localTemperatureSensorPin = 0;

////-- PINOUTS END

int count = 0;

byte ledSelectData;
byte ledSelectDataArray[3];
byte* data;
byte dataArray[10];

void setup()
{
pinMode(latchPin, OUTPUT);
pinMode(ledSelectPin, OUTPUT);
pinMode(clockPin, OUTPUT);
pinMode(dataPin, OUTPUT);

// 0 is ON, 1 is OFF
dataArray[0] = 0xC0; //11000000
dataArray[1] = 0xF9; //11111001
dataArray[2] = 0xA4; //10100100
dataArray[3] = 0xB0; //10110000
dataArray[4] = 0x99; //10011001
dataArray[5] = 0x92; //10010010
dataArray[6] = 0x82; //10000010
dataArray[7] = 0xF8; //11111000
dataArray[8] = 0x80; //10000000
dataArray[9] = 0x90; //10010000

ledSelectDataArray[0] = 0x01; //LED 1
ledSelectDataArray[1] = 0x02; //LED 2
ledSelectDataArray[2] = 0x04; //LED 3
}

void loop()
{

data = getLocalTemperature(localTemperatureSensorPin);
outputTemp(ledSelectDataArray, ledSelectPin, dataPin, clockPin, data, latchPin);

}

void shiftOut(byte myledSelectData, int myledSelectPin, int myDataPin, int myClockPin, byte myDataOut)
{
int i=0;
int pinState;
int ledSelectPinState;

digitalWrite(myDataPin, 0);
digitalWrite(myClockPin, 0);
digitalWrite(myledSelectPin, 0);

for(i=7; i>=0;i--)
{
digitalWrite(myClockPin, 0);

if(myDataOut & (1<<i) )
{
pinState = 1;
}
else
{
pinState = 0;
}

if(myledSelectData & (1<<i) )
{
ledSelectPinState = 1;
}
else
{
ledSelectPinState = 0;
}

digitalWrite(myledSelectPin, ledSelectPinState);
digitalWrite(myDataPin, pinState);

digitalWrite(myClockPin, 1);

digitalWrite(myDataPin, 0);
digitalWrite(myledSelectPin, 0);
}

digitalWrite(myClockPin, 0);
}

void outputTemp(byte* myledSelectData, int myledSelectPin, int myDataPin, int myClockPin, byte* myData, int myLatchPin)
{
int i=0;

for(i=0;i<=2;i++)
{
digitalWrite(myLatchPin, 0);

if(i==1)
{
byte ledDot = 0x7F;
byte tempData = myData[i];
myData[i] = (tempData & ledDot);
}

shiftOut(myledSelectData[i], myledSelectPin, myDataPin, myClockPin, myData[i]);

digitalWrite(myLatchPin, 1);
delay(500);
}
}

byte* getLocalTemperature(int localTemperatureSensorPin)
{

int tempReading = 0;
byte temperatureArray[3];

float tempRatio = 0;
float temp2Kelvin = 0;
float temperature = 0;

tempReading = analogRead(localTemperatureSensorPin);

tempRatio = (tempReading/1023)*5;
temp2Kelvin = tempRatio/(0.0001);
temperature = ((temp2Kelvin - 273) * 1.8) + 32;

temperatureArray[0] = dataArray[5];
temperatureArray[1] = dataArray[0];
temperatureArray[2] = dataArray[5];

return temperatureArray;
}
``````

For analog signals you don’t need to set pinMode correct? I thought that was only for digital signals.

I was able to determine that the analogRead function was inputting the signal from the temperature sensor; however, it seems like I can't manually change the value of temperatureArray, or data when the getLocalTemperature function returns. This only occurs when using the analogRead function.

The code below outputs gibberish:

``````void loop()
{

data = getLocalTemperature(localTemperatureSensorPin);
outputTemp(ledSelectDataArray, ledSelectPin, dataPin, clockPin, data, latchPin);

}
``````

The code below outputs gibberish:

``````void loop()
{

data = getLocalTemperature(localTemperatureSensorPin);
data[0] = dataArray[1];
data[1] = dataArray[2];
data[2] = dataArray[3];
outputTemp(ledSelectDataArray, ledSelectPin, dataPin, clockPin, data, latchPin);

}
``````

However, the code below does work:

``````void loop()
{

data[0] = dataArray[1];
data[1] = dataArray[2];
data[2] = dataArray[3];
outputTemp(ledSelectDataArray, ledSelectPin, dataPin, clockPin, data, latchPin);

}
``````

Is this a memory issue?

You're problem has nothing to do with analogRead. The problem is here:

``````byte* getLocalTemperature(int localTemperatureSensorPin)
{
byte temperatureArray[3];

// snipped

temperatureArray[0] = dataArray[5];
temperatureArray[1] = dataArray[0];
temperatureArray[2] = dataArray[5];

return temperatureArray;
``````

temperatureArray is a local variable. It goes out of scope when the function ends. You are returning a pointer to that local variable. There is no telling what that pointer points to when the memory gets reused.

The getLocalTemperature function should return a float - the temperature. If you need for it to do multiple things, you can change the declaration of temperatureArray:

``````byte *temperatureArray = new byte[3];
``````

This will allocate space on the heap for the values. The variable is still a local variable, and still goes out of scope at the end of the function, but the allocated memory does not.

If you do this, you will need to de-allocate the space in the calling routine:

``````delete [] data;
data = NULL;
``````

This goes after the outputTemp call in loop.

Thanks PaulS! That solved my problem ;D

I couldn't use the code:

``````byte *temperatureArray = new byte[3];
``````

and

``````delete [] data;
data = NULL;
``````

Instead I used:

``````byte* temperatureData = (byte *) malloc(3);
``````

and

`````` free(data);
data = NULL;
``````

I have the getLocalTemperature function setup to output the temperature already coded to correctly display into the 7-segment LCD. Is it better to have another function that converts float to byte (7-segment coded)?

The new/delete vs. malloc/free issue is a C++ vs. C issue. Trips me quite often. Glad you saw the intent and were able to make the correct change.

I have the getLocalTemperature function setup to output the temperature already coded to correctly display into the 7-segment LCD. Is it better to have another function that converts float to byte (7-segment coded)?

The end result is the same. From a purist's point of view, each subroutine should do one thing, and only one thing, and should do that thing well.

You getLocalTemperature routine gets the sensor reading, adjusts it, and converts it to BCD.

You might decide that the seven segment LED is not the correct output device, and that a LCD is more to your liking.

As it's written now, getLocalTemperature would need to change because of a hardware change that it totally unrelated to the temperature acquisition function.

For small programs, you can get away with this stuff. For large programs, it becomes a nightmare trying to find which functions are affected.

So, yes, I'd recommend two functions - one to get the temperature (corrected, as need be), and one to convert a number to a BCD-compatible format.

You might decide to add a clock, and alternate the time and temperature on the display (OK, maybe not with a 3 character display). Then, the fact that you have a number to BCD converter handy will mean not having to duplicate a bunch of code.