interrupt routine gets called only once

jensvanhoof · December 29, 2011, 1:09pm

Hi guys, I have a really weird problem and I cannot lay my finger on it!

Basically, I want to send serial data on specific timeslots within a second. The seconds are synchronised by a Pulse-per-second signal that comes out of a GPS receiver. What I want to do is toggle a led on and off every time a pulse is received.

I do this using an interrupt on digital pin 3 (interrupt pin 1) on my Duemilanove. It works like a charm when I disable all the code in the loop() procedure, but when I re-enable all the code, it seems to work only once and after that, it just stops toggling on and off.

Although the code in my loop() statement hasn't been debugged and could very well have some logical errors in it, I don't get why the interrupt routine doesn't work. I thought that, wherever the program is, the interrupt routine should be executed when the interrupt is detected. I'm not using any delay() functions.

Any ideas? Thanks a lot!
Jens

void setup() {
  pinMode(3, INPUT);           // set pin to input
  digitalWrite(3, HIGH);       // turn on pullup resistors
  attachInterrupt(1,startGPS,RISING);

void loop() {
  
  a:
  
  if(Puls==1){
    NaPPS();
  }
  
  if(PulsInGeheugen == 1){
    if(millis() - marker > Vertraging[Herhaling]){
      UpdateTally();
      WriteTally();
      Herhaling+=1;
      
      if(Herhaling<AantalHerhalingen){
        goto b;
      } else {
        PulsInGeheugen = 0; }
    }
  
  }
  
  b:
  
    
 //Dit is NA 'PulsInGeheugen = 1', 
  
  if(Serial.available() > 0){ 
    //NMEA Bytes Available  > 0
    RxNumber+=1;
    Answer[RxNumber] = Serial.read();
    goto a;
  } else {
    //NMEA Bytes NOT available
    if(RxNumber > 0){
      //RxNumber >0
      if(Answer[RxNumber]==10){
        //Endbyte Received
        GPGGA_ontvangen = 0;
        for(int i=1;i<7;i++){
           if(Answer[i]==gpgga[i-1]){
               GPGGA_ontvangen++;
           }
        }
        
        if(GPGGA_ontvangen == 6){
          //Valid GPGGA phrase
          PositieKomma = 0;
          for(int i =1;i<RxNumber+1;i++){
            if(Answer[i]==44){
              PositieKomma = PositieKomma + 1;
              kommas[PositieKomma] = i;
            }
          }
          Lock = Answer[(kommas[6])+1];
          
          
          
        } else {
          //No Valid GPGGA phrase
          RxNumber = 0;
        
      }
    } else { 
      //Geen eindbyte
      goto a; }
 
}
goto a;
}
}

void startGPS(){
  Puls = 1;
  led = 1 - led;
  if(led==1){digitalWrite(8,HIGH);} else { digitalWrite(8,LOW);}
 // digitalWrite(8,HIGH);
}

robtillaart · December 29, 2011, 1:28pm

I stopped reading your code when I encountered the first goto in the code. Goto is a statement that can easily corrupt stack and heap administration so it should not be used (or at least with the greatest possible care).

I'll have a look at the code ... to check how to rewrite it properly.

jensvanhoof · December 29, 2011, 1:30pm

Owkay. Actually that makes sense. I'll try to get rid of them firstly.

Thanks!

jensvanhoof · December 29, 2011, 1:33pm

Hmm.. Actually it looks like I can simply delete them as they are pointing to a point that will be executed next anyway.

system · December 29, 2011, 1:48pm

It would be useful to see the whole sketch.
What does 44 mean?

jensvanhoof · December 29, 2011, 1:53pm

//27/12/2011:
//16 ingangen worden gelezen en als seriële code elke seconde verzonden.
//Géén interrupt aanwezig
//Géén GPS timing aanwezig

#include <Wire.h>
#include <math.h>

#define expander1 B0100000 //tally 1 - 8
#define expander2 B0100001 //tally 9 - 16

int DecVal = 0;
int DecVal2 = 0;
int Start1 = 240;
int Start2 = 15;
int Lock = 0;

byte returnByte = 0;
byte returnByte2 = 0;

int AantalHerhalingen = 6;
int TimeSlots[6];
int HowManyTimeSlots = 32;
int Herhaling = 1;
int PulsInGeheugen = 0;
int RxNumber = 0;
int Puls = 0;
int Vertraging[6];
int led = 0;
//int lock = 0;
int GPGGA_ontvangen = 0;
int PositieKomma = 0;
int kommas[15];


char Answer[100];
char gpgga[7] = "$GPGGA";

unsigned long marker = millis();
 
void setup() {
  pinMode(3, INPUT);           // set pin to input
  digitalWrite(3, HIGH);       // turn on pullup resistors
  attachInterrupt(1,startGPS,RISING);
  Wire.begin();
  pinMode(8, OUTPUT);
  Serial.begin(9600);
  //digitalWrite(8,HIGH);
  
  for(int i=1;i<AantalHerhalingen+1;i++){
    Vertraging[i]= TimeSlots[i]*(1000/HowManyTimeSlots)-(1000/HowManyTimeSlots);
  }
}
 
void loop() {
  
  //a:
  
  if(Puls==1){
    NaPPS();
  }
  
  if(PulsInGeheugen == 1){
    if(millis() - marker > Vertraging[Herhaling]){
      UpdateTally();
      WriteTally();
      Herhaling+=1;
      
      if(Herhaling<AantalHerhalingen){
        //goto b;
      } else {
        PulsInGeheugen = 0;
        Herhaling = 1;
       }
    }
  
  }
  
  //b:
  
    
 //Dit is NA 'PulsInGeheugen = 1', 
  
  if(Serial.available() > 0){ 
    //NMEA Bytes Available  > 0
    RxNumber+=1;
    Answer[RxNumber] = Serial.read();
    //goto a;
    
  } else {
    //NMEA Bytes NOT available
    if(RxNumber > 0){
      //RxNumber >0
      if(Answer[RxNumber]==10){
        //Endbyte Received
        GPGGA_ontvangen = 0;
        for(int i=1;i<7;i++){
           if(Answer[i]==gpgga[i-1]){
               GPGGA_ontvangen++;
           }
        }
        
        if(GPGGA_ontvangen == 6){
          //Valid GPGGA phrase
          PositieKomma = 0;
          for(int i =1;i<RxNumber+1;i++){
            if(Answer[i]==44){
              PositieKomma = PositieKomma + 1;
              kommas[PositieKomma] = i;
            }
          }
          Lock = Answer[(kommas[6])+1];
          
          
          
        } else {
          //No Valid GPGGA phrase
          RxNumber = 0;
        
      }
    } else { 
      //Geen eindbyte
     // goto a; 
   }
 
}
//goto a;
}

}
 
void NaPPS(){
  //led = 1 - led;
  //if(led==0){digitalWrite(8,LOW); } else {digitalWrite(8,HIGH);}
  if(Lock>0){ //locked?
    marker = millis();
    PulsInGeheugen = 1;
  }
    
    //b:
    Puls = 0;
  
}
void UpdateTally(){
  returnByte = expanderRead(1);
  returnByte2 = expanderRead(2);
  returnByte = ~returnByte; //NOT poort (knoppen hangen aan massa)
  returnByte2 = ~returnByte2;
  DecVal = 0;
  DecVal2 = 0;
  for(int i=0;i<8;i++){
    
    if(bitRead(returnByte,i)==1){
      DecVal = DecVal + pow(2,i);
      if(i>1){ DecVal+=1; } 
    }
    
    if(bitRead(returnByte2,i)==1){
      DecVal2 = DecVal2 + pow(2,i);
      if(i>1){ DecVal2+=1; } 
    }
  }
}
 
byte expanderRead(int id) {
  byte _data;
  switch(id){
    case 1:
        Wire.requestFrom(expander1, 1);
        if(Wire.available()) {
           _data = Wire.read();
        }
        return _data;
    case 2:
       Wire.requestFrom(expander2, 1);
       if(Wire.available()) {
         _data = Wire.read();
       }
        return _data;
   
  }
}

void WriteTally(){
Serial.write(Start1);
Serial.write(Start2);
Serial.write(DecVal);
Serial.write(DecVal);
Serial.write(DecVal2);
Serial.write(DecVal2);
//delay(1000);
}

void startGPS(){
  Puls = 1;
  led = 1 - led;
  if(led==1){digitalWrite(8,HIGH);} else { digitalWrite(8,LOW);}
 // digitalWrite(8,HIGH);
}

With the phrase containing '44' I'm looking for the position of the "," sign inside the serial data that comes from the GPS. It's transmitted as NMEA data:

$--GGA,hhmmss.ss,llll.ll,a,yyyyy.yy,a,x,xx,x.x,x.x,M,x.x,M,x.x,xxxx

With the komma, I'm seperating the different information in the string. There's a parameter which tells me if there is a GPS lock which I need to determine if the timing is still accurate.

system · December 29, 2011, 1:57pm

If you're looking for a comma, why not say ',' ?
Puls needs to be qualified 'volatile.

jensvanhoof · December 29, 2011, 2:02pm

I didn't realize that I could write it simply as ',' I'm still new to C and Arduino.

I added the 'Volatile' qualifier Thanks!

robtillaart · December 29, 2011, 2:15pm

      if(Herhaling<AantalHerhalingen){
        //goto b;
      } else {
        PulsInGeheugen = 0;
        Herhaling = 1;
       }

to

      if (Herhaling >= AantalHerhalingen) 
      {
        PulsInGeheugen = 0;
        Herhaling = 1;
       }

Note that arrays start with index 0 in C so an array kommas[6] has index 0..5 !!

jensvanhoof · December 29, 2011, 3:23pm

Note that arrays start with index 0 in C so an array kommas[6] has index 0..5 !!

You are the man!!

Nice. It works now! Turns out that I was trying to assign values to non-existing members of an array. I guess that made everyting stop and the interrupt wasn't even called once, which put me on the wrong foot. Man, that took me all day to figure out!

Thanks a lot!

robtillaart · December 29, 2011, 6:55pm

You're welcome,

please spent some time at the reference section of arduino.cc and try to find a copy of Kerningham and Ritchie's C book.

jensvanhoof · December 29, 2011, 11:25pm

http://cg.inf.unideb.hu/eng/rtornai/Kernighan_Ritchie_Language_C.pdf

Thanks a lot!

system · May 5, 2021, 7:40pm