• 30.05.2024, 08:02
  • Register
  • Login

  • You are not logged in.

 

Problem mit C und switch-case (avr + gcc)

Dear visitor, welcome to Aqua Computer Forum. If this is your first visit here, please read the Help. It explains how this page works. You must be registered before you can use all the page's features. Please use the registration form, to register here or read more information about the registration process. If you are already registered, please login here.

Freakmaster

Senior Member

Posts: 1,791

Problem mit C und switch-case (avr + gcc)

Saturday, June 27th 2009, 2:42pm

Servus zusammen!

Ich habe leider ein "kleines" Problem mit einer switch-case Anweisung.

Source code

 
1
2
3
4
5
6
7
8
9
10
11
12
13

int mode = 1;

switch ( mode )
{
case 1:
   mode_1();
case 2:
   mode_2();
case 3:
   mode_3();
default:
   mode_0();
}


Der springt mir hierbei jedesmal in den "default" rein...?
Habs auch schon so versucht:

Source code

 
1
2
3

...
case '2':
...


Was mache ich denn hier bitte falsch?

This post has been edited 1 times, last edit by "Freakmaster" (Jun 27th 2009, 2:44pm)

Da steht nix :-P

hurra

God

Posts: 5,051

Saturday, June 27th 2009, 2:46pm

Muss da nicht noch überall ein "break;" rein?
Ich weiß schon, warum ich lieber viele if-Statements schreibe, anstatt switch zu benutzen.

sebastian

Administrator

Posts: 10,533

Saturday, June 27th 2009, 3:05pm

das muß überall ein break rein, es werden sonst alle statements vom einsprunggpunkt bis zum ende abgearbeitet.

Freakmaster

Senior Member

Posts: 1,791

Saturday, June 27th 2009, 3:55pm

gnarf -.-
Man sollte doch mal zwischendrin ne Pause machen...

danke!

Jetzt hab ich allerdings noch ein weiteres Problem:

Ich habe einen 3-fach DIP-Schalter am µC der jeweils einen PIN gegen GND schaltet wenn er eingeschalten ist. Pull-ups sind aktiviert.
Damit möchte ich 6 verschiedene Intervalle einstellen können.

Ich frage so den Zustand der einzelnen Pins ab:

Quoted

if (!(PIND&(1<<PD0)))
{
pin1 = 1;
}

if (!(PIND&(1<<PD1)))
{
pin2 = 2;
}

if (!(PIND&(1<<PD2)))
{
pin3 = 4;
}

char mode = (pin1 + pin2 + pin3);


Und hier dann die switch-case Konstruktion:

Quoted

switch ( mode )
{
case 0:
dauerleuchten(); //funktioniert
break;
case 1:
eineinhalb_hertz();//funktioniert nicht - läuft mit 3 hertz
break;
case 2:
drei_hertz(); //funktioniert nicht - läuft mit 5 hertz
break;
case 3:
fuenf_hertz(); //funktioniert nicht - läuft mit 20 hertz
break;
case 4:
fuenf_hertz_blink(); //funktioniert nicht - führt 1 aus
break;
case 5:
zwanzig_hertz(); //kann ich grad nicht testen
break;
case 6:
vierzig_hertz(); //funktioniert nicht - führt 1 aus
break;
default:
PORTD &= ~(1<< PD6);
break;
}



Scheinbar wird irgendwie falsch zusammengezählt oder?
nur: wie soll man die variablen sonst addieren???
achja: hier noch eine beispielfunktion - es ist jeweils nur die zeit vom _delay_ms() geändert:

Quoted

void eineinhalb_hertz () //Blinken bei 1,5 Hz
{
PORTD ^= (1<< PD6);
_delay_ms(666);
}

This post has been edited 3 times, last edit by "Freakmaster" (Jun 27th 2009, 4:00pm)

Da steht nix :-P

sebastian

Administrator

Posts: 10,533

Saturday, June 27th 2009, 9:58pm

das ist niccht gerade übersichtlich was du da zusammprogrammierst.
ich würde das so machen:

Source code

 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26

#define DIP1 	0x01
#define DIP2    0x02
#define DIP3    0x04

#define DIP1IN	!(PIND&DIP1)
#define DIP2IN	!(PIND&DIP2)
#define DIP3IN	!(PIND&DIP3)

#define MODE1 (DIP1&DIP2)
#define MODE2 (DIP1)
#define MODE3 (DIP3&DIP1)

uchar temp=0;

if(DIP1IN)
   temp |= DIP1;
if(DIP2IN)
    temp |= DIP2;
if(DIP3IN)
    temp |= DIP3;

switch(temp){
   case MODE1:
   break;

}

Freakmaster

Senior Member

Posts: 1,791

Saturday, June 27th 2009, 11:35pm

Quoted from "sebastian"

das ist niccht gerade übersichtlich was du da zusammprogrammierst.


Ich lerns ja auch gerade erst ;)

Danke für den Tipp! Ich werd mal versuchen etwas aufzuräumen :)
Da steht nix :-P

sebastian

Administrator

Posts: 10,533

Sunday, June 28th 2009, 7:26am

Quoted from "Freakmaster"

ich lerns ja auch gerade erst ;)
Das war auch nicht böse gemeint, aber versuche immer so zu Programmieren das Code für sich sebst spricht. Also Kommentare mehr oder weniger bei den einfachen sachen überflüssig sind.
Dann ist der code auch in 1/2 Jahr wenn du mal was ändern mußst vernünftig lesbar und du weisst sofort ohne suchen was du da machst, das geht gerade um Variablennamen und IO definitionen.
IO Definitionen kapsle ich immer als define aus. so kann man auch mal den code auf eine andere HW portieren und muß nur seine Defines ändern und nicht den ganzen code durchsuchen.

Meine Variante ist spart dir zb auch gleich mal 3 Variablen ein und ist kompliert kleiner und schneller.

Source code

 
1

char mode = (pin1 + pin2 + pin3);

wird bei mir alles mit temp geamcht.

Und wichtig, temp jedes mal auf 0 neu initialsieren. Das müsstest du auch mit pin1 + pin2 + pin3 machen.

EDIT:

mal ein Beispiel:

Source code

 
1
2
3
4
5

void eineinhalb_hertz ()//Blinken bei 1,5 Hz
 {
 PORTD ^= (1<< PD6);
 _delay_ms(666);
 }


ich mach das so:

Source code

 
1
2
3
4
5
6
7
8
9
10

//LED
#define LED_PORT PORTD
#define LED 6
#define ledToggle() LED_PORT ^= (1<<LED)

void eineinhalb_hertz ()//Blinken bei 1,5 Hz
 {
ledToggle();
_delay_ms(666);
 }


ABER du hast noch einen Fehler drin: siehe AVR-LIBC

Quoted


Function Documentation

void _delay_ms ( double __ms )

Perform a delay of __ms milliseconds, using _delay_loop_2().
The macro F_CPU is supposed to be defined to a constant defining the CPU clock frequency (in Hertz).
The maximal possible delay is 262.14 ms / F_CPU in MHz.


du msst also _delay_ms kapseln,

Source code

 
1
2
3
4
5
6

void delayms(uint time)
{
while(time){
time--;
_delay_ms(1);
}


aber um ein blinken zu erzeugen musst du nicht mit der Delay funktion rechenzeit verbrennen.
Das geht auch deutlich besser wenn du dir aus einem Timer eine Zeitbasis holst und diese nimmst. Mit _delay_xx blockiert dir deine komplette Anwendung (bis auf die IRQs).

Freakmaster

Senior Member

Posts: 1,791

Sunday, June 28th 2009, 1:09pm

Quoted from "sebastian"

Quoted from "Freakmaster"

ich lerns ja auch gerade erst ;)
Das war auch nicht böse gemeint,...


Das dachte ich mir schon - deswegen auch der Smiley :)
Für Verbesserungsvorschläge bin ich selbstverständlich immer offen!

Ich habe leider derzeit noch ein gewisses Problem mit der Syntax von Variablen :S
Java, shell, Bash, ksh, csh und tsh - da blickt man irgendwann nicht mehr durch wenn man zwar alles aber viel zu selten benutzt :thumbdown:

Sollte man eigentlich so viel wie möglich zwegs Performance in Funktionen auslagern oder sind die eher für die Sauberkeit des Codes von Wert weil der Compiler die Funktionen an den Stellen wo sie aufgerufen werden komplett "ausschreibt"?

Na dann werde ich mich mal mit Timern spielen...
Da steht nix :-P

hurra

God

Posts: 5,051

Sunday, June 28th 2009, 1:16pm

Also defines wirken sich nicht auf die Performance aus. Beim Kompilieren werden die defines sozusagen ersetzt.
Echte Funktionen dagegen können soweit ich weiß schon Performance kosten. Allerdings ist das denk ich bei dir im Moment sowas von vernachlässigbar. Versuch doch erstmal mit nem Timer zu arbeiten, anstelle von den sleeps.

Freakmaster

Senior Member

Posts: 1,791

Sunday, June 28th 2009, 1:31pm

Quoted from "hurra"

Also defines wirken sich nicht auf die Performance aus. Beim Kompilieren werden die defines sozusagen ersetzt.
Echte Funktionen dagegen können soweit ich weiß schon Performance kosten. Allerdings ist das denk ich bei dir im Moment sowas von vernachlässigbar. Versuch doch erstmal mit nem Timer zu arbeiten, anstelle von den sleeps.


Sprich die defines sind nur für die Lesbarkeit da.
Dass die Performance bei dieser Anwendung noch keine Rolle spielt ist mir schon klar aber wenn ich schon lerne, dann doch gleich richtig :)

Wie gesagt den Timer werde ich mir mal zu gemüte führen.
Da steht nix :-P

sebastian

Administrator

Posts: 10,533

Sunday, June 28th 2009, 2:10pm

Das ist jetzt mal explizit auf den AVR und teileweise auch andere µController bezogen.

Defines bingen 1. einen Vorteil weil man sie besser lesen kann und sie zentral in *.h files verwaltet werden können, somit hat man halbwegs portablen code.

so etwas wie das hier: temp |= DIP1; in eine funktion auszulagen macht keien sinn, da es im assembler code nur eine zeile ist.
aber ein _delay_ms() zu kapseln macht sinn, da der compiler das inline immer wieder neu einsetzt.
Ich habe mit angewöhnt funktionen/berechnungen die mehr als 2x gebraucht werden in eine funktion zu packen.
Defines können mitunter einen recht guten Geschwindigkeitsvorteil bieten.
Zb würde ich die die Dip Schalter abfrage kapseln.

Source code

 
1

uchar getDipSwitch(void);

Somit bist du dann in der Implementation der Funktion unabhängig und mußt nur noch an der Funktion selbst rumoptimieren oder etwas ändern.
Genau so handhabe ich es mit den Hardware Routinen. Die Initialsierung wird in void init(void) ausgelagert.
Nix ist stressiger als ne aufgeblasene Main.c mit ein paar 1000 zeilen wo man kaum noch die eigentliche funktion erkennen kann.

Clark

God

Posts: 2,336

Monday, June 29th 2009, 8:19am

Quoted from "sebastian"

das ist niccht gerade übersichtlich was du da zusammprogrammierst.
ich würde das so machen:

Source code

 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26

#define DIP1 	0x01
#define DIP2    0x02
#define DIP3    0x04

#define DIP1IN	!(PIND&DIP1)
#define DIP2IN	!(PIND&DIP2)
#define DIP3IN	!(PIND&DIP3)

#define MODE1 (DIP1&DIP2)
#define MODE2 (DIP1)
#define MODE3 (DIP3&DIP1)

uchar temp=0;

if(DIP1IN)
   temp |= DIP1;
if(DIP2IN)
    temp |= DIP2;
if(DIP3IN)
    temp |= DIP3;

switch(temp){
   case MODE1:
   break;

}


ich würde das übrigens noch geringfügig verändern:

btw.: mir fiel das gerade auf: Du hast da sogar nen gravierenden Fehler drin ;)

Source code

 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

#define DIP1 	0x01
#define DIP2    0x02
#define DIP3    0x04

#define DIP1IN	(!PIND)&DIP1
#define DIP2IN	(!PIND)&DIP2
#define DIP3IN	(!PIND)&DIP3

#define MODE1 (DIP1&DIP2)
#define MODE2 (DIP1)
#define MODE3 (DIP3&DIP1)

uchar temp = DIP1IN | DIP2IN | DIP3IN;


switch(temp){
   case MODE1:
   break;

}


Spart 3 if statements und sowas erfreut den Prozessor ;)

und ist sogar noch zu kürzen auf:

Source code

 
1

temp = (!PIND)&(DIP1|DIP2|DIP3);


gerade bei Microcontroller sachen ist es mE zielführend gleich sprungoptimiert zu programmieren (also möglichst wenig IF- Abfragen einzubauen).

This post has been edited 2 times, last edit by "Clark" (Jun 29th 2009, 8:29am)

c++: The power, elegance and simplicity of a hand grenade.

hurra

God

Posts: 5,051

Monday, June 29th 2009, 8:35am

Sollte man bei defines nicht sogar immer ne Klammer um den Gesamtausdruck machen?

Source code

 
1
2
3
4
5
6
7

#define DIP1  (0x01)
#define DIP2 (0x02)
#define DIP3 (0x04)

#define DIP1IN ((!PIND)&DIP1)
#define DIP2IN ((!PIND)&DIP2)
#define DIP3IN ((!PIND)&DIP3)

E: Ahh, hab gerade nochmal nachgelesen. Das macht eigentlich nur bei Makros mit Parametern Sinn.

This post has been edited 1 times, last edit by "hurra" (Jun 29th 2009, 9:04am)

Clark

God

Posts: 2,336

Monday, June 29th 2009, 9:38am

Quoted from "hurra"

Sollte man bei defines nicht sogar immer ne Klammer um den Gesamtausdruck machen?

Source code

 
1
2
3
4
5
6
7

#define DIP1  (0x01)
#define DIP2 (0x02)
#define DIP3 (0x04)

#define DIP1IN ((!PIND)&DIP1)
#define DIP2IN ((!PIND)&DIP2)
#define DIP3IN ((!PIND)&DIP3)

E: Ahh, hab gerade nochmal nachgelesen. Das macht eigentlich nur bei Makros mit Parametern Sinn.


Naja, es macht sinn, sobald es nicht nur eine einzelne Variable ist.

also auch

Source code

 
1

#define DIP1IN ((!PIND)&DIP1)

sollte so geklammert sein.
reichen tut aber auch:

Source code

 
1

#define DIP1IN (!PIND&DIP1)

die obere Schreibweise hat den Vorteil, dass sie jeder sofort versteht.

Wenn man nicht klammert kann das Probleme geben:

Source code

 
1
2
3

#define DIP1IN !PIND&DIP1

int x = 5+ DIP1IN * 7;


was kommt raus?
erwarten sollte man ja 5 + WertvomPin mal sieben.
Aber was kommt raus?

Lösen wirs mal auf:

Source code

 
1

5+!PIND&DIP1*7

und leider sind die Präferenzen wie folgt (hier jetzt absichtlich geklammert):

Source code

 
1

(5+(!PIND))&(DIP1*7)


Uups, das ist ja was ganz anderes...
c++: The power, elegance and simplicity of a hand grenade.

Freakmaster

Senior Member

Posts: 1,791

Monday, June 29th 2009, 10:23am

So! Hier nochmal überarbeitet - ich hasse es, wenn ich code einfach so tippen muss und ihn nicht ausprobieren kann ;D



Ich habs jetzt mal die PINs und die Werte der DIPs aufgetrennt um das ganze etwas übersichtlicher und portabel vom experimentierboard auf die fertige Platine zu machen:
(gibt es bei C eigentlich eine Konvention, dass man Variablen komplett groß, klein oder sonstirgendwie schreibt?)

Quoted


#define port PORTD
#define pin PIND

#define pin_dip1 0x01 //PD0
#define pin_dip2 0x02 //PD1
#define pin_dip3 0x04 //PD2
#define pin_active 0x08 //PD3
#define pin_led 0x20 //PD5

#define dip1val 1 //wert von dip1 (1)
#define dip2val 2 //wert von dip2 (2)
#define dip3val 4 //wert von dip3 (4)

#define dip1in (!(pin)&pin_dip1)
#define dip2in (!(pin)&pin_dip2)
#define dip3in (!(pin)&pin_dip3)

#define active (!(pin)&pin_active)

#define led_toggle() port ^= (1<<pin_led);
#define led_off() port &= ~(1<<pin_led);
#define led_on() port |= (1<<pin_led);


Die Abfrage habe ich vom Sebastian übernommen:

Quoted


if(dip1in)
temp |= dip1val;
if(dip2in)
temp |= dip2val;
if(dip3in)
temp |= dip3val;


Der switch/case schaut dann so aus:

Quoted

switch(temp){
case 0:
break;
case 1:
break;
...
}



Aktuell les ich mich grad in Timer und interrupts ein um die _delay_ms(); rauswerfen zu können...

This post has been edited 8 times, last edit by "Freakmaster" (Jun 29th 2009, 10:50am)

Da steht nix :-P

Clark

God

Posts: 2,336

Monday, June 29th 2009, 10:58am

Quoted from "Freakmaster"

So! Hier nochmal überarbeitet - ich hasse es, wenn ich code einfach so tippen muss und ihn nicht ausprobieren kann ;D



Ich habs jetzt mal die PINs und die Werte der DIPs aufgetrennt um das ganze etwas übersichtlicher und portabel vom experimentierboard auf die fertige Platine zu machen:
(gibt es bei C eigentlich eine Konvention, dass man Variablen komplett groß, klein oder sonstirgendwie schreibt?)


es gibt so ca. hunderte von Konventionen.

Normalerweise werden defines gross geschrieben:
#define PORT PORTD
(was eigentlich blödsinn ist: du definierst eine Konstante nach einer Konstante...)

Quoted



Quoted


#define port PORTD
#define pin PIND

#define pin_dip1 0x01 //PD0
#define pin_dip2 0x02 //PD1
#define pin_dip3 0x04 //PD2
#define pin_active 0x08 //PD3
#define pin_led 0x20 //PD5

#define dip1val 1 //wert von dip1 (1)
#define dip2val 2 //wert von dip2 (2)
#define dip3val 4 //wert von dip3 (4)

#define dip1in (!(pin)&pin_dip1)
#define dip2in (!(pin)&pin_dip2)
#define dip3in (!(pin)&pin_dip3)

#define active (!(pin)&pin_active)

sicher, dass active low aktiv ist?

Quoted

Quoted


#define led_toggle() port ^= (1<<pin_led);
#define led_off() port &= ~(1<<pin_led);
#define led_on() port |= (1<<pin_led);


Die Abfrage habe ich vom Sebastian übernommen:

Quoted


if(dip1in)
temp |= dip1val;
if(dip2in)
temp |= dip2val;
if(dip3in)
temp |= dip3val;


Ich würde dir die Variante empfehlen, die ich oben geschrieben habe, sie ist a) schneller b) für den Prozessor "angenehmer" ;)

Source code

 
1

temp = (!PIND)&(DIP1|DIP2|DIP3);

das reduziert die gesamte rechenlast an der Stelle auf ein einziges nicht und eine binäre und verknüpfung. (Den Rest drüfte der Compiler dort optimieren).
(oder gleich manuell: temp = !PIND&0x7; //hat den Nachteil, dass es sich nicht mehr so leicht anpassen lässt...)

Quoted


Der switch/case schaut dann so aus:

Quoted

switch(temp){
case 0:
break;
case 1:
break;
...
}



Aktuell les ich mich grad in Timer und interrupts ein um die _delay_ms(); rauswerfen zu können...

Viel Erfolg ;)

This post has been edited 1 times, last edit by "Clark" (Jun 29th 2009, 11:01am)

c++: The power, elegance and simplicity of a hand grenade.

Freakmaster

Senior Member

Posts: 1,791

Monday, June 29th 2009, 12:07pm

Quoted from "Clark"

es gibt so ca. hunderte von Konventionen.


Na wenn das so ist, dann halt ich mich mal an meine Konvention ;D

Quoted from "Clark"

sicher, dass active low aktiv ist?

Jup ist low aktiv ->

Source code

 
1
2

portregister &= ~(1<<pin_active)[...];	//Pins als Eingänge
port |= (1<<pin_active)[...];	//pull-up Widerstände einschalten


Quoted


Ich würde dir die Variante empfehlen, die ich oben geschrieben habe, sie ist a) schneller b) für den Prozessor "angenehmer" ;)

Source code

 
1

temp = (!PIND)&(DIP1|DIP2|DIP3);

das reduziert die gesamte rechenlast an der Stelle auf ein einziges nicht und eine binäre und verknüpfung.


Hier habe ich aber doch das Problem, dass wenn ich statt den Pins PD0 PD1 und PD2 die Pins PD4, PD5 und PD6 als Eingänge nutze ich falsche Werte herausbekomme...?
Deswegen habe ichs aufgeteilt.

Die Rechenlast hätte ich ja außerdem nur ein einziges mal und zwar beim anlegen der Versorgungsspannung an den Controller, da sie bei mir in meiner init-funktion steht die vor der while(1) aufgerufen wird.
In der while(1) steht dann der switch/case.
Um die Modis dann umzuschalten muss ich natürlich die Spannungsversorgung trennen das ist mir klar - aber das ist Absicht so :)

This post has been edited 1 times, last edit by "Freakmaster" (Jun 29th 2009, 12:08pm)

Da steht nix :-P

Similar threads