Hi zusammen! Ich möchte eine UART-Kommunikation mit einem Handy aufbauen. Ich empfange die Handydaten per UartInterrupt und brüte seit vier Tagen über einen wahrscheinlich total dummen Fehler... Ihr kennt es sicher, je länger man grübelt, desto blinder wird man... Ich habe mein Code mal auf ein übersichtliches Maß reduziert und im angehängten File direkt das Problem beschrieben... Würd mich sehr über nen Schupps in die richtige Richtung freuen... Danke und Gruß Danny
Du gehst mir für meine Begriffe etwas zu sorglos mit deinem uart read Index um. Als Ausgleich dafür hast du viel zu viele fixe Annahmen über Stringlängen drinnen. Ganz zum Schluss, die do-while Schleife sollte eine while Schleife sein, sonst passiert es dir, dass du eine Message komplett überliest. Teile die Dinge auf: Erst kopierst du dir eine komplette(!) Message aus dem UART Buffer in eine String Variable um. Also ALLES vom Beginn der Message bis zum Ende. Musst du sowieso, wenn dein Ringbuffer erst einmal zum Ring geschlossen wurde. Und erst dann zerlegst du diese Message in ihre Einzelteile. Keine Annahmen über Argumentlängen. Sowas ist immer schlecht. Zb zeigt dein Beispiel die 2-te Zahl 2 stellig, dein Code wertet sie aber 3-stellig aus. Zerlege die Message anhand der dir bekannten Trennzeichen in Einzelteile und werte die dann entsprechend aus. Um einen Zahlen-String in eine Zahl umzuformen kannst du zb atoi nehmen oder strtol und bist so von Zeichenzahlen unabhängig. Auf lange Sicht bringt dieses Message-Zerlegen auf Einzelzeichenbasis nix ausser Ärger. --- Code wird nicht übersichtlicher, wenn jede 2.te Zeile eine Leerzeile ist. Er wird nur in die Länge gezogen. Ditto für viele Leerzeilen hintereinander. 1 reicht vollkommen um anzuzeigen, dass ein neuer Gedanke, ein neuer Codeblock beginnt.
In
volatile char kommando[5];
kannst du keinen String mit strlen(String)==5 unterbringen ("+CBC:").
Für einen String mit diesen Zeichen brauchst du ein Array der Länge 6,
weil ja hinten noch das \0 Zeichen drann kommt.
Selbiges hier
char kapaz[3];
kapaz[0] = uart_rx_buffer[uart_rx_read++];
kapaz[1] = uart_rx_buffer[uart_rx_read++];
kapaz[2] = uart_rx_buffer[uart_rx_read++];
kapaz[3] = '\0';
ein kapaz[3] existiert schlicht und ergreifend nicht. Ein Array der
Größe 3 hat die Elemente 0, 1, 2. Zähl nach, sind genau 3 Stück. Einen
Index 3 hingegen kann es dann nicht mehr geben. Dazu müsste das Array
die Länge 4 haben.
nun bin ich verblüfft :) Vielen Dank Karl Heinz für deine schnelle Hilfe zu dieser Uhrzeit! Ich wollte die komplette Nachricht im FIFO belassen und von dort aus zerlegen, weil (ja zugegeben nur) im Normalfall nur das vom Handy gesendet wird was ich vorher definitiv anfordere. Somit habe ich die Kontrolle darüber. Die längste Nachricht kann 200 Zeichen lang sein und somit müsste ich den FIFO und zusätzlich 200 Byte RAM zur Verfügung stellen, das wollte ich mir sparen... Sollte ich wirklich "nur" auf den dummen Stringlänge + \0 reingefallen sein? Das werd ich sofort testen... also nur testen ob der Fehler dann weg ist, denn Recht hast du und das war schonmal ein grosser Baum der den Blick auf den Wald verdeckt hat :)
mh, schade... an den "zu kurzen" Arrays lags nicht... noch ein Satz zu der zweistellig dargestellten, aber dreistellig ausgewerteten Zahl. Das ist nötig weil dort auch eine 100 stehen kann. Die von mir gemachte Wandlung mit atoi() hat in diesem Fall fehlerfrei funktioniert, daher habe ich es so gelassen... dann werde ich nun Deinen Rat befolgen und komplette Nachrichten laden, ich schätze ich melde mich morgen früh nochmal :) erst einmal vielen Dank und eine angenehme Nacht
Danny schrieb: > mh, schade... an den "zu kurzen" Arrays lags nicht... nimm noch die fehlerhafte do-while Schleife mit dazu. IMHO läuft dir da der read Index zu weit. Tip: do-while Schleifen sind in C relativ selten. Die überwiegende Mehrheit sind normale while Schleifen. Deine letzte sollte nicht so lauten
1 | do{ |
2 | uart_rx_read++; |
3 | |
4 | }while(uart_rx_buffer[uart_rx_read] != '\0'); |
5 | |
6 | uart_rx_read++; |
sondern so
1 | // bis zum \0 vortasten
|
2 | while(uart_rx_buffer[uart_rx_read] != '\0') { |
3 | uart_rx_read++; |
4 | }
|
5 | // und über den \0 drüber
|
6 | uart_rx_read++; |
Überleg dir einfach, was der Unterschied ist, wenn der Code an diese Stelle kommt, und uart_rx_buffer[uart_rx_read] bereits ein \0 Zeichen ist (weil du alle Zeichen die zur Message gehören schon sauber aufgearbeitet hast). Deine restlichen uart_rx_read Erhöhungen im Code sind alle nicht wirklich nachvollziehbar. Ich kann beim besten Willen nicht sagen ob das alles koscher ist. (Unter anderem auch durch den unübersichtlichen Stil). Ich denke aber, du hast dich da kräftig verfranst. Nimm dir einen Zettel und spiel (du als Computer) das alles mal durch. ZU dieser Stunde mach ich auch schon Fehler, aber wenn ich das im Kopf durchspiele (mit deiner doumentierten Eingabe) dann greifst du da auf völlig falsche Bytes zu.
fall Du diese Erhöhung mit evt nicht koscher meinst:
// und über den \0 drüber
uart_rx_read++;
Mein Ziel war: \0 ist erreicht, Schleife bricht ab - und dann muss ich
den "Zeiger" auf den nächsten Beginn / nächste gültige Stelle erhöhen um
ab dort wieder von neuem zu lesen (und das getriggert durch mein
uart_rx_stack)...
Karl Heinz, noch ein zum grübeln :) wenn mein Programm aus einem definitiv im uart_rx_puffer liegenden "+CBC: 0,80" zeitweise sogar ein "+BC: " macht, so glaube ich nicht an überlaufende Zeiger oder "ein zu weit schalten". Dann wäre es evt um eine Stelle verschoben, oder totaler Müll. Aber hier fehlt ja definitiv ein Zeichen, als wäre es rausgelöscht. Kannst Du Dir irgendeine wie auch geartete Interrupt-Konstellation vorstellen die so etwas mit sich bringen kann?
> im Normalfall nur das vom Handy gesendet wird was ich vorher > definitiv anfordere. Somit habe ich die Kontrolle darüber. OK. NIchts desto trotz sind da ein paar Hilfsfunktionen angesagt. So was
1 | if(uart_rx_buffer[++uart_rx_read] == '1'){ |
2 | |
3 | handy.laedt = 1; |
4 | }
|
5 | else{ |
6 | |
7 | handy.laedt = 0; |
8 | }
|
9 | |
10 | uart_rx_read += 2; |
11 | |
12 | kapaz[0] = uart_rx_buffer[uart_rx_read++]; |
13 | kapaz[1] = uart_rx_buffer[uart_rx_read++]; |
14 | kapaz[2] = uart_rx_buffer[uart_rx_read++]; |
15 | kapaz[3] = '\0'; |
ist ausser fehleranfällig nur noch fehleranfällig. Der Code könnte zb so aussehen:
1 | char kommando[6]; |
2 | |
3 | for( i = 0; i < 5 && uart_rx_buffer[uart_rx_read] != '\0'; ++i ) |
4 | kommando[i] = uart_rx_buffer[uart_rx_read++]; |
5 | |
6 | kommando[i] = '\0'; |
7 | |
8 | strcat(uart_tx_buffer, kommando); |
9 | |
10 | // an dieser Stelle
|
11 | // uart_rx_read ist der Index des Zeichens nach dem Kommando
|
12 | // oder "zeigt" auf das abschliessende \0
|
13 | |
14 | if( strcmp( kommando, "+CBC:" ) ) { |
15 | // den ' ' überlesen, falls er da ist
|
16 | if( uart_rx_buffer[uart_rx_read] == ' ' ) |
17 | uart_rx_read++; |
18 | |
19 | lade = read_int(); |
20 | |
21 | // den ',' überlesen, falls er da ist
|
22 | if( uart_rx_buffer[uart_rx_read] == ',' ) |
23 | uart_rx_read++; |
24 | handy_kapazitaet = read_int(); |
25 | |
26 | // an dieser Stelle ist das Kommando zu Ende. Wenn alles
|
27 | // richtig ist, dann MUSS uart_rx_buffer[uart_rx_read] ein \0
|
28 | // Zeichen sein. Das testen wir besser mal
|
29 | if( uart_rx_buffer[uart_rx_read] == '\0' ) { |
30 | if( lade ) |
31 | handy.laedt = 1; |
32 | else
|
33 | handy.laedt = 0; |
34 | |
35 | if(handy_kapazitaet < 60) { |
36 | laden_ein
|
37 | led_1_ein
|
38 | }
|
39 | else if(handy_kapazitaet > 90) { |
40 | laden_aus
|
41 | led_1_aus
|
42 | }
|
43 | }
|
44 | }
|
45 | |
46 | |
47 | ....
|
48 | // bis zum \0 vortasten
|
49 | while(uart_rx_buffer[uart_rx_read] != '\0') { |
50 | uart_rx_read++; |
51 | }
|
52 | // und über den \0 drüber auf den Anfang des nächsten Kommandos
|
53 | uart_rx_read++; |
Das ist die Zielvorstellung. Damit das so geht, wie muss die Funktion read_int() aussehen? Was muss sie tun? So muss ein Zeichen nach dem anderen vom UART Buffer lesen und zu einer Zahl zusammenfügen. uart_rx_read muss dabei auf den Anfang der Zahl verweisen und es werden so lange Ziffern zur Zahl 'hinzugefügt', solange im Text Ziffern vorkommen. Mit dem ersten Nicht-Ziffern Zeichen ist die Sache vorbei und uart_rx_read soll dann der Index des ersten Zeichens nach der Zahl sein. Steht überhaupt keien Zahl im Text, dann ist das Ergebnis 0 und uart_rx_read verändert seinen Wert überhaupt nicht (damit ein ev. \0 Zeichen nicht übersprungen wird)
1 | int read_int( void ) |
2 | {
|
3 | int result = 0; |
4 | |
5 | while( uart_rx_buffer[uart_rx_read] >= '0' && |
6 | uart_rx_buffer[uart_rx_read] <= '9' ) |
7 | result = 10 * result + (uart_rx_buffer[uart_rx_read++] - '0'); |
8 | |
9 | return result; |
10 | }
|
Edit: Es ist jetzt 01:21 Ich hab im Code noch ein paar Nachbesserungen gemacht, die kritisch sind. Wenn du dir vorher was davon geholt hast, hol ihn dir nochmal.
habe ich eben schon gesehen :) vielen Dank und soll ich Dir was sagen? Da Du mich nun mit der Nase draufgedrückt hast, habe ich nun definitv beide Fehlerstellen gefunden :) Die wars: kapaz[0] = uart_rx_buffer[uart_rx_read++]; kapaz[1] = uart_rx_buffer[uart_rx_read++]; kapaz[2] = uart_rx_buffer[uart_rx_read++]; kapaz[3] = '\0'; Ich habe nur an die Auswertung gedacht, die mit atoi() fehlerfrei war. Ausserdem habe ich gedacht, bei einer zweistelligen Zahl wäre es problemlos, es folgt schließlich ein \0 und damit ist mein Array ja terminiert. Das ich in dem Fall aber den uart_rx_read einmal zu viel inkrementiere hab ich nicht bedacht... Die Stelle habe ich schnell testweise angepasst und freute mich, aber noch ein wenig zu früh, denn genau alle 16 Übertragungen hatte ich reproduzierbar wieder einen Fehler... Und siehe da.... guck mal hier: volatile char uart_rx_buffer[255] =""; volatile uint8_t uart_rx_read =0; volatile uint8_t uart_rx_write =0; volatile uint8_t uart_rx_stack=0; Zeiger gibt 256 "Möglichkeiten", Array bietet aber nur 255... Ich glaub ich kann heut echt glücklich ins Bett. Karl Heinz, vielen Dank für deine Hilfe mitten in der Nacht, ich werde deinen Rat befolgen und mein Programm entsprechend deiner Vorschläge anpassen - aber erst morgen :) Angenehme Nacht! :)
Thread beobachten
Seitenaufteilung abschaltenBitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.