Arduino Uno Serial stört Timer1 Interrupts?

Gast #6474695
Lesenswert?

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):
1
#include <Wire.h>
2

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

7
const char *INIT_COMMAND = "re";
8
const char *TABLE_LAYOUT_COMMAND = "tl";
9
const char *HIGHLIGHT_BOX_COMMAND = "hb";
10
const char *DELIMITER = "|";
11

12
char readData[BUFLEN];
13
int currentIndex = 0;
14
unsigned long lastLEDsOn = 0;
15
char c;
16

17
void setup() {
18
  for (int i = 0; i < sizeof(LED_PINS); i++) {
19
    pinMode(i, OUTPUT);  
20
  }
21
  clearReadData();
22

23
  Serial.begin(9600);
24
  while(!Serial) {
25
     //wait
26
  }
27
}
28
void resetLEDs()
29
{
30
  for (int i = 0; i < sizeof(LED_PINS); i++) {
31
    digitalWrite(i, LOW);
32
  }
33
}
34

35
void setLEDs(byte input)
36
{
37
  for (int i = 0; i < sizeof(LED_PINS); i++) {
38
    byte mask = 1 << i;
39
    
40
    if (mask & input) {
41
      digitalWrite(LED_PINS[i], HIGH);
42
    } else {
43
      digitalWrite(LED_PINS[i], LOW);
44
    }
45
  }
46

47
  lastLEDsOn = millis();
48
}
49

50
void clearReadData() {
51
  for (int i = 0; i < BUFLEN; i++) {
52
    readData[i] = '\0';
53
  }
54
  currentIndex = 0;
55
}
56

57
void processReadData()
58
{
59
    char *token;
60
    token = strtok(readData, DELIMITER);
61

62
    if (token != NULL) {
63
      if (strcmp(token, INIT_COMMAND) == 0) {
64
        Serial.write("bin\n");
65
        return;
66
      }
67

68
      if (strcmp(token, HIGHLIGHT_BOX_COMMAND) == 0) {
69
        token = strtok(NULL, DELIMITER);
70
        setLEDs(atoi(token));
71
      }
72
    }
73
}
74

75
void loop() {
76
  if (lastLEDsOn != 0 && millis() - lastLEDsOn > LED_FLASH_DURATION) {
77
    lastLEDsOn = 0;
78
    resetLEDs();
79
  }
80
  
81
  while (Serial && Serial.available() > 0) {
82
    c = Serial.read();
83
    if (c == '\n') {
84
      processReadData();
85
      clearReadData();
86
    } else {
87
      readData[currentIndex] = c;
88

89
      if (currentIndex < BUFLEN - 1) {
90
        currentIndex = currentIndex + 1;
91
      }
92
    }
93
  }
94
}

Diese Lösung arbeitet mit
1
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:
1
#include <Wire.h>
2

3
const uint8_t BUFLEN = 20;
4
char readData[BUFLEN];
5
int currentIndex = 0;
6

7
const char *INIT_COMMAND = "re";
8
const char *TABLE_LAYOUT_COMMAND = "tl";
9
const char *HIGHLIGHT_BOX_COMMAND = "hb";
10
const char *DELIMITER = "|";
11

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

15
void setup() {
16
  for (int i = 0; i < sizeof(pin_indices); i++) {
17
    pinMode(i, OUTPUT);  
18
  }
19
  clearReadData();
20

21
  TCCR1A = 0;
22
  TCCR1B = 0;
23
  TCCR1B |= (1 << CS10);
24
  TCCR1B |= (1 << CS12);
25

26
  Serial.begin(9600);
27
  while(!Serial) {
28
    // wait
29
  }
30
}
31

32
void resetTimer()
33
{ 
34
  noInterrupts();
35

36
  TCNT1 = 18661;
37
  TIMSK1 |= (1 << TOIE1);
38
  
39
  interrupts();
40
}
41

42
ISR(TIMER1_OVF_vect)        
43
{
44
  resetLEDs();
45
  TIMSK1 &= ~(1 << TOIE1);
46
}
47

48
void resetLEDs()
49
{
50
  for (int i = 0; i < sizeof(pin_indices); i++) {
51
    digitalWrite(i, LOW);
52
  }
53
}
54

55
void setLEDs(byte input)
56
{
57
  for (int i = 0; i < sizeof(pin_indices); i++) {
58
    byte mask = 1 << i;
59
    
60
    if (mask & input) {
61
      digitalWrite(pin_indices[i], HIGH);
62
    } else {
63
      digitalWrite(pin_indices[i], LOW);
64
    }
65
  }
66
}
67

68
void clearReadData() {
69
  for (int i = 0; i < BUFLEN; i++) {
70
    readData[i] = '\0';
71
  }
72
  currentIndex = 0;
73
}
74

75
void processReadData()
76
{
77
    char *token;
78
    token = strtok(readData, DELIMITER);
79

80
    if (token != NULL) {
81
      if (strcmp(token, INIT_COMMAND) == 0) {
82
        Serial.write("bin\n");
83
        return;
84
      }
85

86
      if (strcmp(token, HIGHLIGHT_BOX_COMMAND) == 0) {
87
        token = strtok(NULL, DELIMITER);
88
        setLEDs(atoi(token));
89
        resetTimer();
90
      }
91
    }
92
}
93

94
void loop() {
95
  while (Serial && Serial.available() > 0) {
96
    c = Serial.read();
97
    if (c == '\n') {
98
      processReadData();
99
      clearReadData();
100
    } else {
101
      readData[currentIndex] = c;
102

103
      if (currentIndex < BUFLEN - 1) {
104
        currentIndex = currentIndex + 1;
105
      }
106
    }
107
  }
108
}

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:
1
int currentIndex = 0;
2
int pin_indices[] = {9, 8, 7, 6, 5, 4, 3, 2};
3

4
void setup() {
5
  for (int i = 0; i < sizeof(pin_indices); i++) {
6
    pinMode(i, OUTPUT);  
7
  }
8

9
  TCCR1A = 0;
10
  TCCR1B = 0;
11
  TCCR1B |= (1 << CS10);
12
  TCCR1B |= (1 << CS12);
13
}
14

15
void resetTimer()
16
{ 
17
  noInterrupts();
18

19
  TCNT1 = 18661;
20
  TIMSK1 |= (1 << TOIE1);
21
  
22
  interrupts();
23
}
24

25
ISR(TIMER1_OVF_vect)        
26
{
27
  resetLEDs();
28
  TIMSK1 &= ~(1 << TOIE1);
29
}
30

31
void resetLEDs()
32
{
33
  for (int i = 0; i < sizeof(pin_indices); i++) {
34
    digitalWrite(i, LOW);
35
  }
36
}
37

38
void setLEDs(byte input)
39
{
40
  for (int i = 0; i < sizeof(pin_indices); i++) {
41
    byte mask = 1 << i;
42
    
43
    if (mask & input) {
44
      digitalWrite(pin_indices[i], HIGH);
45
    } else {
46
      digitalWrite(pin_indices[i], LOW);
47
    }
48
  }
49
}
50

51
void loop() {
52
  delay(1);
53
  if (currentIndex % 5000 == 0 || currentIndex % 7000 == 0) {
54
    resetTimer();
55
    resetLEDs();
56
    setLEDs(currentIndex);
57
  } 
58

59
  currentIndex++;
60
}

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 (:
#6474734
Lesenswert?

1
void resetLEDs()
2
{
3
  for (int i = 0; i < sizeof(LED_PINS); i++) {
4
    digitalWrite(i, LOW);
5
  }

Nö, so nicht. Da fehlt der Zugriff auf LED_PINS[].
Eher so
1
void resetLEDs()
2
{
3
  for (int i = 0; i < sizeof(LED_PINS); i++) {
4
    digitalWrite(LED_PINS[i], LOW);
5
  }

Den Rest hab ich nur überflogen.
Gast #6474738
Lesenswert?

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.
#6474740
Lesenswert?

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();
}
Gast #6474758
Lesenswert?

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.
Gast #6474784
Lesenswert?

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:
1
constexpr byte pin_indices[]  {9, 8, 7, 6, 5, 4, 3, 2};
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.
#6474878
Lesenswert?

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.
1
#include <util/atomic.h>
2
...
3

4
void preSetTimer1 (void)  // Voreinstellungen, läuft noch nicht los
5
{
6
  ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
7
  {
8
    TCCR1B = 0;             // Resets
9
    TCCR1A = 0;             //
10
    TIMSK1 = 0;             //
11
    TCNT1  = 0;             //
12
    OCR1A  = COMPAREvalue;
13
    OCR1B  = COMPAREvalue / 2;
14
    TCCR1A = _BV(COM1B0) | _BV(COM1A0);   
15
    TCCR1B = _BV(WGM12);    
16
  }
17
}
18

19
void runTimer1 (const unsigned int prescaler)
20
{
21
  ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
22
  {
23
    switch (prescaler) {  
24
      case    1: TCCR1B |= _BV(CS10);              break;
25
      case    8: TCCR1B |= _BV(CS11);              break;
26
      case   64: TCCR1B |= _BV(CS11) | _BV(CS10);  break;
27
      case  256: TCCR1B |= _BV(CS12);              break;
28
      case 1024: TCCR1B |= _BV(CS12) | _BV(CS10);  break;
29
      default:   break;
30
    }
31
  }
32
}
33

34
void stopTimer1 (void)
35
{
36
  ATOMIC_BLOCK (ATOMIC_RESTORESTATE)
37
  {
38
    TCCR1B &= ~( _BV(CS12) | _BV(CS11) | _BV(CS10) );
39
  }
40
}

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

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.
Gast #6474920
Lesenswert?

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.
#6474937
Lesenswert?

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
1
while (Serial && Serial.available() > 0) {
2
in 
3
while (Serial.available() > 0) {
4

5
und char c lokal machen.

und statt ständig die Größe ermitteln entweder einmal ermitteln oder 
fest einsetzen und überall anwenden.
1
const byte ANZAHL_LEDS = 8;
2
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.
Gast #6474943
Lesenswert?

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.
Gast #6474964
Lesenswert?

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.
Gast #6475322
Lesenswert?

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 (:

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren