View previous topic :: View next topic |
Author |
Message |
picman62
Joined: 12 Dec 2014 Posts: 77
|
PWM interrupt failing to turn off |
Posted: Thu Jan 08, 2015 12:05 pm |
|
|
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);
} |
|
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19378
|
|
Posted: Thu Jan 08, 2015 1:44 pm |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Thu Jan 08, 2015 1:57 pm |
|
|
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
} |
|
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Thu Jan 08, 2015 1:59 pm |
|
|
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
} |
|
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19378
|
|
Posted: Thu Jan 08, 2015 2:16 pm |
|
|
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..... |
|
![](templates/subSilver/images/spacer.gif) |
temtronic
Joined: 01 Jul 2010 Posts: 9177 Location: Greensville,Ontario
|
|
Posted: Thu Jan 08, 2015 3:40 pm |
|
|
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 |
|
![](templates/subSilver/images/spacer.gif) |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Thu Jan 08, 2015 4:06 pm |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Thu Jan 08, 2015 4:52 pm |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19378
|
|
Posted: Fri Jan 09, 2015 2:07 am |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Fri Jan 09, 2015 4:52 am |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Fri Jan 09, 2015 5:01 am |
|
|
Just did a test without text01. No change. So, nothing to do with this. Will try a font generator. |
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Fri Jan 09, 2015 8:13 am |
|
|
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 |
|
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19378
|
|
Posted: Fri Jan 09, 2015 10:08 am |
|
|
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. |
|
![](templates/subSilver/images/spacer.gif) |
picman62
Joined: 12 Dec 2014 Posts: 77
|
|
Posted: Sat Jan 10, 2015 5:50 am |
|
|
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? |
|
![](templates/subSilver/images/spacer.gif) |
|