Wie viel "freier" Code im Programm?

[Ingo (Gast)]

Was meint ihr, sollte man jeden Codeschnippsel, sei es eine Zeile oder 
Befehl, in eine Funktion packen?

Zum Bsp.

 ToggleLed();

Anstatt

Portx ^= (1<<LED_PIN);

Wobei Zweiteres der Inhalt der Funktion darstellt. Was gehört zum "Guten 
Ton"?
Geschwindigkeit sollte das selbe sein, der Compiler wird sowas wohl 
sowieso inlinen.


Ingo

[Eumel (Gast)]

Ingo schrieb:
> Geschwindigkeit sollte das selbe sein, der Compiler wird sowas wohl
> sowieso inlinen.

Ja?

[Dr. Sommer (Gast)]

Ingo schrieb:
> Was meint ihr, sollte man jeden Codeschnippsel, sei es eine Zeile oder
> Befehl, in eine Funktion packen?
wenn dieser Schnipsel oft (ab 3x ;-) ) vorkommt und sich die 
theoretische Möglichkeit ergibt, diesen eventuell mal ändern zu wollen 
(zB weil man die LED an einen anderen Pin hängen will) - Ja.
Um sicherzustellen dass das tatsächlich inlined wird kann man noch ein 
"inline" an die Funktionsdeklaration dranschreiben.

[Claus M. (energy)]

Weder, noch.

Profis (wie ich ;-)) würden das so machen:

union blabla {
  int i16bit;
  struct {
        unsigned led1:1;
        unsigned led2:1;
        unsigned led3:1;
        unsigned reserved:13;
    } bits;
};


blabla* LEDport = 0xadressevonport;



und dann im Code:

LEDport.bits.led1 ^=1;

[Dr. Sommer (Gast)]

> Profis (wie ich ;-)) würden das so machen:
Hat leider den kleinen Nachteil, dass man die Zuordnung Led<->Port nur 
schwer ändern kann; Led<->Pin geht aber.

[Simon K. (simon)]

Claus M. schrieb:
> Weder, noch.
>
> Profis (wie ich ;-)) würden das so machen:

Alle Profis, bis auf die, die den C Standard kennen, der nämlich sagt, 
dass eine Union immer NUR über dasjenige Element gelesen werden darf, 
über das sie zuvor beschrieben wurde.

[Claus M. (energy)]

Simon K. schrieb:
> Union immer NUR über dasjenige Element gelesen werden darf,
>
> über das sie zuvor beschrieben wurde.

und wo mache ich was anderes???

[Claus M. (energy)]

Dr. Sommer schrieb:
> Hat leider den kleinen Nachteil, dass man die Zuordnung Led<->Port nur
>
> schwer ändern kann;


warum, einfach die adresse ändern.

[Max D. (max_d)]

Warum nicht einfach ein Makro (= #define) nehmen ?

[Simon K. (simon)]

Claus M. schrieb:
> Simon K. schrieb:
>> Union immer NUR über dasjenige Element gelesen werden darf,
>>
>> über das sie zuvor beschrieben wurde.
>
> und wo mache ich was anderes???

Gegenfrage: Wo schreibst du mit der Union in den Speicher? Nirgendwo.
Du benutzt also ein unbekanntes Speicherlayout der Union um Sachen aus 
dem Speicher zu lesen.

[MaWin (Gast)]

> Profis (wie ich ;-)) würden das so machen:

Vorausgesetzt, der Port ist überhaupt über Speicheradressen ansprechbar 
und benötigt nicht spezielle Instruktionen wie IN und OUT.

> Geschwindigkeit sollte das selbe sein, der Compiler wird sowas wohl
> sowieso inlinen.

Na ja, besser nicht bei space optimierung statt speed optimierung.

Man sollte prinzipell nichts doppelt schreiben, weil man es dann doppelt 
ändern muß.

In Assembler lohnt sich eine Funktion für deine Anweisungen schon wenn 
sie nur 2 mal verwendet wird, in Hochsprachen ist es manchmal 
zweifelhaft aber was kann der Programmierer für schlechte Kompiler.

Aber wenn sie nur ein mal aufgerufen wird, ist eine Funktion (auch wenn 
sie ge-inline-t wird) eher blöd: Man muß sie suchen (denn aus TogglePort 
geht nun wirklich nicht hervor welches) und im Quelltext vom Lesefluss 
weg woanders hin springen

[Thomas E. (thomase)]

Claus M. schrieb:
> Profis (wie ich ;-)) würden das so machen:
Du bist ja sogar ein doppelter Profi. 16-Bit-Integer, um einen 
8-Bit-Port zu beschreiben. Respekt.

#define LED_PORT PORTA
#define LED_TOGGLE PINA
#define LED_0 3

PORTA ^= (1 << LED_0);
     wird zu:
     43e:  82 b1         in  r24, 0x02  ; 2
     440:  98 e0         ldi  r25, 0x08  ; 8
     442:  89 27         eor  r24, r25
     444:  82 b9         out  0x02, r24  ; 2

LED_TOGGLE |= (1 << LED_0);
     wird zu:
     43e:  03 9a         sbi  0x00, 3  ; 0

LED_PORT |= (1 << LED_0);
     wird zu:
     43e:  13 9a         sbi  0x02, 3  ; 2

LED_PORT &= ~(1 << LED_0);
     wird zu:
     43e:  13 98         cbi  0x02, 3  ; 2

Jetzt rate mal, wie Profis das machen...

mfg.

[Claus M. (energy)]

Thomas Eckmann schrieb:
> Du bist ja sogar ein doppelter Profi. 16-Bit-Integer, um einen
>
> 8-Bit-Port zu beschreiben. Respekt.

Mit 8-Bit Hobbybastler-uC gebe ich mich halt normalerweise nicht ab ;-). 
Außerdem ging es um das Prinzip.


>Autor: Simon K. (simon) schrieb:
>Gegenfrage: Wo schreibst du mit der Union in den Speicher? Nirgendwo.
>Du benutzt also ein unbekanntes Speicherlayout der Union um Sachen aus
>dem Speicher zu lesen.

Ich glaube du hast es einfach immer noch nicht kapiert.

[Claus M. (energy)]

Es ging mir nicht um einen speziellen uC.

Und das hier:

Thomas Eckmann schrieb:
> LED_PORT &= ~(1 << LED_0);

ist hässlich wie ein Haufen Ka****

[W.S. (Gast)]

Ingo schrieb:
> Was meint ihr, sollte man jeden Codeschnippsel, sei es eine Zeile oder
> Befehl, in eine Funktion packen?

Nein. Nicht jeden.

Ob man das macht oder nicht, kommt auf die näheren Umstände an. 
Allgemein kann man jedoch sagen, daß ein zu heftiges Zerpflücken der 
Gesamtfunktion in Minischnipsel nicht gut ist, weil zu unleserlich und 
oft auch zu uneffektvem Code führt. Also mache lieber Funktionen von 
sinnvollem Umfang, weder Mammutfunktionen noch 1000 Minischnipsel.

W.S.

[Klaus (Gast)]

Claus M. schrieb:
> union blabla {
>   int i16bit;
>   struct {
>         unsigned led1:1;
>         unsigned led2:1;
>         unsigned led3:1;
>         unsigned reserved:13;
>     } bits;
> };

Kommt mir bekannt vor. In den PIC24 Headerfiles von Microchip findet 
man:

extern volatile unsigned int  LATA __attribute__((__sfr__));
typedef struct tagLATABITS {
  unsigned LATA0:1;
  unsigned LATA1:1;
  unsigned LATA2:1;
  unsigned LATA3:1;
  unsigned LATA4:1;
  unsigned :2;
  unsigned LATA7:1;
} LATABITS;
extern volatile LATABITS LATAbits __attribute__((__sfr__));

und ich schreib dann noch dazu

#define LED1 LATAbits.LATA0
.
.
LED1 ^= 1;

und wenn die LED 1 an einen anderen Port kommt, wird nur das define 
geändert.

MfG Klaus