Hi everyone, I have a strange Problem: I try to increment a counter inside a Interrupt Service Routine, but it seems as if it doesn't work whysoever. The ISR is triggered by a Timer Compare. The counter either gets set null or gets lost. I tried declaring it volatile, but this didn't work either. Here the Code in which i unsuccessfully tried sending the memory address of the counter in the loop and in the ISR: /* this goes on your arduino for use with Processing example SimpleSerialArduinoscope */ #include <avr/io.h> #include <avr/interrupt.h> volatile int val[512] = { 0 }; int i = 0; long u; void setup() { //pinMode(0, INPUT); Serial.begin(115200); cli(); // disable interrupts while messing with their settings TCCR1A = 0x00; // clear default timer settings, this kills the millis() funciton TCCR1B = 0x00; TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode TCCR1B &= ~(0 << CS12); // Set timer prescaling by setting 3 bits TCCR1B |= (1 << CS11 | 1 << CS10); // 001=1, 010=8, 011=64, 101=1024 TIMSK1 |= (1 << OCIE1B); // Enable CTC interrupt OCR1B = 5000; // Set CTC compare value //ADCSRA |= (1 << ADEN); // enable ADC ADMUX = B01100000; // left adjusted output to increase sample rate, 8bit mode SMCR |= (1 << SM0); // Enable sleep AD conversion mode SMCR &= ~( (1 << SM2) | (1 << SM1) ); //SREG |= (1 << ADIE); // Enable ADC ready interrupt ADCSRB |= ( ( 1 << ADTS2 | 1 << ADTS0 ) ); // configure ADC to trigger on Timer Compare Match B ADCSRB &= ~(1 << ADTS1); sei(); // turn interrupts back on ADCSRA = B11001111; // set ADC prescaler to 128, ADC EN, ADC Start } void loop() { // i += 1; // if(i==512){ // i=0; // } //u = (long)i; Serial.println((HEX)&i); } void tx(){ cli(); for (int q=0;q<512;q++){ Serial.print(val[q]); Serial.print(","); } Serial.println(); i = 0; sei(); } void rx(){ } ISR(ADC_vect) { //Serial.println((int)ADCH); val[i] = (int)ADCH; i++; Serial.println((HEX)&i); if(i==512){ tx(); } } The Platform is a Atmel Atmega 1280. Target is to build a simple USB oscilloscope. Has anybody an idea what the problem could be? Thanks in advance, ToKo
@ Tobias K. (tobias_k83) >I have a strange Problem: I try to increment a counter inside a >Interrupt Service Routine, but it seems as if it doesn't work whysoever. i must be volatile too, see article [[Interrupt]]. Regards Falk
Ahhhh, no links from mikrocontroller.net are working here :-0 http://www.mikrocontroller.net/articles/Interrupt
volatile is one thing. You need it. But the other thing is, that you have to make sure, to access i in an atomare fashion. You failed to do that. BTW: even if this is a test programm, your ISR does to much lengthy things and corrupts your timing. Output to the RS232 does not belong in an ISR.
volatile didnt work either... =(
And please: leave the Interrupt Enable Bits alone inside an ISR! No cli and sei That is one sure way to shoot yourself in the knee
> volatile didnt work either... =(
volatile ist necessary in that case but it is only half of the story.
atomic access is the other half.
okay, atomic access is totally new to me. how do I do that? How can i shorten or modify my code so it doesn't corrupt the timing? my plan was to fill an array and transmit it when it's full.
okay, my idea would be to put the tx() inside an if(i==512)... into loop, so that cli/sei can be used (and transmission is not disturbed). but what about atomic access? Did you mean the functions of <util/atomic.h>?
cleaned version without atomic access: #include <avr/io.h> #include <avr/interrupt.h> volatile int val[512] = { 0 }; volatile int i = 0; void setup() { Serial.begin(115200); cli(); // disable interrupts while messing with their settings TCCR1A = 0x00; // clear default timer settings, this kills the millis() funciton TCCR1B = 0x00; TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode TCCR1B &= ~(0 << CS12); // Set timer prescaling by setting 3 bits TCCR1B |= (1 << CS11 | 1 << CS10); // 001=1, 010=8, 011=64, 101=1024 TIMSK1 |= (1 << OCIE1B); // Enable CTC interrupt OCR1B = 5000; // Set CTC compare value ADMUX = B01100000; // left adjusted output to increase sample rate, 8bit mode SMCR |= (1 << SM0); // Enable sleep AD conversion mode SMCR &= ~( (1 << SM2) | (1 << SM1) ); //SREG |= (1 << ADIE); // Enable ADC ready interrupt ADCSRB |= ( ( 1 << ADTS2 | 1 << ADTS0 ) ); // configure ADC to trigger on Timer Compare Match B ADCSRB &= ~(1 << ADTS1); sei(); // turn interrupts back on ADCSRA = B11001111; // set ADC prescaler to 128, ADC EN, ADC Start } void loop() { if(i==512){ } tx(); } void tx(){ cli(); for (int q=0;q<512;q++){ Serial.print(val[q]); Serial.print(","); } Serial.println(); i = 0; sei(); } void rx(){ } ISR(ADC_vect) { val[i] = (int)ADCH; i++; } sorry, I am still a bit a kind of noob, is this code better for doing the job? According to the link above it would be useful if I reduce the array size to 128 as well as the datatype to Byte to workaround using ATOMIC_BLOCK(ATOMIC_FORCEON) { } right?
Sorry, I meant an array size of 255 -> byte
@ Tobias K. (tobias_k83) Iam sure you are a navtive german speaker. So it is much easyer to do so . . . ;-) >okay, my idea would be to put the tx() inside an if(i==512)... into >loop, so that cli/sei can be used (and transmission is not disturbed). >but what about atomic access? Did you mean the functions of ><util/atomic.h>? Read the article. http://www.mikrocontroller.net/articles/Interrupt
Hi Falk, richtig geraten =) hab den Artikel gelesen, aber so richtig verstanden hab ich ihn nicht: PINx = (PORTx & 0x0F) ^ lcd_data; ist laut Artikel die Sollvorgehensweise. Was bedeutet das in meinem Fall? ich meine Atomarer als val[i]=ADCH gehts ja nicht, oder soll ich da direkt mit Speicheradressen statt mit val[i] arbeiten? Atomar heißt ja nur, dass währenddessen nichts anderes ausgeführt werden kann oder?
Tobias K. wrote: > PINx = (PORTx & 0x0F) ^ lcd_data; > ist laut Artikel die Sollvorgehensweise. Das ist eine mögliche Vorgehensweise, aber nur bei Ports, nicht bei Speichervariablen. Bei denen ist eine dieser ATOMIC_BLOCK Klammern genau richtig. Und zwar bei jedem Zugriff ausserhalb der ISR. Nicht atomar kann heissen: unteres Byte lesen (alt), Interrupt mischt mit und zählt hoch, oberes Byte lesen (neu). Das gibt dann gelesenen Datensalat und ist nur durch temporäre Abschaltung des Interrupts zu vermeiden.
@Tobias K. (tobias_k83) >richtig geraten =) hab den Artikel gelesen, aber so richtig verstanden >hab ich ihn nicht: Dann halt nochmal lesen und nachdenken. > PINx = (PORTx & 0x0F) ^ lcd_data; >ist laut Artikel die Sollvorgehensweise. Ist ein Sonderfall für IO-Ports bei neueren AVRs. Ungünstiges Beispiel. > Was bedeutet das in meinem Fall? > ich meine Atomarer als val[i]=ADCH gehts ja nicht, Das ist auch OK, weil es in der ISR ist. Die kann nicht unterbrochen werden (wenn man keine Tricks mit verschachtelten Interrupts macht). Aber warum nicht val[i]=AWDW; ? Das ist ein einfacher, korrekter, VOLLSTÄNDIGER Zugriff auf den ADC. > oder soll ich >da direkt mit Speicheradressen statt mit val[i] arbeiten? Nein. > Atomar heißt > ja nur, dass währenddessen nichts anderes ausgeführt werden kann oder? Sicher, aber das ist auch wichtig. Atomar muss der Zugriff im Hauptprogramm ein. Warum, steht im Artikel. AUsserdem ist deine Variante mal wiedr sehr ungünstig. Atomat muss und sollte NUR das Kopieren der globalen Variable i (ungünstiger Name!) in eine lokale Variable sein. Die Ausgabe sollte NICHT in den atomaren Block, weil sie die ganze Zeit deine Interrupts blockiert. Eher so.
1 | #include <avr/io.h> |
2 | #include <avr/interrupt.h> |
3 | |
4 | volatile int val[512] = { 0 }; |
5 | volatile int i = 0; |
6 | |
7 | void setup() { |
8 | Serial.begin(115200); |
9 | cli(); // disable interrupts while |
10 | messing with their settings |
11 | TCCR1A = 0x00; // clear default timer |
12 | settings, this kills the millis() funciton |
13 | TCCR1B = 0x00; |
14 | TCCR1B |= (1 << WGM12); // Configure timer 1 for |
15 | CTC mode |
16 | TCCR1B &= ~(0 << CS12); // Set timer prescaling by |
17 | setting 3 bits |
18 | TCCR1B |= (1 << CS11 | 1 << CS10); // 001=1, 010=8, 011=64, |
19 | 101=1024 |
20 | TIMSK1 |= (1 << OCIE1B); // Enable CTC interrupt |
21 | OCR1B = 5000; // Set CTC compare value |
22 | |
23 | ADMUX = B01100000; // left adjusted output to |
24 | increase sample rate, 8bit mode |
25 | SMCR |= (1 << SM0); // Enable sleep AD |
26 | conversion mode |
27 | SMCR &= ~( (1 << SM2) | (1 << SM1) ); |
28 | //SREG |= (1 << ADIE); // Enable ADC ready
|
29 | interrupt
|
30 | ADCSRB |= ( ( 1 << ADTS2 | 1 << ADTS0 ) ); // configure ADC to trigger |
31 | on Timer Compare Match B |
32 | ADCSRB &= ~(1 << ADTS1); |
33 | sei(); // turn interrupts back on |
34 | ADCSRA = B11001111; // set ADC prescaler to |
35 | 128, ADC EN, ADC Start |
36 | }
|
37 | |
38 | void loop() { |
39 | if(i==512){ |
40 | }
|
41 | tx(); |
42 | }
|
43 | |
44 | void tx(){ |
45 | int tmp; |
46 | |
47 | cli(); |
48 | tmp = i; |
49 | i = 0; |
50 | sei(); |
51 | for (int q=0;q<512;q++){ |
52 | Serial.print(val[q]); |
53 | Serial.print(","); |
54 | }
|
55 | Serial.println(); |
56 | }
|
57 | |
58 | void rx(){ |
59 | |
60 | }
|
61 | |
62 | ISR(ADC_vect) |
63 | {
|
64 | val[i] = ADCW; |
65 | i++; |
66 | }
|
MfG Falk
okay, cool. Das heißt ISR(ADC_vect) { val[i] = ADCH; i++; } passt soweit, nur in der Loop und (?) in tx() kommen die Klammern zum Einsatz? Habe die Variablen alle mal auf 8bit runtergeschraubt, was ja die Zwangsatomisierung erübrigen sollte, da kam aber auch nix bei rum... Vielen Dank schonmal für euer feedback!
Hi Falk, ich wollte absichtlich nur das High Register lesen, weil man dadurch die Sampling rate hoch bekommt. hab den ADC auch entsprechend left adjusted konfiguriert. Werde ich gleich mal probieren, Deine Variante =)
BTW: warum ist i als Name ungünstig?
mist, kann es sein dass ich auch das globale array in ein lokales kopieren muss? In der Form funktionierts noch nicht...
@ Tobias K. (tobias_k83) >BTW: warum ist i als Name ungünstig? Das ist ein Name für einen (lokalen) Index oder so, was man halt immer mal wieder schnell braucht. Globale Variablen sollte man mit aussagekräftigen Namen versehen, oder z.B. duch eine Vorsilbe ala gTime (g wie global). >mist, kann es sein dass ich auch das globale array in ein lokales >kopieren muss? Sicher, dort wird ja auch in der ISR zugegriffen. MFG Falk
grrrr... der globale Counter wird immer noch nicht hochgezählt...
Woran erkennst du das? Poste VOLLSTÄNDIGEN Code als Anhang.
ich sende aus der ISR heraus gCnt (anhang geht irgendwie nich... hier noch mal gepastet:) /* this goes on your arduino for use with Processing example SimpleSerialArduinoscope */ #include <avr/io.h> #include <avr/interrupt.h> //#include <util/atomic.h> volatile int val[255] = { 0 }; volatile byte gCnt = 0; void setup() { Serial.begin(115200); cli(); // disable interrupts while messing with their settings TCCR1A = 0x00; // clear default timer settings, this kills the millis() funciton TCCR1B = 0x00; TCCR1B |= (1 << WGM12); // Configure timer 1 for CTC mode TCCR1B &= ~(0 << CS12); // Set timer prescaling by setting 3 bits TCCR1B |= (1 << CS11 | 1 << CS10); // 001=1, 010=8, 011=64, 101=1024 TIMSK1 |= (1 << OCIE1B); // Enable CTC interrupt OCR1B = 5000; // Set CTC compare value ADMUX = B01100000; // left adjusted output to increase sample rate, 8bit mode SMCR |= (1 << SM0); // Enable sleep AD conversion mode SMCR &= ~( (1 << SM2) | (1 << SM1) ); //SREG |= (1 << ADIE); // Enable ADC ready interrupt ADCSRB |= ( ( 1 << ADTS2 | 1 << ADTS0 ) ); // configure ADC to trigger on Timer Compare Match B ADCSRB &= ~(1 << ADTS1); sei(); // turn interrupts back on ADCSRA = B11001111; // set ADC prescaler to 128, ADC EN, ADC Start } void loop() { if(gCnt==255){ tx(); } } void tx(){ byte tmp; int valTmp[255]; cli(); tmp = gCnt; for(byte i;i<255;i++){ valTmp[i] = val[i]; } gCnt=0; sei(); for (byte q=0;q<255;q++){ Serial.print(val[q]); Serial.print(","); } Serial.println(); } void rx(){ } ISR(ADC_vect) { val[gCnt] = ADCW; Serial.print((int)gCnt); gCnt += 1; }
Öhm, sprach ich nicht von VOLLSTÄNDIGEM Code? Ich sehe kein main()
Ja sorry, hab ich vergessen zu sagen: ich programmiere den Atmega mit der Arduinoplattform, da gibts meines Wissens kein main.
Okay, ich habe jetzt einfach Ockham's Razor geschwungen und es geht: ISR(ADC_vect) { Serial.println((int)ADCH); } Trotzdem vielen dank für den Exkurs in atomare Interrupt Behandlung =)
Please log in before posting. Registration is free and takes only a minute.
Existing account
Do you have a Google/GoogleMail account? No registration required!
Log in with Google account
Log in with Google account
No account? Register here.