 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Salfer
Joined: 20 Mar 2014
Posts: 15
|
| BIG problem with Serial interrupt |
 Posted: Mon May 05, 2014 4:29 pm |
 |
|
Hi everyone! I have a big problem with serial interrupt (#INT_RDA) because my program enter in this interrupt the first time but don't enter the second time.
I don't know if the problem is the version of CCS (v4.140) or the MPLAB (v8.87). I use the CCS compiler with the MPLAB.
In the simulation the program seems ok but in the practice is strange.
I use an Arduino Due that sends to the PIC a data like this ("$ADQ,P01,CH1\r") and the PIC that I use is the 16F88 that responds to Arduino. The power supply of the 16f88 is 3,3 because Arduino due works 3.3V.
This is my code. If there are something bad responds me please I need a lot of help with this :(
Is not necessary check all code, only the serial interrupt part and the fuses.
THANKS!!
| Code: | #include <16f88.h>
#device ADC=10
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#fuses HS, NOWDT, NOLVP, NOBROWNOUT, NOPROTECT, PUT, INTRC_IO
#use delay (internal=8MHz)
#use RS232(UART1)
//**VARIABLES PARA GESTIONAR LA TRAMA RECIBIDA Y LA TRAMA A ENVIAR POR PUERTO SERIE**//
long valor_adc=0; //Variable para lectura ADC
int P_W=0; //puntero escritura
int P_R=0; //puntero lectura
int i=0; //puntero bucle for
int j=0;
int inicio=0;
int final=0;
char string_valor_adc[]="0000";//Cadena de caracteres para guardar valor ADC en Carcteres
char trama_entrada[91]={0}; //String para guardar el dato de entrada por UART
char direccion_pic[]="P01";
char tipo_dato[]="$ADQ";
char canal[]="CH0";
char dato[]="$ADQ,P01,CH0\r";
char datomio[]="$ADQ,P01,CH1,3333\r";
char miretorno[]="\r";
char misimbolo[]="$";
short flag_final=0;
short flag_permiso=0;
short flag_tst=0;
short flag_envio=0;
void lectura(){
for(P_R=0; P_R<91; P_R++){
if(trama_entrada[P_R]==misimbolo[0]){
inicio=P_R;
for(P_R=inicio; P_R<91; P_R++){
if(trama_entrada[P_R]==miretorno[0]){
P_W=0;
final=P_R;
flag_permiso=1;
break;
}
}
break;
}
}
}
void escritura(){
for(i=inicio; i<=final; i++){
dato[j]=trama_entrada[i];
j++;
}
i=0;
j=0;
for(i=0;i<91;i++){
trama_entrada[i]='\0';
}
return;
}
void procesamiento(){
for(i=0; i<3; i++){
direccion_pic[i]=dato[i+5];
}
direccion_pic[3]='\0';
if(strcmp(direccion_pic, (char*)"P01")==0){
for(i=0;i<3;i++){
direccion_pic[i]='\0';
}
for(i=0; i<4; i++){
tipo_dato[i]=dato[i];
}
tipo_dato[4]='\0';
if(strcmp(tipo_dato, (char*)"$ADQ")==0){
for(i=0;i<4;i++){
tipo_dato[i]='\0';
}
flag_tst=0;
for(i=0; i<3; i++){
canal[i]=dato[i+9];
}
canal[3]='\0';
if(strcmp(canal,(char*)"CH0")==0){
set_adc_channel(0);
}
else if(strcmp(canal,(char*)"CH1")==0){
set_adc_channel(1);
}
delay_us(20);//retardo para leer ADC
valor_adc=read_adc();//lectura ADC
}
else if(strcmp(tipo_dato,(char*)"TST")==0){
for(i=0;i<4;i++){
tipo_dato[i]='\0';
}
flag_tst=1;
}
flag_envio=1;
}
return;
}
void enviar(){
switch(flag_tst){
case 0:
sprintf(string_valor_adc,"%04ld",valor_adc);
output_high(PIN_B0);
printf("$ADQ,P01,%s,%s\r", canal,string_valor_adc);
output_low(PIN_B0);
for(i=0;i<4;i++){
string_valor_adc[i]='\0';
}
for(i=0;i<3;i++){
canal[i]='\0';
}
break;
case 1:
output_high(PIN_B0);
printf("$TST,P01\r");
output_low(PIN_B0);
flag_tst=0;
break;
}
return;
}
void main(){
output_low(PIN_B3);
output_low(PIN_B0);
disable_interrupts(INT_RDA);
enable_interrupts(GLOBAL); //HABIlLITO TODAS LAS INTERRUPCIONES GLOBALES
enable_interrupts(INT_RDA); //HABILITO LA INTERRUPCIÓN UART
setup_adc_ports(sAN0|sAN1|sAN2|sAN3|sAN4|sAN5); //INDICO EL PIN A0/A1/A2/A3/A4/A5 COMO ENTRADA ANALÓGICA
setup_adc(ADC_CLOCK_INTERNAL); //CLOCK INTERNO PARA CONVERSIÓN ADC
while(1){
if(flag_final==1){
flag_final=0;
lectura();
if(flag_permiso==1){
flag_permiso=0;
escritura();
procesamiento();
if(flag_envio==1){
flag_envio=0;
enviar();
}
}
}
}
}
#INT_RDA
void INT_UART(void) {
trama_entrada[P_W]=getc();
if(trama_entrada[P_W]=='\r'){
P_W=0;
flag_final=1;
}
else{
P_W++;
}
}
|
Last edited by Salfer on Mon May 05, 2014 4:59 pm; edited 1 time in total |
|
 |
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Mon May 05, 2014 4:55 pm |
|
|
try removing this line from your handler
clear_interrupt(INT_RDA);
the CCS code does it FOR you at the end of the function
also you might try to test that the pointer in the handler does not let the fill pointer P_W go beyond say 89 decimal into trama_entrada[]
AND even consider filling the whole array with zeros - not just the first element
at INIT time.
Last edited by asmboy on Mon May 05, 2014 4:59 pm; edited 1 time in total |
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Mon May 05, 2014 4:58 pm |
|
|
| asmboy wrote: | try removing this line from your handler
clear_interrupt(INT_RDA);
the CCS code does it FOR you at the end of the function |
OK but the program continue without work :( I'm very worried I have done all |
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Mon May 05, 2014 5:06 pm |
|
|
| asmboy wrote: | try removing this line from your handler
clear_interrupt(INT_RDA);
the CCS code does it FOR you at the end of the function
also you might try to test that the pointer in the handler does not let the fill pointer P_W go beyond say 89 decimal into trama_entrada[]
AND even consider filling the whole array with zeros - not just the first element
at INIT time. |
Ok I will consider it. A lor of Thanks!! but the programs continue without enter in the INT_RDA I don't know what is the problem. |
|
|
ezflyr
Joined: 25 Oct 2010
Posts: 1019
Location: Tewksbury, MA
|
|
| Posted: Mon May 05, 2014 5:27 pm |
|
|
Hi,
At a minimum add 'errors' to your #use rs232 directive. This has been discussed a million ( or more!) times on the forum.....
John |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19540
|
|
| Posted: Tue May 06, 2014 12:43 am |
|
|
Several other comments though:
| Code: |
#include <16f88.h>
#device ADC=10
#fuses HS, NOWDT, NOLVP, NOBROWNOUT, NOPROTECT, PUT, INTRC_IO
#use delay (internal=8MHz)
#use RS232(UART1, ERRORS)
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
|
The fuse an clock defines must _always_ be in front of anything that uses them. This includes things like stdio....
ERRORS already mentioned.
I suspect you have one of the compiler versions that is disabling GIE when printf is used. Do a search here. This was a problem that appeared, was fixed, then appeared again, and was eventually fixed around 5.012ish. This would explain your problem.
As a general comment, avoid using names like 'long', and 'short'. These have different meanings on different chips. Be explicit. Include 'stdint.h', and use types like uint8_t, which say exactly what you are using, and will work whatever chip is involved. It's a problem 'waiting to happen'.
As a further comment, add size testing on your buffer variable. You may 'expect' that a carriage return will arrive before the buffer overflows, but in only needs to be missed once, for the code to destroy other variables, and crash.
Then read the data sheet. Is ADC_CLOCK_INTERNAL recommended at your clock rate....
I suspect the GIE problem is the 'killer' at present, but think about a bit of redefinition for the future. |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Tue May 06, 2014 3:45 am |
|
|
Then a few comments on coding style:
- Please check the formatting of your code. It is now difficult to read because indentation for the lines is using different styles and sizes everywhere.
- Please use defines instead of hard coded values. For example you have the magic value '91' in many places where you mean the size of one of the arrays. Add something like | Code: | | #define RECEIVE_ARRAY_SIZE 91 |
at the top of your program.
Same applies to the I/O pins. Compare these two: | Code: | output_low(PIN_B0);
#define LED PIN_B0
output_low(LED); |
- Use the standard C functions whenever possible to create more efficient and easier to read programs:
| Code: | for(i=0;i<91;i++){
trama_entrada[i]='\0';
} |
can be replaced by: | Code: | | memset(trama_entrada, 0, sizeof(trama_entrada)); |
|
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Tue May 06, 2014 5:12 am |
|
|
| Ttelmah wrote: | Several other comments though:
| Code: |
#include <16f88.h>
#device ADC=10
#fuses HS, NOWDT, NOLVP, NOBROWNOUT, NOPROTECT, PUT, INTRC_IO
#use delay (internal=8MHz)
#use RS232(UART1, ERRORS)
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
|
The fuse an clock defines must _always_ be in front of anything that uses them. This includes things like stdio....
ERRORS already mentioned.
I suspect you have one of the compiler versions that is disabling GIE when printf is used. Do a search here. This was a problem that appeared, was fixed, then appeared again, and was eventually fixed around 5.012ish. This would explain your problem.
As a general comment, avoid using names like 'long', and 'short'. These have different meanings on different chips. Be explicit. Include 'stdint.h', and use types like uint8_t, which say exactly what you are using, and will work whatever chip is involved. It's a problem 'waiting to happen'.
As a further comment, add size testing on your buffer variable. You may 'expect' that a carriage return will arrive before the buffer overflows, but in only needs to be missed once, for the code to destroy other variables, and crash.
Then read the data sheet. Is ADC_CLOCK_INTERNAL recommended at your clock rate....
I suspect the GIE problem is the 'killer' at present, but think about a bit of redefinition for the future. |
Ttelmah YES. The problem is the PRINTF. When I use PRINTF for send the data by USART the program can enter ONLY one time. But when I use PUTC for send the data by USART the program CAN enter again in the INT_RDA, but now the program can enter ONLY four times in INT_RDA.
What is the problem? The PIC16F88 could be the problem?
And the last cuestion: If I have installed CCS v5.012 I shouldn't have problems with PRINTF no?
Thanks
|
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Tue May 06, 2014 5:16 am |
|
|
| ckielstra wrote: | Then a few comments on coding style:
- Please check the formatting of your code. It is now difficult to read because indentation for the lines is using different styles and sizes everywhere.
- Please use defines instead of hard coded values. For example you have the magic value '91' in many places where you mean the size of one of the arrays. Add something like | Code: | | #define RECEIVE_ARRAY_SIZE 91 |
at the top of your program.
Same applies to the I/O pins. Compare these two: | Code: | output_low(PIN_B0);
#define LED PIN_B0
output_low(LED); |
- Use the standard C functions whenever possible to create more efficient and easier to read programs:
| Code: | for(i=0;i<91;i++){
trama_entrada[i]='\0';
} |
can be replaced by: | Code: | | memset(trama_entrada, 0, sizeof(trama_entrada)); |
|
Thanks I will change it in my program later. I need improve my programing. But The big problem now is that I don' know why my program work bad. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19540
|
|
| Posted: Tue May 06, 2014 6:45 am |
|
|
I suggest you 'revector' the print.
Code as:
| Code: |
void putc_gie(char chr)
{
putc(chr);
enable_interrupts(GLOBAL);
}
//then where you printf
printf(putc_gie,"what you want to print");
|
As I said, if you search the forum, you will find this problem being discussed. I remember it as being an issue with 4.140, hence my guess as to what happens. Problem is that the compiler disables the GIE when doing a table access, but then forgets it needs to be 'on', if waiting to receive data while sending. Encapsulating the putc, ensures that the interrupt gets re-enabled between the characters. |
|
|
drh
Joined: 12 Jul 2004
Posts: 192
Location: Hemet, California USA
|
|
| Posted: Tue May 06, 2014 7:45 am |
|
|
| Quote: |
#fuses HS, NOWDT, NOLVP, NOBROWNOUT, NOPROTECT, PUT, INTRC_IO
#use delay (internal=8MHz)
#use RS232(UART1)
|
Which are you using, a xtal or the internal osc.?
Specify one or the other, not both.
_________________
David |
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Tue May 06, 2014 1:27 pm |
|
|
| drh wrote: | | Quote: |
#fuses HS, NOWDT, NOLVP, NOBROWNOUT, NOPROTECT, PUT, INTRC_IO
#use delay (internal=8MHz)
#use RS232(UART1)
|
Which are you using, a xtal or the internal osc.?
Specify one or the other, not both. |
Now I will change it Thanks. (I will use internal clock) |
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Tue May 06, 2014 3:37 pm |
|
|
| Ttelmah wrote: | I suggest you 'revector' the print.
Code as:
| Code: |
void putc_gie(char chr)
{
putc(chr);
enable_interrupts(GLOBAL);
}
//then where you printf
printf(putc_gie,"what you want to print");
|
As I said, if you search the forum, you will find this problem being discussed. I remember it as being an issue with 4.140, hence my guess as to what happens. Problem is that the compiler disables the GIE when doing a table access, but then forgets it needs to be 'on', if waiting to receive data while sending. Encapsulating the putc, ensures that the interrupt gets re-enabled between the characters. |
If I need send by serial port a data like this:
data[19]="$ADQ,P01,CH1,3333\r";
The part of the code that sends by serial port is it. It is correct?. It seems that this part works OK but ONLY follow print by serial port FOUR TIMES the data. What is the problem now? I am using an Arduino that reads this data.
(The INT_RDA works OK because the led that I programing show it)
Is the problem the model of the pic (16f88)? buffers USART full? I don't know
| Quote: | for(i=0;i<19;i++){
putc(data[i]);
enable_interrupts(GLOBAL);
} |
Other cuestion is... when my program print by serial port the data that I want see the value of ADC not change. Is the same in the four times that get see the data that I print by serial port. Is necessary clear the bit activation of the ADC when I read its value?
Thanks.  |
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Wed May 07, 2014 4:12 am |
|
|
| The problem was the version. If I use CCS 5.015 code works OK. But the value of ADC not change. I need clear de adc before read adc? |
|
|
Salfer
Joined: 20 Mar 2014
Posts: 15
|
|
| Posted: Thu May 08, 2014 2:34 am |
|
|
| The code now works OK. I installed a new version of CCS and redefined the variables and the code now works ok. Thanks everyone. |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
