einfacher C-Code -> Denkfehler??

[steve (Gast)]

Hallo Leute,

habe ich in meinem Code ein Denkfehler?
Ein Unterprogramm wird immer wieder in main aufgeruffen.

>>int main(void) {
>>    while(1) {
>>        Code();
>>    }
>>}

Wenn ich den Taster (E0,1) drücke, dann soll die LED (A0,3) angehen und 
auch wenn der Taster (E0,1) nicht mehr gedrückt wird soll die LED 
leuchten.

>>void Code(void){
>>    If(E0 & ( 1 << 1) ) {
>>        A0 |= ( 1 << 3 );
>>    }
>>}

Leider ist es so, dass sobald ich den Taster los lasse, wird auch die 
LED gelöscht.

Was mache ich falsch? Wo liegt mein Fehler? Ich teste das Programm auf 
STK500 (ATmega8)

Gruß

[Magnus Müller (Gast)]

steve schrieb:
> Was mache ich falsch? Wo liegt mein Fehler? Ich teste das Programm auf

Dein Fehler besteht darin, dass du nicht das komplette Programm postest. 
Sprich: der Fehler befindet sich in dem Teil, den du hier nicht zeigst.

Gruß,
Magnetus

[funky (Gast)]

gehts nur mir so, das ich 0x08 einleuchtender finde als 1 << 3 ??

[Vlad T. (vlad_tepesch)]

ich glaub schon.

optimalerweise sollte man statt 3 aber noch PIN3 schreiben.
oder findest du

  ADMX = 0xC7;

aussagekräfiger als folgendes:

  ADMUX  = (1<<REFS1) | (1<<REFS0)    /* internal reference */
         | (0<<ADLAR)                 /* rightadjust result */
         | (0<MUX3) | (1<<MUX2) | (1<<MUX1) | (1<<MUX0); /* Select  ADC-Pin7  */

[Maxxie (Gast)]

Ja,

allerdings mag ich hier gerne ein einfaches Makro:
#define BIT(n) (1 << (n))

dann wirds vernünftig lesbar.

[Vlad T. (vlad_tepesch)]

>Dein Fehler besteht darin, dass du nicht das komplette Programm postest.
>Sprich: der Fehler befindet sich in dem Teil, den du hier nicht zeigst.

Möglicherweise liegt der Fehler aber auch in deinem Versuchsaufbau.
Sicher das du die led nicht ausversehen direkt mit dem schalter 
schaltest, weil du irgend eine Brücke falsch gesetzt hast?


>allerdings mag ich hier gerne ein einfaches Makro:
>#define BIT(n) (1 << (n))
>dann wirds vernünftig lesbar.

Geschmackssache. Auf jeden Fall sollte man sich auf eine Variante 
festlegen und auf jeden Fall die von den avr-Headern bereitgestellten 
Kosntanten benutzen.

[Magnus Müller (Gast)]

funky schrieb:
> gehts nur mir so, das ich 0x08 einleuchtender finde als 1 << 3 ??

Ja.

"1 << 3" ist zwar auch nicht das Gelbe vom Ei, aber immer noch besser 
als "0x08".

Besser wäre es so:

#define DDR_LED    DDRA
#define PORT_LED   PORTA
#define LED0       (1<<0)
#define LED1       (1<<1)
#define LED2       (1<<2)
#define LED3       (1<<3)

void main (void)
{
   DDR_LED = LED0|LED1|LED2|LED3;
   PORT_LED = 0;

   // tu irgendwas...

   while (1)
   {
      if(irgendeine Bedingung)
         PORT_LED |= LED3;
   }
}

[funky (Gast)]

aber 0x08 ist schneller :P  (oder nicht?)

[Maxxie (Gast)]

Nein.

[funky (Gast)]

ah, die Konstanten werden schon vor der Laufzeit berechnet, oder?

[Mark B. (markbrandis)]

Warum aber

#define LED3       (1<<3)

besser sein soll als

#define LED3       (0x08)

, das erschließt sich mir nicht so ganz... ich benutze dann ja eh keine 
"magic numbers" mehr im Code, dafür hab ich ja das define.

[Maxxie (Gast)]

Ja, zumindest in den letzten 20 Jahren ;-)
Es ist nicht nur schneller bei der Ausführung, sondern schon beim 
compilieren, wenn der Parser auch complexe konstante Ausdrücke als 
konstanten Wert an den Compiler übergibt.

[Maxxie (Gast)]

@Mark,

na auch deine #defines muss jemand anderes in angemessener Zeit lesen 
können. Code-Reviews mit lauter 0xDEADBEEF sind meist nicht die 
Angenehmsten ;-)

[Vlad T. (vlad_tepesch)]

lol, bekommt man direkt lust auf Grillen.

[Mark B. (markbrandis)]

Maxxie schrieb:
> @Mark,
>
> na auch deine #defines muss jemand anderes in angemessener Zeit lesen
> können. Code-Reviews mit lauter 0xDEADBEEF sind meist nicht die
> Angenehmsten ;-)

Hehe, ich kenn DEBB und AFFE...
Naja aber die Zweierpotenzen sollte man schon drauf haben, oder? Zumal 
wenn da sowas steht wie:

#define LED0 (0x01)
#define LED1 (0x02)
#define LED2 (0x04)
#define LED3 (0x08)
#define LED4 (0x10)
#define LED5 (0x20)
#define LED6 (0x40)
#define LED7 (0x80)

Also das sollte man vielleicht gerade noch so vom Verständnis hinkriegen 
:-)

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

> Code-Reviews mit lauter 0xDEADBEEF sind meist nicht die Angenehmsten ;-)
Aber es versüüßt den Tag :-)
0xBADCAB1E
0xBADCOFFE
0xDEADC0DE
0xC001BABE

[steve (Gast)]

Hallo Leute,

ich habe den "Fehler" gefunden, aber ich verstehen trotzdem nicht was 
das damit zu tun hat.

Und zwar sobald ich die Globale Interupts (sei();) einschalte, habe ich 
dieses Problem.
Mein Code sieht man im Anhang?
Warum kommt das so?

Gruß Steve

[Gast (Gast)]

Ist ja normal, du schaltes den Interrupt des Timer0 frei, hast aber 
keine Routine, welche den Interrupt abarbeiten kann.


versuch mal ISR(TIM0_OVF_vect) oder so ähnlich.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

> ...du schaltes den Interrupt des Timer0 frei, hast aber
> keine Routine, welche den Interrupt abarbeiten kann.
Deshalb wird der Reset-Vektor angesprungen
--> das Programm startet neu
--> die LED ist wieder aus.

[Karl H. (kbuchegg)]

Lothar Miller schrieb:
>> ...du schaltes den Interrupt des Timer0 frei, hast aber
>> keine Routine, welche den Interrupt abarbeiten kann.
> Deshalb wird der Reset-Vektor angesprungen
> --> das Programm startet neu
> --> die LED ist wieder aus.

Womit wir wieder bei dem Punkt wären, den Magnus 10 Minuten nach 
Fragestellung angemerkt hat:

> Sprich: der Fehler befindet sich in dem Teil, den du hier nicht zeigst.

[steve (Gast)]

scheise bin ich .....

[HB (Gast)]

Einfacher gehts nicht

#define LED1_ON            (PORTA &= ~0x01)
#define LED1_OFF            (PORTA |=  0x01)
#define TASTER1            (PINE &  0x01)


While(1)
{
 if(TASTER1)LED1_ON
       .
       .
       .
       .
 irgendwo...LED1_OFF

mfg
HB

[Vlad T. (vlad_tepesch)]

0xFA11BACC hab ich mal in nem Programm als Fehlercode gesehen.

[Wegstaben V. (wegstabenverbuchsler)]

und es fehlt auch noch der

0xBADEAFFE

[Christopher G. (cbg)]

// Schaltplan nachschaun wie mans angehaengt hat
#define LED_PORT PORTA
#define LED_DDR DDRA
#define LED1 (1<<PA0)
#define LED2 (1<<PA1)
// oder
#define LED3 PA5
#define LED4 PA6
...
int main(void)
{
  LED_DDR |= LED1 | LED2;
  LED_PORT &= ~(LED1 | LED2);
  // bzw
  LED_DDR |= (1<<LED3) | (1<<LED4);
  LED_PORT &= ~((1<<LED3) | (1<<LED4));
  ...
  return 0;
}
...

Prost!
0xDeadBeefBadF00d

[Klaus (Gast)]

dann fehlt aber auch noch 0xB00B1E5 :)

[gast (Gast)]

@funky

>gehts nur mir so, das ich 0x08 einleuchtender finde als 1 << 3 ??

nein, mir geht es genauso

[Karl H. (kbuchegg)]

Mark Brandis schrieb:
> Warum aber
>
> #define LED3       (1<<3)
>
> besser sein soll als
>
> #define LED3       (0x08)

Weil ich bei 0x08 wieder nachdenken muss, welches Bit denn nun gesetzt 
ist. Bei (1<<3) brauch ich das nicht. Es steht schon dort: Bit 3 ist 
gesetzt. Wenn die Led aus irgendeinem Grund von Pin 3 auf Pin 6 versetzt 
werden muss, schreib ich einfach

#define LED3       (1<<6)

und fertig.
Bei deiner Version muss ich wieder überlegen, welche Hexzahl sich 
ergibt, wenn Bit 6 gesetzt ist.

Lass den Compiler die Routinearbeiten erledigen. Der macht das 
zuverlässiger. Die Kunst beim Programmieren besteht auch darin, sich den 
Source Code so zu organisieren, dass man Änderungen einfach machen 
kann ohne grossartig nachdenken oder analysieren zu müssen.

> ich benutze dann ja eh keine "magic numbers" mehr im Code, dafür hab
> ich ja das define.

"magic numbers" zu vermeiden ist das eine. Aber es ist nur die halbe 
Miete.

#define NR_ENTRIES  5
int Entries[NR_ENTRIES] = { 10, 80, 100, 40, 80 };

int main()
{
  for( i = 0; i < NR_ENTRIES; ++i )
    // mach was mit Entries[i]
}

benutzt auch keine direkten 'magic numbers' im Code.
Trotzdem hast du dir unnötigerweise einen möglichen Stolperstein 
eingehandelt. NR_ENTRIES und die Anzahl der Initialisierungen müssen 
übereinstimmen.

int Entries[] = { 10, 80, 100, 40, 80 };
#define NR_ENTRIES  (sizeof(Entries) / sizeof(*Entries))

int main()
{
  for( i = 0; i < NR_ENTRIES; ++i )
    // mach was mit Entries[i]
}

Hier passt sich alles von selbst an. Will ich einen Wert mehr im Array, 
schreibe ich ihn einfach hin. Der Compiler erledigt den Rest

int Entries[] = { 10, 80, 100, 40, 80, 400 };
#define NR_ENTRIES  (sizeof(Entries) / sizeof(*Entries))

int main()
{
  for( i = 0; i < NR_ENTRIES; ++i )
    // mach was mit Entries[i]
}

(Und nein: Beides ist gleich effizient. Nur um gleich dem Einwand der 
erhöhten Laufzeit vorzubeugen. Die paar Hunderstel Sekunden, die der 
Compiler zum Parsen und Bewertung des konstanten Ausdrucks benötigt, 
können wir getrost vergessen. Spätestens wenn du nur ein einziges mal 
einen 2.ten Compilerlauf benötigst, weil der Compiler warnt, dass die 
Anzahl der Initialisierungen nicht mit der Array-Size übereinstimmt, 
hast du mehr Zeit unnötig verbrutzelt als das Schreiben und die 
zusätzliche Compilerlast in vielen Tausend Compilerdurchgängen benötigt)

> Also das sollte man vielleicht gerade noch so vom Verständnis hinkriegen

Klar sollte man das. Aber wozu, wenn es eine Schreibweise gibt, bei der 
die Bitnummer direkt dort steht? Und an dieser Stelle interessiert mich 
die Bitnummer viel mehr als alles andere, weil sie mir sagt, nach 
welchem Pin ich auf dem µC suchen muss, wenn ich ein Multimeter 
draufhalten will. Die Hex-Zahl bringt dagegen keinen Vorteil an dieser 
Stelle, ausser den, den Programmierer geistig beweglich zu halten und 
seine Fertigkeit im Erkennen von gesetzten Bits in einer Hexzahl zu 
trainieren.

In der alternativen Schreibweise muss ich mir lediglich merken, dass das 
Muster  1 << x  ein Datenwort ergibt, welches an der Stelle x ein 1-Bit 
aufweist. Diese Erkentniss brauchst du aber in der µC-Programmierung 
sowieso alle Nase lang.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

> Weil ich bei 0x08 nachdenken muss, welches Bit denn nun gesetzt ist.
Nachdenken schadet nicht...

Wenn ich z.B. ein Kommandoregister initialisiere, dann sagt mir

 kreg = 0x47;

dass ich alle Bits angeschaut, bewertet und anschliessend auf 1 oder 0 
gesetzt habe. Wenn ich dagegen

 kreg = (1<<6)|(1<<2)|(1<<1)|(1<<0);

ansehe, dann könnte man ja meinen, ich hätte nur die Bits angefasst, von 
denen ich meinte, sie wären wichtig. Hier könnte der geneigte Leser des 
Programms den Verdacht hegen, dass ich eines einfach vergessen habe. 
Ausführlich müsste ich also schreiben

 kreg =  (0<<7)|(1<<6)|(0<<5)|(0<<4)|(0<<3)|(1<<2)|(1<<1)|(1<<0);

Leserlich ist das jetzt nicht mehr.

> Die Hex-Zahl bringt dagegen keinen Vorteil an dieser Stelle, ausser den,
> den Programmierer geistig beweglich zu halten und seine Fertigkeit im
> Erkennen von gesetzten Bits in einer Hexzahl zu trainieren.
... und kompakter zu Schreiben und zu Lesen zu sein. Es ist doch nur ein 
klitzekleiner Mehraufwand, sich neben all dem anderen Zeug noch die 
Hexzahlen anzulernen.

> Die Hex-Zahl bringt dagegen keinen Vorteil an dieser Stelle...
Aber an anderer Stelle doch? Dann muß ich sie auch lesen können, und 
Übung macht bekanntlich den Meister   ;-)

[Christopher G. (cbg)]

Lothar Miller schrieb:
> Wenn ich z.B. ein Kommandoregister initialisiere, dann sagt mir
>

 kreg = 0x47;

> dass ich alle Bits angeschaut, bewertet und anschliessend auf 1 oder 0
> gesetzt habe. Wenn ich dagegen
>

 kreg = (1<<6)|(1<<2)|(1<<1)|(1<<0);

> ansehe, dann könnte man ja meinen, ich hätte nur die Bits angefasst, von
> denen ich meinte, sie wären wichtig. Hier könnte der geneigte Leser des
> Programms den Verdacht hegen, dass ich eines einfach vergessen habe.
> Ausführlich müsste ich also schreiben
>

 kreg =  (0<<7)|(1<<6)|(0<<5)|(0<<4)|(0<<3)|(1<<2)|(1<<1)|(1<<0);
>

> Leserlich ist das jetzt nicht mehr.
Falsch!
Du hast bei der HEX Schreibweise vergessen 0xB8 mal 0x00 zu addieren!

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Christopher G. schrieb:
> Falsch!
> Du hast bei der HEX Schreibweise vergessen 0xB8 mal 0x00 zu addieren!
Rechne mal nach, das hab ich schon im Kopf gemacht  :-o
Und sogar noch 0xFF dazuverundet  ;-)

[Jochen64 (Gast)]

Vielleicht tritt irgendeine Exception auf, und Dein Programm ist in 
Wirklichkeit gar nicht in der Schleife, sondern in einem ständigen 
Reset?

[Christopher G. (cbg)]

Gut, dann denke mal nach was (0<<3) | (0<<7) ergibt.
Zuweisung ist Zuweisung! Hex oda Shift sagt genau nichts darüber aus, ob 
man die ganze Funktion des Registers beachtet!

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

> Gut, dann denke mal nach was (0<<3) | (0<<7) ergibt.
Ich weiß das schon, glaubs mir   ;-)
Aber ich könnte mir gut vorstellen, dass das einige ins Schleudern 
bringt.

> Hex oda Shift sagt genau nichts darüber aus, ob man die ganze Funktion
> des Registers beachtet!
Nein, aber in Hex habe ich jedem Bit explizit einen Wert zugewiesen, ich 
kann nicht einfach eines vergessen.

BTW: was ist eigentlich in C der Wert 0987 als Hex-Zahl dargestellt?


@  Jochen64:
Das eigentliche Problem ist schon lange gelöst:
Interrupts verwendet, ohne eine Interruptroutine zu haben.
Beitrag "Re: einfacher C-Code -> Denkfehler??"

[Karl H. (kbuchegg)]

Lothar Miller schrieb:

>> Die Hex-Zahl bringt dagegen keinen Vorteil an dieser Stelle...
> Aber an anderer Stelle doch?

Klar.
Kommt immer darauf an, in welchem Zusammenhang ich eine Zahl benutze.

#define SECS_PER_HALF_HOUR     60*60

#define SECS_PER_HALF_HOUR     3C*1C


Hier ist Dezimal 'besser' als 'Hex'

Es gibt kein 'pauschales' besser.

[Christopher G. (cbg)]

Lothar Miller schrieb:
> Nein, aber in Hex habe ich jedem Bit explizit einen Wert zugewiesen, ich
> kann nicht einfach eines vergessen.
Bei

FOO = (1<<3);

 habe ich auch jedem Bit einen Wert zugewiesen.
Bei

FOO |= (1<<3); //bzw &= ~(..);

 siehts anders aus, tortzdem heisst es bei Hex nicht, dass man alles 
beachtet hat.

> BTW: was ist eigentlich in C der Wert 0987 als Hex-Zahl dargestellt?
Ich verstehe die Frage nicht. 0987 ist eine Oktalzahl in C, da sie mit 0 
beginnt. Als Hex wäre es einfach 0x987 (bzw 0x0987 wennst die führende 0 
willst).

[Peter D. (peda)]

HB schrieb:
> Einfacher gehts nicht
>
> #define LED1_ON            (PORTA &= ~0x01)
> #define LED1_OFF            (PORTA |=  0x01)
> #define TASTER1            (PINE &  0x01)
>
>
> While(1)
> {
>  if(TASTER1)LED1_ON
>        .
>        .
>        .
>        .
>  irgendwo...LED1_OFF


Ich finde Bitvariablen einfacher.
Man kann sie auf 0 oder 1 testen bzw. setzen, wie jede andere Variable 
auch.
Und man kann ihnen vor allem aussagekräftige Namen geben:

#include <avr\io.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 SBIT(x,y)       SBIT_(x,y)




#define LED_GREEN       SBIT( PORTB, PB0 )
#define LED_GREEN_DDR   SBIT( DDRB,  PB0 )

#define KEY_ON_PIN      SBIT( PINB,  PB1 )
#define KEY_ON_PULL     SBIT( PORTB, PB1 )


int main( void )
{
  LED_GREEN_DDR = 1;    // output
  KEY_ON_PULL = 1;      // input with pullup

  for(;;){
    if( KEY_ON_PIN == 1 )
      LED_GREEN = 1;
    else
      LED_GREEN = 0;
  }
}

Peter

[Programmierer (Gast)]

> #define SECS_PER_HALF_HOUR     60*60
>
> #define SECS_PER_HALF_HOUR     3C*1C

Ist aber Müll... Klammern vergessen... Kann also häßliche Seiteneffekte 
haben. Ausserdem ist "3C" kein gültiges C...

Besser ist:

#define SECS_PER_HALF_HOUR     (60 * 60)

#define SECS_PER_HALF_HOUR     (0x3C * 0x1C)

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

> 0987 ist eine Oktalzahl in C
Yeah, reingefallen :-D
9 und 8 gibts in einer Oktalzahl nicht :-o

Übrigens:
0321 ist in C nicht 0x0321, sondern
            0   3   2   1
binär      000 011 010 001
und damit  0000 1101 0001
            0     D    1

In C gilt also:
0321 = 0xD1

[Christopher G. (cbg)]

Lothar Miller schrieb:
>> 0987 ist eine Oktalzahl in C
> Yeah, reingefallen :-D
> 9 und 8 gibts in einer Oktalzahl nicht :-o
Du Hund! (:

> In C gilt also:
> 0321 = 0xD1
Dazu war die Fragestellung nicht präzise genug.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Christopher G. schrieb:
>> 0321 = 0xD1
> Dazu war die Fragestellung nicht präzise genug.
Du hast schneller eine Ausrede, als eine Maus ein Loch.
Ich bin beeindruckt ;-)

[Christopher G. (cbg)]

Auf der Uni lernt man sowas (auch wenns keine Ausrede war ;) ).

[Magnus Müller (Gast)]

Programmierer schrieb:
>> #define SECS_PER_HALF_HOUR     60*60
>>
>> #define SECS_PER_HALF_HOUR     3C*1C
>
> Ist aber Müll... Klammern vergessen... Kann also häßliche Seiteneffekte
> haben. Ausserdem ist "3C" kein gültiges C...
>
> Besser ist:
>
>

> #define SECS_PER_HALF_HOUR     (60 * 60)
> 
> #define SECS_PER_HALF_HOUR     (0x3C * 0x1C)
>

Veto!

1. Eine halbe Stunde hat 30 * 60 Sekunden.

2. 0x1C sind 28 und passen nun gar nicht ins Schema ;)

Ansonsten stimme ich der Aussage von "Programmierer" zu.

[Karl H. (kbuchegg)]

Magnus Müller schrieb:

> Ansonsten stimme ich der Aussage von "Programmierer" zu.


Recht hat er.
Ich bin beim Schreiben gestört worden (Achtung: Chef kommt!) und habe 
kurzfristig auf Senden gedrückt :-)

Der Sinn dürfte aber rübergekommen sein.
Ob Hex oder Dezimal oder Binär oder die 1<<x Schreibweise:
Es kommt auf den Verwendungszweck an, welche Darstellung vernünftiger 
ist.

[Programmierer (Gast)]

> Veto!
>
> 1. Eine halbe Stunde hat 30 * 60 Sekunden.
>
> 2. 0x1C sind 28 und passen nun gar nicht ins Schema ;)

Sehr gut aufgepasst!

Es ist immer wieder spannend, wieviele Fehler man in nur zwei Zeilen 
Code einbauen kann. Faszinierend.

[Magnus Müller (Gast)]

> Es ist immer wieder spannend, wieviele Fehler man in nur zwei Zeilen
> Code einbauen kann. Faszinierend.

Dieser Satz hätte von Mr. Spock stammen können. Allerdings hätte er an 
Stelle statt "man" wohl eher "ein Humanoid" geschrieben ;)