Some characters lost in serial communication

So how large are your buffers?

Is the code that inserts the data for transmission waiting if the transmit buffer is full?

And note that your receive code just inserts data - can you guarantee that the receive buffer never gets full?

Using CrapperTerminal?

do the transfer to the PC in main

Thanks all for your kind response.

RxBuffer is 100 bytes whereas I have kept TxBuffer 8 bytes. I am already doing the transfer in main loop. In ISR_Serial I want to just copy the RxBuffer to tmpdata.

        while(1) {                                              // continuous loop (any no can do except 0)
                Check_USB_or_logger_mode();                     // check whether mode is changed, if yes show on lcd

                if(ser_data_avail){                             // if data available
                        if(logger_bit)
                                gps_rcvd_proc();
                        else
                                data_rcvd_proc();
                }
.
.
.

The data_rcvd_proc is as follows:

void data_rcvd_proc()
{
        char *recd_cmd;                 // command part of the string
        char *recd_dat;                 // data part of the string
        unsigned char xdata tmp_data[17];

        ES0=0;                          // disable serial interrupt
        if (strpos (RxBuffer, '=') != -1){              // if '=' found
                recd_cmd = strtok(RxBuffer,"=");
                recd_dat = strtok(NULL,"=");
.
.
.
        else if(strcmp(RxBuffer, "NAME")==0){
                Read_E_STR(&tmp_data[0], name_add, 16);
                printf("%s%s", tmp_data, crlf);
        }
.
.
.

From PC I send command say "NAME=1234567890123456CR". When mcu receives it it is stored in EEPROM. When I command from PC to retrieve the same string again, I send "NAMECR", then the received string is "12356789012456"

What I am wondering is that though the if statement is not satisfied, the problem occurs, whereas commenting out the total if loop there is no problem.

And ISR relies on speed.

If your receive interrupt takes too long, then the UART receive register will overflow because the UART will receive 2 consecutive characters before your processor have enough time for that second receive interrupt.

If you want to do any processing in the UART interrupt, that processing should be limited to single-character processing. Like looking for a specific character (maybe end of line) so you could inform the main loop when you have a full line of text available.

But the ISR must have a fixed maximum run time whatever the input data looks like, so you can guarantee that the ISR can end and the processor can trig a new interrupt fast enough for you to always be able to extract the next received character. And that goes even if you happens to use other interrupt sources too, that competes for processing time.

strcpy() is _not_ a good function to have in an ISR. Worse than that - with a 8051 processor you should try to have no function calls at all in the ISR because of the limited stack capabilities of the processor architecture.

Many library routines written using the C51 compiler are typically NOT reentrant. So what if a strcpy() is just being processed in the main application and gets interrupted with a reentrant call of strcpy()? This is my primary reason (secondary: speed) not to use those library functions in an interrupt routine.

I will describe the whole story, if time permits, please read.

Basically I have developed GPS Logger project successfully. The parts I have used were P89V51RD2, LCD, 24C512 EEPROM, RTC, GPS module, FT232R USB UART. I have used 74HCT241 buffer to select mcu communication with the GPS Logger or USB UART. Lattitude, Longitude, Altitude, Date, Time & one more parameter which is captured from an external source are displayed & logged by pressing a button. I have also developed VB application to retrieve the stored data, as well as view/modify some settings.

As suggested by you people, I had not used the strcpy() function in the ISR).

EVERYTHING WAS WORKING PERFECTLY & THESE DEVICES ARE ALREADY WORKING IN THE FIELD. But sometimes the RTC got corrupted (may be on power on or power off). I was also using the RTC general purpose RAM to store some settings including the current address pointer of the EEPROM.

So I decided to make some changes as not using RTC at all, & get date/time from GPS itself. Following strings are received by the GPS module per second.

$GPGGA,120058.000,2400.0000,N,12100.0000,E,0,00,0.0,0.0,M,0.0,M,,0000*67
$GPGSA,A,1,,,,,,,,,,,,,0.0,0.0,0.0*30
$GPGSV,1,1,01,04,00,000,00*4C
$GPRMC,120058.000,V,2400.0000,N,12100.0000,E,000.0,000.0,280606,,,N*76
$GPVTG,000.0,T,,M,000.0,N,000.0,K,N*02

Previously I was using the $GPGGA string without any problem. As now I also need date & time from the gps module, I decided to use the $GPRMC string. There is no altitude in this string which is acceptable to me.

While interpreting the &GPRMC string I got a number of junk characters, may be another string was overlapped, in the meantime. But I was getting continuous update of data. To avoid the junk characters, then I coded a valid_nmea() function, so that update will only be there if the received string's checksum is valid. But then the update was very very slow. So I concluded that I am not getting the proper string most of the times.

I tried a number of things but the problem was only solved by storing the received string into another variable in the ISR itself.

Now the logger is working perfectly, I am getting regular updates, nearly every second without any junk data. But the addition of that piece of code caused error in PC communication, as I have described earlier. So I added if statement, still same!!!

I will try using the if statement at the very beginning of the ISR & let you know the results. Thanks for reading.

1.
What's your connection with PC? Also by UART?
P89V51RD2 has only one UART. How do you communicate with GPS module and PC?

2.
Your external oscillator frequency? Do you use X2 mode of P89V51RD2?
P89V51RD2 is a 12T/6T 8051 variant with 40MHz max. speed. So it can only supply 6.x MIPS (with much lower efficiency) and will be tired like a dog.

3.
How about your application's computing load now? Have you measured the load rate? How about the resolution of the LCD and the refresh rate?
Check if: too many interrupts, too many data processing, too many LCD refreshing, terrible task dispatching, bad programming habits...

Oh, you're using FT232R USB-UART.
In my opinion, you should keep your interrupt routines as neat as possible, and I doubt so many lib functions here (say, 'printf') efficient.

you have way too much 'garbage' in your serial ISR.

A 'clean' serial ISR will just store a char in a (ring) buffer and/or transmit a char from a (ring) buffer.

ESPECIALLY you should totally avoid using library routines in an ISR you have no idea, whatsoever, what time they take, if they are reentrant, if ....

after being informed not to use library routines in an ISR you have not posted a new one.

Erik

Thanks all.

Ejack Zhang,

1. I have used FT232R for PC communication, I get CBUS3 signal low on USB enumeration. I am using that signal to switch over the UART path from GPS module to PC. This caused no error in my previous version.

2. I am not using X2 mode of P89V51RD2 & 11.0592Mhz crystal is used.

3. Sorry, I have no idea on how to measure the load rate. The LCD is refreshed continuously in the main loop. But I have set high priority to serial interrupt. In addition to serial, only Timer0 interrupt is used.

Erik Malund sir, if I don't use the strcpy() library routine, the PC communication problem is solved. But in GPS Logger mode, I am not getting data from the GPS module properly. On receiving ser_data_avail flag, I check for the $GPRMC string. If available, I analyze the string. I am getting junk data in the string's fields. And if I add valid_nmea() function to check the checksum, most of the times the string is rejected.

I didn't have any problem with $GPGGA string, though the string was not copied (using strcpy) to an another variable. But with the $GPRMC, data is received properly only if I use strcpy(), then there is problem in PC mode...

"I didn't have any problem with $GPGGA string, though the string was not copied (using strcpy) to an another variable. But with the $GPRMC, data is received properly only if I use strcpy(), then there is problem in PC mode..."

So you have a bug somewhere in your code, or one ring buffer that is too small in relation to how often your main loop looks for data there. Or your serial ISR is serviced too seldom.

But why try a strcpy() in the ISR instead of fixing your original error?

As long as the serial ISR is executing fast enough, then it will not drop any single character from the UART. As long as the ring buffer for received NMEA data is big enough compared to how often your main loop retrieves data and put into a string, you will not lose any data in the ring buffer either.

If you don't use a ring buffer, but a linear buffer (i.e. a full line directly), then you will get big issues when the GPS directly after end-of-line of one NMEA string directly starts to send out the next NMEA string. Any code that just looks at a '\r' or '\n' and restarts the index at zero will directly start to overwrite the previous NMEA string. That is a reason to have a ring buffer that can buffer that can fit enough characters while your main loop performs the last computations on the most recently received NMEA string, before you throw that line away and starts to pick up new data from the ring buffer again.

When you have issues - take one step back. Look at your code. Think what it does. Ponder where you get your timing issues. Rewrite the code to work instead of throwing extra code just in the hope it's a good solution.

It is not the ISR that should make a full-line copy of received NMEA data. Such copying means you get a huge load spike in the ISR depending on what characters you receive. That is bad. Very bad. You want your ISR to be as uniform as possible - similar amount of work every time. How else, do you think you will be able to really test the system if there are big random differences in runtime between different ISR activations?

Thank you, as per your suggestions, I will modify it & will come back soon.

It is not the ISR that should make a full-line copy of received NMEA data. Such copying means you get a huge load spike in the ISR depending on what characters you receive. That is bad. Very bad. You want your ISR to be as uniform as possible - similar SMALL amount of work every time. How else, do you think you will be able to really test the system if there are big random differences in runtime between different ISR activations?

a small addition to Pers statement

KISS means Keep ISRs Short and Simple

Thanks Eric, I was really thinking to search for the meaning of KISS. But I am really frustrated to solve my current problem, so I postponed it.