Problem beim Empfang von Strings

[Marco G. (grmg2010)]

Moin,

ich habe Probleme beim Empfangen von Strings auf meinem ATMEGA168.

Hintergrund: Ich versende vom PC mehrere Strings mit Header und /crlf am 
Ende an den Controller. Mit Hilfe der Header sollen verschiedene 
Aktionen ausgeführt werden. Zum Testen habe ich zwei Strings genommen, 
die einmalig per Knopfdruck im Abstand von 250ms versendet werden.

Der erste String wird richtig übernommen, nur der zweite ist immer Null. 
Als Empfangsroutine nutze ich die hier in der Wiki eingetragene 
Funktion. Der restliche Quellcode sieht wie folgt aus:

#define F_CPU 8000000UL
#define UART_MAXSTRLEN 20

#include <avr/io.h>
#include <avr/pgmspace.h>
#include <avr/eeprom.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <stdio.h>
#include <string.h>
#include "avr.h"
#include "M41T93MYZ.h"
#include "SPI.h"
#include "UART.h"



uint8_t Alarmanlage, status = 0, counterTemp = 0, TempAvailable = 0, UARTREceived = 0;
uint8_t Thour, TMinute,TSecond, TWDay, TDay, TMonth, TYear = 0;
char Buffer[10], BUfferHour[3], BufferMinute[3], BufferSecond[3], BufferTemperatur[5];
char *ptr, BufferBCD[10], BMinute[10], BHour[10], BSecond[10];
uint8_t test, x, y, z;
uint8_t Hour_bcd, SetHourBCD, minute_bcd,SetMinuteBCD, second_bcd, SetSecondBCD;

/*UART Receive Variablen*/
char Received_String[UART_MAXSTRLEN + 1];
uint8_t uart_str_complete = 0;
uint8_t uart_str_count = 0;
char uart_string[UART_MAXSTRLEN + 1] = "";

ISR(USART_RX_vect)
{
  unsigned char nextChar;
  
  nextChar = UDR0;
  if( uart_str_complete == 0 )
  {  
    if( nextChar != '\n' &&
    nextChar != '\r' &&
    uart_str_count < UART_MAXSTRLEN )
    {
      uart_string[uart_str_count] = nextChar;
      uart_str_count++;
    }
    else
    {
      uart_string[uart_str_count] = '\0';
      uart_str_count = 0;
      uart_str_complete = 1;
    }
  }
}

int main(void)
{  
  avr_init();

    while(1)
    {
    
    if (uart_str_complete == 1)
    {
      strcpy(Received_String, uart_string);
      strcpy(uart_string, " ");
      uart_str_complete = 0;
    }
    
    if (strstr("h", Received_String) == 0)
    {
      strcpy(BHour, Received_String + 1);
      /*ptr = Received_String + 1;*/
      
      Hour_bcd = atoi(BHour);
      SetHourBCD = DezToBCD(Hour_bcd);
//       itoa(SetHourBCD, BufferBCD, 2);
//       Send_UART_Char(BufferBCD);  
      strcpy(Received_String, " ");
      x = 1;
      led_bl(1);
    }
    
    if (strstr("m", Received_String) == 0)
    {
      strcpy(BMinute, Received_String + 1);
      Send_UART_Char(BMinute);
      /*ptr = Received_String + 1;*/      
      minute_bcd = atoi(ptr);
//       SetMinuteBCD = DezToBCD(minute_bcd);
//       itoa(SetMinuteBCD, Buffer, 2);
      Send_UART_Char(Buffer);
      strcpy(Received_String, " ");
      y = 1;
      led_rt(1);
    }
    
    if (x == 1 && y == 1)
    {
      itoa(SetHourBCD, BufferBCD, 2);
      Send_UART_Char(BufferBCD);
      itoa(SetMinuteBCD, Buffer, 2);
      Send_UART_Char(Buffer);
      //led_rt(1);
      set_clock(0x00, SetMinuteBCD, SetHourBCD );
      x = 0;
      y = 0;
    }

    }
}

Was habe ich übersehen? Ist die von mir veranlasste Pause beim senden zu 
kurz? Oder liegt es an etwas Anderem?

Gruß

[Matthias S. (Firma: matzetronics) (mschoeldgen)]

Es lohnt sich, Variablen die als Kommunikation zwischen ISR und 
Hauptprogramm dienen, als 'volatile' zu erklären. Denn der Compiler 
sieht im Unterprogramm der ISR erstmal ein Software Stückchen, das nie 
ausgeführt wird. Im Hauptprogramm wird z.B. 'uart_str_complete' immer 
nur gelesen, und das erkennt der Compiler.

Also

volatile uint8_t uart_str_complete = 0;
volatile uint8_t uart_str_count = 0;
volatile char uart_string[UART_MAXSTRLEN + 1] = "";

 sollte dich weiterbringen. Leider schreibst du ja nix über das 
Verhalten der Test LEDs.
Es könnte auch sein, das es sich lohnt, das alles zu vereinfachen. Zum 
Stellen der Uhr würde auch ein 'h0945' String reichen. die ersten beiden 
Stellen sind dann Stunden und die letzten die Minuten.

[Peter II (Gast)]

strcpy(BHour, Received_String + 1);

BHour ist nur 10 Zeichen lang, Received_String kann aber bis 20 Zeichen 
enthalten.


Welche Strings überträgst du denn?

Was willst du damit immer erreichen?

strcpy(Received_String, " ");

Was soll das Leerzeichen im String? Wenn du den String leer machen 
willst, dann schreibe einfach ein \0 rein. Aber das ist eigentlich gar 
nicht notwendig, weil der Inhalt ja eh immer überschrieben wird.

Received_String[0] = 0;

µC haben wenig Speicher, warum verschwendest du ihn dann?

char BMinute[10], BHour[10], BSecond[10];

du brauchst doch immer nur einen davon, dann kann man immer die gleiche 
Variabel nutzen. Es ist sogar so, das du es überhaupt nicht bracuhst.

strcpy(BHour, Received_String + 1);
Hour_bcd = atoi(BHour);

warum nicht gleich so?:

Hour_bcd = atoi(Received_String + 1);

[Johann L. (gjlayde)]

Argumente von strstr sind "Heuhaufen" und "Nadel", nicht umgekehrt: In 
einem Heuhaufen "m" wirst nicht viele Nadeln finden :o)

[Marco G. (grmg2010)]

Matthias S. schrieb:
> Es könnte auch sein, das es sich lohnt, das alles zu vereinfachen. Zum
> Stellen der Uhr würde auch ein 'h0945' String reichen.

Daran hatte ich auch schon gedacht, nur fehlt mir die richtige Idee, um 
die Stunden daraus zu extrahieren. Die Minuten wären kein Problem, da 
kann ich ja einfach die entsprechende Anzahl von Stellen Springen. Ich 
könnten bei den Stunden ja zweimal den String mit strcpy bearbeiten, 
einmal von vorne gezählt und einmal von hinten. Aber es muss doch auch 
einfacher gehen.

Die Leds machen was sie sollen. ich erhalte auch die Ausgabe der 
Variablem auf dem PC

Peter II schrieb:
> Wenn du den String leer machen
> willst, dann schreibe einfach ein \0 rein. Aber das ist eigentlich gar
> nicht notwendig, weil der Inhalt ja eh immer überschrieben wird.

Stimmt, aber ich sende nicht immer einen String. Das heißt ich würde 
immer in einer der if-Abfragen landen.

Johann L. schrieb:
> Argumente von strstr sind "Heuhaufen" und "Nadel", nicht umgekehrt: In
> einem Heuhaufen "m" wirst nicht viele Nadeln finden :o)

Sehr schöner Vergleich, der stimmt. Habe ich jetzt korrigiert.

Auf den ersten Blick schein es jetzt zu funktionieren. Ich muss noch mal 
weiter testen.

Vielen Dank schon einmal für die Tipps.

[S. R. (svenska)]

Marco G. schrieb:
> Matthias S. schrieb:
>> Es könnte auch sein, das es sich lohnt, das alles zu vereinfachen. Zum
>> Stellen der Uhr würde auch ein 'h0945' String reichen.
>
> Daran hatte ich auch schon gedacht, nur fehlt mir die richtige Idee, um
> die Stunden daraus zu extrahieren.

Du weißt, dass das erste Zeichen der Befehl ist, das zweite und dritte 
Zeichen die Stunden, das vierte und fünfte Zeichen die Minuten. Außerdem 
weißt du, dass ein C-String ein Array aus Zeichen ist, und dass der 
ASCII-Code für die Ziffern aufsteigend ist und mit '0' anfängt. Fertig 
ist der simple Parser:

uint8_t h,m;
if(Received_String[0] == 'h') {
  h = (Received_String[1] - '0') * 10 + (Received_String[2] - '0');
  m = (Received_String[3] - '0') * 10 + (Received_String[4] - '0');
} else {
  // anderer Befehl
}

Vermutlich ist es sinnvoll, noch abzuprüfen, ob Received_String[1..4] 
auch Ziffern sind (d.h. zwischen einschließlich '0' und '9' liegen) und 
ob der String auch vollständig empfangen wurde.

Nachtrag: BCD ist übrigens des Teufels. ;-)

Du brauchst den String nicht vom UART extra kopieren, weil du nach dem 
Parsen sämtliche Informationen daraus extrahiert hast und eleganter (in 
Integern) speicherst. Der String kann danach weg.

Den Parser selbst kannst du übrigens - je nach Komplexität der Eingaben 
- auch direkt im UART-Interrupt als Zustandsautomat implementieren. 
Jedes Zeichen wird direkt beim Empfang ausgewertet und wenn der String 
vollständig war, wird das Hauptprogramm informiert. Damit bekommst du 
keine Probleme, wenn jemand zehntausend 'X' schickt, denn die werden 
alle nacheinander ignoriert.