 |
 |
View previous topic :: View next topic |
Author |
Message |
lan ahmad
Joined: 23 Jul 2012 Posts: 13
|
|
Posted: Fri Feb 08, 2013 7:14 am |
|
|
This is my coding to display time and date..my problem here is, the routine doesnot repeat. so i get only the fix time..can someone help..
here is my data from gps that i want to extract
Code: | $GPRMC,235852.000,A,0216.8807,N,10216.7840,E,0.83,117.82,171212 |
Code: | #include <16f876a.h>
#use delay(clock=20000000)
#fuses hs,nowdt,nolvp,noprotect
#define use_portb_lcd true
#include "flex_lcd.c"
#use rs232(baud=9600,parity=N,xmit=PIN_C6,rcv=PIN_C7,bits=8,stream=PORT1)
unsigned char SBUF;
char time[16], date[16];
#int_RDA
void RDA_isr()
{
static int i, j, start, scount;
SBUF = getc();
switch(SBUF)
{
case 0x24: /* '$' */ // Start of a sentence
start = 1;
break;
default:
break;
}/* end of switch(SBUF) */
switch(start)
{
case 1:
if(scount++ >= 4)
{
start = 2;
}
break;
case 2:
if(SBUF == 0x43) /* 'C' */ // this line contains the Date.Time information
{
start = 3;
scount++;
}
else
{
start = 0;
scount = 0;
}
break;
case 3:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 4;
}
break;
case 4:
if(SBUF != 0x2C) /* 'comma' */
{
if(i < 6)
{
time[i++] = SBUF;// stuff the variable 'time' with the current
break;
}
}
start = 5;
break;
case 5:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 6;
}
break;
case 6:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 7;
}
break;
case 7:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 8;
}
break;
case 8:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 9;
}
break;
case 9:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 10;
}
break;
case 10:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 11;
}
break;
case 11:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 12;
}
break;
case 12:
if(SBUF == 0X2C) /* comma */
{
scount++;
start = 13;
}
break;
case 13:
if(SBUF != 0x2C) /* 'comma' */
{
if(j < 6)
{
date[j++] = SBUF;// stuff the variable 'time' with the current
break;
}
}
break;
default:
scount = 0;
start = 0;
break;
} /* end of switch */
}// end of RDA()
void main()
{
lcd_init();
delay_ms(50);
enable_interrupts(INT_RDA);
enable_interrupts(GLOBAL);
printf(lcd_putc,"\f initialize gps");
delay_ms(3000);
while(true)
{
lcd_gotoxy(2,1);
printf(lcd_putc,"Time= %c%c:%c%c:%c%c",time[0], time[1], time[2], time[3], time[4], time[5]);
lcd_gotoxy(2,2);
printf(lcd_putc,"Date= %c%c/%c%c/%c%c",date[0], date[1], date[2], date[3], date[4], date[5]);
}
} |
|
|
 |
Douglas Kennedy
Joined: 07 Sep 2003 Posts: 755 Location: Florida
|
|
Posted: Fri Feb 08, 2013 8:50 am |
|
|
Well you have a lot of code most of it in the RDA ISR. The rule for an ISR is less code is better coding. The isr ( interrupt service routine) should signal the receipt of a sentence after placing it in a buffer. The isr is called every time a char from the GPS shows up. It is acceptable to watch for the $ set a flag and examine in subsequent calls the GPRMC cancelling and going back to waiting for the next $ if the sequence GPRMC isn't met. Now the time spent in the isr is critical since you must be out of it before the next char could arrive. Many after getting the sequence GPRMC will place the remainder of the sentence into a character array a.k.a a buffer. After the GPRMC sentence is processed into the buffer the isr sets a sentence received flag for the main program. The main program waits for this flag and if it is set it starts to parse the buffer. This takes the parsing load off the isr allowing it to always catch an inbound char. The main routine operates mostly operates in the inter sentence space. The buffer needs to be able to store the last full sentence and the next partial sentence being received. Alternatively ( not so good) the main routine can disable the isr while it parses the received sentence.
Much is learned by writing incorrect code believing it is correct. It leads to learning from mistakes and since it is unpleasant just like touching a hot stove the learning is often lifelong. The thing to keep most in mind when working with UART's is what the word asynchronous means and its impact on the code you might write. |
|
 |
Ttelmah
Joined: 11 Mar 2010 Posts: 19734
|
|
Posted: Fri Feb 08, 2013 10:29 am |
|
|
If you look carefully, though though the code is massive in the ISR, it is a state machine, and as such what happens each time the ISR triggers, should be short. This is a reasonable approach. The layout though makes it hard to track what is happening, and whether the machine will operate as it should. Also (unfortunately), he is using 'default'. This makes the code inefficient. In CCS, a simple state machine _without default_, will code as a jump table. As soon as you add the default , this doesn't happen. It is much more efficient, to test for the statements not used yourself, and then have a state table without a default statement. Some redesign of the code (simple things like 'if (Sbuf==',')', will make the code a lot easier to understand - also stick to the C convention on variable names - all capitals is reserved for defines. Give the states names using an enum. Using a switch for the first test on Sbuf, is just stupid coding...
It looks as if the big problem is that the first test for the '$', does not reset scount....
Best Wishes |
|
 |
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Fri Feb 08, 2013 11:36 am |
|
|
i and j are never reset to zero again. |
|
 |
Ttelmah
Joined: 11 Mar 2010 Posts: 19734
|
|
Posted: Fri Feb 08, 2013 12:17 pm |
|
|
Yes.
I only looked at the first couple of states, and realised these would not parse correctly because of scount, but a couple of states later, we have the same problem from these other counters.....
Best Wishes |
|
 |
asmboy
Joined: 20 Nov 2007 Posts: 2128 Location: albany ny
|
|
Posted: Sat Feb 09, 2013 12:34 pm |
|
|
Dear Ian, what everybody is trying to say about your ISR is
that it needs improvement in your basic coding approach.
My rule for evaluating an ISR is this:
The BEST ISR , has the LEAST possible code and
consumes the fewest clock cycles possible when it is called.
Think of an ISR as being like a jail cell.
If you find yourself IN there, your best interest lies in getting out as fast as you can, and dealing with the all the details from OUTSIDE the cell.
ie: in your main() program or its sub functions |
|
 |
ezflyr
Joined: 25 Oct 2010 Posts: 1019 Location: Tewksbury, MA
|
|
Posted: Sat Feb 09, 2013 6:38 pm |
|
|
Hi Ian,
A couple of thoughts:
1. Add 'errors' to your #use rs232 statement to prevent a UART lockup due to a buffer overrun.....
2. Did you write that code? If so, I'm surprised you couldn't debug it yourself. Anytime I write code containing a 'state' machine, I always add enough diagnostic printf statements, and then run as many input combinations as necessary to test the code. When I recently did this with similar code, I used a PC and Hyperterminal in place of the GPS module to see how things behaved! If you add printfs to an interrupt, be sure to remove them after testing! Learn to troubleshoot!
3. Your ISR is pretty ugly. My eyes became crossed after reading about 10 lines..... When I did a similar project, I did it a bit differently. I used the ISR to receive a complete NMEA message, and then parsed the message in Main() to extract the time/date. I started with the CCS example program 'ex_sisr.c', which implements a circular buffer serial receiving scheme. In my ISR I coded a state machine to exclude all NMEA strings except the one I wanted, $GPRMC. When the full string is received, it gets parsed in Main() to extract the time/date. This method is desirable because you can reuse the same code if you ever wish to receive other NMEA messages, simply by changing the state machine in the ISR. I think it's also more reliable due to the circular buffering, and it's much cleaner to read!
John |
|
 |
|
|
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
|