Variable per Timer zurücksetzen

[Lokus P. (derschatten)]

Ich möchte gerne mittels Timer eine Variable setzen.

Die Schaltung wird mit einem Drehencoder betrieben.
(Der Code entspricht dem Beispiel -> 
http://www.mikrocontroller.net/articles/Drehgeber)

Das sieht in etwa so aus:

ISR (TIM0_COMPA_vect)
{
  if(timeCount > 0)
  {
    timeCount--;
    if(timeCount == 0)
    {
      val = valwert;
    }
  }
}

int8_t encode_read(void)
{
  int8_t val;

  cli();
  val = enc_delta;
  enc_delta = val & 1;
  sei();
  return val >> 1;
}

int8_t encode_read_timeout(void)
{
  int8_t tmp = encode_read();

  if(tmp != valwert)
    timeCount = 3000;
  return tmp;
}

int main(void)
{
  ioinit();                            // PORT setzen
  eeprom_var();                          // EEPROM-Werte auslesen
  reset();                            // C64 Reset durchführen
  encode_init();                          // ENCODER initialisieren
  sei();                              // Interrupt aktivieren

  for(;;)
  {
    val += encode_read_timeout();                // ENCODER-Wert auslesen

    if (val > 3) val = 3;                    // ENCODER-Wert auf 3 limitieren
    if (val < 0) val = 0;

    switch(val)
    {
      case 0:
        AUSGANG_PORT |= led_setzen[val];          // Aktiven LED-PIN setzen
        AUSGANG_PORT &= ~(led_loeschen[val]);        // Inaktive LED-PIN löschen 
        if (get_key_short(TASTER))              // bei kurzem Tastendruck
        {
          kernal_id(CBM);
          valwert = val;
          reset();
        }
        if(get_key_long(TASTER))              // bei langem Tastendruck
        {
          kernal_id(CBM);
          eeprom_write_byte(&eeprom_kernalid, CBM);    // KERNAL-Wert in EEPROM speichern
          eeprom_write_byte(&eeprom_encoderid, val);    // ENCODER-Wert in EEPROM speichern
          reset();
        }
        break;
      case 1:
        AUSGANG_PORT |= led_setzen[val];
        AUSGANG_PORT &= ~(led_loeschen[val]);
        if (get_key_short(TASTER))
        {
          kernal_id(SJIFFY);
          valwert = val;
          reset();
        }
        if(get_key_long(TASTER))
        {
          kernal_id(SJIFFY);
          eeprom_write_byte(&eeprom_kernalid, SJIFFY);
          eeprom_write_byte(&eeprom_encoderid, val);
          reset();
        }
        break;
      case 2:
        AUSGANG_PORT |= led_setzen[val];
        AUSGANG_PORT &= ~(led_loeschen[val]);
        if (get_key_short(TASTER))
        {
          kernal_id(SPEED);
          valwert = val;
          reset();
        }
        if(get_key_long(TASTER))
        {
          kernal_id(SPEED);
          eeprom_write_byte(&eeprom_kernalid, SPEED);
          eeprom_write_byte(&eeprom_encoderid, val);
          reset();
        }
        break;
      case 3:
        AUSGANG_PORT |= led_setzen[val];
        AUSGANG_PORT &= ~(led_loeschen[val]);
        if (get_key_short(TASTER))
        {
          kernal_id(DOLPHIN);
          valwert = val;
          reset();
        }
        if(get_key_long(TASTER))
        {
          kernal_id(DOLPHIN);
          eeprom_write_byte(&eeprom_kernalid, DOLPHIN);
          eeprom_write_byte(&eeprom_encoderid, val);
          reset();
        }
        break;
    }
  }
}

Das funktioniert soweit, das der Timer reagiert wenn der Encoder den 
Wert 0 hat. Also wenn ich bei CASE 0 die taste betätige und anschließen 
mit dem Encoder die Werte erhöhe, springt dieser nach etwa 3 Sekunden 
wieder auf CASE 0. Bei CASE 1,2,3 tut er das allerdings nicht. Da bleibt 
er auf dem aktuellen Wert.

[Thomas E. (thomase)]

Manfred W. schrieb:
> Das sieht in etwa so aus:
Schön. Und wie sieht der gesamte Code aus?
Wie soll man dir helfen, wenn du etwas wesentliches, nämlich die 
Deklaration der Variablen unterschlägst?

ISR (TIM0_COMPA_vect)
{
  if(timeCount > 0)
  {
    timeCount--;
    if(timeCount == 0)
    {
      val = valwert;
    }
  }
}

int8_t encode_read(void)
{
  int8_t val;

  cli();
  val = enc_delta;
  enc_delta = val & 1;
  sei();
  return val >> 1;
}

Hier passt mit val etwas schon mal ganz gewaltig nicht, also gevaltig.

mfg.

[Georg G. (df2au)]

Mach mal val volatile. Zugriffe auf eine Variable, die vom Interrupt 
verändert wird, werden sonst seltsam optimiert. Außerdem sollte val 
global sichtbar sein.
Gleiches gilt für timeCount.

Und es ist gefährlich, eine globale Variable mit gleichem Namen noch 
einmal lokal in einer Funktion zu nutzen. Da sollte man genau wissen, 
was man tut.

Compiliert das Programm wirklich ohne Warnungen?

[Lokus P. (derschatten)]

Compiliert fehlerfrei.

Ich häng mal den kompletten Code an.

Abgesehen davon habe ich gemerkt das die Auswertung noch einen anderen 
Fehler hat.
ich limitiere ja den Encoderwert im Hauptprogram von 0-3.
encode_read_timeout prüft aber den Wert schon vorher.
Das bedeutet wenn ich schneller drehe läuft der Timer gar nicht an, weil 
der Encoder auf einem Wert springt der nicht abgefragt wird in der 
Switch-Schleife.

Ich müßte das also komplett ummoddeln.

[Lokus P. (derschatten)]

Ich habe jetzt mal die Variablen eindeutig gemacht.
Das mit der limiterung des Encoder-Wertes weiß ich noch nicht wie ich 
das am besten löse.

Aber wie siehts mit dem Timer aus.
Also meine Überlegung war wie folgt:

1.) Beim drücken einer taste wird der aktuelle Encoderwert in die 
Variable "encoderwert" gespeichert.
2.) "encode_read_timeout()" überprüft un ob beim Drehen sich der 
aktuelle Encoderwert vom gespeicherten unterscheidet.
3.) Ist das der Fall läuft der Timer 300 rückwerts.
4.) Wird 0 erreicht erhält der Encoder den gespeicherten Wert (encoder = 
encoderwert;), was zur Folge hat das CASE das entsprechende Programm 
aufruft.

Aber, das funkt nicht so wie ich will.

[Stefan E. (sternst)]

Manfred W. schrieb:
> Aber, das funkt nicht so wie ich will.

Da fehlt immer noch (mindestens) ein volatile.

[Lokus P. (derschatten)]

und welches?
die Variable encoder?

Das wars aber auch nicht.

[Lokus P. (derschatten)]

Ich habe eher die Vermutung das der aus der Zählerschleife nicht 
herauskommt.

[Lokus P. (derschatten)]

Kann mir da keiner einen Tip geben?

[Lokus P. (derschatten)]

Warum funktioniert das nur wenn die Variable "encoderwert" 0 ist ?
Wenn die Variable größer 0 ist läuft die IF-Schleife permanent und 
erreicht somit nie das Ende des Zählers "timeCount"

int8_t encode_read_timeout(void)
{
  int8_t tmp = encode_read();

  if(tmp != encoderwert)
    timeCount = 3000;
  return tmp;
}

[Karl H. (kbuchegg)]

Das ergibt keinen Sinn

int8_t encode_read_timeout(void)
{
  int8_t tmp = encode_read();

  if(tmp != encoderwert)
    timeCount = 3000;
  return tmp;
}

...

    encoder += encode_read_timeout();                // ENCODER-Wert auslesen

    if (encoder > 3) encoder = 3;                    // ENCODER-Wert auf 3 limitieren
    if (encoder < 0) encoder = 0;

    switch(encoder)
    {
      case 0:
        AUSGANG_PORT |= led_setzen[encoder];          // Aktiven LED-PIN setzen
        AUSGANG_PORT &= ~(led_loeschen[encoder]);        // Inaktive LED-PIN löschen 
        if (get_key_short(TASTER))              // bei kurzem Tastendruck
        {
          kernal_id(CBM);
          encoderwert = encoder;
....

encoderwert ist also offensichtlich eine Zahl von 0 bis 4, eine 
'Einstellung' die sich aus der Aufsummierung der Drehbewegungen des 
Encoders ergibt.

In der Funktion encode_read_timeout kriegst du aber einen Wert von 
encode_read. Dieser Wert ist aber NICHT die Stellung des Encoders, 
sondern ein Mass dafür um wieviele Einheiten sich der Encoder seit der 
letzten Abfrage gedreht hat.

Du vergleichst hier Äpfel mit Birnen. Auf der einen Seite benutzt du 
einen Wert der die tatsächliche Stellung des Encoders wiederspiegelt, 
auf der anderen Seite eine Veränderung das Wertes. Das passt nicht 
zusammen.


Und PS: es gibt keine If-Schleifen. Eine Schleife ist ein 
Programmierkonstrukt, welches gegebenenfalls wiederholt wird, entweder 
bedingt oder unbedingt. Aber zentrales Element einer Schleife ist die 
Wiederholung. Ein If ist einfach nur die Auswahl von 2 Möglichkeiten und 
hat als solches mit einer Schleife überhaupt nichts zu tun.

[Lokus P. (derschatten)]

jetzt wo dus schreibst...

Die Encoderdrehung muß natürlich mit dem Wert 0 verglichen werden, alles 
andere ergibt keinen Sinn.

int8_t encode_read_timeout(void)
{
  int8_t tmp = encode_read();

  if(tmp != 0)
    timeCount = 3000;
  return tmp;
}

Allerdings reagiert das noch etwas seltsam.
Wenn ich den Encoder schneller drehe, wird anscheinend keine Drehung 
registriert.

OK, damit läuft es besser:

    if (encoder > 2) encoder = 3;                // ENCODER-Wert auf 3 limitieren
    if (encoder < 1) encoder = 0;

Kann es sein das diese Abfrage auch zu langsam ist für einen Encoder?
Kann ich den Encoder auf eine andere Art limitieren? Am besten bevor er 
gedreht wird.

[Lokus P. (derschatten)]

An dem if iegt es aber anscheinend nicht.
Sondern das die Abfrage ob der Encoder gedreht wurde nicht immer 
reagiert.
Das macht aber jetzt keinen Unterschied ob ich schnell oder langsam 
drehe.

[Karl H. (kbuchegg)]

Lokus Pokus schrieb:

> OK, damit läuft es besser:
>
>

>     if (encoder > 2) encoder = 3;                // ENCODER-Wert auf 3 
> limitieren
>     if (encoder < 1) encoder = 0;
>

> Kann es sein das diese Abfrage auch zu langsam ist für einen Encoder?

Denke ich eigentlich nicht.


Wie macht sich das 'Symptom' bemerkbar?


Ist eigentlich das hier

  if(timeCount > 0)
  {
    timeCount--;
    if(timeCount == 0)
    {
      encoder = encoderwert;
    }
  }

immer noch in der ISR?
Denn was das hier in Summe bewirkt (zumindest hab ich mir das im Kopf so 
zusammengereimt) ist: wird der Encoder gedreht, hat man nach Stillstand 
des Encoders 3 Sekunden (wenn die Timereinstellung stimmt), um die 
Verdrehung mit einem Tastendruck zu bestätigen. Bestätigt man nicht in 
der Zeit, dann wird die Auswahl wieder zurückgesetzt.
Wenn das so beabsichtigt ist, dann ist es ok.

[Karl H. (kbuchegg)]

Das hier

int8_t encoder          = 0;

sollte ein

volatile int8_t encoder          = 0;

sein, wenn der Wert sowohl in der ISR als auch in der Hauptschleife 
verändert werden soll und sich die beiden ganz sicher nicht ins Gehege 
kommen sollen.

[Lokus P. (derschatten)]

Ist bereits gesetzt.
Genauso ist es gedacht.

Wird in 3 Sekunden keine Taste betätigt springt der Encoder (die LED) 
auf den ursprünglichen Wert. Es soll somit der Wert der mit der Taste 
bestätigt wird, fixiert werden.

Also das Symptom ist folgendes:
Ich stelle den Encoder auf 0 und drücke die Taste. Der Wert 0 wird in 
Variable "encoderwert" gespeichert.
Drehe dann den Encoder in eine Richtung 2 oder 3 Stellen weiter wandert 
die LED und bleibt dort stehen. Der Zähler springt somit nicht an.
Drehe ich den Encoder einen schritt zurück oder vor arbeitet der Zähler 
wieder und der Encoder (die LED) springt auf 0.
Manchmal ist jedoch ein vor und zurück erforderlich damit der Zähler 
wieder läuft.

Das dubiose dabei ist, es passiert nicht immer.
Und es ist unabhängig davon auf welcher Position der Encoder beim start 
steht. Also wenn ich den Wert 1 fixiere, passiert das gleiche.

ich gehe mal davon aus das der Encoder selbst richtig zählt, sonst würde 
auch die entsprechende LED nicht leuchten.

[Lokus P. (derschatten)]

Ich habe nun folgendes Versucht um sichtbar zu machen ob die Routine 
überhaupt eine Drehung des Encoders erkennt.
Dazu habe ich mir bei

  if(tmp != 0)
  {
    timeCount = 3000;
    eeprom_write_byte(&eeprom_testid, tmp);
  }

wie man sieht die Variable ins EEPROM geschrieben und anschließend 
ausgelesen.

Der Wert ergibt 1. Also wird die Drehung korrekt erkannt.

Weiter gehts zum Timer:

  if(timeCount > 0)
  {
    timeCount--;
    if(timeCount == 0)
    {
      eeprom_write_byte(&eeprom_test2id, timeCount);
      encoder = encoderwert;
    }
  }

Hier ebenfalls zur Kontrolle ob dieser überhaupt läuft, den Wert ins 
EEPROM geschrieben (Zuvor den Wert test2id natürlich auf einen höheren 
Wert gesetzt)

Und auch hier passt noch alles. Der Timer zählt bis 0.

Das Problem muß also meiner Meinung nach zwischen:

encoder = encoderwert;

und

  for(;;)
  {
    ADCSRA &= !(1<<ADEN);                    // ADC dekativieren                    
    encoder += encode_read_timeout();              // ENCODER-Wert auslesen

liegen.

Hier wird dem Encoder mehr oder weniger ein Wert aufgedrängt. Könnte das 
dass Problem sein?

[Lokus P. (derschatten)]

Ich habe das jetzt mal so gelöst:

  if(timeCount > 0)
  {
    timeCount--;
  }

  for(;;)
  {
//    ADCSRA &= !(1<<ADEN);                    // ADC dekativieren    
        
    encoder += encode_read_timeout();              // ENCODER-Wert auslesen
    if (timeCount == 0)  encoder = encoderwert;
    if (encoder < 1) encoder = 0;                // ENCODER-Wert darf nicht kleiner als 0
    if (encoder > 2) encoder = 3;                // und größer als 3 werden

Sieht zwar nicht schön aus, dürfte aber funktionieren. Zumindest sind 
einige Drehtests problemlos gelaufen.

Vielleicht könnte man das noch eleganter lösen, wenn jemand eine Idee 
hat.