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

HELP!!! BUG DETECTED IN COMPILER (PCWH 3.222)

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



Joined: 22 Mar 2007
Posts: 23
Location: Buenos Aires, Argentina

View user's profile Send private message

HELP!!! BUG DETECTED IN COMPILER (PCWH 3.222)
PostPosted: Thu Mar 22, 2007 5:26 pm     Reply with quote

Hello, this is my first message (although i was reading many time before).

I have encountered a problem in the compiler PCWH v 3.222. My program basically reads the port from the PC (in RS485, 2400 bps), buffers this information and retransmit this into a radio module (rs232, TTL, 9600 bps).

Because it ONLY receives from a RS485 IC, it has no needs to drive the "data enable" line. In my first program, i used the "enable" option in the #use rs232 directive, but after a modification I quit this "enable" option and the program does not work anymore (this sent false data to rs232 to the radio).

This rs232 is all for hardware (rx pin in rs485 driver and tx pin to the radio module). I don?t understand why i MUST put the enable option to a pin which is in the air for the program to work. It disappointed me so bad, and i want to know if this bug is repaired in the new versions of PCWH.

Here is the program:

Code:

#include <16F628.h>

#use delay(clock=4000000, restart_wdt)
#use rs232(baud=2400, xmit=PIN_B2, rcv=PIN_B1, enable=PIN_A2, restart_wdt)

#fuses XT,WDT,PUT,PROTECT,BROWNOUT,NOMCLR,NOLVP,NOCPD

#byte T1CON = 0x10
#byte OPTION = 0x81

#bit CREN = 0x18.4

#define LED PIN_A1
#define RADIO PIN_A0
#define CONFIG PIN_B4

unsigned char receive;
unsigned short int flag=0;
unsigned short int final=0;
unsigned char cod_repe, rx[74]={0};
unsigned int i=0;

void _init_radio(void) { // configura radio en 433.92
   setup_uart(9600);
   delay_ms(1500);
   printf("%C%C%C%C", 0x13, 0x01, 0x80, 0xD7);
   delay_ms(100);
   printf("%C%C%C%C", 0x13, 0x03, 0xA6, 0x20);
   delay_ms(100);
   printf("%C%C%C%C", 0x13, 0x09, 0x98, 0x50);
   delay_ms(100);
   printf("%C%C%C%C", 0x13, 0x05, 0x94, 0xA0);
   delay_ms(100);
   printf("%C%C%C%C", 0x16, 0x00, 0x53, 0x00);
   delay_ms(200);
   setup_uart(FALSE); // apaga RS-232
   output_float(PIN_B2); // transmit en alta impedancia
   output_float(PIN_B1); // receive en alta impedancia
   output_float(CONFIG); // pata de configuracion en alta impedancia
   output_high(RADIO); // apaga la radio
   delay_ms(1000);
   output_low(CONFIG); // sale del modo de configuracion
   output_low(RADIO); // enciende la radio
   delay_ms(1000);
// apaga la radio nuevamente
   output_high(RADIO); // apaga la radio
   delay_ms(1000);
   output_low(RADIO); // enciende la radio
   setup_uart(TRUE); // habilita puerto serie
   delay_ms(1500);
   setup_uart(2400);
}

void main(void) {
   unsigned int x;
   setup_wdt(WDT_2304MS);
   restart_wdt();
   output_high(CONFIG); // ingresa al modo de configuracion de la radio
   output_low(RADIO); // enciende la radio
   OPTION = 0xC1;
   T1CON = 0x31; // timer 1 on, ps 1:8
   _init_radio();
   CREN = 0;
   CREN = 1; // recepcion continua
   cod_repe = read_eeprom(0x01);
   enable_interrupts(GLOBAL);
   enable_interrupts(INT_RDA);
   for(;;) {
      restart_wdt();
      if(flag == 1) {
         setup_uart(9600);
         printf("%C%C", 252, cod_repe); // envia cabezal
         for(x = 0; x < i; x++) { // envia bytes de datos
            putc(rx[x]);
            restart_wdt();
         }
         flag = 0;
         setup_uart(2400);
         CREN = 1; // habilita nuevamente recepcion
      }
   }
}

#INT_RDA
void _recep(void) {
   receive = getc();
   if(receive == 254) {
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() <570>= 570) // sale si no llega codigo repetidora
         return;
      if(getc() != 254)
         return;
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() <570>= 570) // sale si no llega codigo repetidora
         return;
      if(getc() != cod_repe)
         return;
      CREN = 0;
      output_high(RADIO); // apaga la radio
      setup_uart(FALSE); // apaga RS-232
      output_float(PIN_B2); // transmit en alta impedancia
      output_float(PIN_B1); // receive en alta impedancia
      output_float(CONFIG); // pata de configuracion en alta impedancia
      delay_ms(500);
      output_high(CONFIG); // ingresa al modo de configuracion de la radio
      output_low(RADIO); // enciende la radio en modo configuracion
      setup_uart(TRUE);
      _init_radio();
      CREN = 1; // recepcion continua
   }
   if(receive == 252) {
      final = 0;
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() <570>= 570) // sale si no llega codigo repetidora
         return;
      if(getc() != cod_repe)
         return;
      output_high(LED); // prende luz de recepcion
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() <570>= 570)
         return;
      for(i = 0; final == 0; i++) {
         rx[i] = getc();
         set_timer1(0);
         while((kbhit() == 0) && (get_timer1() <570>= 570)
            final = 1;
         if(i == 74) {
            flag = 1;
            final = 1;
            CREN = 0; // desactiva recepcion
            output_low(LED); // apaga luz
            return;
         }
      }
      CREN = 0; // desactiva recepcion
      flag = 1;
      output_low(LED); // apaga luz
   }
}




thaks you very much!!!!!!!
Diego. Very Happy Very Happy
metalm2
Guest







PostPosted: Fri Mar 23, 2007 12:35 pm     Reply with quote

I have tested this program in the PCWH 4.023 with no positive results (the error was the same).

I am very dissapointed with this product and this bug causes me to leave this and use another compiler.

thanks,
Diego.
Ttelmah
Guest







PostPosted: Fri Mar 23, 2007 4:05 pm     Reply with quote

Don't blame the compiler for your code.
As posted, I assume you had not read the forum code posting guidelines, and have left html enabled. The code as shown is tripe, however much of this may be because of html. Lines that 'scream' fault, are the while loops inside the interrupt routine, which as posted make no sense at all, with every loop causing a 'return...
Even worse though, you have delays in the interrupt code. This implies that interrupts will be disabled in all delays in your external code, and the UART _will_ become hardware locked up. I'd suspect that the 'enable' routine, as an 'aside', adds monitoring for the UART status, and just happens to clear the error this causes. Adding 'ERRORS' to the RS232 declaration, will make this happen without the enable line.
The code is a real example of how _not_ to program interupt events.

Best Wishes
metalm



Joined: 22 Mar 2007
Posts: 23
Location: Buenos Aires, Argentina

View user's profile Send private message

PostPosted: Fri Mar 23, 2007 4:38 pm     Reply with quote

Thanks for the response, that "while" bucles are bad because the HTML code is activated. I post it again:

Code:

#include <16F628.h>

#use delay(clock=4000000, restart_wdt)
#use rs232(baud=2400, xmit=PIN_B2, rcv=PIN_B1, enable=PIN_A2, restart_wdt)

#fuses XT,WDT,PUT,PROTECT,BROWNOUT,NOMCLR,NOLVP,NOCPD

#byte T1CON = 0x10
#byte OPTION = 0x81

#bit CREN = 0x18.4

#define LED PIN_A1
#define RADIO PIN_A0
#define CONFIG PIN_B4

unsigned char receive;
unsigned short int flag=0;
unsigned short int final=0;
unsigned char cod_repe, rx[74]={0};
unsigned int i=0;

void _init_radio(void) { // configura radio en 433.92
   setup_uart(9600);
   delay_ms(1500);
   printf("%C%C%C%C", 0x13, 0x01, 0x80, 0xD7);
   delay_ms(100);
   printf("%C%C%C%C", 0x13, 0x03, 0xA6, 0x20);
   delay_ms(100);
   printf("%C%C%C%C", 0x13, 0x09, 0x98, 0x50);
   delay_ms(100);
   printf("%C%C%C%C", 0x13, 0x05, 0x94, 0xA0);
   delay_ms(100);
   printf("%C%C%C%C", 0x16, 0x00, 0x53, 0x00);
   delay_ms(200);
   setup_uart(FALSE); // apaga RS-232
   output_float(PIN_B2); // transmit en alta impedancia
   output_float(PIN_B1); // receive en alta impedancia
   output_float(CONFIG); // pata de configuracion en alta impedancia
   output_high(RADIO); // apaga la radio
   delay_ms(1000);
   output_low(CONFIG); // sale del modo de configuracion
   output_low(RADIO); // enciende la radio
   delay_ms(1000);
// apaga la radio nuevamente
   output_high(RADIO); // apaga la radio
   delay_ms(1000);
   output_low(RADIO); // enciende la radio
   setup_uart(TRUE); // habilita puerto serie
   delay_ms(1500);
   setup_uart(2400);
}

void main(void) {
   unsigned int x;
   setup_wdt(WDT_2304MS);
   restart_wdt();
   output_high(CONFIG); // ingresa al modo de configuracion de la radio
   output_low(RADIO); // enciende la radio
   OPTION = 0xC1;
   T1CON = 0x31; // timer 1 on, ps 1:8
   _init_radio();
   CREN = 0;
   CREN = 1; // recepcion continua
   cod_repe = read_eeprom(0x01);
   enable_interrupts(GLOBAL);
   enable_interrupts(INT_RDA);
   for(;;) {
      restart_wdt();
      if(flag == 1) {
         setup_uart(9600);
         printf("%C%C", 252, cod_repe); // envia cabezal
         for(x = 0; x < i; x++) { // envia bytes de datos
            putc(rx[x]);
            restart_wdt();
         }
         flag = 0;
         setup_uart(2400);
         CREN = 1; // habilita nuevamente recepcion
      }
   }
}

#INT_RDA
void _recep(void) {
   receive = getc();
   if(receive == 254) {
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() < 570))
         restart_wdt();
      if(get_timer1() >= 570) // sale si no llega codigo repetidora
         return;
      if(getc() != 254)
         return;
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() < 570))
         restart_wdt();
      if(get_timer1() >= 570) // sale si no llega codigo repetidora
         return;
      if(getc() != cod_repe)
         return;
      CREN = 0;
      output_high(RADIO); // apaga la radio
      setup_uart(FALSE); // apaga RS-232
      output_float(PIN_B2); // transmit en alta impedancia
      output_float(PIN_B1); // receive en alta impedancia
      output_float(CONFIG); // pata de configuracion en alta impedancia
      delay_ms(500);
      output_high(CONFIG); // ingresa al modo de configuracion de la radio
      output_low(RADIO); // enciende la radio en modo configuracion
      setup_uart(TRUE);
      _init_radio();
      CREN = 1; // recepcion continua
   }
   if(receive == 252) {
      final = 0;
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() < 570))
         restart_wdt();
      if(get_timer1() >= 570) // sale si no llega codigo repetidora
         return;
      if(getc() != cod_repe)
         return;
      output_high(LED); // prende luz de recepcion
      set_timer1(0);
      while((kbhit() == 0) && (get_timer1() < 570))
         restart_wdt();
      if(get_timer1() >= 570)
         return;
      for(i = 0; final == 0; i++) {
         rx[i] = getc();
         set_timer1(0);
         while((kbhit() == 0) && (get_timer1() < 570))
            restart_wdt();
         if(get_timer1() >= 570)
            final = 1;
         if(i == 74) {
            flag = 1;
            final = 1;
            CREN = 0; // desactiva recepcion
            output_low(LED); // apaga luz
            return;
         }
      }
      CREN = 0; // desactiva recepcion
      flag = 1;
      output_low(LED); // apaga luz
   }
}


The program originally was without any delay into the interrupt routine.
This program only works with the "enable" directive in the #USE RS232 preprocessor.
Sorry about the blame, i made this in the work and made me cost so much effort and "extra hours" to discover this error. In addition my native language isn't english and it cost me LOTS to understand and to express me...

What kind of difference made the "enable" option? I think it isn't my error because my father is about to finish his program and he encountered another problem with other things.. (he changes only a line and the MCU does a thing completely different).

Can anyone tellme if this is my error? Is there a form to send this program to CCS official support?

Do you think this compiller can replace compillers like HT-PIC or IAR?

thanks so much,
Diego.
ckielstra



Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands

View user's profile Send private message

PostPosted: Fri Mar 23, 2007 5:51 pm     Reply with quote

I don't have v3.222 so tested with v3.226 instead. I compared the assembly output of the program with and without the enable option and the code differs more than I expected. The code with the enable keyword present calls a subroutine for each character to be transmitted while the version without the enable keyword handles each transmission inline.

The generated code differences look fine to me, it doesn't make me believe the compiler is in error. Much more likely your code is the problem. I don't like to say it, but your code is a good example of how interrupts are not to be used. Most likely the correct working version is just lucky, a small timing difference makes it work or something like that.

My suggestion, rewrite your code so it is not using interrupts.

And a small but dangerous error to fix:
Code:
if(i == 74)
If you ever get here you have a buffer overflow. Either change this to 73 or make your buffer 75 bytes long.
Another bug is that in this situation the last received character will not be stored in rx[] and is lost.

Quote:
Do you think this compiller can replace compillers like HT-PIC or IAR?
This is a question worth it's own thread.
metalm



Joined: 22 Mar 2007
Posts: 23
Location: Buenos Aires, Argentina

View user's profile Send private message

PostPosted: Sat Mar 24, 2007 10:53 pm     Reply with quote

Thanks for your response,

I don't understand what is wrong in my code into the interrupt, what things cannot be placed into an interrupt?


Testing the program with the enable option activated (unused but activated):

I send the string: (2400 bps)
<252><cod_repe><0><36><200>
then this device sends this same string to the radio in 9600 bps.

The program without the enable option:

I send this same string in 2400 bps:
<252><cod_repe><0><36><200>
This sends to the radio module:
<252><cod_repe><0><128><128><128>

It looks like this program is not handling good the baudrates, because when I send some string to the radio in 2400 bps (the radio works in 9600), this sends <128><128>.....

now into the program:

when "i" variable becomes 74, the "for(i = 0; final == 0; i++)" gets broken and the CREN bit is cleared to prevent overflow errors.
This CREN bit is set later , when the bufferized data is retransmited.

Why do you think my interrupt code is bad? this is making me feeling like an idiot! Sad Sad Sad
I never take a programming course, but i think that if i don't put accidentally this enable option, I had been thinking that my program is bad but "logically" must work fine, isn't it?

equally I am so glad with your help and I prefeer to this to be my error, because this compiller saves much work for me because it's ease of use and built in functions...


thanks,
Diego.
Ttelmah
Guest







PostPosted: Sun Mar 25, 2007 3:08 am     Reply with quote

It is important to understand what an interrupt 'means'. It is a way to jump to a specific piece of code, as a result of a hardware event.

However there are some very big limitations relating to it's use. For instance, in the case of the 'serial' interrupt, the hardware has just two characters of buffering. While you are inside the interrupt handler, the chip cannot respond to this or other interrupts (ignoring high priority interrupts for simplicity). So, in the case of serial comms, you have the first 'rule'. The handler, _must not_ in any situation, take longer than two character times, or at best data will be lost, and at worst, the UART will be hung. For 9600bps, with normal '10 bit' characters (one start, 8 data bits, and one stop), 2.08mSec. Also, the _cumulative_ time (the average length of the handler, as opposed to the 'worst case', must not exceed one character time (1.04mSec).

Now this also applies to every other interrupt in use. If (for instance), you have two interrupts occurring (serial, plus a timer), the worst case total time in any handler 'combination', must not exceed the shortest 'repeat' interrupt interval required.

Now you then run into the next problem. The chip, does not support a variable stack (it only has a stack for addresses). This means it cannot support 're-entrancy' (where a routine is called from inside itself) - it is actually possible to generate re-entrant code, using a software driven stack, but it involves a huge programming overhead on this hardware... The problem here is that if _any_ routine is called inside an interrupt, the compiler has to _ensure_ that this cannot be called inside the same routine in the 'main' code. To ensure this, interrupts will be disabled during the same routine in the 'main'. This implies (in the example given), that interrupts _will_ be disabled in all 'delay' calls outside the interrupt handler. Since these are long, the same problem with not handling the interrupt events will then appear here. So, in general, you should not call any code inside the handler, unless you have either handled this yourself (by creating two copies of the code), or are sure that the delays this introduces are acceptable.

Then you have to look at timings involved in hardware actions. If (for instance), you wait for a second character inside an interrupt event, then this basically cannot take less than the 'character time', and so runs back into making the handler slow. So again waiting for events, other than the one that has already triggered the interrupt, again needs to be treated with great care (typical example of a 'problem', is printing inside the handler, or waiting for moe than one character).

So the 'rule of thumb', is as follows:
Make interrupt handlers as short as possible, and should always be shorter than the interval expected between interrupts.
Only 'break' this, if you fully understand the hardware and the implications of what is going on...

Now we then have to look at transmission. The chip, again has a small hardware buffer (maximum of two characters). If you 'send' a character, and the buffer is empty, the code will simply write the data to the buffer, and return immediately. This allows your code to be getting on with other things, while the transmission takes place. There is no hardware support for an 'enable' line, so if this is used, extra code is introduced, which checks the status of the transmit buffer. The difference here is, that with 'enable' turned on, the code, once a character is sent, has to _wait_ for this to be physically sent before turning off the enable line, and returning. Now if this makes the code 'work', it implies that the program flow is such, that if it starts doing something else, it is going wrong. Effectively, the code isonly working, if it waits for each character to be sent.

Now, to do things at 'intervals', with data being received, and sent, the normal way to work, is with a timer driven 'state machine'. What you do, is have a 'main' program, which loops quickly at some 'tick' interval (on the PC for example, the core 'tick', is 18mSec in W9x, or 10mSec in latter OS's). This loop, 'polls' flags representing the state of buffers etc.., (and in the case of the PC, separate program routines), and then hands 'control' to the required routine to handle the current events. In your case, what should be happening, is that the interrupt routine, does just _one_ job. Retrieves a single character. There is no point at all, in having a 'timeout' handler in the interrupt, since the fact that an interrupt has occured, means that there _is_ a character to retrieve. The character is stored in a buffer, and the code returns. Nothing else. Now in your main, instead of having huge pauses, the code should loop quite tightly, and if a long delay is needed, a counter for the number of loops is used, which then triggers the code to move 'on' to the next 'job'. When characters are seen, again a 'state' variable is changed, to reflect what has actually been seen, changing what is looked for next, and eventually what is actually 'done' with the data.

So basically, the actual 'structure' of your code, has problems with the limitations of the processor...

Best Wishes
metalm2
Guest







PostPosted: Wed Apr 04, 2007 9:03 am     Reply with quote

thanks for your response.

I insist in this because if the error is in MY program the debugger must advertise me what is wrong, and it couldn't be that removing the "enable" switch will cause the program to work unexpectedly.

If i must keep viewing the assembler in any program, so why i will use a C compiler, supposedly made to simplify work?


thanks again,
Diego.
ckielstra



Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands

View user's profile Send private message

PostPosted: Wed Apr 04, 2007 10:01 am     Reply with quote

Quote:
I insist in this because if the error is in MY program the debugger must advertise me what is wrong
Computers always do exactly what you tell them to do, that this often is not the same as what you want them to do is a real pitty but you can't blame the compiler for that.

Quote:
and it couldn't be that removing the "enable" switch will cause the program to work unexpectedly.
Welcome into the real world! You were just lucky that one flaw in your program (the unneeded ENABLE option) obscured another error.

Quote:
If i must keep viewing the assembler in any program, so why i will use a C compiler, supposedly made to simplify work?
Who says you have to keep viewing the assembler code? You made a design error in your program and this can be solved without any assembly knowledge. The reason I looked into the generated assembly code was out of curiousity to understand the low level differences between what happens with the enable keyword present and left out.

Quote:
when "i" variable becomes 74, the "for(i = 0; final == 0; i++)" gets broken and the CREN bit is cleared to prevent overflow errors.
True, you break out of the loop but only after executing
Code:
         rx[i] = getc();
The rx[] array has a size of 74. Counting starts at 0 so the last item in the array is rx[73]. Writing to rx[74] will overwrite other data in RAM with unpredictable results.


Again: Interrupt routines are supposed to be executing for very short times only. The easiest way to improve your program is to get rid of the interrupt.
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