Ok still stumped on my averaging code

Somewhere something is going wrong with my averaging code, I am reading temperatures and light from sensors which is working fine (ish), so I have been running it for the past day or so.

I currently get displayed

70.1 67.4 67.4 71.7 for heat which is current, average, min, max
81.9 4.8 4.7 89.9 for light same order

it seems that my averages are working their way down over night and staying there, as it was 70 up until 10pm last night and its 70 now at 10am the average shouldnt be the same as the minimum!

I am working on a rolling average, temperatures are measured in 10ths of a degree (ie 700 = 70 degrees), and to allow for rounding errors I multiply by 10 before averaging!

Here is the relevent code sections, can anyone spot any problems?

I left out the reading and display code, as this is working fine, readlight and read temp return ints, and as I get values displayed are working fine.

So I can only assume that I have made a mistake in my syntax somewhere, or in the way I calculate the average!

// *****************************************************************************
//                          VARIABLE INITIALIZATION
// *****************************************************************************
const int SAMPLES_TO_AVERAGE = 30 ;                   // Used for Analog Average
const int MAX_AVERAGE_SAMPLES = 24*60*60 ;            // Used for Average to avoid overflow set to 1 days worth of seconds

const int TempSensor = 3 ;
const int LightSensor =0 ;


char Buffer[8] ;                                      // Storage for Display strings

int LastLight = 0 ;                // Last light value read
int MaxLight = 0 ;                 
int MinLight = 9999 ;
long LightRunningAverage = 0;
long LightAverageCount   = 0 ;

int LastTemp = 0 ;
int MaxTemp = 0 ;
int MinTemp = 9999 ;
long TempRunningAverage = 0 ;
long TempAverageCount   = 0 ;

//unsigned long counter = 0 ;

LedControl lc=LedControl(9,10,11,4);
// *****************************************************************************
//                             SETUP FUNCTION
// *****************************************************************************
void setup() {
     Serial.begin(9600) ;
     initializeDisplays() ;
     DisplayString(3,"LOAdInG_") ;
     lc.clearDisplay(3) ;
}

// *****************************************************************************
//                             LOOP FUNCTION
// *****************************************************************************
void loop() { 

     ProcessLight() ;   // read light and calculate averages
     MatrixLight() ;    // display the word light on matrix display
     DisplayInt(3,0,LastLight) ;                // display the last reading
     DisplayInt(3,1,LightRunningAverage/10) ;   // display the average
     DisplayInt(2,0,MinLight) ;                 // Display Minimum value
     DisplayInt(2,1,MaxLight) ;                 // Display Maximum Value

     delay(1000) ;

     ProcessTemp() ;
     MatrixHeat() ;
     DisplayInt(3,0,LastTemp) ;
     DisplayInt(3,1,TempRunningAverage / 10) ;
     DisplayInt(2,0,MinTemp) ;
     DisplayInt(2,1,MaxTemp) ;

     delay(1000) ;
}
// *****************************************************************************
//                       Process Light Readings
// *****************************************************************************
void ProcessLight() {

     int Light = readLight(LightSensor) ;


     if (LightAverageCount < MAX_AVERAGE_SAMPLES) {
          LightRunningAverage = ((LightRunningAverage * LightAverageCount) + Light*10) / (LightAverageCount + 1) ;     
          LightAverageCount++ ;  
     } 
     else {
          LightRunningAverage = ((LightRunningAverage * (LightAverageCount-1) + Light*10 )) / (LightAverageCount);
     }
     if (Light > MaxLight) {
          MaxLight = Light ; 
     }

     if (Light < MinLight) { 
          MinLight = Light ;
     }
}
// *****************************************************************************
//                       Process Temperature Readings
// *****************************************************************************
void ProcessTemp() {

     int Temp = readTempF() ;
     long t1 = 0 ;
     long t2 = 0 ;
     long t3 = 0 ;

     if (TempAverageCount < MAX_AVERAGE_SAMPLES) {
          TempRunningAverage = ((TempRunningAverage * TempAverageCount) + (LastTemp*10)) / (TempAverageCount + 1) ;     
          TempAverageCount++ ;  
     } 
     else {
          TempRunningAverage = ((TempRunningAverage * (TempAverageCount-1) + LastTemp*10 )) / (TempAverageCount);
     }

     if (Temp > MaxTemp) {
          MaxTemp = Temp ; 
     }
     if (Temp < MinTemp) { 
          MinTemp = Temp ;
     }
}

First obvious (to me ;) ) problem is that MAX_AVERAGE_SAMPLES overflows the 15-bit signed int datatype, perhaps resulting in frequent resets.

Upgrade to 32 bits:

static const uint32_t MAX_AVERAGE_SAMPLES = 24L*60L*60L;

Also promote your other long's that will never be required to hold a -ve value as a final or intermediate value to uint32_t. There may be other bugs but you need to clear this one up first. :)

Thanks, I made that change, obviously I need to collect some data to see if it helps, that may be the problem originally I ran 100 samples to test the code, but I eventually want to average over a 48 hour period.

I had the longs as unsigned but had issues so I changed them back to regular longs!

Again It seems that the Light is working, getting a nice middling average, but temperature is sticking at the minimum vavlue and by observation, I am getting more larger values than the minimum!

Also the average should never re-set once the maximum number of samples is arrived at the average deletes one sample and adds the next, it was the best way i could think of with limited memory to average 86400 values!

What are t1, t2, and t3 declared for in ProcessTemp?

but temperature is sticking at the minimum vavlue and by observation, I am getting more larger values than the minimum!

Do you mean that the temperature being observed is staying at the minimum, or that the average temperature is not being computed correctly?

If the temperature varies, I'd [u]expect[/u] to get larger values than the minimum.

Also the average should never re-set once the maximum number of samples is arrived at the average deletes one sample and adds the next

Of course the average should change. If you turn the lights and heat off for a week, and start the Arduino, you will get low numbers for average temperature and light level.

Then, at the end of the week, crank up the heat and turn the lights on. At the end of another week, I'd expect to see a big increase in average temperature and light levels.

In any case, I think there is a fundamental flaw in how you are computing the temperature average.

You take a reading, and store it in Temp. Temp is then NOT used in computing the average, regardless of the number of readings taken. The value used in computing the running average, LastTemp is never reset to Temp.

LastTemp is not subtracted from the running average.

Quote: What are t1, t2, and t3 declared for in ProcessTemp?

they where for debuggingI broke the formula down ad printed to the serial port

Quote: but temperature is sticking at the minimum vavlue and by observation, I am getting more larger values than the minimum!

Do you mean that the temperature being observed is staying at the minimum, or that the average temperature is not being computed correctly? Quote:

The average is staying at the minimum, which after observing the current temperatures displayed wouldnt make sense!

If the temperature varies, I'd expect to get larger values than the minimum.

Quote: Of course the average should change. If you turn the lights and heat off for a week, and start the Arduino, you will get low numbers for average temperature and light level.

Yes the average should vary, but it seems to move down to the minimum then stick, never get higher again, and as for LastTemp it is set in the reading routine, rather than the Processing Routine, I just left the temp = readTempC() as it did the same!

The puzzling thing is that Light readings seem to work, but temperature dont, I have made sure that LastTemp and LastLight have valid readings, so it has to be either in the way I calculate or the way I declare the variables, unfortunately I cant seem the reason its not working!

(BTW I have 30 years of assembler and BASIC programming experience along with a reasonable knowledge of maths)

     int [glow]Light [/glow]= readLight(LightSensor) ;


     if (LightAverageCount < MAX_AVERAGE_SAMPLES) {
          LightRunningAverage = ((LightRunningAverage * LightAverageCount) + [glow]Light[/glow]*10) / (LightAverageCount + 1) ;
          LightAverageCount++ ;
     }
     else {
          LightRunningAverage = ((LightRunningAverage * (LightAverageCount-1) + [glow]Light[/glow]*10 )) / (LightAverageCount);
     }

For computing the average light level, you use the reading that you took.

     int [glow]Temp [/glow]= readTempF() ;

     if (TempAverageCount < MAX_AVERAGE_SAMPLES) {
          TempRunningAverage = ((TempRunningAverage * TempAverageCount) + ([glow]LastTemp[/glow]*10)) / (TempAverageCount + 1) ;
          TempAverageCount++ ;
     }
     else {
          TempRunningAverage = ((TempRunningAverage * (TempAverageCount-1) + [glow]LastTemp[/glow]*10 )) / (TempAverageCount);
     }

For averaging the temperature, you read the temperature, and average something else.

(BTW I have 30 years of assembler and BASIC programming experience along with a reasonable knowledge of maths)

Well, I have 30+ years experience with Fortran, C, C++, and C#. I still manage to make mistakes once in a while.

except this isnt a mistake :-

LastTemp is set, temp in the routine isnt used, i just left it in rather than change the function to a void, lazy but I removed some code after changing the circutry!

#
/ *****************************************************************************
//                 Read value from light sensor re-scale 0-1000
// *****************************************************************************
int readLight(int analogPort) {

     SetDefaultReference() ;
     LastLight = int(readAnalog(LightSensor,SAMPLES_TO_AVERAGE)*1000/1023) ;
     return(LastLight) ;
}
//*********************************************************************
//                   Read Tempreature from LM35 in F
//*********************************************************************
int readTempF() {

     LastTemp = readTempC() * 9 / 5 + 320 ;
     return(LastTemp);  
}
//*********************************************************************
//                   Read Tempreature from LM35 in C
//*********************************************************************
int readTempC() {

     SetInternalReference();
     LastTemp = ((100*1.1*readAnalog(TempSensor,SAMPLES_TO_AVERAGE))/1024)*10 ;
     return( LastTemp);  
}

Just to clarify, that I get readings Min/Max/Current fine, Average is being fed values, its just that the average output is not what it should be.

The current temprerature has been 71+ for the last hour, yet the average hasnt moved from 69.9, where as the range is 69.9 to 71.7!

This to me should result in the average rising!

full sketch less process temp/light :-

/ Input:
// LM35Z Temperature sensor on analog port 3
// LDR Voltage divider on analog port 0
// Output:
// 4x 4 digit LED displays on MAX7219 controlled by pins 9,10,11   - Displays 3,2
// 2x 8x8 matrix LED displays on MAX 7219 controlled by same pins  - Displays 1,0
// *****************************************************************************
//                             LIBRARY INCLUDES
// *****************************************************************************
#include "LedControl.h"
// *****************************************************************************
//                          VARIABLE INITIALIZATION
// *****************************************************************************
const int SAMPLES_TO_AVERAGE = 30 ;                   // Used for Analog Average
static const uint32_t MAX_AVERAGE_SAMPLES = 24L*60L*60L;   // Used for Average to avoid overflow set to 1 days worth of seconds

const int TempSensor = 3 ;
const int LightSensor =0 ;


char Buffer[8] ;                                      // Storage for Display strings

int LastLight = 0 ;                // Last light value read
int MaxLight = 0 ;                 
int MinLight = 9999 ;
long LightRunningAverage = 0;
long LightAverageCount   = 0 ;

int LastTemp = 0 ;
int MaxTemp = 0 ;
int MinTemp = 9999 ;
long TempRunningAverage = 0 ;
long TempAverageCount   = 0 ;

//unsigned long counter = 0 ;

LedControl lc=LedControl(9,10,11,4);
// *****************************************************************************
//                             SETUP FUNCTION
// *****************************************************************************
void setup() {
     Serial.begin(9600) ;
     initializeDisplays() ;
     DisplayString(3,"LOAdInG_") ;
     lc.clearDisplay(3) ;
}

// *****************************************************************************
//                             LOOP FUNCTION
// *****************************************************************************
void loop() { 

     ProcessLight() ;   // read light and calculate averages
     MatrixLight() ;    // display the word light on matrix display
     DisplayInt(3,0,LastLight) ;                // display the last reading
     DisplayInt(3,1,LightRunningAverage/10) ;   // display the average
     DisplayInt(2,0,MinLight) ;                 // Display Minimum value
     DisplayInt(2,1,MaxLight) ;                 // Display Maximum Value

     delay(1000) ;

     ProcessTemp() ;
     MatrixHeat() ;
     DisplayInt(3,0,LastTemp) ;
     DisplayInt(3,1,TempRunningAverage / 10) ;
     DisplayInt(2,0,MinTemp) ;
     DisplayInt(2,1,MaxTemp) ;

     delay(1000) ;
}
// *****************************************************************************
//                     Display Light on LED Matrix
// *****************************************************************************
void MatrixLight() {
     lc.setRow(0,0,B01111110);
     lc.setRow(0,1,B01000000);
     lc.setRow(0,2,B00000000);
     lc.setRow(0,3,B01111110);
     lc.setRow(0,4,B00000000);
     lc.setRow(0,5,B01001110);
     lc.setRow(0,6,B01001010);
     lc.setRow(0,7,B01111110);
     lc.setRow(1,0,B00000000);
     lc.setRow(1,1,B01111110);
     lc.setRow(1,2,B00001000);
     lc.setRow(1,3,B01111110);
     lc.setRow(1,4,B00000000);
     lc.setRow(1,5,B00000010);
     lc.setRow(1,6,B01111110);
     lc.setRow(1,7,B00000010);
}
// *****************************************************************************
//                     Display Heat on LED Matrix
// *****************************************************************************
void MatrixHeat() {
     lc.setRow(0,0,B01111110);                   
     lc.setRow(1,0,B01111110);            
     lc.setRow(0,1,B00001000);                   
     lc.setRow(1,1,B00010010);            
     lc.setRow(0,2,B01111110);                   
     lc.setRow(1,2,B01111110);            
     lc.setRow(0,3,B00000000);                   
     lc.setRow(1,3,B00000000);            
     lc.setRow(0,4,B01111110);                   
     lc.setRow(1,4,B00000010);            
     lc.setRow(0,5,B01001010);                   
     lc.setRow(1,5,B01111110);            
     lc.setRow(0,6,B01000010);                   
     lc.setRow(1,6,B00000010);            
     lc.setRow(0,7,B00000000);                   
     lc.setRow(1,7,B00000000);            

}
// *****************************************************************************
//                     Int to 8 digit LED
// *****************************************************************************
void DisplayInt(int Display,int SubDisplay,int value) {

     sprintf(Buffer,"%d",value) ;
     DisplayStringRight(Display,SubDisplay,Buffer) ;
}
// *****************************************************************************
//                     Int to 8 digit LED
// *****************************************************************************
void DisplayInt(int Display,int value) {

     sprintf(Buffer,"%d",value) ;

     DisplayStringRight(Display,Buffer) ;
}
// *****************************************************************************
//                      String to 8 digit LED
// *****************************************************************************
void DisplayStringRight(int Display,char* String) {

     for (int i= 7 ; i > (7- strlen(String)) ; i--) {
          lc.setChar(Display,i,String[i-(8-strlen(String))],false) ;
     }
     for (int i = 7-strlen(String) ; i >= 0 ; i--) {
          lc.setChar(Display,i,' ',false) ;
     }
}
// *****************************************************************************
//                      String to 8 digit LED
// *****************************************************************************
void DisplayString(int Display,char* String) {

     for (int i= 0 ; i < strlen(String) ; i++) {
          lc.setChar(Display,i,String[i],false) ;
     }
     for (int i = strlen(String) ; i < 8 ; i++) {
          lc.setChar(Display,i,' ',false) ;
     }
}
// *****************************************************************************
//                      String to 4 digit LED
// *****************************************************************************
void DisplayStringRight(int Display,int SubDisplay,char* String) {

     int offset = SubDisplay * 4 ;

     for (int i= 3+offset ; i > (3+offset- strlen(String)) ; i--) {
          lc.setChar(Display,i,String[i-(4+offset-strlen(String))],i==2 || i ==6) ;
     }
     for (int i = 3+offset-strlen(String) ; i >= offset ; i--) {
          lc.setChar(Display,i,' ',false) ;
     }
}
//*********************************************************************
//              Initialize Displays
//*********************************************************************
void initializeDisplays() {

     lc.clearDisplay(0);
     lc.clearDisplay(1);
     lc.clearDisplay(2);
     lc.clearDisplay(3);

     lc.shutdown(0,false);
     lc.shutdown(1,false);
     lc.shutdown(2,false);
     lc.shutdown(3,false);

     lc.setIntensity(0,6);
     lc.setIntensity(1,6);
     lc.setIntensity(2,6);
     lc.setIntensity(3,6);
}
//*********************************************************************
//              Set internal reference discard next results
//*********************************************************************
void SetInternalReference() {

     analogReference(INTERNAL) ;
     ChangeOverDelay();
}
//*********************************************************************
//               Set DEFAULT reference discard next results
//*********************************************************************
void SetDefaultReference() {

     analogReference(DEFAULT) ;
     ChangeOverDelay();
}
//*********************************************************************
//         Read Some Values and Delay while reference changes
//*********************************************************************
void ChangeOverDelay() {
     for (int j =0 ; j<10 ; j++) {
          analogRead(0) ;
          delay(40) ;
     }
}
//*********************************************************************
//              Averaging Read From Port
//*********************************************************************
long readAnalog(int Port, int Samples) {

     long Accumulator = 0 ;

     for (int i = 0; i < Samples ; i++) { 
          Accumulator = Accumulator + analogRead(Port); 
     }  
     return(Accumulator / Samples) ;
}
// *****************************************************************************
//                 Read value from light sensor re-scale 0-1000
// *****************************************************************************
int readLight(int analogPort) {

     SetDefaultReference() ;
     LastLight = int(readAnalog(LightSensor,SAMPLES_TO_AVERAGE)*1000/1023) ;
     return(LastLight) ;
}
//*********************************************************************
//                   Read Tempreature from LM35 in F
//*********************************************************************
int readTempF() {

     LastTemp = readTempC() * 9 / 5 + 320 ;
     return(LastTemp);  
}
//*********************************************************************
//                   Read Tempreature from LM35 in C
//*********************************************************************
int readTempC() {

     SetInternalReference();
     LastTemp = ((100*1.1*readAnalog(TempSensor,SAMPLES_TO_AVERAGE))/1024)*10 ;
     return( LastTemp);  
}

Posting portions of the code is not very helpful. We don't know all the variable types.

What I would do, though, is print MinTemp, MaxTemp, Temp, and the averages at the end of each pass through ProcessTemp.

If readTempC and readTempF are operating on global variables, what is the purpose of the function returning a value?

Post size restricted the code!

it returns a variable as originally I only had 8 digit display, I now added a second and a second matrix, and decided to display the last temperature, as it should make no difference to the code I left it in! So now I have 4x 4digit LED displays displaying Current Temp/Average/Min/Max but the problem is in the averaging despite being above 69.9 degrees since earlier this morning hasn't risen!

Post size restricted the code!

Maybe that should be telling you something...

Part of the problem is that you are using integers to provide the data for averaging. The temperature would need to rise more than one degree to begin affecting the average, for a period of time.

Since you want to display temperature with a decimal point, why are you not using floats? The sensor returns an integer value, but that is multiplied by 1.1 which converts the value to a float. Then, the result is multiplied by 100, and divided by 1024, then multiplied by 10. I’d expect you to get better results if readTempC and readTempF returned floats. Then, the multiplication of LastTemp by 10 during the averaging would not be necessary.

A small change in temperature, like the increase from 69.9 to 71+, might not have any impact on the average, if the temperature has been being averaged over a long period of time.

Adding some Serial.print and Serial.println statements to ProcessTemp() will be more useful, in the long run, than staring at (unfortunately) parts of the code.

From coding in assembler using ints and scaling is often easier than floats since my data is 0-1023 which is an int it makes sense to return an int scaled to where I want it.

but given that I return values which do change then the average should change the multiplication by 10 and division came from analyzing the results as I was getting a variance the x10 should actually result in the average changing more frequently than it would otherwise.

I do a fair bit of analasys of numbers and the logic is sound, I just think somewhere that the code is changing what should happen in the real world! But I still cant expain why the average is sticing at the minimum, if there was a flaw in inputs I should get other answers which make less sense.

Fortunately right now the sun has come out and the last hour or so has raised the max to 73 but again the average doesnt change! its still the minimum!

Well, if the sun is out and the temperature is 73, I can understand why you are reluctant to make code changes.

It's raining and 58 here, so I'd happily make the code changes, but I don't have a temperature sensor, so that makes it difficult for me.

Lol its 73 inside probably 58 outside, glad we learn August is summer at school, since its rained every day since August started!

TempRunningAverage = ((TempRunningAverage * TempAverageCount) + (LastTemp*10)) / (TempAverageCount + 1);
TempAverageCount++ ;

Integer divisions in C/C++ always rounds down. You are accumulating these errors in TempRunningAverage.

A better solution is to store the sum of the temperatures and divide by count in the display function instead.

Good point I was assuming that it would round down but that would make sense, thats why i included the *10 /10 idea.

I will modify the routine and try that, see if that looses the error!

So far so good!

Averages are looking good, only time will tell now if I get the creep to the minimum!

Thanks DrHex for the pointer hopefully that will solve the problem

after modifications : -

// *****************************************************************************
//                       Process Light Readings
// *****************************************************************************
void ProcessLight() {

     int Light = readLight(LightSensor) ;


     if (LightAverageCount < MAX_AVERAGE_SAMPLES) {
          LightRunningTotal = (LightRunningTotal  + Light) ;     
          LightAverageCount++ ;  
     } 
     else {
          LightRunningTotal = LightRunningTotal  + Light - (LightRunningTotal / LightAverageCount);
     }
     if (Light > MaxLight) {
          MaxLight = Light ; 
     }

     if (Light < MinLight) { 
          MinLight = Light ;
     }
}
// *****************************************************************************
//                       Process Temperature Readings
// *****************************************************************************
void ProcessTemp() {

     int Temp = readTempF() ;
     
     if ( TempAverageCount < MAX_AVERAGE_SAMPLES) {
           TempRunningTotal = ( TempRunningTotal  +  Temp) ;     
           TempAverageCount++ ;  
     } 
     else {
           TempRunningTotal =  TempRunningTotal  +  Temp - ( TempRunningTotal /  TempAverageCount);
     }

     if (Temp > MaxTemp) {
          MaxTemp = Temp ; 
     }
     if (Temp < MinTemp) { 
          MinTemp = Temp ;
     }
}

And

// *****************************************************************************
//                             LOOP FUNCTION
// *****************************************************************************
void loop() { 

     ProcessLight() ;   // read light and calculate averages
     MatrixLight() ;    // display the word light on matrix display
     DisplayInt(3,0,LastLight) ;                // display the last reading
     DisplayInt(3,1,LightRunningTotal/LightAverageCount) ;   // display the average
     DisplayInt(2,0,MinLight) ;                 // Display Minimum value
     DisplayInt(2,1,MaxLight) ;                 // Display Maximum Value

     delay(1000) ;

     ProcessTemp() ;
     MatrixHeat() ;
     DisplayInt(3,0,LastTemp) ;
     DisplayInt(3,1,TempRunningTotal/TempAverageCount) ;
     DisplayInt(2,0,MinTemp) ;
     DisplayInt(2,1,MaxTemp) ;

     delay(1000) ;
}

Problem solved, I now have averages that actually move!

Perhaps now I can plug my graphing routines back into the code and get my high low graphs on the matrix!!!