DDR und PORT in einem Struct

**CrazyMetal** · 01.07.2013, 09:10

Hallo Zusammen,

ich arbeite mich grade in die Microcontroller-Entwicklung mit C ein und versuche das DDR und PROT, sowie Pin in einem Struct zu halten.
Ich würde gerne meine LED-Funktionen in einer Headerfile auslagern, das Ziel hierbei ist eine bessere Les- und Wartbarkeit des Codes.

Hier etwas Code, ich denke damit ist klar was ich versuche zu basteln:

Code:

struct LED
{
	unsigned char DDR;
	unsigned char PORT;
	unsigned char PIN;
};

void InitLED(struct LED *led)
{
	led->DDR |= 1<<led->PIN;
}

void LedOn(struct LED *led)
{
	led->PORT | 1<<led->PIN;
}

void main()
{
	struct LED ledStoerung = {DDRA, PORTA, PA0};	// LED definieren
	
	InitLED(&ledStoerung);	// den Port der LED initialisieren
	LedOn(&ledStoerung);	// die LED einschalten	
}

Leider denkt sich die LED z.Zt. noch "es ist Montag, strahl man selber." Und ich finde grade nicht woran es hängt, vielleicht habt Ihr ja einen brandheißen Tipp für mich.

Viele Grüße,
Crazy

**Besserwessi** · 01.07.2013, 09:43

Bis jetzt wird der Inhalt der Struct verändert, und der ist im RAM.

Das ganze Prinzip hat ein Problem: der Compiler packt die Addressen für den Zugriff auch die IO Ports mit in den Code, die sind also Teil des ASM Befehle. Mit der Indirekten Methode über Funktionen bekommt man bestenfalls eine weniger Effektive Lösung, wo er Compiler dann über Pointer auf die IO Adressen zugreift. Vorzuziehen wäre eine Lösung über #Define , also Makros die schon der Compiler (bzw. Preporcessor) auflösen kann. Wobei das mit der Lesbarkeit und Wartbarkeit so eine Sache ist: mit den BEfehlen direkt im Code hat man alle Informationen da - anders muss man noch die Funktionen zur Abkapselung kennen, und viel kürzer wird es auch nicht.

**CrazyMetal** · 01.07.2013, 11:00

Zitat von Besserwessi

Bis jetzt wird der Inhalt der Struct verändert, und der ist im RAM.
Das ganze Prinzip hat ein Problem: der Compiler packt die Addressen für den Zugriff auch die IO Ports mit in den Code,
die sind also Teil des ASM Befehle.

Ok, klar weshalb die LED nicht will.

Zitat von Besserwessi

Mit der Indirekten Methode über Funktionen bekommt man bestenfalls eine weniger
Effektive Lösung, wo er Compiler dann über Pointer auf die IO Adressen zugreift.

Hmm, was heißt für dich "weniger Effektiv"? Wenn der Microcontroller 2 Zyklen mehr braucht stört mich das nicht, da er dann noch fix genug ist.

Zitat von Besserwessi

Vorzuziehen wäre eine Lösung über #Define , also Makros die schon der Compiler (bzw. Preporcessor) auflösen kann.

Die sähe dann konkret wie aus?

Zitat von Besserwessi

Wobei das mit der Lesbarkeit und Wartbarkeit so eine Sache ist: mit den BEfehlen direkt im Code hat man alle Informationen da - anders muss man noch die Funktionen zur Abkapselung kennen, und viel kürzer wird es auch nicht.

Das soll hier jetzt nicht in eine Diskussion über Vor- und Nachteile unterschiedlicher Codestrukturierungen ausufern ...
Aber, ich finde eine Kapselung generell besser lesbar und wartbarer, als alles in eine Mörderfunktion zu zerren, zudem muss sich der Kollege welcher das später mal anschaut und ggf. ergänzen soll nicht immer fragen, "ist das jetzt ne LED, oder..?"

Ja, klar man schreibt das auch im Kommentar hin, ich persönlich finde das eben besser zu lesen. - Zugegeben über Sinn und Zweck bei einer LED kann man streiten (oder auch nich), aber die LED ist hier auch erstmal nur als Beispiel gedacht.

**oberallgeier** · 01.07.2013, 12:45

... Wenn der Microcontroller 2 Zyklen mehr braucht stört mich das nicht, da er dann noch fix genug ist ...

Stimmt, im Prinzip. Und ich habe (fast) immer viel Reserve im Zielcontroller, weil Codes die Neigung haben, mit dem Alter des Projektes umfangreicher zu werden.

... Aber, ich finde eine Kapselung generell besser lesbar und wartbarer ...

Erstmal - ich fühle mich immer noch als C-Anfänger, zumindest nicht als Könner. Deine Lösung finde ich pfiffig, ein bisschen sophisticated aber für mich nicht wirklich besser lesbar - und damit auch nicht besser pflegbar. Meine Lösung ist recht konventionell (denke ich) und gut lesbar - weil die #define´s schon recht praktisch sind.

Aber es ist ja DEIN Projekt - und jeder löst ne Aufgabe nach seinem Geschmack (und Können natürlich). Und strukts sind für mich eben nicht die erste Wahl.

Mein Beispiel, weder als Vorbild noch unbedingt zum Lesen gedacht.

Code:

// Auszug aus ~com~.h
// ==================
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
                                //
  #define SetBit(ADDR,BIT)       ((ADDR)  |=  (1<<(BIT)))       // Setzt Bit
  #define ClrBit(ADDR,BIT)       ((ADDR)  &= ~(1<<(BIT)))       // Löscht Bit
  #define ToggleBit(ADDR,BIT)    ((ADDR)  ^=  (1<<(BIT)))       // Toogelt Bit
// - - - - - - - - - - - - - - - -
  #define       PrtLED  PORTD   // Port für gn+rt-LED Tim_rh (Timerrhythmus)
  #define       L1g         4   // Auf PD4 grüne LED
  #define       L1r         5   // Auf PD5 rote  LED
                        // ##>>> (Heartbeat)LEDs schalten Aode -<|- Portpin <<<##
  #define       FlgLED      2   // LED - Schaltflag, aktuell nur Heartbeat = on
                        //      1=> Heartbeat, 
                        //      2=> Togglebeat rtLED in ISR (TIMER1_COMPA_vect)
// Ende Auszug aus ~com~.h
// =======================
// - - - - - - - - - - - - - - - -
// Auszug aus ~main~.c
// ===================
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// . . .
//              ===================================================
// ####>>>>     Initialisierung der Anschlüsse für R5M auf mega1284: <<<<####
//           (           PB0   1 A   A  40  PA0         (CN 11 -Audi)
// CN_12     (           PB1   2 A   A  39  PA1         (CN 11 -Audi)
// PORTB     (           PB2   3 A   A  38  PA2         (CN 11 -Audi)
// (CIR od.  /           PB3   4 A   A  37  PA3         (CN 11 -Audi)
//  LCD ...  \           PB4   5 A   A  36  PA4, Servo10
//  siehe    (     MOSI, PB5   6 A   A  35  PA5, Servo9
//  unten    (     MISO, PB6   7 A   A  34  PA6, Servo8
//           (      SCK, PB7   8 A   A  33  PA7, Servo7
//  Taste /RES,       /RESET   9        32  AREF ref Vcc, aktuell
//                       Vcc  10        31  GND                        
//                       GND  11        30  AVcc                       
//                     XTAL2  12     A  29  PC7, Servo6
//                     XTAL1  13     A  28  PC6, Servo5
//           RXD0,       PD0  14 EU  A  27  PC5, Servo4
//           TXD0,       PD1  15 EU  A  26  PC4, Servo3
//           RXD1,       PD2  16 EU  A  25  PC3, Servo2
//           TXD1,       PD3  17 A   A  24  PC2, Servo1
//           L1g         PD4  18 A   E  23  PC1, SDA   
//           L1r         PD5  19 A   E  22  PC0, SCL   
//             TasteA_1, PD6  20 EU  EU 21  PD7, TasteB_2, PCINT31, (Sound)
// - - - - - - - - - - - - - - -
// Ports+Pins als Ein- (0) oder Ausgänge (1) konfigurieren, Pull Ups (1) aktivieren
//      A = Ausgang, E = Eingang ohne , EU = Eingang MIT PullUp
  DDRA  = 0b11111111;   // siehe aktuell oben
  PORTA = 0b00000000;   //    und Port/Pull Ups (1)  aktivieren
                        //
  DDRB  = 0b01111111;   // siehe aktuell oben
  PORTB = 0b10000000;   //    und Port/Pull Ups (1)  aktivieren
                        //
  DDRC  = 0b11111100;   // PC0..7 (mega1284), I2C = PC0 + PC1
  PORTC = 0b00000000;   // bis auf I2C : Alle OHNE Pullup !!
                        // 
  DDRD  = 0b00111100;   // -> siehe Schaltplan m-32-plus
  PORTD = 0b11000011;   //    Pull Ups aktivieren, NICHT bei extINT0/~1
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// . . . und unmittelbar nach diesen ersten, wirksamen Codezeilen eine
//        Kontrollsequenz um . . . steht dabei:
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  for(i=0; i<10; i++)   // grüneLED1 blinken lassen bevor Interrupts erlaubt sind,
  {                     //   um ungewollte Resets u.ä. besser erkennen zu können
    SetBit(PORTD, L1g); // gnLED/PD4 schalten EIN, HELL
    wms(3);             // Millisekundenwait
    ClrBit(PORTD, L1g); // gnLED/PD4 aus
    wms(97);            //
// Nur zur Erläuterung in diesem Posting:
// Der besseren Auffälligkeit halber wurde die alternierend flackernde rote LED zugefügt.
    SetBit(PORTD, L1r); // rtLED/PD5 schalten EIN, HELL
    wms(3);             // 
    ClrBit(PORTD, L1r); // rtLED/PD5 aus WENN T1 nicht gedrückt
    wms(97);            //
  }             // Ende von for(i=0; i<10; i++)
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
//
// Ende Auszug aus ~main~.h
// ========================

**CrazyMetal** · 01.07.2013, 13:31

Zitat von oberallgeier

Deine Lösung finde ich pfiffig, ein bisschen sophisticated aber für mich nicht wirklich besser
lesbar - und damit auch nicht besser pflegbar. Meine Lösung ist recht konventionell (denke ich)
und gut lesbar - weil die #define´s schon recht praktisch sind.

Wie du schon schreibst, es ist Geschmackssache. Dennoch halte ich "sprechende Funktionsnamen" für besser lesbar,
wenn ich eine Funktion aufrufe die "LedOn()" heißt, weiß ich gleich beim Code lesen was dort vor sich geht. Der Vorteil
der Wartbarkeit ist hier auch gegeben. Angenommen (als Beispiel) ich habe eine Funktion die LEDs blinken lassen kann,
dann brauche ich nur die Funktion aufrufen und das LED-Struct übergeben. Zum einen kann ich in einer Zeile lesen was passiert,
zum anderen: Sollte sich an der Funktionalität des Blinkens was ändern, so ändere ich eine Funktion und die Änderung ist
umgesetzt, ohne dass ich das Projekt durchsuchen muss und ggf. eine Stelle übersehe wo ich eine Änderung hätte machen müssen.

Zitat von oberallgeier

Erst mal - ich fühle mich immer noch als C-Anfänger, zumindest nicht als Könner.

Auch wenn ich gelernter Softwareentwickler bin, so ist C momentan noch Neuland (Achtung: Kanzler-Witz) für mich.
Dennoch haben sich halt einige Prinzipien für Programmierung und Codegestaltung bisher bei jedem großen Projekt bewährt,
das ist einer der Gründe, weshalb ich dieses Vorgehen auch bei einem Microcontroller wähle.

Zitat von oberallgeier

Und strukts sind für mich eben nicht die erste Wahl.

Ja und Nein. Ich sehe hier einige Vorteile. Mein aktuelles Projekt läuft auf einem ATMEGA128 und hat viele LEDs und einen LED-Bar, die PORTs B, C, E und F sind hierbei teilweise mit LEDs belegt. Wenn ich deinen Code richtig überfolgen habe, so müsste ich pro LED ein define-Statement haben und bei der Verwendung von SetBit noch immer aufpassen, dass ich den passenden Port verwende.

Meinst du nicht es wäre andersrum einfacher, wenn ich den Schaltplan und das Gerät kenne und im Code lese LedOn(&ledStoerung_1);", hierbei weiß ich gleich worum es sich handelt und was passieren wird.

Puh! Genug getippt.

Vielleicht hat ja einer von Euch schon mal einen ähnlichen Stunt gemacht und kann mir sagen wo ich momentan auf dem Holzweg bin.

Ich habe meine Funktion so geändert

Code:

struct LED
{
	volatile unsigned char *ddr;
	volatile unsigned char *port;
	volatile unsigned char *pin;
};

void InitLed(struct LED *led)
{
	*led->ddr |= 1<<(*led->pin);
}

void LedOn(struct LED *led)
{
	*led->port |= 1<<(*led->pin);
}

void main()
{
	struct LED led1 = {&DDRA, &PORTA, PA0};
	struct LED led2 = {&DDRA, &PORTA, PA1};
	
	InitLed(&led1);
	InitLed(&led2);
}

leider geht beim STK500 nur die LED 0 (<-- Null) an.

Viele Grüße,
Crazy

**oberallgeier** · 01.07.2013, 13:53

... Dennoch halte ich "sprechende Funktionsnamen" für besser lesbar, wenn "LedOn()" heißt ...

Da bin ich völlig Deiner Meinung, ich mache genau das mit meinen #define´s. Und ich zeige das jetzt nicht wegen der Besserwisserei - sondern nur um (m)eine Möglichkeit mit diesen gestuften #define´s aufzuzeigen. Ach so, die Taster schalten gegen GND.

Code:

// - - - - - - - - - - - - - - - -
// in der *.h so:
// . . .
  #define PrtTAST       PIND    //
  #define Tst_1            6    //
  #define Tst_2            7    //
...
  #define Taste1_an     IsBitClr (PrtTAST, Tst_1)       // Taster 1 gedrückt ??
  #define Taste1_aus    IsBitSet (PrtTAST, Tst_1)       // Taster 1 gelöst ??
  #define Taste2_an     IsBitClr (PrtTAST, Tst_2)       // Taster 2 gedrückt ??
  #define Taste2_aus    IsBitSet (PrtTAST, Tst_2)       // Taster 2 gelöst ??
// - - - - - - - - - - - - - - - -
//      Im Code so benutzt
// . . .
    if (TasteA_an)              // Wenn "A" gedrückt, dann hochzählen
    {                           //
      wms (   10);              //
      while (TasteA_an)  {}     // Weiter nur mit gelöster Taste
      while (TasteA_aus) {}     //
      mnuptr ++;                //
      if (mnuptr ==  9) mnuptr = 1;
    }                           //
// . . .
// - - - - - - - - - - - - - - - -

**CrazyMetal** · 01.07.2013, 14:00

Zitat von oberallgeier

(..) ich mache genau das mit meinen #define´s.
Und ich zeige das jetzt nicht wegen der Besserwisserei - sondern nur um (m)eine Möglichkeit mit diesen gestuften #define´s aufzuzeigen.

Don't panic. Ich habe das auch nicht als Besserwisserei verstanden

und ich ziehe die Möglichkeit auch in Erwägung, dennoch wäre mir ein Struct schon sehr am Herzen gelegen...

**sternst** · 01.07.2013, 14:28

Zitat von CrazyMetal

Ich habe meine Funktion so geändert

Code:

struct LED
{
    volatile unsigned char *ddr;
    volatile unsigned char *port;
    volatile unsigned char *pin;
};

void InitLed(struct LED *led)
{
    *led->ddr |= 1<<(*led->pin);
}

void LedOn(struct LED *led)
{
    *led->port |= 1<<(*led->pin);
}

void main()
{
    struct LED led1 = {&DDRA, &PORTA, PA0};
    struct LED led2 = {&DDRA, &PORTA, PA1};
    
    InitLed(&led1);
    InitLed(&led2);
}

Denk nochmal genau nach, was 'pin' eigentlich sein soll.

**oberallgeier** · 01.07.2013, 15:32

Zitat von sternst

Denk nochmal genau nach, was 'pin' eigentlich sein soll.

Ach Mist, genau deswegen hatte ich noch mein Tastenbeispiel nachgeschoben - und habs trotzdem übersehen

**CrazyMetal** · 02.07.2013, 08:43

Hey sternst,
vielen Dank für den Schubser, der Wald und die vielen Bäume....

Code:

struct LED
{
    volatile unsigned char *ddr;
    volatile unsigned char *port;
    uint8_t pin;
};

void InitLed(struct LED *led)
{
    *led->ddr |= 1<<(*led).pin;
}

void LedOn(struct LED *led)
{
    *led->port |= 1<<(*led).pin;
}

void main()
{
    struct LED led1 = {&DDRA, &PORTA, PA0};
    struct LED led2 = {&DDRA, &PORTA, PA1};
    
    InitLed(&led1);
    InitLed(&led2);
}

Ich habe außerdem den Weg von oberallgeier probiert:

Code:

#define SetBit(ADDR,BIT) (ADDR |= (1<<BIT))
#define ClrBit(ADDR,BIT) (ADDR &= ~(1<<BIT))
#define TglBit(ADDR,BIT) (ADDR ^= (1<<BIT))

#define Led_1_An() SetBit(PORTA,PA0)
#define Led_1_Aus() ClrBit(PORTA,PA0)
#define Tgl_Led_1() TglBit(PORTA,PA0)

#define Led_2_An() SetBit(PORTA,PA1)
#define Led_2_Aus() ClrBit(PORTA,PA1)
#define Tgl_Led_2() TglBit(PORTA,PA1)

Bei meinem aktuellen Projekt werde ich die define-Variante verwenden, doch die Flexibilität der Structs ist mir für Projekte wichtig, wo ich während des Betriebs Pins konfigurieren können möchte ohne den Microcontroller neu programmieren zu müssen, außerdem bietet sich hier noch eine "verkettete Liste" an.

Viele Grüße,
Crazy