calgorithmaveragepicmikroc

My function solving for average value delivers random values


I'm using MikroC to try and program a PIC4550 to retrieve data from pH Sensor. The program consist of retrieving data 40 times and calculating its average value with function Average below

double Average(int* arr, int number)
{
    int i;
    int ave;
    int sum = arr[0];
    for(i = 0;i<number;i++)
    {
        sum=sum+arr[i];
    }
    ave = sum/number;
    return ave;
}

My problem is it delivers random values very far from the right answer most of the time. For example, the right answer is 633 but it delivers -185. Can anyone point out my mistake? thanks. My whole code is below

#define volt 5.00 //system voltage
#define arraylength 40 // number of data collection
#define offsetph 5   //deviate compensate

double Average(int*, int);

void main() {
     float ph_res=0;
     float pHsensor_output[40];
     double avepH;
     double pHvoltage;
     float offsetpH=0.38;  //change to actual pH offset
     float pHlevel;
     char testing[20];
     char pher[20];
     int i;
     int pH_index=0;
     UART1_Init(9600);
     ADCON1=0x00;
     CMCON=7;

     TRISA=0xFF;


   while(1){
    UART1_Write_Text("Reading pH....\r\n");
     for(i = 0; i<40; i++) {
            pHsensor_output[i] = ADC_Read(1);
            sprintf(testing,"READING: %.2f\r\n",pHsensor_output[i]);
            UART1_Write_Text(testing);
            delay_us(1000);
        }
        avepH = Average(pHsensor_output, 40);
        sprintf(testing,"AVG: %f\r\n",avepH);
        UART1_Write_Text(testing);
        pHvoltage = avepH*5/1024;
        pHlevel = 3.5*pHvoltage+offsetph;
        if(pHlevel!=pH_res){
            pH_res = pHlevel;

        }
             sprintf(pher,"pH: %f\r\n",pH_res);
            UART1_Write_Text(pher);

     delay_ms(2000);
       } }
double Average(int* arr, int number)
{
    int i;
    int ave;
    int sum = arr[0];
    for(i = 0;i<number;i++)
    {
        sum=sum+arr[i];
    }
    ave = sum/number;
    return ave;
}

Solution

  • Several of the comments have pointed out that you are at risk of overflow during your summation. If this is a 16-bit machine, INT_MAX is 32767. As per your example, you're not getting close to this, but it's something to be careful of.

    Anyway, you're collecting values into an array of floats:

     float pHsensor_output[40];
     ...
     pHsensor_output[i] = ADC_Read(1);
    

    But later passing that to a function expecting an array of ints:

    double Average(int* arr, int number)
    ...
      avepH = Average(pHsensor_output, 40);
    

    (The fact that Average calculates with ints and returns a double means you might get a truncated answer, but it's not the problem here.)

    Your computer is now looking at a pattern of bits that might be '633' when viewed as a float, but almost certainly won't be when viewed as an integer. So who knows what the result will be when you take the average of them?!

    Try changing your function to something like:

    double Average(float* arr, int number)
    {
        int i;
        double ave;
        double sum = 0.0; // NOTE: not arr[0];
        for(i = 0;i<number;i++)
        {
            sum=sum+arr[i];
        }
        ave = sum/number;
        return ave;
    }
    

    Addendum: your compiler really should have complained about this. Did it really keep quiet? GCC barfs immediately.

    error: cannot convert ‘float*’ to ‘int*’ for argument ‘1’ to ‘double Average(int*, int)’
    

    Edit: It's always nice to understand by seeing, so I tried a little example. Mine is a desktop, not a PIC so int is 32-bits not 16, but hopefully the point is the same....

    int main()
    {
        union {
            float my_float;
            int my_int;
        } hack;
    
        printf ("sizeof(float) %lu, sizeof(int) %lu\n", sizeof(float), sizeof(int));
    
        hack.my_float = 633.0;
        printf( "%f -> %d\n", hack.my_float, hack.my_int );
    
        int sum = 40 * hack.my_int;
        int average = sum / 40;
    
        printf( "sum %d  -->  %d\n", sum, average );
    
        return 0;
    }
    

    Yields:

    sizeof(float) 4, sizeof(int) 4
    633.000000 -> 1142833152
    sum -1531314176  -->  -38282854
    

    What this 'shows' is that a nice float becomes a crazy int (when the same pattern of bits is looked at differently1), and after that, all bets are off. Clearly, the summation overflows, and hence the average comes out negative; not what you'd have expected at all!

    1 The 'union hack' I used in my example says to the computer: "take the memory location holding a float and look at the exact same bits but pretend it's an int. This is what's happening when you pass a pointer to some floats to a function expecting a pointer to ints.

    But if you do an assignment 'cleanly' you get what you'd expect:

        int nice_int = hack.my_float;  // 633.0
        printf( "good %f  ->  %d\n", hack.my_float, nice_int );
    

    yields:

    good 633.000000  ->  633