Probleme beim auslesen eines Strings über UART

[Euro]

Hallo,

ich bräuchte mal grad ein wenig Hilfe.
Ich versuche grad ein Programm zu schreiben was die Daten eines GPS-Empfängers ausliest und mir auf einem Display anzeigt.

Dafür hab ich mir eine Funktion ReadString() geschrieben welche nach dem Aufruf solange die Daten, die über UART kommen, in ein char-Array schreibt bis ein Komma kommt.

Hier mal zum Beispiel:
$GPRMC,155406.515,A,5246.9750,N,01022.7382,E,35.94 ,045.00,270808,,*3F
Aufgerufen wird die Funktion nach "$GPRMC," und soll mir die Zeit auslesen ("155406.515"). Was dann aber auf dem Display erscheint ist "15540", dann ein komplett schwarzes Zeichen und zuletzt ein "B". Es gibt auch kein Fehler wie Buffer Overflow oder ähnliches.
LCD und UART Library stammen von Peter Fleury.


#include <avr/io.h>
#include <stdlib.h>
#include <avr/interrupt.h>
#include "rncontrol.h"
#include "lcd.c"
#include "uart.c"

#define UART_BAUD_RATE 9600

#ifndef F_CPU
#def F_CPU 16000000
#endif


void WaitForChar(char zeichen)
{
char thisZeichen;

thisZeichen=(unsigned char)uart_getc();
while (thisZeichen!=zeichen)
{
thisZeichen=(unsigned char)uart_getc();
}
}

void WaitForString(char string[])
{
int ind=0;
char cc;
while(string[ind]!='\0')
{

cc=(unsigned char)uart_getc();
if (cc!=string[ind])
{
continue;
}
ind++;

}
}

char* ReadString() //Um diese Funktion geht es
{
char str[20];
int zeichen;
int i=0;

for(int j=0;j<20;j++)
{
str[j]='\0';
}

while(1)
{
zeichen=uart_getc();
//Warten auf Daten
if(zeichen& UART_NO_DATA)
{
continue;
}
else if(zeichen & UART_FRAME_ERROR)
{
lcd_clrscr();
lcd_puts("Frame ERROR");
waitms(500);
}
else if(zeichen & UART_OVERRUN_ERROR)
{
lcd_clrscr();
lcd_puts("Overrun ERROR");
waitms(500);
}
else if(zeichen & UART_BUFFER_OVERFLOW)
{
lcd_clrscr();
lcd_puts("Buffer OVERFLOW");
waitms(500);
}
//Abbruch wenn ','
else if ((unsigned char)zeichen==',')
{
str[i]='\0';
break;
}
else
{
str[i]=(unsigned char)zeichen;
i++;
}
}
return str;
}

char* ReadStringAnz(int anz) //Funktioniert einwandfrei
{
char str[20];
int zeichen;
int i=0;

for(int j=0;j<20;j++)
{
str[j]='\0';
}

while(1)
{
zeichen=uart_getc();
//Warten auf Daten
if(zeichen& UART_NO_DATA)
{
continue;
}
else if(zeichen & UART_FRAME_ERROR)
{
lcd_clrscr();
lcd_puts("Frame ERROR");
waitms(500);
}
else if(zeichen & UART_OVERRUN_ERROR)
{
lcd_clrscr();
lcd_puts("Overrun ERROR");
waitms(500);
}
else if(zeichen & UART_BUFFER_OVERFLOW)
{
lcd_clrscr();
lcd_puts("Buffer OVERFLOW");
waitms(500);
}
//Abbruch wenn Anzahl an Zeichen erreicht
else if (i>=anz)
{
str[i]='\0';
break;
}
else
{
str[i]=(unsigned char)zeichen;
i++;
}
}
return str;
}



int main(void) {


char* time;
char* av;
char* date;
char* lat;
char* lon;
char* ns;
char* ew;
char* spd;
char* crs;
char* hohe;
char zeichen;
int i=0;
int temp;


lcd_init(LCD_DISP_ON);

uart_init( UART_BAUD_SELECT(UART_BAUD_RATE,F_CPU) );
sei();
waitms(1000);

lcd_clrscr();

lcd_puts("Waiting for Data");
uart_getc(); //Vermeidung von Buffer Overflow beim erstmaligen lesen
uart_getc();
uart_getc();

waitms(1000);


while(1) //Main-Loop
{
WaitForString("$GPRMC,");
time=ReadString(); //Zeit auslesen


lcd_clrscr();
lcd_puts(time);

}




return 0;
}

Hoffe mir kann jemand sagen wo mein Fehler ist.

Gruß Euro

[sternst]

Hoffe mir kann jemand sagen wo mein Fehler ist.
Nun, einer springt mir sofort ins Auge:


char* ReadString() //Um diese Funktion geht es
{
char str[20];
...
return str;
}


str ist eine lokale Variable auf dem Stack und existiert nach dem return nicht mehr. Und zu dem Zeitpunkt, wo du den Inhalt überprüfst, ist sie dann auch schon wieder teilweise mit anderen Daten überschrieben.
Zwei Möglichkeiten das zu ändern:
1) Globale Variable benutzen.
2) Die lokale Variable static machen: static char str[20];

[sternst]

Ok, noch ein paar mehr Anmerkungen ;-)


char* ReadStringAnz(int anz) //Funktioniert einwandfrei
Wenn diese Funktion dieses Verhalten nicht zeigt, ist das lediglich Zufall. Hier gilt natürlich das Gleiche. Wenn du beide Funktionen nutzen willst, wäre Variante 1 (globale Variable) platzsparender.


for(int j=0;j<20;j++)
{
str[j]='\0';
}
Das ist überflüssig. Verschwendet nur Platz ohne irgendeinen Vorteil zu bringen.

Und zuletzt noch eine eher ästhetische Anmerkung: die if-else-if-Wurst finde ich unschön und unübersichtlich. Mein Gegenvorschlag:

char* ReadString ( void ) {

static unsigned char str[20];
unsigned int zeichen;
unsigned char i = 0;

while (i < 19) {

// warte auf nächstes Zeichen
do
zeichen = uart_getc();
while (zeichen & UART_NO_DATA);

// Fehler?
if (zeichen & 0xff00) {
lcd_clrscr();
if (zeichen & UART_FRAME_ERROR)
lcd_puts("Frame ERROR");
else if (zeichen & UART_OVERRUN_ERROR)
lcd_puts("Overrun ERROR");
else if (zeichen & UART_BUFFER_OVERFLOW)
lcd_puts("Buffer OVERFLOW");
waitms(500);
continue;
}

//Abbruch wenn ','
if (zeichen == ',')
break;

// Zeichen eintragen
str[i] = zeichen;
i++;
}

// terminieren
str[i] = '\0';

return str;
}

[Euro]

Danke für die Hilfe!
Hab jetzt mal alles mit globalen Variablen gemacht und es klappt. \:D/
Deinen "Gegenvorschlag" hab ich auch mit ins Programm genommen. Ist auf jedem Fall kürzer als das if - else if - else Geflecht.

Gruß Euro