ATMega8A LED blinkt statt konstantes Lechten

[Bert S. (kautschuck)]

Hi,
Ich arbeite mich ein bisschen in AVR gcc ein und verwende das AVR 
Studio. Ich habe eine LED über einen Transistor angeschlossen, dass 
ganze wird über den Port PB0 angesteuert. Als Eingang zum wechseln des 
Modus (LED leuchtet  = 1 bzw. leuchtet nicht = 0) verwende ich einen 
Taster mit Pull Up Pfad am Eingang PD0. Wenn ich den Taster drücke, 
wechselt die LED den Modus, doch wenn die LED in den Modus 1 geht,blinkt 
sie, im Modus 0 ist sie konstant aus. Ich habe keine Ahnung woher das 
blinken kommt. Die Frequenz ist unabhängig vom sleep() und etwa 3Hz.

Hier noch mein Code:

#include <avr/io.h>
#include <util/delay.h>
#include <stdio.h>
void sleep( uint8_t ms );
uint8_t key_pressed (volatile uint8_t *inputreg, uint8_t inputbit);

int main(void)
{
  int x = 1000;
    DDRB  = 0xFF;             // (3)
  DDRD  = 0x00;
    PORTB = 0x01;             // (4)
    
    while(1) {  
    sleep(20); 
    uint8_t i = key_pressed (&PIND, PD0);
    if(i == 1) {
      PORTB ^= ( 1 << PB0 ); 
    }  
   }                         // (7)
   return 0;  
}

void sleep ( uint8_t ms )
{
    for(ms ; ms > 0; ms--) _delay_ms(1);
}

uint8_t key_pressed (volatile uint8_t *inputreg, uint8_t inputbit)
{
  static uint8_t last_state = 0;
  if (last_state == (*inputreg & (1<<inputbit)))
    return 0; /* keine Änderung */
  
  /* Wenn doch, warten bis etwaiges Prellen vorbei ist: */
  _delay_ms(20);
  
  /* Zustand für nächsten Aufruf merken: */
  last_state = *inputreg & (1<<inputbit);
  
  /* und den entprellten Tastendruck zurückgeben: */
  return *inputreg & (1<<inputbit);
}

Gruss Bert

[aha! (Gast)]

Bert Siegfried schrieb:
>  Lechten

wird wohl nur ein Schreibfehler sein!

[Rolf (Gast)]

Bert Siegfried schrieb:

>

uint8_t key_pressed (volatile uint8_t *inputreg, uint8_t inputbit)
{
  static uint8_t last_state = 0;
  if (last_state == (*inputreg & (1<<inputbit)))
    return 0; /* keine Änderung */
  
  /* Wenn doch, warten bis etwaiges Prellen vorbei ist: */
  _delay_ms(20);
  
  /* Zustand für nächsten Aufruf merken: */
  last_state = *inputreg & (1<<inputbit);
  
  /* und den entprellten Tastendruck zurückgeben: */
  return *inputreg & (1<<inputbit);
}

Die Funktion wird immer nur genau 1 mal ausgeführt, dabei ist last_state 
am Anfang immer 0.
Deine Variable last_state ist lokal zur Funktion, sie wird bei jedem 
Aufruf der Funktion neu deklariert und mit 0 initialisiert. Daher kann 
sich diese Variable nichts für den nächsten Aufruf der Funktion merken.

[Marc V. (Firma: Vescomp) (logarithmus)]

Rolf schrieb:
> Deine Variable last_state ist lokal zur Funktion, sie wird bei jedem
> Aufruf der Funktion neu deklariert und mit 0 initialisiert. Daher kann
> sich diese Variable nichts für den nächsten Aufruf der Funktion merken.

 Das stimmt nicht, aber static variables in Funktionen sind (für mich)
 eine schlechte Praxis. Es wird Speicherplatz gebraucht, praktisch wird
 damit eine globale Variable deklariert, Compiler kann auf diese
 Variable nicht so einfach zugreifen, es wird einfach komplizierter. ;-)
 Der einzige Vorteil(?) ist, dass man mehrere Variablen mit denselben
 Namen deklarieren kann.

 Einfacher wäre es, last_state beim Aufruf als Parameter übergeben und
 beim Return zurückgeben.

[Michael L. (michaelx)]

Das die LED blinkt, liegt doch auf der Hand.
Das hast du selbst programmiert:

f(i == 1) {
      PORTB ^= ( 1 << PB0 ); 
    }

Wenn i == 1 ist, wird PB0 bei jedem Durchlauf negiert.

[Erni (Gast)]

Hallo Bert,

ich habe auch noch nicht gesehen, warum deine LED blinkt. Jedoch habe 
ich einige Vorschläge bezüglich "Software Engineering":

1.)
Das
>uint8_t i = key_pressed (&PIND, PD0);
>if(i == 1) {
wird dir irgendwann das Genick brechen. Die "1" in der IF Abfrage stimmt 
nur so lange, wie du den Pin 0 eines Ports benutzt. Wenn du jetzt Pin 1 
nehmen würdest, müsstest du PD0 auf PD1 ändern !UND! if(i==1) auf 
if(i==2). Du musst also an zwei Stellen was ändern. Noch ist dein 
Programm überschaubar, aber wenn es größer wird, holst du dir den Teufel 
ins Haus
Mach dir ein Macro á la
#define INPUTBIT_PD0 (1<<PD0)
und verwende das im Code.

2.)
Wenn ich das richtig sehe, macht deine Funktion "key_pressed" etwas 
anderes, als ihr Name impliziert. Ich meine, die Funktion gibt nur 
einmalig eine "1" zurück, wenn der Taster nach einem Drücken den Kontakt 
wieder öffnet. Es ist also keine Abfrage eines Zustandes sondern eher 
ein Signalimpuls in Form eines Rückgabewertes im Falle eines Events.

3.)
Ich finde, du solltest die Variable "last_state" anders nennen. So 
erinnert sie zu stark an ein Argument einer statemachine. Nenn sie z.B. 
"lastKeyState".

4.) Wo ich gerade beim Thema bin, es gibt ein paar Regeln, wie Variablen 
#defines und Co im Quelltext geschrieben werden. Goggle mal nach "The 
Clean Coder"
Variablen fangen z.B. immr mit einem kleinem Buchstaben an, haben 
üblicherweise keine Unterstriche und jedes neue Wort innerhalb der 
Variablen bekommt einen großen Buchstaben. Manche sagen auch, dass z.B. 
lokal definierte Variablen eine Kennzeichung brauchen.
#defines werden durchgehen groß geschrieben.

Gruß,
Erni

[stefanus (Gast)]

Die LED blinkt weil der Pin bei jedem Schleifendurchlauf invertiert 
wird.

while(1) {  
    sleep(20); 
    uint8_t i = key_pressed (&PIND, PD0);
    if(i == 1) {
      PORTB ^= ( 1 << PB0 ); 
    }  
}

Das heist in Pseudosprache frage im 20ms Intervall die Taste ab, und 
wenn sie geerade gedrückt ist, dann schalte die LED um.

Daraus folgt: Solange die Taste gedrückt ist, geht die LED alle 20ms 
abwechselnd an und aus.

[Marc V. (Firma: Vescomp) (logarithmus)]

stefanus schrieb:
> Daraus folgt: Solange die Taste gedrückt ist, geht die LED alle 20ms
> abwechselnd an und aus.

 Und wie passen da 3Hz rein ?

[stefanus (Gast)]

Ich glaube, er hat die key_pressed() funktion nicht so dargestellt, wie 
er sie tatsächloch verwendet. Ansonsten wäre das beschriebene Verhalten 
nicht erklärbar.

Wenn die funktion tut, was ihr Name andeutet, dann ist das Verhalten 
logisch, wegen "PORTB ^= ( 1 << PB0 )".

Wenn die Funktion jedoch tatsächlich so aussieht wie oben, dann hat Erni 
sicher Recht, dann dürfte die LED nicht blinken.

Wobei mich das Tkming nachdenklich stimmt. 20ms and/aus würde ich eher 
als flimmern bezeichnen. Blinken geht langsamer. Also hat wohl auch noch 
ein Problem mit der Taktfrequenz oder vieleicht mit dem Watchdog.

Ich glaube, Bert hat nicht den Quelltext verkürzt und dabei die 
entscheidenden Teile ausgelassen.

[stefanus (Gast)]

> Und wie passen da 3Hz rein ?

Huch, zwei Dumme ein gedanke. Ist mir auch gerade aufgefallen. Das passt 
gar nicht.

[Thomas E. (thomase)]

Dein sleep() funktioniert nicht.
Warnings sind nicht zum ignorieren da!

Aber mit _delay_ms(20) geht es.

Auch wenn da ein grosser Bock drin ist:

uint8_t i = key_pressed (&PIND, PD0);
if(i == 1)

Zufällig macht es hier aber trotzdem das Richtige.

Grundsätzlich aber bitte so:

if(key_pressed (&PIND, PD0))

Die Variable kannst du dir auch sparen.

Das Programm ist nicht sonderlich optimal, böse Zungen würden es als 
"Total beschissen" bezeichnen, aber ich bin ja nicht böse.

Immerhin funktioniert es aber. Und das ist ja auch schon mal was.

Deinen Fehler wirst du also woanders suchen müssen. Im Programm findest 
du ihn nicht.

Mögliche Ursachen:

- mit falschem Controller compiliert
- falsches Hexfile
- Hardwarefehler: "Ich habe eine LED über einen Transistor
  angeschlossen," Da schwant mir, ehrlich gesagt, Böses.
  Wenn deine Hardware annähernd aufwändig ist wie dein Programm,
  schaltest du mit dem Transistor eine 2mA-LED.

mfg.

[Marc V. (Firma: Vescomp) (logarithmus)]

Thomas Eckmann schrieb:
> Das Programm ist nicht sonderlich optimal, böse Zungen würden es als
> "Total beschissen" bezeichnen, aber ich bin ja nicht böse.
 Ich auch nicht, aber...

> Deinen Fehler wirst du also woanders suchen müssen. Im Programm findest
> du ihn nicht.
>
> Mögliche Ursachen:
>
> - mit falschem Controller compiliert
 Oder:
 20ms + 20ms = 40ms
 8MHz compiliert, läuft auf 1MHz
 ergibt:
 40ms * 8 = 320ms = ungefahr 3Hz.

[Michael L. (michaelx)]

Ups, die 3 Hz hab ich gar nicht gelesen. Nach der Überschrift stach mir 
gleich das ^= ins Auge. Aber Thomas und Marc haben schon den Finger 
drauf ...

[Holger L. (max5v)]

Marc Vesely schrieb:
> 20ms + 20ms = 40ms
>  8MHz compiliert, läuft auf 1MHz
>  ergibt:
>  40ms * 8 = 320ms = ungefahr 3Hz.

Das hört sich gut an.

Aus delay.h
#ifndef F_CPU
/* prevent compiler error by supplying a default */
# warning "F_CPU not defined for <util/delay.h>"
# define F_CPU 1000000UL
#endif

Entweder die Zeile mit dem define für F_Cpu wurde nicht kopiert, oder 
sie fehlt.

[MWS (Gast)]

Holger L. schrieb:
> Aus delay.h
> #ifndef F_CPU
> /* prevent compiler error by supplying a default */
> # warning "F_CPU not defined for <util/delay.h>"
> # define F_CPU 1000000UL
> #endif
>
> Entweder die Zeile mit dem define für F_Cpu wurde nicht kopiert, oder
> sie fehlt.

Würde die Delay-Funktion eine niedrigere als die tatsächliche FCPU 
annehmen, so würde das Delay schneller, aber nicht langsamer ausgeführt.

[Marc V. (Firma: Vescomp) (logarithmus)]

MWS schrieb:
> Würde die Delay-Funktion eine niedrigere als die tatsächliche FCPU
> annehmen, so würde das Delay schneller, aber nicht langsamer ausgeführt.

 Hmmm.
 Blinkt etwa 8x langsamer, du willst also sagen, dass seine CPU mit
 80MHz läuft ?
 Klingt logisch.

[MWS (Gast)]

Marc Vesely schrieb:
>  Hmmm.
>  Blinkt etwa 8x schneller, du willst also sagen, dass seine CPU mit
>  80MHz läuft ?
>  Klingt logisch.

Nein, ich sprach davon, dass falls der Delay-Funktion das F_CPU fehlen 
würde und diese 1MHz annimmt, die dann erzeugten Verzögerungsschleifen 
zu kurz ausfallen, wenn die CPU mit tatsächlichen 8MHz läuft.

Ergo, ist dieser Fehlerfall hier unwahrscheinlich. Möglich ist dagegen, 
dass F_CPU auf 8MHz eingestellt ist, die CPU dagegen per Fuse auf 1MHz 
läuft.

Hast Du jetzt verstanden?

[Marc V. (Firma: Vescomp) (logarithmus)]

MWS schrieb:
> Ergo, ist dieser Fehlerfall hier unwahrscheinlich. Möglich ist dagegen,
> dass F_CPU auf 8MHz eingestellt ist, die CPU dagegen per Fuse auf 1MHz
> läuft.
>
> Hast Du jetzt verstanden?

 LOL.
 Bist du doof, oder was ?

Marc Vesely schrieb:
> Oder:
>  20ms + 20ms = 40ms
>  8MHz compiliert, läuft auf 1MHz
>  ergibt:
>  40ms * 8 = 320ms = ungefahr 3Hz.

 Hast du Schwierigkeiten beim lesen oder beim Verstehen ?

[Holger L. (max5v)]

Nein, aber wenn F_CPU nicht eingetragen wurde wird es mit 1 MHz 
berechnet.
Außerdem :
Bert Siegfried schrieb:
> Die Frequenz ist unabhängig vom sleep()

[MWS (Gast)]

Marc Vesely schrieb:
>  LOL.
>  Bist du doof, oder was ?
>
> Marc Vesely schrieb:
>> Oder:
>>  20ms + 20ms = 40ms
>>  8MHz compiliert, läuft auf 1MHz
>>  ergibt:
>>  40ms * 8 = 320ms = ungefahr 3Hz.
>
>  Hast du Schwierigkeiten beim lesen oder beim Verstehen ?

Mensch, Vaseline, Du hast eine Auffassungsgabe wie 'ne Dose Kekse, 
hältst aber andere für doof?

1) mein erstes Post oben war an Holger L. gerichtet, der annahm, dass 
F_CPU nicht zur Delay-Funktion durchgereicht wurde und deshalb als 
Standard 1MHz setzte. Diese Fehlermöglichkeit hab' ich logisch 
ausgeschlossen.

2) Die von Dir ins Spiel gebrachte mögliche Fehlerquelle hab' ich nicht 
in Frage gestellt, ich hab' sie sogar nochmal bekräftigt, worauf ich von 
Dir dummen Bub blöd angequatscht wurde.

Wenn also jemand Deine These lediglich nochmal wiederholt, so sagt er 
damit nicht, dass diese falsch wäre. Jetzt verstanden?

[Marc V. (Firma: Vescomp) (logarithmus)]

MWS schrieb:
> Wenn also jemand Deine These lediglich nochmal wiederholt, so sagt er
> damit nicht, dass diese falsch wäre. Jetzt verstanden?

 LOL.
 Redest du auch so viel wie du schreibst, um nichts zu sagen ?
 Politik studiert ?

[Holger L. (max5v)]

Ojeh ojeh, so ein schöner Sonntag.

Was haltet ihr von Brown-Out detection und Versorgung mit Batterie ?

[Marc V. (Firma: Vescomp) (logarithmus)]

Holger L. schrieb:
> Außerdem :
> Bert Siegfried schrieb:
>> Die Frequenz ist unabhängig vom sleep()

Thomas Eckmann schrieb:
> Dein sleep() funktioniert nicht.
> Warnings sind nicht zum ignorieren da!

 Nicht ausprobiert, vertraue Thomas (blind).

[MWS (Gast)]

Marc Vesely schrieb:
>  LOL.
>  Redest du auch so viel wie du schreibst, um nichts zu sagen ?
>  Politik studiert ?

OK, man kann's Dir leider nicht verständlich machen.
Versuch's nochmal, wenn Dir ein bisserl mehr Hirn gewachsen ist.

[Marc V. (Firma: Vescomp) (logarithmus)]

Holger L. schrieb:
> Was haltet ihr von Brown-Out detection und Versorgung mit Batterie ?

 In Zusammenhang mit ?

[Holger L. (max5v)]

In Zusammenhang mit der Led, evtl. genügt diese ja bereits aus um an die 
2,7 V / 4 V Grenze zu kommen.
Eventuell ist es auch der Watch-dog ?

[Thomas E. (thomase)]

Marc Vesely schrieb:
> 40ms * 8 = 320ms = ungefahr 3Hz.

Das würde die Zeit erklären. Aber nicht, dass es überhaupt blinkt.

Eine Vermutung ist, dass er als Transistor einen dicken Mosfet dran hat, 
der mit seiner Kapazität beim Schalten die Stromversorgung kurz 
zusammenbrechen lässt und nach einem Reset geht es dann wieder von vorne 
los.


mfg.

[Marc V. (Firma: Vescomp) (logarithmus)]

Holger L. schrieb:
> In Zusammenhang mit der Led, evtl. genügt diese ja bereits aus um an die
> 2,7 V / 4 V Grenze zu kommen.

 Möglich, aber eine LED mit 20mA ?

Thomas Eckmann schrieb:
> Eine Vermutung ist, dass er als Transistor einen dicken Mosfet dran hat,
> der mit seiner Kapazität beim Schalten die Stromversorgung kurz
> zusammenbrechen lässt und nach einem Reset geht es dann wieder von vorne
> los.

 Das erklärt das Blinken auch nicht.
 Wenn die Stromversorgung beim Schalten zusammenbricht:
   a) LED wird mit Log.1 eingeschaltet, dann gelingt das Programm
      gar nicht bis zur Abfrage.
   b) LED wird mit Log.0 eingeschaltet, dann würde es nur ein
      kurzes Blitzen geben (wenn überhaupt).

 Oder ich sehe das alles falsch.

[Michael L. (michaelx)]

Thomas Eckmann schrieb:
> Marc Vesely schrieb:
>> 40ms * 8 = 320ms = ungefahr 3Hz.
>
> Das würde die Zeit erklären. Aber nicht, dass es überhaupt blinkt.
>
> ...

Die Ursache fürs Blinken liegt doch im Code. Das wurde nicht nur von mir 
schon weiter oben festgestellt.

[Thomas E. (thomase)]

Marc Vesely schrieb:
> Oder ich sehe das alles falsch.

Das Problem ist, dass wir das gar nicht sehen und uns auf den 
subjektiven Eindruck des TO verlassen. Was der eine als Blitzen 
wahrnimmt, ist für jemand anderes Blinken und für den nachsten leuchtet 
es ständig mit kurzen Unterbrechungen.

mfg.

[Marc V. (Firma: Vescomp) (logarithmus)]

Michael L. schrieb:
> Die Ursache fürs Blinken liegt doch im Code. Das wurde nicht nur von mir
> schon weiter oben festgestellt.

 Nicht ganz.
 Es sei denn, für Compiler ist:

   static uint8_t last_state = 0;

 gar nicht static.
 Dann könnte das mit Blinken und 1MHz statt 8MHz die einzig logische
 Erklärung sein.
 Da ich das aber für wenig wahrscheinlich halte, wird wohl sein
 gepostetes und sein geflashtes Code unterschiedlich sein.

[Michael L. (michaelx)]

Marc Vesely schrieb:

>  Nicht ganz.

Klappt wohl heute nicht ganz mit dem Lesen?

Ohne das hier würde nichts blinken:

PORTB ^= ( 1 << PB0 );

;-)

[Marc V. (Firma: Vescomp) (logarithmus)]

Michael L. schrieb:
> Klappt wohl heute nicht ganz mit dem Lesen?

 Und bei dir ?

    uint8_t i = key_pressed (&PIND, PD0);
    if(i == 1) {
      PORTB ^= ( 1 << PB0 ); 
    }

[Michael L. (michaelx)]

Marc Vesely schrieb:
>  Und bei dir ?

Du hast Recht. Irgendwas wird da noch sein. Vorhin, kurz nach dem 
letzten Posting hats mich erst mal weg gedreht. Ich glaube, ich sollte 
mit meiner fetten Erkältung etwas kürzer treten.

:-(