CODEWITHSUNDEEP
cembeddedpic
Günkut Ağabeyoğlu
Günkut Ağabeyoğlu

Reputation: 424

C language array & for loop does not work properly

I am using PIC18F26K83 and I am trying to measure temperature by using NTCALUG02A103F. I have calculated the ADC outputs and the temperatures that they are equals to. I have created 2 arrays: 1 for ADC values and 1 for the temperature for that ADC value. In for loop I compare the value from ADC and the value from the array and if my ADC value is less than array value it keeps decrementing the array element and eventually it will end up in 258 ADC value which is last element of my array. However, in for loop it never gets decremented. Even though my ADC value is 2000 it does not continue. It gets stuck at i=32. Here is the code :

int i;
int temperature;
int temp_data;
int temp_ADC_array[34] = {259,  293,  332,  377,  428,  487,  555,  632,  720,
                          821,  934,  1062, 1203, 1360, 1531, 1715, 1910, 2113,
                          2320, 2528, 2731, 2926, 3108, 3274, 3422, 3552, 3663,
                          3756, 3833, 3895, 3945, 3983, 4013, 4036};
int temp_array[34] = {125, 120, 115, 110, 105, 100, 95,  90,  85,  80, 75, 70,
                      65,  60,  55,  50,  45,  40,  35,  30,  25,  20, 15, 10,
                      5,   0,   -5,  -10, -15, -20, -25, -30, -35, -40};
void main() {
  while (1) {
    temp_data = ADC_READ(3);
    for (i = 33; temp_data < temp_ADC_array[i]; i--) {
      temperature = temp_array[i];
    }
  }

Edit: This for loop does not work either : 

for (i = 0; i < 34; i++) {
  if (temp_data > temp_ADC_array[33 - i]) {
    temperature = temp_array[33 - i];
    break;
  }
}

Edit 2: I test it with a led, I do not have USB on my circuit so I cannot use debugger. Here is how I test the temperature: ( I check the blinking of the led)

for(i; temperature>=0; temperature=temperature-10){
  led=1;
  delay_ms(1000);
  led=0;
  delay_ms(1000);
}   
delay_ms(5000);

Upvotes: 0

Views: 214

Answers (2)

Lundin
Lundin

Reputation: 213711

You read beyond the array, into item [-1] and so on, since the loop never stops if you read a value lower than 259.

The loop should have been written as for example

for(i=0; i<SIZE; i++) 
{ 
  if(temp_data < temp_ADC_array[SIZE-1-i]) 
  { 
    found = true; 
    break;
  }
}

But you can forget that too, since you have a sorted look-up table and therefore should be using binary search instead. It will likely speed up everything considerably, especially since 16 bit comparisons isn't the PIC's forte.

With "O log(n)" you'd end up with 5-6 comparisons worst case, instead of 34 ones. Even though binary search means some pointer juggling overhead, it still ought to out-perform your linear search drastically. This will matter a lot on such a slow CPU, particularly since ADC-related code tend to be performance critical.

Upvotes: 2

lucidbrot
lucidbrot

Reputation: 6156

This answer is just a repetition of my comment. The purpose of it is to make it possible to accept it as an answer, so that people see instantly that this question was resolved.

I think your first loop might be stopping before temparature is assigned when the condition is no longer true. That means that temperature would have the value one too early. The second loop looks fine to me though

The first loop was

void main() {
  while (1) {
    temp_data = ADC_READ(3);
    for (i = 33; temp_data < temp_ADC_array[i]; i--) {
      temperature = temp_array[i];
    }
  }

And the problem with it is that it stops looping as soon as the condition temp_data < temp_ADC_array[i] is no longer fulfilled. So if that happens at i==0, the temperature will still be temp_array[1].

The second loop was

for (i = 0; i < 34; i++) {
  if (temp_data > temp_ADC_array[33 - i]) {
    temperature = temp_array[33 - i];
    break;
  }
}

which should work because it only breaks after assigning the correct temperature.

Upvotes: 5

Related Questions