Frage zu Struktur für IO-Port bei AVRs

[Jan (Gast)]

Guten Abend,

ich bin gerade dabei, mit ein paar Standart-Funktionen und Strukturen 
für zukünftige 8 Bit AVR- Projekte anzulegen.
Habe mich jetzt mal durch die ganzen Header mit ettlichen defines des 
GCC gebissen und letztendlich eine Struktur entworfen, so wie ich es für 
richtig halte.

Nun hätte ich die Bitte, dass ihr mal kurz darüber schaut und mir ein 
Feedback gebt, ob ich die Header-Files richtig verstanden habe und die 
Struktur so passt.
Insbesondere bei den Makros setBit etc bin ich mir unsicher, da ich ja 
hier auf den Wert des Pointers zugreifen müsste?....

Vielen Dank schonmal.

Gruß Jan

Header:

#if defined (__AVR_ATtiny13A__)
  #define NUMPIN  5
#else
  #error "Wrong device selected, please adapt number of Pins."
#endif

#include <stdint.h>


#ifndef GPIO_H_
#define GPIO_H_

#define INPUT  0
#define OUTPUT  1

typedef struct
{
  uint8_t      *PR;        /* Address of Port-Register */
  uint8_t      *DR;        /* Address of Data-Direction-Register */
  uint8_t      Pin;        /* Pin-Number */
  uint8_t      Drctn[NUMPIN];    /* Array for Data-Direction of each Pin */
  uint8_t      PinCount;      /* Total number of Pins per Port */
}
GPIO;

#define setPin(PR,Pin)    ((*PR) |= (1<<(Pin)))
#define getPin(PR,Pin)    ((*PR) & (1<<(Pin)))
#define clearPin(PR,Pin)  ((*PR) &= ~(1<<(Pin)))
#define togglePin(PR,Pin)  ((*PR) ^= (1<<(Pin)))

#define setPort(PR)      ((*PR) = 0xFF)
#define getPort(PR)      ((*PR) & 0xFF)
#define clearPort(PR)    ((*PR) = 0x00)
#define togglePort(PR)    ((*PR) ^= 0xFF)

void GPIO_ini(GPIO *GPIO_Inst);

Und das Ini-File:

 #include "gpio.h"

void GPIO_ini(GPIO *GPIO_Inst)
{
  uint8_t i;
  
  for(i=0; i<=(GPIO_Inst->PinCount); i++)
     *(GPIO_Inst->DR) = (GPIO_Inst->Drctn[i]);
}

[Jan (Gast)]

Edit:

I

Jan schrieb:
> for(i=0; i<=(GPIO_Inst->PinCount); i++)
>
>      *(GPIO_Inst->DR) = (GPIO_Inst->Drctn[i]);

Sollte eigentlich

*(GPIO_Inst->DR) |= (GPIO_Inst->Drctn[i]);

heißen

[Jörg W. (dl8dtl) (Moderator)]

Jan schrieb:
> uint8_t      *PR;        /* Address of Port-Register */
>   uint8_t      *DR;        /* Address of Data-Direction-Register */

Die Zeiger müssen auf ein "volatile"-Objekt zeigen.

[Fabian O. (xfr)]

Du weißt schon, dass der ATtiny13A nur 64 Byte RAM hat? Ein einziges 
GPIO-Struct belegt davon schon 11 Byte, ohne dass es irgendeine 
Information enthält, die im RAM sein muss ...

[Jan (Gast)]

Ich zähle "nur" 10 Byte, wie kommst du auf 11?

Ja ist mir durchaus bewusst, aber die Anwendung für den Tiny ist auch 
nicht sehr Ressourcen-hungrig. Wird ein einfacher PWM-Dimmer.
Sicher kann man sowas auch ohne wilde Strukturen programmieren, mir ging 
es aber auch darum mal einen "Grundstock" für zukünftige Programme (die 
natürlich nicht auf dem Tiny laufen) zu legen. Dann bietet sich eine 
Modularisierung schon an.

@ Jörg: Danke für den Hinweis, hab ich im Eifer des Gefechts total 
übersehen.

[Ingo (Gast)]

Wozu blähst du einen AVR so auf? Halte für ich etwas overkilled...


Ingo

[Jan (Gast)]

Wie gesagt, meine Idee war die Software in verschiedene Schichten zu 
gliedern (GPIO gehört beispielsweise zur Hardware-Kapselung und muss 
controllerspezifisch angepasst werden). Das erleichtert die Portierung 
des Codes auf andere Zielsysteme.
Sicher wirkt das nun auf den ersten Blick total aufgebläht, aber ich 
verspreche mir davon Zeitersparnis bei größeren Projekten, da bei allen 
Schichten oberhalb der Hardware-Kapselung alle Schnittstellen gleich 
bleiben können.

[Wolfgang (Gast)]

Jan schrieb:
> Wie gesagt, meine Idee war die Software in verschiedene Schichten zu
> gliedern (GPIO gehört beispielsweise zur Hardware-Kapselung und muss
> controllerspezifisch angepasst werden). Das erleichtert die Portierung
> des Codes auf andere Zielsysteme.

Das kannst du auf deinem Entwicklungsrechner so betrachten.

> Sicher wirkt das nun auf den ersten Blick total aufgebläht, aber ich
> verspreche mir davon Zeitersparnis bei größeren Projekten, da bei allen
> Schichten oberhalb der Hardware-Kapselung alle Schnittstellen gleich
> bleiben können.

Stimmt, auf einem kleinen µC kannst du das getrost als Hintergrundwissen 
betrachten, wenn deine Entwicklungsumgebung das nicht selbständig für 
das Zielsystem ausreichend optimiert.
Auf Controllern, bei denen man schon mit Funktionsaufrufen sparen muß, 
damit der Stack nicht überläuft, gelten andere Gesetze.

[Peter D. (peda)]

Um gut lesbaren und portablen Code zu erzeugen, ist es einfacher, die 
Pins als Bitvariablen zu definieren:

#include "sbit.h"

#define LED0            PORT_B0
#define LED0_oe         DDR_B0          // output enable
#define LED1            PORT_B2
#define LED1_oe         DDR_B2          // output enable
#define KEY0_in         PIN_B1
#define KEY0_pu         PORT_B1         // Pulllup on
#define KEY1_in         PIN_B4
#define KEY1_pu         PORT_B4         // Pulllup on


int main()
{
  LED0_oe = 1;          // output
  LED1_oe = 1;          // output
  KEY0_pu = 1;          // pullup on
  KEY1_pu = 1;          // pullup on

  for(;;){
    if( KEY0_in == 0 && KEY1_in == 1 ){
      LED0 = 1;
      LED1 = 0;
    }
    if( KEY1_in == 0 && KEY0_in == 1 ){
      LED0 = 0;
      LED1 = 1;
    }
  }
}

Das ist nicht nur gut lesbar, sondern erzeugt auch sehr effizienten Code 
(siehe lst-File).

[Fabian O. (xfr)]

Oder man schreibt sich ein paar Headerdateien mit Inline-Funktionen. Die 
kann man einfach austauschen, wenn man das Programm auf eine andere 
Plattform portieren möchte. Wenn sich nur die Ports/Pins ändern, reicht 
es, die config-Datei zu ändern.

leds_config.h

#define LEDS_OUT PORTB
#define LEDS_DIR DDRB

#define LED_0 _BV(PINB0)
#define LED_1 _BV(PINB2)
#define LEDS_ALL LED_0 LED_1

keys_config.h

#define KEYS_IN     PINB
#define KEYS_PULLUP PORTB

#define KEY_0 _BV(PINB1)
#define KEY_1 _BV(PINB4)
#define KEYS_ALL KEY_0 KEY_1

keys.h

#include <avr/io.h>
#include "keys_config.h"

static inline void keys_init(void) {
  KEYS_PULLUP |= KEYS_ALL;
}

static inline bool keys_pressed(uint8_t key) {
  return KEYS_IN & key;
}

leds.h

#include <avr/io.h>
#inlude "leds_config.h"

static inline void leds_init(void) {
  LEDS_DIR |= LEDS_ALL;
}

static inline void leds_on(uint8_t led) {
  LED_OUT |= led;
}

static inline void leds_off(uint8_t led) {
  LED_OUT &= ~led;
}

main.c

#include "keys.h"
#include "leds.h"

int main()
{
  leds_init();
  keys_init();

  while(1) {
    if (key_pressed(KEY_0) && !key_pressed(KEY_1)) {
      leds_on(LED_0);
      leds_off(LED_1);
    }
    if (key_pressed(KEY_1) && !key_pressed(KEY_0)) {
      leds_on(LED_1);
      leds_off(LED_0);  
    }
  }
}

[Peter D. (peda)]

@Fabian O.

das geht nur solange gut, solange Du alles an einem Port hast 
(ATtiny13). Bei größeren MCs mit mehreren Ports hast Du trauer.

[Fabian O. (xfr)]

Peter Dannegger schrieb:
> Bei größeren MCs mit mehreren Ports hast Du trauer.

Dafür hast Du Trauer, wenn Du die GPIOs mit einem Funktionsaufruf statt 
per Speicherzugriff steuern musst ... ;)

Aber Du hast schon recht, wenn man mehrere Ports mit der gleichen 
Inline-Funktion bedienen will/muss, ist es nicht mehr so hübsch. Aber 
auch nicht unmöglich. Ich hatte genau diese Situation in einem Projekt, 
bei dem eine Status-LED in einer neuen Hardwareversion an einen anderen 
Port gewandert ist.

Die Datei sah dann so aus:

#define LEDS_SHIFT_A 2
#define LEDS_SHIFT_R 0

#define LED_GREEN  (PIN0_bm << LEDS_SHIFT_A) // PA0
#define LED_YELLOW (PIN0_bm << LEDS_SHIFT_R) // PR0
#define LED_RED    (PIN1_bm << LEDS_SHIFT_R) // PR1

#define LEDS_ALL_A  LED_GREEN
#define LEDS_ALL_R (LED_YELLOW | LED_RED)
#define LEDS_ALL   (LED_GREEN | LED_YELLOW | LED_RED)

static inline void leds_init(void)
{
  PORTA.DIRSET = LEDS_ALL_A >> LEDS_SHIFT_A;
  PORTA.OUTCLR = LEDS_ALL_A >> LEDS_SHIFT_A;
  PORTR.DIRSET = LEDS_ALL_R >> LEDS_SHIFT_R;
  PORTR.OUTCLR = LEDS_ALL_R >> LEDS_SHIFT_R;
}

static inline void leds_on(uint8_t mask)
{
  uint8_t bits;
  bits = (mask & LEDS_ALL_A) >> LEDS_SHIFT_A;
  if (bits) {
    PORTA.OUTSET = bits;
  }
  bits = (mask & LEDS_ALL_R) >> LEDS_SHIFT_R;
  if (bits) {
    PORTR.OUTSET = bits;
  }
}

static inline void leds_off(uint8_t mask)
{
  uint8_t bits;
  bits = (mask & LEDS_ALL_A) >> LEDS_SHIFT_A;
  if (bits) {
    PORTA.OUTCLR = bits;
  }
  bits = (mask & LEDS_ALL_R) >> LEDS_SHIFT_R;
  if (bits) {
    PORTR.OUTCLR = bits;
  }
}

static inline void leds_toggle(uint8_t mask)
{
  uint8_t bits;
  bits = (mask & LEDS_ALL_A) >> LEDS_SHIFT_A;
  if (bits) {
    PORTA.OUTTGL = bits;
  }
  bits = (mask & LEDS_ALL_R) >> LEDS_SHIFT_R;
  if (bits) {
    PORTR.OUTTGL = bits;
  }
}

Dank Inlining und Constant Folding bleibt von den Funktionen bei Aufruf 
mit konstantem Parameter nur noch das reine Ändern des Ports übrig. 
Weiterer Vorteil: Man kann ohne Overhead mehrere LEDs gleichzeitig 
ändern.

[Stefan (Gast)]

Kennst du diese Makros?:

// Macro used to write to a single I/O pin
#define writeBit(port,bit,value) { if ((value)>0) (port) |= (1<<bit); else (port) &= ~(1<<bit); } 

// Macro used to read from a single I/O pin
#define readBit(port,bit) (((port) >> (bit)) & 1)

// how to use with pin PB3
writeBit(PORTB,3,0);
writeBit(PORTB,3,1);

uint8_t a=readBit(PORTB,3);

if (readBit(PORTB,3)) {
  ...
}

Die sind zwar nicht ganz so portabel, wie deine Strukturen, haben dafür 
aber den Vorteil, dass sie kein RAM und keinen zusätzlichen Code 
erzeugen (da der Compiler das sehr gut optimiert).

[Stefan (Gast)]

Sorry, es muss natürlich readBit(PINB,3) heissen.