Problem mit Code

[Mathias O. (m-obi)]

Guten Morgen,

ich hab ein Problem mit meinem Code, der will nicht richtig 
funktionieren. Ich möchte mit jeweils einen Taster, einen Port toggeln. 
Ich verwende einen Atmega16 mit externen 16 Mhz, dies hab ich im 
Makefile festgelegt.

#include <avr/io.h>
#include <util/delay.h>

#define Taster_1 (PIND&(1<<PD2))
#define Taster_2 (PIND&(1<<PD3))
#define Taster_3 (PIND&(1<<PD4))

volatile uint8_t var_1 = false;

uint8_t key_pressed(const volatile uint8_t input)
{
  static uint8_t last_state = 0;
  if ( last_state == input ) return 0; 
  _delay_ms(20);
  last_state = input;
  return input;
}


int main(void)
{
    
  DDRA = 0b11111111;
  DDRB = 0b11111111;
  DDRC = 0b11111111;
  DDRD = 0b11100011;

  while(1)
  {     

    if (key_pressed(Taster_1)) PORTC ^= 0xff;
    
    if (key_pressed(Taster_3)) PORTA ^= 0xff;
    
    }
    return 0;
}

Kann mir jemand helfen?

[Peter (Gast)]

>ich hab ein Problem mit meinem Code...

Und was ist das Problem? Beschreibung?

[Mathias O. (m-obi)]

Wenn ich die Taster drücke flackert die LED, als wenn die Entprellung 
nicht funktioniert und somit spinnt er auch dann beim toggeln.

[Peter (Gast)]

Ich bin jetzt noch nicht der Profi in C beim Atmel aber ich würde sagen 
das volatile braucht man bei diesem code nicht, es werden ja keien ISR 
genutzt.

Das sollte aber nicht die Lösung für das Problem sein.

[Oliver (Gast)]

>ich hab ein Problem mit meinem Code, der will nicht richtig
>funktionieren.

Wie genau äussert sich das?

Ansonsten:

Tasten müssen grundsätzlich entprellt werden. Das ist nicht so trivial, 
wie es zunächst aussieht. Eprobt und bewährt ist dafür die Methode von 
Peter Danneger, siehe Codesammlung.

Dann speichert dein key_pressed() last_State nicht für jede Taste 
einzeln, sondern nur den Wert des jeweils letzten Aufrufs. Wenn du die 
Funktion mehrfach hintereinander für verschiedene Tasten aufrufst, gibt 
es Durcheinander.

Auch hier: Nimm den Code aus der Codesammlung.

Oliver

[Walter T. (nicolas)]

Du übergibst den Status des Tasters am Anfang in Deine 
"Entprellroutine", wenn man sie so nennen darf, der Taster wird nur 
einmal abgefragt.

[Johannes M. (johnny-m)]

@Mathias:
Erstens solltest Du doch langsam wissen, dass man für eine Anfrage einen 
aussagekräftigen Betreff wählen soll, wie es in den Forenregeln steht. 
"Problem mit Code" ist kein aussagekräftiger Betreff!

Zweitens erwartet Deine Funktion einen "const volatile uint8_t", das, 
was Du da übergibst, ist aber alles andere als ein "const volatile 
uint8_t"...

[Peter (Gast)]

> Dann speichert dein key_pressed() last_State nicht für jede Taste
> einzeln, sondern nur den Wert des jeweils letzten Aufrufs.
Das ist doch die Lösung das Problemes, Wenn Taste 1 gedrück und Taste 2 
nicht gedrückt dann ist der Laststate immer entgegengesetzt. Damit 
Blinken die Lampen ein 40ms Takt.

[Peter (Gast)]

Du fragst abwechselnd Taster_1 und Taster_2 ab, aber speicherst den 
Zustand immer in der selben Variabel "last_state". Sobald Du eine Taste 
drückst ist (last_state == input) immer wieder false und die LED 
flakert...

Zum entprellen schlage ich volgende Änderung vor:

uint8_t key_pressed(const volatile uint8_t input)
{
  static uint8_t last_state = 0;
  uint8_t new_input;
  new_input = input;
  if ( last_state == new_input) return 0; 
  _delay_ms(20);
  // last_state = input; // input könnte geändert haben wegen prellung
  last_state = new_input;
  return new_input;
}

[Johannes M. (johnny-m)]

@Peter:
Ja, das hat mich grad auch verwirrt, was er da für komische Dinge in 
seiner "Entprell"-Routine macht. War grad dabei, mal zu versuchen, 
nachzuvollziehen, was da eigentlich genau passiert.

[Micha (Gast)]

Du speicherst in last_state maximal die aktuell gedrückte Taste - alle 
anderen werden nicht gespeichert. Da du dann noch bei jedem Aufruf von 
key_pressed eine andere Maske übergibst toggeln die LEDs...

Deine key_pressed hat mit Entprellung nichts zu tun, aber das weißt du 
bestimmt!?

[Micha (Gast)]

> Zum entprellen schlage ich volgende Änderung vor:

uint8_t key_pressed(const volatile uint8_t input)
{
  static uint8_t last_state = 0;
  uint8_t new_input;
  new_input = input;
  if ( last_state == new_input) return 0; 
  _delay_ms(20);
  // last_state = input; // input könnte geändert haben wegen prellung
  last_state = new_input;
  return new_input;
}

Korrigiere mich falls ich mich irre - aber entprellt wird auch auf diese 
Weise nichts. Bei beiden Versionen wird 20ms gewartet bis der aktuelle 
Wert in last_state geschrieben wird, mehr nicht. Ohne das _delay_ms(20); 
spart man sich 20ms und an der Funktion ändert sich nichts.

[Mathias O. (m-obi)]

Aber ich finde man sollte Interrupts nicht für sowas benutzen wie bei 
Peter.
Das müsste doch eigentlich viel kürzer gehen. In anderen 
Programmiersprachen macht man einfach nur eine Einschaltverzögung von 
20ms.

[Peter (Gast)]

> Korrigiere mich falls ich mich irre - aber entprellt wird auch auf diese
> Weise nichts. Bei beiden Versionen wird 20ms gewartet bis der aktuelle
> Wert in last_state geschrieben wird, mehr nicht.
Doch schon, denn nach 20ms ist das prellen vorbei, damit wird also die 
Zeit in der die Kontakte nicht sauber geschlossen sind einfach 
übersprungen.

[Karl H. (kbuchegg)]

Micha wrote:

> Korrigiere mich falls ich mich irre - aber entprellt wird auch auf diese
> Weise nichts.

Du irrst nicht.
Die Funktion ist immer noch haarscharf dieselbe wie vor der 
Modifikation. Nur eine Variable mehr ist unvolviert in die ein sowieso 
innerhalb der Funktion feststehender Wert einmal umkopiert wird.

> Ohne das _delay_ms(20); spart man sich 20ms

Das stimmt nicht ganz. Ohne das delay_ms ist die Funktion zu schnell, so 
dass der Aufrufer die Tasten schneller abfragt. Kurz gesagt: 
Funktionalität ist über das ganze Programm verstreut und wenn man an 
einer Ecke etwas ändert, hat das Auswirkungen auf die andere Ecke. 
Schlechter Programmierstil.

[Walter T. (nicolas)]

Es wird aber nur Zeit übersprungen, bis der schon vorher eingelesene 
Wert geschrieben wird - es wird am Anfang der Status des Tasters 
übergeben, kein Zeiger wo der Status zu finden wäre.

[Micha (Gast)]

> Zum entprellen schlage ich volgende Änderung vor:

uint8_t key_pressed(const volatile uint8_t input)
{
  static uint8_t last_state = 0;
  uint8_t new_input;
  new_input = input;
  if ( last_state == new_input) return 0; 
  _delay_ms(20);
  // last_state = input; // input könnte geändert haben wegen prellung
  last_state = new_input;
  return new_input;
}

Abgesehen von alledem macht diese Funktion dasselbe wie die 
ursprüngliche, nur dass eine weitere Variable ver(sch)wendet wird...

@ Mathias Obetzhauser
Benutze Peter Danneggers Version. Die funktioniert und verschwendet 
keine Zeit mit warten. Und man muss auch das Rad nicht immer wieder neu 
erfinden...

[Karl H. (kbuchegg)]

Mathias Obetzhauser wrote:
> Aber ich finde man sollte Interrupts nicht für sowas benutzen wie bei
> Peter.
> Das müsste doch eigentlich viel kürzer gehen. In anderen
> Programmiersprachen macht man einfach nur eine Einschaltverzögung von
> 20ms.

Wie schon angesprochen:
Hol dir aus der Codesammlung die PeDa Tastenentprellung. Die ist 100% 
fullproof und funktioniert klaglos.

Tastenentprellung ist nicht so simpel, wie es auf den ersten Blick 
aussieht.

[Micha (Gast)]

> Aber ich finde man sollte Interrupts nicht für sowas benutzen wie bei
> Peter.
> Das müsste doch eigentlich viel kürzer gehen. In anderen
> Programmiersprachen macht man einfach nur eine Einschaltverzögung von
> 20ms.

Von der Laufzeit her ist Peters Version garantiert kürzer. Daher vermute 
ich du meinst mit "kürzer" weniger Code, richtig?

Falls ja kannst du wählen zwischen
- regelmäßig 20ms sinnlos mit warten verschwenden und etwas Code sparen 
oder
- etwas mehr Code schreiben und dafür mehr Rechenzeit übrig haben.

Auf die obige Weise geht es auf jeden Fall nicht! Und bis das 100% 
sicher läuft hast du (sehr) wahrscheinlich genausoviel Code wie bei 
Peters Version.

Und mit der verwendeten Programmiersprache hat das alles überhaupt 
nichts zu tun!

[Mathias O. (m-obi)]

Dann muss ich die woll nehmen von Peter. Ich möchte gerne die ganze 
Entprellroutine in eine Datei packen und dann wenn ich Tasten habe 
einfach includen. Und halt dann nur nen Aufruf.

#include <avr/io.h>
#include <util/debouce.h>

#define Taster_1 (PIND&(1<<PD2))

int main(void)
{
    
  DDRA = 0b11111111;
  DDRD = 0b11100011;

  while(1)
  {     
  if (debounce(Taster_1)) PORTA ^= 0xff;
  }
  return 0;
}

Kann mir jemand dann mal zeigen wie die debounce.h aussehen muss?

[Micha (Gast)]

Nimm einfach den Code von hier 
Beitrag "Universelle Tastenabfrage" und teile ihn auf ein 
.c und ein .h auf.