Arduino Uno Serial stört Timer1 Interrupts?

[TrudelStrudel (Gast)]

Guten Abend zusammen,

ich habe vor kurzem meine ersten Schritte mit dem Arduino uno rev3 
gewagt und bin jetzt auf ein Problem gestoßen, für dessen Lösung mir der 
Ansatz fehlt.

Grundsätzlich möchte ich gesteuert durch Kommunikation über die serielle 
Schnittstelle LEDs anschalten können. Wenn 3 Sekunden nicht der "Befehl" 
kommt andere LEDs zu schalten, sollen aktuell noch leuchtende LEDs 
ausgeschalten werden.

Ich habe jetzt eine Lösung die genau das tut (es handelt sich dabei um 
den ersten "c Code" den ich je geschrieben habe, also habt erbarmen. Man 
sieht ihm meine Java Wurzeln vermutlich sehr an):

#include <Wire.h>

const unsigned short int LED_FLASH_DURATION = 3000;
const uint8_t BUFLEN = 20;
const int LED_PINS[] = {9, 8, 7, 6, 5, 4, 3, 2};

const char *INIT_COMMAND = "re";
const char *TABLE_LAYOUT_COMMAND = "tl";
const char *HIGHLIGHT_BOX_COMMAND = "hb";
const char *DELIMITER = "|";

char readData[BUFLEN];
int currentIndex = 0;
unsigned long lastLEDsOn = 0;
char c;

void setup() {
  for (int i = 0; i < sizeof(LED_PINS); i++) {
    pinMode(i, OUTPUT);  
  }
  clearReadData();

  Serial.begin(9600);
  while(!Serial) {
     //wait
  }
}
void resetLEDs()
{
  for (int i = 0; i < sizeof(LED_PINS); i++) {
    digitalWrite(i, LOW);
  }
}

void setLEDs(byte input)
{
  for (int i = 0; i < sizeof(LED_PINS); i++) {
    byte mask = 1 << i;
    
    if (mask & input) {
      digitalWrite(LED_PINS[i], HIGH);
    } else {
      digitalWrite(LED_PINS[i], LOW);
    }
  }

  lastLEDsOn = millis();
}

void clearReadData() {
  for (int i = 0; i < BUFLEN; i++) {
    readData[i] = '\0';
  }
  currentIndex = 0;
}

void processReadData()
{
    char *token;
    token = strtok(readData, DELIMITER);

    if (token != NULL) {
      if (strcmp(token, INIT_COMMAND) == 0) {
        Serial.write("bin\n");
        return;
      }

      if (strcmp(token, HIGHLIGHT_BOX_COMMAND) == 0) {
        token = strtok(NULL, DELIMITER);
        setLEDs(atoi(token));
      }
    }
}

void loop() {
  if (lastLEDsOn != 0 && millis() - lastLEDsOn > LED_FLASH_DURATION) {
    lastLEDsOn = 0;
    resetLEDs();
  }
  
  while (Serial && Serial.available() > 0) {
    c = Serial.read();
    if (c == '\n') {
      processReadData();
      clearReadData();
    } else {
      readData[currentIndex] = c;

      if (currentIndex < BUFLEN - 1) {
        currentIndex = currentIndex + 1;
      }
    }
  }
}

Diese Lösung arbeitet mit

millis()

. Ursprünglich wollte ich es, aus Interesse und weil ich das ganze 
primär mache um zu lernen, selber mit dem Timer1 und einem Overflow 
Interupt machen.

Das sieht dann bei mir so aus:

#include <Wire.h>

const uint8_t BUFLEN = 20;
char readData[BUFLEN];
int currentIndex = 0;

const char *INIT_COMMAND = "re";
const char *TABLE_LAYOUT_COMMAND = "tl";
const char *HIGHLIGHT_BOX_COMMAND = "hb";
const char *DELIMITER = "|";

char c;
int pin_indices[] = {9, 8, 7, 6, 5, 4, 3, 2};

void setup() {
  for (int i = 0; i < sizeof(pin_indices); i++) {
    pinMode(i, OUTPUT);  
  }
  clearReadData();

  TCCR1A = 0;
  TCCR1B = 0;
  TCCR1B |= (1 << CS10);
  TCCR1B |= (1 << CS12);

  Serial.begin(9600);
  while(!Serial) {
    // wait
  }
}

void resetTimer()
{ 
  noInterrupts();

  TCNT1 = 18661;
  TIMSK1 |= (1 << TOIE1);
  
  interrupts();
}

ISR(TIMER1_OVF_vect)        
{
  resetLEDs();
  TIMSK1 &= ~(1 << TOIE1);
}

void resetLEDs()
{
  for (int i = 0; i < sizeof(pin_indices); i++) {
    digitalWrite(i, LOW);
  }
}

void setLEDs(byte input)
{
  for (int i = 0; i < sizeof(pin_indices); i++) {
    byte mask = 1 << i;
    
    if (mask & input) {
      digitalWrite(pin_indices[i], HIGH);
    } else {
      digitalWrite(pin_indices[i], LOW);
    }
  }
}

void clearReadData() {
  for (int i = 0; i < BUFLEN; i++) {
    readData[i] = '\0';
  }
  currentIndex = 0;
}

void processReadData()
{
    char *token;
    token = strtok(readData, DELIMITER);

    if (token != NULL) {
      if (strcmp(token, INIT_COMMAND) == 0) {
        Serial.write("bin\n");
        return;
      }

      if (strcmp(token, HIGHLIGHT_BOX_COMMAND) == 0) {
        token = strtok(NULL, DELIMITER);
        setLEDs(atoi(token));
        resetTimer();
      }
    }
}

void loop() {
  while (Serial && Serial.available() > 0) {
    c = Serial.read();
    if (c == '\n') {
      processReadData();
      clearReadData();
    } else {
      readData[currentIndex] = c;

      if (currentIndex < BUFLEN - 1) {
        currentIndex = currentIndex + 1;
      }
    }
  }
}

Allerdings funktioniert das nicht. Die LEDs werden nie ausgeschaltet. 
Ich habe dann eine Menge Dinge probiert und kam dann irgendwann auf den 
Trichter, dass das vielleicht irgendwie mit der Kommunikation über die 
serielle Schnittstelle zu tun hat, da diese wohl auch interrupts benutzt 
(?). Ich habe dann eine Variante gebaut, die sowas ähnliches wie die 2. 
Variante macht, nur ohne dabei Serial zu benutzen und tatsächlich hat 
das dann funktioniert:

int currentIndex = 0;
int pin_indices[] = {9, 8, 7, 6, 5, 4, 3, 2};

void setup() {
  for (int i = 0; i < sizeof(pin_indices); i++) {
    pinMode(i, OUTPUT);  
  }

  TCCR1A = 0;
  TCCR1B = 0;
  TCCR1B |= (1 << CS10);
  TCCR1B |= (1 << CS12);
}

void resetTimer()
{ 
  noInterrupts();

  TCNT1 = 18661;
  TIMSK1 |= (1 << TOIE1);
  
  interrupts();
}

ISR(TIMER1_OVF_vect)        
{
  resetLEDs();
  TIMSK1 &= ~(1 << TOIE1);
}

void resetLEDs()
{
  for (int i = 0; i < sizeof(pin_indices); i++) {
    digitalWrite(i, LOW);
  }
}

void setLEDs(byte input)
{
  for (int i = 0; i < sizeof(pin_indices); i++) {
    byte mask = 1 << i;
    
    if (mask & input) {
      digitalWrite(pin_indices[i], HIGH);
    } else {
      digitalWrite(pin_indices[i], LOW);
    }
  }
}

void loop() {
  delay(1);
  if (currentIndex % 5000 == 0 || currentIndex % 7000 == 0) {
    resetTimer();
    resetLEDs();
    setLEDs(currentIndex);
  } 

  currentIndex++;
}

Wisst ihr, was in der 2. Variante falsch läuft? Wenn das Problem kein 
einfacher Fehler in meiner Logik ist sondern irgendwie damit 
zusammenhängt das Serial irgendwie den Timer1 benutzt/manipuliert, dann 
reicht mir ein Stubs in die richtige Richtung.

Besten Dank für das Lesen bis hierher.
Schönen Abend noch (:

[Falk B. (falk)]

TrudelStrudel schrieb:
> Ich habe jetzt eine Lösung die genau das tut (es handelt sich dabei um
> den ersten "c Code" den ich je geschrieben habe, also habt erbarmen. Man
> sieht ihm meine Java Wurzeln vermutlich sehr an):

Und scheinbar auch dein erster Forumsbeitrag? Lange Quelltexte gehören 
in den Anhang!

[TrudelStrudel (Gast)]

"Lang" ist relativ. Aber gut, für das nächste mal dann. Ändern kann ich 
es jetzt nicht mehr.

[Falk B. (falk)]

void resetLEDs()
{
  for (int i = 0; i < sizeof(LED_PINS); i++) {
    digitalWrite(i, LOW);
  }

Nö, so nicht. Da fehlt der Zugriff auf LED_PINS[].
Eher so

void resetLEDs()
{
  for (int i = 0; i < sizeof(LED_PINS); i++) {
    digitalWrite(LED_PINS[i], LOW);
  }

Den Rest hab ich nur überflogen.

[TrudelStrudel (Gast)]

Ohje das stimmt wohl.
Interessant, dass mir das bei den "funktionierenden" Varianten nicht 
aufgefallen ist.

Aber auch wenn ich das korrigiere funktioniert die 2. Variante, also die 
mit Timmer und Serial, nicht.

[Falk B. (falk)]

for (int i = 0; i < sizeof(LED_PINS); i++) {
    pinMode(i, OUTPUT);
  }

Hier fehlt das auch. Hier ist es drin.

void setLEDs(byte input)
{
  for (int i = 0; i < sizeof(LED_PINS); i++) {
    byte mask = 1 << i;

    if (mask & input) {
      digitalWrite(LED_PINS[i], HIGH);
    } else {
      digitalWrite(LED_PINS[i], LOW);
    }
  }

  lastLEDsOn = millis();
}

[TrudelStrudel (Gast)]

Das ist ja hochgradig verwirrend.
Wie konnte das denn bisher funktioniert haben?!

Noch irritierender ist, dass es jetzt noch Korrektur nicht mehr 
funktioniert. Aber das schaue ich mir morgen weiter an. Vielen Dank 
schon mal.

[Falk B. (falk)]

TrudelStrudel schrieb:
> Das ist ja hochgradig verwirrend.
> Wie konnte das denn bisher funktioniert haben?!

Ganz einfach, deine Schleife geht von 0-7, deine Pins von 2-9, da gibt 
es ein paar Überschneidungen.

[Einer K. (Gast)]

Falk B. schrieb  :
> Ganz einfach, deine Schleife geht von 0-7,
Das stimmt nicht, sie geht von 0 bis 15


TrudelStrudel schrieb:
> sizeof(pin_indices)
Ist doppelt so groß wie das Array.

Abhilfe:
int pin_indices[] = {9, 8, 7, 6, 5, 4, 3, 2}; // falsch
constexpr byte pin_indices[]  {9, 8, 7, 6, 5, 4, 3, 2}; // besser

TrudelStrudel schrieb:
> (es handelt sich dabei um
> den ersten "c Code" den ich je geschrieben habe,
Das glaube ich dir nicht, denn das ist C++ (auch wenn du das noch nicht 
weißt)

TrudelStrudel schrieb:
> for (int i = 0; i < sizeof(pin_indices); i++) {
>     pinMode(i, OUTPUT);
>   }


Alternativ:

constexpr byte pin_indices[]  {9, 8, 7, 6, 5, 4, 3, 2};
for (const byte pin:pin_indices) pinMode(pin, OUTPUT);

Dank des "range based for loop" kann es auch keine 
Bereichsüberschreitung beim Arrayzugriff geben.

[Veit D. (devil-elec)]

Hallo,

dein Timer 1 wird sicherlich nicht das machen was du denkst. Die Timer 
werden für PWM (analogWrite) im Hintergrund voreingestellt. Timer 0 für 
millis. Das heißt, möchte man einen Timer selbst konfigurieren, muss man 
alle Timerregister löschen oder durch direkte Zuweisung überschreiben. 
Nur verodern läuft schief. Dann bleiben die bisherigen gesetzten Bits 
erhalten. Ich habe mir angewöhnt erstmal alles zu löschen und dann neu 
zu konfigurieren. Dabei kann man ohne Gefahr verodern |=. Ich fange beim 
konfigureren immer mit dem stoppen des Timers an, hier Register TCCR1B.

Bsp.

#include <util/atomic.h>
...

void preSetTimer1 (void)  // Voreinstellungen, läuft noch nicht los
{
  ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
  {
    TCCR1B = 0;             // Resets
    TCCR1A = 0;             //
    TIMSK1 = 0;             //
    TCNT1  = 0;             //
    OCR1A  = COMPAREvalue;
    OCR1B  = COMPAREvalue / 2;
    TCCR1A = _BV(COM1B0) | _BV(COM1A0);   
    TCCR1B = _BV(WGM12);    
  }
}

void runTimer1 (const unsigned int prescaler)
{
  ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
  {
    switch (prescaler) {  
      case    1: TCCR1B |= _BV(CS10);              break;
      case    8: TCCR1B |= _BV(CS11);              break;
      case   64: TCCR1B |= _BV(CS11) | _BV(CS10);  break;
      case  256: TCCR1B |= _BV(CS12);              break;
      case 1024: TCCR1B |= _BV(CS12) | _BV(CS10);  break;
      default:   break;
    }
  }
}

void stopTimer1 (void)
{
  ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
  {
    TCCR1B &= ~( _BV(CS12) | _BV(CS11) | _BV(CS10) );
  }
}

Edit:
In welchen Modus soll dein Timer überhaupt laufen? Was soll er bewirken?

[Veit D. (devil-elec)]

Hallo,

mach einmal das hier:

In der IDE alle Ausgaben einschalten.
Datei > Voreinstellungen >
- Ausführliche Ausgabe während > beide Haken rein
- Compilerwarnungen > "ALLE"
Zeilennummern und Codefaltung sind auch hilfreich.
Speichern beim Überprüfen und Hochladen würde ich abschalten.

[Einer K. (Gast)]

Veit D. schrieb:
> Das heißt, möchte man einen Timer selbst konfigurieren, muss man
> alle Timerregister löschen oder durch direkte Zuweisung überschreiben.

Macht ihm ja...
Zumindest die wichtigen beiden Kontrollregister.

TrudelStrudel schrieb:
> TCCR1A = 0;
> TCCR1B = 0;
> TCCR1B |= (1 << CS10);
> TCCR1B |= (1 << CS12);

Sieht zumindest für mich so aus.

Veit D. schrieb:
> In welchen Modus soll dein Timer überhaupt laufen? Was soll er bewirken?
Das würde mich auch interessieren.

[Veit D. (devil-elec)]

Hallo,

das TIMSK1 Register wird nicht gelöscht und mit dem TOIE1 Bit "nur" 
verodert. Müßte man testen ob da noch ein Bit gesetzt ist und ob dadurch 
etwas stört.

Beim Timersetting habe ich herausgefunden das es der 3s Timer sein soll. 
Prescaler 1024 im Normal Modus. Hier wird das Interrupt Bit TOIE1 
gelöscht zum abschalten und zusammen mit dem Counter Startwert wieder 
gesetzt. Sollte eigentlich keine Probleme machen, auch wenn ich den 
Timer stoppen/starten würde. Vielleicht statt gezieltes Bit löschen 
TIMSK1 = 0 ganz löschen.

Was man noch ändern sollte wäre

while (Serial && Serial.available() > 0) {
in 
while (Serial.available() > 0) {

und char c lokal machen.

und statt ständig die Größe ermitteln entweder einmal ermitteln oder 
fest einsetzen und überall anwenden.

const byte ANZAHL_LEDS = 8;
const byte pin_indices[ANZAHL_LEDS] = {9, 8, 7, 6, 5, 4, 3, 2};

Ansonsten sehe ich aktuell auch noch nicht das große Problem. Da hilft 
nur Sketchrückbau und die Timergeschichte und Einlesegeschichte getrennt 
zu testen. Aktuell fällt mir kein Grund ein warum irgendein Timer die 
Serielle stören sollte.

Bekommst du mit dem asm Dump auch DWARF Errors?
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 15ee
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 15fa
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 1606
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 1612
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 161e
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 162a
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 1636
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 1642
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 164e
C:\avrToolchain\avr-gcc-10.2.0_mingw64_binutils2.35/bin/avr-objdump: 
DWARF error: could not find variable specification at offset 165a

Auch wenn ich den Sketch korrigiere damit er keine Warnungen mehr wirft 
kommen genau die gleichen DWARF Errors. Kompiliere ich irgendwas anderes 
gibts keine DWARF Errors.

[Einer K. (Gast)]

Veit D. schrieb:
> das TIMSK1 Register wird nicht gelöscht und mit dem TOIE1 Bit "nur"
> verodert. Müßte man testen ob da noch ein Bit gesetzt ist und ob dadurch
> etwas stört.
Das Register ist 0.
Arduino fasst das nicht an.

Veit D. schrieb:
> Bekommst du mit dem asm Dump auch DWARF Errors?
Nicht getestet.

Aber da der Index fürchterlich aus dem Ruder läuft, sind wir im Bereich 
des UB, und darum darf das auch passieren.

Nachtrag
Eine schnelle Suche sagt:
> error::dwarf - dwarf debuginfo quality problems ...
> If a script requires such data, but the compiler did not
> preserve enough of it, pass-2 errors may result.
Passt also irgendwie schon, die Meldung.

[Veit D. (devil-elec)]

Hallo,

Timer, okay alles klar. Danke.

Die Indexe hatte ich korrigiert, dennoch DWARF Errors. Habe dann solange 
alles rausgehauen bis das weg war. Am Ende liegst am Wire.h include. 
Irre. Benötigt der TO sowieso nicht.

[Veit D. (devil-elec)]

Hallo,

habs einmal korrigiert soweit ich das überblicke. Ungetestet mangels 
Syntax der Kommandos.

[Einer K. (Gast)]

Getestet:
Du hast recht, nach korrigiertem Index schimpft der 10.2.0, bzw. die 
binutils2.35, auch bei mir.
avr-gcc-10.1.0 und die Vorgänger zeigen das "Wire" Problem nicht.

Nagut...
Werde Wire mal (irgendwann) daraufhin untersuchen, wo/ob es da 
klemmt.....
Befürchte allerdings, dass eher avr-objdump oder die *.elf Erzeugung 
wackelt.

[TrudelStrudel (Gast)]

Hallo zusammen.

erstmal vielen Dank für die vielen guten und hilfreichen Tipps.

Nachdem ich jetzt die ganzen Indexgeschichten korrigiert habe, läuft es 
auch mit Timer + Serial und ich habe eine Menge gelernt (:

[Veit D. (devil-elec)]

Hallo,

freut uns alle wenns geholfen hat.

Wenn ich leicht Offtopic werden darf?
Würde vorschlagen zum Thema DWARF Error hier weiter zu machen.
Beitrag "DWARF error: invalid abstract instance DIE ref"