Hallo @ all.
Ich fange gerade an mit C zu programmieren und habe ein paar Fragen zu
folgender Situation :
Ich möchte eine per UART empfangenen String (GCC-Tutorial)
weiterverarbeiten, der bzw. die Strings sind wie folgt aufgebaut :
-Stelle 1 [1-3] => Unterscheidung an welchem Port die Daten ausgegeben
werden sollen
-Stelle 2 bis 4 bzw.5 [0-9] => Wert der auf mehreren 7 Segmentanzeigen
angezeigt werden soll
-Stelle 5 bzw. 6 [:] => 'Endezeichen'
1
/* Zeichen empfangen */
2
uint8_tuart_getc(void)
3
{
4
while(!(UCSRA&(1<<RXC)))// warten bis Zeichen verfuegbar
5
;
6
returnUDR;// Zeichen aus UDR an Aufrufer zurueckgeben
7
}
8
9
voiduart_gets(char*Buffer,uint8_tMaxLen)
10
{
11
uint8_tNextChar;
12
uint8_tStringLen=0;
13
14
NextChar=uart_getc();// Warte auf und empfange das nächste Zeichen
15
16
// Sammle solange Zeichen, bis:
17
// * entweder das String Ende Zeichen kam
18
// * oder das aufnehmende Array voll ist
19
while(NextChar!='\n'&&StringLen<MaxLen-1){
20
*Buffer++=NextChar;
21
StringLen++;
22
NextChar=uart_getc();
23
}
24
25
// Noch ein '\0' anhängen um einen Standard
26
// C-String daraus zu machen
27
*Buffer='\0';
28
}
Ich dachte mir jetzt nach der Zeile '*Buffer='\0' ' zunächst via
1
switch(buffer[1])
2
case('1'):....;
3
break;
4
case('2'):....;
5
break;
6
....
zu unterscheiden für welche Anzeigen die Werte gedacht sind und
scheitere an meinem geplanten weiteren Wunsch dann danach die Stellen 2
bis 4/5 einzulesen und als geeigneten Wert für die Anzeigen so
abzuspeichern das ich sie dann bitweise am entsprechenden Port an das
Schieberegister ausgeben kann, zB 1 = 0x00000011;
Ich krieg es nicht auff die Reihe das in den entsprechenden Variablen zu
speichern und dann auszugeben...
Wäre für jede Hilfestellung dankbar !!!
MfG,
Oliver.
Ich habe mal versucht, das wie folgt zu lösen (=> siehe Anlage).
Wäre sehr nett wenn ihr mich auf Fehler oder Verbesserungen hinweisen
könntet.
Danke im Voraus,
Oliver.
Das wichtigste was du unbedingt beherzigen solltest:
Jede Funktion soll nur das machen, was auch aussen drauf steht!
Die Funktion heisst uart_gets. In langen Worten: hole einen String von
der UART.
Die Funktion heisst nicht uart_gets_and_set_7_seg_Led!
D.h. deine Funktion macht viel zu viel!
Teile dir die Problemkreise auf. uart_gets ist dafür zuständig einen
String über die UART zu empfangen. Nicht mehr aber auch nicht weniger.
Was mit diesem String passiert, ist nicht das Bier von uart_gets. Dafür
ist u.U. eine andere Funktion zuständig, aber auf jeden Fall nicht
uart_gets.
Nur so kriegst du dann einen Baukasten, in dem du Funktionen
wiederverwenden kannst und die du in weitere Projekte übernehmen kannst.
Zu deiner restlichen Auswertung:
Das ist übelste Copy&Paste Programmierung. Such nach gemeinsamen Teilen
und lagere die in eigene Funktionen aus.
zb. Sieht bei dir ausnahmslos jeder case-Fall so aus (ICh hab es jetzt
nur anhand von Stichproben kontrolliert)
1
if(LED_0[y]==0)
2
{
3
PORTB&=~(1<<PB1);
4
}
5
else
6
{
7
PORTB|=(1<<PB1);
8
}
9
PORTB&=~(1<<PB2);
10
PORTB|=(1<<PB2);
11
}
12
PORTB&=~(1<<PB2);
13
PORTB|=(1<<PB2);
das einzige was sich ändert ist LED_0 oder LED_1 oder LED_2 ... da ganz
oben.
D.h. der ganze restliche Teil ist heisser Kandidat für eine Funktion
1
voidSetBit(charstate)
2
{
3
if(state=='0')
4
{
5
PORTB&=~(1<<PB1);
6
}
7
else
8
{
9
PORTB|=(1<<PB1);
10
}
11
PORTB&=~(1<<PB2);
12
PORTB|=(1<<PB2);
13
}
Damit würde deine Auswertung schon abmagern:
1
if(Buffer[0]=="1")//Fallunterscheidung welche 7-Segment Reihe die Daten anzeigen soll
2
{
3
PORTB&=~(1<<PB3);
4
for(x=1;x<6;x++)
5
{
6
switch(Buffer[x])
7
{
8
case0:
9
for(y=0;y<8;y++)
10
SetBit(LED_0[y]);
11
PORTB&=~(1<<PB2);
12
PORTB|=(1<<PB2);
13
break;
14
case1:
15
for(y=0;y<8;y++)
16
SetBit)LED_1[y]);
17
PORTB&=~(1<<PB2);
18
PORTB|=(1<<PB2);
19
break;
20
case2:
21
for(y=0;y<8;y++)
22
SetBit(LED_2[y]);
23
PORTB&=~(1<<PB2);
24
PORTB|=(1<<PB2);
25
break;
26
27
....
Siehst du um wieviel einfacher das schon ist. Aber es geht noch weiter.
Im Grunde sind die ganzen case-Fälle immer noch gleich. Einzig im
LED-Argument unterscheiden sie sich. -> Funktion dafür machen.
1
voidAllBits(charleds[])
2
{
3
unsignedchary;
4
5
for(y=0;y<8;y++)
6
SetBit(leds[y]);
7
PORTB&=~(1<<PB2);
8
PORTB|=(1<<PB2);
9
}
und in der Verwendung erhalten wir dann
1
......
2
3
if(Buffer[0]=="1")//Fallunterscheidung welche 7-Segment Reihe die Daten anzeigen soll
4
{
5
PORTB&=~(1<<PB3);
6
for(x=1;x<6;x++)
7
{
8
switch(Buffer[x])
9
{
10
case0:SetBits(LED_0);break;
11
case1:SetBits(LED_1);break;
12
case2:SetBits(LED_2);break;
13
case3:SetBits(LED_3);break;
14
....
Wow. Nicht schlecht. Der Code verkürzt sich rapide.
Aber noch ist nicht Ende der Fahnenstange. Der ganze Teil rund um die
x-Schleife wird in jedem if( Buffer[0] == '1' ) Teil wiederholt -> ab in
eine einzige Funktion damit.
PS: Buffer[0]=="1"
Das wird zwar vom Compiler akzeptiert, macht aber etwas ganz anderes als
du vermutest. Strings kann man nicht auf diese Art vergleichen. Brauchst
du auch nicht, denn du hast keine Strings. Du hast Character. Charcter
schrieben sich mit ' und nicht mit ". Mit " ist das immer ein String.
Buffer[0] == '0'
Fazit: Wann immer du versucht bist, Code einfach zu duplizieren, überleg
dir ob eine Funktion nicht besser wäre. Letztenendes hast du weniger
Arbeit, und baust weniger Fehler ein, wenn du gleich von vorneherein
Funktionen machst. Dass es mit Copy&Paste in der Entwicklung schneller
geht, ist ein Mythos.
Und der wichtigste und schwerwiegenste Fehler, den du machst: Du benutzt
kein C-Buch um die Sprache zu lernen. Die Methode 'Schaun wir mal und
holen uns dann Tipps von anderen' funktioniert bei Programmiersprachen
nicht. Du brauchst Literatur!
PS2: Ist das richtig so, dass du nach jedem Bit-setzen mittels PB2 einen
Puls generierst und nach allen 8 Bits noch einen Puls nachschiebst?
@Karl heinz:
Danke für deine Hinweise, ist nachvollziehbar und war mir auch schon
bewusst,dass ich da noch verkürzen kann und ich werde das mal
durcharbeiten.
(Und auch noch mal mein altes C-Buch rauskramen,hat schon Staub
angesetzt)
Das Nachschieben des Pulses habe ich aus dem Programmierbeispiel des
7-Segment Herstellers übernommen (siehe Anlage,ist allerdings für einen
PIC*)
MfG,
Oliver.
gonZoX schrieb:
> Das Nachschieben des Pulses habe ich aus dem Programmierbeispiel des> 7-Segment Herstellers übernommen (siehe Anlage,ist allerdings für einen> PIC*)
Schau dir das Codefragment noch einmal genau an!
Da wird kein Puls nachgeschoben. Übrleg einfach mal, wie sich die
Manipulationenen am CLK auswirken und in welchem Zustand die CLK Leitung
jeweils vorher war. Nur eine Änderung von CLK ist interessant.
Am Ende der 'for-Schleife' steht CLK auf 1, wird danach nochmal auf 1
und wieder auf Null gesetzt, also kein eigentlicher Wechsel sondern ein
Rückstellen auf Null und einmal 'CLK=1;' hätte man sich sparen
können,oder ?
MfG,Oliver.
Schau dir die Funktion SetLeds noch mal genauer an. Da ist noch jede
Menge weiterer Vereinfachungsspielraum.
* Gemeinsame Funktionalität auslagern
* was ist die Bedeutung der 'magischen Konstanten' im switch-case.
Warum 48?
Was ist leichter zu verstehen?
switch( Zeichen )
case 48: ...
case 49: ...
oder
switch( Zeichen )
case '0': ...
case '1': ...
In welcher Variante sehe ich besser, was mit dem switch beabsichtigt
wird.
(Hast du schon einmal darüber nachgedacht, die ganzen LED_x in einem 2
dimensionalen Array zusammenzufassen? Dann würde sich nämlich der ganze
switch-case in Luft auflösen)
Es heißt zwar, dass vorzeitige Optimierung die Wurzel allen Übels ist.
Das bedeutet aber nicht, dass man vorsätzlich Laufzeit verbraten soll
....
1
if(Zeile[0]=='1')
2
{
3
..
4
}
5
6
if(Zeile[0]=='2')
7
{
8
..
9
}
10
11
if(Zeile[0]=='3')
12
{
13
..
14
}
Wenn Zeile[0] gleich '1' ist, dann KANN es nicht mehr gleich '2 oder '3'
sein. Dieser Test ist daher völlig überflüssig und kann ausgelassen
werden.
1
if(Zeile[0]=='1')
2
{
3
..
4
}
5
6
elseif(Zeile[0]=='2')
7
{
8
..
9
}
10
11
elseif(Zeile[0]=='3')
12
{
13
..
14
}
Warum läuft eigentlich x einmal bis 6 und einmal bis 5. Und was passiert
eigentlich, wenn der String gar nicht so lange ist (also sagen wir mal
aus 2 Zeichen besteht). Denkste du nicht es wäre besser, eine
universelle Funktion zu schreiben, die mit beliebigen 'Zahlen' klar
kommt?
Danke wieder einmal.
Die switch Anweisung hatte ich in der Weise gestaltet da ja die case
Unterscheidung einen Integer erwartet, aber der Cast von z.B. '1' in 49
findet ja automatisch statt, da hast du recht. ( Nur wie ich zuerst
abgefragt hatte ' case 1 ' funktionierte logischerweise nicht).
Ist es von der Performance her ein Unterschied, ob ich 3x if abfrage
oder via if .., else if, oder geht es nur um den besseren Stil ?
Das mit dem 2 dimensionalen Array muss ich mal versuchen
umzusetzen,danke für den Hinweis.
MfG,
Oliver.
Oliver Müller schrieb:
> Ist es von der Performance her ein Unterschied, ob ich 3x if abfrage> oder via if .., else if, oder geht es nur um den besseren Stil ?
Eine Abfrage die nicht gemacht wird, verbraucht auch keine Zeit :-)
OK. Das ist jetzt nicht die Welt, ein Programm welches in Zeitnot gerät,
kann man damit nicht retten, aber ein bischen was ist es auch.
AUsserdem hat es einen gewissen dokumentarischen Wert.
gonZoX schrieb:
> for(x=1;x<6;x++)> {> SetBits( LEDNR[Zeile[x]] );> }> PORTB |= (1 << PB3);> }> else if(Zeile[0]=='2')>> Habe ich das so richtig umgesetzt ???
Das kann nicht stimmen. Denn Zeile[x] hat zb den Wert 49 (für '1') und
das Array ist in dieser Dimension aber nur 10 Einträge groß :-)
Aber Zeile[x] - '0' würde für Zeile[x] gleich '0' zb 0 ergeben. Und
für '1' würde 1 rauskommen etc.
(Über die Verwendung eines char-Arrays zu Kodierung der Bits sollten wir
uns sowieso noch unterhalten)
:) Ja den falschen Index hatte ich übersehen,Danke.
>(Über die Verwendung eines char-Arrays zu Kodierung der Bits sollten wir>uns sowieso noch unterhalten)
Ich denke du meinst ich sollte die Bits 'direkt' im Array speichern
anstelle ihrer Entsprechung als String.
Allerdings bekomme ich das gedanklich noch nicht umgesetzt ....
MfG,Oliver.
gonZoX schrieb:
> Allerdings bekomme ich das gedanklich noch nicht umgesetzt ....
Dachte ich mir schon. Drum habe ich bisher auch noch nichts gesagt. Nur
ist jetzt ein Punkt erreicht an dem ein eigentlich unnützes 2D Array
eingeführt wird, nur damit die Bitcodierungen weiterhin als String
bestehen bleiben können. Das würde wegfallen.
Auf der anderen Seite schadet es auch nicht, wenn du ein wenig Übung im
Umgang mit Arrays kriegst :-)
:) Nein das schadet mir garantiert nicht :)
Wenn ich heute Abend ein paar ruhige Minuten habe, werde ich mich da mal
dransetzen und versuchen zu begreifen ...
MfG,
Oliver.
Der Index war sogar doppelt falsch da er ja bei 1 startet und der
entsprechende Array Inhalt dementsprechend an der Stelle x-1 liegt,
würde bedeuten ' Zeile[x-1]-'0' ', oder nicht ?
:) Aber der Weg soll ja eh nicht weiter beschritten werden :)
Meine Werte habe ich jetzt wie folgt gesetzt :
gonZoX schrieb:
> Der Index war sogar doppelt falsch da er ja bei 1 startet und der> entsprechende Array Inhalt dementsprechend an der Stelle x-1 liegt,> würde bedeuten ' Zeile[x-1]-'0' ', oder nicht ?
Nein. Das war schon richtig so.
Wenn Zeile[x] den Wert '5' enthält, dann sind die auszugebenden Daten
bei LEDNR[5][..] zu finden. Also musst du auch LEDNR[5] an die
Ausgabefunktion übergeben.
>> :) Aber der Weg soll ja eh nicht weiter beschritten werden :)
Zur Übung könntes du das ja einfach mal probieren :_)
>> Meine Werte habe ich jetzt wie folgt gesetzt :>
> und werde dann mal versuchen nachzuvollziehen wie ich das richtig> übergebe.
Das sollte ja kein Problem sein. Die Funktion SetBits kriegt dann einen
unsigned char und muss sich die Bits aus dem Wert herausholen.
>Das sollte ja kein Problem sein. Die Funktion SetBits kriegt dann einen>unsigned char und muss sich die Bits aus dem Wert herausholen.
Doch das ist leider ein Problem für mich,ich begreife noch nicht wie ich
das lösen kann.
Vorher habe ich ja einen String,also ein char array übergeben und bin
mit dem Index durch die Positionen gewandert, das geht in mein kleines
Hirn vom Verständnis noch rein :),
aber jetzt übergebe ich mit dem entsprechenden unsigned char ein byte
und ich sehe nicht wie ich das bitweise auslese und entsprechend
reagiere ....
MfG,
Oliver.
... ich bedaure es wirklich sehr wenn ich den Wissenden auf den 'Keks'
gehe, aber ich stehe total auf dem Schlauch und kann anscheinend ohne
passendes Beispiel die Sache nicht selber umsetzen,sorry.
Wenn ich einer Funktion also einen unsigned char mit dem Wert z.B. 0xFC
übergebe, wie kann ich dann die entsprechenden Bits auslesen und
entsprechend ob 0 oder 1 reagieren.
Reines Vorsagen ist ansich wegen des fehlenden Lernerfolges keine gute
Lösung, aber ich komme leider nicht weiter ....
Es wäre sehr nett wenn sich jemand erbarmen könnte mir das plausibel zu
machen.
Danke im Voraus,
Oliver.
> Wenn ich einer Funktion also einen unsigned char mit dem Wert z.B. 0xFC> übergebe, wie kann ich dann die entsprechenden Bits auslesen und> entsprechend ob 0 oder 1 reagieren.
Du brauchst eine Ver-UND-ung mit einer Maske.
Wenn du wissen willst, ob im Wert 0xFC das Bit 3 gesetzt ist:
(Bit-Zählweise ist 7...0)
Bit 76543210
0xFC = 0b11111100
dann musst du dieses Bit mit 1 und alle anderen bits mit 0 ver-und-en:
0x08 = 0b00001000
daraus ergibt sich:
0xFC = 0b11111100
0x08 = 0b00001000
-----------------
00001000 --> Jawoll, dieses Bit ist gesetzt
Beim Bit 1 wäre das
0xFC = 0b11111100
0x08 = 0b00000010
-----------------
00000000 --> Dieses Bit ist nicht gesetzt
Du kannst natürlich auch mehrere Bits auf einmal kontrollieren:
Beim Bit 7 und Bit 3
0xFC = 0b11111100
0x08 = 0b10001000 ---.
----------------- v
10001000 == 10001000 --> Diese Bits sind beide gesetzt
Beim Bit 3 und Bit 0
0xFC = 0b11111100
0x08 = 0b00001001 ---.
----------------- v
00001000 != 00001001 --> Es sind nicht alle Bits gesetzt
Der bitweise UND Operator in C ist & (nicht verwechseln mit &&)
Danke für deine Hilfe.
Ich habe ja bisher einen String übergeben und dann positionsweise
geprüft, ob '0' oder '1' und dann entsprechend die Ports gesetzt.
Ich sehe noch nicht den Vorteil wenn ich jetzt anstelle des Strings ein
Byte übergebe und dann jedes Bit einzeln überprüfe und entsprechend
reagiere ???
Danke im Voraus,
Oliver.
gonZoX schrieb:
> Ich sehe noch nicht den Vorteil wenn ich jetzt anstelle des Strings ein> Byte übergebe und dann jedes Bit einzeln überprüfe und entsprechend> reagiere ???
Dein String hat 8 Bytes verbraucht, jetzt brauchst du nur noch 1 Byte.
Du hast aber nicht nur 1 String, du hast 8 davon. Macht 64 Bytes und das
im Kontrast zu den 8 Bytes mit Bitmaskieren.
Rechenzeitmässig hält sich beides in etwa die Waage. Den Zugriff in das
Array kriegst du auch nicht gratis, genausowenig wie das Ausmaskieren
des richtigen Bits.
1
voidSetBits(unsignedcharleds)
2
{
3
unsignedchary;
4
unsignedcharmask=0x01;
5
6
for(y=0;y<8;y++)
7
{
8
if((leds&mask)==0)
9
{
10
PORTB&=~(1<<PB1);
11
}
12
else
13
{
14
PORTB|=(1<<PB1);
15
}
16
PORTB&=~(1<<PB2);
17
PORTB|=(1<<PB2);
18
19
mask<<=1;
20
}
21
}
Bleibt noch anzuführen, dass es ich bei solchen Maskieroperationen um
eine Standardtechnik in der µC-Progammierung handelt. Wie willst du zb
einen Eingang an einem Port auswerten, wenn du nicht weist, wie du das
eine interessierende Bit isolieren kannst? Der Umgang mit &, | und
Schiebeoperationen ist in der µC Programmierung sowas wie das kleine
Einmaleins.
Danke, jetzt habe ich das 'Durchlaufen' des Bytes mittels der sich immer
verschiebenden Maske verstanden, sicherlich muss ich da noch genug
lernen um das 'kleine Einmaleins' zu beherschen.
Gäbe es jetzt noch eine direktere Maßnahme den entsprechenden Port ohne
die if-Abfrage zu setzen,so nach der Methode 'PB1 = 0' bzw 'PB1 = leds &
mask' ?
(Ich habe halt in dem einen Beispiel für einen Pic gesehen, dass man da
die Ports so direkt setzen kann)
Danke für eure Mühe,
Oliver.
> Ich habe halt in dem einen Beispiel für einen Pic gesehen, dass man da> die Ports so direkt setzen kann
Das sind herstellerspezifische nichtportable Compilererweiterungen...
gonZoX schrieb:
> Gäbe es jetzt noch eine direktere Maßnahme den entsprechenden Port ohne> die if-Abfrage zu setzen,so nach der Methode 'PB1 = 0' bzw 'PB1 = leds &> mask' ?
Man kann diese Abfrage in C etwas verstecken aber grundsätzlich ist sie
immer da.
denn ( leds & mask ) ergibt ja nicht 1, wenn das Bit gesetzt ist.
Ausserdem: Bit setzen und Bit löschen sind prinzipiell unterschiedliche
Operationen.
> (Ich habe halt in dem einen Beispiel für einen Pic gesehen, dass man da> die Ports so direkt setzen kann)
Auch das mündet irgendwo immer in dieser Abfrage. Nur weil du sie nicht
direkt siehst, heisst das ja nicht, dass sie nicht da ist.