CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

PWM interrupt failing to turn off

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PWM interrupt failing to turn off
PostPosted: Thu Jan 08, 2015 12:05 pm     Reply with quote

Hello.
Proceeding in my first project of programming a PIC, I am facing an unsolved issue. At least for me.

I am programming the PIC 16F877A to send 3 different frequencies. Each one related to a particular page and text in the LCD. Each PWM signal is in a loop where the signal stays on for 100ms and stops for 500ms. Each operation starts when I press button (B7) and when I press it again, signal stops. Then I press button (B4) and page changes to next text when I start the same thing with another frequency. All duty cycles are set to 75%.

The problem starts when I press B7 to stop the transmission. It simply won't stop. I have to press the button several times to make it finally stop.
All of this happens in the PROTEUS simulator. I still did not test in an actual harware.

I noticed that when I have only 'cont==1' case, with the others 'shaded', button stops at the first time or in the second one in the worst case. But when I include all three cases (pages), problem gets worse and the last case 'cont==3', simply does not stop at all, no matter how many times I press B7.

Please, someone take a look at the code I wrote and check for possible mistakes I am making or at best, suggest an instruction to overcome this problem.
Thanks in advance.



Code:
#include <16f877a.h>
#use delay(clock = 20000000)
#include <LCD.C>
#define LCD_ENABLE_PIN PIN_D0
#define LCD_RS_PIN PIN_D1
#define LCD_RW_PIN PIN_D2
#define LCD_DATA4 PIN_D4
#define LCD_DATA5 PIN_D5
#define LCD_DATA6 PIN_D6
#define LCD_DATA7 PIN_D7
#fuses HS,NOWDT,NOPROTECT,NOLVP
BYTE signal = 0;

//#use fast_io(c)
//#use fast_io(b)
#include <lcd.c>
#int_rb


void button_isr()
{
   delay_ms (30);  //debounce
   if( !input(PIN_B7) && !signal )
      signal = 1;
   else
   
   
   if( !input(PIN_B7) && signal )
      signal = 0;   
}

void main()
{
   
   enable_interrupts(global);
   enable_interrupts(int_rb);

   ext_int_edge( H_TO_L );

   output_drive(pin_c2);
 
   setup_adc_ports(AN0_VREF_VREF);
   setup_adc(  ADC_CLOCK_INTERNAL  );

   
   port_B_pullups(0xFF);
   setup_adc_ports(AN0);
   lcd_init();

   lcd_init();

    INT1 SW1;
    INT1 SW2;
    INT1 SW3;
    INT1 SW4;
   
   
        BOOLEAN ISPRESSED_KEY1=FALSE;       // Boolean logic=0;
        BOOLEAN ISPRESSED_KEY2=FALSE;       // Boolean logic=0;
        BOOLEAN ISPRESSED_KEY3=FALSE;       // Boolean logic=0;
        BOOLEAN ISPRESSED_KEY4=FALSE;       // Boolean logic=0;

     if ( (SW1 && !ISPRESSED_KEY1) || (SW2 && !ISPRESSED_KEY2) || (SW3 && !ISPRESSED_KEY3)|| (SW4 && !ISPRESSED_KEY4) )
   {
   
   DELAY_MS(10);
             
   if ( (SW1 && !ISPRESSED_KEY1) ) // redefine values: 1000,2500,15000

   {
   
    counter=1000;
     
   if(cont >= 1) counter=2500;
   if(cont >= 2) counter=15000;
   if(cont >= 4) counter=1000;
 
   if(++cont >= 5) cont = 1;
   
 
   ISPRESSED_KEY1=TRUE ;
   
   }
     
   if ( (SW2 && !ISPRESSED_KEY2) ) // increments value  10
   {
   counter+=10;               
   ISPRESSED_KEY2=TRUE;
 
                 
   }
                       
   if ( (SW3 && !ISPRESSED_KEY3) ) // decrements value 10
   {
                       
   counter-=10;
   ISPRESSED_KEY3=TRUE;

   }
   
   if ( (SW4 && !ISPRESSED_KEY4) )
   
  {
   
      ISPRESSED_KEY4=TRUE;
   
   if(cont==1) //page text 01
           
   do // interrupts to loop state of PWM and out of it
     
   {
   
    if(signal) // if PWM is sent through C2 pin
   
   {     
           setup_timer_2(T2_DIV_BY_4,249,1);
           set_pwm1_duty(748);//enable PWM   
           setup_ccp1(CCP_PWM); //enable PWM
           delay_ms(100);
           setup_ccp1(CCP_OFF);//disable PWM
           delay_ms(500);
   }
       
   }
   
   
    while(input(pin_b4)==1);     
   
       
    if (cont==2) // page text02
   
    do
   
   {
   
    if(signal)
       
   {
           setup_timer_2(T2_DIV_BY_4,142,1);
           set_pwm1_duty(430);//enable PWM
           setup_ccp1(CCP_PWM); //enable PWM
           delay_ms(100);
           setup_ccp1(CCP_OFF);//disable PWM
           delay_ms(1000);
         
         
      }
     
   }
   
   
    while(input(pin_b4)==1);
   
   
           
               
    if (cont==3) // page text 03
   
       
    do
   
   {
   
    if(signal)
   
   {     
         setup_timer_2(T2_DIV_BY_4,96,1);
         set_pwm1_duty(292);//enable PWM
         setup_ccp1(CCP_PWM); //enable PWM
         delay_ms(100);
         setup_ccp1(CCP_OFF);//disable PWM
         delay_ms(1000);       
         
      }
     
   }
   
      while(input(pin_b4)==1);
   
 

   }
Ttelmah



Joined: 11 Mar 2010
Posts: 19378

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 1:44 pm     Reply with quote

Of course it won't.
You'll be having a compiler warning when you compile. Something along the line of 'interrupt disabled to prevent re-entrancy'. Search on the forum for why you shouldn't use delays inside an ISR. Basically most of the time inside your code the interrupts will be disabled.
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 1:57 pm     Reply with quote

Thank you very much Ttelmah. I did not now about this.
I removed the delay string and now it is working.
Nice learning.

Since you have answered to solve this issue at an instant, I would like to go over a small one about the LCD display.

I have an animation that 'black bars' move to the right side until display is filled. It runs ok in the Proteus simulator. But when I conect the hardware, a lot of unwanted characters end up mixed with the 'black bars'. This should not be happening.
Here's the code. Can you give a light on why this is happening?

Code:
 while(TRUE){

   lcd_gotoxy(3,1);
   lcd_putc("TEXT ABCD");// text of display
   
 while(TRUE){
   Cont=0;
   
   while (Cont<16)
   {
   load [Cont]=255;//bar filling
   Printf(lcd_putc,"\n\%s",load,);
   Cont++;
 
   delay_ms(150); //bargraph
   }
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 1:59 pm     Reply with quote

Thank you very much Ttelmah. I did not now about this.
I removed the delay string and now it is working.
Nice learning.

Since you have answered to solve this issue at an instant, I would like to go over a small one about the LCD display.

I have an animation that 'black bars' move to the right side until display is filled. It runs ok in the Proteus simulator. But when I conect the hardware, a lot of unwanted characters end up mixed with the 'black bars'. This should not be happening.
Here's the code. Can you shed a light about what could be the problem?
Thanks in advance.

Code:
 while(TRUE){

   lcd_gotoxy(3,1);
   lcd_putc("TEXT ABCD");// text of display
   
 while(TRUE){
   Cont=0;
   
   while (Cont<16)
   {
   load [Cont]=255;//bar filling
   Printf(lcd_putc,"\n\%s",load,);
   Cont++;
 
   delay_ms(150); //bargraph
   }
Ttelmah



Joined: 11 Mar 2010
Posts: 19378

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 2:16 pm     Reply with quote

Not for sure, but remember a 'string' needs a null terminator. You fill the bar with 255 characters, but don't add a terminator. As such the printf, will print everything in memory after the last 255, till it happens to find a 0.....
temtronic



Joined: 01 Jul 2010
Posts: 9177
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 3:40 pm     Reply with quote

You need to add a delay_ms(500); before the lcd_init(); and delete the 2nd lcd_init(); when you code for real hardware.

While Proteus doesn't care the real hardware does !! Every LCd module needs a certain 'setup time' to get 'organized' BEFORE the PIC tried to talk to it.

it's one of 100 things that you either learn by reading all pages of the datasheets, looking what others have done/said or by 'trial and error'.


hth
jay
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 4:06 pm     Reply with quote

Tons of small things wrong here:
Quote:
#include <16f877a.h>
#use delay(clock = 20000000)
#include <LCD.C>
#define LCD_ENABLE_PIN PIN_D0
#define LCD_RS_PIN PIN_D1
#define LCD_RW_PIN PIN_D2
#define LCD_DATA4 PIN_D4
#define LCD_DATA5 PIN_D5
#define LCD_DATA6 PIN_D6
#define LCD_DATA7 PIN_D7
#fuses HS,NOWDT,NOPROTECT,NOLVP
BYTE signal = 0;

//#use fast_io(c)
//#use fast_io(b)
#include <lcd.c>
#int_rb


void button_isr()
{
delay_ms (30); //debounce

See corrections below:
Code:

// *** These 3 lines should be in the order shown below:
#include <16f877a.h>
#fuses HS,NOWDT,NOPROTECT,NOLVP
#use delay(clock = 20000000)

BYTE signal = 0;

// *** These #defines go just above the #include for lcd.c
#define LCD_ENABLE_PIN PIN_D0
#define LCD_RS_PIN PIN_D1
#define LCD_RW_PIN PIN_D2
#define LCD_DATA4 PIN_D4
#define LCD_DATA5 PIN_D5
#define LCD_DATA6 PIN_D6
#define LCD_DATA7 PIN_D7
#include <lcd.c>

#int_rb   // *** This goes on the line just above the isr.
void button_isr()
{
 


Some more comments:
Code:

void main()
{
   
   enable_interrupts(global);
   enable_interrupts(int_rb);

   ext_int_edge( H_TO_L );  // *** This does not affect #int_rb

   output_drive(pin_c2);
 
   setup_adc_ports(AN0_VREF_VREF);    // *** This is overridden below
   setup_adc(  ADC_CLOCK_INTERNAL  ); // *** Not optimum, but semi-OK

   
   port_B_pullups(0xFF);    // *** This PIC uses TRUE or FALSE here.
   setup_adc_ports(AN0);   // *** This line overrides the line above.
   lcd_init();

   lcd_init();    // *** Why two calls to the same thing ?

None of these changes are important to your main code, but they will
make your program cleaner and easier to understand.
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Thu Jan 08, 2015 4:52 pm     Reply with quote

Thanks for all replies.

Ttelmah: Can you elaborate on this. What would be a null terminator?

Temtronic: I did as you said and added a 500ms delay, but unfortunately no change. While the bars go moving to the right, some &, %,/ go getting along the way.

PCM Programmer: Thanks for 'cleaning the house' as this is my first project and have only been into C for 3 weeks now. And tough you said the ordering of strings were not important for the program, there was a noticeable improvement in stability in the initialization routine.

I will keep trying to find what is happening with the printf thing.
Ttelmah



Joined: 11 Mar 2010
Posts: 19378

View user's profile Send private message

PostPosted: Fri Jan 09, 2015 2:07 am     Reply with quote

Oh my God!, basic C.....

In C, a 'string', is an array of characters _terminated with a null_ ('\0') character.

The string print function, works by sending characters _until it reaches this null character_. Then stops.

You have an array you are filling with '255' characters. What is going to stop the print?.
I'd guess that Proteus is filling unused memory locations with '0' characters, so it stops when it gets to the end of the array. The real chip doesn't do this. If you want to display (say) six a's, from a string, it needs to contain:
a
a
a
a
a
a
\0

The print will then print the 6 a's and stop. However if you don't have the terminator, then it'll walk off through memory, printing as text whatever numbers it finds, until eventually it does hit a null.

This is why a 16 character string, must be allocated 17 characters of storage.

The sprint function will automatically add a null terminator.

So:
Code:

    char buffer[8];
    sprintf(buffer,"aaaaaa");

Will put the six a's and the terminator into the buffer (the eighth character will be undefined).

This is in every basic C textbook.
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Fri Jan 09, 2015 4:52 am     Reply with quote

Thanks, but it did not work.
Guess it has something to do with the text01 that appears in conjunction with the bar "cont".

Or the LCD has a problem in the lower line.
I will continue searching for a fix. Maybe testing a font generator to see if I get rid of this problem.
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Fri Jan 09, 2015 5:01 am     Reply with quote

Just did a test without text01. No change. So, nothing to do with this. Will try a font generator.
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Fri Jan 09, 2015 8:13 am     Reply with quote

Got it!

The problem is in the characters themselves. So as long as '255s' go up the display, it meets every garbage along the way.
Solution:
I created 'int cont2' and 'char load2' with value 254 for blank space. Then I sent it forward to 'eat' every garbage along the way prior to the '255s' path.

Worked like a charm.
So, this procedure is the one necessary procedure to clean up the display prior to the animation.

Code:
while(TRUE){
   
   while(Cont2<15){
   load2[Cont2]=254;
   load[Cont2]=254;
   Cont2++;
   }
   Cont2=15;
   Cont=0;
   
   while (Cont<16)
   {
   load [Cont]=255;//bars go up
   printf(lcd_putc,   "\n\%s"   ,load);
   Cont++;
   delay_ms(150); //bargraph
Ttelmah



Joined: 11 Mar 2010
Posts: 19378

View user's profile Send private message

PostPosted: Fri Jan 09, 2015 10:08 am     Reply with quote

As a comment, you seem to be determined to make the processor do much more work, use a lot more RAM, than it has to for this. No point at all in building a string, and sending it, just send the characters:
Code:

void bar(int8 num)
{   
    int8 ctr;
    for (ctr=0;ctr<16;ctr++)
    {
        if (ctr<num)
           putc(255);
        else
           putc(' ');
    }
}


Call with num from 0 to 15 for how many bar characters you want.
Will clear the display line where bars are not printed.
Delay as required, position to the line required, etc., before/during use.
picman62



Joined: 12 Dec 2014
Posts: 77

View user's profile Send private message

PostPosted: Sat Jan 10, 2015 5:50 am     Reply with quote

Hi Ttelmah.
This is supposed to go prior void main() right?
Well, I placed you code and removed the
Code:
  while(Cont2<16){
   load2[Cont2]=254;
   load[Cont2]=254;
   Cont2++;
   }
   Cont2=16;
   Cont=0;

And left the instruction to LCD display
Code:
while (Cont<16)
   {
   load [Cont]=255;
   printf(lcd_putc,   "\n\%s"   ,load);
   Cont++;
   delay_ms(150); //bargraph
   }


Same thing happened as before with the mixed characters showing.

Also I noticed that there was no change in RAM memory either with your code or with the one I created with blank (254s).

So, maybe this is a bug from CCD version 5.008 and I do need to create blank spaces?
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group