undefinierter Rückgabewert

[Thorsten (Gast)]

Hi Forum,

anbei ein kleiner Auszug aus meinem Code:

int Px (void)
{
 if (PIN_READ(PIND, PD6)) return 1;
 if (PIN_READ(PIND, PD5)) return 2;
 if (PIN_READ(PIND, PD4)) return 3;
}

int main (void)
{
 int Programmschalter = Px();

 switch (Programmschalter)
 {
  case 1: P1(val1, val3);
        break;
  case 2: P2(val0,val2);
        break;
  case 3: P3(val4, val5);
        break;
 }
}
P1, P2 & P3 führen für das Problem unwichtige Funktionen aus.
Port D hab ich als Eingang mit Pullup deklariert.
Das Problem ist dass die Funktion Px einen falschen Rückgabewert 
liefert, oder die Mehrfachauswahl die falsche Funktion ausführt.
Wenn ich case 1 und case 2 vertasche funktioniert dies, allerdings 
funktioniert case 3 dann immer noch nicht.


Das ganze Programm läuft auf einem ATMEGA32, programmiert wird über 
JTAGICE /MK2

Kann mir von euch jemand sagen wo das Problem liegt?

Danke

Thorsten

[Oliver (Gast)]

Kommt da keine Warnung vom Compiler? Wenn keiner der drei if's in Px 
wahr ist, was wird dann wohl zurückgegeben?

Und wer oder was ist PIN_READ?

Oliver

[Thorsten (Gast)]

Keine Compilerwarnung!

Wenn keine if wahr ist wird 0 zurück gegeben.

PIN_READ => bringt mir den Wert an dem Portpin.

[Stefan B. (stefan)]

@ Thorsten

Kannst du das auch für Laien erklären? Hat das Programm ein Problem und 
wenn ja welches?

> Das Problem ist dass die Funktion Px einen falschen
> Rückgabewert liefert,...

Hat das Programm immer noch das Problem, wenn du den berechtigten 
Einwand von Oliver berücksichtigst z.B. in dieser Form:

int Px (void)
{
 int ret = 0;

 if (PIN_READ(PIND, PD6)) ret = 1;
 else if (PIN_READ(PIND, PD5)) ret = 2;
 else if (PIN_READ(PIND, PD4)) ret = 3;

 return ret;
}

> ... oder die Mehrfachauswahl die falsche Funktion ausführt.

Woran machst du das fest, an einer falschen Reaktion auf eine Eingabe an 
Pin4/5 oder 6? Vielleicht macht die uns unbekannte Funktion oder das 
Makro PIN_READ() was anderes als du erwartest (Wert an Portpin bringen)?

[Rolf Magnus (Gast)]

> Wenn keine if wahr ist wird 0 zurück gegeben.

Das möchtest du vielleicht, aber so ist es nicht. Der zurückgegebene 
Wert ist undefiniert.

[Rahul D. (rahul)]

Sowas fängt man mit dem default-case ab!

>Das möchtest du vielleicht, aber so ist es nicht. Der zurückgegebene
>Wert ist undefiniert.

Genau.

[Peter D. (peda)]

Also wenn keine Warnung erfolgt, dann kann nur sein, daß dieses ominöse 
"PIN_READ" immer wahr ist und die Funktion "Px" zu einem "return 1" 
optimiert.


Es ist nicht gerade hilfreich, wenn man wichtige Funktion per Glaskugel 
eruieren muß.


Peter

[Oliver (Gast)]

../Main.c:16: warning: control reaches end of non-void function

Zumindest der gcc 4.1.1. meckert, selbst wenn PIN_READ immer wahr ist.

Oliver

[peter (Gast)]

Tja, die Funktion oder das Makro zu PIN_READ() musst Du schon auch 
posten...

Werden damit Taster oder Schalter abgefragt? Wie sieht's mit der 
Entprellung aus...?

Gruzzo

[Thorsten (Gast)]

sorry dass ich mich ne Weile nicht gemeldet hab, kleine PC-Probleme...

Es geht mir nicht darum was passiert wenn keine Bedingung wahr ist, denn 
meine Schaltung befindet sich momentan noch in der Entwicklungsphase und 
da hab ich immer eine der drei Bedingungen wahr.

dieses ominöse PIN_READ hab ich vergessen zu posten, hier die Definition 
dazu:

#define PIN_READ(PORT,PIN) (PORT)&(1<<(PIN));

Ich lasse mir mittlerweile zur Kontrolle meinen Rückgabewert auf meinem 
Display anzeigen und bin auf folgendes gestoßen:
wenn PD6 auf GND wird 3 zurückgegeben
wenn PD5 auf GND wird ebenfalls 3 zurückgegeben
wenn PD4 auf GND wird 2 zurückgegeben.

Sein soll es eigentlich so:

PD6 -> GND = Rückgabewert 3
PD5 -> GND = Rückgabewert 2
PD4 -> GND = Rückgabewert 1

[Stefan B. (stefan)]

Dann stimmt wohl die Hardware nicht. Wie wolltest du es aufbauen und wie 
hast du es aufgebaut?

[Thorsten (Gast)]

Die HW stimmt, wollte zum testen mitels einer Drahtbrücke, welche fest 
mit in meienen Texttoolsockel eingepresst wird den entsprechenden Pin 
auf GND ziehen und so mach ich es auch, wie gesagt, ist nicht die ganz 
feine Lösung, tuts aber zum versuchen...

[Rolf Magnus (Gast)]

> #define PIN_READ(PORT,PIN) (PORT)&(1<<(PIN));

So geht das bei dir durch den Compiler? Aus:

 if (PIN_READ(PIND, PD6)) ret = 1;

wird damit ja:

 if ((PID)&(1<<(PD6));) ret = 1;

Das sollte eigentlich eine Compiler-Fehlermeldung produzieren. Außerdem 
wolltest du vermutlich eher ein ~(1<<(PD6)) statt (1<<(PD6)).

> if (PIN_READ(PIND, PD6)) return 1;
> if (PIN_READ(PIND, PD5)) return 2;
> if (PIN_READ(PIND, PD4)) return 3;

> Sein soll es eigentlich so:
>
> PD6 -> GND = Rückgabewert 3
> PD5 -> GND = Rückgabewert 2
> PD4 -> GND = Rückgabewert 1

Was denn nun?

PS: Du liest so übrigens den Port dreimal nacheinander aus. Es wäre 
besser, ihn nur einmal zu lesen. Falls sonst eine Änderung genau 
zwischen zwei Lesezugriffen kommt, passiert irgendwas unerwartetes.

[Stefan B. (stefan)]

Die internen Pullups in dem bisher ungenannten AVR sind eingeschaltet?

[123 (Gast)]

> PS: Du liest so übrigens den Port dreimal nacheinander aus. Es wäre
> besser, ihn nur einmal zu lesen. Falls sonst eine Änderung genau
> zwischen zwei Lesezugriffen kommt, passiert irgendwas unerwartetes.
Er liest den Port nur dreimal aus, wenn kein Pin auf GND liegt, sonst 
kommt er gar nicht zum mehrfachen Auslesen, da das dazwischengepfuschte 
Return das verhindert. Soll heissen: Return immer nur am Ende einer 
Funktion - möglichst kein verfrühtes rausspringen.

Also der Code strotzt nur so von Nebenläufigkeiten und Unsauberkeiten.

[Thorsten (Gast)]

Pullups sind aktiviert.

> Sein soll es eigentlich so:
>
> PD6 -> GND = Rückgabewert 3
> PD5 -> GND = Rückgabewert 2
> PD4 -> GND = Rückgabewert 1

sollte natürlich heisen:

 PD6 -> GND = Rückgabewert 1
 PD5 -> GND = Rückgabewert 2
 PD4 -> GND = Rückgabewert 3

Der Compiler meckert nicht.

Was bewirkt ~(1<<(PD6))

Eigentlich will ich nur abfragen welcher der drei Pins auf Masse liegt 
und je nachdem ein zugewiesenes Unterprogramm ausführen.


ich häng mal mein komplettes Programm dazu, das Problem befindet sich 
relativ weit unten, bei der switch Anweisung....

[Karl H. (kbuchegg)]

Thorsten wrote:
> Pullups sind aktiviert.
>
> Was bewirkt ~(1<<(PD6))

Die Tilde dreht alle Bits um
In deinem Fall ist es nicht notwendig. Du willst
ja mit der UND Verknüpfung nur dieses eine interessierende
Bit herausmaskieren. Also muss auch dieses 1 Bit auf 1
stehen.

#define PIN_READ(PORT,PIN) (PORT)&(1<<(PIN))

war schon (fast) richtig.
Was du dir aber auf jeden Fall angewöhnen musst, ist rund
um Makros noch eine zusätzliche Klammer machen:

#define PIN_READ(PORT,PIN) ((PORT)&(1<<(PIN)))

ansonsten kann das zu bösen Überraschungen in der
Operatorenreihenfolge führen.

>
> Eigentlich will ich nur abfragen welcher der drei Pins auf Masse liegt

Dann solltest du das auch genau so formulieren wie du es
hier geschrieben hast. Auf Masse liegen <==> Pin ist 0

Ergo:

  if (PIN_READ(PIND, PD6) == 0)
    ret = 1;

Dazu muss aber im Makro das zusätzliche Klammernpaar vorhanden
sein. Ansonsten kommt es bereits zu den angekündigten
'bösen Überraschungen'

Ohne Klammern:

  if (PIN_READ(PIND, PD6) == 0)

das Makro eingesetzt:

  if ( (PIND)&(1<<(PD6)) == 0 )

nur blöderweise bindet == stärker als &, sodass der Compiler
dies als

  if( PIND & ( ( 1 << PD6 ) == 0 ) )

auswertet. Nicht ganz das was du willst.

------

Am allerbesten ist es aber, wenn du dir diesen Makro-Quatsch
komplett abgewöhnst:

unsigned char IsPinSet( volatile unsigned char* Port, unsigned char Pin)
{
  if( *Port & ( 1 << Pin ) )
    return 1;
  return 0;
}

bzw

unsigned char IsPinCleared( volatile unsigned char* Port, unsigned char Pin)
{
  return !IsPinSet( Port, Pin );
}

dann brauchst du die ganze Funktion und den 'Programmschalter'
überhaupt nicht mehr und schreibst ganz einfach:

  if( IsPinCleared( PIND, PD6 ) )
    P1(result1, result3);

  else if( IsPinCleared( PIND, PD5 ) )
    ledenable = P2(result0, result2, ledenable, result1, result3);

  else if( IsPinCleared( PIND, PD4 ) )
    timeenable = P3(result0, result2, timeenable, result1, result3);

"Say what you mean"

[Rolf Magnus (Gast)]

> Was bewirkt ~(1<<(PD6))

Es invertiert den Wert bitweise. Stimmt aber auch nicht. Da hab ich wohl 
Blödsinn geschrieben. Hier mal eine Liste, was rauskommt, wenn jeweils 
einer der drei Pins auf Masse liegt und du auf Pin 4 abfragst:

654   654
110 & 001 = 000   -> false
101 & 001 = 001   -> true
011 & 001 = 001   -> true

110 & 110 = 110   -> true
101 & 110 = 100   -> true
011 & 110 = 010   -> true

Man sieht schon, daß es heißen müßte:

#define PIN_READ(PORT,PIN) !((PORT)&(1<<(PIN)))

@123:

> Er liest den Port nur dreimal aus, wenn kein Pin auf GND liegt, sonst
> kommt er gar nicht zum mehrfachen Auslesen, da das dazwischengepfuschte
> Return das verhindert. Soll heissen: Return immer nur am Ende einer
> Funktion - möglichst kein verfrühtes rausspringen.

Darum ging es mir ja nicht. Der Port sollte immer nur genau einmal 
ausgelesen werden, also etwa:

unit8_t portd = PORTD;
if (PIN_READ(portd, PD6)) return 1;
if (PIN_READ(portd, PD5)) return 2;
if (PIN_READ(portd, PD4)) return 3;
return -1; /* Fehlerfall */

Der Code ist damit übrigens auch etwas schneller und kompakter, wenn 
auch nicht viel.
Außerdem gibt's keine Probleme wegen mehrerer return-Statements.

[Karl H. (kbuchegg)]

Rolf Magnus wrote:

> Darum ging es mir ja nicht. Der Port sollte immer nur genau einmal
> ausgelesen werden, also etwa:
>
>

> unit8_t portd = PORTD;
> if (PIN_READ(portd, PD6)) return 1;
> if (PIN_READ(portd, PD5)) return 2;
> if (PIN_READ(portd, PD4)) return 3;
> return -1; /* Fehlerfall */
>

>


Sein Pins sind ja low-aktiv.
Ergo:

unit8_t portd = PORTD;
if (PIN_READ(portd, PD6) == 0) return 1;
if (PIN_READ(portd, PD5) == 0) return 2;
if (PIN_READ(portd, PD4) == 0) return 3;
return -1; /* Fehlerfall */

[Stefan B. (stefan)]

Ist das der ursprüngliche Code? Ich glaube da sind ein paar Sachen aus 
der Diskussion reingepfriemelt und zwar falsch reingepfriemelt

/*Programmschalterauswahl*/
  int Px(void)
  {
    int ret = 0;
    if(PIN_READ(PIND, PD6)) ret = 1;
    if(PIN_READ(PIND, PD5)) ret = 2;
    if(PIN_READ(PIND, PD4)) ret = 3;

    return ret;
  }//end Programmschalter

Das hat eine andere Funktionalität als dein ursprüngliches Beispiel, 
wenn mehrere Pins gleichzeitig "aktiv" sind.

4 5 6 alt neu
=============
0 1 1  1   2
0 0 1  1   1
0 0 0  ?   0
1 0 1  1   3
1 0 0  3   3
1 1 0  2   3
0 1 0  2   2


Du willst abfragen, ob ein Pin an Port auf GND gezogen ist, d.h. 0 ist. 
Das geht entweder so

if ((PORTD & (1<<(PD6)) == 0) ...

oder

if (!(PORTD & (1<<(PD6))) ...

oder

if (!(PIN_READ(PIND, PD6))) ...

Das

#define PIN_READ(PORT,PIN) (PORT) & ~(1 << (PIN))

funktioniert so nicht. Du solltest nichts übernehmen, was du nicht 
verstehst. Das

#define PIN_READ(PORT,PIN) (PORT) & (1 << (PIN))

war schon halbwegs OK. Sicherer ist es mit zusätzlichen Klammern

#define PIN_READ(PORT,PIN) ((PORT) & (1 << (PIN)))

Dann geht auch

if (!PIN_READ(PIND, PD6)) ...

Mit der Abfrage auf 0 und else Schachtelung (Priorität 6 > 5 > 4)

4 5 6 neu2
==========
0 1 1  3
0 0 1  2
0 0 0  1
1 0 1  2
1 0 0  1
1 1 0  1
0 1 0  1

[Thorsten (Gast)]

@ Rolf Magnus

>Man sieht schon, daß es heißen müßte:
>#define PIN_READ(PORT,PIN) !((PORT)&(1<<(PIN)))

So funktionierts wie ich es mir vorgestellt hab.

Besten Dank

[Peter D. (peda)]

Karl heinz Buchegger wrote:
> Am allerbesten ist es aber, wenn du dir diesen Makro-Quatsch
> komplett abgewöhnst:
>
>

> unsigned char IsPinSet( volatile unsigned char* Port, unsigned char Pin)
> {
>   if( *Port & ( 1 << Pin ) )
>     return 1;
>   return 0;
> }
>

Oh, oh.

Wenn man den Code unnötig aufblähen und verlangsamen will, kann man es 
so machen.

Warum willst Du konstante Ausdrücke erst umständlich zur Laufzeit 
berechnen lassen ?


Der wesentlich effizientere und besser lesbare Weg, ist die Verwendung 
von Bitvariablen:

#include <avr\io.h>
#include <inttypes.h>


struct bits {
  uint8_t b0:1;
  uint8_t b1:1;
  uint8_t b2:1;
  uint8_t b3:1;
  uint8_t b4:1;
  uint8_t b5:1;
  uint8_t b6:1;
  uint8_t b7:1;
} __attribute__((__packed__));


#define SBIT(port,pin) ((*(volatile struct bits*)&port).b##pin)


#define TASTE1          SBIT(PIND, 6)
#define TASTE2          SBIT(PIND, 5)
#define TASTE3          SBIT(PIND, 4)


uint8_t Px (void)
{
 if( TASTE1 == 0 )
   return 1;
 if( TASTE2 == 0 )
   return 2;
 if( TASTE3 == 0 )
   return 3;
 return 0;
}

Macros sind kein Quatsch, man kann damit die Lesbarkeit wesentlich 
steigern.


Peter

[Karl H. (kbuchegg)]

Peter Dannegger wrote:
> Karl heinz Buchegger wrote:
>> Am allerbesten ist es aber, wenn du dir diesen Makro-Quatsch
>> komplett abgewöhnst:
>>
>>

>> unsigned char IsPinSet( volatile unsigned char* Port, unsigned char Pin)
>> {
>>   if( *Port & ( 1 << Pin ) )
>>     return 1;
>>   return 0;
>> }
>>

>
> Oh, oh.
>
> Wenn man den Code unnötig aufblähen und verlangsamen will, kann man es
> so machen.
>
> Warum willst Du konstante Ausdrücke erst umständlich zur Laufzeit
> berechnen lassen ?
>

Weil ich dem Compiler, speziell dem Optimizer, vertraue.
Zur Not markiere ich diese Funktion noch als inline-Funktion
und der Punkt 'umständlich zur Laufzeit ausrechnen' ist vom
Tisch.

>
> Macros sind kein Quatsch, man kann damit die Lesbarkeit wesentlich
> steigern.

Mit Funktionen geht das noch viel besser, ohne dass man in
die Fallen wie Doppelauswertung bzw. falsche Operatorenreihenfolge
infolge vergessener Klammern im Makro hineinläuft.

In Zeiten, in denen Compiler routinemässig funktionsinlineing
betreiben, bzw. dazu gezwungen werden können, haben Funktions-
makros weitgehend ihre Daseinsberechtigung verloren. Sie
bringen gegenüber einer Funktion keine Vorteile aber jede
Menge Nachteile bzw. Fallstricke.

[Peter D. (peda)]

Karl heinz Buchegger wrote:

> Mit Funktionen geht das noch viel besser, ohne dass man in
> die Fallen wie Doppelauswertung bzw. falsche Operatorenreihenfolge
> infolge vergessener Klammern im Makro hineinläuft.


Jetzt verrate mir doch mal, warum das mit Funktionen besser sein soll.

An dem Macro ist überhaupt nichts problematisch.

Aber ich kann viel besser lesbaren Text erzeugen, weil ich die 
Bitvariable direkt vergleichen oder ihr ne 1 oder 0 zuweisen kann.

Mit Funktionen bräuchte ich 4 unterschiedliche (0,1 testen bzw. 
zuweisen) und müßte immer umständlich 2 Argumente übergeben (Adresse, 
Bitnummer).

Die Bits kann ich dagegen ganz bequem einmal im h-File definieren und 
überall verwenden.

Auch widerstrebt es mir, Funktionen in einem h-File zu definieren.
Das muß man aber, wenn man sie in mehreren Objekten braucht.
Objektübergreifendes Inlining geht ja nicht.


Ich gebe daher der besseren Lesbarkeit und wesentlich geringerem 
Schreibaufwand eindeutig den Vorzug.


Ich hab früher mal &= und |= verwendet, als ich noch nicht wuste, daß 
auch der AVR-GCC Bitvariablen kann.

Ich hab dann mal ein altes Programm auf Bitvariablen umgeschrieben und 
es ist sogar deutlich weniger Code geworden.


Peter

[Frank J. (frajo)]

Diese Bit test Funktionen sind doch schon in der avr-libc als Makros 
definiert. Aus avr-libc 1.4.5 avr/io.h -> avr/sfr_defs.h :

IO register bit manipulation

#define   bit_is_set(sfr, bit)   (_SFR_BYTE(sfr) & _BV(bit))
#define   bit_is_clear(sfr, bit)   (!(_SFR_BYTE(sfr) & _BV(bit)))
#define   loop_until_bit_is_set(sfr, bit)   do { } while (bit_is_clear(sfr, bit))
#define   loop_until_bit_is_clear(sfr, bit)   do { } while (bit_is_set(sfr, bit))

Auch hier mit zusäztlichen Klammern und ohne ; am Ende.

[Karl H. (kbuchegg)]

Peter Dannegger wrote:
> Karl heinz Buchegger wrote:
>
>> Mit Funktionen geht das noch viel besser, ohne dass man in
>> die Fallen wie Doppelauswertung bzw. falsche Operatorenreihenfolge
>> infolge vergessener Klammern im Makro hineinläuft.
>
>
> Jetzt verrate mir doch mal, warum das mit Funktionen besser sein soll.
>
> An dem Macro ist überhaupt nichts problematisch.

#define MAX(a,b) ((a) > (b) ? (a) : (b))

   MAX( i++, j++)

nenne ich schon problematisch.

unsigned char Max( unsigned char a, unsigned char b )
{
  return a > b ? a : b;
}

Gut in C gibt es dafür keine schön generische Lösung, weil
es keine Templates gibt, aber in anderen Fällen steht eine
Funktionslösung einem Makro in nichts nach.

>
> Aber ich kann viel besser lesbaren Text erzeugen, weil ich die
> Bitvariable direkt vergleichen oder ihr ne 1 oder 0 zuweisen kann.

Das kann ich mit Funktionen ebenfalls erreichen.

>
> Mit Funktionen bräuchte ich 4 unterschiedliche (0,1 testen bzw.
> zuweisen) und müßte immer umständlich 2 Argumente übergeben (Adresse,
> Bitnummer).

Was ist dir lieber:
Klarheit beim Aufrufer oder ein paar Tastendrücke einsparen.

>
> Die Bits kann ich dagegen ganz bequem einmal im h-File definieren und
> überall verwenden.
>

Nichts dagegen einzuwenden

> Auch widerstrebt es mir, Funktionen in einem h-File zu definieren.
> Das muß man aber, wenn man sie in mehreren Objekten braucht.

Selbiges machst du mit einem Makro genauso. Das ist also
ein 0-Argument.

>
> Ich gebe daher der besseren Lesbarkeit und wesentlich geringerem
> Schreibaufwand eindeutig den Vorzug.

Ich gebe im Zweifelsfall der Lesbarkeit und der Wiedererkennbarkeit
den Vorzug. Weniger Tippaufwand ist selten ein gutes Argument :-)

  if( IsBitSet( PIND, PD6 ) )

ist für mich um einiges leichter zu entschlüsseln als

  if( PIND & ( 1 << PD6 ) )

Der Sinn des ersten erschliest sich beim Drüberlesen. Der
Code ist sein eigener Kommentar.
Beim zweiten muss ich erst mal schauen: Ist da jetzt ein & oder
ein |? Sind irgendwelche ! oder ~ im Spiel? Ja nachdem, welche
Anhäufung von Sonderzeichen vorliegt, ist das eine Abfrage eines
Pins auf high oder low oder aller anderen Pins ausser dem einen
auf high oder low.

Schreibe ich das ganze in eine Funktion und gebe der Funktion
einen vernünftigen Namen, dann brauche ich diese Überlegungen
nicht mehr: Der Funktionsname erzählt mir alles.

Und das beste daran: Nach dem inlinen ist der Code genauso
kompakt wie mit einem Makro.

Klar: Ich benutze auch die 2.te Form (aus Gewohnheit). Aber: Es
gibt keinen Grund dafür!
(Ich hab auch früher viiiiieeeeel schlimmere Dinge mit Makros
gelöst. Aber irgendwann steigt man auf C++ um und kommt drauf,
dass Funktionen die Sache dann doch vereinfachen. Vor allem
wenn man nach Jahren wieder in den Code reinsieht und vor lauter
Makro Expansionen nicht weiss wo man anfangen soll. Und eines
sollte man nie unterschätzen: Es gibt mit Makros ein paar wirklich
fiese Fallen!)

[Peter D. (peda)]

Karl heinz Buchegger wrote:

>> An dem Macro ist überhaupt nichts problematisch.
>
> #define MAX(a,b) ((a) > (b) ? (a) : (b))
>
>    MAX( i++, j++)
>
> nenne ich schon problematisch.


Und was hat das jetzt mit Bitvariablen zu tun ???


> Ich gebe im Zweifelsfall der Lesbarkeit und der Wiedererkennbarkeit
> den Vorzug. Weniger Tippaufwand ist selten ein gutes Argument :-)
>
>   if( IsBitSet( PIND, PD6 ) )
>
> ist für mich um einiges leichter zu entschlüsseln als
>
>   if( PIND & ( 1 << PD6 ) )
>
> Der Sinn des ersten erschliest sich beim Drüberlesen.


Wie es scheint, hast Du Dir meinen Beitrag überhaupt nicht angesehen.
Ich hab doch gesagt, das mit &= und |= war früher einmal.

Jetzt benutze ich:

#define TASTE_OK          SBIT(PIND, 6)
...
 if( TASTE_OK == 0 )

Und das ist doch viel einfacher und besser lesbar, als

if( IsBitSet( PIND, PD6 ) )

Ich benutze auch grundsätzlich keine festen Ports und Pins.
Woher weiß ich denn in größeren Quelltexten, was PIND.6 bedeutet.
Wenn da aber steht:

 if( TASTE_OK == 0 )

weiß ich genau bescheid.
Und wenn in ner anderen Schaltung andere Pins benutzt werden, muß ich 
das nur einmalig im define anpassen.


> Der
> Code ist sein eigener Kommentar.

Genau meine Meinung und genau deshalb nehme ich ja die Bitvaraiablen.


Peter

[Karl H. (kbuchegg)]

Peter Dannegger wrote:
> Karl heinz Buchegger wrote:
>
>>> An dem Macro ist überhaupt nichts problematisch.
>>
>> #define MAX(a,b) ((a) > (b) ? (a) : (b))
>>
>>    MAX( i++, j++)
>>
>> nenne ich schon problematisch.
>
>
> Und was hat das jetzt mit Bitvariablen zu tun ???

Gar nichts.
Aber sehr viel mit der Aussage:

>> Mit Funktionen geht das noch viel besser, ohne dass man in
>> die Fallen wie Doppelauswertung bzw. falsche Operatorenreihenfolge
>> infolge vergessener Klammern im Makro hineinläuft.

und die hast du ja schliesslich angegangen.
Und dieses Makro, das der Poster ursprünglich hatte

#define PIN_READ(PORT,PIN) (PORT)&(1<<(PIN));


hat sehr wohl ein Klammern-Problem (wenn ich den ; mal aussen vor lasse)

if( PIN_READ( PIND, PD6 ) == 0 )

macht nicht das Gewünschte, weil == in der Operatorenreihenfolge
vor & kommt.

> Jetzt benutze ich:
>

> #define TASTE_OK          SBIT(PIND, 6)
> ...
>  if( TASTE_OK == 0 )
>

>
> Und das ist doch viel einfacher und besser lesbar, als
>

> if( IsBitSet( PIND, PD6 ) )
>

kein Einwand.

>
> Ich benutze auch grundsätzlich keine festen Ports und Pins.
> Woher weiß ich denn in größeren Quelltexten, was PIND.6 bedeutet.
> Wenn da aber steht:
>

>  if( TASTE_OK == 0 )
>

> weiß ich genau bescheid.

Eben.
Und wenn da steht

   if( Taste_Enter_pressed() )

sehe ich das genau so gut.

> Und wenn in ner anderen Schaltung andere Pins benutzt werden, muß ich
> das nur einmalig im define anpassen.

Gegen derartige Definitionen hab ich auch nichts.

>
>
>> Der
>> Code ist sein eigener Kommentar.
>
> Genau meine Meinung und genau deshalb nehme ich ja die Bitvaraiablen.

Mir geht es darum, dass man bei Makros die mehr machen
als einfach nur ein paar Werte festzulegen, abschätzen soll,
ob eine Funktion nicht besser wäre. Nicht mehr, nicht weniger.

Ich werde dich nicht überzeugen und du wirst mich nicht
überzeugen. Also lassen wirs.