ASCII Verarbeitung optimieren

[µC-noob (Gast)]

Hallo, ich habe folgendes Programm geschrieben, womit ich Kommandos 
durch def. Zeichenabfolgen z.B. über das Hyperterminal an meinen µC 
sende, und dieser dann irgend etwas ausführt.
Ascii ist Vorgabe sonst hätt ich das mit einfachen dezimalzahlen 
gemacht.

Ich verwende einen ATmega 128 mit 16MHz

Der Code unten wird jede ms aufgerufen.

Ich finde aber der Code ist sehr unübersichtlich und aufwendig, vor 
allem, wenn ich noch weitere bedingungen hinzufüge (unten wird nur eine 
Bedingung bearbeitet).

Hat jemand einen Vorschlag wie ich das verbessern/ Optimieren kann.

void triggerCommand (void)
{
  unsigned char getChar = 0;

  static unsigned char savedChar[10];
  static unsigned char chosenPin;

  static unsigned char ENTER_Push[5];

  getChar = uart0_getc();

  LCDLine1[0] = savedChar[0];
  LCDLine1[1] = savedChar[1];
  LCDLine1[2] = savedChar[2];
  
  if( getChar == 'T' )  //Set Trigger
    savedChar[0] = 'T';
  

  if( savedChar[0] == 'T' )    //If T (Trigger) recieved
  {
    if( getChar == 'R' )  //If T + R (Trigger) recieved
      savedChar[1] = 'R';

    if( savedChar[1] == 'R' )  //If Trigger Mode
    {
      if( getChar == 'P' )  //If P (Pin) recieved
        savedChar[2] = 'P';
      else if( getChar == 'S' )
        savedChar[2] = 'S';
    
      if ( savedChar[2] == 'P' )
      {
        if ( getChar == 'I' )
          savedChar[3] = 'I';
        else if( getChar == 'E' )
          savedChar[3] = 'E';

        if( savedChar[3] == 'I' )
        {
          //chosenPin = atoi(getChar);

          if ( getChar == '\r' || getChar == '\n');
          {
            ENTER_Push[0] = 1;
          }
        }
      }
    }
  }
  
  add_menutext(&LCDLine1[0], "%d", chosenPin);

  if (ENTER_Push[0] == 1)
  {
    togglePeriod_slow(100, 4); //Beispielaufruf
    ENTER_Push[0] = 0;
  }

}

was sagt ihr dazu??


Gruß und danke
joe

[Falk B. (falk)]

@ µC-noob (Gast)

>Der Code unten wird jede ms aufgerufen.

Warum so oft?

>Hat jemand einen Vorschlag wie ich das verbessern/ Optimieren kann.

Hmm, warum liest du nicht erst ein komplettes Kommando ein, welchen mit 
RETRUN ('\r') abgeschlossen wird. Den empfangenen String kann man dann 
viel einfacher analysieren. Etwa so.

  char rx_buffer[16];

  while(1) {                                // endless main loop
    i=getc();
    putc(i);
    if (pos<16) {
      rx_buffer[pos]=i;
      pos++;
    }
    if (i=='\r') {
      switch (rx_buffer[0]) {
        case 'C': cmd_C(rx_buffer, pos); break;
        case 'I': cmd_I(rx_buffer, pos); break;  
        case 'O': cmd_O(rx_buffer, pos); break;        
        case 'P': cmd_P(rx_buffer, pos); break;
        default: put_error(); break; // command illegal
      }
      pos=0;
    }
  }

MFG
Falk

[Kai G. (runtimeterror)]

Ich glaube, du musst mir erklären, was der Code tun soll... das ist mir 
zu aufwendig zu lesen.

[UBoot-Stocki (Gast)]

Abo

[µC-noob (Gast)]

???? Abo ?????
Hä??


Danke erstmal,

hab das Programm erstmal zu testzwecken folgendermaßen angepasst:

void triggerCommand (void)
{

  static unsigned char rx_buffer[16];
  static unsigned char pos = 0; 
  unsigned char actChar = 0;
  
  static unsigned char testLCD = 0;  
                         
    actChar = uart0_getc();

  LCDLine3[7] = actChar;

    if ( pos < 16 )//Hier vermute ich den Fehler
  {
      rx_buffer[pos] = actChar;
      pos++;
    uart0_putc(actChar);
    }

    if (actChar =='\r') 
    {
      switch (rx_buffer[0]) 
      {
        case 'T': 
    testLCD = 1;
    break;
      }
      pos=0;
    }
}
  if (testLCD == 1)
     LCD_puts("eee");

Ich denke das liegt daran, dass die Bufferpos immer hochgezählt wird, 
auch wenn kein zeichen gesendet wird, korrigiert mich wenn ich Stuss 
erzähl??
Da ich den Code ja periodisch aufrufe, aber nicht jede Periode ein Char 
gesendet wird klappt das nann so nicht.

Aber wie stell ich es an, dass nur wenn ein zeichen empfangen wird, also 
nach einer eingabe im Hiperterminal, der Buffer gefüllt wird??

Danke
Joe

[Falk B. (falk)]

@ µC-noob (Gast)

>Ich denke das liegt daran, dass die Bufferpos immer hochgezählt wird,
>auch wenn kein zeichen gesendet wird, korrigiert mich wenn ich Stuss
>erzähl??

Stuss. pos wird nur höht, wenn ein zeichen empfangen wurde UND pos <15 
ist, um einen Bufferüberlauf zu vermeiden.

>Aber wie stell ich es an, dass nur wenn ein zeichen empfangen wird, also
>nach einer eingabe im Hiperterminal, der Buffer gefüllt wird??

Ganz einfach. Deine Funktion uart0_getc() muss so aufgebaut sein, dass 
sie nur dann ein Zeichen liefert, wenn eins im UART RX buffer steht. 
Wenn NICHT, wird NICHT gewartet. ODer halt einfach vorher das Flag 
abfragen und entsprechend verzweigen. Denk aber auch dran, dass dann 
deine Datenrate auf 1000 Zeichen/s begrenzt ist, also knapp 9k6 Baud.

MFG
Falk

[Karl H. (kbuchegg)]

µC-noob wrote:

> Aber wie stell ich es an, dass nur wenn ein zeichen empfangen wird, also
> nach einer eingabe im Hiperterminal, der Buffer gefüllt wird??

In dem Fall wird dir wohl uart0_getc() einen Returncode liefern
müssen, der genau das aussagt: Ich habe kein Zeichen empfangen.


Und darauf reagierst du dann. Zb. indem die Funktion triggerCommand
dann gar nicht weitermacht, sondern gleich retourniert.

Im übrigen kann ich nur Falk's Aussage noch mal unterstreichen:
Trenne das Empfangen eines Kommandos von der Auswertung. Trennen
heist in diesem Fall: Das sollen auch verschiedene Funktionen
sein.
Die eine Funktion empfängt einen String (Eine Sequenz von Zeichen
die zb. durch Empfang eines '\r' abgeschlossen wird).
Die andere Funktion bekommt einen String übergeben und wertet
ihn aus.

Gerade bei einfachen Systemen ist der Gedanke natürlich
verführerisch, dass alles in eine Funktion zu packen. Du
hast in deinem ersten Programm schon gesehen wohin das führt:
In endlose if-then-else Orgien. Und dabei hast du noch nicht
mal Fehlerbehandlung mit dabei.
Bei kleinen Programmen und wenigen Kommandos an der Schnittstelle
mag das sogar noch leidlich funktionieren. Sobald du aber mehr als,
sagen wir mal 8 bis 10 Kommandos hast, die ev. nicht mehr nur aus
einem Buchstaben bestehen, ist so ein Ansatz nicht mehr vernünftig
handhabbar.

[µC-noob (Gast)]

Erstmal herzlichen Dank für die Hilfe.


So sieht der Code derzeit aus:

void cmd_T(char *rx_buffer, unsigned char pos)
{
  if ( rx_buffer[1] == 'R' )
  {
    if( rx_buffer[2] == 'P' )
    {
      if( rx_buffer[3] == 'I' ) //if command TRPI (Triggerpin)
      {
        togglePeriod_slow(100, 4);  //To Test
      }
      else if ( rx_buffer[3] == 'E' ) //if command TRPE (Triggerperiod)
      {
        togglePeriod_slow(100, 2); //To Test
      }
    }
  }
}

void cmd_P(char *rx_buffer, unsigned char pos)
{
  
      add_menutext(&LCDLine3[0], "aaa",0);  

}

void put_error(void)
{

}


void triggerCommand (void)
{
  static char rx_buffer[16];
  static unsigned char pos = 0; 
  unsigned char actChar = 0;
          
    actChar = uart0_getc();

  if (pos < 16 ) 
  {
    rx_buffer[pos] = actChar;
    pos++;
  }

  if ( actChar == '\r' ) 
  {
    switch (rx_buffer[0]) 
    {
      case 'T':   
        cmd_T(rx_buffer, pos); 
        rx_buffer[0] = ' ';
      break;
      case 'P': 
        cmd_P(rx_buffer, pos); 
      break;  

      default: put_error(); break; // command illegal
    }
    pos = 0;
  }
}

Es läuft noch nicht 100%,
so wird nicht nach einem Commando ein Enter abgewartet.

Und wenn ein Zeichen im Hypertrm eingegeben wird, bevor mind. einmal 
Enter gedrückt wurde, sendet er ununterbrochen das zeichen bis einmal 
Enter gedrückt wurde.
Ich werde versuchen den Fehler herauszufinden, falls euch etwas 
auffällt, oder ihr noch Verbesserungsvorschläge habt, immer gerne...

bis dann
Gruß

[Falk B. (falk)]

@ µC-noob (Gast)

>Es läuft noch nicht 100%,
>so wird nicht nach einem Commando ein Enter abgewartet.

>Und wenn ein Zeichen im Hypertrm eingegeben wird, bevor mind. einmal
>Enter gedrückt wurde, sendet er ununterbrochen das zeichen bis einmal
>Enter gedrückt wurde.
>Ich werde versuchen den Fehler herauszufinden, falls euch etwas
>auffällt, oder ihr noch Verbesserungsvorschläge habt, immer gerne...

Liest du auch mal die Posts? Deine Funktion uart0_getc() muss angepasst 
werden. Wie sieht die aus?
Der Effekt hängt wahrscheinlich mit dem Rest des Programms zusammen.

MfG
Falk

[Karl H. (kbuchegg)]

µC-noob wrote:
>
>

> void cmd_T(char *rx_buffer, unsigned char pos)
> {
>   if ( rx_buffer[1] == 'R' )
>   {
>     if( rx_buffer[2] == 'P' )
>     {
>       if( rx_buffer[3] == 'I' ) //if command TRPI (Triggerpin)
>       {
>         togglePeriod_slow(100, 4);  //To Test
>       }
>       else if ( rx_buffer[3] == 'E' ) //if command TRPE (Triggerperiod)
>       {
>         togglePeriod_slow(100, 2); //To Test
>       }
>     }
>   }
> }
>

So könnte das aussehen

void cmd_T( char *rx_buffer )
{
  if( strcmp( rx_buffer, "RPI" ) == 0 )
    togglePeriod_slow(100, 4);

  else if( strcmp( rx_buffer, "RPE" ) == 0 )
    togglePeriod_slow(100, 2);

  ....  // nächstes Kommando
}

Dazu musst du aber C-Strings beim Empfang bilden, also
welche die mit einem '\0' Character abgeschlossen sind.

http://www.mikrocontroller.net//articles/FAQ#Wie_funktioniert_String-Verarbeitung_in_C.3F

> Es läuft noch nicht 100%,

Wundert mich nicht. Um den Fall, dass uart0_getc kein Zeichen
empfangen hat, hast du dich immer noch nicht gekümmert.

Was ist denn da dran so schwer (Ich rate jetzt einfach mal
wie deine uart0_getc aussehen könnte)

uint8_t uart0_getc(void)
{
    if (!(UCSRA & (1<<RXC)))   // wenn kein Zeichen verfügbar
       return 0;               // ... dann eine 0 zurück
    return UDR;                // ... ansonsten das empfangene Zeichen
}

Anstatt dem üblichen while ein if und wenn dessen Bedingung wahr ist
und daher kein Zeichen vorliegt, gehts gleich mit 0 zum Aufrufer
zurück. 0 deshalb, da du anscheinend Textbasierte Kommunikation
machst, und kein druckbares ASCII Zeichen den ASCII Code 0 hat.

Beim Aufrufer

uint8_t receiveString( char *rx_buffer, uint8_t bufferSize, uint8_t *pos )
{
  char actChar = uart0_getc();
  if( actChar == 0 )
    return 0;

  if( actChar == '\r' || *pos == bufferSize - 1 ) {
    rx_buffer[ *pos ] = '\0';   // String abschliessen
    *pos = 0;
    return 1;
  }

  rx_buffer[*pos] = actChar;
  *pos++;
  return 0;
}

und das ganze dann in einem Programm eingebaut

char Buffer[40];
uint8_t BufferCnt;

....

int main()
{
   .....


   while( 1 ) {

     if( receiveString( Buffer, sizeof( Buffer ), &BufferCnt ) )
       cmd_T( Buffer, sizeof( Buffer ) );
   }
}

Siehst du wie sich jede Funktion nur um den Mist kümmert, für den
sie gemacht wurde? Die receive_string kümmert sich nur darum
eine Zeile von der UART zu lesen. Dafür kümmert sich cmd_T nur
darum diesen String auszuwerten.

Warnung: ungetesteter Code! Das Prinzip stimmt aber da mag noch der
eine oder andere Syntax Error drinn sein.

[µC-noob (Gast)]

Danke

Hat auf Anhieb funktioniert.

Erstmal, natürlich lese ich die Posts, ich wär ja blöd wenn ichs nicht 
täte bei der tollen Hilfe!

Folgendermaßen sieht der Code jetzt aus.
Ein paar wenige Punkte, die so nicht funktionierten, habe ich 
auskommentiert.

#include "string.h"


char Buffer[40];
unsigned char BufferCnt;


void cmd_T( char *rx_buffer )
{
  if( strcmp( rx_buffer, "TRPIN" ) == 0 )
    togglePeriod_slow(100, 4);

  else if( strcmp( rx_buffer, "TRPER" ) == 0 )
    togglePeriod_slow(100, 2);

  else if( strcmp( rx_buffer, "TRPER" ) == 0 )
    togglePeriod_slow(100, 2);   

  // nächstes Kommando
}


unsigned char receiveString( char *rx_buffer, unsigned char bufferSize ) //, unsigned char *pos )
{
  static unsigned char pos = 0;
  char actChar = uart0_getc();

  uart0_putc(actChar);

  if( actChar == 0 )
    return 0;

  if( actChar == '\r' || pos == bufferSize - 1 ) 
  {
    rx_buffer[ pos ] = '\0';   // String abschliessen
    pos = 0;
    return 1;
  }

  rx_buffer[pos] = actChar;
  pos++;
  return 0;
}


void task_uartCommand(void)
{
     if( receiveString( Buffer, sizeof(Buffer)) ) //, &BufferCnt ) )
       cmd_T( Buffer ); //, sizeof( Buffer ) );
}

Hier dann noch die getc() (Anm.: Code von Peter Fleury)

unsigned int uart0_getc(void)
{    
    unsigned char tmptail;
    unsigned char data;


    if ( UART0_RxHead == UART0_RxTail )
    {
        return 0; //Davor return 0x0100, angepasst an: kein return (Fehler?)
    }
    
   
    // calculate /store buffer index
    
    tmptail = (UART0_RxTail + 1) & UART0_RX_BUFFER_MASK;
    UART0_RxTail = tmptail; 
    

    // get data from receive buffer
    
    data = UART0_RxBuf[tmptail];
    

    return (UART0_LastRxError << 8) + data;

}

Jetzt noch eine abschließende Frage,
Leide muss ich nicht immer Text verarbeiten. Nicht falsch verstehen über 
den UART empfange ich immer ASCII - Zeichen.

Aber es kann auch vorkommen dass eine Zahl (0 <= Zahl <=65535) 
eingetippt wird
z.B
Hypertrm: '1'
oder Hypertrm: '1233'

Diese müssen dann in ein int gewandelt und verarbeitet werden.

Ich wollte es folgendermaßen machen:

void command_Integer(unsigned char commandValue, char *rx_buffer)
{
  static unsigned char pin = 0;
  static unsigned int period = 0;

  if ( commandValue == TRIGGER_PIN )
  {
    pin = atoi(rx_buffer);
  }
  else if ( commandValue == TRIGGER_PER )
  {
      period = atoi(rx_buffer);
  }

  else if ( commandValue == TRIGGER_START )
  {
    togglePeriod_slow(period, pin);
  }
    
}

void cmd_T( char *rx_buffer )
{
    static unsigned char command = 0;

  if( strcmp( rx_buffer, "TRPIN" ) == 0 )
    command = TRIGGER_PIN;// togglePeriod_slow(100, 4);

  else if( strcmp( rx_buffer, "TRPER" ) == 0 )
    command = TRIGGER_PER; //togglePeriod_slow(100, 2);

  else if( strcmp( rx_buffer, "TRSTART" ) == 0 )
    command = TRIGGER_START; //togglePeriod_slow(100, 2);   


  command_Integer(command, rx_buffer);


  // nächstes Kommando
}

Und es Funktioniert auch.
Aber:
Geht es irgendwie hier noch zu prüfen, ob nicht zwischen der zahl 
schrott gesendet wurde.
Wird erkannt:
'100'
Wird nicht erkannt:
'1trz4'

Aber bevor ich wieder nur unnötig kompliziertes Zeug mache, wollt ich 
nach eurer Meinung Fragen, wie ichs am klügsten anstellen kann??

Danke nochmal,

Gruß

[Karl H. (kbuchegg)]

µC-noob wrote:

> Geht es irgendwie hier noch zu prüfen, ob nicht zwischen der zahl
> schrott gesendet wurde.
> Wird erkannt:
> '100'
> Wird nicht erkannt:
> '1trz4'

atoi ist hier die falsche Funktion.
Im Grunde gibt es 3 Möglichkeiten:

* die Konversion selbst schreiben. Dann kann man klarerweise
  die entsprechenden Tests einbauen

* Eine Prüffunktion schreiben, die den String abklappert, ob
  auch nur Ziffern und eventuell noch '+' und '-' darin
  vorkommen. Diese Funktion stellt nur fest, ob der angegebene
  String eine Zahl sein kann. Nicht mehr. Nennen wir sie mal

  uint8_t IsZahl( const char* String )
  {
    // gibt es ein Vorzeichen?
    if( *String == '+' || *String == '-' )
      String++;

    // ein leerer String wird nicht als Zahl akzeptiert
    if( *String == '\0' )
      return 0;

    // ein String ist dann eine Zahl, wenn er nur aus
    // Digits, also den Buchstaben '0' bis '9' besteht
    while( *String ) {
      if( !isdigit( *String ) )  // Oha. Kein Digit -> keine Zahl
        return 0;
      String++;
    }

    // alles in Ordnung
    return 1;
  }

* eine andere Konvertier - Funktion nehmen.
  Was müsste das Ziel sein?
  Das Ziel müsste sein, nach der Konversion nachsehen, ob
  auch alle Zeichen des Strings konsumiert wurden. Wenn
  dein String also zb. so aussieht "123" und nach der
  Konversion hast du festgestellt, dass auch 3 Zeichen
  konsumiert wurden, dann waren offensichtlich keine
  Schrottzeichen im String enthalten.

  Mit diesenm Hintergedanken, lies dir mal diese Site durch:
  http://www2.informatik.uni-halle.de/lehre/c/c_str2n.html

[Fred (Gast)]

Oder du sparst dir den ganzen Stress und benutzt sscanf()...:]

Ein Kommando-Interpreter sollte folgendes machen:

- nach dem ersten Leerzeichen suchen
- nachschauen ob die Zeichen vor dem Leerzeichen ein Kommando darstellen
- Kommandofunktion aufrufen und den Rest des empfangenen Strings (nach 
dem Kommando

[Fred (Gast)]

Oder du sparst dir den ganzen Stress und benutzt sscanf()...:]

Ein Kommando-Interpreter sollte folgendes machen:

- nach dem ersten Leerzeichen suchen
- nachschauen ob die Zeichen vor dem Leerzeichen ein Kommando darstellen
- Kommandofunktion aufrufen und den Rest des empfangenen Strings (nach
  dem Kommando) der sscanf vorwerfen

Funktioniert so

iReturn = sscanf(sParastring, " %u %d", &uPara1, &iPara2);

Wenn dein Kommando 2 Parameter erwartet (einen unsigned int udn einen 
int) prüfst du ob iReturn = 2 ist und wertest uPara1 und iPara2 aus. 
Ansonsten gibst du einen Fehler aus.

[Karl H. (kbuchegg)]

Fred wrote:
> Oder du sparst dir den ganzen Stress und benutzt sscanf()...:]

Ist sicherlich eine Möglichkeit.
Auf der anderen Seite sind sscanf  scanf  printf / sprintf
nicht gerade Resourcenschoner. Auf einem Mega128 spielt das
keine grosse Rolle. Auf einem Mega8 kann das aber den Unterschied
machen zwischen: Programm passt in den Speicher und Programm
passt nicht in den Speicher.

Was mit sscanf auch nicht so schön ist. Die Ausgabe von
qualifizierten Fehlermeldungen, ala: 'Parameter 2 invalid'
ist schwierig. Muss kein Problem sein, kann es aber manchmal
sein.

[Fred (Gast)]

Resourcenfresser: Genau deiner Meinung...

Fehlermeldungen: Ich mach es in der Regel so, daß ich ein "Syntax: xy 
<0..5> <-255..255>" ausgebe. Der Benutzer muss sich selbst überlegen wo 
er Mist gebaut hat.

[µC-noob (Gast)]

Ok, danke erstmal,

bei mir haperts noch an einer Stelle:

digitValue = (unsigned int)strtoul(rx_buffer, &controlChar, 5);

Aber was für einen Wert bekommt control Char wenn alles stimmt,
hab mirs übers LCD anzeigen lassen und dort war ein Schwarzer balken zu 
sehen, also ASCII 0xDB

Dann wollt ich das so machen:

digitValue = (unsigned int)strtoul("546", &controlChar, 5);

for (i = 0; i < 5; i++)
{
 if(controlChar[i] != 0xDB)
   digitError = 1;
}

Aber so läuft gar nix mehr... sollte doch eigentlich, da ich ja nur 
zahlen im String stehen hab??

Gruß

[Karl H. (kbuchegg)]

#include <stdio.h>
#include <stdlib.h>

int main()
{
  char Text[] = "123";
  char* End;

  unsigned long Zahl = strtoul( Text, &End, 10 );

  if( *End == '\0' )
    printf( "OK %u\n", Zahl );

  else if( End == Text )
    printf( "Keine Zahl vorhanden\n" );

  else
    printf( "Zahl vorhanden %u, aber da kommt noch was im String\n", Zahl );

}

Das 2-te Arugument zu strtoul ist die Adresse eines Character
Poitners. Dort hinterlässt strtoul, wo es aufgehört hat die
Zahl zu lesen. Geschah dies am Ende des Strings dann sind
da keine Zeichen mehr vorhanden. Geschieht dies irgendwo
im String, dann kommen noch der Zahl noch Zeichen, die nicht
zur Zahl gehört haben. Hat strtoul überhaupt nichts gelesen,
dann ist der Ende Pointer ganz am Anfang des Strings
(ist auch logisch, strtoul hat ja sofort aufgehört zu lesen) und
es war überhaupt nichts im String was eine Zahl sein könnte.

[µC-noob (Gast)]

Gaaaanz großen Dank...

Gruß
Joe