[SOLVED] PIC16F877A ADC interrupt not working

[NECAIBIA]

I am using PIC16F877A to light up an LED I use ADC interrupt but the LED doesn't light up. here is my code please help me.

unsigned int k= 0 ;
  unsigned adc_value1,adc_value2;
  float V=0,V1,I1,I=0;
void interrupt()
{
  if (PIR1.ADIF)
  {
    PIR1.ADIF=0; // clear AD interrupt flag
        for (k=0;k<10;k++)
        {
         ADCON0 = 0b10000001; // select channel 0
         adc_value1 = ADC_Read(0);
         V1=adc_value1*5.0;
         V1=V1/1024.0;
         V1=V1*6.0;
         V=V+V1;
         
         ADCON0 = 0b10001001; // select channel 1
         adc_value2 = ADC_Read(1);
         I1=adc_value2*5.0;
         I1=I1/1024.0;
         I1=I1-2.5;
         I1=I1/0.185;
         I=I+I1;
       }
         I=I/10;
         V=V/10;

         Delay_Cyc(3); //wait acquisition time
         ADCON0.F2=1; //start conversion again
  }
}
void main() 
{
   ADC_Init();
   ADCON1 = 0b11000010;
   ADCON0 = 0b10000001; // select channel 0
   PIE1.ADIE = 1; //Enable ADC interrupt
   INTCON.PEIE = 1; //Enable peripheral interrupt
   INTCON.GIE = 1; //Enable global interrupt
   Delay_us(10); //wait for acquisition time
   ADCON0.F2 = 1; //start conversion
   TRISA=1;
   TRISD=0;
   PORTD=0;
       while (1)
  {
    if(V>I)
    {
      portd.b2=1;
    }
    if(V<i)
    {
      portd.b2=0;
    }
  }
}</i)

 

[betwixt]

It will not work because the program structure is wrong. Consider what triggers the interrupt routine, it is the completion of an analog conversion to a digital value. There is only one ADC inside the 16F877A and only that can set the ADIF bit and cause the interrupt. What you are doing is switching channels before reading a value and expecting a result without checking if the conversion is completed. Setting the ADC channel routes one of the input pins to the single ADC module so you have to repeat the whole process with another pin selected to measure more than one source.

What you need to do is:
1. set the ADC channel and leave a short period for the input stage to settle.
2. set the GO bit to start the conversion
3. in the ISR, check the ADIF bit is telling you the ADC is finished
4. copy the value to your destination variable
5. clear the ADIF bit.

Then go back to step 1 for the next channel.

Try to copy the ADC result in the ISR then leave it as quickly as possible. NEVER run loops inside the ISR because they can take so long that a following interrupt is missed. Do all your math in the main() routines where time isn't so critical.

Brian.

 

[NECAIBIA]

betwixt, thank for your explanation. Actually I want to use interrupt (ADC interrupt or other...) to measure a voltage and a current for 10 us and use these measurements in a While Loop. I used another interrupt (Timer 0) but I do not know if we can use ADC in Timer 0 like in the program below. I am a beginner in programming. I understood what you said, but I do not know how to correct my problem.Please help me. Thank in advance.

---

Code C - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
unsigned int  step,Alpha,adc_count=0;
  float P_pv_old=0,V_pv_old=0,I_pv_old=0,P_pv,V_pv,I_pv;
 
 
void InitTimer0(){
  OPTION_REG         = 0x88;
  TMR0                 = 206;
  INTCON         = 0xA0;
}
 
void Interrupt(){
  if (TMR0IF_bit){
    TMR0IF_bit         = 0;
    TMR0                 =206;
    
     V_pv = (ADC_Read(0)*5.0/1024)*6.0;
     I_pv = ((ADC_Read(1)*5.0/1024)-2.5)/0.185;
 
  }
}
void main()
{
     ADC_Init();
     InitTimer0();
     TRISA=0xFF;
     TRISC=0;
     PORTC=0;
     ADCON1 = 0b11000010; //0x82; // AN0->AN4 selected as analog input
     PWM1_Init(10000);
     PWM1_Start();
     Alpha = 0;
     PWM1_Set_Duty(Alpha);
     step=1;
 
     while(1)
  {   
     P_pv = V_pv*I_pv;
 
 
     if(P_pv > P_pv_old)
     {
            if (V_pv > V_pv_old)
             {
                     Alpha = Alpha-step;
              }
            if (V_pv < V_pv_old)
            {
                Alpha = Alpha+step;
 
              }
       }
         if(P_pv < P_pv_old)
         {
            if (V_pv > V_pv_old)
            {
 
                Alpha = Alpha+step;
 
             }
            if (V_pv < V_pv_old)
            {
                Alpha = Alpha-step;
 
              }
 
           }
 
            PWM1_Set_Duty(Alpha);
            P_pv_old = P_pv;
            V_pv_old = V_pv;
            I_pv_old = I_pv;
   }
 }

---

 

[Aussie Susan]

Even in your second case, use the timer to only set a (volatile) flag that is checked in the main loop where you can perform your actual measurements and calculations.
The general rule is that an ISR should execute as quickly as possible. Especially with these smaller MCUs, while an ISR is executing, no other code can be worked on.
Also remember that any value that is altered within an ISR must be declared as volatile to tell the compiler that its value can be 'changed behind your back'. Otherwise, a compiler is quite within its rights to read a variable's value into a register and, by examining the subsequent code, work out whether or not the value has been altered and, if not, then continue to use the register version as accessing that is much quicker than re-reading the variable fro memory. IF an ISR has changed the value of the variable, then the compiler would continue to use the old value. Declaring the variable as volatile tells the compiler to always re-read the value from memory.
Susan

 

[betwixt]

This is how to restructure it:

unsigned int  step,Alpha,adc_count=0;
  float P_pv_old=0,V_pv_old=0,I_pv_old=0,P_pv,V_pv,I_pv;
 
 
void InitTimer0(){
  OPTION_REG         = 0x88;
  TMR0                 = 206;
  INTCON         = 0xA0;
}
 
void Interrupt()
{
  if (TMR0IF_bit)
    {
        TMR0IF_bit         = 0;
        TMR0                 =206;
        [COLOR="#FF0000"]TimeToReadTheADC = 1;[/COLOR]
    }
}


void main()
{
     ADC_Init();
     InitTimer0();
     TRISA=0xFF;
     TRISC=0;
     PORTC=0;
     ADCON1 = 0b11000010; //0x82; // AN0->AN4 selected as analog input
     PWM1_Init(10000);
     PWM1_Start();
     Alpha = 0;
     PWM1_Set_Duty(Alpha);
     step=1;
 
     while(1)
    {   
        if(TimeToReadTheADC)        // [COLOR="#FF0000"]<- check if it's time to read the ADC[/COLOR]
        {
            TimeToReadTheADC = 0;   // [COLOR="#FF0000"]<- reset the flag for next time
[/COLOR]
            V_pv = (ADC_Read(0)*5.0/1024)*6.0; // [COLOR="#FF0000"]<- moved from ISR[/COLOR]
            I_pv = ((ADC_Read(1)*5.0/1024)-2.5)/0.185;
            
            P_pv = V_pv*I_pv;
                
            if(P_pv > P_pv_old)
            {
                    if (V_pv > V_pv_old)
                    {
                            Alpha = Alpha-step;
                    }
                    if (V_pv < V_pv_old)
                    {
                        Alpha = Alpha+step;
        
                    }
            }
        
            if(P_pv < P_pv_old)
            {
                if (V_pv > V_pv_old)
                {
        
                    Alpha = Alpha+step;
        
                }
                if (V_pv < V_pv_old)
                {
                    Alpha = Alpha-step;
        
                }
        
            }
        
            PWM1_Set_Duty(Alpha);
            P_pv_old = P_pv;
            V_pv_old = V_pv;
            I_pv_old = I_pv;
        }
    }
 }

Note that I created a new variable 'TimeToReadADC' which is set inside the ISR when a timer interrupt occurs and is then read and reset inside the main routine. All the ISR does is deal with the timer then tell the main routine to do all the work.

There are some important things to consider and I cannot help you without knowing how fast your processor runs:

The ISR is entered every time TMR0 rolls over, that means every 50 counts (206 -> 256) at whatever clock speed and prescaler setting you have used. As the interrupt has priority over the code running in main(), you have to be certain that interrupts don't happen faster than your main code can handle reading the ADC and processing the results. If it does, you run the risk of missing the next time slot.

I don't know whether your compiler uses interrupts or not in the ADC_Read() function. I suspect it uses polling rather than interrupts, this means there is a hidden delay before that function returns a value. How long that delay is depends on the ADC clock settings so you should be aware of them when working out the total time needed between TMR0 interrupts.

Brian.

 

[NECAIBIA]

betwixt , Thank you very much Sir for your help.The program is work now thanks to your last modification,you can see it below. but Please I want you to tell me if is correct or not.I want my program work and the interruption cut the program for 10 us in this time (10 us) only the ADC work. In short the ADC only works during the time of the interruption (10 us). processor Clock frequency 20 MHz.

---

Code C - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
unsigned TimeToReadTheADC=0,i=0,adc_value1,adc_value2;
     unsigned int  Alpha,step;
     float V_pv=0.0,V_pv1=0.0,I_pv=0,I_pv1=0.0,P_pv_old=0.0,V_pv_old=0,I_pv_old=0.0,P_pv;
     
//Prescaler 1:1; TMR0 Preload = 206; Actual Interrupt Time : 10 us
void InitTimer0(){
  OPTION_REG     = 0x88;
  TMR0       = 206;
  INTCON     = 0xA0;
}
void Interrupt(){
  if (TMR0IF_bit){
    TMR0IF_bit   = 0;
    TMR0         = 206;
    TimeToReadTheADC=1;
  }
}
void main()
{
     InitTimer0();
     ADC_Init();
     ADCON1 = 0b10000010;
     TRISA=1;
     TRISD=0;
     PORTD=0;
     PWM1_Init(10000);
     PWM1_Start();
     Alpha = 0;
     PWM1_Set_Duty(Alpha);
     step=3;
       while (1)
  {
         if(TimeToReadTheADC)
     {
         TimeToReadTheADC=0;
        for (i=0;i<10;i++)
        {
         adc_value1 = ADC_Read(0);
         V_pv1=adc_value1*5.0;
         V_pv1=V_pv1/1024.0;
         V_pv1=V_pv1*6.0;
         V_pv=V_pv+V_pv1;  //V_pv=V_pv1;
 
         adc_value2 = ADC_Read(1);
         I_pv1=adc_value2*5.0;
         I_pv1=I_pv1/1024.0;
         I_pv1=I_pv1-2.5;
         I_pv1=I_pv1/0.185;
         I_pv=I_pv+I_pv1;   //I_pv=I_pv1;
       }
        I_pv=I_pv/10; //filtering
        V_pv=V_pv/10; //filtering
     }
     
     P_pv = V_pv*I_pv;
     if(P_pv > P_pv_old)
     {
            if (V_pv > V_pv_old)
             {
                     Alpha = Alpha-step;
              }
            if (V_pv < V_pv_old)
            {
                Alpha = Alpha+step;
 
              }
       }
         if(P_pv < P_pv_old)
         {
            if (V_pv > V_pv_old)
            {
 
                Alpha = Alpha+step;
 
             }
            if (V_pv < V_pv_old)
            {
                Alpha = Alpha-step;
 
              }
 
           }
 
            PWM1_Set_Duty(Alpha);
            P_pv_old = P_pv;
            V_pv_old = V_pv;
            I_pv_old = I_pv;
 
 
  }
}

---

 

[NECAIBIA]

Susan, hank you very much for your explanation it's so helpful. Please can you see my last program and tell me if it's correct or not.

 

[betwixt]

Microchip do not recommend setting GIE at the same time as other interrupt enable bits in the INTCON register so it would be a good idea to set the TMR0IE bit first then GIE in the next instruction. It usually does work OK though.

10uS isn't very long, you are asking for 100,000 samples per second. I doubt you will manage that fast but what we don't know is how fast the ADC clock is running. It is configurable but your ADC_Init() function doesn't say what it has set the clock to. If you look in the data sheet it tells you there are various clock options and how long each setting makes the ADC return a result. I suspect the ADC may take longer than the interrupt allows though.

Brian.

 

[Aussie Susan]

Also, please re-read my comments about the 'volatile' qualifier.
Susan

 

[NECAIBIA]

betwixt and Susan thank you very much. The qualifier volatile solve my problem.

- - - Updated - - -

betwixt and Susan thank you so mach. The problem is solved thanks to the qualifier volatile that I missed.