Hallo, ich bin gerade dabei mich in AVR-GCC einzuarbeiten (Atmel
Studio).
Ich bin dabei einen String über UART einzulesen(Volatile Variable) und
diesen dann zu verarbeiten. Um die verschieden Befehle zu unterscheiden
nutze ich "strcmp".
Der Compiler bringt mir anschließend folgende Meldung:
"Warnung 11 passing argument 1 of 'strcmp' discards 'volatile'
qualifier from pointer target type [enabled by default]"
gibt es eine bessere Möglichkeit den String auszuwerten?
AVR-Anfänger schrieb im Beitrag #4055258:
> gibt es eine bessere Möglichkeit den String auszuwerten?
Nein.
An dieser Stelle ist es höchst wahrscheinlich ok, wenn du das volatile
wegcastest.
AVR-Anfänger schrieb im Beitrag #4055258:
> "Warnung 11 passing argument 1 of 'strcmp' discards 'volatile'> qualifier from pointer target type [enabled by default]"
Der Compiler will dir damit sagen, dass strcmp() nicht berücksichtigt,
dass der String volatile ist und sich während des Vergleichs ändern
könnte.
Die Warnung bekommst Du weg, indem Du den String in ein (char *)
castest, also:
Ja, den Sinn der Meldung habe ich verstanden. Könnte durch das casten
Probleme auftreten?
Am liebsten wäre mir die Switch Case Anweisung, aber leider fkt diese
nur mit Integern :/
AVR-Anfänger schrieb im Beitrag #4055275:
> Ja, den Sinn der Meldung habe ich verstanden. Könnte durch das casten> Probleme auftreten?
Erstmal nicht. Aber wenn sich der String wirklich während des Vergleichs
ändern könnte (z.B. wegen ISR), dann ist das suboptimal. Besser wäre es
dann, den entsprechenden Interrupt vor dem Vergleich ab- und nachher
wieder einzuschalten.
AVR-Anfänger schrieb:> Ja, den Sinn der Meldung habe ich verstanden. Könnte durch das casten> Probleme auftreten?
Theoretisch: ja
FAQ: Was hat es mit volatile auf sich
Praktisch ist es unwahrscheinlich, weil derartige Programme meist
umfangreich genug sind, dass der Optimizer nicht zuschlagen wird.
Man kann sich natürlich auch sein eigenes strcmp schreiben, welches das
volatile in der Argumentliste hat.
Frank M. schrieb:> AVR-Anfänger schrieb:>> Ja, den Sinn der Meldung habe ich verstanden. Könnte durch das casten>> Probleme auftreten?>> Erstmal nicht. Aber wenn sich der String wirklich während des Vergleichs> ändern könnte
Dabei gehts bei volatile gar nicht.
FAQ: Was hat es mit volatile auf sich
Das sich der String während der Arbeit mit ihm ändern könnte, ist eine
davon losgelöste Problematik und wird von volatile nicht abgedeckt oder
angesprochen.
Karl Heinz schrieb:> Man kann sich natürlich auch sein eigenes strcmp schreiben, welches das> volatile in der Argumentliste hat.
Wahrscheinlich hat er so etwas ähnliches wie einen Ringbuffer, auf den
er direkt den strcmp() loslassen will. Das halte ich für nicht korrekt.
Besser wäre es, den String aus dem Ringbuffer zeichenweise zu
extrahieren und auf das Resultat den strcmp() anzuwenden.
Aber das ist alles Spekulation. Besser wäre es, wenn der TO seinen
Source (wenigstens auszugsweise) hier vorstellt.
AVR-Anfänger schrieb:> hier ein auszug aus meinem Quellcode:
In dem kommt nur ein einziges volatile vor. Und genau dieses brauchst du
nicht.
Lies dir das mal durch
FAQ: Was hat es mit volatile auf sich
dann verstehst du auch, welche volatile du brauchst und welche nicht.
Wenn Du uart_string nur in einer Funktion nutzt, die ausschließlich von
der ISR() aufgerufen wird, ist das volatile komplett überflüssig. Also
streiche es.
>> Wenn Du uart_string nur in einer Funktion nutzt, die ausschließlich von> der ISR() aufgerufen wird, ist das volatile komplett überflüssig. Also> streiche es.
Einspruch.
Die Auswertung sollte dort überhaupt nicht sein.
Die gehört da so nicht rein. Alles was nicht mehr wirklich trivial ist,
hat in einer ISR nichts mehr verloren.
Karl Heinz schrieb:>> Wenn Du uart_string nur in einer Funktion nutzt, die ausschließlich von>> der ISR() aufgerufen wird, ist das volatile komplett überflüssig. Also>> streiche es.>> Einspruch.> Die Auswertung sollte dort überhaupt nicht sein.
Da hast Du natürlich recht. Eine ISR sollte sich schnellstmöglich
beenden. Ich wollte mit dem obigen Satz nur das Prinzip verdeutlichen
und die Betonung dabei auf das Wort "ausschließlich" legen. Ich habe von
"Nutzung" gesprochen und habe keine Wertung über seine Art der Nutzung
getroffen ;-)
> Die gehört da so nicht rein. Alles was nicht mehr wirklich trivial ist,> hat in einer ISR nichts mehr verloren.
Korrekt.
Der Source hat noch ganz andere Schwächen - abgesehen vom
String-Parsen in einer Funktion, die von einer ISR aufgerufen wird:
Ab dem zweiten "if (strcmp ...)" sollte der TO ein "else"
davorschreiben, damit der arme µC nicht weiter jeden möglichen String
checkt, obwohl er ihn längst gefunden hat.
Die uart_puts()-Aufrufe sind ein absolutes NoGo, wenn man sich noch im
Interrupt befindet.
@TO: Du solltest den String in der ISR lediglich speichern, aber besser
im Hauptprogramm auswerten. Dann braucht man natürlich wieder einen
volatile Buffer für den String.
Frank M. schrieb:> Ab dem zweiten "if (strcmp ...)" sollte der TO ein "else"> davorschreiben, damit der arme µC nicht weiter jeden möglichen String> checkt, obwohl er ihn längst gefunden hat.>> Die uart_puts()-Aufrufe sind ein absolutes NoGo, wenn man sich noch im> Interrupt befindet.
Nicht nur das.
Das ganze Prinzip, zuerst einen einzelnen String zusammen zu setzen, um
ihn dann in einem Rutsch auszugeben, ist nichts anderes als
Fleissaufgabe für den µC. Es gibt keinen Grund dafür. Der Empfänger kann
nicht unterscheiden, ob der String vom Sender in einem Rutsch
'aufgegeben' wurde, oder ob da mehrere Sende Funktionen beteiligt waren.
* Aufbauend auf uart_puts sollte er sich erst mal eine Funktion
uart_puti schreiben, damit er in der Verwendung nicht dauernd die itoa
hat
1
voiduart_puti(intwert)
2
{
3
chartxt[8];
4
itoa(wert,txt,10);
5
uart_puts(txt);
6
}
* dann sollte er die eigentliche Aufgabe aufteilen. Anstatt des Monsters
und bingo: kein Mensch braucht da noch einen strcat, was erstes Zeit
spart und zweitens die Gefahr eines Buffer Overruns eliminiert.
Ob get_color wirklich einen String braucht um die Farbe anzugeben, ist
auch fraglich. Aber sagen wir mal, das stimmt so. Dann so
1
voiduart1_auswerten(void)
2
{
3
charuart_temp[UART_MAXSTRLEN+1];
4
intColorValue;
5
6
strcpy(uart_temp,(char*)uart_string);// Reihenfolge! zuerst den String in Sicherheit bringen
7
uart_str_complete=0;// dann die UART wieder freigeben
8
9
uart_puts(uart_temp);
10
uart_puts(" : ");
11
12
ColorValue=get_color(uart_temp);// soll -1 bei einer unbekannten Farbe liefern!
13
if(ColorValue==-1)
14
uart_puts(" Unbekannt");
15
else
16
uart_puti(ColorValue);
17
18
uart_putc(CR);
19
}
Der/Die einzige(n) strcmp, die jetzt noch vorkommen, stecken in
get_color
Karl Heinz schrieb:> uint8_t strvcmp( volatile const char* str1, volatile const char* str2 )> {> while( *str1++ == *str2++ )> ;>> return *str1 - *str2;> }
Perfekt, der rückgabewert kann hier nie 0 sein, und nen Bufferoverflow
hat man meistens auch noch, Genial!
Besser noch die Nullterminierung prüfen und den letzten Index
korrigieren, dann stimmts wieder:
Daniel A. schrieb:> Besser noch die Nullterminierung prüfen und den letzten Index> korrigieren, dann stimmts wieder:
Danke. Schuldig in allen Punkten.
Man sollte nicht vor dem ersten Kaffee Codeschnipsel aus dem Ärmel
schütteln.
uint8_t passt auch nicht mit dem Return-Wert zusammen.
Beide Argumente als volatile zu definieren ist auch ziemlich
kontraproduktiv.
Alles in allem: Streichen :-)
ich habe jetzt alles umgesetzt und es funktioniert alles.
Ich habe in der uart_puti() Funktion die ITOA Funktion durch die sprintf
Funktion ersetzt, dadurch kann ich die Zahlenwerte immer im gleichen
Format mit vorgestellten Nullen ausgeben. Ist das so konform?
AVR-Anfänger schrieb im Beitrag #4055567:
> ich habe jetzt alles umgesetzt und es funktioniert alles.
Vielleicht postest Du nochmal das Ergebnis? Denn eines sollte man sich
merken: Programmcode wird eigentlich nie fertig :-)
> Ich habe in der uart_puti() Funktion die ITOA Funktion durch die sprintf> Funktion ersetzt, dadurch kann ich die Zahlenwerte immer im gleichen> Format mit vorgestellten Nullen ausgeben. Ist das so konform?
sprintf() benötigt bedeutend Flash-Speicher als itoa(). Wenn Du aber
genügend davon zur Verfügung hast, ist das vollkommen in Ordnung.
Frank M. schrieb:> uint8_t passt auch nicht mit dem Return-Wert zusammen.
Wenn man nicht dieselbe Semantik wie strcmp braucht sondern nur auf 0
oder nicht-0 geht, dann passt das schon.
> Beide Argumente als volatile zu definieren ist auch ziemlich> kontraproduktiv.
Ähm. Das volatile war genau der Grund, warum eine eigene Funktion
vorgschalgen wird. Damit man nicht beim Aufruf von strcmp dauernd das
volatile wegcasten muss.
brauchst du keinen strcpy.
strtok modifiziert dein uart_temp und liefert dir Pointer auf den
jeweiligen String Anfang.
1
char*part1;
2
char*part2;
3
4
part1=strtok(uart_temp,":");
5
part2=strtok(NULL,";");
die kannst du dann ganz normal weiter benutzen.
Ich bin zwar unter den ersten, die normalerweise sagen: überlass das
Optimieren dem Compiler. Aber wenn du so wie hier
atol mehrmals brauchst, lohnt es sich dafür Zwischenvariablen
abzustellen, in denen du die Ergebnisse der Konvertierung ablegst.
Man muss sein Glück ja auch nicht mutwillig herausfordern.
> brauchst du keinen strcpy.>> strtok modifiziert dein uart_temp und liefert dir Pointer auf den> jeweiligen String Anfang.>>
1
>char*part1;
2
>char*part2;
3
>
4
>part1=strtok(uart_temp,":");
5
>part2=strtok(NULL,";");
6
>
>> die kannst du dann ganz normal weiter benutzen.
Und du solltest natürlich auch prüfen, ob das alles geklappt hat!
Nicht auf NULL Pointer zu prüfen, ist der Fehler, der für die
überwiegende Mehrheit aller Fehler verantwortlich ist!
1
char*part1;
2
char*part2;
3
4
part1=strtok(uart_temp,":");
5
part2=strtok(NULL,";");
6
7
if(part1!=NULL&&part2!=NULL){
8
....
9
10
}
11
else
12
uart_puts("Syntaxfehler!\r\n");
Und das mit dem zusätzlichen Argument true/false bei den
Ausgabefunktionen würde ich mir nochmal überlegen. Das lohnt
normalerweise nicht.
Das hier
1
uart_puts("RED:",false);
2
uart_puti(get_color("RED"),false);
3
uart_puts("GREEN:",false);
4
uart_puti(get_color("GREEN"),false);
5
uart_puts("BLUE:",false);
6
uart_puti(get_color("Blue"),true);
schreibt sich länger als das hier
1
uart_puts("RED:");
2
uart_puti(get_color("RED"));
3
uart_puts("GREEN:");
4
uart_puti(get_color("GREEN"));
5
uart_puts("BLUE:");
6
uart_puti(get_color("Blue"));
7
uart_puts("\r\n");
wenn du dir was gutes tun willst, dann überleg lieber ob es nicht
vernünftig wäre, ein paar Leerzeichen einzustreuen. Seit du 6 Jahre alt
bist, hast du dein Gehirn darauf trainiert, dass zwischen Wörtern ein
Leerraum steht. Du machst das mitlerweile ganz unbewusst und musst keine
aktive Identifikationsarbeit der einzelnen Wörter beim Lesen mehr
aufbringen. In diesem Sinne ist zb
1
[c]
2
uart_puts("RED:");
3
uart_puti(get_color("RED"));
4
uart_puts("GREEN:");
5
uart_puti(get_color("GREEN"));
6
uart_puts("BLUE:");
7
uart_puti(get_color("Blue"));
8
uart_puts("\r\n");
leichter zu lesen, weil du keine bewusste Gehirnaktivität dafür
aufbringen musst, in dem Buchstabenwust erst mal die einzelnen Wörter zu
finden.
So was hier
1
Interrupt_config_TCS34725(true,0x01,0x00,0x00,(atoi(tStr_2)>>8),((uint8_t)atoi(tStr_2)));//aufspalten des dec wertes in hex werte!
geht gar nicht. Um wieviel einfacher ist es
1
ConfigWert=atoi(tStr_2);
2
3
Interrupt_config_TCS34725(true,
4
0x01,0x00,0x00,
5
ConfigWert>>8,ConfigWert
6
);
zu lesen, wobei ich mich frage, warum sich die Funktion die Zerlegung in
die beiden Bytes nicht selbst macht.
so, wurde nun auch umgesetzt:)...
ich bekomme vom Compiler noch die Warnung das die Funktion sprintf
implicit deklariert wurde, aber woran soll das denn liegen?
AVR-Anfänger schrieb im Beitrag #4055627:
> so, wurde nun auch umgesetzt:)...>> ich bekomme vom Compiler noch die Warnung das die Funktion sprintf> implicit deklariert wurde, aber woran soll das denn liegen?
stdio.h includieren
AVR-Anfänger schrieb im Beitrag #4055640:
> hmm peinlich.. wieso kann die Funktion dann trotzdem benutzt> werden,> obwohl sie noch nicht eingebunden ist?
Sie ist eingebunden auch ohne das Inkludieren. Du machst dem Kompiler
mit dem Inkludieren die Funktion bekannt. Wenn der Kompiler die Funktion
nicht kennt, geht er davon aus, dass sie doch existiert und gibt eine
Warnung aus. Das Einbinden übernimmt der Linker.
AVR-Anfänger schrieb im Beitrag #4055640:
> hmm peinlich.. wieso kann die Funktion dann trotzdem benutzt werden,> obwohl sie noch nicht eingebunden ist?
Die Funktion existiert ja.
Das einzige was der Compiler nicht weiss, das ist wie die
Funktionsargumente aussehen. Und daher warnt er dich, dass in diesem
Fall dann Standardannahmen in Kraft treten. Die können richtig sein oder
auch nicht. In deinem Fall sind sie richtig. Das muss aber nicht immer
so sein, und dann hat man seltsame Fehler. Daher die Warnung.
Die Funktion selber steht ja nicht im Header File. Im Header File steht
ja nur der Funktionsprotoyp. Du selbst hast ja in deinem Header File
auch nichts anderes gemacht: Da steht die Information drinnen, welche
Funktionen es gibt, wieviele Argumente jede Funktion hat und welche
Datentypen gelten sowie der Return Datentyp der Funktion. Das reicht,
damit man beim Aufruf der Funktion prüfen kann, ob alles in Ordnung ist
und ob eventuelle Konvertierungen gemacht werden müssen.
WEnn du aber keinen Prototypen hast, dann musst du dem Programmierer
vertrauen, dass er seinen Aufruf korrekt gemacht hat.
Mit der Existenz der Funktion hat das erst mal nichts zu tun. Zu diesem
Zeitpunkt in der Übersetzung geht es nur um die Prüfung, ob der Aufruf
der Funktion technisch korrekt ist und ob Konvertierungen notwendig
sind.