Kann man diese Funktionen als Makro schreiben?

[Peter Z. (Gast)]

Hallo,
ich habe hier drei Funktionen, Port_set, Port_clr und Port_toggel.

//***************************************************************************
void Port_set(unsigned char port,unsigned char bit)
{
  bit += 16;
  switch(port)
  {
    case 0: GP0SET = 1 << bit;break;
    case 1: GP1SET = 1 << bit;break;
    case 2: GP2SET = 1 << bit;break;
    case 3: GP3SET = 1 << bit;break;
    case 4: GP4SET = 1 << bit;break;
  }
}
//***************************************************************************
void Port_clr(unsigned char port,unsigned char bit)
{
  bit += 16;
  switch(port)
  {
    case 0: GP0CLR = 1 << bit;break;
    case 1: GP1CLR = 1 << bit;break;
    case 2: GP2CLR = 1 << bit;break;
    case 3: GP3CLR = 1 << bit;break;
    case 4: GP4CLR = 1 << bit;break;
  }
}
//***************************************************************************
void Port_tgl(unsigned char port,unsigned char bit)
{
  bit += 16;
  switch(port)
  {
    case 0: GP0DAT ^= 1 << bit;break;
    case 1: GP1DAT ^= 1 << bit;break;
    case 2: GP2DAT ^= 1 << bit;break;
    case 3: GP3DAT ^= 1 << bit;break;
    case 4: GP4DAT ^= 1 << bit;break;
  }
}
//***************************************************************************

Wäre es nicht besser, das irgendwie mit #define als Makro zu schreiben 
und sich diesen umständlichen und CPU-Zeit fressenden Funktionsaufruf zu 
sparen?

[TestX .. (xaos)]

deklarier die funktionen als inline und schau dir an was dein compiler 
raus baut

[Roman (Gast)]

Makros sind für Schreibfaule.

[Peter Z. (Gast)]

Roman schrieb:
> Makros sind für Schreibfaule.
Ach?
Mir gehts mehr um die kostbare CPU-Zeit

[Karl H. (kbuchegg)]

Was sagt dein Profiling?
Ist die Zeit ein Problem?

Wenn nicht, dann lass es so.

Oder schreib dir Einzelmakros für jeden Port

Ob du an der verwendenden Stelle

    Port_tgl( 1, 5 );

schreibst, oder gleich

    GP1DAT ^= 1 << 5;


oder

    TOGGLE_BIT( GP1DAT, 5 );

ist gehupft wie gesprungen. Aber da du von alleine nicht auf die Idee 
kommst, lass es erst mal so, solange keine Notwendigkeit besteht.

[Peter Z. (Gast)]

Andi D. schrieb:
> deklarier die funktionen als inline und schau dir an was dein compiler
> raus baut

Der Compiler ist der Realview von Keil, der µC ist der ADµC7126.
Ok sorry hier ist das GCC-Forum, mein Fehler...

Wenn ich der Funktionsdefinition ein inline voranstelle gibt's 
Fehlermeldungen zuhauf...

[Peter Z. (Gast)]

Karl Heinz Buchegger schrieb:
> Ist die Zeit ein Problem?
ja

und außerdem wollte ich dieses

bit += 16;

wegen der Lesbarkeit raushaben.
Bei diesem ADµC ist es so:

Table 87. GPxSET MMR Bit Descriptions
Bit       Description
[31:24]   Reserved.
[23:16]   Data Port x set bit.Set to 1 by the user to set
          a bit on Port x;
          also sets the corresponding bit in the GPxDAT MMR.
          Cleared to 0 by the user; does not affect the data output.
[15:0]    Reserved


Im Code soll einfach nur sowas wie

   Port_set(3,7);
   Port_clr(3,7);
   
   Port_tgl(4,5);

stehen

[Karl H. (kbuchegg)]

Peter Zz schrieb:

> Im Code soll einfach nur sowas wie
>

>    Port_set(3,7);
>    Port_clr(3,7);
> 
>    Port_tgl(4,5);
>

> stehen

Das soll dort SO überhaupt nicht stehen. Die direkten Zahlen willst du 
DA nämlich überhaupt nicht haben.
Wenn schon, dann soll da stehen

   Port_set( LED_PORT, ERROR_LED );


Und ob da dann eine Funktion aufgerufen wird, oder ein Makro expandiert 
wird

#define LED_PORT    GP1DAT
#define ERROR_LED   5

#define BIT_SET(port,bit)  (port) |= ( 1 << ((bit) + 16)

....

   BIT_SET( LED_PORT, ERROR_LED );

reisst sich in der Verwendung nichts. Aber du hast das was du willst.

Und kauf dir ein C-Buch und arbeite es durch.

[Rolf Magnus (Gast)]

Peter Zz schrieb:
> Wenn ich der Funktionsdefinition ein inline voranstelle gibt's
> Fehlermeldungen zuhauf...

Ach, und der Compiler gibt kein weiteres Indiz dafür, um welche Fehler 
es sich handeln könnte oder wo die aufgetreten sind?

[Peter D. (peda)]

Peter Zz schrieb:
> Wenn ich der Funktionsdefinition ein inline voranstelle gibt's
> Fehlermeldungen zuhauf...

Finde ich auch sonderbar, warum immer derartige Aussagen mit null Inhalt 
gepostet werden.

Hat exakt die gleiche Aussagekraft wie dieser Wetterbericht:
"Auch heute ist im Verlaufe des Tages mit Wetter zu rechnen."

Ein simples copy&paste der ersten Fehlermeldung scheint ja höllisch 
kompliziert zu sein.


Peter

[Peter Z. (Gast)]

Peter Dannegger schrieb:
> Ein simples copy&paste der ersten Fehlermeldung scheint ja höllisch
> kompliziert zu sein.

Leider geht unter µVision3 ein "Copy and paste" nicht,
nur als Screenshoot, siehe Dateianhang

[Kan a. (Firma: Basta) (kanasta)]

Peter Zz schrieb:
> nur als Screenshoot, siehe Dateianhang

Wer zu blöd ist, die Fehlermeldung lesbar zu posten, hat es schwer, 
Hilfe zu bekommen.

[Peter D. (peda)]

Peter Zz schrieb:
> Leider geht unter µVision3 ein "Copy and paste" nicht,

Dann wäre das das erste Windowsprogramm, wo es nicht geht.

Außerdem werden Fehlermeldungen vom Compiler in ein Textfile 
geschrieben. Such mal im Projekt nach einem File, das diesen Text 
beinhaltet.


Peter

[Peter Z. (Gast)]

Peter Dannegger schrieb:
> Dann wäre das das erste Windowsprogramm, wo es nicht geht.

Ist z.B. AVRStudio 4.xx kein Windowsprogramm?


> Außerdem werden Fehlermeldungen vom Compiler in ein Textfile
> geschrieben. Such mal im Projekt nach einem File, das diesen Text
> beinhaltet.

Das werd ich machen.

Aber eigentlich sollte dieser Thread nicht in Richtung inline Funktion 
gehen, sondern mir geht es eher um Makros.

Ich habe jetzt:

#define Port_output(port,bit) (port) |= (1 << ((bit) + 24))
#define Bit_set(port,bit) (port) = (1 << ((bit) + 16))
#define Bit_clr(port,bit) (port) = (1 << ((bit) + 16))
#define Bit_tgl(port,bit) (port) ^= (1 << ((bit) + 16))

wobei die im Programm dann z.B. so stehen:

Port_output(GP4DAT,2);
Bit_set(GP4SET,2);
Bit_clr(GP4CLR,2);
Bit_tgl(GP4DAT,2);

dies funktioniert auch, allerdings würde ich das lieber ohne diesen 
Register-Namen (GPxDAT, GPxSET, GPxCLR) haben.
Das folgende Beispiel sollte das so machen, aber funktioniert nicht:

#define Port_output(port,bit) GP(port)DAT |= (1 << ((bit) + 24))
#define Bit_set(port,bit) GP(port)SET = (1 << ((bit) + 16))
#define Bit_clr(port,bit) GP(port)CLR = (1 << ((bit) + 16))
#define Bit_tgl(port,bit) GP(port)DAT ^= (1 << ((bit) + 16))

Fehlermeldung ist: Expected ";"

[Peter D. (peda)]

> error #77-d this declaration has no storage class or type specifier

Die Fehlermeldung sagt mir nichts, klingt aber interessant.
Google findet da einiges, muß irgendwie mit ARM7 und C++ zu tun haben.

Da ist wohl irgendne Schweinerei in den Macros GPxSET.


Peter

[Karl H. (kbuchegg)]

Peter Zz schrieb:

> wobei die im Programm dann z.B. so stehen:
>

> Port_output(GP4DAT,2);
> Bit_set(GP4SET,2);
> Bit_clr(GP4CLR,2);
> Bit_tgl(GP4DAT,2);
>

> dies funktioniert auch, allerdings würde ich das lieber ohne diesen
> Register-Namen (GPxDAT, GPxSET, GPxCLR) haben.

Du hast den entscheidenden Witz an der Sache

#define LED_PORT    GP1DAT
#define ERROR_LED   5

#define BIT_SET(port,bit)  (port) |= ( 1 << ((bit) + 16)

....

   BIT_SET( LED_PORT, ERROR_LED );

und warum das sehr viel besser ist als

    BIT_SET( 1, 5 );

noch nicht verstanden. Lass das mal auf dich wirken und überleg dir, 
worin wohl der programmtechnisch strategische Unterschied zwischen den 
folgenden beiden Zeilen besteht

    BIT_SET( LED_PORT, ERROR_LED );
    BIT_SET( 1, 5 );

[Karl H. (kbuchegg)]

> Das folgende Beispiel sollte das so machen,

Nein. Das sollte es nicht.

Wie schon geasgt: Du brauchst ein C-Buch.
Ohne wird das nichts

Für Puristen: Das wäre ein Einsatzfall für den Preprozessor Tokenize 
Operator ##

Aber wie schon ausgeführt: Es gibt eine WESENTLICH bessere Möglichkeit. 
Und die solltest du auch gehen! Dein Programmierstil und die Wartbarkeit 
deines Programmes wird es dir danken.

[Peter Z. (Gast)]

Peter Dannegger schrieb:
> Die Fehlermeldung sagt mir nichts, klingt aber interessant.
> Google findet da einiges, muß irgendwie mit ARM7 und C++ zu tun haben.

ich will dieses inline im Moment nicht vertiefen
Lieber die Makro's


> Da ist wohl irgendne Schweinerei in den Macros GPxSET.
Meinst du in dem von mir geschriebenen  Makros?

#define Port_output(port,bit) (port) |= (1 << ((bit) + 24))
#define Bit_set(port,bit) (port) = (1 << ((bit) + 16))
#define Bit_clr(port,bit) (port) = (1 << ((bit) + 16));
#define Bit_tgl(port,bit) (port) ^= (1 << ((bit) + 16))

Bei diesem ADµC7126 gibt es pro Port 5 verschiedene Register:
GPxCON
GPxDAT
GPxSET
GPxCLR
GPxPAR
Die sind tatsächlich vorhanden, keine Makros!

[Karl H. (kbuchegg)]

Peter Zz schrieb:

> Die sind tatsächlich vorhanden, keine Makros!


Das sind mit einiger Sicherheit irgendwelche Makros, mit denen die Namen 
auf Adresspointer umgemappt werden.
Die haben garantiert ihren Compiler nicht dahingehend erweitert, dass er 
diese Namen als Schlüsselwörter kennt.

[Peter Z. (Gast)]

Karl Heinz Buchegger schrieb:
> BIT_SET( LED_PORT, ERROR_LED );
>
> und warum das sehr viel besser ist als
>     BIT_SET( 1, 5 );
>
> noch nicht verstanden. Lass das mal auf dich wirken und überleg dir,
> worin wohl der programmtechnisch strategische Unterschied zwischen den
> folgenden beiden Zeilen besteht
>     BIT_SET( LED_PORT, ERROR_LED );
>     BIT_SET( 1, 5 );

Doch doch, schon verstanden, schon gewusst.
Ich würde dann sogar noch weiter gehen:

#define Error_LED_an  Bit_set(GP2SET,3);
#define Error_LED_aus Bit_clr(GP2CLR,3);

[Karl H. (kbuchegg)]

Karl Heinz Buchegger schrieb:
> Peter Zz schrieb:
>
>> Die sind tatsächlich vorhanden, keine Makros!
>
>
> Das sind mit einiger Sicherheit irgendwelche Makros, mit denen die Namen
> auf Adresspointer umgemappt werden.
> Die haben garantiert ihren Compiler nicht dahingehend erweitert, dass er
> diese Namen als Schlüsselwörter kennt.

Moment mal.
Da steckt noch mehr dahinter.

Wenn das hier

    case 3: GP3CLR = 1 << bit;break;

korrekt ist (das ist eine einfache Zuweisung!), dann muss da mehr 
dahinter sein.

Such doch mal deine System Include Files durch. Die Grossschreibung von 
GP3CLR weißt darauf hin, dass es sich da um ein Makro handelt. In 
irgendeinem System Header File wird es da einen #define dafür geben.

(Ich schätze mal, dass da eine ähnliche Technik dahinter steckt, wie die 
Bitunion, die PeDa gerne verwendet)

[Pic T. (pic)]

Alles ungeprüft, aber theoretisch:

#ifndef CONCAT
#define CONCAT(x,y)  x##y
#endif
#define Bit_Port_(x,y) (CONCAT(x,y))
#define Bit_Port(x,y) Bit_Port_(CONCAT(GP,y),x)
#define Bit_(u,v,x,y,z) (Bit_Port(u,x) CONCAT(v,=) ( 1 << ((y) + z)))

#define Bit_set(x,y) Bit_(SET,|,x,y,16)
#define Bit_clr(x,y) Bit_(CLR,|,x,y,16)
#define Bit_tgl(x,y) Bit_(DAT,^,x,y,16)

[Peter Z. (Gast)]

Karl Heinz Buchegger schrieb:
> Die Grossschreibung von
> GP3CLR weißt darauf hin, dass es sich da um ein Makro handelt.

Ne, schau mal ADuC7126 POrt-Register.pdf

[Karl H. (kbuchegg)]

Peter Zz schrieb:
> Karl Heinz Buchegger schrieb:
>> Die Grossschreibung von
>> GP3CLR weißt darauf hin, dass es sich da um ein Makro handelt.
>
> Ne, schau mal ADuC7126 POrt-Register.pdf


Uninteressant.
Beim AVR heissen die Dingeer im Datenblatt auch PORTA. Trotzdem ist 
PORTA in der Programmiersprache als #define realisiert.

Die Frage ist nicht, wie das am Prozessor heißt, die Frage ist: Wie ist 
es dem Compiler beigebracht worden! Das sind 2 Paar verschiedene Schuhe.

Der COmpiler kennt von Haus aus nur ein paar Schlüsselwörter
if, while, do, goto, int, long, double, float, struct, union, ... und 
natürlich die ganzen Operatoren.
Alles andere wird in den Systemheaderfiles aus diesen Bausteinen 
zusammengebaut.


Zum PDF.
Ein GP1SET könnte zb so in einem Header File stehen

#define GP1SET (*(volatile unsigned char*)0xFFFFF434)

könnte so sein. Muss aber nicht.

[Peter Z. (Gast)]

#define          GP0CON                                     (*(volatile unsigned long      *) 0xFFFFF400)
#define          GP1CON                                     (*(volatile unsigned long      *) 0xFFFFF404)
#define          GP2CON                                     (*(volatile unsigned long      *) 0xFFFFF408)
#define          GP3CON                                     (*(volatile unsigned long      *) 0xFFFFF40C)
#define          GP4CON                                     (*(volatile unsigned long      *) 0xFFFFF410)
#define          GP0DAT                                     (*(volatile unsigned long      *) 0xFFFFF420)
#define          GP0SET                                     (*(volatile unsigned long      *) 0xFFFFF424)
#define          GP0CLR                                     (*(volatile unsigned long      *) 0xFFFFF428)
#define          GP0PAR                                     (*(volatile unsigned long      *) 0xFFFFF42C)
#define          GP1DAT                                     (*(volatile unsigned long      *) 0xFFFFF430)
#define          GP1SET                                     (*(volatile unsigned long      *) 0xFFFFF434)
#define          GP1CLR                                     (*(volatile unsigned long      *) 0xFFFFF438)
#define          GP1PAR                                     (*(volatile unsigned long      *) 0xFFFFF43C)
#define          GP2DAT                                     (*(volatile unsigned long      *) 0xFFFFF440)
#define          GP2SET                                     (*(volatile unsigned long      *) 0xFFFFF444)
#define          GP2CLR                                     (*(volatile unsigned long      *) 0xFFFFF448)
#define          GP2PAR                                     (*(volatile unsigned long      *) 0xFFFFF44C)
#define          GP3DAT                                     (*(volatile unsigned long      *) 0xFFFFF450)
#define          GP3SET                                     (*(volatile unsigned long      *) 0xFFFFF454)
#define          GP3CLR                                     (*(volatile unsigned long      *) 0xFFFFF458)
#define          GP3PAR                                     (*(volatile unsigned long      *) 0xFFFFF45C)
#define          GP4DAT                                     (*(volatile unsigned long      *) 0xFFFFF460)
#define          GP4SET                                     (*(volatile unsigned long      *) 0xFFFFF464)
#define          GP4CLR                                     (*(volatile unsigned long      *) 0xFFFFF468)
#define          GP4PAR                                     (*(volatile unsigned long      *) 0xFFFFF46C)

So steht es in der Datei ADuC7126.h

[Karl H. (kbuchegg)]

pic tech schrieb:
> Alles ungeprüft, aber theoretisch:
>
> #ifndef CONCAT
> #define CONCAT(x,y)  x##y
> #endif
> #define Bit_Port_(x,y) (CONCAT(x,y))
> #define Bit_Port(x,y) Bit_Port_(CONCAT(GP,y),x)
> #define Bit_(u,v,x,y,z) (Bit_Port(u,x) CONCAT(v,=) ( 1 << ((y) + z)))
>
> #define Bit_set(x,y) Bit_(SET,|,x,y,16)
> #define Bit_clr(x,y) Bit_(CLR,|,x,y,16)
> #define Bit_tgl(x,y) Bit_(DAT,^,x,y,16)

Ich hab da, glaub ich, einen Fehler gemacht.

Wir von der AVR Fraktion sind daran gewöhnt, dass wir uns um die 
Bitverknüpfung selbst kümmern müssen. Bei diesem µC scheint es aber so 
zu sein, dass alleine die Zuweisung an zb ein xxxCLR Register reicht, um 
das "Bit Clearing" (schreckliches Wort) auszulösen.

D.h. die Operation braucht man gar nicht. Die ist implizit im 
'Register'-Namen schon drinnen.

So gesehen muss ich zurückrudern.
Die Sache mit dem Tokinize ist doch nicht so schlecht.

[Pic T. (pic)]

Ich habe es mal mit GCC ausprobiert, und eine kleine Änderung gemacht,
eventuell ist aber set sowie clr besser als dat für input/output zu 
verwenden.


#ifndef CONCAT
#define CONCAT(x,y)  x##y
#endif
#define Bit_Port_(x,y) (CONCAT(x,y))
#define Bit_Port(x,y) Bit_Port_(CONCAT(GP,y),x)
#define Bit_(u,v,x,y,z) (Bit_Port(u,x) v ( 1 << ((y) + z)))

#define Bit_set(x,y) Bit_(SET,|=,x,y,16)
#define Bit_clr(x,y) Bit_(CLR,|=,x,y,16)
#define Bit_tgl(x,y) Bit_(DAT,^=,x,y,16)
#define Bit_out(x,y) Bit_(DAT,|=,x,y,24)
#define Bit_inp(x,y) Bit_(DAT,&=~,x,y,24)



Bit_inp(4,2);
Bit_out(4,2);
Bit_set(4,2);
Bit_clr(4,2);
Bit_tgl(4,2);

/* output of gcc -E run:
# 1 "x.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "x.c"
# 16 "x.c"
((GP4DAT) &=~ ( 1 << ((2) + 24)));
((GP4DAT) |= ( 1 << ((2) + 24)));
((GP4SET) |= ( 1 << ((2) + 16)));
((GP4CLR) |= ( 1 << ((2) + 16)));
((GP4DAT) ^= ( 1 << ((2) + 16)));
*/

[Sam P. (Gast)]

Auf die einfachste Lösung ist noch keiner gekommen: Der Compiler kennt 
schlicht und ergreifend das Schlüsselwort 'inline' nicht.

Und es ist egal, da die korrekte Lösung, die das Anliegen des TO und die 
Belange der Wartbarkeit erfüllt, bereits gepostet wurde.


Was die Fehlermeldung angeht, wird da sicherlich folgendes passieren:

Der Compiler stößt in Zeile 114 auf das Wörtchen 'inline'. Die Zeile 
compilierte vorher fehlerfrei (nehme ich an), daher kann nur das 
'inline' der Verursacher sein. Nun ist dem Compiler dieses Schlüsselwort 
unbekannt, das ist ja auch kein Bestandteil älterer C-Standards. Also 
interpretiert er 'inline' als Deklaration einer Variablen eben dieses 
Namens. Und weil da kein Typ davor stand, kommt die Fehlermeldung "this 
declaration has no type". Stimmt ja auch.

Die restlichen Fehlermeldungen sind ohnehin Grütze, verlässilch ist ja 
immer nur die erste Fehlermeldung.

[Pic T. (pic)]

Oder expandiert mit den Defines der Ports:
(((*(volatile unsigned long *) 0xFFFFF460)) &=~ ( 1 << ((2) + 24)));
(((*(volatile unsigned long *) 0xFFFFF460)) |= ( 1 << ((2) + 24)));
(((*(volatile unsigned long *) 0xFFFFF464)) |= ( 1 << ((2) + 16)));
(((*(volatile unsigned long *) 0xFFFFF468)) |= ( 1 << ((2) + 16)));
(((*(volatile unsigned long *) 0xFFFFF460)) ^= ( 1 << ((2) + 16)));

Im produktiven Einsatz solltest du aber folgendes haben, ich lass jetzt
die ifdef weg:

#define CONCAT__(a,b) a##b
#define CONCAT_(a,b) CONCAT__(a,b)
#define CONCAT(a,b)  CONCAT_(a,b)

[Rolf Magnus (Gast)]

Sam P. schrieb:
> Auf die einfachste Lösung ist noch keiner gekommen: Der Compiler kennt
> schlicht und ergreifend das Schlüsselwort 'inline' nicht.

Mir ist diese Idee auch gekommen, aber erst nach dir. ;-)
Vermutlich wird der Compiler nicht ISO-C-konform sein, sondern nur 
dessen Vorgänger implementieren. Da gab es noch kein inline.

[Peter D. (peda)]

Ich verstehe nicht, was die ARM-Entwickler sich bei diesen umständlichen 
Set- und Clear-Registern bloß gedacht haben.
Da hat man nun 4 Miliarden Befehle, aber für ein paar Bitbefehle hats 
nicht mehr gereicht.

Jeden 8-Bitter ohne Bitbefehle würde man sofort in die Tonne kloppen. 
Aber nur weil 32 Bit ja so hip ist, toleriert man bei denen diesen Mist.

Ich bleib da lieber bei 8 Bit und kann dann bequem und gut lesbar 
schreiben:

bit = 0;
bit = 1;

Peter