Hello everybody,
My serial ISR is as below. When I send a string to the microcontroller some characters are not received. The characters are lost in a peculiar way. eg. 1234567890123456 is received as 12356789012456 & 6543210987654321 is received as 65421098765321. Exactly 4th & 13th characters are lost in the mentioned 16 character string, not tested further.
void ISR_Serial (void) interrupt 4 // using 2 { unsigned char in_byte; if (RI){ // If data is received.. RI = 0; in_byte = SBUF; // get character in var RxBuffer[ RxInChar ] = in_byte; // save it in the buffer at RxInChar location if(RxInChar==RX_BUF_SIZE) // if buffer full, (care should be taken by sending app) RxInChar=0; // then make it 0 else if(in_byte==0x0D){ // whether byte was CR? RxBuffer[ RxInChar ] = '\0'; // & replace it by NULL char RxInChar=0; // then make pointer to 0 (for next transmission) if(!pc_mode_active) strcpy(tmpdata,RxBuffer); ser_data_avail=1; // set data available flag for main routine } else RxInChar++; // increase the location pointer } if( TxEnabled ){ // If data is transmitted if (TI){ // if previous byte is finished transmitting TI = 0; if (TxOutChar != TxInChar){ // adjust queue and transmit character... SBUF = TxBuffer [TxOutChar++]; TxOutChar %= TX_BUF_SIZE;//% is the modulus operator } else TxEnabled = 0; // nothing else to transmit, so disable function... } } }
If I comment out following two lines, there is no problem. (though pc_mode_active is true at this time).
RxInChar=0; // then make pointer to 0 (for next transmission) // if(!pc_mode_active) // strcpy(tmpdata,RxBuffer); ser_data_avail=1; // set data available flag for main routine
The baudrate is 115200. RCAP2H = 0xFF & RCAP2L = 0xFD, in VB I've used "115200,N,8,1"
As pc_mode_active is true at default, the 'if loop' should not execute. Similarly there should not be a problem because the code will reach to this point only on receiveing CR which is at the last position. Please help.
Thanks in advance.
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.
But I am really frustrated to solve my current problem
that is the problem, instead ignore what you have and rewrite your small program [section] from scratch using ring buffers instead of trying "to solve my current problem"
Many have been stuck "trying to fix" something where the code was good, but the concept was wrong. That is not a good route.
PS -one mans opinion- "so I postponed it" never do that for more than a day or two, you will never get ahead by putting problems aside
Sorry, some confusion is here (may be my English). I have not postponed my original problem, I had just postponed to search for the meaning for KISS.
In this case, you seem to have forgotten that the GPS can send multiple lines every second, depending on what NMEA strings you have enabled. So there will only be inter-character delays from when it has received the first line until the ISR is busy filling that same line buffer with characters for the next NMEA string.
Your main loop main see the initial characters from the second string and then somewhere the data in the buffer will switch back to the remaining characters from the previous line. It is only the last text line in each burst you get (once/second) that your main loop is able to correctly process - unless your code have managed to clear your "i got data" flag for the new line when it was done with the previous line - but after the ISR had already detected the end-of-line for the next line.
The special thing with interrupts and real hardware is that the hardware doesn't stop just because you look at a piece of code in your main loop. Don't expect that the main loop code can do its work undisturbed - it gets constantly interrupted by new data comming in (or outgoing data being sent).
You must always be structured when looking at the code and trying to figure out what the code _can_ do. Interrupt-based code, just like threaded code, requires that you write the code with proper synchornization. Your code isn't infinitely fast. So when you call a function, the hardware will still have time to tick the timers, baudrate generators etc while that function runs.
You write that the GPS sends this, every second:
But when the ISR gets the $GPGSV string and then sets the flag that it has received a full line, the UART may have time to receive $GPRMC before your main loop looks at the buffer - so it falsely think you have a GPRMC string to decode.
In the same way - when that $GPRMC string is finally received and flagged as available, the UART is already busy getting the start of the $GPVTG string - which will destroy the $GPRMC string in your buffer if the main loop wasn't fast enough.
It is possible to have a small state machine in the ISR that just trigs on $GPRMC and starts receiving data after this prefix is found. Resets whenever a newline is seen. That would block overwrites at the cost of that little state machine. Or the ISR need to buffer data in a ring buffer big enough for the main loop poll frequency.
The above overwrites should be obvious if thinking about the timing of your code. A printout of your line now and then should be able to show you that you are likely to have the start from one string and the end from a different string. And real funny issues if the ISR manages to overwrite the terminating zero in the buffer before the main loop was done with the processing - suddenly you can get the code to start hunting for a random string termination in the processor memory.
Example of a state machine for NMEA decoding:
www.visualgps.net/.../NMEAParser.html
And look in the downloads section of this site for an example of interrupt-driven, ring-buffered serial comms...
Thanks all for your reply.
As per Per's remarks, the NMEA sentences are really overwritten/missed, because the technique I used is not perfect besides I also enable/disable the ES0 register in main loop. But still it is acceptable to me if I get proper sentence once in 3/4 seconds. Which was achieved at the cost of corruption in PC communication.
Now I decided to concentrate on the PC communication problem when gps code is still there in ISR.
One thing I forget to tell you that, I was using baud rate of 9600 for gps whereas 115200 for PC mode. I was changing the value of RCAP2L register on switching. Now I have tried 2 things and the results are as..
1. I reduced the baud rate in PC comm to 57,600. The missing character problem was solved.
2. As lowering baud will take long time to read the EEPROM content into the PC, I decided not to lower the baud rate but use the X2 mode which I HAVE NOT USED EARLIER. Now total PC communication is lost??? I have set 115200 in both mcu & PC. Any idea?